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 <dongsheng.x.zhang@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>

Author: donsheng

hypervisor/dm/vpci/hostbridge.c

@@ -39,7 +39,7 @@
 #include <pci.h>
 #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,
-};
-

hypervisor/dm/vpci/partition_mode.c

@@ -33,6 +33,7 @@
 #include <logmsg.h>
 #include "pci_priv.h"
 
+
 static inline bool is_hostbridge(const struct pci_vdev *vdev)
 {
 	return (vdev->vbdf.value == 0U);
@@ -54,39 +55,41 @@ 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;
 	uint32_t idx;
 	struct pci_bar *pbar, *vbar;
 
 	pdev = find_pci_pdev(pbdf);
-	if (pdev != NULL) {
-		vdev->pdev = pdev;
-
-		/* 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;
-			}
+	ASSERT(pdev != NULL, "pdev is NULL");
+
+	vdev->pdev = pdev;
+
+	/* 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,25 +108,18 @@ 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);
-			}
 		}
 	}
 
 	return 0;
 }
 
 /**
- * @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!");
+			}
+		}
 	}
 }
 

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;

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,
-};
-

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)
 {

hypervisor/include/dm/vpci.h

@@ -32,18 +32,6 @@
 
 #include <pci.h>
 
-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);