Re: [PATCH v1 13/14] arm_mpam: Handle resource instances mapped to different controls

From: Amit Singh Tomar <amitsinght@marvell.com>
To: Peter Newman <peternewman@google.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>,
	Dave Martin <Dave.Martin@arm.com>
Subject: Re: [PATCH v1 13/14] arm_mpam: Handle resource instances mapped to different controls
Date: Wed, 8 May 2024 19:25:33 +0530	[thread overview]
Message-ID: <c25073f0-5af2-42ca-bf61-e85df2711e3c@marvell.com> (raw)
In-Reply-To: <CALPaoCi1xABN+7LL4Xyi4Se87f8PM8769KiyZkzTESurZSyDqg@mail.gmail.com>

Hi Peter,

>>
>> It looks like "club" is used as a synonym to "class" here to evade the bigger issue that mpam_classes are not defined correctly as DSPRI resources should 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 resources in a (level x {memory,cache}) needs to be revised.
>>
>> On Marvell platform, DSPRI control register (MPAMCFG_PRI_NS), and Identification Register (MPAMF_PRI_IDR_NS)  are implemented within the LLC MPAM block (the address range contains control and identification registers for CPOR, and DSPRI), and therefore we treat DSPRI as one of the L3 resource. However, 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 firmware 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, 
This greatly relies on the implementation.

For an MSC, identical control may be situated at two separate indexes, 
each with different attributes. For instance, let's take an example
of MARVELL implementation, There are three different resources at index 
0,1,2. These are enumerated in TAD_CMN_MPAM_RIS_E:

0: MSC
1: LTG
2: DTG

LLC MSC, resource at index 1, and 2 possesses cache portion partitioning 
feature, i.e., If MPAMCFG_PART_SEL_NS[RIS] is set to 1 (LTG) or to 2 
(DTG), then MPAMF_IDR_NS[HAS_CPOR_PART] is set to 1. LTG resource has 16 
portion bitmap, and DTG has 18 portion bitmap (mapped to same CPOR 
control), and only one can be configured.

so I can't see why the work done in
> __resource_props_mismatch() would be applicable.
>
IMHO, __resource_props_mismatch would be needed for such cases.

Thanks
-Amit

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel