[PATCH 1/3] x86/apic: Remove "focus disabled" for 64bit case

[PATCH 1/3] x86/apic: Remove "focus disabled" for 64bit case

[Wei Jiangang]

Disable processor focus for 64bit causes a crash,
Call Trace as following:

  [<ffffffff8133499f>] dump_stack+0x63/0x84
  [<ffffffff810800f1>] __warn+0xd1/0xf0
  [<ffffffff8108016f>] warn_slowpath_fmt+0x5f/0x80
  [<ffffffff81068ab2>] ex_handler_wrmsr_unsafe+0x62/0x70
  [<ffffffff81068b29>] fixup_exception+0x39/0x50
  [<ffffffff8102e030>] do_general_protection+0x80/0x160
  [<ffffffff816a89a8>] general_protection+0x28/0x30
  [<ffffffff81062554>] ? native_write_msr+0x4/0x30
  [<ffffffff81059c52>] ? native_apic_msr_write+0x32/0x40
  [<ffffffff81d92964>] init_bsp_APIC+0x5f/0x118
  [<ffffffff81d87489>] init_ISA_irqs+0x19/0x4c
  [<ffffffff81d87512>] native_init_IRQ+0xd/0x377
  [<ffffffff81d874fe>] init_IRQ+0x42/0x49
  [<ffffffff81d7afc6>] start_kernel+0x2ce/0x4c8
  [<ffffffff81d7aae6>] ? set_init_arg+0x55/0x55
  [<ffffffff81d7a120>] ? early_idt_handler_array+0x120/0x120
  [<ffffffff81d7a5db>] x86_64_start_reservations+0x2f/0x31
  [<ffffffff81d7a729>] x86_64_start_kernel+0x14c/0x16f

Keep a consistent implementation with the setup_local_APIC(),
always use processor focus for 64bit.
more details refer to commit 89c38c2867eb ("x86: apic - unify
setup_local_APIC")

Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
---
 arch/x86/kernel/apic/apic.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 60078a67d7e3..0273b652c689 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1154,9 +1154,7 @@ void __init init_bsp_APIC(void)
 	if ((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) &&
 	    (boot_cpu_data.x86 == 15))
 		value &= ~APIC_SPIV_FOCUS_DISABLED;
-	else
 #endif
-		value |= APIC_SPIV_FOCUS_DISABLED;
 	value |= SPURIOUS_APIC_VECTOR;
 	apic_write(APIC_SPIV, value);
 
-- 
1.9.3

[PATCH 1/3] x86/apic: Remove "focus disabled" for 64bit case

[Wei Jiangang]

Disable processor focus for 64bit causes a crash,
Call Trace as following:

  [<ffffffff8133499f>] dump_stack+0x63/0x84
  [<ffffffff810800f1>] __warn+0xd1/0xf0
  [<ffffffff8108016f>] warn_slowpath_fmt+0x5f/0x80
  [<ffffffff81068ab2>] ex_handler_wrmsr_unsafe+0x62/0x70
  [<ffffffff81068b29>] fixup_exception+0x39/0x50
  [<ffffffff8102e030>] do_general_protection+0x80/0x160
  [<ffffffff816a89a8>] general_protection+0x28/0x30
  [<ffffffff81062554>] ? native_write_msr+0x4/0x30
  [<ffffffff81059c52>] ? native_apic_msr_write+0x32/0x40
  [<ffffffff81d92964>] init_bsp_APIC+0x5f/0x118
  [<ffffffff81d87489>] init_ISA_irqs+0x19/0x4c
  [<ffffffff81d87512>] native_init_IRQ+0xd/0x377
  [<ffffffff81d874fe>] init_IRQ+0x42/0x49
  [<ffffffff81d7afc6>] start_kernel+0x2ce/0x4c8
  [<ffffffff81d7aae6>] ? set_init_arg+0x55/0x55
  [<ffffffff81d7a120>] ? early_idt_handler_array+0x120/0x120
  [<ffffffff81d7a5db>] x86_64_start_reservations+0x2f/0x31
  [<ffffffff81d7a729>] x86_64_start_kernel+0x14c/0x16f

Keep a consistent implementation with the setup_local_APIC(),
always use processor focus for 64bit.
more details refer to commit 89c38c2867eb ("x86: apic - unify
setup_local_APIC")

Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
---
 arch/x86/kernel/apic/apic.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 60078a67d7e3..0273b652c689 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1154,9 +1154,7 @@ void __init init_bsp_APIC(void)
 	if ((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) &&
 	    (boot_cpu_data.x86 == 15))
 		value &= ~APIC_SPIV_FOCUS_DISABLED;
-	else
 #endif
-		value |= APIC_SPIV_FOCUS_DISABLED;
 	value |= SPURIOUS_APIC_VECTOR;
 	apic_write(APIC_SPIV, value);
 
-- 
1.9.3




_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 2/3] x86/apic: Update comment about disabling processor focus

[Wei Jiangang]

Fix references to discarded end_level_ioapic_irq().

Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
---
 arch/x86/kernel/apic/apic.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 0273b652c689..8e25b9b2d351 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1346,7 +1346,6 @@ void setup_local_APIC(void)
 	 * Actually disabling the focus CPU check just makes the hang less
 	 * frequent as it makes the interrupt distributon model be more
 	 * like LRU than MRU (the short-term load is more even across CPUs).
-	 * See also the comment in end_level_ioapic_irq().  --macro
 	 */
 
 	/*
-- 
1.9.3

[PATCH 2/3] x86/apic: Update comment about disabling processor focus

[Wei Jiangang]

Fix references to discarded end_level_ioapic_irq().

Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
---
 arch/x86/kernel/apic/apic.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 0273b652c689..8e25b9b2d351 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1346,7 +1346,6 @@ void setup_local_APIC(void)
 	 * Actually disabling the focus CPU check just makes the hang less
 	 * frequent as it makes the interrupt distributon model be more
 	 * like LRU than MRU (the short-term load is more even across CPUs).
-	 * See also the comment in end_level_ioapic_irq().  --macro
 	 */
 
 	/*
-- 
1.9.3




_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 3/3] x86/apic: Improved the setting of interrupt mode for bsp

[Wei Jiangang]

If we specify the 'notsc' parameter for the dump-capture kernel,
and then trigger a crash(panic) by using "ALT-SysRq-c" or
"echo c > /proc/sysrq-trigger", the dump-capture kernel will
hang in calibrate_delay_converge() and wait for jiffies changes.
serial log as follows:

    tsc: Fast TSC calibration using PIT
    tsc: Detected 2099.947 MHz processor
    Calibrating delay loop...

The reason for jiffies not changes is there's no timer interrupt
passed to dump-capture kernel.

In fact, once kernel panic occurs, the local APIC is disabled
by lapic_shutdown() in reboot path.
generly speaking, local APIC state can be initialized by BIOS
after Power-Up or Reset, which doesn't apply to kdump case.
so the kernel has to be responsible for initialize the interrupt
mode properly according the latest status of APIC in bootup path.

An MP operating system is booted under either PIC mode or
virtual wire mode. Later, the operating system switches to
symmetric I/O mode as it enters multiprocessor mode.
Two kinds of virtual wire mode are defined in Intel MP spec:
virtual wire mode via local APIC or via I/O APIC.

Now we determine the mode of APIC only through a SMP BIOS(MP table).
That's not enough.
It's better to do further check if APIC works with effective mode,
and do some porper setting.

Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
---
 arch/x86/include/asm/io_apic.h |  5 ++++
 arch/x86/kernel/apic/apic.c    | 55 +++++++++++++++++++++++++++++++++++++++++-
 arch/x86/kernel/apic/io_apic.c | 28 +++++++++++++++++++++
 3 files changed, 87 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index 6cbf2cfb3f8a..6550bd43fa39 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -190,6 +190,7 @@ static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
 }
 
 extern void setup_IO_APIC(void);
+extern bool virtual_wire_via_ioapic(void);
 extern void enable_IO_APIC(void);
 extern void disable_IO_APIC(void);
 extern void setup_ioapic_dest(void);
@@ -231,6 +232,10 @@ static inline void io_apic_init_mappings(void) { }
 #define native_disable_io_apic		NULL
 
 static inline void setup_IO_APIC(void) { }
+static inline bool virtual_wire_via_ioapic(void)
+{
+	return true;
+}
 static inline void enable_IO_APIC(void) { }
 static inline void setup_ioapic_dest(void) { }
 
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 8e25b9b2d351..04358e0cf1e2 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1124,6 +1124,53 @@ void __init sync_Arb_IDs(void)
 }
 
 /*
+ * Return false means the virtual wire mode through-local-apic is inactive
+ */
+static bool virtual_wire_via_lapic(void)
+{
+	unsigned int value;
+
+	/* Check the APIC global enable/disable flag firstly */
+	if (boot_cpu_data.x86 >= 6) {
+		u32 h, l;
+
+		rdmsr(MSR_IA32_APICBASE, l, h);
+		/*
+		 * If local APIC is disabled by BIOS
+		 * do nothing, but return true
+		 */
+		if (!(l & MSR_IA32_APICBASE_ENABLE))
+			return true;
+	}
+
+	/* Check the software enable/disable flag */
+	value = apic_read(APIC_SPIV);
+	if (!(value & APIC_SPIV_APIC_ENABLED))
+		return false;
+
+	/*
+	 * Virtual wire mode via local APIC requests
+	 * APIC to enable the LINT0 for edge-trggered ExtINT delivery mode
+	 * and LINT1 for level-triggered NMI delivery mode
+	 */
+	value = apic_read(APIC_LVT0);
+	if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_EXTINT)
+		return false;
+
+	value = apic_read(APIC_LVT1);
+	if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_NMI)
+		return false;
+
+	return true;
+}
+
+static bool check_virtual_wire_mode(void)
+{
+	/* Neither of virtual wire mode is active, return false */
+	return  virtual_wire_via_lapic() || virtual_wire_via_ioapic();
+}
+
+/*
  * An initial setup of the virtual wire mode.
  */
 void __init init_bsp_APIC(void)
@@ -1133,8 +1180,14 @@ void __init init_bsp_APIC(void)
 	/*
 	 * Don't do the setup now if we have a SMP BIOS as the
 	 * through-I/O-APIC virtual wire mode might be active.
+	 *
+	 * It's better to do further check if either through-I/O-APIC
+	 * or through-local-APIC is active.
+	 * the worst case is that both of them are inactive,
+	 * If so, We need to enable the virtual wire mode via through-local-APIC
 	 */
-	if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC))
+	if ((smp_found_config && check_virtual_wire_mode()) ||
+		!boot_cpu_has(X86_FEATURE_APIC))
 		return;
 
 	/*
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 446702ed99dc..5a32c26938ac 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1379,6 +1379,34 @@ void __init print_IO_APICs(void)
 /* Where if anywhere is the i8259 connect in external int mode */
 static struct { int pin, apic; } ioapic_i8259 = { -1, -1 };
 
+/*
+ * Return false means the virtual wire mode via I/O APIC is inactive
+ */
+bool virtual_wire_via_ioapic(void)
+{
+	int apic, pin;
+
+	for_each_ioapic_pin(apic, pin) {
+		/* See if any of the pins is in ExtINT mode */
+		struct IO_APIC_route_entry entry = ioapic_read_entry(apic, pin);
+
+		/*
+		 * If the interrupt line is enabled and in ExtInt mode
+		 * I have found the pin where the i8259 is connected.
+		 */
+		if ((entry.mask == 0) && (entry.delivery_mode == dest_ExtINT))
+			return true;
+	}
+
+	/*
+	 * Virtual wire mode via I/O APIC requests
+	 * I/O APIC be connected to i8259 in chapter 3.6.2.2 of the MP v1.4 spec
+	 * If no pin in ExtInt mode,
+	 * the through-I/O-APIC virtual wire mode can be regarded inactive.
+	 */
+	return false;
+}
+
 void __init enable_IO_APIC(void)
 {
 	int i8259_apic, i8259_pin;
-- 
1.9.3

[PATCH 3/3] x86/apic: Improved the setting of interrupt mode for bsp

[Wei Jiangang]

If we specify the 'notsc' parameter for the dump-capture kernel,
and then trigger a crash(panic) by using "ALT-SysRq-c" or
"echo c > /proc/sysrq-trigger", the dump-capture kernel will
hang in calibrate_delay_converge() and wait for jiffies changes.
serial log as follows:

    tsc: Fast TSC calibration using PIT
    tsc: Detected 2099.947 MHz processor
    Calibrating delay loop...

The reason for jiffies not changes is there's no timer interrupt
passed to dump-capture kernel.

In fact, once kernel panic occurs, the local APIC is disabled
by lapic_shutdown() in reboot path.
generly speaking, local APIC state can be initialized by BIOS
after Power-Up or Reset, which doesn't apply to kdump case.
so the kernel has to be responsible for initialize the interrupt
mode properly according the latest status of APIC in bootup path.

An MP operating system is booted under either PIC mode or
virtual wire mode. Later, the operating system switches to
symmetric I/O mode as it enters multiprocessor mode.
Two kinds of virtual wire mode are defined in Intel MP spec:
virtual wire mode via local APIC or via I/O APIC.

Now we determine the mode of APIC only through a SMP BIOS(MP table).
That's not enough.
It's better to do further check if APIC works with effective mode,
and do some porper setting.

Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
---
 arch/x86/include/asm/io_apic.h |  5 ++++
 arch/x86/kernel/apic/apic.c    | 55 +++++++++++++++++++++++++++++++++++++++++-
 arch/x86/kernel/apic/io_apic.c | 28 +++++++++++++++++++++
 3 files changed, 87 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index 6cbf2cfb3f8a..6550bd43fa39 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -190,6 +190,7 @@ static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
 }
 
 extern void setup_IO_APIC(void);
