diff mbox series

[1/2] x2APIC: translate IO-APIC entries when enabling the IOMMU

Message ID 20191010110339.6447-2-roger.pau@citrix.com (mailing list archive)
State Superseded
Headers show
Series iommu: fixes for interrupt remapping enabling | expand

Commit Message

Roger Pau Monné Oct. 10, 2019, 11:03 a.m. UTC 
When interrupt remapping is enabled as part of enabling x2APIC the
IO-APIC entries also need to be translated to the new format and added
to the interrupt remapping table.

This prevents IOMMU interrupt remapping faults when booting on
hardware that has unmasked IO-APIC pins.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Juergen Gross <jgross@suse.com>
---
 xen/arch/x86/apic.c           | 12 ++++++++++--
 xen/arch/x86/io_apic.c        |  5 +++--
 xen/include/asm-x86/io_apic.h |  3 ++-
 3 files changed, 15 insertions(+), 5 deletions(-)

Comments

Roger Pau Monné Oct. 10, 2019, 11:24 a.m. UTC | #1 
On Thu, Oct 10, 2019 at 01:03:38PM +0200, Roger Pau Monne wrote:
> When interrupt remapping is enabled as part of enabling x2APIC the
> IO-APIC entries also need to be translated to the new format and added
> to the interrupt remapping table.
> 
> This prevents IOMMU interrupt remapping faults when booting on
> hardware that has unmasked IO-APIC pins.
> 
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Juergen Gross <jgross@suse.com>
> ---
>  xen/arch/x86/apic.c           | 12 ++++++++++--
>  xen/arch/x86/io_apic.c        |  5 +++--
>  xen/include/asm-x86/io_apic.h |  3 ++-
>  3 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
> index 6cdb50cf41..9810de7473 100644
> --- a/xen/arch/x86/apic.c
> +++ b/xen/arch/x86/apic.c
> @@ -515,7 +515,7 @@ static void resume_x2apic(void)
>      iommu_enable_x2apic();
>      __enable_x2apic();
>  
> -    restore_IO_APIC_setup(ioapic_entries);
> +    restore_IO_APIC_setup(ioapic_entries, true);
>      unmask_8259A();
>  
>  out:
> @@ -887,6 +887,7 @@ void __init x2apic_bsp_setup(void)
>  {
>      struct IO_APIC_route_entry **ioapic_entries = NULL;
>      const char *orig_name;
> +    bool iommu_enabled = true;

There's no need for this local variable, x2apic_enabled can be used
instead.

Will send a new version, sorry.

Roger.
diff mbox series

Patch

diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index 6cdb50cf41..9810de7473 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -515,7 +515,7 @@  static void resume_x2apic(void)
     iommu_enable_x2apic();
     __enable_x2apic();
 
-    restore_IO_APIC_setup(ioapic_entries);
+    restore_IO_APIC_setup(ioapic_entries, true);
     unmask_8259A();
 
 out:
@@ -887,6 +887,7 @@  void __init x2apic_bsp_setup(void)
 {
     struct IO_APIC_route_entry **ioapic_entries = NULL;
     const char *orig_name;
+    bool iommu_enabled = true;
 
     if ( !cpu_has_x2apic )
         return;
@@ -934,6 +935,7 @@  void __init x2apic_bsp_setup(void)
         if ( !x2apic_enabled )
         {
             printk("Not enabling x2APIC (upon firmware request)\n");
+            iommu_enabled = false;
             goto restore_out;
         }
         /* fall through */
@@ -944,6 +946,7 @@  void __init x2apic_bsp_setup(void)
 
         printk(XENLOG_ERR
                "Failed to enable Interrupt Remapping: Will not enable x2APIC.\n");
+        iommu_enabled = false;
         goto restore_out;
     }
 
@@ -961,7 +964,12 @@  void __init x2apic_bsp_setup(void)
         printk("Switched to APIC driver %s\n", genapic.name);
 
 restore_out:
-    restore_IO_APIC_setup(ioapic_entries);
+    /*
+     * NB: do not use raw mode when restoring entries if the iommu has been
+     * enabled during the process, because the entries need to be translated
+     * and added to the remapping table in that case.
+     */
+    restore_IO_APIC_setup(ioapic_entries, !iommu_enabled);
     unmask_8259A();
 
 out:
diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index 5d25862bd8..37eabc16c9 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -379,7 +379,8 @@  void mask_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries)
 /*
  * Restore IO APIC entries which was saved in ioapic_entries.
  */
-int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries)
+int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries,
+                          bool raw)
 {
     int apic, pin;
 
@@ -394,7 +395,7 @@  int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries)
             return -ENOMEM;
 
         for (pin = 0; pin < nr_ioapic_entries[apic]; pin++)
-	    ioapic_write_entry(apic, pin, 1, ioapic_entries[apic][pin]);
+	    ioapic_write_entry(apic, pin, raw, ioapic_entries[apic][pin]);
     }
 
     return 0;
diff --git a/xen/include/asm-x86/io_apic.h b/xen/include/asm-x86/io_apic.h
index 0b041f0565..998905186b 100644
--- a/xen/include/asm-x86/io_apic.h
+++ b/xen/include/asm-x86/io_apic.h
@@ -197,7 +197,8 @@  extern struct IO_APIC_route_entry **alloc_ioapic_entries(void);
 extern void free_ioapic_entries(struct IO_APIC_route_entry **ioapic_entries);
 extern int save_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
 extern void mask_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
-extern int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
+extern int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries,
+                                 bool raw);
 
 unsigned highest_gsi(void);