[PATCH 0/3] iommu/amd: Fix booting with amd_iommu=off

[PATCH 0/3] iommu/amd: Fix booting with amd_iommu=off

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Hi,

it turned out that booting a kernel with amd_iommu=off on a machine
that has an AMD IOMMU causes an early kernel crash. There are two
reasons for this, and fixing one of them is already sufficient, but
both reasons deserve fixing, which is done in this patch-set.

Regards,

	Joerg

Joerg Roedel (3):
  iommu/amd: Move Stoney Ridge check to detect_ivrs()
  iommu/amd: Don't call early_amd_iommu_init() when AMD IOMMU is
    disabled
  iommu/amd: Keep track of amd_iommu_irq_remap state

 drivers/iommu/amd/init.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

-- 
2.30.2

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

[PATCH 1/3] iommu/amd: Move Stoney Ridge check to detect_ivrs()

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

The AMD IOMMU will not be enabled on AMD Stoney Ridge systems. Bail
out even earlier and refuse to even detect the IOMMU there.

References: https://bugzilla.kernel.org/show_bug.cgi?id=212133
References: https://bugzilla.suse.com/show_bug.cgi?id=1183132
Cc: stable@vger.kernel.org # v5.11
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/amd/init.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 9126efcbaf2c..3280e6f5b720 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -2714,7 +2714,6 @@ static int __init early_amd_iommu_init(void)
 	struct acpi_table_header *ivrs_base;
 	int i, remap_cache_sz, ret;
 	acpi_status status;
-	u32 pci_id;
 
 	if (!amd_iommu_detected)
 		return -ENODEV;
@@ -2804,16 +2803,6 @@ static int __init early_amd_iommu_init(void)
 	if (ret)
 		goto out;
 
-	/* Disable IOMMU if there's Stoney Ridge graphics */
-	for (i = 0; i < 32; i++) {
-		pci_id = read_pci_config(0, i, 0, 0);
-		if ((pci_id & 0xffff) == 0x1002 && (pci_id >> 16) == 0x98e4) {
-			pr_info("Disable IOMMU on Stoney Ridge\n");
-			amd_iommu_disabled = true;
-			break;
-		}
-	}
-
 	/* Disable any previously enabled IOMMUs */
 	if (!is_kdump_kernel() || amd_iommu_disabled)
 		disable_iommus();
@@ -2880,6 +2869,7 @@ static bool detect_ivrs(void)
 {
 	struct acpi_table_header *ivrs_base;
 	acpi_status status;
+	int i;
 
 	status = acpi_get_table("IVRS", 0, &ivrs_base);
 	if (status == AE_NOT_FOUND)
@@ -2892,6 +2882,17 @@ static bool detect_ivrs(void)
 
 	acpi_put_table(ivrs_base);
 
+	/* Don't use IOMMU if there is Stoney Ridge graphics */
+	for (i = 0; i < 32; i++) {
+		u32 pci_id;
+
+		pci_id = read_pci_config(0, i, 0, 0);
+		if ((pci_id & 0xffff) == 0x1002 && (pci_id >> 16) == 0x98e4) {
+			pr_info("Disable IOMMU on Stoney Ridge\n");
+			return false;
+		}
+	}
+
 	/* Make sure ACS will be enabled during PCI probe */
 	pci_request_acs();
 
-- 
2.30.2

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

[PATCH 2/3] iommu/amd: Don't call early_amd_iommu_init() when AMD IOMMU is disabled

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Don't even try to initialize the AMD IOMMU hardware when amd_iommu=off has been
passed on the kernel command line.

References: https://bugzilla.kernel.org/show_bug.cgi?id=212133
References: https://bugzilla.suse.com/show_bug.cgi?id=1183132
Cc: stable@vger.kernel.org # v5.11
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/amd/init.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 3280e6f5b720..61dae1800b7f 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -2919,12 +2919,12 @@ static int __init state_next(void)
 		}
 		break;
 	case IOMMU_IVRS_DETECTED:
-		ret = early_amd_iommu_init();
-		init_state = ret ? IOMMU_INIT_ERROR : IOMMU_ACPI_FINISHED;
-		if (init_state == IOMMU_ACPI_FINISHED && amd_iommu_disabled) {
-			pr_info("AMD IOMMU disabled\n");
+		if (amd_iommu_disabled) {
 			init_state = IOMMU_CMDLINE_DISABLED;
 			ret = -EINVAL;
+		} else {
+			ret = early_amd_iommu_init();
+			init_state = ret ? IOMMU_INIT_ERROR : IOMMU_ACPI_FINISHED;
 		}
 		break;
 	case IOMMU_ACPI_FINISHED:
-- 
2.30.2

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

[PATCH 3/3] iommu/amd: Keep track of amd_iommu_irq_remap state

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

The amd_iommu_irq_remap variable is set to true in amd_iommu_prepare().
But if initialization fails it is not set to false. Fix that and
correctly keep track of whether irq remapping is enabled or not.

References: https://bugzilla.kernel.org/show_bug.cgi?id=212133
References: https://bugzilla.suse.com/show_bug.cgi?id=1183132
Fixes: b34f10c2dc59 ("iommu/amd: Stop irq_remapping_select() matching when remapping is disabled")
Cc: stable@vger.kernel.org # v5.11
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/amd/init.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 61dae1800b7f..321f5906e6ed 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -3002,8 +3002,11 @@ int __init amd_iommu_prepare(void)
 	amd_iommu_irq_remap = true;
 
 	ret = iommu_go_to_state(IOMMU_ACPI_FINISHED);
-	if (ret)
+	if (ret) {
+		amd_iommu_irq_remap = false;
 		return ret;
+	}
+
 	return amd_iommu_irq_remap ? 0 : -ENODEV;
 }
 