+extern bool virtual_wire_via_ioapic(void);
 extern void enable_IO_APIC(void);
 extern void disable_IO_APIC(void);
 extern void setup_ioapic_dest(void);
@@ -231,6 +232,10 @@ static inline void io_apic_init_mappings(void) { }
 #define native_disable_io_apic		NULL
 
 static inline void setup_IO_APIC(void) { }
+static inline bool virtual_wire_via_ioapic(void)
+{
+	return true;
+}
 static inline void enable_IO_APIC(void) { }
 static inline void setup_ioapic_dest(void) { }
 
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 8e25b9b2d351..04358e0cf1e2 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1124,6 +1124,53 @@ void __init sync_Arb_IDs(void)
 }
 
 /*
+ * Return false means the virtual wire mode through-local-apic is inactive
+ */
+static bool virtual_wire_via_lapic(void)
+{
+	unsigned int value;
+
+	/* Check the APIC global enable/disable flag firstly */
+	if (boot_cpu_data.x86 >= 6) {
+		u32 h, l;
+
+		rdmsr(MSR_IA32_APICBASE, l, h);
+		/*
+		 * If local APIC is disabled by BIOS
+		 * do nothing, but return true
+		 */
+		if (!(l & MSR_IA32_APICBASE_ENABLE))
+			return true;
+	}
+
+	/* Check the software enable/disable flag */
+	value = apic_read(APIC_SPIV);
+	if (!(value & APIC_SPIV_APIC_ENABLED))
+		return false;
+
+	/*
+	 * Virtual wire mode via local APIC requests
+	 * APIC to enable the LINT0 for edge-trggered ExtINT delivery mode
+	 * and LINT1 for level-triggered NMI delivery mode
+	 */
+	value = apic_read(APIC_LVT0);
+	if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_EXTINT)
+		return false;
+
+	value = apic_read(APIC_LVT1);
+	if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_NMI)
+		return false;
+
+	return true;
+}
+
+static bool check_virtual_wire_mode(void)
+{
+	/* Neither of virtual wire mode is active, return false */
+	return  virtual_wire_via_lapic() || virtual_wire_via_ioapic();
+}
+
+/*
  * An initial setup of the virtual wire mode.
  */
 void __init init_bsp_APIC(void)
@@ -1133,8 +1180,14 @@ void __init init_bsp_APIC(void)
 	/*
 	 * Don't do the setup now if we have a SMP BIOS as the
 	 * through-I/O-APIC virtual wire mode might be active.
+	 *
+	 * It's better to do further check if either through-I/O-APIC
+	 * or through-local-APIC is active.
+	 * the worst case is that both of them are inactive,
+	 * If so, We need to enable the virtual wire mode via through-local-APIC
 	 */
-	if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC))
+	if ((smp_found_config && check_virtual_wire_mode()) ||
+		!boot_cpu_has(X86_FEATURE_APIC))
 		return;
 
 	/*
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 446702ed99dc..5a32c26938ac 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1379,6 +1379,34 @@ void __init print_IO_APICs(void)
 /* Where if anywhere is the i8259 connect in external int mode */
 static struct { int pin, apic; } ioapic_i8259 = { -1, -1 };
 
+/*
+ * Return false means the virtual wire mode via I/O APIC is inactive
+ */
+bool virtual_wire_via_ioapic(void)
+{
+	int apic, pin;
+
+	for_each_ioapic_pin(apic, pin) {
+		/* See if any of the pins is in ExtINT mode */
+		struct IO_APIC_route_entry entry = ioapic_read_entry(apic, pin);
+
+		/*
+		 * If the interrupt line is enabled and in ExtInt mode
+		 * I have found the pin where the i8259 is connected.
+		 */
+		if ((entry.mask == 0) && (entry.delivery_mode == dest_ExtINT))
+			return true;
+	}
+
+	/*
+	 * Virtual wire mode via I/O APIC requests
+	 * I/O APIC be connected to i8259 in chapter 3.6.2.2 of the MP v1.4 spec
+	 * If no pin in ExtInt mode,
+	 * the through-I/O-APIC virtual wire mode can be regarded inactive.
+	 */
+	return false;
+}
+
 void __init enable_IO_APIC(void)
 {
 	int i8259_apic, i8259_pin;
-- 
1.9.3




_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 3/3] x86/apic: Improved the setting of interrupt mode for bsp

[Baoquan He]

Hi Jiangang,

This is very nice, should be the stuff Eric and Ingo would like to see.
But I have several questions:

1) Are you not going to clean up the old legacy irq mode setting code in
1st kernel?

2)I call them legacy irq mode because not only apic virtual wire mode
exists, but the PIC mode in x86 32bit system. You need consider it too.
Then init_bsp_APIC need be renamaed to an appropriate one. And assume
this has been tested on x86 32bit system. 

3) 

*)About IO-APIC setting as virtual wire mode, I am always confused. In
MP Spec 3.6.2.2, it says "the interrupt signal passes through both the
I/O APIC and the BSP’s local APIC". That means IO-APIC virtual wire mode
need both IO-APIC and LAPIC to be set, and with my understanding only
pin of IO-APIC is set as ExtInt, LAPIC should be pass-through.

*)But in Intel Arch manual 3A 10.1, there's only below sentences to mention
it:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The I/O APIC also has a “virtual wire mode” that allows it to communicate
with a standard 8259A-style external interrupt controller. Note that the
local APIC can be disabled. This allows an associated processor core to
receive interrupts directly from an 8259A interrupt controller.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Eric's code in native_disable_io_apic() has the same point as above
words.

*)However please read code comments in irq_remapping_disable_io_apic(),
Joerg's description give me a different impression that we can choose
to only use LAPIC virtual wire mode. Joerg is IOMMU maintainers, he is
very familiar with io-apic since IOMMU need take over io-apic entry
filling, there must be reason he wrote that. Add Joerg to CC list.

Seems it's difficult to find a system with IO-APIC virtual wire mode,
maybe we can just keep it as is. Not sure if Intel engineers can help
explain and confirm this.

That's all I can think of.

Thanks
Baoquan

On 07/22/16 at 04:10pm, Wei Jiangang wrote:
> If we specify the 'notsc' parameter for the dump-capture kernel,
> and then trigger a crash(panic) by using "ALT-SysRq-c" or
> "echo c > /proc/sysrq-trigger", the dump-capture kernel will
> hang in calibrate_delay_converge() and wait for jiffies changes.
> serial log as follows:
> 
>     tsc: Fast TSC calibration using PIT
>     tsc: Detected 2099.947 MHz processor
>     Calibrating delay loop...
> 
> The reason for jiffies not changes is there's no timer interrupt
> passed to dump-capture kernel.
> 
> In fact, once kernel panic occurs, the local APIC is disabled
> by lapic_shutdown() in reboot path.
> generly speaking, local APIC state can be initialized by BIOS
> after Power-Up or Reset, which doesn't apply to kdump case.
> so the kernel has to be responsible for initialize the interrupt
> mode properly according the latest status of APIC in bootup path.
> 
> An MP operating system is booted under either PIC mode or
> virtual wire mode. Later, the operating system switches to
> symmetric I/O mode as it enters multiprocessor mode.
> Two kinds of virtual wire mode are defined in Intel MP spec:
> virtual wire mode via local APIC or via I/O APIC.
> 
> Now we determine the mode of APIC only through a SMP BIOS(MP table).
> That's not enough.
> It's better to do further check if APIC works with effective mode,
> and do some porper setting.
> 
> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
> Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
> ---
>  arch/x86/include/asm/io_apic.h |  5 ++++
>  arch/x86/kernel/apic/apic.c    | 55 +++++++++++++++++++++++++++++++++++++++++-
>  arch/x86/kernel/apic/io_apic.c | 28 +++++++++++++++++++++
>  3 files changed, 87 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
> index 6cbf2cfb3f8a..6550bd43fa39 100644
> --- a/arch/x86/include/asm/io_apic.h
> +++ b/arch/x86/include/asm/io_apic.h
> @@ -190,6 +190,7 @@ static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
>  }
>  
>  extern void setup_IO_APIC(void);
> +extern bool virtual_wire_via_ioapic(void);
>  extern void enable_IO_APIC(void);
>  extern void disable_IO_APIC(void);
>  extern void setup_ioapic_dest(void);
> @@ -231,6 +232,10 @@ static inline void io_apic_init_mappings(void) { }
>  #define native_disable_io_apic		NULL
>  
>  static inline void setup_IO_APIC(void) { }
> +static inline bool virtual_wire_via_ioapic(void)
> +{
> +	return true;
> +}
>  static inline void enable_IO_APIC(void) { }
>  static inline void setup_ioapic_dest(void) { }
>  
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index 8e25b9b2d351..04358e0cf1e2 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -1124,6 +1124,53 @@ void __init sync_Arb_IDs(void)
>  }
>  
>  /*
> + * Return false means the virtual wire mode through-local-apic is inactive
> + */
> +static bool virtual_wire_via_lapic(void)
> +{
> +	unsigned int value;
> +
> +	/* Check the APIC global enable/disable flag firstly */
> +	if (boot_cpu_data.x86 >= 6) {
> +		u32 h, l;
> +
> +		rdmsr(MSR_IA32_APICBASE, l, h);
> +		/*
> +		 * If local APIC is disabled by BIOS
> +		 * do nothing, but return true
> +		 */
> +		if (!(l & MSR_IA32_APICBASE_ENABLE))
> +			return true;
> +	}
> +
> +	/* Check the software enable/disable flag */
> +	value = apic_read(APIC_SPIV);
> +	if (!(value & APIC_SPIV_APIC_ENABLED))
> +		return false;
> +
> +	/*
> +	 * Virtual wire mode via local APIC requests
> +	 * APIC to enable the LINT0 for edge-trggered ExtINT delivery mode
> +	 * and LINT1 for level-triggered NMI delivery mode
> +	 */
> +	value = apic_read(APIC_LVT0);
> +	if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_EXTINT)
> +		return false;
> +
> +	value = apic_read(APIC_LVT1);
> +	if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_NMI)
> +		return false;
> +
> +	return true;
> +}
> +
> +static bool check_virtual_wire_mode(void)
> +{
> +	/* Neither of virtual wire mode is active, return false */
> +	return  virtual_wire_via_lapic() || virtual_wire_via_ioapic();
> +}
> +
> +/*
>   * An initial setup of the virtual wire mode.
>   */
>  void __init init_bsp_APIC(void)
> @@ -1133,8 +1180,14 @@ void __init init_bsp_APIC(void)
>  	/*
>  	 * Don't do the setup now if we have a SMP BIOS as the
>  	 * through-I/O-APIC virtual wire mode might be active.
> +	 *
> +	 * It's better to do further check if either through-I/O-APIC
> +	 * or through-local-APIC is active.
> +	 * the worst case is that both of them are inactive,
> +	 * If so, We need to enable the virtual wire mode via through-local-APIC
>  	 */
> -	if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC))
> +	if ((smp_found_config && check_virtual_wire_mode()) ||
> +		!boot_cpu_has(X86_FEATURE_APIC))
>  		return;
>  
>  	/*
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 446702ed99dc..5a32c26938ac 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -1379,6 +1379,34 @@ void __init print_IO_APICs(void)
>  /* Where if anywhere is the i8259 connect in external int mode */
>  static struct { int pin, apic; } ioapic_i8259 = { -1, -1 };
>  
> +/*
> + * Return false means the virtual wire mode via I/O APIC is inactive
> + */
> +bool virtual_wire_via_ioapic(void)
> +{
> +	int apic, pin;
> +
> +	for_each_ioapic_pin(apic, pin) {
> +		/* See if any of the pins is in ExtINT mode */
> +		struct IO_APIC_route_entry entry = ioapic_read_entry(apic, pin);
> +
> +		/*
> +		 * If the interrupt line is enabled and in ExtInt mode
> +		 * I have found the pin where the i8259 is connected.
> +		 */
> +		if ((entry.mask == 0) && (entry.delivery_mode == dest_ExtINT))
> +			return true;
> +	}
> +
> +	/*
> +	 * Virtual wire mode via I/O APIC requests
> +	 * I/O APIC be connected to i8259 in chapter 3.6.2.2 of the MP v1.4 spec
> +	 * If no pin in ExtInt mode,
> +	 * the through-I/O-APIC virtual wire mode can be regarded inactive.
> +	 */
> +	return false;
> +}
> +
>  void __init enable_IO_APIC(void)
>  {
>  	int i8259_apic, i8259_pin;
> -- 
> 1.9.3
> 
> 
> 

Re: [PATCH 3/3] x86/apic: Improved the setting of interrupt mode for bsp

[Baoquan He]

Hi Jiangang,

This is very nice, should be the stuff Eric and Ingo would like to see.
But I have several questions:

1) Are you not going to clean up the old legacy irq mode setting code in
1st kernel?

