Received-SPF: pass (google.com: domain of linux-kernel+bounces-140923-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Thu, 11 Apr 2024 16:35:58 +0100
From: Dave Martin <Dave.Martin@arm.com>
To: Shawn Wang <shawnwang@linux.alibaba.com>
Cc: James Morse <james.morse@arm.com>, x86@kernel.org,
	linux-kernel@vger.kernel.org, Fenghua Yu <fenghua.yu@intel.com>,
	Reinette Chatre <reinette.chatre@intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	H Peter Anvin <hpa@zytor.com>, Babu Moger <Babu.Moger@amd.com>,
	shameerali.kolothum.thodi@huawei.com,
	D Scott Phillips OS <scott@os.amperecomputing.com>,
	carl@os.amperecomputing.com, lcherian@marvell.com,
	bobo.shaobowang@huawei.com, tan.shaopeng@fujitsu.com,
	baolin.wang@linux.alibaba.com, Jamie Iles <quic_jiles@quicinc.com>,
	Xin Hao <xhao@linux.alibaba.com>, peternewman@google.com,
	dfustini@baylibre.com, amitsinght@marvell.com,
	David Hildenbrand <david@redhat.com>,
	Rex Nie <rex.nie@jaguarmicro.com>
Subject: Re: [PATCH v1 10/31] x86/resctrl: Move monitor init work to a
 resctrl init call
Message-ID: <ZhgDXhcUgS/uV3p5@e133380.arm.com>
References: <20240321165106.31602-1-james.morse@arm.com>
 <20240321165106.31602-11-james.morse@arm.com>
 <beee157c-6e7b-466e-93cd-81a9eb033fe7@linux.alibaba.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <beee157c-6e7b-466e-93cd-81a9eb033fe7@linux.alibaba.com>

On Wed, Apr 10, 2024 at 10:57:30AM +0800, Shawn Wang wrote:
> Hi James,
> 
> On 3/22/24 12:50 AM, James Morse wrote:
> > rdt_get_mon_l3_config() is called from the architecture's
> > resctrl_arch_late_init(), and initialises both architecture specific
> > fields, such as hw_res->mon_scale and resctrl filesystem fields
> > by calling dom_data_init().
> > 
> > To separate the filesystem and architecture parts of resctrl, this
> > function needs splitting up.
> > 
> > Add resctrl_mon_resource_init() to do the filesystem specific work,
> > and call it from resctrl_init(). This runs later, but is still before
> > the filesystem is mounted and the rmid_ptrs[] array can be used.
> > 
> > Signed-off-by: James Morse <james.morse@arm.com>
> > ---
> >   arch/x86/kernel/cpu/resctrl/internal.h |  1 +
> >   arch/x86/kernel/cpu/resctrl/monitor.c  | 24 +++++++++++++++++-------
> >   arch/x86/kernel/cpu/resctrl/rdtgroup.c |  4 ++++
> >   3 files changed, 22 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> > index 031948322eab..7a0c74779c53 100644
> > --- a/arch/x86/kernel/cpu/resctrl/internal.h
> > +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> > @@ -540,6 +540,7 @@ int rdtgroup_mondata_show(struct seq_file *m, void *arg);
> >   void mon_event_read(struct rmid_read *rr, struct rdt_resource *r,
> >   		    struct rdt_domain *d, struct rdtgroup *rdtgrp,
> >   		    int evtid, int first);
> > +int resctrl_mon_resource_init(void);
> >   void mbm_setup_overflow_handler(struct rdt_domain *dom,
> >   				unsigned long delay_ms,
> >   				int exclude_cpu);
> > diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
> > index 06565153ceb2..929ec1430b45 100644
> > --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> > +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> > @@ -1003,12 +1003,28 @@ static void l3_mon_evt_init(struct rdt_resource *r)
> >   		list_add_tail(&mbm_local_event.list, &r->evt_list);
> >   }
> > +int resctrl_mon_resource_init(void)
> > +{
> > +	struct rdt_resource *r = resctrl_arch_get_resource(RDT_RESOURCE_L3);
> > +	int ret;
> > +
> > +	if (!r->mon_capable)
> > +		return 0;
> > +
> > +	ret = dom_data_init(r);
> > +	if (ret)
> > +		return ret;
> > +
> > +	l3_mon_evt_init(r);
> > +
> > +	return 0;
> > +}
> > +
> 
> Now x86 platform defaults to all monitoring features on the L3 cache, but some monitoring features may also be on the L2 cache or memory controller according to the MPAM spec. For example, the memory bandwidth monitors could be on the memory controller.
> 
> Will resctrl_mon_resource_init() consider this scenario?
> 
> Best Regards,
> Shawn

My understanding is limited here, so this will probably have to wait
until James gets back for a full answer.

The resctrl userspace filesystem interface looks like it can describe
monitors at multiple levels of the memory system, so my assumption would
be that it should be possible to wire it up.

This series focuses on refactoring and does not try to add new
functionality, though.

Cheers
---Dave