| Message ID | 20240429032403.74910-8-smostafa@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | SMMUv3 nested translation support | expand |
Hi Mostafa, On 4/29/24 05:23, Mostafa Saleh wrote: > According to ARM SMMU architecture specification (ARM IHI 0070 F.b), > In "5.2 Stream Table Entry": > [51:6] S1ContextPtr > If Config[1] == 1 (stage 2 enabled), this pointer is an IPA translated by > stage 2 and the programmed value must be within the range of the IAS. > > In "5.4.1 CD notes": > The translation table walks performed from TTB0 or TTB1 are always performed > in IPA space if stage 2 translations are enabled. > > This patch implements translation of the S1 context descriptor pointer and > TTBx base addresses through the S2 stage (IPA -> PA) > > smmuv3_do_translate() is updated to have one arg which is translation > class, this is useful for: s/for/to? > - Decide wether a translation is stage-2 only or use the STE config. > - Populate the class in case of faults, WALK_EABT is lefat as it as left unchanged? > it is always triggered from TT access so no need to use the input > class. > > In case for stage-2 only translation, which only used in nesting, the in case of S2 translation used in the contexted of a nested translation, ... > stage and asid are saved and restored before and after calling > smmu_translate(). > > Translating CD or TTBx can fail for the following reasons: > 1) Large address size: This is described in > (3.4.3 Address sizes of SMMU-originated accesses) > - For CD ptr larger than IAS, for SMMUv3.1, it can trigger either > C_BAD_STE or Translation fault, we implement the latter as it > requires no extra code. > - For TTBx, if larger than the effective stage 1 output address size, it > triggers C_BAD_CD. > > 2) Faults from PTWs (7.3 Event records) > - F_ADDR_SIZE: large address size after first level causes stage 2 Address > Size fault (Also in 3.4.3 Address sizes of SMMU-originated accesses) > - F_PERMISSION: Same as an address translation. However, when > CLASS == CD, the access is implicitly Data and a read. > - F_ACCESS: Same as an address translation. > - F_TRANSLATION: Same as an address translation. > - F_WALK_EABT: Same as an address translation. > These are already implemented in the PTW logic, so no extra handling > required. > > As, there is multiple locations where the address is calculated from > cached entry, a new macro is introduced CACHED_ENTRY_TO_ADDR. > > Signed-off-by: Mostafa Saleh <smostafa@google.com> > --- > hw/arm/smmuv3.c | 76 ++++++++++++++++++++++++++++++------ > include/hw/arm/smmu-common.h | 3 ++ > 2 files changed, 66 insertions(+), 13 deletions(-) > > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c > index cc61708160..cc61c82321 100644 > --- a/hw/arm/smmuv3.c > +++ b/hw/arm/smmuv3.c > @@ -337,14 +337,33 @@ static int smmu_get_ste(SMMUv3State *s, dma_addr_t addr, STE *buf, > > } > > +static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr, > + SMMUTransCfg *cfg, > + SMMUEventInfo *event, > + IOMMUAccessFlags flag, > + SMMUTLBEntry **out_entry, > + SMMUTranslationClass class); > /* @ssid > 0 not supported yet */ > -static int smmu_get_cd(SMMUv3State *s, STE *ste, uint32_t ssid, > - CD *buf, SMMUEventInfo *event) > +static int smmu_get_cd(SMMUv3State *s, STE *ste, SMMUTransCfg *cfg, > + uint32_t ssid, CD *buf, SMMUEventInfo *event) > { > dma_addr_t addr = STE_CTXPTR(ste); > int ret, i; > + SMMUTranslationStatus status; > + SMMUTLBEntry *entry; > > trace_smmuv3_get_cd(addr); > + > + if (cfg->stage == SMMU_NESTED) { > + status = smmuv3_do_translate(s, addr, cfg, event, > + IOMMU_RO, &entry, SMMU_CLASS_CD); > + if (status != SMMU_TRANS_SUCCESS) { So I guess you rely on event being populated by above CD S2 translate(). it does not need to be patched, correct? May be worth a comment. > + return -EINVAL; > + } > + > + addr = CACHED_ENTRY_TO_ADDR(entry, addr); > + } > + > /* TODO: guarantee 64-bit single-copy atomicity */ > ret = dma_memory_read(&address_space_memory, addr, buf, sizeof(*buf), > MEMTXATTRS_UNSPECIFIED); > @@ -659,10 +678,13 @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste, > return 0; > } > > -static int decode_cd(SMMUTransCfg *cfg, CD *cd, SMMUEventInfo *event) > +static int decode_cd(SMMUv3State *s, SMMUTransCfg *cfg, > + CD *cd, SMMUEventInfo *event) > { > int ret = -EINVAL; > int i; > + SMMUTranslationStatus status; > + SMMUTLBEntry *entry; > > if (!CD_VALID(cd) || !CD_AARCH64(cd)) { > goto bad_cd; > @@ -713,9 +735,21 @@ static int decode_cd(SMMUTransCfg *cfg, CD *cd, SMMUEventInfo *event) > > tt->tsz = tsz; > tt->ttb = CD_TTB(cd, i); > + > if (tt->ttb & ~(MAKE_64BIT_MASK(0, cfg->oas))) { > goto bad_cd; > } > + > + /* Translate the TTBx, from IPA to PA if nesting is enabled. */ > + if (cfg->stage == SMMU_NESTED) { > + status = smmuv3_do_translate(s, tt->ttb, cfg, event, IOMMU_RO, > + &entry, SMMU_CLASS_TT); > + if (status != SMMU_TRANS_SUCCESS) { same here. > + return -EINVAL; > + } > + tt->ttb = CACHED_ENTRY_TO_ADDR(entry, tt->ttb); > + } > + > tt->had = CD_HAD(cd, i); > trace_smmuv3_decode_cd_tt(i, tt->tsz, tt->ttb, tt->granule_sz, tt->had); > } > @@ -767,12 +801,12 @@ static int smmuv3_decode_config(IOMMUMemoryRegion *mr, SMMUTransCfg *cfg, > return 0; > } > > - ret = smmu_get_cd(s, &ste, 0 /* ssid */, &cd, event); > + ret = smmu_get_cd(s, &ste, cfg, 0 /* ssid */, &cd, event); > if (ret) { > return ret; > } > > - return decode_cd(cfg, &cd, event); > + return decode_cd(s, cfg, &cd, event); > } > > /** > @@ -832,13 +866,29 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr, > SMMUTransCfg *cfg, > SMMUEventInfo *event, > IOMMUAccessFlags flag, > - SMMUTLBEntry **out_entry) > + SMMUTLBEntry **out_entry, > + SMMUTranslationClass class) > { > SMMUPTWEventInfo ptw_info = {}; > SMMUState *bs = ARM_SMMU(s); > SMMUTLBEntry *cached_entry = NULL; > + int asid, stage; > + bool S2_only = class != SMMU_CLASS_IN; > + > + if (S2_only) { Please add a comment explaining that class value is used to identify S2-only forced translation in the context of a nested translation. In that case we hackily override the original config to reach our goal and then restore the original config. That's pretty hacky. Let's see if any other reviewer has a better idea ;-) on my end I understand it and I can bear the trick :) > + asid = cfg->asid; > + stage = cfg->stage; > + cfg->asid = -1; > + cfg->stage = SMMU_STAGE_2; > + } > > cached_entry = smmu_translate(bs, cfg, addr, flag, &ptw_info); > + > + if (S2_only) { > + cfg->asid = asid; > + cfg->stage = stage; > + } > + > if (!cached_entry) { > /* All faults from PTW has S2 field. */ > event->u.f_walk_eabt.s2 = (ptw_info.stage == SMMU_STAGE_2); > @@ -855,7 +905,7 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr, > event->type = SMMU_EVT_F_TRANSLATION; > event->u.f_translation.addr = addr; > event->u.f_translation.addr2 = ptw_info.addr; > - event->u.f_translation.class = SMMU_CLASS_IN; > + event->u.f_translation.class = class; > event->u.f_translation.rnw = flag & 0x1; > } > break; > @@ -864,7 +914,7 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr, > event->type = SMMU_EVT_F_ADDR_SIZE; > event->u.f_addr_size.addr = addr; > event->u.f_addr_size.addr2 = ptw_info.addr; > - event->u.f_addr_size.class = SMMU_CLASS_IN; > + event->u.f_addr_size.class = class; > event->u.f_addr_size.rnw = flag & 0x1; > } > break; > @@ -873,7 +923,7 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr, > event->type = SMMU_EVT_F_ACCESS; > event->u.f_access.addr = addr; > event->u.f_access.addr2 = ptw_info.addr; > - event->u.f_access.class = SMMU_CLASS_IN; > + event->u.f_access.class = class; > event->u.f_access.rnw = flag & 0x1; > } > break; > @@ -882,7 +932,7 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr, > event->type = SMMU_EVT_F_PERMISSION; > event->u.f_permission.addr = addr; > event->u.f_permission.addr2 = ptw_info.addr; > - event->u.f_permission.class = SMMU_CLASS_IN; > + event->u.f_permission.class = class; > event->u.f_permission.rnw = flag & 0x1; > } > break; > @@ -943,15 +993,15 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, > goto epilogue; > } > > - status = smmuv3_do_translate(s, addr, cfg, &event, flag, &cached_entry); > + status = smmuv3_do_translate(s, addr, cfg, &event, flag, > + &cached_entry, SMMU_CLASS_IN); > > epilogue: > qemu_mutex_unlock(&s->mutex); > switch (status) { > case SMMU_TRANS_SUCCESS: > entry.perm = cached_entry->entry.perm; > - entry.translated_addr = cached_entry->entry.translated_addr + > - (addr & cached_entry->entry.addr_mask); > + entry.translated_addr = CACHED_ENTRY_TO_ADDR(cached_entry, addr); > entry.addr_mask = cached_entry->entry.addr_mask; > trace_smmuv3_translate_success(mr->parent_obj.name, sid, addr, > entry.translated_addr, entry.perm, > diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h > index 96eb017e50..09d3b9e734 100644 > --- a/include/hw/arm/smmu-common.h > +++ b/include/hw/arm/smmu-common.h > @@ -37,6 +37,9 @@ > #define VMSA_IDXMSK(isz, strd, lvl) ((1ULL << \ > VMSA_BIT_LVL(isz, strd, lvl)) - 1) > > +#define CACHED_ENTRY_TO_ADDR(ent, addr) (ent)->entry.translated_addr + \ > + ((addr) & (ent)->entry.addr_mask); > + nit; this could be introduced in a separate patch since you have a caller in smmuv3_translate(). This may help the reviewer to focus on the most important class related changes. Besides Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric > /* > * Page table walk error types > */
Hi Eric, On Wed, May 15, 2024 at 03:15:02PM +0200, Eric Auger wrote: > Hi Mostafa, > > On 4/29/24 05:23, Mostafa Saleh wrote: > > According to ARM SMMU architecture specification (ARM IHI 0070 F.b), > > In "5.2 Stream Table Entry": > > [51:6] S1ContextPtr > > If Config[1] == 1 (stage 2 enabled), this pointer is an IPA translated by > > stage 2 and the programmed value must be within the range of the IAS. > > > > In "5.4.1 CD notes": > > The translation table walks performed from TTB0 or TTB1 are always performed > > in IPA space if stage 2 translations are enabled. > > > > This patch implements translation of the S1 context descriptor pointer and > > TTBx base addresses through the S2 stage (IPA -> PA) > > > > smmuv3_do_translate() is updated to have one arg which is translation > > class, this is useful for: > s/for/to? Will do. > > - Decide wether a translation is stage-2 only or use the STE config. > > - Populate the class in case of faults, WALK_EABT is lefat as it as > left unchanged? Yup, that's a typo. > > it is always triggered from TT access so no need to use the input > > class. > > > > In case for stage-2 only translation, which only used in nesting, the > in case of S2 translation used in the contexted of a nested translation, ... Will do. > > stage and asid are saved and restored before and after calling > > smmu_translate(). > > > > Translating CD or TTBx can fail for the following reasons: > > 1) Large address size: This is described in > > (3.4.3 Address sizes of SMMU-originated accesses) > > - For CD ptr larger than IAS, for SMMUv3.1, it can trigger either > > C_BAD_STE or Translation fault, we implement the latter as it > > requires no extra code. > > - For TTBx, if larger than the effective stage 1 output address size, it > > triggers C_BAD_CD. > > > > 2) Faults from PTWs (7.3 Event records) > > - F_ADDR_SIZE: large address size after first level causes stage 2 Address > > Size fault (Also in 3.4.3 Address sizes of SMMU-originated accesses) > > - F_PERMISSION: Same as an address translation. However, when > > CLASS == CD, the access is implicitly Data and a read. > > - F_ACCESS: Same as an address translation. > > - F_TRANSLATION: Same as an address translation. > > - F_WALK_EABT: Same as an address translation. > > These are already implemented in the PTW logic, so no extra handling > > required. > > > > As, there is multiple locations where the address is calculated from > > cached entry, a new macro is introduced CACHED_ENTRY_TO_ADDR. > > > > Signed-off-by: Mostafa Saleh <smostafa@google.com> > > --- > > hw/arm/smmuv3.c | 76 ++++++++++++++++++++++++++++++------ > > include/hw/arm/smmu-common.h | 3 ++ > > 2 files changed, 66 insertions(+), 13 deletions(-) > > > > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c > > index cc61708160..cc61c82321 100644 > > --- a/hw/arm/smmuv3.c > > +++ b/hw/arm/smmuv3.c > > @@ -337,14 +337,33 @@ static int smmu_get_ste(SMMUv3State *s, dma_addr_t addr, STE *buf, > > > > } > > > > +static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr, > > + SMMUTransCfg *cfg, > > + SMMUEventInfo *event, > > + IOMMUAccessFlags flag, > > + SMMUTLBEntry **out_entry, > > + SMMUTranslationClass class); > > /* @ssid > 0 not supported yet */ > > -static int smmu_get_cd(SMMUv3State *s, STE *ste, uint32_t ssid, > > - CD *buf, SMMUEventInfo *event) > > +static int smmu_get_cd(SMMUv3State *s, STE *ste, SMMUTransCfg *cfg, > > + uint32_t ssid, CD *buf, SMMUEventInfo *event) > > { > > dma_addr_t addr = STE_CTXPTR(ste); > > int ret, i; > > + SMMUTranslationStatus status; > > + SMMUTLBEntry *entry; > > > > trace_smmuv3_get_cd(addr); > > + > > + if (cfg->stage == SMMU_NESTED) { > > + status = smmuv3_do_translate(s, addr, cfg, event, > > + IOMMU_RO, &entry, SMMU_CLASS_CD); > > + if (status != SMMU_TRANS_SUCCESS) { > So I guess you rely on event being populated by above CD S2 translate(). > it does not need to be patched, correct? > May be worth a comment. Yes, only the class is different, I will add a comment. > > + return -EINVAL; > > + } > > + > > + addr = CACHED_ENTRY_TO_ADDR(entry, addr); > > + } > > + > > /* TODO: guarantee 64-bit single-copy atomicity */ > > ret = dma_memory_read(&address_space_memory, addr, buf, sizeof(*buf), > > MEMTXATTRS_UNSPECIFIED); > > @@ -659,10 +678,13 @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste, > > return 0; > > } > > > > -static int decode_cd(SMMUTransCfg *cfg, CD *cd, SMMUEventInfo *event) > > +static int decode_cd(SMMUv3State *s, SMMUTransCfg *cfg, > > + CD *cd, SMMUEventInfo *event) > > { > > int ret = -EINVAL; > > int i; > > + SMMUTranslationStatus status; > > + SMMUTLBEntry *entry; > > > > if (!CD_VALID(cd) || !CD_AARCH64(cd)) { > > goto bad_cd; > > @@ -713,9 +735,21 @@ static int decode_cd(SMMUTransCfg *cfg, CD *cd, SMMUEventInfo *event) > > > > tt->tsz = tsz; > > tt->ttb = CD_TTB(cd, i); > > + > > if (tt->ttb & ~(MAKE_64BIT_MASK(0, cfg->oas))) { > > goto bad_cd; > > } > > + > > + /* Translate the TTBx, from IPA to PA if nesting is enabled. */ > > + if (cfg->stage == SMMU_NESTED) { > > + status = smmuv3_do_translate(s, tt->ttb, cfg, event, IOMMU_RO, > > + &entry, SMMU_CLASS_TT); > > + if (status != SMMU_TRANS_SUCCESS) { > same here. > > + return -EINVAL; > > + } > > + tt->ttb = CACHED_ENTRY_TO_ADDR(entry, tt->ttb); > > + } > > + > > tt->had = CD_HAD(cd, i); > > trace_smmuv3_decode_cd_tt(i, tt->tsz, tt->ttb, tt->granule_sz, tt->had); > > } > > @@ -767,12 +801,12 @@ static int smmuv3_decode_config(IOMMUMemoryRegion *mr, SMMUTransCfg *cfg, > > return 0; > > } > > > > - ret = smmu_get_cd(s, &ste, 0 /* ssid */, &cd, event); > > + ret = smmu_get_cd(s, &ste, cfg, 0 /* ssid */, &cd, event); > > if (ret) { > > return ret; > > } > > > > - return decode_cd(cfg, &cd, event); > > + return decode_cd(s, cfg, &cd, event); > > } > > > > /** > > @@ -832,13 +866,29 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr, > > SMMUTransCfg *cfg, > > SMMUEventInfo *event, > > IOMMUAccessFlags flag, > > - SMMUTLBEntry **out_entry) > > + SMMUTLBEntry **out_entry, > > + SMMUTranslationClass class) > > { > > SMMUPTWEventInfo ptw_info = {}; > > SMMUState *bs = ARM_SMMU(s); > > SMMUTLBEntry *cached_entry = NULL; > > + int asid, stage; > > + bool S2_only = class != SMMU_CLASS_IN; > > + > > + if (S2_only) { > Please add a comment explaining that class value is used to identify > S2-only forced translation in the context of a nested translation. > In that case we hackily override the original config to reach our goal > and then restore the original config. > That's pretty hacky. Let's see if any other reviewer has a better idea > ;-) on my end I understand it and I can bear the trick :) > Indeed, this is unclear, I will add a comment. I thought about this case a lot, I found this to be the least intrusive way while having tolerable readability, it'd be great if someone has a better idea. > > > + asid = cfg->asid; > > + stage = cfg->stage; > > + cfg->asid = -1; > > + cfg->stage = SMMU_STAGE_2; > > + } > > > > cached_entry = smmu_translate(bs, cfg, addr, flag, &ptw_info); > > + > > + if (S2_only) { > > + cfg->asid = asid; > > + cfg->stage = stage; > > + } > > + > > if (!cached_entry) { > > /* All faults from PTW has S2 field. */ > > event->u.f_walk_eabt.s2 = (ptw_info.stage == SMMU_STAGE_2); > > @@ -855,7 +905,7 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr, > > event->type = SMMU_EVT_F_TRANSLATION; > > event->u.f_translation.addr = addr; > > event->u.f_translation.addr2 = ptw_info.addr; > > - event->u.f_translation.class = SMMU_CLASS_IN; > > + event->u.f_translation.class = class; > > event->u.f_translation.rnw = flag & 0x1; > > } > > break; > > @@ -864,7 +914,7 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr, > > event->type = SMMU_EVT_F_ADDR_SIZE; > > event->u.f_addr_size.addr = addr; > > event->u.f_addr_size.addr2 = ptw_info.addr; > > - event->u.f_addr_size.class = SMMU_CLASS_IN; > > + event->u.f_addr_size.class = class; > > event->u.f_addr_size.rnw = flag & 0x1; > > } > > break; > > @@ -873,7 +923,7 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr, > > event->type = SMMU_EVT_F_ACCESS; > > event->u.f_access.addr = addr; > > event->u.f_access.addr2 = ptw_info.addr; > > - event->u.f_access.class = SMMU_CLASS_IN; > > + event->u.f_access.class = class; > > event->u.f_access.rnw = flag & 0x1; > > } > > break; > > @@ -882,7 +932,7 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr, > > event->type = SMMU_EVT_F_PERMISSION; > > event->u.f_permission.addr = addr; > > event->u.f_permission.addr2 = ptw_info.addr; > > - event->u.f_permission.class = SMMU_CLASS_IN; > > + event->u.f_permission.class = class; > > event->u.f_permission.rnw = flag & 0x1; > > } > > break; > > @@ -943,15 +993,15 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, > > goto epilogue; > > } > > > > - status = smmuv3_do_translate(s, addr, cfg, &event, flag, &cached_entry); > > + status = smmuv3_do_translate(s, addr, cfg, &event, flag, > > + &cached_entry, SMMU_CLASS_IN); > > > > epilogue: > > qemu_mutex_unlock(&s->mutex); > > switch (status) { > > case SMMU_TRANS_SUCCESS: > > entry.perm = cached_entry->entry.perm; > > - entry.translated_addr = cached_entry->entry.translated_addr + > > - (addr & cached_entry->entry.addr_mask); > > + entry.translated_addr = CACHED_ENTRY_TO_ADDR(cached_entry, addr); > > entry.addr_mask = cached_entry->entry.addr_mask; > > trace_smmuv3_translate_success(mr->parent_obj.name, sid, addr, > > entry.translated_addr, entry.perm, > > diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h > > index 96eb017e50..09d3b9e734 100644 > > --- a/include/hw/arm/smmu-common.h > > +++ b/include/hw/arm/smmu-common.h > > @@ -37,6 +37,9 @@ > > #define VMSA_IDXMSK(isz, strd, lvl) ((1ULL << \ > > VMSA_BIT_LVL(isz, strd, lvl)) - 1) > > > > +#define CACHED_ENTRY_TO_ADDR(ent, addr) (ent)->entry.translated_addr + \ > > + ((addr) & (ent)->entry.addr_mask); > > + > nit; this could be introduced in a separate patch since you have a > caller in smmuv3_translate(). This may help the reviewer to focus on the > most important class related changes. Sure, I can introduce a small patch before this one with this macro. Thanks, Mostafa > Besides > Reviewed-by: Eric Auger <eric.auger@redhat.com> > > Eric > > > /* > > * Page table walk error types > > */ >
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c index cc61708160..cc61c82321 100644 --- a/hw/arm/smmuv3.c +++ b/hw/arm/smmuv3.c @@ -337,14 +337,33 @@ static int smmu_get_ste(SMMUv3State *s, dma_addr_t addr, STE *buf, } +static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr, + SMMUTransCfg *cfg, + SMMUEventInfo *event, + IOMMUAccessFlags flag, + SMMUTLBEntry **out_entry, + SMMUTranslationClass class); /* @ssid > 0 not supported yet */ -static int smmu_get_cd(SMMUv3State *s, STE *ste, uint32_t ssid, - CD *buf, SMMUEventInfo *event) +static int smmu_get_cd(SMMUv3State *s, STE *ste, SMMUTransCfg *cfg, + uint32_t ssid, CD *buf, SMMUEventInfo *event) { dma_addr_t addr = STE_CTXPTR(ste); int ret, i; + SMMUTranslationStatus status; + SMMUTLBEntry *entry; trace_smmuv3_get_cd(addr); + + if (cfg->stage == SMMU_NESTED) { + status = smmuv3_do_translate(s, addr, cfg, event, + IOMMU_RO, &entry, SMMU_CLASS_CD); + if (status != SMMU_TRANS_SUCCESS) { + return -EINVAL; + } + + addr = CACHED_ENTRY_TO_ADDR(entry, addr); + } + /* TODO: guarantee 64-bit single-copy atomicity */ ret = dma_memory_read(&address_space_memory, addr, buf, sizeof(*buf), MEMTXATTRS_UNSPECIFIED); @@ -659,10 +678,13 @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste, return 0; } -static int decode_cd(SMMUTransCfg *cfg, CD *cd, SMMUEventInfo *event) +static int decode_cd(SMMUv3State *s, SMMUTransCfg *cfg, + CD *cd, SMMUEventInfo *event) { int ret = -EINVAL; int i; + SMMUTranslationStatus status; + SMMUTLBEntry *entry; if (!CD_VALID(cd) || !CD_AARCH64(cd)) { goto bad_cd; @@ -713,9 +735,21 @@ static int decode_cd(SMMUTransCfg *cfg, CD *cd, SMMUEventInfo *event) tt->tsz = tsz; tt->ttb = CD_TTB(cd, i); + if (tt->ttb & ~(MAKE_64BIT_MASK(0, cfg->oas))) { goto bad_cd; } + + /* Translate the TTBx, from IPA to PA if nesting is enabled. */ + if (cfg->stage == SMMU_NESTED) { + status = smmuv3_do_translate(s, tt->ttb, cfg, event, IOMMU_RO, + &entry, SMMU_CLASS_TT); + if (status != SMMU_TRANS_SUCCESS) { + return -EINVAL; + } + tt->ttb = CACHED_ENTRY_TO_ADDR(entry, tt->ttb); + } + tt->had = CD_HAD(cd, i); trace_smmuv3_decode_cd_tt(i, tt->tsz, tt->ttb, tt->granule_sz, tt->had); } @@ -767,12 +801,12 @@ static int smmuv3_decode_config(IOMMUMemoryRegion *mr, SMMUTransCfg *cfg, return 0; } - ret = smmu_get_cd(s, &ste, 0 /* ssid */, &cd, event); + ret = smmu_get_cd(s, &ste, cfg, 0 /* ssid */, &cd, event); if (ret) { return ret; } - return decode_cd(cfg, &cd, event); + return decode_cd(s, cfg, &cd, event); } /** @@ -832,13 +866,29 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr, SMMUTransCfg *cfg, SMMUEventInfo *event, IOMMUAccessFlags flag, - SMMUTLBEntry **out_entry) + SMMUTLBEntry **out_entry, + SMMUTranslationClass class) { SMMUPTWEventInfo ptw_info = {}; SMMUState *bs = ARM_SMMU(s); SMMUTLBEntry *cached_entry = NULL; + int asid, stage; + bool S2_only = class != SMMU_CLASS_IN; + + if (S2_only) { + asid = cfg->asid; + stage = cfg->stage; + cfg->asid = -1; + cfg->stage = SMMU_STAGE_2; + } cached_entry = smmu_translate(bs, cfg, addr, flag, &ptw_info); + + if (S2_only) { + cfg->asid = asid; + cfg->stage = stage; + } + if (!cached_entry) { /* All faults from PTW has S2 field. */ event->u.f_walk_eabt.s2 = (ptw_info.stage == SMMU_STAGE_2); @@ -855,7 +905,7 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr, event->type = SMMU_EVT_F_TRANSLATION; event->u.f_translation.addr = addr; event->u.f_translation.addr2 = ptw_info.addr; - event->u.f_translation.class = SMMU_CLASS_IN; + event->u.f_translation.class = class; event->u.f_translation.rnw = flag & 0x1; } break; @@ -864,7 +914,7 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr, event->type = SMMU_EVT_F_ADDR_SIZE; event->u.f_addr_size.addr = addr; event->u.f_addr_size.addr2 = ptw_info.addr; - event->u.f_addr_size.class = SMMU_CLASS_IN; + event->u.f_addr_size.class = class; event->u.f_addr_size.rnw = flag & 0x1; } break; @@ -873,7 +923,7 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr, event->type = SMMU_EVT_F_ACCESS; event->u.f_access.addr = addr; event->u.f_access.addr2 = ptw_info.addr; - event->u.f_access.class = SMMU_CLASS_IN; + event->u.f_access.class = class; event->u.f_access.rnw = flag & 0x1; } break; @@ -882,7 +932,7 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr, event->type = SMMU_EVT_F_PERMISSION; event->u.f_permission.addr = addr; event->u.f_permission.addr2 = ptw_info.addr; - event->u.f_permission.class = SMMU_CLASS_IN; + event->u.f_permission.class = class; event->u.f_permission.rnw = flag & 0x1; } break; @@ -943,15 +993,15 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, goto epilogue; } - status = smmuv3_do_translate(s, addr, cfg, &event, flag, &cached_entry); + status = smmuv3_do_translate(s, addr, cfg, &event, flag, + &cached_entry, SMMU_CLASS_IN); epilogue: qemu_mutex_unlock(&s->mutex); switch (status) { case SMMU_TRANS_SUCCESS: entry.perm = cached_entry->entry.perm; - entry.translated_addr = cached_entry->entry.translated_addr + - (addr & cached_entry->entry.addr_mask); + entry.translated_addr = CACHED_ENTRY_TO_ADDR(cached_entry, addr); entry.addr_mask = cached_entry->entry.addr_mask; trace_smmuv3_translate_success(mr->parent_obj.name, sid, addr, entry.translated_addr, entry.perm, diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h index 96eb017e50..09d3b9e734 100644 --- a/include/hw/arm/smmu-common.h +++ b/include/hw/arm/smmu-common.h @@ -37,6 +37,9 @@ #define VMSA_IDXMSK(isz, strd, lvl) ((1ULL << \ VMSA_BIT_LVL(isz, strd, lvl)) - 1) +#define CACHED_ENTRY_TO_ADDR(ent, addr) (ent)->entry.translated_addr + \ + ((addr) & (ent)->entry.addr_mask); + /* * Page table walk error types */
According to ARM SMMU architecture specification (ARM IHI 0070 F.b), In "5.2 Stream Table Entry": [51:6] S1ContextPtr If Config[1] == 1 (stage 2 enabled), this pointer is an IPA translated by stage 2 and the programmed value must be within the range of the IAS. In "5.4.1 CD notes": The translation table walks performed from TTB0 or TTB1 are always performed in IPA space if stage 2 translations are enabled. This patch implements translation of the S1 context descriptor pointer and TTBx base addresses through the S2 stage (IPA -> PA) smmuv3_do_translate() is updated to have one arg which is translation class, this is useful for: - Decide wether a translation is stage-2 only or use the STE config. - Populate the class in case of faults, WALK_EABT is lefat as it as it is always triggered from TT access so no need to use the input class. In case for stage-2 only translation, which only used in nesting, the stage and asid are saved and restored before and after calling smmu_translate(). Translating CD or TTBx can fail for the following reasons: 1) Large address size: This is described in (3.4.3 Address sizes of SMMU-originated accesses) - For CD ptr larger than IAS, for SMMUv3.1, it can trigger either C_BAD_STE or Translation fault, we implement the latter as it requires no extra code. - For TTBx, if larger than the effective stage 1 output address size, it triggers C_BAD_CD. 2) Faults from PTWs (7.3 Event records) - F_ADDR_SIZE: large address size after first level causes stage 2 Address Size fault (Also in 3.4.3 Address sizes of SMMU-originated accesses) - F_PERMISSION: Same as an address translation. However, when CLASS == CD, the access is implicitly Data and a read. - F_ACCESS: Same as an address translation. - F_TRANSLATION: Same as an address translation. - F_WALK_EABT: Same as an address translation. These are already implemented in the PTW logic, so no extra handling required. As, there is multiple locations where the address is calculated from cached entry, a new macro is introduced CACHED_ENTRY_TO_ADDR. Signed-off-by: Mostafa Saleh <smostafa@google.com> --- hw/arm/smmuv3.c | 76 ++++++++++++++++++++++++++++++------ include/hw/arm/smmu-common.h | 3 ++ 2 files changed, 66 insertions(+), 13 deletions(-)