* [PATCH] vfio/pci: add support for VF token
[not found] <CGME20230320073522epcms2p48f682ecdb73e0ae1a4850ad0712fd780@epcms2p4>
@ 2023-03-20 7:35 ` Minwoo Im
2023-03-20 10:03 ` Cédric Le Goater
2023-03-22 21:19 ` Minwoo Im
2023-03-24 8:42 ` Minwoo Im
2 siblings, 1 reply; 6+ messages in thread
From: Minwoo Im @ 2023-03-20 7:35 UTC (permalink / raw)
To: qemu-devel@nongnu.org
Cc: alex.williamson@redhat.com, SSDR Gost Dev, Klaus Birkelund Jensen,
Minwoo Im
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] vfio/pci: add support for VF token
2023-03-20 7:35 ` [PATCH] vfio/pci: add support for VF token Minwoo Im
@ 2023-03-20 10:03 ` Cédric Le Goater
2023-03-20 15:39 ` Alex Williamson
0 siblings, 1 reply; 6+ messages in thread
From: Cédric Le Goater @ 2023-03-20 10:03 UTC (permalink / raw)
To: minwoo.im, qemu-devel@nongnu.org
Cc: alex.williamson@redhat.com, SSDR Gost Dev, Klaus Birkelund Jensen
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 *);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] vfio/pci: add support for VF token
2023-03-20 10:03 ` Cédric Le Goater
@ 2023-03-20 15:39 ` Alex Williamson
0 siblings, 0 replies; 6+ messages in thread
From: Alex Williamson @ 2023-03-20 15:39 UTC (permalink / raw)
To: Cédric Le Goater
Cc: minwoo.im, qemu-devel@nongnu.org, SSDR Gost Dev,
Klaus Birkelund Jensen
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 *);
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] vfio/pci: add support for VF token
[not found] <CGME20230320073522epcms2p48f682ecdb73e0ae1a4850ad0712fd780@epcms2p4>
2023-03-20 7:35 ` [PATCH] vfio/pci: add support for VF token Minwoo Im
@ 2023-03-22 21:19 ` Minwoo Im
2023-03-23 18:46 ` Alex Williamson
2023-03-24 8:42 ` Minwoo Im
2 siblings, 1 reply; 6+ messages in thread
From: Minwoo Im @ 2023-03-22 21:19 UTC (permalink / raw)
To: Alex Williamson
Cc: Cédric Le Goater, qemu-devel@nongnu.org, SSDR Gost Dev,
Klaus Birkelund Jensen
> 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!
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] vfio/pci: add support for VF token
2023-03-22 21:19 ` Minwoo Im
@ 2023-03-23 18:46 ` Alex Williamson
0 siblings, 0 replies; 6+ messages in thread
From: Alex Williamson @ 2023-03-23 18:46 UTC (permalink / raw)
To: Minwoo Im
Cc: Cédric Le Goater, qemu-devel@nongnu.org, SSDR Gost Dev,
Klaus Birkelund Jensen
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] vfio/pci: add support for VF token
[not found] <CGME20230320073522epcms2p48f682ecdb73e0ae1a4850ad0712fd780@epcms2p4>
2023-03-20 7:35 ` [PATCH] vfio/pci: add support for VF token Minwoo Im
2023-03-22 21:19 ` Minwoo Im
@ 2023-03-24 8:42 ` Minwoo Im
2 siblings, 0 replies; 6+ messages in thread
From: Minwoo Im @ 2023-03-24 8:42 UTC (permalink / raw)
To: Alex Williamson
Cc: Cédric Le Goater, qemu-devel@nongnu.org, SSDR Gost Dev,
Klaus Birkelund Jensen
> -----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!
^ permalink raw reply [flat|nested] 6+ messages in thread