public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] x86/x2apic: Fix hang-up of defconfig kernel on resume from s2ram
@ 2026-02-02  9:51 Shashank Balaji
  2026-02-02  9:51 ` [PATCH 1/3] x86/x2apic: disable x2apic on resume if the kernel expects so Shashank Balaji
  0 siblings, 1 reply; 10+ messages in thread
From: Shashank Balaji @ 2026-02-02  9:51 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Suresh Siddha, K. Y. Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov,
	Broadcom internal kernel review list, Jan Kiszka, Paolo Bonzini,
	Vitaly Kuznetsov, Juergen Gross, Boris Ostrovsky
  Cc: Ingo Molnar, linux-kernel, linux-hyperv, virtualization,
	jailhouse-dev, kvm, xen-devel, Rahul Bukte, Shashank Balaji,
	Daniel Palmer, Tim Bird, stable

On resume from s2ram, a defconfig kernel gets into a state where the x2apic
hardware state and the kernel's perceived state are different.

On boot, x2apic is enabled by the firmware, and then the kernel does the
following (relevant lines from dmesg):

	[    0.000381] x2apic: enabled by BIOS, switching to x2apic ops
	[    0.009939] APIC: Switched APIC routing to: cluster x2apic
	[    0.095151] x2apic: IRQ remapping doesn't support X2APIC mode
	[    0.095154] x2apic disabled
	[    0.095551] APIC: Switched APIC routing to: physical flat

defconfig has CONFIG_IRQ_REMAP=n, which leads to x2apic being disabled,
because on bare metal, x2apic has an architectural dependence on interrupt
remapping.

While resuming from s2ram, x2apic is enabled again by the firmware, but
the kernel continues using the physical flat apic routing. This causes a
hang-up and no console output.

Patch 1 fixes this in lapic_resume by disabling x2apic when the kernel expects
it to be disabled.
Patch 2 enables CONFIG_IRQ_REMAP in defconfig so that defconfig kernels at
least don't disable x2apic because of a lack of IRQ_REMAP support.
Patch 3 is a non-functional change renaming x2apic_available to
x2apic_without_ir_available in struct x86_hyper_init, to better convey
the semantic.

Signed-off-by: Rahul Bukte <rahul.bukte@sony.com>
Signed-off-by: Shashank Balaji <shashank.mahadasyam@sony.com>
---
Shashank Balaji (3):
      x86/x2apic: disable x2apic on resume if the kernel expects so
      x86/defconfig: add CONFIG_IRQ_REMAP
      x86/virt: rename x2apic_available to x2apic_without_ir_available

 arch/x86/configs/x86_64_defconfig |  1 +
 arch/x86/include/asm/x86_init.h   |  4 ++--
 arch/x86/kernel/apic/apic.c       | 10 ++++++++--
 arch/x86/kernel/cpu/acrn.c        |  2 +-
 arch/x86/kernel/cpu/bhyve.c       |  2 +-
 arch/x86/kernel/cpu/mshyperv.c    |  2 +-
 arch/x86/kernel/cpu/vmware.c      |  2 +-
 arch/x86/kernel/jailhouse.c       |  2 +-
 arch/x86/kernel/kvm.c             |  2 +-
 arch/x86/kernel/x86_init.c        | 12 ++++++------
 arch/x86/xen/enlighten_hvm.c      |  4 ++--
 11 files changed, 25 insertions(+), 18 deletions(-)
---
base-commit: 18f7fcd5e69a04df57b563360b88be72471d6b62
change-id: 20260201-x2apic-fix-85c8c1b5cb90

Best regards,
-- 
Shashank Balaji <shashank.mahadasyam@sony.com>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] x86/x2apic: disable x2apic on resume if the kernel expects so
  2026-02-02  9:51 [PATCH 0/3] x86/x2apic: Fix hang-up of defconfig kernel on resume from s2ram Shashank Balaji
@ 2026-02-02  9:51 ` Shashank Balaji
  2026-02-03 21:08   ` Sohil Mehta
  0 siblings, 1 reply; 10+ messages in thread
From: Shashank Balaji @ 2026-02-02  9:51 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Suresh Siddha, K. Y. Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov,
	Broadcom internal kernel review list, Jan Kiszka, Paolo Bonzini,
	Vitaly Kuznetsov, Juergen Gross, Boris Ostrovsky
  Cc: Ingo Molnar, linux-kernel, linux-hyperv, virtualization,
	jailhouse-dev, kvm, xen-devel, Rahul Bukte, Shashank Balaji,
	Daniel Palmer, Tim Bird, stable

In lapic_resume, ensure x2apic is actually disabled when the kernel expects it
to be disabled, i.e. when x2apic_mode = 0.

x2apic_mode is set to 0 and x2apic is disabled on boot if the kernel doesn't
support irq remapping or for other reasons. On resume from s2ram
(/sys/power/mem_sleep = deep), firmware can re-enable x2apic, but the kernel
continues using the xapic interface because it didn't check to see if someone
enabled x2apic behind its back, which causes hangs. This situation happens on
defconfig + bare metal + s2ram, on which this fix has been tested.

Fixes: 6e1cb38a2aef ("x64, x2apic/intr-remap: add x2apic support, including enabling interrupt-remapping")
Cc: stable@vger.kernel.org
Co-developed-by: Rahul Bukte <rahul.bukte@sony.com>
Signed-off-by: Rahul Bukte <rahul.bukte@sony.com>
Signed-off-by: Shashank Balaji <shashank.mahadasyam@sony.com>
---
 arch/x86/kernel/apic/apic.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index d93f87f29d03..cc64d61f82cf 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2456,6 +2456,12 @@ static void lapic_resume(void *data)
 	if (x2apic_mode) {
 		__x2apic_enable();
 	} else {
+		/*
+		 * x2apic may have been re-enabled by the
+		 * firmware on resuming from s2ram
+		 */
+		__x2apic_disable();
+
 		/*
 		 * Make sure the APICBASE points to the right address
 		 *

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] x86/x2apic: disable x2apic on resume if the kernel expects so
  2026-02-02  9:51 ` [PATCH 1/3] x86/x2apic: disable x2apic on resume if the kernel expects so Shashank Balaji
