stable/for-ingo-v3.5 of IOAPIC abstraction (and then some users) for v3.5

GIT PULL Konrad Rzeszutek Wilk 2 participants 4 messages May 1, 2012

Subsystems (2)

x86 Xen hypervisor x86

lore

Share & tools

lore

Konrad Rzeszutek Wilk

author

2012-05-01 19:42 UTC #0 lore raw

Hey Ingo,

Please git pull the following branch for v3.5:

git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git stable/for-ingo-v3.5

It has the proper fix for the IO-APIC having 0xfdfdfd... contents when booting under Xen. It also abstracts the IO-APIC in a different layer allowing for re-using of the generic code.

Compared to the patch you have already in the tree (136d249ef7dbf0fefa292082cc40be1ea864cbd6 x86/ioapic: Add io_apic_ops driver layer to allow interception) it does four things:

1) Exposes the io_apic baremetal functions and uses the x86_io_apic_ops function table instead of keeping it in the io_apic. This makes the code fit within the rest of x86_ops functions.
2) Use the x86_io_apic_ops to re-direct the (*read) to the Xen emulated one.
3) Removes the work-around.
4) Implements a read hypercall to get the contents of the IOAPIC. Note:
There is no (*write) as it is not needed.

With these patches, this is what we end up seeing (for Xen):

ACPI: IOAPIC (id[0x00] address[0xfec00000] gsi_base[0])
-IOAPIC[0]: apic_id 0, version 253, address 0xfec00000, GSI 0-253 +IOAPIC[0]: apic_id 0, version 32, address 0xfec00000, GSI 0-23
ACPI: IOAPIC (id[0x01] address[0xfec3f000] gsi_base[24])
-IOAPIC[1]: apic_id 1, version 253, address 0xfec3f000, GSI 24-277 +IOAPIC[1]: apic_id 1, version 32, address 0xfec3f000, GSI 24-47
ACPI: IOAPIC (id[0x02] address[0xfec7f000] gsi_base[48])
-IOAPIC[2]: apic_id 2, version 253, address 0xfec7f000, GSI 48-301 +IOAPIC[2]: apic_id 2, version 32, address 0xfec7f000, GSI 48-71

for baremetal - there is no change in the output or functionality.

Please pull!

 arch/x86/include/asm/io_apic.h  |   35 +++++++++++++++++++----------
 arch/x86/include/asm/x86_init.h |    9 ++++++-
 arch/x86/kernel/apic/io_apic.c  |   46 +++-----------------------------------
 arch/x86/kernel/setup.c         |    2 +-
 arch/x86/kernel/x86_init.c      |    8 ++++++
 arch/x86/xen/Makefile           |    2 +-
 arch/x86/xen/apic.c             |   30 +++++++++++++++++++++++++
 arch/x86/xen/enlighten.c        |    2 +
 arch/x86/xen/mmu.c              |    4 +--
 arch/x86/xen/xen-ops.h          |    4 +++
 10 files changed, 82 insertions(+), 60 deletions(-)

Konrad Rzeszutek Wilk (3):
x86/apic: Replace io_apic_ops with x86_io_apic_ops.
xen/x86: Implement x86_apic_ops
Revert "xen/x86: Workaround 'x86/ioapic: Add register level checks to detect bogus io-apic entries'"

Lin Ming (1):
xen/apic: implement io apic read with hypercall

And the full diff.

arch/x86/include/asm/io_apic.h +23 ~~-12~~

index 2c4943d..73d8c53 100644
@@ -5,7 +5,7 @@
 #include <asm/mpspec.h>
 #include <asm/apicdef.h>
 #include <asm/irq_vectors.h>
