mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v5] x86/apic: Use EILVT register count from APIC_EFEAT
@ 2026-09-25 16:17 Naveen N Rao (AMD)
  2026-09-26  0:38 ` Borislav Petkov
  0 siblings, 1 reply; 4+ messages in thread
From: Naveen N Rao (AMD) @ 2026-09-25 16:17 UTC (permalink / raw)
  To: Borislav Petkov, Dave Hansen
  Cc: linux-kernel, x86, Thomas Gleixner, Ingo Molnar, Bharata B Rao,
	Manali Shukla, Nikunj A Dadhania, H. Peter Anvin, Robert Richter,
	Christian Ludloff

Future AMD processors will be increasing the number of EILVT registers.
Rather than hardcoding the maximum EILVT register count and using that
everywhere, introduce a variable in 'struct apic' to track the EILVT
register count.

The number of EILVT registers is exposed through the extended APIC
Feature Register (APIC_EFEAT) bits 23:16 on platforms that support the
AMD Extended APIC Register space (X86_FEATURE_EXTAPIC).  Use this to
initialize the count and fall back to the current default from AMD
family 0x10 (APIC_EILVT_NR_AMD_10H, which is 4) otherwise. Since this
value is no longer a compile-time constant, update eilvt_offsets to be
dynamically allocated.

Drop the now-redundant APIC_EILVT_NR_MAX macro. Other than during EILVT
register offset allocation (which now uses apic->eilvt_regs_count), that
macro was being used in the IBS driver for determining the EILVT offset
for AMD family 0x10 since the EILVT offsets were not assigned by the
BIOS. Switch that to use APIC_EILVT_NR_AMD_10H, which reflects the
correct EILVT register count for that family.

Note: because the EILVT register count is now derived from APIC_EFEAT,
it is possible that the register count is less than 4 (1 or 0 even) on
some AMD K8 parts (rather than the previous default of 4), which should
more accurately reflect the correct EILVT register count on those parts.


Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
Tested-by: Manali Shukla <manali.shukla@amd.com>
Tested-by: Bharata B Rao <bharata@amd.com>
---
Changes since v4 (*):
- Squash into a single patch (Boris)
- Use the value from APIC_EFEAT rather than forcing the previous default 
  if the EILVT register count in APCI_EFEAT is zero (Boris)
- Pick up Bharata's Tested-by, and retain Manali's tag since the change 
  is minimal


- Naveen

(*) http://lore.kernel.org/r/cover.1788425679.git.naveen@kernel.org


 arch/x86/include/asm/apic.h    |  2 ++
 arch/x86/include/asm/apicdef.h |  2 +-
 arch/x86/events/amd/ibs.c      |  4 ++--
 arch/x86/kernel/apic/apic.c    | 16 ++++++++++++++--
 4 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 9cd493d467d4..578cc28b3134 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -317,6 +317,8 @@ struct apic {
 
 	void	(*update_vector)(unsigned int cpu, unsigned int vector, bool set);
 
+	u32	eilvt_regs_count;
+
 	char	*name;
 };
 
diff --git a/arch/x86/include/asm/apicdef.h b/arch/x86/include/asm/apicdef.h
index bc125c4429dc..32a242ae0455 100644
--- a/arch/x86/include/asm/apicdef.h
+++ b/arch/x86/include/asm/apicdef.h
@@ -134,12 +134,12 @@
 #define		APIC_TDR_DIV_64		0x9
 #define		APIC_TDR_DIV_128	0xA
 #define	APIC_EFEAT	0x400
+#define		APIC_EFEAT_XLC(x)	(((x) >> 16) & 0xff)
 #define	APIC_ECTRL	0x410
 #define APIC_SEOI	0x420
 #define APIC_IER	0x480
 #define APIC_EILVTn(n)	(0x500 + 0x10 * n)
 #define		APIC_EILVT_NR_AMD_10H	4
-#define		APIC_EILVT_NR_MAX	APIC_EILVT_NR_AMD_10H
 
 #define APIC_BASE (fix_to_virt(FIX_APIC_BASE))
 #define APIC_BASE_MSR		0x800
diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
index 3531f9c23b8c..555912ac520f 100644
--- a/arch/x86/events/amd/ibs.c
+++ b/arch/x86/events/amd/ibs.c
@@ -1839,13 +1839,13 @@ static void force_ibs_eilvt_setup(void)
 
 	preempt_disable();
 	/* find the next free available EILVT entry, skip offset 0 */
-	for (offset = 1; offset < APIC_EILVT_NR_MAX; offset++) {
+	for (offset = 1; offset < APIC_EILVT_NR_AMD_10H; offset++) {
 		if (get_eilvt(offset))
 			break;
 	}
 	preempt_enable();
 
-	if (offset == APIC_EILVT_NR_MAX) {
+	if (offset == APIC_EILVT_NR_AMD_10H) {
 		pr_debug("No EILVT entry available\n");
 		return;
 	}
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 90025451ace2..434e118b71c8 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -341,7 +341,7 @@ static void __setup_APIC_LVTT(unsigned int clocks, int oneshot, int irqen)
  * necessarily a BIOS bug.
  */
 
-static atomic_t eilvt_offsets[APIC_EILVT_NR_MAX];
+static atomic_t *eilvt_offsets;
 
 static inline int eilvt_entry_is_changeable(unsigned int old, unsigned int new)
 {
@@ -354,7 +354,7 @@ static unsigned int reserve_eilvt_offset(int offset, unsigned int new)
 {
 	unsigned int rsvd, vector;
 
-	if (offset >= APIC_EILVT_NR_MAX)
+	if (!eilvt_offsets || offset >= apic->eilvt_regs_count)
 		return ~0;
 
 	rsvd = atomic_read(&eilvt_offsets[offset]);
@@ -410,6 +410,17 @@ int setup_APIC_eilvt(u8 offset, u8 vector, u8 msg_type, u8 mask)
 }
 EXPORT_SYMBOL_GPL(setup_APIC_eilvt);
 
+static __init void init_eilvt(void)
+{
+	if (cpu_feature_enabled(X86_FEATURE_EXTAPIC))
+		apic->eilvt_regs_count = APIC_EFEAT_XLC(apic_read(APIC_EFEAT));
+	else if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
+		apic->eilvt_regs_count = APIC_EILVT_NR_AMD_10H;
+
+	if (apic->eilvt_regs_count)
+		eilvt_offsets = kzalloc_objs(atomic_t, apic->eilvt_regs_count);
+}
+
 /*
  * Program the next event, relative to now
  */
@@ -2345,6 +2356,7 @@ static void __init apic_bsp_setup(bool upmode)
 	if (upmode)
 		apic_bsp_up_setup();
 	setup_local_APIC();
+	init_eilvt();
 
 	enable_IO_APIC();
 	end_local_APIC_setup();

base-commit: 630761837841036e97af4af09a77fda2e6a28347
-- 
2.55.0


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

* Re: [PATCH v5] x86/apic: Use EILVT register count from APIC_EFEAT
  2026-09-25 16:17 [PATCH v5] x86/apic: Use EILVT register count from APIC_EFEAT Naveen N Rao (AMD)
@ 2026-09-26  0:38 ` Borislav Petkov
  2026-09-28  6:50   ` Naveen N Rao
  0 siblings, 1 reply; 4+ messages in thread
From: Borislav Petkov @ 2026-09-26  0:38 UTC (permalink / raw)
  To: Naveen N Rao (AMD)
  Cc: Dave Hansen, linux-kernel, x86, Thomas Gleixner, Ingo Molnar,
	Bharata B Rao, Manali Shukla, Nikunj A Dadhania, H. Peter Anvin,
	Robert Richter, Christian Ludloff

On Fri, Sep 25, 2026 at 09:47:06PM +0530, Naveen N Rao (AMD) wrote:
> Future AMD processors will be increasing the number of EILVT registers.
> Rather than hardcoding the maximum EILVT register count and using that
> everywhere, introduce a variable in 'struct apic' to track the EILVT
> register count.
> 
> The number of EILVT registers is exposed through the extended APIC
> Feature Register (APIC_EFEAT) bits 23:16 on platforms that support the
> AMD Extended APIC Register space (X86_FEATURE_EXTAPIC).  Use this to
> initialize the count and fall back to the current default from AMD
> family 0x10 (APIC_EILVT_NR_AMD_10H, which is 4) otherwise. Since this
> value is no longer a compile-time constant, update eilvt_offsets to be
> dynamically allocated.
> 
> Drop the now-redundant APIC_EILVT_NR_MAX macro. Other than during EILVT
> register offset allocation (which now uses apic->eilvt_regs_count), that
> macro was being used in the IBS driver for determining the EILVT offset
> for AMD family 0x10 since the EILVT offsets were not assigned by the
> BIOS. Switch that to use APIC_EILVT_NR_AMD_10H, which reflects the
> correct EILVT register count for that family.
> 
> Note: because the EILVT register count is now derived from APIC_EFEAT,
> it is possible that the register count is less than 4 (1 or 0 even) on
> some AMD K8 parts (rather than the previous default of 4), which should
> more accurately reflect the correct EILVT register count on those parts.

Please, do not talk about *what* the patch is doing in the commit message
- that should be obvious from the diff itself. Rather, concentrate on the
*why* it needs to be done and why your patch exists.

It is perfectly fine to explain non-trivial aspects of the code the patch is
touching but do not regurgitate what it does.

See also https://docs.kernel.org/process/submitting-patches.html for
additional inspiration.

Also, that second note about clamping it:

https://sashiko.dev/#/patchset/20260925161706.1619042-1-naveen%40kernel.org

does sound relevant.

The first one, OTOH, is a very good example of a confused LLM:

"The APIC ExtLvtCnt (XLC) field represents the maximum EILVT index..."

Apparently, it couldn't download the APM.

:-P


> Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
> Tested-by: Manali Shukla <manali.shukla@amd.com>
> Tested-by: Bharata B Rao <bharata@amd.com>

Are you sure they tested your new version so quickly?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH v5] x86/apic: Use EILVT register count from APIC_EFEAT
  2026-09-26  0:38 ` Borislav Petkov