2)I call them legacy irq mode because not only apic virtual wire mode
exists, but the PIC mode in x86 32bit system. You need consider it too.
Then init_bsp_APIC need be renamaed to an appropriate one. And assume
this has been tested on x86 32bit system. 

3) 

*)About IO-APIC setting as virtual wire mode, I am always confused. In
MP Spec 3.6.2.2, it says "the interrupt signal passes through both the
I/O APIC and the BSP’s local APIC". That means IO-APIC virtual wire mode
need both IO-APIC and LAPIC to be set, and with my understanding only
pin of IO-APIC is set as ExtInt, LAPIC should be pass-through.

*)But in Intel Arch manual 3A 10.1, there's only below sentences to mention
it:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The I/O APIC also has a “virtual wire mode” that allows it to communicate
with a standard 8259A-style external interrupt controller. Note that the
local APIC can be disabled. This allows an associated processor core to
receive interrupts directly from an 8259A interrupt controller.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Eric's code in native_disable_io_apic() has the same point as above
words.

*)However please read code comments in irq_remapping_disable_io_apic(),
Joerg's description give me a different impression that we can choose
to only use LAPIC virtual wire mode. Joerg is IOMMU maintainers, he is
very familiar with io-apic since IOMMU need take over io-apic entry
filling, there must be reason he wrote that. Add Joerg to CC list.

Seems it's difficult to find a system with IO-APIC virtual wire mode,
maybe we can just keep it as is. Not sure if Intel engineers can help
explain and confirm this.

That's all I can think of.

Thanks
Baoquan

On 07/22/16 at 04:10pm, Wei Jiangang wrote:
> If we specify the 'notsc' parameter for the dump-capture kernel,
> and then trigger a crash(panic) by using "ALT-SysRq-c" or
> "echo c > /proc/sysrq-trigger", the dump-capture kernel will
> hang in calibrate_delay_converge() and wait for jiffies changes.
> serial log as follows:
> 
>     tsc: Fast TSC calibration using PIT
>     tsc: Detected 2099.947 MHz processor
>     Calibrating delay loop...
> 
> The reason for jiffies not changes is there's no timer interrupt
> passed to dump-capture kernel.
> 
> In fact, once kernel panic occurs, the local APIC is disabled
> by lapic_shutdown() in reboot path.
> generly speaking, local APIC state can be initialized by BIOS
> after Power-Up or Reset, which doesn't apply to kdump case.
> so the kernel has to be responsible for initialize the interrupt
> mode properly according the latest status of APIC in bootup path.
> 
> An MP operating system is booted under either PIC mode or
> virtual wire mode. Later, the operating system switches to
> symmetric I/O mode as it enters multiprocessor mode.
> Two kinds of virtual wire mode are defined in Intel MP spec:
> virtual wire mode via local APIC or via I/O APIC.
> 
> Now we determine the mode of APIC only through a SMP BIOS(MP table).
> That's not enough.
> It's better to do further check if APIC works with effective mode,
> and do some porper setting.
> 
> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
> Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
> ---
>  arch/x86/include/asm/io_apic.h |  5 ++++
>  arch/x86/kernel/apic/apic.c    | 55 +++++++++++++++++++++++++++++++++++++++++-
>  arch/x86/kernel/apic/io_apic.c | 28 +++++++++++++++++++++
>  3 files changed, 87 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
> index 6cbf2cfb3f8a..6550bd43fa39 100644
> --- a/arch/x86/include/asm/io_apic.h
> +++ b/arch/x86/include/asm/io_apic.h
> @@ -190,6 +190,7 @@ static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
>  }
>  
>  extern void setup_IO_APIC(void);
> +extern bool virtual_wire_via_ioapic(void);
>  extern void enable_IO_APIC(void);
>  extern void disable_IO_APIC(void);
>  extern void setup_ioapic_dest(void);
> @@ -231,6 +232,10 @@ static inline void io_apic_init_mappings(void) { }
>  #define native_disable_io_apic		NULL
>  
>  static inline void setup_IO_APIC(void) { }
> +static inline bool virtual_wire_via_ioapic(void)
> +{
> +	return true;
> +}
>  static inline void enable_IO_APIC(void) { }
>  static inline void setup_ioapic_dest(void) { }
>  
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index 8e25b9b2d351..04358e0cf1e2 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -1124,6 +1124,53 @@ void __init sync_Arb_IDs(void)
>  }
>  
>  /*
> + * Return false means the virtual wire mode through-local-apic is inactive
> + */
> +static bool virtual_wire_via_lapic(void)
> +{
> +	unsigned int value;
> +
> +	/* Check the APIC global enable/disable flag firstly */
> +	if (boot_cpu_data.x86 >= 6) {
> +		u32 h, l;
> +
> +		rdmsr(MSR_IA32_APICBASE, l, h);
> +		/*
> +		 * If local APIC is disabled by BIOS
> +		 * do nothing, but return true
> +		 */
> +		if (!(l & MSR_IA32_APICBASE_ENABLE))
> +			return true;
> +	}
> +
> +	/* Check the software enable/disable flag */
> +	value = apic_read(APIC_SPIV);
> +	if (!(value & APIC_SPIV_APIC_ENABLED))
> +		return false;
> +
> +	/*
> +	 * Virtual wire mode via local APIC requests
> +	 * APIC to enable the LINT0 for edge-trggered ExtINT delivery mode
> +	 * and LINT1 for level-triggered NMI delivery mode
> +	 */
> +	value = apic_read(APIC_LVT0);
> +	if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_EXTINT)
> +		return false;
> +
> +	value = apic_read(APIC_LVT1);
> +	if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_NMI)
> +		return false;
> +
> +	return true;
> +}
> +
> +static bool check_virtual_wire_mode(void)
> +{
> +	/* Neither of virtual wire mode is active, return false */
> +	return  virtual_wire_via_lapic() || virtual_wire_via_ioapic();
> +}
> +
> +/*
>   * An initial setup of the virtual wire mode.
>   */
>  void __init init_bsp_APIC(void)
> @@ -1133,8 +1180,14 @@ void __init init_bsp_APIC(void)
>  	/*
>  	 * Don't do the setup now if we have a SMP BIOS as the
>  	 * through-I/O-APIC virtual wire mode might be active.
> +	 *
> +	 * It's better to do further check if either through-I/O-APIC
> +	 * or through-local-APIC is active.
> +	 * the worst case is that both of them are inactive,
> +	 * If so, We need to enable the virtual wire mode via through-local-APIC
>  	 */
> -	if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC))
> +	if ((smp_found_config && check_virtual_wire_mode()) ||
> +		!boot_cpu_has(X86_FEATURE_APIC))
>  		return;
>  
>  	/*
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 446702ed99dc..5a32c26938ac 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -1379,6 +1379,34 @@ void __init print_IO_APICs(void)
>  /* Where if anywhere is the i8259 connect in external int mode */
>  static struct { int pin, apic; } ioapic_i8259 = { -1, -1 };
>  
> +/*
> + * Return false means the virtual wire mode via I/O APIC is inactive
> + */
> +bool virtual_wire_via_ioapic(void)
> +{
> +	int apic, pin;
> +
> +	for_each_ioapic_pin(apic, pin) {
> +		/* See if any of the pins is in ExtINT mode */
> +		struct IO_APIC_route_entry entry = ioapic_read_entry(apic, pin);
> +
> +		/*
> +		 * If the interrupt line is enabled and in ExtInt mode
> +		 * I have found the pin where the i8259 is connected.
> +		 */
> +		if ((entry.mask == 0) && (entry.delivery_mode == dest_ExtINT))
> +			return true;
> +	}
> +
> +	/*
> +	 * Virtual wire mode via I/O APIC requests
> +	 * I/O APIC be connected to i8259 in chapter 3.6.2.2 of the MP v1.4 spec
> +	 * If no pin in ExtInt mode,
> +	 * the through-I/O-APIC virtual wire mode can be regarded inactive.
> +	 */
> +	return false;
> +}
> +
>  void __init enable_IO_APIC(void)
>  {
>  	int i8259_apic, i8259_pin;
> -- 
> 1.9.3
> 
> 
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 3/3] x86/apic: Improved the setting of interrupt mode for bsp

[Wei, Jiangang]

Hi He,

Thanks for your response firstly.

On Fri, 2016-07-22 at 18:40 +0800, Baoquan He wrote:
> Hi Jiangang,
> 
> This is very nice, should be the stuff Eric and Ingo would like to see.
> But I have several questions:
> 
> 1) Are you not going to clean up the old legacy irq mode setting code in
> 1st kernel?
Yes, I would like to pay more attention on fixing kdump's failure with
notsc.
No plan to clean up the irq mode setting codes in the crash kernel
reboot path.
If you are interested in it, please go on.

> 
> 2)I call them legacy irq mode because not only apic virtual wire mode
> exists, but the PIC mode in x86 32bit system. You need consider it too.
> Then init_bsp_APIC need be renamaed to an appropriate one. And assume
> this has been tested on x86 32bit system. 

Thanks for your reminders.
As the comment of init_bsp_APIC(), it's just used to setup the virtual
wire mode.
so I won't change its name.

But i agree with that PIC mode should be considered.
Next version will be like below,

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 04358e0cf1e2..d40bab947a2a 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1186,7 +1186,7 @@ void __init init_bsp_APIC(void)
         * the worst case is that both of them are inactive,
         * If so, We need to enable the virtual wire mode via
through-local-APIC
         */
-       if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC)
+       if ( pic_mode || (smp_found_config && check_virtual_wire_mode())
||
                !boot_cpu_has(X86_FEATURE_APIC))
                return;
> 
> 3) 
> 
> *)About IO-APIC setting as virtual wire mode, I am always confused. In
> MP Spec 3.6.2.2, it says "the interrupt signal passes through both the
> I/O APIC and the BSP’s local APIC". That means IO-APIC virtual wire mode
> need both IO-APIC and LAPIC to be set, and with my understanding only
> pin of IO-APIC is set as ExtInt, LAPIC should be pass-through.
> 
> *)But in Intel Arch manual 3A 10.1, there's only below sentences to mention
> it:
> 
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> The I/O APIC also has a “virtual wire mode” that allows it to communicate
> with a standard 8259A-style external interrupt controller. Note that the
> local APIC can be disabled. This allows an associated processor core to
> receive interrupts directly from an 8259A interrupt controller.
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Eric's code in native_disable_io_apic() has the same point as above
> words.

IMO，
the through-IO-APIC mode has no relationship with the setting of local
APIC.
that's why i only check the pin of IO-APIC in virtual_wire_via_ioapic().

Thanks
wei
> 
> *)However please read code comments in irq_remapping_disable_io_apic(),
> Joerg's description give me a different impression that we can choose
> to only use LAPIC virtual wire mode. Joerg is IOMMU maintainers, he is
> very familiar with io-apic since IOMMU need take over io-apic entry
> filling, there must be reason he wrote that. Add Joerg to CC list.
> 
> Seems it's difficult to find a system with IO-APIC virtual wire mode,
> maybe we can just keep it as is. Not sure if Intel engineers can help
> explain and confirm this.
> 
> That's all I can think of.
> 
> Thanks
> Baoquan
> 
> On 07/22/16 at 04:10pm, Wei Jiangang wrote:
> > If we specify the 'notsc' parameter for the dump-capture kernel,
> > and then trigger a crash(panic) by using "ALT-SysRq-c" or
> > "echo c > /proc/sysrq-trigger", the dump-capture kernel will
> > hang in calibrate_delay_converge() and wait for jiffies changes.
> > serial log as follows:
> > 
> >     tsc: Fast TSC calibration using PIT
> >     tsc: Detected 2099.947 MHz processor
> >     Calibrating delay loop...
> > 
> > The reason for jiffies not changes is there's no timer interrupt
> > passed to dump-capture kernel.
> > 
> > In fact, once kernel panic occurs, the local APIC is disabled
> > by lapic_shutdown() in reboot path.
> > generly speaking, local APIC state can be initialized by BIOS
> > after Power-Up or Reset, which doesn't apply to kdump case.
> > so the kernel has to be responsible for initialize the interrupt
> > mode properly according the latest status of APIC in bootup path.
> > 
> > An MP operating system is booted under either PIC mode or
> > virtual wire mode. Later, the operating system switches to
> > symmetric I/O mode as it enters multiprocessor mode.
> > Two kinds of virtual wire mode are defined in Intel MP spec:
> > virtual wire mode via local APIC or via I/O APIC.
> > 
> > Now we determine the mode of APIC only through a SMP BIOS(MP table).
> > That's not enough.
> > It's better to do further check if APIC works with effective mode,
> > and do some porper setting.
> > 
> > Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
> > Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
> > ---
> >  arch/x86/include/asm/io_apic.h |  5 ++++
> >  arch/x86/kernel/apic/apic.c    | 55 +++++++++++++++++++++++++++++++++++++++++-
> >  arch/x86/kernel/apic/io_apic.c | 28 +++++++++++++++++++++
> >  3 files changed, 87 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
> > index 6cbf2cfb3f8a..6550bd43fa39 100644
> > --- a/arch/x86/include/asm/io_apic.h
> > +++ b/arch/x86/include/asm/io_apic.h
> > @@ -190,6 +190,7 @@ static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
> >  }
> >  
> >  extern void setup_IO_APIC(void);
> > +extern bool virtual_wire_via_ioapic(void);
> >  extern void enable_IO_APIC(void);
> >  extern void disable_IO_APIC(void);
> >  extern void setup_ioapic_dest(void);
> > @@ -231,6 +232,10 @@ static inline void io_apic_init_mappings(void) { }
> >  #define native_disable_io_apic		NULL
> >  
> >  static inline void setup_IO_APIC(void) { }
> > +static inline bool virtual_wire_via_ioapic(void)
> > +{
> > +	return true;
> > +}
> >  static inline void enable_IO_APIC(void) { }
> >  static inline void setup_ioapic_dest(void) { }
> >  
> > diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> > index 8e25b9b2d351..04358e0cf1e2 100644
> > --- a/arch/x86/kernel/apic/apic.c
> > +++ b/arch/x86/kernel/apic/apic.c
> > @@ -1124,6 +1124,53 @@ void __init sync_Arb_IDs(void)
> >  }
> >  
> >  /*
> > + * Return false means the virtual wire mode through-local-apic is inactive
> > + */
> > +static bool virtual_wire_via_lapic(void)
> > +{
> > +	unsigned int value;
> > +
> > +	/* Check the APIC global enable/disable flag firstly */
> > +	if (boot_cpu_data.x86 >= 6) {
> > +		u32 h, l;
> > +
> > +		rdmsr(MSR_IA32_APICBASE, l, h);
> > +		/*
> > +		 * If local APIC is disabled by BIOS
> > +		 * do nothing, but return true
> > +		 */
> > +		if (!(l & MSR_IA32_APICBASE_ENABLE))
> > +			return true;
> > +	}
> > +
> > +	/* Check the software enable/disable flag */
> > +	value = apic_read(APIC_SPIV);
> > +	if (!(value & APIC_SPIV_APIC_ENABLED))
> > +		return false;
> > +
> > +	/*
> > +	 * Virtual wire mode via local APIC requests
> > +	 * APIC to enable the LINT0 for edge-trggered ExtINT delivery mode
> > +	 * and LINT1 for level-triggered NMI delivery mode
> > +	 */
> > +	value = apic_read(APIC_LVT0);
> > +	if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_EXTINT)
> > +		return false;
> > +
> > +	value = apic_read(APIC_LVT1);
> > +	if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_NMI)
> > +		return false;
> > +
> > +	return true;
> > +}
> > +
> > +static bool check_virtual_wire_mode(void)
> > +{
> > +	/* Neither of virtual wire mode is active, return false */
> > +	return  virtual_wire_via_lapic() || virtual_wire_via_ioapic();
> > +}
> > +
> > +/*
> >   * An initial setup of the virtual wire mode.
> >   */
> >  void __init init_bsp_APIC(void)
> > @@ -1133,8 +1180,14 @@ void __init init_bsp_APIC(void)
> >  	/*
> >  	 * Don't do the setup now if we have a SMP BIOS as the
> >  	 * through-I/O-APIC virtual wire mode might be active.
> > +	 *
> > +	 * It's better to do further check if either through-I/O-APIC
> > +	 * or through-local-APIC is active.
> > +	 * the worst case is that both of them are inactive,
> > +	 * If so, We need to enable the virtual wire mode via through-local-APIC
> >  	 */
> > -	if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC))
> > +	if ((smp_found_config && check_virtual_wire_mode()) ||
> > +		!boot_cpu_has(X86_FEATURE_APIC))
> >  		return;
> >  
> >  	/*
> > diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> > index 446702ed99dc..5a32c26938ac 100644
> > --- a/arch/x86/kernel/apic/io_apic.c
> > +++ b/arch/x86/kernel/apic/io_apic.c
> > @@ -1379,6 +1379,34 @@ void __init print_IO_APICs(void)
> >  /* Where if anywhere is the i8259 connect in external int mode */
> >  static struct { int pin, apic; } ioapic_i8259 = { -1, -1 };
> >  
> > +/*
> > + * Return false means the virtual wire mode via I/O APIC is inactive
> > + */
> > +bool virtual_wire_via_ioapic(void)
> > +{
> > +	int apic, pin;
> > +
> > +	for_each_ioapic_pin(apic, pin) {
> > +		/* See if any of the pins is in ExtINT mode */
> > +		struct IO_APIC_route_entry entry = ioapic_read_entry(apic, pin);
> > +
> > +		/*
> > +		 * If the interrupt line is enabled and in ExtInt mode
> > +		 * I have found the pin where the i8259 is connected.
> > +		 */
> > +		if ((entry.mask == 0) && (entry.delivery_mode == dest_ExtINT))
> > +			return true;
> > +	}
> > +
> > +	/*
> > +	 * Virtual wire mode via I/O APIC requests
> > +	 * I/O APIC be connected to i8259 in chapter 3.6.2.2 of the MP v1.4 spec
> > +	 * If no pin in ExtInt mode,
> > +	 * the through-I/O-APIC virtual wire mode can be regarded inactive.
> > +	 */
> > +	return false;
> > +}
> > +
> >  void __init enable_IO_APIC(void)
> >  {
> >  	int i8259_apic, i8259_pin;
> > -- 
> > 1.9.3
> > 
> > 
> > 
> 
> 

Re: [PATCH 3/3] x86/apic: Improved the setting of interrupt mode for bsp

[Wei, Jiangang]

Hi He,

Thanks for your response firstly.

On Fri, 2016-07-22 at 18:40 +0800, Baoquan He wrote:
> Hi Jiangang,
> 
> This is very nice, should be the stuff Eric and Ingo would like to see.
> But I have several questions:
> 
> 1) Are you not going to clean up the old legacy irq mode setting code in
> 1st kernel?
Yes, I would like to pay more attention on fixing kdump's failure with
notsc.
No plan to clean up the irq mode setting codes in the crash kernel
reboot path.
If you are interested in it, please go on.

> 
> 2)I call them legacy irq mode because not only apic virtual wire mode
> exists, but the PIC mode in x86 32bit system. You need consider it too.
> Then init_bsp_APIC need be renamaed to an appropriate one. And assume
> this has been tested on x86 32bit system. 

Thanks for your reminders.
As the comment of init_bsp_APIC(), it's just used to setup the virtual
wire mode.
so I won't change its name.

But i agree with that PIC mode should be considered.
Next version will be like below,

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 04358e0cf1e2..d40bab947a2a 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1186,7 +1186,7 @@ void __init init_bsp_APIC(void)
         * the worst case is that both of them are inactive,
         * If so, We need to enable the virtual wire mode via
through-local-APIC
         */
-       if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC)
+       if ( pic_mode || (smp_found_config && check_virtual_wire_mode())
||
                !boot_cpu_has(X86_FEATURE_APIC))
                return;
> 
> 3) 
> 
> *)About IO-APIC setting as virtual wire mode, I am always confused. In
> MP Spec 3.6.2.2, it says "the interrupt signal passes through both the
> I/O APIC and the BSP’s local APIC". That means IO-APIC virtual wire mode
> need both IO-APIC and LAPIC to be set, and with my understanding only
> pin of IO-APIC is set as ExtInt, LAPIC should be pass-through.
> 
> *)But in Intel Arch manual 3A 10.1, there's only below sentences to mention
> it:
> 
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> The I/O APIC also has a “virtual wire mode” that allows it to communicate
> with a standard 8259A-style external interrupt controller. Note that the
> local APIC can be disabled. This allows an associated processor core to
> receive interrupts directly from an 8259A interrupt controller.
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Eric's code in native_disable_io_apic() has the same point as above
> words.

IMO，
the through-IO-APIC mode has no relationship with the setting of local
APIC.
that's why i only check the pin of IO-APIC in virtual_wire_via_ioapic().

Thanks
wei
> 
> *)However please read code comments in irq_remapping_disable_io_apic(),
> Joerg's description give me a different impression that we can choose
> to only use LAPIC virtual wire mode. Joerg is IOMMU maintainers, he is
> very familiar with io-apic since IOMMU need take over io-apic entry
> filling, there must be reason he wrote that. Add Joerg to CC list.
> 
> Seems it's difficult to find a system with IO-APIC virtual wire mode,
> maybe we can just keep it as is. Not sure if Intel engineers can help
> explain and confirm this.
> 
> That's all I can think of.
> 
> Thanks
> Baoquan
> 
> On 07/22/16 at 04:10pm, Wei Jiangang wrote:
> > If we specify the 'notsc' parameter for the dump-capture kernel,
> > and then trigger a crash(panic) by using "ALT-SysRq-c" or
> > "echo c > /proc/sysrq-trigger", the dump-capture kernel will
> > hang in calibrate_delay_converge() and wait for jiffies changes.
> > serial log as follows:
> > 
> >     tsc: Fast TSC calibration using PIT
> >     tsc: Detected 2099.947 MHz processor
> >     Calibrating delay loop...
> > 
> > The reason for jiffies not changes is there's no timer interrupt
> > passed to dump-capture kernel.
> > 
> > In fact, once kernel panic occurs, the local APIC is disabled
> > by lapic_shutdown() in reboot path.
> > generly speaking, local APIC state can be initialized by BIOS
> > after Power-Up or Reset, which doesn't apply to kdump case.
> > so the kernel has to be responsible for initialize the interrupt
> > mode properly according the latest status of APIC in bootup path.
> > 
> > An MP operating system is booted under either PIC mode or
> > virtual wire mode. Later, the operating system switches to
> > symmetric I/O mode as it enters multiprocessor mode.
> > Two kinds of virtual wire mode are defined in Intel MP spec:
> > virtual wire mode via local APIC or via I/O APIC.
> > 
> > Now we determine the mode of APIC only through a SMP BIOS(MP table).
> > That's not enough.
> > It's better to do further check if APIC works with effective mode,
> > and do some porper setting.
> > 
> > Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
> > Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
> > ---
> >  arch/x86/include/asm/io_apic.h |  5 ++++
> >  arch/x86/kernel/apic/apic.c    | 55 +++++++++++++++++++++++++++++++++++++++++-
> >  arch/x86/kernel/apic/io_apic.c | 28 +++++++++++++++++++++
> >  3 files changed, 87 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
> > index 6cbf2cfb3f8a..6550bd43fa39 100644
> > --- a/arch/x86/include/asm/io_apic.h
> > +++ b/arch/x86/include/asm/io_apic.h
> > @@ -190,6 +190,7 @@ static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
> >  }
> >  
> >  extern void setup_IO_APIC(void);
> > +extern bool virtual_wire_via_ioapic(void);
> >  extern void enable_IO_APIC(void);
> >  extern void disable_IO_APIC(void);
> >  extern void setup_ioapic_dest(void);
> > @@ -231,6 +232,10 @@ static inline void io_apic_init_mappings(void) { }
> >  #define native_disable_io_apic		NULL
> >  
> >  static inline void setup_IO_APIC(void) { }
> > +static inline bool virtual_wire_via_ioapic(void)
> > +{
> > +	return true;
> > +}
> >  static inline void enable_IO_APIC(void) { }
> >  static inline void setup_ioapic_dest(void) { }
> >  
> > diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> > index 8e25b9b2d351..04358e0cf1e2 100644
> > --- a/arch/x86/kernel/apic/apic.c
> > +++ b/arch/x86/kernel/apic/apic.c
> > @@ -1124,6 +1124,53 @@ void __init sync_Arb_IDs(void)
> >  }
> >  
> >  /*
> > + * Return false means the virtual wire mode through-local-apic is inactive
> > + */
> > +static bool virtual_wire_via_lapic(void)
> > +{
> > +	unsigned int value;
> > +
> > +	/* Check the APIC global enable/disable flag firstly */
> > +	if (boot_cpu_data.x86 >= 6) {
> > +		u32 h, l;
> > +
> > +		rdmsr(MSR_IA32_APICBASE, l, h);
> > +		/*
> > +		 * If local APIC is disabled by BIOS
> > +		 * do nothing, but return true
> > +		 */
> > +		if (!(l & MSR_IA32_APICBASE_ENABLE))
> > +			return true;
> > +	}
> > +
> > +	/* Check the software enable/disable flag */
> > +	value = apic_read(APIC_SPIV);
> > +	if (!(value & APIC_SPIV_APIC_ENABLED))
> > +		return false;
> > +
> > +	/*
> > +	 * Virtual wire mode via local APIC requests
> > +	 * APIC to enable the LINT0 for edge-trggered ExtINT delivery mode
> > +	 * and LINT1 for level-triggered NMI delivery mode
> > +	 */
> > +	value = apic_read(APIC_LVT0);
> > +	if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_EXTINT)
> > +		return false;
> > +
> > +	value = apic_read(APIC_LVT1);
> > +	if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_NMI)
> > +		return false;
> > +
> > +	return true;
> > +}
> > +
> > +static bool check_virtual_wire_mode(void)
> > +{
> > +	/* Neither of virtual wire mode is active, return false */
> > +	return  virtual_wire_via_lapic() || virtual_wire_via_ioapic();
> > +}
> > +
> > +/*
> >   * An initial setup of the virtual wire mode.
> >   */
> >  void __init init_bsp_APIC(void)
> > @@ -1133,8 +1180,14 @@ void __init init_bsp_APIC(void)
> >  	/*
> >  	 * Don't do the setup now if we have a SMP BIOS as the
> >  	 * through-I/O-APIC virtual wire mode might be active.
> > +	 *
> > +	 * It's better to do further check if either through-I/O-APIC
> > +	 * or through-local-APIC is active.
> > +	 * the worst case is that both of them are inactive,
> > +	 * If so, We need to enable the virtual wire mode via through-local-APIC
> >  	 */
> > -	if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC))
> > +	if ((smp_found_config && check_virtual_wire_mode()) ||
> > +		!boot_cpu_has(X86_FEATURE_APIC))
> >  		return;
> >  
> >  	/*
> > diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> > index 446702ed99dc..5a32c26938ac 100644
> > --- a/arch/x86/kernel/apic/io_apic.c
> > +++ b/arch/x86/kernel/apic/io_apic.c
> > @@ -1379,6 +1379,34 @@ void __init print_IO_APICs(void)
> >  /* Where if anywhere is the i8259 connect in external int mode */
> >  static struct { int pin, apic; } ioapic_i8259 = { -1, -1 };
> >  
> > +/*
> > + * Return false means the virtual wire mode via I/O APIC is inactive
> > + */
> > +bool virtual_wire_via_ioapic(void)
> > +{
> > +	int apic, pin;
> > +
> > +	for_each_ioapic_pin(apic, pin) {
> > +		/* See if any of the pins is in ExtINT mode */
> > +		struct IO_APIC_route_entry entry = ioapic_read_entry(apic, pin);
> > +
> > +		/*
> > +		 * If the interrupt line is enabled and in ExtInt mode
> > +		 * I have found the pin where the i8259 is connected.
> > +		 */
> > +		if ((entry.mask == 0) && (entry.delivery_mode == dest_ExtINT))
> > +			return true;
> > +	}
> > +
> > +	/*
> > +	 * Virtual wire mode via I/O APIC requests
> > +	 * I/O APIC be connected to i8259 in chapter 3.6.2.2 of the MP v1.4 spec
> > +	 * If no pin in ExtInt mode,
> > +	 * the through-I/O-APIC virtual wire mode can be regarded inactive.
> > +	 */
> > +	return false;
> > +}
> > +
> >  void __init enable_IO_APIC(void)
> >  {
> >  	int i8259_apic, i8259_pin;
> > -- 
> > 1.9.3
> > 
> > 
> > 
> 
> 



_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 3/3] x86/apic: Improved the setting of interrupt mode for bsp

