[RFC PATCH 0/6] Unify the Interrupt Mode and setup it as soon as possible

[RFC PATCH 0/6] Unify the Interrupt Mode and setup it as soon as possible

[Dou Liyang]

According to Ingo's and Eric's advice[1,2], Try my best to optimize the 
init of Interrupt Mode for x86.

The MP specification defines three different interrupt modes as follows:

 1. PIC Mode
 2. Virtual Wire Mode
 3. Symmetic I/O Mode

Currently, In kernel,

1. Setup the Virtual Wire Mode during the IRQ initialization(
step 1 in the following figure).
2. Enable and Setup the Symmetic I/O Mode either during the
SMP-capabe system prepares CPUs(step 2) or during the UP system 
initializes itself(step 3).

  start_kernel
+---------------+
|
+--> .......
|
|    setup_arch
+--> +-------+
|
|    init_IRQ
+-> +--+-----+
|      |        init_ISA_irqs
|      +------> +-+--------+
|                 |         +----------------+
+--->             +------>  | 1.init_bsp_APIC|
|     .......               +----------------+
+--->
|     rest_init
+--->---+-----+
|       |   kernel_init
|       +> ----+-----+
|              |   kernel_init_freeable
|              +->  ----+-------------+
|                       |     smp_prepare_cpus
|                       +---> +----+---------+
|                       |          |   +-------------------+
|                       |          +-> |2.  apic_bsp_setup |
|                       |              +-------------------+
|                       |
v                       |     smp_init
                        +---> +---+----+
                                  |    +-------------------+
                                  +--> |3.  apic_bsp_setup |
                                       +-------------------+

The purpose of this patchset is Unifing these setup steps and executing as
soon as possible as follows:

   start_kernel
---------------+
|
|
|
|     init_IRQ
+---->---+----+
|        |
|        |      +--------------------+
|        +----> | 4. init_bsp_APIC   |
|               +--------------------+
v

By the way, Also fix a bug about kexec[3].


Some doubts, need help:

1. Patchset has influence on IOMMU in enable_IR_x2apic(). Not sure
it can be in advance?

2. Due to 

Commit 8c3ba8d04924 ("x86, apic: ack all pending irqs when crashed/on kexec")

 ..., patchset also needs TSC and uses the "cpu_khz" in setup_local_APIC().
And a warning[4] will be triggered when crashed/on kexec. Not sure how to
modify?

[1]. https://lkml.org/lkml/2016/8/2/929
[2]. https://lkml.org/lkml/2016/8/1/506
[3]. https://lkml.org/lkml/2016/7/25/1118
[4]. WARN_ON(max_loops <= 0) in setup_local_APIC()

Dou Liyang (6):
  x86/apic: Replace init_bsp_APIC() with apic_virture_wire_mode_setup()
  x86/apic: Construct a framework for setuping APIC mode as soon as
    possible
  x86/apic: Extract APIC timer related code from apic_bsp_setup()
  x86/apic: Make the APIC mode setup earlier for SMP-capable system
  x86/apic: Make the APIC mode setup earlier for UP system
  x86/apic: Remove the apic_virture_wire_mode_setup()

 arch/x86/include/asm/apic.h    |   7 +-
 arch/x86/include/asm/io_apic.h |   2 +
 arch/x86/kernel/apic/apic.c    | 218 ++++++++++++++++++++++++-----------------
 arch/x86/kernel/apic/io_apic.c |   4 +-
 arch/x86/kernel/irqinit.c      |   6 +-
 arch/x86/kernel/smpboot.c      |  68 ++-----------
 6 files changed, 149 insertions(+), 156 deletions(-)

-- 
2.5.5

[RFC PATCH 1/6] x86/apic: Replace init_bsp_APIC() with apic_virture_wire_mode_setup()

[Dou Liyang]

The init_bsp_APIC() setups the virtual wire mode through the local
APIC.

The function name is unsuitable which might imply that the BSP's
APIC will be initialized here, actually, where it will be done is
almost at the end of start_kernel(). And the CONFIG X86_64 is also
imply the X86_LOCAL_APIC is y.

Clarify it, also remove the redundant macros to increase
readability

Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
 arch/x86/include/asm/apic.h | 2 ++
 arch/x86/kernel/apic/apic.c | 4 ++--
 arch/x86/kernel/irqinit.c   | 5 ++---
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 730ef65..20ac73c 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -127,6 +127,7 @@ extern void disconnect_bsp_APIC(int virt_wire_setup);
 extern void disable_local_APIC(void);
 extern void lapic_shutdown(void);
 extern void sync_Arb_IDs(void);
+extern void apic_virture_wire_mode_setup(void);
 extern void init_bsp_APIC(void);
 extern void setup_local_APIC(void);
 extern void init_apic_mappings(void);
@@ -170,6 +171,7 @@ static inline void disable_local_APIC(void) { }
 # define setup_boot_APIC_clock x86_init_noop
 # define setup_secondary_APIC_clock x86_init_noop
 static inline void lapic_update_tsc_freq(void) { }
+static inline void apic_virture_wire_mode_setup(void) {}
 #endif /* !CONFIG_X86_LOCAL_APIC */
 
 #ifdef CONFIG_X86_X2APIC
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 8ccb7ef..f4fc949 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1154,9 +1154,9 @@ void __init sync_Arb_IDs(void)
 }
 
 /*
- * An initial setup of the virtual wire mode.
+ * Setup the through-local-APIC virtual wire mode.
  */
-void __init init_bsp_APIC(void)
+void apic_virture_wire_mode_setup(void)
 {
 	unsigned int value;
 
diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c
index 1423ab1..b6ef4ea 100644
--- a/arch/x86/kernel/irqinit.c
+++ b/arch/x86/kernel/irqinit.c
@@ -72,9 +72,8 @@ void __init init_ISA_irqs(void)
 	struct irq_chip *chip = legacy_pic->chip;
 	int i;
 
-#if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC)
-	init_bsp_APIC();
-#endif
+	apic_virture_wire_mode_setup();
+
 	legacy_pic->init(0);
 
 	for (i = 0; i < nr_legacy_irqs(); i++)
-- 
2.5.5

[RFC PATCH 2/6] x86/apic: Construct a framework for setuping APIC mode as soon as possible

[Dou Liyang]

Now, there are two ways to setup local apic and io-apic in X86 arch:
  1. In an SMP-capable system, it will be done when preparing the
cpus in native_smp_prepare_boot_cpu().
  2. If UP_LATE_INIT is y, it will be done in smp_init()

And, there are many switches in kernel which can determine the way of
APIC mode setup, as shown below:

  1. kconfig :
     CONFIG_X86_64; CONFIG_X86_LOCAL_APIC; CONFIG_x86_IO_APIC
  2. kernel option: disable_apic; skip_ioapic_setup
  3. BIOS : boot_cpu_has(X86_FEATURE_APIC)
  4. MP table: smp_found_config
  5. ACPI: acpi_lapic; acpi_ioapic; nr_ioapic

The setup is late which cause the dump-capture kernel hangs with 'notsc'
option in 1st kernel option. and the use of these switches is messily.

Before make the APIC mode setup earlier, construct a framework first to
prepare for the work and make the logic clear.

Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
 arch/x86/include/asm/apic.h |  2 ++
 arch/x86/kernel/apic/apic.c | 73 +++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/irqinit.c   |  3 ++
 3 files changed, 78 insertions(+)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 20ac73c..c973f18 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -172,6 +172,8 @@ static inline void disable_local_APIC(void) { }
 # define setup_secondary_APIC_clock x86_init_noop
 static inline void lapic_update_tsc_freq(void) { }
 static inline void apic_virture_wire_mode_setup(void) {}
+static inline void init_bsp_APIC(void) {}
+
 #endif /* !CONFIG_X86_LOCAL_APIC */
 
 #ifdef CONFIG_X86_X2APIC
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index f4fc949..bf4ccd0 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1153,6 +1153,63 @@ void __init sync_Arb_IDs(void)
 			APIC_INT_LEVELTRIG | APIC_DM_INIT);
 }
 
+enum apic_bsp_mode {
+	APIC_BSP_MODEL_PIC = 0,
+	APIC_BSP_MODEL_VIRTUAL_WIRE,
+	APIC_BSP_MODEL_SYMMETRIC_IO,
+	APIC_BSP_MODEL_COUNT
+};
+
+static int __init apic_bsp_mode_check(void)
+{
+
+	/* Check kernel option */
+	if (disable_apic) {
+		pr_info("APIC disabled by kernel option\n");
+		return APIC_BSP_MODEL_PIC;
+	}
+	/* Check BOIS */
+#ifdef CONFIG_X86_64
+	/* On 64-bit, The APIC is integrated, So, must have APIC feature */
+	if (!boot_cpu_has(X86_FEATURE_APIC)) {
+		disable_apic = 1;
+		pr_info("Apic disabled by BIOS\n");
+		return APIC_BSP_MODEL_PIC;
+	}
+#else
+	if (!boot_cpu_has(X86_FEATURE_APIC) &&
+		APIC_INTEGRATED(boot_cpu_apic_version)) {
+		pr_err("BIOS bug, local APIC #%d not detected!...\n",
+			boot_cpu_physical_apicid);
+		pr_err("... forcing use of dummy APIC emulation (tell your hw vendor)\n");
+		return APIC_BSP_MODEL_PIC;
+	}
+#endif
+	/*
+	 * Check MP table, if neither an integrated nor a separate chip
+	 * doesn't exist.
+	 */
+	if (!boot_cpu_has(X86_FEATURE_APIC) && !smp_found_config) {
+		pr_info("BOIS don't support APIC, and no SMP configuration.\n");
+		return APIC_BSP_MODEL_PIC;
+	}
+
+	/* Check MP table, ps: if the virtual wire has been setup */
+	if (!smp_found_config) {
+		disable_ioapic_support();
+
+		/* Check local APIC, if SMP_NO_CONFIG */
+		if (!acpi_lapic)
+			pr_info("SMP motherboard not detected\n");
+
+		return APIC_BSP_MODEL_VIRTUAL_WIRE;
+	}
+
+	/* Other checks of ACPI options will be done in each setup function */
+
+	return APIC_BSP_MODEL_SYMMETRIC_IO;
+}
+
 /*
  * Setup the through-local-APIC virtual wire mode.
  */
