From: Jason Gunthorpe <jgg@nvidia.com>
To: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: jean-philippe@linaro.org, kevin.tian@intel.com,
ashok.raj@intel.com, kvm@vger.kernel.org, vivek.gautam@arm.com,
jasowang@redhat.com, stefanha@gmail.com, yi.y.sun@intel.com,
alex.williamson@redhat.com, iommu@lists.linux-foundation.org,
Lingshan.Zhu@intel.com, hao.wu@intel.com, jun.j.tian@intel.com
Subject: Re: [Patch v8 04/10] vfio/type1: Support binding guest page tables to PASID
Date: Tue, 2 Mar 2021 13:15:51 -0400 [thread overview]
Message-ID: <20210302171551.GK4247@nvidia.com> (raw)
In-Reply-To: <20210302091319.1446a47b@jacob-builder>
On Tue, Mar 02, 2021 at 09:13:19AM -0800, Jacob Pan wrote:
> Hi Jason,
>
> On Tue, 2 Mar 2021 08:56:28 -0400, Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> > On Wed, Mar 03, 2021 at 04:35:39AM +0800, Liu Yi L wrote:
> > >
> > > +static int vfio_dev_bind_gpasid_fn(struct device *dev, void *data)
> > > +{
> > > + struct domain_capsule *dc = (struct domain_capsule *)data;
> > > + unsigned long arg = *(unsigned long *)dc->data;
> > > +
> > > + return iommu_uapi_sva_bind_gpasid(dc->domain, dev,
> > > + (void __user *)arg);
> >
> > This arg buisness is really tortured. The type should be set at the
> > ioctl, not constantly passed down as unsigned long or worse void *.
> >
> > And why is this passing a __user pointer deep into an iommu_* API??
> >
> The idea was that IOMMU UAPI (not API) is independent of VFIO or other user
> driver frameworks. The design is documented here:
> Documentation/userspace-api/iommu.rst
> IOMMU UAPI handles the type and sanitation of user provided data.
Why? If it is uapi it has defined types and those types should be
completely clear from the C code, not obfuscated.
I haven't looked at the design doc yet, but this is a just a big red
flag, you shouldn't be tunneling one subsytems uAPI through another
subsystem.
If you need to hook two subsystems together it should be more
directly, like VFIO takes in the IOMMU FD and 'registers' itself in
some way with the IOMMU then you can do the IOMMU actions through the
IOMMU FD and it can call back to VFIO as needed.
At least in this way we can swap VFIO for other things in the API.
Having every subsystem that wants to implement IOMMU also implement
tunneled ops seems very backwards.
> Could you be more specific about your concerns?
Avoid using unsigned long, void * and flex arrays to describe
concretely typed things.
Jason
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2021-03-02 17:16 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-02 20:35 [Patch v8 00/10] vfio: expose virtual Shared Virtual Addressing to VMs Liu Yi L
2021-03-02 20:35 ` [Patch v8 01/10] iommu: Report domain nesting info Liu Yi L
2021-03-02 20:35 ` [Patch v8 02/10] iommu/smmu: Report empty " Liu Yi L
2021-03-02 20:35 ` [Patch v8 03/10] vfio/type1: Report iommu nesting info to userspace Liu Yi L
2021-03-02 12:52 ` Jason Gunthorpe
2021-03-03 9:53 ` Liu, Yi L
2021-03-02 20:35 ` [Patch v8 04/10] vfio/type1: Support binding guest page tables to PASID Liu Yi L
2021-03-02 12:56 ` Jason Gunthorpe
2021-03-02 17:13 ` Jacob Pan
2021-03-02 17:15 ` Jason Gunthorpe [this message]
2021-03-03 19:42 ` Jacob Pan
2021-03-03 19:45 ` Jason Gunthorpe
2021-03-04 7:20 ` Liu, Yi L
2021-03-04 12:52 ` Jason Gunthorpe
2021-03-02 20:35 ` [Patch v8 05/10] vfio/type1: Allow invalidating first-level/stage IOMMU cache Liu Yi L
2021-03-02 20:35 ` [Patch v8 06/10] iommu: Pass domain to sva_unbind_gpasid() Liu Yi L
2021-03-02 20:35 ` [Patch v8 07/10] vfio/type1: Add vSVA support for IOMMU-backed mdevs Liu Yi L
2021-03-02 20:35 ` [Patch v8 08/10] vfio/pci: Expose PCIe PASID capability to userspace Liu Yi L
2021-03-02 20:35 ` [Patch v8 09/10] vfio: Document dual stage control Liu Yi L
2021-03-02 20:35 ` [Patch v8 10/10] iommu/vt-d: Support reporting nesting capability info Liu Yi L
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=20210302171551.GK4247@nvidia.com \
--to=jgg@nvidia.com \
--cc=Lingshan.Zhu@intel.com \
--cc=alex.williamson@redhat.com \
--cc=ashok.raj@intel.com \
--cc=hao.wu@intel.com \
--cc=iommu@lists.linux-foundation.org \
--cc=jacob.jun.pan@linux.intel.com \
--cc=jasowang@redhat.com \
--cc=jean-philippe@linaro.org \
--cc=jun.j.tian@intel.com \
--cc=kevin.tian@intel.com \
--cc=kvm@vger.kernel.org \
--cc=stefanha@gmail.com \
--cc=vivek.gautam@arm.com \
--cc=yi.y.sun@intel.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox