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

Lu Baolu
2019-11-18T01:23:18Z

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
>