@@ -1202,6 +1259,22 @@ void apic_virture_wire_mode_setup(void)
 	apic_write(APIC_LVT1, value);
 }
 
+/* init the interrupt routing model for the BSP */
+void __init init_bsp_APIC(void)
+{
+	switch (apic_bsp_mode_check()) {
+	case APIC_BSP_MODEL_PIC:
+		pr_info("Keep in PIC mode(8259)\n");
+		return;
+	case APIC_BSP_MODEL_VIRTUAL_WIRE:
+		pr_info("switch to virtual wire model.\n");
+		return;
+	case APIC_BSP_MODEL_SYMMETRIC_IO:
+		pr_info("switch to symmectic I/O model.\n");
+		return;
+	}
+}
+
 static void lapic_setup_esr(void)
 {
 	unsigned int oldvalue, value, maxlvt;
diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c
index b6ef4ea..f30fb16 100644
--- a/arch/x86/kernel/irqinit.c
+++ b/arch/x86/kernel/irqinit.c
@@ -197,4 +197,7 @@ void __init native_init_IRQ(void)
 #ifdef CONFIG_X86_32
 	irq_ctx_init(smp_processor_id());
 #endif
+
+	/* init the IRQ Mode for BSP */
+	init_bsp_APIC();
 }
-- 
2.5.5

[RFC PATCH 3/6] x86/apic: Extract APIC timer related code from apic_bsp_setup()

[Dou Liyang]

The apic_bsp_setup() contains the APIC timer related code, which
leads to hard reuse the local APIC and I/O APIC setup independently.

Extract the related code to a single function for setuping APIC in
advance.

Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
 arch/x86/include/asm/io_apic.h |  2 ++
 arch/x86/kernel/apic/apic.c    | 28 +++++++++++++++++++++-------
 arch/x86/kernel/apic/io_apic.c |  4 +---
 3 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index 6cbf2cf..535ca00 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -189,6 +189,7 @@ static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
 	return x86_io_apic_ops.read(apic, reg);
 }
 
+extern void check_timer(void);
 extern void setup_IO_APIC(void);
 extern void enable_IO_APIC(void);
 extern void disable_IO_APIC(void);
@@ -230,6 +231,7 @@ static inline void io_apic_init_mappings(void) { }
 #define native_io_apic_read		NULL
 #define native_disable_io_apic		NULL
 
+static inline void check_timer(void) { }
 static inline void setup_IO_APIC(void) { }
 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 bf4ccd0..0ba8a85 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2324,6 +2324,25 @@ static void __init apic_bsp_up_setup(void)
 	physid_set_mask_of_physid(boot_cpu_physical_apicid, &phys_cpu_present_map);
 }
 
+/* Setup local APIC timer and get the Id*/
+static int __init apic_bsp_timer_setup(void)
+{
+	int id;
+
+	if (x2apic_mode)
+		id = apic_read(APIC_LDR);
+	else
+		id = GET_APIC_LOGICAL_ID(apic_read(APIC_LDR));
+
+	if (!skip_ioapic_setup && nr_ioapics && nr_legacy_irqs())
+		check_timer();
+
+	/* Setup local timer */
+	x86_init.timers.setup_percpu_clockev();
+
+	return id;
+}
+
 /**
  * apic_bsp_setup - Setup function for local apic and io-apic
  * @upmode:		Force UP mode (for APIC_init_uniprocessor)
@@ -2340,17 +2359,12 @@ int __init apic_bsp_setup(bool upmode)
 		apic_bsp_up_setup();
 	setup_local_APIC();
 
-	if (x2apic_mode)
-		id = apic_read(APIC_LDR);
-	else
-		id = GET_APIC_LOGICAL_ID(apic_read(APIC_LDR));
-
 	enable_IO_APIC();
 	end_local_APIC_setup();
 	irq_remap_enable_fault_handling();
 	setup_IO_APIC();
-	/* Setup local timer */
-	x86_init.timers.setup_percpu_clockev();
+
+	id = apic_bsp_timer_setup();
 	return id;
 }
 
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 347bb9f..e19b88f 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2047,7 +2047,7 @@ static int mp_alloc_timer_irq(int ioapic, int pin)
  *
  * FIXME: really need to revamp this for all platforms.
  */
-static inline void __init check_timer(void)
+void __init check_timer(void)
 {
 	struct irq_data *irq_data = irq_get_irq_data(0);
 	struct mp_chip_data *data = irq_data->chip_data;
@@ -2278,8 +2278,6 @@ void __init setup_IO_APIC(void)
 	sync_Arb_IDs();
 	setup_IO_APIC_irqs();
 	init_IO_APIC_traps();
-	if (nr_legacy_irqs())
-		check_timer();
 
 	ioapic_initialized = 1;
 }
-- 
2.5.5

[RFC PATCH 4/6] x86/apic: Make the APIC mode setup earlier for SMP-capable system

[Dou Liyang]

In the SMP-capable system, enable and setup the APIC mode in
native_smp_prepare_boot_cpu() which almost be called at the end
of start_kernel().

The MP table or ACPI has been read earlier, and time_init() which
is called before the APIC mode setup may need the IRQ.

Move the APIC mode setup code to init_IRQ(). Do it at the end of
IRQ initialization for SMP-capable system.

Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
 arch/x86/include/asm/apic.h |  3 ++-
 arch/x86/kernel/apic/apic.c | 39 ++++++++++++++++++++++++++++++++-------
 arch/x86/kernel/smpboot.c   | 10 +---------
 3 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index c973f18..be2abc3 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -146,7 +146,8 @@ static inline int apic_force_enable(unsigned long addr)
 extern int apic_force_enable(unsigned long addr);
 #endif
 
-extern int apic_bsp_setup(bool upmode);
+extern int apic_bsp_timer_setup(void);
+extern void apic_bsp_setup(bool upmode);
 extern void apic_ap_setup(void);
 
 /*
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 0ba8a85..ce8f88d 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1157,6 +1157,7 @@ enum apic_bsp_mode {
 	APIC_BSP_MODEL_PIC = 0,
 	APIC_BSP_MODEL_VIRTUAL_WIRE,
 	APIC_BSP_MODEL_SYMMETRIC_IO,
+	APIC_BSP_MODEL_SYMMETRIC_IO_NO_ROUTING,
 	APIC_BSP_MODEL_COUNT
 };
 
@@ -1207,7 +1208,29 @@ static int __init apic_bsp_mode_check(void)
 
 	/* Other checks of ACPI options will be done in each setup function */
 
+#ifdef CONFIG_SMP
+	if (read_apic_id() != boot_cpu_physical_apicid) {
+		pr_info("Boot APIC ID in local APIC unexpected (%d vs %d)",
+			read_apic_id(), boot_cpu_physical_apicid);
+
+		disable_ioapic_support();
+		/*Do nothing, just switch back to PIC here */
+		return APIC_BSP_MODEL_PIC;
+	}
+
+	/*
+	 * If SMP should be disabled, then really disable it!
+	 * No need setup apic routing ?
+	 */
+	if (!setup_max_cpus) {
+		pr_info("SMP mode deactivated\n");
+		return APIC_BSP_MODEL_SYMMETRIC_IO_NO_ROUTING;
+	}
+
 	return APIC_BSP_MODEL_SYMMETRIC_IO;
+#else
+	return APIC_BSP_MODEL_PIC;
+#endif
 }
 
 /*
@@ -1271,6 +1294,12 @@ void __init init_bsp_APIC(void)
 		return;
 	case APIC_BSP_MODEL_SYMMETRIC_IO:
 		pr_info("switch to symmectic I/O model.\n");
+		default_setup_apic_routing();
+		apic_bsp_setup(false);
+		return;
+	case APIC_BSP_MODEL_SYMMETRIC_IO_NO_ROUTING:
+		pr_info("switch to symmectic I/O model with no apic routing.\n");
+		apic_bsp_setup(false);
 		return;
 	}
 }
@@ -2325,7 +2354,7 @@ static void __init apic_bsp_up_setup(void)
 }
 
 /* Setup local APIC timer and get the Id*/
-static int __init apic_bsp_timer_setup(void)
+int __init apic_bsp_timer_setup(void)
 {
 	int id;
 
@@ -2350,10 +2379,8 @@ static int __init apic_bsp_timer_setup(void)
  * Returns:
  * apic_id of BSP APIC
  */
-int __init apic_bsp_setup(bool upmode)
+void __init apic_bsp_setup(bool upmode)
 {
-	int id;
-
 	connect_bsp_APIC();
 	if (upmode)
 		apic_bsp_up_setup();
@@ -2363,9 +2390,6 @@ int __init apic_bsp_setup(bool upmode)
 	end_local_APIC_setup();
 	irq_remap_enable_fault_handling();
 	setup_IO_APIC();
-
-	id = apic_bsp_timer_setup();
-	return id;
 }
 
 /*
@@ -2404,6 +2428,7 @@ int __init APIC_init_uniprocessor(void)
 
 	default_setup_apic_routing();
 	apic_bsp_setup(true);
+	apic_bsp_timer_setup();
 	return 0;
 }
 
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index bd1f1ad..a556281 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1332,20 +1332,12 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
 		return;
 	case SMP_FORCE_UP:
 		disable_smp();
-		apic_bsp_setup(false);
 		return;
 	case SMP_OK:
 		break;
 	}
 
-	if (read_apic_id() != boot_cpu_physical_apicid) {
-		panic("Boot APIC ID in local APIC unexpected (%d vs %d)",
-		     read_apic_id(), boot_cpu_physical_apicid);
-		/* Or can we switch back to PIC here? */
-	}
-
-	default_setup_apic_routing();
-	cpu0_logical_apicid = apic_bsp_setup(false);
+	cpu0_logical_apicid = apic_bsp_timer_setup();
 
 	pr_info("CPU0: ");
 	print_cpu_info(&cpu_data(0));
