| Message ID | 20240701110241.2005222-12-smostafa@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SMMUv3 nested translation support | expand |
On Mon, Jul 01, 2024 at 11:02:33AM +0000, Mostafa Saleh wrote: > When nested translation is requested, do the following: > > - Translate stage-1 table address IPA into PA through stage-2. > - Translate stage-1 table walk output (IPA) through stage-2. > - Create a single TLB entry from stage-1 and stage-2 translations > using logic introduced before. > > For stage-1 table translation, the spec (ARM IHI 0070 F.b) says in: > 7.3.12 F_WALK_EABT: > Translation of an IPA for Stage 1 descriptor fetch: > S2 == 1 (stage 2), CLASS == T > So, F_WALK_EABT is used which propagtes to CLASS == TT. I don't think the text applies to this case, the context is: A stage 2 table walk can encounter EABT accessing the physical address of a stage 2 descriptor, because of a: [...] * Translation of an IPA for Stage 1 descriptor fetch So EABT is when failing to load the stage 2 descriptor. I can't find exact text for this case but looking at the flowchart 15.5, I think this should be F_TRANSLATION/F_ADDR_SIZE/F_PERMISSION/F_ACCESS with CLASS=TT and S2. Thanks, Jean > > smmu_ptw() has a new argument SMMUState which include the TLB as > stage-1 table address can be cached in there. > > Also in smmu_ptw() a separate path used for nesting to simplify the > code, although some logic can be combined. > > Signed-off-by: Mostafa Saleh <smostafa@google.com> > --- > hw/arm/smmu-common.c | 72 +++++++++++++++++++++++++++++++----- > include/hw/arm/smmu-common.h | 2 +- > 2 files changed, 64 insertions(+), 10 deletions(-) > > diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c > index 24b7d09e2b..71afd486ba 100644 > --- a/hw/arm/smmu-common.c > +++ b/hw/arm/smmu-common.c > @@ -318,6 +318,38 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova) > return NULL; > } > > +/* Translate stage-1 table address using stage-2 page table. */ > +static inline int translate_table_addr_ipa(dma_addr_t *table_addr, > + SMMUTransCfg *cfg, > + SMMUPTWEventInfo *info, > + SMMUState *bs) > +{ > + dma_addr_t addr = *table_addr; > + SMMUTLBEntry *cached_entry; > + int asid; > + > + /* > + * The translation table walks performed from TTB0 or TTB1 are always > + * performed in IPA space if stage 2 translations are enabled. > + */ > + asid = cfg->asid; > + cfg->stage = SMMU_STAGE_2; > + cfg->asid = -1; > + cached_entry = smmu_translate(bs, cfg, addr, IOMMU_RO, info); > + cfg->asid = asid; > + cfg->stage = SMMU_NESTED; > + > + if (cached_entry) { > + *table_addr = CACHED_ENTRY_TO_ADDR(cached_entry, addr); > + return 0; > + } > + > + info->stage = SMMU_STAGE_2; > + info->type = SMMU_PTW_ERR_WALK_EABT; > + info->addr = addr; > + return -EINVAL; > +} > + > /** > * smmu_ptw_64_s1 - VMSAv8-64 Walk of the page tables for a given IOVA > * @cfg: translation config > @@ -333,7 +365,8 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova) > */ > static int smmu_ptw_64_s1(SMMUTransCfg *cfg, > dma_addr_t iova, IOMMUAccessFlags perm, > - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info) > + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info, > + SMMUState *bs) > { > dma_addr_t baseaddr, indexmask; > SMMUStage stage = cfg->stage; > @@ -381,6 +414,11 @@ static int smmu_ptw_64_s1(SMMUTransCfg *cfg, > goto error; > } > baseaddr = get_table_pte_address(pte, granule_sz); > + if (cfg->stage == SMMU_NESTED) { > + if (translate_table_addr_ipa(&baseaddr, cfg, info, bs)) { > + goto error; > + } > + } > level++; > continue; > } else if (is_page_pte(pte, level)) { > @@ -568,10 +606,8 @@ error: > * combine S1 and S2 TLB entries into a single entry. > * As a result the S1 entry is overriden with combined data. > */ > -static void __attribute__((unused)) combine_tlb(SMMUTLBEntry *tlbe, > - SMMUTLBEntry *tlbe_s2, > - dma_addr_t iova, > - SMMUTransCfg *cfg) > +static void combine_tlb(SMMUTLBEntry *tlbe, SMMUTLBEntry *tlbe_s2, > + dma_addr_t iova, SMMUTransCfg *cfg) > { > if (tlbe_s2->entry.addr_mask < tlbe->entry.addr_mask) { > tlbe->entry.addr_mask = tlbe_s2->entry.addr_mask; > @@ -596,14 +632,19 @@ static void __attribute__((unused)) combine_tlb(SMMUTLBEntry *tlbe, > * @perm: tentative access type > * @tlbe: returned entry > * @info: ptw event handle > + * @bs: smmu state which includes TLB instance > * > * return 0 on success > */ > int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm, > - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info) > + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info, SMMUState *bs) > { > + int ret; > + SMMUTLBEntry tlbe_s2; > + dma_addr_t ipa; > + > if (cfg->stage == SMMU_STAGE_1) { > - return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info); > + return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info, bs); > } else if (cfg->stage == SMMU_STAGE_2) { > /* > * If bypassing stage 1(or unimplemented), the input address is passed > @@ -621,7 +662,20 @@ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm, > return smmu_ptw_64_s2(cfg, iova, perm, tlbe, info); > } > > - g_assert_not_reached(); > + /* SMMU_NESTED. */ > + ret = smmu_ptw_64_s1(cfg, iova, perm, tlbe, info, bs); > + if (ret) { > + return ret; > + } > + > + ipa = CACHED_ENTRY_TO_ADDR(tlbe, iova); > + ret = smmu_ptw_64_s2(cfg, ipa, perm, &tlbe_s2, info); > + if (ret) { > + return ret; > + } > + > + combine_tlb(tlbe, &tlbe_s2, iova, cfg); > + return 0; > } > > SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr, > @@ -677,7 +731,7 @@ SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr, > } > > cached_entry = g_new0(SMMUTLBEntry, 1); > - status = smmu_ptw(cfg, aligned_addr, flag, cached_entry, info); > + status = smmu_ptw(cfg, aligned_addr, flag, cached_entry, info, bs); > if (status) { > g_free(cached_entry); > return NULL; > diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h > index 1db566d451..cf0fd3ec74 100644 > --- a/include/hw/arm/smmu-common.h > +++ b/include/hw/arm/smmu-common.h > @@ -185,7 +185,7 @@ static inline uint16_t smmu_get_sid(SMMUDevice *sdev) > * pair, according to @cfg translation config > */ > int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm, > - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info); > + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info, SMMUState *bs); > > > /* > -- > 2.45.2.803.g4e1b14247a-goog >
Hi Mostafa, On 7/1/24 13:02, Mostafa Saleh wrote: > When nested translation is requested, do the following: > > - Translate stage-1 table address IPA into PA through stage-2. > - Translate stage-1 table walk output (IPA) through stage-2. > - Create a single TLB entry from stage-1 and stage-2 translations > using logic introduced before. > > For stage-1 table translation, the spec (ARM IHI 0070 F.b) says in: > 7.3.12 F_WALK_EABT: > Translation of an IPA for Stage 1 descriptor fetch: > S2 == 1 (stage 2), CLASS == T > So, F_WALK_EABT is used which propagtes to CLASS == TT. > > smmu_ptw() has a new argument SMMUState which include the TLB as > stage-1 table address can be cached in there. > > Also in smmu_ptw() a separate path used for nesting to simplify the > code, although some logic can be combined. > > Signed-off-by: Mostafa Saleh <smostafa@google.com> > --- > hw/arm/smmu-common.c | 72 +++++++++++++++++++++++++++++++----- > include/hw/arm/smmu-common.h | 2 +- > 2 files changed, 64 insertions(+), 10 deletions(-) > > diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c > index 24b7d09e2b..71afd486ba 100644 > --- a/hw/arm/smmu-common.c > +++ b/hw/arm/smmu-common.c > @@ -318,6 +318,38 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova) > return NULL; > } > > +/* Translate stage-1 table address using stage-2 page table. */ > +static inline int translate_table_addr_ipa(dma_addr_t *table_addr, > + SMMUTransCfg *cfg, > + SMMUPTWEventInfo *info, > + SMMUState *bs) Nit: in general the SMMUState if the 1st arg, as the most global state. > +{ > + dma_addr_t addr = *table_addr; > + SMMUTLBEntry *cached_entry; > + int asid; > + > + /* > + * The translation table walks performed from TTB0 or TTB1 are always > + * performed in IPA space if stage 2 translations are enabled. > + */ > + asid = cfg->asid; > + cfg->stage = SMMU_STAGE_2; > + cfg->asid = -1; > + cached_entry = smmu_translate(bs, cfg, addr, IOMMU_RO, info); > + cfg->asid = asid; > + cfg->stage = SMMU_NESTED; > + > + if (cached_entry) { > + *table_addr = CACHED_ENTRY_TO_ADDR(cached_entry, addr); > + return 0; > + } > + > + info->stage = SMMU_STAGE_2; > + info->type = SMMU_PTW_ERR_WALK_EABT; > + info->addr = addr; so I guess also here the recorded address should be the IOVA (Jean's previous comment)? Eric > + return -EINVAL; > +} > + > /** > * smmu_ptw_64_s1 - VMSAv8-64 Walk of the page tables for a given IOVA > * @cfg: translation config > @@ -333,7 +365,8 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova) > */ > static int smmu_ptw_64_s1(SMMUTransCfg *cfg, > dma_addr_t iova, IOMMUAccessFlags perm, > - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info) > + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info, > + SMMUState *bs) > { > dma_addr_t baseaddr, indexmask; > SMMUStage stage = cfg->stage; > @@ -381,6 +414,11 @@ static int smmu_ptw_64_s1(SMMUTransCfg *cfg, > goto error; > } > baseaddr = get_table_pte_address(pte, granule_sz); > + if (cfg->stage == SMMU_NESTED) { > + if (translate_table_addr_ipa(&baseaddr, cfg, info, bs)) { > + goto error; > + } > + } > level++; > continue; > } else if (is_page_pte(pte, level)) { > @@ -568,10 +606,8 @@ error: > * combine S1 and S2 TLB entries into a single entry. > * As a result the S1 entry is overriden with combined data. > */ > -static void __attribute__((unused)) combine_tlb(SMMUTLBEntry *tlbe, > - SMMUTLBEntry *tlbe_s2, > - dma_addr_t iova, > - SMMUTransCfg *cfg) > +static void combine_tlb(SMMUTLBEntry *tlbe, SMMUTLBEntry *tlbe_s2, > + dma_addr_t iova, SMMUTransCfg *cfg) > { > if (tlbe_s2->entry.addr_mask < tlbe->entry.addr_mask) { > tlbe->entry.addr_mask = tlbe_s2->entry.addr_mask; > @@ -596,14 +632,19 @@ static void __attribute__((unused)) combine_tlb(SMMUTLBEntry *tlbe, > * @perm: tentative access type > * @tlbe: returned entry > * @info: ptw event handle > + * @bs: smmu state which includes TLB instance > * > * return 0 on success > */ > int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm, > - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info) > + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info, SMMUState *bs) > { > + int ret; > + SMMUTLBEntry tlbe_s2; > + dma_addr_t ipa; > + > if (cfg->stage == SMMU_STAGE_1) { > - return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info); > + return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info, bs); > } else if (cfg->stage == SMMU_STAGE_2) { > /* > * If bypassing stage 1(or unimplemented), the input address is passed > @@ -621,7 +662,20 @@ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm, > return smmu_ptw_64_s2(cfg, iova, perm, tlbe, info); > } > > - g_assert_not_reached(); > + /* SMMU_NESTED. */ > + ret = smmu_ptw_64_s1(cfg, iova, perm, tlbe, info, bs); > + if (ret) { > + return ret; > + } > + > + ipa = CACHED_ENTRY_TO_ADDR(tlbe, iova); > + ret = smmu_ptw_64_s2(cfg, ipa, perm, &tlbe_s2, info); > + if (ret) { > + return ret; > + } > + > + combine_tlb(tlbe, &tlbe_s2, iova, cfg); > + return 0; > } > > SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr, > @@ -677,7 +731,7 @@ SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr, > } > > cached_entry = g_new0(SMMUTLBEntry, 1); > - status = smmu_ptw(cfg, aligned_addr, flag, cached_entry, info); > + status = smmu_ptw(cfg, aligned_addr, flag, cached_entry, info, bs); > if (status) { > g_free(cached_entry); > return NULL; > diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h > index 1db566d451..cf0fd3ec74 100644 > --- a/include/hw/arm/smmu-common.h > +++ b/include/hw/arm/smmu-common.h > @@ -185,7 +185,7 @@ static inline uint16_t smmu_get_sid(SMMUDevice *sdev) > * pair, according to @cfg translation config > */ > int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm, > - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info); > + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info, SMMUState *bs); > > > /*
Hi Jean, On Thu, Jul 04, 2024 at 07:31:10PM +0100, Jean-Philippe Brucker wrote: > On Mon, Jul 01, 2024 at 11:02:33AM +0000, Mostafa Saleh wrote: > > When nested translation is requested, do the following: > > > > - Translate stage-1 table address IPA into PA through stage-2. > > - Translate stage-1 table walk output (IPA) through stage-2. > > - Create a single TLB entry from stage-1 and stage-2 translations > > using logic introduced before. > > > > For stage-1 table translation, the spec (ARM IHI 0070 F.b) says in: > > 7.3.12 F_WALK_EABT: > > Translation of an IPA for Stage 1 descriptor fetch: > > S2 == 1 (stage 2), CLASS == T > > So, F_WALK_EABT is used which propagtes to CLASS == TT. > > I don't think the text applies to this case, the context is: > > A stage 2 table walk can encounter EABT accessing the physical > address of a stage 2 descriptor, because of a: > [...] > * Translation of an IPA for Stage 1 descriptor fetch > > So EABT is when failing to load the stage 2 descriptor. I can't find > exact text for this case but looking at the flowchart 15.5, I think > this should be F_TRANSLATION/F_ADDR_SIZE/F_PERMISSION/F_ACCESS with > CLASS=TT and S2. I see, thanks for clarifying, I guess that would be another argument that we propagate more info in SMMUPTWEventInfo so we can set the fault CLASS. Thanks, Mostafa > > Thanks, > Jean > > > > > smmu_ptw() has a new argument SMMUState which include the TLB as > > stage-1 table address can be cached in there. > > > > Also in smmu_ptw() a separate path used for nesting to simplify the > > code, although some logic can be combined. > > > > Signed-off-by: Mostafa Saleh <smostafa@google.com> > > --- > > hw/arm/smmu-common.c | 72 +++++++++++++++++++++++++++++++----- > > include/hw/arm/smmu-common.h | 2 +- > > 2 files changed, 64 insertions(+), 10 deletions(-) > > > > diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c > > index 24b7d09e2b..71afd486ba 100644 > > --- a/hw/arm/smmu-common.c > > +++ b/hw/arm/smmu-common.c > > @@ -318,6 +318,38 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova) > > return NULL; > > } > > > > +/* Translate stage-1 table address using stage-2 page table. */ > > +static inline int translate_table_addr_ipa(dma_addr_t *table_addr, > > + SMMUTransCfg *cfg, > > + SMMUPTWEventInfo *info, > > + SMMUState *bs) > > +{ > > + dma_addr_t addr = *table_addr; > > + SMMUTLBEntry *cached_entry; > > + int asid; > > + > > + /* > > + * The translation table walks performed from TTB0 or TTB1 are always > > + * performed in IPA space if stage 2 translations are enabled. > > + */ > > + asid = cfg->asid; > > + cfg->stage = SMMU_STAGE_2; > > + cfg->asid = -1; > > + cached_entry = smmu_translate(bs, cfg, addr, IOMMU_RO, info); > > + cfg->asid = asid; > > + cfg->stage = SMMU_NESTED; > > + > > + if (cached_entry) { > > + *table_addr = CACHED_ENTRY_TO_ADDR(cached_entry, addr); > > + return 0; > > + } > > + > > + info->stage = SMMU_STAGE_2; > > + info->type = SMMU_PTW_ERR_WALK_EABT; > > + info->addr = addr; > > + return -EINVAL; > > +} > > + > > /** > > * smmu_ptw_64_s1 - VMSAv8-64 Walk of the page tables for a given IOVA > > * @cfg: translation config > > @@ -333,7 +365,8 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova) > > */ > > static int smmu_ptw_64_s1(SMMUTransCfg *cfg, > > dma_addr_t iova, IOMMUAccessFlags perm, > > - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info) > > + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info, > > + SMMUState *bs) > > { > > dma_addr_t baseaddr, indexmask; > > SMMUStage stage = cfg->stage; > > @@ -381,6 +414,11 @@ static int smmu_ptw_64_s1(SMMUTransCfg *cfg, > > goto error; > > } > > baseaddr = get_table_pte_address(pte, granule_sz); > > + if (cfg->stage == SMMU_NESTED) { > > + if (translate_table_addr_ipa(&baseaddr, cfg, info, bs)) { > > + goto error; > > + } > > + } > > level++; > > continue; > > } else if (is_page_pte(pte, level)) { > > @@ -568,10 +606,8 @@ error: > > * combine S1 and S2 TLB entries into a single entry. > > * As a result the S1 entry is overriden with combined data. > > */ > > -static void __attribute__((unused)) combine_tlb(SMMUTLBEntry *tlbe, > > - SMMUTLBEntry *tlbe_s2, > > - dma_addr_t iova, > > - SMMUTransCfg *cfg) > > +static void combine_tlb(SMMUTLBEntry *tlbe, SMMUTLBEntry *tlbe_s2, > > + dma_addr_t iova, SMMUTransCfg *cfg) > > { > > if (tlbe_s2->entry.addr_mask < tlbe->entry.addr_mask) { > > tlbe->entry.addr_mask = tlbe_s2->entry.addr_mask; > > @@ -596,14 +632,19 @@ static void __attribute__((unused)) combine_tlb(SMMUTLBEntry *tlbe, > > * @perm: tentative access type > > * @tlbe: returned entry > > * @info: ptw event handle > > + * @bs: smmu state which includes TLB instance > > * > > * return 0 on success > > */ > > int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm, > > - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info) > > + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info, SMMUState *bs) > > { > > + int ret; > > + SMMUTLBEntry tlbe_s2; > > + dma_addr_t ipa; > > + > > if (cfg->stage == SMMU_STAGE_1) { > > - return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info); > > + return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info, bs); > > } else if (cfg->stage == SMMU_STAGE_2) { > > /* > > * If bypassing stage 1(or unimplemented), the input address is passed > > @@ -621,7 +662,20 @@ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm, > > return smmu_ptw_64_s2(cfg, iova, perm, tlbe, info); > > } > > > > - g_assert_not_reached(); > > + /* SMMU_NESTED. */ > > + ret = smmu_ptw_64_s1(cfg, iova, perm, tlbe, info, bs); > > + if (ret) { > > + return ret; > > + } > > + > > + ipa = CACHED_ENTRY_TO_ADDR(tlbe, iova); > > + ret = smmu_ptw_64_s2(cfg, ipa, perm, &tlbe_s2, info); > > + if (ret) { > > + return ret; > > + } > > + > > + combine_tlb(tlbe, &tlbe_s2, iova, cfg); > > + return 0; > > } > > > > SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr, > > @@ -677,7 +731,7 @@ SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr, > > } > > > > cached_entry = g_new0(SMMUTLBEntry, 1); > > - status = smmu_ptw(cfg, aligned_addr, flag, cached_entry, info); > > + status = smmu_ptw(cfg, aligned_addr, flag, cached_entry, info, bs); > > if (status) { > > g_free(cached_entry); > > return NULL; > > diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h > > index 1db566d451..cf0fd3ec74 100644 > > --- a/include/hw/arm/smmu-common.h > > +++ b/include/hw/arm/smmu-common.h > > @@ -185,7 +185,7 @@ static inline uint16_t smmu_get_sid(SMMUDevice *sdev) > > * pair, according to @cfg translation config > > */ > > int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm, > > - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info); > > + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info, SMMUState *bs); > > > > > > /* > > -- > > 2.45.2.803.g4e1b14247a-goog > >
Hi Eric, On Mon, Jul 08, 2024 at 05:19:59PM +0200, Eric Auger wrote: > Hi Mostafa, > > On 7/1/24 13:02, Mostafa Saleh wrote: > > When nested translation is requested, do the following: > > > > - Translate stage-1 table address IPA into PA through stage-2. > > - Translate stage-1 table walk output (IPA) through stage-2. > > - Create a single TLB entry from stage-1 and stage-2 translations > > using logic introduced before. > > > > For stage-1 table translation, the spec (ARM IHI 0070 F.b) says in: > > 7.3.12 F_WALK_EABT: > > Translation of an IPA for Stage 1 descriptor fetch: > > S2 == 1 (stage 2), CLASS == T > > So, F_WALK_EABT is used which propagtes to CLASS == TT. > > > > smmu_ptw() has a new argument SMMUState which include the TLB as > > stage-1 table address can be cached in there. > > > > Also in smmu_ptw() a separate path used for nesting to simplify the > > code, although some logic can be combined. > > > > Signed-off-by: Mostafa Saleh <smostafa@google.com> > > --- > > hw/arm/smmu-common.c | 72 +++++++++++++++++++++++++++++++----- > > include/hw/arm/smmu-common.h | 2 +- > > 2 files changed, 64 insertions(+), 10 deletions(-) > > > > diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c > > index 24b7d09e2b..71afd486ba 100644 > > --- a/hw/arm/smmu-common.c > > +++ b/hw/arm/smmu-common.c > > @@ -318,6 +318,38 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova) > > return NULL; > > } > > > > +/* Translate stage-1 table address using stage-2 page table. */ > > +static inline int translate_table_addr_ipa(dma_addr_t *table_addr, > > + SMMUTransCfg *cfg, > > + SMMUPTWEventInfo *info, > > + SMMUState *bs) > Nit: in general the SMMUState if the 1st arg, as the most global state. > > +{ > > + dma_addr_t addr = *table_addr; > > + SMMUTLBEntry *cached_entry; > > + int asid; > > + > > + /* > > + * The translation table walks performed from TTB0 or TTB1 are always > > + * performed in IPA space if stage 2 translations are enabled. > > + */ > > + asid = cfg->asid; > > + cfg->stage = SMMU_STAGE_2; > > + cfg->asid = -1; > > + cached_entry = smmu_translate(bs, cfg, addr, IOMMU_RO, info); > > + cfg->asid = asid; > > + cfg->stage = SMMU_NESTED; > > + > > + if (cached_entry) { > > + *table_addr = CACHED_ENTRY_TO_ADDR(cached_entry, addr); > > + return 0; > > + } > > + > > + info->stage = SMMU_STAGE_2; > > + info->type = SMMU_PTW_ERR_WALK_EABT; > > + info->addr = addr; > so I guess also here the recorded address should be the IOVA (Jean's > previous comment)? This address maps to FetchAddr and not InputAddr, which is set from the calling function, so that should be correct. (besides event type as Jean mentioned it needs be fixed). Thanks, Mostafa > > Eric > > + return -EINVAL; > > +} > > + > > /** > > * smmu_ptw_64_s1 - VMSAv8-64 Walk of the page tables for a given IOVA > > * @cfg: translation config > > @@ -333,7 +365,8 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova) > > */ > > static int smmu_ptw_64_s1(SMMUTransCfg *cfg, > > dma_addr_t iova, IOMMUAccessFlags perm, > > - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info) > > + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info, > > + SMMUState *bs) > > { > > dma_addr_t baseaddr, indexmask; > > SMMUStage stage = cfg->stage; > > @@ -381,6 +414,11 @@ static int smmu_ptw_64_s1(SMMUTransCfg *cfg, > > goto error; > > } > > baseaddr = get_table_pte_address(pte, granule_sz); > > + if (cfg->stage == SMMU_NESTED) { > > + if (translate_table_addr_ipa(&baseaddr, cfg, info, bs)) { > > + goto error; > > + } > > + } > > level++; > > continue; > > } else if (is_page_pte(pte, level)) { > > @@ -568,10 +606,8 @@ error: > > * combine S1 and S2 TLB entries into a single entry. > > * As a result the S1 entry is overriden with combined data. > > */ > > -static void __attribute__((unused)) combine_tlb(SMMUTLBEntry *tlbe, > > - SMMUTLBEntry *tlbe_s2, > > - dma_addr_t iova, > > - SMMUTransCfg *cfg) > > +static void combine_tlb(SMMUTLBEntry *tlbe, SMMUTLBEntry *tlbe_s2, > > + dma_addr_t iova, SMMUTransCfg *cfg) > > { > > if (tlbe_s2->entry.addr_mask < tlbe->entry.addr_mask) { > > tlbe->entry.addr_mask = tlbe_s2->entry.addr_mask; > > @@ -596,14 +632,19 @@ static void __attribute__((unused)) combine_tlb(SMMUTLBEntry *tlbe, > > * @perm: tentative access type > > * @tlbe: returned entry > > * @info: ptw event handle > > + * @bs: smmu state which includes TLB instance > > * > > * return 0 on success > > */ > > int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm, > > - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info) > > + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info, SMMUState *bs) > > { > > + int ret; > > + SMMUTLBEntry tlbe_s2; > > + dma_addr_t ipa; > > + > > if (cfg->stage == SMMU_STAGE_1) { > > - return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info); > > + return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info, bs); > > } else if (cfg->stage == SMMU_STAGE_2) { > > /* > > * If bypassing stage 1(or unimplemented), the input address is passed > > @@ -621,7 +662,20 @@ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm, > > return smmu_ptw_64_s2(cfg, iova, perm, tlbe, info); > > } > > > > - g_assert_not_reached(); > > + /* SMMU_NESTED. */ > > + ret = smmu_ptw_64_s1(cfg, iova, perm, tlbe, info, bs); > > + if (ret) { > > + return ret; > > + } > > + > > + ipa = CACHED_ENTRY_TO_ADDR(tlbe, iova); > > + ret = smmu_ptw_64_s2(cfg, ipa, perm, &tlbe_s2, info); > > + if (ret) { > > + return ret; > > + } > > + > > + combine_tlb(tlbe, &tlbe_s2, iova, cfg); > > + return 0; > > } > > > > SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr, > > @@ -677,7 +731,7 @@ SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr, > > } > > > > cached_entry = g_new0(SMMUTLBEntry, 1); > > - status = smmu_ptw(cfg, aligned_addr, flag, cached_entry, info); > > + status = smmu_ptw(cfg, aligned_addr, flag, cached_entry, info, bs); > > if (status) { > > g_free(cached_entry); > > return NULL; > > diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h > > index 1db566d451..cf0fd3ec74 100644 > > --- a/include/hw/arm/smmu-common.h > > +++ b/include/hw/arm/smmu-common.h > > @@ -185,7 +185,7 @@ static inline uint16_t smmu_get_sid(SMMUDevice *sdev) > > * pair, according to @cfg translation config > > */ > > int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm, > > - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info); > > + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info, SMMUState *bs); > > > > > > /* >
Hi Mostafa, On 7/9/24 09:18, Mostafa Saleh wrote: > Hi Eric, > > On Mon, Jul 08, 2024 at 05:19:59PM +0200, Eric Auger wrote: >> Hi Mostafa, >> >> On 7/1/24 13:02, Mostafa Saleh wrote: >>> When nested translation is requested, do the following: >>> >>> - Translate stage-1 table address IPA into PA through stage-2. >>> - Translate stage-1 table walk output (IPA) through stage-2. >>> - Create a single TLB entry from stage-1 and stage-2 translations >>> using logic introduced before. >>> >>> For stage-1 table translation, the spec (ARM IHI 0070 F.b) says in: >>> 7.3.12 F_WALK_EABT: >>> Translation of an IPA for Stage 1 descriptor fetch: >>> S2 == 1 (stage 2), CLASS == T >>> So, F_WALK_EABT is used which propagtes to CLASS == TT. >>> >>> smmu_ptw() has a new argument SMMUState which include the TLB as >>> stage-1 table address can be cached in there. >>> >>> Also in smmu_ptw() a separate path used for nesting to simplify the >>> code, although some logic can be combined. >>> >>> Signed-off-by: Mostafa Saleh <smostafa@google.com> >>> --- >>> hw/arm/smmu-common.c | 72 +++++++++++++++++++++++++++++++----- >>> include/hw/arm/smmu-common.h | 2 +- >>> 2 files changed, 64 insertions(+), 10 deletions(-) >>> >>> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c >>> index 24b7d09e2b..71afd486ba 100644 >>> --- a/hw/arm/smmu-common.c >>> +++ b/hw/arm/smmu-common.c >>> @@ -318,6 +318,38 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova) >>> return NULL; >>> } >>> >>> +/* Translate stage-1 table address using stage-2 page table. */ >>> +static inline int translate_table_addr_ipa(dma_addr_t *table_addr, >>> + SMMUTransCfg *cfg, >>> + SMMUPTWEventInfo *info, >>> + SMMUState *bs) >> Nit: in general the SMMUState if the 1st arg, as the most global state. >>> +{ >>> + dma_addr_t addr = *table_addr; >>> + SMMUTLBEntry *cached_entry; >>> + int asid; >>> + >>> + /* >>> + * The translation table walks performed from TTB0 or TTB1 are always >>> + * performed in IPA space if stage 2 translations are enabled. >>> + */ >>> + asid = cfg->asid; >>> + cfg->stage = SMMU_STAGE_2; >>> + cfg->asid = -1; >>> + cached_entry = smmu_translate(bs, cfg, addr, IOMMU_RO, info); >>> + cfg->asid = asid; >>> + cfg->stage = SMMU_NESTED; >>> + >>> + if (cached_entry) { >>> + *table_addr = CACHED_ENTRY_TO_ADDR(cached_entry, addr); >>> + return 0; >>> + } >>> + >>> + info->stage = SMMU_STAGE_2; >>> + info->type = SMMU_PTW_ERR_WALK_EABT; >>> + info->addr = addr; >> so I guess also here the recorded address should be the IOVA (Jean's >> previous comment)? > This address maps to FetchAddr and not InputAddr, which is set from the > calling function, so that should be correct. (besides event type as Jean > mentioned it needs be fixed). Ah OK I mixed them up. Sorry for the noise then Eric > > Thanks, > Mostafa > >> Eric >>> + return -EINVAL; >>> +} >>> + >>> /** >>> * smmu_ptw_64_s1 - VMSAv8-64 Walk of the page tables for a given IOVA >>> * @cfg: translation config >>> @@ -333,7 +365,8 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova) >>> */ >>> static int smmu_ptw_64_s1(SMMUTransCfg *cfg, >>> dma_addr_t iova, IOMMUAccessFlags perm, >>> - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info) >>> + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info, >>> + SMMUState *bs) >>> { >>> dma_addr_t baseaddr, indexmask; >>> SMMUStage stage = cfg->stage; >>> @@ -381,6 +414,11 @@ static int smmu_ptw_64_s1(SMMUTransCfg *cfg, >>> goto error; >>> } >>> baseaddr = get_table_pte_address(pte, granule_sz); >>> + if (cfg->stage == SMMU_NESTED) { >>> + if (translate_table_addr_ipa(&baseaddr, cfg, info, bs)) { >>> + goto error; >>> + } >>> + } >>> level++; >>> continue; >>> } else if (is_page_pte(pte, level)) { >>> @@ -568,10 +606,8 @@ error: >>> * combine S1 and S2 TLB entries into a single entry. >>> * As a result the S1 entry is overriden with combined data. >>> */ >>> -static void __attribute__((unused)) combine_tlb(SMMUTLBEntry *tlbe, >>> - SMMUTLBEntry *tlbe_s2, >>> - dma_addr_t iova, >>> - SMMUTransCfg *cfg) >>> +static void combine_tlb(SMMUTLBEntry *tlbe, SMMUTLBEntry *tlbe_s2, >>> + dma_addr_t iova, SMMUTransCfg *cfg) >>> { >>> if (tlbe_s2->entry.addr_mask < tlbe->entry.addr_mask) { >>> tlbe->entry.addr_mask = tlbe_s2->entry.addr_mask; >>> @@ -596,14 +632,19 @@ static void __attribute__((unused)) combine_tlb(SMMUTLBEntry *tlbe, >>> * @perm: tentative access type >>> * @tlbe: returned entry >>> * @info: ptw event handle >>> + * @bs: smmu state which includes TLB instance >>> * >>> * return 0 on success >>> */ >>> int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm, >>> - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info) >>> + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info, SMMUState *bs) >>> { >>> + int ret; >>> + SMMUTLBEntry tlbe_s2; >>> + dma_addr_t ipa; >>> + >>> if (cfg->stage == SMMU_STAGE_1) { >>> - return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info); >>> + return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info, bs); >>> } else if (cfg->stage == SMMU_STAGE_2) { >>> /* >>> * If bypassing stage 1(or unimplemented), the input address is passed >>> @@ -621,7 +662,20 @@ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm, >>> return smmu_ptw_64_s2(cfg, iova, perm, tlbe, info); >>> } >>> >>> - g_assert_not_reached(); >>> + /* SMMU_NESTED. */ >>> + ret = smmu_ptw_64_s1(cfg, iova, perm, tlbe, info, bs); >>> + if (ret) { >>> + return ret; >>> + } >>> + >>> + ipa = CACHED_ENTRY_TO_ADDR(tlbe, iova); >>> + ret = smmu_ptw_64_s2(cfg, ipa, perm, &tlbe_s2, info); >>> + if (ret) { >>> + return ret; >>> + } >>> + >>> + combine_tlb(tlbe, &tlbe_s2, iova, cfg); >>> + return 0; >>> } >>> >>> SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr, >>> @@ -677,7 +731,7 @@ SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr, >>> } >>> >>> cached_entry = g_new0(SMMUTLBEntry, 1); >>> - status = smmu_ptw(cfg, aligned_addr, flag, cached_entry, info); >>> + status = smmu_ptw(cfg, aligned_addr, flag, cached_entry, info, bs); >>> if (status) { >>> g_free(cached_entry); >>> return NULL; >>> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h >>> index 1db566d451..cf0fd3ec74 100644 >>> --- a/include/hw/arm/smmu-common.h >>> +++ b/include/hw/arm/smmu-common.h >>> @@ -185,7 +185,7 @@ static inline uint16_t smmu_get_sid(SMMUDevice *sdev) >>> * pair, according to @cfg translation config >>> */ >>> int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm, >>> - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info); >>> + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info, SMMUState *bs); >>> >>> >>> /*
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c index 24b7d09e2b..71afd486ba 100644 --- a/hw/arm/smmu-common.c +++ b/hw/arm/smmu-common.c @@ -318,6 +318,38 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova) return NULL; } +/* Translate stage-1 table address using stage-2 page table. */ +static inline int translate_table_addr_ipa(dma_addr_t *table_addr, + SMMUTransCfg *cfg, + SMMUPTWEventInfo *info, + SMMUState *bs) +{ + dma_addr_t addr = *table_addr; + SMMUTLBEntry *cached_entry; + int asid; + + /* + * The translation table walks performed from TTB0 or TTB1 are always + * performed in IPA space if stage 2 translations are enabled. + */ + asid = cfg->asid; + cfg->stage = SMMU_STAGE_2; + cfg->asid = -1; + cached_entry = smmu_translate(bs, cfg, addr, IOMMU_RO, info); + cfg->asid = asid; + cfg->stage = SMMU_NESTED; + + if (cached_entry) { + *table_addr = CACHED_ENTRY_TO_ADDR(cached_entry, addr); + return 0; + } + + info->stage = SMMU_STAGE_2; + info->type = SMMU_PTW_ERR_WALK_EABT; + info->addr = addr; + return -EINVAL; +} + /** * smmu_ptw_64_s1 - VMSAv8-64 Walk of the page tables for a given IOVA * @cfg: translation config @@ -333,7 +365,8 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova) */ static int smmu_ptw_64_s1(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm, - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info) + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info, + SMMUState *bs) { dma_addr_t baseaddr, indexmask; SMMUStage stage = cfg->stage; @@ -381,6 +414,11 @@ static int smmu_ptw_64_s1(SMMUTransCfg *cfg, goto error; } baseaddr = get_table_pte_address(pte, granule_sz); + if (cfg->stage == SMMU_NESTED) { + if (translate_table_addr_ipa(&baseaddr, cfg, info, bs)) { + goto error; + } + } level++; continue; } else if (is_page_pte(pte, level)) { @@ -568,10 +606,8 @@ error: * combine S1 and S2 TLB entries into a single entry. * As a result the S1 entry is overriden with combined data. */ -static void __attribute__((unused)) combine_tlb(SMMUTLBEntry *tlbe, - SMMUTLBEntry *tlbe_s2, - dma_addr_t iova, - SMMUTransCfg *cfg) +static void combine_tlb(SMMUTLBEntry *tlbe, SMMUTLBEntry *tlbe_s2, + dma_addr_t iova, SMMUTransCfg *cfg) { if (tlbe_s2->entry.addr_mask < tlbe->entry.addr_mask) { tlbe->entry.addr_mask = tlbe_s2->entry.addr_mask; @@ -596,14 +632,19 @@ static void __attribute__((unused)) combine_tlb(SMMUTLBEntry *tlbe, * @perm: tentative access type * @tlbe: returned entry * @info: ptw event handle + * @bs: smmu state which includes TLB instance * * return 0 on success */ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm, - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info) + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info, SMMUState *bs) { + int ret; + SMMUTLBEntry tlbe_s2; + dma_addr_t ipa; + if (cfg->stage == SMMU_STAGE_1) { - return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info); + return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info, bs); } else if (cfg->stage == SMMU_STAGE_2) { /* * If bypassing stage 1(or unimplemented), the input address is passed @@ -621,7 +662,20 @@ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm, return smmu_ptw_64_s2(cfg, iova, perm, tlbe, info); } - g_assert_not_reached(); + /* SMMU_NESTED. */ + ret = smmu_ptw_64_s1(cfg, iova, perm, tlbe, info, bs); + if (ret) { + return ret; + } + + ipa = CACHED_ENTRY_TO_ADDR(tlbe, iova); + ret = smmu_ptw_64_s2(cfg, ipa, perm, &tlbe_s2, info); + if (ret) { + return ret; + } + + combine_tlb(tlbe, &tlbe_s2, iova, cfg); + return 0; } SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr, @@ -677,7 +731,7 @@ SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr, } cached_entry = g_new0(SMMUTLBEntry, 1); - status = smmu_ptw(cfg, aligned_addr, flag, cached_entry, info); + status = smmu_ptw(cfg, aligned_addr, flag, cached_entry, info, bs); if (status) { g_free(cached_entry); return NULL; diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h index 1db566d451..cf0fd3ec74 100644 --- a/include/hw/arm/smmu-common.h +++ b/include/hw/arm/smmu-common.h @@ -185,7 +185,7 @@ static inline uint16_t smmu_get_sid(SMMUDevice *sdev) * pair, according to @cfg translation config */ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm, - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info); + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info, SMMUState *bs); /*
When nested translation is requested, do the following: - Translate stage-1 table address IPA into PA through stage-2. - Translate stage-1 table walk output (IPA) through stage-2. - Create a single TLB entry from stage-1 and stage-2 translations using logic introduced before. For stage-1 table translation, the spec (ARM IHI 0070 F.b) says in: 7.3.12 F_WALK_EABT: Translation of an IPA for Stage 1 descriptor fetch: S2 == 1 (stage 2), CLASS == T So, F_WALK_EABT is used which propagtes to CLASS == TT. smmu_ptw() has a new argument SMMUState which include the TLB as stage-1 table address can be cached in there. Also in smmu_ptw() a separate path used for nesting to simplify the code, although some logic can be combined. Signed-off-by: Mostafa Saleh <smostafa@google.com> --- hw/arm/smmu-common.c | 72 +++++++++++++++++++++++++++++++----- include/hw/arm/smmu-common.h | 2 +- 2 files changed, 64 insertions(+), 10 deletions(-)