@ 2026-02-03 21:08   ` Sohil Mehta
  2026-02-04  9:17     ` Shashank Balaji
  0 siblings, 1 reply; 10+ messages in thread
From: Sohil Mehta @ 2026-02-03 21:08 UTC (permalink / raw)
  To: Shashank Balaji, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Suresh Siddha, K. Y. Srinivasan,
	Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
	Alexey Makhalov, Broadcom internal kernel review list, Jan Kiszka,
	Paolo Bonzini, Vitaly Kuznetsov, Juergen Gross, Boris Ostrovsky
  Cc: Ingo Molnar, linux-kernel, linux-hyperv, virtualization,
	jailhouse-dev, kvm, xen-devel, Rahul Bukte, Daniel Palmer,
	Tim Bird, stable

> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index d93f87f29d03..cc64d61f82cf 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -2456,6 +2456,12 @@ static void lapic_resume(void *data)
>  	if (x2apic_mode) {
>  		__x2apic_enable();
>  	} else {
> +		/*
> +		 * x2apic may have been re-enabled by the
> +		 * firmware on resuming from s2ram
> +		 */
> +		__x2apic_disable();
> +

We should likely only disable x2apic on platforms that support it and
need the disabling. How about?

...
} else {
	/*
	 *
	 */
	if (x2apic_enabled())
		__x2apic_disable();

I considered if an error message should be printed along with this. But,
I am not sure if it can really be called a firmware issue. It's probably
just that newer CPUs might have started defaulting to x2apic on.

Can you specify what platform you are encountering this?



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] x86/x2apic: disable x2apic on resume if the kernel expects so
  2026-02-03 21:08   ` Sohil Mehta
@ 2026-02-04  9:17     ` Shashank Balaji
  2026-02-04 18:53       ` Sohil Mehta
  0 siblings, 1 reply; 10+ messages in thread
From: Shashank Balaji @ 2026-02-04  9:17 UTC (permalink / raw)
  To: Sohil Mehta
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Suresh Siddha, K. Y. Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov,
	Broadcom internal kernel review list, Jan Kiszka, Paolo Bonzini,
	Vitaly Kuznetsov, Juergen Gross, Boris Ostrovsky, Ingo Molnar,
	linux-kernel, linux-hyperv, virtualization, jailhouse-dev, kvm,
	xen-devel, Rahul Bukte, Daniel Palmer, Tim Bird, stable

Hi Sohil,

On Tue, Feb 03, 2026 at 01:08:39PM -0800, Sohil Mehta wrote:
> > diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> > index d93f87f29d03..cc64d61f82cf 100644
> > --- a/arch/x86/kernel/apic/apic.c
> > +++ b/arch/x86/kernel/apic/apic.c
> > @@ -2456,6 +2456,12 @@ static void lapic_resume(void *data)
> >  	if (x2apic_mode) {
> >  		__x2apic_enable();
> >  	} else {
> > +		/*
> > +		 * x2apic may have been re-enabled by the
> > +		 * firmware on resuming from s2ram
> > +		 */
> > +		__x2apic_disable();
> > +
> 
> We should likely only disable x2apic on platforms that support it and
> need the disabling. How about?
> 
> ...
> } else {
> 	/*
> 	 *
> 	 */
> 	if (x2apic_enabled())
> 		__x2apic_disable();

__x2apic_disable disables x2apic only if boot_cpu_has(X86_FEATURE_APIC)
and x2apic is already enabled. x2apic_enabled also does the same checks,
the only difference being, it uses rdmsrq_safe instead of just rdmsrq,
which is what __x2apic_disable uses. The safe version is because of
Boris' suggestion [1]. If that's applicable here as well, then rdmsrq in
__x2apic_disable should be changed to rdmsrq_safe.

> I considered if an error message should be printed along with this. But,
> I am not sure if it can really be called a firmware issue. It's probably
> just that newer CPUs might have started defaulting to x2apic on.
> 
> Can you specify what platform you are encountering this?


I'm not sure it's the CPU defaulting to x2apic on. As per Section
12.12.5.1 of the Intel SDM:

	On coming out of reset, the local APIC unit is enabled and is in
	the xAPIC mode: IA32_APIC_BASE[EN]=1 and IA32_APIC_BASE[EXTD]=0.

So, the CPU should be turning on in xapic mode. In fact, when x2apic is
disabled in the firmware, this problem doesn't happen.

Either way, a pr_warn maybe helpful. How about "x2apic re-enabled by the
firmware during resume. Disabling\n"?

[1] https://lore.kernel.org/all/20150116095927.GA18880@pd.tnic/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] x86/x2apic: disable x2apic on resume if the kernel expects so
  2026-02-04  9:17     ` Shashank Balaji
