[PATCH 0/3] Enable legacy irq mode before jump to kexec/kdump kernel

[PATCH 0/3] Enable legacy irq mode before jump to kexec/kdump kernel

[Baoquan He]

Wei Jiangang reported kdump kernel always hang when "notsc" is specified
in boot parameter. After debugging I found there's no timer interrupt
in the current kexec/kdump kernel. This is caused by commit 522e66464467
("x86/apic: Disable I/O APIC before shutdown of the local APIC"). Originally
Eric posted below patch to make system be virtual wire mode in which 8259-
equivalent PIC fields all interrupts and the LAPIC becomes a virtual wire.
Like this interrupts can be delivered from PIC to CPU via the LAPIC's local
interrupt 0 (LINTIN0). In virtual wire APIC mode is disabled while LAPIC
is software enabled and its LINT0 and LINT1 need be programmed specifically.

https://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.11/2.6.11-mm1/broken-out/x86_64-apic-virtwire-on-shutdown.patch

But with commit 522e66464 you can see after disable_IO_APIC had setting
virtual wire mode, lapic_shutdown disabled LAPIC again. Now virtual wire
mode doesn't work, then it cause no timer interrupt during kdump kernel
initialization stage until system enter into APIC mode.

So people may be wondering why only kdump kernel hang, the normal kernel
with "notsc" can still work. This is because BIOS has already built PIC mode
or virtual wire mode while kexec/kdump kernel doesn't go through BIOS
initialization. That is why we have to change system to be PIC mode or
virtual wire mode before jump to kexec/kdump kernel.

Then why kdump kernel didn't hang when "notsc" is not specified. This is
because tsc_init will assign the already calibrated value to lpj_fine.
Then kernel doesn't need to count cpu loops between jiffies with the help
of timer interrupt. So "notsc" is not victim, but a informer.

In patch 1/3 disable_IO_APIC is changed to only contain code of changeing
system to be PIC mode or virtual wire mode and is renamed as
switch_to_legacy_irq_mode. Now only call clear_IO_APIC where IO-APIC need
be disabled, and call switch_to_legacy_irq_mode before jump to kexe/kdump
kernel.

Patch 2/3 and 3/3 are clean up patch.

Baoquan He (3):
  x86/apic/kexec: Enable legacy irq mode before jump to kexec/kdump
    kernel
  x86/apic: Clean up the names of legacy irq mode setting related
    functions
  x86/apic: Clean up the apic delivery mode macro definition

 arch/x86/include/asm/apic.h        |  2 +-
 arch/x86/include/asm/apicdef.h     |  1 -
 arch/x86/include/asm/io_apic.h     |  6 +++---
 arch/x86/kernel/apic/apic.c        | 19 +++++++++++--------
 arch/x86/kernel/apic/io_apic.c     | 32 +++++++++++++++++---------------
 arch/x86/kernel/crash.c            |  2 +-
 arch/x86/kernel/machine_kexec_32.c | 15 +++++----------
 arch/x86/kernel/machine_kexec_64.c | 15 +++++----------
 arch/x86/kernel/reboot.c           |  2 +-
 arch/x86/kernel/x86_init.c         |  2 +-
 drivers/iommu/irq_remapping.c      |  2 +-
 11 files changed, 46 insertions(+), 52 deletions(-)

-- 
2.5.5

[PATCH 1/3] x86/apic/kexec: Enable legacy irq mode before jump to kexec/kdump kernel

[Baoquan He]

It's reported that if kernel parameter "notsc" is specified kdump kernel
always hang after printing boot log as below:
    ---
    tsc: Fast TSC calibration using PIT
    tsc: Detected 2099.947 MHz processor
    Calibrating delay loop...
    ---

The reason is no timer interrupt during kdump kernel initialization.
With "notsc" specified the function tsc_init() will return early and won't
assign already calibrated value to lpj_fine, we have to rely on timer
interrupt to get cpu loops between jiffies. Then system hangs to wait for
the never coming timer interrupt.

This is caused by buggy 'commit 522e66464467 ("x86/apic: Disable I/O APIC
before shutdown of the local APIC")'. In normal kernel it defaults to be
PIC mode or Virtual Wire mode during system initialization before APIC
mode is enabled and this is done by BIOS initialization. With this timer
interrupt can be handled in legacy irq way. But kexec/kdump kernel won't
go through BIOS, so we have to set system as PIC or Virtual Wire mode
before jump to kdump kernel code directly. This is done in disable_IO_APIC
which includes two parts, firstly it calls clear_IO_APIC() to disable
IO-APIC, then it sets LAPIC and IO-APIC to make system be PIC or Virtual
Wire mode. In commit 522e66464 it put disable_IO_APIC earlier so that
local apic is completely disabled.

So in this patch take clear_IO_APIC out from disable_IO_APIC and rename
disable_IO_APIC as switch_to_legacy_irq_mode. Then only call clear_IO_APIC
when IO-APIC need be disabled. And call switch_to_legacy_irq_mode before
kexec/kdump jumping.

Reported-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
Signed-off-by: Baoquan He <bhe@redhat.com>
---
 arch/x86/include/asm/io_apic.h     |  3 ++-
 arch/x86/kernel/apic/io_apic.c     | 12 ++++--------
 arch/x86/kernel/crash.c            |  2 +-
 arch/x86/kernel/machine_kexec_32.c | 15 +++++----------
 arch/x86/kernel/machine_kexec_64.c | 15 +++++----------
 arch/x86/kernel/reboot.c           |  2 +-
 6 files changed, 18 insertions(+), 31 deletions(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index 6cbf2cf..d15192e 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -191,7 +191,8 @@ static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
 
 extern void setup_IO_APIC(void);
 extern void enable_IO_APIC(void);
-extern void disable_IO_APIC(void);
+extern void clear_IO_APIC (void);
+extern void switch_to_legacy_irq_mode(void);
 extern void setup_ioapic_dest(void);
 extern int IO_APIC_get_PCI_irq_vector(int bus, int devfn, int pin);
 extern void print_IO_APICs(void);
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 446702e..d8f5cbe 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -587,7 +587,7 @@ static void clear_IO_APIC_pin(unsigned int apic, unsigned int pin)
 		       mpc_ioapic_id(apic), pin);
 }
 
-static void clear_IO_APIC (void)
+void clear_IO_APIC (void)
 {
 	int apic, pin;
 
@@ -1459,15 +1459,11 @@ void native_disable_io_apic(void)
 }
 
 /*
- * Not an __init, needed by the reboot code
+ * Not an __init, needed by kexec/kdump code.
+ * For safety IO-APIC and Local APIC need be cleared before this.
  */
