| Message ID | 20240712114704.8708-2-joao.m.martins@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/iommufd: IOMMUFD Dirty Tracking | expand |
Hello Joao On 7/12/24 13:46, Joao Martins wrote: > In preparation to skip initialization of the HostIOMMUDevice for mdev, > extract the checks that validate if a device is an mdev into helpers. > > A vfio_set_mdev() is created, and subsystems consult VFIODevice::mdev > to check if it's mdev or not. > > Signed-off-by: Joao Martins <joao.m.martins@oracle.com> > --- > include/hw/vfio/vfio-common.h | 2 ++ > hw/vfio/helpers.c | 18 ++++++++++++++++++ > hw/vfio/pci.c | 9 ++------- > 3 files changed, 22 insertions(+), 7 deletions(-) > > diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h > index e8ddf92bb185..7419466bca92 100644 > --- a/include/hw/vfio/vfio-common.h > +++ b/include/hw/vfio/vfio-common.h > @@ -116,6 +116,7 @@ typedef struct VFIODevice { > DeviceState *dev; > int fd; > int type; > + bool mdev; > bool reset_works; > bool needs_reset; > bool no_mmap; > @@ -231,6 +232,7 @@ void vfio_region_exit(VFIORegion *region); > void vfio_region_finalize(VFIORegion *region); > void vfio_reset_handler(void *opaque); > struct vfio_device_info *vfio_get_device_info(int fd); > +void vfio_set_mdev(VFIODevice *vbasedev); > bool vfio_attach_device(char *name, VFIODevice *vbasedev, > AddressSpace *as, Error **errp); > void vfio_detach_device(VFIODevice *vbasedev); > diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c > index b14edd46edc9..bace0e788a09 100644 > --- a/hw/vfio/helpers.c > +++ b/hw/vfio/helpers.c > @@ -675,3 +675,21 @@ int vfio_device_get_aw_bits(VFIODevice *vdev) > > return HOST_IOMMU_DEVICE_CAP_AW_BITS_MAX; > } > + > +void vfio_set_mdev(VFIODevice *vbasedev) Could you please change this routine to : bool vfio_device_is_mdev(VFIODevice *vbasedev) > +{ > + g_autofree char *tmp = NULL; > + char *subsys; a g_autofree variable is preferable here. > + bool is_mdev; > + > + if (!vbasedev->sysfsdev) { > + return; > + } > + > + tmp = g_strdup_printf("%s/subsystem", vbasedev->sysfsdev); > + subsys = realpath(tmp, NULL); > + is_mdev = subsys && (strcmp(subsys, "/sys/bus/mdev") == 0); simply return is the result here and .... > + free(subsys); > + > + vbasedev->mdev = is_mdev; > +} > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index e03d9f3ba546..585f23a18406 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -2963,12 +2963,10 @@ static void vfio_realize(PCIDevice *pdev, Error **errp) > ERRP_GUARD(); > VFIOPCIDevice *vdev = VFIO_PCI(pdev); > VFIODevice *vbasedev = &vdev->vbasedev; > - char *subsys; > int i, ret; > bool is_mdev; > char uuid[UUID_STR_LEN]; > g_autofree char *name = NULL; > - g_autofree char *tmp = NULL; > > if (vbasedev->fd < 0 && !vbasedev->sysfsdev) { > if (!(~vdev->host.domain || ~vdev->host.bus || > @@ -2997,11 +2995,8 @@ static void vfio_realize(PCIDevice *pdev, Error **errp) > * stays in sync with the active working set of the guest driver. Prevent > * the x-balloon-allowed option unless this is minimally an mdev device. > */ > - tmp = g_strdup_printf("%s/subsystem", vbasedev->sysfsdev); > - subsys = realpath(tmp, NULL); > - is_mdev = subsys && (strcmp(subsys, "/sys/bus/mdev") == 0); > - free(subsys); > - > + vfio_set_mdev(vbasedev); > + is_mdev = vbasedev->mdev; replace with : vbasedev->mdev = vfio_device_is_mdev(vbasedev); and use vbasedev->mdev instead of is_mdev where needed. Thanks, C. > trace_vfio_mdev(vbasedev->name, is_mdev); > > if (vbasedev->ram_block_discard_allowed && !is_mdev) {
On 16/07/2024 10:21, Cédric Le Goater wrote: > Hello Joao > > On 7/12/24 13:46, Joao Martins wrote: >> In preparation to skip initialization of the HostIOMMUDevice for mdev, >> extract the checks that validate if a device is an mdev into helpers. >> >> A vfio_set_mdev() is created, and subsystems consult VFIODevice::mdev >> to check if it's mdev or not. >> >> Signed-off-by: Joao Martins <joao.m.martins@oracle.com> >> --- >> include/hw/vfio/vfio-common.h | 2 ++ >> hw/vfio/helpers.c | 18 ++++++++++++++++++ >> hw/vfio/pci.c | 9 ++------- >> 3 files changed, 22 insertions(+), 7 deletions(-) >> >> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h >> index e8ddf92bb185..7419466bca92 100644 >> --- a/include/hw/vfio/vfio-common.h >> +++ b/include/hw/vfio/vfio-common.h >> @@ -116,6 +116,7 @@ typedef struct VFIODevice { >> DeviceState *dev; >> int fd; >> int type; >> + bool mdev; >> bool reset_works; >> bool needs_reset; >> bool no_mmap; >> @@ -231,6 +232,7 @@ void vfio_region_exit(VFIORegion *region); >> void vfio_region_finalize(VFIORegion *region); >> void vfio_reset_handler(void *opaque); >> struct vfio_device_info *vfio_get_device_info(int fd); >> +void vfio_set_mdev(VFIODevice *vbasedev); >> bool vfio_attach_device(char *name, VFIODevice *vbasedev, >> AddressSpace *as, Error **errp); >> void vfio_detach_device(VFIODevice *vbasedev); >> diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c >> index b14edd46edc9..bace0e788a09 100644 >> --- a/hw/vfio/helpers.c >> +++ b/hw/vfio/helpers.c >> @@ -675,3 +675,21 @@ int vfio_device_get_aw_bits(VFIODevice *vdev) >> return HOST_IOMMU_DEVICE_CAP_AW_BITS_MAX; >> } >> + >> +void vfio_set_mdev(VFIODevice *vbasedev) > > Could you please change this routine to : > > bool vfio_device_is_mdev(VFIODevice *vbasedev) > OK >> +{ >> + g_autofree char *tmp = NULL; >> + char *subsys; > > a g_autofree variable is preferable here. > It was a code move hence why I kept the exact same structure I will change to your suggestion >> + bool is_mdev; >> + >> + if (!vbasedev->sysfsdev) { >> + return; >> + } >> + >> + tmp = g_strdup_printf("%s/subsystem", vbasedev->sysfsdev); >> + subsys = realpath(tmp, NULL); >> + is_mdev = subsys && (strcmp(subsys, "/sys/bus/mdev") == 0); > > simply return is the result here and .... > OK >> + free(subsys); >> + >> + vbasedev->mdev = is_mdev; >> +} >> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c >> index e03d9f3ba546..585f23a18406 100644 >> --- a/hw/vfio/pci.c >> +++ b/hw/vfio/pci.c >> @@ -2963,12 +2963,10 @@ static void vfio_realize(PCIDevice *pdev, Error **errp) >> ERRP_GUARD(); >> VFIOPCIDevice *vdev = VFIO_PCI(pdev); >> VFIODevice *vbasedev = &vdev->vbasedev; >> - char *subsys; >> int i, ret; >> bool is_mdev; >> char uuid[UUID_STR_LEN]; >> g_autofree char *name = NULL; >> - g_autofree char *tmp = NULL; >> if (vbasedev->fd < 0 && !vbasedev->sysfsdev) { >> if (!(~vdev->host.domain || ~vdev->host.bus || >> @@ -2997,11 +2995,8 @@ static void vfio_realize(PCIDevice *pdev, Error **errp) >> * stays in sync with the active working set of the guest driver. Prevent >> * the x-balloon-allowed option unless this is minimally an mdev device. >> */ >> - tmp = g_strdup_printf("%s/subsystem", vbasedev->sysfsdev); >> - subsys = realpath(tmp, NULL); >> - is_mdev = subsys && (strcmp(subsys, "/sys/bus/mdev") == 0); >> - free(subsys); >> - >> + vfio_set_mdev(vbasedev); >> + is_mdev = vbasedev->mdev; > > replace with : > > vbasedev->mdev = vfio_device_is_mdev(vbasedev); > > and use vbasedev->mdev instead of is_mdev where needed. > OK > > Thanks, > > C. > > > > >> trace_vfio_mdev(vbasedev->name, is_mdev); >> if (vbasedev->ram_block_discard_allowed && !is_mdev) { >
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h index e8ddf92bb185..7419466bca92 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -116,6 +116,7 @@ typedef struct VFIODevice { DeviceState *dev; int fd; int type; + bool mdev; bool reset_works; bool needs_reset; bool no_mmap; @@ -231,6 +232,7 @@ void vfio_region_exit(VFIORegion *region); void vfio_region_finalize(VFIORegion *region); void vfio_reset_handler(void *opaque); struct vfio_device_info *vfio_get_device_info(int fd); +void vfio_set_mdev(VFIODevice *vbasedev); bool vfio_attach_device(char *name, VFIODevice *vbasedev, AddressSpace *as, Error **errp); void vfio_detach_device(VFIODevice *vbasedev); diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c index b14edd46edc9..bace0e788a09 100644 --- a/hw/vfio/helpers.c +++ b/hw/vfio/helpers.c @@ -675,3 +675,21 @@ int vfio_device_get_aw_bits(VFIODevice *vdev) return HOST_IOMMU_DEVICE_CAP_AW_BITS_MAX; } + +void vfio_set_mdev(VFIODevice *vbasedev) +{ + g_autofree char *tmp = NULL; + char *subsys; + bool is_mdev; + + if (!vbasedev->sysfsdev) { + return; + } + + tmp = g_strdup_printf("%s/subsystem", vbasedev->sysfsdev); + subsys = realpath(tmp, NULL); + is_mdev = subsys && (strcmp(subsys, "/sys/bus/mdev") == 0); + free(subsys); + + vbasedev->mdev = is_mdev; +} diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index e03d9f3ba546..585f23a18406 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -2963,12 +2963,10 @@ static void vfio_realize(PCIDevice *pdev, Error **errp) ERRP_GUARD(); VFIOPCIDevice *vdev = VFIO_PCI(pdev); VFIODevice *vbasedev = &vdev->vbasedev; - char *subsys; int i, ret; bool is_mdev; char uuid[UUID_STR_LEN]; g_autofree char *name = NULL; - g_autofree char *tmp = NULL; if (vbasedev->fd < 0 && !vbasedev->sysfsdev) { if (!(~vdev->host.domain || ~vdev->host.bus || @@ -2997,11 +2995,8 @@ static void vfio_realize(PCIDevice *pdev, Error **errp) * stays in sync with the active working set of the guest driver. Prevent * the x-balloon-allowed option unless this is minimally an mdev device. */ - tmp = g_strdup_printf("%s/subsystem", vbasedev->sysfsdev); - subsys = realpath(tmp, NULL); - is_mdev = subsys && (strcmp(subsys, "/sys/bus/mdev") == 0); - free(subsys); - + vfio_set_mdev(vbasedev); + is_mdev = vbasedev->mdev; trace_vfio_mdev(vbasedev->name, is_mdev); if (vbasedev->ram_block_discard_allowed && !is_mdev) {
In preparation to skip initialization of the HostIOMMUDevice for mdev, extract the checks that validate if a device is an mdev into helpers. A vfio_set_mdev() is created, and subsystems consult VFIODevice::mdev to check if it's mdev or not. Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- include/hw/vfio/vfio-common.h | 2 ++ hw/vfio/helpers.c | 18 ++++++++++++++++++ hw/vfio/pci.c | 9 ++------- 3 files changed, 22 insertions(+), 7 deletions(-)