Re: [PATCH v2 3/5] x86,fs/resctrl: Move MSR_IA32_L{2,3}_QOS_CFG scope property to x86 resource

[Chen Yu <yu.c.chen@intel.com>]

Hi Reinette,

On Mon, Oct 19, 2026 at 04:06:35PM -0700, Reinette Chatre wrote:
> Hi Chenyu,
> 
> On 10/18/26 10:40 AM, Chen Yu wrote:
> > On Wed, Oct 07, 2026 at 02:06:45PM -0700, Reinette Chatre wrote:
> >>
> >> MSR_IA32_L{2,3}_QOS_CFG enable code and data prioritization on x86. On
> >> Intel the registers have the scope of the associated cache while on AMD
> >> they have CPU scope. The scope is recorded as a property of the cache
> >> resource's control, of which each cache resource currently supports only
> >> one.
> >>
> >> The scope of MSR_IA32_L{2,3}_QOS_CFG is only relevant to x86. Even so, the
> >> property is found among the resctrl filesystem's cache properties while only
> >> x86 code sets and reads it. Keeping the property with the cache control
> >> properties would also duplicate it across all controls of a resource when
> >> resources support multiple controls.
> >>
> >> Make the scope of MSR_IA32_L{2,3}_QOS_CFG a property of the x86 hardware
> >> resource and name it for the registers it describes now that it no longer
> >> needs to be generic.
> >>
> >> Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
> > 
> > [ ... ]
> > 
> >> @@ -140,6 +143,7 @@ struct rdt_hw_resource {
> >>  	unsigned int		mon_scale;
> >>  	unsigned int		mbm_width;
> >>  	bool			cdp_enabled;
> >> +	bool			qos_cfg_has_cpu_scope;
> >>  	bool			mbm_cntr_assign_enabled;
> >>  	bool			sdciae_enabled;
> >>  };
> > 
> > It seems that some flags are in resource scope, while others are in the controller's scope,
> > cache's flags are in the resource scope, MB's flags are in controller's scope, for example "linear".
> 
> The goal of this change is to move this information into the only (x86) architectural driver where it
> is relevant.
> 
> These "flags" are different in a couple of ways:
> a) qos_cfg_has_cpu_scope is specific to x86 architecture (thus located in struct rdt_hw_resource of
>    x86 driver, not struct rdt_resource belonging to resctrl fs) - this information is not exposed
>    to user space nor is it of use to resctrl filesystem. This is just information internal to
>    architecture whether the x86 MSR has CPU or domain scope to accommodate this difference between
>    AMD and Intel.
> b) the MB controller flags like scalar and linear are associated with the controller,
>    exposed to user space, and managed by resctrl filesystem.
>

I see, the hardware-specific attributes/indicators could be in the hardware resource
scope(and usually they are global settings), while the MB controller's flag is not hardware
specific, and they are not global settings.

> To me this qos_cfg_has_cpu_scope flag is closer to the architectural support you are working on
> that contains within x86 architecture driver the decision whether it should use ACPI or MSR than
> it is to control flags.
>

It seems to be a little different in the control_mode case, currently control_mode is in the
struct rdt_resource, but its callback resctrl_arch_control_mode_set() is x86 architecture driver
specific.

> > For now, there is only one controller under the L2/L3 cache resource, so it might
> > not be duplicated if we keep qos_cfg_has_cpu_scope in the controller scope to keep consistence? Or do
> 
> I do not see how qos_cfg_has_cpu_scope is related to properties of controls exposed via resctrl fs.
> resctrl filesystem may request CDP to be enabled/disabled on a control but how that enabling is done, whether
> the architecture uses MMIO, CPU scope MSR, or domain scope MSR, that knowledge is private to the
> architecture.
>

I agree.

> > you mean if in the future there are multiple controllers under this cache resource, all controllers'
> > flags (e.g., qos_cfg_has_cpu_scope) would still share the same value, so it is OK to put the flag in
> > the cache resource? If yes, the code looks good to me,
> 
> This patch is not about moving a cache control flag into the resource but instead about making
> an architecture specific flag (unrelated to a control) invisible to resctrl filesystem. 
>  
> > Review-by: Chen Yu <yu.c.chen@intel.com>
> Thanks, but I do not think we have the same view of this change. Please let me know if you
> still feel this patch is ok after this discussion.
>

Yes, thanks for the detailed explanation. This patch is OK with me.

thanks,
Chenyu

> Reinette
>