| Message ID | 1595917664-33276-3-git-send-email-yi.l.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio: expose virtual Shared Virtual Addressing to VMs | expand |
Yi, On 7/28/20 8:27 AM, Liu Yi L wrote: > IOMMUs that support nesting translation needs report the capability info s/needs/need to > to userspace. It gives information about requirements the userspace needs > to implement plus other features characterizing the physical implementation. > > This patch reports nesting info by DOMAIN_ATTR_NESTING. Caller can get > nesting info after setting DOMAIN_ATTR_NESTING. For VFIO, it is after > selecting VFIO_TYPE1_NESTING_IOMMU. This is not what this patch does ;-) It introduces a new IOMMU UAPI struct that gives information about the nesting capabilities and features. This struct is supposed to be returned by iommu_domain_get_attr() with DOMAIN_ATTR_NESTING attribute parameter, one a domain whose type has been set to DOMAIN_ATTR_NESTING. > > Cc: Kevin Tian <kevin.tian@intel.com> > CC: Jacob Pan <jacob.jun.pan@linux.intel.com> > Cc: Alex Williamson <alex.williamson@redhat.com> > Cc: Eric Auger <eric.auger@redhat.com> > Cc: Jean-Philippe Brucker <jean-philippe@linaro.org> > Cc: Joerg Roedel <joro@8bytes.org> > Cc: Lu Baolu <baolu.lu@linux.intel.com> > Signed-off-by: Liu Yi L <yi.l.liu@intel.com> > Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com> > --- > v5 -> v6: > *) rephrase the feature notes per comments from Eric Auger. > *) rename @size of struct iommu_nesting_info to @argsz. > > v4 -> v5: > *) address comments from Eric Auger. > > v3 -> v4: > *) split the SMMU driver changes to be a separate patch > *) move the @addr_width and @pasid_bits from vendor specific > part to generic part. > *) tweak the description for the @features field of struct > iommu_nesting_info. > *) add description on the @data[] field of struct iommu_nesting_info > > v2 -> v3: > *) remvoe cap/ecap_mask in iommu_nesting_info. > *) reuse DOMAIN_ATTR_NESTING to get nesting info. > *) return an empty iommu_nesting_info for SMMU drivers per Jean' > suggestion. > --- > include/uapi/linux/iommu.h | 74 ++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 74 insertions(+) > > diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h > index 7c8e075..5e4745a 100644 > --- a/include/uapi/linux/iommu.h > +++ b/include/uapi/linux/iommu.h > @@ -332,4 +332,78 @@ struct iommu_gpasid_bind_data { > } vendor; > }; > > +/* > + * struct iommu_nesting_info - Information for nesting-capable IOMMU. > + * userspace should check it before using > + * nesting capability. > + * > + * @argsz: size of the whole structure. > + * @flags: currently reserved for future extension. must set to 0. > + * @format: PASID table entry format, the same definition as struct > + * iommu_gpasid_bind_data @format. > + * @features: supported nesting features. > + * @addr_width: The output addr width of first level/stage translation > + * @pasid_bits: Maximum supported PASID bits, 0 represents no PASID > + * support. > + * @data: vendor specific cap info. data[] structure type can be deduced > + * from @format field. > + * > + * +===============+======================================================+ > + * | feature | Notes | > + * +===============+======================================================+ > + * | SYSWIDE_PASID | IOMMU vendor driver sets it to mandate userspace | > + * | | to allocate PASID from kernel. All PASID allocation | > + * | | free must be mediated through the TBD API. | s/TBD/IOMMU > + * +---------------+------------------------------------------------------+ > + * | BIND_PGTBL | IOMMU vendor driver sets it to mandate userspace | > + * | | bind the first level/stage page table to associated | s/bind/to bind > + * | | PASID (either the one specified in bind request or | > + * | | the default PASID of iommu domain), through IOMMU | > + * | | UAPI. | > + * +---------------+------------------------------------------------------+ > + * | CACHE_INVLD | IOMMU vendor driver sets it to mandate userspace | > + * | | explicitly invalidate the IOMMU cache through IOMMU | to explicitly > + * | | UAPI according to vendor-specific requirement when | > + * | | changing the 1st level/stage page table. | > + * +---------------+------------------------------------------------------+ > + * > + * @data[] types defined for @format: > + * +================================+=====================================+ > + * | @format | @data[] | > + * +================================+=====================================+ > + * | IOMMU_PASID_FORMAT_INTEL_VTD | struct iommu_nesting_info_vtd | > + * +--------------------------------+-------------------------------------+ > + * > + */ > +struct iommu_nesting_info { > + __u32 argsz; > + __u32 flags; > + __u32 format; > +#define IOMMU_NESTING_FEAT_SYSWIDE_PASID (1 << 0) > +#define IOMMU_NESTING_FEAT_BIND_PGTBL (1 << 1) > +#define IOMMU_NESTING_FEAT_CACHE_INVLD (1 << 2) > + __u32 features; > + __u16 addr_width; > + __u16 pasid_bits; > + __u32 padding; > + __u8 data[]; > +}; As opposed to other IOMMU UAPI structs there is no union member at the end. Also this struct is not documented in [PATCH v7 1/7] docs: IOMMU user API. Shouldn't we align. You may also consider to move this patch in Jacob's series for consistency, thoughts? > + > +/* > + * struct iommu_nesting_info_vtd - Intel VT-d specific nesting info. > + * > + * @flags: VT-d specific flags. Currently reserved for future > + * extension. must be set to 0. > + * @cap_reg: Describe basic capabilities as defined in VT-d capability > + * register. > + * @ecap_reg: Describe the extended capabilities as defined in VT-d > + * extended capability register. > + */ > +struct iommu_nesting_info_vtd { > + __u32 flags; > + __u32 padding; > + __u64 cap_reg; > + __u64 ecap_reg; > +}; > + > #endif /* _UAPI_IOMMU_H */ > Thanks Eric
Hi Eric, > From: Auger Eric <eric.auger@redhat.com> > Sent: Thursday, August 13, 2020 8:53 PM > > Yi, > On 7/28/20 8:27 AM, Liu Yi L wrote: > > IOMMUs that support nesting translation needs report the capability info > s/needs/need to > > to userspace. It gives information about requirements the userspace needs > > to implement plus other features characterizing the physical implementation. > > > > This patch reports nesting info by DOMAIN_ATTR_NESTING. Caller can get > > nesting info after setting DOMAIN_ATTR_NESTING. For VFIO, it is after > > selecting VFIO_TYPE1_NESTING_IOMMU. > This is not what this patch does ;-) It introduces a new IOMMU UAPI > struct that gives information about the nesting capabilities and > features. This struct is supposed to be returned by > iommu_domain_get_attr() with DOMAIN_ATTR_NESTING attribute parameter, > one a domain whose type has been set to DOMAIN_ATTR_NESTING. got it. let me apply your suggestion. thanks. :-) > > > > Cc: Kevin Tian <kevin.tian@intel.com> > > CC: Jacob Pan <jacob.jun.pan@linux.intel.com> > > Cc: Alex Williamson <alex.williamson@redhat.com> > > Cc: Eric Auger <eric.auger@redhat.com> > > Cc: Jean-Philippe Brucker <jean-philippe@linaro.org> > > Cc: Joerg Roedel <joro@8bytes.org> > > Cc: Lu Baolu <baolu.lu@linux.intel.com> > > Signed-off-by: Liu Yi L <yi.l.liu@intel.com> > > Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com> > > --- > > v5 -> v6: > > *) rephrase the feature notes per comments from Eric Auger. > > *) rename @size of struct iommu_nesting_info to @argsz. > > > > v4 -> v5: > > *) address comments from Eric Auger. > > > > v3 -> v4: > > *) split the SMMU driver changes to be a separate patch > > *) move the @addr_width and @pasid_bits from vendor specific > > part to generic part. > > *) tweak the description for the @features field of struct > > iommu_nesting_info. > > *) add description on the @data[] field of struct iommu_nesting_info > > > > v2 -> v3: > > *) remvoe cap/ecap_mask in iommu_nesting_info. > > *) reuse DOMAIN_ATTR_NESTING to get nesting info. > > *) return an empty iommu_nesting_info for SMMU drivers per Jean' > > suggestion. > > --- > > include/uapi/linux/iommu.h | 74 > ++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 74 insertions(+) > > > > diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h > > index 7c8e075..5e4745a 100644 > > --- a/include/uapi/linux/iommu.h > > +++ b/include/uapi/linux/iommu.h > > @@ -332,4 +332,78 @@ struct iommu_gpasid_bind_data { > > } vendor; > > }; > > > > +/* > > + * struct iommu_nesting_info - Information for nesting-capable IOMMU. > > + * userspace should check it before using > > + * nesting capability. > > + * > > + * @argsz: size of the whole structure. > > + * @flags: currently reserved for future extension. must set to 0. > > + * @format: PASID table entry format, the same definition as struct > > + * iommu_gpasid_bind_data @format. > > + * @features: supported nesting features. > > + * @addr_width: The output addr width of first level/stage translation > > + * @pasid_bits: Maximum supported PASID bits, 0 represents no PASID > > + * support. > > + * @data: vendor specific cap info. data[] structure type can be deduced > > + * from @format field. > > + * > > + * > +===============+=================================================== > ===+ > > + * | feature | Notes | > > + * > +===============+=================================================== > ===+ > > + * | SYSWIDE_PASID | IOMMU vendor driver sets it to mandate userspace | > > + * | | to allocate PASID from kernel. All PASID allocation | > > + * | | free must be mediated through the TBD API. | > s/TBD/IOMMU got it. > > + * +---------------+------------------------------------------------------+ > > + * | BIND_PGTBL | IOMMU vendor driver sets it to mandate userspace | > > + * | | bind the first level/stage page table to associated | > s/bind/to bind got it. > > + * | | PASID (either the one specified in bind request or | > > + * | | the default PASID of iommu domain), through IOMMU | > > + * | | UAPI. | > > + * +---------------+------------------------------------------------------+ > > + * | CACHE_INVLD | IOMMU vendor driver sets it to mandate userspace | > > > + * | | explicitly invalidate the IOMMU cache through IOMMU | > to explicitly I see. > > + * | | U > > API according to vendor-specific requirement when | > > + * | | changing the 1st level/stage page table. | > > + * +---------------+------------------------------------------------------+ > > + * > > + * @data[] types defined for @format: > > + * > +================================+================================== > ===+ > > + * | @format | @data[] | > > + * > +================================+================================== > ===+ > > + * | IOMMU_PASID_FORMAT_INTEL_VTD | struct iommu_nesting_info_vtd | > > + * +--------------------------------+-------------------------------------+ > > + * > > + */ > > +struct iommu_nesting_info { > > + __u32 argsz; > > + __u32 flags; > > + __u32 format; > > +#define IOMMU_NESTING_FEAT_SYSWIDE_PASID (1 << 0) > > +#define IOMMU_NESTING_FEAT_BIND_PGTBL (1 << 1) > > +#define IOMMU_NESTING_FEAT_CACHE_INVLD (1 << 2) > > + __u32 features; > > + __u16 addr_width; > > + __u16 pasid_bits; > > + __u32 padding; > > + __u8 data[]; > > +}; > As opposed to other IOMMU UAPI structs there is no union member at the > end. nice catch. do you think it would be better to adding a union and put the struct iommu_nesting_info_vtd in it? > Also this struct is not documented in [PATCH v7 1/7] docs: IOMMU > user API. Shouldn't we align. > You may also consider to move this patch in Jacob's series for > consistency, thoughts? this was talked one time between Jacob and me. It was put in this series as the major user of nesting_info is in this series. e.g. vfio checks the SYSWIDE_PASID. but I'm open to merge it with Jacob's series if it would make the merge easier. Thanks, Yi Liu > > + > > +/* > > + * struct iommu_nesting_info_vtd - Intel VT-d specific nesting info. > > + * > > + * @flags: VT-d specific flags. Currently reserved for future > > + * extension. must be set to 0. > > + * @cap_reg: Describe basic capabilities as defined in VT-d capability > > + * register. > > + * @ecap_reg: Describe the extended capabilities as defined in VT-d > > + * extended capability register. > > + */ > > +struct iommu_nesting_info_vtd { > > + __u32 flags; > > + __u32 padding; > > + __u64 cap_reg; > > + __u64 ecap_reg; > > +}; > > + > > #endif /* _UAPI_IOMMU_H */ > > > > Thanks > > Eric >
Hi Yi, On 8/14/20 9:15 AM, Liu, Yi L wrote: > Hi Eric, > >> From: Auger Eric <eric.auger@redhat.com> >> Sent: Thursday, August 13, 2020 8:53 PM >> >> Yi, >> On 7/28/20 8:27 AM, Liu Yi L wrote: >>> IOMMUs that support nesting translation needs report the capability info >> s/needs/need to >>> to userspace. It gives information about requirements the userspace needs >>> to implement plus other features characterizing the physical implementation. >>> >>> This patch reports nesting info by DOMAIN_ATTR_NESTING. Caller can get >>> nesting info after setting DOMAIN_ATTR_NESTING. For VFIO, it is after >>> selecting VFIO_TYPE1_NESTING_IOMMU. >> This is not what this patch does ;-) It introduces a new IOMMU UAPI >> struct that gives information about the nesting capabilities and >> features. This struct is supposed to be returned by >> iommu_domain_get_attr() with DOMAIN_ATTR_NESTING attribute parameter, >> one a domain whose type has been set to DOMAIN_ATTR_NESTING. > > got it. let me apply your suggestion. thanks. :-) > >>> >>> Cc: Kevin Tian <kevin.tian@intel.com> >>> CC: Jacob Pan <jacob.jun.pan@linux.intel.com> >>> Cc: Alex Williamson <alex.williamson@redhat.com> >>> Cc: Eric Auger <eric.auger@redhat.com> >>> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org> >>> Cc: Joerg Roedel <joro@8bytes.org> >>> Cc: Lu Baolu <baolu.lu@linux.intel.com> >>> Signed-off-by: Liu Yi L <yi.l.liu@intel.com> >>> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com> >>> --- >>> v5 -> v6: >>> *) rephrase the feature notes per comments from Eric Auger. >>> *) rename @size of struct iommu_nesting_info to @argsz. >>> >>> v4 -> v5: >>> *) address comments from Eric Auger. >>> >>> v3 -> v4: >>> *) split the SMMU driver changes to be a separate patch >>> *) move the @addr_width and @pasid_bits from vendor specific >>> part to generic part. >>> *) tweak the description for the @features field of struct >>> iommu_nesting_info. >>> *) add description on the @data[] field of struct iommu_nesting_info >>> >>> v2 -> v3: >>> *) remvoe cap/ecap_mask in iommu_nesting_info. >>> *) reuse DOMAIN_ATTR_NESTING to get nesting info. >>> *) return an empty iommu_nesting_info for SMMU drivers per Jean' >>> suggestion. >>> --- >>> include/uapi/linux/iommu.h | 74 >> ++++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 74 insertions(+) >>> >>> diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h >>> index 7c8e075..5e4745a 100644 >>> --- a/include/uapi/linux/iommu.h >>> +++ b/include/uapi/linux/iommu.h >>> @@ -332,4 +332,78 @@ struct iommu_gpasid_bind_data { >>> } vendor; >>> }; >>> >>> +/* >>> + * struct iommu_nesting_info - Information for nesting-capable IOMMU. >>> + * userspace should check it before using >>> + * nesting capability. >>> + * >>> + * @argsz: size of the whole structure. >>> + * @flags: currently reserved for future extension. must set to 0. >>> + * @format: PASID table entry format, the same definition as struct >>> + * iommu_gpasid_bind_data @format. >>> + * @features: supported nesting features. >>> + * @addr_width: The output addr width of first level/stage translation >>> + * @pasid_bits: Maximum supported PASID bits, 0 represents no PASID >>> + * support. >>> + * @data: vendor specific cap info. data[] structure type can be deduced >>> + * from @format field. >>> + * >>> + * >> +===============+=================================================== >> ===+ >>> + * | feature | Notes | >>> + * >> +===============+=================================================== >> ===+ >>> + * | SYSWIDE_PASID | IOMMU vendor driver sets it to mandate userspace | >>> + * | | to allocate PASID from kernel. All PASID allocation | >>> + * | | free must be mediated through the TBD API. | >> s/TBD/IOMMU > > got it. > >>> + * +---------------+------------------------------------------------------+ >>> + * | BIND_PGTBL | IOMMU vendor driver sets it to mandate userspace | >>> + * | | bind the first level/stage page table to associated | >> s/bind/to bind > > got it. > >>> + * | | PASID (either the one specified in bind request or | >>> + * | | the default PASID of iommu domain), through IOMMU | >>> + * | | UAPI. | >>> + * +---------------+------------------------------------------------------+ >>> + * | CACHE_INVLD | IOMMU vendor driver sets it to mandate userspace | >> >>> + * | | explicitly invalidate the IOMMU cache through IOMMU | >> to explicitly > > I see. > >>> + * | | U >>> API according to vendor-specific requirement when | >>> + * | | changing the 1st level/stage page table. | >>> + * +---------------+------------------------------------------------------+ >>> + * >>> + * @data[] types defined for @format: >>> + * >> +================================+================================== >> ===+ >>> + * | @format | @data[] | >>> + * >> +================================+================================== >> ===+ >>> + * | IOMMU_PASID_FORMAT_INTEL_VTD | struct iommu_nesting_info_vtd | >>> + * +--------------------------------+-------------------------------------+ >>> + * >>> + */ >>> +struct iommu_nesting_info { >>> + __u32 argsz; >>> + __u32 flags; >>> + __u32 format; >>> +#define IOMMU_NESTING_FEAT_SYSWIDE_PASID (1 << 0) >>> +#define IOMMU_NESTING_FEAT_BIND_PGTBL (1 << 1) >>> +#define IOMMU_NESTING_FEAT_CACHE_INVLD (1 << 2) >>> + __u32 features; >>> + __u16 addr_width; >>> + __u16 pasid_bits; >>> + __u32 padding; >>> + __u8 data[]; >>> +}; >> As opposed to other IOMMU UAPI structs there is no union member at the >> end. > > nice catch. do you think it would be better to adding a union and > put the struct iommu_nesting_info_vtd in it? Yes I think so. At least it would be consistent with the rest of the API and with the guidelines. > >> Also this struct is not documented in [PATCH v7 1/7] docs: IOMMU >> user API. Shouldn't we align. >> You may also consider to move this patch in Jacob's series for >> consistency, thoughts? > > this was talked one time between Jacob and me. It was put in this > series as the major user of nesting_info is in this series. e.g. > vfio checks the SYSWIDE_PASID. but I'm open to merge it with Jacob's > series if it would make the merge easier. Yep I think it would make sense to move in Jacob's series to have a general understanding of the uapi Thanks Eric > > Thanks, > Yi Liu > >>> + >>> +/* >>> + * struct iommu_nesting_info_vtd - Intel VT-d specific nesting info. >>> + * >>> + * @flags: VT-d specific flags. Currently reserved for future >>> + * extension. must be set to 0. >>> + * @cap_reg: Describe basic capabilities as defined in VT-d capability >>> + * register. >>> + * @ecap_reg: Describe the extended capabilities as defined in VT-d >>> + * extended capability register. >>> + */ >>> +struct iommu_nesting_info_vtd { >>> + __u32 flags; >>> + __u32 padding; >>> + __u64 cap_reg; >>> + __u64 ecap_reg; >>> +}; >>> + >>> #endif /* _UAPI_IOMMU_H */ >>> >> >> Thanks >> >> Eric >> >
On Sun, 16 Aug 2020 14:40:57 +0200 Auger Eric <eric.auger@redhat.com> wrote: > Hi Yi, > > On 8/14/20 9:15 AM, Liu, Yi L wrote: > > Hi Eric, > > > >> From: Auger Eric <eric.auger@redhat.com> > >> Sent: Thursday, August 13, 2020 8:53 PM > >> > >> Yi, > >> On 7/28/20 8:27 AM, Liu Yi L wrote: > >>> IOMMUs that support nesting translation needs report the > >>> capability info > >> s/needs/need to > >>> to userspace. It gives information about requirements the > >>> userspace needs to implement plus other features characterizing > >>> the physical implementation. > >>> > >>> This patch reports nesting info by DOMAIN_ATTR_NESTING. Caller > >>> can get nesting info after setting DOMAIN_ATTR_NESTING. For VFIO, > >>> it is after selecting VFIO_TYPE1_NESTING_IOMMU. > >> This is not what this patch does ;-) It introduces a new IOMMU UAPI > >> struct that gives information about the nesting capabilities and > >> features. This struct is supposed to be returned by > >> iommu_domain_get_attr() with DOMAIN_ATTR_NESTING attribute > >> parameter, one a domain whose type has been set to > >> DOMAIN_ATTR_NESTING. > > > > got it. let me apply your suggestion. thanks. :-) > > > >>> > >>> Cc: Kevin Tian <kevin.tian@intel.com> > >>> CC: Jacob Pan <jacob.jun.pan@linux.intel.com> > >>> Cc: Alex Williamson <alex.williamson@redhat.com> > >>> Cc: Eric Auger <eric.auger@redhat.com> > >>> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org> > >>> Cc: Joerg Roedel <joro@8bytes.org> > >>> Cc: Lu Baolu <baolu.lu@linux.intel.com> > >>> Signed-off-by: Liu Yi L <yi.l.liu@intel.com> > >>> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com> > >>> --- > >>> v5 -> v6: > >>> *) rephrase the feature notes per comments from Eric Auger. > >>> *) rename @size of struct iommu_nesting_info to @argsz. > >>> > >>> v4 -> v5: > >>> *) address comments from Eric Auger. > >>> > >>> v3 -> v4: > >>> *) split the SMMU driver changes to be a separate patch > >>> *) move the @addr_width and @pasid_bits from vendor specific > >>> part to generic part. > >>> *) tweak the description for the @features field of struct > >>> iommu_nesting_info. > >>> *) add description on the @data[] field of struct > >>> iommu_nesting_info > >>> > >>> v2 -> v3: > >>> *) remvoe cap/ecap_mask in iommu_nesting_info. > >>> *) reuse DOMAIN_ATTR_NESTING to get nesting info. > >>> *) return an empty iommu_nesting_info for SMMU drivers per Jean' > >>> suggestion. > >>> --- > >>> include/uapi/linux/iommu.h | 74 > >> ++++++++++++++++++++++++++++++++++++++++++++++ > >>> 1 file changed, 74 insertions(+) > >>> > >>> diff --git a/include/uapi/linux/iommu.h > >>> b/include/uapi/linux/iommu.h index 7c8e075..5e4745a 100644 > >>> --- a/include/uapi/linux/iommu.h > >>> +++ b/include/uapi/linux/iommu.h > >>> @@ -332,4 +332,78 @@ struct iommu_gpasid_bind_data { > >>> } vendor; > >>> }; > >>> > >>> +/* > >>> + * struct iommu_nesting_info - Information for nesting-capable > >>> IOMMU. > >>> + * userspace should check it > >>> before using > >>> + * nesting capability. > >>> + * > >>> + * @argsz: size of the whole structure. > >>> + * @flags: currently reserved for future extension. must > >>> set to 0. > >>> + * @format: PASID table entry format, the same definition > >>> as struct > >>> + * iommu_gpasid_bind_data @format. > >>> + * @features: supported nesting features. > >>> + * @addr_width: The output addr width of first > >>> level/stage translation > >>> + * @pasid_bits: Maximum supported PASID bits, 0 > >>> represents no PASID > >>> + * support. > >>> + * @data: vendor specific cap info. data[] structure type > >>> can be deduced > >>> + * from @format field. > >>> + * > >>> + * > >> +===============+=================================================== > >> ===+ > >>> + * | feature | > >>> Notes | > >>> + * > >> +===============+=================================================== > >> ===+ > >>> + * | SYSWIDE_PASID | IOMMU vendor driver sets it to mandate > >>> userspace | > >>> + * | | to allocate PASID from kernel. All PASID > >>> allocation | > >>> + * | | free must be mediated through the TBD > >>> API. | > >> s/TBD/IOMMU > > > > got it. > > > >>> + * > >>> +---------------+------------------------------------------------------+ > >>> + * | BIND_PGTBL | IOMMU vendor driver sets it to mandate > >>> userspace | > >>> + * | | bind the first level/stage page table to > >>> associated | > >> s/bind/to bind > > > > got it. > > > >>> + * | | PASID (either the one specified in bind > >>> request or | > >>> + * | | the default PASID of iommu domain), > >>> through IOMMU | > >>> + * | | > >>> UAPI. | > >>> + * > >>> +---------------+------------------------------------------------------+ > >>> + * | CACHE_INVLD | IOMMU vendor driver sets it to mandate > >>> userspace | > >> > >>> + * | | explicitly invalidate the IOMMU cache > >>> through IOMMU | > >> to explicitly > > > > I see. > > > >>> + * | | U > >>> API according to vendor-specific requirement when | > >>> + * | | changing the 1st level/stage page > >>> table. | > >>> + * > >>> +---------------+------------------------------------------------------+ > >>> + * > >>> + * @data[] types defined for @format: > >>> + * > >> +================================+================================== > >> ===+ > >>> + * | @format | > >>> @data[] | > >>> + * > >> +================================+================================== > >> ===+ > >>> + * | IOMMU_PASID_FORMAT_INTEL_VTD | struct > >>> iommu_nesting_info_vtd | > >>> + * > >>> +--------------------------------+-------------------------------------+ > >>> + * > >>> + */ > >>> +struct iommu_nesting_info { > >>> + __u32 argsz; > >>> + __u32 flags; > >>> + __u32 format; > >>> +#define IOMMU_NESTING_FEAT_SYSWIDE_PASID (1 << 0) > >>> +#define IOMMU_NESTING_FEAT_BIND_PGTBL (1 << 1) > >>> +#define IOMMU_NESTING_FEAT_CACHE_INVLD (1 << 2) > >>> + __u32 features; > >>> + __u16 addr_width; > >>> + __u16 pasid_bits; > >>> + __u32 padding; > >>> + __u8 data[]; > >>> +}; > >> As opposed to other IOMMU UAPI structs there is no union member at > >> the end. > > > > nice catch. do you think it would be better to adding a union and > > put the struct iommu_nesting_info_vtd in it? > Yes I think so. At least it would be consistent with the rest of the > API and with the guidelines. > > > >> Also this struct is not documented in [PATCH v7 1/7] docs: IOMMU > >> user API. Shouldn't we align. > >> You may also consider to move this patch in Jacob's series for > >> consistency, thoughts? > > > > this was talked one time between Jacob and me. It was put in this > > series as the major user of nesting_info is in this series. e.g. > > vfio checks the SYSWIDE_PASID. but I'm open to merge it with Jacob's > > series if it would make the merge easier. > Yep I think it would make sense to move in Jacob's series to have a > general understanding of the uapi > I a little reluctant to include this in my UAPI set, the reason is that there are two dimensions IOMMU UAPI are extended: 1. Define the protocols in interaction with VFIO, sanity checking, and backward compatibility. 2. Adding more UAPI data structures that are parallel to the existing ones. My patchset is to address #1, this patch is for #2. My thinking is that once we have reached consensus on #1, new UAPI structures such as this patch can just follow the suit. If that is OK with you, I would like to keep them separate to avoid diverging conversations. Thanks, Jacob > Thanks > > Eric > > > > Thanks, > > Yi Liu > > > >>> + > >>> +/* > >>> + * struct iommu_nesting_info_vtd - Intel VT-d specific nesting > >>> info. > >>> + * > >>> + * @flags: VT-d specific flags. Currently reserved for > >>> future > >>> + * extension. must be set to 0. > >>> + * @cap_reg: Describe basic capabilities as defined in > >>> VT-d capability > >>> + * register. > >>> + * @ecap_reg: Describe the extended capabilities as > >>> defined in VT-d > >>> + * extended capability register. > >>> + */ > >>> +struct iommu_nesting_info_vtd { > >>> + __u32 flags; > >>> + __u32 padding; > >>> + __u64 cap_reg; > >>> + __u64 ecap_reg; > >>> +}; > >>> + > >>> #endif /* _UAPI_IOMMU_H */ > >>> > >> > >> Thanks > >> > >> Eric > >> > > > [Jacob Pan]
Hi Jacob, On 8/18/20 6:21 AM, Jacob Pan wrote: > On Sun, 16 Aug 2020 14:40:57 +0200 > Auger Eric <eric.auger@redhat.com> wrote: > >> Hi Yi, >> >> On 8/14/20 9:15 AM, Liu, Yi L wrote: >>> Hi Eric, >>> >>>> From: Auger Eric <eric.auger@redhat.com> >>>> Sent: Thursday, August 13, 2020 8:53 PM >>>> >>>> Yi, >>>> On 7/28/20 8:27 AM, Liu Yi L wrote: >>>>> IOMMUs that support nesting translation needs report the >>>>> capability info >>>> s/needs/need to >>>>> to userspace. It gives information about requirements the >>>>> userspace needs to implement plus other features characterizing >>>>> the physical implementation. >>>>> >>>>> This patch reports nesting info by DOMAIN_ATTR_NESTING. Caller >>>>> can get nesting info after setting DOMAIN_ATTR_NESTING. For VFIO, >>>>> it is after selecting VFIO_TYPE1_NESTING_IOMMU. >>>> This is not what this patch does ;-) It introduces a new IOMMU UAPI >>>> struct that gives information about the nesting capabilities and >>>> features. This struct is supposed to be returned by >>>> iommu_domain_get_attr() with DOMAIN_ATTR_NESTING attribute >>>> parameter, one a domain whose type has been set to >>>> DOMAIN_ATTR_NESTING. >>> >>> got it. let me apply your suggestion. thanks. :-) >>> >>>>> >>>>> Cc: Kevin Tian <kevin.tian@intel.com> >>>>> CC: Jacob Pan <jacob.jun.pan@linux.intel.com> >>>>> Cc: Alex Williamson <alex.williamson@redhat.com> >>>>> Cc: Eric Auger <eric.auger@redhat.com> >>>>> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org> >>>>> Cc: Joerg Roedel <joro@8bytes.org> >>>>> Cc: Lu Baolu <baolu.lu@linux.intel.com> >>>>> Signed-off-by: Liu Yi L <yi.l.liu@intel.com> >>>>> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com> >>>>> --- >>>>> v5 -> v6: >>>>> *) rephrase the feature notes per comments from Eric Auger. >>>>> *) rename @size of struct iommu_nesting_info to @argsz. >>>>> >>>>> v4 -> v5: >>>>> *) address comments from Eric Auger. >>>>> >>>>> v3 -> v4: >>>>> *) split the SMMU driver changes to be a separate patch >>>>> *) move the @addr_width and @pasid_bits from vendor specific >>>>> part to generic part. >>>>> *) tweak the description for the @features field of struct >>>>> iommu_nesting_info. >>>>> *) add description on the @data[] field of struct >>>>> iommu_nesting_info >>>>> >>>>> v2 -> v3: >>>>> *) remvoe cap/ecap_mask in iommu_nesting_info. >>>>> *) reuse DOMAIN_ATTR_NESTING to get nesting info. >>>>> *) return an empty iommu_nesting_info for SMMU drivers per Jean' >>>>> suggestion. >>>>> --- >>>>> include/uapi/linux/iommu.h | 74 >>>> ++++++++++++++++++++++++++++++++++++++++++++++ >>>>> 1 file changed, 74 insertions(+) >>>>> >>>>> diff --git a/include/uapi/linux/iommu.h >>>>> b/include/uapi/linux/iommu.h index 7c8e075..5e4745a 100644 >>>>> --- a/include/uapi/linux/iommu.h >>>>> +++ b/include/uapi/linux/iommu.h >>>>> @@ -332,4 +332,78 @@ struct iommu_gpasid_bind_data { >>>>> } vendor; >>>>> }; >>>>> >>>>> +/* >>>>> + * struct iommu_nesting_info - Information for nesting-capable >>>>> IOMMU. >>>>> + * userspace should check it >>>>> before using >>>>> + * nesting capability. >>>>> + * >>>>> + * @argsz: size of the whole structure. >>>>> + * @flags: currently reserved for future extension. must >>>>> set to 0. >>>>> + * @format: PASID table entry format, the same definition >>>>> as struct >>>>> + * iommu_gpasid_bind_data @format. >>>>> + * @features: supported nesting features. >>>>> + * @addr_width: The output addr width of first >>>>> level/stage translation >>>>> + * @pasid_bits: Maximum supported PASID bits, 0 >>>>> represents no PASID >>>>> + * support. >>>>> + * @data: vendor specific cap info. data[] structure type >>>>> can be deduced >>>>> + * from @format field. >>>>> + * >>>>> + * >>>> +===============+=================================================== >>>> ===+ >>>>> + * | feature | >>>>> Notes | >>>>> + * >>>> +===============+=================================================== >>>> ===+ >>>>> + * | SYSWIDE_PASID | IOMMU vendor driver sets it to mandate >>>>> userspace | >>>>> + * | | to allocate PASID from kernel. All PASID >>>>> allocation | >>>>> + * | | free must be mediated through the TBD >>>>> API. | >>>> s/TBD/IOMMU >>> >>> got it. >>> >>>>> + * >>>>> +---------------+------------------------------------------------------+ >>>>> + * | BIND_PGTBL | IOMMU vendor driver sets it to mandate >>>>> userspace | >>>>> + * | | bind the first level/stage page table to >>>>> associated | >>>> s/bind/to bind >>> >>> got it. >>> >>>>> + * | | PASID (either the one specified in bind >>>>> request or | >>>>> + * | | the default PASID of iommu domain), >>>>> through IOMMU | >>>>> + * | | >>>>> UAPI. | >>>>> + * >>>>> +---------------+------------------------------------------------------+ >>>>> + * | CACHE_INVLD | IOMMU vendor driver sets it to mandate >>>>> userspace | >>>> >>>>> + * | | explicitly invalidate the IOMMU cache >>>>> through IOMMU | >>>> to explicitly >>> >>> I see. >>> >>>>> + * | | U >>>>> API according to vendor-specific requirement when | >>>>> + * | | changing the 1st level/stage page >>>>> table. | >>>>> + * >>>>> +---------------+------------------------------------------------------+ >>>>> + * >>>>> + * @data[] types defined for @format: >>>>> + * >>>> +================================+================================== >>>> ===+ >>>>> + * | @format | >>>>> @data[] | >>>>> + * >>>> +================================+================================== >>>> ===+ >>>>> + * | IOMMU_PASID_FORMAT_INTEL_VTD | struct >>>>> iommu_nesting_info_vtd | >>>>> + * >>>>> +--------------------------------+-------------------------------------+ >>>>> + * >>>>> + */ >>>>> +struct iommu_nesting_info { >>>>> + __u32 argsz; >>>>> + __u32 flags; >>>>> + __u32 format; >>>>> +#define IOMMU_NESTING_FEAT_SYSWIDE_PASID (1 << 0) >>>>> +#define IOMMU_NESTING_FEAT_BIND_PGTBL (1 << 1) >>>>> +#define IOMMU_NESTING_FEAT_CACHE_INVLD (1 << 2) >>>>> + __u32 features; >>>>> + __u16 addr_width; >>>>> + __u16 pasid_bits; >>>>> + __u32 padding; >>>>> + __u8 data[]; >>>>> +}; >>>> As opposed to other IOMMU UAPI structs there is no union member at >>>> the end. >>> >>> nice catch. do you think it would be better to adding a union and >>> put the struct iommu_nesting_info_vtd in it? >> Yes I think so. At least it would be consistent with the rest of the >> API and with the guidelines. >>> >>>> Also this struct is not documented in [PATCH v7 1/7] docs: IOMMU >>>> user API. Shouldn't we align. >>>> You may also consider to move this patch in Jacob's series for >>>> consistency, thoughts? >>> >>> this was talked one time between Jacob and me. It was put in this >>> series as the major user of nesting_info is in this series. e.g. >>> vfio checks the SYSWIDE_PASID. but I'm open to merge it with Jacob's >>> series if it would make the merge easier. >> Yep I think it would make sense to move in Jacob's series to have a >> general understanding of the uapi >> > I a little reluctant to include this in my UAPI set, the reason is that > there are two dimensions IOMMU UAPI are extended: > 1. Define the protocols in interaction with VFIO, sanity checking, and > backward compatibility. > 2. Adding more UAPI data structures that are parallel to the existing > ones. > > My patchset is to address #1, this patch is for #2. My thinking is that > once we have reached consensus on #1, new UAPI structures such as this > patch can just follow the suit. > > If that is OK with you, I would like to keep them separate to avoid > diverging conversations. OK no problem for me, as long as the new APIs follow the rules & guidelines introduced in your series. Thanks Eric > > Thanks, > > Jacob > >> Thanks >> >> Eric >>> >>> Thanks, >>> Yi Liu >>> >>>>> + >>>>> +/* >>>>> + * struct iommu_nesting_info_vtd - Intel VT-d specific nesting >>>>> info. >>>>> + * >>>>> + * @flags: VT-d specific flags. Currently reserved for >>>>> future >>>>> + * extension. must be set to 0. >>>>> + * @cap_reg: Describe basic capabilities as defined in >>>>> VT-d capability >>>>> + * register. >>>>> + * @ecap_reg: Describe the extended capabilities as >>>>> defined in VT-d >>>>> + * extended capability register. >>>>> + */ >>>>> +struct iommu_nesting_info_vtd { >>>>> + __u32 flags; >>>>> + __u32 padding; >>>>> + __u64 cap_reg; >>>>> + __u64 ecap_reg; >>>>> +}; >>>>> + >>>>> #endif /* _UAPI_IOMMU_H */ >>>>> >>>> >>>> Thanks >>>> >>>> Eric >>>> >>> >> > > [Jacob Pan] >
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h index 7c8e075..5e4745a 100644 --- a/include/uapi/linux/iommu.h +++ b/include/uapi/linux/iommu.h @@ -332,4 +332,78 @@ struct iommu_gpasid_bind_data { } vendor; }; +/* + * struct iommu_nesting_info - Information for nesting-capable IOMMU. + * userspace should check it before using + * nesting capability. + * + * @argsz: size of the whole structure. + * @flags: currently reserved for future extension. must set to 0. + * @format: PASID table entry format, the same definition as struct + * iommu_gpasid_bind_data @format. + * @features: supported nesting features. + * @addr_width: The output addr width of first level/stage translation + * @pasid_bits: Maximum supported PASID bits, 0 represents no PASID + * support. + * @data: vendor specific cap info. data[] structure type can be deduced + * from @format field. + * + * +===============+======================================================+ + * | feature | Notes | + * +===============+======================================================+ + * | SYSWIDE_PASID | IOMMU vendor driver sets it to mandate userspace | + * | | to allocate PASID from kernel. All PASID allocation | + * | | free must be mediated through the TBD API. | + * +---------------+------------------------------------------------------+ + * | BIND_PGTBL | IOMMU vendor driver sets it to mandate userspace | + * | | bind the first level/stage page table to associated | + * | | PASID (either the one specified in bind request or | + * | | the default PASID of iommu domain), through IOMMU | + * | | UAPI. | + * +---------------+------------------------------------------------------+ + * | CACHE_INVLD | IOMMU vendor driver sets it to mandate userspace | + * | | explicitly invalidate the IOMMU cache through IOMMU | + * | | UAPI according to vendor-specific requirement when | + * | | changing the 1st level/stage page table. | + * +---------------+------------------------------------------------------+ + * + * @data[] types defined for @format: + * +================================+=====================================+ + * | @format | @data[] | + * +================================+=====================================+ + * | IOMMU_PASID_FORMAT_INTEL_VTD | struct iommu_nesting_info_vtd | + * +--------------------------------+-------------------------------------+ + * + */ +struct iommu_nesting_info { + __u32 argsz; + __u32 flags; + __u32 format; +#define IOMMU_NESTING_FEAT_SYSWIDE_PASID (1 << 0) +#define IOMMU_NESTING_FEAT_BIND_PGTBL (1 << 1) +#define IOMMU_NESTING_FEAT_CACHE_INVLD (1 << 2) + __u32 features; + __u16 addr_width; + __u16 pasid_bits; + __u32 padding; + __u8 data[]; +}; + +/* + * struct iommu_nesting_info_vtd - Intel VT-d specific nesting info. + * + * @flags: VT-d specific flags. Currently reserved for future + * extension. must be set to 0. + * @cap_reg: Describe basic capabilities as defined in VT-d capability + * register. + * @ecap_reg: Describe the extended capabilities as defined in VT-d + * extended capability register. + */ +struct iommu_nesting_info_vtd { + __u32 flags; + __u32 padding; + __u64 cap_reg; + __u64 ecap_reg; +}; + #endif /* _UAPI_IOMMU_H */