@ 2026-02-04 18:53       ` Sohil Mehta
  2026-02-05  6:07         ` Shashank Balaji
  0 siblings, 1 reply; 10+ messages in thread
From: Sohil Mehta @ 2026-02-04 18:53 UTC (permalink / raw)
  To: Shashank Balaji
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Suresh Siddha, K. Y. Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov,
	Broadcom internal kernel review list, Jan Kiszka, Paolo Bonzini,
	Vitaly Kuznetsov, Juergen Gross, Boris Ostrovsky, Ingo Molnar,
	linux-kernel, linux-hyperv, virtualization, jailhouse-dev, kvm,
	xen-devel, Rahul Bukte, Daniel Palmer, Tim Bird, stable

On 2/4/2026 1:17 AM, Shashank Balaji wrote:

> __x2apic_disable disables x2apic only if boot_cpu_has(X86_FEATURE_APIC)
> and x2apic is already enabled. 

I meant the X86_FEATURE_X2APIC and not X86_FEATURE_APIC. But, thinking
about it more, checking that the CPU is really in X2APIC mode by reading
the MSR is good enough.

> x2apic_enabled also does the same checks,
> the only difference being, it uses rdmsrq_safe instead of just rdmsrq,
> which is what __x2apic_disable uses. The safe version is because of
> Boris' suggestion [1]. If that's applicable here as well, then rdmsrq in
> __x2apic_disable should be changed to rdmsrq_safe.

