[dpdk-dev,v4,09/14] virtio: ethdev: check for vfio interface

Message ID 1452778117-30178-10-git-send-email-sshukla@mvista.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Santosh Shukla Jan. 14, 2016, 1:28 p.m. UTC
  Introducing api to check interface type is vfio or not, if interface is vfio
then update struct virtio_vfio_dev {}.

Those two apis are:
- virtio_chk_for_vfio
- virtio_hw_init_by_vfio

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
v3->v4:
- Removed RTE_PCI_DRV_NEED_MAPPING drv flag (as per Review comment from Stephen
  and Suggested by Yuan)
- Introducing vfio interface parsing api which will set/unset is_vfio flag at
  runtime.

drivers/net/virtio/virtio_ethdev.c |  112 +++++++++++++++++++++++++++++++++++-
 1 file changed, 110 insertions(+), 2 deletions(-)
  

Comments

Yuanhan Liu Jan. 15, 2016, 6:35 a.m. UTC | #1
On Thu, Jan 14, 2016 at 06:58:32PM +0530, Santosh Shukla wrote:
> Introducing api to check interface type is vfio or not, if interface is vfio
> then update struct virtio_vfio_dev {}.
> 
> Those two apis are:
> - virtio_chk_for_vfio
> - virtio_hw_init_by_vfio
> 
> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
..
> +/* Init virtio by vfio-way */
> +static int virtio_hw_init_by_vfio(struct virtio_hw *hw,
> +				  struct rte_pci_device *pci_dev)
> +{
> +	struct virtio_vfio_dev *vdev;
> +
> +	vdev = &hw->dev;
> +	if (virtio_chk_for_vfio(pci_dev) < 0) {
> +		vdev->is_vfio = false;
> +		vdev->pci_dev = NULL;
> +		return -1;
> +	}
> +
> +	/* .. So attached interface is vfio */
> +	vdev->is_vfio = true;
> +	vdev->pci_dev = pci_dev;

Normally, I don't like the way of adding yet another "virtio_hw_init_by_xxx".

As suggested in another reply, would pci_dev->kdrv checking be enough?
If so, do it in simple way.

	--ylu
  
Santosh Shukla Jan. 15, 2016, 12:37 p.m. UTC | #2
On Fri, Jan 15, 2016 at 12:05 PM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> On Thu, Jan 14, 2016 at 06:58:32PM +0530, Santosh Shukla wrote:
>> Introducing api to check interface type is vfio or not, if interface is vfio
>> then update struct virtio_vfio_dev {}.
>>
>> Those two apis are:
>> - virtio_chk_for_vfio
>> - virtio_hw_init_by_vfio
>>
>> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> ..
>> +/* Init virtio by vfio-way */
>> +static int virtio_hw_init_by_vfio(struct virtio_hw *hw,
>> +                               struct rte_pci_device *pci_dev)
>> +{
>> +     struct virtio_vfio_dev *vdev;
>> +
>> +     vdev = &hw->dev;
>> +     if (virtio_chk_for_vfio(pci_dev) < 0) {
>> +             vdev->is_vfio = false;
>> +             vdev->pci_dev = NULL;
>> +             return -1;
>> +     }
>> +
>> +     /* .. So attached interface is vfio */
>> +     vdev->is_vfio = true;
>> +     vdev->pci_dev = pci_dev;
>
> Normally, I don't like the way of adding yet another "virtio_hw_init_by_xxx".
>
> As suggested in another reply, would pci_dev->kdrv checking be enough?
> If so, do it in simple way.
>

No, It wont be enough, Virtio could only work for vfio for _noiommu_
mode and for that user need to preset -noiommu parameter, therefore
virtio need to check that parame. pci_dev->kdrv check not enough,
Hence not used.

>         --ylu
  

Patch

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d928339..8f2260f 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1202,6 +1202,105 @@  static int virtio_resource_init(struct rte_pci_device *pci_dev)
 		return virtio_resource_init_by_ioports(pci_dev);
 }
 
+static int virtio_chk_for_vfio(struct rte_pci_device *pci_dev)
+{
+	/*
+	 * 1. check whether vfio-noiommu mode is enabled
+	 * 2. verify pci device attached to vfio-noiommu driver
+	 * root@arm64:/sys/bus/pci/drivers/vfio-pci/0000:00:01.0/iommu_group#
+	 * > cat name
+	 * > vfio-noiommu
+	 */
+
+	/* 1. Chk for vfio: noiommu mode set or not in kernel driver */
+	struct rte_pci_addr *loc;
+	FILE *fp;
+	const char *path = "/sys/module/vfio/parameters/enable_unsafe_noiommu_mode";
+	char filename[PATH_MAX] = {0};
+	char buf[PATH_MAX] = {0};
+
+	fp = fopen(path, "r");
+	if (fp == NULL) {
+		PMD_INIT_LOG(ERR, "can't open %s\n", path);
+		return -1;
+	}
+
+	if (fread(buf, sizeof(char), 1, fp) != 1) {
+		PMD_INIT_LOG(ERR, "can't read from file %s\n", path);
+		fclose(fp);
+		return -1;
+	}
+
+	if (strncmp(buf, "Y", 1) != 0) {
+		PMD_INIT_LOG(ERR, "[%s]: vfio: noiommu mode not set\n", path);
+		fclose(fp);
+		return -1;
+	}
+
+	fclose(fp);
+
+	/* 2. Verify pci device attached to vfio-noiommu driver */
+
+	/* 2.1 chk whether attached driver is vfio-noiommu or not */
+	loc = &pci_dev->addr;
+	snprintf(filename, sizeof(filename),
+		     SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/iommu_group/name",
+		     loc->domain, loc->bus, loc->devid, loc->function);
+
+	/* check for vfio-noiommu */
+	fp = fopen(filename, "r");
+	if (fp == NULL) {
+		PMD_INIT_LOG(ERR, "can't open %s\n", filename);
+		return -1;
+	}
+
+	if (fread(buf, sizeof(char), sizeof("vfio-noiommu"), fp) !=
+		  sizeof("vfio-noiommu")) {
+		PMD_INIT_LOG(ERR, "can't read from file %s\n", filename);
+		fclose(fp);
+		return -1;
+	}
+
+	if (strncmp(buf, "vfio-noiommu", strlen("vfio-noiommu")) != 0) {
+		PMD_INIT_LOG(ERR, "not a vfio-noiommu driver\n");
+		fclose(fp);
+		return -1;
+	}
+
+	fclose(fp);
+
+	/* todo: vfio interrupt handling */
+	return 0;
+}
+
+/* Init virtio by vfio-way */
+static int virtio_hw_init_by_vfio(struct virtio_hw *hw,
+				  struct rte_pci_device *pci_dev)
+{
+	struct virtio_vfio_dev *vdev;
+
+	vdev = &hw->dev;
+	if (virtio_chk_for_vfio(pci_dev) < 0) {
+		vdev->is_vfio = false;
+		vdev->pci_dev = NULL;
+		return -1;
+	}
+
+	/* .. So attached interface is vfio */
+	vdev->is_vfio = true;
+	vdev->pci_dev = pci_dev;
+
+	/* For debug use only */
+	const struct rte_intr_handle *intr_handle;
+	RTE_SET_USED(intr_handle); /* to keep compilar happy */
+	intr_handle = &pci_dev->intr_handle;
+	PMD_INIT_LOG(DEBUG, "vdev->pci_dev %p intr_handle %p vfio_dev_fd %d\n",
+			     vdev->pci_dev, intr_handle,
+			     intr_handle->vfio_dev_fd);
+
+	return 0;
+}
+
 #else
 static int
 virtio_has_msix(const struct rte_pci_addr *loc __rte_unused)
@@ -1215,6 +1314,13 @@  static int virtio_resource_init(struct rte_pci_device *pci_dev __rte_unused)
 	/* no setup required */
 	return 0;
 }
+
+static int virtio_hw_init_by_vfio(struct virtio_hw *hw __rte_unused,
+				  struct rte_pci_device *pci_dev __rte_unused)
+{
+	/* NA */
+	return 0;
+}
 #endif
 
 /*
@@ -1287,8 +1393,10 @@  eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 
 	pci_dev = eth_dev->pci_dev;
 
-	if (virtio_resource_init(pci_dev) < 0)
-		return -1;
+	if (virtio_hw_init_by_vfio(hw, pci_dev) < 0) {
+		if (virtio_resource_init(pci_dev) < 0)
+			return -1;
+	}
 
 	hw->use_msix = virtio_has_msix(&pci_dev->addr);
 	hw->io_base = (uint32_t)(uintptr_t)pci_dev->mem_resource[0].addr;