Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: GDWBM4ICIyYL4g+ZiJ6Xdx2SO77lOhqUOGKVc8oupANUy/4oyKL8rcDsyx1b2bFhJeWLo6fKRF
 u474Zb5Jbbhw==
IronPort-SDR: 7mVjtdxduOhFD/qaBlx9o4lynInMuvOEE55/fJFzvq8O3LoZz3ue7rXJ2xgmFz8gjZXlaOAAMM
 HquB5oNUgcIQ==
Date:   Thu, 15 Apr 2021 11:53:48 -0700
From:   Jacob Pan <jacob.jun.pan@linux.intel.com>
To:     Christoph Hellwig <hch@infradead.org>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        iommu@lists.linux-foundation.org, Joerg Roedel <joro@8bytes.org>,
        Lu Baolu <baolu.lu@linux.intel.com>,
        Jean-Philippe Brucker <jean-philippe@linaro.com>,
        "Tian, Kevin" <kevin.tian@intel.com>,
        Dave Jiang <dave.jiang@intel.com>,
        Raj Ashok <ashok.raj@intel.com>, vkoul@kernel.org,
        Jason Gunthorpe <jgg@nvidia.com>, zhangfei.gao@linaro.org,
        jacob.jun.pan@linux.intel.com
Subject: Re: [PATCH v2 1/2] iommu/sva: Tighten SVA bind API with explicit
 flags
Message-ID: <20210415115348.107554aa@jacob-builder>
In-Reply-To: <20210415064033.GA1938497@infradead.org>
References: <1618414077-28808-1-git-send-email-jacob.jun.pan@linux.intel.com>
        <1618414077-28808-2-git-send-email-jacob.jun.pan@linux.intel.com>
        <20210415064033.GA1938497@infradead.org>
Organization: OTC
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Christoph,

Thanks for the review.

On Thu, 15 Apr 2021 07:40:33 +0100, Christoph Hellwig <hch@infradead.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