-void disable_IO_APIC(void)
+void switch_to_legacy_irq_mode(void)
 {
-	/*
-	 * Clear the IO-APIC before rebooting:
-	 */
-	clear_IO_APIC();
-
 	if (!nr_legacy_irqs())
 		return;
 
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index 9ef978d..236317b 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -182,7 +182,7 @@ void native_machine_crash_shutdown(struct pt_regs *regs)
 #ifdef CONFIG_X86_IO_APIC
 	/* Prevent crash_kexec() from deadlocking on ioapic_lock. */
 	ioapic_zap_locks();
-	disable_IO_APIC();
+	clear_IO_APIC();
 #endif
 	lapic_shutdown();
 #ifdef CONFIG_HPET_TIMER
diff --git a/arch/x86/kernel/machine_kexec_32.c b/arch/x86/kernel/machine_kexec_32.c
index 469b23d..26cacf6 100644
--- a/arch/x86/kernel/machine_kexec_32.c
+++ b/arch/x86/kernel/machine_kexec_32.c
@@ -202,18 +202,13 @@ void machine_kexec(struct kimage *image)
 	local_irq_disable();
 	hw_breakpoint_disable();
 
-	if (image->preserve_context) {
 #ifdef CONFIG_X86_IO_APIC
-		/*
-		 * We need to put APICs in legacy mode so that we can
-		 * get timer interrupts in second kernel. kexec/kdump
-		 * paths already have calls to disable_IO_APIC() in
-		 * one form or other. kexec jump path also need
-		 * one.
-		 */
-		disable_IO_APIC();
+	/*
+	 * We need to put APICs in legacy mode so that we can
+	 * get timer interrupts in second kernel.
+	 */
+	switch_to_legacy_irq_mode();
 #endif
-	}
 
 	control_page = page_address(image->control_code_page);
 	memcpy(control_page, relocate_kernel, KEXEC_CONTROL_CODE_MAX_SIZE);
diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index 5a294e4..a720716 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -269,18 +269,13 @@ void machine_kexec(struct kimage *image)
 	local_irq_disable();
 	hw_breakpoint_disable();
 
-	if (image->preserve_context) {
 #ifdef CONFIG_X86_IO_APIC
-		/*
-		 * We need to put APICs in legacy mode so that we can
-		 * get timer interrupts in second kernel. kexec/kdump
-		 * paths already have calls to disable_IO_APIC() in
-		 * one form or other. kexec jump path also need
-		 * one.
-		 */
-		disable_IO_APIC();
+	/*
+	 * We need to put APICs in legacy mode so that we can
+	 * get timer interrupts in second kernel.
+	 */
+	switch_to_legacy_irq_mode();
 #endif
-	}
 
 	control_page = page_address(image->control_code_page) + PAGE_SIZE;
 	memcpy(control_page, relocate_kernel, KEXEC_CONTROL_CODE_MAX_SIZE);
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index a9b31eb..466c860 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -624,7 +624,7 @@ void native_machine_shutdown(void)
 	 * Even without the erratum, it still makes sense to quiet IO APIC
 	 * before disabling Local APIC.
 	 */
-	disable_IO_APIC();
+	clear_IO_APIC();
 #endif
 
 #ifdef CONFIG_SMP
-- 
2.5.5

[PATCH 2/3] x86/apic: Clean up the names of legacy irq mode setting related functions

[Baoquan He]

X86 MP spec defines 3 different interrupt modes:
  1) PIC Mode—bypasses all APIC components and forces the system to
     operate in single-processor mode.
  2) Virtual Wire Mode—uses an APIC as a virtual wire, but otherwise
     operates the same as PIC Mode.
  3) Symmetric I/O Mode—enables the system to operate with more than
     one processor.

The current disconnect_bsp_APIC includes two parts: one is to set system
as PIC mode if it's available, the other is to change system back to
Virtual Wire mode. Only PIC mode will detach the APIC from the interrupt
system, Virtual Wire mode doesn't.

Besides Virutal Wire mode has two kinds: one is only setting Local APIC
as Virtual Wire mode and interrupts are delivered from the PIC to the
CPU which Local APIC connected to, the other is both Loca APIC and IO-APIC
need be set as Virtual Wire mode.

So based on above knowledge, take IO-APIC Virtual Wire mode setting code
out and wrap it inot a new function ioapic_set_virtual_wire_mode. Meanwhile
change the name of disconnect_bsp_APIC as lapic_set_legacy_irq_mode. These
makes the legacy irq mode setting more understandable.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 arch/x86/include/asm/apic.h    |  2 +-
 arch/x86/include/asm/io_apic.h |  5 ++---
 arch/x86/kernel/apic/apic.c    | 11 ++++++-----
 arch/x86/kernel/apic/io_apic.c | 17 ++++++++++-------
 arch/x86/kernel/x86_init.c     |  2 +-
 drivers/iommu/irq_remapping.c  |  2 +-
 6 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index bc27611..dfe4598 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -125,7 +125,7 @@ extern int get_physical_broadcast(void);
 
 extern int lapic_get_maxlvt(void);
 extern void clear_local_APIC(void);
-extern void disconnect_bsp_APIC(int virt_wire_setup);
+extern void lapic_set_legacy_irq_mode(int virt_wire_setup);
 extern void disable_local_APIC(void);
 extern void lapic_shutdown(void);
 extern void sync_Arb_IDs(void);
diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index d15192e..1aed995 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -182,7 +182,7 @@ extern void disable_ioapic_support(void);
 
 extern void __init io_apic_init_mappings(void);
 extern unsigned int native_io_apic_read(unsigned int apic, unsigned int reg);