-- 
2.30.2

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

Re: [PATCH 0/3] iommu/amd: Fix booting with amd_iommu=off

[Huang Rui]

On Wed, Mar 17, 2021 at 05:10:34PM +0800, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> Hi,
> 
> it turned out that booting a kernel with amd_iommu=off on a machine
> that has an AMD IOMMU causes an early kernel crash. There are two
> reasons for this, and fixing one of them is already sufficient, but
> both reasons deserve fixing, which is done in this patch-set.
> 
> Regards,
> 
> 	Joerg
> 
> Joerg Roedel (3):
>   iommu/amd: Move Stoney Ridge check to detect_ivrs()
>   iommu/amd: Don't call early_amd_iommu_init() when AMD IOMMU is
>     disabled
>   iommu/amd: Keep track of amd_iommu_irq_remap state

Series are Acked-by: Huang Rui <ray.huang@amd.com>

> 
>  drivers/iommu/amd/init.c | 36 ++++++++++++++++++++----------------
>  1 file changed, 20 insertions(+), 16 deletions(-)
> 
> -- 
> 2.30.2
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 2/3] iommu/amd: Don't call early_amd_iommu_init() when AMD IOMMU is disabled

[David Woodhouse]

[-- Attachment #1.1: Type: text/plain, Size: 1310 bytes --]

On Wed, 2021-03-17 at 10:10 +0100, Joerg Roedel wrote:
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 3280e6f5b720..61dae1800b7f 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -2919,12 +2919,12 @@ static int __init state_next(void)
>                 }
>                 break;
>         case IOMMU_IVRS_DETECTED:
> -               ret = early_amd_iommu_init();
> -               init_state = ret ? IOMMU_INIT_ERROR : IOMMU_ACPI_FINISHED;
> -               if (init_state == IOMMU_ACPI_FINISHED && amd_iommu_disabled) {
> -                       pr_info("AMD IOMMU disabled\n");
> +               if (amd_iommu_disabled) {
>                         init_state = IOMMU_CMDLINE_DISABLED;
>                         ret = -EINVAL;
> +               } else {
> +                       ret = early_amd_iommu_init();
> +                       init_state = ret ? IOMMU_INIT_ERROR : IOMMU_ACPI_FINISHED;
>                 }
>                 break;
>         case IOMMU_ACPI_FINISHED:
> -- 

If you've already moved the Stoney Ridge check out of the way, there's
no real reason why you can't just set init_state=IOMMU_CMDLINE_DISABLED
directly from parse_amd_iommu_options(), is there? Then you don't need
the condition here at all?

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

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

Re: [PATCH 2/3] iommu/amd: Don't call early_amd_iommu_init() when AMD IOMMU is disabled

[Joerg Roedel]

On Wed, Mar 17, 2021 at 11:47:11AM +0000, David Woodhouse wrote:
> If you've already moved the Stoney Ridge check out of the way, there's
> no real reason why you can't just set init_state=IOMMU_CMDLINE_DISABLED
> directly from parse_amd_iommu_options(), is there? Then you don't need
> the condition here at all?

True, there is even more room for optimization. The amd_iommu_disabled
variable can go away entirely, including its checks in
early_amd_iommu_init(). I will do a patch-set on-top of this for v5.13
which does more cleanups.

Thanks,

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

Re: [PATCH 2/3] iommu/amd: Don't call early_amd_iommu_init() when AMD IOMMU is disabled

[David Woodhouse]

On 17 March 2021 13:32:35 GMT, Joerg Roedel <joro@8bytes.org> wrote:
>On Wed, Mar 17, 2021 at 11:47:11AM +0000, David Woodhouse wrote:
>> If you've already moved the Stoney Ridge check out of the way,
>there's
>> no real reason why you can't just set
>init_state=IOMMU_CMDLINE_DISABLED
>> directly from parse_amd_iommu_options(), is there? Then you don't
>need
>> the condition here at all?
>
>True, there is even more room for optimization. The amd_iommu_disabled
>variable can go away entirely, including its checks in
>early_amd_iommu_init(). I will do a patch-set on-top of this for v5.13
>which does more cleanups.

If we can get to the point where we don't even need to check amd_iommu_irq_remap in the ...select() function because the IRQ domain is never even registered in the case where the flag ends up false, all the better :)

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 2/3] iommu/amd: Don't call early_amd_iommu_init() when AMD IOMMU is disabled

[Joerg Roedel]

On Wed, Mar 17, 2021 at 01:37:16PM +0000, David Woodhouse wrote:
> If we can get to the point where we don't even need to check
> amd_iommu_irq_remap in the ...select() function because the IRQ domain
> is never even registered in the case where the flag ends up false, all
> the better :)

This should already be achieved with this patch :)

But the check is still needed if something goes wrong during IOMMU
initialization. In this case the IOMMUs are teared down and the memory
is freed. But the IRQ domains stay registered for now, mostly because
the upper-level APIs to register them lack a deregister function.

I havn't looked into the details yet whether it is suffient to call
irq_domain_remove() on a domain created with
arch_create_remap_msi_irq_domain() for example. This needs more research
on my side :)

Regards,

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

Re: [PATCH 0/3] iommu/amd: Fix booting with amd_iommu=off

[Joerg Roedel]

On Wed, Mar 17, 2021 at 06:48:50PM +0800, Huang Rui wrote:
> Series are Acked-by: Huang Rui <ray.huang@amd.com>

Thanks, series is applied for v5.12
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu