Received-SPF: pass (google.com: domain of linux-kernel+bounces-31600-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240117141405.3063506-1-amitsinght@marvell.com>
 <20240117141405.3063506-14-amitsinght@marvell.com> <CALPaoCjf_A5SOr8L+0WMePW02Pzj9nnPT0JLZ_+232vvfEzGCQ@mail.gmail.com>
 <MW4PR18MB508449310648615886A8113BC6702@MW4PR18MB5084.namprd18.prod.outlook.com>
In-Reply-To: <MW4PR18MB508449310648615886A8113BC6702@MW4PR18MB5084.namprd18.prod.outlook.com>
From: Peter Newman <peternewman@google.com>
Date: Fri, 19 Jan 2024 14:05:17 -0800
Message-ID: <CALPaoCi1xABN+7LL4Xyi4Se87f8PM8769KiyZkzTESurZSyDqg@mail.gmail.com>
Subject: Re: [EXT] Re: [PATCH v1 13/14] arm_mpam: Handle resource instances
 mapped to different controls
To: Amit Singh Tomar <amitsinght@marvell.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, 
	"linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, 
	"fenghua.yu@intel.com" <fenghua.yu@intel.com>, 
	"reinette.chatre@intel.com" <reinette.chatre@intel.com>, "james.morse@arm.com" <james.morse@arm.com>, 
	George Cherian <gcherian@marvell.com>, "robh@kernel.org" <robh@kernel.org>, 
	"dfustini@baylibre.com" <dfustini@baylibre.com>, 
	"jonathan.cameron@huawei.com" <jonathan.cameron@huawei.com>, Tony Luck <tony.luck@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

[CC Tony Luck]

Hi Amit,

On Fri, Jan 19, 2024 at 5:01=E2=80=AFAM Amit Singh Tomar <amitsinght@marvel=
l.com> wrote:
>
> Hi Peter,
>
> Thanks for having a look.
>
> -----Original Message-----
> From: Peter Newman <peternewman@google.com>
> Sent: Wednesday, January 17, 2024 11:33 PM
> To: Amit Singh Tomar <amitsinght@marvell.com>
> Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; f=
enghua.yu@intel.com; reinette.chatre@intel.com; james.morse@arm.com; George=
 Cherian <gcherian@marvell.com>; robh@kernel.org; dfustini@baylibre.com; jo=
nathan.cameron@huawei.com
> Subject: [EXT] Re: [PATCH v1 13/14] arm_mpam: Handle resource instances m=
apped to different controls
>
> External Email
>
> ----------------------------------------------------------------------
> Hi Amit,
>
> On Wed, Jan 17, 2024 at 6:15=E2=80=AFAM Amit Singh Tomar <amitsinght@marv=
ell.com> wrote:
>
> >
> > +/* Club different resource properties under a class that resctrl
> > +uses,
> > + * for instance, L3 cache that supports both CPOR, and DSPRI need to
> > +have
> > + * knowledge of both cpbm_wd and dspri_wd. This is needed when two
> > +controls
> > + * are enumerated under differnt RIS Index.
> > + */
> > +static void mpam_enable_club_class_features(struct mpam_class *class,
> > +                                           struct mpam_msc_ris *ris)
>
> It looks like "club" is used as a synonym to "class" here to evade the bi=
gger issue that mpam_classes are not defined correctly as DSPRI resources s=
hould not be in the same mpam_class as the L3 CPOR and CSU features.
>
> This hardware makes it clear that the definition of mpam_class as all res=
ources in a (level x {memory,cache}) needs to be revised.
>
> On Marvell platform, DSPRI control register (MPAMCFG_PRI_NS), and Identif=
ication Register (MPAMF_PRI_IDR_NS)  are implemented within the LLC MPAM bl=
ock (the address range contains control and identification registers for CP=
OR, and DSPRI), and therefore we treat DSPRI as one of the L3 resource. How=
ever, suppose we approach it as totally different standalone resource type =
(PRI) other than Cache storage resource type (CPOR, and CCAP), and define a=
 new MPAM class type for it, there is no standard way to discover this new =
resource type (PRI) from ACPI tables.
>
> I'm concerned about accessing DSPRI related registers, if we are going to=
 tide it to new MPAM class (as we discover whole L3 MPAM block using firmwa=
re tables, and tide it's resources to L3 MPAM class).

This is becoming more of a discussion of MPAM (and resctrl) in
general, so I hope James can participate. Also I should point out that
when discussing MPAM, "resource" refers to a non-RIS MSC or a single
RIS-index on a RIS-enabled MSC, while the "mpam_class" structure in
the code is the counterpart to what RDT (and resctrl) call a resource.

From my reading of the code, the consequence of (RIS) resources being
in the same mpam_class is that they can be programmed uniformly
through a single schema line in the schemata file, so
__resource_props_mismatch() goes to work on eliminating any resources
(and extra control granularity) which are not programmed exactly the
same way. This function seems more geared toward big.LITTLE systems
where the cache controls on one cluster are dissimilar from those on
the peer cluster and would need to be normalized (James, can you
confirm?). But in this situation, it seems like a better idea to
present a separate schema for one cluster's controls from the others.
For example, "L2P" and "L2E", with non-overlapping domains.

In the case of an MSC implementing RIS, the controls are independent
by definition, so I can't see why the work done in
__resource_props_mismatch() would be applicable.

-Peter