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

Mostafa Saleh posted 1 patch 2 months, 1 week ago
drivers/iommu/io-pgtable-arm.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
[PATCH] iommu/io-pgtable-arm: Add misisng concatenated PGD cases
Posted by Mostafa Saleh 2 months, 1 week ago
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.

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.

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)
-- 
2.52.0.487.g5c8c507ade-goog
Re: [PATCH] iommu/io-pgtable-arm: Add misisng concatenated PGD cases
Posted by Robin Murphy 1 month, 4 weeks ago
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)
Re: [PATCH] iommu/io-pgtable-arm: Add misisng concatenated PGD cases
Posted by Mostafa Saleh 1 month, 4 weeks ago
On Tue, Dec 09, 2025 at 11:34:34AM +0000, Robin Murphy wrote:
> 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.

I see, thanks a lot for the explanation, I got confused by the this
statement:
Note: If AArch32 is implemented, IAS == MAX(40, OAS), otherwise IAS == OAS.

However, I think this is still a bug but somewere else, as at the moment
the SMMUv3 dirver will use the SMMU IAS (40-bits) as input for AArch64
stage-2 page tables, so we need either to limit the IAS as:

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index d16d35c78c06..d21153156daa 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2561,7 +2561,7 @@ static int arm_smmu_domain_finalise(struct arm_smmu_domain *smmu_domain,
        case ARM_SMMU_DOMAIN_S2:
                if (enable_dirty)
                        return -EOPNOTSUPP;
-               pgtbl_cfg.ias = smmu->ias;
+               pgtbl_cfg.ias = min(smmu->ias, smmu->oas);
                pgtbl_cfg.oas = smmu->oas;
                fmt = ARM_64_LPAE_S2;
                finalise_stage_fn = arm_smmu_domain_finalise_s2;

Or, don't populate IAS depending on AArch32 support as the driver
doesn't support it, effectively reverting:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f0c453dbcce7767cd868deb809ba68083c93954e

> 
> > 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.

Yes, I don't have access to such HW.

Thanks,
Mostafa
> 
> 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)
>
Re: [PATCH] iommu/io-pgtable-arm: Add misisng concatenated PGD cases
Posted by Robin Murphy 1 month, 4 weeks ago
On 2025-12-09 12:37 pm, Mostafa Saleh wrote:
> On Tue, Dec 09, 2025 at 11:34:34AM +0000, Robin Murphy wrote:
>> 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.
> 
> I see, thanks a lot for the explanation, I got confused by the this
> statement:
> Note: If AArch32 is implemented, IAS == MAX(40, OAS), otherwise IAS == OAS.

Indeed, that appears confusingly contradictory; I've filed a bug.

