Re: [RFC PATCH v2] iommu/xen: Add Xen PV-IOMMU driver

[Robin Murphy <robin.murphy@xxxxxxx>]

On 2024-06-24 6:36 pm, Easwar Hariharan wrote:
> Hi Jason,
>
> On 6/24/2024 9:32 AM, Jason Gunthorpe wrote:
> > On Mon, Jun 24, 2024 at 02:36:45PM +0000, Teddy Astie wrote:
> > > > > +bool xen_iommu_capable(struct device *dev, enum iommu_cap cap)
> > > > > +{
> > > > > +    switch (cap) {
> > > > > +    case IOMMU_CAP_CACHE_COHERENCY:
> > > > > +        return true;
> > > > Will the PV-IOMMU only ever be exposed on hardware where that really is
> > > > always true?
> > > On the hypervisor side, the PV-IOMMU interface always implicitely flush
> > > the IOMMU hardware on map/unmap operation, so at the end of the
> > > hypercall, the cache should be always coherent IMO.
> > Cache coherency is a property of the underlying IOMMU HW and reflects
> > the ability to prevent generating transactions that would bypass the
> > cache.
> >
> > On AMD and Intel IOMMU HW this maps to a bit in their PTEs that must
> > always be set to claim this capability.
> >
> > No ARM SMMU supports it yet.
> Unrelated to this patch: Both the arm-smmu and arm-smmu-v3 drivers claim
> this capability if the device tree/IORT table have the corresponding flags.
>
> I read through DEN0049 to determine what are the knock-on effects, or
> equivalently the requirements to set those flags in the IORT, but came
> up empty. Could you help with what I'm missing to resolve the apparent
> contradiction between your statement and the code?
We did rejig things slightly a while back. The status quo now is that 
IOMMU_CAP_CACHE_COHERENCY mostly covers whether IOMMU mappings can make 
device accesses coherent at all, tied in with the IOMMU_CACHE prot value 
- this is effectively forced for Intel and AMD, while for SMMU we have 
to take a guess, but as commented it's a pretty reasonable assumption 
that if the SMMU's own output for table walks etc. is coherent then its 
translation outputs are likely to be too. The further property of being 
able to then enforce a coherent mapping regardless of what an endpoint 
might try to get around it (PCIe No Snoop etc.) is now under the 
enforce_cache_coherency op - that's what SMMU can't guarantee for now 
due to the IMP-DEF nature of whether S2FWB overrides No Snoop or not.
Thanks,
Robin.