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

Nicolin Chen
2025-09-11T05:38:33Z

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>