IOMMU Archive on lore.kernel.org
 help / color / Atom feed
From: "Tian, Kevin" <kevin.tian@intel.com>
To: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: "Raj, Ashok" <ashok.raj@intel.com>,
	Jean-Philippe Brucker <jean-philippe@linaro.com>,
	LKML <linux-kernel@vger.kernel.org>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	Alex Williamson <alex.williamson@redhat.com>,
	David Woodhouse <dwmw2@infradead.org>
Subject: RE: [PATCH v2 1/3] iommu/uapi: Define uapi version and capabilities
Date: Thu, 16 Apr 2020 01:27:43 +0000
Message-ID: <AADFC41AFE54684AB9EE6CBC0274A5D19D822943@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <20200415083854.6fefac01@jacob-builder>

> From: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Sent: Wednesday, April 15, 2020 11:39 PM
> 
> On Tue, 14 Apr 2020 23:47:40 +0000
> "Tian, Kevin" <kevin.tian@intel.com> wrote:
> 
> > > From: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > > Sent: Wednesday, April 15, 2020 6:32 AM
> > >
> > > On Tue, 14 Apr 2020 10:13:04 -0700
> > > Jacob Pan <jacob.jun.pan@linux.intel.com> wrote:
> > >
> > > > > > >  In any of the proposed solutions, the
> > > > > > > IOMMU driver is ultimately responsible for validating the
> > > > > > > user data, so do we want vfio performing the
> > > > > > > copy_from_user() to an object that could later be assumed
> > > > > > > to be sanitized, or should vfio just pass a user pointer to
> > > > > > > make it obvious that the consumer is responsible for all
> > > > > > > the user protections?  Seems like the latter.
> > > > > > I like the latter as well.
> > > > > >
> > > On a second thought, I think the former is better. Two reasons:
> > >
> > > 1. IOMMU API such as page_response is also used in baremetal. So it
> > > is not suitable to pass a __user *.
> > > https://www.spinics.net/lists/arm-kernel/msg798677.html
> >
> > You can have a wrapped version accepting a __user* and an internal
> > version for kernel pointers.
> >
> I have thought about that also but the problem is that some of the
> flags are processed in the vendor IOMMU ops so it is hard to do that in
> a generic wrapper.

All vendor IOMMU ops are defined in the same header file, so they
can be verified in one common IOMMU wrapper, just like how you
dealt with it in VFIO originally...

> 
> > >
> > > 2. Some data are in the mandatory (fixed offset, never removed or
> > > extended) portion of the uAPI structure. It is simpler for VFIO to
> > > extract that and pass it to IOMMU API. For example, the PASID value
> > > used for unbind_gpasid(). VFIO also need to sanitize the PASID
> > > value to make sure it belongs to the same VM that did the
> > > allocation.
> >
> > I don't think this makes much difference. If anyway you still plan to
> > let IOMMU driver parse some user pointers, why not making a clear
> > split to have it sparse all IOMMU specific fields?
> >
> The plan is not to have IOMMU driver parse user pointers. This is the
> "former" case in Alex's comment. I.e. vfio performing the
> copy_from_user based on argsz in IOMMU uAPI.
> 

I'm confused. I thought Alex proposed the latter one:
---[quote]
> So, __user * will be passed to IOMMU driver if VFIO checks minsz
> include flags and they are valid.
> IOMMU driver can copy the rest based on the mandatory version/minsz and
> flags in the IOMMU uAPI structs.
> Does it sound right? This is really choice #2.

Sounds like each IOMMU UAPI struct just needs to have an embedded size
and flags field, but yes.
----

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

  reply index

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-25 23:17 [PATCH v2 0/3] IOMMU user API enhancement Jacob Pan
2020-03-25 23:17 ` [PATCH v2 1/3] iommu/uapi: Define uapi version and capabilities Jacob Pan
2020-03-26  9:23   ` Christoph Hellwig
2020-03-26 16:44     ` Jacob Pan
2020-03-27  2:49       ` Tian, Kevin
2020-03-27  7:47         ` Christoph Hellwig
2020-03-27 23:53           ` Jacob Pan
2020-03-30  5:40             ` Tian, Kevin
2020-03-30 16:07               ` Jacob Pan
2020-03-31  6:06                 ` Tian, Kevin
2020-03-31 15:54                   ` Jacob Pan
2020-04-01  5:32                     ` Tian, Kevin
2020-04-02 18:36                       ` Jacob Pan
2020-04-13 20:41                         ` Jacob Pan
2020-04-13 22:21                           ` Alex Williamson
2020-04-14  5:05                             ` Jacob Pan
2020-04-14 16:13                               ` Alex Williamson
2020-04-14 17:13                                 ` Jacob Pan
2020-04-14 22:32                                   ` Jacob Pan
2020-04-14 23:47                                     ` Tian, Kevin
2020-04-15 15:38                                       ` Jacob Pan
2020-04-16  1:27                                         ` Tian, Kevin [this message]
2020-04-14  8:15                             ` Christoph Hellwig
2020-04-14  8:11                           ` Christoph Hellwig
2020-04-14 16:06                             ` Jacob Pan
2020-03-25 23:17 ` [PATCH v2 2/3] iommu/uapi: Use unified UAPI version Jacob Pan
2020-03-25 23:17 ` [PATCH v2 3/3] iommu/uapi: Add helper function for size lookup Jacob Pan

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=AADFC41AFE54684AB9EE6CBC0274A5D19D822943@SHSMSX104.ccr.corp.intel.com \
    --to=kevin.tian@intel.com \
    --cc=alex.williamson@redhat.com \
    --cc=ashok.raj@intel.com \
    --cc=dwmw2@infradead.org \
    --cc=iommu@lists.linux-foundation.org \
    --cc=jacob.jun.pan@linux.intel.com \
    --cc=jean-philippe@linaro.com \
    --cc=linux-kernel@vger.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

IOMMU Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-iommu/0 linux-iommu/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-iommu linux-iommu/ https://lore.kernel.org/linux-iommu \
		iommu@lists.linux-foundation.org
	public-inbox-index linux-iommu

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.linux-foundation.lists.iommu


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git