Hello Matthew,
On 5/23/23 18:51, Matthew Rosato wrote:
On 5/9/23 5:59 PM, Alex Williamson wrote:
From: Minwoo Im <minwoo...@samsung.com>
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.st...@gimli.home/
[2]
https://lore.kernel.org/linux-pci/158396044753.5601.14804870681174789709.st...@gimli.home/
Cc: Alex Williamson <alex.william...@redhat.com>
Signed-off-by: Minwoo Im <minwoo...@samsung.com>
Reviewed-by: Klaus Jensen <k.jen...@samsung.com>
Link:
https://lore.kernel.org/r/20230320073522epcms2p48f682ecdb73e0ae1a4850ad0712fd780@epcms2p4
Signed-off-by: Alex Williamson <alex.william...@redhat.com>
Hi Minwoo, Alex,
I'm seeing a regression in vfio-pci on s390 and bisect points to this commit.
I don't believe it's specific to s390 though, but rather when not using this
new vf-token, see below...
---
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 ec9a854361ac..cf27f28936cb 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;
^ here we copy the pointer when a vf-token was not specified.
+ }
+
+ ret = vfio_get_device(group, name, vbasedev, errp);
+ g_free(name);
^ and then free it regardless. But I don't think we meant to free what
vbasedev->name points to, this was meant to free a duplicate string. I'm
subsequently seeing qemu crashes later on e.g. during device unplug.
I think doing a strdup in either case would fix the issue OR skipping the g_free when
qemu_uuid_is_null(&vdev->vf_token).
FWIW, I tried the following and it resolved the issue for me:
Zhenzhong provided the exact same fix :
https://lore.kernel.org/qemu-devel/20230517024651.82248-1-zhenzhong.d...@intel.com/
Thanks,
C.
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index bf27a39905..73874a94de 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2994,7 +2994,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
qemu_uuid_unparse(&vdev->vf_token, uuid);
name = g_strdup_printf("%s vf_token=%s", vbasedev->name, uuid);
} else {
- name = vbasedev->name;
+ name = g_strdup(vbasedev->name);
}
ret = vfio_get_device(group, name, vbasedev, errp);
