On Wed, Mar 03, 2021 at 11:42:12AM -0800, Jacob Pan wrote:
> Hi Jason,
>
> On Tue, 2 Mar 2021 13:15:51 -0400, Jason Gunthorpe <jgg@xxxxxxxxxx> wrote:
>
> > 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@xxxxxxxxxx>
> > > 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.
> >
> From the user's p.o.v., it is plain c code nothing obfuscated. As for
> kernel handling of the data types, it has to be answered by the bigger
> question of how we deal with sharing IOMMU among multiple subsystems with
> UAPIs.
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1;
t=1614800733; bh=ESjraPQ1U+x3dvWd7l3HBKlTBu3ySX2nWO4QM44ApEU=;
h=ARC-Seal:ARC-Message-Signature:ARC-Authentication-Results:Date:
From:To:CC:Subject:Message-ID:References:Content-Type:
Content-Disposition:In-Reply-To:X-ClientProxiedBy:MIME-Version:
X-MS-Exchange-MessageSentRepresentingType:X-Header;
b=Jyxs64SLsrGfmoPaAHc58Bpre6/+/wDGs/dAMDviWTwRc+2Miw0+jOBGQXNLz/lE8
KbS/K02BZmgn/jJwl994Po6nS3kGgTg0of6AFSd2MqtaZPMx+aMJ3prec9hwpHaXmN
SiosC+FviWoFQHmEvWSQoVeEMS093zQ+sjcsymUkMXYHGYQRqebW101Mii6/0hT3iv
Su4YvAyaKmOpWT8sayI4K0ICIhRxWAOT5P78FrXofij9o3X9T9F/9Bo+S5BWSCMzMr
+dM2KsTd4Ecac9hNemigs87T/tiCh50XaZoc8WMRFWGYMpha6KFdCV5wWL0Yzauyt7
H7uCWkmg/QTCw==
As I said, don't obfuscate types like this in the kernel. It is not
good style.
> However, IOMMU is a system device which has little value to be exposed to
> the userspace. Not to mention the device-IOMMU affinity/topology. VFIO
> nicely abstracts IOMMU from the userspace, why do we want to reverse that?
The other patch was talking about a /dev/ioasid - why can't this stuff
be run over that?
Jason
