Re: [PATCH] iommu/io-pgtable-arm: Add misisng concatenated PGD cases

[Robin Murphy <robin.murphy@arm.com>]

On 2025-11-30 7:45 pm, Mostafa Saleh wrote:
> arm_lpae_concat_mandatory() assumes that OAS >= IAS which is not
> correct for SMMUs supporting AArch32, and have OAS = 32/36 bits,
> as IAS would be 40 bits.

But that is only when *using* AArch32 format. The bit in chapter 3.4 of 
the SMMU architecture is talking about the maximum IAS that an SMMU 
implementation needs to be able to accommodate based on its 
configuration, but it does then attempt to clarify that the actual IPA 
size in use by any given context should depend on the VMSA format in use:

"VMSAv8-32 LPAE always supports an IPA size of 40 bits, whereas 
VMSAv8-64 and VMSAv9-128 limits the maximum IPA size to the maximum PA 
size."

Rule R_SRKBC in the Arm ARM lays out the exact T0SZ constraints with 
this AArch32/AArch64 detail.

> In that case, concatenation is mandatory in some cases.
> Document those and add the checks, this can be simplified; instead
> of adding extra checks we can say that concatenation is mandatory for:
> - 4K, start level = 0 and OAS <= 42 bits.
> - 16K, start level = 1 and OAS <= 40 bits.
> Which cover all the missing cases.

AArch32 uses a fixed 4K granule, so this cannot impact 16K. Note that we 
don't support AArch32 for SMMUv3 anyway, and while this does technically 
apply to SMMUv1/2, Arm's MMU-400/401/500 implementations all have OAS 
fixed at 40/40/48 bits respectively, so I'd imagine it's quite a rare 
case to encounter in practice.

Thanks,
Robin.

> Reported-by: Tomasz Nowicki <tnowicki@google.com>
> Fixes: 4dcac8407fe1 ("iommu/io-pgtable-arm: Fix stage-2 concatenation with 16K")
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> ---
>   drivers/iommu/io-pgtable-arm.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index e6626004b323..ecf27e86b429 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -223,6 +223,8 @@ static inline int arm_lpae_max_entries(int i, struct arm_lpae_io_pgtable *data)
>    *   b) 40 bits PA size with 16K: use level 2 instead of level 1 (16 tables for ias = oas)
>    *   c) 42 bits PA size with 4K: use level 1 instead of level 0 (8 tables for ias = oas)
>    *   d) 48 bits PA size with 16K: use level 1 instead of level 0 (2 tables for ias = oas)
> + *   f) 32/36 bits PA size with 4K and 40 bits IAS: use level 1 instead of level 0
> + *   g) 32/36 bits PA size with 16K and 40 bits IAS: use level 2 instead of level 1
>    */
>   static inline bool arm_lpae_concat_mandatory(struct io_pgtable_cfg *cfg,
>   					     struct arm_lpae_io_pgtable *data)
> @@ -234,13 +236,13 @@ static inline bool arm_lpae_concat_mandatory(struct io_pgtable_cfg *cfg,
>   	if ((ARM_LPAE_GRANULE(data) == SZ_16K) && (data->start_level == 0))
>   		return (oas == 48) || (ias == 48);
>   
> -	/* Covers 2.a and 2.c */
> +	/* Covers 2.a, 2.c and 2.f */
>   	if ((ARM_LPAE_GRANULE(data) == SZ_4K) && (data->start_level == 0))
> -		return (oas == 40) || (oas == 42);
> +		return (oas <= 42);
>   
> -	/* Case 2.b */
> +	/* Case 2.b and 2.g */
>   	return (ARM_LPAE_GRANULE(data) == SZ_16K) &&
> -	       (data->start_level == 1) && (oas == 40);
> +	       (data->start_level == 1) && (oas <= 40);
>   }
>   
>   static dma_addr_t __arm_lpae_dma_addr(void *pages)