[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ZN5fpi1SDp2uqQnn@nvidia.com>
Date: Thu, 17 Aug 2023 14:57:58 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Nicolin Chen <nicolinc@...dia.com>
Cc: Robin Murphy <robin.murphy@....com>, will@...nel.org,
joro@...tes.org, mshavit@...gle.com, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, iommu@...ts.linux.dev
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
Powered by blists - more mailing lists