bisected: [PATCH V4] x86/apic: Always explicitly disarm TSC-deadline timer

PATCH In review Sergey Senozhatsky 3 participants 10 messages Nov 28, 2024 – Dec 3, 2024

Subsystems (3)

Stable branch x86 Kbuild

lore

Share & tools

lore

Sergey Senozhatsky

author

2024-11-28 11:18 UTC #0 lore raw

Hi folks,
sorry if messed something up, this email has never been in my inbox.

Quoted 4 lines

Disable the TSC Deadline timer in lapic_timer_shutdown() by writing to MSR_IA32_TSC_DEADLINE when in TSC-deadline mode. Also avoid writing to the initial-count register (APIC_TMICT) which is ignored in TSC-deadline mode.

So this commit hit stable and we now see section mismatch errors:

// stripped

WARNING: vmlinux.o(__ex_table+0x447c): Section mismatch in reference from the (unknown reference) (unknown) to the (unknown reference) .irqentry.text:(unknown) The relocation at __ex_table+0x447c references
section ".irqentry.text" which is not in the list of
authorized sections.

WARNING: vmlinux.o(__ex_table+0x4480): Section mismatch in reference from the (unknown reference) (unknown) to the (unknown reference) .irqentry.text:(unknown) The relocation at __ex_table+0x4480 references
section ".irqentry.text" which is not in the list of
authorized sections.

FATAL: modpost: Section mismatches detected.

Specifically because of wrmsrl.

I'm aware of the section mismatch errors on linux-5.4 (I know), not aware of any other stable versions (but I haven't checked). Is this something specific to linux-5.4?

Sergey Senozhatsky

author

2024-11-28 11:22 UTC #1 lore raw

Quoted 4 lines · Sergey Senozhatsky

Quoted 4 lines
Disable the TSC Deadline timer in lapic_timer_shutdown() by writing to MSR_IA32_TSC_DEADLINE when in TSC-deadline mode. Also avoid writing to the initial-count register (APIC_TMICT) which is ignored in TSC-deadline mode.

Upstream commit ffd95846c6ec6cf1f93da411ea10d504036cab42 (forgot to mention)

Thomas Gleixner

2024-11-30 11:21 UTC #2 lore raw

In reply to Sergey Senozhatsky

Quoted 19 lines · Sergey Senozhatsky

Quoted 4 lines
Disable the TSC Deadline timer in lapic_timer_shutdown() by writing to MSR_IA32_TSC_DEADLINE when in TSC-deadline mode. Also avoid writing to the initial-count register (APIC_TMICT) which is ignored in TSC-deadline mode.
So this commit hit stable and we now see section mismatch errors:
// stripped
WARNING: vmlinux.o(__ex_table+0x447c): Section mismatch in reference from the (unknown reference) (unknown) to the (unknown reference) .irqentry.text:(unknown) The relocation at __ex_table+0x447c references
section ".irqentry.text" which is not in the list of
authorized sections.
WARNING: vmlinux.o(__ex_table+0x4480): Section mismatch in reference from the (unknown reference) (unknown) to the (unknown reference) .irqentry.text:(unknown) The relocation at __ex_table+0x4480 references
section ".irqentry.text" which is not in the list of
authorized sections.

Quoted 7 lines

FATAL: modpost: Section mismatches detected.
Specifically because of wrmsrl.
I'm aware of the section mismatch errors on linux-5.4 (I know), not aware of any other stable versions (but I haven't checked). Is this something specific to linux-5.4?

So it seems the compiler inlines the inner guts of sysvec_apic_timer_interrupt() and local_apic_timer_interrupt().

Can you try the patch below?

Thanks,

tglx
---

arch/x86/kernel/apic/apic.c +1 -1

@@ -1007,7 +1007,7 @@ void setup_secondary_APIC_clock(void)
 /*
  * The guts of the apic timer interrupt
  */
-static void local_apic_timer_interrupt(void)
+static noinline void local_apic_timer_interrupt(void)
 {
 	struct clock_event_device *evt = this_cpu_ptr(&lapic_events);

Sergey Senozhatsky

author

2024-11-30 11:45 UTC #3 lore raw

Quoted 14 lines · Thomas Gleixner

Quoted 14 lines
WARNING: vmlinux.o(__ex_table+0x447c): Section mismatch in reference from the (unknown reference) (unknown) to the (unknown reference) .irqentry.text:(unknown) The relocation at __ex_table+0x447c references
section ".irqentry.text" which is not in the list of
authorized sections.
WARNING: vmlinux.o(__ex_table+0x4480): Section mismatch in reference from the (unknown reference) (unknown) to the (unknown reference) .irqentry.text:(unknown) The relocation at __ex_table+0x4480 references
section ".irqentry.text" which is not in the list of
authorized sections.
FATAL: modpost: Section mismatches detected.
Specifically because of wrmsrl.

Quoted 8 lines

Quoted 3 lines
I'm aware of the section mismatch errors on linux-5.4 (I know), not aware of any other stable versions (but I haven't checked). Is this something specific to linux-5.4?
So it seems the compiler inlines the inner guts of sysvec_apic_timer_interrupt() and local_apic_timer_interrupt().
Can you try the patch below?