> However, I think this is still a bug but somewere else, as at the moment
> the SMMUv3 dirver will use the SMMU IAS (40-bits) as input for AArch64
> stage-2 page tables, so we need either to limit the IAS as:
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index d16d35c78c06..d21153156daa 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2561,7 +2561,7 @@ static int arm_smmu_domain_finalise(struct arm_smmu_domain *smmu_domain,
>          case ARM_SMMU_DOMAIN_S2:
>                  if (enable_dirty)
>                          return -EOPNOTSUPP;
> -               pgtbl_cfg.ias = smmu->ias;
> +               pgtbl_cfg.ias = min(smmu->ias, smmu->oas);
>                  pgtbl_cfg.oas = smmu->oas;
>                  fmt = ARM_64_LPAE_S2;
>                  finalise_stage_fn = arm_smmu_domain_finalise_s2;
> 
> Or, don't populate IAS depending on AArch32 support as the driver
> doesn't support it, effectively reverting:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f0c453dbcce7767cd868deb809ba68083c93954e

It does appear we've missed the detail here. TBH I'm not really sure why 
we're bothering to consider the theoretical maximum IAS at all when it 
only makes any difference to a format we've never cared about supporting 
anyway. Frankly I'd be inclined to just remove smmu->ias altogether - 
even if we did ever want to support LPAE format, it would be just as 
trivial for that to hard-code pgtbl_cfg.ias = 40 based on the 
architecture rules.

Thanks,
Robin.
Re: [PATCH] iommu/io-pgtable-arm: Add misisng concatenated PGD cases
Posted by Will Deacon 1 month, 4 weeks ago
On Tue, Dec 09, 2025 at 01:33:36PM +0000, Robin Murphy wrote:
> On 2025-12-09 12:37 pm, Mostafa Saleh wrote:
> > On Tue, Dec 09, 2025 at 11:34:34AM +0000, Robin Murphy wrote:
> > > 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.
> > 
> > I see, thanks a lot for the explanation, I got confused by the this
> > statement:
> > Note: If AArch32 is implemented, IAS == MAX(40, OAS), otherwise IAS == OAS.
> 
> Indeed, that appears confusingly contradictory; I've filed a bug.

I think the spec has always been worded like this. My reading is that, in
isolation:

  - VMSAv8-32 LPAE always uses a 40-bit IAS
  - VMSAv8-64 has IAS == OAS and this can be smaller than 40 bits

so if AArch32 is implemented, we know that the hardware supports at
least a 40-bit IAS and in that case the VMSAv8-64 IAS can be bigger
than the OAS.

That seems consistent to me; where is the contradiction?

Will (jet-lagged so may well be talking rubbish!)
Re: [PATCH] iommu/io-pgtable-arm: Add misisng concatenated PGD cases
Posted by Robin Murphy 1 month, 4 weeks ago
On 2025-12-10 12:42 am, Will Deacon wrote:
> On Tue, Dec 09, 2025 at 01:33:36PM +0000, Robin Murphy wrote:
>> On 2025-12-09 12:37 pm, Mostafa Saleh wrote:
>>> On Tue, Dec 09, 2025 at 11:34:34AM +0000, Robin Murphy wrote:
>>>> 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.
>>>
>>> I see, thanks a lot for the explanation, I got confused by the this
>>> statement:
>>> Note: If AArch32 is implemented, IAS == MAX(40, OAS), otherwise IAS == OAS.
>>
>> Indeed, that appears confusingly contradictory; I've filed a bug.
> 
> I think the spec has always been worded like this. My reading is that, in
> isolation:
> 
>    - VMSAv8-32 LPAE always uses a 40-bit IAS
>    - VMSAv8-64 has IAS == OAS and this can be smaller than 40 bits
> 
> so if AArch32 is implemented, we know that the hardware supports at
> least a 40-bit IAS and in that case the VMSAv8-64 IAS can be bigger
> than the OAS.

No, the VMSAv8-64 IAS can never be bigger than OAS, as that would 
violate VMSA (see rules R_DTLMN and R_SRKBC). The SMMU spec seems mostly 
fixated on the notional maximum IAS that an SMMU must accommodate in 
general across all supported formats; the limitations of using any 
particular format must still apply though (similarly, the fact that 
AArch32 has a fixed 40-bit output doesn't mean that OAS and S2PS don't 
still matter for AArch64 format.)

Note that AArch32 is still free to use less than 40 bits of IAS if it 
wants to, by setting S2T0SZ appropriately, it's just guaranteed to 
support a *minimum* S2T0SZ of 24 regardless of OAS, unlike AArch64.

Cheers,
Robin.

> That seems consistent to me; where is the contradiction?
> 
> Will (jet-lagged so may well be talking rubbish!)
Re: [PATCH] iommu/io-pgtable-arm: Add misisng concatenated PGD cases
Posted by Will Deacon 1 month, 4 weeks ago
On Wed, Dec 10, 2025 at 12:06:10PM +0000, Robin Murphy wrote:
> On 2025-12-10 12:42 am, Will Deacon wrote:
> > On Tue, Dec 09, 2025 at 01:33:36PM +0000, Robin Murphy wrote:
> > > On 2025-12-09 12:37 pm, Mostafa Saleh wrote:
> > > > On Tue, Dec 09, 2025 at 11:34:34AM +0000, Robin Murphy wrote:
> > > > > 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.
> > > > 
> > > > I see, thanks a lot for the explanation, I got confused by the this
> > > > statement:
> > > > Note: If AArch32 is implemented, IAS == MAX(40, OAS), otherwise IAS == OAS.
> > > 
> > > Indeed, that appears confusingly contradictory; I've filed a bug.
> > 
> > I think the spec has always been worded like this. My reading is that, in
> > isolation:
> > 
> >    - VMSAv8-32 LPAE always uses a 40-bit IAS
> >    - VMSAv8-64 has IAS == OAS and this can be smaller than 40 bits
> > 
> > so if AArch32 is implemented, we know that the hardware supports at
> > least a 40-bit IAS and in that case the VMSAv8-64 IAS can be bigger
> > than the OAS.
> 
> No, the VMSAv8-64 IAS can never be bigger than OAS, as that would violate
> VMSA (see rules R_DTLMN and R_SRKBC). The SMMU spec seems mostly fixated on
> the notional maximum IAS that an SMMU must accommodate in general across all
> supported formats; the limitations of using any particular format must still
> apply though (similarly, the fact that AArch32 has a fixed 40-bit output
> doesn't mean that OAS and S2PS don't still matter for AArch64 format.)

A VMSAv8-32 stage-1 could still input a 40-bit IAS into a VMSAv8-64
stage-2 when the OAS < 40-bit though, right? If that's the only case
where this happens, then I agree we should ditch the complexity from the
driver on the grounds that we don't ever use the 32-bit formats.

Will
Re: [PATCH] iommu/io-pgtable-arm: Add misisng concatenated PGD cases
Posted by Robin Murphy 1 month, 3 weeks ago
On 2025-12-11 2:09 am, Will Deacon wrote:
> On Wed, Dec 10, 2025 at 12:06:10PM +0000, Robin Murphy wrote:
>> On 2025-12-10 12:42 am, Will Deacon wrote:
>>> On Tue, Dec 09, 2025 at 01:33:36PM +0000, Robin Murphy wrote:
>>>> On 2025-12-09 12:37 pm, Mostafa Saleh wrote:
>>>>> On Tue, Dec 09, 2025 at 11:34:34AM +0000, Robin Murphy wrote:
>>>>>> 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.
>>>>>
>>>>> I see, thanks a lot for the explanation, I got confused by the this
>>>>> statement:
>>>>> Note: If AArch32 is implemented, IAS == MAX(40, OAS), otherwise IAS == OAS.
>>>>
>>>> Indeed, that appears confusingly contradictory; I've filed a bug.
>>>
>>> I think the spec has always been worded like this. My reading is that, in
>>> isolation:
>>>
>>>     - VMSAv8-32 LPAE always uses a 40-bit IAS
>>>     - VMSAv8-64 has IAS == OAS and this can be smaller than 40 bits
>>>
>>> so if AArch32 is implemented, we know that the hardware supports at
>>> least a 40-bit IAS and in that case the VMSAv8-64 IAS can be bigger
>>> than the OAS.
>>
>> No, the VMSAv8-64 IAS can never be bigger than OAS, as that would violate
>> VMSA (see rules R_DTLMN and R_SRKBC). The SMMU spec seems mostly fixated on
>> the notional maximum IAS that an SMMU must accommodate in general across all
>> supported formats; the limitations of using any particular format must still
>> apply though (similarly, the fact that AArch32 has a fixed 40-bit output
>> doesn't mean that OAS and S2PS don't still matter for AArch64 format.)
> 
> A VMSAv8-32 stage-1 could still input a 40-bit IAS into a VMSAv8-64
> stage-2 when the OAS < 40-bit though, right? If that's the only case
> where this happens, then I agree we should ditch the complexity from the
> driver on the grounds that we don't ever use the 32-bit formats.

Yes, it is perfectly possible with either format to configure the S1 
*output* size (CD.IPS, or 40 when CD.AA64==0) to be larger than the S2 
input size (STE.S2T0SZ), and if S1 tries to use an IPA beyond the actual 
S2 range then it gets a S2 translation fault, per VMSA rule R_BZHGM.

Cheers,
Robin.
Re: [PATCH] iommu/io-pgtable-arm: Add misisng concatenated PGD cases
Posted by Mostafa Saleh 1 month, 4 weeks ago
On Tue, Dec 09, 2025 at 01:33:36PM +0000, Robin Murphy wrote:
> On 2025-12-09 12:37 pm, Mostafa Saleh wrote:
> > On Tue, Dec 09, 2025 at 11:34:34AM +0000, Robin Murphy wrote:
> > > 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.
> > 
> > I see, thanks a lot for the explanation, I got confused by the this
> > statement:
> > Note: If AArch32 is implemented, IAS == MAX(40, OAS), otherwise IAS == OAS.
> 
> Indeed, that appears confusingly contradictory; I've filed a bug.
> 
> > However, I think this is still a bug but somewere else, as at the moment
> > the SMMUv3 dirver will use the SMMU IAS (40-bits) as input for AArch64
> > stage-2 page tables, so we need either to limit the IAS as:
> > 
> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > index d16d35c78c06..d21153156daa 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > @@ -2561,7 +2561,7 @@ static int arm_smmu_domain_finalise(struct arm_smmu_domain *smmu_domain,
> >          case ARM_SMMU_DOMAIN_S2:
> >                  if (enable_dirty)
> >                          return -EOPNOTSUPP;
> > -               pgtbl_cfg.ias = smmu->ias;
> > +               pgtbl_cfg.ias = min(smmu->ias, smmu->oas);
> >                  pgtbl_cfg.oas = smmu->oas;
> >                  fmt = ARM_64_LPAE_S2;
> >                  finalise_stage_fn = arm_smmu_domain_finalise_s2;
> > 
> > Or, don't populate IAS depending on AArch32 support as the driver
> > doesn't support it, effectively reverting:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f0c453dbcce7767cd868deb809ba68083c93954e
> 
> It does appear we've missed the detail here. TBH I'm not really sure why
> we're bothering to consider the theoretical maximum IAS at all when it only
> makes any difference to a format we've never cared about supporting anyway.
> Frankly I'd be inclined to just remove smmu->ias altogether - even if we did
> ever want to support LPAE format, it would be just as trivial for that to
> hard-code pgtbl_cfg.ias = 40 based on the architecture rules.
> 

Makes sense, I will prepare a patch.

Thanks,
Mostafa

> Thanks,
> Robin.