From: "Liu, Yi L" <email@example.com>
To: Alex Williamson <firstname.lastname@example.org>,
"Tian, Kevin" <email@example.com>
Cc: "Raj, Ashok" <firstname.lastname@example.org>,
Jonathan Corbet <email@example.com>,
Jean-Philippe Brucker <firstname.lastname@example.org>,
Christoph Hellwig <email@example.com>,
David Woodhouse <firstname.lastname@example.org>
Subject: RE: [PATCH v2 1/3] docs: IOMMU user API
Date: Fri, 19 Jun 2020 02:15:36 +0000 [thread overview]
Message-ID: <DM5PR11MB1435FA1C517087E8C5DEACB3C3980@DM5PR11MB1435.namprd11.prod.outlook.com> (raw)
> From: Alex Williamson <email@example.com>
> Sent: Friday, June 19, 2020 5:48 AM
> On Wed, 17 Jun 2020 08:28:24 +0000
> "Tian, Kevin" <firstname.lastname@example.org> wrote:
> > > From: Liu, Yi L <email@example.com>
> > > Sent: Wednesday, June 17, 2020 2:20 PM
> > >
> > > > From: Jacob Pan <firstname.lastname@example.org>
> > > > Sent: Tuesday, June 16, 2020 11:22 PM
> > > >
> > > > On Thu, 11 Jun 2020 17:27:27 -0700
> > > > Jacob Pan <email@example.com> wrote:
> > > >
> > > > > >
> > > > > > But then I thought it even better if VFIO leaves the entire
> > > > > > copy_from_user() to the layer consuming it.
> > > > > >
> > > > > OK. Sounds good, that was what Kevin suggested also. I just wasn't
> > > > > sure how much VFIO wants to inspect, I thought VFIO layer wanted to do
> > > > > a sanity check.
> > > > >
> > > > > Anyway, I will move copy_from_user to iommu uapi layer.
> > > >
> > > > Just one more point brought up by Yi when we discuss this offline.
> > > >
> > > > If we move copy_from_user to iommu uapi layer, then there will be
> > > multiple
> > > > copy_from_user calls for the same data when a VFIO container has
> > > multiple domains,
> > > > devices. For bind, it might be OK. But might be additional overhead for TLB
> > > flush
> > > > request from the guest.
> > >
> > > I think it is the same with bind and TLB flush path. will be multiple
> > > copy_from_user.
> > multiple copies is possibly fine. In reality we allow only one group per
> > nesting container (as described in patch [03/15]), and usually there
> > is just one SVA-capable device per group.
> > >
> > > BTW. for moving data copy to iommy layer, there is another point which
> > > need to consider. VFIO needs to do unbind in bind path if bind failed,
> > > so it will assemble unbind_data and pass to iommu layer. If iommu layer
> > > do the copy_from_user, I think it will be failed. any idea?
> If a call into a UAPI fails, there should be nothing to undo. Creating
> a partial setup for a failed call that needs to be undone by the caller
> is not good practice.
is it still a problem if it's the VFIO to undo the partial setup before
returning to user space?
> > This might be mitigated if we go back to use the same bind_data for both
> > bind/unbind. Then you can reuse the user object for unwinding.
> > However there is another case where VFIO may need to assemble the
> > bind_data itself. When a VM is killed, VFIO needs to walk allocated PASIDs
> > and unbind them one-by-one. In such case copy_from_user doesn't work
> > since the data is created by kernel. Alex, do you have a suggestion how this
> > usage can be supported? e.g. asking IOMMU driver to provide two sets of
> > APIs to handle user/kernel generated requests?
> Yes, it seems like vfio would need to make use of a driver API to do
> this, we shouldn't be faking a user buffer in the kernel in order to
> call through to a UAPI. Thanks,
ok, so if VFIO wants to issue unbind by itself, it should use an API which
passes kernel buffer to IOMMU layer. If the unbind request is from user
space, then VFIO should use another API which passes user buffer pointer
to IOMMU layer. makes sense. will align with jacob.
iommu mailing list
next prev parent reply other threads:[~2020-06-19 2:15 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-11 4:12 [PATCH v2 0/3] IOMMU user API enhancement Jacob Pan
2020-06-11 4:12 ` [PATCH v2 1/3] docs: IOMMU user API Jacob Pan
2020-06-11 6:33 ` Lu Baolu
2020-06-12 22:05 ` Jacob Pan
2020-06-11 9:30 ` Jonathan Cameron
2020-06-12 22:53 ` Jacob Pan
2020-06-11 13:55 ` Jonathan Corbet
2020-06-11 16:38 ` Jacob Pan
2020-06-11 15:47 ` Alex Williamson
2020-06-11 19:52 ` Jacob Pan
2020-06-11 20:40 ` Alex Williamson
2020-06-12 0:27 ` Jacob Pan
2020-06-12 7:38 ` Tian, Kevin
2020-06-12 13:09 ` Jacob Pan
2020-06-16 15:22 ` Jacob Pan
2020-06-17 6:20 ` Liu, Yi L
2020-06-17 8:28 ` Tian, Kevin
2020-06-18 21:48 ` Alex Williamson
2020-06-19 2:15 ` Liu, Yi L [this message]
2020-06-19 2:54 ` Alex Williamson
2020-06-19 3:30 ` Liu, Yi L
2020-06-19 16:37 ` Alex Williamson
2020-06-21 5:46 ` Liu, Yi L
2020-06-11 4:12 ` [PATCH v2 2/3] iommu/uapi: Add argsz for user filled data Jacob Pan
2020-06-11 16:49 ` Alex Williamson
2020-06-12 0:02 ` Jacob Pan
2020-06-11 4:12 ` [PATCH v2 3/3] iommu/vt-d: Sanity check uapi argsz filled by users Jacob Pan
2020-06-11 17:08 ` Alex Williamson
2020-06-11 20:02 ` Jacob Pan
2020-06-11 20:55 ` Alex Williamson
2020-06-11 23:58 ` 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).