All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iommu/amd: Make use of EFR from IVHD when available
@ 2021-01-18  5:19 Suravee Suthikulpanit
  2021-01-20 11:02 ` Suravee Suthikulpanit
  0 siblings, 1 reply; 2+ messages in thread
From: Suravee Suthikulpanit @ 2021-01-18  5:19 UTC (permalink / raw
  To: iommu; +Cc: brijesh.singh, Jon.Grimm, will.deacon

IOMMU Extended Feature Register (EFR) is used to communicate
the supported features for each IOMMU to the IOMMU driver.
This is normally read from the PCI MMIO register offset 0x30,
and used by the iommu_feature() helper function.

However, there are certain scenarios where the information is needed
prior to PCI initialization, and the iommu_feature() function is used
prematurely w/o warning. This has caused incorrect initialization of IOMMU.

The EFR is also available in the IVHD header, and is available to
the driver prior to PCI initialization. Therefore, default to using
the IVHD EFR instead.

Fixes: 6d39bdee238f ("iommu/amd: Enforce 4k mapping for certain IOMMU data structures")
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 drivers/iommu/amd/amd_iommu.h       |  3 ++-
 drivers/iommu/amd/amd_iommu_types.h |  4 +++
 drivers/iommu/amd/init.c            | 39 +++++++++++++++++++++++++++--
 3 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index 6b8cbdf71714..0a89e9c4f7b3 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -86,7 +86,8 @@ static inline bool is_rd890_iommu(struct pci_dev *pdev)
 
 static inline bool iommu_feature(struct amd_iommu *iommu, u64 f)
 {
-	if (!(iommu->cap & (1 << IOMMU_CAP_EFR)))
+	/* features == 0 means EFR is not supported */
+	if (!iommu->features)
 		return false;
 
 	return !!(iommu->features & f);
diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index 553587827771..35331e458dd1 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -387,6 +387,10 @@
 #define IOMMU_CAP_NPCACHE 26
 #define IOMMU_CAP_EFR     27
 
+/* IOMMU IVINFO */
+#define IOMMU_IVINFO_OFFSET          36
+#define IOMMU_IVINFO_EFRSUP_SHIFT    0
+
 /* IOMMU Feature Reporting Field (for IVHD type 10h */
 #define IOMMU_FEAT_GASUP_SHIFT	6
 
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 6a1f7048dacc..28b1d2feec96 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -257,6 +257,8 @@ static void init_device_table_dma(void);
 
 static bool amd_iommu_pre_enabled = true;
 
+static u32 amd_iommu_ivinfo;
+
 bool translation_pre_enabled(struct amd_iommu *iommu)
 {
 	return (iommu->flags & AMD_IOMMU_FLAG_TRANS_PRE_ENABLED);
@@ -1577,6 +1579,14 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
 
 		if (h->efr_reg & BIT(IOMMU_EFR_XTSUP_SHIFT))
 			amd_iommu_xt_mode = IRQ_REMAP_X2APIC_MODE;
+
+		/*
+		 * For IVHD type 0x11/0x40, EFR is also available via IVHD.
+		 * Default to IVHD EFR since it is available sooner
+		 * (i.e. before PCI init).
+		 */
+		if (amd_iommu_ivinfo & (1 << IOMMU_IVINFO_EFRSUP_SHIFT))
+			iommu->features = h->efr_reg;
 		break;
 	default:
 		return -EINVAL;
@@ -1770,6 +1780,29 @@ static const struct attribute_group *amd_iommu_groups[] = {
 	NULL,
 };
 
+/*
+ * Note: IVHD 0x11 and 0x40 also contains exact copy
+ * of the IOMMU Extended Feature Register [MMIO Offset 0030h].
+ * Default to EFR in IVHD since it is available sooner (i.e. before PCI init).
+ * However, sanity check and warn if they conflict.
+ */
+static void __init iommu_init_features(struct amd_iommu *iommu)
+{
+	u64 features;
+
+	if (!(iommu->cap & (1 << IOMMU_CAP_EFR)))
+		return;
+
+	/* read extended feature bits */
+	features = readq(iommu->mmio_base + MMIO_EXT_FEATURES);
+
+	if (iommu->features && (features != iommu->features))
+		pr_err(FW_BUG "EFR mismatch. Use IVHD EFR (%#llx : %#llx\n).",
+		       features, iommu->features);
+	else
+		iommu->features = features;
+}
+
 static int __init iommu_init_pci(struct amd_iommu *iommu)
 {
 	int cap_ptr = iommu->cap_ptr;
@@ -1789,8 +1822,7 @@ static int __init iommu_init_pci(struct amd_iommu *iommu)
 	if (!(iommu->cap & (1 << IOMMU_CAP_IOTLB)))
 		amd_iommu_iotlb_sup = false;
 
-	/* read extended feature bits */
-	iommu->features = readq(iommu->mmio_base + MMIO_EXT_FEATURES);
+	iommu_init_features(iommu);
 
 	if (iommu_feature(iommu, FEATURE_GT)) {
 		int glxval;
@@ -2661,6 +2693,9 @@ static int __init early_amd_iommu_init(void)
 	if (ret)
 		goto out;
 
+	/* Store IVRS IVinfo field. */
+	amd_iommu_ivinfo = *((u32 *)((u8 *)ivrs_base + IOMMU_IVINFO_OFFSET));
+
 	amd_iommu_target_ivhd_type = get_highest_supported_ivhd_type(ivrs_base);
 	DUMP_printk("Using IVHD type %#x\n", amd_iommu_target_ivhd_type);
 
-- 
2.17.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] iommu/amd: Make use of EFR from IVHD when available
  2021-01-18  5:19 [PATCH] iommu/amd: Make use of EFR from IVHD when available Suravee Suthikulpanit
@ 2021-01-20 11:02 ` Suravee Suthikulpanit
  0 siblings, 0 replies; 2+ messages in thread