[Xunlei Pang]

On 2016/07/25 at 11:04, Wei, Jiangang wrote:
> Hi He,
>
> Thanks for your response firstly.
>
> On Fri, 2016-07-22 at 18:40 +0800, Baoquan He wrote:
>> Hi Jiangang,
>>
>> This is very nice, should be the stuff Eric and Ingo would like to see.
>> But I have several questions:
>>
>> 1) Are you not going to clean up the old legacy irq mode setting code in
>> 1st kernel?
> Yes, I would like to pay more attention on fixing kdump's failure with
> notsc.
> No plan to clean up the irq mode setting codes in the crash kernel
> reboot path.
> If you are interested in it, please go on.
>
>> 2)I call them legacy irq mode because not only apic virtual wire mode
>> exists, but the PIC mode in x86 32bit system. You need consider it too.
>> Then init_bsp_APIC need be renamaed to an appropriate one. And assume
>> this has been tested on x86 32bit system. 
> Thanks for your reminders.
> As the comment of init_bsp_APIC(), it's just used to setup the virtual
> wire mode.
> so I won't change its name.
>
> But i agree with that PIC mode should be considered.
> Next version will be like below,
>
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index 04358e0cf1e2..d40bab947a2a 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -1186,7 +1186,7 @@ void __init init_bsp_APIC(void)
>          * the worst case is that both of them are inactive,
>          * If so, We need to enable the virtual wire mode via
> through-local-APIC
>          */
> -       if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC)
> +       if ( pic_mode || (smp_found_config && check_virtual_wire_mode())

I think this is needless, as init_bsp_APIC() is made under the following condition:
#if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC)

> ||
>                 !boot_cpu_has(X86_FEATURE_APIC))
>                 return;
>> 3) 
>>
>> *)About IO-APIC setting as virtual wire mode, I am always confused. In
>> MP Spec 3.6.2.2, it says "the interrupt signal passes through both the
>> I/O APIC and the BSP’s local APIC". That means IO-APIC virtual wire mode
>> need both IO-APIC and LAPIC to be set, and with my understanding only
>> pin of IO-APIC is set as ExtInt, LAPIC should be pass-through.
>>
>> *)But in Intel Arch manual 3A 10.1, there's only below sentences to mention
>> it:
>>
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> The I/O APIC also has a “virtual wire mode” that allows it to communicate
>> with a standard 8259A-style external interrupt controller. Note that the
>> local APIC can be disabled. This allows an associated processor core to
>> receive interrupts directly from an 8259A interrupt controller.
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Eric's code in native_disable_io_apic() has the same point as above
>> words.
> IMO，
> the through-IO-APIC mode has no relationship with the setting of local
> APIC.
> that's why i only check the pin of IO-APIC in virtual_wire_via_ioapic().

For through-IO-APIC mode, I think the bsp local apic should be on, but its LINT0 pin must not
be set to be "ExtINT"(I guess with this mode set will make it to be through-LAPIC mode), and
we can clearly see the fact from disconnect_bsp_APIC()'s implementation.

Regards,
Xunlei
> Thanks
> wei
>> *)However please read code comments in irq_remapping_disable_io_apic(),
>> Joerg's description give me a different impression that we can choose
>> to only use LAPIC virtual wire mode. Joerg is IOMMU maintainers, he is
>> very familiar with io-apic since IOMMU need take over io-apic entry
>> filling, there must be reason he wrote that. Add Joerg to CC list.
>>
>> Seems it's difficult to find a system with IO-APIC virtual wire mode,
>> maybe we can just keep it as is. Not sure if Intel engineers can help
>> explain and confirm this.
>>
>> That's all I can think of.
>>
>> Thanks
>> Baoquan
>>
>> On 07/22/16 at 04:10pm, Wei Jiangang wrote:
>>> If we specify the 'notsc' parameter for the dump-capture kernel,
>>> and then trigger a crash(panic) by using "ALT-SysRq-c" or
>>> "echo c > /proc/sysrq-trigger", the dump-capture kernel will
>>> hang in calibrate_delay_converge() and wait for jiffies changes.
>>> serial log as follows:
>>>
>>>     tsc: Fast TSC calibration using PIT
>>>     tsc: Detected 2099.947 MHz processor
>>>     Calibrating delay loop...
>>>
>>> The reason for jiffies not changes is there's no timer interrupt
>>> passed to dump-capture kernel.
>>>
>>> In fact, once kernel panic occurs, the local APIC is disabled
>>> by lapic_shutdown() in reboot path.
>>> generly speaking, local APIC state can be initialized by BIOS
>>> after Power-Up or Reset, which doesn't apply to kdump case.
>>> so the kernel has to be responsible for initialize the interrupt
>>> mode properly according the latest status of APIC in bootup path.
>>>
>>> An MP operating system is booted under either PIC mode or
>>> virtual wire mode. Later, the operating system switches to
>>> symmetric I/O mode as it enters multiprocessor mode.
>>> Two kinds of virtual wire mode are defined in Intel MP spec:
>>> virtual wire mode via local APIC or via I/O APIC.
>>>
>>> Now we determine the mode of APIC only through a SMP BIOS(MP table).
>>> That's not enough.
>>> It's better to do further check if APIC works with effective mode,
>>> and do some porper setting.
>>>
>>> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
>>> Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
>>> ---
>>>  arch/x86/include/asm/io_apic.h |  5 ++++
>>>  arch/x86/kernel/apic/apic.c    | 55 +++++++++++++++++++++++++++++++++++++++++-
>>>  arch/x86/kernel/apic/io_apic.c | 28 +++++++++++++++++++++
>>>  3 files changed, 87 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
>>> index 6cbf2cfb3f8a..6550bd43fa39 100644
>>> --- a/arch/x86/include/asm/io_apic.h
>>> +++ b/arch/x86/include/asm/io_apic.h
>>> @@ -190,6 +190,7 @@ static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
>>>  }
>>>  
>>>  extern void setup_IO_APIC(void);
>>> +extern bool virtual_wire_via_ioapic(void);
>>>  extern void enable_IO_APIC(void);
>>>  extern void disable_IO_APIC(void);
>>>  extern void setup_ioapic_dest(void);
>>> @@ -231,6 +232,10 @@ static inline void io_apic_init_mappings(void) { }
>>>  #define native_disable_io_apic		NULL
>>>  
>>>  static inline void setup_IO_APIC(void) { }
>>> +static inline bool virtual_wire_via_ioapic(void)
>>> +{
>>> +	return true;
>>> +}
>>>  static inline void enable_IO_APIC(void) { }
>>>  static inline void setup_ioapic_dest(void) { }
>>>  
>>> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
>>> index 8e25b9b2d351..04358e0cf1e2 100644
>>> --- a/arch/x86/kernel/apic/apic.c
>>> +++ b/arch/x86/kernel/apic/apic.c
>>> @@ -1124,6 +1124,53 @@ void __init sync_Arb_IDs(void)
>>>  }
>>>  
>>>  /*
>>> + * Return false means the virtual wire mode through-local-apic is inactive
>>> + */
>>> +static bool virtual_wire_via_lapic(void)
>>> +{
>>> +	unsigned int value;
>>> +
>>> +	/* Check the APIC global enable/disable flag firstly */
>>> +	if (boot_cpu_data.x86 >= 6) {
>>> +		u32 h, l;
>>> +
>>> +		rdmsr(MSR_IA32_APICBASE, l, h);
>>> +		/*
>>> +		 * If local APIC is disabled by BIOS
>>> +		 * do nothing, but return true
>>> +		 */
>>> +		if (!(l & MSR_IA32_APICBASE_ENABLE))
>>> +			return true;
>>> +	}
>>> +
>>> +	/* Check the software enable/disable flag */
>>> +	value = apic_read(APIC_SPIV);
>>> +	if (!(value & APIC_SPIV_APIC_ENABLED))
>>> +		return false;
>>> +
>>> +	/*
>>> +	 * Virtual wire mode via local APIC requests
>>> +	 * APIC to enable the LINT0 for edge-trggered ExtINT delivery mode
>>> +	 * and LINT1 for level-triggered NMI delivery mode
>>> +	 */
>>> +	value = apic_read(APIC_LVT0);
>>> +	if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_EXTINT)
>>> +		return false;
>>> +
>>> +	value = apic_read(APIC_LVT1);
>>> +	if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_NMI)
>>> +		return false;
>>> +
>>> +	return true;
>>> +}
>>> +
>>> +static bool check_virtual_wire_mode(void)
>>> +{
>>> +	/* Neither of virtual wire mode is active, return false */
>>> +	return  virtual_wire_via_lapic() || virtual_wire_via_ioapic();
>>> +}
>>> +
>>> +/*
>>>   * An initial setup of the virtual wire mode.
>>>   */
>>>  void __init init_bsp_APIC(void)
>>> @@ -1133,8 +1180,14 @@ void __init init_bsp_APIC(void)
>>>  	/*
>>>  	 * Don't do the setup now if we have a SMP BIOS as the
>>>  	 * through-I/O-APIC virtual wire mode might be active.
>>> +	 *
>>> +	 * It's better to do further check if either through-I/O-APIC
>>> +	 * or through-local-APIC is active.
>>> +	 * the worst case is that both of them are inactive,
>>> +	 * If so, We need to enable the virtual wire mode via through-local-APIC
>>>  	 */
>>> -	if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC))
>>> +	if ((smp_found_config && check_virtual_wire_mode()) ||
>>> +		!boot_cpu_has(X86_FEATURE_APIC))
>>>  		return;
>>>  
>>>  	/*
>>> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
>>> index 446702ed99dc..5a32c26938ac 100644
>>> --- a/arch/x86/kernel/apic/io_apic.c
>>> +++ b/arch/x86/kernel/apic/io_apic.c
>>> @@ -1379,6 +1379,34 @@ void __init print_IO_APICs(void)
>>>  /* Where if anywhere is the i8259 connect in external int mode */
>>>  static struct { int pin, apic; } ioapic_i8259 = { -1, -1 };
>>>  
>>> +/*
>>> + * Return false means the virtual wire mode via I/O APIC is inactive
>>> + */
>>> +bool virtual_wire_via_ioapic(void)
>>> +{
>>> +	int apic, pin;
>>> +
>>> +	for_each_ioapic_pin(apic, pin) {
>>> +		/* See if any of the pins is in ExtINT mode */
>>> +		struct IO_APIC_route_entry entry = ioapic_read_entry(apic, pin);
>>> +
>>> +		/*
>>> +		 * If the interrupt line is enabled and in ExtInt mode
>>> +		 * I have found the pin where the i8259 is connected.
>>> +		 */
>>> +		if ((entry.mask == 0) && (entry.delivery_mode == dest_ExtINT))
>>> +			return true;
>>> +	}
>>> +
>>> +	/*
>>> +	 * Virtual wire mode via I/O APIC requests
>>> +	 * I/O APIC be connected to i8259 in chapter 3.6.2.2 of the MP v1.4 spec
>>> +	 * If no pin in ExtInt mode,
>>> +	 * the through-I/O-APIC virtual wire mode can be regarded inactive.
>>> +	 */
>>> +	return false;
>>> +}
>>> +
>>>  void __init enable_IO_APIC(void)
>>>  {
>>>  	int i8259_apic, i8259_pin;
>>> -- 
>>> 1.9.3
>>>
>>>
>>>
>>
>
>

Re: [PATCH 3/3] x86/apic: Improved the setting of interrupt mode for bsp

[Xunlei Pang]

On 2016/07/25 at 11:04, Wei, Jiangang wrote:
> Hi He,
>
> Thanks for your response firstly.
>
> On Fri, 2016-07-22 at 18:40 +0800, Baoquan He wrote:
>> Hi Jiangang,
>>
>> This is very nice, should be the stuff Eric and Ingo would like to see.
>> But I have several questions:
>>
>> 1) Are you not going to clean up the old legacy irq mode setting code in
>> 1st kernel?
> Yes, I would like to pay more attention on fixing kdump's failure with
> notsc.
> No plan to clean up the irq mode setting codes in the crash kernel
> reboot path.
> If you are interested in it, please go on.
>
>> 2)I call them legacy irq mode because not only apic virtual wire mode
>> exists, but the PIC mode in x86 32bit system. You need consider it too.
>> Then init_bsp_APIC need be renamaed to an appropriate one. And assume
>> this has been tested on x86 32bit system. 
> Thanks for your reminders.
> As the comment of init_bsp_APIC(), it's just used to setup the virtual
> wire mode.
> so I won't change its name.
>
> But i agree with that PIC mode should be considered.
> Next version will be like below,
>
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index 04358e0cf1e2..d40bab947a2a 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -1186,7 +1186,7 @@ void __init init_bsp_APIC(void)
>          * the worst case is that both of them are inactive,
>          * If so, We need to enable the virtual wire mode via
> through-local-APIC
>          */
> -       if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC)
> +       if ( pic_mode || (smp_found_config && check_virtual_wire_mode())

I think this is needless, as init_bsp_APIC() is made under the following condition:
#if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC)

> ||
>                 !boot_cpu_has(X86_FEATURE_APIC))
>                 return;
>> 3) 
>>
>> *)About IO-APIC setting as virtual wire mode, I am always confused. In
>> MP Spec 3.6.2.2, it says "the interrupt signal passes through both the
>> I/O APIC and the BSP’s local APIC". That means IO-APIC virtual wire mode
>> need both IO-APIC and LAPIC to be set, and with my understanding only
>> pin of IO-APIC is set as ExtInt, LAPIC should be pass-through.
>>
>> *)But in Intel Arch manual 3A 10.1, there's only below sentences to mention
>> it:
>>
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> The I/O APIC also has a “virtual wire mode” that allows it to communicate
>> with a standard 8259A-style external interrupt controller. Note that the
>> local APIC can be disabled. This allows an associated processor core to
>> receive interrupts directly from an 8259A interrupt controller.
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Eric's code in native_disable_io_apic() has the same point as above
>> words.
> IMO，
> the through-IO-APIC mode has no relationship with the setting of local
> APIC.
> that's why i only check the pin of IO-APIC in virtual_wire_via_ioapic().

