[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
Now, there are many switches in kernel which are used to determine
the final interrupt delivery mode, 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) CPU Capability: boot_cpu_has(X86_FEATURE_APIC)
4) MP table: smp_found_config
5) ACPI: acpi_lapic; acpi_ioapic; nr_ioapic
These switches are disordered and scattered and there are also some
dependencies with each other. These make the code difficult to
maintain and read.
Construct a selector to unify them into a single function, then,
Use this selector to get an interrupt delivery mode directly.
Signed-off-by: Dou Liyang <[email protected]>
---
arch/x86/kernel/apic/apic.c | 59 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 98b3dd8..01bde03 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1235,6 +1235,65 @@ void __init sync_Arb_IDs(void)
APIC_INT_LEVELTRIG | APIC_DM_INIT);
}
+enum apic_intr_mode {
+ APIC_PIC,
+ APIC_VIRTUAL_WIRE,
+ APIC_SYMMETRIC_IO,
+};
+
+static int __init apic_intr_mode_select(void)
+{
+ /* Check kernel option */
+ if (disable_apic) {
+ pr_info("APIC disabled via kernel command line\n");
+ return APIC_PIC;
+ }
+
+ /* Check BIOS */
+#ifdef CONFIG_X86_64
+ /* On 64-bit, the APIC must be integrated, Check local APIC only */
+ if (!boot_cpu_has(X86_FEATURE_APIC)) {
+ disable_apic = 1;
+ pr_info("APIC disabled by BIOS\n");
+ return APIC_PIC;
+ }
+#else
+ /*
+ * On 32-bit, check whether there is a separate chip or integrated
+ * APIC
+ */
+
+ /* Has a separate chip ? */
+ if (!boot_cpu_has(X86_FEATURE_APIC) && !smp_found_config) {
+ disable_apic = 1;
+
+ return APIC_PIC;
+ }
+
+ /* Has a local APIC ? */
+ if (!boot_cpu_has(X86_FEATURE_APIC) &&
+ APIC_INTEGRATED(boot_cpu_apic_version)) {
+ disable_apic = 1;
+ pr_err(FW_BUG "Local APIC %d not detected, force emulation\n",
+ boot_cpu_physical_apicid);
+
+ return APIC_PIC;
+ }
+#endif
+
+ /* Check MP table or ACPI MADT configuration */
+ if (!smp_found_config) {
+ disable_ioapic_support();
+
+ if (!acpi_lapic)
+ pr_info("APIC: ACPI MADT or MP tables are not detected\n");
+
+ return APIC_VIRTUAL_WIRE;
+ }
+
+ return APIC_SYMMETRIC_IO;
+}
+
/*
* An initial setup of the virtual wire mode.
*/
--
2.5.5
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 <[email protected]>
---
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
In UniProcessor kernel with UP_LATE_INIT=y, it enables and setups
interrupt delivery mode in up_late_init().
Unify it to apic_intr_mode_init(), remove APIC_init_uniprocessor().
Signed-off-by: Dou Liyang <[email protected]>
---
arch/x86/include/asm/apic.h | 1 -
arch/x86/kernel/apic/apic.c | 47 ++++++---------------------------------------
2 files changed, 6 insertions(+), 42 deletions(-)
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 01f3fc8..983a0dc 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -144,7 +144,6 @@ void register_lapic_address(unsigned long address);
extern void setup_boot_APIC_clock(void);
extern void setup_secondary_APIC_clock(void);
extern void lapic_update_tsc_freq(void);
-extern int APIC_init_uniprocessor(void);
#ifdef CONFIG_X86_64
static inline int apic_force_enable(unsigned long addr)
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 7e2b130..8b9d778 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1359,7 +1359,7 @@ void __init init_bsp_APIC(void)
/* Init the interrupt delivery mode for the BSP */
void __init apic_intr_mode_init(void)
{
- bool upmode = false;
+ bool upmode = IS_ENABLED(CONFIG_UP_LATE_INIT);
apic_intr_mode = apic_intr_mode_select();
@@ -2470,51 +2470,16 @@ 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)
+#ifdef CONFIG_UP_LATE_INIT
+void __init up_late_init(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;
+ apic_intr_mode_init();
- /*
- * 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();
+ if (apic_intr_mode == APIC_PIC)
+ return;
- default_setup_apic_routing();
- apic_bsp_setup(true);
/* Setup local timer */
x86_init.timers.setup_percpu_clockev();
- return 0;
-}
-
-#ifdef CONFIG_UP_LATE_INIT
-void __init up_late_init(void)
-{
- APIC_init_uniprocessor();
}
#endif
--
2.5.5
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 <[email protected]>
---
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
In the SMP-capable system, enable and setup the interrupt delivery
mode in native_smp_prepare_cpus().
This design mixs the APIC and SMP together, it has highly coupling.
Make the initialization of interrupt mode independent, Unify and
refine it to apic_intr_mode_init() for SMP-capable system.
Signed-off-by: Dou Liyang <[email protected]>
---
arch/x86/kernel/apic/apic.c | 41 ++++++++++++++++++++++++++++++++++++++---
arch/x86/kernel/smpboot.c | 14 ++------------
2 files changed, 40 insertions(+), 15 deletions(-)
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 62d1abf..bf29778 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1238,7 +1238,9 @@ void __init sync_Arb_IDs(void)
enum apic_intr_mode {
APIC_PIC,
APIC_VIRTUAL_WIRE,
+ APIC_VIRTUAL_WIRE_NO_CONFIG,
APIC_SYMMETRIC_IO,
+ APIC_SYMMETRIC_IO_NO_ROUTING,
};
static int __init apic_intr_mode_select(void)
@@ -1285,12 +1287,29 @@ static int __init apic_intr_mode_select(void)
if (!smp_found_config) {
disable_ioapic_support();
- if (!acpi_lapic)
+ if (!acpi_lapic) {
pr_info("APIC: ACPI MADT or MP tables are not detected\n");
+ return APIC_VIRTUAL_WIRE_NO_CONFIG;
+ }
+
return APIC_VIRTUAL_WIRE;
}
+#ifdef CONFIG_SMP
+ /* If SMP should be disabled, then really disable it! */
+ if (!setup_max_cpus) {
+ pr_info("APIC: SMP mode deactivated\n");
+ return APIC_SYMMETRIC_IO_NO_ROUTING;
+ }
+
+ 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? */
+ }
+#endif
+
return APIC_SYMMETRIC_IO;
}
@@ -1346,17 +1365,33 @@ void __init init_bsp_APIC(void)
/* Init the interrupt delivery mode for the BSP */
void __init apic_intr_mode_init(void)
{
+ bool upmode = false;
+
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;
+ default_setup_apic_routing();
+ break;
+ case APIC_VIRTUAL_WIRE_NO_CONFIG:
+ pr_info("APIC: switch to virtual wire mode setup "
+ "with no configuration\n");
+ upmode = true;
+ default_setup_apic_routing();
+ break;
case APIC_SYMMETRIC_IO:
pr_info("APIC: switch to symmectic I/O mode setup\n");
- return;
+ default_setup_apic_routing();
+ break;
+ case APIC_SYMMETRIC_IO_NO_ROUTING:
+ pr_info("APIC: switch to symmectic I/O mode setup "
+ "in no SMP routine\n");
+ break;
}
+
+ apic_bsp_setup(upmode);
}
static void lapic_setup_esr(void)
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index b0f1098..5ca41a5 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1329,18 +1329,17 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
set_cpu_sibling_map(0);
+ apic_intr_mode_init();
+
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:
disable_smp();
- apic_bsp_setup(false);
/* Setup local timer */
x86_init.timers.setup_percpu_clockev();
return;
@@ -1348,15 +1347,6 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
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();
- apic_bsp_setup(false);
-
/* Setup local timer */
x86_init.timers.setup_percpu_clockev();
--
2.5.5
Kernel use timer_irq_works() to detects the timer IRQs. It calls
mdelay(10) to delay ten ticks and check whether the timer IRQ work
or not. The mdelay() depends on the loops_per_jiffy which is set up
in calibrate_delay(). Current kernel defaults the IRQ 0 is available
when it calibrates delay.
But it is wrong in the dump-capture kernel with 'notsc' option inherited
from 1st kernel option. dump-capture kernel can't make sure the timer IRQ
works well.
The correct design is making the interrupt mode setup and checking timer
IRQ works in advance of calibrate_delay(). That results in the mdelay()
being unusable in timer_irq_works().
Preparatory patch to make the setup in advance. Refactor the delay logic
by waiting for some cycles. In the system with X86_FEATURE_TSC feature,
Use rdtsc(), others will call __delay() directly.
Note: regard 4G as the max CPU frequence of current single CPU.
Signed-off-by: Dou Liyang <[email protected]>
---
arch/x86/kernel/apic/io_apic.c | 45 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 43 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index b4f5f73..a87a4ba 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1585,6 +1585,43 @@ static int __init notimercheck(char *s)
}
__setup("no_timer_check", notimercheck);
+static void __init delay_with_tsc(void)
+{
+ unsigned long long start, now;
+ unsigned long end = jiffies + 4;
+
+ start = rdtsc();
+
+ /*
+ * We don't know the TSC frequency yet, but waiting for
+ * 40000000000/HZ TSC cycles is safe:
+ * 4 GHz == 10 jiffies
+ * 1 GHz == 40 jiffies
+ */
+ do {
+ rep_nop();
+ now = rdtsc();
+ } while ((now - start) < 40000000000UL / HZ &&
+ time_before_eq(jiffies, end));
+}
+
+static void __init delay_without_tsc(void)
+{
+ unsigned long end = jiffies + 4;
+ int band = 1;
+
+ /*
+ * We don't know any frequency yet, but waiting for
+ * 40940000000/HZ cycles is safe:
+ * 4 GHz == 10 jiffies
+ * 1 GHz == 40 jiffies
+ * 1 << 1 + 1 << 2 +...+ 1 << 11 = 4094
+ */
+ do {
+ __delay(((1U << band++) * 10000000UL) / HZ);
+ } while (band < 12 && time_before_eq(jiffies, end));
+}
+
/*
* There is a nasty bug in some older SMP boards, their mptable lies
* about the timer IRQ. We do the following to work around the situation:
@@ -1603,8 +1640,12 @@ static int __init timer_irq_works(void)
local_save_flags(flags);
local_irq_enable();
- /* Let ten ticks pass... */
- mdelay((10 * 1000) / HZ);
+
+ if (boot_cpu_has(X86_FEATURE_TSC))
+ delay_with_tsc();
+ else
+ delay_without_tsc();
+
local_irq_restore(flags);
/*
--
2.5.5
In start_kernel(), firstly, it works on the default interrupy mode, then
switch to the final mode. Normally, Booting with BIOS reset is OK.
But, At dump-capture kernel, it boot up without BIOS reset, default mode
may not be compatible with the actual registers, that causes the delivery
interrupt to fail.
Try to set up the final mode as soon as possible. according to the parts
which split from that initialization:
1) Set up the APIC/IOAPIC (including testing whether the timer
interrupt works)
2) Calibrate TSC
3) Set up the local APIC timer
-- From Thomas Gleixner
Initializing the mode should be earlier than calibrating TSC as soon as
possible and needs testing whether the timer interrupt works at the same
time.
call it behind timers init, which meets the above conditions.
Signed-off-by: Dou Liyang <[email protected]>
---
arch/x86/kernel/apic/apic.c | 2 --
arch/x86/kernel/smpboot.c | 2 --
arch/x86/kernel/time.c | 5 +++++
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index c5526eb..9503838 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2473,8 +2473,6 @@ void __init apic_bsp_setup(bool upmode)
#ifdef CONFIG_UP_LATE_INIT
void __init up_late_init(void)
{
- x86_init.irqs.intr_mode_init();
-
if (apic_intr_mode == APIC_PIC)
return;
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 6045074..913b595 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1289,8 +1289,6 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
set_cpu_sibling_map(0);
- x86_init.irqs.intr_mode_init();
-
smp_sanity_check();
switch (apic_intr_mode) {
diff --git a/arch/x86/kernel/time.c b/arch/x86/kernel/time.c
index e0754cd..3ceb834 100644
--- a/arch/x86/kernel/time.c
+++ b/arch/x86/kernel/time.c
@@ -84,6 +84,11 @@ void __init hpet_time_init(void)
static __init void x86_late_time_init(void)
{
x86_init.timers.timer_init();
+ /*
+ * After PIT/HPET timers init, select and setup
+ * the final interrupt mode for delivering IRQs.
+ */
+ x86_init.irqs.intr_mode_init();
tsc_init();
}
--
2.5.5
The init_bsp_APIC() which works for the virtual wire mode is used
in ISA irq initialization at the booting time.
Currently, enable and setup the interrupt mode has been unified
and advanced just behind the timer IRQ setup. Kernel switches to
the final interrupt delivery mode directly. So init_bsp_APIC()
is redundant.
Remove the init_bsp_APIC() function.
Signed-off-by: Dou Liyang <[email protected]>
---
arch/x86/include/asm/apic.h | 1 -
arch/x86/kernel/apic/apic.c | 49 ---------------------------------------------
arch/x86/kernel/irqinit.c | 3 ---
3 files changed, 53 deletions(-)
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 983a0dc..7d247b2 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -136,7 +136,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 init_bsp_APIC(void);
extern void apic_intr_mode_init(void);
extern void setup_local_APIC(void);
extern void init_apic_mappings(void);
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 9503838..12f338e 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1307,55 +1307,6 @@ static int __init apic_intr_mode_select(void)
return APIC_SYMMETRIC_IO;
}
-/*
- * An initial setup of the virtual wire mode.
- */
-void __init init_bsp_APIC(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 delivery mode for the BSP */
void __init apic_intr_mode_init(void)
{
diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c
index 7468c69..488c9e2 100644
--- a/arch/x86/kernel/irqinit.c
+++ b/arch/x86/kernel/irqinit.c
@@ -72,9 +72,6 @@ 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
legacy_pic->init(0);
for (i = 0; i < nr_legacy_irqs(); i++)
--
2.5.5
Linux uses acpi_early_init() to put the ACPI table management into
the late stage from the early stage where the mapped ACPI tables is
temporary and should be unmapped.
But, now initializing interrupt delivery mode should map and parse the
DMAR table earlier in the early stage. This causes an ACPI error when
Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
the DMAR table after using in the early stage.
Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
be mapped and parsed in late stage like before.
Reported-by: Xiaolong Ye <[email protected]>
Signed-off-by: Dou Liyang <[email protected]>
Cc: [email protected]
Cc: Rafael J. Wysocki <[email protected]>
Cc: Zheng, Lv <[email protected]>
Cc: Julian Wollrath <[email protected]>
---
Test in my own PC(Lenovo M4340).
Ask help for doing regression testing for the bug said in commit c4e1acbb35e4
("ACPI / init: Invoke early ACPI initialization later").
init/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/init/main.c b/init/main.c
index df58a41..7a09467 100644
--- a/init/main.c
+++ b/init/main.c
@@ -654,12 +654,12 @@ asmlinkage __visible void __init start_kernel(void)
kmemleak_init();
setup_per_cpu_pageset();
numa_policy_init();
+ acpi_early_init();
if (late_time_init)
late_time_init();
calibrate_delay();
pidmap_init();
anon_vma_init();
- acpi_early_init();
#ifdef CONFIG_X86
if (efi_enabled(EFI_RUNTIME_SERVICES))
efi_enter_virtual_mode();
--
2.5.5
XEN PV overrides smp_prepare_cpus(). xen_pv_smp_prepare_cpus()
initializes interrupts in the XEN PV specific way and does not invoke
native_smp_prepare_cpus(). As a consequence, x86_init.intr_mode_init() is
not invoked either.
The invocation of x86_init.intr_mode_init() will be moved from
native_smp_prepare_cpus() in a follow up patch to solve <INSERT
REASON/PROBLEM>.
That move would cause the invocation of x86_init.intr_mode_init() for XEN
PV platforms. To prevent that, override the default x86_init.intr_mode_init()
callback with a noop().
[Rewrited by Thomas Gleixner <[email protected]>]
Signed-off-by: Dou Liyang <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/x86/xen/enlighten_pv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 811e4dd..07147dd 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -1250,6 +1250,7 @@ asmlinkage __visible void __init xen_start_kernel(void)
x86_platform.get_nmi_reason = xen_get_nmi_reason;
x86_init.resources.memory_setup = xen_memory_setup;
+ x86_init.irqs.intr_mode_init = x86_init_noop;
x86_init.oem.arch_setup = xen_arch_setup;
x86_init.oem.banner = xen_banner;
--
2.5.5
X86 and XEN initialize interrupt delivery mode in different way.
Ordinary conditional function calls will make the code mess.
Add an unconditional x86_init_ops function which defaults to the
standard function and can be overridden by the early platform code.
Signed-off-by: Dou Liyang <[email protected]>
---
arch/x86/include/asm/x86_init.h | 2 ++
arch/x86/kernel/apic/apic.c | 2 +-
arch/x86/kernel/smpboot.c | 2 +-
arch/x86/kernel/x86_init.c | 1 +
4 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index 7ba7e90..f45acdf 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -50,11 +50,13 @@ struct x86_init_resources {
* are set up.
* @intr_init: interrupt init code
* @trap_init: platform specific trap setup
+ * @intr_mode_init: interrupt delivery mode setup
*/
struct x86_init_irqs {
void (*pre_vector_init)(void);
void (*intr_init)(void);
void (*trap_init)(void);
+ void (*intr_mode_init)(void);
};
/**
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 8b9d778..c5526eb 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2473,7 +2473,7 @@ void __init apic_bsp_setup(bool upmode)
#ifdef CONFIG_UP_LATE_INIT
void __init up_late_init(void)
{
- apic_intr_mode_init();
+ x86_init.irqs.intr_mode_init();
if (apic_intr_mode == APIC_PIC)
return;
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index f006f07..6045074 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1289,7 +1289,7 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
set_cpu_sibling_map(0);
- apic_intr_mode_init();
+ x86_init.irqs.intr_mode_init();
smp_sanity_check();
diff --git a/arch/x86/kernel/x86_init.c b/arch/x86/kernel/x86_init.c
index a088b2c..a7889b9 100644
--- a/arch/x86/kernel/x86_init.c
+++ b/arch/x86/kernel/x86_init.c
@@ -55,6 +55,7 @@ struct x86_init_ops x86_init __initdata = {
.pre_vector_init = init_ISA_irqs,
.intr_init = native_init_IRQ,
.trap_init = x86_init_noop,
+ .intr_mode_init = apic_intr_mode_init
},
.oem = {
--
2.5.5
Calling native_smp_prepare_cpus() to prepare for SMP bootup, does
some sanity checking, enables APIC mode and disables SMP feature.
Now, APIC mode setup has been unified to apic_intr_mode_init(),
some sanity checks are redundant and need to be cleanup.
Mark the apic_intr_mode extern to refine the switch and remove
the redundant sanity check.
Signed-off-by: Dou Liyang <[email protected]>
---
arch/x86/include/asm/apic.h | 9 +++++++
arch/x86/kernel/apic/apic.c | 12 ++++------
arch/x86/kernel/smpboot.c | 57 +++++++--------------------------------------
3 files changed, 22 insertions(+), 56 deletions(-)
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 4e550c7..01f3fc8 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -53,6 +53,15 @@ extern int local_apic_timer_c2_ok;
extern int disable_apic;
extern unsigned int lapic_timer_frequency;
+extern enum apic_intr_mode_id apic_intr_mode;
+enum apic_intr_mode_id {
+ APIC_PIC,
+ APIC_VIRTUAL_WIRE,
+ APIC_VIRTUAL_WIRE_NO_CONFIG,
+ APIC_SYMMETRIC_IO,
+ APIC_SYMMETRIC_IO_NO_ROUTING
+};
+
#ifdef CONFIG_SMP
extern void __inquire_remote_apic(int apicid);
#else /* CONFIG_SMP */
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index bf29778..7e2b130 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1235,13 +1235,7 @@ void __init sync_Arb_IDs(void)
APIC_INT_LEVELTRIG | APIC_DM_INIT);
}
-enum apic_intr_mode {
- APIC_PIC,
- APIC_VIRTUAL_WIRE,
- APIC_VIRTUAL_WIRE_NO_CONFIG,
- APIC_SYMMETRIC_IO,
- APIC_SYMMETRIC_IO_NO_ROUTING,
-};
+enum apic_intr_mode_id apic_intr_mode;
static int __init apic_intr_mode_select(void)
{
@@ -1367,7 +1361,9 @@ void __init apic_intr_mode_init(void)
{
bool upmode = false;
- switch (apic_intr_mode_select()) {
+ apic_intr_mode = apic_intr_mode_select();
+
+ switch (apic_intr_mode) {
case APIC_PIC:
pr_info("APIC: keep in PIC mode(8259)\n");
return;
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 5ca41a5..f006f07 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.
*/
@@ -1250,29 +1233,6 @@ static int __init smp_sanity_check(unsigned max_cpus)
physid_set(hard_smp_processor_id(), phys_cpu_present_map);
}
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)
@@ -1331,19 +1291,20 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
apic_intr_mode_init();
- switch (smp_sanity_check(max_cpus)) {
- case SMP_NO_CONFIG:
- disable_smp();
- return;
- case SMP_NO_APIC:
+ smp_sanity_check();
+
+ switch (apic_intr_mode) {
+ case APIC_PIC:
+ case APIC_VIRTUAL_WIRE_NO_CONFIG:
disable_smp();
return;
- case SMP_FORCE_UP:
+ case APIC_SYMMETRIC_IO_NO_ROUTING:
disable_smp();
/* Setup local timer */
x86_init.timers.setup_percpu_clockev();
return;
- case SMP_OK:
+ case APIC_VIRTUAL_WIRE:
+ case APIC_SYMMETRIC_IO:
break;
}
--
2.5.5
apic_bsp_setup() sets up the local APIC, I/O APIC and APIC timer.
The local APIC and I/O APIC setup belongs to interrupt delivery mode
setup. Setting up the local APIC timer for booting CPU is another job
and has nothing to do with interrupt delivery mode setup.
Split local APIC timer setup from the APIC setup, keep it in the
original position for SMP and UP kernel for preparation.
Signed-off-by: Dou Liyang <[email protected]>
---
arch/x86/kernel/apic/apic.c | 4 ++--
arch/x86/kernel/smpboot.c | 5 +++++
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 1a4cc54..5421f7f 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2444,8 +2444,6 @@ int __init apic_bsp_setup(bool upmode)
end_local_APIC_setup();
irq_remap_enable_fault_handling();
setup_IO_APIC();
- /* Setup local timer */
- x86_init.timers.setup_percpu_clockev();
return id;
}
@@ -2485,6 +2483,8 @@ int __init APIC_init_uniprocessor(void)
default_setup_apic_routing();
apic_bsp_setup(true);
+ /* Setup local timer */
+ x86_init.timers.setup_percpu_clockev();
return 0;
}
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index b474c8d..25697fb 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1333,6 +1333,8 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
case SMP_FORCE_UP:
disable_smp();
apic_bsp_setup(false);
+ /* Setup local timer */
+ x86_init.timers.setup_percpu_clockev();
return;
case SMP_OK:
break;
@@ -1347,6 +1349,9 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
default_setup_apic_routing();
cpu0_logical_apicid = apic_bsp_setup(false);
+ /* Setup local timer */
+ x86_init.timers.setup_percpu_clockev();
+
pr_info("CPU0: ");
print_cpu_info(&cpu_data(0));
--
2.5.5
Hi,
Can the problem be fixed by invoking acpi_put_table() for mapped DMAR table?
Thanks
Lv
> From: Dou Liyang [mailto:[email protected]]
> Sent: Friday, July 14, 2017 1:53 PM
> To: [email protected]; [email protected]
> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; Dou Liyang
> <[email protected]>; [email protected]; Rafael J. Wysocki <[email protected]>; Zheng,
> Lv <[email protected]>; Julian Wollrath <[email protected]>
> Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
>
> Linux uses acpi_early_init() to put the ACPI table management into
> the late stage from the early stage where the mapped ACPI tables is
> temporary and should be unmapped.
>
> But, now initializing interrupt delivery mode should map and parse the
> DMAR table earlier in the early stage. This causes an ACPI error when
> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
> the DMAR table after using in the early stage.
>
> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
> be mapped and parsed in late stage like before.
>
> Reported-by: Xiaolong Ye <[email protected]>
> Signed-off-by: Dou Liyang <[email protected]>
> Cc: [email protected]
> Cc: Rafael J. Wysocki <[email protected]>
> Cc: Zheng, Lv <[email protected]>
> Cc: Julian Wollrath <[email protected]>
> ---
> Test in my own PC(Lenovo M4340).
> Ask help for doing regression testing for the bug said in commit c4e1acbb35e4
> ("ACPI / init: Invoke early ACPI initialization later").
>
> init/main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/init/main.c b/init/main.c
> index df58a41..7a09467 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -654,12 +654,12 @@ asmlinkage __visible void __init start_kernel(void)
> kmemleak_init();
> setup_per_cpu_pageset();
> numa_policy_init();
> + acpi_early_init();
> if (late_time_init)
> late_time_init();
> calibrate_delay();
> pidmap_init();
> anon_vma_init();
> - acpi_early_init();
> #ifdef CONFIG_X86
> if (efi_enabled(EFI_RUNTIME_SERVICES))
> efi_enter_virtual_mode();
> --
> 2.5.5
>
>
Hi, Zheng
At 07/18/2017 01:18 PM, Zheng, Lv wrote:
> Hi,
>
> Can the problem be fixed by invoking acpi_put_table() for mapped DMAR table?
Invoking acpi_put_table() is my first choice. But it made the kernel
*panic* when we try to get the table again in intel_iommu_init() in
late stage.
I am also confused that:
There are two places where we used DMAR table in Linux:
1) In detect_intel_iommu() in ACPI early stage:
...
status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
....
if (dmar_tbl) {
acpi_put_table(dmar_tbl);
dmar_tbl = NULL;
}
2) In dmar_table_init() in ACPI late stage:
...
status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
...
As we know, dmar_table_init() is called by intel_iommu_init() and
intel_prepare_irq_remapping().
When I invoked acpi_put_table() in the intel_prepare_irq_remapping() in
early stage like 1) shows, kernel will panic.
Thanks,
dou.
>
> Thanks
> Lv
>
>> From: Dou Liyang [mailto:[email protected]]
>> Sent: Friday, July 14, 2017 1:53 PM
>> To: [email protected]; [email protected]
>> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected]; Dou Liyang
>> <[email protected]>; [email protected]; Rafael J. Wysocki <[email protected]>; Zheng,
>> Lv <[email protected]>; Julian Wollrath <[email protected]>
>> Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
>>
>> Linux uses acpi_early_init() to put the ACPI table management into
>> the late stage from the early stage where the mapped ACPI tables is
>> temporary and should be unmapped.
>>
>> But, now initializing interrupt delivery mode should map and parse the
>> DMAR table earlier in the early stage. This causes an ACPI error when
>> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
>> the DMAR table after using in the early stage.
>>
>> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
>> be mapped and parsed in late stage like before.
>>
>> Reported-by: Xiaolong Ye <[email protected]>
>> Signed-off-by: Dou Liyang <[email protected]>
>> Cc: [email protected]
>> Cc: Rafael J. Wysocki <[email protected]>
>> Cc: Zheng, Lv <[email protected]>
>> Cc: Julian Wollrath <[email protected]>
>> ---
>> Test in my own PC(Lenovo M4340).
>> Ask help for doing regression testing for the bug said in commit c4e1acbb35e4
>> ("ACPI / init: Invoke early ACPI initialization later").
>>
>> init/main.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/init/main.c b/init/main.c
>> index df58a41..7a09467 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -654,12 +654,12 @@ asmlinkage __visible void __init start_kernel(void)
>> kmemleak_init();
>> setup_per_cpu_pageset();
>> numa_policy_init();
>> + acpi_early_init();
>> if (late_time_init)
>> late_time_init();
>> calibrate_delay();
>> pidmap_init();
>> anon_vma_init();
>> - acpi_early_init();
>> #ifdef CONFIG_X86
>> if (efi_enabled(EFI_RUNTIME_SERVICES))
>> efi_enter_virtual_mode();
>> --
>> 2.5.5
>>
>>
>
>
>
>
On 07/18/17 at 02:08pm, Dou Liyang wrote:
> Hi, Zheng
>
> At 07/18/2017 01:18 PM, Zheng, Lv wrote:
> > Hi,
> >
> > Can the problem be fixed by invoking acpi_put_table() for mapped DMAR table?
>
> Invoking acpi_put_table() is my first choice. But it made the kernel
> *panic* when we try to get the table again in intel_iommu_init() in
> late stage.
>
> I am also confused that:
>
> There are two places where we used DMAR table in Linux:
>
> 1) In detect_intel_iommu() in ACPI early stage:
>
> ...
> status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
> ....
> if (dmar_tbl) {
> acpi_put_table(dmar_tbl);
> dmar_tbl = NULL;
> }
>
> 2) In dmar_table_init() in ACPI late stage:
>
> ...
> status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
> ...
>
> As we know, dmar_table_init() is called by intel_iommu_init() and
> intel_prepare_irq_remapping().
>
> When I invoked acpi_put_table() in the intel_prepare_irq_remapping() in
> early stage like 1) shows, kernel will panic.
That's because acpi_put_table() will make the table pointer be NULL,
while dmar_table_init() will skip parse_dmar_table() calling if
dmar_table_initialized is set to 1 in intel_prepare_irq_remapping().
Dmar hardware support interrupt remapping and io remapping separately. But
intel_iommu_init() is called later than intel_prepare_irq_remapping().
So what if make dmar_table_init() a reentrant function? You can just
have a try, but maybe not a good idea, the dmar table will be parsed
twice.
>
>
> Thanks,
>
> dou.
> >
> > Thanks
> > Lv
> >
> > > From: Dou Liyang [mailto:[email protected]]
> > > Sent: Friday, July 14, 2017 1:53 PM
> > > To: [email protected]; [email protected]
> > > Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected]; [email protected]; Dou Liyang
> > > <[email protected]>; [email protected]; Rafael J. Wysocki <[email protected]>; Zheng,
> > > Lv <[email protected]>; Julian Wollrath <[email protected]>
> > > Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
> > >
> > > Linux uses acpi_early_init() to put the ACPI table management into
> > > the late stage from the early stage where the mapped ACPI tables is
> > > temporary and should be unmapped.
> > >
> > > But, now initializing interrupt delivery mode should map and parse the
> > > DMAR table earlier in the early stage. This causes an ACPI error when
> > > Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
> > > the DMAR table after using in the early stage.
> > >
> > > Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
> > > be mapped and parsed in late stage like before.
> > >
> > > Reported-by: Xiaolong Ye <[email protected]>
> > > Signed-off-by: Dou Liyang <[email protected]>
> > > Cc: [email protected]
> > > Cc: Rafael J. Wysocki <[email protected]>
> > > Cc: Zheng, Lv <[email protected]>
> > > Cc: Julian Wollrath <[email protected]>
> > > ---
> > > Test in my own PC(Lenovo M4340).
> > > Ask help for doing regression testing for the bug said in commit c4e1acbb35e4
> > > ("ACPI / init: Invoke early ACPI initialization later").
> > >
> > > init/main.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/init/main.c b/init/main.c
> > > index df58a41..7a09467 100644
> > > --- a/init/main.c
> > > +++ b/init/main.c
> > > @@ -654,12 +654,12 @@ asmlinkage __visible void __init start_kernel(void)
> > > kmemleak_init();
> > > setup_per_cpu_pageset();
> > > numa_policy_init();
> > > + acpi_early_init();
> > > if (late_time_init)
> > > late_time_init();
> > > calibrate_delay();
> > > pidmap_init();
> > > anon_vma_init();
> > > - acpi_early_init();
> > > #ifdef CONFIG_X86
> > > if (efi_enabled(EFI_RUNTIME_SERVICES))
> > > efi_enter_virtual_mode();
> > > --
> > > 2.5.5
> > >
> > >
> >
> >
> >
> >
>
>
Hi Baoquan,
At 07/18/2017 04:45 PM, [email protected] wrote:
> On 07/18/17 at 02:08pm, Dou Liyang wrote:
>> Hi, Zheng
>>
>> At 07/18/2017 01:18 PM, Zheng, Lv wrote:
>>> Hi,
>>>
>>> Can the problem be fixed by invoking acpi_put_table() for mapped DMAR table?
>>
>> Invoking acpi_put_table() is my first choice. But it made the kernel
>> *panic* when we try to get the table again in intel_iommu_init() in
>> late stage.
>>
>> I am also confused that:
>>
>> There are two places where we used DMAR table in Linux:
>>
>> 1) In detect_intel_iommu() in ACPI early stage:
>>
>> ...
>> status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
>> ....
>> if (dmar_tbl) {
>> acpi_put_table(dmar_tbl);
>> dmar_tbl = NULL;
>> }
>>
>> 2) In dmar_table_init() in ACPI late stage:
>>
>> ...
>> status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
>> ...
>>
>> As we know, dmar_table_init() is called by intel_iommu_init() and
>> intel_prepare_irq_remapping().
>>
>> When I invoked acpi_put_table() in the intel_prepare_irq_remapping() in
>> early stage like 1) shows, kernel will panic.
>
> That's because acpi_put_table() will make the table pointer be NULL,
> while dmar_table_init() will skip parse_dmar_table() calling if
> dmar_table_initialized is set to 1 in intel_prepare_irq_remapping().
>
Correctly.
I have considered and removed the *dmar_table_initialized* in this
situation. So, dmar_table_init() didn't skip parse_dmar_table()
calling.
I didn't dig into the cause, I think it is interesting, I will do it
right now and share with you later.
> Dmar hardware support interrupt remapping and io remapping separately. But
> intel_iommu_init() is called later than intel_prepare_irq_remapping().
> So what if make dmar_table_init() a reentrant function? You can just
> have a try, but maybe not a good idea, the dmar table will be parsed
> twice.
Yes, It is precisely one reason that I gave up invoking
acpi_put_table().
Thanks,
dou.
>
>>
>>
>> Thanks,
>>
>> dou.
>>>
>>> Thanks
>>> Lv
>>>
>>>> From: Dou Liyang [mailto:[email protected]]
>>>> Sent: Friday, July 14, 2017 1:53 PM
>>>> To: [email protected]; [email protected]
>>>> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected];
>>>> [email protected]; [email protected]; [email protected]; Dou Liyang
>>>> <[email protected]>; [email protected]; Rafael J. Wysocki <[email protected]>; Zheng,
>>>> Lv <[email protected]>; Julian Wollrath <[email protected]>
>>>> Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
>>>>
>>>> Linux uses acpi_early_init() to put the ACPI table management into
>>>> the late stage from the early stage where the mapped ACPI tables is
>>>> temporary and should be unmapped.
>>>>
>>>> But, now initializing interrupt delivery mode should map and parse the
>>>> DMAR table earlier in the early stage. This causes an ACPI error when
>>>> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
>>>> the DMAR table after using in the early stage.
>>>>
>>>> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
>>>> be mapped and parsed in late stage like before.
>>>>
>>>> Reported-by: Xiaolong Ye <[email protected]>
>>>> Signed-off-by: Dou Liyang <[email protected]>
>>>> Cc: [email protected]
>>>> Cc: Rafael J. Wysocki <[email protected]>
>>>> Cc: Zheng, Lv <[email protected]>
>>>> Cc: Julian Wollrath <[email protected]>
>>>> ---
>>>> Test in my own PC(Lenovo M4340).
>>>> Ask help for doing regression testing for the bug said in commit c4e1acbb35e4
>>>> ("ACPI / init: Invoke early ACPI initialization later").
>>>>
>>>> init/main.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/init/main.c b/init/main.c
>>>> index df58a41..7a09467 100644
>>>> --- a/init/main.c
>>>> +++ b/init/main.c
>>>> @@ -654,12 +654,12 @@ asmlinkage __visible void __init start_kernel(void)
>>>> kmemleak_init();
>>>> setup_per_cpu_pageset();
>>>> numa_policy_init();
>>>> + acpi_early_init();
>>>> if (late_time_init)
>>>> late_time_init();
>>>> calibrate_delay();
>>>> pidmap_init();
>>>> anon_vma_init();
>>>> - acpi_early_init();
>>>> #ifdef CONFIG_X86
>>>> if (efi_enabled(EFI_RUNTIME_SERVICES))
>>>> efi_enter_virtual_mode();
>>>> --
>>>> 2.5.5
>>>>
>>>>
>>>
>>>
>>>
>>>
>>
>>
>
>
>
Hi Baoquan,
At 07/18/2017 04:45 PM, [email protected] wrote:
> On 07/18/17 at 02:08pm, Dou Liyang wrote:
>> Hi, Zheng
>>
>> At 07/18/2017 01:18 PM, Zheng, Lv wrote:
>>> Hi,
>>>
>>> Can the problem be fixed by invoking acpi_put_table() for mapped DMAR table?
>>
>> Invoking acpi_put_table() is my first choice. But it made the kernel
>> *panic* when we try to get the table again in intel_iommu_init() in
>> late stage.
>>
>> I am also confused that:
>>
>> There are two places where we used DMAR table in Linux:
>>
>> 1) In detect_intel_iommu() in ACPI early stage:
>>
>> ...
>> status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
>> ....
>> if (dmar_tbl) {
>> acpi_put_table(dmar_tbl);
>> dmar_tbl = NULL;
>> }
>>
>> 2) In dmar_table_init() in ACPI late stage:
>>
>> ...
>> status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
>> ...
>>
>> As we know, dmar_table_init() is called by intel_iommu_init() and
>> intel_prepare_irq_remapping().
>>
>> When I invoked acpi_put_table() in the intel_prepare_irq_remapping() in
>> early stage like 1) shows, kernel will panic.
>
> That's because acpi_put_table() will make the table pointer be NULL,
> while dmar_table_init() will skip parse_dmar_table() calling if
> dmar_table_initialized is set to 1 in intel_prepare_irq_remapping().
>
> Dmar hardware support interrupt remapping and io remapping separately. But
> intel_iommu_init() is called later than intel_prepare_irq_remapping().
> So what if make dmar_table_init() a reentrant function? You can just
> have a try, but maybe not a good idea, the dmar table will be parsed
> twice.
The true reason why the kernel panic is that acpi_put_table() only
released DMAR table structure, but not released the remapping
structures in DMAR table, such as DRHD, RMRR. So the address of
RMRR parsed in early ACPI stage will be used in late ACPI stage in
intel_iommu_init(), which make the kernel panic.
The solution is invoking the intel_iommu_free_dmars() before
dmar_table_init() in intel_iommu_init() to release the RMRR.
Demo code will show at the bottom.
I prefer to invoke acpi_early_init() earlier. But it needs a regression
test[1].
I am looking for Thinkpad x121e (AMD E-450 APU) to test. I have tested
it in Thinkpad s430, It's OK.
BTY, I am confused how does the ACPI subsystem affect PIT which
will be used to fast calibrate CPU frequency[2].
Do you have any idea?
[1] https://lkml.org/lkml/2014/3/10/123
[2] https://lkml.org/lkml/2014/3/12/3
drivers/iommu/dmar.c | 27 +++++++++++----------------
drivers/iommu/intel-iommu.c | 2 ++
drivers/iommu/intel_irq_remapping.c | 17 ++++++++++++++++-
include/linux/dmar.h | 2 ++
init/main.c | 2 +-
5 files changed, 32 insertions(+), 18 deletions(-)
diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index c8b0329..e6261b7 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -68,6 +68,8 @@ DECLARE_RWSEM(dmar_global_lock);
LIST_HEAD(dmar_drhd_units);
struct acpi_table_header * __initdata dmar_tbl;
+struct acpi_table_header * __initdata dmar_tbl_original;
+
static int dmar_dev_scope_status = 1;
static unsigned long dmar_seq_ids[BITS_TO_LONGS(DMAR_UNITS_SUPPORTED)];
@@ -627,6 +629,7 @@ parse_dmar_table(void)
* fixed map.
*/
dmar_table_detect();
+ dmar_tbl_original = dmar_tbl;
/*
* ACPI tables may not be DMA protected by tboot, so use DMAR copy
@@ -811,26 +814,18 @@ int __init dmar_dev_scope_init(void)
int __init dmar_table_init(void)
{
- static int dmar_table_initialized;
int ret;
- if (dmar_table_initialized == 0) {
- ret = parse_dmar_table();
- if (ret < 0) {
- if (ret != -ENODEV)
- pr_info("Parse DMAR table failure.\n");
- } else if (list_empty(&dmar_drhd_units)) {
- pr_info("No DMAR devices found\n");
- ret = -ENODEV;
- }
-
- if (ret < 0)
- dmar_table_initialized = ret;
- else
- dmar_table_initialized = 1;
+ ret = parse_dmar_table();
+ if (ret < 0) {
+ if (ret != -ENODEV)
+ pr_info("Parse DMAR table failure.\n");
+ } else if (list_empty(&dmar_drhd_units)) {
+ pr_info("No DMAR devices found\n");
+ ret = -ENODEV;
}
- return dmar_table_initialized < 0 ? dmar_table_initialized : 0;
+ return ret;
}
static void warn_invalid_dmar(u64 addr, const char *message)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 687f18f..90f74f4 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -4832,6 +4832,8 @@ int __init intel_iommu_init(void)
}
down_write(&dmar_global_lock);
+
+ intel_iommu_free_dmars();
if (dmar_table_init()) {
if (force_on)
panic("tboot: Failed to initialize DMAR table\n");
diff --git a/drivers/iommu/intel_irq_remapping.c
b/drivers/iommu/intel_irq_remapping.c
index a5b89f6..ccaacda 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -675,7 +675,7 @@ static void __init intel_cleanup_irq_remapping(void)
pr_warn("Failed to enable irq remapping. You are vulnerable to
irq-injection attacks.\n");
}
-static int __init intel_prepare_irq_remapping(void)
+static int __init __intel_prepare_irq_remapping(void)
{
struct dmar_drhd_unit *drhd;
struct intel_iommu *iommu;
@@ -743,6 +743,21 @@ static int __init intel_prepare_irq_remapping(void)
return -ENODEV;
}
+static int __init intel_prepare_irq_remapping(void)
+{
+ int ret;
+
+ ret = __intel_prepare_irq_remapping();
+
+ if (dmar_tbl_original) {
+ acpi_put_table(dmar_tbl_original);
+ dmar_tbl_original = NULL;
+ dmar_tbl = NULL;
+ }
+
+ return ret;
+}
+
/*
* Set Posted-Interrupts capability.
*/
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index e8ffba1..987b076 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -50,6 +50,8 @@ struct dmar_dev_scope {
#ifdef CONFIG_DMAR_TABLE
extern struct acpi_table_header *dmar_tbl;
+extern struct acpi_table_header *dmar_tbl_original;
+
struct dmar_drhd_unit {
struct list_head list; /* list of drhd units */
struct acpi_dmar_header *hdr; /* ACPI header */
diff --git a/init/main.c b/init/main.c
index 52dee20..052481f 100644
--- a/init/main.c
+++ b/init/main.c
@@ -655,12 +655,12 @@ asmlinkage __visible void __init start_kernel(void)
kmemleak_init();
setup_per_cpu_pageset();
numa_policy_init();
- acpi_early_init();
if (late_time_init)
late_time_init();
calibrate_delay();
pidmap_init();
anon_vma_init();
+ acpi_early_init();
#ifdef CONFIG_X86
if (efi_enabled(EFI_RUNTIME_SERVICES))
efi_enter_virtual_mode();
Thanks,
dou.
>
>>
>>
>> Thanks,
>>
>> dou.
>>>
>>> Thanks
>>> Lv
>>>
>>>> From: Dou Liyang [mailto:[email protected]]
>>>> Sent: Friday, July 14, 2017 1:53 PM
>>>> To: [email protected]; [email protected]
>>>> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected];
>>>> [email protected]; [email protected]; [email protected]; Dou Liyang
>>>> <[email protected]>; [email protected]; Rafael J. Wysocki <[email protected]>; Zheng,
>>>> Lv <[email protected]>; Julian Wollrath <[email protected]>
>>>> Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
>>>>
>>>> Linux uses acpi_early_init() to put the ACPI table management into
>>>> the late stage from the early stage where the mapped ACPI tables is
>>>> temporary and should be unmapped.
>>>>
>>>> But, now initializing interrupt delivery mode should map and parse the
>>>> DMAR table earlier in the early stage. This causes an ACPI error when
>>>> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
>>>> the DMAR table after using in the early stage.
>>>>
>>>> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
>>>> be mapped and parsed in late stage like before.
>>>>
>>>> Reported-by: Xiaolong Ye <[email protected]>
>>>> Signed-off-by: Dou Liyang <[email protected]>
>>>> Cc: [email protected]
>>>> Cc: Rafael J. Wysocki <[email protected]>
>>>> Cc: Zheng, Lv <[email protected]>
>>>> Cc: Julian Wollrath <[email protected]>
>>>> ---
>>>> Test in my own PC(Lenovo M4340).
>>>> Ask help for doing regression testing for the bug said in commit c4e1acbb35e4
>>>> ("ACPI / init: Invoke early ACPI initialization later").
>>>>
>>>> init/main.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/init/main.c b/init/main.c
>>>> index df58a41..7a09467 100644
>>>> --- a/init/main.c
>>>> +++ b/init/main.c
>>>> @@ -654,12 +654,12 @@ asmlinkage __visible void __init start_kernel(void)
>>>> kmemleak_init();
>>>> setup_per_cpu_pageset();
>>>> numa_policy_init();
>>>> + acpi_early_init();
>>>> if (late_time_init)
>>>> late_time_init();
>>>> calibrate_delay();
>>>> pidmap_init();
>>>> anon_vma_init();
>>>> - acpi_early_init();
>>>> #ifdef CONFIG_X86
>>>> if (efi_enabled(EFI_RUNTIME_SERVICES))
>>>> efi_enter_virtual_mode();
>>>> --
>>>> 2.5.5
>>>>
>>>>
>>>
>>>
>>>
>>>
>>
>>
>
>
>
On 07/26/17 at 08:19pm, Dou Liyang wrote:
> Hi Baoquan,
> > > There are two places where we used DMAR table in Linux:
> > >
> > > 1) In detect_intel_iommu() in ACPI early stage:
> > >
> > > ...
> > > status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
> > > ....
> > > if (dmar_tbl) {
> > > acpi_put_table(dmar_tbl);
> > > dmar_tbl = NULL;
> > > }
> > >
> > > 2) In dmar_table_init() in ACPI late stage:
> > >
> > > ...
> > > status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
> > > ...
> > >
> > > As we know, dmar_table_init() is called by intel_iommu_init() and
> > > intel_prepare_irq_remapping().
> > >
> > > When I invoked acpi_put_table() in the intel_prepare_irq_remapping() in
> > > early stage like 1) shows, kernel will panic.
> >
> > That's because acpi_put_table() will make the table pointer be NULL,
> > while dmar_table_init() will skip parse_dmar_table() calling if
> > dmar_table_initialized is set to 1 in intel_prepare_irq_remapping().
> >
> > Dmar hardware support interrupt remapping and io remapping separately. But
> > intel_iommu_init() is called later than intel_prepare_irq_remapping().
> > So what if make dmar_table_init() a reentrant function? You can just
> > have a try, but maybe not a good idea, the dmar table will be parsed
> > twice.
>
> The true reason why the kernel panic is that acpi_put_table() only
> released DMAR table structure, but not released the remapping
> structures in DMAR table, such as DRHD, RMRR. So the address of
> RMRR parsed in early ACPI stage will be used in late ACPI stage in
> intel_iommu_init(), which make the kernel panic.
>
> The solution is invoking the intel_iommu_free_dmars() before
> dmar_table_init() in intel_iommu_init() to release the RMRR.
> Demo code will show at the bottom.
>
> I prefer to invoke acpi_early_init() earlier. But it needs a regression
> test[1].
Good work, in fact not only intel iommu matters here, I gues you haven't
tried amd system with a AMD-VI which does the amd iommu work. It's
similar to intel iommu and the same principle but different acpi tables.
So it's fine you want to put acpi_early_init earlier.
>
> I am looking for Thinkpad x121e (AMD E-450 APU) to test. I have tested
> it in Thinkpad s430, It's OK.
>
> BTY, I am confused how does the ACPI subsystem affect PIT which
> will be used to fast calibrate CPU frequency[2].
I checked code, and have no any idea. Add Joey Lee to list, see if he
can help answer this.
>
> Do you have any idea?
>
> [1] https://lkml.org/lkml/2014/3/10/123
> [2] https://lkml.org/lkml/2014/3/12/3
>
>
> drivers/iommu/dmar.c | 27 +++++++++++----------------
> drivers/iommu/intel-iommu.c | 2 ++
> drivers/iommu/intel_irq_remapping.c | 17 ++++++++++++++++-
> include/linux/dmar.h | 2 ++
> init/main.c | 2 +-
> 5 files changed, 32 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
> index c8b0329..e6261b7 100644
> --- a/drivers/iommu/dmar.c
> +++ b/drivers/iommu/dmar.c
> @@ -68,6 +68,8 @@ DECLARE_RWSEM(dmar_global_lock);
> LIST_HEAD(dmar_drhd_units);
>
> struct acpi_table_header * __initdata dmar_tbl;
> +struct acpi_table_header * __initdata dmar_tbl_original;
> +
> static int dmar_dev_scope_status = 1;
> static unsigned long dmar_seq_ids[BITS_TO_LONGS(DMAR_UNITS_SUPPORTED)];
>
> @@ -627,6 +629,7 @@ parse_dmar_table(void)
> * fixed map.
> */
> dmar_table_detect();
> + dmar_tbl_original = dmar_tbl;
>
> /*
> * ACPI tables may not be DMA protected by tboot, so use DMAR copy
> @@ -811,26 +814,18 @@ int __init dmar_dev_scope_init(void)
>
> int __init dmar_table_init(void)
> {
> - static int dmar_table_initialized;
> int ret;
>
> - if (dmar_table_initialized == 0) {
> - ret = parse_dmar_table();
> - if (ret < 0) {
> - if (ret != -ENODEV)
> - pr_info("Parse DMAR table failure.\n");
> - } else if (list_empty(&dmar_drhd_units)) {
> - pr_info("No DMAR devices found\n");
> - ret = -ENODEV;
> - }
> -
> - if (ret < 0)
> - dmar_table_initialized = ret;
> - else
> - dmar_table_initialized = 1;
> + ret = parse_dmar_table();
> + if (ret < 0) {
> + if (ret != -ENODEV)
> + pr_info("Parse DMAR table failure.\n");
> + } else if (list_empty(&dmar_drhd_units)) {
> + pr_info("No DMAR devices found\n");
> + ret = -ENODEV;
> }
>
> - return dmar_table_initialized < 0 ? dmar_table_initialized : 0;
> + return ret;
> }
>
> static void warn_invalid_dmar(u64 addr, const char *message)
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 687f18f..90f74f4 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -4832,6 +4832,8 @@ int __init intel_iommu_init(void)
> }
>
> down_write(&dmar_global_lock);
> +
> + intel_iommu_free_dmars();
> if (dmar_table_init()) {
> if (force_on)
> panic("tboot: Failed to initialize DMAR table\n");
> diff --git a/drivers/iommu/intel_irq_remapping.c
> b/drivers/iommu/intel_irq_remapping.c
> index a5b89f6..ccaacda 100644
> --- a/drivers/iommu/intel_irq_remapping.c
> +++ b/drivers/iommu/intel_irq_remapping.c
> @@ -675,7 +675,7 @@ static void __init intel_cleanup_irq_remapping(void)
> pr_warn("Failed to enable irq remapping. You are vulnerable to
> irq-injection attacks.\n");
> }
>
> -static int __init intel_prepare_irq_remapping(void)
> +static int __init __intel_prepare_irq_remapping(void)
> {
> struct dmar_drhd_unit *drhd;
> struct intel_iommu *iommu;
> @@ -743,6 +743,21 @@ static int __init intel_prepare_irq_remapping(void)
> return -ENODEV;
> }
>
> +static int __init intel_prepare_irq_remapping(void)
> +{
> + int ret;
> +
> + ret = __intel_prepare_irq_remapping();
> +
> + if (dmar_tbl_original) {
> + acpi_put_table(dmar_tbl_original);
> + dmar_tbl_original = NULL;
> + dmar_tbl = NULL;
> + }
> +
> + return ret;
> +}
> +
> /*
> * Set Posted-Interrupts capability.
> */
> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> index e8ffba1..987b076 100644
> --- a/include/linux/dmar.h
> +++ b/include/linux/dmar.h
> @@ -50,6 +50,8 @@ struct dmar_dev_scope {
>
> #ifdef CONFIG_DMAR_TABLE
> extern struct acpi_table_header *dmar_tbl;
> +extern struct acpi_table_header *dmar_tbl_original;
> +
> struct dmar_drhd_unit {
> struct list_head list; /* list of drhd units */
> struct acpi_dmar_header *hdr; /* ACPI header */
> diff --git a/init/main.c b/init/main.c
> index 52dee20..052481f 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -655,12 +655,12 @@ asmlinkage __visible void __init start_kernel(void)
> kmemleak_init();
> setup_per_cpu_pageset();
> numa_policy_init();
> - acpi_early_init();
> if (late_time_init)
> late_time_init();
> calibrate_delay();
> pidmap_init();
> anon_vma_init();
> + acpi_early_init();
> #ifdef CONFIG_X86
> if (efi_enabled(EFI_RUNTIME_SERVICES))
> efi_enter_virtual_mode();
>
> Thanks,
> dou.
> >
> > >
> > >
> > > Thanks,
> > >
> > > dou.
> > > >
> > > > Thanks
> > > > Lv
> > > >
> > > > > From: Dou Liyang [mailto:[email protected]]
> > > > > Sent: Friday, July 14, 2017 1:53 PM
> > > > > To: [email protected]; [email protected]
> > > > > Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected];
> > > > > [email protected]; [email protected]; [email protected]; Dou Liyang
> > > > > <[email protected]>; [email protected]; Rafael J. Wysocki <[email protected]>; Zheng,
> > > > > Lv <[email protected]>; Julian Wollrath <[email protected]>
> > > > > Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
> > > > >
> > > > > Linux uses acpi_early_init() to put the ACPI table management into
> > > > > the late stage from the early stage where the mapped ACPI tables is
> > > > > temporary and should be unmapped.
> > > > >
> > > > > But, now initializing interrupt delivery mode should map and parse the
> > > > > DMAR table earlier in the early stage. This causes an ACPI error when
> > > > > Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
> > > > > the DMAR table after using in the early stage.
> > > > >
> > > > > Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
> > > > > be mapped and parsed in late stage like before.
> > > > >
> > > > > Reported-by: Xiaolong Ye <[email protected]>
> > > > > Signed-off-by: Dou Liyang <[email protected]>
> > > > > Cc: [email protected]
> > > > > Cc: Rafael J. Wysocki <[email protected]>
> > > > > Cc: Zheng, Lv <[email protected]>
> > > > > Cc: Julian Wollrath <[email protected]>
> > > > > ---
> > > > > Test in my own PC(Lenovo M4340).
> > > > > Ask help for doing regression testing for the bug said in commit c4e1acbb35e4
> > > > > ("ACPI / init: Invoke early ACPI initialization later").
> > > > >
> > > > > init/main.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/init/main.c b/init/main.c
> > > > > index df58a41..7a09467 100644
> > > > > --- a/init/main.c
> > > > > +++ b/init/main.c
> > > > > @@ -654,12 +654,12 @@ asmlinkage __visible void __init start_kernel(void)
> > > > > kmemleak_init();
> > > > > setup_per_cpu_pageset();
> > > > > numa_policy_init();
> > > > > + acpi_early_init();
> > > > > if (late_time_init)
> > > > > late_time_init();
> > > > > calibrate_delay();
> > > > > pidmap_init();
> > > > > anon_vma_init();
> > > > > - acpi_early_init();
> > > > > #ifdef CONFIG_X86
> > > > > if (efi_enabled(EFI_RUNTIME_SERVICES))
> > > > > efi_enter_virtual_mode();
> > > > > --
> > > > > 2.5.5
> > > > >
> > > > >
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> >
> >
> >
>
>
Hi Baoquan,
At 07/27/2017 02:08 PM, [email protected] wrote:
> On 07/26/17 at 08:19pm, Dou Liyang wrote:
>> Hi Baoquan,
>>>> There are two places where we used DMAR table in Linux:
>>>>
>>>> 1) In detect_intel_iommu() in ACPI early stage:
>>>>
>>>> ...
>>>> status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
>>>> ....
>>>> if (dmar_tbl) {
>>>> acpi_put_table(dmar_tbl);
>>>> dmar_tbl = NULL;
>>>> }
>>>>
>>>> 2) In dmar_table_init() in ACPI late stage:
>>>>
>>>> ...
>>>> status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
>>>> ...
>>>>
>>>> As we know, dmar_table_init() is called by intel_iommu_init() and
>>>> intel_prepare_irq_remapping().
>>>>
>>>> When I invoked acpi_put_table() in the intel_prepare_irq_remapping() in
>>>> early stage like 1) shows, kernel will panic.
>>>
>>> That's because acpi_put_table() will make the table pointer be NULL,
>>> while dmar_table_init() will skip parse_dmar_table() calling if
>>> dmar_table_initialized is set to 1 in intel_prepare_irq_remapping().
>>>
>>> Dmar hardware support interrupt remapping and io remapping separately. But
>>> intel_iommu_init() is called later than intel_prepare_irq_remapping().
>>> So what if make dmar_table_init() a reentrant function? You can just
>>> have a try, but maybe not a good idea, the dmar table will be parsed
>>> twice.
>>
>> The true reason why the kernel panic is that acpi_put_table() only
>> released DMAR table structure, but not released the remapping
>> structures in DMAR table, such as DRHD, RMRR. So the address of
>> RMRR parsed in early ACPI stage will be used in late ACPI stage in
>> intel_iommu_init(), which make the kernel panic.
>>
>> The solution is invoking the intel_iommu_free_dmars() before
>> dmar_table_init() in intel_iommu_init() to release the RMRR.
>> Demo code will show at the bottom.
>>
>> I prefer to invoke acpi_early_init() earlier. But it needs a regression
>> test[1].
>
> Good work, in fact not only intel iommu matters here, I gues you haven't
> tried amd system with a AMD-VI which does the amd iommu work. It's
> similar to intel iommu and the same principle but different acpi tables.
Yes, I missed it, I didn't try it in AMD platform. If I want to use
acpi_put_table(), I should consider both the Intel and AMD platform.
> So it's fine you want to put acpi_early_init earlier.
Got it.
>
>>
>> I am looking for Thinkpad x121e (AMD E-450 APU) to test. I have tested
>> it in Thinkpad s430, It's OK.
>>
>> BTY, I am confused how does the ACPI subsystem affect PIT which
>> will be used to fast calibrate CPU frequency[2].
>
> I checked code, and have no any idea. Add Joey Lee to list, see if he
> can help answer this.
Thank you very much.
sincerely,
dou.
>
>>
>> Do you have any idea?
>>
>> [1] https://lkml.org/lkml/2014/3/10/123
>> [2] https://lkml.org/lkml/2014/3/12/3
>>
>>
>> drivers/iommu/dmar.c | 27 +++++++++++----------------
>> drivers/iommu/intel-iommu.c | 2 ++
>> drivers/iommu/intel_irq_remapping.c | 17 ++++++++++++++++-
>> include/linux/dmar.h | 2 ++
>> init/main.c | 2 +-
>> 5 files changed, 32 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
>> index c8b0329..e6261b7 100644
>> --- a/drivers/iommu/dmar.c
>> +++ b/drivers/iommu/dmar.c
>> @@ -68,6 +68,8 @@ DECLARE_RWSEM(dmar_global_lock);
>> LIST_HEAD(dmar_drhd_units);
>>
>> struct acpi_table_header * __initdata dmar_tbl;
>> +struct acpi_table_header * __initdata dmar_tbl_original;
>> +
>> static int dmar_dev_scope_status = 1;
>> static unsigned long dmar_seq_ids[BITS_TO_LONGS(DMAR_UNITS_SUPPORTED)];
>>
>> @@ -627,6 +629,7 @@ parse_dmar_table(void)
>> * fixed map.
>> */
>> dmar_table_detect();
>> + dmar_tbl_original = dmar_tbl;
>>
>> /*
>> * ACPI tables may not be DMA protected by tboot, so use DMAR copy
>> @@ -811,26 +814,18 @@ int __init dmar_dev_scope_init(void)
>>
>> int __init dmar_table_init(void)
>> {
>> - static int dmar_table_initialized;
>> int ret;
>>
>> - if (dmar_table_initialized == 0) {
>> - ret = parse_dmar_table();
>> - if (ret < 0) {
>> - if (ret != -ENODEV)
>> - pr_info("Parse DMAR table failure.\n");
>> - } else if (list_empty(&dmar_drhd_units)) {
>> - pr_info("No DMAR devices found\n");
>> - ret = -ENODEV;
>> - }
>> -
>> - if (ret < 0)
>> - dmar_table_initialized = ret;
>> - else
>> - dmar_table_initialized = 1;
>> + ret = parse_dmar_table();
>> + if (ret < 0) {
>> + if (ret != -ENODEV)
>> + pr_info("Parse DMAR table failure.\n");
>> + } else if (list_empty(&dmar_drhd_units)) {
>> + pr_info("No DMAR devices found\n");
>> + ret = -ENODEV;
>> }
>>
>> - return dmar_table_initialized < 0 ? dmar_table_initialized : 0;
>> + return ret;
>> }
>>
>> static void warn_invalid_dmar(u64 addr, const char *message)
>> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
>> index 687f18f..90f74f4 100644
>> --- a/drivers/iommu/intel-iommu.c
>> +++ b/drivers/iommu/intel-iommu.c
>> @@ -4832,6 +4832,8 @@ int __init intel_iommu_init(void)
>> }
>>
>> down_write(&dmar_global_lock);
>> +
>> + intel_iommu_free_dmars();
>> if (dmar_table_init()) {
>> if (force_on)
>> panic("tboot: Failed to initialize DMAR table\n");
>> diff --git a/drivers/iommu/intel_irq_remapping.c
>> b/drivers/iommu/intel_irq_remapping.c
>> index a5b89f6..ccaacda 100644
>> --- a/drivers/iommu/intel_irq_remapping.c
>> +++ b/drivers/iommu/intel_irq_remapping.c
>> @@ -675,7 +675,7 @@ static void __init intel_cleanup_irq_remapping(void)
>> pr_warn("Failed to enable irq remapping. You are vulnerable to
>> irq-injection attacks.\n");
>> }
>>
>> -static int __init intel_prepare_irq_remapping(void)
>> +static int __init __intel_prepare_irq_remapping(void)
>> {
>> struct dmar_drhd_unit *drhd;
>> struct intel_iommu *iommu;
>> @@ -743,6 +743,21 @@ static int __init intel_prepare_irq_remapping(void)
>> return -ENODEV;
>> }
>>
>> +static int __init intel_prepare_irq_remapping(void)
>> +{
>> + int ret;
>> +
>> + ret = __intel_prepare_irq_remapping();
>> +
>> + if (dmar_tbl_original) {
>> + acpi_put_table(dmar_tbl_original);
>> + dmar_tbl_original = NULL;
>> + dmar_tbl = NULL;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> /*
>> * Set Posted-Interrupts capability.
>> */
>> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
>> index e8ffba1..987b076 100644
>> --- a/include/linux/dmar.h
>> +++ b/include/linux/dmar.h
>> @@ -50,6 +50,8 @@ struct dmar_dev_scope {
>>
>> #ifdef CONFIG_DMAR_TABLE
>> extern struct acpi_table_header *dmar_tbl;
>> +extern struct acpi_table_header *dmar_tbl_original;
>> +
>> struct dmar_drhd_unit {
>> struct list_head list; /* list of drhd units */
>> struct acpi_dmar_header *hdr; /* ACPI header */
>> diff --git a/init/main.c b/init/main.c
>> index 52dee20..052481f 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -655,12 +655,12 @@ asmlinkage __visible void __init start_kernel(void)
>> kmemleak_init();
>> setup_per_cpu_pageset();
>> numa_policy_init();
>> - acpi_early_init();
>> if (late_time_init)
>> late_time_init();
>> calibrate_delay();
>> pidmap_init();
>> anon_vma_init();
>> + acpi_early_init();
>> #ifdef CONFIG_X86
>> if (efi_enabled(EFI_RUNTIME_SERVICES))
>> efi_enter_virtual_mode();
>>
>> Thanks,
>> dou.
>>>
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> dou.
>>>>>
>>>>> Thanks
>>>>> Lv
>>>>>
>>>>>> From: Dou Liyang [mailto:[email protected]]
>>>>>> Sent: Friday, July 14, 2017 1:53 PM
>>>>>> To: [email protected]; [email protected]
>>>>>> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected];
>>>>>> [email protected]; [email protected]; [email protected]; Dou Liyang
>>>>>> <[email protected]>; [email protected]; Rafael J. Wysocki <[email protected]>; Zheng,
>>>>>> Lv <[email protected]>; Julian Wollrath <[email protected]>
>>>>>> Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
>>>>>>
>>>>>> Linux uses acpi_early_init() to put the ACPI table management into
>>>>>> the late stage from the early stage where the mapped ACPI tables is
>>>>>> temporary and should be unmapped.
>>>>>>
>>>>>> But, now initializing interrupt delivery mode should map and parse the
>>>>>> DMAR table earlier in the early stage. This causes an ACPI error when
>>>>>> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
>>>>>> the DMAR table after using in the early stage.
>>>>>>
>>>>>> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
>>>>>> be mapped and parsed in late stage like before.
>>>>>>
>>>>>> Reported-by: Xiaolong Ye <[email protected]>
>>>>>> Signed-off-by: Dou Liyang <[email protected]>
>>>>>> Cc: [email protected]
>>>>>> Cc: Rafael J. Wysocki <[email protected]>
>>>>>> Cc: Zheng, Lv <[email protected]>
>>>>>> Cc: Julian Wollrath <[email protected]>
>>>>>> ---
>>>>>> Test in my own PC(Lenovo M4340).
>>>>>> Ask help for doing regression testing for the bug said in commit c4e1acbb35e4
>>>>>> ("ACPI / init: Invoke early ACPI initialization later").
>>>>>>
>>>>>> init/main.c | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/init/main.c b/init/main.c
>>>>>> index df58a41..7a09467 100644
>>>>>> --- a/init/main.c
>>>>>> +++ b/init/main.c
>>>>>> @@ -654,12 +654,12 @@ asmlinkage __visible void __init start_kernel(void)
>>>>>> kmemleak_init();
>>>>>> setup_per_cpu_pageset();
>>>>>> numa_policy_init();
>>>>>> + acpi_early_init();
>>>>>> if (late_time_init)
>>>>>> late_time_init();
>>>>>> calibrate_delay();
>>>>>> pidmap_init();
>>>>>> anon_vma_init();
>>>>>> - acpi_early_init();
>>>>>> #ifdef CONFIG_X86
>>>>>> if (efi_enabled(EFI_RUNTIME_SERVICES))
>>>>>> efi_enter_virtual_mode();
>>>>>> --
>>>>>> 2.5.5
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>
>
>
Hi,
> From: Dou Liyang [mailto:[email protected]]
> Sent: Tuesday, July 18, 2017 5:44 PM
> Subject: Re: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
>
> Hi Baoquan,
>
> At 07/18/2017 04:45 PM, [email protected] wrote:
> > On 07/18/17 at 02:08pm, Dou Liyang wrote:
> >> Hi, Zheng
> >>
> >> At 07/18/2017 01:18 PM, Zheng, Lv wrote:
> >>> Hi,
> >>>
> >>> Can the problem be fixed by invoking acpi_put_table() for mapped DMAR table?
> >>
> >> Invoking acpi_put_table() is my first choice. But it made the kernel
> >> *panic* when we try to get the table again in intel_iommu_init() in
> >> late stage.
> >>
> >> I am also confused that:
> >>
> >> There are two places where we used DMAR table in Linux:
> >>
> >> 1) In detect_intel_iommu() in ACPI early stage:
> >>
> >> ...
> >> status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
> >> ....
> >> if (dmar_tbl) {
> >> acpi_put_table(dmar_tbl);
> >> dmar_tbl = NULL;
> >> }
> >>
> >> 2) In dmar_table_init() in ACPI late stage:
> >>
> >> ...
> >> status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
> >> ...
> >>
> >> As we know, dmar_table_init() is called by intel_iommu_init() and
> >> intel_prepare_irq_remapping().
> >>
> >> When I invoked acpi_put_table() in the intel_prepare_irq_remapping() in
> >> early stage like 1) shows, kernel will panic.
> >
> > That's because acpi_put_table() will make the table pointer be NULL,
> > while dmar_table_init() will skip parse_dmar_table() calling if
> > dmar_table_initialized is set to 1 in intel_prepare_irq_remapping().
> >
>
> Correctly.
>
> I have considered and removed the *dmar_table_initialized* in this
> situation. So, dmar_table_init() didn't skip parse_dmar_table()
> calling.
>
> I didn't dig into the cause, I think it is interesting, I will do it
> right now and share with you later.
>
> > Dmar hardware support interrupt remapping and io remapping separately. But
> > intel_iommu_init() is called later than intel_prepare_irq_remapping().
> > So what if make dmar_table_init() a reentrant function? You can just
> > have a try, but maybe not a good idea, the dmar table will be parsed
> > twice.
>
> Yes, It is precisely one reason that I gave up invoking
> acpi_put_table().
Parsing a table twice is not a problem on x86.
If you check the code, there are many examples.
It's actually required if you want to use a table both in early stage and late stage.
Thanks
>
> Thanks,
>
> dou.
>
> >
> >>
> >>
> >> Thanks,
> >>
> >> dou.
> >>>
> >>> Thanks
> >>> Lv
> >>>
> >>>> From: Dou Liyang [mailto:[email protected]]
> >>>> Sent: Friday, July 14, 2017 1:53 PM
> >>>> To: [email protected]; [email protected]
> >>>> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected];
> >>>> [email protected]; [email protected]; [email protected]; Dou Liyang
> >>>> <[email protected]>; [email protected]; Rafael J. Wysocki <[email protected]>;
> Zheng,
> >>>> Lv <[email protected]>; Julian Wollrath <[email protected]>
> >>>> Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
> >>>>
> >>>> Linux uses acpi_early_init() to put the ACPI table management into
> >>>> the late stage from the early stage where the mapped ACPI tables is
> >>>> temporary and should be unmapped.
> >>>>
> >>>> But, now initializing interrupt delivery mode should map and parse the
> >>>> DMAR table earlier in the early stage. This causes an ACPI error when
> >>>> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
> >>>> the DMAR table after using in the early stage.
> >>>>
> >>>> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
> >>>> be mapped and parsed in late stage like before.
> >>>>
> >>>> Reported-by: Xiaolong Ye <[email protected]>
> >>>> Signed-off-by: Dou Liyang <[email protected]>
> >>>> Cc: [email protected]
> >>>> Cc: Rafael J. Wysocki <[email protected]>
> >>>> Cc: Zheng, Lv <[email protected]>
> >>>> Cc: Julian Wollrath <[email protected]>
> >>>> ---
> >>>> Test in my own PC(Lenovo M4340).
> >>>> Ask help for doing regression testing for the bug said in commit c4e1acbb35e4
> >>>> ("ACPI / init: Invoke early ACPI initialization later").
> >>>>
> >>>> init/main.c | 2 +-
> >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/init/main.c b/init/main.c
> >>>> index df58a41..7a09467 100644
> >>>> --- a/init/main.c
> >>>> +++ b/init/main.c
> >>>> @@ -654,12 +654,12 @@ asmlinkage __visible void __init start_kernel(void)
> >>>> kmemleak_init();
> >>>> setup_per_cpu_pageset();
> >>>> numa_policy_init();
> >>>> + acpi_early_init();
> >>>> if (late_time_init)
> >>>> late_time_init();
> >>>> calibrate_delay();
> >>>> pidmap_init();
> >>>> anon_vma_init();
> >>>> - acpi_early_init();
> >>>> #ifdef CONFIG_X86
> >>>> if (efi_enabled(EFI_RUNTIME_SERVICES))
> >>>> efi_enter_virtual_mode();
> >>>> --
> >>>> 2.5.5
> >>>>
> >>>>
> >>>
> >>>
> >>>
> >>>
> >>
> >>
> >
> >
> >
>
Hi, Zheng
At 07/28/2017 09:53 AM, Zheng, Lv wrote:
[...]
>>
>>> Dmar hardware support interrupt remapping and io remapping separately. But
>>> intel_iommu_init() is called later than intel_prepare_irq_remapping().
>>> So what if make dmar_table_init() a reentrant function? You can just
>>> have a try, but maybe not a good idea, the dmar table will be parsed
>>> twice.
>>
>> Yes, It is precisely one reason that I gave up invoking
>> acpi_put_table().
>
> Parsing a table twice is not a problem on x86.
> If you check the code, there are many examples.
> It's actually required if you want to use a table both in early stage and late stage.
>
I am sorry I don't explain clearly.
Yeah, in current kernel, The DMAR table has been parsed twice as well.
one is in detect_intel_iommu(), the other is in dmar_table_init().
Here we focus on the reentrant dmar_table_init(), this func contains
many callback for parsing different DMAR type structures, such as DRHD,
RMRR.
acpi_put_table() can release the mapped DMAR address, but we should
clear the remapping structure types manually. and I think parsing these
DMAR type structures again may have an influence on interrupt remapping.
Thanks,
dou.
> Thanks
>
>>
>> Thanks,
>>
>> dou.
>>
>>>
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> dou.
>>>>>
>>>>> Thanks
>>>>> Lv
>>>>>
>>>>>> From: Dou Liyang [mailto:[email protected]]
>>>>>> Sent: Friday, July 14, 2017 1:53 PM
>>>>>> To: [email protected]; [email protected]
>>>>>> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected];
>>>>>> [email protected]; [email protected]; [email protected]; Dou Liyang
>>>>>> <[email protected]>; [email protected]; Rafael J. Wysocki <[email protected]>;
>> Zheng,
>>>>>> Lv <[email protected]>; Julian Wollrath <[email protected]>
>>>>>> Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
>>>>>>
>>>>>> Linux uses acpi_early_init() to put the ACPI table management into
>>>>>> the late stage from the early stage where the mapped ACPI tables is
>>>>>> temporary and should be unmapped.
>>>>>>
>>>>>> But, now initializing interrupt delivery mode should map and parse the
>>>>>> DMAR table earlier in the early stage. This causes an ACPI error when
>>>>>> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
>>>>>> the DMAR table after using in the early stage.
>>>>>>
>>>>>> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
>>>>>> be mapped and parsed in late stage like before.
>>>>>>
>>>>>> Reported-by: Xiaolong Ye <[email protected]>
>>>>>> Signed-off-by: Dou Liyang <[email protected]>
>>>>>> Cc: [email protected]
>>>>>> Cc: Rafael J. Wysocki <[email protected]>
>>>>>> Cc: Zheng, Lv <[email protected]>
>>>>>> Cc: Julian Wollrath <[email protected]>
>>>>>> ---
>>>>>> Test in my own PC(Lenovo M4340).
>>>>>> Ask help for doing regression testing for the bug said in commit c4e1acbb35e4
>>>>>> ("ACPI / init: Invoke early ACPI initialization later").
>>>>>>
>>>>>> init/main.c | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/init/main.c b/init/main.c
>>>>>> index df58a41..7a09467 100644
>>>>>> --- a/init/main.c
>>>>>> +++ b/init/main.c
>>>>>> @@ -654,12 +654,12 @@ asmlinkage __visible void __init start_kernel(void)
>>>>>> kmemleak_init();
>>>>>> setup_per_cpu_pageset();
>>>>>> numa_policy_init();
>>>>>> + acpi_early_init();
>>>>>> if (late_time_init)
>>>>>> late_time_init();
>>>>>> calibrate_delay();
>>>>>> pidmap_init();
>>>>>> anon_vma_init();
>>>>>> - acpi_early_init();
>>>>>> #ifdef CONFIG_X86
>>>>>> if (efi_enabled(EFI_RUNTIME_SERVICES))
>>>>>> efi_enter_virtual_mode();
>>>>>> --
>>>>>> 2.5.5
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>>
>>
>
>
>
>
Hi,
At 07/14/2017 01:52 PM, Dou Liyang wrote:
> Linux uses acpi_early_init() to put the ACPI table management into
> the late stage from the early stage where the mapped ACPI tables is
> temporary and should be unmapped.
>
> But, now initializing interrupt delivery mode should map and parse the
> DMAR table earlier in the early stage. This causes an ACPI error when
> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
> the DMAR table after using in the early stage.
>
> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
> be mapped and parsed in late stage like before.
>
> Reported-by: Xiaolong Ye <[email protected]>
> Signed-off-by: Dou Liyang <[email protected]>
> Cc: [email protected]
> Cc: Rafael J. Wysocki <[email protected]>
> Cc: Zheng, Lv <[email protected]>
> Cc: Julian Wollrath <[email protected]>
> ---
> Test in my own PC(Lenovo M4340).
> Ask help for doing regression testing for the bug said in commit c4e1acbb35e4
> ("ACPI / init: Invoke early ACPI initialization later").
>
Now, I can prove this patch doesn't result in the bug[1] which made the
fast TSC calibration using PIT failed in a Thinkpad x121e (AMD E-450
APU).
The true reason of the bug is enabling ACPI subsystem earlier than
using PIT, not the SCI setup. invoking acpi_enable_subsystem() later
could fix this bug as Julian tested and said[2].
And, I found that Commit b064a8fa77df (" ACPI / init: Switch over
platform to the ACPI mode later") split the ACPI early initialization
code into acpi_early_init() and acpi_subsystem_init(). executing
acpi_enable_subsystem() at the original early ACPI initialization spot.
The sequence of them shows below:
start_kernel
+---------------+
|
+--> .......
|
| late_time_init()
+--> +-------+
|
+--> .......
|
| acpi_early_init()
+--> +-------+
|
+--> .......
|
| acpi_subsystem_init()
+-> +--------+
We make sure the acpi_subsystem_init() is called later than
late_time_init(), the bug will be avoided.
This patch changes the sequence of late_time_init() and
acpi_early_init(), doesn't effect acpi_subsystem_init().
So, this patch is OK.
Btw, Thanks very much for Borislav Petkov, he will have access to
Thinkpad x121e from Mid-August and will test this series.
[1] https://lkml.org/lkml/2014/3/10/123
[2] https://lkml.org/lkml/2014/3/12/311
Thanks
dou.
> init/main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/init/main.c b/init/main.c
> index df58a41..7a09467 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -654,12 +654,12 @@ asmlinkage __visible void __init start_kernel(void)
> kmemleak_init();
> setup_per_cpu_pageset();
> numa_policy_init();
> + acpi_early_init();
> if (late_time_init)
> late_time_init();
> calibrate_delay();
> pidmap_init();
> anon_vma_init();
> - acpi_early_init();
> #ifdef CONFIG_X86
> if (efi_enabled(EFI_RUNTIME_SERVICES))
> efi_enter_virtual_mode();
>
Hi Baoquan,
At 07/27/2017 02:08 PM, [email protected] wrote:
> On 07/26/17 at 08:19pm, Dou Liyang wrote:
>> Hi Baoquan,
[...]
>>
>> I am looking for Thinkpad x121e (AMD E-450 APU) to test. I have tested
>> it in Thinkpad s430, It's OK.
>>
>> BTY, I am confused how does the ACPI subsystem affect PIT which
>> will be used to fast calibrate CPU frequency[2].
In combination with what Joey said[1] and the code, I guess,
The acpi_enable() called in acpi_enable_system() transfers the system
into ACPI mode and write data to an I/O port. PIT works using the I/O
port. In Thinkpad x121e with ACPI mode, the counter in PIT couldn't
count right and might got an error value.
[1] https://lkml.org/lkml/2014/3/12/3
Thanks,
dou.
>
> I checked code, and have no any idea. Add Joey Lee to list, see if he
> can help answer this.
>
>>
>> Do you have any idea?
>>
>> [1] https://lkml.org/lkml/2014/3/10/123
>> [2] https://lkml.org/lkml/2014/3/12/3
>>
>>
>> drivers/iommu/dmar.c | 27 +++++++++++----------------
>> drivers/iommu/intel-iommu.c | 2 ++
>> drivers/iommu/intel_irq_remapping.c | 17 ++++++++++++++++-
>> include/linux/dmar.h | 2 ++
>> init/main.c | 2 +-
>> 5 files changed, 32 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
>> index c8b0329..e6261b7 100644
>> --- a/drivers/iommu/dmar.c
>> +++ b/drivers/iommu/dmar.c
>> @@ -68,6 +68,8 @@ DECLARE_RWSEM(dmar_global_lock);
>> LIST_HEAD(dmar_drhd_units);
>>
>> struct acpi_table_header * __initdata dmar_tbl;
>> +struct acpi_table_header * __initdata dmar_tbl_original;
>> +
>> static int dmar_dev_scope_status = 1;
>> static unsigned long dmar_seq_ids[BITS_TO_LONGS(DMAR_UNITS_SUPPORTED)];
>>
>> @@ -627,6 +629,7 @@ parse_dmar_table(void)
>> * fixed map.
>> */
>> dmar_table_detect();
>> + dmar_tbl_original = dmar_tbl;
>>
>> /*
>> * ACPI tables may not be DMA protected by tboot, so use DMAR copy
>> @@ -811,26 +814,18 @@ int __init dmar_dev_scope_init(void)
>>
>> int __init dmar_table_init(void)
>> {
>> - static int dmar_table_initialized;
>> int ret;
>>
>> - if (dmar_table_initialized == 0) {
>> - ret = parse_dmar_table();
>> - if (ret < 0) {
>> - if (ret != -ENODEV)
>> - pr_info("Parse DMAR table failure.\n");
>> - } else if (list_empty(&dmar_drhd_units)) {
>> - pr_info("No DMAR devices found\n");
>> - ret = -ENODEV;
>> - }
>> -
>> - if (ret < 0)
>> - dmar_table_initialized = ret;
>> - else
>> - dmar_table_initialized = 1;
>> + ret = parse_dmar_table();
>> + if (ret < 0) {
>> + if (ret != -ENODEV)
>> + pr_info("Parse DMAR table failure.\n");
>> + } else if (list_empty(&dmar_drhd_units)) {
>> + pr_info("No DMAR devices found\n");
>> + ret = -ENODEV;
>> }
>>
>> - return dmar_table_initialized < 0 ? dmar_table_initialized : 0;
>> + return ret;
>> }
>>
>> static void warn_invalid_dmar(u64 addr, const char *message)
>> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
>> index 687f18f..90f74f4 100644
>> --- a/drivers/iommu/intel-iommu.c
>> +++ b/drivers/iommu/intel-iommu.c
>> @@ -4832,6 +4832,8 @@ int __init intel_iommu_init(void)
>> }
>>
>> down_write(&dmar_global_lock);
>> +
>> + intel_iommu_free_dmars();
>> if (dmar_table_init()) {
>> if (force_on)
>> panic("tboot: Failed to initialize DMAR table\n");
>> diff --git a/drivers/iommu/intel_irq_remapping.c
>> b/drivers/iommu/intel_irq_remapping.c
>> index a5b89f6..ccaacda 100644
>> --- a/drivers/iommu/intel_irq_remapping.c
>> +++ b/drivers/iommu/intel_irq_remapping.c
>> @@ -675,7 +675,7 @@ static void __init intel_cleanup_irq_remapping(void)
>> pr_warn("Failed to enable irq remapping. You are vulnerable to
>> irq-injection attacks.\n");
>> }
>>
>> -static int __init intel_prepare_irq_remapping(void)
>> +static int __init __intel_prepare_irq_remapping(void)
>> {
>> struct dmar_drhd_unit *drhd;
>> struct intel_iommu *iommu;
>> @@ -743,6 +743,21 @@ static int __init intel_prepare_irq_remapping(void)
>> return -ENODEV;
>> }
>>
>> +static int __init intel_prepare_irq_remapping(void)
>> +{
>> + int ret;
>> +
>> + ret = __intel_prepare_irq_remapping();
>> +
>> + if (dmar_tbl_original) {
>> + acpi_put_table(dmar_tbl_original);
>> + dmar_tbl_original = NULL;
>> + dmar_tbl = NULL;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> /*
>> * Set Posted-Interrupts capability.
>> */
>> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
>> index e8ffba1..987b076 100644
>> --- a/include/linux/dmar.h
>> +++ b/include/linux/dmar.h
>> @@ -50,6 +50,8 @@ struct dmar_dev_scope {
>>
>> #ifdef CONFIG_DMAR_TABLE
>> extern struct acpi_table_header *dmar_tbl;
>> +extern struct acpi_table_header *dmar_tbl_original;
>> +
>> struct dmar_drhd_unit {
>> struct list_head list; /* list of drhd units */
>> struct acpi_dmar_header *hdr; /* ACPI header */
>> diff --git a/init/main.c b/init/main.c
>> index 52dee20..052481f 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -655,12 +655,12 @@ asmlinkage __visible void __init start_kernel(void)
>> kmemleak_init();
>> setup_per_cpu_pageset();
>> numa_policy_init();
>> - acpi_early_init();
>> if (late_time_init)
>> late_time_init();
>> calibrate_delay();
>> pidmap_init();
>> anon_vma_init();
>> + acpi_early_init();
>> #ifdef CONFIG_X86
>> if (efi_enabled(EFI_RUNTIME_SERVICES))
>> efi_enter_virtual_mode();
>>
>> Thanks,
>> dou.
>>>
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> dou.
>>>>>
>>>>> Thanks
>>>>> Lv
>>>>>
>>>>>> From: Dou Liyang [mailto:[email protected]]
>>>>>> Sent: Friday, July 14, 2017 1:53 PM
>>>>>> To: [email protected]; [email protected]
>>>>>> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected];
>>>>>> [email protected]; [email protected]; [email protected]; Dou Liyang
>>>>>> <[email protected]>; [email protected]; Rafael J. Wysocki <[email protected]>; Zheng,
>>>>>> Lv <[email protected]>; Julian Wollrath <[email protected]>
>>>>>> Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
>>>>>>
>>>>>> Linux uses acpi_early_init() to put the ACPI table management into
>>>>>> the late stage from the early stage where the mapped ACPI tables is
>>>>>> temporary and should be unmapped.
>>>>>>
>>>>>> But, now initializing interrupt delivery mode should map and parse the
>>>>>> DMAR table earlier in the early stage. This causes an ACPI error when
>>>>>> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
>>>>>> the DMAR table after using in the early stage.
>>>>>>
>>>>>> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
>>>>>> be mapped and parsed in late stage like before.
>>>>>>
>>>>>> Reported-by: Xiaolong Ye <[email protected]>
>>>>>> Signed-off-by: Dou Liyang <[email protected]>
>>>>>> Cc: [email protected]
>>>>>> Cc: Rafael J. Wysocki <[email protected]>
>>>>>> Cc: Zheng, Lv <[email protected]>
>>>>>> Cc: Julian Wollrath <[email protected]>
>>>>>> ---
>>>>>> Test in my own PC(Lenovo M4340).
>>>>>> Ask help for doing regression testing for the bug said in commit c4e1acbb35e4
>>>>>> ("ACPI / init: Invoke early ACPI initialization later").
>>>>>>
>>>>>> init/main.c | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/init/main.c b/init/main.c
>>>>>> index df58a41..7a09467 100644
>>>>>> --- a/init/main.c
>>>>>> +++ b/init/main.c
>>>>>> @@ -654,12 +654,12 @@ asmlinkage __visible void __init start_kernel(void)
>>>>>> kmemleak_init();
>>>>>> setup_per_cpu_pageset();
>>>>>> numa_policy_init();
>>>>>> + acpi_early_init();
>>>>>> if (late_time_init)
>>>>>> late_time_init();
>>>>>> calibrate_delay();
>>>>>> pidmap_init();
>>>>>> anon_vma_init();
>>>>>> - acpi_early_init();
>>>>>> #ifdef CONFIG_X86
>>>>>> if (efi_enabled(EFI_RUNTIME_SERVICES))
>>>>>> efi_enter_virtual_mode();
>>>>>> --
>>>>>> 2.5.5
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>
>
>
On 07/31/17 at 07:20pm, Dou Liyang wrote:
> Hi Baoquan,
>
> At 07/27/2017 02:08 PM, [email protected] wrote:
> > On 07/26/17 at 08:19pm, Dou Liyang wrote:
> > > Hi Baoquan,
> [...]
> > >
> > > I am looking for Thinkpad x121e (AMD E-450 APU) to test. I have tested
> > > it in Thinkpad s430, It's OK.
> > >
> > > BTY, I am confused how does the ACPI subsystem affect PIT which
> > > will be used to fast calibrate CPU frequency[2].
>
> In combination with what Joey said[1] and the code, I guess,
>
> The acpi_enable() called in acpi_enable_system() transfers the system
> into ACPI mode and write data to an I/O port. PIT works using the I/O
> port. In Thinkpad x121e with ACPI mode, the counter in PIT couldn't
> count right and might got an error value.
>
> [1] https://lkml.org/lkml/2014/3/12/3
Awesome! I guess you are possibly correct. A MUST job for entering
into ACPI mode is to enable SCI and hook a handler. SCI will take over
all acpi interrupt events. I googled SCI and vaguely remember someone
said the I/O port r/w event also will be handled by SCI, but didn't find
a clear description in ACPI spec. If that cause failure, it might
happen inside pit_expect_msb() called by quick_pit_calibrate(), maybe
you can try to add debug message in this place, not very sure if it
be debugged.
Maybe tglx or Rafael can help have a look and confirm if it's correct
or not.
Thanks
Baoquan
> >
> > I checked code, and have no any idea. Add Joey Lee to list, see if he
> > can help answer this.
> >
> > >
> > > Do you have any idea?
> > >
> > > [1] https://lkml.org/lkml/2014/3/10/123
> > > [2] https://lkml.org/lkml/2014/3/12/3
> > >
> > >
> > > drivers/iommu/dmar.c | 27 +++++++++++----------------
> > > drivers/iommu/intel-iommu.c | 2 ++
> > > drivers/iommu/intel_irq_remapping.c | 17 ++++++++++++++++-
> > > include/linux/dmar.h | 2 ++
> > > init/main.c | 2 +-
> > > 5 files changed, 32 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
> > > index c8b0329..e6261b7 100644
> > > --- a/drivers/iommu/dmar.c
> > > +++ b/drivers/iommu/dmar.c
> > > @@ -68,6 +68,8 @@ DECLARE_RWSEM(dmar_global_lock);
> > > LIST_HEAD(dmar_drhd_units);
> > >
> > > struct acpi_table_header * __initdata dmar_tbl;
> > > +struct acpi_table_header * __initdata dmar_tbl_original;
> > > +
> > > static int dmar_dev_scope_status = 1;
> > > static unsigned long dmar_seq_ids[BITS_TO_LONGS(DMAR_UNITS_SUPPORTED)];
> > >
> > > @@ -627,6 +629,7 @@ parse_dmar_table(void)
> > > * fixed map.
> > > */
> > > dmar_table_detect();
> > > + dmar_tbl_original = dmar_tbl;
> > >
> > > /*
> > > * ACPI tables may not be DMA protected by tboot, so use DMAR copy
> > > @@ -811,26 +814,18 @@ int __init dmar_dev_scope_init(void)
> > >
> > > int __init dmar_table_init(void)
> > > {
> > > - static int dmar_table_initialized;
> > > int ret;
> > >
> > > - if (dmar_table_initialized == 0) {
> > > - ret = parse_dmar_table();
> > > - if (ret < 0) {
> > > - if (ret != -ENODEV)
> > > - pr_info("Parse DMAR table failure.\n");
> > > - } else if (list_empty(&dmar_drhd_units)) {
> > > - pr_info("No DMAR devices found\n");
> > > - ret = -ENODEV;
> > > - }
> > > -
> > > - if (ret < 0)
> > > - dmar_table_initialized = ret;
> > > - else
> > > - dmar_table_initialized = 1;
> > > + ret = parse_dmar_table();
> > > + if (ret < 0) {
> > > + if (ret != -ENODEV)
> > > + pr_info("Parse DMAR table failure.\n");
> > > + } else if (list_empty(&dmar_drhd_units)) {
> > > + pr_info("No DMAR devices found\n");
> > > + ret = -ENODEV;
> > > }
> > >
> > > - return dmar_table_initialized < 0 ? dmar_table_initialized : 0;
> > > + return ret;
> > > }
> > >
> > > static void warn_invalid_dmar(u64 addr, const char *message)
> > > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> > > index 687f18f..90f74f4 100644
> > > --- a/drivers/iommu/intel-iommu.c
> > > +++ b/drivers/iommu/intel-iommu.c
> > > @@ -4832,6 +4832,8 @@ int __init intel_iommu_init(void)
> > > }
> > >
> > > down_write(&dmar_global_lock);
> > > +
> > > + intel_iommu_free_dmars();
> > > if (dmar_table_init()) {
> > > if (force_on)
> > > panic("tboot: Failed to initialize DMAR table\n");
> > > diff --git a/drivers/iommu/intel_irq_remapping.c
> > > b/drivers/iommu/intel_irq_remapping.c
> > > index a5b89f6..ccaacda 100644
> > > --- a/drivers/iommu/intel_irq_remapping.c
> > > +++ b/drivers/iommu/intel_irq_remapping.c
> > > @@ -675,7 +675,7 @@ static void __init intel_cleanup_irq_remapping(void)
> > > pr_warn("Failed to enable irq remapping. You are vulnerable to
> > > irq-injection attacks.\n");
> > > }
> > >
> > > -static int __init intel_prepare_irq_remapping(void)
> > > +static int __init __intel_prepare_irq_remapping(void)
> > > {
> > > struct dmar_drhd_unit *drhd;
> > > struct intel_iommu *iommu;
> > > @@ -743,6 +743,21 @@ static int __init intel_prepare_irq_remapping(void)
> > > return -ENODEV;
> > > }
> > >
> > > +static int __init intel_prepare_irq_remapping(void)
> > > +{
> > > + int ret;
> > > +
> > > + ret = __intel_prepare_irq_remapping();
> > > +
> > > + if (dmar_tbl_original) {
> > > + acpi_put_table(dmar_tbl_original);
> > > + dmar_tbl_original = NULL;
> > > + dmar_tbl = NULL;
> > > + }
> > > +
> > > + return ret;
> > > +}
> > > +
> > > /*
> > > * Set Posted-Interrupts capability.
> > > */
> > > diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> > > index e8ffba1..987b076 100644
> > > --- a/include/linux/dmar.h
> > > +++ b/include/linux/dmar.h
> > > @@ -50,6 +50,8 @@ struct dmar_dev_scope {
> > >
> > > #ifdef CONFIG_DMAR_TABLE
> > > extern struct acpi_table_header *dmar_tbl;
> > > +extern struct acpi_table_header *dmar_tbl_original;
> > > +
> > > struct dmar_drhd_unit {
> > > struct list_head list; /* list of drhd units */
> > > struct acpi_dmar_header *hdr; /* ACPI header */
> > > diff --git a/init/main.c b/init/main.c
> > > index 52dee20..052481f 100644
> > > --- a/init/main.c
> > > +++ b/init/main.c
> > > @@ -655,12 +655,12 @@ asmlinkage __visible void __init start_kernel(void)
> > > kmemleak_init();
> > > setup_per_cpu_pageset();
> > > numa_policy_init();
> > > - acpi_early_init();
> > > if (late_time_init)
> > > late_time_init();
> > > calibrate_delay();
> > > pidmap_init();
> > > anon_vma_init();
> > > + acpi_early_init();
> > > #ifdef CONFIG_X86
> > > if (efi_enabled(EFI_RUNTIME_SERVICES))
> > > efi_enter_virtual_mode();
> > >
> > > Thanks,
> > > dou.
> > > >
> > > > >
> > > > >
> > > > > Thanks,
> > > > >
> > > > > dou.
> > > > > >
> > > > > > Thanks
> > > > > > Lv
> > > > > >
> > > > > > > From: Dou Liyang [mailto:[email protected]]
> > > > > > > Sent: Friday, July 14, 2017 1:53 PM
> > > > > > > To: [email protected]; [email protected]
> > > > > > > Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected];
> > > > > > > [email protected]; [email protected]; [email protected]; Dou Liyang
> > > > > > > <[email protected]>; [email protected]; Rafael J. Wysocki <[email protected]>; Zheng,
> > > > > > > Lv <[email protected]>; Julian Wollrath <[email protected]>
> > > > > > > Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
> > > > > > >
> > > > > > > Linux uses acpi_early_init() to put the ACPI table management into
> > > > > > > the late stage from the early stage where the mapped ACPI tables is
> > > > > > > temporary and should be unmapped.
> > > > > > >
> > > > > > > But, now initializing interrupt delivery mode should map and parse the
> > > > > > > DMAR table earlier in the early stage. This causes an ACPI error when
> > > > > > > Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
> > > > > > > the DMAR table after using in the early stage.
> > > > > > >
> > > > > > > Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
> > > > > > > be mapped and parsed in late stage like before.
> > > > > > >
> > > > > > > Reported-by: Xiaolong Ye <[email protected]>
> > > > > > > Signed-off-by: Dou Liyang <[email protected]>
> > > > > > > Cc: [email protected]
> > > > > > > Cc: Rafael J. Wysocki <[email protected]>
> > > > > > > Cc: Zheng, Lv <[email protected]>
> > > > > > > Cc: Julian Wollrath <[email protected]>
> > > > > > > ---
> > > > > > > Test in my own PC(Lenovo M4340).
> > > > > > > Ask help for doing regression testing for the bug said in commit c4e1acbb35e4
> > > > > > > ("ACPI / init: Invoke early ACPI initialization later").
> > > > > > >
> > > > > > > init/main.c | 2 +-
> > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/init/main.c b/init/main.c
> > > > > > > index df58a41..7a09467 100644
> > > > > > > --- a/init/main.c
> > > > > > > +++ b/init/main.c
> > > > > > > @@ -654,12 +654,12 @@ asmlinkage __visible void __init start_kernel(void)
> > > > > > > kmemleak_init();
> > > > > > > setup_per_cpu_pageset();
> > > > > > > numa_policy_init();
> > > > > > > + acpi_early_init();
> > > > > > > if (late_time_init)
> > > > > > > late_time_init();
> > > > > > > calibrate_delay();
> > > > > > > pidmap_init();
> > > > > > > anon_vma_init();
> > > > > > > - acpi_early_init();
> > > > > > > #ifdef CONFIG_X86
> > > > > > > if (efi_enabled(EFI_RUNTIME_SERVICES))
> > > > > > > efi_enter_virtual_mode();
> > > > > > > --
> > > > > > > 2.5.5
> > > > > > >
> > > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > >
> > > > >
> > > >
> > > >
> > > >
> > >
> > >
> >
> >
> >
>
>
Hi Rafael, Zheng,
At 07/31/2017 06:50 PM, Dou Liyang wrote:
> Hi,
>
> At 07/14/2017 01:52 PM, Dou Liyang wrote:
>> Linux uses acpi_early_init() to put the ACPI table management into
>> the late stage from the early stage where the mapped ACPI tables is
>> temporary and should be unmapped.
>>
>> But, now initializing interrupt delivery mode should map and parse the
>> DMAR table earlier in the early stage. This causes an ACPI error when
>> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
>> the DMAR table after using in the early stage.
>>
>> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
>> be mapped and parsed in late stage like before.
>>
>> Reported-by: Xiaolong Ye <[email protected]>
>> Signed-off-by: Dou Liyang <[email protected]>
>> Cc: [email protected]
>> Cc: Rafael J. Wysocki <[email protected]>
>> Cc: Zheng, Lv <[email protected]>
>> Cc: Julian Wollrath <[email protected]>
>> ---
>> Test in my own PC(Lenovo M4340).
>> Ask help for doing regression testing for the bug said in commit
>> c4e1acbb35e4
>> ("ACPI / init: Invoke early ACPI initialization later").
>>
>
> Now, I can prove this patch doesn't result in the bug[1] which made the
> fast TSC calibration using PIT failed in a Thinkpad x121e (AMD E-450
> APU).
>
> The true reason of the bug is enabling ACPI subsystem earlier than
> using PIT, not the SCI setup. invoking acpi_enable_subsystem() later
> could fix this bug as Julian tested and said[2].
>
> And, I found that Commit b064a8fa77df (" ACPI / init: Switch over
> platform to the ACPI mode later") split the ACPI early initialization
> code into acpi_early_init() and acpi_subsystem_init(). executing
> acpi_enable_subsystem() at the original early ACPI initialization spot.
>
> The sequence of them shows below:
>
> start_kernel
> +---------------+
> |
> +--> .......
> |
> | late_time_init()
> +--> +-------+
> |
> +--> .......
> |
> | acpi_early_init()
> +--> +-------+
> |
> +--> .......
> |
> | acpi_subsystem_init()
> +-> +--------+
>
> We make sure the acpi_subsystem_init() is called later than
> late_time_init(), the bug will be avoided.
>
> This patch changes the sequence of late_time_init() and
> acpi_early_init(), doesn't effect acpi_subsystem_init().
>
> So, this patch is OK.
>
> Btw, Thanks very much for Borislav Petkov, he will have access to
> Thinkpad x121e from Mid-August and will test this series.
>
Almost one month passed, Borislav have tested this series in Thinkpad
x121e and I also have tested in my box and QEmu again. It is OK.
BTW,
1) I found your commit b064a8fa77df (" ACPI / init: Switch over
platform to the ACPI mode later") split the ACPI early initialization
code into acpi_early_init() and acpi_subsystem_init(). Actually enabling
the ACPI subsystem is in acpi_subsystem_init().
2) As we discussed earlier, invoking acpi_put_table() is not good for
this situation.
So I do this patch, Is that goot to you? Any comments will be welcome.
If it is OK, As the patches need to be re-based, and I also found
several spelling mistake, I will send a new version next week.
Thanks,
dou.
> [1] https://lkml.org/lkml/2014/3/10/123
> [2] https://lkml.org/lkml/2014/3/12/311
>
>
> Thanks
> dou.
>
>> init/main.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/init/main.c b/init/main.c
>> index df58a41..7a09467 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -654,12 +654,12 @@ asmlinkage __visible void __init start_kernel(void)
>> kmemleak_init();
>> setup_per_cpu_pageset();
>> numa_policy_init();
>> + acpi_early_init();
>> if (late_time_init)
>> late_time_init();
>> calibrate_delay();
>> pidmap_init();
>> anon_vma_init();
>> - acpi_early_init();
>> #ifdef CONFIG_X86
>> if (efi_enabled(EFI_RUNTIME_SERVICES))
>> efi_enter_virtual_mode();
>>
Hi Liyang,
On 08/24/17 at 11:54am, Dou Liyang wrote:
> > > Test in my own PC(Lenovo M4340).
> > > Ask help for doing regression testing for the bug said in commit
> > > c4e1acbb35e4
> > > ("ACPI / init: Invoke early ACPI initialization later").
> > >
> >
> > Now, I can prove this patch doesn't result in the bug[1] which made the
> > fast TSC calibration using PIT failed in a Thinkpad x121e (AMD E-450
> > APU).
> >
> > The true reason of the bug is enabling ACPI subsystem earlier than
> > using PIT, not the SCI setup. invoking acpi_enable_subsystem() later
Seems redhat mail server was down earlier, I didn't receive new mail in
this thread. Just curious, do you know why the fast tsc calibration
using PIT will fail if enabling ACPI subsystem earlier than using PIT?
Thanks
Baoquan
> > could fix this bug as Julian tested and said[2].
> >
> > And, I found that Commit b064a8fa77df (" ACPI / init: Switch over
> > platform to the ACPI mode later") split the ACPI early initialization
> > code into acpi_early_init() and acpi_subsystem_init(). executing
> > acpi_enable_subsystem() at the original early ACPI initialization spot.
> >
> > The sequence of them shows below:
> >
> > start_kernel
> > +---------------+
> > |
> > +--> .......
> > |
> > | late_time_init()
> > +--> +-------+
> > |
> > +--> .......
> > |
> > | acpi_early_init()
> > +--> +-------+
> > |
> > +--> .......
> > |
> > | acpi_subsystem_init()
> > +-> +--------+
> >
> > We make sure the acpi_subsystem_init() is called later than
> > late_time_init(), the bug will be avoided.
> >
> > This patch changes the sequence of late_time_init() and
> > acpi_early_init(), doesn't effect acpi_subsystem_init().
> >
> > So, this patch is OK.
> >
> > Btw, Thanks very much for Borislav Petkov, he will have access to
> > Thinkpad x121e from Mid-August and will test this series.
> >
>
> Almost one month passed, Borislav have tested this series in Thinkpad
> x121e and I also have tested in my box and QEmu again. It is OK.
>
> BTW,
> 1) I found your commit b064a8fa77df (" ACPI / init: Switch over
> platform to the ACPI mode later") split the ACPI early initialization
> code into acpi_early_init() and acpi_subsystem_init(). Actually enabling
> the ACPI subsystem is in acpi_subsystem_init().
>
> 2) As we discussed earlier, invoking acpi_put_table() is not good for
> this situation.
>
> So I do this patch, Is that goot to you? Any comments will be welcome.
>
> If it is OK, As the patches need to be re-based, and I also found
> several spelling mistake, I will send a new version next week.
>
> Thanks,
> dou.
>
> > [1] https://lkml.org/lkml/2014/3/10/123
> > [2] https://lkml.org/lkml/2014/3/12/311
> >
> >
> > Thanks
> > dou.
> >
> > > init/main.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/init/main.c b/init/main.c
> > > index df58a41..7a09467 100644
> > > --- a/init/main.c
> > > +++ b/init/main.c
> > > @@ -654,12 +654,12 @@ asmlinkage __visible void __init start_kernel(void)
> > > kmemleak_init();
> > > setup_per_cpu_pageset();
> > > numa_policy_init();
> > > + acpi_early_init();
> > > if (late_time_init)
> > > late_time_init();
> > > calibrate_delay();
> > > pidmap_init();
> > > anon_vma_init();
> > > - acpi_early_init();
> > > #ifdef CONFIG_X86
> > > if (efi_enabled(EFI_RUNTIME_SERVICES))
> > > efi_enter_virtual_mode();
> > >
>
>
Hi Baoquan,
Thanks for your reply.
At 08/24/2017 04:05 PM, Baoquan He wrote:
> Hi Liyang,
>
> On 08/24/17 at 11:54am, Dou Liyang wrote:
>>>> Test in my own PC(Lenovo M4340).
>>>> Ask help for doing regression testing for the bug said in commit
>>>> c4e1acbb35e4
>>>> ("ACPI / init: Invoke early ACPI initialization later").
>>>>
>>>
>>> Now, I can prove this patch doesn't result in the bug[1] which made the
>>> fast TSC calibration using PIT failed in a Thinkpad x121e (AMD E-450
>>> APU).
>>>
>>> The true reason of the bug is enabling ACPI subsystem earlier than
>>> using PIT, not the SCI setup. invoking acpi_enable_subsystem() later
>
> Seems redhat mail server was down earlier, I didn't receive new mail in
> this thread. Just curious, do you know why the fast tsc calibration
> using PIT will fail if enabling ACPI subsystem earlier than using PIT?
>
It's related to particular hardware, As you know, I tested in many
kinds of PC and laptop and PIT works well no matter before or after
enabling ACPI subsystem.
In pit_verify_msb(), we use inb(0x42) to read the current MSB,
Normally, the value is continuously, like following shows:
msb = fe
msb = fd
msb = fc
msb = fb
msb = fa
msb = f9
msb = f8
msb = f7
msb = f6
...
But, if in some particular hardware, you will see like that:
msb = fe
msb = f0
msb = ed
msb = e9
msb = e0
msb = db
...
In this case, the count in pit_expect_msb() is always zero.
So we will see "Fast TSC calibration failed" in our dmesg log.
For the further deep reason why the hardware failed, I'm sorry
I can't answer and don't know how to investigate. For hardware,
I usually change a new one directly and know very little.
Thanks,
dou.
> Thanks
> Baoquan
>
>>> could fix this bug as Julian tested and said[2].
>>>
>>> And, I found that Commit b064a8fa77df (" ACPI / init: Switch over
>>> platform to the ACPI mode later") split the ACPI early initialization
>>> code into acpi_early_init() and acpi_subsystem_init(). executing
>>> acpi_enable_subsystem() at the original early ACPI initialization spot.
>>>
>>> The sequence of them shows below:
>>>
>>> start_kernel
>>> +---------------+
>>> |
>>> +--> .......
>>> |
>>> | late_time_init()
>>> +--> +-------+
>>> |
>>> +--> .......
>>> |
>>> | acpi_early_init()
>>> +--> +-------+
>>> |
>>> +--> .......
>>> |
>>> | acpi_subsystem_init()
>>> +-> +--------+
>>>
>>> We make sure the acpi_subsystem_init() is called later than
>>> late_time_init(), the bug will be avoided.
>>>
>>> This patch changes the sequence of late_time_init() and
>>> acpi_early_init(), doesn't effect acpi_subsystem_init().
>>>
>>> So, this patch is OK.
>>>
>>> Btw, Thanks very much for Borislav Petkov, he will have access to
>>> Thinkpad x121e from Mid-August and will test this series.
>>>
>>
>> Almost one month passed, Borislav have tested this series in Thinkpad
>> x121e and I also have tested in my box and QEmu again. It is OK.
>>
>> BTW,
>> 1) I found your commit b064a8fa77df (" ACPI / init: Switch over
>> platform to the ACPI mode later") split the ACPI early initialization
>> code into acpi_early_init() and acpi_subsystem_init(). Actually enabling
>> the ACPI subsystem is in acpi_subsystem_init().
>>
>> 2) As we discussed earlier, invoking acpi_put_table() is not good for
>> this situation.
>>
>> So I do this patch, Is that goot to you? Any comments will be welcome.
>>
>> If it is OK, As the patches need to be re-based, and I also found
>> several spelling mistake, I will send a new version next week.
>>
>> Thanks,
>> dou.
>>
>>> [1] https://lkml.org/lkml/2014/3/10/123
>>> [2] https://lkml.org/lkml/2014/3/12/311
>>>
>>>
>>> Thanks
>>> dou.
>>>
>>>> init/main.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/init/main.c b/init/main.c
>>>> index df58a41..7a09467 100644
>>>> --- a/init/main.c
>>>> +++ b/init/main.c
>>>> @@ -654,12 +654,12 @@ asmlinkage __visible void __init start_kernel(void)
>>>> kmemleak_init();
>>>> setup_per_cpu_pageset();
>>>> numa_policy_init();
>>>> + acpi_early_init();
>>>> if (late_time_init)
>>>> late_time_init();
>>>> calibrate_delay();
>>>> pidmap_init();
>>>> anon_vma_init();
>>>> - acpi_early_init();
>>>> #ifdef CONFIG_X86
>>>> if (efi_enabled(EFI_RUNTIME_SERVICES))
>>>> efi_enter_virtual_mode();
>>>>
>>
>>
>
>
>
On 08/24/17 at 05:28pm, Dou Liyang wrote:
> Hi Baoquan,
>
> Thanks for your reply.
>
> At 08/24/2017 04:05 PM, Baoquan He wrote:
> > Hi Liyang,
> >
> > On 08/24/17 at 11:54am, Dou Liyang wrote:
> > > > > Test in my own PC(Lenovo M4340).
> > > > > Ask help for doing regression testing for the bug said in commit
> > > > > c4e1acbb35e4
> > > > > ("ACPI / init: Invoke early ACPI initialization later").
> > > > >
> > > >
> > > > Now, I can prove this patch doesn't result in the bug[1] which made the
> > > > fast TSC calibration using PIT failed in a Thinkpad x121e (AMD E-450
> > > > APU).
> > > >
> > > > The true reason of the bug is enabling ACPI subsystem earlier than
> > > > using PIT, not the SCI setup. invoking acpi_enable_subsystem() later
> >
> > Seems redhat mail server was down earlier, I didn't receive new mail in
> > this thread. Just curious, do you know why the fast tsc calibration
> > using PIT will fail if enabling ACPI subsystem earlier than using PIT?
> >
> It's related to particular hardware, As you know, I tested in many
> kinds of PC and laptop and PIT works well no matter before or after
> enabling ACPI subsystem.
>
> In pit_verify_msb(), we use inb(0x42) to read the current MSB,
>
> Normally, the value is continuously, like following shows:
>
> msb = fe
> msb = fd
> msb = fc
> msb = fb
> msb = fa
> msb = f9
> msb = f8
> msb = f7
> msb = f6
> ...
>
> But, if in some particular hardware, you will see like that:
>
> msb = fe
> msb = f0
> msb = ed
> msb = e9
> msb = e0
> msb = db
> ...
>
> In this case, the count in pit_expect_msb() is always zero.
> So we will see "Fast TSC calibration failed" in our dmesg log.
Thanks for telling!
It's truly weird that the TSC becomes unstable only if enabling ACPI
subsystem earlier than using PIT.
Let's see what other people say about this.
Btw, you will resend another round, right? Then I would like to test
your new post.
Thanks
Baoquan
>
> For the further deep reason why the hardware failed, I'm sorry
> I can't answer and don't know how to investigate. For hardware,
> I usually change a new one directly and know very little.
>
> Thanks,
> dou.
>
>
> > Thanks
> > Baoquan
> >
> > > > could fix this bug as Julian tested and said[2].
> > > >
> > > > And, I found that Commit b064a8fa77df (" ACPI / init: Switch over
> > > > platform to the ACPI mode later") split the ACPI early initialization
> > > > code into acpi_early_init() and acpi_subsystem_init(). executing
> > > > acpi_enable_subsystem() at the original early ACPI initialization spot.
> > > >
> > > > The sequence of them shows below:
> > > >
> > > > start_kernel
> > > > +---------------+
> > > > |
> > > > +--> .......
> > > > |
> > > > | late_time_init()
> > > > +--> +-------+
> > > > |
> > > > +--> .......
> > > > |
> > > > | acpi_early_init()
> > > > +--> +-------+
> > > > |
> > > > +--> .......
> > > > |
> > > > | acpi_subsystem_init()
> > > > +-> +--------+
> > > >
> > > > We make sure the acpi_subsystem_init() is called later than
> > > > late_time_init(), the bug will be avoided.
> > > >
> > > > This patch changes the sequence of late_time_init() and
> > > > acpi_early_init(), doesn't effect acpi_subsystem_init().
> > > >
> > > > So, this patch is OK.
> > > >
> > > > Btw, Thanks very much for Borislav Petkov, he will have access to
> > > > Thinkpad x121e from Mid-August and will test this series.
> > > >
> > >
> > > Almost one month passed, Borislav have tested this series in Thinkpad
> > > x121e and I also have tested in my box and QEmu again. It is OK.
> > >
> > > BTW,
> > > 1) I found your commit b064a8fa77df (" ACPI / init: Switch over
> > > platform to the ACPI mode later") split the ACPI early initialization
> > > code into acpi_early_init() and acpi_subsystem_init(). Actually enabling
> > > the ACPI subsystem is in acpi_subsystem_init().
> > >
> > > 2) As we discussed earlier, invoking acpi_put_table() is not good for
> > > this situation.
> > >
> > > So I do this patch, Is that goot to you? Any comments will be welcome.
> > >
> > > If it is OK, As the patches need to be re-based, and I also found
> > > several spelling mistake, I will send a new version next week.
> > >
> > > Thanks,
> > > dou.
> > >
> > > > [1] https://lkml.org/lkml/2014/3/10/123
> > > > [2] https://lkml.org/lkml/2014/3/12/311
> > > >
> > > >
> > > > Thanks
> > > > dou.
> > > >
> > > > > init/main.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/init/main.c b/init/main.c
> > > > > index df58a41..7a09467 100644
> > > > > --- a/init/main.c
> > > > > +++ b/init/main.c
> > > > > @@ -654,12 +654,12 @@ asmlinkage __visible void __init start_kernel(void)
> > > > > kmemleak_init();
> > > > > setup_per_cpu_pageset();
> > > > > numa_policy_init();
> > > > > + acpi_early_init();
> > > > > if (late_time_init)
> > > > > late_time_init();
> > > > > calibrate_delay();
> > > > > pidmap_init();
> > > > > anon_vma_init();
> > > > > - acpi_early_init();
> > > > > #ifdef CONFIG_X86
> > > > > if (efi_enabled(EFI_RUNTIME_SERVICES))
> > > > > efi_enter_virtual_mode();
> > > > >
> > >
> > >
> >
> >
> >
>
>
At 08/24/2017 06:21 PM, Baoquan He wrote:
> On 08/24/17 at 05:28pm, Dou Liyang wrote:
>> Hi Baoquan,
>>
>> Thanks for your reply.
>>
>> At 08/24/2017 04:05 PM, Baoquan He wrote:
>>> Hi Liyang,
>>>
>>> On 08/24/17 at 11:54am, Dou Liyang wrote:
>>>>>> Test in my own PC(Lenovo M4340).
>>>>>> Ask help for doing regression testing for the bug said in commit
>>>>>> c4e1acbb35e4
>>>>>> ("ACPI / init: Invoke early ACPI initialization later").
>>>>>>
>>>>>
>>>>> Now, I can prove this patch doesn't result in the bug[1] which made the
>>>>> fast TSC calibration using PIT failed in a Thinkpad x121e (AMD E-450
>>>>> APU).
>>>>>
>>>>> The true reason of the bug is enabling ACPI subsystem earlier than
>>>>> using PIT, not the SCI setup. invoking acpi_enable_subsystem() later
>>>
>>> Seems redhat mail server was down earlier, I didn't receive new mail in
>>> this thread. Just curious, do you know why the fast tsc calibration
>>> using PIT will fail if enabling ACPI subsystem earlier than using PIT?
>>>
>> It's related to particular hardware, As you know, I tested in many
>> kinds of PC and laptop and PIT works well no matter before or after
>> enabling ACPI subsystem.
>>
>> In pit_verify_msb(), we use inb(0x42) to read the current MSB,
>>
>> Normally, the value is continuously, like following shows:
>>
>> msb = fe
>> msb = fd
>> msb = fc
>> msb = fb
>> msb = fa
>> msb = f9
>> msb = f8
>> msb = f7
>> msb = f6
>> ...
>>
>> But, if in some particular hardware, you will see like that:
>>
>> msb = fe
>> msb = f0
>> msb = ed
>> msb = e9
>> msb = e0
>> msb = db
>> ...
>>
>> In this case, the count in pit_expect_msb() is always zero.
>> So we will see "Fast TSC calibration failed" in our dmesg log.
>
> Thanks for telling!
>
> It's truly weird that the TSC becomes unstable only if enabling ACPI
> subsystem earlier than using PIT.
>
> Let's see what other people say about this.
>
> Btw, you will resend another round, right? Then I would like to test
> your new post.
Yes, I am waiting ACPI maintainers advice and I prepare to re-base it
next week when rc7 is out.
Thanks,
dou.
>
> Thanks
> Baoquan
>
>>
>> For the further deep reason why the hardware failed, I'm sorry
>> I can't answer and don't know how to investigate. For hardware,
>> I usually change a new one directly and know very little.
>>
>> Thanks,
>> dou.
>>
>>
>>> Thanks
>>> Baoquan
>>>
>>>>> could fix this bug as Julian tested and said[2].
>>>>>
>>>>> And, I found that Commit b064a8fa77df (" ACPI / init: Switch over
>>>>> platform to the ACPI mode later") split the ACPI early initialization
>>>>> code into acpi_early_init() and acpi_subsystem_init(). executing
>>>>> acpi_enable_subsystem() at the original early ACPI initialization spot.
>>>>>
>>>>> The sequence of them shows below:
>>>>>
>>>>> start_kernel
>>>>> +---------------+
>>>>> |
>>>>> +--> .......
>>>>> |
>>>>> | late_time_init()
>>>>> +--> +-------+
>>>>> |
>>>>> +--> .......
>>>>> |
>>>>> | acpi_early_init()
>>>>> +--> +-------+
>>>>> |
>>>>> +--> .......
>>>>> |
>>>>> | acpi_subsystem_init()
>>>>> +-> +--------+
>>>>>
>>>>> We make sure the acpi_subsystem_init() is called later than
>>>>> late_time_init(), the bug will be avoided.
>>>>>
>>>>> This patch changes the sequence of late_time_init() and
>>>>> acpi_early_init(), doesn't effect acpi_subsystem_init().
>>>>>
>>>>> So, this patch is OK.
>>>>>
>>>>> Btw, Thanks very much for Borislav Petkov, he will have access to
>>>>> Thinkpad x121e from Mid-August and will test this series.
>>>>>
>>>>
>>>> Almost one month passed, Borislav have tested this series in Thinkpad
>>>> x121e and I also have tested in my box and QEmu again. It is OK.
>>>>
>>>> BTW,
>>>> 1) I found your commit b064a8fa77df (" ACPI / init: Switch over
>>>> platform to the ACPI mode later") split the ACPI early initialization
>>>> code into acpi_early_init() and acpi_subsystem_init(). Actually enabling
>>>> the ACPI subsystem is in acpi_subsystem_init().
>>>>
>>>> 2) As we discussed earlier, invoking acpi_put_table() is not good for
>>>> this situation.
>>>>
>>>> So I do this patch, Is that goot to you? Any comments will be welcome.
>>>>
>>>> If it is OK, As the patches need to be re-based, and I also found
>>>> several spelling mistake, I will send a new version next week.
>>>>
>>>> Thanks,
>>>> dou.
>>>>
>>>>> [1] https://lkml.org/lkml/2014/3/10/123
>>>>> [2] https://lkml.org/lkml/2014/3/12/311
>>>>>
>>>>>
>>>>> Thanks
>>>>> dou.
>>>>>
>>>>>> init/main.c | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/init/main.c b/init/main.c
>>>>>> index df58a41..7a09467 100644
>>>>>> --- a/init/main.c
>>>>>> +++ b/init/main.c
>>>>>> @@ -654,12 +654,12 @@ asmlinkage __visible void __init start_kernel(void)
>>>>>> kmemleak_init();
>>>>>> setup_per_cpu_pageset();
>>>>>> numa_policy_init();
>>>>>> + acpi_early_init();
>>>>>> if (late_time_init)
>>>>>> late_time_init();
>>>>>> calibrate_delay();
>>>>>> pidmap_init();
>>>>>> anon_vma_init();
>>>>>> - acpi_early_init();
>>>>>> #ifdef CONFIG_X86
>>>>>> if (efi_enabled(EFI_RUNTIME_SERVICES))
>>>>>> efi_enter_virtual_mode();
>>>>>>
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>
>
>
On Thursday, August 24, 2017 5:54:28 AM CEST Dou Liyang wrote:
> Hi Rafael, Zheng,
>
> At 07/31/2017 06:50 PM, Dou Liyang wrote:
> > Hi,
> >
> > At 07/14/2017 01:52 PM, Dou Liyang wrote:
> >> Linux uses acpi_early_init() to put the ACPI table management into
> >> the late stage from the early stage where the mapped ACPI tables is
> >> temporary and should be unmapped.
> >>
> >> But, now initializing interrupt delivery mode should map and parse the
> >> DMAR table earlier in the early stage. This causes an ACPI error when
> >> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
> >> the DMAR table after using in the early stage.
> >>
> >> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
> >> be mapped and parsed in late stage like before.
> >>
> >> Reported-by: Xiaolong Ye <[email protected]>
> >> Signed-off-by: Dou Liyang <[email protected]>
> >> Cc: [email protected]
> >> Cc: Rafael J. Wysocki <[email protected]>
> >> Cc: Zheng, Lv <[email protected]>
> >> Cc: Julian Wollrath <[email protected]>
> >> ---
> >> Test in my own PC(Lenovo M4340).
> >> Ask help for doing regression testing for the bug said in commit
> >> c4e1acbb35e4
> >> ("ACPI / init: Invoke early ACPI initialization later").
> >>
> >
> > Now, I can prove this patch doesn't result in the bug[1] which made the
> > fast TSC calibration using PIT failed in a Thinkpad x121e (AMD E-450
> > APU).
> >
> > The true reason of the bug is enabling ACPI subsystem earlier than
> > using PIT, not the SCI setup. invoking acpi_enable_subsystem() later
> > could fix this bug as Julian tested and said[2].
> >
> > And, I found that Commit b064a8fa77df (" ACPI / init: Switch over
> > platform to the ACPI mode later") split the ACPI early initialization
> > code into acpi_early_init() and acpi_subsystem_init(). executing
> > acpi_enable_subsystem() at the original early ACPI initialization spot.
> >
> > The sequence of them shows below:
> >
> > start_kernel
> > +---------------+
> > |
> > +--> .......
> > |
> > | late_time_init()
> > +--> +-------+
> > |
> > +--> .......
> > |
> > | acpi_early_init()
> > +--> +-------+
> > |
> > +--> .......
> > |
> > | acpi_subsystem_init()
> > +-> +--------+
> >
> > We make sure the acpi_subsystem_init() is called later than
> > late_time_init(), the bug will be avoided.
> >
> > This patch changes the sequence of late_time_init() and
> > acpi_early_init(), doesn't effect acpi_subsystem_init().
> >
> > So, this patch is OK.
> >
> > Btw, Thanks very much for Borislav Petkov, he will have access to
> > Thinkpad x121e from Mid-August and will test this series.
> >
>
> Almost one month passed, Borislav have tested this series in Thinkpad
> x121e and I also have tested in my box and QEmu again. It is OK.
>
> BTW,
> 1) I found your commit b064a8fa77df (" ACPI / init: Switch over
> platform to the ACPI mode later") split the ACPI early initialization
> code into acpi_early_init() and acpi_subsystem_init(). Actually enabling
> the ACPI subsystem is in acpi_subsystem_init().
>
> 2) As we discussed earlier, invoking acpi_put_table() is not good for
> this situation.
>
> So I do this patch, Is that goot to you? Any comments will be welcome.
>
> If it is OK, As the patches need to be re-based, and I also found
> several spelling mistake, I will send a new version next week.
OK, but does it depend on anything? Or does anything depend on it?
It is [12/13] in a series, so it looks like it doesn't depend on the
previous patches in it, but the next one may depend on it? Which is the
case?
Thanks,
Rafael
Hi Rafael,
At 08/25/2017 12:38 AM, Rafael J. Wysocki wrote:
> On Thursday, August 24, 2017 5:54:28 AM CEST Dou Liyang wrote:
>> Hi Rafael, Zheng,
>>
>> At 07/31/2017 06:50 PM, Dou Liyang wrote:
>>> Hi,
>>>
>>> At 07/14/2017 01:52 PM, Dou Liyang wrote:
>>>> Linux uses acpi_early_init() to put the ACPI table management into
>>>> the late stage from the early stage where the mapped ACPI tables is
>>>> temporary and should be unmapped.
>>>>
>>>> But, now initializing interrupt delivery mode should map and parse the
>>>> DMAR table earlier in the early stage. This causes an ACPI error when
>>>> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
>>>> the DMAR table after using in the early stage.
>>>>
>>>> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
>>>> be mapped and parsed in late stage like before.
>>>>
>>>> Reported-by: Xiaolong Ye <[email protected]>
>>>> Signed-off-by: Dou Liyang <[email protected]>
>>>> Cc: [email protected]
>>>> Cc: Rafael J. Wysocki <[email protected]>
>>>> Cc: Zheng, Lv <[email protected]>
>>>> Cc: Julian Wollrath <[email protected]>
>>>> ---
>>>> Test in my own PC(Lenovo M4340).
>>>> Ask help for doing regression testing for the bug said in commit
>>>> c4e1acbb35e4
>>>> ("ACPI / init: Invoke early ACPI initialization later").
>>>>
>>>
>>> Now, I can prove this patch doesn't result in the bug[1] which made the
>>> fast TSC calibration using PIT failed in a Thinkpad x121e (AMD E-450
>>> APU).
>>>
>>> The true reason of the bug is enabling ACPI subsystem earlier than
>>> using PIT, not the SCI setup. invoking acpi_enable_subsystem() later
>>> could fix this bug as Julian tested and said[2].
>>>
>>> And, I found that Commit b064a8fa77df (" ACPI / init: Switch over
>>> platform to the ACPI mode later") split the ACPI early initialization
>>> code into acpi_early_init() and acpi_subsystem_init(). executing
>>> acpi_enable_subsystem() at the original early ACPI initialization spot.
>>>
>>> The sequence of them shows below:
>>>
>>> start_kernel
>>> +---------------+
>>> |
>>> +--> .......
>>> |
>>> | late_time_init()
>>> +--> +-------+
>>> |
>>> +--> .......
>>> |
>>> | acpi_early_init()
>>> +--> +-------+
>>> |
>>> +--> .......
>>> |
>>> | acpi_subsystem_init()
>>> +-> +--------+
>>>
>>> We make sure the acpi_subsystem_init() is called later than
>>> late_time_init(), the bug will be avoided.
>>>
>>> This patch changes the sequence of late_time_init() and
>>> acpi_early_init(), doesn't effect acpi_subsystem_init().
>>>
>>> So, this patch is OK.
>>>
>>> Btw, Thanks very much for Borislav Petkov, he will have access to
>>> Thinkpad x121e from Mid-August and will test this series.
>>>
>>
>> Almost one month passed, Borislav have tested this series in Thinkpad
>> x121e and I also have tested in my box and QEmu again. It is OK.
>>
>> BTW,
>> 1) I found your commit b064a8fa77df (" ACPI / init: Switch over
>> platform to the ACPI mode later") split the ACPI early initialization
>> code into acpi_early_init() and acpi_subsystem_init(). Actually enabling
>> the ACPI subsystem is in acpi_subsystem_init().
>>
>> 2) As we discussed earlier, invoking acpi_put_table() is not good for
>> this situation.
>>
>> So I do this patch, Is that goot to you? Any comments will be welcome.
>>
>> If it is OK, As the patches need to be re-based, and I also found
>> several spelling mistake, I will send a new version next week.
>
> OK, but does it depend on anything? Or does anything depend on it?
>
It depends on nothing and can be considered independent.
[11/13] patch in this series depends on it. [11/13] patch caused an
ACPI error, we used this patch to fix it.
> It is [12/13] in a series, so it looks like it doesn't depend on the
> previous patches in it, but the next one may depend on it? Which is the
> case?
>
The second case(the next one may depend on it) is what I want.
But, seems I made a mistake about the order of the patches. I should
put it before [11/13] to avoid the ACPI error.
I will adjust the order of the patches in the next version, and post
the whole series to you.
Thanks,
dou.
On Friday, August 25, 2017 4:06:11 AM CEST Dou Liyang wrote:
> Hi Rafael,
>
> At 08/25/2017 12:38 AM, Rafael J. Wysocki wrote:
> > On Thursday, August 24, 2017 5:54:28 AM CEST Dou Liyang wrote:
> >> Hi Rafael, Zheng,
> >>
> >> At 07/31/2017 06:50 PM, Dou Liyang wrote:
> >>> Hi,
> >>>
> >>> At 07/14/2017 01:52 PM, Dou Liyang wrote:
> >>>> Linux uses acpi_early_init() to put the ACPI table management into
> >>>> the late stage from the early stage where the mapped ACPI tables is
> >>>> temporary and should be unmapped.
> >>>>
> >>>> But, now initializing interrupt delivery mode should map and parse the
> >>>> DMAR table earlier in the early stage. This causes an ACPI error when
> >>>> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
> >>>> the DMAR table after using in the early stage.
> >>>>
> >>>> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
> >>>> be mapped and parsed in late stage like before.
> >>>>
> >>>> Reported-by: Xiaolong Ye <[email protected]>
> >>>> Signed-off-by: Dou Liyang <[email protected]>
> >>>> Cc: [email protected]
> >>>> Cc: Rafael J. Wysocki <[email protected]>
> >>>> Cc: Zheng, Lv <[email protected]>
> >>>> Cc: Julian Wollrath <[email protected]>
> >>>> ---
> >>>> Test in my own PC(Lenovo M4340).
> >>>> Ask help for doing regression testing for the bug said in commit
> >>>> c4e1acbb35e4
> >>>> ("ACPI / init: Invoke early ACPI initialization later").
> >>>>
> >>>
> >>> Now, I can prove this patch doesn't result in the bug[1] which made the
> >>> fast TSC calibration using PIT failed in a Thinkpad x121e (AMD E-450
> >>> APU).
> >>>
> >>> The true reason of the bug is enabling ACPI subsystem earlier than
> >>> using PIT, not the SCI setup. invoking acpi_enable_subsystem() later
> >>> could fix this bug as Julian tested and said[2].
> >>>
> >>> And, I found that Commit b064a8fa77df (" ACPI / init: Switch over
> >>> platform to the ACPI mode later") split the ACPI early initialization
> >>> code into acpi_early_init() and acpi_subsystem_init(). executing
> >>> acpi_enable_subsystem() at the original early ACPI initialization spot.
> >>>
> >>> The sequence of them shows below:
> >>>
> >>> start_kernel
> >>> +---------------+
> >>> |
> >>> +--> .......
> >>> |
> >>> | late_time_init()
> >>> +--> +-------+
> >>> |
> >>> +--> .......
> >>> |
> >>> | acpi_early_init()
> >>> +--> +-------+
> >>> |
> >>> +--> .......
> >>> |
> >>> | acpi_subsystem_init()
> >>> +-> +--------+
> >>>
> >>> We make sure the acpi_subsystem_init() is called later than
> >>> late_time_init(), the bug will be avoided.
> >>>
> >>> This patch changes the sequence of late_time_init() and
> >>> acpi_early_init(), doesn't effect acpi_subsystem_init().
> >>>
> >>> So, this patch is OK.
> >>>
> >>> Btw, Thanks very much for Borislav Petkov, he will have access to
> >>> Thinkpad x121e from Mid-August and will test this series.
> >>>
> >>
> >> Almost one month passed, Borislav have tested this series in Thinkpad
> >> x121e and I also have tested in my box and QEmu again. It is OK.
> >>
> >> BTW,
> >> 1) I found your commit b064a8fa77df (" ACPI / init: Switch over
> >> platform to the ACPI mode later") split the ACPI early initialization
> >> code into acpi_early_init() and acpi_subsystem_init(). Actually enabling
> >> the ACPI subsystem is in acpi_subsystem_init().
> >>
> >> 2) As we discussed earlier, invoking acpi_put_table() is not good for
> >> this situation.
> >>
> >> So I do this patch, Is that goot to you? Any comments will be welcome.
> >>
> >> If it is OK, As the patches need to be re-based, and I also found
> >> several spelling mistake, I will send a new version next week.
> >
> > OK, but does it depend on anything? Or does anything depend on it?
> >
>
> It depends on nothing and can be considered independent.
OK
Please send it as an independent patch, then.
> [11/13] patch in this series depends on it. [11/13] patch caused an
> ACPI error, we used this patch to fix it.
So the ordering of patches in the series should be different, then.
It should be ordered so as to avoid triggering the warning at all,
so this patch should go before the [11/13].
> > It is [12/13] in a series, so it looks like it doesn't depend on the
> > previous patches in it, but the next one may depend on it? Which is the
> > case?
> >
>
> The second case(the next one may depend on it) is what I want.
>
> But, seems I made a mistake about the order of the patches. I should
> put it before [11/13] to avoid the ACPI error.
Right.
> I will adjust the order of the patches in the next version, and post
> the whole series to you.
Please just CC it to linux-acpi.
Thanks,
Rafael
Hi Rafael,
At 08/25/2017 08:27 PM, Rafael J. Wysocki wrote:
> On Friday, August 25, 2017 4:06:11 AM CEST Dou Liyang wrote:
[...]
>>>>
>>>> BTW,
>>>> 1) I found your commit b064a8fa77df (" ACPI / init: Switch over
>>>> platform to the ACPI mode later") split the ACPI early initialization
>>>> code into acpi_early_init() and acpi_subsystem_init(). Actually enabling
>>>> the ACPI subsystem is in acpi_subsystem_init().
>>>>
>>>> 2) As we discussed earlier, invoking acpi_put_table() is not good for
>>>> this situation.
>>>>
>>>> So I do this patch, Is that goot to you? Any comments will be welcome.
>>>>
>>>> If it is OK, As the patches need to be re-based, and I also found
>>>> several spelling mistake, I will send a new version next week.
>>>
>>> OK, but does it depend on anything? Or does anything depend on it?
>>>
>>
>> It depends on nothing and can be considered independent.
>
> OK
>
> Please send it as an independent patch, then.
>
>> [11/13] patch in this series depends on it. [11/13] patch caused an
>> ACPI error, we used this patch to fix it.
>
> So the ordering of patches in the series should be different, then.
>
> It should be ordered so as to avoid triggering the warning at all,
> so this patch should go before the [11/13].
Yes, Indeed.
>
>>> It is [12/13] in a series, so it looks like it doesn't depend on the
>>> previous patches in it, but the next one may depend on it? Which is the
>>> case?
>>>
>>
>> The second case(the next one may depend on it) is what I want.
>>
>> But, seems I made a mistake about the order of the patches. I should
>> put it before [11/13] to avoid the ACPI error.
>
> Right.
>
>> I will adjust the order of the patches in the next version, and post
>> the whole series to you.
>
> Please just CC it to linux-acpi.
Got it. Will just CC it to linux-acpi, and CC the whole series to you.
Thanks,
dou.
>
> Thanks,
> Rafael
>
>
>
>