-extern void native_disable_io_apic(void);
+extern void switch_to_legacy_irq_mode(void);
 
 static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
 {
@@ -192,7 +192,6 @@ static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
 extern void setup_IO_APIC(void);
 extern void enable_IO_APIC(void);
 extern void clear_IO_APIC (void);
-extern void switch_to_legacy_irq_mode(void);
 extern void setup_ioapic_dest(void);
 extern int IO_APIC_get_PCI_irq_vector(int bus, int devfn, int pin);
 extern void print_IO_APICs(void);
@@ -229,7 +228,7 @@ static inline void mp_save_irq(struct mpc_intsrc *m) { }
 static inline void disable_ioapic_support(void) { }
 static inline void io_apic_init_mappings(void) { }
 #define native_io_apic_read		NULL
-#define native_disable_io_apic		NULL
+#define switch_to_legacy_irq_mode	NULL
 
 static inline void setup_IO_APIC(void) { }
 static inline void enable_IO_APIC(void) { }
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 60078a6..9bb2770 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1934,13 +1934,14 @@ static void __init connect_bsp_APIC(void)
 }
 
 /**
- * disconnect_bsp_APIC - detach the APIC from the interrupt system
- * @virt_wire_setup:	indicates, whether virtual wire mode is selected
+ * lapic_set_legacy_irq_mode - switch Local APIC back to be legacy irq mode.
+ * @virt_wire_setup:   indicates, whether virtual wire mode is selected
  *
- * Virtual wire mode is necessary to deliver legacy interrupts even when the
- * APIC is disabled.
+ * If PIC mode is available, LAPIC need be disconnected with CPU. Otherwise
+ * enable LAPIC and set it to be virtual wire mode. However if IO-APIC has
+ * been virtual wire mode, LVT0 of LAPIC need be masked.
  */
-void disconnect_bsp_APIC(int virt_wire_setup)
+void lapic_set_legacy_irq_mode(int virt_wire_setup)
 {
 	unsigned int value;
 
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index d8f5cbe..f3f3abe 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1430,7 +1430,7 @@ void __init enable_IO_APIC(void)
 	clear_IO_APIC();
 }
 
-void native_disable_io_apic(void)
+static void ioapic_set_virtual_wire_mode(void)
 {
 	/*
 	 * If the i8259 is routed through an IOAPIC
@@ -1453,21 +1453,24 @@ void native_disable_io_apic(void)
 		 */
 		ioapic_write_entry(ioapic_i8259.apic, ioapic_i8259.pin, entry);
 	}
-
-	if (boot_cpu_has(X86_FEATURE_APIC) || apic_from_smp_config())
-		disconnect_bsp_APIC(ioapic_i8259.pin != -1);
 }
 
 /*
- * Not an __init, needed by kexec/kdump code.
- * For safety IO-APIC and Local APIC need be cleared before this.
+ * In legacy irq mode, full DOS compatibility with the uniprocessor PC/AT is
+ * provided by using the APICs in conjunction with standard 8259A-equivalent
+ * programmable interrupt controllers (PICs). It's necessary to deliver legacy
+ * interrupts even when APIC mode is not enabled. This is required by kexec/
+ * kdump before enter into the 2nd kernel.
  */
 void switch_to_legacy_irq_mode(void)
 {
 	if (!nr_legacy_irqs())
 		return;
 
-	x86_io_apic_ops.disable();
+	ioapic_set_virtual_wire_mode();
+
+	if (boot_cpu_has(X86_FEATURE_APIC) || apic_from_smp_config())
+		lapic_set_legacy_irq_mode(ioapic_i8259.pin != -1);
 }
 
 #ifdef CONFIG_X86_32
diff --git a/arch/x86/kernel/x86_init.c b/arch/x86/kernel/x86_init.c
index dad5fe9..1177c64 100644
--- a/arch/x86/kernel/x86_init.c
+++ b/arch/x86/kernel/x86_init.c
@@ -138,5 +138,5 @@ void arch_restore_msi_irqs(struct pci_dev *dev)
 
 struct x86_io_apic_ops x86_io_apic_ops = {
 	.read			= native_io_apic_read,
-	.disable		= native_disable_io_apic,
+	.disable		= switch_to_legacy_irq_mode,
 };
diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
index 49721b4..751472d 100644
--- a/drivers/iommu/irq_remapping.c
+++ b/drivers/iommu/irq_remapping.c
@@ -37,7 +37,7 @@ static void irq_remapping_disable_io_apic(void)
 	 * now.
 	 */
 	if (boot_cpu_has(X86_FEATURE_APIC) || apic_from_smp_config())
-		disconnect_bsp_APIC(0);
+		lapic_set_legacy_irq_mode(0);
 }
 
 static void __init irq_remapping_modify_x86_ops(void)
-- 
2.5.5

[PATCH 3/3] x86/apic: Clean up the apic delivery mode macro definition

[Baoquan He]

Arch x86 use ICR to send IPI, and the delivery mode in ICR can be set.
They are defined like APIC_DM_xxxxx in arch/x86/include/asm/apicdef.h.
LVT table also includes a delivery mode field to set the type of
interrupt to be sent to processor, they are defined like APIC_MODE_xxxx.
And ExtINT can only be set for LVT LINT0/1, ICR can't use ExtINT either.

However the current code defines APIC_DM_EXTINT and mixed the usage of
it with APIC_MODE_EXTINT. Here clean it up.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 arch/x86/include/asm/apicdef.h | 1 -
 arch/x86/kernel/apic/apic.c    | 8 +++++---
 arch/x86/kernel/apic/io_apic.c | 7 +++++--
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/apicdef.h b/arch/x86/include/asm/apicdef.h
index c46bb99..cbca926 100644
--- a/arch/x86/include/asm/apicdef.h
+++ b/arch/x86/include/asm/apicdef.h
@@ -85,7 +85,6 @@
 #define		APIC_DM_NMI		0x00400
 #define		APIC_DM_INIT		0x00500
 #define		APIC_DM_STARTUP		0x00600
-#define		APIC_DM_EXTINT		0x00700
 #define		APIC_VECTOR_MASK	0x000FF
 #define	APIC_ICR2	0x310
 #define		GET_APIC_DEST_FIELD(x)	(((x) >> 24) & 0xFF)
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 9bb2770..7bf639d 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1163,7 +1163,8 @@ void __init init_bsp_APIC(void)
 	/*
 	 * Set up the virtual wire mode.
 	 */
-	apic_write(APIC_LVT0, APIC_DM_EXTINT);
+	value = SET_APIC_DELIVERY_MODE(0, APIC_MODE_EXTINT);
+	apic_write(APIC_LVT0, value);
 	value = APIC_DM_NMI;
 	if (!lapic_is_integrated())		/* 82489DX */
 		value |= APIC_LVT_LEVEL_TRIGGER;
