[PATCH] iommu/arm-smmu-v3: Improve uAPI comment for IOMMU_HW_INFO_TYPE_ARM_SMMUV3

[PATCH] iommu/arm-smmu-v3: Improve uAPI comment for IOMMU_HW_INFO_TYPE_ARM_SMMUV3

[Jason Gunthorpe]

Be specific about what fields should be accessed in the idr result and
give other guidance to the VMM on how it should generate the
vIDR. Discussion on the list, and review of the qmeu implementation
understood this needs to be clearer and more detailed.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 include/uapi/linux/iommufd.h | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

As posted in the other thread, it is a enough of an improvement I'd like to
pick it up for the iommufd PR.

Thanks,
Jason

diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
index 747d3d9baa3dda..1e0345a62a3ab8 100644
--- a/include/uapi/linux/iommufd.h
+++ b/include/uapi/linux/iommufd.h
@@ -549,16 +549,25 @@ struct iommu_hw_info_vtd {
  * For the details of @idr, @iidr and @aidr, please refer to the chapters
  * from 6.3.1 to 6.3.6 in the SMMUv3 Spec.
  *
- * User space should read the underlying ARM SMMUv3 hardware information for
- * the list of supported features.
+ * This reports the raw HW capability, and not all bits are meaningful to be
+ * read by userspace. Only the following fields should be used:
  *
- * Note that these values reflect the raw HW capability, without any insight if
- * any required kernel driver support is present. Bits may be set indicating the
- * HW has functionality that is lacking kernel software support, such as BTM. If
- * a VMM is using this information to construct emulated copies of these
- * registers it should only forward bits that it knows it can support.
+ * idr[0]: ST_LEVEL, TERM_MODEL, STALL_MODEL, TTENDIAN , CD2L, ASID16, TTF
+ * idr[1]: SIDSIZE, SSIDSIZE
+ * idr[3]: BBML, RIL
+ * idr[5]: VAX, GRAN64K, GRAN16K, GRAN4K
  *
- * In future, presence of required kernel support will be indicated in flags.
+ * - S1P should be assumed to be true if a NESTED HWPT can be created
+ * - VFIO/iommufd only support platforms with COHACC, it should be assumed to be
+ *   true.
+ * - ATS is a per-device property. If the VMM describes any devices as ATS
+ *   capable in ACPI/DT it should set the corresponding idr.
+ *
+ * This list may expand in future (eg E0PD, AIE, PBHA, D128, DS etc). It is
+ * important that VMMs do not read bits outside the list to allow for
+ * compatibility with future kernels. Several features in the SMMUv3
+ * architecture are not currently supported by the kernel for nesting: HTTU,
+ * BTM, MPAM and others.
  */
 struct iommu_hw_info_arm_smmuv3 {
 	__u32 flags;

base-commit: d68beb276ba26cec47350a6d468e967673ee0c56
-- 
2.43.0

RE: [PATCH] iommu/arm-smmu-v3: Improve uAPI comment for IOMMU_HW_INFO_TYPE_ARM_SMMUV3

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Wednesday, November 13, 2024 2:52 AM
> 
> Be specific about what fields should be accessed in the idr result and
> give other guidance to the VMM on how it should generate the
> vIDR. Discussion on the list, and review of the qmeu implementation

s/qmeu/qemu/

> understood this needs to be clearer and more detailed.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  include/uapi/linux/iommufd.h | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> As posted in the other thread, it is a enough of an improvement I'd like to
> pick it up for the iommufd PR.
> 
> Thanks,
> Jason
> 
> diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
> index 747d3d9baa3dda..1e0345a62a3ab8 100644
> --- a/include/uapi/linux/iommufd.h
> +++ b/include/uapi/linux/iommufd.h
> @@ -549,16 +549,25 @@ struct iommu_hw_info_vtd {
>   * For the details of @idr, @iidr and @aidr, please refer to the chapters
>   * from 6.3.1 to 6.3.6 in the SMMUv3 Spec.
>   *
> - * User space should read the underlying ARM SMMUv3 hardware
> information for
> - * the list of supported features.
> + * This reports the raw HW capability, and not all bits are meaningful to be
> + * read by userspace. Only the following fields should be used:
>   *
> - * Note that these values reflect the raw HW capability, without any insight if
> - * any required kernel driver support is present. Bits may be set indicating
> the
> - * HW has functionality that is lacking kernel software support, such as BTM.
> If
> - * a VMM is using this information to construct emulated copies of these
> - * registers it should only forward bits that it knows it can support.
> + * idr[0]: ST_LEVEL, TERM_MODEL, STALL_MODEL, TTENDIAN , CD2L,
> ASID16, TTF
> + * idr[1]: SIDSIZE, SSIDSIZE
> + * idr[3]: BBML, RIL
> + * idr[5]: VAX, GRAN64K, GRAN16K, GRAN4K
>   *
> - * In future, presence of required kernel support will be indicated in flags.

probably add a umbrella sentence to mark out following features not
reported via IDRs

> + * - S1P should be assumed to be true if a NESTED HWPT can be created
> + * - VFIO/iommufd only support platforms with COHACC, it should be
> assumed to be
> + *   true.
> + * - ATS is a per-device property. If the VMM describes any devices as ATS
> + *   capable in ACPI/DT it should set the corresponding idr.
> + *
> + * This list may expand in future (eg E0PD, AIE, PBHA, D128, DS etc). It is
> + * important that VMMs do not read bits outside the list to allow for
> + * compatibility with future kernels. Several features in the SMMUv3
> + * architecture are not currently supported by the kernel for nesting: HTTU,
> + * BTM, MPAM and others.

Not sure why HTTU etc. are remarked separately instead of appending to
the list of E0PD etc. there is a reason for kernel not reporting a certain
feature now, no matter it's because the kernel hasn't supported it or
supported it but just doesn't want to report. Knowing the difference 
doesn't help the business in VMM...

>   */
>  struct iommu_hw_info_arm_smmuv3 {
>  	__u32 flags;
> 
> base-commit: d68beb276ba26cec47350a6d468e967673ee0c56
> --
> 2.43.0

If ARM maintainers are OK with the list of the baseline features:

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

Re: [PATCH] iommu/arm-smmu-v3: Improve uAPI comment for IOMMU_HW_INFO_TYPE_ARM_SMMUV3

[Jason Gunthorpe]

On Fri, Nov 15, 2024 at 08:27:12AM +0000, Tian, Kevin wrote:
> > - * a VMM is using this information to construct emulated copies of these
> > - * registers it should only forward bits that it knows it can support.
> > + * idr[0]: ST_LEVEL, TERM_MODEL, STALL_MODEL, TTENDIAN , CD2L,
> > ASID16, TTF
> > + * idr[1]: SIDSIZE, SSIDSIZE
> > + * idr[3]: BBML, RIL
> > + * idr[5]: VAX, GRAN64K, GRAN16K, GRAN4K
> >   *
> > - * In future, presence of required kernel support will be indicated in flags.
> 
> probably add a umbrella sentence to mark out following features not
> reported via IDRs

 *
 * The following bits in the vIDR should be obtained from other sources:
 * - S1P should be assumed to be true if a NESTED HWPT can be created

> 
> > + * - S1P should be assumed to be true if a NESTED HWPT can be created
> > + * - VFIO/iommufd only support platforms with COHACC, it should be
> > assumed to be
> > + *   true.
> > + * - ATS is a per-device property. If the VMM describes any devices as ATS
> > + *   capable in ACPI/DT it should set the corresponding idr.
> > + *
> > + * This list may expand in future (eg E0PD, AIE, PBHA, D128, DS etc). It is
> > + * important that VMMs do not read bits outside the list to allow for
> > + * compatibility with future kernels. Several features in the SMMUv3
> > + * architecture are not currently supported by the kernel for nesting: HTTU,
> > + * BTM, MPAM and others.
> 
> Not sure why HTTU etc. are remarked separately instead of appending to
> the list of E0PD etc. 

We know HTTU requires a kernel change, E0PD I think won't.

> there is a reason for kernel not reporting a certain feature now, no
> matter it's because the kernel hasn't supported it or supported it
> but just doesn't want to report. Knowing the difference doesn't help
> the business in VMM...

Right, it doesn't help the VMM, this is more guidance for future
people that might want to enable these things.

Thanks,
Jason

Re: [PATCH] iommu/arm-smmu-v3: Improve uAPI comment for IOMMU_HW_INFO_TYPE_ARM_SMMUV3

[Jason Gunthorpe]

On Tue, Nov 12, 2024 at 02:51:36PM -0400, Jason Gunthorpe wrote:
> Be specific about what fields should be accessed in the idr result and
> give other guidance to the VMM on how it should generate the
> vIDR. Discussion on the list, and review of the qmeu implementation
> understood this needs to be clearer and more detailed.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  include/uapi/linux/iommufd.h | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)

Applied to for-rc

Thanks,
Jason