From 026250fd8ae1cf1d5d31ea9ec7cc93b63b0f8428 Mon Sep 17 00:00:00 2001 From: dongshen Date: Wed, 6 Mar 2019 16:24:52 -0800 Subject: [PATCH] HV: centralize the pci cfg read/write sanity checking code Do the pci cfg read/write sanity checking before the request is dispatched to submodules, so that the checking is centralized rather than scattered across multiple files/places Tracked-On: #2534 Signed-off-by: dongshen Acked-by: Eddie Dong --- hypervisor/dm/vpci/hostbridge.c | 11 ----------- hypervisor/dm/vpci/msi.c | 1 + hypervisor/dm/vpci/pci_pt.c | 11 ----------- hypervisor/dm/vpci/sharing_mode.c | 18 ++++++++---------- hypervisor/dm/vpci/vpci.c | 27 +++++++++++++++++++++++---- 5 files changed, 32 insertions(+), 36 deletions(-) diff --git a/hypervisor/dm/vpci/hostbridge.c b/hypervisor/dm/vpci/hostbridge.c index c481ff1e8..df7d3cb83 100644 --- a/hypervisor/dm/vpci/hostbridge.c +++ b/hypervisor/dm/vpci/hostbridge.c @@ -91,12 +91,6 @@ void vdev_hostbridge_deinit(__unused const struct pci_vdev *vdev) int32_t vdev_hostbridge_cfgread(const struct pci_vdev *vdev, uint32_t offset, uint32_t bytes, uint32_t *val) { - /* Assumption: access needed to be aligned on 1/2/4 bytes */ - if ((offset & (bytes - 1U)) != 0U) { - *val = 0xFFFFFFFFU; - return -EINVAL; - } - *val = pci_vdev_read_cfg(vdev, offset, bytes); return 0; @@ -105,11 +99,6 @@ int32_t vdev_hostbridge_cfgread(const struct pci_vdev *vdev, uint32_t offset, int32_t vdev_hostbridge_cfgwrite(struct pci_vdev *vdev, uint32_t offset, uint32_t bytes, uint32_t val) { - /* Assumption: access needed to be aligned on 1/2/4 bytes */ - if ((offset & (bytes - 1U)) != 0U) { - return -EINVAL; - } - if (!pci_bar_access(offset)) { pci_vdev_write_cfg(vdev, offset, bytes, val); } diff --git a/hypervisor/dm/vpci/msi.c b/hypervisor/dm/vpci/msi.c index e8b9e9900..f3adea379 100644 --- a/hypervisor/dm/vpci/msi.c +++ b/hypervisor/dm/vpci/msi.c @@ -118,6 +118,7 @@ static int32_t vmsi_remap(const struct pci_vdev *vdev, bool enable) int32_t vmsi_cfgread(const struct pci_vdev *vdev, uint32_t offset, uint32_t bytes, uint32_t *val) { int32_t ret; + /* For PIO access, we emulate Capability Structures only */ if (msicap_access(vdev, offset)) { *val = pci_vdev_read_cfg(vdev, offset, bytes); diff --git a/hypervisor/dm/vpci/pci_pt.c b/hypervisor/dm/vpci/pci_pt.c index a554b0618..df8d6e61c 100644 --- a/hypervisor/dm/vpci/pci_pt.c +++ b/hypervisor/dm/vpci/pci_pt.c @@ -105,12 +105,6 @@ void vdev_pt_deinit(const struct pci_vdev *vdev) int32_t vdev_pt_cfgread(const struct pci_vdev *vdev, uint32_t offset, uint32_t bytes, uint32_t *val) { - /* Assumption: access needed to be aligned on 1/2/4 bytes */ - if ((offset & (bytes - 1U)) != 0U) { - *val = 0xFFFFFFFFU; - return -EINVAL; - } - /* PCI BARs is emulated */ if (pci_bar_access(offset)) { *val = pci_vdev_read_cfg(vdev, offset, bytes); @@ -184,11 +178,6 @@ static void vdev_pt_cfgwrite_bar(struct pci_vdev *vdev, uint32_t offset, int32_t vdev_pt_cfgwrite(struct pci_vdev *vdev, uint32_t offset, uint32_t bytes, uint32_t val) { - /* Assumption: access needed to be aligned on 1/2/4 bytes */ - if ((offset & (bytes - 1U)) != 0U) { - return -EINVAL; - } - /* PCI BARs are emulated */ if (pci_bar_access(offset)) { vdev_pt_cfgwrite_bar(vdev, offset, bytes, val); diff --git a/hypervisor/dm/vpci/sharing_mode.c b/hypervisor/dm/vpci/sharing_mode.c index 46d4f27a3..446e5fddd 100644 --- a/hypervisor/dm/vpci/sharing_mode.c +++ b/hypervisor/dm/vpci/sharing_mode.c @@ -50,7 +50,7 @@ void sharing_mode_cfgread(__unused struct acrn_vpci *vpci, union pci_bdf bdf, vdev = sharing_mode_find_vdev_sos(bdf); /* vdev == NULL: Could be hit for PCI enumeration from guests */ - if ((vdev == NULL) || ((bytes != 1U) && (bytes != 2U) && (bytes != 4U))) { + if (vdev == NULL) { *val = ~0U; } else { if ((vmsi_cfgread(vdev, offset, bytes, val) != 0) @@ -67,15 +67,13 @@ void sharing_mode_cfgwrite(__unused struct acrn_vpci *vpci, union pci_bdf bdf, { struct pci_vdev *vdev; - if ((bytes == 1U) || (bytes == 2U) || (bytes == 4U)) { - vdev = sharing_mode_find_vdev_sos(bdf); - if (vdev != NULL) { - if ((vmsi_cfgwrite(vdev, offset, bytes, val) != 0) - && (vmsix_cfgwrite(vdev, offset, bytes, val) != 0) - ) { - /* Not handled by any handlers, passthru to physical device */ - pci_pdev_write_cfg(vdev->pdev->bdf, offset, bytes, val); - } + vdev = sharing_mode_find_vdev_sos(bdf); + if (vdev != NULL) { + if ((vmsi_cfgwrite(vdev, offset, bytes, val) != 0) + && (vmsix_cfgwrite(vdev, offset, bytes, val) != 0) + ) { + /* Not handled by any handlers, passthru to physical device */ + pci_pdev_write_cfg(vdev->pdev->bdf, offset, bytes, val); } } } diff --git a/hypervisor/dm/vpci/vpci.c b/hypervisor/dm/vpci/vpci.c index bb0024cb6..cea7dc917 100644 --- a/hypervisor/dm/vpci/vpci.c +++ b/hypervisor/dm/vpci/vpci.c @@ -67,6 +67,21 @@ static void pci_cfgaddr_io_write(struct acrn_vm *vm, uint16_t addr, size_t bytes } } +static inline bool vpci_is_valid_access_offset(uint32_t offset, uint32_t bytes) +{ + return ((offset & (bytes - 1U)) == 0U); +} + +static inline bool vpci_is_valid_access_byte(uint32_t bytes) +{ + return ((bytes == 1U) || (bytes == 2U) || (bytes == 4U)); +} + +static inline bool vpci_is_valid_access(uint32_t offset, uint32_t bytes) +{ + return (vpci_is_valid_access_byte(bytes) && vpci_is_valid_access_offset(offset, bytes)); +} + static uint32_t pci_cfgdata_io_read(struct acrn_vm *vm, uint16_t addr, size_t bytes) { struct acrn_vpci *vpci = &vm->vpci; @@ -75,11 +90,13 @@ static uint32_t pci_cfgdata_io_read(struct acrn_vm *vm, uint16_t addr, size_t by uint32_t val = ~0U; if (pi->cached_enable) { + if (vpci_is_valid_access(pi->cached_reg + offset, bytes)) { #ifdef CONFIG_PARTITION_MODE - partition_mode_cfgread(vpci, pi->cached_bdf, pi->cached_reg + offset, bytes, &val); + partition_mode_cfgread(vpci, pi->cached_bdf, pi->cached_reg + offset, bytes, &val); #else - sharing_mode_cfgread(vpci, pi->cached_bdf, pi->cached_reg + offset, bytes, &val); + sharing_mode_cfgread(vpci, pi->cached_bdf, pi->cached_reg + offset, bytes, &val); #endif + } pci_cfg_clear_cache(pi); } @@ -93,11 +110,13 @@ static void pci_cfgdata_io_write(struct acrn_vm *vm, uint16_t addr, size_t bytes uint16_t offset = addr - PCI_CONFIG_DATA; if (pi->cached_enable) { + if (vpci_is_valid_access(pi->cached_reg + offset, bytes)) { #ifdef CONFIG_PARTITION_MODE - partition_mode_cfgwrite(vpci, pi->cached_bdf, pi->cached_reg + offset, bytes, val); + partition_mode_cfgwrite(vpci, pi->cached_bdf, pi->cached_reg + offset, bytes, val); #else - sharing_mode_cfgwrite(vpci, pi->cached_bdf, pi->cached_reg + offset, bytes, val); + sharing_mode_cfgwrite(vpci, pi->cached_bdf, pi->cached_reg + offset, bytes, val); #endif + } pci_cfg_clear_cache(pi); } }