@@ -1377,10 +1378,11 @@ void setup_local_APIC(void)
 	 */
 	value = apic_read(APIC_LVT0) & APIC_LVT_MASKED;
 	if (!cpu && (pic_mode || !value)) {
-		value = APIC_DM_EXTINT;
+		value = SET_APIC_DELIVERY_MODE(0, APIC_MODE_EXTINT);
 		apic_printk(APIC_VERBOSE, "enabled ExtINT on CPU#%d\n", cpu);
 	} else {
-		value = APIC_DM_EXTINT | APIC_LVT_MASKED;
+		value = SET_APIC_DELIVERY_MODE(0, APIC_MODE_EXTINT);
+		value = value | APIC_LVT_MASKED;
 		apic_printk(APIC_VERBOSE, "masked ExtINT on CPU#%d\n", cpu);
 	}
 	apic_write(APIC_LVT0, value);
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index f3f3abe..a911564 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2053,6 +2053,7 @@ static inline void __init check_timer(void)
 	int apic1, pin1, apic2, pin2;
 	unsigned long flags;
 	int no_pin1 = 0;
+	unsigned int value;
 
 	local_irq_save(flags);
 
@@ -2070,7 +2071,8 @@ static inline void __init check_timer(void)
 	 * The AEOI mode will finish them in the 8259A
 	 * automatically.
 	 */
-	apic_write(APIC_LVT0, APIC_LVT_MASKED | APIC_DM_EXTINT);
+	value = SET_APIC_DELIVERY_MODE(0, APIC_MODE_EXTINT);
+	apic_write(APIC_LVT0, APIC_LVT_MASKED | value);
 	legacy_pic->init(1);
 
 	pin1  = find_isa_irq_pin(0, mp_INT);
@@ -2171,7 +2173,8 @@ static inline void __init check_timer(void)
 
 	legacy_pic->init(0);
 	legacy_pic->make_irq(0);
-	apic_write(APIC_LVT0, APIC_DM_EXTINT);
+	value = SET_APIC_DELIVERY_MODE(0, APIC_MODE_EXTINT);
+	apic_write(APIC_LVT0, value);
 
 	unlock_ExtINT_logic();
 
-- 
2.5.5

Re: [PATCH 0/3] Enable legacy irq mode before jump to kexec/kdump kernel

[Wei, Jiangang]

Hi Baoquan He,

Well, Indeed there‘s a relationship between the dump-capture hangs in
calibrate_delay_converge() and the interrupt mode.

but there‘s no essential difference between your patches and mine that
calls disable_IO_APIC() again.  
Actually, disable_IO_APIC will set APIC to virtual wire mode.

In fact,
Eric and Ingo suggested that "it should be fixed in the bootup path of
the dump kernel, not the crash kernel reboot path", which is convincing
and reasonable.

And i find a better method can fix the problem.
It's better to set virtual wire mode for apic in init_bsp_APIC(), which
in the bootup path of dump kernel.
But now, init_bsp_APIC doesn't initialize the apic to vitual wire mode
when smp_found_config is non-zero.

FYI, I'm working on this point. later i will send patches to mail list.

Wei

On Wed, 2016-07-20 at 10:58 +0800, Baoquan He wrote:
> Wei Jiangang reported kdump kernel always hang when "notsc" is specified
> in boot parameter. After debugging I found there's no timer interrupt
> in the current kexec/kdump kernel. This is caused by commit 522e66464467
> ("x86/apic: Disable I/O APIC before shutdown of the local APIC"). Originally
> Eric posted below patch to make system be virtual wire mode in which 8259-
> equivalent PIC fields all interrupts and the LAPIC becomes a virtual wire.
> Like this interrupts can be delivered from PIC to CPU via the LAPIC's local
> interrupt 0 (LINTIN0). In virtual wire APIC mode is disabled while LAPIC
> is software enabled and its LINT0 and LINT1 need be programmed specifically.
> 
> https://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.11/2.6.11-mm1/broken-out/x86_64-apic-virtwire-on-shutdown.patch
> 
> But with commit 522e66464 you can see after disable_IO_APIC had setting
> virtual wire mode, lapic_shutdown disabled LAPIC again. Now virtual wire
> mode doesn't work, then it cause no timer interrupt during kdump kernel
> initialization stage until system enter into APIC mode.
> 
> So people may be wondering why only kdump kernel hang, the normal kernel
> with "notsc" can still work. This is because BIOS has already built PIC mode
> or virtual wire mode while kexec/kdump kernel doesn't go through BIOS
> initialization. That is why we have to change system to be PIC mode or
> virtual wire mode before jump to kexec/kdump kernel.
> 
> Then why kdump kernel didn't hang when "notsc" is not specified. This is
> because tsc_init will assign the already calibrated value to lpj_fine.
> Then kernel doesn't need to count cpu loops between jiffies with the help
> of timer interrupt. So "notsc" is not victim, but a informer.
> 
> In patch 1/3 disable_IO_APIC is changed to only contain code of changeing
> system to be PIC mode or virtual wire mode and is renamed as
> switch_to_legacy_irq_mode. Now only call clear_IO_APIC where IO-APIC need
> be disabled, and call switch_to_legacy_irq_mode before jump to kexe/kdump
> kernel.
> 
> Patch 2/3 and 3/3 are clean up patch.
> 
> Baoquan He (3):
>   x86/apic/kexec: Enable legacy irq mode before jump to kexec/kdump
>     kernel
>   x86/apic: Clean up the names of legacy irq mode setting related
>     functions
>   x86/apic: Clean up the apic delivery mode macro definition
> 
>  arch/x86/include/asm/apic.h        |  2 +-
>  arch/x86/include/asm/apicdef.h     |  1 -
>  arch/x86/include/asm/io_apic.h     |  6 +++---
>  arch/x86/kernel/apic/apic.c        | 19 +++++++++++--------
>  arch/x86/kernel/apic/io_apic.c     | 32 +++++++++++++++++---------------
>  arch/x86/kernel/crash.c            |  2 +-
>  arch/x86/kernel/machine_kexec_32.c | 15 +++++----------
>  arch/x86/kernel/machine_kexec_64.c | 15 +++++----------
>  arch/x86/kernel/reboot.c           |  2 +-
>  arch/x86/kernel/x86_init.c         |  2 +-
>  drivers/iommu/irq_remapping.c      |  2 +-
>  11 files changed, 46 insertions(+), 52 deletions(-)
> 

Re: [PATCH 0/3] Enable legacy irq mode before jump to kexec/kdump kernel

[bhe]

Hi Jiangang,

On 07/20/16 at 03:54am, Wei, Jiangang wrote:
> Hi Baoquan He,
> 
> Well, Indeed there‘s a relationship between the dump-capture hangs in
> calibrate_delay_converge() and the interrupt mode.
> 
> but there‘s no essential difference between your patches and mine that
> calls disable_IO_APIC() again.  
> Actually, disable_IO_APIC will set APIC to virtual wire mode.

