Re: [PATCH 02/10] iommu/vt-d: Fix CPU and IOMMU SVM feature matching checks

From: Lu Baolu <baolu.lu@linux.intel.com>
To: Jacob Pan <jacob.jun.pan@linux.intel.com>,
	iommu@lists.linux-foundation.org,
	LKML <linux-kernel@vger.kernel.org>,
	Joerg Roedel <joro@8bytes.org>,
	David Woodhouse <dwmw2@infradead.org>
Cc: baolu.lu@linux.intel.com, "Tian, Kevin" <kevin.tian@intel.com>,
	Raj Ashok <ashok.raj@intel.com>, Yi Liu <yi.l.liu@intel.com>,
	Eric Auger <eric.auger@redhat.com>
Subject: Re: [PATCH 02/10] iommu/vt-d: Fix CPU and IOMMU SVM feature matching checks
Date: Mon, 18 Nov 2019 09:20:12 +0800	[thread overview]
Message-ID: <c5cac5ff-fc4c-f31d-ebad-8defa95169fb@linux.intel.com> (raw)
In-Reply-To: <1573859377-75924-3-git-send-email-jacob.jun.pan@linux.intel.com>

Hi,

On 11/16/19 7:09 AM, Jacob Pan wrote:
> The current code checks CPU and IOMMU feature set for SVM support but
> the result is never stored nor used. Therefore, SVM can still be used
> even when these checks failed.
> 
> This patch consolidates code for checking PASID, CPU vs. IOMMU paging
> mode compatibility, as well as provides specific error messages for
> each failed checks.
> 
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>


Acked-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu

> ---
>   drivers/iommu/intel-iommu.c | 10 ++--------
>   drivers/iommu/intel-svm.c   | 40 +++++++++++++++++++++++++++-------------
>   include/linux/intel-iommu.h |  4 +++-
>   3 files changed, 32 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 3f974919d3bd..d598168e410d 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -3289,10 +3289,7 @@ static int __init init_dmars(void)
>   
>   		if (!ecap_pass_through(iommu->ecap))
>   			hw_pass_through = 0;
> -#ifdef CONFIG_INTEL_IOMMU_SVM
> -		if (pasid_supported(iommu))
> -			intel_svm_init(iommu);
> -#endif
> +		intel_svm_check(iommu);
>   	}
>   
>   	/*
> @@ -4471,10 +4468,7 @@ static int intel_iommu_add(struct dmar_drhd_unit *dmaru)
>   	if (ret)
>   		goto out;
>   
> -#ifdef CONFIG_INTEL_IOMMU_SVM
> -	if (pasid_supported(iommu))
> -		intel_svm_init(iommu);
> -#endif
> +	intel_svm_check(iommu);
>   
>   	if (dmaru->ignored) {
>   		/*
> diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
> index 9b159132405d..e9773d714862 100644
> --- a/drivers/iommu/intel-svm.c
> +++ b/drivers/iommu/intel-svm.c
> @@ -23,19 +23,6 @@
>   
>   static irqreturn_t prq_event_thread(int irq, void *d);
>   
> -int intel_svm_init(struct intel_iommu *iommu)
> -{
> -	if (cpu_feature_enabled(X86_FEATURE_GBPAGES) &&
> -			!cap_fl1gp_support(iommu->cap))
> -		return -EINVAL;
> -
> -	if (cpu_feature_enabled(X86_FEATURE_LA57) &&
> -			!cap_5lp_support(iommu->cap))
> -		return -EINVAL;
> -
> -	return 0;
> -}
> -
>   #define PRQ_ORDER 0
>   
>   int intel_svm_enable_prq(struct intel_iommu *iommu)
> @@ -99,6 +86,33 @@ int intel_svm_finish_prq(struct intel_iommu *iommu)
>   	return 0;
>   }
>   
> +static inline bool intel_svm_capable(struct intel_iommu *iommu)
> +{
> +	return iommu->flags & VTD_FLAG_SVM_CAPABLE;
> +}
> +
> +void intel_svm_check(struct intel_iommu *iommu)
> +{
> +	if (!pasid_supported(iommu))
> +		return;
> +
> +	if (cpu_feature_enabled(X86_FEATURE_GBPAGES) &&
> +	    !cap_fl1gp_support(iommu->cap)) {
> +		pr_err("%s SVM disabled, incompatible 1GB page capability\n",
> +			iommu->name);
> +		return;
> +	}
> +
> +	if (cpu_feature_enabled(X86_FEATURE_LA57) &&
> +	    !cap_5lp_support(iommu->cap)) {
> +		pr_err("%s SVM disabled, incompatible paging mode\n",
> +			iommu->name);
> +		return;
> +	}
> +
> +	iommu->flags |= VTD_FLAG_SVM_CAPABLE;
> +}
> +
>   static void intel_flush_svm_range_dev (struct intel_svm *svm, struct intel_svm_dev *sdev,
>   				unsigned long address, unsigned long pages, int ih)
>   {
> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
> index 63118991824c..7dcfa1c4a844 100644
> --- a/include/linux/intel-iommu.h
> +++ b/include/linux/intel-iommu.h
> @@ -657,7 +657,7 @@ void iommu_flush_write_buffer(struct intel_iommu *iommu);
>   int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct device *dev);
>   
>   #ifdef CONFIG_INTEL_IOMMU_SVM
> -int intel_svm_init(struct intel_iommu *iommu);
> +extern void intel_svm_check(struct intel_iommu *iommu);
>   extern int intel_svm_enable_prq(struct intel_iommu *iommu);
>   extern int intel_svm_finish_prq(struct intel_iommu *iommu);
>   
> @@ -685,6 +685,8 @@ struct intel_svm {
>   };
>   
>   extern struct intel_iommu *intel_svm_device_to_iommu(struct device *dev);
> +#else
> +static inline void intel_svm_check(struct intel_iommu *iommu) {}
>   #endif
>   
>   #ifdef CONFIG_INTEL_IOMMU_DEBUGFS
>