Re: [PATCH v4 08/14] virtio: pci: extend virtio pci rw api for vfio interface

On Mon, Jan 18, 2016 at 11:41 AM, Yuanhan Liu
[off-list ref] wrote:

On Fri, Jan 15, 2016 at 07:12:04PM +0530, Santosh Shukla wrote:

quoted

On Fri, Jan 15, 2016 at 6:13 PM, Santosh Shukla [off-list ref] wrote:

quoted

On Fri, Jan 15, 2016 at 11:57 AM, Yuanhan Liu
[off-list ref] wrote:

quoted

On Thu, Jan 14, 2016 at 06:58:31PM +0530, Santosh Shukla wrote:

quoted

So far virtio handle rw access for uio / ioport interface, This patch to extend
the support for vfio interface. For that introducing private struct
virtio_vfio_dev{
      - is_vfio
      - pci_dev
      };
Signed-off-by: Santosh Shukla <redacted>

...

quoted

+/* For vfio only */
+struct virtio_vfio_dev {
+     bool            is_vfio;        /* True: vfio i/f,
+                                      * False: not a vfio i/f

Well, this is weird; you are adding a flag to tell whether it's a
vfio device __inside__ a vfio struct.

Back to the topic, this flag is not necessary to me: you can
check the pci_dev->kdrv flag.

yes, I'll replace is_vfio with pci_dev->kdrv.

quoted

quoted

+                                      */
+     struct rte_pci_device *pci_dev; /* vfio dev */

Note that I have already added this field into virtio_hw struct
at my latest virtio 1.0 pmd patchset.

While I told you before that you should not develop patches based
on my patcheset, I guess you can do that now. Since it should be
in good shape and close to be merged.

Okay, Before rebasing my v5 patch on your 1.0 virtio patch, I like to
understand which qemu version support virtio 1.0 spec?

Ignore, I figured out in other thread,
qemu version >2.4, such as 2.4.1 or 2.5.0.

It will not matter. You can continue using the old legacy virtio, which
is the default case: my patchset keeps the backward compatibility.

What's worty noting is that virtio 1.0 uses memory mmaped bar space for
configuration, instead of ioport reading/writing. Therefore, I'd suggest
you to keep testing with legacy virtio, to make sure the VFIO stuff works.
And off course, virtio 1.0 testing is also welcome, to make sure it works
on ARM as well.

I am testing for virtio 1.0 and 0.95 for arm including your patch,
soon we;ll post the patch series that is rebased on / dependent on
below patchset:
- virtio 1.0
- vfio-noiommu
- KDRV check by huawei

IMO, we should start merging the dependent patches as because I'll
have to rebase, then do regression across the platform at least for
x86/arm64 and it's quite a work now.

Beside that I have few question specific to vfio in virtio pmd driver;
- vfio don't need resource_init functionality as it uses struct
rte_pci_dev but it need parsing so to make sure
    1. user has setted no_iommu mode
    2. virtio pci device attached to vfio-no-iommu driver or not.

So for 1) I am thinking to add RTE_KDRV_VFIO_NOIOMMU mode and a helper
function like pci_vfio_is_iommu(), such that  pc_xxx_scan() function
updates dev->kdrv with RTE_KDRV_VFIO_NOIOMMU at driver probe time.

case 2) would check for _noiommu mode and then would verify that
driver is attached or not?

above two case applicable to both virtio spec 1.0 and 0.95. I have
done changes for those two case for v5 patch series,l any comment
welcome before I push patch for review.

Thanks.

        --yliu