| Message ID | 20241202140604.422235-2-smostafa@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fix missing case for concatenation | expand |
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@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); I'll make that change when applying... Will
On Mon, Dec 9, 2024 at 11:58 PM Will Deacon <will@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@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
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); + + /* Covers 2.a and 2.c */ + if ((ARM_LPAE_GRANULE(data) == SZ_4K) && (data->start_level == 0)) + return (oas == 40) || (oas == 42); + + /* Case 2.b */ + return (ARM_LPAE_GRANULE(data) == SZ_16K) && + (data->start_level == 1) && (oas == 40); +} + static bool selftest_running = false; static dma_addr_t __arm_lpae_dma_addr(void *pages) @@ -1006,18 +1033,12 @@ arm_64_lpae_alloc_pgtable_s2(struct io_pgtable_cfg *cfg, void *cookie) if (!data) return NULL; - /* - * Concatenate PGDs at level 1 if possible in order to reduce - * the depth of the stage-2 walk. - */ - if (data->start_level == 0) { - unsigned long pgd_pages; - - pgd_pages = ARM_LPAE_PGD_SIZE(data) / sizeof(arm_lpae_iopte); - if (pgd_pages <= ARM_LPAE_S2_MAX_CONCAT_PAGES) { - data->pgd_bits += data->bits_per_level; - data->start_level++; - } + if (arm_lpae_concat_mandatory(data)) { + if (WARN_ON((ARM_LPAE_PGD_SIZE(data) / sizeof(arm_lpae_iopte)) > + ARM_LPAE_S2_MAX_CONCAT_PAGES)) + return NULL; + data->pgd_bits += data->bits_per_level; + data->start_level++; } /* VTCR */
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@google.com> --- drivers/iommu/io-pgtable-arm.c | 45 +++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 12 deletions(-)