@ 2026-09-28  6:50   ` Naveen N Rao
  2026-09-29  3:27     ` Borislav Petkov
  0 siblings, 1 reply; 4+ messages in thread
From: Naveen N Rao @ 2026-09-28  6:50 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Dave Hansen, linux-kernel, x86, Thomas Gleixner, Ingo Molnar,
	Bharata B Rao, Manali Shukla, Nikunj A Dadhania, H. Peter Anvin,
	Robert Richter, Christian Ludloff

On Fri, Sep 25, 2026 at 05:38:33PM -0700, Borislav Petkov wrote:
> On Fri, Sep 25, 2026 at 09:47:06PM +0530, Naveen N Rao (AMD) wrote:
> > Future AMD processors will be increasing the number of EILVT registers.
> > Rather than hardcoding the maximum EILVT register count and using that
> > everywhere, introduce a variable in 'struct apic' to track the EILVT
> > register count.
> > 
> > The number of EILVT registers is exposed through the extended APIC
> > Feature Register (APIC_EFEAT) bits 23:16 on platforms that support the
> > AMD Extended APIC Register space (X86_FEATURE_EXTAPIC).  Use this to
> > initialize the count and fall back to the current default from AMD
> > family 0x10 (APIC_EILVT_NR_AMD_10H, which is 4) otherwise. Since this
> > value is no longer a compile-time constant, update eilvt_offsets to be
> > dynamically allocated.
> > 
> > Drop the now-redundant APIC_EILVT_NR_MAX macro. Other than during EILVT
> > register offset allocation (which now uses apic->eilvt_regs_count), that
> > macro was being used in the IBS driver for determining the EILVT offset
> > for AMD family 0x10 since the EILVT offsets were not assigned by the
> > BIOS. Switch that to use APIC_EILVT_NR_AMD_10H, which reflects the
> > correct EILVT register count for that family.
> > 
> > Note: because the EILVT register count is now derived from APIC_EFEAT,
> > it is possible that the register count is less than 4 (1 or 0 even) on
> > some AMD K8 parts (rather than the previous default of 4), which should
> > more accurately reflect the correct EILVT register count on those parts.
> 
> Please, do not talk about *what* the patch is doing in the commit message
> - that should be obvious from the diff itself. Rather, concentrate on the
> *why* it needs to be done and why your patch exists.
> 
> It is perfectly fine to explain non-trivial aspects of the code the patch is
> touching but do not regurgitate what it does.

