[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fb55a025-348e-800c-e368-48be075d8e9c@arm.com>
Date: Thu, 7 Apr 2022 16:17:11 +0100
From: Robin Murphy <robin.murphy@....com>
To: Jason Gunthorpe <jgg@...dia.com>,
"Tian, Kevin" <kevin.tian@...el.com>
Cc: Christoph Hellwig <hch@....de>,
Alex Williamson <alex.williamson@...hat.com>,
Lu Baolu <baolu.lu@...ux.intel.com>,
Christian Benvenuti <benve@...co.com>,
Cornelia Huck <cohuck@...hat.com>,
David Woodhouse <dwmw2@...radead.org>,
Gerald Schaefer <gerald.schaefer@...ux.ibm.com>,
"iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
Jason Wang <jasowang@...hat.com>,
Joerg Roedel <joro@...tes.org>,
"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
"linux-s390@...r.kernel.org" <linux-s390@...r.kernel.org>,
Matthew Rosato <mjrosato@...ux.ibm.com>,
"Michael S. Tsirkin" <mst@...hat.com>,
Nelson Escobar <neescoba@...co.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
Rob Clark <robdclark@...il.com>,
Suravee Suthikulpanit <suravee.suthikulpanit@....com>,
"virtualization@...ts.linux-foundation.org"
<virtualization@...ts.linux-foundation.org>,
Will Deacon <will@...nel.org>
Subject: Re: [PATCH 1/5] iommu: Replace uses of IOMMU_CAP_CACHE_COHERENCY with
dev_is_dma_coherent()
On 2022-04-07 14:59, Jason Gunthorpe wrote:
> On Thu, Apr 07, 2022 at 07:18:48AM +0000, Tian, Kevin wrote:
>>> From: Jason Gunthorpe <jgg@...dia.com>
>>> Sent: Thursday, April 7, 2022 1:17 AM
>>>
>>> On Wed, Apr 06, 2022 at 06:10:31PM +0200, Christoph Hellwig wrote:
>>>> On Wed, Apr 06, 2022 at 01:06:23PM -0300, Jason Gunthorpe wrote:
>>>>> On Wed, Apr 06, 2022 at 05:50:56PM +0200, Christoph Hellwig wrote:
>>>>>> On Wed, Apr 06, 2022 at 12:18:23PM -0300, Jason Gunthorpe wrote:
>>>>>>>> Oh, I didn't know about device_get_dma_attr()..
>>>>>>
>>>>>> Which is completely broken for any non-OF, non-ACPI plaform.
>>>>>
>>>>> I saw that, but I spent some time searching and could not find an
>>>>> iommu driver that would load independently of OF or ACPI. ie no IOMMU
>>>>> platform drivers are created by board files. Things like Intel/AMD
>>>>> discover only from ACPI, etc.
>>
>> Intel discovers IOMMUs (and optionally ACPI namespace devices) from
>> ACPI, but there is no ACPI description for PCI devices i.e. the current
>> logic of device_get_dma_attr() cannot be used on PCI devices.
>
> Oh? So on x86 acpi_get_dma_attr() returns DEV_DMA_NON_COHERENT or
> DEV_DMA_NOT_SUPPORTED?
I think it _should_ return DEV_DMA_COHERENT on x86/IA-64 (unless a _CCA
method was actually present to say otherwise), based on
acpi_init_coherency(), but I only know for sure what happens on arm64.
> I think I should give up on this and just redefine the existing iommu
> cap flag to IOMMU_CAP_CACHE_SUPPORTED or something.
TBH I don't see any issue with current name, but I'd certainly be happy
to nail down a specific definition for it, along the lines of "this
means that IOMMU_CACHE mappings are generally coherent". That works for
things like Arm's S2FWB making it OK to assign an otherwise-non-coherent
device without extra hassle.
For the specific case of overriding PCIe No Snoop (which is more
problematic from an Arm SMMU PoV) when assigning to a VM, would that not
be easier solved by just having vfio-pci clear the "Enable No Snoop"
control bit in the endpoint's PCIe capability?
>>> We could alternatively use existing device_get_dma_attr() as a default
>>> with an iommu wrapper and push the exception down through the iommu
>>> driver and s390 can override it.
>>>
>>
>> if going this way probably device_get_dma_attr() should be renamed to
>> device_fwnode_get_dma_attr() instead to make it clearer?
>
> I'm looking at the few users:
>
> drivers/ata/ahci_ceva.c
> drivers/ata/ahci_qoriq.c
> - These are ARM only drivers. They are trying to copy the dma-coherent
> property from its DT/ACPI definition to internal register settings
> which look like they tune how the AXI bus transactions are created.
>
> I'm guessing the SATA IP block's AXI interface can be configured to
> generate coherent or non-coherent requests and it has to be set
> in a way that is consistent with the SOC architecture and match
> what the DMA API expects the device will do.
>
> drivers/crypto/ccp/sp-platform.c
> - Only used on ARM64 and also programs a HW register similar to the
> sata drivers. Refuses to work if the FW property is not present.
>
> drivers/net/ethernet/amd/xgbe/xgbe-platform.c
> - Seems to be configuring another ARM AXI block
>
> drivers/gpu/drm/panfrost/panfrost_drv.c
> - Robin's commit comment here is good, and one of the things this
> controls is if the coherent_walk is set for the io-pgtable-arm.c
> code which avoids DMA API calls
>
> drivers/gpu/drm/tegra/uapi.c
> - Returns DRM_TEGRA_CHANNEL_CAP_CACHE_COHERENT to userspace. No idea.
>
> My take is that the drivers using this API are doing it to make sure
> their HW blocks are setup in a way that is consistent with the DMA API
> they are also using, and run in constrained embedded-style
> environments that know the firmware support is present.
>
> So in the end it does not seem suitable right now for linking to
> IOMMU_CACHE..
That seems a pretty good summary - I think they're basically all
"firmware told Linux I'm coherent so I'd better act coherent" cases, but
that still doesn't necessarily mean that they're *forced* to respect
that. One of the things on my to-do list is to try adding a
DMA_ATTR_NO_SNOOP that can force DMA cache maintenance for coherent
devices, primarily to hook up in Panfrost (where there is a bit of a
performance to claw back on the coherent AmLogic SoCs by leaving certain
buffers non-cacheable).
Cheers,
Robin.
Powered by blists - more mailing lists