linux-kernel - Re: [PATCH v5 21/40] x86/resctrl: Change mon_event_config

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: <a0936b56-4a60-4fb3-be65-91e65d360aca@intel.com>
Date: Wed, 23 Oct 2024 15:19:13 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: James Morse <james.morse@....com>, <x86@...nel.org>,
	<linux-kernel@...r.kernel.org>
CC: Fenghua Yu <fenghua.yu@...el.com>, Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, H Peter Anvin
	<hpa@...or.com>, Babu Moger <Babu.Moger@....com>,
	<shameerali.kolothum.thodi@...wei.com>, D Scott Phillips OS
	<scott@...amperecomputing.com>, <carl@...amperecomputing.com>,
	<lcherian@...vell.com>, <bobo.shaobowang@...wei.com>,
	<tan.shaopeng@...itsu.com>, <baolin.wang@...ux.alibaba.com>, Jamie Iles
	<quic_jiles@...cinc.com>, Xin Hao <xhao@...ux.alibaba.com>,
	<peternewman@...gle.com>, <dfustini@...libre.com>, <amitsinght@...vell.com>,
	David Hildenbrand <david@...hat.com>, Rex Nie <rex.nie@...uarmicro.com>,
	"Dave Martin" <dave.martin@....com>, Shaopeng Tan
	<tan.shaopeng@...fujitsu.com>
Subject: Re: [PATCH v5 21/40] x86/resctrl: Change
 mon_event_config_{read,write}() to be arch helpers

Hi James,

On 10/4/24 11:03 AM, James Morse wrote:
> @@ -315,6 +322,24 @@ int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid);
>  
>  bool __init resctrl_arch_is_evt_configurable(enum resctrl_event_id evt);
>  
> +/**
> + * resctrl_arch_mon_event_config_write() - Write the config for a counter.

Please avoid the term "counter" for events ... the upcoming AMD work adds support
for counters*. 

> + * @info:	struct resctrl_mon_config_info describing the resource, domain
> + *		and event.

Expected "config" to appear as part of information about function
that claims to writes config?

> + *
> + * Must be called on a CPU that is a member of the specified domain.

I am not sure about this. Is this documentation intended to support authors of
arch code? In that case it may be instead useful to know that this function will be
called on a CPU that is a member of the specified domain to avoid confusion from arch
side whether it needs to take some action to ensure function is called on right CPU.

	Called on a CPU that is a member of the specified domain.

> + */
> +void resctrl_arch_mon_event_config_write(void *info);
> +
> +/**
> + * resctrl_arch_mon_event_config_read() - Read the config for a counter.

counter -> event

> + * @info:	struct resctrl_mon_config_info describing the resource, domain
> + *		and event.

Copy&paste? No information on how config is returned.

> + *
> + * Must be called on a CPU that is a member of the specified domain.

Same comment as above.

> + */
> +void resctrl_arch_mon_event_config_read(void *info);
> +
>  /*
>   * Update the ctrl_val and apply this config right now.
>   * Must be called on one of the domain's CPUs.

Reinette


* Awaiting Arm's feedback on that on whether it works for MPAM.