For through-IO-APIC mode, I think the bsp local apic should be on, but its LINT0 pin must not
be set to be "ExtINT"(I guess with this mode set will make it to be through-LAPIC mode), and
we can clearly see the fact from disconnect_bsp_APIC()'s implementation.

Regards,
Xunlei
> Thanks
> wei
>> *)However please read code comments in irq_remapping_disable_io_apic(),
>> Joerg's description give me a different impression that we can choose
>> to only use LAPIC virtual wire mode. Joerg is IOMMU maintainers, he is
>> very familiar with io-apic since IOMMU need take over io-apic entry
>> filling, there must be reason he wrote that. Add Joerg to CC list.
>>
>> Seems it's difficult to find a system with IO-APIC virtual wire mode,
>> maybe we can just keep it as is. Not sure if Intel engineers can help
>> explain and confirm this.
>>
>> That's all I can think of.
>>
>> Thanks
>> Baoquan
>>
>> On 07/22/16 at 04:10pm, Wei Jiangang wrote:
>>> If we specify the 'notsc' parameter for the dump-capture kernel,
>>> and then trigger a crash(panic) by using "ALT-SysRq-c" or
>>> "echo c > /proc/sysrq-trigger", the dump-capture kernel will
>>> hang in calibrate_delay_converge() and wait for jiffies changes.
>>> serial log as follows:
>>>
>>>     tsc: Fast TSC calibration using PIT
>>>     tsc: Detected 2099.947 MHz processor
>>>     Calibrating delay loop...
>>>
>>> The reason for jiffies not changes is there's no timer interrupt
>>> passed to dump-capture kernel.
>>>
>>> In fact, once kernel panic occurs, the local APIC is disabled
>>> by lapic_shutdown() in reboot path.
>>> generly speaking, local APIC state can be initialized by BIOS
>>> after Power-Up or Reset, which doesn't apply to kdump case.
>>> so the kernel has to be responsible for initialize the interrupt
>>> mode properly according the latest status of APIC in bootup path.
>>>
>>> An MP operating system is booted under either PIC mode or
>>> virtual wire mode. Later, the operating system switches to
>>> symmetric I/O mode as it enters multiprocessor mode.
>>> Two kinds of virtual wire mode are defined in Intel MP spec:
>>> virtual wire mode via local APIC or via I/O APIC.
>>>
>>> Now we determine the mode of APIC only through a SMP BIOS(MP table).
>>> That's not enough.
>>> It's better to do further check if APIC works with effective mode,
>>> and do some porper setting.
>>>
>>> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
>>> Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
>>> ---
>>>  arch/x86/include/asm/io_apic.h |  5 ++++
>>>  arch/x86/kernel/apic/apic.c    | 55 +++++++++++++++++++++++++++++++++++++++++-
>>>  arch/x86/kernel/apic/io_apic.c | 28 +++++++++++++++++++++
>>>  3 files changed, 87 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
>>> index 6cbf2cfb3f8a..6550bd43fa39 100644
>>> --- a/arch/x86/include/asm/io_apic.h
>>> +++ b/arch/x86/include/asm/io_apic.h
>>> @@ -190,6 +190,7 @@ static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
>>>  }
>>>  
>>>  extern void setup_IO_APIC(void);
>>> +extern bool virtual_wire_via_ioapic(void);
>>>  extern void enable_IO_APIC(void);
>>>  extern void disable_IO_APIC(void);
>>>  extern void setup_ioapic_dest(void);
>>> @@ -231,6 +232,10 @@ static inline void io_apic_init_mappings(void) { }
>>>  #define native_disable_io_apic		NULL
>>>  
>>>  static inline void setup_IO_APIC(void) { }
>>> +static inline bool virtual_wire_via_ioapic(void)
>>> +{
>>> +	return true;
>>> +}
>>>  static inline void enable_IO_APIC(void) { }
>>>  static inline void setup_ioapic_dest(void) { }
>>>  
>>> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
>>> index 8e25b9b2d351..04358e0cf1e2 100644
>>> --- a/arch/x86/kernel/apic/apic.c
>>> +++ b/arch/x86/kernel/apic/apic.c
>>> @@ -1124,6 +1124,53 @@ void __init sync_Arb_IDs(void)
>>>  }
>>>  
>>>  /*
>>> + * Return false means the virtual wire mode through-local-apic is inactive
>>> + */
>>> +static bool virtual_wire_via_lapic(void)
>>> +{
>>> +	unsigned int value;
>>> +
>>> +	/* Check the APIC global enable/disable flag firstly */
>>> +	if (boot_cpu_data.x86 >= 6) {
>>> +		u32 h, l;
>>> +
>>> +		rdmsr(MSR_IA32_APICBASE, l, h);
>>> +		/*
>>> +		 * If local APIC is disabled by BIOS
>>> +		 * do nothing, but return true
>>> +		 */
>>> +		if (!(l & MSR_IA32_APICBASE_ENABLE))
>>> +			return true;
>>> +	}
>>> +
>>> +	/* Check the software enable/disable flag */
>>> +	value = apic_read(APIC_SPIV);
>>> +	if (!(value & APIC_SPIV_APIC_ENABLED))
>>> +		return false;
>>> +
>>> +	/*
>>> +	 * Virtual wire mode via local APIC requests
>>> +	 * APIC to enable the LINT0 for edge-trggered ExtINT delivery mode
>>> +	 * and LINT1 for level-triggered NMI delivery mode
>>> +	 */
>>> +	value = apic_read(APIC_LVT0);
>>> +	if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_EXTINT)
>>> +		return false;
>>> +
>>> +	value = apic_read(APIC_LVT1);
>>> +	if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_NMI)
>>> +		return false;
>>> +
>>> +	return true;
>>> +}
>>> +
>>> +static bool check_virtual_wire_mode(void)
>>> +{
>>> +	/* Neither of virtual wire mode is active, return false */
>>> +	return  virtual_wire_via_lapic() || virtual_wire_via_ioapic();
>>> +}
>>> +
>>> +/*
>>>   * An initial setup of the virtual wire mode.
>>>   */
>>>  void __init init_bsp_APIC(void)
>>> @@ -1133,8 +1180,14 @@ void __init init_bsp_APIC(void)
>>>  	/*
>>>  	 * Don't do the setup now if we have a SMP BIOS as the
>>>  	 * through-I/O-APIC virtual wire mode might be active.
>>> +	 *
>>> +	 * It's better to do further check if either through-I/O-APIC
>>> +	 * or through-local-APIC is active.
>>> +	 * the worst case is that both of them are inactive,
>>> +	 * If so, We need to enable the virtual wire mode via through-local-APIC
>>>  	 */
>>> -	if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC))
>>> +	if ((smp_found_config && check_virtual_wire_mode()) ||
>>> +		!boot_cpu_has(X86_FEATURE_APIC))
>>>  		return;
>>>  
>>>  	/*
>>> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
>>> index 446702ed99dc..5a32c26938ac 100644
>>> --- a/arch/x86/kernel/apic/io_apic.c
>>> +++ b/arch/x86/kernel/apic/io_apic.c
>>> @@ -1379,6 +1379,34 @@ void __init print_IO_APICs(void)
>>>  /* Where if anywhere is the i8259 connect in external int mode */
>>>  static struct { int pin, apic; } ioapic_i8259 = { -1, -1 };
>>>  
>>> +/*
>>> + * Return false means the virtual wire mode via I/O APIC is inactive
>>> + */
>>> +bool virtual_wire_via_ioapic(void)
>>> +{
>>> +	int apic, pin;
>>> +
>>> +	for_each_ioapic_pin(apic, pin) {
>>> +		/* See if any of the pins is in ExtINT mode */
>>> +		struct IO_APIC_route_entry entry = ioapic_read_entry(apic, pin);
>>> +
>>> +		/*
>>> +		 * If the interrupt line is enabled and in ExtInt mode
>>> +		 * I have found the pin where the i8259 is connected.
>>> +		 */
>>> +		if ((entry.mask == 0) && (entry.delivery_mode == dest_ExtINT))
>>> +			return true;
>>> +	}
>>> +
>>> +	/*
>>> +	 * Virtual wire mode via I/O APIC requests
>>> +	 * I/O APIC be connected to i8259 in chapter 3.6.2.2 of the MP v1.4 spec
>>> +	 * If no pin in ExtInt mode,
>>> +	 * the through-I/O-APIC virtual wire mode can be regarded inactive.
>>> +	 */
>>> +	return false;
>>> +}
>>> +
>>>  void __init enable_IO_APIC(void)
>>>  {
>>>  	int i8259_apic, i8259_pin;
>>> -- 
>>> 1.9.3
>>>
>>>
>>>
>>
>
>


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 3/3] x86/apic: Improved the setting of interrupt mode for bsp

[Wei, Jiangang]

