[SRU X] [PATCH 0/1] Need to effectively disable iommu if "intel_iommu=off" is passed as parameter

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU X] [PATCH 0/1] Need to effectively disable iommu if "intel_iommu=off" is passed as parameter

Guilherme Piccoli
[Impact]

* If the parameter "intel_iommu=off" is passed for the kernel in a boot
instance coming from a previously iommu-enabled kernel (like a kexec/kdump
scenario), currently Xenial kernel (4.4 version) is missing a patch to
effectively disable the iommu. As a result, the previous DMA mappings remain
and the new kernel will try to use them, which is wrong since we don't have
iommu initialized to translate the addresses.

* The upstream patch proposed to SRU here, 161b28aae165
("iommu/vt-d: Make sure IOMMUs are off when intel_iommu=off") fixes the
issue by effectively disabling the iommu in case it was requested.

* One important use case is in a scenario of iommu bug that is leading to a
kernel crash; kdump naturally will have the iommu tables copied from previous
kernel in kdump boot time, so if iommu tables are bogus, we could inherit the
bug in the kdump kernel preventing a successful dump.
Have the ability to disable iommu (given that the functionality is there) is
essential.


[Test Case]

* Boot a kernel with "intel_iommu=on", and then kexec a new kernel with
"intel_iommu=off" parameter. A kdump test is also possible, by changing the
kdump command-line to disable iommu (after booting a kernel with iommu enabled);
then, induce a crash and the issue will show.

* There are test results attached in the Launchpad bug.
The messages seen in kernel log when the issue happens are like:

[36.489918] DMAR: DRHD: handling fault status reg 202
[36.734830] DMAR: DMAR:[DMA Read] Request device [03:00.0] fault addr 3179a000
[36.734830] DMAR:[fault reason 06] PTE Read access is not set


[Regression potential]

* iommu operations are complex and subject to HW issues eventually. Having
iommu enabled and then boot a kernel with this component disabled is
inherently a "danger" operation from drivers (or any users of such mappings)
point-of-view. That said, I don't see a potential regression for this patch,
in a way currently an user can't disable iommu properly if it was once
enabled, and this simple patch fixes it

* Care should be taken of course to not confuse regressions with problems
induced by disabling the iommu after it was enabled before (which would show
that the patch works fine indeed, by allowing iommu to get disabled) - some
drivers/devices may require iommu to be enabled in order to work, in such case
disabling it would prevent the device to work.

Joerg Roedel (1):
  iommu/vt-d: Make sure IOMMUs are off when intel_iommu=off

 drivers/iommu/intel-iommu.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

--
2.19.2


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU X] [PATCH 1/1] iommu/vt-d: Make sure IOMMUs are off when intel_iommu=off

Guilherme Piccoli
From: Joerg Roedel <[hidden email]>

BugLink: https://launchpad.net/bugs/1810328

When booting into a kexec kernel with intel_iommu=off, and
the previous kernel had intel_iommu=on, the IOMMU hardware
is still enabled and gets not disabled by the new kernel.

This causes the boot to fail because DMA is blocked by the
hardware. Disable the IOMMUs when we find it enabled in the
kexec kernel and boot with intel_iommu=off.

Signed-off-by: Joerg Roedel <[hidden email]>
(backported from commit 161b28aae1651aa7ad63ec14753aa8a751154340 upstream)
[gpiccoli: context adjustment]
Signed-off-by: Guilherme G. Piccoli <[hidden email]>
---
 drivers/iommu/intel-iommu.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 49b266433f4c..03f817d118f3 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -4648,6 +4648,15 @@ const struct attribute_group *intel_iommu_groups[] = {
  NULL,
 };
 
+static void intel_disable_iommus(void)
+{
+ struct intel_iommu *iommu = NULL;
+ struct dmar_drhd_unit *drhd;
+
+ for_each_iommu(iommu, drhd)
+ iommu_disable_translation(iommu);
+}
+
 int __init intel_iommu_init(void)
 {
  int ret = -ENODEV;
@@ -4676,8 +4685,15 @@ int __init intel_iommu_init(void)
  goto out_free_dmar;
  }
 