I don't know if there is a strong justification for changing to
rdmsrq_safe() over here. Also, that would be beyond the scope of this
patch. In general, it's better to avoid such changes unless an actual
issue pops up.

> 
>> I considered if an error message should be printed along with this. But,
>> I am not sure if it can really be called a firmware issue. It's probably
>> just that newer CPUs might have started defaulting to x2apic on.
>>
>> Can you specify what platform you are encountering this?
> 
> 
> I'm not sure it's the CPU defaulting to x2apic on. As per Section
> 12.12.5.1 of the Intel SDM:
> 
> 	On coming out of reset, the local APIC unit is enabled and is in
> 	the xAPIC mode: IA32_APIC_BASE[EN]=1 and IA32_APIC_BASE[EXTD]=0.
> 
> So, the CPU should be turning on in xapic mode. In fact, when x2apic is
> disabled in the firmware, this problem doesn't happen.
> 

It's a bit odd then that the firmware chooses to enable x2apic without
the OS requesting it.

Linux maintains a concept of X2APIC_ON_LOCKED in x2apic_state which is
based on the hardware preference to keep the apic in X2APIC mode.

When you have x2apic enabled in firmware, but the system is in XAPIC
mode, can you read the values in MSR_IA32_ARCH_CAPABILITIES and
MSR_IA32_XAPIC_DISABLE_STATUS?

XAPIC shouldn't be disabled because you are running in that mode. But,
it would be good to confirm.


> Either way, a pr_warn maybe helpful. How about "x2apic re-enabled by the
> firmware during resume. Disabling\n"?

I mainly want to make sure the firmware is really at fault before we add
such a print. But it seems likely now that the firmware messed up.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] x86/x2apic: disable x2apic on resume if the kernel expects so
  2026-02-04 18:53       ` Sohil Mehta
@ 2026-02-05  6:07         ` Shashank Balaji
  2026-02-05 23:18           ` Sohil Mehta
  0 siblings, 1 reply; 10+ messages in thread
From: Shashank Balaji @ 2026-02-05  6:07 UTC (permalink / raw)
  To: Sohil Mehta
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Suresh Siddha, K. Y. Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov,
	Broadcom internal kernel review list, Jan Kiszka, Paolo Bonzini,
	Vitaly Kuznetsov, Juergen Gross, Boris Ostrovsky, Ingo Molnar,
	linux-kernel, linux-hyperv, virtualization, jailhouse-dev, kvm,
	xen-devel, Rahul Bukte, Daniel Palmer, Tim Bird, stable

On Wed, Feb 04, 2026 at 10:53:28AM -0800, Sohil Mehta wrote:
> On 2/4/2026 1:17 AM, Shashank Balaji wrote:
> 
> > __x2apic_disable disables x2apic only if boot_cpu_has(X86_FEATURE_APIC)
> > and x2apic is already enabled. 
> 
> I meant the X86_FEATURE_X2APIC and not X86_FEATURE_APIC.

My bad, I got that wrong. __x2apic_disable checks for X86_FEATURE_APIC,
while x2apic_enabled checks for X86_FEATURE_X2APIC.

> But, thinking about it more, checking that the CPU is really in X2APIC mode
> by reading the MSR is good enough.

But yes, I agree.

> > x2apic_enabled also does the same checks,
> > the only difference being, it uses rdmsrq_safe instead of just rdmsrq,
> > which is what __x2apic_disable uses. The safe version is because of
> > Boris' suggestion [1]. If that's applicable here as well, then rdmsrq in
> > __x2apic_disable should be changed to rdmsrq_safe.
> 
> I don't know if there is a strong justification for changing to
> rdmsrq_safe() over here. Also, that would be beyond the scope of this
> patch. In general, it's better to avoid such changes unless an actual
> issue pops up.

Makes sense.

> >> I considered if an error message should be printed along with this. But,
> >> I am not sure if it can really be called a firmware issue. It's probably
> >> just that newer CPUs might have started defaulting to x2apic on.
> >>
> >> Can you specify what platform you are encountering this?
> > 
> > 
> > I'm not sure it's the CPU defaulting to x2apic on. As per Section
> > 12.12.5.1 of the Intel SDM:
> > 
> > 	On coming out of reset, the local APIC unit is enabled and is in
> > 	the xAPIC mode: IA32_APIC_BASE[EN]=1 and IA32_APIC_BASE[EXTD]=0.
> > 
> > So, the CPU should be turning on in xapic mode. In fact, when x2apic is
> > disabled in the firmware, this problem doesn't happen.
> > 
> 
> It's a bit odd then that the firmware chooses to enable x2apic without
> the OS requesting it.

Well, the firmware has a setting saying "Enable x2apic", which was
enabled. So it did what the setting says

> Linux maintains a concept of X2APIC_ON_LOCKED in x2apic_state which is
> based on the hardware preference to keep the apic in X2APIC mode.
> 
> When you have x2apic enabled in firmware, but the system is in XAPIC
> mode, can you read the values in MSR_IA32_ARCH_CAPABILITIES and
> MSR_IA32_XAPIC_DISABLE_STATUS?
> 
> XAPIC shouldn't be disabled because you are running in that mode. But,
> it would be good to confirm.

With x2apic enabled by the firmware, and after kernel switches to xapic
(because no interrupt remapping support), bit 21 (XAPIC_DISABLE_STATUS)
of MSR_IA32_ARCH_CAPABILITIES is 0, and MSR_IA32_XAPIC_DISABLE_STATUS
MSR is not available.
 
> > Either way, a pr_warn maybe helpful. How about "x2apic re-enabled by the
> > firmware during resume. Disabling\n"?
> 
> I mainly want to make sure the firmware is really at fault before we add
> such a print. But it seems likely now that the firmware messed up.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] x86/x2apic: disable x2apic on resume if the kernel expects so
  2026-02-05  6:07         ` Shashank Balaji
