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

[Alex Williamson <alex.williamson@redhat.com>]

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 *);  
> 
>