Re: [PATCH v4 02/13] iommu/arm-smmu-v3: Add smmu_s1_cfg to smmu_master

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, Jul 14, 2023 at 12:16:16AM +0800, Michael Shavit wrote:
> On Thu, Jul 13, 2023 at 10:29 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
> > It would make alot more sense if the STE value used by an unmanaged S1
> > domain was located in/near the unmanaged domain or called 'unmanaged
> > S1 STE' or something if it really has to be in the master. Why does
> > this even need to be stored, can't we compute it?
> 
> struct s1_cfg* and struct s2_cfg* are precisely what is used to
> compute an STE. For example, when s1_cfg is set, arm_smmu_write_strtab
> will write the s1_cfg's CD table dma_pointer into the STE's
> STRTAB_STE_0_CFG field. When neither are set, the STE fields are
> written to enable bypass (or abort depending on the config).

I guess I never really understood why these were precomputed and
stored at all. Even more confusing is why we need to keep pointers to
them anywhere? Compute the STE and CDE directly from the source data
when you need it?

eg If I want to install an IDENITY domain into a STE then I compute
the STE for identity and go ahead and do it.

> > I'd think the basic mental model should be to extract the STE from the
> > thing you intend to install. Either the default CD table, or from the
> > iommu_domain. ie some 'get STE from iommu_domain' function?
> 
> I don't follow this. When we attach a domain with pasid (whether
> through SVA or the set_dev_pasid API) , we don't want to install an
> entirely new CD table. 

The master object owns an optional CD table. If it is exsists it is
used by every domain that is attached to that master.

In the code flow there are two entry points to attach a domain, attach
to a PASID or attach to a RID.

For attach to PASID the code should always force the master to have a
CD table and then attach the domain to the CD table.

For attach to RID the code should do a bunch of checks and decide if
it should force the master to have a CD table and attach the domain to
that, or directly attach the domain to the STE.

When the master gains a CD table then the CD table object becomes
attached to the STE. In all cases we should be able to point to the
object the STE points at and don't need a cfg or pointer to cfg since
the object itself can provide the cfg.

In all cases when you go to compute a STE you figure out what object
is attached to it (CD or domain), compute the correct STE for that
object, the set it. Same for he CDE, query the correct CDE from the
iommu_domain when you attach it to the table.

There should be no such thing as a "default" STE, and I question if it
makes sense to even precompute the s1/s2_cfg values during finalize at
all..

> We want to write something (page-table pointer) to a common CD
> table. Where should the s1_cfg which owns that common table live? 

I would suggest a 'cd table struct' that as all the stuff related to
the CD table, including an API to cacluate the STE this CD table
requires. If not in actual code with a real struct, then in a logical
sense in that a chunk of the master struct is the "CD table".

> I thought we concluded that it should be owned by the
> arm_smmu_master rather than any domain (to avoid dependencies
> between domains a-la aux-domain). 

Yes, I'm not saying anything against that, just how and where the STE
and CDE values flow around.

Jason