| 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
| ||
|
Message-ID: <f2951303-6fe9-4674-bd16-0dbef39cc1d4@intel.com>
Date: Tue, 10 Feb 2026 14:57:29 -0800
From: Reinette Chatre <reinette.chatre@...el.com>
To: Ben Horgan <ben.horgan@....com>
CC: <amitsinght@...vell.com>, <baisheng.gao@...soc.com>,
<baolin.wang@...ux.alibaba.com>, <carl@...amperecomputing.com>,
<dave.martin@....com>, <david@...nel.org>, <dfustini@...libre.com>,
<fenghuay@...dia.com>, <gshan@...hat.com>, <james.morse@....com>,
<jonathan.cameron@...wei.com>, <kobak@...dia.com>, <lcherian@...vell.com>,
<linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
<peternewman@...gle.com>, <punit.agrawal@....qualcomm.com>,
<quic_jiles@...cinc.com>, <rohit.mathew@....com>,
<scott@...amperecomputing.com>, <sdonthineni@...dia.com>,
<tan.shaopeng@...itsu.com>, <xhao@...ux.alibaba.com>,
<catalin.marinas@....com>, <will@...nel.org>, <corbet@....net>,
<maz@...nel.org>, <oupton@...nel.org>, <joey.gouly@....com>,
<suzuki.poulose@....com>, <kvmarm@...ts.linux.dev>, <zengheng4@...wei.com>,
<linux-doc@...r.kernel.org>, Shaopeng Tan <tan.shaopeng@...fujitsu.com>
Subject: Re: [PATCH v4 13/41] arm_mpam: resctrl: Add boilerplate cpuhp and
domain allocation
Hi Ben,
On 2/3/26 1:43 PM, Ben Horgan wrote:
...
> diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
> new file mode 100644
> index 000000000000..4c2248c92955
> --- /dev/null
> +++ b/drivers/resctrl/mpam_resctrl.c
> @@ -0,0 +1,343 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2025 Arm Ltd.
> +
> +#define pr_fmt(fmt) "%s:%s: " fmt, KBUILD_MODNAME, __func__
> +
> +#include <linux/arm_mpam.h>
> +#include <linux/cacheinfo.h>
> +#include <linux/cpu.h>
> +#include <linux/cpumask.h>
> +#include <linux/errno.h>
> +#include <linux/list.h>
> +#include <linux/printk.h>
> +#include <linux/rculist.h>
> +#include <linux/resctrl.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +
> +#include <asm/mpam.h>
> +
> +#include "mpam_internal.h"
> +
> +/*
> + * The classes we've picked to map to resctrl resources, wrapped
> + * in with their resctrl structure.
> + * Class pointer may be NULL.
> + */
> +static struct mpam_resctrl_res mpam_resctrl_controls[RDT_NUM_RESOURCES];
> +
> +#define for_each_mpam_resctrl_control(res, rid) \
> + for (rid = 0, res = &mpam_resctrl_controls[rid]; \
> + rid < RDT_NUM_RESOURCES; \
> + rid++, res = &mpam_resctrl_controls[rid])
> +
> +/* The lock for modifying resctrl's domain lists from cpuhp callbacks. */
> +static DEFINE_MUTEX(domain_list_lock);
> +
> +static bool exposed_alloc_capable;
> +static bool exposed_mon_capable;
> +
> +bool resctrl_arch_alloc_capable(void)
> +{
> + return exposed_alloc_capable;
> +}
> +
> +bool resctrl_arch_mon_capable(void)
> +{
> + return exposed_mon_capable;
> +}
> +
> +/*
> + * MSC may raise an error interrupt if it sees an out or range partid/pmg,
> + * and go on to truncate the value. Regardless of what the hardware supports,
> + * only the system wide safe value is safe to use.
> + */
> +u32 resctrl_arch_get_num_closid(struct rdt_resource *ignored)
> +{
> + return mpam_partid_max + 1;
> +}
> +
> +struct rdt_resource *resctrl_arch_get_resource(enum resctrl_res_level l)
> +{
> + if (l >= RDT_NUM_RESOURCES)
> + return NULL;
> +
> + return &mpam_resctrl_controls[l].resctrl_res;
> +}
> +
> +static int mpam_resctrl_control_init(struct mpam_resctrl_res *res)
> +{
> + /* TODO: initialise the resctrl resources */
> +
> + return 0;
> +}
> +
> +static int mpam_resctrl_pick_domain_id(int cpu, struct mpam_component *comp)
> +{
> + struct mpam_class *class = comp->class;
> +
> + if (class->type == MPAM_CLASS_CACHE)
> + return comp->comp_id;
> +
> + /* TODO: repaint domain ids to match the L3 domain ids */
> + /* Otherwise, expose the ID used by the firmware table code. */
> + return comp->comp_id;
> +}
> +
> +static void mpam_resctrl_domain_hdr_init(int cpu, struct mpam_component *comp,
> + struct rdt_domain_hdr *hdr)
> +{
> + lockdep_assert_cpus_held();
> +
> + INIT_LIST_HEAD(&hdr->list);
> + hdr->id = mpam_resctrl_pick_domain_id(cpu, comp);
> + cpumask_set_cpu(cpu, &hdr->cpu_mask);
One addition via the resctrl telemetry enabling is a new rdt_domain_hdr::rid
used for some additional checks on the header.
https://lore.kernel.org/all/20251217172121.12030-2-tony.luck@intel.com/
I may be missing something here though since the additional checking that this
new field supports should have complained loudly ... unless this was tested with
only the L3 resource that happens to be 0.
...
> +static struct mpam_resctrl_dom *
> +mpam_resctrl_get_domain_from_cpu(int cpu, struct mpam_resctrl_res *res)
> +{
> + struct mpam_resctrl_dom *dom;
> + struct rdt_resource *r = &res->resctrl_res;
> +
> + lockdep_assert_cpus_held();
> +
> + list_for_each_entry_rcu(dom, &r->ctrl_domains, resctrl_ctrl_dom.hdr.list) {
> + if (cpumask_test_cpu(cpu, &dom->ctrl_comp->affinity))
> + return dom;
> + }
I notice here that that only the ctrl_domains list is searched ...
> +
> + return NULL;
> +}
> +
> +int mpam_resctrl_online_cpu(unsigned int cpu)
> +{
> + struct mpam_resctrl_res *res;
> + enum resctrl_res_level rid;
> +
> + guard(mutex)(&domain_list_lock);
> + for_each_mpam_resctrl_control(res, rid) {
> + struct mpam_resctrl_dom *dom;
> +
> + if (!res->class)
> + continue; // dummy_resource;
> +
> + dom = mpam_resctrl_get_domain_from_cpu(cpu, res);
Consider a system that only supports monitoring (exposed_alloc_capable == false,
exposed_mon_capable == true). Since mpam_resctrl_get_domain_from_cpu() only
searches control domains then it looks to me as though dom will always be false
here?
> + if (!dom) {
> + dom = mpam_resctrl_alloc_domain(cpu, res);
Would this (on hypothetical exposed_alloc_capable == false, exposed_mon_capable == true system)
then cause a new domain to be allocated for each CPU with a single CPU in its cpumask
instead of allocating a single monitoring domain with multiple CPUs in its mask?
> + } else {
> + if (exposed_alloc_capable) {
> + struct rdt_ctrl_domain *ctrl_d = &dom->resctrl_ctrl_dom;
> +
> + mpam_resctrl_online_domain_hdr(cpu, &ctrl_d->hdr);
> + }
> + if (exposed_mon_capable) {
> + struct rdt_l3_mon_domain *mon_d = &dom->resctrl_mon_dom;
> +
> + mpam_resctrl_online_domain_hdr(cpu, &mon_d->hdr);
> + }
> + }
> + if (IS_ERR(dom))
> + return PTR_ERR(dom);
> + }
> +
> + resctrl_online_cpu(cpu);
> +
> + return 0;
> +}
> +
> +void mpam_resctrl_offline_cpu(unsigned int cpu)
> +{
> + struct mpam_resctrl_res *res;
> + enum resctrl_res_level rid;
> +
> + resctrl_offline_cpu(cpu);
> +
> + guard(mutex)(&domain_list_lock);
> + for_each_mpam_resctrl_control(res, rid) {
> + struct mpam_resctrl_dom *dom;
> + struct rdt_l3_mon_domain *mon_d;
> + struct rdt_ctrl_domain *ctrl_d;
> + bool ctrl_dom_empty, mon_dom_empty;
> +
> + if (!res->class)
> + continue; // dummy resource
> +
> + dom = mpam_resctrl_get_domain_from_cpu(cpu, res);
> + if (WARN_ON_ONCE(!dom))
Similar to above ... it looks to me as though this WARN may always be
encountered on a system that only supports monitoring.
> + continue;
> +
> + if (exposed_alloc_capable) {
> + ctrl_d = &dom->resctrl_ctrl_dom;
> + ctrl_dom_empty = mpam_resctrl_offline_domain_hdr(cpu, &ctrl_d->hdr);
> + if (ctrl_dom_empty)
> + resctrl_offline_ctrl_domain(&res->resctrl_res, ctrl_d);
> + } else {
> + ctrl_dom_empty = true;
> + }
> +
> + if (exposed_mon_capable) {
> + mon_d = &dom->resctrl_mon_dom;
> + mon_dom_empty = mpam_resctrl_offline_domain_hdr(cpu, &mon_d->hdr);
> + if (mon_dom_empty)
> + resctrl_offline_mon_domain(&res->resctrl_res, &mon_d->hdr);
> + } else {
> + mon_dom_empty = true;
> + }
> +
> + if (ctrl_dom_empty && mon_dom_empty)
> + kfree(dom);
> + }
> +}
> +
Reinette
Powered by blists - more mailing lists