[PATCH v2 03/15] bus/pci: rework MSIX discovery with VFIO

[Xia, Chenbo]

> -----Original Message-----
> From: David Marchand <david.marchand at redhat.com>
> Sent: Monday, August 21, 2023 7:36 PM
> To: dev at dpdk.org
> Cc: thomas at monjalon.net; ferruh.yigit at amd.com; Xia, Chenbo
> <chenbo.xia at intel.com>; nipun.gupta at amd.com; Richardson, Bruce
> <bruce.richardson at intel.com>; Burakov, Anatoly <anatoly.burakov at intel.com>
> Subject: [PATCH v2 03/15] bus/pci: rework MSIX discovery with VFIO
> 
> This is a preparatory step before using new helpers for finding PCI
> capabilities.
> In the code querying PCI capabilities for checking MSIX availability,
> replace direct calls to VFIO fd with the existing helpers for reading
> PCI configuration space: this requires setting VFIO fd in the PCI
> device object than was done before this change and removes the need to
> pass around this vfio_dev_fd variable.
> 
> Signed-off-by: David Marchand <david.marchand at redhat.com>
> ---
>  drivers/bus/pci/linux/pci_vfio.c | 27 ++++++++++-----------------
>  1 file changed, 10 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/bus/pci/linux/pci_vfio.c
> b/drivers/bus/pci/linux/pci_vfio.c
> index 8fa7fa458f..958f8b3b52 100644
> --- a/drivers/bus/pci/linux/pci_vfio.c
> +++ b/drivers/bus/pci/linux/pci_vfio.c
> @@ -107,23 +107,16 @@ pci_vfio_write_config(const struct rte_pci_device
> *dev,
> 
>  /* get PCI BAR number where MSI-X interrupts are */
>  static int
> -pci_vfio_get_msix_bar(const struct rte_pci_device *dev, int fd,
> +pci_vfio_get_msix_bar(const struct rte_pci_device *dev,
>  	struct pci_msix_table *msix_table)
>  {
>  	int ret;
>  	uint32_t reg;
>  	uint16_t flags;
>  	uint8_t cap_id, cap_offset;
> -	uint64_t size, offset;
> -
> -	if (pci_vfio_get_region(dev, VFIO_PCI_CONFIG_REGION_INDEX,
> -		&size, &offset) != 0) {
> -		RTE_LOG(ERR, EAL, "Cannot get offset of CONFIG region.\n");
> -		return -1;
> -	}
> 
>  	/* read PCI capability pointer from config space */
> -	ret = pread64(fd, &reg, sizeof(reg), offset + PCI_CAPABILITY_LIST);
> +	ret = rte_pci_read_config(dev, &reg, sizeof(reg),
> PCI_CAPABILITY_LIST);
>  	if (ret != sizeof(reg)) {
>  		RTE_LOG(ERR, EAL,
>  			"Cannot read capability pointer from PCI config
> space!\n");
> @@ -136,7 +129,7 @@ pci_vfio_get_msix_bar(const struct rte_pci_device *dev,
> int fd,
>  	while (cap_offset) {
> 
>  		/* read PCI capability ID */
> -		ret = pread64(fd, &reg, sizeof(reg), offset + cap_offset);
> +		ret = rte_pci_read_config(dev, &reg, sizeof(reg), cap_offset);
>  		if (ret != sizeof(reg)) {
>  			RTE_LOG(ERR, EAL,
>  				"Cannot read capability ID from PCI config
> space!\n");
> @@ -148,7 +141,7 @@ pci_vfio_get_msix_bar(const struct rte_pci_device *dev,
> int fd,
> 
>  		/* if we haven't reached MSI-X, check next capability */
>  		if (cap_id != PCI_CAP_ID_MSIX) {
> -			ret = pread64(fd, &reg, sizeof(reg), offset +
> cap_offset);
> +			ret = rte_pci_read_config(dev, &reg, sizeof(reg),
> cap_offset);
>  			if (ret != sizeof(reg)) {
>  				RTE_LOG(ERR, EAL,
>  					"Cannot read capability pointer from PCI
> config space!\n");
> @@ -163,14 +156,14 @@ pci_vfio_get_msix_bar(const struct rte_pci_device
> *dev, int fd,
>  		/* else, read table offset */
>  		else {
>  			/* table offset resides in the next 4 bytes */
> -			ret = pread64(fd, &reg, sizeof(reg), offset + cap_offset
> + 4);
> +			ret = rte_pci_read_config(dev, &reg, sizeof(reg),
> cap_offset + 4);
>  			if (ret != sizeof(reg)) {
>  				RTE_LOG(ERR, EAL,
>  					"Cannot read table offset from PCI config
> space!\n");
>  				return -1;
>  			}
> 
> -			ret = pread64(fd, &flags, sizeof(flags), offset +
> cap_offset + 2);
> +			ret = rte_pci_read_config(dev, &flags, sizeof(flags),
> cap_offset + 2);
>  			if (ret != sizeof(flags)) {
>  				RTE_LOG(ERR, EAL,
>  					"Cannot read table flags from PCI config
> space!\n");
> @@ -306,9 +299,6 @@ pci_vfio_setup_interrupts(struct rte_pci_device *dev,
> int vfio_dev_fd)
>  		if (rte_intr_fd_set(dev->intr_handle, fd))
>  			return -1;
> 
> -		if (rte_intr_dev_fd_set(dev->intr_handle, vfio_dev_fd))
> -			return -1;
> -
>  		switch (i) {
>  		case VFIO_PCI_MSIX_IRQ_INDEX:
>  			intr_mode = RTE_INTR_MODE_MSIX;
> @@ -838,6 +828,9 @@ pci_vfio_map_resource_primary(struct rte_pci_device
> *dev)
>  	if (ret)
>  		return ret;
> 
> +	if (rte_intr_dev_fd_set(dev->intr_handle, vfio_dev_fd))
> +		goto err_vfio_dev_fd;
> +
>  	/* allocate vfio_res and get region info */
>  	vfio_res = rte_zmalloc("VFIO_RES", sizeof(*vfio_res), 0);
>  	if (vfio_res == NULL) {
> @@ -869,7 +862,7 @@ pci_vfio_map_resource_primary(struct rte_pci_device
> *dev)
>  	/* get MSI-X BAR, if any (we have to know where it is because we
> can't
>  	 * easily mmap it when using VFIO)
>  	 */
> -	ret = pci_vfio_get_msix_bar(dev, vfio_dev_fd, &vfio_res->msix_table);
> +	ret = pci_vfio_get_msix_bar(dev, &vfio_res->msix_table);
>  	if (ret < 0) {
>  		RTE_LOG(ERR, EAL, "%s cannot get MSI-X BAR number!\n",
>  				pci_addr);
> --
> 2.41.0

Reviewed-by: Chenbo Xia <chenbo.xia at intel.com>