You didn't read my patch log carefully. disable_IO_APIC not only set
APIC to PIC mode or virtual wire mode, but call clear_IO_APIC.

> 
> In fact,
> Eric and Ingo suggested that "it should be fixed in the bootup path of
> the dump kernel, not the crash kernel reboot path", which is convincing
> and reasonable.

Well this patch doesn't do differently with Eric's original implemention
in kexec/kdump path.
By taking out clear_IO_APIC from disable_IO_APIC, the left code of
disable_IO_APIC will only do the virtual wire setting. So for
kexec/kdump path, code basically is the same as Eric's method. But for
poweroff/halt/reboot, it's enough to call clear_IO_APIC to disable
IO-APIC.

> 
> And i find a better method can fix the problem.
> It's better to set virtual wire mode for apic in init_bsp_APIC(), which
> in the bootup path of dump kernel.
> But now, init_bsp_APIC doesn't initialize the apic to vitual wire mode
> when smp_found_config is non-zero.

And virtual wire mode have two kinds, IO-APIC virtual wire mode and
LAPIC virtual wire mode. Please read code comments in init_bsp_APIC,
IO-APIC virtual wire mode could be active or need be set, you can't
detect IO-APIC pin connected to i8259 equvialent PIC.

> 
> FYI, I'm working on this point. later i will send patches to mail list.
> 
> Wei
> 
> On Wed, 2016-07-20 at 10:58 +0800, Baoquan He wrote:
> > Wei Jiangang reported kdump kernel always hang when "notsc" is specified
> > in boot parameter. After debugging I found there's no timer interrupt
> > in the current kexec/kdump kernel. This is caused by commit 522e66464467
> > ("x86/apic: Disable I/O APIC before shutdown of the local APIC"). Originally
> > Eric posted below patch to make system be virtual wire mode in which 8259-
> > equivalent PIC fields all interrupts and the LAPIC becomes a virtual wire.
> > Like this interrupts can be delivered from PIC to CPU via the LAPIC's local
> > interrupt 0 (LINTIN0). In virtual wire APIC mode is disabled while LAPIC
> > is software enabled and its LINT0 and LINT1 need be programmed specifically.
> > 
> > https://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.11/2.6.11-mm1/broken-out/x86_64-apic-virtwire-on-shutdown.patch
> > 
> > But with commit 522e66464 you can see after disable_IO_APIC had setting
> > virtual wire mode, lapic_shutdown disabled LAPIC again. Now virtual wire
> > mode doesn't work, then it cause no timer interrupt during kdump kernel
> > initialization stage until system enter into APIC mode.
> > 
> > So people may be wondering why only kdump kernel hang, the normal kernel
> > with "notsc" can still work. This is because BIOS has already built PIC mode
> > or virtual wire mode while kexec/kdump kernel doesn't go through BIOS
> > initialization. That is why we have to change system to be PIC mode or
> > virtual wire mode before jump to kexec/kdump kernel.
> > 
> > Then why kdump kernel didn't hang when "notsc" is not specified. This is
> > because tsc_init will assign the already calibrated value to lpj_fine.
> > Then kernel doesn't need to count cpu loops between jiffies with the help
> > of timer interrupt. So "notsc" is not victim, but a informer.
> > 
> > In patch 1/3 disable_IO_APIC is changed to only contain code of changeing
> > system to be PIC mode or virtual wire mode and is renamed as
> > switch_to_legacy_irq_mode. Now only call clear_IO_APIC where IO-APIC need
> > be disabled, and call switch_to_legacy_irq_mode before jump to kexe/kdump
> > kernel.
> > 
> > Patch 2/3 and 3/3 are clean up patch.
> > 
> > Baoquan He (3):
> >   x86/apic/kexec: Enable legacy irq mode before jump to kexec/kdump
> >     kernel
> >   x86/apic: Clean up the names of legacy irq mode setting related
> >     functions
> >   x86/apic: Clean up the apic delivery mode macro definition
> > 
> >  arch/x86/include/asm/apic.h        |  2 +-
> >  arch/x86/include/asm/apicdef.h     |  1 -
> >  arch/x86/include/asm/io_apic.h     |  6 +++---
> >  arch/x86/kernel/apic/apic.c        | 19 +++++++++++--------
> >  arch/x86/kernel/apic/io_apic.c     | 32 +++++++++++++++++---------------
> >  arch/x86/kernel/crash.c            |  2 +-
> >  arch/x86/kernel/machine_kexec_32.c | 15 +++++----------
> >  arch/x86/kernel/machine_kexec_64.c | 15 +++++----------
> >  arch/x86/kernel/reboot.c           |  2 +-
> >  arch/x86/kernel/x86_init.c         |  2 +-
> >  drivers/iommu/irq_remapping.c      |  2 +-
> >  11 files changed, 46 insertions(+), 52 deletions(-)
> > 
> 
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 0/3] Enable legacy irq mode before jump to kexec/kdump kernel

[bhe]

On 07/20/16 at 03:54am, Wei, Jiangang wrote:
> Hi Baoquan He,
> 
> Well, Indeed there‘s a relationship between the dump-capture hangs in
> calibrate_delay_converge() and the interrupt mode.
> 
> but there‘s no essential difference between your patches and mine that
> calls disable_IO_APIC() again.  
> Actually, disable_IO_APIC will set APIC to virtual wire mode.

Well, about this I want to explain a little bit. Usually people posted a
patch, reviewers can give comments, suggestions or other ideas. During
reviewing stage patch author need answer questions from people
interested. So this is an interactive action, reviewers can also learn
knowledge, meanwhile give comments.

When reviewing your patch, I have many questions, and only get two
pieces of information, kdump kernel hang with notsc, and disable_IO_APIC
can save it. You even can't answer people's question why disable_IO_APIC
need be called twice with your patch. I have to dig code and read intel
arch manual and MP spec and Eric's original patch thread, and add debug
bug to verify all. How can it be like you said "no essential difference
between your patches and mine"? 


> 
> In fact,
> Eric and Ingo suggested that "it should be fixed in the bootup path of
> the dump kernel, not the crash kernel reboot path", which is convincing
> and reasonable.
> 
> And i find a better method can fix the problem.
> It's better to set virtual wire mode for apic in init_bsp_APIC(), which
> in the bootup path of dump kernel.
> But now, init_bsp_APIC doesn't initialize the apic to vitual wire mode
> when smp_found_config is non-zero.