@ 2026-02-05 23:18           ` Sohil Mehta
  2026-02-06  3:44             ` Shashank Balaji
  2026-02-06  8:57             ` Shashank Balaji
  0 siblings, 2 replies; 10+ messages in thread
From: Sohil Mehta @ 2026-02-05 23:18 UTC (permalink / raw)
  To: Shashank Balaji
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Suresh Siddha, K. Y. Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov,
	Broadcom internal kernel review list, Jan Kiszka, Paolo Bonzini,
	Vitaly Kuznetsov, Juergen Gross, Boris Ostrovsky, Ingo Molnar,
	linux-kernel, linux-hyperv, virtualization, jailhouse-dev, kvm,
	xen-devel, Rahul Bukte, Daniel Palmer, Tim Bird, stable

On 2/4/2026 10:07 PM, Shashank Balaji wrote:
> On Wed, Feb 04, 2026 at 10:53:28AM -0800, Sohil Mehta wrote:

>> It's a bit odd then that the firmware chooses to enable x2apic without
>> the OS requesting it.
> 
> Well, the firmware has a setting saying "Enable x2apic", which was
> enabled. So it did what the setting says
> 

The expectation would be that firmware would restore to the same state
before lapic_suspend().

>  
>>> Either way, a pr_warn maybe helpful. How about "x2apic re-enabled by the
>>> firmware during resume. Disabling\n"?
>>
>> I mainly want to make sure the firmware is really at fault before we add
>> such a print. But it seems likely now that the firmware messed up.

Maybe a warning would be useful to encourage firmware to fix this going
forward. I don't have a strong preference on the wording, but how about?

pr_warn_once("x2apic unexpectedly re-enabled by the firmware during
resume.\n");

A few nits:

For the code comments, you can use more of the line width. Generally, 72
(perhaps even 80) chars is okay for comments dependent on the code in
the vicinity.

The tip tree has slightly unique preferences, such as capitalizing the
first word of the patch title.

Please refer:
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#patch-submission-notes



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] x86/x2apic: disable x2apic on resume if the kernel expects so
  2026-02-05 23:18           ` Sohil Mehta
