| Message ID | 20241115172235.1493328-1-smostafa@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drivers/io-pgtable-arm: Fix stage concatenation with 16K | expand |
Hi Robin, On Fri, Nov 15, 2024 at 06:29:20PM +0000, Robin Murphy wrote: > 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@google.com> > > > > Signed-off-by: Mostafa Saleh <smostafa@google.com> > > --- > > 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. Ah, you are right, I got a bit confused, it's not actually about the input size, but the fact that the SMMUv3 driver uses IAS = OAS for stage-2. And constraint R_SRKBC is about the OAS size. So in case we change that IAS = OAS for stage-2, we could end up using concatenation when it's not mandatory. I don't think that's a bad thing (I was actually thinking of changing the code to always use concatenation if possible) But, I can re-write the code in terms of OAS + granule instead so it's more robust incase we change the stage-2 config in the future. Thanks, Mostafa > > 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); >
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 + * 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);
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@google.com> Signed-off-by: Mostafa Saleh <smostafa@google.com> --- drivers/iommu/io-pgtable-arm.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)