From a7f528cfd0d5ffe9d014900d7c7b83b29583cd56 Mon Sep 17 00:00:00 2001 From: dongshen Date: Wed, 6 Mar 2019 14:38:42 -0800 Subject: [PATCH] HV: remove vdev ops for partition mode Remove vdev ops for partition mode, change related code to directly call the corresponding functions instead Remove struct pci_vdev_ops from vpci.h Add @pre for pci_find_vdev_by_pbdf and pci_find_vdev_by_vbdf/partition_mode_vpci_init Change the return value from int32_t to void to comply with misra c and add ASSERT/panic in the functions (if necessary): vdev_hostbridge_init vdev_hostbridge_deinit vdev_pt_init vdev_pt_deinit Still use pr_err in partition_mode_cfgread and partition_mode_cfgwrite to check if vdev cfgread/cfgwrite access is aligned on 1/2/4 bytes, which is the only case that vdev cfgread/cfgwrite will return nonzero, pr_err will be removed in subsequent patch titled "unify the sharing mode and partition mode coding style for similar functions" Remove @pre for local variables Add ASSERT in partition_mode_pdev_init to check if pdev is NULL (user config error) Tracked-On: #2534 Signed-off-by: dongshen Acked-by: Eddie Dong --- hypervisor/dm/vpci/hostbridge.c | 15 +---- hypervisor/dm/vpci/partition_mode.c | 87 ++++++++++++++++------------- hypervisor/dm/vpci/pci_priv.h | 8 +-- hypervisor/dm/vpci/pci_pt.c | 35 +++++------- hypervisor/dm/vpci/vdev.c | 6 +- hypervisor/include/dm/vpci.h | 21 ------- 6 files changed, 73 insertions(+), 99 deletions(-) diff --git a/hypervisor/dm/vpci/hostbridge.c b/hypervisor/dm/vpci/hostbridge.c index d4bb58cd1..c481ff1e8 100644 --- a/hypervisor/dm/vpci/hostbridge.c +++ b/hypervisor/dm/vpci/hostbridge.c @@ -39,7 +39,7 @@ #include #include "pci_priv.h" -int32_t vdev_hostbridge_init(struct pci_vdev *vdev) +void vdev_hostbridge_init(struct pci_vdev *vdev) { /* PCI config space */ pci_vdev_write_cfg_u16(vdev, PCIR_VENDOR, (uint16_t)0x8086U); @@ -82,13 +82,10 @@ int32_t vdev_hostbridge_init(struct pci_vdev *vdev) pci_vdev_write_cfg_u8(vdev, 0xf5U, (uint8_t)0xfU); pci_vdev_write_cfg_u8(vdev, 0xf6U, (uint8_t)0x1cU); pci_vdev_write_cfg_u8(vdev, 0xf7U, (uint8_t)0x1U); - - return 0; } -int32_t vdev_hostbridge_deinit(__unused const struct pci_vdev *vdev) +void vdev_hostbridge_deinit(__unused const struct pci_vdev *vdev) { - return 0; } int32_t vdev_hostbridge_cfgread(const struct pci_vdev *vdev, uint32_t offset, @@ -119,11 +116,3 @@ int32_t vdev_hostbridge_cfgwrite(struct pci_vdev *vdev, uint32_t offset, return 0; } - -const struct pci_vdev_ops pci_ops_vdev_hostbridge = { - .init = vdev_hostbridge_init, - .deinit = vdev_hostbridge_deinit, - .cfgwrite = vdev_hostbridge_cfgwrite, - .cfgread = vdev_hostbridge_cfgread, -}; - diff --git a/hypervisor/dm/vpci/partition_mode.c b/hypervisor/dm/vpci/partition_mode.c index 7b103a82b..debc5f842 100644 --- a/hypervisor/dm/vpci/partition_mode.c +++ b/hypervisor/dm/vpci/partition_mode.c @@ -33,6 +33,7 @@ #include #include "pci_priv.h" + static inline bool is_hostbridge(const struct pci_vdev *vdev) { return (vdev->vbdf.value == 0U); @@ -54,6 +55,9 @@ static inline bool is_valid_bar(const struct pci_bar *bar) return (is_valid_bar_type(bar) && is_valid_bar_size(bar)); } +/** + * @pre vdev != NULL + */ static void partition_mode_pdev_init(struct pci_vdev *vdev, union pci_bdf pbdf) { struct pci_pdev *pdev; @@ -61,32 +65,31 @@ static void partition_mode_pdev_init(struct pci_vdev *vdev, union pci_bdf pbdf) struct pci_bar *pbar, *vbar; pdev = find_pci_pdev(pbdf); - if (pdev != NULL) { - vdev->pdev = pdev; + ASSERT(pdev != NULL, "pdev is NULL"); - /* Sanity checking for vbar */ - for (idx = 0U; idx < (uint32_t)PCI_BAR_COUNT; idx++) { - pbar = &vdev->pdev->bar[idx]; - vbar = &vdev->bar[idx]; + vdev->pdev = pdev; - if (is_valid_bar(pbar)) { - vbar->size = (pbar->size < 0x1000U) ? 0x1000U : pbar->size; - vbar->type = PCIBAR_MEM32; - } else { - /* Mark this vbar as invalid */ - vbar->size = 0UL; - vbar->type = PCIBAR_NONE; - } + /* Sanity checking for vbar */ + for (idx = 0U; idx < (uint32_t)PCI_BAR_COUNT; idx++) { + pbar = &vdev->pdev->bar[idx]; + vbar = &vdev->bar[idx]; + + if (is_valid_bar(pbar)) { + vbar->size = (pbar->size < 0x1000U) ? 0x1000U : pbar->size; + vbar->type = PCIBAR_MEM32; + } else { + /* Mark this vbar as invalid */ + vbar->size = 0UL; + vbar->type = PCIBAR_NONE; } } + + vdev_pt_init(vdev); } /** * @pre vm != NULL - * @pre vm->vpci->pci_vdev_cnt <= CONFIG_MAX_PCI_DEV_NUM - * @pre vm_config != NULL - * @pre vdev != NULL - * @pre ptdev_config != NULL + * @pre vm->vpci.pci_vdev_cnt <= CONFIG_MAX_PCI_DEV_NUM */ static int32_t partition_mode_vpci_init(const struct acrn_vm *vm) { @@ -105,17 +108,9 @@ static int32_t partition_mode_vpci_init(const struct acrn_vm *vm) vdev->vbdf.value = ptdev_config->vbdf.value; if (is_hostbridge(vdev)) { - vdev->ops = &pci_ops_vdev_hostbridge; + vdev_hostbridge_init(vdev); } else { partition_mode_pdev_init(vdev, ptdev_config->pbdf); - vdev->ops = &pci_ops_vdev_pt; - } - - if (vdev->ops->init != NULL) { - if (vdev->ops->init(vdev) != 0) { - pr_err("%s() failed at PCI device (vbdf %x)!", __func__, - vdev->vbdf); - } } } @@ -123,7 +118,8 @@ static int32_t partition_mode_vpci_init(const struct acrn_vm *vm) } /** - * @pre vdev != NULL + * @pre vm != NULL + * @pre vm->vpci.pci_vdev_cnt <= CONFIG_MAX_PCI_DEV_NUM */ static void partition_mode_vpci_deinit(const struct acrn_vm *vm) { @@ -132,10 +128,11 @@ static void partition_mode_vpci_deinit(const struct acrn_vm *vm) for (i = 0U; i < vm->vpci.pci_vdev_cnt; i++) { vdev = (struct pci_vdev *) &(vm->vpci.pci_vdevs[i]); - if ((vdev->ops != NULL) && (vdev->ops->deinit != NULL)) { - if (vdev->ops->deinit(vdev) != 0) { - pr_err("vdev->ops->deinit failed!"); - } + + if (is_hostbridge(vdev)) { + vdev_hostbridge_deinit(vdev); + } else { + vdev_pt_deinit(vdev); } } } @@ -145,9 +142,16 @@ static void partition_mode_cfgread(struct acrn_vpci *vpci, union pci_bdf vbdf, { struct pci_vdev *vdev = pci_find_vdev_by_vbdf(vpci, vbdf); - if ((vdev != NULL) && (vdev->ops != NULL) - && (vdev->ops->cfgread != NULL)) { - (void)vdev->ops->cfgread(vdev, offset, bytes, val); + if (vdev != NULL) { + if (is_hostbridge(vdev)) { + if (vdev_hostbridge_cfgread(vdev, offset, bytes, val) != 0) { + pr_err("vdev_hostbridge_cfgread failed!"); + } + } else { + if (vdev_pt_cfgread(vdev, offset, bytes, val) != 0) { + pr_err("vdev_pt_cfgread failed!"); + } + } } } @@ -156,9 +160,16 @@ static void partition_mode_cfgwrite(struct acrn_vpci *vpci, union pci_bdf vbdf, { struct pci_vdev *vdev = pci_find_vdev_by_vbdf(vpci, vbdf); - if ((vdev != NULL) && (vdev->ops != NULL) - && (vdev->ops->cfgwrite != NULL)) { - (void)vdev->ops->cfgwrite(vdev, offset, bytes, val); + if (vdev != NULL) { + if (is_hostbridge(vdev)) { + if (vdev_hostbridge_cfgwrite(vdev, offset, bytes, val) != 0) { + pr_err("vdev_hostbridge_cfgwrite failed!"); + } + } else { + if (vdev_pt_cfgwrite(vdev, offset, bytes, val) != 0){ + pr_err("vdev_pt_cfgwrite failed!"); + } + } } } diff --git a/hypervisor/dm/vpci/pci_priv.h b/hypervisor/dm/vpci/pci_priv.h index 0bcd289c9..d06a207db 100644 --- a/hypervisor/dm/vpci/pci_priv.h +++ b/hypervisor/dm/vpci/pci_priv.h @@ -69,15 +69,15 @@ static inline void pci_vdev_write_cfg_u32(struct pci_vdev *vdev, uint32_t offset #ifdef CONFIG_PARTITION_MODE extern const struct vpci_ops partition_mode_vpci_ops; -int32_t vdev_hostbridge_init(struct pci_vdev *vdev); +void vdev_hostbridge_init(struct pci_vdev *vdev); int32_t vdev_hostbridge_cfgread(const struct pci_vdev *vdev, uint32_t offset, uint32_t bytes, uint32_t *val); int32_t vdev_hostbridge_cfgwrite(struct pci_vdev *vdev, uint32_t offset, uint32_t bytes, uint32_t val); -int32_t vdev_hostbridge_deinit(__unused const struct pci_vdev *vdev); +void vdev_hostbridge_deinit(__unused const struct pci_vdev *vdev); -int32_t vdev_pt_init(struct pci_vdev *vdev); +void vdev_pt_init(struct pci_vdev *vdev); int32_t vdev_pt_cfgread(const struct pci_vdev *vdev, uint32_t offset, uint32_t bytes, uint32_t *val); int32_t vdev_pt_cfgwrite(struct pci_vdev *vdev, uint32_t offset, uint32_t bytes, uint32_t val); -int32_t vdev_pt_deinit(const struct pci_vdev *vdev); +void vdev_pt_deinit(const struct pci_vdev *vdev); #else extern const struct vpci_ops sharing_mode_vpci_ops; diff --git a/hypervisor/dm/vpci/pci_pt.c b/hypervisor/dm/vpci/pci_pt.c index fdbdb3a94..a554b0618 100644 --- a/hypervisor/dm/vpci/pci_pt.c +++ b/hypervisor/dm/vpci/pci_pt.c @@ -42,33 +42,31 @@ static inline uint32_t pci_bar_base(uint32_t bar) return bar & PCIM_BAR_MEM_BASE; } -static int32_t vdev_pt_init_validate(struct pci_vdev *vdev) +static int32_t validate(const struct pci_vdev *vdev) { uint32_t idx; + int32_t ret = 0; for (idx = 0U; idx < PCI_BAR_COUNT; idx++) { if ((vdev->bar[idx].base != 0x0UL) || ((vdev->bar[idx].size & 0xFFFUL) != 0x0UL) || ((vdev->bar[idx].type != PCIBAR_MEM32) && (vdev->bar[idx].type != PCIBAR_NONE))) { - return -EINVAL; + ret = -EINVAL; + break; } } - return 0; + return ret; } -int32_t vdev_pt_init(struct pci_vdev *vdev) +void vdev_pt_init(struct pci_vdev *vdev) { int32_t ret; struct acrn_vm *vm = vdev->vpci->vm; uint16_t pci_command; - ret = vdev_pt_init_validate(vdev); - if (ret != 0) { - pr_err("Error, invalid bar defined"); - return ret; - } + ASSERT(validate(vdev) == 0, "Error, invalid bar defined"); /* Create an iommu domain for target VM if not created */ if (vm->iommu == NULL) { @@ -82,24 +80,26 @@ int32_t vdev_pt_init(struct pci_vdev *vdev) ret = assign_iommu_device(vm->iommu, (uint8_t)vdev->pdev->bdf.bits.b, (uint8_t)(vdev->pdev->bdf.value & 0xFFU)); + if (ret != 0) { + panic("failed to assign iommu device!"); + } pci_command = (uint16_t)pci_pdev_read_cfg(vdev->pdev->bdf, PCIR_COMMAND, 2U); /* Disable INTX */ pci_command |= 0x400U; pci_pdev_write_cfg(vdev->pdev->bdf, PCIR_COMMAND, 2U, pci_command); - - return ret; } -int32_t vdev_pt_deinit(const struct pci_vdev *vdev) +void vdev_pt_deinit(const struct pci_vdev *vdev) { int32_t ret; struct acrn_vm *vm = vdev->vpci->vm; ret = unassign_iommu_device(vm->iommu, (uint8_t)vdev->pdev->bdf.bits.b, (uint8_t)(vdev->pdev->bdf.value & 0xFFU)); - - return ret; + if (ret != 0) { + panic("failed to unassign iommu device!"); + } } int32_t vdev_pt_cfgread(const struct pci_vdev *vdev, uint32_t offset, @@ -200,10 +200,3 @@ int32_t vdev_pt_cfgwrite(struct pci_vdev *vdev, uint32_t offset, return 0; } -const struct pci_vdev_ops pci_ops_vdev_pt = { - .init = vdev_pt_init, - .deinit = vdev_pt_deinit, - .cfgwrite = vdev_pt_cfgwrite, - .cfgread = vdev_pt_cfgread, -}; - diff --git a/hypervisor/dm/vpci/vdev.c b/hypervisor/dm/vpci/vdev.c index 2f361953f..c2f265121 100644 --- a/hypervisor/dm/vpci/vdev.c +++ b/hypervisor/dm/vpci/vdev.c @@ -65,7 +65,8 @@ void pci_vdev_write_cfg(struct pci_vdev *vdev, uint32_t offset, uint32_t bytes, } /** - * @pre tmp != NULL + * @pre vpci != NULL + * @pre vpci->pci_vdev_cnt <= CONFIG_MAX_PCI_DEV_NUM */ struct pci_vdev *pci_find_vdev_by_vbdf(const struct acrn_vpci *vpci, union pci_bdf vbdf) { @@ -86,7 +87,8 @@ struct pci_vdev *pci_find_vdev_by_vbdf(const struct acrn_vpci *vpci, union pci_b } /** - * @pre tmp != NULL + * @pre vpci != NULL + * @pre vpci->pci_vdev_cnt <= CONFIG_MAX_PCI_DEV_NUM */ struct pci_vdev *pci_find_vdev_by_pbdf(const struct acrn_vpci *vpci, union pci_bdf pbdf) { diff --git a/hypervisor/include/dm/vpci.h b/hypervisor/include/dm/vpci.h index be4288824..20f3d7001 100644 --- a/hypervisor/include/dm/vpci.h +++ b/hypervisor/include/dm/vpci.h @@ -32,18 +32,6 @@ #include -struct pci_vdev; -struct pci_vdev_ops { - int32_t (*init)(struct pci_vdev *vdev); - - int32_t (*deinit)(const struct pci_vdev *vdev); - - int32_t (*cfgwrite)(struct pci_vdev *vdev, uint32_t offset, - uint32_t bytes, uint32_t val); - - int32_t (*cfgread)(const struct pci_vdev *vdev, uint32_t offset, - uint32_t bytes, uint32_t *val); -}; struct msix_table_entry { uint64_t addr; @@ -77,10 +65,6 @@ union pci_cfgdata { }; struct pci_vdev { -#ifdef CONFIG_PARTITION_MODE - const struct pci_vdev_ops *ops; -#endif - const struct acrn_vpci *vpci; /* The bus/device/function triple of the virtual PCI device. */ union pci_bdf vbdf; @@ -121,11 +105,6 @@ struct acrn_vpci { struct pci_vdev pci_vdevs[CONFIG_MAX_PCI_DEV_NUM]; }; -#ifdef CONFIG_PARTITION_MODE -extern const struct pci_vdev_ops pci_ops_vdev_hostbridge; -extern const struct pci_vdev_ops pci_ops_vdev_pt; -#endif - void vpci_init(struct acrn_vm *vm); void vpci_cleanup(const struct acrn_vm *vm); void vpci_set_ptdev_intr_info(const struct acrn_vm *target_vm, uint16_t vbdf, uint16_t pbdf);