All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@nvidia.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	Kevin Tian <kevin.tian@intel.com>,
	Chaitanya Kulkarni <kch@nvidia.com>,
	Ashok Raj <ashok.raj@intel.com>,
	kvm@vger.kernel.org, rafael@kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Cornelia Huck <cohuck@redhat.com>, Will Deacon <will@kernel.org>,
	linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org,
	Alex Williamson <alex.williamson@redhat.com>,
	Jacob jun Pan <jacob.jun.pan@intel.com>,
	linux-pci@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com>,
	Diana Craciun <diana.craciun@oss.nxp.com>
Subject: Re: [PATCH 02/11] driver core: Set DMA ownership during driver bind/unbind
Date: Mon, 15 Nov 2021 11:56:13 -0400	[thread overview]
Message-ID: <20211115155613.GA2388278@nvidia.com> (raw)
In-Reply-To: <8499f0ab-9701-2ca2-ac7a-842c36c54f8a@arm.com>

On Mon, Nov 15, 2021 at 03:37:18PM +0000, Robin Murphy wrote:

> IOMMUs, and possibly even fewer of them support VFIO, so I'm in full
> agreement with Greg and Christoph that this absolutely warrants being scoped
> per-bus. I mean, we literally already have infrastructure to prevent drivers
> binding if the IOMMU/DMA configuration is broken or not ready yet; why would
> we want a totally different mechanism to prevent driver binding when the
> only difference is that that configuration *is* ready and working to the
> point that someone's already claimed it for other purposes?

I see, that does make sense

I see these implementations:

drivers/amba/bus.c:     .dma_configure  = platform_dma_configure,
drivers/base/platform.c:        .dma_configure  = platform_dma_configure,
drivers/bus/fsl-mc/fsl-mc-bus.c:        .dma_configure  = fsl_mc_dma_configure,
drivers/pci/pci-driver.c:       .dma_configure  = pci_dma_configure,
drivers/gpu/host1x/bus.c:       .dma_configure = host1x_dma_configure,

Other than host1x they all work with VFIO.

Also, there is no bus->dma_unconfigure() which would be needed to
restore the device as well.

So, would you rather see duplicated code into the 4 drivers, and a new
bus op to 'unconfigure dma'

Or, a 'dev_configure_dma()' function that is roughly:

        if (dev->bus->dma_configure) {
                ret = dev->bus->dma_configure(dev);
                if (ret)
                        return ret;
                if (!drv->suppress_auto_claim_dma_owner) {
                       ret = iommu_device_set_dma_owner(dev, DMA_OWNER_KERNEL,
                                                        NULL);
                       if (ret)
                               ret;
                }
         }

And a pair'd undo.

This is nice because we can enforce dev->bus->dma_configure when doing
a user bind so everything holds together safely without relying on
each bus_type to properly implement security.

Jason

WARNING: multiple messages have this Message-ID (diff)
From: Jason Gunthorpe via iommu <iommu@lists.linux-foundation.org>
To: Robin Murphy <robin.murphy@arm.com>
Cc: Alex Williamson <alex.williamson@redhat.com>,
	Kevin Tian <kevin.tian@intel.com>,
	Chaitanya Kulkarni <kch@nvidia.com>,
	Ashok Raj <ashok.raj@intel.com>,
	kvm@vger.kernel.org, rafael@kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Cornelia Huck <cohuck@redhat.com>,
	linux-kernel@vger.kernel.org,
	Christoph Hellwig <hch@infradead.org>,
	iommu@lists.linux-foundation.org,
	Jacob jun Pan <jacob.jun.pan@intel.com>,
	linux-pci@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com>,
	Will Deacon <will@kernel.org>,
	Diana Craciun <diana.craciun@oss.nxp.com>
Subject: Re: [PATCH 02/11] driver core: Set DMA ownership during driver bind/unbind
Date: Mon, 15 Nov 2021 11:56:13 -0400	[thread overview]
Message-ID: <20211115155613.GA2388278@nvidia.com> (raw)
In-Reply-To: <8499f0ab-9701-2ca2-ac7a-842c36c54f8a@arm.com>

On Mon, Nov 15, 2021 at 03:37:18PM +0000, Robin Murphy wrote:

> IOMMUs, and possibly even fewer of them support VFIO, so I'm in full
> agreement with Greg and Christoph that this absolutely warrants being scoped
> per-bus. I mean, we literally already have infrastructure to prevent drivers
> binding if the IOMMU/DMA configuration is broken or not ready yet; why would
> we want a totally different mechanism to prevent driver binding when the
> only difference is that that configuration *is* ready and working to the
> point that someone's already claimed it for other purposes?

I see, that does make sense

I see these implementations:

