| Message ID | 20230205094411.793816-8-smostafa@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add stage-2 translation for SMMUv3 | expand |
Hi Mostafa, On 2/5/23 10:44, Mostafa Saleh wrote: > Check if with the configured start level, ia_bits and granularity we > can have a valid page table as described in ARM ARM D8.2 Translation > process. > The idea is to see for the highest possible number of IPA bits, how > many concatenated tables we would need, if it is more than 16, then > this is not possible. This rather checks the validity and consistency of the STE S2 fields. The patch title sounds a bit misleading to me. > > Signed-off-by: Mostafa Saleh <smostafa@google.com> > --- > hw/arm/smmuv3.c | 29 +++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c > index 6633fe40fa..c49b341287 100644 > --- a/hw/arm/smmuv3.c > +++ b/hw/arm/smmuv3.c > @@ -341,6 +341,28 @@ static int smmu_get_cd(SMMUv3State *s, STE *ste, uint32_t ssid, > return 0; > } > > +/* > + * Return true if s2 page table config is valid. > + * This checks with the configured start level, ia_bits and granularity we can > + * have a valid page table as described in ARM ARM D8.2 Translation process. > + * The idea here is to see for the highest possible number of IPA bits, how > + * many concatenated tables we would need, if it is more than 16, then this is > + * not possible. > + */ > +static bool s2_pgtable_config_valid(uint8_t sl0, uint8_t t0sz, uint8_t gran) > +{ > + int level = get_start_level(sl0, gran); > + uint64_t ia_bits = 64 - t0sz; s/ia/ipa > + uint64_t mx = (1ULL << ia_bits) - 1; s/mx/max_ipa > + int nr_concat = pgd_idx(level, gran, mx) + 1; > + > + if (nr_concat > SMMU_MAX_S2_CONCAT) { > + return false; > + } > + > + return true; > +} > + > /* Returns < 0 in case of invalid STE, 0 otherwise */ > static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg, > STE *ste, SMMUEventInfo *event) > @@ -407,6 +429,13 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg, > goto bad_ste; > } > > + if (!s2_pgtable_config_valid(cfg->s2cfg.sl0, cfg->s2cfg.tsz, > + cfg->s2cfg.granule_sz)) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "SMMUv3 STE stage 2 config not valid!\n"); > + goto bad_ste; > + } > + To me this would need to be integrated into the STE decoding patch. This latter shall be self-contained if possible to ease the review Thanks Eric > /* This is still here as stage 2 has not been fully enabled yet. */ > qemu_log_mask(LOG_UNIMP, "SMMUv3 does not support stage 2 yet\n"); > goto bad_ste;
Hi Eric, > > can have a valid page table as described in ARM ARM D8.2 Translation > > process. > > The idea is to see for the highest possible number of IPA bits, how > > many concatenated tables we would need, if it is more than 16, then > > this is not possible. > > This rather checks the validity and consistency of the STE S2 fields. > The patch title sounds a bit misleading to me. I will update the wording. > > +static bool s2_pgtable_config_valid(uint8_t sl0, uint8_t t0sz, uint8_t gran) > > +{ > > + int level = get_start_level(sl0, gran); > > + uint64_t ia_bits = 64 - t0sz; > s/ia/ipa I will update it. > > + uint64_t mx = (1ULL << ia_bits) - 1; > s/mx/max_ipa I will update it. > > > > + if (!s2_pgtable_config_valid(cfg->s2cfg.sl0, cfg->s2cfg.tsz, > > + cfg->s2cfg.granule_sz)) { > > + qemu_log_mask(LOG_GUEST_ERROR, > > + "SMMUv3 STE stage 2 config not valid!\n"); > > + goto bad_ste; > > + } > > + > To me this would need to be integrated into the STE decoding patch. This > latter shall be self-contained if possible to ease the review I will squash it, I was trying to keep patches small, but it makes sense to validate STE in the same patch parsing it. Thanks, Mostafa
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c index 6633fe40fa..c49b341287 100644 --- a/hw/arm/smmuv3.c +++ b/hw/arm/smmuv3.c @@ -341,6 +341,28 @@ static int smmu_get_cd(SMMUv3State *s, STE *ste, uint32_t ssid, return 0; } +/* + * Return true if s2 page table config is valid. + * This checks with the configured start level, ia_bits and granularity we can + * have a valid page table as described in ARM ARM D8.2 Translation process. + * The idea here is to see for the highest possible number of IPA bits, how + * many concatenated tables we would need, if it is more than 16, then this is + * not possible. + */ +static bool s2_pgtable_config_valid(uint8_t sl0, uint8_t t0sz, uint8_t gran) +{ + int level = get_start_level(sl0, gran); + uint64_t ia_bits = 64 - t0sz; + uint64_t mx = (1ULL << ia_bits) - 1; + int nr_concat = pgd_idx(level, gran, mx) + 1; + + if (nr_concat > SMMU_MAX_S2_CONCAT) { + return false; + } + + return true; +} + /* Returns < 0 in case of invalid STE, 0 otherwise */ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg, STE *ste, SMMUEventInfo *event) @@ -407,6 +429,13 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg, goto bad_ste; } + if (!s2_pgtable_config_valid(cfg->s2cfg.sl0, cfg->s2cfg.tsz, + cfg->s2cfg.granule_sz)) { + qemu_log_mask(LOG_GUEST_ERROR, + "SMMUv3 STE stage 2 config not valid!\n"); + goto bad_ste; + } + /* This is still here as stage 2 has not been fully enabled yet. */ qemu_log_mask(LOG_UNIMP, "SMMUv3 does not support stage 2 yet\n"); goto bad_ste;
Check if with the configured start level, ia_bits and granularity we can have a valid page table as described in ARM ARM D8.2 Translation process. The idea is to see for the highest possible number of IPA bits, how many concatenated tables we would need, if it is more than 16, then this is not possible. Signed-off-by: Mostafa Saleh <smostafa@google.com> --- hw/arm/smmuv3.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)