| Message ID | 20240212135643.5858-7-joao.m.martins@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio/iommufd: IOMMUFD Dirty Tracking | expand |
On Mon, Feb 12, 2024 at 01:56:41PM +0000, Joao Martins wrote: > Allow disabling hugepages to be dirty track at base page > granularity in similar vein to vfio_type1_iommu.disable_hugepages > but per IOAS. No objection to this, but I just wanted to observe I didn't imagine using this option for this purpose. It should work OK but it is a pretty big an inefficient hammer :) Jason
Joao Martins <joao.m.martins@oracle.com> writes: > Allow disabling hugepages to be dirty track at base page > granularity in similar vein to vfio_type1_iommu.disable_hugepages > but per IOAS. > > Signed-off-by: Joao Martins <joao.m.martins@oracle.com> [...] > diff --git a/qapi/qom.json b/qapi/qom.json > index 84af23fe245d..9ad27e2b939b 100644 > --- a/qapi/qom.json > +++ b/qapi/qom.json > @@ -809,7 +809,7 @@ > # Since: 9.0 > ## > { 'struct': 'IOMMUFDProperties', > - 'data': { '*fd': 'str' } } > + 'data': { '*fd': 'str', '*hugepages': 'bool' } } > > ## > # @RngProperties: Missing documentation for the new member. The latest QAPI PR is making this a hard error.
On 12/02/2024 17:17, Markus Armbruster wrote: > Joao Martins <joao.m.martins@oracle.com> writes: > >> Allow disabling hugepages to be dirty track at base page >> granularity in similar vein to vfio_type1_iommu.disable_hugepages >> but per IOAS. >> >> Signed-off-by: Joao Martins <joao.m.martins@oracle.com> > > [...] > >> diff --git a/qapi/qom.json b/qapi/qom.json >> index 84af23fe245d..9ad27e2b939b 100644 >> --- a/qapi/qom.json >> +++ b/qapi/qom.json >> @@ -809,7 +809,7 @@ >> # Since: 9.0 >> ## >> { 'struct': 'IOMMUFDProperties', >> - 'data': { '*fd': 'str' } } >> + 'data': { '*fd': 'str', '*hugepages': 'bool' } } >> >> ## >> # @RngProperties: > > Missing documentation for the new member. > > The latest QAPI PR is making this a hard error. > Gah, sorry. I think I didn't have that PR yet as I didn't hit any build errors. Missing the doc was pure distraction. Will fix it for the next version. Joao
Hi Joao, On 12/02/2024 15:56, Joao Martins wrote: > External email: Use caution opening links or attachments > > > Allow disabling hugepages to be dirty track at base page > granularity in similar vein to vfio_type1_iommu.disable_hugepages > but per IOAS. > > Signed-off-by: Joao Martins <joao.m.martins@oracle.com> > --- > backends/iommufd.c | 36 ++++++++++++++++++++++++++++++++++++ > backends/trace-events | 1 + > hw/vfio/iommufd.c | 4 ++++ > include/sysemu/iommufd.h | 4 ++++ > qapi/qom.json | 2 +- > 5 files changed, 46 insertions(+), 1 deletion(-) > > diff --git a/backends/iommufd.c b/backends/iommufd.c > index dd676d493c37..72fd98a9a50c 100644 > --- a/backends/iommufd.c > +++ b/backends/iommufd.c > @@ -29,6 +29,7 @@ static void iommufd_backend_init(Object *obj) > be->fd = -1; > be->users = 0; > be->owned = true; > + be->hugepages = 1; > } > > static void iommufd_backend_finalize(Object *obj) > @@ -63,6 +64,14 @@ static bool iommufd_backend_can_be_deleted(UserCreatable *uc) > return !be->users; > } > > +static void iommufd_backend_set_hugepages(Object *obj, bool enabled, > + Error **errp) > +{ > + IOMMUFDBackend *be = IOMMUFD_BACKEND(obj); > + > + be->hugepages = enabled; > +} > + > static void iommufd_backend_class_init(ObjectClass *oc, void *data) > { > UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); > @@ -70,6 +79,11 @@ static void iommufd_backend_class_init(ObjectClass *oc, void *data) > ucc->can_be_deleted = iommufd_backend_can_be_deleted; > > object_class_property_add_str(oc, "fd", NULL, iommufd_backend_set_fd); > + > + object_class_property_add_bool(oc, "hugepages", NULL, > + iommufd_backend_set_hugepages); > + object_class_property_set_description(oc, "hugepages", > + "Set to 'off' to disable hugepages"); > } > > int iommufd_backend_connect(IOMMUFDBackend *be, Error **errp) > @@ -106,6 +120,28 @@ out: > trace_iommufd_backend_disconnect(be->fd, be->users); > } > > +int iommufd_backend_set_option(int fd, uint32_t object_id, > + uint32_t option_id, uint64_t val64) > +{ > + int ret; > + struct iommu_option option = { > + .size = sizeof(option), > + .option_id = option_id, > + .op = IOMMU_OPTION_OP_SET, > + .val64 = val64, > + .object_id = object_id, > + }; > + > + ret = ioctl(fd, IOMMU_OPTION, &option); > + if (ret) { > + error_report("Failed to set option %x to value %"PRIx64" %m", option_id, > + val64); > + } > + trace_iommufd_backend_set_option(fd, object_id, option_id, val64, ret); > + > + return ret; > +} > + > int iommufd_backend_alloc_ioas(IOMMUFDBackend *be, uint32_t *ioas_id, > Error **errp) > { > diff --git a/backends/trace-events b/backends/trace-events > index 11a27cb114b6..076166552881 100644 > --- a/backends/trace-events > +++ b/backends/trace-events > @@ -15,6 +15,7 @@ iommufd_backend_unmap_dma_non_exist(int iommufd, uint32_t ioas, uint64_t iova, u > iommufd_backend_unmap_dma(int iommufd, uint32_t ioas, uint64_t iova, uint64_t size, int ret) " iommufd=%d ioas=%d iova=0x%"PRIx64" size=0x%"PRIx64" (%d)" > iommufd_backend_alloc_hwpt(int iommufd, uint32_t dev_id, uint32_t pt_id, uint32_t flags, uint32_t hwpt_type, uint32_t len, uint64_t data_ptr, uint32_t out_hwpt_id, int ret) " iommufd=%d dev_id=%u pt_id=%u flags=0x%x hwpt_type=%u len=%u data_ptr=0x%"PRIx64" out_hwpt=%u (%d)" > iommufd_backend_alloc_ioas(int iommufd, uint32_t ioas, int ret) " iommufd=%d ioas=%d (%d)" > +iommufd_backend_set_option(int iommufd, uint32_t object_id, uint32_t option_id, uint64_t val, int ret) " iommufd=%d object_id=%u option_id=%u val64=0x%"PRIx64" (%d)" > iommufd_backend_free_id(int iommufd, uint32_t id, int ret) " iommufd=%d id=%d (%d)" > iommufd_backend_set_dirty(int iommufd, uint32_t hwpt_id, bool start, int ret) " iommufd=%d hwpt=%d enable=%d (%d)" > iommufd_backend_get_dirty_bitmap(int iommufd, uint32_t hwpt_id, uint64_t iova, uint64_t size, uint64_t page_size, int ret) " iommufd=%d hwpt=%d iova=0x%"PRIx64" size=0x%"PRIx64" page_size=0x%"PRIx64" (%d)" > diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c > index 79b13bd262cc..697d40841d7f 100644 > --- a/hw/vfio/iommufd.c > +++ b/hw/vfio/iommufd.c > @@ -521,6 +521,10 @@ static int iommufd_cdev_attach(const char *name, VFIODevice *vbasedev, > goto err_alloc_ioas; > } > > + if (!vbasedev->iommufd_dev.iommufd->hugepages) { > + iommufd_backend_set_option(vbasedev->iommufd_dev.iommufd->fd, ioas_id, > + IOMMU_OPTION_HUGE_PAGES, 0); Shouldn't we fail device attach if iommufd_backend_set_option() fails? Thanks. > + } > trace_iommufd_cdev_alloc_ioas(vbasedev->iommufd_dev.iommufd->fd, ioas_id); > > container = g_malloc0(sizeof(*container)); > diff --git a/include/sysemu/iommufd.h b/include/sysemu/iommufd.h > index ba19b7ea4c19..bc6607e3d444 100644 > --- a/include/sysemu/iommufd.h > +++ b/include/sysemu/iommufd.h > @@ -19,6 +19,7 @@ struct IOMMUFDBackend { > /*< protected >*/ > int fd; /* /dev/iommu file descriptor */ > bool owned; /* is the /dev/iommu opened internally */ > + bool hugepages; /* are hugepages enabled on the IOAS */ > uint32_t users; > > /*< public >*/ > @@ -30,6 +31,9 @@ void iommufd_backend_disconnect(IOMMUFDBackend *be); > int iommufd_backend_alloc_ioas(IOMMUFDBackend *be, uint32_t *ioas_id, > Error **errp); > void iommufd_backend_free_id(IOMMUFDBackend *be, uint32_t id); > +int iommufd_backend_set_option(int fd, uint32_t object_id, > + uint32_t option_id, > + uint64_t val64); > int iommufd_backend_map_dma(IOMMUFDBackend *be, uint32_t ioas_id, hwaddr iova, > ram_addr_t size, void *vaddr, bool readonly); > int iommufd_backend_unmap_dma(IOMMUFDBackend *be, uint32_t ioas_id, > diff --git a/qapi/qom.json b/qapi/qom.json > index 84af23fe245d..9ad27e2b939b 100644 > --- a/qapi/qom.json > +++ b/qapi/qom.json > @@ -809,7 +809,7 @@ > # Since: 9.0 > ## > { 'struct': 'IOMMUFDProperties', > - 'data': { '*fd': 'str' } } > + 'data': { '*fd': 'str', '*hugepages': 'bool' } } > > ## > # @RngProperties: > -- > 2.39.3 >
On 19/02/2024 10:05, Avihai Horon wrote: > Hi Joao, > > On 12/02/2024 15:56, Joao Martins wrote: >> External email: Use caution opening links or attachments >> >> >> Allow disabling hugepages to be dirty track at base page >> granularity in similar vein to vfio_type1_iommu.disable_hugepages >> but per IOAS. >> >> Signed-off-by: Joao Martins <joao.m.martins@oracle.com> >> --- >> backends/iommufd.c | 36 ++++++++++++++++++++++++++++++++++++ >> backends/trace-events | 1 + >> hw/vfio/iommufd.c | 4 ++++ >> include/sysemu/iommufd.h | 4 ++++ >> qapi/qom.json | 2 +- >> 5 files changed, 46 insertions(+), 1 deletion(-) >> >> diff --git a/backends/iommufd.c b/backends/iommufd.c >> index dd676d493c37..72fd98a9a50c 100644 >> --- a/backends/iommufd.c >> +++ b/backends/iommufd.c >> @@ -29,6 +29,7 @@ static void iommufd_backend_init(Object *obj) >> be->fd = -1; >> be->users = 0; >> be->owned = true; >> + be->hugepages = 1; >> } >> >> static void iommufd_backend_finalize(Object *obj) >> @@ -63,6 +64,14 @@ static bool iommufd_backend_can_be_deleted(UserCreatable *uc) >> return !be->users; >> } >> >> +static void iommufd_backend_set_hugepages(Object *obj, bool enabled, >> + Error **errp) >> +{ >> + IOMMUFDBackend *be = IOMMUFD_BACKEND(obj); >> + >> + be->hugepages = enabled; >> +} >> + >> static void iommufd_backend_class_init(ObjectClass *oc, void *data) >> { >> UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); >> @@ -70,6 +79,11 @@ static void iommufd_backend_class_init(ObjectClass *oc, >> void *data) >> ucc->can_be_deleted = iommufd_backend_can_be_deleted; >> >> object_class_property_add_str(oc, "fd", NULL, iommufd_backend_set_fd); >> + >> + object_class_property_add_bool(oc, "hugepages", NULL, >> + iommufd_backend_set_hugepages); >> + object_class_property_set_description(oc, "hugepages", >> + "Set to 'off' to disable hugepages"); >> } >> >> int iommufd_backend_connect(IOMMUFDBackend *be, Error **errp) >> @@ -106,6 +120,28 @@ out: >> trace_iommufd_backend_disconnect(be->fd, be->users); >> } >> >> +int iommufd_backend_set_option(int fd, uint32_t object_id, >> + uint32_t option_id, uint64_t val64) >> +{ >> + int ret; >> + struct iommu_option option = { >> + .size = sizeof(option), >> + .option_id = option_id, >> + .op = IOMMU_OPTION_OP_SET, >> + .val64 = val64, >> + .object_id = object_id, >> + }; >> + >> + ret = ioctl(fd, IOMMU_OPTION, &option); >> + if (ret) { >> + error_report("Failed to set option %x to value %"PRIx64" %m", option_id, >> + val64); >> + } >> + trace_iommufd_backend_set_option(fd, object_id, option_id, val64, ret); >> + >> + return ret; >> +} >> + >> int iommufd_backend_alloc_ioas(IOMMUFDBackend *be, uint32_t *ioas_id, >> Error **errp) >> { >> diff --git a/backends/trace-events b/backends/trace-events >> index 11a27cb114b6..076166552881 100644 >> --- a/backends/trace-events >> +++ b/backends/trace-events >> @@ -15,6 +15,7 @@ iommufd_backend_unmap_dma_non_exist(int iommufd, uint32_t >> ioas, uint64_t iova, u >> iommufd_backend_unmap_dma(int iommufd, uint32_t ioas, uint64_t iova, >> uint64_t size, int ret) " iommufd=%d ioas=%d iova=0x%"PRIx64" size=0x%"PRIx64" >> (%d)" >> iommufd_backend_alloc_hwpt(int iommufd, uint32_t dev_id, uint32_t pt_id, >> uint32_t flags, uint32_t hwpt_type, uint32_t len, uint64_t data_ptr, uint32_t >> out_hwpt_id, int ret) " iommufd=%d dev_id=%u pt_id=%u flags=0x%x hwpt_type=%u >> len=%u data_ptr=0x%"PRIx64" out_hwpt=%u (%d)" >> iommufd_backend_alloc_ioas(int iommufd, uint32_t ioas, int ret) " iommufd=%d >> ioas=%d (%d)" >> +iommufd_backend_set_option(int iommufd, uint32_t object_id, uint32_t >> option_id, uint64_t val, int ret) " iommufd=%d object_id=%u option_id=%u >> val64=0x%"PRIx64" (%d)" >> iommufd_backend_free_id(int iommufd, uint32_t id, int ret) " iommufd=%d >> id=%d (%d)" >> iommufd_backend_set_dirty(int iommufd, uint32_t hwpt_id, bool start, int >> ret) " iommufd=%d hwpt=%d enable=%d (%d)" >> iommufd_backend_get_dirty_bitmap(int iommufd, uint32_t hwpt_id, uint64_t >> iova, uint64_t size, uint64_t page_size, int ret) " iommufd=%d hwpt=%d >> iova=0x%"PRIx64" size=0x%"PRIx64" page_size=0x%"PRIx64" (%d)" >> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c >> index 79b13bd262cc..697d40841d7f 100644 >> --- a/hw/vfio/iommufd.c >> +++ b/hw/vfio/iommufd.c >> @@ -521,6 +521,10 @@ static int iommufd_cdev_attach(const char *name, >> VFIODevice *vbasedev, >> goto err_alloc_ioas; >> } >> >> + if (!vbasedev->iommufd_dev.iommufd->hugepages) { >> + iommufd_backend_set_option(vbasedev->iommufd_dev.iommufd->fd, ioas_id, >> + IOMMU_OPTION_HUGE_PAGES, 0); > > Shouldn't we fail device attach if iommufd_backend_set_option() fails? > Let handle error correctly and fail the attach.
diff --git a/backends/iommufd.c b/backends/iommufd.c index dd676d493c37..72fd98a9a50c 100644 --- a/backends/iommufd.c +++ b/backends/iommufd.c @@ -29,6 +29,7 @@ static void iommufd_backend_init(Object *obj) be->fd = -1; be->users = 0; be->owned = true; + be->hugepages = 1; } static void iommufd_backend_finalize(Object *obj) @@ -63,6 +64,14 @@ static bool iommufd_backend_can_be_deleted(UserCreatable *uc) return !be->users; } +static void iommufd_backend_set_hugepages(Object *obj, bool enabled, + Error **errp) +{ + IOMMUFDBackend *be = IOMMUFD_BACKEND(obj); + + be->hugepages = enabled; +} + static void iommufd_backend_class_init(ObjectClass *oc, void *data) { UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); @@ -70,6 +79,11 @@ static void iommufd_backend_class_init(ObjectClass *oc, void *data) ucc->can_be_deleted = iommufd_backend_can_be_deleted; object_class_property_add_str(oc, "fd", NULL, iommufd_backend_set_fd); + + object_class_property_add_bool(oc, "hugepages", NULL, + iommufd_backend_set_hugepages); + object_class_property_set_description(oc, "hugepages", + "Set to 'off' to disable hugepages"); } int iommufd_backend_connect(IOMMUFDBackend *be, Error **errp) @@ -106,6 +120,28 @@ out: trace_iommufd_backend_disconnect(be->fd, be->users); } +int iommufd_backend_set_option(int fd, uint32_t object_id, + uint32_t option_id, uint64_t val64) +{ + int ret; + struct iommu_option option = { + .size = sizeof(option), + .option_id = option_id, + .op = IOMMU_OPTION_OP_SET, + .val64 = val64, + .object_id = object_id, + }; + + ret = ioctl(fd, IOMMU_OPTION, &option); + if (ret) { + error_report("Failed to set option %x to value %"PRIx64" %m", option_id, + val64); + } + trace_iommufd_backend_set_option(fd, object_id, option_id, val64, ret); + + return ret; +} + int iommufd_backend_alloc_ioas(IOMMUFDBackend *be, uint32_t *ioas_id, Error **errp) { diff --git a/backends/trace-events b/backends/trace-events index 11a27cb114b6..076166552881 100644 --- a/backends/trace-events +++ b/backends/trace-events @@ -15,6 +15,7 @@ iommufd_backend_unmap_dma_non_exist(int iommufd, uint32_t ioas, uint64_t iova, u iommufd_backend_unmap_dma(int iommufd, uint32_t ioas, uint64_t iova, uint64_t size, int ret) " iommufd=%d ioas=%d iova=0x%"PRIx64" size=0x%"PRIx64" (%d)" iommufd_backend_alloc_hwpt(int iommufd, uint32_t dev_id, uint32_t pt_id, uint32_t flags, uint32_t hwpt_type, uint32_t len, uint64_t data_ptr, uint32_t out_hwpt_id, int ret) " iommufd=%d dev_id=%u pt_id=%u flags=0x%x hwpt_type=%u len=%u data_ptr=0x%"PRIx64" out_hwpt=%u (%d)" iommufd_backend_alloc_ioas(int iommufd, uint32_t ioas, int ret) " iommufd=%d ioas=%d (%d)" +iommufd_backend_set_option(int iommufd, uint32_t object_id, uint32_t option_id, uint64_t val, int ret) " iommufd=%d object_id=%u option_id=%u val64=0x%"PRIx64" (%d)" iommufd_backend_free_id(int iommufd, uint32_t id, int ret) " iommufd=%d id=%d (%d)" iommufd_backend_set_dirty(int iommufd, uint32_t hwpt_id, bool start, int ret) " iommufd=%d hwpt=%d enable=%d (%d)" iommufd_backend_get_dirty_bitmap(int iommufd, uint32_t hwpt_id, uint64_t iova, uint64_t size, uint64_t page_size, int ret) " iommufd=%d hwpt=%d iova=0x%"PRIx64" size=0x%"PRIx64" page_size=0x%"PRIx64" (%d)" diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c index 79b13bd262cc..697d40841d7f 100644 --- a/hw/vfio/iommufd.c +++ b/hw/vfio/iommufd.c @@ -521,6 +521,10 @@ static int iommufd_cdev_attach(const char *name, VFIODevice *vbasedev, goto err_alloc_ioas; } + if (!vbasedev->iommufd_dev.iommufd->hugepages) { + iommufd_backend_set_option(vbasedev->iommufd_dev.iommufd->fd, ioas_id, + IOMMU_OPTION_HUGE_PAGES, 0); + } trace_iommufd_cdev_alloc_ioas(vbasedev->iommufd_dev.iommufd->fd, ioas_id); container = g_malloc0(sizeof(*container)); diff --git a/include/sysemu/iommufd.h b/include/sysemu/iommufd.h index ba19b7ea4c19..bc6607e3d444 100644 --- a/include/sysemu/iommufd.h +++ b/include/sysemu/iommufd.h @@ -19,6 +19,7 @@ struct IOMMUFDBackend { /*< protected >*/ int fd; /* /dev/iommu file descriptor */ bool owned; /* is the /dev/iommu opened internally */ + bool hugepages; /* are hugepages enabled on the IOAS */ uint32_t users; /*< public >*/ @@ -30,6 +31,9 @@ void iommufd_backend_disconnect(IOMMUFDBackend *be); int iommufd_backend_alloc_ioas(IOMMUFDBackend *be, uint32_t *ioas_id, Error **errp); void iommufd_backend_free_id(IOMMUFDBackend *be, uint32_t id); +int iommufd_backend_set_option(int fd, uint32_t object_id, + uint32_t option_id, + uint64_t val64); int iommufd_backend_map_dma(IOMMUFDBackend *be, uint32_t ioas_id, hwaddr iova, ram_addr_t size, void *vaddr, bool readonly); int iommufd_backend_unmap_dma(IOMMUFDBackend *be, uint32_t ioas_id, diff --git a/qapi/qom.json b/qapi/qom.json index 84af23fe245d..9ad27e2b939b 100644 --- a/qapi/qom.json +++ b/qapi/qom.json @@ -809,7 +809,7 @@ # Since: 9.0 ## { 'struct': 'IOMMUFDProperties', - 'data': { '*fd': 'str' } } + 'data': { '*fd': 'str', '*hugepages': 'bool' } } ## # @RngProperties:
Allow disabling hugepages to be dirty track at base page granularity in similar vein to vfio_type1_iommu.disable_hugepages but per IOAS. Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- backends/iommufd.c | 36 ++++++++++++++++++++++++++++++++++++ backends/trace-events | 1 + hw/vfio/iommufd.c | 4 ++++ include/sysemu/iommufd.h | 4 ++++ qapi/qom.json | 2 +- 5 files changed, 46 insertions(+), 1 deletion(-)