drivers/amba/bus.c:     .dma_configure  = platform_dma_configure,
drivers/base/platform.c:        .dma_configure  = platform_dma_configure,
drivers/bus/fsl-mc/fsl-mc-bus.c:        .dma_configure  = fsl_mc_dma_configure,
drivers/pci/pci-driver.c:       .dma_configure  = pci_dma_configure,
drivers/gpu/host1x/bus.c:       .dma_configure = host1x_dma_configure,

Other than host1x they all work with VFIO.

Also, there is no bus->dma_unconfigure() which would be needed to
restore the device as well.

So, would you rather see duplicated code into the 4 drivers, and a new
bus op to 'unconfigure dma'

Or, a 'dev_configure_dma()' function that is roughly:

        if (dev->bus->dma_configure) {
                ret = dev->bus->dma_configure(dev);
                if (ret)
                        return ret;
                if (!drv->suppress_auto_claim_dma_owner) {
                       ret = iommu_device_set_dma_owner(dev, DMA_OWNER_KERNEL,
                                                        NULL);
                       if (ret)
                               ret;
                }
         }

And a pair'd undo.

This is nice because we can enforce dev->bus->dma_configure when doing
a user bind so everything holds together safely without relying on
each bus_type to properly implement security.

Jason
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

  reply	other threads:[~2021-11-15 15:56 UTC|newest]