On Mon, 2016-07-25 at 16:21 +0800, Xunlei Pang wrote:
> On 2016/07/25 at 11:04, Wei, Jiangang wrote:
> > Hi He,
> >
> > Thanks for your response firstly.
> >
> > On Fri, 2016-07-22 at 18:40 +0800, Baoquan He wrote:
> >> Hi Jiangang,
> >>
> >> This is very nice, should be the stuff Eric and Ingo would like to see.
> >> But I have several questions:
> >>
> >> 1) Are you not going to clean up the old legacy irq mode setting code in
> >> 1st kernel?
> > Yes, I would like to pay more attention on fixing kdump's failure with
> > notsc.
> > No plan to clean up the irq mode setting codes in the crash kernel
> > reboot path.
> > If you are interested in it, please go on.
> >
> >> 2)I call them legacy irq mode because not only apic virtual wire mode
> >> exists, but the PIC mode in x86 32bit system. You need consider it too.
> >> Then init_bsp_APIC need be renamaed to an appropriate one. And assume
> >> this has been tested on x86 32bit system. 
> > Thanks for your reminders.
> > As the comment of init_bsp_APIC(), it's just used to setup the virtual
> > wire mode.
> > so I won't change its name.
> >
> > But i agree with that PIC mode should be considered.
> > Next version will be like below,
> >
> > diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> > index 04358e0cf1e2..d40bab947a2a 100644
> > --- a/arch/x86/kernel/apic/apic.c
> > +++ b/arch/x86/kernel/apic/apic.c
> > @@ -1186,7 +1186,7 @@ void __init init_bsp_APIC(void)
> >          * the worst case is that both of them are inactive,
> >          * If so, We need to enable the virtual wire mode via
> > through-local-APIC
> >          */
> > -       if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC)
> > +       if ( pic_mode || (smp_found_config && check_virtual_wire_mode())
> 
> I think this is needless, as init_bsp_APIC() is made under the following condition:
> #if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC)

Nope,
I suggest you take a look at default_get_smp_config().
.........
#if defined(CONFIG_X86_LOCAL_APIC) && defined(CONFIG_X86_32)
	if (mpf->feature2 & (1 << 7)) {
		pr_info("    IMCR and PIC compatibility mode.\n");
		pic_mode = 1;
	} else {
		pr_info("    Virtual Wire compatibility mode.\n");
		pic_mode = 0;
	}
#endif
.......

if we declare CONFIG_X86_32 and CONFIG_X86_LOCAL_APIC, . init_bsp_APIC
may be called.  so check pic_mode is useful.

> 
> > ||
> >                 !boot_cpu_has(X86_FEATURE_APIC))
> >                 return;
> >> 3) 
> >>
> >> *)About IO-APIC setting as virtual wire mode, I am always confused. In
> >> MP Spec 3.6.2.2, it says "the interrupt signal passes through both the
> >> I/O APIC and the BSP’s local APIC". That means IO-APIC virtual wire mode
> >> need both IO-APIC and LAPIC to be set, and with my understanding only
> >> pin of IO-APIC is set as ExtInt, LAPIC should be pass-through.
> >>
> >> *)But in Intel Arch manual 3A 10.1, there's only below sentences to mention
> >> it:
> >>
> >> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> The I/O APIC also has a “virtual wire mode” that allows it to communicate
> >> with a standard 8259A-style external interrupt controller. Note that the
> >> local APIC can be disabled. This allows an associated processor core to
> >> receive interrupts directly from an 8259A interrupt controller.
> >> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>
> >> Eric's code in native_disable_io_apic() has the same point as above
> >> words.
> > IMO，
> > the through-IO-APIC mode has no relationship with the setting of local
> > APIC.
> > that's why i only check the pin of IO-APIC in virtual_wire_via_ioapic().
> 
> For through-IO-APIC mode, I think the bsp local apic should be on, but its LINT0 pin must not
> be set to be "ExtINT"(I guess with this mode set will make it to be through-LAPIC mode), and
> we can clearly see the fact from disconnect_bsp_APIC()'s implementation.

Yes, your opinion looks reasonable.
both through-IO-APIC and through-local-APIC enable the software flag.

        /* For the spurious interrupt use vector F, and enable it */
	value = apic_read(APIC_SPIV);
	value &= ~APIC_VECTOR_MASK;
	value |= APIC_SPIV_APIC_ENABLED;
	value |= 0xf;
	apic_write(APIC_SPIV, value);

I will accept this point in v2 patch.

Thanks,
wei
> 
> Regards,
> Xunlei
> > Thanks
> > wei
> >> *)However please read code comments in irq_remapping_disable_io_apic(),
> >> Joerg's description give me a different impression that we can choose
> >> to only use LAPIC virtual wire mode. Joerg is IOMMU maintainers, he is
> >> very familiar with io-apic since IOMMU need take over io-apic entry
> >> filling, there must be reason he wrote that. Add Joerg to CC list.
> >>
> >> Seems it's difficult to find a system with IO-APIC virtual wire mode,
> >> maybe we can just keep it as is. Not sure if Intel engineers can help
> >> explain and confirm this.
> >>
> >> That's all I can think of.
> >>
> >> Thanks
> >> Baoquan
> >>
> >> On 07/22/16 at 04:10pm, Wei Jiangang wrote:
> >>> If we specify the 'notsc' parameter for the dump-capture kernel,
> >>> and then trigger a crash(panic) by using "ALT-SysRq-c" or
> >>> "echo c > /proc/sysrq-trigger", the dump-capture kernel will
> >>> hang in calibrate_delay_converge() and wait for jiffies changes.
> >>> serial log as follows:
> >>>
> >>>     tsc: Fast TSC calibration using PIT
> >>>     tsc: Detected 2099.947 MHz processor
> >>>     Calibrating delay loop...
> >>>
> >>> The reason for jiffies not changes is there's no timer interrupt
> >>> passed to dump-capture kernel.
> >>>
> >>> In fact, once kernel panic occurs, the local APIC is disabled
> >>> by lapic_shutdown() in reboot path.
> >>> generly speaking, local APIC state can be initialized by BIOS
> >>> after Power-Up or Reset, which doesn't apply to kdump case.
> >>> so the kernel has to be responsible for initialize the interrupt
> >>> mode properly according the latest status of APIC in bootup path.
> >>>
> >>> An MP operating system is booted under either PIC mode or
> >>> virtual wire mode. Later, the operating system switches to
> >>> symmetric I/O mode as it enters multiprocessor mode.
> >>> Two kinds of virtual wire mode are defined in Intel MP spec:
> >>> virtual wire mode via local APIC or via I/O APIC.
> >>>
> >>> Now we determine the mode of APIC only through a SMP BIOS(MP table).
> >>> That's not enough.
> >>> It's better to do further check if APIC works with effective mode,
> >>> and do some porper setting.
> >>>
> >>> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
> >>> Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
> >>> ---
> >>>  arch/x86/include/asm/io_apic.h |  5 ++++
> >>>  arch/x86/kernel/apic/apic.c    | 55 +++++++++++++++++++++++++++++++++++++++++-
> >>>  arch/x86/kernel/apic/io_apic.c | 28 +++++++++++++++++++++
> >>>  3 files changed, 87 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
> >>> index 6cbf2cfb3f8a..6550bd43fa39 100644
> >>> --- a/arch/x86/include/asm/io_apic.h
> >>> +++ b/arch/x86/include/asm/io_apic.h
> >>> @@ -190,6 +190,7 @@ static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
> >>>  }
> >>>  
> >>>  extern void setup_IO_APIC(void);
> >>> +extern bool virtual_wire_via_ioapic(void);
> >>>  extern void enable_IO_APIC(void);
> >>>  extern void disable_IO_APIC(void);
> >>>  extern void setup_ioapic_dest(void);
> >>> @@ -231,6 +232,10 @@ static inline void io_apic_init_mappings(void) { }
> >>>  #define native_disable_io_apic		NULL
> >>>  
> >>>  static inline void setup_IO_APIC(void) { }
> >>> +static inline bool virtual_wire_via_ioapic(void)
> >>> +{
> >>> +	return true;
> >>> +}
> >>>  static inline void enable_IO_APIC(void) { }
> >>>  static inline void setup_ioapic_dest(void) { }
> >>>  
> >>> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> >>> index 8e25b9b2d351..04358e0cf1e2 100644
> >>> --- a/arch/x86/kernel/apic/apic.c
> >>> +++ b/arch/x86/kernel/apic/apic.c
> >>> @@ -1124,6 +1124,53 @@ void __init sync_Arb_IDs(void)
> >>>  }
> >>>  
> >>>  /*
> >>> + * Return false means the virtual wire mode through-local-apic is inactive
> >>> + */
> >>> +static bool virtual_wire_via_lapic(void)
> >>> +{
> >>> +	unsigned int value;
> >>> +
> >>> +	/* Check the APIC global enable/disable flag firstly */
> >>> +	if (boot_cpu_data.x86 >= 6) {
> >>> +		u32 h, l;
> >>> +
> >>> +		rdmsr(MSR_IA32_APICBASE, l, h);
> >>> +		/*
> >>> +		 * If local APIC is disabled by BIOS
> >>> +		 * do nothing, but return true
> >>> +		 */
> >>> +		if (!(l & MSR_IA32_APICBASE_ENABLE))
> >>> +			return true;
> >>> +	}
> >>> +
> >>> +	/* Check the software enable/disable flag */
> >>> +	value = apic_read(APIC_SPIV);
> >>> +	if (!(value & APIC_SPIV_APIC_ENABLED))
> >>> +		return false;
> >>> +
> >>> +	/*
> >>> +	 * Virtual wire mode via local APIC requests
> >>> +	 * APIC to enable the LINT0 for edge-trggered ExtINT delivery mode
> >>> +	 * and LINT1 for level-triggered NMI delivery mode
> >>> +	 */
> >>> +	value = apic_read(APIC_LVT0);
> >>> +	if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_EXTINT)
> >>> +		return false;
> >>> +
> >>> +	value = apic_read(APIC_LVT1);
> >>> +	if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_NMI)
> >>> +		return false;
> >>> +
> >>> +	return true;
> >>> +}
> >>> +
> >>> +static bool check_virtual_wire_mode(void)
> >>> +{
> >>> +	/* Neither of virtual wire mode is active, return false */
> >>> +	return  virtual_wire_via_lapic() || virtual_wire_via_ioapic();
> >>> +}
> >>> +
> >>> +/*
> >>>   * An initial setup of the virtual wire mode.
> >>>   */
> >>>  void __init init_bsp_APIC(void)
> >>> @@ -1133,8 +1180,14 @@ void __init init_bsp_APIC(void)
> >>>  	/*
> >>>  	 * Don't do the setup now if we have a SMP BIOS as the
> >>>  	 * through-I/O-APIC virtual wire mode might be active.
> >>> +	 *
> >>> +	 * It's better to do further check if either through-I/O-APIC
> >>> +	 * or through-local-APIC is active.
> >>> +	 * the worst case is that both of them are inactive,
> >>> +	 * If so, We need to enable the virtual wire mode via through-local-APIC
> >>>  	 */
> >>> -	if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC))
> >>> +	if ((smp_found_config && check_virtual_wire_mode()) ||
> >>> +		!boot_cpu_has(X86_FEATURE_APIC))
> >>>  		return;
> >>>  
> >>>  	/*
> >>> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> >>> index 446702ed99dc..5a32c26938ac 100644
> >>> --- a/arch/x86/kernel/apic/io_apic.c
> >>> +++ b/arch/x86/kernel/apic/io_apic.c
> >>> @@ -1379,6 +1379,34 @@ void __init print_IO_APICs(void)
> >>>  /* Where if anywhere is the i8259 connect in external int mode */
> >>>  static struct { int pin, apic; } ioapic_i8259 = { -1, -1 };
> >>>  
> >>> +/*
> >>> + * Return false means the virtual wire mode via I/O APIC is inactive
> >>> + */
> >>> +bool virtual_wire_via_ioapic(void)
> >>> +{
> >>> +	int apic, pin;
> >>> +
> >>> +	for_each_ioapic_pin(apic, pin) {
> >>> +		/* See if any of the pins is in ExtINT mode */
> >>> +		struct IO_APIC_route_entry entry = ioapic_read_entry(apic, pin);
> >>> +
> >>> +		/*
> >>> +		 * If the interrupt line is enabled and in ExtInt mode
> >>> +		 * I have found the pin where the i8259 is connected.
> >>> +		 */
> >>> +		if ((entry.mask == 0) && (entry.delivery_mode == dest_ExtINT))
> >>> +			return true;
> >>> +	}
> >>> +
> >>> +	/*
> >>> +	 * Virtual wire mode via I/O APIC requests
> >>> +	 * I/O APIC be connected to i8259 in chapter 3.6.2.2 of the MP v1.4 spec
> >>> +	 * If no pin in ExtInt mode,
> >>> +	 * the through-I/O-APIC virtual wire mode can be regarded inactive.
> >>> +	 */
> >>> +	return false;
> >>> +}
> >>> +
> >>>  void __init enable_IO_APIC(void)
> >>>  {
> >>>  	int i8259_apic, i8259_pin;
> >>> -- 
> >>> 1.9.3
> >>>
> >>>
> >>>
> >>
> >
> >
> 
> 
> 

Re: [PATCH 3/3] x86/apic: Improved the setting of interrupt mode for bsp

[Wei, Jiangang]

On Mon, 2016-07-25 at 16:21 +0800, Xunlei Pang wrote:
> On 2016/07/25 at 11:04, Wei, Jiangang wrote:
> > Hi He,
> >
> > Thanks for your response firstly.
> >
> > On Fri, 2016-07-22 at 18:40 +0800, Baoquan He wrote:
> >> Hi Jiangang,
> >>
> >> This is very nice, should be the stuff Eric and Ingo would like to see.
> >> But I have several questions:
> >>
> >> 1) Are you not going to clean up the old legacy irq mode setting code in
> >> 1st kernel?
> > Yes, I would like to pay more attention on fixing kdump's failure with
> > notsc.
> > No plan to clean up the irq mode setting codes in the crash kernel
> > reboot path.
> > If you are interested in it, please go on.
> >
> >> 2)I call them legacy irq mode because not only apic virtual wire mode
> >> exists, but the PIC mode in x86 32bit system. You need consider it too.
> >> Then init_bsp_APIC need be renamaed to an appropriate one. And assume
> >> this has been tested on x86 32bit system. 
> > Thanks for your reminders.
> > As the comment of init_bsp_APIC(), it's just used to setup the virtual
> > wire mode.
> > so I won't change its name.
> >
> > But i agree with that PIC mode should be considered.
> > Next version will be like below,
> >
> > diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> > index 04358e0cf1e2..d40bab947a2a 100644
> > --- a/arch/x86/kernel/apic/apic.c
> > +++ b/arch/x86/kernel/apic/apic.c
> > @@ -1186,7 +1186,7 @@ void __init init_bsp_APIC(void)
> >          * the worst case is that both of them are inactive,
> >          * If so, We need to enable the virtual wire mode via
> > through-local-APIC
> >          */
> > -       if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC)
> > +       if ( pic_mode || (smp_found_config && check_virtual_wire_mode())
> 
> I think this is needless, as init_bsp_APIC() is made under the following condition:
> #if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC)

Nope,
I suggest you take a look at default_get_smp_config().
.........
#if defined(CONFIG_X86_LOCAL_APIC) && defined(CONFIG_X86_32)
	if (mpf->feature2 & (1 << 7)) {
		pr_info("    IMCR and PIC compatibility mode.\n");
		pic_mode = 1;
	} else {
		pr_info("    Virtual Wire compatibility mode.\n");
		pic_mode = 0;
	}
#endif
.......

if we declare CONFIG_X86_32 and CONFIG_X86_LOCAL_APIC, . init_bsp_APIC
may be called.  so check pic_mode is useful.

> 
> > ||
> >                 !boot_cpu_has(X86_FEATURE_APIC))
> >                 return;
> >> 3) 
> >>
> >> *)About IO-APIC setting as virtual wire mode, I am always confused. In
> >> MP Spec 3.6.2.2, it says "the interrupt signal passes through both the
> >> I/O APIC and the BSP’s local APIC". That means IO-APIC virtual wire mode
> >> need both IO-APIC and LAPIC to be set, and with my understanding only
> >> pin of IO-APIC is set as ExtInt, LAPIC should be pass-through.
> >>
> >> *)But in Intel Arch manual 3A 10.1, there's only below sentences to mention
> >> it:
> >>
> >> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> The I/O APIC also has a “virtual wire mode” that allows it to communicate
> >> with a standard 8259A-style external interrupt controller. Note that the
> >> local APIC can be disabled. This allows an associated processor core to
> >> receive interrupts directly from an 8259A interrupt controller.
> >> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>
> >> Eric's code in native_disable_io_apic() has the same point as above
> >> words.
> > IMO，
> > the through-IO-APIC mode has no relationship with the setting of local
> > APIC.
> > that's why i only check the pin of IO-APIC in virtual_wire_via_ioapic().
> 
> For through-IO-APIC mode, I think the bsp local apic should be on, but its LINT0 pin must not
> be set to be "ExtINT"(I guess with this mode set will make it to be through-LAPIC mode), and
> we can clearly see the fact from disconnect_bsp_APIC()'s implementation.

