Hi Jacob,
On 11/19/19 6:56 PM, Jacob Pan wrote:
> Shared Virtual Memory(SVM) is based on a collective set of hardware
> features detected at runtime. There are requirements for matching CPU
> and IOMMU capabilities.
>
> 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. The consequences can be:
> 1. CPU uses 5-level paging mode for virtual address of 57 bits, but
> IOMMU can only support 4-level paging mode with 48 bits address for DMA.
> 2. 1GB page size is used by CPU but IOMMU does not support it. VT-d
> unrecoverable faults may be generated.
>
> The best solution to fix these problems is to prevent them in the first
> place.
>
> This patch consolidates code for checking PASID, CPU vs. IOMMU paging
> mode compatibility, as well as provides specific error messages for
> each failed checks. On sane hardware configurations, these error message
> shall never appear in kernel log.
>
> Signed-off-by: Jacob Pan <[email protected]>
> Acked-by: Lu Baolu <[email protected]>
Reviewed-by: Eric Auger <[email protected]>
Thanks
Eric
> ---
> drivers/iommu/intel-iommu.c | 10 ++--------
> drivers/iommu/intel-svm.c | 40 +++++++++++++++++++++++++++-------------
> include/linux/intel-iommu.h | 5 ++++-
> 3 files changed, 33 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..716c543488f6 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 ed11ef594378..7dcfa1c4a844 100644
> --- a/include/linux/intel-iommu.h
> +++ b/include/linux/intel-iommu.h
> @@ -433,6 +433,7 @@ enum {
>
> #define VTD_FLAG_TRANS_PRE_ENABLED (1 << 0)
> #define VTD_FLAG_IRQ_REMAP_PRE_ENABLED (1 << 1)
> +#define VTD_FLAG_SVM_CAPABLE (1 << 2)
>
> extern int intel_iommu_sm;
>
> @@ -656,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);
>
> @@ -684,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
>
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
