RE: [PATCH RFC v2 03/11] iommu/sva: Add iommu_domain type for SVA

[Tian, Kevin]

> From: Robin Murphy <robin.murphy@xxxxxxx>
> Sent: Wednesday, April 6, 2022 8:32 PM
> 
> On 2022-04-06 06:58, Tian, Kevin wrote:
> >> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> >> Sent: Wednesday, April 6, 2022 9:24 AM
> >>
> >> On Wed, Apr 06, 2022 at 01:00:13AM +0000, Tian, Kevin wrote:
> >>
> >>>> Because domains wrap more than just the IOPTE format, they have
> >>>> additional data related to the IOMMU HW block itself. Imagine a SOC
> >>>> with two IOMMU HW blocks that can both process the CPU IOPTE
> format,
> >>>> but have different configuration.
> >>>
> >>> Curious. Is it hypothesis or real? If real can you help give a concrete
> >>> example?
> >>
> >> Look at arm_smmu_attach_dev() - the domain has exactly one smmu
> >> pointer which contains the base address for the SMMU IP block. If the
> >> domain doesn't match the smmu pointer from the struct device it won't
> >> allow attaching.
> >>
> >> I know of ARM SOCs with many copies of the SMMU IP block.
> >>
> >> So at least with current drivers ARM seems to have this limitation.
> >>
> >
> > I saw that code, but before this series it is used only for stage-2 instead
> > of SVA. and I didn't see similar check in the old sva related paths (though
> > it doesn't use domain):
> >
> > arm_smmu_master_sva_enable_iopf()
> > arm_smmu_master_enable_sva{}
> > __arm_smmu_sva_bind()
> >
> > If I didn't overlook some trick hiding in the call chain of those functions,
> > is there a bug in the existing SMMU sva logic or is it conceptually correct
> > to not have such check for SVA?
> 
> The current SVA APIs are all device-based, so implicitly reflect
> whichever SMMU instance serves the given device. Once domains come into
> the picture, callers are going to have to be more aware that a domain
> may be specific to a particular IOMMU instance, and potentially allocate
> separate domains for separate devices to represent the same address
> space, much like vfio_iommu_type1_attach_group() does.

The current definition of domain is specific to stage-2. It is interesting 
whether we want to keep the same stage-2 constraints to stage-1 when
extending the domain concept to cover both. From what you explained
SVA conceptually is device-based thus SMMU instance doesn't matter.
Then allowing one domain to wrap CPU page table cross devices under
different SMMU instances is also one choice, as long as this domain is
differentiated from stage-2 domain in proper way. I get you may not
want to go this route for smmu driver as explained below, but let's
align on that it's just an implementation choice instead of a conceptual
requirement (I didn't see how IOMMU instances can have heterogenous
configurations when walking the same CPU page table). 😊

> 
> It's not really worth IOMMU drivers trying to support a domain spanning
> potentially-heterogeneous instances internally, since they can't
> reasonably know what matters in any particular situation. That's
> primarily why we've never tried to do it in the SMMU drivers. It's a lot
> easier for relevant callers to look at what they get and figure out
> whether any mismatch in capabilities is tolerable or not.
> 
> Robin.
> 
> > If the former then yes we have to take SMMU IP block into consideration
> > thus could have multiple domains per CPU page table. If the latter then
> > this is not a valid example for that configuration.
> >
> > Which one is correct?
> >
> > Thanks
> > Kevin