Yes, your opinion looks reasonable.
both through-IO-APIC and through-local-APIC enable the software flag.

        /* For the spurious interrupt use vector F, and enable it */
	value = apic_read(APIC_SPIV);
	value &= ~APIC_VECTOR_MASK;
	value |= APIC_SPIV_APIC_ENABLED;
	value |= 0xf;
	apic_write(APIC_SPIV, value);

I will accept this point in v2 patch.

Thanks,
wei
> 
> Regards,
> Xunlei
> > Thanks
> > wei
> >> *)However please read code comments in irq_remapping_disable_io_apic(),
> >> Joerg's description give me a different impression that we can choose
> >> to only use LAPIC virtual wire mode. Joerg is IOMMU maintainers, he is
> >> very familiar with io-apic since IOMMU need take over io-apic entry
> >> filling, there must be reason he wrote that. Add Joerg to CC list.
> >>
> >> Seems it's difficult to find a system with IO-APIC virtual wire mode,
> >> maybe we can just keep it as is. Not sure if Intel engineers can help
> >> explain and confirm this.
> >>
> >> That's all I can think of.
> >>
> >> Thanks
> >> Baoquan
> >>
> >> On 07/22/16 at 04:10pm, Wei Jiangang wrote:
> >>> If we specify the 'notsc' parameter for the dump-capture kernel,
> >>> and then trigger a crash(panic) by using "ALT-SysRq-c" or
> >>> "echo c > /proc/sysrq-trigger", the dump-capture kernel will
> >>> hang in calibrate_delay_converge() and wait for jiffies changes.
> >>> serial log as follows:
> >>>
> >>>     tsc: Fast TSC calibration using PIT
> >>>     tsc: Detected 2099.947 MHz processor
> >>>     Calibrating delay loop...
> >>>
> >>> The reason for jiffies not changes is there's no timer interrupt
> >>> passed to dump-capture kernel.
> >>>
> >>> In fact, once kernel panic occurs, the local APIC is disabled
> >>> by lapic_shutdown() in reboot path.
> >>> generly speaking, local APIC state can be initialized by BIOS
> >>> after Power-Up or Reset, which doesn't apply to kdump case.
> >>> so the kernel has to be responsible for initialize the interrupt
> >>> mode properly according the latest status of APIC in bootup path.
> >>>
> >>> An MP operating system is booted under either PIC mode or
> >>> virtual wire mode. Later, the operating system switches to
> >>> symmetric I/O mode as it enters multiprocessor mode.
> >>> Two kinds of virtual wire mode are defined in Intel MP spec:
> >>> virtual wire mode via local APIC or via I/O APIC.
> >>>
> >>> Now we determine the mode of APIC only through a SMP BIOS(MP table).
> >>> That's not enough.
> >>> It's better to do further check if APIC works with effective mode,
> >>> and do some porper setting.
> >>>
> >>> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
> >>> Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
> >>> ---
> >>>  arch/x86/include/asm/io_apic.h |  5 ++++
> >>>  arch/x86/kernel/apic/apic.c    | 55 +++++++++++++++++++++++++++++++++++++++++-
> >>>  arch/x86/kernel/apic/io_apic.c | 28 +++++++++++++++++++++
> >>>  3 files changed, 87 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
> >>> index 6cbf2cfb3f8a..6550bd43fa39 100644
> >>> --- a/arch/x86/include/asm/io_apic.h
> >>> +++ b/arch/x86/include/asm/io_apic.h
> >>> @@ -190,6 +190,7 @@ static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
> >>>  }
> >>>  
> >>>  extern void setup_IO_APIC(void);
> >>> +extern bool virtual_wire_via_ioapic(void);
> >>>  extern void enable_IO_APIC(void);
> >>>  extern void disable_IO_APIC(void);
> >>>  extern void setup_ioapic_dest(void);
> >>> @@ -231,6 +232,10 @@ static inline void io_apic_init_mappings(void) { }
> >>>  #define native_disable_io_apic		NULL
> >>>  
> >>>  static inline void setup_IO_APIC(void) { }
> >>> +static inline bool virtual_wire_via_ioapic(void)
> >>> +{
> >>> +	return true;
> >>> +}
> >>>  static inline void enable_IO_APIC(void) { }
> >>>  static inline void setup_ioapic_dest(void) { }
> >>>  
> >>> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> >>> index 8e25b9b2d351..04358e0cf1e2 100644
> >>> --- a/arch/x86/kernel/apic/apic.c
> >>> +++ b/arch/x86/kernel/apic/apic.c
> >>> @@ -1124,6 +1124,53 @@ void __init sync_Arb_IDs(void)
> >>>  }
> >>>  
> >>>  /*
> >>> + * Return false means the virtual wire mode through-local-apic is inactive
> >>> + */
> >>> +static bool virtual_wire_via_lapic(void)
> >>> +{
> >>> +	unsigned int value;
> >>> +
> >>> +	/* Check the APIC global enable/disable flag firstly */
> >>> +	if (boot_cpu_data.x86 >= 6) {
> >>> +		u32 h, l;
> >>> +
> >>> +		rdmsr(MSR_IA32_APICBASE, l, h);
> >>> +		/*
> >>> +		 * If local APIC is disabled by BIOS
> >>> +		 * do nothing, but return true
> >>> +		 */
> >>> +		if (!(l & MSR_IA32_APICBASE_ENABLE))
> >>> +			return true;
> >>> +	}
> >>> +
> >>> +	/* Check the software enable/disable flag */
> >>> +	value = apic_read(APIC_SPIV);
> >>> +	if (!(value & APIC_SPIV_APIC_ENABLED))
> >>> +		return false;
> >>> +
> >>> +	/*
> >>> +	 * Virtual wire mode via local APIC requests
> >>> +	 * APIC to enable the LINT0 for edge-trggered ExtINT delivery mode
> >>> +	 * and LINT1 for level-triggered NMI delivery mode
> >>> +	 */
> >>> +	value = apic_read(APIC_LVT0);
> >>> +	if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_EXTINT)
> >>> +		return false;
> >>> +
> >>> +	value = apic_read(APIC_LVT1);
> >>> +	if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_NMI)
> >>> +		return false;
> >>> +
> >>> +	return true;
> >>> +}
> >>> +
> >>> +static bool check_virtual_wire_mode(void)
> >>> +{
> >>> +	/* Neither of virtual wire mode is active, return false */
> >>> +	return  virtual_wire_via_lapic() || virtual_wire_via_ioapic();
> >>> +}
> >>> +
> >>> +/*
> >>>   * An initial setup of the virtual wire mode.
> >>>   */
> >>>  void __init init_bsp_APIC(void)
> >>> @@ -1133,8 +1180,14 @@ void __init init_bsp_APIC(void)
> >>>  	/*
> >>>  	 * Don't do the setup now if we have a SMP BIOS as the
> >>>  	 * through-I/O-APIC virtual wire mode might be active.
> >>> +	 *
> >>> +	 * It's better to do further check if either through-I/O-APIC
> >>> +	 * or through-local-APIC is active.
> >>> +	 * the worst case is that both of them are inactive,
> >>> +	 * If so, We need to enable the virtual wire mode via through-local-APIC
> >>>  	 */
> >>> -	if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC))
> >>> +	if ((smp_found_config && check_virtual_wire_mode()) ||
> >>> +		!boot_cpu_has(X86_FEATURE_APIC))
> >>>  		return;
> >>>  
> >>>  	/*
> >>> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> >>> index 446702ed99dc..5a32c26938ac 100644
> >>> --- a/arch/x86/kernel/apic/io_apic.c
> >>> +++ b/arch/x86/kernel/apic/io_apic.c
> >>> @@ -1379,6 +1379,34 @@ void __init print_IO_APICs(void)
> >>>  /* Where if anywhere is the i8259 connect in external int mode */
> >>>  static struct { int pin, apic; } ioapic_i8259 = { -1, -1 };
> >>>  
> >>> +/*
> >>> + * Return false means the virtual wire mode via I/O APIC is inactive
> >>> + */
> >>> +bool virtual_wire_via_ioapic(void)
> >>> +{
> >>> +	int apic, pin;
> >>> +
> >>> +	for_each_ioapic_pin(apic, pin) {
> >>> +		/* See if any of the pins is in ExtINT mode */
> >>> +		struct IO_APIC_route_entry entry = ioapic_read_entry(apic, pin);
> >>> +
> >>> +		/*
> >>> +		 * If the interrupt line is enabled and in ExtInt mode
> >>> +		 * I have found the pin where the i8259 is connected.
> >>> +		 */
> >>> +		if ((entry.mask == 0) && (entry.delivery_mode == dest_ExtINT))
> >>> +			return true;
> >>> +	}
> >>> +
> >>> +	/*
> >>> +	 * Virtual wire mode via I/O APIC requests
> >>> +	 * I/O APIC be connected to i8259 in chapter 3.6.2.2 of the MP v1.4 spec
> >>> +	 * If no pin in ExtInt mode,
> >>> +	 * the through-I/O-APIC virtual wire mode can be regarded inactive.
> >>> +	 */
> >>> +	return false;
> >>> +}
> >>> +
> >>>  void __init enable_IO_APIC(void)
> >>>  {
> >>>  	int i8259_apic, i8259_pin;
> >>> -- 
> >>> 1.9.3
> >>>
> >>>
> >>>
> >>
> >
> >
> 
> 
> 



_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec