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

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Jon Nettleton [mailto:jon at solid-run.com]
> Sent: 16 September 2021 08:52
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi at huawei.com>
> Cc: Robin Murphy <robin.murphy at arm.com>; Lorenzo Pieralisi
> <lorenzo.pieralisi at arm.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>; 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 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?

Yes. But that looks tricky as I can't find that readily available on Arm, like the
efi_mem_attributes(). I will take a look.

Please let me know if there is one or any other easy way to retrieve it.

Thanks,
Shameer

> 
> -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.