Re: [PATCH v2 12/14] iommu: Consolidate the default_domain setup to one function

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Mar 30, 2023 at 03:29:00PM +0100, Robin Murphy wrote:

> > > +    /* The domain must be attached before we can establish any
> > > mappings */
> > > +    for_each_group_device(group, gdev)
> > > +        iommu_create_device_direct_mappings(dom, gdev->dev);
> > 
> > It's better to move creating direct mappings before setting the domain
> > to the group devices.
> > 
> > The VT-d platforms allow the firmware to access the memory regions
> > defined in RMRR ACPI table. If we set an empty domain to the device
> > while the firmware DMA accesses the RMRR memory, it might result in
> > spurious DMA faults.
> 
> Yes, logically IOMMU_RESV_DIRECT and IOMMU_RESV_DIRECT_RELAXABLE regions
> *must* be mapped before their device is attached, in order to guarantee
> continuity. Yes, this means the RMR stuff for Arm SMMU still isn't truly
> working properly - it still needs domain_alloc fixing to get rid of the
> whole horrible "finalise on first attach" idiom, which sadly I haven't had
> time to get back to yet :(

Yes, this is the only reason I put it in this order is because otherwise it
doesn't work at all on ARM and just quiely NOPs itself without any
error reporting. That seemed bad..

IIRC the existing code had both orderings :( I'll add a note about
this justification

Thanks,
Jason