[PATCH v2 1/2] iommu/io-pgtable-arm: Fix stage-2 concatenation with 16K

[Mostafa Saleh]

On Mon, Dec 9, 2024 at 11:58 PM Will Deacon <will at kernel.org> wrote:
>
> On Mon, Dec 02, 2024 at 02:06:03PM +0000, Mostafa Saleh wrote:
> > At the moment, io-pgtable-arm uses concatenation only if it is
> > possible at level 0, which misses a case where concatenation is
> > mandatory at level 1 according to R_SRKBC in Arm spec DDI0487 K.a.
> >
> > Also, that means concatenation can be used when not mandated,
> > contradicting the comment on the code. However, these cases can only
> > happen if the SMMUv3 driver is changed to use ias != oas for stage-2.
> >
> > This patch re-writes the code to use concatenation only if mandatory,
> > fixing the missing case for level-1 and granule 16K with PA = 40 bits.
> >
> > Signed-off-by: Mostafa Saleh <smostafa at google.com>
> > ---
> >  drivers/iommu/io-pgtable-arm.c | 45 +++++++++++++++++++++++++---------
> >  1 file changed, 33 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> > index 6b9bb58a414f..0055876b3527 100644
> > --- a/drivers/iommu/io-pgtable-arm.c
> > +++ b/drivers/iommu/io-pgtable-arm.c
> > @@ -223,6 +223,33 @@ static inline int arm_lpae_max_entries(int i, struct arm_lpae_io_pgtable *data)
> >       return ptes_per_table - (i & (ptes_per_table - 1));
> >  }
> >
> > +/*
> > + * Check if concatenated PGDs are mandatory according to Arm DDI0487 (K.a)
> > + * 1) R_DXBSH: For 16KB, and 48-bit input size, use level 1 instead of 0.
> > + * 2) R_SRKBC: After de-ciphering the table for PA size and valid initial lookup
> > + *   a) 40 bits PA size with 4K: use level 1 instead of level 0 (2 tables for ias = oas)
> > + *   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)
> > + */
> > +static inline bool arm_lpae_concat_mandatory(struct arm_lpae_io_pgtable *data)
> > +{
> > +     unsigned int ias = data->iop.cfg.ias;
> > +     unsigned int oas = data->iop.cfg.oas;
> > +
> > +     /* Covers 1  and 2.d */
> > +     if ((ARM_LPAE_GRANULE(data) == SZ_16K) && (data->start_level == 0))
> > +             return (oas == 48) || (ias = 48);
>
> I'm guessing the second disjunct here should be:
>
>         (ias == 48);
>

Ouch, my bad.

> I'll make that change when applying...
>
> Will