I thought I have explained the why. What am I missing?

Perhaps you are saying paragraph 2 is not needed? Though I fail to see 
how it can hurt (it does clarify why the fallback is the value that it 
is).

> 
> Also, that second note about clamping it:
> 
> https://sashiko.dev/#/patchset/20260925161706.1619042-1-naveen%40kernel.org
> 
> does sound relevant.

I had addressed this in the cover note on the previous version and 
didn't repeat it here:
https://lore.kernel.org/all/cover.1788425679.git.naveen@kernel.org/

  3. Need to clamp the maximum EILVT register count to prevent incorrect 
     MMIO accesses: this is not an issue since all offsets being 
     programmed are appropriately clamped at the source.

> 
> The first one, OTOH, is a very good example of a confused LLM:
> 
> "The APIC ExtLvtCnt (XLC) field represents the maximum EILVT index..."
> 
> Apparently, it couldn't download the APM.
> 
> :-P

Oh yeah, and no amount of me calling this out explicitly in the commit 
log seemed to help. See patch 2 here, where I added that it is the 
actual count:
https://lore.kernel.org/all/39d39c3b91d174f4090fb954c6690edae9ed8295.1784619898.git.naveen@kernel.org/

Sashiko review of that:
https://sashiko.dev/#/patchset/cover.1784619898.git.naveen@kernel.org

