Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1687085 > unrolled thread

[PATCH v7 00/13] Unify the interrupt delivery mode and do its setup in advance

Started byDou Liyang <douly.fnst@cn.fujitsu.com>
First post2017-07-14 08:00 +0200
Last post2017-07-14 08:00 +0200
Articles 3 — 1 participant

Back to article view | Back to linux.kernel

Contents

  [PATCH v7 00/13] Unify the interrupt delivery mode and do its setup in advance Dou Liyang <douly.fnst@cn.fujitsu.com> - 2017-07-14 08:00 +0200
    [PATCH v7 02/13] x86/apic: Prepare for unifying the interrupt delivery modes setup Dou Liyang <douly.fnst@cn.fujitsu.com> - 2017-07-14 08:00 +0200
    [PATCH v7 04/13] x86/apic: Move logical APIC ID away from apic_bsp_setup() Dou Liyang <douly.fnst@cn.fujitsu.com> - 2017-07-14 08:00 +0200

#1687085 — [PATCH v7 00/13] Unify the interrupt delivery mode and do its setup in advance

FromDou Liyang <douly.fnst@cn.fujitsu.com>
Date2017-07-14 08:00 +0200
Subject[PATCH v7 00/13] Unify the interrupt delivery mode and do its setup in advance
Message-ID<u31NT-7vg-3@gated-at.bofh.it>
[Background]

MP specification defines three different interrupt delivery modes as follows:

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

They will be setup in the different periods of booting time:
 1. *PIC Mode*, the default interrupt delivery modes, will be set first.
 2. *Virtual Wire Mode* will be setup during ISA IRQ initialization( step 1
    in the figure.1).
 3. *Symmetric I/O Mode*'s setup is related to the system
    3.1 In SMP-capable system, setup during prepares CPUs(step 2) 
    3.2 In UP system, setup during 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 |
                                       +-------------------+
    figure.1 The flow chart of the kernel startup process

[Problem]

1. Cause kernel in an unmatched mode at the beginning of booting time.
2. Cause the dump-capture kernel hangs with 'notsc' option inherited
   from 1st kernel option.
3. Cause the code hard to read and maintain.

As Ingo's and Eric's discusses[1,2], it need to be refactor.

[Solution]

1. Construct a selector to unify these switches

       +------------+
       |disable_apic+--------------------+
       +------------+       true         |
              |false                     |
              |                          |
 +------------v------------------+       |
 |!boot_cpu_has(X86_FEATURE_APIC)+-------+
 +-------------------------------+  true |
              |false                     |
              |                          |
      +-------v---------+                v
      |!smp_found_config|            PIC MODE
      +---------------+-+
       |false         |true
       |              |
       v          +---v---------+
SYMMETRIC IO MODE | !acpi_lapic |
                  +------+------+
                         |
                         v
                   VIRTUAL WIRE MODE 

2. Unifying these setup steps of SMP-capable and UP system

   start_kernel
---------------+
|
|
|
|    x86_late_time_init
+---->---+------------+
|        |
|        |      +------------------------+
|        +----> | 4. init_interrupt_mode |
|               +------------------------+
v


3. Execute the function as soon as possible.

[Test]

1. In a theoretical code analysis, the patchset can wrap the original
logic.

1) The original logic of the interrupt delivery mode setup：

-Step O_1) Keep in PIC mode or virtual wire mode:

  Check (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC))
    true:  PIC mode
    false: virtual wire mode

-Step O_2) Try to switch to symmetric IO mode:
  O_2_1) In up system:

    -Check disable_apic
      ture: O_S_1 (original situation 1)
    -Check whether there is a separate or integrated chip
      don't has: O_S_2
    -Check !smp_found_config
      ture: O_S_3
    -Others:
      O_S_4

  O_2_2) In smp-capable system:

    -Check !smp_found_config && !acpi_lapic
      true: goto O_2_1
    -Check if it is LAPIC
      don't has: O_S_5
    -Check !max_cpus
      true: O_S_6
    -read_apic_id() != boot_cpu_physical_apicid
      true: O_S_7
    -Others:
            O_S_8

2) After that patchset, the new logic:

-Step N_1) Skip step O_1 and try to switch to the final interrupt mode
   -Check disable_apic
     ture: N_S_1 (New situation 1)
   -Check whether there is a separate or integrated chip
     ture: N_S_2
   -Check if (!smp_found_config)
     ture: N_S_3
   -Check !setup_max_cpus
     ture: N_S_4
   -Check read_apic_id() != boot_cpu_physical_apicid
     ture: N_S_5
   -Others:
           N_S_6

