| Message ID | 20230205094411.793816-7-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: > Parse stage-2 configuration and populate it in SMMUTransCfg. > Configs in this patch (s2g, ttb, tsz, sl0). above 'sentence' a bit cryptic. > Checking validity of values added when possible. > > MAX IPA supported is 48 bits and only AA64 tables are supported. > > Signed-off-by: Mostafa Saleh <smostafa@google.com> > --- > hw/arm/smmuv3.c | 43 +++++++++++++++++++++++++++++++++++- > include/hw/arm/smmu-common.h | 1 + > 2 files changed, 43 insertions(+), 1 deletion(-) > > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c > index 54dd8e5ec1..6633fe40fa 100644 > --- a/hw/arm/smmuv3.c > +++ b/hw/arm/smmuv3.c > @@ -366,7 +366,48 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg, > return 0; > } > > - if (STE_CFG_S2_ENABLED(config)) { > + if (STAGE2_SUPPORTED(s->features) && STE_CFG_S2_ENABLED(config)) { Don't you want to check both S1 and S2 aren't set? > + cfg->stage = 2; > + > + if (STE_S2AA64(ste) == 0x0) { > + qemu_log_mask(LOG_UNIMP, > + "SMMUv3 AArch32 tables not supported\n"); > + goto bad_ste; > + } > + > + switch (STE_S2TG(ste)) { > + case 0x0: /* 4KB */ > + cfg->s2cfg.granule_sz = 12; > + break; > + case 0x1: /* 64KB */ > + cfg->s2cfg.granule_sz = 16; > + break; > + case 0x2: /* 16KB */ > + cfg->s2cfg.granule_sz = 14; > + break; > + default: > + qemu_log_mask(LOG_GUEST_ERROR, > + "SMMUv3 bad STE S2TG: %x\n", STE_S2TG(ste)); > + goto bad_ste; > + } > + > + cfg->s2cfg.vttb = STE_S2TTB(ste); > + cfg->s2cfg.tsz = STE_S2T0SZ(ste); What about IDR3.STT currently 0 so S2T0SZ <= 39 don't you need to check against SMMU_IDR3.STT/S2TG • In architectures after SMMUv3.0: – If STE.S2TG selects a 4KB or 16KB granule, the minimum valid value for this field is MAX(16, 64-IAS). – If STE.S2TG selects a 64KB granule, the minimum valid value for this field is (64-IAS). > + > + if ((64 - cfg->s2cfg.tsz) > SMMU_MAX_IPA_BITS) { > + qemu_log_mask(LOG_UNIMP, "SMMUv3 IPA too big! TS0Z = %x\n", > + cfg->s2cfg.tsz); > + goto bad_ste; > + } > + > + cfg->s2cfg.sl0 = STE_S2SL0(ste); > + if (cfg->s2cfg.sl0 == 0x3) { > + qemu_log_mask(LOG_UNIMP, > + "SMMUv3 STE->SL0 0x3 has no meaning!\n"); > + goto bad_ste; what about S2PS, S2VMID? you may either squash [RFC PATCH 11/16] hw/arm/smmuv3: Read VMID from STE into that patch or at least mention in the commit msg that S2VMID will be dealt with separately 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; > } > diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h > index 1e666e8b6d..7906e359d9 100644 > --- a/include/hw/arm/smmu-common.h > +++ b/include/hw/arm/smmu-common.h > @@ -28,6 +28,7 @@ > #define SMMU_PCI_DEVFN(sid) (sid & 0xFF) > > #define SMMU_MAX_VA_BITS 48 > +#define SMMU_MAX_IPA_BITS 48 > #define SMMU_MAX_LEVELS 4 > > /*
Hi Eric, On Wed, Feb 15, 2023 at 06:47:52PM +0100, Eric Auger wrote: > On 2/5/23 10:44, Mostafa Saleh wrote: > > Parse stage-2 configuration and populate it in SMMUTransCfg. > > Configs in this patch (s2g, ttb, tsz, sl0). > above 'sentence' a bit cryptic. I will reword it. > > +++ b/hw/arm/smmuv3.c > > @@ -366,7 +366,48 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg, > > return 0; > > } > > > > - if (STE_CFG_S2_ENABLED(config)) { > > + if (STAGE2_SUPPORTED(s->features) && STE_CFG_S2_ENABLED(config)) { > Don't you want to check both S1 and S2 aren't set? Yes, currently this is ignored, but looking at the SMMU manual it is illegal to configure an unsupported stage, I will update it. > > + break; > > + case 0x2: /* 16KB */ > > + cfg->s2cfg.granule_sz = 14; > > + break; > > + default: > > + qemu_log_mask(LOG_GUEST_ERROR, > > + "SMMUv3 bad STE S2TG: %x\n", STE_S2TG(ste)); > > + goto bad_ste; > > + } > > + > > + cfg->s2cfg.vttb = STE_S2TTB(ste); > > + cfg->s2cfg.tsz = STE_S2T0SZ(ste); > What about IDR3.STT currently 0 so S2T0SZ <= 39 > > don't you need to check against SMMU_IDR3.STT/S2TG > > • In architectures after SMMUv3.0: > – If STE.S2TG selects a 4KB or 16KB granule, the minimum valid value for > this field is MAX(16, > 64-IAS). > – If STE.S2TG selects a 64KB granule, the minimum valid value for this > field is (64-IAS). I will add a function to validate S2T0SZ based on the behaviour after SMMUv3.0 and checks against STT disabled. I believe it is safe just to check S2T0SZ <= 39 without checking IDR3.STT as we don’t support it for now. > > + cfg->s2cfg.tsz); > > + goto bad_ste; > > + } > > + > > + cfg->s2cfg.sl0 = STE_S2SL0(ste); > > + if (cfg->s2cfg.sl0 == 0x3) { > > + qemu_log_mask(LOG_UNIMP, > > + "SMMUv3 STE->SL0 0x3 has no meaning!\n"); > > + goto bad_ste; > what about S2PS, S2VMID? > > you may either squash [RFC PATCH 11/16] hw/arm/smmuv3: Read VMID from > STE into that patch or at least mention in the commit msg that S2VMID > will be dealt with separately I will squash it with "[RFC PATCH 11/16] hw/arm/smmuv3: Read VMID from STE" I missed S2PS, I will add it in V2. Thanks, Mostafa
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c index 54dd8e5ec1..6633fe40fa 100644 --- a/hw/arm/smmuv3.c +++ b/hw/arm/smmuv3.c @@ -366,7 +366,48 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg, return 0; } - if (STE_CFG_S2_ENABLED(config)) { + if (STAGE2_SUPPORTED(s->features) && STE_CFG_S2_ENABLED(config)) { + cfg->stage = 2; + + if (STE_S2AA64(ste) == 0x0) { + qemu_log_mask(LOG_UNIMP, + "SMMUv3 AArch32 tables not supported\n"); + goto bad_ste; + } + + switch (STE_S2TG(ste)) { + case 0x0: /* 4KB */ + cfg->s2cfg.granule_sz = 12; + break; + case 0x1: /* 64KB */ + cfg->s2cfg.granule_sz = 16; + break; + case 0x2: /* 16KB */ + cfg->s2cfg.granule_sz = 14; + break; + default: + qemu_log_mask(LOG_GUEST_ERROR, + "SMMUv3 bad STE S2TG: %x\n", STE_S2TG(ste)); + goto bad_ste; + } + + cfg->s2cfg.vttb = STE_S2TTB(ste); + cfg->s2cfg.tsz = STE_S2T0SZ(ste); + + if ((64 - cfg->s2cfg.tsz) > SMMU_MAX_IPA_BITS) { + qemu_log_mask(LOG_UNIMP, "SMMUv3 IPA too big! TS0Z = %x\n", + cfg->s2cfg.tsz); + goto bad_ste; + } + + cfg->s2cfg.sl0 = STE_S2SL0(ste); + if (cfg->s2cfg.sl0 == 0x3) { + qemu_log_mask(LOG_UNIMP, + "SMMUv3 STE->SL0 0x3 has no meaning!\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; } diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h index 1e666e8b6d..7906e359d9 100644 --- a/include/hw/arm/smmu-common.h +++ b/include/hw/arm/smmu-common.h @@ -28,6 +28,7 @@ #define SMMU_PCI_DEVFN(sid) (sid & 0xFF) #define SMMU_MAX_VA_BITS 48 +#define SMMU_MAX_IPA_BITS 48 #define SMMU_MAX_LEVELS 4 /*
Parse stage-2 configuration and populate it in SMMUTransCfg. Configs in this patch (s2g, ttb, tsz, sl0). Checking validity of values added when possible. MAX IPA supported is 48 bits and only AA64 tables are supported. Signed-off-by: Mostafa Saleh <smostafa@google.com> --- hw/arm/smmuv3.c | 43 +++++++++++++++++++++++++++++++++++- include/hw/arm/smmu-common.h | 1 + 2 files changed, 43 insertions(+), 1 deletion(-)