Re: [PATCH v5] x86/apic: Use EILVT register count from APIC_EFEAT

[Naveen N Rao]

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@xxxxxxxxxx/

  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@xxxxxxxxxx/

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

> 
> 
> > Signed-off-by: Naveen N Rao (AMD) <naveen@xxxxxxxxxx>
> > Tested-by: Manali Shukla <manali.shukla@xxxxxxx>
> > Tested-by: Bharata B Rao <bharata@xxxxxxx>
> 
> 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