O_S_1 is covered in N_S_1
O_S_2 is covered in N_S_2
O_S_3 is covered in N_S_3
O_S_4 is covered in N_S_6
O_S_5 is covered in N_S_2
O_S_6 is covered in N_S_4
O_S_7 is covered in N_S_5
O_S_8 is covered in N_S_6

2. In the actual test, It also can work well in the situations of
my test matrix

The factors of test matrix:

 X86  | SMP |LOCAL APIC|I/O APIC|UP_LATE_INIT|
----- |-----|----------|--------|------------|
32-bit|  Y  |     Y    |    Y   |     Y      |
64-bit|  N  |     N    |    N   |     N      |
xen PV |  
xen HVM|

disable_apic|X86_FEATURE_APIC|smp_found_config|
------------|----------------|----------------|
      0     |        0       |        0       |
      1     |        1       |        1       |

acpi_lapic|acpi_ioapic|setup_max_cpus|
----------|-----------|--------------|
     0    |     0     |      =0      |
     1    |     1     |      >0      |

[Link]

[1]. https://lkml.org/lkml/2016/8/2/929
[2]. https://lkml.org/lkml/2016/8/1/506

For previous discussion, please refer to:
  https://lkml.org/lkml/2017/5/10/323
  https://www.spinics.net/lists/kernel/msg2491620.html
  https://lkml.org/lkml/2017/3/29/481
  https://lkml.org/lkml/2017/6/30/17
  https://lkml.org/lkml/2017/7/3/269

Changes V6 --> V7:

  - add a new patch: p12

Changes V5 --> V6:

  - change the check order for X86_32 in apic_intr_mode_select()
  - replace the apic_printk with pr_info in apic_intr_mode_init()
  - add a seperate helper function for get the logical apicid
  - remove the extra argument upmode in apic_intr_mode_select()
  - cleanup the logic of apic_intr_mode_init()
  - replcae the 'ticks = jiffies' with 'end = jiffies + 4'
  - rewrite the 9th and 10th patches's changelog

Changes V4 --> V5:

  - remove the RFC presix
  - remove the 1/12 patch in V4
  - merge 2 patches together for SMP-capable system
  - replace the *_interrupt_* with *_intr_*
  - replace the pr_info with apic_printk in apic_intr_mode_init()
  - add a patch for PV xen to bypass intr_mode_init()

Changes V3 --> V4:

  - Move interrupt_mode_init to x86_init_ops instead of the use of
    header files
  - Replace "return" with "break" in case of APIC_SYMMETRIC_IO_NO_ROUTING
  - Setup upmode earlier for UP system.
  - Check interrupt mode before per cpu clock event setup.

Changes V2 --> V3:

  - Rebase the patches.
  - Change two function name:
      apic_bsp_mode_check --> apic_interrupt_mode_select
      init_interrupt_mode --> apic_interrupt_mode_init
  - Find a new waiting way to check whether timer IRQs work or not
  - Refine the switch logic in apic_interrupt_mode_init()
  - Consistently start sentences with upper case letters
  - Fix some typos and comments
  - Try my best to rewrite some changelog again

Changes since V1: 

  - Move the initialization from init_IRQ() to x86_late_time_init()
  - Use a threshold to refactor the check logic in timer_irq_works()
  - Rename the framework to a selector
  - Split two patches
  - Consistently start sentences with upper case letters
  - Fix some typos
  - Rewrite the changelog


Dou Liyang (13):
  x86/apic: Construct a selector for the interrupt delivery mode
  x86/apic: Prepare for unifying the interrupt delivery modes setup
  x86/apic: Split local APIC timer setup from the APIC setup
  x86/apic: Move logical APIC ID away from apic_bsp_setup()
  x86/apic: Unify interrupt mode setup for SMP-capable system
  x86/apic: Mark the apic_intr_mode extern for sanity check cleanup
  x86/apic: Unify interrupt mode setup for UP system
  x86/ioapic: Refactor the delay logic in timer_irq_works()
  x86/init: add intr_mode_init to x86_init_ops
  x86/xen: Bypass intr mode setup in enlighten_pv system
  x86/time: Initialize interrupt mode behind timer init
  ACPI / init: Invoke early ACPI initialization earlier
  x86/apic: Remove the init_bsp_APIC()

 arch/x86/include/asm/apic.h     |  15 +++-
 arch/x86/include/asm/x86_init.h |   2 +
 arch/x86/kernel/apic/apic.c     | 190 +++++++++++++++++++++-------------------
 arch/x86/kernel/apic/io_apic.c  |  45 +++++++++-
 arch/x86/kernel/irqinit.c       |   3 -
 arch/x86/kernel/smpboot.c       |  80 +++++------------
 arch/x86/kernel/time.c          |   5 ++
 arch/x86/kernel/x86_init.c      |   1 +
 arch/x86/xen/enlighten_pv.c     |   1 +
 init/main.c                     |   2 +-
 10 files changed, 188 insertions(+), 156 deletions(-)