From: Suravee Suthikulpanit @ 2021-01-20 11:02 UTC (permalink / raw
  To: iommu; +Cc: brijesh.singh, Jon.Grimm, will.deacon

I will send out v2 of this patch. Please ignore this v1.

Thanks,
Suravee

On 1/18/21 12:19 PM, Suravee Suthikulpanit wrote:
> IOMMU Extended Feature Register (EFR) is used to communicate
> the supported features for each IOMMU to the IOMMU driver.
> This is normally read from the PCI MMIO register offset 0x30,
> and used by the iommu_feature() helper function.
> 
> However, there are certain scenarios where the information is needed
> prior to PCI initialization, and the iommu_feature() function is used
> prematurely w/o warning. This has caused incorrect initialization of IOMMU.
> 
> The EFR is also available in the IVHD header, and is available to
> the driver prior to PCI initialization. Therefore, default to using
> the IVHD EFR instead.
> 
> Fixes: 6d39bdee238f ("iommu/amd: Enforce 4k mapping for certain IOMMU data structures")
> Tested-by: Brijesh Singh <brijesh.singh@amd.com>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>   drivers/iommu/amd/amd_iommu.h       |  3 ++-
>   drivers/iommu/amd/amd_iommu_types.h |  4 +++
>   drivers/iommu/amd/init.c            | 39 +++++++++++++++++++++++++++--
>   3 files changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
> index 6b8cbdf71714..0a89e9c4f7b3 100644
> --- a/drivers/iommu/amd/amd_iommu.h
> +++ b/drivers/iommu/amd/amd_iommu.h
> @@ -86,7 +86,8 @@ static inline bool is_rd890_iommu(struct pci_dev *pdev)
>   
>   static inline bool iommu_feature(struct amd_iommu *iommu, u64 f)
>   {
> -	if (!(iommu->cap & (1 << IOMMU_CAP_EFR)))
> +	/* features == 0 means EFR is not supported */
> +	if (!iommu->features)
>   		return false;
>   
>   	return !!(iommu->features & f);
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 553587827771..35331e458dd1 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -387,6 +387,10 @@
>   #define IOMMU_CAP_NPCACHE 26
>   #define IOMMU_CAP_EFR     27
>   
> +/* IOMMU IVINFO */
> +#define IOMMU_IVINFO_OFFSET          36
> +#define IOMMU_IVINFO_EFRSUP_SHIFT    0
> +
>   /* IOMMU Feature Reporting Field (for IVHD type 10h */
>   #define IOMMU_FEAT_GASUP_SHIFT	6
>   
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 6a1f7048dacc..28b1d2feec96 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -257,6 +257,8 @@ static void init_device_table_dma(void);
>   
>   static bool amd_iommu_pre_enabled = true;
>   
> +static u32 amd_iommu_ivinfo;
> +
>   bool translation_pre_enabled(struct amd_iommu *iommu)
>   {
>   	return (iommu->flags & AMD_IOMMU_FLAG_TRANS_PRE_ENABLED);
> @@ -1577,6 +1579,14 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
>   
>   		if (h->efr_reg & BIT(IOMMU_EFR_XTSUP_SHIFT))
>   			amd_iommu_xt_mode = IRQ_REMAP_X2APIC_MODE;
> +
> +		/*
> +		 * For IVHD type 0x11/0x40, EFR is also available via IVHD.
> +		 * Default to IVHD EFR since it is available sooner
> +		 * (i.e. before PCI init).
> +		 */
> +		if (amd_iommu_ivinfo & (1 << IOMMU_IVINFO_EFRSUP_SHIFT))
> +			iommu->features = h->efr_reg;
>   		break;
>   	default:
>   		return -EINVAL;
> @@ -1770,6 +1780,29 @@ static const struct attribute_group *amd_iommu_groups[] = {
>   	NULL,
>   };
>   
> +/*
> + * Note: IVHD 0x11 and 0x40 also contains exact copy
> + * of the IOMMU Extended Feature Register [MMIO Offset 0030h].
> + * Default to EFR in IVHD since it is available sooner (i.e. before PCI init).
> + * However, sanity check and warn if they conflict.
> + */
> +static void __init iommu_init_features(struct amd_iommu *iommu)
> +{
> +	u64 features;
> +
> +	if (!(iommu->cap & (1 << IOMMU_CAP_EFR)))
> +		return;
> +
> +	/* read extended feature bits */
> +	features = readq(iommu->mmio_base + MMIO_EXT_FEATURES);
> +
> +	if (iommu->features && (features != iommu->features))
> +		pr_err(FW_BUG "EFR mismatch. Use IVHD EFR (%#llx : %#llx\n).",
> +		       features, iommu->features);
> +	else
> +		iommu->features = features;
> +}
> +
>   static int __init iommu_init_pci(struct amd_iommu *iommu)
>   {
>   	int cap_ptr = iommu->cap_ptr;
> @@ -1789,8 +1822,7 @@ static int __init iommu_init_pci(struct amd_iommu *iommu)
>   	if (!(iommu->cap & (1 << IOMMU_CAP_IOTLB)))
>   		amd_iommu_iotlb_sup = false;
>   
> -	/* read extended feature bits */
> -	iommu->features = readq(iommu->mmio_base + MMIO_EXT_FEATURES);
> +	iommu_init_features(iommu);
>   
>   	if (iommu_feature(iommu, FEATURE_GT)) {
>   		int glxval;
> @@ -2661,6 +2693,9 @@ static int __init early_amd_iommu_init(void)
>   	if (ret)
>   		goto out;
>   
> +	/* Store IVRS IVinfo field. */
> +	amd_iommu_ivinfo = *((u32 *)((u8 *)ivrs_base + IOMMU_IVINFO_OFFSET));
> +
>   	amd_iommu_target_ivhd_type = get_highest_supported_ivhd_type(ivrs_base);
>   	DUMP_printk("Using IVHD type %#x\n", amd_iommu_target_ivhd_type);
>   
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-01-20 11:03 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-18  5:19 [PATCH] iommu/amd: Make use of EFR from IVHD when available Suravee Suthikulpanit
2021-01-20 11:02 ` Suravee Suthikulpanit

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.