Thread overview: 120+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-15  2:05 [PATCH 00/11] Fix BUG_ON in vfio_iommu_group_notifier() Lu Baolu
2021-11-15  2:05 ` Lu Baolu
2021-11-15  2:05 ` [PATCH 01/11] iommu: Add device dma ownership set/release interfaces Lu Baolu
2021-11-15  2:05   ` Lu Baolu
2021-11-15 13:14   ` Christoph Hellwig
2021-11-15 13:14     ` Christoph Hellwig
2021-11-16  1:57     ` Lu Baolu
2021-11-16  1:57       ` Lu Baolu
2021-11-16 13:46       ` Jason Gunthorpe
2021-11-16 13:46         ` Jason Gunthorpe via iommu
2021-11-17  5:22         ` Lu Baolu
2021-11-17  5:22           ` Lu Baolu
2021-11-17 13:35           ` Jason Gunthorpe
2021-11-17 13:35             ` Jason Gunthorpe via iommu
2021-11-18  1:12             ` Lu Baolu
2021-11-18  1:12               ` Lu Baolu
2021-11-18 14:10               ` Jason Gunthorpe
2021-11-18 14:10                 ` Jason Gunthorpe via iommu
2021-11-18  2:39         ` Tian, Kevin
2021-11-18  2:39           ` Tian, Kevin
2021-11-18 13:33           ` Jason Gunthorpe
2021-11-18 13:33             ` Jason Gunthorpe via iommu
2021-11-19  5:44             ` Tian, Kevin
2021-11-19  5:44               ` Tian, Kevin
2021-11-19 11:14               ` Lu Baolu
2021-11-19 11:14                 ` Lu Baolu
2021-11-19 15:06                 ` Jörg Rödel
2021-11-19 15:06                   ` Jörg Rödel
2021-11-19 15:43                   ` Jason Gunthorpe
2021-11-19 15:43                     ` Jason Gunthorpe via iommu
2021-11-20 11:16                   ` Lu Baolu
2021-11-20 11:16                     ` Lu Baolu
2021-11-19 12:56               ` Jason Gunthorpe
2021-11-19 12:56                 ` Jason Gunthorpe via iommu
2021-11-15 20:38   ` Bjorn Helgaas
2021-11-15 20:38     ` Bjorn Helgaas
2021-11-16  1:52     ` Lu Baolu
2021-11-16  1:52       ` Lu Baolu
2021-11-15  2:05 ` [PATCH 02/11] driver core: Set DMA ownership during driver bind/unbind Lu Baolu
2021-11-15  2:05   ` Lu Baolu
2021-11-15  6:59   ` Greg Kroah-Hartman
2021-11-15  6:59     ` Greg Kroah-Hartman
2021-11-15 13:20     ` Christoph Hellwig
2021-11-15 13:20       ` Christoph Hellwig
2021-11-15 13:38     ` Jason Gunthorpe via iommu
2021-11-15 13:38       ` Jason Gunthorpe
2021-11-15 13:19   ` Christoph Hellwig
2021-11-15 13:19     ` Christoph Hellwig
2021-11-15 13:24     ` Jason Gunthorpe
2021-11-15 13:24       ` Jason Gunthorpe via iommu
2021-11-15 15:37       ` Robin Murphy
2021-11-15 15:37         ` Robin Murphy
2021-11-15 15:56         ` Jason Gunthorpe [this message]
2021-11-15 15:56           ` Jason Gunthorpe via iommu
2021-11-15 18:15           ` Christoph Hellwig
2021-11-15 18:15             ` Christoph Hellwig
2021-11-15 18:35           ` Robin Murphy
2021-11-15 18:35             ` Robin Murphy
2021-11-15 19:39             ` Jason Gunthorpe via iommu
2021-11-15 19:39               ` Jason Gunthorpe
2021-11-15  2:05 ` [PATCH 03/11] PCI: pci_stub: Suppress kernel DMA ownership auto-claiming Lu Baolu
2021-11-15  2:05   ` Lu Baolu
2021-11-15 13:21   ` Christoph Hellwig
2021-11-15 13:21     ` Christoph Hellwig
2021-11-15 13:31     ` Jason Gunthorpe via iommu
2021-11-15 13:31       ` Jason Gunthorpe
2021-11-15 15:14       ` Robin Murphy
2021-11-15 15:14         ` Robin Murphy
2021-11-15 16:17         ` Jason Gunthorpe
2021-11-15 16:17           ` Jason Gunthorpe via iommu
2021-11-15 17:54           ` Robin Murphy
2021-11-15 17:54             ` Robin Murphy
2021-11-15 18:19             ` Christoph Hellwig
2021-11-15 18:19               ` Christoph Hellwig
2021-11-15 18:44               ` Robin Murphy
2021-11-15 18:44                 ` Robin Murphy
2021-11-15 19:22             ` Jason Gunthorpe via iommu
2021-11-15 19:22               ` Jason Gunthorpe
2021-11-15 20:58               ` Robin Murphy
2021-11-15 20:58                 ` Robin Murphy
2021-11-15 21:19                 ` Jason Gunthorpe via iommu
2021-11-15 21:19                   ` Jason Gunthorpe
2021-11-15 20:48   ` Bjorn Helgaas
2021-11-15 20:48     ` Bjorn Helgaas
2021-11-15 22:17   ` Bjorn Helgaas
2021-11-15 22:17     ` Bjorn Helgaas
2021-11-16  6:05     ` Lu Baolu
2021-11-16  6:05       ` Lu Baolu
2021-11-15  2:05 ` [PATCH 04/11] PCI: portdrv: " Lu Baolu
2021-11-15  2:05   ` Lu Baolu
2021-11-15 20:44   ` Bjorn Helgaas
2021-11-15 20:44     ` Bjorn Helgaas
2021-11-16  7:24     ` Lu Baolu
2021-11-16  7:24       ` Lu Baolu
2021-11-16 20:22       ` Bjorn Helgaas
2021-11-16 20:22         ` Bjorn Helgaas
2021-11-16 20:48         ` Jason Gunthorpe
2021-11-16 20:48           ` Jason Gunthorpe via iommu
2021-11-15  2:05 ` [PATCH 05/11] iommu: Add security context management for assigned devices Lu Baolu
2021-11-15  2:05   ` Lu Baolu
2021-11-15 13:22   ` Christoph Hellwig
2021-11-15 13:22     ` Christoph Hellwig
2021-11-16  7:25     ` Lu Baolu
2021-11-16  7:25       ` Lu Baolu
2021-11-15  2:05 ` [PATCH 06/11] iommu: Expose group variants of dma ownership interfaces Lu Baolu
2021-11-15  2:05   ` Lu Baolu
2021-11-15 13:27   ` Christoph Hellwig
2021-11-15 13:27     ` Christoph Hellwig
2021-11-16  9:42     ` Lu Baolu
2021-11-16  9:42       ` Lu Baolu
2021-11-15  2:05 ` [PATCH 07/11] vfio: Use DMA_OWNER_USER to declaim passthrough devices Lu Baolu
2021-11-15  2:05   ` Lu Baolu
2021-11-15  2:05 ` [PATCH 08/11] vfio: Remove use of vfio_group_viable() Lu Baolu
2021-11-15  2:05   ` Lu Baolu
2021-11-15  2:05 ` [PATCH 09/11] vfio: Delete the unbound_list Lu Baolu
2021-11-15  2:05   ` Lu Baolu
2021-11-15  2:05 ` [PATCH 10/11] vfio: Remove iommu group notifier Lu Baolu
2021-11-15  2:05   ` Lu Baolu
2021-11-15  2:05 ` [PATCH 11/11] iommu: Remove iommu group changes notifier Lu Baolu
2021-11-15  2:05   ` Lu Baolu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211115155613.GA2388278@nvidia.com \
    --to=jgg@nvidia.com \
    --cc=alex.williamson@redhat.com \
    --cc=ashok.raj@intel.com \
    --cc=bhelgaas@google.com \
    --cc=cohuck@redhat.com \
    --cc=diana.craciun@oss.nxp.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@infradead.org \
    --cc=iommu@lists.linux-foundation.org \
    --cc=jacob.jun.pan@intel.com \
    --cc=kch@nvidia.com \
    --cc=kevin.tian@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=robin.murphy@arm.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.