Re: [PATCH v1 18/31] x86/resctrl: Allow resctrl_arch_mon_event_config_write() to return an error

Reinette Chatre
2024-04-09T03:23:43Z

Hi James,

On 3/21/2024 9:50 AM, James Morse wrote:
> resctrl_arch_mon_event_config_write() writes a bitmap of events provided
> by user-space into the configuration register for the monitors.
>
> This assumes that all architectures support all the features each bit
> corresponds to.
>
> MPAM can filter monitors based on read, write, or both, but there are
> many more options in the existing bitmap. To allow this interface to
> work for machines with MPAM, allow the architecture helper to return
> an error if an incompatible bitmap is set.
>
> When valid values are provided, there is no change in behaviour. If
> an invalid value is provided, currently it is silently ignored, but
> last_cmd_status is updated. After this change, the parser will stop
> at the first invalid value and return an error to user-space. This
> matches the way changes to the schemata file are made.
>

Is this needed? With move of mbm_cfg_mask to rdt_resource I expect
MPAM would use it to set what the valid values are. With that done,
when user space provides a value, mon_config_write() compares user
provided value against mbm_cfg_mask and will already return early
(before attempting to write to hardware) with error
if value is not supported. This seems to accomplish the goal of this
patch?

> Signed-off-by: James Morse <james.morse@arm.com>
> ---

...
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index 8a7367d1ce45..6705d7960dfd 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -200,6 +200,7 @@ struct resctrl_mon_config_info {
> struct rdt_domain *d;
> u32 evtid;
> u32 mon_config;
> + int err;
> };

Please take care to use consistent spacing.

Reinette