Re: [External] [PATCH RFC 08/19] RISC-V: QoS: add resctrl interface for CBQRI controllers

[Drew Fustini <fustini@kernel.org>]

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@kernel.org> wrote:
> >
> > Add interface for CBQRI controller drivers to make use of the resctrl
> > filesystem.
> >
> > Co-developed-by: Adrien Ricciardi <aricciardi@baylibre.com>
> > Signed-off-by: Adrien Ricciardi <aricciardi@baylibre.com>
> > Signed-off-by: Drew Fustini <fustini@kernel.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