linux-kernel - Re: [PATCH 02/15] x86/resctrl: Remove hard-coded memory bandwidth event configuration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20f60b82-b2de-0ddb-661c-1ec48cbb44b4@amd.com>
Date:   Thu, 7 Dec 2023 17:37:45 -0600
From:   "Moger, Babu" <bmoger@....com>
To:     Reinette Chatre <reinette.chatre@...el.com>,
        "corbet@....net" <corbet@....net>,
        "fenghua.yu@...el.com" <fenghua.yu@...el.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "bp@...en8.de" <bp@...en8.de>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>
Cc:     "x86@...nel.org" <x86@...nel.org>, "hpa@...or.com" <hpa@...or.com>,
        "paulmck@...nel.org" <paulmck@...nel.org>,
        "rdunlap@...radead.org" <rdunlap@...radead.org>,
        "tj@...nel.org" <tj@...nel.org>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "seanjc@...gle.com" <seanjc@...gle.com>,
        "Phillips, Kim" <kim.phillips@....com>,
        "jmattson@...gle.com" <jmattson@...gle.com>,
        "ilpo.jarvinen@...ux.intel.com" <ilpo.jarvinen@...ux.intel.com>,
        "jithu.joseph@...el.com" <jithu.joseph@...el.com>,
        "kan.liang@...ux.intel.com" <kan.liang@...ux.intel.com>,
        "Dadhania, Nikunj" <nikunj.dadhania@....com>,
        "daniel.sneddon@...ux.intel.com" <daniel.sneddon@...ux.intel.com>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "rick.p.edgecombe@...el.com" <rick.p.edgecombe@...el.com>,
        "rppt@...nel.org" <rppt@...nel.org>,
        "maciej.wieczor-retman@...el.com" <maciej.wieczor-retman@...el.com>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "eranian@...gle.com" <eranian@...gle.com>,
        "peternewman@...gle.com" <peternewman@...gle.com>,
        "Giani, Dhaval" <Dhaval.Giani@....com>
Subject: Re: [PATCH 02/15] x86/resctrl: Remove hard-coded memory bandwidth
 event configuration

Hi Reinette,

> -----Original Message-----
> From: Reinette Chatre <reinette.chatre@...el.com>
> Sent: Thursday, December 7, 2023 1:02 PM
> To: Moger, Babu <Babu.Moger@....com>; corbet@....net;
> fenghua.yu@...el.com; tglx@...utronix.de; mingo@...hat.com;
> bp@...en8.de; dave.hansen@...ux.intel.com
> Cc: x86@...nel.org; hpa@...or.com; paulmck@...nel.org;
> rdunlap@...radead.org; tj@...nel.org; peterz@...radead.org;
> seanjc@...gle.com; Phillips, Kim <kim.phillips@....com>;
> jmattson@...gle.com; ilpo.jarvinen@...ux.intel.com;
> jithu.joseph@...el.com; kan.liang@...ux.intel.com; Dadhania, Nikunj
> <nikunj.dadhania@....com>; daniel.sneddon@...ux.intel.com;
> pbonzini@...hat.com; rick.p.edgecombe@...el.com; rppt@...nel.org;
> maciej.wieczor-retman@...el.com; linux-doc@...r.kernel.org; linux-
> kernel@...r.kernel.org; eranian@...gle.com; peternewman@...gle.com;
> Giani, Dhaval <Dhaval.Giani@....com>
> Subject: Re: [PATCH 02/15] x86/resctrl: Remove hard-coded memory
> bandwidth event configuration
> 
> Hi Babu,
> 
> On 12/6/2023 11:17 AM, Moger, Babu wrote:
> > On 12/6/23 12:32, Reinette Chatre wrote:
> >> On 12/6/2023 9:17 AM, Moger, Babu wrote:
> >>> On 12/5/23 17:21, Reinette Chatre wrote:
> >>>> On 11/30/2023 4:57 PM, Babu Moger wrote:
> 
> ...
> 
> >>>>>  static void mondata_config_read(struct rdt_domain *d, struct
> >>>>> mon_config_info *mon_info) @@ -1621,7 +1621,7 @@ static int
> mbm_config_write_domain(struct rdt_resource *r,
> >>>>>  	int ret = 0;
> >>>>>
> >>>>>  	/* mon_config cannot be more than the supported set of events */
> >>>>> -	if (val > MAX_EVT_CONFIG_BITS) {
> >>>>> +	if (val > resctrl_max_evt_bitmask) {
> >>>>>  		rdt_last_cmd_puts("Invalid event configuration\n");
> >>>>>  		return -EINVAL;
> >>>>>  	}
> >>>>
> >>>> This does not look right. resctrl_max_evt_bitmask contains the
> >>>> supported types. A user may set a value that is less than
> >>>> resctrl_max_evt_bitmask but yet have an unsupported bit set, no?
> >>>
> >>> I think I have to make this clear in the patch. There is no
> >>> difference in the definition. Hardware supports all the events reported by
> the cpuid.
> >>
> >> I'll try to elaborate using an example. Let's say AMD decides to make
> >> hardware with hypothetical support mask of:
> >> 	resctrl_max_evt_bitmask = 0x4F (no support for Slow Mem).
> >>
> >> What if user attempts to set config that enables monitoring of Slow Mem:
> >> 	val = 0x30
> >>
> >> In the above example, val is not larger than resctrl_max_evt_bitmask
> >> but it is an invalid config, no?
> >
> > Yes. It is invalid config in this case.
> >
> > How about changing the check to something like this?
> >
> > if ((val & resctrl_max_evt_bitmask) != val) {
> >                 rdt_last_cmd_puts("Invalid event configuration\n");
> >   		return -EINVAL;
> >    }
> 
> This would address the scenario. I also think that it will be helpful to print the
> valid bitmask as part of the error message. The original implementation
> specified that all bits are valid and in doing so no interface accompanied the
> feature to share with users what the valid bits are. The only way user space
> can learn this is is to read the *_config files after the first resctrl mount after a
> system boot to see with which config values the system was initialized with
> (assuming system was initialized with all supported bits enabled).

Sure. Will add the error message including the valid bitmask.
Thanks
Babu