[PATCH v7 2/9] ACPI/IORT: Add support for RMR node parsing

[Jon Nettleton]

On Thu, Sep 16, 2021 at 9:26 AM Shameerali Kolothum Thodi
<shameerali.kolothum.thodi at huawei.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Jon Nettleton [mailto:jon at solid-run.com]
> > Sent: 06 September 2021 20:51
> > To: Robin Murphy <robin.murphy at arm.com>
> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi at arm.com>; Shameerali Kolothum Thodi
> > <shameerali.kolothum.thodi at huawei.com>; Laurentiu Tudor
> > <laurentiu.tudor at nxp.com>; linux-arm-kernel
> > <linux-arm-kernel at lists.infradead.org>; ACPI Devel Maling List
> > <linux-acpi at vger.kernel.org>; Linux IOMMU
> > <iommu at lists.linux-foundation.org>; Linuxarm <linuxarm at huawei.com>;
> > Joerg Roedel <joro at 8bytes.org>; Will Deacon <will at kernel.org>;
> > wanghuiqiang <wanghuiqiang at huawei.com>; Guohanjun (Hanjun Guo)
> > <guohanjun at huawei.com>; Steven Price <steven.price at arm.com>; Sami
> > Mujawar <Sami.Mujawar at arm.com>; Eric Auger <eric.auger at redhat.com>;
> > yangyicong <yangyicong at huawei.com>
> > Subject: Re: [PATCH v7 2/9] ACPI/IORT: Add support for RMR node parsing
> >
> [...]
>
> > > >
> > > > On the prot value assignment based on the remapping flag, I'd like
> > > > to hear Robin/Joerg's opinion, I'd avoid being in a situation where
> > > > "normally" this would work but then we have to quirk it.
> > > >
> > > > Is this a valid assumption _always_ ?
> > >
> > > No. Certainly applying IOMMU_CACHE without reference to the device's
> > > _CCA attribute or how CPUs may be accessing a shared buffer could lead
> > > to a loss of coherency. At worst, applying IOMMU_MMIO to a
> > > device-private buffer *could* cause the device to lose coherency with
> > > itself if the memory underlying the RMR may have allocated into system
> > > caches. Note that the expected use for non-remappable RMRs is the
> > > device holding some sort of long-lived private data in system RAM -
> > > the MSI doorbell trick is far more of a niche hack really.
> > >
> > > At the very least I think we need to refer to the device's memory
> > > access properties here.
> > >
> > > Jon, Laurentiu - how do RMRs correspond to the EFI memory map on your
> > > firmware? I'm starting to think that as long as the underlying memory
> > > is described appropriately there then we should be able to infer
> > > correct attributes from the EFI memory type and flags.
> >
> > The devices are all cache coherent and marked as _CCA, 1.  The Memory
> > regions are in the virt table as ARM_MEMORY_REGION_ATTRIBUTE_DEVICE.
> >
> > The current chicken and egg problem we have is that during the fsl-mc-bus
> > initialization we call
> >
> > error = acpi_dma_configure_id(&pdev->dev, DEV_DMA_COHERENT,
> >                                               &mc_stream_id);
> >
> > which gets deferred because the SMMU has not been initialized yet. Then we
> > initialize the RMR tables but there is no device reference there to be able to
> > query device properties, only the stream id.  After the IORT tables are parsed
> > and the SMMU is setup, on the second device probe we associate everything
> > based on the stream id and the fsl-mc-bus device is able to claim its 1-1 DMA
> > mappings.
>
> Can we solve this order problem by delaying the iommu_alloc_resv_region()
> to the iommu_dma_get_rmr_resv_regions(dev, list) ? We could invoke
> device_get_dma_attr() from there which I believe will return the _CCA attribute.
>
> Or is that still early to invoke that?

That looks like it should work. Do we then also need to parse through the
VirtualMemoryTable matching the start and end addresses to determine the
other memory attributes like MMIO?

-Jon

>
> Thanks,
> Shameer
>
> > cat /sys/kernel/iommu_groups/0/reserved_regions
> > 0x0000000001000000 0x0000000010ffffff direct-relaxable
> > 0x0000000008000000 0x00000000080fffff msi
> > 0x000000080c000000 0x000000081bffffff direct-relaxable
> > 0x0000001c00000000 0x0000001c001fffff direct-relaxable
> > 0x0000002080000000 0x000000209fffffff direct-relaxable
> >
> > -Jon
> >
> > >
> > > Robin.