> 
> 
> > Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
> > Tested-by: Manali Shukla <manali.shukla@amd.com>
> > Tested-by: Bharata B Rao <bharata@amd.com>
> 
> Are you sure they tested your new version so quickly?

No, and I never claimed they did. If anything, I have called this out 
explicitly as part of the changelog:
  - Pick up Bharata's Tested-by, and retain Manali's tag since the change
    is minimal

Retaining their tags was a deliberate decision on my part knowing the 
tests they are doing and that they would not be exercizing the fallback 
path, which this version changes.

I'm absolutely ok to drop that, but no, I didn't claim they tested this 
version.


- Naveen


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

* Re: [PATCH v5] x86/apic: Use EILVT register count from APIC_EFEAT
  2026-09-28  6:50   ` Naveen N Rao
@ 2026-09-29  3:27     ` Borislav Petkov
  0 siblings, 0 replies; 4+ messages in thread
From: Borislav Petkov @ 2026-09-29  3:27 UTC (permalink / raw)
  To: Naveen N Rao
  Cc: Dave Hansen, linux-kernel, x86, Thomas Gleixner, Ingo Molnar,
	Bharata B Rao, Manali Shukla, Nikunj A Dadhania, H. Peter Anvin,
	Robert Richter, Christian Ludloff

On Mon, Sep 28, 2026 at 12:20:04PM +0530, Naveen N Rao wrote:
> On Fri, Sep 25, 2026 at 05:38:33PM -0700, Borislav Petkov wrote:
> > On Fri, Sep 25, 2026 at 09:47:06PM +0530, Naveen N Rao (AMD) wrote:
> > > Future AMD processors will be increasing the number of EILVT registers.
> > > Rather than hardcoding the maximum EILVT register count and using that
> > > everywhere, introduce a variable in 'struct apic' to track the EILVT
> > > register count.
> > > 
> > > The number of EILVT registers is exposed through the extended APIC
> > > Feature Register (APIC_EFEAT) bits 23:16 on platforms that support the
> > > AMD Extended APIC Register space (X86_FEATURE_EXTAPIC).  Use this to
> > > initialize the count and fall back to the current default from AMD
> > > family 0x10 (APIC_EILVT_NR_AMD_10H, which is 4) otherwise. Since this
> > > value is no longer a compile-time constant, update eilvt_offsets to be
> > > dynamically allocated.
> > > 
> > > Drop the now-redundant APIC_EILVT_NR_MAX macro. Other than during EILVT
> > > register offset allocation (which now uses apic->eilvt_regs_count), that
> > > macro was being used in the IBS driver for determining the EILVT offset
> > > for AMD family 0x10 since the EILVT offsets were not assigned by the
> > > BIOS. Switch that to use APIC_EILVT_NR_AMD_10H, which reflects the
> > > correct EILVT register count for that family.
> > > 
> > > Note: because the EILVT register count is now derived from APIC_EFEAT,
> > > it is possible that the register count is less than 4 (1 or 0 even) on
> > > some AMD K8 parts (rather than the previous default of 4), which should
> > > more accurately reflect the correct EILVT register count on those parts.
> > 
> > Please, do not talk about *what* the patch is doing in the commit message
> > - that should be obvious from the diff itself. Rather, concentrate on the
> > *why* it needs to be done and why your patch exists.
> > 
> > It is perfectly fine to explain non-trivial aspects of the code the patch is
> > touching but do not regurgitate what it does.
> 
> I thought I have explained the why. What am I missing?
> 
> Perhaps you are saying paragraph 2 is not needed? Though I fail to see 
> how it can hurt (it does clarify why the fallback is the value that it 
> is).

