Messages in this thread | | | Date | Thu, 17 Aug 2023 14:57:58 -0300 | From | Jason Gunthorpe <> | Subject | Re: [PATCH v2] iommu/arm-smmu-v3: Allow default substream bypass with a pasid support |
| |
On Thu, Aug 17, 2023 at 09:58:54AM -0700, Nicolin Chen wrote: > > > > @@ -1290,6 +1291,9 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid, > > > > > > > > if (smmu_domain) { > > > > switch (smmu_domain->stage) { > > > > + case ARM_SMMU_DOMAIN_BYPASS_S1DSS: > > > > + s1dss = STRTAB_STE_1_S1DSS_BYPASS; > > > > + fallthrough; > > > > case ARM_SMMU_DOMAIN_S1: > > > > cd_table = &master->cd_table; > > > > break; > > > > > > Eg, I think the code looks much nicer if the logic here is more like: > > > > > > if (master->cd_table.cdtab) > > > arm_smmu_cd_table_get_ste(master->cd_table, &ste) > > So, this means that cd_table is present, indicating either "S1 > translated" or "S1 enabled but S1DSS"...
Yes, if cd table is present then cd table always provides the STE. So this is S1 page tables on any SSID or SSID=0 IDENTITY
> > > else if (master->domain) > > > arm_smmu_domain_get_ste(master->domain, &ste); > > ... and this means that cd_table isn't present, indicating S1 > bypass or S2....
Meaning S2 page table on the RID or IDENTITY.
> > > And you'd check in arm_smmu_cd_table_get_ste() to learn the CD > > > parameters and also what SSID=0 is. If SSID=0 is IDENTITY then > > > arm_smmu_cd_table_get_ste would return with S1DSS set. > > > > > > arm_smmu_domain_get_ste() would multiplex based on the domain type. > > ... it then means we need arm_smmu_write_ctx_desc() also when > attaching an IDENTITY domain.
Yes, if a cd_table is present then it would be good to always write a consistent CD table entry meaning IDENTITY for SSID. Even if all this does is zero out the CD table entry.
> > > > @@ -2456,7 +2471,8 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) > > > > list_add(&master->domain_head, &smmu_domain->devices); > > > > spin_unlock_irqrestore(&smmu_domain->devices_lock, flags); > > > > > > > > - if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) { > > > > + if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1 || > > > > + smmu_domain->stage == ARM_SMMU_DOMAIN_BYPASS_S1DSS) { > > > > if (!master->cd_table.cdtab) { > > > > ret = arm_smmu_alloc_cd_tables(master); > > > > if (ret) { > > > > > > So more like: > > > > > > if (smmu_domain == IDENTIY && arm_smmu_support_ssid(dev)) > > > arm_smmu_alloc_cd_tables() > > OK. ARM_SMMU_DOMAIN_S1 with ssid=0 still needs a cd_table though.
Yes, but the atomic ste update is a bit nicer, though it can be something to do separately as it is just saving a bit of cache.
Jason
| |