Xen devel - Apr 2013 - [PATCH] [RFC] iommu/crash: Interrupt remapping is also disabled on crash

On the crash path in nmi_shootdown_cpus(), we shut down the IOMMU, then
disable the IOAPIC.

On systems which support interrupt remapping, the variable iommu_intremap
remains set, meaning that disable_IO_APIC() issues interrupt remapping
invalidate requests.

Pre Sandy-Bridge Intel systems and AMD systems appear to cope with this (in so
far as "they seem to work fine"), but Sandy-Bridge systems certainly
do not,
resulting in a panic() when the Queued Invalidation wait descriptor is not
executed, and subsequent deadlock because of reentering the kexec_crash()
path.

This behaviour can be fixed by also indicating that interrupt remapping is not
enabled after shutting down the IOMMU.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

--
This patch is RFC because of several concerns.

* Do we want to turn off more of the IOMMU feature variables as well?  I
  suspect so.

* It looks to me as if this will also cause problems on the S3 path, as
  iommu_disable_x2apic_IR() is called after iommu_shutdown(), yet the
  variables cant change or iommu_resume() will leave things disabled.

diff -r 98f80bf9e286 -r 4247933ebeaa xen/drivers/passthrough/iommu.c
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -601,7 +601,7 @@ void iommu_crash_shutdown(void)
     const struct iommu_ops *ops = iommu_get_ops();
     if ( iommu_enabled )
         ops->crash_shutdown();
-    iommu_enabled = 0;
+    iommu_enabled = iommu_intremap = 0;
 }
 
 int iommu_do_domctl(

>>> On 15.04.13 at 17:04, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
> On the crash path in nmi_shootdown_cpus(), we shut down the IOMMU, then
> disable the IOAPIC.
> 
> On systems which support interrupt remapping, the variable iommu_intremap
> remains set, meaning that disable_IO_APIC() issues interrupt remapping
> invalidate requests.
> 
> Pre Sandy-Bridge Intel systems and AMD systems appear to cope with this (in
> so
> far as "they seem to work fine"), but Sandy-Bridge systems
certainly do not,
> resulting in a panic() when the Queued Invalidation wait descriptor is not
> executed, and subsequent deadlock because of reentering the kexec_crash()
> path.
> 
> This behaviour can be fixed by also indicating that interrupt remapping is 
> not
> enabled after shutting down the IOMMU.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> --
> This patch is RFC because of several concerns.
> 
> * Do we want to turn off more of the IOMMU feature variables as well?  I
>   suspect so.
Which ones in particular? I don''t think the other ones really matter.

Jan
> * It looks to me as if this will also cause problems on the S3 path, as
>   iommu_disable_x2apic_IR() is called after iommu_shutdown(), yet the
>   variables cant change or iommu_resume() will leave things disabled.
> 
> diff -r 98f80bf9e286 -r 4247933ebeaa xen/drivers/passthrough/iommu.c
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -601,7 +601,7 @@ void iommu_crash_shutdown(void)
>      const struct iommu_ops *ops = iommu_get_ops();
>      if ( iommu_enabled )
>          ops->crash_shutdown();
> -    iommu_enabled = 0;
> +    iommu_enabled = iommu_intremap = 0;
>  }
>  
>  int iommu_do_domctl(