-
+#include <asm/x86_init.h>
 /*
  * Intel IO-APIC support for SMP and UP systems.
  *

@@ -21,15 +21,6 @@
 #define IO_APIC_REDIR_LEVEL_TRIGGER	(1 << 15)
 #define IO_APIC_REDIR_MASKED		(1 << 16)
 
-struct io_apic_ops {
-	void		(*init)  (void);
-	unsigned int	(*read)  (unsigned int apic, unsigned int reg);
-	void		(*write) (unsigned int apic, unsigned int reg, unsigned int value);
-	void		(*modify)(unsigned int apic, unsigned int reg, unsigned int value);
-};
-
-void __init set_io_apic_ops(const struct io_apic_ops *);
-
 /*
  * The structure of the IO-APIC:
  */

@@ -156,7 +147,6 @@ struct io_apic_irq_attr;
 extern int io_apic_set_pci_routing(struct device *dev, int irq,
 		 struct io_apic_irq_attr *irq_attr);
 void setup_IO_APIC_irq_extra(u32 gsi);
-extern void ioapic_and_gsi_init(void);
 extern void ioapic_insert_resources(void);
 
 int io_apic_setup_irq_pin_once(unsigned int irq, int node, struct io_apic_irq_attr *attr);

@@ -185,12 +175,29 @@ extern void mp_save_irq(struct mpc_intsrc *m);
 
 extern void disable_ioapic_support(void);
 
+extern void __init native_io_apic_init_mappings(void);
+extern unsigned int native_io_apic_read(unsigned int apic, unsigned int reg);
+extern void native_io_apic_write(unsigned int apic, unsigned int reg, unsigned int val);
+extern void native_io_apic_modify(unsigned int apic, unsigned int reg, unsigned int val);
+
+static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
+{
+	return x86_io_apic_ops.read(apic, reg);
+}
+
+static inline void io_apic_write(unsigned int apic, unsigned int reg, unsigned int value)
+{
+	x86_io_apic_ops.write(apic, reg, value);
+}
+static inline void io_apic_modify(unsigned int apic, unsigned int reg, unsigned int value)
+{
+	x86_io_apic_ops.modify(apic, reg, value);
+}
 #else  /* !CONFIG_X86_IO_APIC */
 
 #define io_apic_assign_pci_irqs 0
 #define setup_ioapic_ids_from_mpc x86_init_noop
 static const int timer_through_8259 = 0;
-static inline void ioapic_and_gsi_init(void) { }
 static inline void ioapic_insert_resources(void) { }
 #define gsi_top (NR_IRQS_LEGACY)
 static inline int mp_find_ioapic(u32 gsi) { return 0; }

@@ -212,6 +219,10 @@ static inline int restore_ioapic_entries(void)
 
 static inline void mp_save_irq(struct mpc_intsrc *m) { };
 static inline void disable_ioapic_support(void) { }
+#define native_io_apic_init_mappings	NULL
+#define native_io_apic_read		NULL
+#define native_io_apic_write		NULL
+#define native_io_apic_modify		NULL
 #endif
 
 #endif /* _ASM_X86_IO_APIC_H */

arch/x86/include/asm/x86_init.h +8 -1

index 764b66a..c090af1 100644
@@ -188,11 +188,18 @@ struct x86_msi_ops {
 	void (*restore_msi_irqs)(struct pci_dev *dev, int irq);
 };
 
+struct x86_io_apic_ops {
+	void		(*init)  (void);
+	unsigned int	(*read)  (unsigned int apic, unsigned int reg);
+	void		(*write) (unsigned int apic, unsigned int reg, unsigned int value);
+	void		(*modify)(unsigned int apic, unsigned int reg, unsigned int value);
+};
+
 extern struct x86_init_ops x86_init;
 extern struct x86_cpuinit_ops x86_cpuinit;
 extern struct x86_platform_ops x86_platform;
 extern struct x86_msi_ops x86_msi;
-
+extern struct x86_io_apic_ops x86_io_apic_ops;
 extern void x86_init_noop(void);
 extern void x86_init_uint_noop(unsigned int unused);

arch/x86/kernel/apic/io_apic.c +4 ~~-42~~