This may be do-able, let's see what Eric and Ingo will say.

> 
> FYI, I'm working on this point. later i will send patches to mail list.
> 
> Wei
> 
> On Wed, 2016-07-20 at 10:58 +0800, Baoquan He wrote:
> > Wei Jiangang reported kdump kernel always hang when "notsc" is specified
> > in boot parameter. After debugging I found there's no timer interrupt
> > in the current kexec/kdump kernel. This is caused by commit 522e66464467
> > ("x86/apic: Disable I/O APIC before shutdown of the local APIC"). Originally
> > Eric posted below patch to make system be virtual wire mode in which 8259-
> > equivalent PIC fields all interrupts and the LAPIC becomes a virtual wire.
> > Like this interrupts can be delivered from PIC to CPU via the LAPIC's local
> > interrupt 0 (LINTIN0). In virtual wire APIC mode is disabled while LAPIC
> > is software enabled and its LINT0 and LINT1 need be programmed specifically.
> > 
> > https://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.11/2.6.11-mm1/broken-out/x86_64-apic-virtwire-on-shutdown.patch
> > 
> > But with commit 522e66464 you can see after disable_IO_APIC had setting
> > virtual wire mode, lapic_shutdown disabled LAPIC again. Now virtual wire
> > mode doesn't work, then it cause no timer interrupt during kdump kernel
> > initialization stage until system enter into APIC mode.
> > 
> > So people may be wondering why only kdump kernel hang, the normal kernel
> > with "notsc" can still work. This is because BIOS has already built PIC mode
> > or virtual wire mode while kexec/kdump kernel doesn't go through BIOS
> > initialization. That is why we have to change system to be PIC mode or
> > virtual wire mode before jump to kexec/kdump kernel.
> > 
> > Then why kdump kernel didn't hang when "notsc" is not specified. This is
> > because tsc_init will assign the already calibrated value to lpj_fine.
> > Then kernel doesn't need to count cpu loops between jiffies with the help
> > of timer interrupt. So "notsc" is not victim, but a informer.
> > 
> > In patch 1/3 disable_IO_APIC is changed to only contain code of changeing
> > system to be PIC mode or virtual wire mode and is renamed as
> > switch_to_legacy_irq_mode. Now only call clear_IO_APIC where IO-APIC need
> > be disabled, and call switch_to_legacy_irq_mode before jump to kexe/kdump
> > kernel.
> > 
> > Patch 2/3 and 3/3 are clean up patch.
> > 
> > Baoquan He (3):
> >   x86/apic/kexec: Enable legacy irq mode before jump to kexec/kdump
> >     kernel
> >   x86/apic: Clean up the names of legacy irq mode setting related
> >     functions
> >   x86/apic: Clean up the apic delivery mode macro definition
> > 
> >  arch/x86/include/asm/apic.h        |  2 +-
> >  arch/x86/include/asm/apicdef.h     |  1 -
> >  arch/x86/include/asm/io_apic.h     |  6 +++---
> >  arch/x86/kernel/apic/apic.c        | 19 +++++++++++--------
> >  arch/x86/kernel/apic/io_apic.c     | 32 +++++++++++++++++---------------
> >  arch/x86/kernel/crash.c            |  2 +-
> >  arch/x86/kernel/machine_kexec_32.c | 15 +++++----------
> >  arch/x86/kernel/machine_kexec_64.c | 15 +++++----------
> >  arch/x86/kernel/reboot.c           |  2 +-
> >  arch/x86/kernel/x86_init.c         |  2 +-
> >  drivers/iommu/irq_remapping.c      |  2 +-
> >  11 files changed, 46 insertions(+), 52 deletions(-)
> > 
> 
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 0/3] Enable legacy irq mode before jump to kexec/kdump kernel

[Thomas Gleixner]

On Wed, 20 Jul 2016, bhe@redhat.com wrote:
> On 07/20/16 at 03:54am, Wei, Jiangang wrote:
>
> >  In fact, Eric and Ingo suggested that "it should be fixed in the bootup
> > path of the dump kernel, not the crash kernel reboot path", which is
> > convincing and reasonable.
> 
> Well this patch doesn't do differently with Eric's original implemention
> in kexec/kdump path.
> By taking out clear_IO_APIC from disable_IO_APIC, the left code of
> disable_IO_APIC will only do the virtual wire setting. So for
> kexec/kdump path, code basically is the same as Eric's method. But for
> poweroff/halt/reboot, it's enough to call clear_IO_APIC to disable
> IO-APIC.

You're completely ignoring what Jiangang said: 

 "it should be fixed in the bootup path of the dump kernel, not the crash
  kernel reboot path"

and that's the right way to do it. End of story.

Thanks,

	tglx

Re: [PATCH 0/3] Enable legacy irq mode before jump to kexec/kdump kernel

[bhe]

On 07/20/16 at 08:32am, Thomas Gleixner wrote:
> On Wed, 20 Jul 2016, bhe@redhat.com wrote:
> > On 07/20/16 at 03:54am, Wei, Jiangang wrote:
> >
> > >  In fact, Eric and Ingo suggested that "it should be fixed in the bootup
> > > path of the dump kernel, not the crash kernel reboot path", which is
> > > convincing and reasonable.
> > 
> > Well this patch doesn't do differently with Eric's original implemention
> > in kexec/kdump path.
> > By taking out clear_IO_APIC from disable_IO_APIC, the left code of
> > disable_IO_APIC will only do the virtual wire setting. So for
> > kexec/kdump path, code basically is the same as Eric's method. But for
> > poweroff/halt/reboot, it's enough to call clear_IO_APIC to disable
> > IO-APIC.
> 
> You're completely ignoring what Jiangang said: 
> 
>  "it should be fixed in the bootup path of the dump kernel, not the crash
>   kernel reboot path"
> 
> and that's the right way to do it. End of story.

Thanks, tglx. What I did is like reverting commit 522e6646446. But it
would be great if we can change to fix it in bootup path.

Thanks
Baoquan

Re: [PATCH 3/3] x86/apic: Clean up the apic delivery mode macro definition

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2356 bytes --]

Hi,

[auto build test ERROR on iommu/next]
[also build test ERROR on v4.7-rc7 next-20160722]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Baoquan-He/Enable-legacy-irq-mode-before-jump-to-kexec-kdump-kernel/20160724-054857
base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: i386-randconfig-i0-201630 (attached as .config)
compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   arch/x86/kvm/lapic.c: In function '__apic_accept_irq':
>> arch/x86/kvm/lapic.c:963:7: error: 'APIC_DM_EXTINT' undeclared (first use in this function)
     case APIC_DM_EXTINT:
          ^
   arch/x86/kvm/lapic.c:963:7: note: each undeclared identifier is reported only once for each function it appears in

