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

Shameerali Kolothum Thodi
2021-09-16T07:26:45Z

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

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.