linux-kernel - Re: [PATCH v4 04/22] iommu/vt-d: add bind_pasid

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 5 Jun 2018 10:32:53 -0700
From:   Jacob Pan <jacob.jun.pan@...ux.intel.com>
To:     Jean-Philippe Brucker <jean-philippe.brucker@....com>
Cc:     "Tian, Kevin" <kevin.tian@...el.com>,
        Alex Williamson <alex.williamson@...hat.com>,
        "iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Joerg Roedel <joro@...tes.org>,
        David Woodhouse <dwmw2@...radead.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
        "Liu, Yi L" <yi.l.liu@...el.com>,
        "Raj, Ashok" <ashok.raj@...el.com>,
        Christoph Hellwig <hch@...radead.org>,
        Lu Baolu <baolu.lu@...ux.intel.com>,
        Yi L <yi.l.liu@...ux.intel.com>,
        Auger Eric <eric.auger@...hat.com>,
        jacob.jun.pan@...ux.intel.com
Subject: Re: [PATCH v4 04/22] iommu/vt-d: add bind_pasid_table function

On Thu, 31 May 2018 10:09:46 +0100
Jean-Philippe Brucker <jean-philippe.brucker@....com> wrote:

> On 30/05/18 20:52, Jacob Pan wrote:
> >> However I think the model number should be added to
> >> pasid_table_config. For one thing it gives us a simple
> >> sanity-check, but it also tells which other fields are valid in
> >> pasid_table_config. Arm-smmu-v3 needs at least two additional
> >> 8-bit fields describing the PASID table format (number of levels
> >> and PASID0 behaviour), which are written to device context tables
> >> when installing the PASID table pointer.
> >>  
> > We had model number field in v2 of this patchset. My thought was
> > that since the config info is meant to be generic, we shouldn't
> > include model info. But I also think a simple sanity check can be
> > useful, would that be sufficient to address Alex's concern? Of
> > course we still need sysfs for more specific IOMMU features.
> > 
> > Would this work?
> > enum pasid_table_model {
> > 	PASID_TABLE_FORMAT_HOST,
> > 	PASID_TABLE_FORMAT_ARM_1LVL,
> > 	PASID_TABLE_FORMAT_ARM_2LVL,  
> 
> I'd rather use a single PASID_TABLE_FORMAT_ARM, because "2LVL" may be
> further split into 2LVL_4k or 2LVL_64k leaf tables... I think it's
> best if I add an arch-specific field in pasid_table_config for that,
> and for the PASID0 configuration, when adding FORMAT_ARM in a future
> patch
> 
sounds good. will only use PASID_TABLE_FORMAT_ARM.
> > 	PASID_TABLE_FORMAT_AMD,
> > 	PASID_TABLE_FORMAT_INTEL,
> > };
> > 
> > /**
> >  * PASID table data used to bind guest PASID table to the host
> > IOMMU. This will
> >  * enable guest managed first level page tables.
> >  * @version: for future extensions and identification of the data
> > format
> >  * @bytes: size of this structure
> >  * @model: PASID table format for different IOMMU models
> >  * @base_ptr:	PASID table pointer
> >  * @pasid_bits:	number of bits supported in the guest PASID
> > table, must be less
> >  *		or equal than the host supported PASID size.
> >  */
> > struct pasid_table_config {
> > 	__u32 version;
> > #define PASID_TABLE_CFG_VERSION_1 1
> > 	__u32 bytes;  
> 
> "bytes" could be passed by VFIO as argument to bind_pasid_table, since
> it can deduce it from argsz
> 
Are you suggesting we wrap this struct in a vfio struct with argsz? or
we directly use this struct?

I need to clarify how vfio will use this.
- User program:
struct pasid_table_config ptc = { .bytes = sizeof(ptc) };
ptc.version = 1;
ioctl(device, VFIO_DEVICE_BIND_PASID_TABLE, &ptc);

- Kernel:
minsz = offsetofend(struct pasid_table_config, pasid_bits);
if (ptc.bytes < minsz)
	return -EINVAL;