-- 
2.5.5

[RFC PATCH 5/6] x86/apic: Make the APIC mode setup earlier for UP system

[Dou Liyang]

The SMP-capable system has already enable and setup the APIC mode
as soon as possible.

Do it for UP system and make the code clear.

Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
 arch/x86/include/asm/apic.h |  2 ++
 arch/x86/kernel/apic/apic.c | 81 +++++++++++++++++----------------------------
 arch/x86/kernel/smpboot.c   | 58 ++++----------------------------
 3 files changed, 38 insertions(+), 103 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index be2abc3..fb06fe5 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -55,6 +55,8 @@ extern unsigned int lapic_timer_frequency;
 
 #ifdef CONFIG_SMP
 extern void __inquire_remote_apic(int apicid);
+extern int disable_smp_by_APIC;
+
 #else /* CONFIG_SMP */
 static inline void __inquire_remote_apic(int apicid)
 {
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index ce8f88d..c93c33d 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -169,6 +169,9 @@ __setup("apicpmtimer", setup_apicpmtimer);
 
 unsigned long mp_lapic_addr;
 int disable_apic;
+/* disable smp flag according to APIC configuration */
+int disable_smp_by_APIC;
+
 /* Disable local APIC timer from the kernel commandline or via dmi quirk */
 static int disable_apic_timer __initdata;
 /* Local APIC timer works in C2 */
@@ -1157,13 +1160,13 @@ enum apic_bsp_mode {
 	APIC_BSP_MODEL_PIC = 0,
 	APIC_BSP_MODEL_VIRTUAL_WIRE,
 	APIC_BSP_MODEL_SYMMETRIC_IO,
+	APIC_BSP_MODEL_SYMMETRIC_IO_NO_CONFIG,
 	APIC_BSP_MODEL_SYMMETRIC_IO_NO_ROUTING,
 	APIC_BSP_MODEL_COUNT
 };
 
-static int __init apic_bsp_mode_check(void)
+static int __init apic_bsp_mode_check(int *upmode)
 {
-
 	/* Check kernel option */
 	if (disable_apic) {
 		pr_info("APIC disabled by kernel option\n");
@@ -1200,8 +1203,11 @@ static int __init apic_bsp_mode_check(void)
 		disable_ioapic_support();
 
 		/* Check local APIC, if SMP_NO_CONFIG */
-		if (!acpi_lapic)
-			pr_info("SMP motherboard not detected\n");
+		if (!acpi_lapic) {
+			*upmode = true;
+			pr_info("SMP motherboard not detected.\n");
+			return APIC_BSP_MODEL_SYMMETRIC_IO_NO_CONFIG;
+		}
 
 		return APIC_BSP_MODEL_VIRTUAL_WIRE;
 	}
@@ -1229,7 +1235,13 @@ static int __init apic_bsp_mode_check(void)
 
 	return APIC_BSP_MODEL_SYMMETRIC_IO;
 #else
-	return APIC_BSP_MODEL_PIC;
+#ifdef CONFIG_UP_LATE_INIT
+		/* In UP system, If it supports late init */
+		*upmode = true;
+		return APIC_BSP_MODEL_SYMMETRIC_IO;
+#else
+		return APIC_BSP_MODEL_PIC;
+#endif
 #endif
 }
 
@@ -1285,9 +1297,12 @@ void apic_virture_wire_mode_setup(void)
 /* init the interrupt routing model for the BSP */
 void __init init_bsp_APIC(void)
 {
-	switch (apic_bsp_mode_check()) {
+	int upmode = false;
+
+	switch (apic_bsp_mode_check(&upmode)) {
 	case APIC_BSP_MODEL_PIC:
 		pr_info("Keep in PIC mode(8259)\n");
+		disable_smp_by_APIC = 1;
 		return;
 	case APIC_BSP_MODEL_VIRTUAL_WIRE:
 		pr_info("switch to virtual wire model.\n");
@@ -1295,13 +1310,17 @@ void __init init_bsp_APIC(void)
 	case APIC_BSP_MODEL_SYMMETRIC_IO:
 		pr_info("switch to symmectic I/O model.\n");
 		default_setup_apic_routing();
-		apic_bsp_setup(false);
-		return;
+		break;
+	case APIC_BSP_MODEL_SYMMETRIC_IO_NO_CONFIG:
+		pr_info("switch to symmectic I/O model with no SMP config.\n");
+		disable_smp_by_APIC = 2;
+		default_setup_apic_routing();
+		break;
 	case APIC_BSP_MODEL_SYMMETRIC_IO_NO_ROUTING:
 		pr_info("switch to symmectic I/O model with no apic routing.\n");
-		apic_bsp_setup(false);
-		return;
+		break;
 	}
+	apic_bsp_setup(upmode);
 }
 
 static void lapic_setup_esr(void)
@@ -2392,50 +2411,10 @@ void __init apic_bsp_setup(bool upmode)
 	setup_IO_APIC();
 }
 
-/*
- * This initializes the IO-APIC and APIC hardware if this is
- * a UP kernel.
- */
-int __init APIC_init_uniprocessor(void)
-{
-	if (disable_apic) {
-		pr_info("Apic disabled\n");
-		return -1;
-	}
-#ifdef CONFIG_X86_64
-	if (!boot_cpu_has(X86_FEATURE_APIC)) {
-		disable_apic = 1;
-		pr_info("Apic disabled by BIOS\n");
-		return -1;
-	}
-#else
-	if (!smp_found_config && !boot_cpu_has(X86_FEATURE_APIC))
-		return -1;
-
-	/*
-	 * Complain if the BIOS pretends there is one.
-	 */
-	if (!boot_cpu_has(X86_FEATURE_APIC) &&
-	    APIC_INTEGRATED(boot_cpu_apic_version)) {
-		pr_err("BIOS bug, local APIC 0x%x not detected!...\n",
-			boot_cpu_physical_apicid);
-		return -1;
-	}
-#endif
-
-	if (!smp_found_config)
-		disable_ioapic_support();
-
-	default_setup_apic_routing();
-	apic_bsp_setup(true);
-	apic_bsp_timer_setup();
-	return 0;
-}
-
 #ifdef CONFIG_UP_LATE_INIT
 void __init up_late_init(void)
 {
-	APIC_init_uniprocessor();
+	apic_bsp_timer_setup();
 }
 #endif
 
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index a556281..b7a810c 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1183,17 +1183,10 @@ static __init void disable_smp(void)
 	cpumask_set_cpu(0, topology_core_cpumask(0));
 }
 
-enum {
-	SMP_OK,
-	SMP_NO_CONFIG,
-	SMP_NO_APIC,
-	SMP_FORCE_UP,
-};
-
 /*
  * Various sanity checks.
  */
-static int __init smp_sanity_check(unsigned max_cpus)
+static void __init smp_sanity_check(void)
 {
 	preempt_disable();
 
@@ -1231,16 +1224,6 @@ static int __init smp_sanity_check(unsigned max_cpus)
 	}
 
 	/*
-	 * If we couldn't find an SMP configuration at boot time,
-	 * get out of here now!
-	 */
-	if (!smp_found_config && !acpi_lapic) {
-		preempt_enable();
-		pr_notice("SMP motherboard not detected\n");
-		return SMP_NO_CONFIG;
-	}
-
-	/*
 	 * Should not be necessary because the MP table should list the boot
 	 * CPU too, but we do it for the sake of robustness anyway.
 	 */
@@ -1251,28 +1234,6 @@ static int __init smp_sanity_check(unsigned max_cpus)
 	}
 	preempt_enable();
 
-	/*
-	 * If we couldn't find a local APIC, then get out of here now!
-	 */
-	if (APIC_INTEGRATED(boot_cpu_apic_version) &&
-	    !boot_cpu_has(X86_FEATURE_APIC)) {
-		if (!disable_apic) {
-			pr_err("BIOS bug, local APIC #%d not detected!...\n",
-				boot_cpu_physical_apicid);
-			pr_err("... forcing use of dummy APIC emulation (tell your hw vendor)\n");
-		}
-		return SMP_NO_APIC;
-	}
-
-	/*
-	 * If SMP should be disabled, then really disable it!
-	 */
-	if (!max_cpus) {
-		pr_info("SMP mode deactivated\n");
-		return SMP_FORCE_UP;
-	}
-
-	return SMP_OK;
 }
 
 static void __init smp_cpu_index_default(void)
@@ -1321,20 +1282,13 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
 
 	set_cpu_sibling_map(0);
 
