| Message ID | 20240603061023.269738-8-zhenzhong.duan@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add a host IOMMU device abstraction to check with vIOMMU | expand |
Hi Zhenzhong, On 6/3/24 08:10, Zhenzhong Duan wrote: > Utilize range_get_last_bit() to get host IOMMU address width and > package it in HostIOMMUDeviceCaps for query with .get_cap(). > > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> > --- > hw/vfio/container.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/hw/vfio/container.c b/hw/vfio/container.c > index c4fca2dfca..48800fe92f 100644 > --- a/hw/vfio/container.c > +++ b/hw/vfio/container.c > @@ -1136,6 +1136,31 @@ static void vfio_iommu_legacy_class_init(ObjectClass *klass, void *data) > vioc->pci_hot_reset = vfio_legacy_pci_hot_reset; > }; > > +static bool hiod_legacy_vfio_realize(HostIOMMUDevice *hiod, void *opaque, > + Error **errp) > +{ > + VFIODevice *vdev = opaque; > + /* iova_ranges is a sorted list */ > + GList *l = g_list_last(vdev->bcontainer->iova_ranges); > + > + /* Only support query HOST_IOMMU_DEVICE_CAP_AW_BITS with legacy backend */ I don't get the comment as HOST_IOMMU_DEVICE_CAP_AW_BITS support seems to be introduced in [PATCH v6 11/19] backends/iommufd: Implement HostIOMMUDeviceClass::get_cap() handler > + if (l) { > + Range *range = l->data; > + hiod->caps.aw_bits = range_get_last_bit(range) + 1; > + } else { > + hiod->caps.aw_bits = 0xff; why this value? > + } > + > + return true; > +} > + > +static void hiod_legacy_vfio_class_init(ObjectClass *oc, void *data) > +{ > + HostIOMMUDeviceClass *hioc = HOST_IOMMU_DEVICE_CLASS(oc); > + > + hioc->realize = hiod_legacy_vfio_realize; > +}; > + > static const TypeInfo types[] = { > { > .name = TYPE_VFIO_IOMMU_LEGACY, > @@ -1144,6 +1169,7 @@ static const TypeInfo types[] = { > }, { > .name = TYPE_HOST_IOMMU_DEVICE_LEGACY_VFIO, > .parent = TYPE_HOST_IOMMU_DEVICE, > + .class_init = hiod_legacy_vfio_class_init, > } > }; > Thanks Eric
Hi Eric, >-----Original Message----- >From: Eric Auger <eric.auger@redhat.com> >Subject: Re: [PATCH v6 07/19] vfio/container: Implement >HostIOMMUDeviceClass::realize() handler > >Hi Zhenzhong, > >On 6/3/24 08:10, Zhenzhong Duan wrote: >> Utilize range_get_last_bit() to get host IOMMU address width and >> package it in HostIOMMUDeviceCaps for query with .get_cap(). >> >> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> >> --- >> hw/vfio/container.c | 26 ++++++++++++++++++++++++++ >> 1 file changed, 26 insertions(+) >> >> diff --git a/hw/vfio/container.c b/hw/vfio/container.c >> index c4fca2dfca..48800fe92f 100644 >> --- a/hw/vfio/container.c >> +++ b/hw/vfio/container.c >> @@ -1136,6 +1136,31 @@ static void >vfio_iommu_legacy_class_init(ObjectClass *klass, void *data) >> vioc->pci_hot_reset = vfio_legacy_pci_hot_reset; >> }; >> >> +static bool hiod_legacy_vfio_realize(HostIOMMUDevice *hiod, void >*opaque, >> + Error **errp) >> +{ >> + VFIODevice *vdev = opaque; >> + /* iova_ranges is a sorted list */ >> + GList *l = g_list_last(vdev->bcontainer->iova_ranges); >> + >> + /* Only support query HOST_IOMMU_DEVICE_CAP_AW_BITS with >legacy backend */ >I don't get the comment as HOST_IOMMU_DEVICE_CAP_AW_BITS support >seems >to be introduced in [PATCH v6 11/19] backends/iommufd: Implement >HostIOMMUDeviceClass::get_cap() handler Sorry about my poor English, I mean legacy backend only support HOST_IOMMU_DEVICE_CAP_AW_BITS, no other caps. May be only: /* Only support query HOST_IOMMU_DEVICE_CAP_AW_BITS */ >> + if (l) { >> + Range *range = l->data; >> + hiod->caps.aw_bits = range_get_last_bit(range) + 1; >> + } else { >> + hiod->caps.aw_bits = 0xff; >why this value? 0xff means no limitation on aw_bits from host side. Aw_bits check should always pass. This could be a case that an old kernel doesn't support query iova ranges. Will add a define like: #define HOST_IOMMU_DEVICE_CAP_AW_BITS_MAX 0xff Thanks Zhenzhong >> + } >> + >> + return true; >> +} >> + >> +static void hiod_legacy_vfio_class_init(ObjectClass *oc, void *data) >> +{ >> + HostIOMMUDeviceClass *hioc = HOST_IOMMU_DEVICE_CLASS(oc); >> + >> + hioc->realize = hiod_legacy_vfio_realize; >> +}; >> + >> static const TypeInfo types[] = { >> { >> .name = TYPE_VFIO_IOMMU_LEGACY, >> @@ -1144,6 +1169,7 @@ static const TypeInfo types[] = { >> }, { >> .name = TYPE_HOST_IOMMU_DEVICE_LEGACY_VFIO, >> .parent = TYPE_HOST_IOMMU_DEVICE, >> + .class_init = hiod_legacy_vfio_class_init, >> } >> }; >> > >Thanks > >Eric
Hi Zhenzhong, On 6/4/24 04:45, Duan, Zhenzhong wrote: > Hi Eric, > >> -----Original Message----- >> From: Eric Auger <eric.auger@redhat.com> >> Subject: Re: [PATCH v6 07/19] vfio/container: Implement >> HostIOMMUDeviceClass::realize() handler >> >> Hi Zhenzhong, >> >> On 6/3/24 08:10, Zhenzhong Duan wrote: >>> Utilize range_get_last_bit() to get host IOMMU address width and >>> package it in HostIOMMUDeviceCaps for query with .get_cap(). >>> >>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> >>> --- >>> hw/vfio/container.c | 26 ++++++++++++++++++++++++++ >>> 1 file changed, 26 insertions(+) >>> >>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c >>> index c4fca2dfca..48800fe92f 100644 >>> --- a/hw/vfio/container.c >>> +++ b/hw/vfio/container.c >>> @@ -1136,6 +1136,31 @@ static void >> vfio_iommu_legacy_class_init(ObjectClass *klass, void *data) >>> vioc->pci_hot_reset = vfio_legacy_pci_hot_reset; >>> }; >>> >>> +static bool hiod_legacy_vfio_realize(HostIOMMUDevice *hiod, void >> *opaque, >>> + Error **errp) >>> +{ >>> + VFIODevice *vdev = opaque; >>> + /* iova_ranges is a sorted list */ >>> + GList *l = g_list_last(vdev->bcontainer->iova_ranges); >>> + >>> + /* Only support query HOST_IOMMU_DEVICE_CAP_AW_BITS with >> legacy backend */ >> I don't get the comment as HOST_IOMMU_DEVICE_CAP_AW_BITS support >> seems >> to be introduced in [PATCH v6 11/19] backends/iommufd: Implement >> HostIOMMUDeviceClass::get_cap() handler > Sorry about my poor English, I mean legacy backend only support > HOST_IOMMU_DEVICE_CAP_AW_BITS, no other caps. > May be only: > > /* Only support query HOST_IOMMU_DEVICE_CAP_AW_BITS */ no problem. Then I would put this comment in the commit msg instead. Something like "the realize function populates the capabilities. For now only the aw_bits caps is computed". > >>> + if (l) { >>> + Range *range = l->data; >>> + hiod->caps.aw_bits = range_get_last_bit(range) + 1; >>> + } else { >>> + hiod->caps.aw_bits = 0xff; >> why this value? > 0xff means no limitation on aw_bits from host side. Aw_bits check > should always pass. This could be a case that an old kernel doesn't > support query iova ranges. > > Will add a define like: > > #define HOST_IOMMU_DEVICE_CAP_AW_BITS_MAX 0xff Wouldn't 64 bits be a better choice? Also maybe add a comment explaining that iova_ranges may be void for old kernels that do not support the query? Eric > > Thanks > Zhenzhong > >>> + } >>> + >>> + return true; >>> +} >>> + >>> +static void hiod_legacy_vfio_class_init(ObjectClass *oc, void *data) >>> +{ >>> + HostIOMMUDeviceClass *hioc = HOST_IOMMU_DEVICE_CLASS(oc); >>> + >>> + hioc->realize = hiod_legacy_vfio_realize; >>> +}; >>> + >>> static const TypeInfo types[] = { >>> { >>> .name = TYPE_VFIO_IOMMU_LEGACY, >>> @@ -1144,6 +1169,7 @@ static const TypeInfo types[] = { >>> }, { >>> .name = TYPE_HOST_IOMMU_DEVICE_LEGACY_VFIO, >>> .parent = TYPE_HOST_IOMMU_DEVICE, >>> + .class_init = hiod_legacy_vfio_class_init, >>> } >>> }; >>> >> Thanks >> >> Eric
>-----Original Message----- >From: Eric Auger <eric.auger@redhat.com> >Subject: Re: [PATCH v6 07/19] vfio/container: Implement >HostIOMMUDeviceClass::realize() handler > >Hi Zhenzhong, > >On 6/4/24 04:45, Duan, Zhenzhong wrote: >> Hi Eric, >> >>> -----Original Message----- >>> From: Eric Auger <eric.auger@redhat.com> >>> Subject: Re: [PATCH v6 07/19] vfio/container: Implement >>> HostIOMMUDeviceClass::realize() handler >>> >>> Hi Zhenzhong, >>> >>> On 6/3/24 08:10, Zhenzhong Duan wrote: >>>> Utilize range_get_last_bit() to get host IOMMU address width and >>>> package it in HostIOMMUDeviceCaps for query with .get_cap(). >>>> >>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> >>>> --- >>>> hw/vfio/container.c | 26 ++++++++++++++++++++++++++ >>>> 1 file changed, 26 insertions(+) >>>> >>>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c >>>> index c4fca2dfca..48800fe92f 100644 >>>> --- a/hw/vfio/container.c >>>> +++ b/hw/vfio/container.c >>>> @@ -1136,6 +1136,31 @@ static void >>> vfio_iommu_legacy_class_init(ObjectClass *klass, void *data) >>>> vioc->pci_hot_reset = vfio_legacy_pci_hot_reset; >>>> }; >>>> >>>> +static bool hiod_legacy_vfio_realize(HostIOMMUDevice *hiod, void >>> *opaque, >>>> + Error **errp) >>>> +{ >>>> + VFIODevice *vdev = opaque; >>>> + /* iova_ranges is a sorted list */ >>>> + GList *l = g_list_last(vdev->bcontainer->iova_ranges); >>>> + >>>> + /* Only support query HOST_IOMMU_DEVICE_CAP_AW_BITS with >>> legacy backend */ >>> I don't get the comment as HOST_IOMMU_DEVICE_CAP_AW_BITS >support >>> seems >>> to be introduced in [PATCH v6 11/19] backends/iommufd: Implement >>> HostIOMMUDeviceClass::get_cap() handler >> Sorry about my poor English, I mean legacy backend only support >> HOST_IOMMU_DEVICE_CAP_AW_BITS, no other caps. >> May be only: >> >> /* Only support query HOST_IOMMU_DEVICE_CAP_AW_BITS */ >no problem. Then I would put this comment in the commit msg instead. >Something like "the realize function populates the capabilities. For now >only the aw_bits caps is computed". Sure. > > >> >>>> + if (l) { >>>> + Range *range = l->data; >>>> + hiod->caps.aw_bits = range_get_last_bit(range) + 1; >>>> + } else { >>>> + hiod->caps.aw_bits = 0xff; >>> why this value? >> 0xff means no limitation on aw_bits from host side. Aw_bits check >> should always pass. This could be a case that an old kernel doesn't >> support query iova ranges. >> >> Will add a define like: >> >> #define HOST_IOMMU_DEVICE_CAP_AW_BITS_MAX 0xff >Wouldn't 64 bits be a better choice? Yes, 64 bits is large enough, will it. > Also maybe add a comment explaining >that iova_ranges may be void for old kernels that do not support the query? Sure. Thanks Zhenzhong
diff --git a/hw/vfio/container.c b/hw/vfio/container.c index c4fca2dfca..48800fe92f 100644 --- a/hw/vfio/container.c +++ b/hw/vfio/container.c @@ -1136,6 +1136,31 @@ static void vfio_iommu_legacy_class_init(ObjectClass *klass, void *data) vioc->pci_hot_reset = vfio_legacy_pci_hot_reset; }; +static bool hiod_legacy_vfio_realize(HostIOMMUDevice *hiod, void *opaque, + Error **errp) +{ + VFIODevice *vdev = opaque; + /* iova_ranges is a sorted list */ + GList *l = g_list_last(vdev->bcontainer->iova_ranges); + + /* Only support query HOST_IOMMU_DEVICE_CAP_AW_BITS with legacy backend */ + if (l) { + Range *range = l->data; + hiod->caps.aw_bits = range_get_last_bit(range) + 1; + } else { + hiod->caps.aw_bits = 0xff; + } + + return true; +} + +static void hiod_legacy_vfio_class_init(ObjectClass *oc, void *data) +{ + HostIOMMUDeviceClass *hioc = HOST_IOMMU_DEVICE_CLASS(oc); + + hioc->realize = hiod_legacy_vfio_realize; +}; + static const TypeInfo types[] = { { .name = TYPE_VFIO_IOMMU_LEGACY, @@ -1144,6 +1169,7 @@ static const TypeInfo types[] = { }, { .name = TYPE_HOST_IOMMU_DEVICE_LEGACY_VFIO, .parent = TYPE_HOST_IOMMU_DEVICE, + .class_init = hiod_legacy_vfio_class_init, } };
Utilize range_get_last_bit() to get host IOMMU address width and package it in HostIOMMUDeviceCaps for query with .get_cap(). Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> --- hw/vfio/container.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)