Re: [PATCH RFC v2 06/17] RISC-V: QoS: define CBQRI resctrl resources and domains

[Drew Fustini <fustini@kernel.org>]

On Fri, Feb 13, 2026 at 03:15:57PM -0800, Reinette Chatre wrote:
> Hi Drew,
> 
> (caveat for all my comments)
> I scanned this series for resctrl interactions and my comments are
> focused around that instead of a thorough review of all the code
> within. 

Thanks for spending the time to review. It has helped me to realize that
there is a lot cruft leftover from when I was developing based on the
mpam snapshot at the time (2023) and never reorganized after rebasing on
upstream.

> 
> On 1/28/26 12:27 PM, Drew Fustini wrote:
> > diff --git a/arch/riscv/kernel/qos/internal.h b/arch/riscv/kernel/qos/internal.h
> > index ff2c7eff50be..c0402dd06cfa 100644
> > --- a/arch/riscv/kernel/qos/internal.h
> > +++ b/arch/riscv/kernel/qos/internal.h
> > @@ -65,6 +65,11 @@
> >  #define CBQRI_BC_ALLOC_CTL_OP_READ_LIMIT   2
> >  #define CBQRI_BC_ALLOC_CTL_STATUS_SUCCESS  1
> >  
> > +int qos_resctrl_setup(void);
> > +void qos_resctrl_exit(void);
> > +int qos_resctrl_online_cpu(unsigned int cpu);
> > +int qos_resctrl_offline_cpu(unsigned int cpu);
> > +
> >  /* Capacity Controller hardware capabilities */
> >  struct riscv_cbqri_capacity_caps {
> >  	u16 ncblks; /* number of capacity blocks */
> > @@ -125,4 +130,26 @@ struct cbqri_controller {
> >  	bool mon_capable;
> >  };
> >  
> > +struct cbqri_resctrl_res {
> > +	struct rdt_resource     resctrl_res;
> > +	struct cbqri_controller controller;
> > +	u32 max_rcid;
> > +	u32 max_mcid;
> > +};
> > +
> > +struct cbqri_resctrl_dom {
> > +	struct rdt_domain_hdr       resctrl_dom_hdr;
> 
> The resctrl_dom_hdr above should not be needed and indeed does
> not seem to be used in this series. The two members below it,
> struct rdt_ctrl_domain and struct rdt_mon_domain (now named
> rdt_l3_mon_domain), have the necessary struct rdt_domain_hdr as
> their first member.

Thank you for the insight. I think added this when trying get the code
working again after rebasing from an old mpam snapshot to upstream last
year. It does seem like I didn't understand the data structures well
enough and should clean up cbqri_resctrl_dom.
> 
> Without any comments or changelog that describes the other members
> I am not able to really consider their inclusion here. From the
> names it seems that most members are intended to support the
> control domain usage so it is not clear to me why the cbqri domain
> contains both a control and monitor domain while they are interacted
> with separately by resctrl. From what I can tell resctrl_mon_dom
> is not used by this implementation.
> 
> As hinted above, when you rebase you will find that struct rdt_mon_domain
> is now named struct rdt_l3_mon_domain (see commit 4bc3ef46ff41
> ("x86,fs/resctrl: Rename struct rdt_mon_domain and rdt_hw_mon_domain")) to
> make it obvious that it is specific to L3 monitoring. Having this
> as a member for all cbqri domains will then become increasingly confusing
> since the cbqri_resctrl_dom is also used for L2 cache allocation as well as the
> memory bandwidth allocation that are not at L3 scope.
> 
> I think it may be simplest to drop the resctrl_mon_dom member until
> monitoring is supported?

Thanks for the suggestion. Monitoring broke when rebasing on upstream
and also the realization that my approach to MBM was too much of a hack.
I think that does make sense for me to remove all the monitoring code
until I have a working implementation based on the current upstream.

Thanks,
Drew