vim +/APIC_DM_EXTINT +963 arch/x86/kvm/lapic.c

66450a21f arch/x86/kvm/lapic.c Jan Kiszka      2013-03-13  957  		smp_wmb();
66450a21f arch/x86/kvm/lapic.c Jan Kiszka      2013-03-13  958  		set_bit(KVM_APIC_SIPI, &apic->pending_events);
3842d135f arch/x86/kvm/lapic.c Avi Kivity      2010-07-27  959  		kvm_make_request(KVM_REQ_EVENT, vcpu);
d76901750 arch/x86/kvm/lapic.c Marcelo Tosatti 2008-09-08  960  		kvm_vcpu_kick(vcpu);
97222cc83 drivers/kvm/lapic.c  Eddie Dong      2007-09-12  961  		break;
97222cc83 drivers/kvm/lapic.c  Eddie Dong      2007-09-12  962  
23930f952 arch/x86/kvm/lapic.c Jan Kiszka      2008-09-26 @963  	case APIC_DM_EXTINT:
23930f952 arch/x86/kvm/lapic.c Jan Kiszka      2008-09-26  964  		/*
23930f952 arch/x86/kvm/lapic.c Jan Kiszka      2008-09-26  965  		 * Should only be called by kvm_apic_local_deliver() with LVT0,
23930f952 arch/x86/kvm/lapic.c Jan Kiszka      2008-09-26  966  		 * before NMI watchdog was enabled. Already handled by

:::::: The code at line 963 was first introduced by commit
:::::: 23930f9521c9c4d4aa96cdb9d1e1703f3782bb94 KVM: x86: Enable NMI Watchdog via in-kernel PIT source

:::::: TO: Jan Kiszka <jan.kiszka@siemens.com>
:::::: CC: Avi Kivity <avi@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 25292 bytes --]

Re: [PATCH 3/3] x86/apic: Clean up the apic delivery mode macro definition

[kbuild test robot]

Hi,

[auto build test WARNING on iommu/next]
[also build test WARNING on v4.7-rc7 next-20160722]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Baoquan-He/Enable-legacy-irq-mode-before-jump-to-kexec-kdump-kernel/20160724-054857
base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   include/linux/compiler.h:232:8: sparse: attribute 'no_sanitize_address': unknown attribute
   arch/x86/kvm/lapic.c:186:15: sparse: incompatible types in comparison expression (different address spaces)
   arch/x86/kvm/lapic.c:963:14: sparse: undefined identifier 'APIC_DM_EXTINT'
>> arch/x86/kvm/lapic.c:963:14: sparse: incompatible types for 'case' statement
   arch/x86/kvm/lapic.c:699:15: sparse: incompatible types in comparison expression (different address spaces)
   arch/x86/kvm/lapic.c:799:15: sparse: incompatible types in comparison expression (different address spaces)
   arch/x86/kvm/lapic.c:963:14: sparse: Expected constant expression in case statement
   arch/x86/kvm/lapic.c: In function '__apic_accept_irq':
   arch/x86/kvm/lapic.c:963:7: error: 'APIC_DM_EXTINT' undeclared (first use in this function)
     case APIC_DM_EXTINT:
          ^~~~~~~~~~~~~~
   arch/x86/kvm/lapic.c:963:7: note: each undeclared identifier is reported only once for each function it appears in

vim +/case +963 arch/x86/kvm/lapic.c

c5ec15340 drivers/kvm/lapic.c  He, Qing        2007-09-03  947  				   vcpu->vcpu_id);
c5ec15340 drivers/kvm/lapic.c  He, Qing        2007-09-03  948  		}
97222cc83 drivers/kvm/lapic.c  Eddie Dong      2007-09-12  949  		break;
97222cc83 drivers/kvm/lapic.c  Eddie Dong      2007-09-12  950  
97222cc83 drivers/kvm/lapic.c  Eddie Dong      2007-09-12  951  	case APIC_DM_STARTUP:
1b10bf31a arch/x86/kvm/lapic.c Jan Kiszka      2008-09-30  952  		apic_debug("SIPI to vcpu %d vector 0x%02x\n",
c5ec15340 drivers/kvm/lapic.c  He, Qing        2007-09-03  953  			   vcpu->vcpu_id, vector);
6da7e3f64 arch/x86/kvm/lapic.c Gleb Natapov    2009-03-05  954  		result = 1;
66450a21f arch/x86/kvm/lapic.c Jan Kiszka      2013-03-13  955  		apic->sipi_vector = vector;
66450a21f arch/x86/kvm/lapic.c Jan Kiszka      2013-03-13  956  		/* make sure sipi_vector is visible for the receiver */
66450a21f arch/x86/kvm/lapic.c Jan Kiszka      2013-03-13  957  		smp_wmb();
66450a21f arch/x86/kvm/lapic.c Jan Kiszka      2013-03-13  958  		set_bit(KVM_APIC_SIPI, &apic->pending_events);
3842d135f arch/x86/kvm/lapic.c Avi Kivity      2010-07-27  959  		kvm_make_request(KVM_REQ_EVENT, vcpu);
d76901750 arch/x86/kvm/lapic.c Marcelo Tosatti 2008-09-08  960  		kvm_vcpu_kick(vcpu);
97222cc83 drivers/kvm/lapic.c  Eddie Dong      2007-09-12  961  		break;
97222cc83 drivers/kvm/lapic.c  Eddie Dong      2007-09-12  962  
23930f952 arch/x86/kvm/lapic.c Jan Kiszka      2008-09-26 @963  	case APIC_DM_EXTINT:
23930f952 arch/x86/kvm/lapic.c Jan Kiszka      2008-09-26  964  		/*
23930f952 arch/x86/kvm/lapic.c Jan Kiszka      2008-09-26  965  		 * Should only be called by kvm_apic_local_deliver() with LVT0,
23930f952 arch/x86/kvm/lapic.c Jan Kiszka      2008-09-26  966  		 * before NMI watchdog was enabled. Already handled by
23930f952 arch/x86/kvm/lapic.c Jan Kiszka      2008-09-26  967  		 * kvm_apic_accept_pic_intr().
23930f952 arch/x86/kvm/lapic.c Jan Kiszka      2008-09-26  968  		 */
23930f952 arch/x86/kvm/lapic.c Jan Kiszka      2008-09-26  969  		break;
23930f952 arch/x86/kvm/lapic.c Jan Kiszka      2008-09-26  970  
97222cc83 drivers/kvm/lapic.c  Eddie Dong      2007-09-12  971  	default:

