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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aTgX9qhaLyrQ5iye@google.com>
Date: Tue, 9 Dec 2025 12:37:10 +0000
From: Mostafa Saleh <smostafa@...gle.com>
To: Robin Murphy <robin.murphy@....com>
Cc: iommu@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, will@...nel.org, joro@...tes.org,
	Tomasz Nowicki <tnowicki@...gle.com>
Subject: Re: [PATCH] iommu/io-pgtable-arm: Add misisng concatenated PGD cases

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@...gle.com>
> > Fixes: 4dcac8407fe1 ("iommu/io-pgtable-arm: Fix stage-2 concatenation with 16K")
> > Signed-off-by: Mostafa Saleh <smostafa@...gle.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)
>