-	switch (smp_sanity_check(max_cpus)) {
-	case SMP_NO_CONFIG:
-		disable_smp();
-		if (APIC_init_uniprocessor())
-			pr_notice("Local APIC not detected. Using dummy APIC emulation.\n");
-		return;
-	case SMP_NO_APIC:
-		disable_smp();
-		return;
-	case SMP_FORCE_UP:
+	smp_sanity_check();
+
+	if (disable_smp_by_APIC) {
+		if (disable_smp_by_APIC == 2)
+			apic_bsp_timer_setup();
 		disable_smp();
 		return;
-	case SMP_OK:
-		break;
 	}
 
 	cpu0_logical_apicid = apic_bsp_timer_setup();
-- 
2.5.5

[RFC PATCH 6/6] x86/apic: Remove the apic_virture_wire_mode_setup()

[Dou Liyang]

Currently, enable and setup the interrupt mode has been advanced
and has already included the virtual wire mode.

Remove the apic_virture_wire_mode_setup() which works for the
virtual wire mode originally.

Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
 arch/x86/include/asm/apic.h |  2 --
 arch/x86/kernel/apic/apic.c | 51 +--------------------------------------------
 arch/x86/kernel/irqinit.c   |  2 --
 3 files changed, 1 insertion(+), 54 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index fb06fe5..a9f73f4 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -129,7 +129,6 @@ extern void disconnect_bsp_APIC(int virt_wire_setup);
 extern void disable_local_APIC(void);
 extern void lapic_shutdown(void);
 extern void sync_Arb_IDs(void);
-extern void apic_virture_wire_mode_setup(void);
 extern void init_bsp_APIC(void);
 extern void setup_local_APIC(void);
 extern void init_apic_mappings(void);
@@ -174,7 +173,6 @@ static inline void disable_local_APIC(void) { }
 # define setup_boot_APIC_clock x86_init_noop
 # define setup_secondary_APIC_clock x86_init_noop
 static inline void lapic_update_tsc_freq(void) { }
-static inline void apic_virture_wire_mode_setup(void) {}
 static inline void init_bsp_APIC(void) {}
 
 #endif /* !CONFIG_X86_LOCAL_APIC */
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index c93c33d..06d87fd 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1245,55 +1245,6 @@ static int __init apic_bsp_mode_check(int *upmode)
 #endif
 }
 
-/*
- * Setup the through-local-APIC virtual wire mode.
- */
-void apic_virture_wire_mode_setup(void)
-{
-	unsigned int value;
-
-	/*
-	 * Don't do the setup now if we have a SMP BIOS as the
-	 * through-I/O-APIC virtual wire mode might be active.
-	 */
-	if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC))
-		return;
-
-	/*
-	 * Do not trust the local APIC being empty at bootup.
-	 */
-	clear_local_APIC();
-
-	/*
-	 * Enable APIC.
-	 */
-	value = apic_read(APIC_SPIV);
-	value &= ~APIC_VECTOR_MASK;
-	value |= APIC_SPIV_APIC_ENABLED;
-
-#ifdef CONFIG_X86_32
-	/* This bit is reserved on P4/Xeon and should be cleared */
-	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);
-
-	/*
-	 * Set up the virtual wire mode.
-	 */
-	apic_write(APIC_LVT0, APIC_DM_EXTINT);
-	value = APIC_DM_NMI;
-	if (!lapic_is_integrated())		/* 82489DX */
-		value |= APIC_LVT_LEVEL_TRIGGER;
-	if (apic_extnmi == APIC_EXTNMI_NONE)
-		value |= APIC_LVT_MASKED;
-	apic_write(APIC_LVT1, value);
-}
-
 /* init the interrupt routing model for the BSP */
 void __init init_bsp_APIC(void)
 {
@@ -1306,7 +1257,7 @@ void __init init_bsp_APIC(void)
 		return;
 	case APIC_BSP_MODEL_VIRTUAL_WIRE:
 		pr_info("switch to virtual wire model.\n");
-		return;
+		break;
 	case APIC_BSP_MODEL_SYMMETRIC_IO:
 		pr_info("switch to symmectic I/O model.\n");
 		default_setup_apic_routing();
diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c
index f30fb16..dc2deca 100644
--- a/arch/x86/kernel/irqinit.c
+++ b/arch/x86/kernel/irqinit.c
@@ -72,8 +72,6 @@ void __init init_ISA_irqs(void)
 	struct irq_chip *chip = legacy_pic->chip;
 	int i;
 
-	apic_virture_wire_mode_setup();
-
 	legacy_pic->init(0);
 
 	for (i = 0; i < nr_legacy_irqs(); i++)
-- 
2.5.5

Re: [RFC PATCH 0/6] Unify the Interrupt Mode and setup it as soon as possible

[Baoquan He]

Hi Liyang,

This is awesome. I planned to do this after kaslr back porting, glad to
see your posting. I like below diagram and the idea of patch 2/6
framework. Will review and see what I can do to help since rhel bug from
FJ is assigned to me.

Thanks for the effort!

And add Joerg to this thread since he knows IOMMU very well.

Thanks
Baoquan

On 03/29/17 at 10:55pm, Dou Liyang wrote:
> According to Ingo's and Eric's advice[1,2], Try my best to optimize the 
> init of Interrupt Mode for x86.
> 
> The MP specification defines three different interrupt modes as follows:
> 
>  1. PIC Mode
>  2. Virtual Wire Mode
>  3. Symmetic I/O Mode
> 
> Currently, In kernel,
> 
> 1. Setup the Virtual Wire Mode during the IRQ initialization(
> step 1 in the following figure).
> 2. Enable and Setup the Symmetic I/O Mode either during the
> SMP-capabe system prepares CPUs(step 2) or during the UP system 
> initializes itself(step 3).
> 
>   start_kernel
> +---------------+
> |
> +--> .......
> |
> |    setup_arch
> +--> +-------+
> |
> |    init_IRQ
> +-> +--+-----+
> |      |        init_ISA_irqs
> |      +------> +-+--------+
> |                 |         +----------------+
> +--->             +------>  | 1.init_bsp_APIC|
> |     .......               +----------------+
> +--->
> |     rest_init
> +--->---+-----+
> |       |   kernel_init
> |       +> ----+-----+
> |              |   kernel_init_freeable
> |              +->  ----+-------------+
> |                       |     smp_prepare_cpus
> |                       +---> +----+---------+
> |                       |          |   +-------------------+
> |                       |          +-> |2.  apic_bsp_setup |
> |                       |              +-------------------+
> |                       |
> v                       |     smp_init
>                         +---> +---+----+
>                                   |    +-------------------+
>                                   +--> |3.  apic_bsp_setup |
>                                        +-------------------+
> 
> The purpose of this patchset is Unifing these setup steps and executing as
> soon as possible as follows:
> 
>    start_kernel
> ---------------+
> |
> |
> |
> |     init_IRQ
> +---->---+----+
> |        |
> |        |      +--------------------+
> |        +----> | 4. init_bsp_APIC   |
> |               +--------------------+
> v
> 
> By the way, Also fix a bug about kexec[3].
> 
> 
> Some doubts, need help:
> 
> 1. Patchset has influence on IOMMU in enable_IR_x2apic(). Not sure
> it can be in advance?
> 
> 2. Due to 
> 
> Commit 8c3ba8d04924 ("x86, apic: ack all pending irqs when crashed/on kexec")
> 
>  ..., patchset also needs TSC and uses the "cpu_khz" in setup_local_APIC().
> And a warning[4] will be triggered when crashed/on kexec. Not sure how to
> modify?
> 
> [1]. https://lkml.org/lkml/2016/8/2/929
> [2]. https://lkml.org/lkml/2016/8/1/506
> [3]. https://lkml.org/lkml/2016/7/25/1118
> [4]. WARN_ON(max_loops <= 0) in setup_local_APIC()
> 
> Dou Liyang (6):
>   x86/apic: Replace init_bsp_APIC() with apic_virture_wire_mode_setup()
>   x86/apic: Construct a framework for setuping APIC mode as soon as
>     possible
>   x86/apic: Extract APIC timer related code from apic_bsp_setup()
>   x86/apic: Make the APIC mode setup earlier for SMP-capable system
>   x86/apic: Make the APIC mode setup earlier for UP system
>   x86/apic: Remove the apic_virture_wire_mode_setup()
> 
>  arch/x86/include/asm/apic.h    |   7 +-
>  arch/x86/include/asm/io_apic.h |   2 +
>  arch/x86/kernel/apic/apic.c    | 218 ++++++++++++++++++++++++-----------------
>  arch/x86/kernel/apic/io_apic.c |   4 +-
>  arch/x86/kernel/irqinit.c      |   6 +-
>  arch/x86/kernel/smpboot.c      |  68 ++-----------
>  6 files changed, 149 insertions(+), 156 deletions(-)
> 
> -- 
> 2.5.5
> 
> 
> 

Re: [RFC PATCH 0/6] Unify the Interrupt Mode and setup it as soon as possible

[Dou Liyang]

Hi Baoquan,

At 03/30/2017 10:08 AM, Baoquan He wrote:
> Hi Liyang,
>
> This is awesome. I planned to do this after kaslr back porting, glad to
> see your posting. I like below diagram and the idea of patch 2/6
> framework. Will review and see what I can do to help since rhel bug from
> FJ is assigned to me.
>

Thanks very much for your join! We have investigated the bug almost
half a year. :)

In my opinion,
If we plan to refactor the process of APIC initialization for the bug.
There must be lots of work need to be done. This patchset is just the
first step. When I test it, I am thinking about:

1. The check and logic in each enable and setup LAPIC/IOAPIC functions.
2. The process of IRQ remapping.
3. The check and init of APIC timer.
4. The relationship between the various switches, such as If
the smp_found_config is 1, the acpi_lapic must be 1.

And following work to me are:

1. Use more test cases to test.
2. learn the IOMMU.
3. trace the APIC timer code.
4. make the check logic more clear.

Hope to be helpful to you.

> Thanks for the effort!
>
> And add Joerg to this thread since he knows IOMMU very well.

oops, Yes, I forgot it, Thanks!

Thanks
Liyang

