linux-kernel - Re: [PATCH v2 1/2] iommu/sva: Tighten SVA bind API with explicit flags

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:   Thu, 15 Apr 2021 11:53:48 -0700
From:   Jacob Pan <jacob.jun.pan@...ux.intel.com>
To:     Christoph Hellwig <hch@...radead.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        iommu@...ts.linux-foundation.org, Joerg Roedel <joro@...tes.org>,
        Lu Baolu <baolu.lu@...ux.intel.com>,
        Jean-Philippe Brucker <jean-philippe@...aro.com>,
        "Tian, Kevin" <kevin.tian@...el.com>,
        Dave Jiang <dave.jiang@...el.com>,
        Raj Ashok <ashok.raj@...el.com>, vkoul@...nel.org,
        Jason Gunthorpe <jgg@...dia.com>, zhangfei.gao@...aro.org,
        jacob.jun.pan@...ux.intel.com
Subject: Re: [PATCH v2 1/2] iommu/sva: Tighten SVA bind API with explicit
 flags

Hi Christoph,

Thanks for the review.

On Thu, 15 Apr 2021 07:40:33 +0100, Christoph Hellwig <hch@...radead.org>
wrote:

> On Wed, Apr 14, 2021 at 08:27:56AM -0700, Jacob Pan wrote:
> >  static int idxd_enable_system_pasid(struct idxd_device *idxd)
> >  {
> > -	int flags;
> > +	unsigned int flags;
> >  	unsigned int pasid;
> >  	struct iommu_sva *sva;
> >  
> > -	flags = SVM_FLAG_SUPERVISOR_MODE;
> > +	flags = IOMMU_SVA_BIND_SUPERVISOR;
> >  
> > -	sva = iommu_sva_bind_device(&idxd->pdev->dev, NULL, &flags);
> > +	sva = iommu_sva_bind_device(&idxd->pdev->dev, NULL, flags);  
> 
> Please also remove the now pointless flags variable.
> 
Good catch.

> > +iommu_sva_bind_device(struct device *dev, struct mm_struct *mm,
> > unsigned int flags)  
> 
> Pleae avoid the pointless overly long line.
> 
> > -#define SVM_FLAG_GUEST_PASID		(1<<3)
> > +#define SVM_FLAG_GUEST_PASID		(1<<2)  
> 
> This flag is entirely unused, please just remove it in a prep patch
> rather than renumbering it.
> 
You are right. The flag was set and intended to be used by the guest IO
page request patches by Baolu.

As you might be aware, we are restructuring the guest SVA uAPI according to
Jason's proposal, can we wait until we have a clear solution? We may
refactor lots of code.

> >  static inline struct iommu_sva *
> > -iommu_sva_bind_device(struct device *dev, struct mm_struct *mm, void
> > *drvdata) +iommu_sva_bind_device(struct device *dev, struct mm_struct
> > *mm, unsigned int flags)  
> 
> Same overy long line here.
This is temporary as the mm parameter will be removed in the next patch.

Thanks,

Jacob