Re: [PATCH v3 08/10] perf/x86/rapl: Modify the generic variable names to *_pkg*

["Zhang, Rui" <rui.zhang@intel.com>]

Hi, Peter,

On Fri, 2024-07-05 at 11:24 +0200, Peter Zijlstra wrote:
> On Fri, Jul 05, 2024 at 02:18:30AM +0000, Zhang, Rui wrote:
> 
> > > > > I have a doubt about this, won't the future Intel multi-die
> > > > > systems 
> > > > > have die-scope for the RAPL PMU like Casecadelake-AP?
> > > > 
> > > > For future multi-die systems that I know, the RAPL is still
> > > > package
> > > > scope 
> > > 
> > > I think in that case we can go with rule 2, it would be future
> > > proof
> > > for Intel systems. If you agree, I can make the change in next
> > > version.
> > > 
> > > Something like below?,
> > > 
> > > -#define rapl_pmu_is_pkg_scope()                         \
> > > -        (boot_cpu_data.x86_vendor == X86_VENDOR_AMD || 
> > > \                                                                
> > >     
> > >                                                                  
> > >     
> > >                          
> > > -        boot_cpu_data.x86_vendor == X86_VENDOR_HYGON)
> > > 
> > > +#define rapl_pmu_is_die_scope()                        \
> > > +       (boot_cpu_data.x86_model_id == CASCADELAKE)
> > > 
> > sounds good to me. Just a reminder that using boot_cpu_data.vfm is
> > a
> > better choice here.
> > 
> > And it would be great to get Peter' view on this.
> 
> Peter is confused :-) So you're saying that:
> 
>  - old Intel is pkg wide (it has no DIE enumeration)

right.

>  - Cascadelake (part of the skylake refresh) is per-DIE

right.
It enumerates DIE and its RAPL control (RAPL MSR scope) is also per-
DIE.

>  - modern Intel is pkg wide (they have no DIE enumeration)

right.

>  - future Intel will be pkg wide

see detailed comment below.
> 

> And this works because for everything that does not enumerate a
> specific
> DIE topology, it ends up begin the same as the PKG topology.

Right.

> 
> But what about future products that have DIE but are not CASCADE
> (what
> about COOPERLAKE) ?

For the one that I know, it has Die enumeration but its RAPL control is
still pkg wide.
However, the RAPL control for it (and other future Xeon platforms) is
not done via MSR interface so it does not break this RAPL PMU code.

Future Intel client platforms still rely on MSR to do RAPL control, but
their RAPL control scope (and if they will enumerate DIE) is not clear.

But still, IMO, this suggests that the RAPL control scope is not
architectural and a quirk list (probably ends up with Casecade-AP only)
makes more sense here.

thanks,
rui

> 
> If this really is a one off for CASCADE, then yes, I think we should
> be
> doing what Dhananjay suggests, and then the PKG naming is fine.
> 
>