>
> Thanks
> Baoquan
>
> On 03/29/17 at 10:55pm, Dou Liyang wrote:
>> According to Ingo's and Eric's advice[1,2], Try my best to optimize the
>> init of Interrupt Mode for x86.
>>
>> The MP specification defines three different interrupt modes as follows:
>>
>>  1. PIC Mode
>>  2. Virtual Wire Mode
>>  3. Symmetic I/O Mode
>>
>> Currently, In kernel,
>>
>> 1. Setup the Virtual Wire Mode during the IRQ initialization(
>> step 1 in the following figure).
>> 2. Enable and Setup the Symmetic I/O Mode either during the
>> SMP-capabe system prepares CPUs(step 2) or during the UP system
>> initializes itself(step 3).
>>
>>   start_kernel
>> +---------------+
>> |
>> +--> .......
>> |
>> |    setup_arch
>> +--> +-------+
>> |
>> |    init_IRQ
>> +-> +--+-----+
>> |      |        init_ISA_irqs
>> |      +------> +-+--------+
>> |                 |         +----------------+
>> +--->             +------>  | 1.init_bsp_APIC|
>> |     .......               +----------------+
>> +--->
>> |     rest_init
>> +--->---+-----+
>> |       |   kernel_init
>> |       +> ----+-----+
>> |              |   kernel_init_freeable
>> |              +->  ----+-------------+
>> |                       |     smp_prepare_cpus
>> |                       +---> +----+---------+
>> |                       |          |   +-------------------+
>> |                       |          +-> |2.  apic_bsp_setup |
>> |                       |              +-------------------+
>> |                       |
>> v                       |     smp_init
>>                         +---> +---+----+
>>                                   |    +-------------------+
>>                                   +--> |3.  apic_bsp_setup |
>>                                        +-------------------+
>>
>> The purpose of this patchset is Unifing these setup steps and executing as
>> soon as possible as follows:
>>
>>    start_kernel
>> ---------------+
>> |
>> |
>> |
>> |     init_IRQ
>> +---->---+----+
>> |        |
>> |        |      +--------------------+
>> |        +----> | 4. init_bsp_APIC   |
>> |               +--------------------+
>> v
>>
>> By the way, Also fix a bug about kexec[3].
>>
>>
>> Some doubts, need help:
>>
>> 1. Patchset has influence on IOMMU in enable_IR_x2apic(). Not sure
>> it can be in advance?
>>
>> 2. Due to
>>
>> Commit 8c3ba8d04924 ("x86, apic: ack all pending irqs when crashed/on kexec")
>>
>>  ..., patchset also needs TSC and uses the "cpu_khz" in setup_local_APIC().
>> And a warning[4] will be triggered when crashed/on kexec. Not sure how to
>> modify?
>>
>> [1]. https://lkml.org/lkml/2016/8/2/929
>> [2]. https://lkml.org/lkml/2016/8/1/506
>> [3]. https://lkml.org/lkml/2016/7/25/1118
>> [4]. WARN_ON(max_loops <= 0) in setup_local_APIC()
>>
>> Dou Liyang (6):
>>   x86/apic: Replace init_bsp_APIC() with apic_virture_wire_mode_setup()
>>   x86/apic: Construct a framework for setuping APIC mode as soon as
>>     possible
>>   x86/apic: Extract APIC timer related code from apic_bsp_setup()
>>   x86/apic: Make the APIC mode setup earlier for SMP-capable system
>>   x86/apic: Make the APIC mode setup earlier for UP system
>>   x86/apic: Remove the apic_virture_wire_mode_setup()
>>
>>  arch/x86/include/asm/apic.h    |   7 +-
>>  arch/x86/include/asm/io_apic.h |   2 +
>>  arch/x86/kernel/apic/apic.c    | 218 ++++++++++++++++++++++++-----------------
>>  arch/x86/kernel/apic/io_apic.c |   4 +-
>>  arch/x86/kernel/irqinit.c      |   6 +-
>>  arch/x86/kernel/smpboot.c      |  68 ++-----------
>>  6 files changed, 149 insertions(+), 156 deletions(-)
>>
>> --
>> 2.5.5
>>
>>
>>
>
>
>

Re: [RFC PATCH 0/6] Unify the Interrupt Mode and setup it as soon as possible

[Dou Liyang]

At 03/30/2017 11:03 AM, Dou Liyang wrote:
> Hi Baoquan,
>
> At 03/30/2017 10:08 AM, Baoquan He wrote:
>> Hi Liyang,
>>
>> This is awesome. I planned to do this after kaslr back porting, glad to
>> see your posting. I like below diagram and the idea of patch 2/6
>> framework. Will review and see what I can do to help since rhel bug from
>> FJ is assigned to me.
>>
>
> Thanks very much for your join! We have investigated the bug almost
> half a year. :)
>
> In my opinion,
> If we plan to refactor the process of APIC initialization for the bug.
> There must be lots of work need to be done. This patchset is just the
> first step. When I test it, I am thinking about:
>
> 1. The check and logic in each enable and setup LAPIC/IOAPIC functions.
> 2. The process of IRQ remapping.
> 3. The check and init of APIC timer.
> 4. The relationship between the various switches, such as If
> the smp_found_config is 1, the acpi_lapic must be 1.
>
> And following work to me are:
>
> 1. Use more test cases to test.
> 2. learn the IOMMU.
> 3. trace the APIC timer code.
> 4. make the check logic more clear.
>
> Hope to be helpful to you.
>
>> Thanks for the effort!
>>
>> And add Joerg to this thread since he knows IOMMU very well.
>

ahh,

--cc joro@8bytes.org, not joro@8types.org

Thanks
Liyang

> oops, Yes, I forgot it, Thanks!
>
> Thanks
> Liyang
>
>>
>> Thanks
>> Baoquan
>>
>> On 03/29/17 at 10:55pm, Dou Liyang wrote:
>>> According to Ingo's and Eric's advice[1,2], Try my best to optimize the
>>> init of Interrupt Mode for x86.
>>>
>>> The MP specification defines three different interrupt modes as follows:
>>>
>>>  1. PIC Mode
>>>  2. Virtual Wire Mode
>>>  3. Symmetic I/O Mode
>>>
>>> Currently, In kernel,
>>>
>>> 1. Setup the Virtual Wire Mode during the IRQ initialization(
>>> step 1 in the following figure).
>>> 2. Enable and Setup the Symmetic I/O Mode either during the
>>> SMP-capabe system prepares CPUs(step 2) or during the UP system
>>> initializes itself(step 3).
>>>
>>>   start_kernel
>>> +---------------+
>>> |
>>> +--> .......
>>> |
>>> |    setup_arch
>>> +--> +-------+
>>> |
>>> |    init_IRQ
>>> +-> +--+-----+
>>> |      |        init_ISA_irqs
>>> |      +------> +-+--------+
>>> |                 |         +----------------+
>>> +--->             +------>  | 1.init_bsp_APIC|
>>> |     .......               +----------------+
>>> +--->
>>> |     rest_init
>>> +--->---+-----+
>>> |       |   kernel_init
>>> |       +> ----+-----+
>>> |              |   kernel_init_freeable
>>> |              +->  ----+-------------+
>>> |                       |     smp_prepare_cpus
>>> |                       +---> +----+---------+
>>> |                       |          |   +-------------------+
>>> |                       |          +-> |2.  apic_bsp_setup |
>>> |                       |              +-------------------+
>>> |                       |
>>> v                       |     smp_init
>>>                         +---> +---+----+
>>>                                   |    +-------------------+
>>>                                   +--> |3.  apic_bsp_setup |
>>>                                        +-------------------+
>>>
>>> The purpose of this patchset is Unifing these setup steps and
>>> executing as
>>> soon as possible as follows:
>>>
>>>    start_kernel
>>> ---------------+
>>> |
>>> |
>>> |
>>> |     init_IRQ
>>> +---->---+----+
>>> |        |
>>> |        |      +--------------------+
>>> |        +----> | 4. init_bsp_APIC   |
>>> |               +--------------------+
>>> v
>>>
>>> By the way, Also fix a bug about kexec[3].
>>>
>>>
>>> Some doubts, need help:
>>>
>>> 1. Patchset has influence on IOMMU in enable_IR_x2apic(). Not sure
>>> it can be in advance?
>>>
>>> 2. Due to
>>>
>>> Commit 8c3ba8d04924 ("x86, apic: ack all pending irqs when crashed/on
>>> kexec")
>>>
>>>  ..., patchset also needs TSC and uses the "cpu_khz" in
>>> setup_local_APIC().
>>> And a warning[4] will be triggered when crashed/on kexec. Not sure
>>> how to
>>> modify?
>>>
>>> [1]. https://lkml.org/lkml/2016/8/2/929
>>> [2]. https://lkml.org/lkml/2016/8/1/506
>>> [3]. https://lkml.org/lkml/2016/7/25/1118
>>> [4]. WARN_ON(max_loops <= 0) in setup_local_APIC()
>>>
>>> Dou Liyang (6):
>>>   x86/apic: Replace init_bsp_APIC() with apic_virture_wire_mode_setup()
>>>   x86/apic: Construct a framework for setuping APIC mode as soon as
>>>     possible
>>>   x86/apic: Extract APIC timer related code from apic_bsp_setup()
>>>   x86/apic: Make the APIC mode setup earlier for SMP-capable system
>>>   x86/apic: Make the APIC mode setup earlier for UP system
>>>   x86/apic: Remove the apic_virture_wire_mode_setup()
>>>
>>>  arch/x86/include/asm/apic.h    |   7 +-
>>>  arch/x86/include/asm/io_apic.h |   2 +
>>>  arch/x86/kernel/apic/apic.c    | 218
>>> ++++++++++++++++++++++++-----------------
>>>  arch/x86/kernel/apic/io_apic.c |   4 +-
>>>  arch/x86/kernel/irqinit.c      |   6 +-
>>>  arch/x86/kernel/smpboot.c      |  68 ++-----------
>>>  6 files changed, 149 insertions(+), 156 deletions(-)
>>>
>>> --
>>> 2.5.5
>>>
>>>
>>>
>>
>>
>>

Re: [RFC PATCH 0/6] Unify the Interrupt Mode and setup it as soon as possible

[Baoquan He]

On 03/30/17 at 11:09am, Dou Liyang wrote:
> 
> 
> At 03/30/2017 11:03 AM, Dou Liyang wrote:
> > Hi Baoquan,
> > 
> > At 03/30/2017 10:08 AM, Baoquan He wrote:
> > > Hi Liyang,
> > > 
> > > This is awesome. I planned to do this after kaslr back porting, glad to
> > > see your posting. I like below diagram and the idea of patch 2/6
> > > framework. Will review and see what I can do to help since rhel bug from
> > > FJ is assigned to me.
> > > 
> > 
> > Thanks very much for your join! We have investigated the bug almost
> > half a year. :)
> > 
> > In my opinion,
> > If we plan to refactor the process of APIC initialization for the bug.
> > There must be lots of work need to be done. This patchset is just the
> > first step. When I test it, I am thinking about:
> > 
> > 1. The check and logic in each enable and setup LAPIC/IOAPIC functions.
> > 2. The process of IRQ remapping.
> > 3. The check and init of APIC timer.
> > 4. The relationship between the various switches, such as If
> > the smp_found_config is 1, the acpi_lapic must be 1.
> > 
> > And following work to me are:
> > 
> > 1. Use more test cases to test.
> > 2. learn the IOMMU.
> > 3. trace the APIC timer code.
> > 4. make the check logic more clear.
> > 
> > Hope to be helpful to you.

