linux-kernel - Re: [PATCH v4 15/22] iommu/vt-d: Replace Intel specific PASID allocator with IOASID

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]

Message-ID: <20190624143626.0dd8d46d@jacob-builder>
Date:   Mon, 24 Jun 2019 14:36:26 -0700
From:   Jacob Pan <jacob.jun.pan@...ux.intel.com>
To:     Jonathan Cameron <jonathan.cameron@...wei.com>
Cc:     <iommu@...ts.linux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Joerg Roedel <joro@...tes.org>,
        David Woodhouse <dwmw2@...radead.org>,
        "Eric Auger" <eric.auger@...hat.com>,
        Alex Williamson <alex.williamson@...hat.com>,
        Jean-Philippe Brucker <jean-philippe.brucker@....com>,
        "Tian, Kevin" <kevin.tian@...el.com>,
        Raj Ashok <ashok.raj@...el.com>,
        Andriy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        jacob.jun.pan@...ux.intel.com
Subject: Re: [PATCH v4 15/22] iommu/vt-d: Replace Intel specific PASID
 allocator with IOASID

On Tue, 18 Jun 2019 16:57:09 +0100
Jonathan Cameron <jonathan.cameron@...wei.com> wrote:

> On Sun, 9 Jun 2019 06:44:15 -0700
> Jacob Pan <jacob.jun.pan@...ux.intel.com> wrote:
> 
> > Make use of generic IOASID code to manage PASID allocation,
> > free, and lookup. Replace Intel specific code.
> > 
> > Signed-off-by: Jacob Pan <jacob.jun.pan@...ux.intel.com>  
> Hi Jacob,
> 
> One question inline.
> 
Hi Jonathan,

Sorry, I got distracted and missed your review. My answer
below.

Thanks!

Jacob

> Jonathan
> 
> > ---
> >  drivers/iommu/intel-iommu.c | 11 +++++------
> >  drivers/iommu/intel-pasid.c | 36
> > ------------------------------------ drivers/iommu/intel-svm.c   |
> > 37 +++++++++++++++++++++---------------- 3 files changed, 26
> > insertions(+), 58 deletions(-)
> > 
> > diff --git a/drivers/iommu/intel-iommu.c
> > b/drivers/iommu/intel-iommu.c index 5b84994..39b63fe 100644
> > --- a/drivers/iommu/intel-iommu.c
> > +++ b/drivers/iommu/intel-iommu.c
> > @@ -5167,7 +5167,7 @@ static void auxiliary_unlink_device(struct
> > dmar_domain *domain, domain->auxd_refcnt--;
> >  
> >  	if (!domain->auxd_refcnt && domain->default_pasid > 0)
> > -		intel_pasid_free_id(domain->default_pasid);
> > +		ioasid_free(domain->default_pasid);
> >  }
> >  
> >  static int aux_domain_add_dev(struct dmar_domain *domain,
> > @@ -5185,10 +5185,9 @@ static int aux_domain_add_dev(struct
> > dmar_domain *domain, if (domain->default_pasid <= 0) {
> >  		int pasid;
> >  
> > -		pasid = intel_pasid_alloc_id(domain, PASID_MIN,
> > -
> > pci_max_pasids(to_pci_dev(dev)),
> > -					     GFP_KERNEL);
> > -		if (pasid <= 0) {
> > +		pasid = ioasid_alloc(NULL, PASID_MIN,
> > pci_max_pasids(to_pci_dev(dev)) - 1,
> > +				domain);  
> 
> Is there any point in passing the domain in as the private pointer
> here? I can't immediately see anywhere it is read back?
> 
Good point, I agree. There is no need for passing the domain. aux domain
and its defaul_pasid has 1:1 mapping.

> It is also rather confusing as the same driver stashes two different
> types of data in the same xarray.

On VT-d, there should be only one type of data for all PASID users that
needs private data. Whether it is native SVA or guest SVA. Only an
internal flag will tell them apart.