linux-kernel - Re: [PATCH v2 04/17] driver core: platform: Add driver dma ownership management

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20211128231509.GA966332@nvidia.com>
Date:   Sun, 28 Nov 2021 19:15:09 -0400
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Lu Baolu <baolu.lu@...ux.intel.com>,
        Joerg Roedel <joro@...tes.org>,
        Alex Williamson <alex.williamson@...hat.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Christoph Hellwig <hch@...radead.org>,
        Kevin Tian <kevin.tian@...el.com>,
        Ashok Raj <ashok.raj@...el.com>, Will Deacon <will@...nel.org>,
        Robin Murphy <robin.murphy@....com>,
        Dan Williams <dan.j.williams@...el.com>, rafael@...nel.org,
        Diana Craciun <diana.craciun@....nxp.com>,
        Cornelia Huck <cohuck@...hat.com>,
        Eric Auger <eric.auger@...hat.com>,
        Liu Yi L <yi.l.liu@...el.com>,
        Jacob jun Pan <jacob.jun.pan@...el.com>,
        Chaitanya Kulkarni <kch@...dia.com>,
        Stuart Yoder <stuyoder@...il.com>,
        Laurentiu Tudor <laurentiu.tudor@....com>,
        Thierry Reding <thierry.reding@...il.com>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Li Yang <leoyang.li@....com>, iommu@...ts.linux-foundation.org,
        linux-pci@...r.kernel.org, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 04/17] driver core: platform: Add driver dma ownership
 management

On Sun, Nov 28, 2021 at 09:10:14AM +0100, Greg Kroah-Hartman wrote:
> On Sun, Nov 28, 2021 at 10:50:38AM +0800, Lu Baolu wrote:
> > Multiple platform devices may be placed in the same IOMMU group because
> > they cannot be isolated from each other. These devices must either be
> > entirely under kernel control or userspace control, never a mixture. This
> > checks and sets DMA ownership during driver binding, and release the
> > ownership during driver unbinding.
> > 
> > Driver may set a new flag (suppress_auto_claim_dma_owner) to disable auto
> > claiming DMA_OWNER_DMA_API ownership in the binding process. For instance,
> > the userspace framework drivers (vfio etc.) which need to manually claim
> > DMA_OWNER_PRIVATE_DOMAIN_USER when assigning a device to userspace.
> 
> Why would any vfio driver be a platform driver?  

Why not? VFIO implements drivers for most physical device types
these days. Why wouldn't platform be included?

> That should never be the case as they obviously are not platform
> drivers, they are virtual ones.

Huh?

> > diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
> > index 7c96f169d274..779bcf2a851c 100644
> > +++ b/include/linux/platform_device.h
> > @@ -210,6 +210,7 @@ struct platform_driver {
> >  	struct device_driver driver;
> >  	const struct platform_device_id *id_table;
> >  	bool prevent_deferred_probe;
> > +	bool suppress_auto_claim_dma_owner;
> 
> What platform driver needs this change?

It is in patch 12:

--- a/drivers/vfio/platform/vfio_platform.c
+++ b/drivers/vfio/platform/vfio_platform.c
@@ -76,6 +76,7 @@ static struct platform_driver vfio_platform_driver = {
        .driver = {
                .name   = "vfio-platform",
        },
+       .suppress_auto_claim_dma_owner = true,
 };

Which is how VFIO provides support to DPDK for some Ethernet
controllers embedded in a few ARM SOCs.

It is also used in patch 17 in five tegra platform_drivers to make
their sharing of an iommu group between possibly related
platform_driver's safer.

> >  	USE_PLATFORM_PM_SLEEP_OPS
> > @@ -1478,7 +1505,8 @@ struct bus_type platform_bus_type = {
> >  	.probe		= platform_probe,
> >  	.remove		= platform_remove,
> >  	.shutdown	= platform_shutdown,
> > -	.dma_configure	= platform_dma_configure,
> > +	.dma_configure	= _platform_dma_configure,
> 
> What happened to the original platform_dma_configure() function?

It is still called. The issue here is that platform_dma_configure has
nothing to do with platform and is being re-used by AMBA.

Probably the resolution to both remarks is to rename
platform_dma_configure to something sensible (firwmare dma configure
maybe?) and use it in all places that do the of & acpi stuff -
pci/amba/platform at least.

Jason