Re: [PATCH v5 03/15] iommupt: Add the basic structure of the iommu implementation

[Nicolin Chen <nicolinc@nvidia.com>]

On Wed, Sep 03, 2025 at 02:46:30PM -0300, Jason Gunthorpe wrote:
> The top level struct used by the drivers is pt_iommu_table_FMT. It
> contains the other structs to allow container_of() to move between the
> driver, iommu page table, generic page table, and generic format layers.
> 
>    struct pt_iommu_table_amdv1 {
>        struct pt_iommu {
> 	      struct iommu_domain domain;
>        } iommu;
>        struct pt_amdv1 {
> 	      struct pt_common {
> 	      } common;

According to include/linux/generic_pt/common.h, it should be just:
		struct pt_common common;
?

> +static int pt_iommu_init_domain(struct pt_iommu *iommu_table,
> +				struct iommu_domain *domain)
> +{
...
> +	/*
> +	 * A 64-bit high address space table on a 32-bit system cannot work.
> +	 */

Nit: this fits to a single line.

> +#ifdef pt_iommu_fmt_hw_info
> +#define pt_iommu_table_hw_info CONCATENATE(pt_iommu_table, _hw_info)
> +#define pt_iommu_hw_info CONCATENATE(CONCATENATE(pt_iommu_, PTPFX), hw_info)
> +void pt_iommu_hw_info(struct pt_iommu_table *fmt_table,
> +		      struct pt_iommu_table_hw_info *info)
> +{
> +	struct pt_iommu *iommu_table = &fmt_table->iommu;
> +	struct pt_common *common = common_from_iommu(iommu_table);
> +	struct pt_range top_range = pt_top_range(common);
> +
> +	pt_iommu_fmt_hw_info(fmt_table, &top_range, info);
> +}
> +EXPORT_SYMBOL_NS_GPL(pt_iommu_hw_info, "GENERIC_PT_IOMMU");
> +#endif
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("IOMMU Page table implementation for " __stringify(PTPFX_RAW));
> +MODULE_IMPORT_NS("GENERIC_PT");
> +
> +#endif

Nit: maybe
#endif /* __GENERIC_PT_IOMMU_PT_H */
?

> +struct pt_iommu_cfg {
> +	/**
> +	 * @features: Features required. Only these features will be turned on.
> +	 * The feature list should reflect what the IOMMU HW is capable of.
> +	 */
> +	unsigned int features;
> +	/**
> +	 * @hw_max_vasz_lg2: Maximum VA the IOMMU HW can support. This will
> +	 * imply the top level of the table.
> +	 */
> +	u8 hw_max_vasz_lg2;
> +	/**
> +	 * @hw_max_oasz_lg2: Maximum OA the IOMMU HW can support. The format
> +	 * might select a lower maximum OA.
> +	 */
> +	u8 hw_max_oasz_lg2;
> +};
> +
> +#endif

#endif /*  __GENERIC_PT_IOMMU_H */

Otherwise,

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>