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

[Dou Liyang]

Hi,

Follow Juergen's advice, +CC xen-devel and linux-acpi

In case a single patch of a series isn't stand alone it would be nice
to receive at least the cover letter of the series in order to know
what its all about.

Thanks,
	dou.

At 08/28/2017 11:20 AM, Dou Liyang wrote:
> Changes V7 --> V8:
>
>   - Change the order of [12/13] patch and [11/13]patch suggested by Rafael J. Wysocki.
>   - Fix some comments.
>   - Do more tests in Thinkpad x121e   -- Thanks for Borislav Petkov's help.
>
> [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
>   https://lkml.org/lkml/2017/7/14/20
>
> 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
>   ACPI / init: Invoke early ACPI initialization earlier
>   x86/time: Initialize interrupt mode behind timer init
>   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     | 188 +++++++++++++++++++++-------------------
>  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, 186 insertions(+), 156 deletions(-)