-- 
2.5.5

[toc] | [next] | [standalone]

#1687086 — [PATCH v7 02/13] x86/apic: Prepare for unifying the interrupt delivery modes setup

FromDou Liyang <douly.fnst@cn.fujitsu.com>
Date2017-07-14 08:00 +0200
Subject[PATCH v7 02/13] x86/apic: Prepare for unifying the interrupt delivery modes setup
Message-ID<u31NV-7vg-29@gated-at.bofh.it>
In reply to#1687085
There are three positions for initializing the interrupt delivery
modes:

1) In IRQ initial function, may setup the through-local-APIC
   virtual wire mode.

2) In an SMP-capable system, will try to switch to symmetric I/O
   model when preparing the cpus in native_smp_prepare_cpus().

3) In UP system with UP_LATE_INIT=y, will set up local APIC and
   I/O APIC in smp_init().

Switching to symmetric I/O mode is so late, which causes kernel
in an unmatched mode at the beginning of booting time. And it
causes the dump-capture kernel hangs with 'notsc' option inherited
from 1st kernel option.

Provide a new function to unify that three positions. Preparatory
patch to initialize an interrupt mode directly.

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

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 5f01671..1a970f5 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -128,6 +128,7 @@ extern void disable_local_APIC(void);
 extern void lapic_shutdown(void);
 extern void sync_Arb_IDs(void);
 extern void init_bsp_APIC(void);
+extern void apic_intr_mode_init(void);
 extern void setup_local_APIC(void);
 extern void init_apic_mappings(void);
 void register_lapic_address(unsigned long address);
@@ -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_intr_mode_init(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 01bde03..1a4cc54 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1343,6 +1343,22 @@ void __init init_bsp_APIC(void)
 	apic_write(APIC_LVT1, value);
 }
 
+/* Init the interrupt delivery mode for the BSP */
+void __init apic_intr_mode_init(void)
+{
+	switch (apic_intr_mode_select()) {
+	case APIC_PIC:
+		pr_info("APIC: keep in PIC mode(8259)\n");
+		return;
+	case APIC_VIRTUAL_WIRE:
+		pr_info("APIC: switch to virtual wire mode setup\n");
+		return;
+	case APIC_SYMMETRIC_IO:
+		pr_info("APIC: switch to symmectic I/O mode setup\n");
+		return;
+	}
+}
+
 static void lapic_setup_esr(void)
 {
 	unsigned int oldvalue, value, maxlvt;
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1687087 — [PATCH v7 04/13] x86/apic: Move logical APIC ID away from apic_bsp_setup()

FromDou Liyang <douly.fnst@cn.fujitsu.com>
Date2017-07-14 08:00 +0200
Subject[PATCH v7 04/13] x86/apic: Move logical APIC ID away from apic_bsp_setup()
Message-ID<u31NW-7vg-31@gated-at.bofh.it>
In reply to#1687085
apic_bsp_setup() sets and returns logical APIC ID for initializing
cpu0_logical_apicid in SMP-capable system.

The id has nothing to do with the initialization of local APIC and
I/O APIC. And apic_bsp_setup() should be called for interrupt mode
setup intently.

Move the id setup into a separate helper function for cleanup and
mark apic_bsp_setup() void.

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

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 1a970f5..4e550c7 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -146,7 +146,7 @@ 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 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 5421f7f..62d1abf 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2426,25 +2426,17 @@ static void __init apic_bsp_up_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();
 	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();
-	return id;
 }
 
 /*
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 25697fb..b0f1098 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1287,6 +1287,14 @@ static void __init smp_cpu_index_default(void)
 	}
 }
 
+static void __init smp_get_logical_apicid(void)
+{
+	if (x2apic_mode)
+		cpu0_logical_apicid = apic_read(APIC_LDR);
+	else
+		cpu0_logical_apicid = GET_APIC_LOGICAL_ID(apic_read(APIC_LDR));
+}
+
 /*
  * Prepare for SMP bootup.  The MP table or ACPI has been read
  * earlier.  Just do some sanity checking here and enable APIC mode.
@@ -1347,11 +1355,13 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
 	}
 
 	default_setup_apic_routing();
-	cpu0_logical_apicid = apic_bsp_setup(false);
+	apic_bsp_setup(false);
 
 	/* Setup local timer */
 	x86_init.timers.setup_percpu_clockev();
 
+	smp_get_logical_apicid();
+
 	pr_info("CPU0: ");
 	print_cpu_info(&cpu_data(0));
 
-- 
2.5.5

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web