kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@ziepe.ca>
To: Joerg Roedel <joro@8bytes.org>
Cc: "Tian, Kevin" <kevin.tian@intel.com>,
	Christoph Hellwig <hch@lst.de>,
	Alex Williamson <alex.williamson@redhat.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Lu Baolu <baolu.lu@linux.intel.com>,
	Will Deacon <will@kernel.org>,
	Kirti Wankhede <kwankhede@nvidia.com>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: Re: [PATCH 3/6] vfio: remove the unused mdev iommu hook
Date: Mon, 17 May 2021 10:35:00 -0300	[thread overview]
Message-ID: <20210517133500.GP1096940@ziepe.ca> (raw)
In-Reply-To: <YKJnPGonR+d8rbu/@8bytes.org>

On Mon, May 17, 2021 at 02:53:16PM +0200, Joerg Roedel wrote:
> On Mon, May 17, 2021 at 09:30:10AM -0300, Jason Gunthorpe wrote:
> > On Mon, May 17, 2021 at 02:22:06PM +0200, Joerg Roedel wrote:
> > > Yes, I know, We have the AUX-domain specific functions now, but I
> > > suggested a while back to turn the mdev code into its own bus
> > > implementation and let the IOMMU driver deal with whether it has an mdev
> > > or a pdev. When that is done the aux-specific functions can go away.
> > 
> > Yuk, no.
> > 
> > PASID is not connected to the driver model. It is inherently wrong to
> > suggest this.
> 
> There will be no drivers for that, but an mdev is a container for
> resources of a physical device which can be assigned to a virtual
> machine or a user-space process. In this way it fits very well into the
> existing abstractions.

There world is a lot bigger than just mdev and vfio.

> > PASID is a property of a PCI device and any PCI device driver should
> > be able to spawn PASIDs without silly restrictions.
> 
> Some points here:
> 
> 	1) The concept of PASIDs is not limited to PCI devices.

Do I have to type PASID/stream id/etc/etc in every place? We all know
this is gerenal, it is why Intel is picking IOASID for the uAPI name.

> 	2) An mdev is not a restriction. It is an abstraction with which
> 	   other parts of the kernel can work.

mdev is really gross, new things should avoid using it. It is also
100% VFIO specific and should never be used outside VFIO.

My recent work significantly eliminates the need for mdev at all, the
remaining gaps have to do with the way mdev hackily overrides the
iommu mechanisms in VFIO.

Tying any decision to the assumption that mdev will, or even should,
continue is not aligned with the direction things are going.

> > Fixing the IOMMU API is clearly needed here to get a clean PASID
> > implementation in the kernel.
> 
> You still have to convince me that this is needed and a good idea. By
> now I am not even remotely convinced and putting words like 'fixing',
> 'clearly' and 'silly' in a sentence is by far not enough for this to
> happen.

Well, I'm sorry, but there is a huge other thread talking about the
IOASID design in great detail and why this is all needed. Jumping into
this thread without context and basically rejecting all the
conclusions that were reached over the last several weeks is really
not helpful - especially since your objection is not technical.

I think you should wait for Intel to put together the /dev/ioasid uAPI
proposal and the example use cases it should address then you can give
feedback there, with proper context.

In this case the mdev specific auxdomain for PASID use is replaced by
sane /dev/ioasid objects - and how that gets implemented through the
iommu layer would still be a big TBD. Though I can't forsee any
quality implementation of /dev/ioasid being driven by a one io page
table per struct device scheme.

Hopefully Intel will make the reasons why, and the use cases
supporting the desgin, clear in their RFC.

> To be clear, I agree that the aux-domain specifics should be removed
> from the IOMMU-API, but the mdev-abstraction itself still makes sense.

Expect mdev is basically legacy too. 

Jason

  reply	other threads:[~2021-05-17 13:35 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-10  6:53 more iommu dead code removal Christoph Hellwig
2021-05-10  6:54 ` [PATCH 1/6] iommu: remove the unused dev_has_feat method Christoph Hellwig
2021-05-10  6:54 ` [PATCH 2/6] iommu: remove the unused iommu_aux_get_pasid interface Christoph Hellwig
2021-05-10  6:54 ` [PATCH 3/6] vfio: remove the unused mdev iommu hook Christoph Hellwig
2021-05-10 15:54   ` Jason Gunthorpe
2021-05-13  3:28     ` Tian, Kevin
2021-05-13 12:00       ` Jason Gunthorpe
2021-05-14  6:27         ` Tian, Kevin
2021-05-14  6:54         ` Tian, Kevin
2021-05-14 12:19           ` Jason Gunthorpe
2021-05-14 12:58             ` Tian, Kevin
2021-05-14 13:31               ` Jason Gunthorpe
2021-05-17 12:22                 ` Joerg Roedel
2021-05-17 12:30                   ` Jason Gunthorpe
2021-05-17 12:53                     ` Joerg Roedel
2021-05-17 13:35                       ` Jason Gunthorpe [this message]
2021-05-17 15:35                         ` Joerg Roedel
2021-05-19 15:23                           ` Robin Murphy
2021-05-19 18:06                             ` Jason Gunthorpe
2021-05-19 23:12                               ` Tian, Kevin
2021-05-19 23:24                                 ` Jason Gunthorpe
2021-05-20 14:13                                   ` Robin Murphy
2021-05-20 14:34                                     ` Jason Gunthorpe
2021-05-24 18:18                                       ` Robin Murphy
2021-05-25  0:00                                         ` Jason Gunthorpe
2021-06-30  9:08                           ` Tian, Kevin
2021-07-22 13:34                             ` Christoph Hellwig
2021-07-23  5:36                               ` Tian, Kevin
2021-07-23  5:41                                 ` Christoph Hellwig
2021-07-23  5:44                                   ` Tian, Kevin
2021-07-22  6:02                           ` Tian, Kevin
2021-05-14 13:17         ` Tian, Kevin
2021-05-14 13:39           ` Jason Gunthorpe
2021-05-14 14:28             ` Tian, Kevin
2021-05-14 14:44               ` Jason Gunthorpe
2021-05-10  6:54 ` [PATCH 4/6] iommu: remove iommu_aux_{attach,detach}_device Christoph Hellwig
2021-05-10  6:54 ` [PATCH 5/6] iommu: remove IOMMU_DEV_FEAT_AUX Christoph Hellwig
2021-05-10  6:54 ` [PATCH 6/6] iommu: remove iommu_dev_feature_enabled Christoph Hellwig
2021-05-10 11:54 ` more iommu dead code removal Jason Gunthorpe

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=20210517133500.GP1096940@ziepe.ca \
    --to=jgg@ziepe.ca \
    --cc=alex.williamson@redhat.com \
    --cc=baolu.lu@linux.intel.com \
    --cc=dwmw2@infradead.org \
    --cc=hch@lst.de \
    --cc=iommu@lists.linux-foundation.org \
    --cc=joro@8bytes.org \
    --cc=kevin.tian@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=kwankhede@nvidia.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=will@kernel.org \
    --subject='Re: [PATCH 3/6] vfio: remove the unused mdev iommu hook' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).