I went and rewrote your commit message without the code explanations:

"Future AMD processors will increase the number of EILVT registers. Track the
EILVT register max count in a variable. Use the current default of 4 EILVT max
count from F10h times as the fallback.

Use the APIC_EILVT_NR_AMD_10H macro only in force_ibs_eilvt_setup() which is
F10 specific anyway."

That's it. Everything else can be read out from the patch itself and you don't
need to spell it again in the commit message.

> > Also, that second note about clamping it:
> > 
> > https://sashiko.dev/#/patchset/20260925161706.1619042-1-naveen%40kernel.org
> > 
> > does sound relevant.
> 
> I had addressed this in the cover note on the previous version and 
> didn't repeat it here:
> https://lore.kernel.org/all/cover.1788425679.git.naveen@kernel.org/
> 
>   3. Need to clamp the maximum EILVT register count to prevent incorrect 
>      MMIO accesses: this is not an issue since all offsets being 
>      programmed are appropriately clamped at the source.

"Table 16-2. APIC Registers

...

500-570h	Extended Interrupt [7:0] Local Vector Table Registers	00000000h"

I'm reading this as, EILVT max cannot be more than 8 regs. Right?

So doing a  if > 8 at read time would be a good sanity-check, no?

> Oh yeah, and no amount of me calling this out explicitly in the commit 
> log seemed to help. See patch 2 here, where I added that it is the 
> actual count:
> https://lore.kernel.org/all/39d39c3b91d174f4090fb954c6690edae9ed8295.1784619898.git.naveen@kernel.org/
> 
> Sashiko review of that:
> https://sashiko.dev/#/patchset/cover.1784619898.git.naveen@kernel.org

Recent experience tells me that I cannot trust AI one bit.
 
> No, and I never claimed they did. If anything, I have called this out 
> explicitly as part of the changelog:
>   - Pick up Bharata's Tested-by, and retain Manali's tag since the change
>     is minimal
> 
> Retaining their tags was a deliberate decision on my part knowing the 
> tests they are doing and that they would not be exercizing the fallback 
> path, which this version changes.
> 
> I'm absolutely ok to drop that, but no, I didn't claim they tested this 
> version.

Just drop the Tested-by tags. They haven't tested the patch and that's it.
It's not like you fixed comments or something else immaterial to code.
A Tested-by should mean what it says, not the person tested some old version
of the patch.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

end of thread, other threads:[~2026-09-29  3:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-25 16:17 [PATCH v5] x86/apic: Use EILVT register count from APIC_EFEAT Naveen N Rao (AMD)
2026-09-26  0:38 ` Borislav Petkov
2026-09-28  6:50   ` Naveen N Rao
2026-09-29  3:27     ` Borislav Petkov

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

all inboxes | Powered by JetHome®