- if (no_iommu || dmar_disabled)
+ if (no_iommu || dmar_disabled) {
+ /*
+ * Make sure the IOMMUs are switched off, even when we
+ * boot into a kexec kernel and the previous kernel left
+ * them enabled
+ */
+ intel_disable_iommus();
  goto out_free_dmar;
+ }
 
  if (list_empty(&dmar_rmrr_units))
  pr_info("No RMRR found\n");
--
2.19.2


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU X] [PATCH 1/1] iommu/vt-d: Make sure IOMMUs are off when intel_iommu=off

Kleber Souza
On 1/2/19 8:40 PM, Guilherme G. Piccoli wrote:

> From: Joerg Roedel <[hidden email]>
>
> BugLink: https://launchpad.net/bugs/1810328
>
> When booting into a kexec kernel with intel_iommu=off, and
> the previous kernel had intel_iommu=on, the IOMMU hardware
> is still enabled and gets not disabled by the new kernel.
>
> This causes the boot to fail because DMA is blocked by the
> hardware. Disable the IOMMUs when we find it enabled in the
> kexec kernel and boot with intel_iommu=off.
>
> Signed-off-by: Joerg Roedel <[hidden email]>
> (backported from commit 161b28aae1651aa7ad63ec14753aa8a751154340 upstream)
> [gpiccoli: context adjustment]
> Signed-off-by: Guilherme G. Piccoli <[hidden email]>

This is a patch that would have been nice if someone had sent it to
-stable 4.4 :-)

Acked-by: Kleber Sacilotto de Souza <[hidden email]>

> ---
>  drivers/iommu/intel-iommu.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 49b266433f4c..03f817d118f3 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -4648,6 +4648,15 @@ const struct attribute_group *intel_iommu_groups[] = {
>   NULL,
>  };
>  
> +static void intel_disable_iommus(void)
> +{
> + struct intel_iommu *iommu = NULL;
> + struct dmar_drhd_unit *drhd;
> +
> + for_each_iommu(iommu, drhd)
> + iommu_disable_translation(iommu);
> +}
> +
>  int __init intel_iommu_init(void)
>  {
>   int ret = -ENODEV;
> @@ -4676,8 +4685,15 @@ int __init intel_iommu_init(void)
>   goto out_free_dmar;
>   }
>  
> - if (no_iommu || dmar_disabled)
> + if (no_iommu || dmar_disabled) {
> + /*
> + * Make sure the IOMMUs are switched off, even when we
> + * boot into a kexec kernel and the previous kernel left
> + * them enabled
> + */
> + intel_disable_iommus();
>   goto out_free_dmar;
> + }
>  
>   if (list_empty(&dmar_rmrr_units))
>   pr_info("No RMRR found\n");



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: ACK: [SRU X] [PATCH 1/1] iommu/vt-d: Make sure IOMMUs are off when intel_iommu=off

Guilherme Piccoli
On 08/01/2019 09:51, Kleber Souza wrote:
> [...]
> This is a patch that would have been nice if someone had sent it to
> -stable 4.4 :-)

Thanks for the ACK Kleber. Totally agree with you, but stable process
missed this patch so I thought in adding this at least in our kernel heheh

Cheers,


Guilherme

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU X] [PATCH 1/1] iommu/vt-d: Make sure IOMMUs are off when intel_iommu=off

Stefan Bader-2
In reply to this post by Guilherme Piccoli
On 02.01.19 20:40, Guilherme G. Piccoli wrote:

> From: Joerg Roedel <[hidden email]>
>
> BugLink: https://launchpad.net/bugs/1810328
>
> When booting into a kexec kernel with intel_iommu=off, and
> the previous kernel had intel_iommu=on, the IOMMU hardware
> is still enabled and gets not disabled by the new kernel.
>
> This causes the boot to fail because DMA is blocked by the
> hardware. Disable the IOMMUs when we find it enabled in the
> kexec kernel and boot with intel_iommu=off.
>
> Signed-off-by: Joerg Roedel <[hidden email]>
> (backported from commit 161b28aae1651aa7ad63ec14753aa8a751154340 upstream)
> [gpiccoli: context adjustment]
> Signed-off-by: Guilherme G. Piccoli <[hidden email]>
Acked-by: Stefan Bader <[hidden email]>