Thanks for telling, I will also check.

> > > 
> > > And add Joerg to this thread since he knows IOMMU very well.
> > 
> 
> ahh,
> 
> --cc joro@8bytes.org, not joro@8types.org

Yes, indeed. Thanks.

> 
> > oops, Yes, I forgot it, Thanks!
> > 
> > Thanks
> > Liyang
> > 
> > > 
> > > Thanks
> > > Baoquan
> > > 
> > > On 03/29/17 at 10:55pm, Dou Liyang wrote:
> > > > According to Ingo's and Eric's advice[1,2], Try my best to optimize the
> > > > init of Interrupt Mode for x86.
> > > > 
> > > > The MP specification defines three different interrupt modes as follows:
> > > > 
> > > >  1. PIC Mode
> > > >  2. Virtual Wire Mode
> > > >  3. Symmetic I/O Mode
> > > > 
> > > > Currently, In kernel,
> > > > 
> > > > 1. Setup the Virtual Wire Mode during the IRQ initialization(
> > > > step 1 in the following figure).
> > > > 2. Enable and Setup the Symmetic I/O Mode either during the
> > > > SMP-capabe system prepares CPUs(step 2) or during the UP system
> > > > initializes itself(step 3).
> > > > 
> > > >   start_kernel
> > > > +---------------+
> > > > |
> > > > +--> .......
> > > > |
> > > > |    setup_arch
> > > > +--> +-------+
> > > > |
> > > > |    init_IRQ
> > > > +-> +--+-----+
> > > > |      |        init_ISA_irqs
> > > > |      +------> +-+--------+
> > > > |                 |         +----------------+
> > > > +--->             +------>  | 1.init_bsp_APIC|
> > > > |     .......               +----------------+
> > > > +--->
> > > > |     rest_init
> > > > +--->---+-----+
> > > > |       |   kernel_init
> > > > |       +> ----+-----+
> > > > |              |   kernel_init_freeable
> > > > |              +->  ----+-------------+
> > > > |                       |     smp_prepare_cpus
> > > > |                       +---> +----+---------+
> > > > |                       |          |   +-------------------+
> > > > |                       |          +-> |2.  apic_bsp_setup |
> > > > |                       |              +-------------------+
> > > > |                       |
> > > > v                       |     smp_init
> > > >                         +---> +---+----+
> > > >                                   |    +-------------------+
> > > >                                   +--> |3.  apic_bsp_setup |
> > > >                                        +-------------------+
> > > > 
> > > > The purpose of this patchset is Unifing these setup steps and
> > > > executing as
> > > > soon as possible as follows:
> > > > 
> > > >    start_kernel
> > > > ---------------+
> > > > |
> > > > |
> > > > |
> > > > |     init_IRQ
> > > > +---->---+----+
> > > > |        |
> > > > |        |      +--------------------+
> > > > |        +----> | 4. init_bsp_APIC   |
> > > > |               +--------------------+
> > > > v
> > > > 
> > > > By the way, Also fix a bug about kexec[3].
> > > > 
> > > > 
> > > > Some doubts, need help:
> > > > 
> > > > 1. Patchset has influence on IOMMU in enable_IR_x2apic(). Not sure
> > > > it can be in advance?
> > > > 
> > > > 2. Due to
> > > > 
> > > > Commit 8c3ba8d04924 ("x86, apic: ack all pending irqs when crashed/on
> > > > kexec")
> > > > 
> > > >  ..., patchset also needs TSC and uses the "cpu_khz" in
> > > > setup_local_APIC().
> > > > And a warning[4] will be triggered when crashed/on kexec. Not sure
> > > > how to
> > > > modify?
> > > > 
> > > > [1]. https://lkml.org/lkml/2016/8/2/929
> > > > [2]. https://lkml.org/lkml/2016/8/1/506
> > > > [3]. https://lkml.org/lkml/2016/7/25/1118
> > > > [4]. WARN_ON(max_loops <= 0) in setup_local_APIC()
> > > > 
> > > > Dou Liyang (6):
> > > >   x86/apic: Replace init_bsp_APIC() with apic_virture_wire_mode_setup()
> > > >   x86/apic: Construct a framework for setuping APIC mode as soon as
> > > >     possible
> > > >   x86/apic: Extract APIC timer related code from apic_bsp_setup()
> > > >   x86/apic: Make the APIC mode setup earlier for SMP-capable system
> > > >   x86/apic: Make the APIC mode setup earlier for UP system
> > > >   x86/apic: Remove the apic_virture_wire_mode_setup()
> > > > 
> > > >  arch/x86/include/asm/apic.h    |   7 +-
> > > >  arch/x86/include/asm/io_apic.h |   2 +
> > > >  arch/x86/kernel/apic/apic.c    | 218
> > > > ++++++++++++++++++++++++-----------------
> > > >  arch/x86/kernel/apic/io_apic.c |   4 +-
> > > >  arch/x86/kernel/irqinit.c      |   6 +-
> > > >  arch/x86/kernel/smpboot.c      |  68 ++-----------
> > > >  6 files changed, 149 insertions(+), 156 deletions(-)
> > > > 
> > > > --
> > > > 2.5.5
> > > > 
> > > > 
> > > > 
> > > 
> > > 
> > > 
> 
> 

Re: [RFC PATCH 1/6] x86/apic: Replace init_bsp_APIC() with apic_virture_wire_mode_setup()

[Thomas Gleixner]

On Wed, 29 Mar 2017, Dou Liyang wrote:

> The init_bsp_APIC() setups the virtual wire mode through the local
> APIC.
> 
> The function name is unsuitable which might imply that the BSP's
> APIC will be initialized here, actually, where it will be done is
> almost at the end of start_kernel(). And the CONFIG X86_64 is also
> imply the X86_LOCAL_APIC is y.

Correct, but X86_32 can have X86_LOCAL_APIC=n. And by removing the ifdefs
you break that.

>  /*
> - * An initial setup of the virtual wire mode.
> + * Setup the through-local-APIC virtual wire mode.
>   */
> -void __init init_bsp_APIC(void)
> +void apic_virture_wire_mode_setup(void)

s/virture/virtual/ ?

Why is this function not longer marked __init ?

Thanks,

	tglx

Re: [RFC PATCH 1/6] x86/apic: Replace init_bsp_APIC() with apic_virture_wire_mode_setup()

[Thomas Gleixner]

On Wed, 5 Apr 2017, Thomas Gleixner wrote:

> On Wed, 29 Mar 2017, Dou Liyang wrote:
> 
> > The init_bsp_APIC() setups the virtual wire mode through the local
> > APIC.
> > 
> > The function name is unsuitable which might imply that the BSP's
> > APIC will be initialized here, actually, where it will be done is
> > almost at the end of start_kernel(). And the CONFIG X86_64 is also
> > imply the X86_LOCAL_APIC is y.
> 
> Correct, but X86_32 can have X86_LOCAL_APIC=n. And by removing the ifdefs
> you break that.

Oops. Sorry, the function is replaced by an empty stub for the APIC=n
case. So that's correct.

> >  /*
> > - * An initial setup of the virtual wire mode.
> > + * Setup the through-local-APIC virtual wire mode.
> >   */
> > -void __init init_bsp_APIC(void)
> > +void apic_virture_wire_mode_setup(void)
> 
> s/virture/virtual/ ?
> 
> Why is this function not longer marked __init ?
> 
> Thanks,
> 
> 	tglx
> 

Re: [RFC PATCH 2/6] x86/apic: Construct a framework for setuping APIC mode as soon as possible

[Thomas Gleixner]

On Wed, 29 Mar 2017, Dou Liyang wrote:

> Now, there are two ways to setup local apic and io-apic in X86 arch:
>   1. In an SMP-capable system, it will be done when preparing the
> cpus in native_smp_prepare_boot_cpu().
>   2. If UP_LATE_INIT is y, it will be done in smp_init()
> 
> And, there are many switches in kernel which can determine the way of
> APIC mode setup, as shown below:
> 
>   1. kconfig :
>      CONFIG_X86_64; CONFIG_X86_LOCAL_APIC; CONFIG_x86_IO_APIC
>   2. kernel option: disable_apic; skip_ioapic_setup
>   3. BIOS : boot_cpu_has(X86_FEATURE_APIC)
>   4. MP table: smp_found_config
>   5. ACPI: acpi_lapic; acpi_ioapic; nr_ioapic
> 
> The setup is late which cause the dump-capture kernel hangs with 'notsc'
> option in 1st kernel option. and the use of these switches is messily.
> 
> Before make the APIC mode setup earlier, construct a framework first to
> prepare for the work and make the logic clear.

Calling that a framework is slightly exaggerated. It's a selector function
for the interrupt delivery mode.

>  #ifdef CONFIG_X86_X2APIC
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index f4fc949..bf4ccd0 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -1153,6 +1153,63 @@ void __init sync_Arb_IDs(void)
>  			APIC_INT_LEVELTRIG | APIC_DM_INIT);
>  }
>  
> +enum apic_bsp_mode {
> +	APIC_BSP_MODEL_PIC = 0,
> +	APIC_BSP_MODEL_VIRTUAL_WIRE,
> +	APIC_BSP_MODEL_SYMMETRIC_IO,
> +	APIC_BSP_MODEL_COUNT
> +};
> +
> +static int __init apic_bsp_mode_check(void)
> +{
> +
> +	/* Check kernel option */
> +	if (disable_apic) {
> +		pr_info("APIC disabled by kernel option\n");

kernel option is ambiguous. "APIC disabled via kernel command line" is telling
clearly where this comes from.

> +		return APIC_BSP_MODEL_PIC;
> +	}
> +	/* Check BOIS */
> +#ifdef CONFIG_X86_64
> +	/* On 64-bit, The APIC is integrated, So, must have APIC feature */
> +	if (!boot_cpu_has(X86_FEATURE_APIC)) {
> +		disable_apic = 1;
> +		pr_info("Apic disabled by BIOS\n");

Please use APIC consistently. 

> +		return APIC_BSP_MODEL_PIC;
> +	}
> +#else
> +	if (!boot_cpu_has(X86_FEATURE_APIC) &&
> +		APIC_INTEGRATED(boot_cpu_apic_version)) {
> +		pr_err("BIOS bug, local APIC #%d not detected!...\n",
> +			boot_cpu_physical_apicid);

Please make that

		pr_err(FW_BUG, "Local APIC %d not detected, force emulation",
		       boot_cpu_physical_apicid);

and for consistency reasosns this should also set disable_apic to 1.

> +		return APIC_BSP_MODEL_PIC;
> +	}
> +#endif
> +	/*
> +	 * Check MP table, if neither an integrated nor a separate chip
> +	 * doesn't exist.
> +	 */
> +	if (!boot_cpu_has(X86_FEATURE_APIC) && !smp_found_config) {
> +		pr_info("BOIS don't support APIC, and no SMP configuration.\n");

s/BOIS/BIOS/

and for consistency reasosns this should also set disable_apic to 1.


> +		return APIC_BSP_MODEL_PIC;
> +	}
> +
> +	/* Check MP table, ps: if the virtual wire has been setup */
> +	if (!smp_found_config) {
> +		disable_ioapic_support();
> +
> +		/* Check local APIC, if SMP_NO_CONFIG */
> +		if (!acpi_lapic)
> +			pr_info("SMP motherboard not detected\n");
> +
> +		return APIC_BSP_MODEL_VIRTUAL_WIRE;
> +	}
> +
> +	/* Other checks of ACPI options will be done in each setup function */
> +
> +	return APIC_BSP_MODEL_SYMMETRIC_IO;
> +}
> +
>  /*
>   * Setup the through-local-APIC virtual wire mode.
>   */
> @@ -1202,6 +1259,22 @@ void apic_virture_wire_mode_setup(void)
>  	apic_write(APIC_LVT1, value);
>  }
>  
> +/* init the interrupt routing model for the BSP */
> +void __init init_bsp_APIC(void)
> +{
> +	switch (apic_bsp_mode_check()) {
> +	case APIC_BSP_MODEL_PIC:
> +		pr_info("Keep in PIC mode(8259)\n");
> +		return;
> +	case APIC_BSP_MODEL_VIRTUAL_WIRE:
> +		pr_info("switch to virtual wire model.\n");

Please consistenly start sentences with upper case letters.

Thanks,

	tglx

Re: [RFC PATCH 3/6] x86/apic: Extract APIC timer related code from apic_bsp_setup()

[Thomas Gleixner]

On Wed, 29 Mar 2017, Dou Liyang wrote:
> +/* Setup local APIC timer and get the Id*/
> +static int __init apic_bsp_timer_setup(void)

This does not make sense. The id and the timers have nothing to do with
each other. 

> +{
> +	int id;
> +
> +	if (x2apic_mode)
> +		id = apic_read(APIC_LDR);
> +	else
> +		id = GET_APIC_LOGICAL_ID(apic_read(APIC_LDR));
> +
> +	if (!skip_ioapic_setup && nr_ioapics && nr_legacy_irqs())
> +		check_timer();

Why are you moving this to the APIC? check_timer() has absolutely nothing
to do with the apic timer. It's a IOAPIC only issue and required to test
that the IRQ0 interrupt delivery works through the IOAPIC.

Thanks,

	tglx

Re: [RFC PATCH 1/6] x86/apic: Replace init_bsp_APIC() with apic_virture_wire_mode_setup()

[Dou Liyang]

Hi Thomas,

Thank you very much!

At 04/05/2017 06:43 PM, Thomas Gleixner wrote:
> On Wed, 29 Mar 2017, Dou Liyang wrote:
>
>> The init_bsp_APIC() setups the virtual wire mode through the local
>> APIC.
>>
>> The function name is unsuitable which might imply that the BSP's
>> APIC will be initialized here, actually, where it will be done is
>> almost at the end of start_kernel(). And the CONFIG X86_64 is also
>> imply the X86_LOCAL_APIC is y.
>
> Correct, but X86_32 can have X86_LOCAL_APIC=n. And by removing the ifdefs
> you break that.
>
>>  /*
>> - * An initial setup of the virtual wire mode.
>> + * Setup the through-local-APIC virtual wire mode.
>>   */
>> -void __init init_bsp_APIC(void)
>> +void apic_virture_wire_mode_setup(void)
>
> s/virture/virtual/ ?

Yes, It is.

>
> Why is this function not longer marked __init ?
>

Oops. The __init is necessary. I will modify it.

Thanks,
	Liyang.


> Thanks,
>
> 	tglx
>
>
>

Re: [RFC PATCH 1/6] x86/apic: Replace init_bsp_APIC() with apic_virture_wire_mode_setup()

[Dou Liyang]

Hi Thomas,

At 04/05/2017 06:50 PM, Thomas Gleixner wrote:
> On Wed, 5 Apr 2017, Thomas Gleixner wrote:
>
>> On Wed, 29 Mar 2017, Dou Liyang wrote:
>>
>>> The init_bsp_APIC() setups the virtual wire mode through the local
>>> APIC.
>>>
>>> The function name is unsuitable which might imply that the BSP's
>>> APIC will be initialized here, actually, where it will be done is
>>> almost at the end of start_kernel(). And the CONFIG X86_64 is also
>>> imply the X86_LOCAL_APIC is y.
>>
>> Correct, but X86_32 can have X86_LOCAL_APIC=n. And by removing the ifdefs
>> you break that.
>
> Oops. Sorry, the function is replaced by an empty stub for the APIC=n
> case. So that's correct.
>

I forgot to clear it in the change log. I will add this comment to the
change log.

Thanks,
	Liyang

>>>  /*
>>> - * An initial setup of the virtual wire mode.
>>> + * Setup the through-local-APIC virtual wire mode.
>>>   */
>>> -void __init init_bsp_APIC(void)
>>> +void apic_virture_wire_mode_setup(void)
>>
>> s/virture/virtual/ ?
>>
>> Why is this function not longer marked __init ?
>>
>> Thanks,
>>
>> 	tglx
>>
>
>
>

Re: [RFC PATCH 2/6] x86/apic: Construct a framework for setuping APIC mode as soon as possible

[Dou Liyang]

Hi Thomas,

At 04/05/2017 07:46 PM, Thomas Gleixner wrote:
> On Wed, 29 Mar 2017, Dou Liyang wrote:
>
>> Now, there are two ways to setup local apic and io-apic in X86 arch:
>>   1. In an SMP-capable system, it will be done when preparing the
>> cpus in native_smp_prepare_boot_cpu().
>>   2. If UP_LATE_INIT is y, it will be done in smp_init()
>>
>> And, there are many switches in kernel which can determine the way of
>> APIC mode setup, as shown below:
>>
>>   1. kconfig :
>>      CONFIG_X86_64; CONFIG_X86_LOCAL_APIC; CONFIG_x86_IO_APIC
>>   2. kernel option: disable_apic; skip_ioapic_setup
>>   3. BIOS : boot_cpu_has(X86_FEATURE_APIC)
>>   4. MP table: smp_found_config
>>   5. ACPI: acpi_lapic; acpi_ioapic; nr_ioapic
>>
>> The setup is late which cause the dump-capture kernel hangs with 'notsc'
>> option in 1st kernel option. and the use of these switches is messily.
>>
>> Before make the APIC mode setup earlier, construct a framework first to
>> prepare for the work and make the logic clear.
>
> Calling that a framework is slightly exaggerated. It's a selector function
> for the interrupt delivery mode.
>

Yes, Selector is what I want.

To tell the truth, I've been looking for a word that can summarize this
patch for a long time. :)


>>  #ifdef CONFIG_X86_X2APIC
>> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
>> index f4fc949..bf4ccd0 100644
>> --- a/arch/x86/kernel/apic/apic.c
>> +++ b/arch/x86/kernel/apic/apic.c
>> @@ -1153,6 +1153,63 @@ void __init sync_Arb_IDs(void)
>>  			APIC_INT_LEVELTRIG | APIC_DM_INIT);
>>  }
>>
>> +enum apic_bsp_mode {
>> +	APIC_BSP_MODEL_PIC = 0,
>> +	APIC_BSP_MODEL_VIRTUAL_WIRE,
>> +	APIC_BSP_MODEL_SYMMETRIC_IO,
>> +	APIC_BSP_MODEL_COUNT
>> +};
>> +
>> +static int __init apic_bsp_mode_check(void)
>> +{
>> +
>> +	/* Check kernel option */
>> +	if (disable_apic) {
>> +		pr_info("APIC disabled by kernel option\n");
>
> kernel option is ambiguous. "APIC disabled via kernel command line" is telling
> clearly where this comes from.
>

Thanks, will modify in my next version.

>> +		return APIC_BSP_MODEL_PIC;
>> +	}
>> +	/* Check BOIS */
>> +#ifdef CONFIG_X86_64
>> +	/* On 64-bit, The APIC is integrated, So, must have APIC feature */
>> +	if (!boot_cpu_has(X86_FEATURE_APIC)) {
>> +		disable_apic = 1;
>> +		pr_info("Apic disabled by BIOS\n");
>
> Please use APIC consistently.

Oops, Yes.

>
>> +		return APIC_BSP_MODEL_PIC;
>> +	}
>> +#else
>> +	if (!boot_cpu_has(X86_FEATURE_APIC) &&
>> +		APIC_INTEGRATED(boot_cpu_apic_version)) {
>> +		pr_err("BIOS bug, local APIC #%d not detected!...\n",
>> +			boot_cpu_physical_apicid);
>
> Please make that
>
> 		pr_err(FW_BUG, "Local APIC %d not detected, force emulation",
> 		       boot_cpu_physical_apicid);
>
> and for consistency reasosns this should also set disable_apic to 1.
>
>> +		return APIC_BSP_MODEL_PIC;

Yes. I will.

>> +	}
>> +#endif
>> +	/*
>> +	 * Check MP table, if neither an integrated nor a separate chip
>> +	 * doesn't exist.
>> +	 */
>> +	if (!boot_cpu_has(X86_FEATURE_APIC) && !smp_found_config) {
>> +		pr_info("BOIS don't support APIC, and no SMP configuration.\n");
>
> s/BOIS/BIOS/
>
> and for consistency reasosns this should also set disable_apic to 1.
>

OK.

>
>> +		return APIC_BSP_MODEL_PIC;
>> +	}
>> +
>> +	/* Check MP table, ps: if the virtual wire has been setup */
>> +	if (!smp_found_config) {
>> +		disable_ioapic_support();
>> +
>> +		/* Check local APIC, if SMP_NO_CONFIG */
>> +		if (!acpi_lapic)
>> +			pr_info("SMP motherboard not detected\n");
>> +
>> +		return APIC_BSP_MODEL_VIRTUAL_WIRE;
>> +	}
>> +
>> +	/* Other checks of ACPI options will be done in each setup function */
>> +
>> +	return APIC_BSP_MODEL_SYMMETRIC_IO;
>> +}
>> +
>>  /*
>>   * Setup the through-local-APIC virtual wire mode.
>>   */
>> @@ -1202,6 +1259,22 @@ void apic_virture_wire_mode_setup(void)
>>  	apic_write(APIC_LVT1, value);
>>  }
>>
>> +/* init the interrupt routing model for the BSP */
>> +void __init init_bsp_APIC(void)
>> +{
>> +	switch (apic_bsp_mode_check()) {
>> +	case APIC_BSP_MODEL_PIC:
>> +		pr_info("Keep in PIC mode(8259)\n");
>> +		return;
>> +	case APIC_BSP_MODEL_VIRTUAL_WIRE:
>> +		pr_info("switch to virtual wire model.\n");
>
> Please consistently start sentences with upper case letters.

Yes, I will check all the sentences and comments again carefully.

Thanks,
	Liyang
>
> Thanks,
>
> 	tglx
>
>
>

Re: [RFC PATCH 0/6] Unify the Interrupt Mode and setup it as soon as possible

[Thomas Gleixner]

On Wed, 29 Mar 2017, Dou Liyang wrote:
> The purpose of this patchset is Unifing these setup steps and executing as
> soon as possible as follows:
> 
>    start_kernel
> ---------------+
> |
> |
> |
> |     init_IRQ
> +---->---+----+
> |        |
> |        |      +--------------------+
> |        +----> | 4. init_bsp_APIC   |
> |               +--------------------+
> v
> 
> By the way, Also fix a bug about kexec[3].
> 
> 
> Some doubts, need help:
> 
> 1. Patchset has influence on IOMMU in enable_IR_x2apic(). Not sure
> it can be in advance?

That should work.

> 2. Due to 
> 
> Commit 8c3ba8d04924 ("x86, apic: ack all pending irqs when crashed/on kexec")
> 
>  ..., patchset also needs TSC and uses the "cpu_khz" in setup_local_APIC().
> And a warning[4] will be triggered when crashed/on kexec. Not sure how to
> modify?

The local APIC timer initialization cannot be run from init_IRQ().

The problem here is that CPU and TSC frequency calibration depends on the
PIT/HPET interrupt (irq 0) working for machines which cannot calibrate via
MSR/CPUID or if fast calibration via PIT fails.

So we need to split that initialization into several parts:

1) Set up the APIC/IOAPIC (including testing whether the timer interrupt
   works)

2) Calibrate TSC

