From: "Tian, Kevin" <firstname.lastname@example.org>
To: Jacob Pan <email@example.com>
Cc: "Raj, Ashok" <firstname.lastname@example.org>,
Jean-Philippe Brucker <email@example.com>,
Alex Williamson <firstname.lastname@example.org>,
David Woodhouse <email@example.com>
Subject: RE: [PATCH v2 1/3] iommu/uapi: Define uapi version and capabilities
Date: Thu, 16 Apr 2020 01:27:43 +0000 [thread overview]
Message-ID: <AADFC41AFE54684AB9EE6CBC0274A5D19D822943@SHSMSX104.ccr.corp.intel.com> (raw)
> From: Jacob Pan <firstname.lastname@example.org>
> Sent: Wednesday, April 15, 2020 11:39 PM
> On Tue, 14 Apr 2020 23:47:40 +0000
> "Tian, Kevin" <email@example.com> wrote:
> > > From: Jacob Pan <firstname.lastname@example.org>
> > > Sent: Wednesday, April 15, 2020 6:32 AM
> > >
> > > On Tue, 14 Apr 2020 10:13:04 -0700
> > > Jacob Pan <email@example.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:
> 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.
iommu mailing list
next prev parent reply other threads:[~2020-04-16 1:27 UTC|newest]
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
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:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* 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 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).