> ---
>  drivers/iommu/intel-iommu.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 49b266433f4c..03f817d118f3 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -4648,6 +4648,15 @@ const struct attribute_group *intel_iommu_groups[] = {
>   NULL,
>  };
>  
> +static void intel_disable_iommus(void)
> +{
> + struct intel_iommu *iommu = NULL;
> + struct dmar_drhd_unit *drhd;
> +
> + for_each_iommu(iommu, drhd)
> + iommu_disable_translation(iommu);
> +}
> +
>  int __init intel_iommu_init(void)
>  {
>   int ret = -ENODEV;
> @@ -4676,8 +4685,15 @@ int __init intel_iommu_init(void)
>   goto out_free_dmar;
>   }
>  
> - if (no_iommu || dmar_disabled)
> + if (no_iommu || dmar_disabled) {
> + /*
> + * Make sure the IOMMUs are switched off, even when we
> + * boot into a kexec kernel and the previous kernel left
> + * them enabled
> + */
> + intel_disable_iommus();
>   goto out_free_dmar;
> + }
>  
>   if (list_empty(&dmar_rmrr_units))
>   pr_info("No RMRR found\n");
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

APPLIED/cmt: [SRU X] [PATCH 0/1] Need to effectively disable iommu if "intel_iommu=off" is passed as parameter

Khaled Elmously
In reply to this post by Guilherme Piccoli
Changed
(backported from commit 161b28aae1651aa7ad63ec14753aa8a751154340 upstream)
to
(backported from commit 161b28aae1651aa7ad63ec14753aa8a751154340)


On 2019-01-02 17:40:15 , Guilherme G. Piccoli wrote:

> [Impact]
>
> * If the parameter "intel_iommu=off" is passed for the kernel in a boot
> instance coming from a previously iommu-enabled kernel (like a kexec/kdump
> scenario), currently Xenial kernel (4.4 version) is missing a patch to
> effectively disable the iommu. As a result, the previous DMA mappings remain
> and the new kernel will try to use them, which is wrong since we don't have
> iommu initialized to translate the addresses.
>
> * The upstream patch proposed to SRU here, 161b28aae165
> ("iommu/vt-d: Make sure IOMMUs are off when intel_iommu=off") fixes the
> issue by effectively disabling the iommu in case it was requested.
>
> * One important use case is in a scenario of iommu bug that is leading to a
> kernel crash; kdump naturally will have the iommu tables copied from previous
> kernel in kdump boot time, so if iommu tables are bogus, we could inherit the
> bug in the kdump kernel preventing a successful dump.
> Have the ability to disable iommu (given that the functionality is there) is
> essential.
>
>
> [Test Case]
>
> * Boot a kernel with "intel_iommu=on", and then kexec a new kernel with
> "intel_iommu=off" parameter. A kdump test is also possible, by changing the
> kdump command-line to disable iommu (after booting a kernel with iommu enabled);
> then, induce a crash and the issue will show.
>
> * There are test results attached in the Launchpad bug.
> The messages seen in kernel log when the issue happens are like:
>
> [36.489918] DMAR: DRHD: handling fault status reg 202
> [36.734830] DMAR: DMAR:[DMA Read] Request device [03:00.0] fault addr 3179a000
> [36.734830] DMAR:[fault reason 06] PTE Read access is not set
>
>
> [Regression potential]
>
> * iommu operations are complex and subject to HW issues eventually. Having
> iommu enabled and then boot a kernel with this component disabled is
> inherently a "danger" operation from drivers (or any users of such mappings)
> point-of-view. That said, I don't see a potential regression for this patch,
> in a way currently an user can't disable iommu properly if it was once
> enabled, and this simple patch fixes it
>
> * Care should be taken of course to not confuse regressions with problems
> induced by disabling the iommu after it was enabled before (which would show
> that the patch works fine indeed, by allowing iommu to get disabled) - some
> drivers/devices may require iommu to be enabled in order to work, in such case
> disabling it would prevent the device to work.
>
> Joerg Roedel (1):
>   iommu/vt-d: Make sure IOMMUs are off when intel_iommu=off
>
>  drivers/iommu/intel-iommu.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> --
> 2.19.2
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team