Hi Mostafa, On 2/5/23 10:44, Mostafa Saleh wrote: > Right now, either stage-1 or stage-2 are supported, this simplifies > how we can deal with TLBs. > This patch makes TLB lookup work if stage-2 is enabled instead of > stage-1. > TLB lookup is done before a PTW, if a valid entry is found we won't > do the PTW. > To be able to do TLB lookup, we need the correct tagging info, as > granularity and input size, so we get this based on the supported > translation stage. The TLB entries are added correctly from each > stage PTW. > > When nested translation is supported, this would need to change, for > example if we go with a combined TLB implementation, we would need to > use the min of the granularities in TLB. > > As stage-2 shouldn't be tagged by ASID, it will be set to -1 if S1P > is not enabled. > > Signed-off-by: Mostafa Saleh <[email protected]> > --- > hw/arm/smmuv3.c | 44 +++++++++++++++++++++++++++++++++----------- > 1 file changed, 33 insertions(+), 11 deletions(-) > > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c > index c18460a4ff..769c735697 100644 > --- a/hw/arm/smmuv3.c > +++ b/hw/arm/smmuv3.c > @@ -653,6 +653,8 @@ static int smmuv3_decode_config(IOMMUMemoryRegion *mr, > SMMUTransCfg *cfg, > return ret; > } > > + /* ASID defaults to -1 if s1 is not supported. */ > + cfg->asid = -1; > if (cfg->aborted || cfg->bypassed || !STAGE1_SUPPORTED(s->features)) { > return 0; > } > @@ -733,6 +735,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion > *mr, hwaddr addr, > SMMUTLBEntry *cached_entry = NULL; > SMMUTransTableInfo *tt; > SMMUTransCfg *cfg = NULL; > + uint8_t granule_sz, tsz; > IOMMUTLBEntry entry = { > .target_as = &address_space_memory, > .iova = addr, > @@ -764,21 +767,40 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion > *mr, hwaddr addr, > goto epilogue; > } > > - tt = select_tt(cfg, addr); maybe we shall adapt select_tt for S2 instead of using temp? I know there is a single range on S2 but well, use tt[0]? > - if (!tt) { > - if (cfg->record_faults) { > - event.type = SMMU_EVT_F_TRANSLATION; > - event.u.f_translation.addr = addr; > - event.u.f_translation.rnw = flag & 0x1; > + if (STAGE1_SUPPORTED(s->features)) { maybe check the enable state instead. > + /* Select stage1 translation table. */ > + tt = select_tt(cfg, addr); > + if (!tt) { > + if (cfg->record_faults) { > + event.type = SMMU_EVT_F_TRANSLATION; > + event.u.f_translation.addr = addr; > + event.u.f_translation.rnw = flag & 0x1; > + } > + status = SMMU_TRANS_ERROR; > + goto epilogue; > } > - status = SMMU_TRANS_ERROR; > - goto epilogue; > - } > + granule_sz = tt->granule_sz; > + tsz = tt->tsz; > > - page_mask = (1ULL << (tt->granule_sz)) - 1; > + } else { > + /* Stage2. */ > + granule_sz = cfg->s2cfg.granule_sz; > + tsz = cfg->s2cfg.tsz; > + } > + /* > + * TLB lookup looks for granule and input size for a translation stage, > + * as only one stage is supported right now, choose the right values > + * from the configuration. > + */ > + page_mask = (1ULL << granule_sz) - 1; > aligned_addr = addr & ~page_mask; > > - cached_entry = smmu_iotlb_lookup(bs, cfg, tt, aligned_addr); > + SMMUTransTableInfo temp = { > + .granule_sz = granule_sz, > + .tsz = tsz, > + }; > + > + cached_entry = smmu_iotlb_lookup(bs, cfg, &temp, aligned_addr); > if (cached_entry) { > if ((flag & IOMMU_WO) && !(cached_entry->entry.perm & IOMMU_WO)) { > status = SMMU_TRANS_ERROR; Thanks
Eric