That works, as far as I can tell, thank you!

Thomas Gleixner

2024-12-01 11:17 UTC #4 lore raw

PATCH 1/1 modpost: Add .irqentry.text to OTHER_SECTIONS

The compiler can fully inline the actual handler function of an interrupt entry into the .irqentry.text entry point. If such a function contains an access which has an exception table entry, modpost complains about a section mismatch:

WARNING: vmlinux.o(__ex_table+0x447c): Section mismatch in reference ...

The relocation at __ex_table+0x447c references section ".irqentry.text" which is not in the list of authorized sections.

Add .irqentry.text to OTHER_SECTIONS to cure the issue.

Reported-by Sergey Senozhatsky <[email protected]>

Signed-off-by Thomas Gleixner <[email protected]>

Cc [email protected]

---
 scripts/mod/modpost.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

scripts/mod/modpost.c +1 -1

@@ -785,7 +785,7 @@ static void check_section(const char *mo
 		".ltext", ".ltext.*"
 #define OTHER_TEXT_SECTIONS ".ref.text", ".head.text", ".spinlock.text", \
 		".fixup", ".entry.text", ".exception.text", \
-		".coldtext", ".softirqentry.text"
+		".coldtext", ".softirqentry.text", ".irqentry.text"
 
 #define ALL_TEXT_SECTIONS  ".init.text", ".exit.text", \
 		TEXT_SECTIONS, OTHER_TEXT_SECTIONS

Masahiro Yamada

2024-12-02 02:03 UTC #5 lore raw

Re: [PATCH] modpost: Add .irqentry.text to OTHER_SECTIONS

Quoted 6 lines · Thomas Gleixner

The compiler can fully inline the actual handler function of an interrupt entry into the .irqentry.text entry point. If such a function contains an access which has an exception table entry, modpost complains about a section mismatch:

Quoted 8 lines

WARNING: vmlinux.o(__ex_table+0x447c): Section mismatch in reference ...
The relocation at __ex_table+0x447c references section ".irqentry.text" which is not in the list of authorized sections.
Add .irqentry.text to OTHER_SECTIONS to cure the issue.
Reported-by Sergey Senozhatsky <[email protected]>

I found the context in LKML.

Closes https://lore.kernel.org/all/[email protected]/ open here

However, is this still relevant to the mainline kernel?

In Linux 5.4.y, I agree this because smp_apic_timer_interrupt() is annotated as __irq_entry:

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/arch/x86/kernel/apic/apic.c?id=v5.4.286#n1145

In this mainline kernel, DEFINE_IDTENTRY_SYSVEC() expands to a normal .text function which is explicitly annotated 'noinline'.

Quoted 9 lines

Signed-off-by Thomas Gleixner <[email protected]>
Cc [email protected]
---
 scripts/mod/modpost.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

scripts/mod/modpost.c static void check_section(const char *mo · line 791

Quoted 8 lines

scripts/mod/modpost.c +1 -1

@@ -785,7 +785,7 @@ static void check_section(const char *mo
                ".ltext", ".ltext.*"
 #define OTHER_TEXT_SECTIONS ".ref.text", ".head.text", ".spinlock.text", \
                ".fixup", ".entry.text", ".exception.text", \
-               ".coldtext", ".softirqentry.text"
+               ".coldtext", ".softirqentry.text", ".irqentry.text"
 
 #define ALL_TEXT_SECTIONS  ".init.text", ".exit.text", \
                TEXT_SECTIONS, OTHER_TEXT_SECTIONS

--
Best Regards
Masahiro Yamada

Thomas Gleixner

2024-12-02 21:03 UTC #6 lore raw

Re: [PATCH] modpost: Add .irqentry.text to OTHER_SECTIONS

Quoted 16 lines · Masahiro Yamada

Quoted 6 lines · Thomas Gleixner
The compiler can fully inline the actual handler function of an interrupt entry into the .irqentry.text entry point. If such a function contains an access which has an exception table entry, modpost complains about a section mismatch:
Quoted 8 lines
WARNING: vmlinux.o(__ex_table+0x447c): Section mismatch in reference ...
The relocation at __ex_table+0x447c references section ".irqentry.text" which is not in the list of authorized sections.
Add .irqentry.text to OTHER_SECTIONS to cure the issue.
Reported-by Sergey Senozhatsky <[email protected]>

Quoted 7 lines

I found the context in LKML.
Closes https://lore.kernel.org/all/[email protected]/ open here
However, is this still relevant to the mainline kernel?
In Linux 5.4.y, I agree this because smp_apic_timer_interrupt() is annotated as __irq_entry:

Correct.

Quoted 3 lines

In this mainline kernel, DEFINE_IDTENTRY_SYSVEC() expands to a normal .text function which is explicitly annotated 'noinline'.

It's not annotated noinline, it's annotated 'noinstr', which puts the code into the .noinstr.text section. That one is indeed covered.

So yes, the fix is only required for pre 5.8 kernels.

Thanks,

tglx

Masahiro Yamada

2024-12-03 15:28 UTC #7 lore raw

Re: [PATCH] modpost: Add .irqentry.text to OTHER_SECTIONS

Quoted 26 lines · Thomas Gleixner

Quoted 16 lines · Masahiro Yamada
Quoted 6 lines · Thomas Gleixner
The compiler can fully inline the actual handler function of an interrupt entry into the .irqentry.text entry point. If such a function contains an access which has an exception table entry, modpost complains about a section mismatch:
Quoted 8 lines
WARNING: vmlinux.o(__ex_table+0x447c): Section mismatch in reference ...
The relocation at __ex_table+0x447c references section ".irqentry.text" which is not in the list of authorized sections.
Add .irqentry.text to OTHER_SECTIONS to cure the issue.
Reported-by Sergey Senozhatsky <[email protected]>
Quoted 7 lines
I found the context in LKML.
Closes https://lore.kernel.org/all/[email protected]/ open here
However, is this still relevant to the mainline kernel?
In Linux 5.4.y, I agree this because smp_apic_timer_interrupt() is annotated as __irq_entry:

Quoted 8 lines

Correct.
Quoted 3 lines
In this mainline kernel, DEFINE_IDTENTRY_SYSVEC() expands to a normal .text function which is explicitly annotated 'noinline'.
It's not annotated noinline, it's annotated 'noinstr', which puts the code into the .noinstr.text section. That one is indeed covered.

The callsite of local_apic_timer_interrupt() is annotated 'noinline' if I correctly understand this line:
https://github.com/torvalds/linux/blob/v6.13-rc1/arch/x86/include/asm/idtentry.h#L272

It expands to:

static noinline void __sysvec_apic_timer_interrupt(struct pt_regs *regs)

{
       [snip]
       local_apic_timer_interrupt();
       [snip]
}

Quoted 1 line

So yes, the fix is only required for pre 5.8 kernels.

This never occurs on x86 after commit f0178fc01fe46, but theoretically this may occur for other architectures.

Now applied to linux-kbuild.
Thanks.

Quoted 4 lines

Thanks,
tglx

Signature · 2 lines

Best Regards
Masahiro Yamada

Thomas Gleixner

2024-12-03 16:10 UTC #8 lore raw

Re: [PATCH] modpost: Add .irqentry.text to OTHER_SECTIONS

Quoted 11 lines · Masahiro Yamada

Quoted 6 lines · Thomas Gleixner
Quoted 3 lines
In this mainline kernel, DEFINE_IDTENTRY_SYSVEC() expands to a normal .text function which is explicitly annotated 'noinline'.
It's not annotated noinline, it's annotated 'noinstr', which puts the code into the .noinstr.text section. That one is indeed covered.
The callsite of local_apic_timer_interrupt() is annotated 'noinline' if I correctly understand this line:
https://github.com/torvalds/linux/blob/v6.13-rc1/arch/x86/include/asm/idtentry.h#L272

You're right. I got lost in the macro maze and looked at the actual sysvec_...() part.

Quoted 4 lines

Quoted 1 line
So yes, the fix is only required for pre 5.8 kernels.
This never occurs on x86 after commit f0178fc01fe46, but theoretically this may occur for other architectures.

Correct.

Thanks,

tglx

Sergey Senozhatsky

author

2024-12-02 04:10 UTC #9 lore raw

Re: [PATCH] modpost: Add .irqentry.text to OTHER_SECTIONS

In reply to Thomas Gleixner

Quoted 11 lines · Thomas Gleixner

The compiler can fully inline the actual handler function of an interrupt entry into the .irqentry.text entry point. If such a function contains an access which has an exception table entry, modpost complains about a section mismatch:
WARNING: vmlinux.o(__ex_table+0x447c): Section mismatch in reference ...
The relocation at __ex_table+0x447c references section ".irqentry.text" which is not in the list of authorized sections.
Add .irqentry.text to OTHER_SECTIONS to cure the issue.

This works. In fact, this looks like a local fix which we applied here on our side, but we were very unsure. Thank you Thomas.