3) Set up the local APIC timer

Thanks,

	tglx

Re: [RFC PATCH 3/6] x86/apic: Extract APIC timer related code from apic_bsp_setup()

[Dou Liyang]

Hi Thomas,

At 04/05/2017 07:56 PM, Thomas Gleixner wrote:
> On Wed, 29 Mar 2017, Dou Liyang wrote:
>> +/* Setup local APIC timer and get the Id*/
>> +static int __init apic_bsp_timer_setup(void)
>
> This does not make sense. The id and the timers have nothing to do with
> each other.

Yes, Indeed. Here is more like the rest work for APIC setup, which
can't be executed earlier, The name is not suitable. I will refactor
it.

>> +{
>> +	int id;
>> +
>> +	if (x2apic_mode)
>> +		id = apic_read(APIC_LDR);
>> +	else
>> +		id = GET_APIC_LOGICAL_ID(apic_read(APIC_LDR));
>> +
>> +	if (!skip_ioapic_setup && nr_ioapics && nr_legacy_irqs())
>> +		check_timer();
>
> Why are you moving this to the APIC? check_timer() has absolutely nothing

Because, the jiffies is used in check_timer() for checking timer irq 
works(timer_irq_works()) and can't work at the beginning.

So, keep check_timer() in IOAPIC setup function (setup_IO_APIC()) which
has been moved to init_IRQ() will make the kernel not boot up.

> to do with the apic timer. It's a IOAPIC only issue and required to test
> that the IRQ0 interrupt delivery works through the IOAPIC.
>

Yes, I see, split check_timer() is not a good way, and move the APIC
initialization to the end of init_IRQ() is also not well.

Thanks,
	Liyang

> Thanks,
>
> 	tglx
>
>
>

Re: [RFC PATCH 0/6] Unify the Interrupt Mode and setup it as soon as possible

[Dou Liyang]

Hi Thomas,

At 04/06/2017 04:43 PM, Thomas Gleixner wrote:
> On Wed, 29 Mar 2017, Dou Liyang wrote:
>> The purpose of this patchset is Unifing these setup steps and executing as
>> soon as possible as follows:
>>
>>    start_kernel
>> ---------------+
>> |
>> |
>> |
>> |     init_IRQ
>> +---->---+----+
>> |        |
>> |        |      +--------------------+
>> |        +----> | 4. init_bsp_APIC   |
>> |               +--------------------+
>> v
>>
>> By the way, Also fix a bug about kexec[3].
>>
>>
>> Some doubts, need help:
>>
>> 1. Patchset has influence on IOMMU in enable_IR_x2apic(). Not sure
>> it can be in advance?
>
> That should work.
>

Got it. Thanks very much.

>> 2. Due to
>>
>> Commit 8c3ba8d04924 ("x86, apic: ack all pending irqs when crashed/on kexec")
>>
>>  ..., patchset also needs TSC and uses the "cpu_khz" in setup_local_APIC().
>> And a warning[4] will be triggered when crashed/on kexec. Not sure how to
>> modify?
>
> The local APIC timer initialization cannot be run from init_IRQ().
>

Yes, I think so.

> The problem here is that CPU and TSC frequency calibration depends on the
> PIT/HPET interrupt (irq 0) working for machines which cannot calibrate via
> MSR/CPUID or if fast calibration via PIT fails.
>

Yes, we use the CPU frequency and tsc before we calibrate them.

> So we need to split that initialization into several parts:
>
> 1) Set up the APIC/IOAPIC (including testing whether the timer interrupt
>    works)
>
> 2) Calibrate TSC
>
> 3) Set up the local APIC timer
>

