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: <8524e71b-2261-42e7-a4a7-648f9e7d7240@intel.com>
Date: Wed, 3 Sep 2025 13:52:51 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: <babu.moger@....com>, <corbet@....net>, <tony.luck@...el.com>,
	<tglx@...utronix.de>, <mingo@...hat.com>, <bp@...en8.de>,
	<dave.hansen@...ux.intel.com>
CC: <Dave.Martin@....com>, <james.morse@....com>, <x86@...nel.org>,
	<hpa@...or.com>, <akpm@...ux-foundation.org>, <rostedt@...dmis.org>,
	<paulmck@...nel.org>, <pawan.kumar.gupta@...ux.intel.com>, <kees@...nel.org>,
	<arnd@...db.de>, <fvdl@...gle.com>, <seanjc@...gle.com>,
	<thomas.lendacky@....com>, <yosry.ahmed@...ux.dev>, <xin@...or.com>,
	<sohil.mehta@...el.com>, <kai.huang@...el.com>, <xiaoyao.li@...el.com>,
	<peterz@...radead.org>, <mario.limonciello@....com>, <xin3.li@...el.com>,
	<perry.yuan@....com>, <chang.seok.bae@...el.com>,
	<linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<peternewman@...gle.com>, <eranian@...gle.com>, <gautham.shenoy@....com>
Subject: Re: [PATCH v17 25/33] fs/resctrl: Provide interface to update the
 event configurations

Hi Babu,

On 9/3/25 11:32 AM, Moger, Babu wrote:
> Hi Reinette,
> 
> On 9/3/25 12:55, Reinette Chatre wrote:
>> Hi Babu,
>>
>> On 9/3/25 10:38 AM, Moger, Babu wrote:
>>> On 9/2/25 21:41, Reinette Chatre wrote:
>>>> On 8/14/25 7:25 PM, Babu Moger wrote:
>>
>>>>> +
>>>>> +ssize_t event_filter_write(struct kernfs_open_file *of, char *buf, size_t nbytes,
>>>>> +			   loff_t off)
>>>>> +{
>>>>> +	struct mon_evt *mevt = rdt_kn_parent_priv(of->kn);
>>>>> +	struct rdt_resource *r;
>>>>> +	u32 evt_cfg = 0;
>>>>> +	int ret = 0;
>>>>> +
>>>>> +	/* Valid input requires a trailing newline */
>>>>> +	if (nbytes == 0 || buf[nbytes - 1] != '\n')
>>>>> +		return -EINVAL;
>>>>> +
>>>>> +	buf[nbytes - 1] = '\0';
>>>>> +
>>>>> +	cpus_read_lock();
>>>>> +	mutex_lock(&rdtgroup_mutex);
>>>>> +
>>>>> +	rdt_last_cmd_clear();
>>>>> +
>>>>> +	r = resctrl_arch_get_resource(mevt->rid);
>>>>> +	if (!resctrl_arch_mbm_cntr_assign_enabled(r)) {
>>>>> +		rdt_last_cmd_puts("mbm_event counter assignment mode is not enabled\n");
>>>>> +		ret = -EINVAL;
>>>>> +		goto out_unlock;
>>>>> +	}
>>>>> +
>>>>> +	ret = resctrl_parse_mem_transactions(buf, &evt_cfg);
>>>>> +	if (!ret && mevt->evt_cfg != evt_cfg) {
>>>>
>>>> ... is evt_cfg of 0 (a) a valid value (that will not cause hardware to fault) and
>>>> (b) a reasonable value to allow? 
>>>>
>>>
>>> The value evt_cfg = 0 is valid and permitted for both ABMC and BMEC. I
>>> have confirmed here through verification and testing. In that case, the
>>> event counter will not be monitoring anything.
>>
>> Thank you for checking.
>>
>> I do not know what a use case for this would be but I also do not see why kernel
>> should prevent user space from doing this. Looks like event_filter_show() will then
>> just print a '\n' that is expected. Are counters expected to always return 0 in this
>> case or will they return an error? I am not clear on what qualifies as "invalid counter
>> configuration" for ABMC that results in RMID_VAL_ERROR.
> 
> The event counters return 0 in this case. I would not think this as an
> invalid event configuration.
> 
> One option is to add a text in last_cmd_status from event_filter_show().
> 
> rdt_last_cmd_printf("Event %s is not configured to monitor any memory
> transactions\n", mevt->name);


event_filter_show() does not return an error in this case so a user may not
check last_cmd_status since the doc specifies that "If the command was successful,
it will read as "ok"." Since the write() succeeded we should not fail the associated
show() to prompt the user to check last_cmd_status though.

Even so, an additional message does not seem necessary to me because as I understand
event_filter_show() returns an empty (just a '\n') file that I think
already reflects that no memory transactions are configured. It remains an awkward
use case to me but from what you stated this is valid for the hardware, it is what the
user requested, and the user can use interface to confirm the state. It does not seem
like changes are needed for this scenario.

Reinette


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