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

Jon Nettleton
2021-09-16T07:52:52Z

On Thu, Sep 16, 2021 at 9:26 AM Shameerali Kolothum Thodi
<shameerali.kolothum.thodi@huawei.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Jon Nettleton [mailto:jon@solid-run.com]
> > Sent: 06 September 2021 20:51
> > To: Robin Murphy <robin.murphy@arm.com>
> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>; Shameerali Kolothum Thodi
> > <shameerali.kolothum.thodi@huawei.com>; Laurentiu Tudor
> > <laurentiu.tudor@nxp.com>; linux-arm-kernel
> > <linux-arm-kernel@lists.infradead.org>; ACPI Devel Maling List
> > <linux-acpi@vger.kernel.org>; Linux IOMMU
> > <iommu@lists.linux-foundation.org>; Linuxarm <linuxarm@huawei.com>;
> > Joerg Roedel <joro@8bytes.org>; Will Deacon <will@kernel.org>;
> > wanghuiqiang <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo)
> > <guohanjun@huawei.com>; Steven Price <steven.price@arm.com>; Sami
> > Mujawar <Sami.Mujawar@arm.com>; Eric Auger <eric.auger@redhat.com>;
> > yangyicong <yangyicong@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.