Yes, correct. the patchset has splited the initialization like that.
And I don't change anything in part 2 to reduce the impact.

For the problem above, the reason is:

When do part 1 in dump-capture kernel, need clear ISR for the pending
interrupt from previous kernel, and use CPU frequency and TSC for
calculating the maximum number of cycles.

But, the CPU frequency and TSC is calibrated in part 2 late than part 1.

So, in part 1, the CPU frequency is 0, and calibrate a wrong maximum
loops.

I try to replace the loops calibration with a new way, which has
nothing to do with TSC, such as set a fixed value for max_loops. Is
that OK?

Or, may need do some work in part 1, 2, re-split the initialization.


Here is the code for clearing ISR:

unsigned long long tsc = 0, ntsc;
	long long max_loops = cpu_khz ? cpu_khz : 1000000;

	if (boot_cpu_has(X86_FEATURE_TSC))
		tsc = rdtsc();
......
......

	do {
		queued = 0;
		for (i = APIC_ISR_NR - 1; i >= 0; i--)
			queued |= apic_read(APIC_IRR + i*0x10);

		for (i = APIC_ISR_NR - 1; i >= 0; i--) {
			value = apic_read(APIC_ISR + i*0x10);
			for (j = 31; j >= 0; j--) {
				if (value & (1<<j)) {
					ack_APIC_irq();
					acked++;
				}
			}
		}
		if (acked > 256) {
			printk(KERN_ERR "LAPIC pending interrupts after %d EOI\n",
			       acked);
			break;
		}
		if (queued) {
			if (boot_cpu_has(X86_FEATURE_TSC) && cpu_khz) {
				ntsc = rdtsc();
				max_loops = (cpu_khz << 10) - (ntsc - tsc);
			} else
				max_loops--;
		}
	} while (queued && max_loops > 0);
	WARN_ON(max_loops <= 0);
... ...


Thanks,
	Liyang.

> Thanks,
>
> 	tglx
>
>
>