Re: [PATCH v4 1/3] perf/amd/ibs: Limit the max EILVT register count for AMD family 0x10

[Naveen N Rao]

Hi Ingo,

On Thu, Jul 23, 2026 at 10:02:51AM +0200, Ingo Molnar wrote:
> 
> * Naveen N Rao (AMD) <naveen@xxxxxxxxxx> wrote:
> 
> > For AMD family 0x10, EILVT offsets are not assigned by BIOS and is
> > instead assigned by picking the next available EILVT offset.  Use the
> > maximum EILVT count for family 0x10 (APIC_EILVT_NR_AMD_10H) rather than
> > an arbitrary maximum EILVT count when looking for the next available
> > EILVT offset.
> > 
> > Signed-off-by: Naveen N Rao (AMD) <naveen@xxxxxxxxxx>
> > Tested-by: Manali Shukla <manali.shukla@xxxxxxx>
> > ---
> >  arch/x86/events/amd/ibs.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > 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++) {
> 
> Nit:
> 
>   s/APIC_EILVT_NR_AMD_10H
>    /APIC_EILVT_NR_MAX_AMD_10H
> 
> to make it clear at first sight that this is a (family specific)
> maximum, not some default count or other random value.

While trying to make this change, I realized _NR_MAX_ may not be 
accurate either. 

APIC_EILVT_NR_AMD_10H is actually representing the number of EILVT 
registers on AMD processor family 0x10 as a fixed entity (not a family 
specific maximum which would imply that the count can be less). This 
particular function (force_ibs_eilvt_setup()) is specific to AMD 0x10 
family. So, the name does seem reasonable to me in hindsight.

I believe its use in the for loop above, and going from a macro that 
represents a generic maximum count to a family-specific count feels less 
intuitive.

I'm leaning towards retaining the current name or renaming it to 
APIC_EILVT_COUNT_AMD_10H to represent the EILVT register count.

Thoughts?


Thanks,
Naveen