82 lines not on this page · raw message

arch/x86/kernel/setup.c +1 -1

index 1a29015..8526317 100644
@@ -1012,7 +1012,7 @@ void __init setup_arch(char **cmdline_p)
 	init_cpu_to_node();
 
 	init_apic_mappings();
-	ioapic_and_gsi_init();
+	x86_io_apic_ops.init();
 
 	kvm_guest_init();

arch/x86/kernel/x86_init.c +8 -0

index 9cf71d0..35c5e54 100644
@@ -18,6 +18,7 @@
 #include <asm/e820.h>
 #include <asm/time.h>
 #include <asm/irq.h>
+#include <asm/io_apic.h>
 #include <asm/pat.h>
 #include <asm/tsc.h>
 #include <asm/iommu.h>

@@ -119,3 +120,10 @@ struct x86_msi_ops x86_msi = {
 	.teardown_msi_irqs = default_teardown_msi_irqs,
 	.restore_msi_irqs = default_restore_msi_irqs,
 };
+
+struct x86_io_apic_ops x86_io_apic_ops = {
+	.init	= native_io_apic_init_mappings,
+	.read	= native_io_apic_read,
+	.write	= native_io_apic_write,
+	.modify	= native_io_apic_modify,
+};

arch/x86/xen/Makefile +1 -1

index add2c2d..96ab2c0 100644
@@ -20,5 +20,5 @@ obj-$(CONFIG_EVENT_TRACING) += trace.o
 obj-$(CONFIG_SMP)		+= smp.o
 obj-$(CONFIG_PARAVIRT_SPINLOCKS)+= spinlock.o
 obj-$(CONFIG_XEN_DEBUG_FS)	+= debugfs.o
-obj-$(CONFIG_XEN_DOM0)		+= vga.o
+obj-$(CONFIG_XEN_DOM0)		+= apic.o vga.o
 obj-$(CONFIG_SWIOTLB_XEN)	+= pci-swiotlb-xen.o

arch/x86/xen/apic.c new file +30 -0

new file mode 100644
index 0000000..1913bf2
@@ -0,0 +1,30 @@
+#include <linux/init.h>
+#include <asm/x86_init.h>
+#include <asm/apic.h>
+#include <xen/interface/physdev.h>
+#include <asm/xen/hypercall.h>
+
+unsigned int xen_io_apic_read(unsigned apic, unsigned reg)
+{
+	struct physdev_apic apic_op;
+	int ret;
+
+	apic_op.apic_physbase = mpc_ioapic_addr(apic);
+	apic_op.reg = reg;
+	ret = HYPERVISOR_physdev_op(PHYSDEVOP_apic_read, &apic_op);
+	if (!ret)
+		return apic_op.value;
+
+	/* fallback to return an emulated IO_APIC values */
+	if (reg == 0x1)
+		return 0x00170020;
+	else if (reg == 0x0)
+		return apic << 24;
+
+	return 0xfd;
+}
+
+void __init xen_init_apic(void)
+{
+	x86_io_apic_ops.read = xen_io_apic_read;
+}

arch/x86/xen/enlighten.c +2 -0

index a8f8844..c2ea9e9 100644
@@ -1362,6 +1362,8 @@ asmlinkage void __init xen_start_kernel(void)
 		xen_start_info->console.domU.mfn = 0;
 		xen_start_info->console.domU.evtchn = 0;
 
+		xen_init_apic();
+
 		/* Make sure ACS will be enabled */
 		pci_request_acs();
 	}

arch/x86/xen/mmu.c +1 -3