@ 2026-02-06  3:44             ` Shashank Balaji
  2026-02-06  8:57             ` Shashank Balaji
  1 sibling, 0 replies; 10+ messages in thread
From: Shashank Balaji @ 2026-02-06  3:44 UTC (permalink / raw)
  To: Sohil Mehta
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Suresh Siddha, K. Y. Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov,
	Broadcom internal kernel review list, Jan Kiszka, Paolo Bonzini,
	Vitaly Kuznetsov, Juergen Gross, Boris Ostrovsky, Ingo Molnar,
	linux-kernel, linux-hyperv, virtualization, jailhouse-dev, kvm,
	xen-devel, Rahul Bukte, Daniel Palmer, Tim Bird, stable

On Thu, Feb 05, 2026 at 03:18:58PM -0800, Sohil Mehta wrote:
> Maybe a warning would be useful to encourage firmware to fix this going
> forward. I don't have a strong preference on the wording, but how about?
> 
> pr_warn_once("x2apic unexpectedly re-enabled by the firmware during
> resume.\n");

That works

> A few nits:
> 
> For the code comments, you can use more of the line width. Generally, 72
> (perhaps even 80) chars is okay for comments dependent on the code in
> the vicinity.
> 
> The tip tree has slightly unique preferences, such as capitalizing the
> first word of the patch title.
> 
> Please refer:
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#patch-submission-notes

Thanks! I noticed that I also didn't use '()' for function names in the
commit message. I'll fix all these and add the pr_warn_once in v2.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] x86/x2apic: disable x2apic on resume if the kernel expects so
  2026-02-05 23:18           ` Sohil Mehta
  2026-02-06  3:44             ` Shashank Balaji
@ 2026-02-06  8:57             ` Shashank Balaji
  2026-02-07  0:37               ` Sohil Mehta
  1 sibling, 1 reply; 10+ messages in thread
From: Shashank Balaji @ 2026-02-06  8:57 UTC (permalink / raw)
  To: Sohil Mehta
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Suresh Siddha, K. Y. Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov,
	Broadcom internal kernel review list, Jan Kiszka, Paolo Bonzini,
	Vitaly Kuznetsov, Juergen Gross, Boris Ostrovsky, Ingo Molnar,
	linux-kernel, linux-hyperv, virtualization, jailhouse-dev, kvm,
	xen-devel, Rahul Bukte, Daniel Palmer, Tim Bird, stable

On Thu, Feb 05, 2026 at 03:18:58PM -0800, Sohil Mehta wrote:
> On 2/4/2026 10:07 PM, Shashank Balaji wrote:
> > On Wed, Feb 04, 2026 at 10:53:28AM -0800, Sohil Mehta wrote:
> 
> >> It's a bit odd then that the firmware chooses to enable x2apic without
> >> the OS requesting it.
> > 
> > Well, the firmware has a setting saying "Enable x2apic", which was
> > enabled. So it did what the setting says
> > 
> 
> The expectation would be that firmware would restore to the same state
> before lapic_suspend().

I'm a bit out of my depth here, but I went looking around, and this is from the
latest ACPI spec (v6.6) [1]:

	When executing from the power-on reset vector as a result of waking
	from an S2 or S3 sleep state, the platform firmware performs only the
	hardware initialization required to restore the system to either the
	state the platform was in prior to the initial operating system boot,
	or to the pre-sleep configuration state. In multiprocessor systems,
	non-boot processors should be placed in the same state as prior to the
	initial operating system boot.

	(further ahead)

	 If this is an S2 or S3 wake, then the platform runtime firmware
	 restores minimum context of the system before jumping to the waking
	 vector. This includes:

	 	CPU configuration. Platform runtime firmware restores the
		pre-sleep configuration or initial boot configuration of each
		CPU (MSR, MTRR, firmware update, SMBase, and so on). Interrupts
		must be disabled (for IA-32 processors, disabled by CLI
		instruction).

		(and other things)

I suppose, in my case, the firmware is restoring initial boot
configuration on S3 resume. And initial boot configuration of x2apic is
set from the firmware's UI "Enable x2apic".

> Maybe a warning would be useful to encourage firmware to fix this going
> forward. I don't have a strong preference on the wording, but how about?
> 
> pr_warn_once("x2apic unexpectedly re-enabled by the firmware during
> resume.\n");

At least as per the spec, it's not something the firmware needs to fix,
and it's not unexpected re-enablement.

Am I missing something?

But it _is_ surprising that this bug went unnoticed for so long :)

[1] https://uefi.org/specs/ACPI/6.6/16_Waking_and_Sleeping.html#initialization

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] x86/x2apic: disable x2apic on resume if the kernel expects so
  2026-02-06  8:57             ` Shashank Balaji
@ 2026-02-07  0:37               ` Sohil Mehta
  0 siblings, 0 replies; 10+ messages in thread