:::::: The code at line 963 was first introduced by commit
:::::: 23930f9521c9c4d4aa96cdb9d1e1703f3782bb94 KVM: x86: Enable NMI Watchdog via in-kernel PIT source

:::::: TO: Jan Kiszka <jan.kiszka@siemens.com>
:::::: CC: Avi Kivity <avi@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH 3/3] x86/apic: Clean up the apic delivery mode macro definition

[Baoquan He]

On 07/24/16 at 07:07am, kbuild test robot wrote:
> Hi,


Sorry, I didn't notice this. And after checking code, APIC_DM_EXTINT
could be added for kvm on purpose thought it's not defined in Intel Arch
manual. I am not familiar with KVM, so please drop this patch to avoid
warning. I will make time to study it later.


Thanks
Baoquan

> 
> [auto build test WARNING on iommu/next]
> [also build test WARNING on v4.7-rc7 next-20160722]
> [cannot apply to tip/x86/core]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Baoquan-He/Enable-legacy-irq-mode-before-jump-to-kexec-kdump-kernel/20160724-054857
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
> reproduce:
>         # apt-get install sparse
>         make ARCH=x86_64 allmodconfig
>         make C=1 CF=-D__CHECK_ENDIAN__
> 
> 
> sparse warnings: (new ones prefixed by >>)
> 
>    include/linux/compiler.h:232:8: sparse: attribute 'no_sanitize_address': unknown attribute
>    arch/x86/kvm/lapic.c:186:15: sparse: incompatible types in comparison expression (different address spaces)
>    arch/x86/kvm/lapic.c:963:14: sparse: undefined identifier 'APIC_DM_EXTINT'
> >> arch/x86/kvm/lapic.c:963:14: sparse: incompatible types for 'case' statement
>    arch/x86/kvm/lapic.c:699:15: sparse: incompatible types in comparison expression (different address spaces)
>    arch/x86/kvm/lapic.c:799:15: sparse: incompatible types in comparison expression (different address spaces)
>    arch/x86/kvm/lapic.c:963:14: sparse: Expected constant expression in case statement
>    arch/x86/kvm/lapic.c: In function '__apic_accept_irq':
>    arch/x86/kvm/lapic.c:963:7: error: 'APIC_DM_EXTINT' undeclared (first use in this function)
>      case APIC_DM_EXTINT:
>           ^~~~~~~~~~~~~~
>    arch/x86/kvm/lapic.c:963:7: note: each undeclared identifier is reported only once for each function it appears in
> 
> vim +/case +963 arch/x86/kvm/lapic.c
> 
> c5ec15340 drivers/kvm/lapic.c  He, Qing        2007-09-03  947  				   vcpu->vcpu_id);
> c5ec15340 drivers/kvm/lapic.c  He, Qing        2007-09-03  948  		}
> 97222cc83 drivers/kvm/lapic.c  Eddie Dong      2007-09-12  949  		break;
> 97222cc83 drivers/kvm/lapic.c  Eddie Dong      2007-09-12  950  
> 97222cc83 drivers/kvm/lapic.c  Eddie Dong      2007-09-12  951  	case APIC_DM_STARTUP:
> 1b10bf31a arch/x86/kvm/lapic.c Jan Kiszka      2008-09-30  952  		apic_debug("SIPI to vcpu %d vector 0x%02x\n",
> c5ec15340 drivers/kvm/lapic.c  He, Qing        2007-09-03  953  			   vcpu->vcpu_id, vector);
> 6da7e3f64 arch/x86/kvm/lapic.c Gleb Natapov    2009-03-05  954  		result = 1;
> 66450a21f arch/x86/kvm/lapic.c Jan Kiszka      2013-03-13  955  		apic->sipi_vector = vector;
> 66450a21f arch/x86/kvm/lapic.c Jan Kiszka      2013-03-13  956  		/* make sure sipi_vector is visible for the receiver */
> 66450a21f arch/x86/kvm/lapic.c Jan Kiszka      2013-03-13  957  		smp_wmb();
> 66450a21f arch/x86/kvm/lapic.c Jan Kiszka      2013-03-13  958  		set_bit(KVM_APIC_SIPI, &apic->pending_events);
> 3842d135f arch/x86/kvm/lapic.c Avi Kivity      2010-07-27  959  		kvm_make_request(KVM_REQ_EVENT, vcpu);
> d76901750 arch/x86/kvm/lapic.c Marcelo Tosatti 2008-09-08  960  		kvm_vcpu_kick(vcpu);
> 97222cc83 drivers/kvm/lapic.c  Eddie Dong      2007-09-12  961  		break;
> 97222cc83 drivers/kvm/lapic.c  Eddie Dong      2007-09-12  962  
> 23930f952 arch/x86/kvm/lapic.c Jan Kiszka      2008-09-26 @963  	case APIC_DM_EXTINT:
> 23930f952 arch/x86/kvm/lapic.c Jan Kiszka      2008-09-26  964  		/*
> 23930f952 arch/x86/kvm/lapic.c Jan Kiszka      2008-09-26  965  		 * Should only be called by kvm_apic_local_deliver() with LVT0,
> 23930f952 arch/x86/kvm/lapic.c Jan Kiszka      2008-09-26  966  		 * before NMI watchdog was enabled. Already handled by
> 23930f952 arch/x86/kvm/lapic.c Jan Kiszka      2008-09-26  967  		 * kvm_apic_accept_pic_intr().
> 23930f952 arch/x86/kvm/lapic.c Jan Kiszka      2008-09-26  968  		 */
> 23930f952 arch/x86/kvm/lapic.c Jan Kiszka      2008-09-26  969  		break;
> 23930f952 arch/x86/kvm/lapic.c Jan Kiszka      2008-09-26  970  
> 97222cc83 drivers/kvm/lapic.c  Eddie Dong      2007-09-12  971  	default:
> 
> :::::: The code at line 963 was first introduced by commit
> :::::: 23930f9521c9c4d4aa96cdb9d1e1703f3782bb94 KVM: x86: Enable NMI Watchdog via in-kernel PIT source
> 
> :::::: TO: Jan Kiszka <jan.kiszka@siemens.com>
> :::::: CC: Avi Kivity <avi@redhat.com>
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation