Re: [PATCH] drivers/io-pgtable-arm: Fix stage concatenation with 16K

From: Robin Murphy
Date: Fri Nov 15 2024 - 13:35:18 EST


On 2024-11-15 5:22 pm, Mostafa Saleh wrote:
According to the Arm spec DDI0487 K.a, in:
"Table D8-9 Implications of the effective minimum T0SZ value on the
initial stage 2 lookup level"

Some combinations of granule and input size with stage-2 would
require to use initial lookup levels that can only be achieved
with concatenated PGDs.

There was one missing case in the current implementation for 16K,
which is 40-bits.

Cc: Daniel Mentz <danielmentz@xxxxxxxxxx>

Signed-off-by: Mostafa Saleh <smostafa@xxxxxxxxxx>
---
drivers/iommu/io-pgtable-arm.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 0e67f1721a3d..9a57874a5cb8 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -1044,10 +1044,18 @@ arm_64_lpae_alloc_pgtable_s2(struct io_pgtable_cfg *cfg, void *cookie)
return NULL;
/*
- * Concatenate PGDs at level 1 if possible in order to reduce
- * the depth of the stage-2 walk.
+ * Some cases where concatenation is mandatory after de-ciphering RSRKBC
+ * in the Arm DDI0487 (K.a):
+ * - 40 bits with 4K: use 2 table at level 1 instead of level 0
+ * - 40 bits with 16K: use 16 tables at level 2 instead of level 1
+ * - 42 bits with 4K: use 8 tabels at level 1 instead of level 0
+ * - 48 bits with 16K: use 2 tabels at level 1 instead of level 0

This confused me, since per R_DXBSH, that last one is the only one which is actually mandatory in general; the others may be valid per R_PZFHQ and R_FBHPY. The additional R_SRKBC constraints come from the PA size, not the choice of T0SZ (and thus ultimately start_level) itself, so although I guess this probably works out true in practice based on how the SMMU drivers happen to behave today, it's none too obvious why.

Thanks,
Robin.

+ * Looking at the possible valid input size, that concludes to always
+ * use level 1 with concatentation if possible or at level 2 only
+ * with 16K.
*/
- if (data->start_level == 0) {
+ if ((data->start_level == 0) ||
+ ((data->start_level == 1) && (ARM_LPAE_GRANULE(data) == SZ_16K))) {
unsigned long pgd_pages;
pgd_pages = ARM_LPAE_PGD_SIZE(data) / sizeof(arm_lpae_iopte);