From: Sohil Mehta @ 2026-02-07  0:37 UTC (permalink / raw)
  To: Shashank Balaji
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Suresh Siddha, K. Y. Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov,
	Broadcom internal kernel review list, Jan Kiszka, Paolo Bonzini,
	Vitaly Kuznetsov, Juergen Gross, Boris Ostrovsky, Ingo Molnar,
	linux-kernel, linux-hyperv, virtualization, jailhouse-dev, kvm,
	xen-devel, Rahul Bukte, Daniel Palmer, Tim Bird, stable

On 2/6/2026 12:57 AM, Shashank Balaji wrote:
> On Thu, Feb 05, 2026 at 03:18:58PM -0800, Sohil Mehta wrote:
>> On 2/4/2026 10:07 PM, Shashank Balaji wrote:
>>> On Wed, Feb 04, 2026 at 10:53:28AM -0800, Sohil Mehta wrote:
>>
>>>> It's a bit odd then that the firmware chooses to enable x2apic without
>>>> the OS requesting it.
>>>
>>> Well, the firmware has a setting saying "Enable x2apic", which was
>>> enabled. So it did what the setting says
>>>
>>

I still think the firmware behavior is flawed. Some confusion
originates from the option "Enable x2apic". Based on just these two
words, the behavior seems highly ambiguous. I interpret it to mean "Make
the x2apic feature available to the kernel". Then it is up to the kernel
whether it decides to use it.

If the intention of the BIOS option is to automatically/always enable
x2apic then there is an architectural way to do it. It can set the bits
that track to x2apic_hw_locked(). But, doing it this way during resume
(behind OS's back) is unexpected.

>>
>> pr_warn_once("x2apic unexpectedly re-enabled by the firmware during
>> resume.\n");
> 
> At least as per the spec, it's not something the firmware needs to fix,
> and it's not unexpected re-enablement.
> 
> Am I missing something?
> 
> But it _is_ surprising that this bug went unnoticed for so long :)


Maybe this BIOS option isn't typically found in a production firmware. I
think it would be preferable to have the pr_warn_once() but I won't push
super hard for it due to the unclear information.

Whatever you choose for v2, can you please briefly describe the
ambiguity in the commit message? It would help other reviewers provide
better insight and be handy if we ever need to come back to this.




^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-02-07  0:37 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-02  9:51 [PATCH 0/3] x86/x2apic: Fix hang-up of defconfig kernel on resume from s2ram Shashank Balaji
2026-02-02  9:51 ` [PATCH 1/3] x86/x2apic: disable x2apic on resume if the kernel expects so Shashank Balaji
2026-02-03 21:08   ` Sohil Mehta
2026-02-04  9:17     ` Shashank Balaji
2026-02-04 18:53       ` Sohil Mehta
2026-02-05  6:07         ` Shashank Balaji
2026-02-05 23:18           ` Sohil Mehta
2026-02-06  3:44             ` Shashank Balaji
2026-02-06  8:57             ` Shashank Balaji
2026-02-07  0:37               ` Sohil Mehta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox