[PATCH] vfio/pci: add support for VF token

Minwoo Im posted 1 patch 1 year, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230320073522epcms2p48f682ecdb73e0ae1a4850ad0712fd780@epcms2p4
Maintainers: Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>
hw/vfio/pci.c | 13 ++++++++++++-
hw/vfio/pci.h |  1 +
2 files changed, 13 insertions(+), 1 deletion(-)
[PATCH] vfio/pci: add support for VF token
Posted by Minwoo Im 1 year, 1 month ago
VF token was introduced [1] to kernel vfio-pci along with SR-IOV
support [2].  This patch adds support VF token among PF and VF(s). To
passthu PCIe VF to a VM, kernel >= v5.7 needs this.

It can be configured with UUID like:

  -device vfio-pci,host=DDDD:BB:DD:F,vf-token=<uuid>,...

[1] https://lore.kernel.org/linux-pci/158396393244.5601.10297430724964025753.stgit@gimli.home/
[2] https://lore.kernel.org/linux-pci/158396044753.5601.14804870681174789709.stgit@gimli.home/

Cc: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Minwoo Im <minwoo.im@samsung.com>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/vfio/pci.c | 13 ++++++++++++-
 hw/vfio/pci.h |  1 +
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index ec9a854361..cf27f28936 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2856,6 +2856,8 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
     int groupid;
     int i, ret;
     bool is_mdev;
