| 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: <aXfWVPg7YTToDKcQ@x1>
Date: Mon, 26 Jan 2026 13:02:12 -0800
From: Drew Fustini <fustini@...nel.org>
To: yunhui cui <cuiyunhui@...edance.com>
Cc: Paul Walmsley <pjw@...nel.org>, Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>, Alexandre Ghiti <alex@...ti.fr>,
Radim Krčmář <rkrcmar@...tanamicro.com>,
Samuel Holland <samuel.holland@...ive.com>,
Adrien Ricciardi <aricciardi@...libre.com>,
Nicolas Pitre <npitre@...libre.com>,
Kornel Dulęba <mindal@...ihalf.com>,
Atish Patra <atish.patra@...ux.dev>,
Atish Kumar Patra <atishp@...osinc.com>,
Vasudevan Srinivasan <vasu@...osinc.com>,
Ved Shanbhogue <ved@...osinc.com>,
Chen Pei <cp0613@...ux.alibaba.com>,
Liu Zhiwei <zhiwei_liu@...ux.alibaba.com>,
Weiwei Li <liwei1518@...il.com>, guo.wenjia23@....com.cn,
liu.qingtao2@....com.cn, Conor Dooley <conor+dt@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Reinette Chatre <reinette.chatre@...el.com>,
Tony Luck <tony.luck@...el.com>, Babu Moger <babu.moger@....com>,
Peter Newman <peternewman@...gle.com>,
Fenghua Yu <fenghua.yu@...el.com>,
James Morse <james.morse@....com>, Ben Horgan <ben.horgan@....com>,
Dave Martin <Dave.Martin@....com>, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org,
x86@...nel.org
Subject: Re: [External] [PATCH RFC 08/19] RISC-V: QoS: add resctrl interface
for CBQRI controllers
On Mon, Jan 26, 2026 at 09:01:30PM +0800, yunhui cui wrote:
> Hi Drew,
>
> On Tue, Jan 20, 2026 at 12:15 PM Drew Fustini <fustini@...nel.org> wrote:
> >
> > Add interface for CBQRI controller drivers to make use of the resctrl
> > filesystem.
> >
> > Co-developed-by: Adrien Ricciardi <aricciardi@...libre.com>
> > Signed-off-by: Adrien Ricciardi <aricciardi@...libre.com>
> > Signed-off-by: Drew Fustini <fustini@...nel.org>
> > ---
> > arch/riscv/kernel/qos/qos_resctrl.c | 1191 +++++++++++++++++++++++++++++++++++
> > 1 file changed, 1191 insertions(+)
> >
> > diff --git a/arch/riscv/kernel/qos/qos_resctrl.c b/arch/riscv/kernel/qos/qos_resctrl.c
> > new file mode 100644
> > index 000000000000..5e3a65342e9b
> > --- /dev/null
> > +++ b/arch/riscv/kernel/qos/qos_resctrl.c
[..]
> > +/* Set capacity block mask (cc_block_mask) */
> > +static void cbqri_set_cbm(struct cbqri_controller *ctrl, u64 cbm)
> > +{
> > + int reg_offset;
> > + u64 reg;
> > +
> > + reg_offset = CBQRI_CC_BLOCK_MASK_OFF;
> > + reg = ioread64(ctrl->base + reg_offset);
> > +
> > + reg = cbm;
> > + iowrite64(reg, ctrl->base + reg_offset);
> > +}
> > +
>
> too much indentation ？
Thanks, will fix indentation for cbqri_set_cbm, cbqri_set_rbwb, and
cbqri_get_rbwb.
> > +static int cbqri_wait_busy_flag(struct cbqri_controller *ctrl, int reg_offset)
> > +{
> > + unsigned long timeout = jiffies + (HZ / 10); /* Timeout after 100ms */
>
> hung task ?
Good point, this is too long. I tried 100 us which wasn't long enough
for Qemu on my system but 1,000 us worked okay.
I will change to 1,000 us.
unsigned long timeout = jiffies + usecs_to_jiffies(1000);
> > +u32 resctrl_arch_get_config(struct rdt_resource *r, struct rdt_ctrl_domain *d,
> > + u32 closid, enum resctrl_conf_type type)
> > +{
> > + struct cbqri_resctrl_dom *hw_dom;
> > + struct cbqri_controller *ctrl;
> > + int reg_offset;
> > + u32 percent;
> > + u32 rbwb;
> > + u64 reg;
> > + int err;
> > +
> > + hw_dom = container_of(d, struct cbqri_resctrl_dom, resctrl_ctrl_dom);
> > +
> > + ctrl = hw_dom->hw_ctrl;
> > +
> > + if (!r->alloc_capable)
> > + return -EINVAL;
>
> u32 ?
Good point. I looked at mpam_resctrl.c and it returns the default value
on error:
https://lore.kernel.org/all/20260112165914.4086692-19-ben.horgan@arm.com/
I will change to 'return resctrl_get_default_ctrl(r)'.
> > +
> > + switch (r->rid) {
> > + case RDT_RESOURCE_L2:
> > + case RDT_RESOURCE_L3:
> > + /* Clear cc_block_mask before read limit operation */
> > + cbqri_set_cbm(ctrl, 0);
> > +
> > + /* Capacity read limit operation for RCID (closid) */
> > + err = cbqri_cc_alloc_op(ctrl, CBQRI_CC_ALLOC_CTL_OP_READ_LIMIT, type, closid);
> > + if (err < 0) {
> > + pr_err("%s(): operation failed: err = %d", __func__, err);
> > + return -EIO;
>
> u32 ?
Same as above.
> > + }
> > +
> > + /* Read capacity block mask for RCID (closid) */
> > + reg_offset = CBQRI_CC_BLOCK_MASK_OFF;
> > + reg = ioread64(ctrl->base + reg_offset);
> > +
> > + /* Update the config value for the closid in this domain */
> > + hw_dom->ctrl_val[closid] = reg;
> > + return hw_dom->ctrl_val[closid];
> > +
> > + case RDT_RESOURCE_MBA:
> > + /* Capacity read limit operation for RCID (closid) */
> > + err = cbqri_bc_alloc_op(ctrl, CBQRI_CC_ALLOC_CTL_OP_READ_LIMIT, closid);
> > + if (err < 0) {
> > + pr_err("%s(): operation failed: err = %d", __func__, err);
> > + return -EIO;
> > + }
> > +
> > + hw_dom->ctrl_val[closid] = cbqri_get_rbwb(ctrl);
> > +
> > + /* Convert from bandwidth blocks to percent */
> > + rbwb = hw_dom->ctrl_val[closid];
> > + rbwb *= 100;
> > + percent = rbwb / ctrl->bc.nbwblks;
> > + if (rbwb % ctrl->bc.nbwblks)
> > + percent++;
>
> set: 10, get 11 ?
The intention was to handle rounding up but I can drop this increment.
[..]
> > +static int cbqri_probe_controller(struct cbqri_controller_info *ctrl_info,
> > + struct cbqri_controller *ctrl)
> > +{
> > + int err = 0, status;
> > + u64 reg;
> > +
> > + pr_info("controller info: type=%d addr=0x%lx size=%lu max-rcid=%u max-mcid=%u",
> > + ctrl_info->type, ctrl_info->addr, ctrl_info->size,
> > + ctrl_info->rcid_count, ctrl_info->mcid_count);
> > +
> > + /* max_rmid is used by resctrl_arch_system_num_rmid_idx() */
> > + max_rmid = ctrl_info->mcid_count;
> > +
> > + ctrl->ctrl_info = ctrl_info;
> > +
> > + /* Try to access the memory-mapped CBQRI registers */
> > + if (!request_mem_region(ctrl_info->addr, ctrl_info->size, "cbqri_controller")) {
> > + pr_warn("%s(): return %d", __func__, err);
> > + return err;
>
> err = 0?
Good point, I will change it to return -EBUSY.
[..]
> > +int qos_resctrl_setup(void)
> > +{
> > + struct rdt_ctrl_domain *domain, *domain_temp;
> > + struct cbqri_controller_info *ctrl_info;
> > + struct cbqri_controller *ctrl;
> > + struct cbqri_resctrl_res *res;
> > + static int found_controllers;
> > + int err = 0;
> > + int id = 0;
> > + int i;
> > +
> > + list_for_each_entry(ctrl_info, &cbqri_controllers, list) {
> > + err = cbqri_probe_controller(ctrl_info, &controllers[found_controllers]);
> > + if (err) {
> > + pr_warn("%s(): failed (%d)", __func__, err);
> > + goto err_unmap_controllers;
> > + }
> > +
> > + found_controllers++;
> > + if (found_controllers > MAX_CONTROLLERS) {
> > + pr_warn("%s(): increase MAX_CONTROLLERS value", __func__);
> > + break;
> > + }
> > + }
> > +
> > + for (i = 0; i < RDT_NUM_RESOURCES; i++) {
> > + res = &cbqri_resctrl_resources[i];
> > + INIT_LIST_HEAD(&res->resctrl_res.ctrl_domains);
> > + INIT_LIST_HEAD(&res->resctrl_res.mon_domains);
> > + res->resctrl_res.rid = i;
> > + }
> > +
> > + for (i = 0; i < found_controllers; i++) {
> > + ctrl = &controllers[i];
>
> controllers[6] ?
This is limitation from the proof-of-concept. I need to refactor this so
that it is dynamically allocated based on the number of controllers
present in the system
Thanks,
Drew
Powered by blists - more mailing lists