index b8e2794..988828b 100644
@@ -1859,7 +1859,6 @@ pgd_t * __init xen_setup_kernel_pagetable(pgd_t *pgd,
 #endif	/* CONFIG_X86_64 */
 
 static unsigned char dummy_mapping[PAGE_SIZE] __page_aligned_bss;
-static unsigned char fake_ioapic_mapping[PAGE_SIZE] __page_aligned_bss;
 
 static void xen_set_fixmap(unsigned idx, phys_addr_t phys, pgprot_t prot)
 {

@@ -1900,7 +1899,7 @@ static void xen_set_fixmap(unsigned idx, phys_addr_t phys, pgprot_t prot)
 		 * We just don't map the IO APIC - all access is via
 		 * hypercalls.  Keep the address in the pte for reference.
 		 */
-		pte = pfn_pte(PFN_DOWN(__pa(fake_ioapic_mapping)), PAGE_KERNEL);
+		pte = pfn_pte(PFN_DOWN(__pa(dummy_mapping)), PAGE_KERNEL);
 		break;
 #endif

@@ -2065,7 +2064,6 @@ void __init xen_init_mmu_ops(void)
 	pv_mmu_ops = xen_mmu_ops;
 
 	memset(dummy_mapping, 0xff, PAGE_SIZE);
-	memset(fake_ioapic_mapping, 0xfd, PAGE_SIZE);
 }
 
 /* Protected by xen_reservation_lock. */

arch/x86/xen/xen-ops.h +4 -0

index b095739..45c0c06 100644
@@ -92,11 +92,15 @@ struct dom0_vga_console_info;
 
 #ifdef CONFIG_XEN_DOM0
 void __init xen_init_vga(const struct dom0_vga_console_info *, size_t size);
+void __init xen_init_apic(void);
 #else
 static inline void __init xen_init_vga(const struct dom0_vga_console_info *info,
 				       size_t size)
 {
 }
+static inline void __init xen_init_apic(void)
+{
+}
 #endif
 
 /* Declare an asm function, along with symbols needed to make it

Josh Boyer

2012-05-01 20:08 UTC #1 lore raw

Quoted 31 lines · Konrad Rzeszutek Wilk

arch/x86/kernel/apic/io_apic.c +1 -1

-static void __init __ioapic_init_mappings(void)
+void __init native_io_apic_init_mappings(void)

{
unsigned long ioapic_phys, idx = FIX_IO_APIC_BASE_0; struct resource *ioapic_res;

arch/x86/kernel/setup.c +1 -1

index 1a29015..8526317 100644
@@ -1012,7 +1012,7 @@ void __init setup_arch(char **cmdline_p)
        init_cpu_to_node();
 
        init_apic_mappings();
-       ioapic_and_gsi_init();
+       x86_io_apic_ops.init();
 
        kvm_guest_init();

arch/x86/kernel/x86_init.c +1 -0

index 9cf71d0..35c5e54 100644
@@ -18,6 +18,7 @@
  #include <asm/e820.h>
  #include <asm/time.h>
  #include <asm/irq.h>
+#include <asm/io_apic.h>
  #include <asm/pat.h>
  #include <asm/tsc.h>
  #include <asm/iommu.h>

arch/x86/kernel/x86_init.c struct x86_msi_ops x86_msi = { · line 129

Quoted 10 lines

arch/x86/kernel/x86_init.c +7 -0

@@ -119,3 +120,10 @@ struct x86_msi_ops x86_msi = {
        .teardown_msi_irqs = default_teardown_msi_irqs,
        .restore_msi_irqs = default_restore_msi_irqs,
  };
+
+struct x86_io_apic_ops x86_io_apic_ops = {
+       .init   = native_io_apic_init_mappings,
+       .read   = native_io_apic_read,
+       .write  = native_io_apic_write,
+       .modify = native_io_apic_modify,
+};

You'll get a section mismatch warning on this struct. It's not a huge deal, but native_io_apic_init_mappings is annotated as __init whereas this struct isn't. In practice it doesn't seem to matter as x86_io_apic_ops.init is only called in setup_arch, but it's still a valid warning.

(First noticed in https://bugzilla.redhat.com/show_bug.cgi?id=817645 )

josh

Konrad Rzeszutek Wilk

author

2012-05-01 20:26 UTC #2 lore raw

Quoted 41 lines · Josh Boyer

Quoted 39 lines · Konrad Rzeszutek Wilk

arch/x86/kernel/apic/io_apic.c +1 -1

-static void __init __ioapic_init_mappings(void)
+void __init native_io_apic_init_mappings(void)

{
unsigned long ioapic_phys, idx = FIX_IO_APIC_BASE_0; struct resource *ioapic_res;

arch/x86/kernel/setup.c +1 -1

index 1a29015..8526317 100644
@@ -1012,7 +1012,7 @@ void __init setup_arch(char **cmdline_p)
        init_cpu_to_node();
 
        init_apic_mappings();
-       ioapic_and_gsi_init();
+       x86_io_apic_ops.init();
 
        kvm_guest_init();

arch/x86/kernel/x86_init.c +6 -0

index 9cf71d0..35c5e54 100644
@@ -18,6 +18,7 @@
  #include <asm/e820.h>
  #include <asm/time.h>
  #include <asm/irq.h>
+#include <asm/io_apic.h>
  #include <asm/pat.h>
  #include <asm/tsc.h>
  #include <asm/iommu.h>

@@ -119,3 +120,10 @@ struct x86_msi_ops x86_msi = {
        .teardown_msi_irqs = default_teardown_msi_irqs,
        .restore_msi_irqs = default_restore_msi_irqs,
  };
+
+struct x86_io_apic_ops x86_io_apic_ops = {
+       .init   = native_io_apic_init_mappings,
+       .read   = native_io_apic_read,
+       .write  = native_io_apic_write,

Quoted 8 lines

arch/x86/kernel/x86_init.c struct x86_msi_ops x86_msi = { · line 129
Quoted 2 lines
arch/x86/kernel/x86_init.c +2 -0
+       .modify = native_io_apic_modify,
+};
You'll get a section mismatch warning on this struct. It's not a huge deal, but native_io_apic_init_mappings is annotated as __init whereas this struct isn't. In practice it doesn't seem to matter as x86_io_apic_ops.init is only called in setup_arch, but it's still a valid warning.

I think that the mismatch disappears if the structure has the word _ops in it. At least that is what I saw (when I ran with the MODULE_SECTION=y with the initial implementation of this and then fixed it up).

However, let me double check - I might have seen that with something else and misremebered it.

Quoted 4 lines

(First noticed in https://bugzilla.redhat.com/show_bug.cgi?id=817645 )
josh

Josh Boyer

2012-05-01 20:44 UTC #3 lore raw

Quoted 17 lines · Konrad Rzeszutek Wilk

Quoted 15 lines · Josh Boyer
arch/x86/kernel/x86_init.c struct x86_msi_ops x86_msi = { · line 129
Quoted 7 lines · Konrad Rzeszutek Wilk
arch/x86/kernel/x86_init.c +7 -0
+
+struct x86_io_apic_ops x86_io_apic_ops = {
+       .init   = native_io_apic_init_mappings,
+       .read   = native_io_apic_read,
+       .write  = native_io_apic_write,
+       .modify = native_io_apic_modify,
+};
You'll get a section mismatch warning on this struct. It's not a huge deal, but native_io_apic_init_mappings is annotated as __init whereas this struct isn't. In practice it doesn't seem to matter as x86_io_apic_ops.init is only called in setup_arch, but it's still a valid warning.

Quoted 6 lines

I think that the mismatch disappears if the structure has the word _ops in it. At least that is what I saw (when I ran with the MODULE_SECTION=y with the initial implementation of this and then fixed it up).
However, let me double check - I might have seen that with something else and misremebered it.

Oh, you might very well be correct. The bug has an older version of this patch that just calls it 'x86_ioapic'. Guess I'll need to poke around and update whatever we're carrying to the latest version.

If it's gone, sorry for the noise.

josh