+    char uuid[UUID_FMT_LEN];
+    char *name;
 
     if (!vbasedev->sysfsdev) {
         if (!(~vdev->host.domain || ~vdev->host.bus ||
@@ -2936,7 +2938,15 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
         goto error;
     }
 
-    ret = vfio_get_device(group, vbasedev->name, vbasedev, errp);
+    if (!qemu_uuid_is_null(&vdev->vf_token)) {
+        qemu_uuid_unparse(&vdev->vf_token, uuid);
+        name = g_strdup_printf("%s vf_token=%s", vbasedev->name, uuid);
+    } else {
+        name = vbasedev->name;
+    }
+
+    ret = vfio_get_device(group, name, vbasedev, errp);
+    g_free(name);
     if (ret) {
         vfio_put_group(group);
         goto error;
@@ -3268,6 +3278,7 @@ static void vfio_instance_init(Object *obj)
 
 static Property vfio_pci_dev_properties[] = {
     DEFINE_PROP_PCI_HOST_DEVADDR("host", VFIOPCIDevice, host),
+    DEFINE_PROP_UUID_NODEFAULT("vf-token", VFIOPCIDevice, vf_token),
     DEFINE_PROP_STRING("sysfsdev", VFIOPCIDevice, vbasedev.sysfsdev),
     DEFINE_PROP_ON_OFF_AUTO("x-pre-copy-dirty-page-tracking", VFIOPCIDevice,
                             vbasedev.pre_copy_dirty_page_tracking,
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index 177abcc8fb..2674476d6c 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -137,6 +137,7 @@ struct VFIOPCIDevice {
     VFIOVGA *vga; /* 0xa0000, 0x3b0, 0x3c0 */
     void *igd_opregion;
     PCIHostDeviceAddress host;
+    QemuUUID vf_token;
     EventNotifier err_notifier;
     EventNotifier req_notifier;
     int (*resetfn)(struct VFIOPCIDevice *);
-- 
2.34.1
Re: [PATCH] vfio/pci: add support for VF token
Posted by Cédric Le Goater 1 year, 1 month ago
On 3/20/23 08:35, Minwoo Im wrote:
> VF token was introduced [1] to kernel vfio-pci along with SR-IOV
> support [2].  This patch adds support VF token among PF and VF(s). To
> passthu PCIe VF to a VM, kernel >= v5.7 needs this.
> 
> It can be configured with UUID like:
> 
>    -device vfio-pci,host=DDDD:BB:DD:F,vf-token=<uuid>,...
> 
> [1] https://lore.kernel.org/linux-pci/158396393244.5601.10297430724964025753.stgit@gimli.home/
> [2] https://lore.kernel.org/linux-pci/158396044753.5601.14804870681174789709.stgit@gimli.home/
> 
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Minwoo Im <minwoo.im@samsung.com>
> Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
> ---
>   hw/vfio/pci.c | 13 ++++++++++++-
>   hw/vfio/pci.h |  1 +
>   2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index ec9a854361..cf27f28936 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2856,6 +2856,8 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>       int groupid;
>       int i, ret;
>       bool is_mdev;
> +    char uuid[UUID_FMT_LEN];
> +    char *name;
>   
>       if (!vbasedev->sysfsdev) {
>           if (!(~vdev->host.domain || ~vdev->host.bus ||
> @@ -2936,7 +2938,15 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>           goto error;
>       }
>   
> -    ret = vfio_get_device(group, vbasedev->name, vbasedev, errp);
> +    if (!qemu_uuid_is_null(&vdev->vf_token)) {
> +        qemu_uuid_unparse(&vdev->vf_token, uuid);
> +        name = g_strdup_printf("%s vf_token=%s", vbasedev->name, uuid);
> +    } else {
> +        name = vbasedev->name;
> +    }
> +
> +    ret = vfio_get_device(group, name, vbasedev, errp);
> +    g_free(name);
>       if (ret) {
>           vfio_put_group(group);
>           goto error;

Shouldn't we set the VF token in the kernel also ? See this QEMU implementation

   https://lore.kernel.org/lkml/20200204161737.34696b91@w520.home/

May be I misunderstood.

Thanks,

C.

> @@ -3268,6 +3278,7 @@ static void vfio_instance_init(Object *obj)
>   
>   static Property vfio_pci_dev_properties[] = {
>       DEFINE_PROP_PCI_HOST_DEVADDR("host", VFIOPCIDevice, host),
> +    DEFINE_PROP_UUID_NODEFAULT("vf-token", VFIOPCIDevice, vf_token),
>       DEFINE_PROP_STRING("sysfsdev", VFIOPCIDevice, vbasedev.sysfsdev),
>       DEFINE_PROP_ON_OFF_AUTO("x-pre-copy-dirty-page-tracking", VFIOPCIDevice,
>                               vbasedev.pre_copy_dirty_page_tracking,
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index 177abcc8fb..2674476d6c 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -137,6 +137,7 @@ struct VFIOPCIDevice {
>       VFIOVGA *vga; /* 0xa0000, 0x3b0, 0x3c0 */
>       void *igd_opregion;
>       PCIHostDeviceAddress host;
> +    QemuUUID vf_token;
>       EventNotifier err_notifier;
>       EventNotifier req_notifier;
>       int (*resetfn)(struct VFIOPCIDevice *);
Re: [PATCH] vfio/pci: add support for VF token
Posted by Alex Williamson 1 year, 1 month ago
On Mon, 20 Mar 2023 11:03:40 +0100
Cédric Le Goater <clg@kaod.org> wrote:

> On 3/20/23 08:35, Minwoo Im wrote:
> > VF token was introduced [1] to kernel vfio-pci along with SR-IOV
> > support [2].  This patch adds support VF token among PF and VF(s). To
> > passthu PCIe VF to a VM, kernel >= v5.7 needs this.
> > 
> > It can be configured with UUID like:
> > 
> >    -device vfio-pci,host=DDDD:BB:DD:F,vf-token=<uuid>,...
> > 
> > [1] https://lore.kernel.org/linux-pci/158396393244.5601.10297430724964025753.stgit@gimli.home/
> > [2] https://lore.kernel.org/linux-pci/158396044753.5601.14804870681174789709.stgit@gimli.home/
> > 
> > Cc: Alex Williamson <alex.williamson@redhat.com>
> > Signed-off-by: Minwoo Im <minwoo.im@samsung.com>
> > Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
> > ---
> >   hw/vfio/pci.c | 13 ++++++++++++-
> >   hw/vfio/pci.h |  1 +
> >   2 files changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> > index ec9a854361..cf27f28936 100644
> > --- a/hw/vfio/pci.c
> > +++ b/hw/vfio/pci.c
> > @@ -2856,6 +2856,8 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
> >       int groupid;
> >       int i, ret;
> >       bool is_mdev;
> > +    char uuid[UUID_FMT_LEN];
> > +    char *name;
> >   
> >       if (!vbasedev->sysfsdev) {
> >           if (!(~vdev->host.domain || ~vdev->host.bus ||
> > @@ -2936,7 +2938,15 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
> >           goto error;
> >       }
> >   
> > -    ret = vfio_get_device(group, vbasedev->name, vbasedev, errp);
> > +    if (!qemu_uuid_is_null(&vdev->vf_token)) {
> > +        qemu_uuid_unparse(&vdev->vf_token, uuid);
> > +        name = g_strdup_printf("%s vf_token=%s", vbasedev->name, uuid);
> > +    } else {
> > +        name = vbasedev->name;
> > +    }
> > +
> > +    ret = vfio_get_device(group, name, vbasedev, errp);
> > +    g_free(name);
> >       if (ret) {
> >           vfio_put_group(group);
> >           goto error;  
> 
> Shouldn't we set the VF token in the kernel also ? See this QEMU implementation
> 
>    https://lore.kernel.org/lkml/20200204161737.34696b91@w520.home/
> 
> May be I misunderstood.
> 

I think you're referring to the part there that calls
VFIO_DEVICE_FEATURE in order to set a VF token.  I don't think that's
necessarily applicable here.  I believe this patch is only trying to
make it so that QEMU can consume a VF associated with a PF owned by a
userspace vfio driver, ie. not QEMU.

Setting the VF token is only relevant to PFs, which would require
significantly more SR-IOV infrastructure in QEMU than sketched out in
that proof-of-concept patch.  Even if we did have a QEMU owned PF where
we wanted to generate VFs, the token we use in that case would likely
need to be kept private to QEMU, not passed on the command line.
Thanks,

Alex

> > @@ -3268,6 +3278,7 @@ static void vfio_instance_init(Object *obj)
> >   
> >   static Property vfio_pci_dev_properties[] = {
> >       DEFINE_PROP_PCI_HOST_DEVADDR("host", VFIOPCIDevice, host),
> > +    DEFINE_PROP_UUID_NODEFAULT("vf-token", VFIOPCIDevice, vf_token),
> >       DEFINE_PROP_STRING("sysfsdev", VFIOPCIDevice, vbasedev.sysfsdev),
> >       DEFINE_PROP_ON_OFF_AUTO("x-pre-copy-dirty-page-tracking", VFIOPCIDevice,
> >                               vbasedev.pre_copy_dirty_page_tracking,
> > diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> > index 177abcc8fb..2674476d6c 100644
> > --- a/hw/vfio/pci.h
> > +++ b/hw/vfio/pci.h
> > @@ -137,6 +137,7 @@ struct VFIOPCIDevice {
> >       VFIOVGA *vga; /* 0xa0000, 0x3b0, 0x3c0 */
> >       void *igd_opregion;
> >       PCIHostDeviceAddress host;
> > +    QemuUUID vf_token;
> >       EventNotifier err_notifier;
> >       EventNotifier req_notifier;
> >       int (*resetfn)(struct VFIOPCIDevice *);  
> 
> 
RE: [PATCH] vfio/pci: add support for VF token
Posted by Minwoo Im 1 year, 1 month ago
> On Mon, 20 Mar 2023 11:03:40 +0100
> Cédric Le Goater <clg@kaod.org> wrote:
> 
> > On 3/20/23 08:35, Minwoo Im wrote:
> > > VF token was introduced [1] to kernel vfio-pci along with SR-IOV
> > > support [2].  This patch adds support VF token among PF and VF(s). To
> > > passthu PCIe VF to a VM, kernel >= v5.7 needs this.
> > >
> > > It can be configured with UUID like:
> > >
> > >    -device vfio-pci,host=DDDD:BB:DD:F,vf-token=<uuid>,...
> > >
> > > [1] https://lore.kernel.org/linux-
> pci/158396393244.5601.10297430724964025753.stgit@gimli.home/
> > > [2] https://lore.kernel.org/linux-
> pci/158396044753.5601.14804870681174789709.stgit@gimli.home/
> > >
> > > Cc: Alex Williamson <alex.williamson@redhat.com>
> > > Signed-off-by: Minwoo Im <minwoo.im@samsung.com>
> > > Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
> > > ---
> > >   hw/vfio/pci.c | 13 ++++++++++++-
> > >   hw/vfio/pci.h |  1 +
> > >   2 files changed, 13 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> > > index ec9a854361..cf27f28936 100644
> > > --- a/hw/vfio/pci.c
> > > +++ b/hw/vfio/pci.c
> > > @@ -2856,6 +2856,8 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
> > >       int groupid;
> > >       int i, ret;
> > >       bool is_mdev;
> > > +    char uuid[UUID_FMT_LEN];
> > > +    char *name;
> > >
> > >       if (!vbasedev->sysfsdev) {
> > >           if (!(~vdev->host.domain || ~vdev->host.bus ||
> > > @@ -2936,7 +2938,15 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
> > >           goto error;
> > >       }
> > >
> > > -    ret = vfio_get_device(group, vbasedev->name, vbasedev, errp);
> > > +    if (!qemu_uuid_is_null(&vdev->vf_token)) {
> > > +        qemu_uuid_unparse(&vdev->vf_token, uuid);
> > > +        name = g_strdup_printf("%s vf_token=%s", vbasedev->name, uuid);
> > > +    } else {
> > > +        name = vbasedev->name;
> > > +    }
> > > +
> > > +    ret = vfio_get_device(group, name, vbasedev, errp);
> > > +    g_free(name);
> > >       if (ret) {
> > >           vfio_put_group(group);
> > >           goto error;
> >
> > Shouldn't we set the VF token in the kernel also ? See this QEMU implementation
> >
> >    https://lore.kernel.org/lkml/20200204161737.34696b91@w520.home/
> >
> > May be I misunderstood.
> >
> 
> I think you're referring to the part there that calls
> VFIO_DEVICE_FEATURE in order to set a VF token.  I don't think that's
> necessarily applicable here.  I believe this patch is only trying to
> make it so that QEMU can consume a VF associated with a PF owned by a
> userspace vfio driver, ie. not QEMU.

Yes, that's what this patch exactly does.

> 
> Setting the VF token is only relevant to PFs, which would require
> significantly more SR-IOV infrastructure in QEMU than sketched out in
> that proof-of-concept patch.  Even if we did have a QEMU owned PF where
> we wanted to generate VFs, the token we use in that case would likely
> need to be kept private to QEMU, not passed on the command line.
> Thanks,

Can we also take a command line property for the PF for that case that
QEMU owns a PF?  I think the one who wants to make QEMU owns PF or VF
should know the VF token.  If I've missed anything, please let me know.

Thanks!
Re: [PATCH] vfio/pci: add support for VF token
Posted by Alex Williamson 1 year, 1 month ago
On Thu, 23 Mar 2023 06:19:45 +0900
Minwoo Im <minwoo.im@samsung.com> wrote:

> > On Mon, 20 Mar 2023 11:03:40 +0100
> > Cédric Le Goater <clg@kaod.org> wrote:
> >   
> > > On 3/20/23 08:35, Minwoo Im wrote:  
> > > > VF token was introduced [1] to kernel vfio-pci along with SR-IOV
> > > > support [2].  This patch adds support VF token among PF and VF(s). To
> > > > passthu PCIe VF to a VM, kernel >= v5.7 needs this.
> > > >
> > > > It can be configured with UUID like:
> > > >
> > > >    -device vfio-pci,host=DDDD:BB:DD:F,vf-token=<uuid>,...
> > > >
> > > > [1] https://lore.kernel.org/linux-  
> > pci/158396393244.5601.10297430724964025753.stgit@gimli.home/  
> > > > [2] https://lore.kernel.org/linux-  
> > pci/158396044753.5601.14804870681174789709.stgit@gimli.home/  
> > > >
> > > > Cc: Alex Williamson <alex.williamson@redhat.com>
> > > > Signed-off-by: Minwoo Im <minwoo.im@samsung.com>
> > > > Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
> > > > ---
> > > >   hw/vfio/pci.c | 13 ++++++++++++-
> > > >   hw/vfio/pci.h |  1 +
> > > >   2 files changed, 13 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> > > > index ec9a854361..cf27f28936 100644
> > > > --- a/hw/vfio/pci.c
> > > > +++ b/hw/vfio/pci.c
> > > > @@ -2856,6 +2856,8 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
> > > >       int groupid;
> > > >       int i, ret;
> > > >       bool is_mdev;
> > > > +    char uuid[UUID_FMT_LEN];
> > > > +    char *name;
> > > >
> > > >       if (!vbasedev->sysfsdev) {
> > > >           if (!(~vdev->host.domain || ~vdev->host.bus ||
> > > > @@ -2936,7 +2938,15 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
> > > >           goto error;
> > > >       }
> > > >
> > > > -    ret = vfio_get_device(group, vbasedev->name, vbasedev, errp);
> > > > +    if (!qemu_uuid_is_null(&vdev->vf_token)) {
> > > > +        qemu_uuid_unparse(&vdev->vf_token, uuid);
> > > > +        name = g_strdup_printf("%s vf_token=%s", vbasedev->name, uuid);
> > > > +    } else {
> > > > +        name = vbasedev->name;
> > > > +    }
> > > > +
> > > > +    ret = vfio_get_device(group, name, vbasedev, errp);
> > > > +    g_free(name);
> > > >       if (ret) {
> > > >           vfio_put_group(group);
> > > >           goto error;  
> > >
> > > Shouldn't we set the VF token in the kernel also ? See this QEMU implementation
> > >
> > >    https://lore.kernel.org/lkml/20200204161737.34696b91@w520.home/
> > >
> > > May be I misunderstood.
> > >  
> > 
> > I think you're referring to the part there that calls
> > VFIO_DEVICE_FEATURE in order to set a VF token.  I don't think that's
> > necessarily applicable here.  I believe this patch is only trying to
> > make it so that QEMU can consume a VF associated with a PF owned by a
> > userspace vfio driver, ie. not QEMU.  
> 
> Yes, that's what this patch exactly does.
> 
> > 
> > Setting the VF token is only relevant to PFs, which would require
> > significantly more SR-IOV infrastructure in QEMU than sketched out in
> > that proof-of-concept patch.  Even if we did have a QEMU owned PF where
> > we wanted to generate VFs, the token we use in that case would likely
> > need to be kept private to QEMU, not passed on the command line.
> > Thanks,  
> 
> Can we also take a command line property for the PF for that case that
> QEMU owns a PF?  I think the one who wants to make QEMU owns PF or VF
> should know the VF token.  If I've missed anything, please let me know.

IIRC, the only case where a VF token is required for a PF is if there
are existing VFs in use.  Opening the PF would then require a token
matching the VFs.  In general though, if the PF is owned by QEMU, the
VF token should likely be an internal secret to QEMU.  Configuring the
PF device with a token suggests that VFs could be created and bound to
other userspace drivers outside of the control of the QEMU instance
that owns the PF.  Therefore I would not suggest adding the ability to
set the VF token for a PF without a strong use case in-hand, an
certainly not when QEMU doesn't expose SR-IOV support to be able to
manage VFs itself.  Thanks,

Alex
RE: [PATCH] vfio/pci: add support for VF token
Posted by Minwoo Im 1 year, 1 month ago

> -----Original Message-----
> From: qemu-devel-bounces+minwoo.im=samsung.com@nongnu.org <qemu-devel-
> bounces+minwoo.im=samsung.com@nongnu.org> On Behalf Of Alex Williamson
> Sent: Friday, March 24, 2023 3:46 AM
> To: Minwoo Im <minwoo.im@samsung.com>
> Cc: Cédric Le Goater <clg@kaod.org>; qemu-devel@nongnu.org; SSDR Gost Dev
> <gost.dev@samsung.com>; Klaus Birkelund Jensen <k.jensen@samsung.com>
> Subject: Re: [PATCH] vfio/pci: add support for VF token
> 
> On Thu, 23 Mar 2023 06:19:45 +0900
> Minwoo Im <minwoo.im@samsung.com> wrote:
> 
> > > On Mon, 20 Mar 2023 11:03:40 +0100
> > > Cédric Le Goater <clg@kaod.org> wrote:
> > >
> > > > On 3/20/23 08:35, Minwoo Im wrote:
> > > > > VF token was introduced [1] to kernel vfio-pci along with SR-IOV
> > > > > support [2].  This patch adds support VF token among PF and VF(s). To
> > > > > passthu PCIe VF to a VM, kernel >= v5.7 needs this.
> > > > >
> > > > > It can be configured with UUID like:
> > > > >
> > > > >    -device vfio-pci,host=DDDD:BB:DD:F,vf-token=<uuid>,...
> > > > >
> > > > > [1] https://lore.kernel.org/linux-
> > > pci/158396393244.5601.10297430724964025753.stgit@gimli.home/
> > > > > [2] https://lore.kernel.org/linux-
> > > pci/158396044753.5601.14804870681174789709.stgit@gimli.home/
> > > > >
> > > > > Cc: Alex Williamson <alex.williamson@redhat.com>
> > > > > Signed-off-by: Minwoo Im <minwoo.im@samsung.com>
> > > > > Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
> > > > > ---
> > > > >   hw/vfio/pci.c | 13 ++++++++++++-
> > > > >   hw/vfio/pci.h |  1 +
> > > > >   2 files changed, 13 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> > > > > index ec9a854361..cf27f28936 100644
> > > > > --- a/hw/vfio/pci.c
> > > > > +++ b/hw/vfio/pci.c
> > > > > @@ -2856,6 +2856,8 @@ static void vfio_realize(PCIDevice *pdev, Error
> **errp)
> > > > >       int groupid;
> > > > >       int i, ret;
> > > > >       bool is_mdev;
> > > > > +    char uuid[UUID_FMT_LEN];
> > > > > +    char *name;
> > > > >
> > > > >       if (!vbasedev->sysfsdev) {
> > > > >           if (!(~vdev->host.domain || ~vdev->host.bus ||
> > > > > @@ -2936,7 +2938,15 @@ static void vfio_realize(PCIDevice *pdev, Error
> **errp)
> > > > >           goto error;
> > > > >       }
> > > > >
> > > > > -    ret = vfio_get_device(group, vbasedev->name, vbasedev, errp);
> > > > > +    if (!qemu_uuid_is_null(&vdev->vf_token)) {
> > > > > +        qemu_uuid_unparse(&vdev->vf_token, uuid);
> > > > > +        name = g_strdup_printf("%s vf_token=%s", vbasedev->name, uuid);
> > > > > +    } else {
> > > > > +        name = vbasedev->name;
> > > > > +    }
> > > > > +
> > > > > +    ret = vfio_get_device(group, name, vbasedev, errp);
> > > > > +    g_free(name);
> > > > >       if (ret) {
> > > > >           vfio_put_group(group);
> > > > >           goto error;
> > > >
> > > > Shouldn't we set the VF token in the kernel also ? See this QEMU
> implementation
> > > >
> > > >    https://lore.kernel.org/lkml/20200204161737.34696b91@w520.home/
> > > >
> > > > May be I misunderstood.
> > > >
> > >
> > > I think you're referring to the part there that calls
> > > VFIO_DEVICE_FEATURE in order to set a VF token.  I don't think that's
> > > necessarily applicable here.  I believe this patch is only trying to
> > > make it so that QEMU can consume a VF associated with a PF owned by a
> > > userspace vfio driver, ie. not QEMU.
> >
> > Yes, that's what this patch exactly does.
> >
> > >
> > > Setting the VF token is only relevant to PFs, which would require
> > > significantly more SR-IOV infrastructure in QEMU than sketched out in
> > > that proof-of-concept patch.  Even if we did have a QEMU owned PF where
> > > we wanted to generate VFs, the token we use in that case would likely
> > > need to be kept private to QEMU, not passed on the command line.
> > > Thanks,
> >
> > Can we also take a command line property for the PF for that case that
> > QEMU owns a PF?  I think the one who wants to make QEMU owns PF or VF
> > should know the VF token.  If I've missed anything, please let me know.
> 
> IIRC, the only case where a VF token is required for a PF is if there
> are existing VFs in use.  Opening the PF would then require a token
> matching the VFs.  In general though, if the PF is owned by QEMU, the
> VF token should likely be an internal secret to QEMU.  Configuring the
> PF device with a token suggests that VFs could be created and bound to
> other userspace drivers outside of the control of the QEMU instance
> that owns the PF.  Therefore I would not suggest adding the ability to
> set the VF token for a PF without a strong use case in-hand, an
> certainly not when QEMU doesn't expose SR-IOV support to be able to
> manage VFs itself.  Thanks,
> 
> Alex
> 

Thanks for the explanation!