linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [RFC 00/12] ARM: MPAM: add support for priority partitioning control
@ 2023-08-15 15:27 Amit Singh Tomar
  2023-08-15 15:27 ` [RFC 01/12] arm_mpam: Handle resource instances mapped to different controls Amit Singh Tomar
                   ` (15 more replies)
  0 siblings, 16 replies; 39+ messages in thread
From: Amit Singh Tomar @ 2023-08-15 15:27 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: fenghua.yu, reinette.chatre, james.morse, gcherian, robh,
	peternewman, Amit Singh Tomar

Arm Memory System Resource Partitioning and Monitoring (MPAM) supports
different controls that can be applied to different resources in the system
For instance, an optional priority partitioning control where priority
value is generated from one MSC, propagates over interconnect to other MSC
(known as downstream priority), or can be applied within an MSC for internal
operations.

Marvell implementation of ARM MPAM supports priority partitioning control
that allows LLC MSC to generate priority values that gets propagated (along with
read/write request from upstream) to DDR Block. Within the DDR block the
priority values is mapped to different traffic class under DDR QoS strategy.
The link[1] gives some idea about DDR QoS strategy, and terms like LPR, VPR
and HPR.

Setup priority partitioning control under Resource control
----------------------------------------------------------
At present, resource control (resctrl) provides basic interface to configure/set-up
CAT (Cache Allocation Technology) and MBA (Memory Bandwidth Allocation) capabilities.
ARM MPAM uses it to support controls like Cache portion partition (CPOR), and 
MPAM bandwidth partitioning.

As an example, "schemata" file under resource control group contains information about
cache portion bitmaps, and memory bandwidth allocation, and these are used to configure
Cache portion partition (CPOR), and MPAM bandwidth partitioning controls.

MB:0=0100
L3:0=ffff

But resctrl doesn't provide a way to set-up other control that ARM MPAM provides
(For instance, Priority partitioning control as mentioned above). To support this,
James has suggested to use already existing schemata to be compatible with 
portable software, and this is the main idea behind this RFC is to have some kind
of discussion on how resctrl can be extended to support priority partitioning control.

To support Priority partitioning control, "schemata" file is updated to accommodate
priority field (upon priority partitioning capability detection), separated from CPBM
using delimiter ",".

L3:0=ffff,f where f indicates downstream priority max value.

These dspri value gets programmed per partition, that can be used to override 
QoS value coming from upstream (CPU).

RFC patch-set[2] is based on James Morse's MPAM snapshot[3] for 6.2, and ACPI
table is based on DEN0065A_MPAM_ACPI_2.0.

Test set-up and results:
------------------------

The downstream priority value feeds into DRAM controller, and one of the important
thing that it does with this value is to service the requests sooner (based on the 
traffic class), hence reducing latency without affecting performance.

Within the DDR QoS traffic class.

0--5 ----> Low priority value
6-10 ----> Medium priority value
11-15 ----> High priority value

Benchmark[4] used is multichase.

Two partition P1 and P2:

Partition P1:
-------------
Assigned core 0
100% BW assignment

Partition P2:
-------------
Assigned cores 1-79
100% BW assignment

Test Script:
-----------
mkdir p1
cd p1
echo 1 > cpus
echo L3:1=8000,5 > schemata   ##### DSPRI set as 5 (lpr)
echo "MB:0=100" > schemata

mkdir p2
cd p2
echo ffff,ffffffff,fffffffe > cpus
echo L3:1=8000,0 > schemata
echo "MB:0=100" > schemata

### Loaded latency run, core 0 does chaseload (pointer chase) with low priority value 5, and cores 1-79 does memory bandwidth run ###
./multiload -v -n 10 -t 80 -m 1G -c chaseload  

cd /sys/fs/resctrl/p1

echo L3:1=8000,a > schemata  ##### DSPRI set as 0xa (vpr)

### Loaded latency run, core 0 does chaseload (pointer chase) with medium priority value a, and cores 1-79 does memory bandwidth run ###
./multiload -v -n 10 -t 80 -m 1G -c chaseload

cd /sys/fs/resctrl/p1

echo L3:1=8000,f > schemata  ##### DSPRI set as 0xf (hpr)

### Loaded latency run where core 0 does chaseload (pointer chase) with high priority value f, and cores 1-79 does memory bandwidth run ###
./multiload -v -n 10 -t 80 -m 1G -c chaseload

Results[5]:

LPR average latency is 204.862(ns) vs VPR average latency is 161.018(ns) vs HPR average latency is 134.210(ns).

[1]: https://drops.dagstuhl.de/opus/volltexte/2021/13934/pdf/LIPIcs-ECRTS-2021-3.pdf
[2]: https://github.com/Amit-Radur/linux/commits/mpam_downstream_priority_work
[3]: https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git/log/?h=mpam/snapshot/v6.2
[4]: https://github.com/google/multichase
[5]:

root@localhost:# ./dspri_test.sh
Info: Loaded Latency chase selected. A -l memload can be used to select a specific memory load
nr_threads = 80
page_size = 4096 bytes
total_memory = 1073741824 (1024.0 MiB)
stride = 256
tlb_locality = 262144
chase = chaseload
memload = stream-sum
run_test_type = RUN_CHASE_LOADED
main: sample_no=0 
main: sample_no=1  avg=204.9(ns)
 main: threads=79, Total(MiB/s)=343018.0, PerThread=4342
main: sample_no=2  avg=206.0(ns)
 main: threads=79, Total(MiB/s)=343038.0, PerThread=4342
main: sample_no=3  avg=206.4(ns)
 main: threads=79, Total(MiB/s)=342443.0, PerThread=4335
main: sample_no=4  avg=206.3(ns)
 main: threads=79, Total(MiB/s)=345156.0, PerThread=4369
main: sample_no=5  avg=205.6(ns)
 main: threads=79, Total(MiB/s)=343807.0, PerThread=4352
main: sample_no=6  avg=205.9(ns)
 main: threads=79, Total(MiB/s)=343593.0, PerThread=4349
main: sample_no=7  avg=206.3(ns)
 main: threads=79, Total(MiB/s)=344770.0, PerThread=4364
main: sample_no=8  avg=205.7(ns)
 main: threads=79, Total(MiB/s)=344935.0, PerThread=4366
main: sample_no=9  avg=205.3(ns)
 main: threads=79, Total(MiB/s)=343189.0, PerThread=4344
main: sample_no=10  avg=206.1(ns)
 main: threads=79, Total(MiB/s)=344455.0, PerThread=4360
ChasAVG=205.848485, ChasGEO=205.847944, ChasBEST=204.861518, ChasWORST=206.443386, ChasDEV=0.008   
LdAvgMibs=343840.400000, LdMaxMibs=345156.000000, LdMinMibs=342443.000000, LdDevMibs=0.008   
Samples	, Byte/thd	, ChaseThds	, ChaseNS	, ChaseMibs	, ChDeviate	, LoadThds	, LdMaxMibs	, LdAvgMibs	, LdDeviate	, ChaseArg	, MemLdArg
10    	, 1073741824 	, 1       	, 204.862 	, 37      	, 0.008   	, 79      	, 345156  	, 343840  	, 0.008   	, chaseload	, stream-sum
Info: Loaded Latency chase selected. A -l memload can be used to select a specific memory load
nr_threads = 80
page_size = 4096 bytes
total_memory = 1073741824 (1024.0 MiB)
stride = 256
tlb_locality = 262144
chase = chaseload
memload = stream-sum
run_test_type = RUN_CHASE_LOADED
main: sample_no=0 
main: sample_no=1  avg=161.4(ns)
 main: threads=79, Total(MiB/s)=342023.0, PerThread=4329
main: sample_no=2  avg=161.3(ns)
 main: threads=79, Total(MiB/s)=341773.0, PerThread=4326
main: sample_no=3  avg=161.4(ns)
 main: threads=79, Total(MiB/s)=342780.0, PerThread=4339
main: sample_no=4  avg=161.6(ns)
 main: threads=79, Total(MiB/s)=341275.0, PerThread=4320
main: sample_no=5  avg=161.0(ns)
 main: threads=79, Total(MiB/s)=342680.0, PerThread=4338
main: sample_no=6  avg=161.9(ns)
 main: threads=79, Total(MiB/s)=341538.0, PerThread=4323
main: sample_no=7  avg=161.5(ns)
 main: threads=79, Total(MiB/s)=345302.0, PerThread=4371
main: sample_no=8  avg=161.5(ns)
 main: threads=79, Total(MiB/s)=341352.0, PerThread=4321
main: sample_no=9  avg=161.5(ns)
 main: threads=79, Total(MiB/s)=341200.0, PerThread=4319
main: sample_no=10  avg=161.5(ns)
 main: threads=79, Total(MiB/s)=341874.0, PerThread=4328
ChasAVG=161.458012, ChasGEO=161.457856, ChasBEST=161.017587, ChasWORST=161.935907, ChasDEV=0.006   
LdAvgMibs=342179.700000, LdMaxMibs=345302.000000, LdMinMibs=341200.000000, LdDevMibs=0.012   
Samples	, Byte/thd	, ChaseThds	, ChaseNS	, ChaseMibs	, ChDeviate	, LoadThds	, LdMaxMibs	, LdAvgMibs	, LdDeviate	, ChaseArg	, MemLdArg
10    	, 1073741824 	, 1       	, 161.018 	, 47      	, 0.006   	, 79      	, 345302  	, 342180  	, 0.012   	, chaseload	, stream-sum
Info: Loaded Latency chase selected. A -l memload can be used to select a specific memory load
nr_threads = 80
page_size = 4096 bytes
total_memory = 1073741824 (1024.0 MiB)
stride = 256
tlb_locality = 262144
chase = chaseload
memload = stream-sum
run_test_type = RUN_CHASE_LOADED
main: sample_no=0 
main: sample_no=1  avg=134.3(ns)
 main: threads=79, Total(MiB/s)=345284.0, PerThread=4371
main: sample_no=2  avg=134.7(ns)
 main: threads=79, Total(MiB/s)=345295.0, PerThread=4371
main: sample_no=3  avg=134.4(ns)
 main: threads=79, Total(MiB/s)=344421.0, PerThread=4360
main: sample_no=4  avg=134.9(ns)
 main: threads=79, Total(MiB/s)=343273.0, PerThread=4345
main: sample_no=5  avg=134.5(ns)
 main: threads=79, Total(MiB/s)=345518.0, PerThread=4374
main: sample_no=6  avg=134.5(ns)
 main: threads=79, Total(MiB/s)=346052.0, PerThread=4380
main: sample_no=7  avg=134.5(ns)
 main: threads=79, Total(MiB/s)=342852.0, PerThread=4340
main: sample_no=8  avg=134.7(ns)
 main: threads=79, Total(MiB/s)=345818.0, PerThread=4377
main: sample_no=9  avg=134.2(ns)
 main: threads=79, Total(MiB/s)=344045.0, PerThread=4355
main: sample_no=10  avg=134.7(ns)
 main: threads=79, Total(MiB/s)=344345.0, PerThread=4359
ChasAVG=134.547983, ChasGEO=134.547841, ChasBEST=134.210254, ChasWORST=134.863073, ChasDEV=0.005   
LdAvgMibs=344690.300000, LdMaxMibs=346052.000000, LdMinMibs=342852.000000, LdDevMibs=0.009   
Samples	, Byte/thd	, ChaseThds	, ChaseNS	, ChaseMibs	, ChDeviate	, LoadThds	, LdMaxMibs	, LdAvgMibs	, LdDeviate	, ChaseArg	, MemLdArg
10    	, 1073741824 	, 1       	, 134.210 	, 57      	, 0.005   	, 79      	, 346052  	, 344690  	, 0.009   	, chaseload	, stream-sum

Amit Singh Tomar (12):
  arm_mpam: Handle resource instances mapped to different controls
  arm_mpam: resctrl: Detect priority partitioning capability
  arm_mpam: resctrl: Define new schemata format for priority partition
  fs/resctrl: Obtain CPBM upon priority partition presence
  fs/resctrl: Set-up downstream priority partition resources
  fs/resctrl: Extend schemata read for priority partition control
  arm_mpam: resctrl: Retrieve priority values from arch code
  fs/resctrl: Schemata write only for intended resource
  fs/resctrl: Extend schemata write for priority partition control
  arm_mpam: resctrl: Facilitate writing downstream priority value
  arm_mpam: Fix Downstream priority mask
  arm_mpam: Program Downstream priority value

 drivers/platform/mpam/mpam_devices.c  |  38 +++++++--
 drivers/platform/mpam/mpam_internal.h |   1 +
 drivers/platform/mpam/mpam_resctrl.c  |  64 +++++++++++---
 fs/resctrl/ctrlmondata.c              | 118 ++++++++++++++++++++++++--
 fs/resctrl/rdtgroup.c                 |  30 +++++++
 include/linux/resctrl.h               |  12 +++
 6 files changed, 235 insertions(+), 28 deletions(-)

-- 
2.25.1


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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [RFC 01/12] arm_mpam: Handle resource instances mapped to different controls
  2023-08-15 15:27 [RFC 00/12] ARM: MPAM: add support for priority partitioning control Amit Singh Tomar
@ 2023-08-15 15:27 ` Amit Singh Tomar
  2023-09-01 12:39   ` Jonathan Cameron
  2023-08-15 15:27 ` [RFC 02/12] arm_mpam: resctrl: Detect priority partitioning capability Amit Singh Tomar
                   ` (14 subsequent siblings)
  15 siblings, 1 reply; 39+ messages in thread
From: Amit Singh Tomar @ 2023-08-15 15:27 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: fenghua.yu, reinette.chatre, james.morse, gcherian, robh,
	peternewman, Amit Singh Tomar

At the moment, configuring multiple resource instances (mapped to same
control) under a resource class is not supported. For instance, on MARVELL
SoC MPAMF_IDR_NS[RIS_MAX] (under LLC MSC) is 0x2, and there are three
different resource 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 have 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.

LLC MSC resource at index 0 has the Priority partitioning features.
If MPAMCFG_PART_SEL_NS[RIS] is set to 0 (MSC), then MPAMF_IDR_NS[HAS_PRI_PART]
is set to 1, leaving HAS_CPOR_PART bit to 0. CPOR and PRI_PART are
mutually exclusive resources as far configuration is concerned.

With this change, multiple resource instances that maps to different
control, say LTG for CPOR, and MSC for PRI_PART is handled properly.

Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
---
 drivers/platform/mpam/mpam_devices.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/mpam/mpam_devices.c b/drivers/platform/mpam/mpam_devices.c
index 589ff1ef2b6b..137cbff925ba 100644
--- a/drivers/platform/mpam/mpam_devices.c
+++ b/drivers/platform/mpam/mpam_devices.c
@@ -1829,6 +1829,19 @@ static void mpam_enable_init_class_features(struct mpam_class *class)
 	class->props = ris->props;
 }
 
+/* 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.
+ */
+static void mpam_enable_club_class_features(struct mpam_class *class,
+					    struct mpam_msc_ris *ris)
+{
+	class->props.features |= ris->props.features;
+	class->props.cpbm_wd |= ris->props.cpbm_wd;
+	class->props.dspri_wd |= ris->props.dspri_wd;
+	class->props.num_csu_mon |= ris->props.num_csu_mon;
+}
+
 /* Merge all the common resource features into class. */
 static void mpam_enable_merge_features(void)
 {
@@ -1843,7 +1856,16 @@ static void mpam_enable_merge_features(void)
 
 		list_for_each_entry(comp, &class->components, class_list) {
 			list_for_each_entry(ris, &comp->ris, comp_list) {
-				__resource_props_mismatch(ris, class);
+				/* There can be multiple resources under a class which is
+				 * mapped to different controls, for instance L3 cache
+				 * can have both CPOR and DSPRI implemented, and following
+				 * would avoid property mismatch later on when different
+				 * resources are present.
+				 */
+				if (class->props.features != ris->props.features)
+					mpam_enable_club_class_features(class, ris);
+				else
+					__resource_props_mismatch(ris, class);
 
 				class->nrdy_usec = max(class->nrdy_usec,
 						     ris->msc->nrdy_usec);
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [RFC 02/12] arm_mpam: resctrl: Detect priority partitioning capability
  2023-08-15 15:27 [RFC 00/12] ARM: MPAM: add support for priority partitioning control Amit Singh Tomar
  2023-08-15 15:27 ` [RFC 01/12] arm_mpam: Handle resource instances mapped to different controls Amit Singh Tomar
@ 2023-08-15 15:27 ` Amit Singh Tomar
  2023-09-01 12:30   ` Jonathan Cameron
  2023-08-15 15:27 ` [RFC 03/12] arm_mpam: resctrl: Define new schemata format for priority partition Amit Singh Tomar
                   ` (13 subsequent siblings)
  15 siblings, 1 reply; 39+ messages in thread
From: Amit Singh Tomar @ 2023-08-15 15:27 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: fenghua.yu, reinette.chatre, james.morse, gcherian, robh,
	peternewman, Amit Singh Tomar

ARM MPAM supports different control that can be applied to different
resources in the system, for instance priority partitioning control
where priority value is generated from one MSC, propagates over
interconnect to other MSC (known as downstream priority), or can be
applied within an MSC for internal operations.

This change lets the resctrl know the about MSC's priority partitioning
capability.

Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
---
 drivers/platform/mpam/mpam_resctrl.c | 14 ++++++++++++++
 include/linux/resctrl.h              |  4 ++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/platform/mpam/mpam_resctrl.c b/drivers/platform/mpam/mpam_resctrl.c
index 1dbfb6f6dd34..09618d9ceb1d 100644
--- a/drivers/platform/mpam/mpam_resctrl.c
+++ b/drivers/platform/mpam/mpam_resctrl.c
@@ -435,6 +435,16 @@ static bool cache_has_usable_cpor(struct mpam_class *class)
 	return (class->props.cpbm_wd <= RESCTRL_MAX_CBM);
 }
 
+static bool cache_has_usable_priority_part(struct mpam_class *class)
+{
+	struct mpam_props *cprops = &class->props;
+
+	if (!mpam_has_feature(mpam_feat_dspri_part, cprops))
+		return false;
+
+	return (class->props.dspri_wd <= RESCTRL_MAX_DSPRI);
+}
+
 static bool cache_has_usable_csu(struct mpam_class *class)
 {
 	struct mpam_props *cprops;
@@ -691,6 +701,7 @@ static int mpam_resctrl_resource_init(struct mpam_resctrl_res *res)
 	    res->resctrl_res.rid == RDT_RESOURCE_L3) {
 		bool has_csu = cache_has_usable_csu(class);
 		bool has_mbwu = class_has_usable_mbwu(class);
+		bool has_ppart = cache_has_usable_priority_part(class);
 
 		/* TODO: Scaling is not yet supported */
 		r->cache.cbm_len = class->props.cpbm_wd;
@@ -718,6 +729,9 @@ static int mpam_resctrl_resource_init(struct mpam_resctrl_res *res)
 			exposed_alloc_capable = true;
 		}
 
+		if (has_ppart)
+			r->priority_cap = true;
+
 		/*
 		 * MBWU counters may be 'local' or 'total' depending on where
 		 * they are in the topology. If The counter is on the L3, its
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index 3ad308e9e226..a98ba5828211 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -38,6 +38,8 @@ int proc_resctrl_show(struct seq_file *m,
  */
 #define RESCTRL_MAX_CBM			32
 
+#define RESCTRL_MAX_DSPRI               63
+
 /* The format for packing fields into the u64 'id' exposed to user-space */
 #define RESCTRL_ID_CLOSID	GENMASK_ULL(31, 0)
 #define RESCTRL_ID_RMID		GENMASK_ULL(63, 32)
@@ -195,6 +197,7 @@ struct resctrl_membw {
  * @rid:		The index of the resource
  * @alloc_capable:	Is allocation available on this machine
  * @mon_capable:	Is monitor feature available on this machine
+ * @priority_capable:   Is priority partitioning feature available on this machine
  * @num_rmid:		Number of RMIDs available
  * @cache_level:	Which cache level defines scope of this resource
  * @cache:		Cache allocation related data
@@ -212,6 +215,7 @@ struct rdt_resource {
 	int			rid;
 	bool			alloc_capable;
 	bool			mon_capable;
+	bool                    priority_cap;
 	int			num_rmid;
 	int			cache_level;
 	struct resctrl_cache	cache;
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [RFC 03/12] arm_mpam: resctrl: Define new schemata format for priority partition
  2023-08-15 15:27 [RFC 00/12] ARM: MPAM: add support for priority partitioning control Amit Singh Tomar
  2023-08-15 15:27 ` [RFC 01/12] arm_mpam: Handle resource instances mapped to different controls Amit Singh Tomar
  2023-08-15 15:27 ` [RFC 02/12] arm_mpam: resctrl: Detect priority partitioning capability Amit Singh Tomar
@ 2023-08-15 15:27 ` Amit Singh Tomar
  2023-08-15 15:27 ` [RFC 04/12] fs/resctrl: Obtain CPBM upon priority partition presence Amit Singh Tomar
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 39+ messages in thread
From: Amit Singh Tomar @ 2023-08-15 15:27 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: fenghua.yu, reinette.chatre, james.morse, gcherian, robh,
	peternewman, Amit Singh Tomar

At the moment, "schemata" file contains information about cache portion
bitmap (CPBM), and memory bandwidth allocation (MBA) in the following
format.

MB:0=0100
L3:0=ffff

Lets' update the "schemata" file to accommodate information about
priority partition control that indicates downstream priority value.

For instance:

L3:0=ffff,f

CPBM is separated from downstream priority value using delimiter ",".

Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
---
 drivers/platform/mpam/mpam_resctrl.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/mpam/mpam_resctrl.c b/drivers/platform/mpam/mpam_resctrl.c
index 09618d9ceb1d..1081ceac41a8 100644
--- a/drivers/platform/mpam/mpam_resctrl.c
+++ b/drivers/platform/mpam/mpam_resctrl.c
@@ -711,7 +711,11 @@ static int mpam_resctrl_resource_init(struct mpam_resctrl_res *res)
 		r->cache.min_cbm_bits = 1;
 
 		/* TODO: kill these properties off as they are derivatives */
-		r->format_str = "%d=%0*x";
+		if (mpam_has_feature(mpam_feat_dspri_part, &class->props))
+			r->format_str = "%d=%0*x,%0*x";
+		else
+			r->format_str = "%d=%0*x";
+
 		r->fflags = RFTYPE_RES_CACHE;
 		r->default_ctrl = BIT_MASK(class->props.cpbm_wd) - 1;
 		r->data_width = (class->props.cpbm_wd + 3) / 4;
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [RFC 04/12] fs/resctrl: Obtain CPBM upon priority partition presence
  2023-08-15 15:27 [RFC 00/12] ARM: MPAM: add support for priority partitioning control Amit Singh Tomar
                   ` (2 preceding siblings ...)
  2023-08-15 15:27 ` [RFC 03/12] arm_mpam: resctrl: Define new schemata format for priority partition Amit Singh Tomar
@ 2023-08-15 15:27 ` Amit Singh Tomar
  2023-08-15 15:27 ` [RFC 05/12] fs/resctrl: Set-up downstream priority partition resources Amit Singh Tomar
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 39+ messages in thread
From: Amit Singh Tomar @ 2023-08-15 15:27 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: fenghua.yu, reinette.chatre, james.morse, gcherian, robh,
	peternewman, Amit Singh Tomar

Resource control group's "schemata" file represents the cache portion
bit map (XXXX), parsed from buffer with "L3:0=XXXX" format. Now, with
the introduction of priority control, "schemata" file format has changed
to "L3:0=XXXX,X", where cpbm (XXXX) is split from priority mask(X) using
using delimiter ",".

With this change, CPBM is properly fetched from schemata file when priority
partition support is present.

Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
---
 fs/resctrl/ctrlmondata.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
index 27d8bc25a4cb..b19ac2509e38 100644
--- a/fs/resctrl/ctrlmondata.c
+++ b/fs/resctrl/ctrlmondata.c
@@ -108,6 +108,9 @@ static bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r)
 	unsigned int cbm_len = r->cache.cbm_len;
 	int ret;
 
+	if (r->priority_cap)
+		buf = strsep(&buf, ",");
+
 	ret = kstrtoul(buf, 16, &val);
 	if (ret) {
 		rdt_last_cmd_printf("Non-hex character in the mask %s\n", buf);
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [RFC 05/12] fs/resctrl: Set-up downstream priority partition resources
  2023-08-15 15:27 [RFC 00/12] ARM: MPAM: add support for priority partitioning control Amit Singh Tomar
                   ` (3 preceding siblings ...)
  2023-08-15 15:27 ` [RFC 04/12] fs/resctrl: Obtain CPBM upon priority partition presence Amit Singh Tomar
@ 2023-08-15 15:27 ` Amit Singh Tomar
  2023-08-17 17:39   ` Fenghua Yu
  2023-08-15 15:27 ` [RFC 06/12] fs/resctrl: Extend schemata read for priority partition control Amit Singh Tomar
                   ` (10 subsequent siblings)
  15 siblings, 1 reply; 39+ messages in thread
From: Amit Singh Tomar @ 2023-08-15 15:27 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: fenghua.yu, reinette.chatre, james.morse, gcherian, robh,
	peternewman, Amit Singh Tomar

Upon resource control group creation, Cache portion bitmap, and Memory
bandwidth allocation gets initialized to the default/maximum values,
obtained from resource control code.

Let's replicate it for priority partition resource, and setup the default
downstream priority value.

Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
---
 drivers/platform/mpam/mpam_resctrl.c |  4 +++-
 fs/resctrl/rdtgroup.c                | 30 ++++++++++++++++++++++++++++
 include/linux/resctrl.h              |  4 ++++
 3 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/mpam/mpam_resctrl.c b/drivers/platform/mpam/mpam_resctrl.c
index 1081ceac41a8..cc843f1b0fb7 100644
--- a/drivers/platform/mpam/mpam_resctrl.c
+++ b/drivers/platform/mpam/mpam_resctrl.c
@@ -733,8 +733,10 @@ static int mpam_resctrl_resource_init(struct mpam_resctrl_res *res)
 			exposed_alloc_capable = true;
 		}
 
-		if (has_ppart)
+		if (has_ppart) {
 			r->priority_cap = true;
+			r->dspri_default_ctrl = BIT_MASK(class->props.dspri_wd) - 1;
+		}
 
 		/*
 		 * MBWU counters may be 'local' or 'total' depending on where
diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index 9c5dfaaa7fa2..bc5fb246ba68 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -3045,6 +3045,21 @@ static int rdtgroup_init_cat(struct resctrl_schema *s, u32 closid)
 	return 0;
 }
 
+static int rdtgroup_init_dspri(struct rdt_resource *r, u32 closid)
+{
+	struct resctrl_staged_config *cfg;
+	struct rdt_domain *d;
+
+	list_for_each_entry(d, &r->domains, list) {
+		cfg = &d->staged_config[CDP_NONE];
+		cfg->new_ctrl = r->dspri_default_ctrl;
+		cfg->have_new_ctrl = true;
+		r->dspri_store = true;
+	}
+
+	return 0;
+}
+
 /* Initialize MBA resource with default values. */
 static void rdtgroup_init_mba(struct rdt_resource *r, u32 closid)
 {
@@ -3082,12 +3097,27 @@ static int rdtgroup_init_alloc(struct rdtgroup *rdtgrp)
 				return ret;
 		}
 
+		if (r->priority_cap)
+			r->dspri_store = false;
+
 		ret = resctrl_arch_update_domains(r, rdtgrp->closid);
 		if (ret < 0) {
 			rdt_last_cmd_puts("Failed to initialize allocations\n");
 			return ret;
 		}
 
+		if (r->priority_cap) {
+			ret = rdtgroup_init_dspri(r, rdtgrp->closid);
+			if (ret < 0)
+				return ret;
+
+			ret = resctrl_arch_update_domains(r, rdtgrp->closid);
+			if (ret < 0) {
+				rdt_last_cmd_puts("Failed to initialize allocations\n");
+				return ret;
+			}
+			r->dspri_store = false;
+		}
 	}
 
 	rdtgrp->mode = RDT_MODE_SHAREABLE;
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index a98ba5828211..d5b0661c0f70 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -198,6 +198,7 @@ struct resctrl_membw {
  * @alloc_capable:	Is allocation available on this machine
  * @mon_capable:	Is monitor feature available on this machine
  * @priority_capable:   Is priority partitioning feature available on this machine
+ * @dspri_store:
  * @num_rmid:		Number of RMIDs available
  * @cache_level:	Which cache level defines scope of this resource
  * @cache:		Cache allocation related data
@@ -206,6 +207,7 @@ struct resctrl_membw {
  * @name:		Name to use in "schemata" file.
  * @data_width:		Character width of data when displaying
  * @default_ctrl:	Specifies default cache cbm or memory B/W percent.
+ * @dspri_default_ctrl: Specifies default downstream priority value.
  * @format_str:		Per resource format string to show domain value
  * @evt_list:		List of monitoring events
  * @fflags:		flags to choose base and info files
@@ -216,6 +218,7 @@ struct rdt_resource {
 	bool			alloc_capable;
 	bool			mon_capable;
 	bool                    priority_cap;
+	bool			dspri_store;
 	int			num_rmid;
 	int			cache_level;
 	struct resctrl_cache	cache;
@@ -224,6 +227,7 @@ struct rdt_resource {
 	char			*name;
 	int			data_width;
 	u32			default_ctrl;
+	u32			dspri_default_ctrl;
 	const char		*format_str;
 	struct list_head	evt_list;
 	unsigned long		fflags;
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [RFC 06/12] fs/resctrl: Extend schemata read for priority partition control
  2023-08-15 15:27 [RFC 00/12] ARM: MPAM: add support for priority partitioning control Amit Singh Tomar
                   ` (4 preceding siblings ...)
  2023-08-15 15:27 ` [RFC 05/12] fs/resctrl: Set-up downstream priority partition resources Amit Singh Tomar
@ 2023-08-15 15:27 ` Amit Singh Tomar
  2023-08-17 17:42   ` Fenghua Yu
  2023-08-15 15:27 ` [RFC 07/12] arm_mpam: resctrl: Retrieve priority values from arch code Amit Singh Tomar
                   ` (9 subsequent siblings)
  15 siblings, 1 reply; 39+ messages in thread
From: Amit Singh Tomar @ 2023-08-15 15:27 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: fenghua.yu, reinette.chatre, james.morse, gcherian, robh,
	peternewman, Amit Singh Tomar

At present, "schemata" file under resource control group reveals
information about Cache portion bitmap and Memory Bandwidth allocation.
With the introduction of priority partition control "schemata" is updated
to adopt priority value.

Let's enable support for reading the priority values for "schemata" file.

Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
---
 drivers/platform/mpam/mpam_resctrl.c |  4 ++++
 fs/resctrl/ctrlmondata.c             | 15 ++++++++++++---
 include/linux/resctrl.h              |  4 ++++
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/mpam/mpam_resctrl.c b/drivers/platform/mpam/mpam_resctrl.c
index cc843f1b0fb7..b491a0f897fd 100644
--- a/drivers/platform/mpam/mpam_resctrl.c
+++ b/drivers/platform/mpam/mpam_resctrl.c
@@ -735,7 +735,11 @@ static int mpam_resctrl_resource_init(struct mpam_resctrl_res *res)
 
 		if (has_ppart) {
 			r->priority_cap = true;
+			if (class->props.dspri_wd > 0x10)
+				class->props.dspri_wd = 0x10;
+
 			r->dspri_default_ctrl = BIT_MASK(class->props.dspri_wd) - 1;
+			r->dspri_data_width = (class->props.dspri_wd + 3) / 4;
 		}
 
 		/*
diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
index b19ac2509e38..8c8a4d09d22c 100644
--- a/fs/resctrl/ctrlmondata.c
+++ b/fs/resctrl/ctrlmondata.c
@@ -379,7 +379,7 @@ static void show_doms(struct seq_file *s, struct resctrl_schema *schema, int clo
 	struct rdt_resource *r = schema->res;
 	struct rdt_domain *dom;
 	bool sep = false;
-	u32 ctrl_val;
+	u32 ctrl_val, dspri_ctrl_val;
 
 	/* Walking r->domains, ensure it can't race with cpuhp */
 	lockdep_assert_cpus_held();
@@ -395,9 +395,18 @@ static void show_doms(struct seq_file *s, struct resctrl_schema *schema, int clo
 			ctrl_val = resctrl_arch_get_config(r, dom, closid,
 							   schema->conf_type);
 
-		seq_printf(s, r->format_str, dom->id, max_data_width,
-			   ctrl_val);
+		if (r->priority_cap) {
+			r->dspri_show = true;
+			dspri_ctrl_val = resctrl_arch_get_config(r, dom, closid,
+								 CDP_NONE);
+			seq_printf(s, r->format_str, dom->id, max_data_width, ctrl_val,
+					r->dspri_data_width, dspri_ctrl_val);
+		} else
+			seq_printf(s, r->format_str, dom->id, max_data_width,
+				   ctrl_val);
+
 		sep = true;
+		r->dspri_show = false;
 	}
 	seq_puts(s, "\n");
 }
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index d5b0661c0f70..d7100c330945 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -199,12 +199,14 @@ struct resctrl_membw {
  * @mon_capable:	Is monitor feature available on this machine
  * @priority_capable:   Is priority partitioning feature available on this machine
  * @dspri_store:
+ * @dspri_show:		flag to indicate downstream priority read
  * @num_rmid:		Number of RMIDs available
  * @cache_level:	Which cache level defines scope of this resource
  * @cache:		Cache allocation related data
  * @membw:		If the component has bandwidth controls, their properties.
  * @domains:		RCU list of all domains for this resource
  * @name:		Name to use in "schemata" file.
+ * @dspri_data_width    Character width of dspri value when displaying
  * @data_width:		Character width of data when displaying
  * @default_ctrl:	Specifies default cache cbm or memory B/W percent.
  * @dspri_default_ctrl: Specifies default downstream priority value.
@@ -219,6 +221,7 @@ struct rdt_resource {
 	bool			mon_capable;
 	bool                    priority_cap;
 	bool			dspri_store;
+	bool                    dspri_show;
 	int			num_rmid;
 	int			cache_level;
 	struct resctrl_cache	cache;
@@ -226,6 +229,7 @@ struct rdt_resource {
 	struct list_head	domains;
 	char			*name;
 	int			data_width;
+	int			dspri_data_width;
 	u32			default_ctrl;
 	u32			dspri_default_ctrl;
 	const char		*format_str;
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [RFC 07/12] arm_mpam: resctrl: Retrieve priority values from arch code
  2023-08-15 15:27 [RFC 00/12] ARM: MPAM: add support for priority partitioning control Amit Singh Tomar
                   ` (5 preceding siblings ...)
  2023-08-15 15:27 ` [RFC 06/12] fs/resctrl: Extend schemata read for priority partition control Amit Singh Tomar
@ 2023-08-15 15:27 ` Amit Singh Tomar
  2023-08-15 15:27 ` [RFC 08/12] fs/resctrl: Schemata write only for intended resource Amit Singh Tomar
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 39+ messages in thread
From: Amit Singh Tomar @ 2023-08-15 15:27 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: fenghua.yu, reinette.chatre, james.morse, gcherian, robh,
	peternewman, Amit Singh Tomar

Downstream priority values can be read from the "schemata" file, which is
fetched from mpam_config structure.

This change does the necessary modifications in arch specific code to
facilitate reading priority values.

Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
---
 drivers/platform/mpam/mpam_internal.h |  1 +
 drivers/platform/mpam/mpam_resctrl.c  | 11 ++++++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/mpam/mpam_internal.h b/drivers/platform/mpam/mpam_internal.h
index 30e32389d394..45552ae6548b 100644
--- a/drivers/platform/mpam/mpam_internal.h
+++ b/drivers/platform/mpam/mpam_internal.h
@@ -160,6 +160,7 @@ struct mpam_config {
 	u32	cpbm;
 	u32	mbw_pbm;
 	u16	mbw_max;
+	u16	dspri;
 };
 
 struct mpam_component
diff --git a/drivers/platform/mpam/mpam_resctrl.c b/drivers/platform/mpam/mpam_resctrl.c
index b491a0f897fd..dc710c90cfc2 100644
--- a/drivers/platform/mpam/mpam_resctrl.c
+++ b/drivers/platform/mpam/mpam_resctrl.c
@@ -864,7 +864,7 @@ u32 resctrl_arch_get_config(struct rdt_resource *r, struct rdt_domain *d,
 	lockdep_assert_cpus_held();
 
 	if (!mpam_is_enabled())
-		return r->default_ctrl;
+		return r->dspri_show ? r->dspri_default_ctrl : r->default_ctrl;
 
 	res = container_of(r, struct mpam_resctrl_res, resctrl_res);
 	dom = container_of(d, struct mpam_resctrl_dom, resctrl_dom);
@@ -876,7 +876,10 @@ u32 resctrl_arch_get_config(struct rdt_resource *r, struct rdt_domain *d,
 	switch (r->rid) {
 	case RDT_RESOURCE_L2:
 	case RDT_RESOURCE_L3:
-		configured_by = mpam_feat_cpor_part;
+		if (r->dspri_show)
+			configured_by = mpam_feat_dspri_part;
+		else
+			configured_by = mpam_feat_cpor_part;
 		break;
 	case RDT_RESOURCE_MBA:
 		if (mba_class_use_mbw_part(cprops)) {
@@ -893,12 +896,14 @@ u32 resctrl_arch_get_config(struct rdt_resource *r, struct rdt_domain *d,
 
 	if (!r->alloc_capable || partid >= resctrl_arch_get_num_closid(r) ||
 	    !mpam_has_feature(configured_by, cfg))
-		return r->default_ctrl;
+		return r->dspri_show ? r->dspri_default_ctrl : r->default_ctrl;
 
 	switch (configured_by) {
 	case mpam_feat_cpor_part:
 		/* TODO: Scaling is not yet supported */
 		return cfg->cpbm;
+	case mpam_feat_dspri_part:
+		return cfg->dspri;
 	case mpam_feat_mbw_part:
 		/* TODO: Scaling is not yet supported */
 		return mbw_pbm_to_percent(cfg->mbw_pbm, cprops);
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [RFC 08/12] fs/resctrl: Schemata write only for intended resource
  2023-08-15 15:27 [RFC 00/12] ARM: MPAM: add support for priority partitioning control Amit Singh Tomar
                   ` (6 preceding siblings ...)
  2023-08-15 15:27 ` [RFC 07/12] arm_mpam: resctrl: Retrieve priority values from arch code Amit Singh Tomar
@ 2023-08-15 15:27 ` Amit Singh Tomar
  2023-08-15 15:27 ` [RFC 09/12] fs/resctrl: Extend schemata write for priority partition control Amit Singh Tomar
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 39+ messages in thread
From: Amit Singh Tomar @ 2023-08-15 15:27 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: fenghua.yu, reinette.chatre, james.morse, gcherian, robh,
	peternewman, Amit Singh Tomar

At present, schemata write (for Cache Portion Bit Map or Memory Bandwidth
Allocation) goes through list of all the resources (supported by schemata),
and regardless of write for one resource, it attempts to write for all other
resources.

As an example, When the intended write is for only MBA resource, it
attempts (by calling resctrl_arch_update_domains) to write for CPBM
as well. Fix it, by doing schemata write based on input schemata supplied
from user.

Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
---
 fs/resctrl/ctrlmondata.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
index 8c8a4d09d22c..ffeb68270968 100644
--- a/fs/resctrl/ctrlmondata.c
+++ b/fs/resctrl/ctrlmondata.c
@@ -354,9 +354,11 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
 		if (is_mba_sc(r))
 			continue;
 
-		ret = resctrl_arch_update_domains(r, rdtgrp->closid);
-		if (ret)
-			goto out;
+		if (!strcmp(resname, s->name)) {
+			ret = resctrl_arch_update_domains(r, rdtgrp->closid);
+			if (ret)
+				goto out;
+		}
 	}
 
 	if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP) {
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [RFC 09/12] fs/resctrl: Extend schemata write for priority partition control
  2023-08-15 15:27 [RFC 00/12] ARM: MPAM: add support for priority partitioning control Amit Singh Tomar
                   ` (7 preceding siblings ...)
  2023-08-15 15:27 ` [RFC 08/12] fs/resctrl: Schemata write only for intended resource Amit Singh Tomar
@ 2023-08-15 15:27 ` Amit Singh Tomar
  2023-08-17 17:27   ` Fenghua Yu
  2023-08-17 17:53   ` Fenghua Yu
  2023-08-15 15:27 ` [RFC 10/12] arm_mpam: resctrl: Facilitate writing downstream priority value Amit Singh Tomar
                   ` (6 subsequent siblings)
  15 siblings, 2 replies; 39+ messages in thread
From: Amit Singh Tomar @ 2023-08-15 15:27 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: fenghua.yu, reinette.chatre, james.morse, gcherian, robh,
	peternewman, Amit Singh Tomar

Currently, Users can pass the configurations for CPBM, and MBA through
schemata file. For instance, CPBM can be passed using:

echo L3:0=ffff > schemata

This change allows, users to pass a new configuration for downstream
priority along with CPBM. For instance, dspri value of "0xa" can be
passed as:

echo L3:0=ffff,a > schemata

Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
---
 fs/resctrl/ctrlmondata.c | 92 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
index ffeb68270968..b444adee2002 100644
--- a/fs/resctrl/ctrlmondata.c
+++ b/fs/resctrl/ctrlmondata.c
@@ -30,6 +30,74 @@ typedef int (ctrlval_parser_t)(struct rdt_parse_data *data,
 			       struct resctrl_schema *s,
 			       struct rdt_domain *d);
 
+static bool dspri_validate(char *buf, unsigned long *data, struct rdt_resource *r)
+{
+
+	char *dup_buf, *dspri_token;
+	unsigned long dspri_val;
+	bool success = true;
+	int ret;
+
+	dup_buf = kstrdup(buf, GFP_KERNEL);
+	if (!dup_buf) {
+		rdt_last_cmd_printf("Failed to allocate buffer %s\n",
+					__func__);
+		success = false;
+		goto out;
+	}
+
+	strsep(&dup_buf, ",");
+	if (!dup_buf) {
+		rdt_last_cmd_printf("Unable to find priority value token %s\n",
+					__func__);
+		success = false;
+		goto out;
+	}
+
+	dspri_token = strsep(&dup_buf, ",");
+	ret = kstrtoul(dspri_token, 16, &dspri_val);
+	if (ret) {
+		rdt_last_cmd_printf("Non-hex character in the mask %s\n", buf);
+		success = false;
+		goto out;
+	}
+
+	if (dspri_val > r->dspri_default_ctrl) {
+		rdt_last_cmd_printf("dspri value %ld out of range [%d-%d]\n", dspri_val,
+					0, r->dspri_default_ctrl);
+		success = false;
+		goto out;
+	}
+
+	*data = dspri_val;
+
+out:
+	kfree(dup_buf);
+	return success;
+}
+
+static int parse_dspri(struct rdt_parse_data *data, struct resctrl_schema *s,
+			struct rdt_domain *d)
+{
+	struct resctrl_staged_config *cfg;
+	struct rdt_resource *r = s->res;
+	unsigned long pri_val;
+
+	cfg = &d->staged_config[s->conf_type];
+	if (cfg->have_new_ctrl) {
+		rdt_last_cmd_printf("Duplicate domain %d\n", d->id);
+		return -EINVAL;
+	}
+
+	if (!dspri_validate(data->buf, &pri_val, r))
+		return -EINVAL;
+
+	cfg->new_ctrl = pri_val;
+	cfg->have_new_ctrl = true;
+
+	return 0;
+}
+
 /*
  * Check whether MBA bandwidth percentage value is correct. The value is
  * checked against the minimum and max bandwidth values specified by the
@@ -293,6 +361,8 @@ static int rdtgroup_parse_resource(char *resname, char *tok,
 ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
 				char *buf, size_t nbytes, loff_t off)
 {
+	char *dup_buf = kstrdup(buf, GFP_KERNEL);
+	struct rdt_parse_data data;
 	struct resctrl_schema *s;
 	struct rdtgroup *rdtgrp;
 	struct rdt_domain *dom;
@@ -354,10 +424,32 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
 		if (is_mba_sc(r))
 			continue;
 
+		if (r->priority_cap)
+			r->dspri_store = false;
+
 		if (!strcmp(resname, s->name)) {
 			ret = resctrl_arch_update_domains(r, rdtgrp->closid);
 			if (ret)
 				goto out;
+
+			if (r->priority_cap) {
+				r->dspri_store = true;
+				list_for_each_entry(dom, &r->domains, list) {
+					ctrlval_parser_t *parse_ctrlval = &parse_dspri;
+					char *dom_data = NULL;
+
+					dom_data = strsep(&dup_buf, ";");
+					dom_data = strim(dom_data);
+					data.buf = dom_data;
+					data.rdtgrp = rdtgrp;
+					if (parse_ctrlval(&data, s, dom))
+						return -EINVAL;
+				}
+
+				ret = resctrl_arch_update_domains(r, rdtgrp->closid);
+				if (ret)
+					goto out;
+			}
 		}
 	}
 
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [RFC 10/12] arm_mpam: resctrl: Facilitate writing downstream priority value
  2023-08-15 15:27 [RFC 00/12] ARM: MPAM: add support for priority partitioning control Amit Singh Tomar
                   ` (8 preceding siblings ...)
  2023-08-15 15:27 ` [RFC 09/12] fs/resctrl: Extend schemata write for priority partition control Amit Singh Tomar
@ 2023-08-15 15:27 ` Amit Singh Tomar
  2023-08-15 15:27 ` [RFC 11/12] arm_mpam: Fix Downstream priority mask Amit Singh Tomar
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 39+ messages in thread
From: Amit Singh Tomar @ 2023-08-15 15:27 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: fenghua.yu, reinette.chatre, james.morse, gcherian, robh,
	peternewman, Amit Singh Tomar

This change provides a way to write downstream priority value (passed from
schemata file) to arch specific resource control code. The priority value
is stored in mpam_config structure, and eventually gets programmed into
MPAMCFG_PRI register.

Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
---
 drivers/platform/mpam/mpam_devices.c |  4 ----
 drivers/platform/mpam/mpam_resctrl.c | 27 +++++++++++++++++----------
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/platform/mpam/mpam_devices.c b/drivers/platform/mpam/mpam_devices.c
index 137cbff925ba..c0c83c04c77c 100644
--- a/drivers/platform/mpam/mpam_devices.c
+++ b/drivers/platform/mpam/mpam_devices.c
@@ -2351,10 +2351,6 @@ int mpam_apply_config(struct mpam_component *comp, u16 partid,
 
 	lockdep_assert_cpus_held();
 
-	if (!memcmp(&comp->cfg[partid], cfg, sizeof(*cfg)))
-		return 0;
-
-	comp->cfg[partid] = *cfg;
 	arg.comp = comp;
 	arg.partid = partid;
 
diff --git a/drivers/platform/mpam/mpam_resctrl.c b/drivers/platform/mpam/mpam_resctrl.c
index dc710c90cfc2..ffd0fa84d37d 100644
--- a/drivers/platform/mpam/mpam_resctrl.c
+++ b/drivers/platform/mpam/mpam_resctrl.c
@@ -919,7 +919,7 @@ int resctrl_arch_update_one(struct rdt_resource *r, struct rdt_domain *d,
 {
 	int err;
 	u32 partid;
-	struct mpam_config cfg;
+	struct mpam_config *cfg;
 	struct mpam_props *cprops;
 	struct mpam_resctrl_res *res;
 	struct mpam_resctrl_dom *dom;
@@ -934,6 +934,7 @@ int resctrl_arch_update_one(struct rdt_resource *r, struct rdt_domain *d,
 	cprops = &res->class->props;
 
 	partid = resctrl_get_config_index(closid, t);
+	cfg = &dom->comp->cfg[partid];
 	if (!r->alloc_capable || partid >= resctrl_arch_get_num_closid(r))
 		return -EINVAL;
 
@@ -941,17 +942,22 @@ int resctrl_arch_update_one(struct rdt_resource *r, struct rdt_domain *d,
 	case RDT_RESOURCE_L2:
 	case RDT_RESOURCE_L3:
 		/* TODO: Scaling is not yet supported */
-		cfg.cpbm = cfg_val;
-		mpam_set_feature(mpam_feat_cpor_part, &cfg);
+		if (r->dspri_store) {
+			cfg->dspri = cfg_val;
+			mpam_set_feature(mpam_feat_dspri_part, cfg);
+		} else {
+			cfg->cpbm = cfg_val;
+			mpam_set_feature(mpam_feat_cpor_part, cfg);
+		}
 		break;
 	case RDT_RESOURCE_MBA:
 		if (mba_class_use_mbw_part(cprops)) {
-			cfg.mbw_pbm = percent_to_mbw_pbm(cfg_val, cprops);
-			mpam_set_feature(mpam_feat_mbw_part, &cfg);
+			cfg->mbw_pbm = percent_to_mbw_pbm(cfg_val, cprops);
+			mpam_set_feature(mpam_feat_mbw_part, cfg);
 			break;
 		} else if (mpam_has_feature(mpam_feat_mbw_max, cprops)) {
-			cfg.mbw_max = percent_to_mbw_max(cfg_val, cprops);
-			mpam_set_feature(mpam_feat_mbw_max, &cfg);
+			cfg->mbw_max = percent_to_mbw_max(cfg_val, cprops);
+			mpam_set_feature(mpam_feat_mbw_max, cfg);
 			break;
 		}
 		fallthrough;
@@ -965,15 +971,15 @@ int resctrl_arch_update_one(struct rdt_resource *r, struct rdt_domain *d,
 	 */
 	if (mpam_resctrl_hide_cdp(r->rid)) {
 		partid = resctrl_get_config_index(closid, CDP_CODE);
-		err = mpam_apply_config(dom->comp, partid, &cfg);
+		err = mpam_apply_config(dom->comp, partid, cfg);
 		if (err)
 			return err;
 
 		partid = resctrl_get_config_index(closid, CDP_DATA);
-		return mpam_apply_config(dom->comp, partid, &cfg);
+		return mpam_apply_config(dom->comp, partid, cfg);
 
 	} else {
-		return mpam_apply_config(dom->comp, partid, &cfg);
+		return mpam_apply_config(dom->comp, partid, cfg);
 	}
 }
 
@@ -998,6 +1004,7 @@ int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid)
 						      cfg->new_ctrl);
 			if (err)
 				return err;
+			cfg->have_new_ctrl = false;
 		}
 	}
 
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [RFC 11/12] arm_mpam: Fix Downstream priority mask
  2023-08-15 15:27 [RFC 00/12] ARM: MPAM: add support for priority partitioning control Amit Singh Tomar
                   ` (9 preceding siblings ...)
  2023-08-15 15:27 ` [RFC 10/12] arm_mpam: resctrl: Facilitate writing downstream priority value Amit Singh Tomar
@ 2023-08-15 15:27 ` Amit Singh Tomar
  2023-09-01 13:32   ` Jonathan Cameron
  2023-08-15 15:27 ` [RFC 12/12] arm_mpam: Program Downstream priority value Amit Singh Tomar
                   ` (4 subsequent siblings)
  15 siblings, 1 reply; 39+ messages in thread
From: Amit Singh Tomar @ 2023-08-15 15:27 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: fenghua.yu, reinette.chatre, james.morse, gcherian, robh,
	peternewman, Amit Singh Tomar

MPAMF_PRI_IDR_NS[DSPRI_WD] determines the number of implemented bits in
the downstream priority field (MPAMCFG_PRI_NS). For instance, if the value
of DSPRI_WD is 4, then the maximum value for dspri is 0xf, and mask should
be GENMASK(3,0).

But with current implementation, it turned out to be GENMASK(4,0) .i.e.
0x1f instead of 0xf.

u16 dspri = GENMASK(rprops->dspri_wd, 0);

Let's fix it, by subtracting 1 from DSPRI_WD value.

Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
---
 drivers/platform/mpam/mpam_devices.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/mpam/mpam_devices.c b/drivers/platform/mpam/mpam_devices.c
index c0c83c04c77c..59022e42920c 100644
--- a/drivers/platform/mpam/mpam_devices.c
+++ b/drivers/platform/mpam/mpam_devices.c
@@ -1099,7 +1099,7 @@ static void mpam_reprogram_ris_partid(struct mpam_msc_ris *ris, u16 partid,
 	struct mpam_msc *msc = ris->msc;
 	u16 bwa_fract = MPAMCFG_MBW_MAX_MAX;
 	struct mpam_props *rprops = &ris->props;
-	u16 dspri = GENMASK(rprops->dspri_wd, 0);
+	u16 dspri = GENMASK((rprops->dspri_wd-1), 0);
 	u16 intpri = GENMASK(rprops->intpri_wd, 0);
 
 	lockdep_assert_held(&msc->lock);
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [RFC 12/12] arm_mpam: Program Downstream priority value
  2023-08-15 15:27 [RFC 00/12] ARM: MPAM: add support for priority partitioning control Amit Singh Tomar
                   ` (10 preceding siblings ...)
  2023-08-15 15:27 ` [RFC 11/12] arm_mpam: Fix Downstream priority mask Amit Singh Tomar
@ 2023-08-15 15:27 ` Amit Singh Tomar
  2023-09-01 13:17   ` Jonathan Cameron
  2023-08-17 19:11 ` [RFC 00/12] ARM: MPAM: add support for priority partitioning control Reinette Chatre
                   ` (3 subsequent siblings)
  15 siblings, 1 reply; 39+ messages in thread
From: Amit Singh Tomar @ 2023-08-15 15:27 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: fenghua.yu, reinette.chatre, james.morse, gcherian, robh,
	peternewman, Amit Singh Tomar

Now that Downstream priorities values can be passed from resource control
schemata file, let's program it into memory mapped Priority Partition
Configuration Register.

Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
---
TODO:
	* Invert Priority value based on DSPRI_0_IS_LOW, suggested
          by James. 
---
 drivers/platform/mpam/mpam_devices.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/mpam/mpam_devices.c b/drivers/platform/mpam/mpam_devices.c
index 59022e42920c..8af6424bb27b 100644
--- a/drivers/platform/mpam/mpam_devices.c
+++ b/drivers/platform/mpam/mpam_devices.c
@@ -1153,8 +1153,12 @@ static void mpam_reprogram_ris_partid(struct mpam_msc_ris *ris, u16 partid,
 
 		if (mpam_has_feature(mpam_feat_intpri_part, rprops))
 			pri_val |= FIELD_PREP(MPAMCFG_PRI_INTPRI, intpri);
-		if (mpam_has_feature(mpam_feat_dspri_part, rprops))
-			pri_val |= FIELD_PREP(MPAMCFG_PRI_DSPRI, dspri);
+		if (mpam_has_feature(mpam_feat_dspri_part, rprops)) {
+			if (mpam_has_feature(mpam_feat_dspri_part, cfg)) {
+				pri_val |= FIELD_PREP(MPAMCFG_PRI_DSPRI, cfg->dspri & dspri);
+			} else
+				pri_val |= FIELD_PREP(MPAMCFG_PRI_DSPRI, dspri);
+		}
 
 		mpam_write_partsel_reg(msc, PRI, pri_val);
 	}
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [RFC 09/12] fs/resctrl: Extend schemata write for priority partition control
  2023-08-15 15:27 ` [RFC 09/12] fs/resctrl: Extend schemata write for priority partition control Amit Singh Tomar
@ 2023-08-17 17:27   ` Fenghua Yu
  2023-08-17 17:53   ` Fenghua Yu
  1 sibling, 0 replies; 39+ messages in thread
From: Fenghua Yu @ 2023-08-17 17:27 UTC (permalink / raw)
  To: Amit Singh Tomar, linux-kernel, linux-arm-kernel
  Cc: reinette.chatre, james.morse, gcherian, robh, peternewman

Hi, Amit,

On 8/15/23 08:27, Amit Singh Tomar wrote:
> Currently, Users can pass the configurations for CPBM, and MBA through
> schemata file. For instance, CPBM can be passed using:
> 
> echo L3:0=ffff > schemata
> 
> This change allows, users to pass a new configuration for downstream
> priority along with CPBM. For instance, dspri value of "0xa" can be
> passed as:
> 
> echo L3:0=ffff,a > schemata
> 
> Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
> ---
>   fs/resctrl/ctrlmondata.c | 92 ++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 92 insertions(+)
> 
> diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
> index ffeb68270968..b444adee2002 100644
> --- a/fs/resctrl/ctrlmondata.c
> +++ b/fs/resctrl/ctrlmondata.c
> @@ -30,6 +30,74 @@ typedef int (ctrlval_parser_t)(struct rdt_parse_data *data,
>   			       struct resctrl_schema *s,
>   			       struct rdt_domain *d);
>   
> +static bool dspri_validate(char *buf, unsigned long *data, struct rdt_resource *r)
> +{
> +
> +	char *dup_buf, *dspri_token;
> +	unsigned long dspri_val;
> +	bool success = true;
> +	int ret;
> +
> +	dup_buf = kstrdup(buf, GFP_KERNEL);
> +	if (!dup_buf) {
> +		rdt_last_cmd_printf("Failed to allocate buffer %s\n",
> +					__func__);
> +		success = false;
> +		goto out;
> +	}
> +
> +	strsep(&dup_buf, ",");
> +	if (!dup_buf) {
> +		rdt_last_cmd_printf("Unable to find priority value token %s\n",
> +					__func__);
> +		success = false;
> +		goto out;
> +	}
> +
> +	dspri_token = strsep(&dup_buf, ",");
> +	ret = kstrtoul(dspri_token, 16, &dspri_val);
> +	if (ret) {
> +		rdt_last_cmd_printf("Non-hex character in the mask %s\n", buf);
> +		success = false;
> +		goto out;
> +	}
> +
> +	if (dspri_val > r->dspri_default_ctrl) {
> +		rdt_last_cmd_printf("dspri value %ld out of range [%d-%d]\n", dspri_val,
> +					0, r->dspri_default_ctrl);
> +		success = false;
> +		goto out;
> +	}
> +
> +	*data = dspri_val;
> +
> +out:
> +	kfree(dup_buf);
> +	return success;
> +}
> +
> +static int parse_dspri(struct rdt_parse_data *data, struct resctrl_schema *s,
> +			struct rdt_domain *d)
> +{
> +	struct resctrl_staged_config *cfg;
> +	struct rdt_resource *r = s->res;
> +	unsigned long pri_val;
> +
> +	cfg = &d->staged_config[s->conf_type];
> +	if (cfg->have_new_ctrl) {
> +		rdt_last_cmd_printf("Duplicate domain %d\n", d->id);
> +		return -EINVAL;
> +	}
> +
> +	if (!dspri_validate(data->buf, &pri_val, r))
> +		return -EINVAL;
> +
> +	cfg->new_ctrl = pri_val;
> +	cfg->have_new_ctrl = true;
> +
> +	return 0;
> +}
> +
>   /*
>    * Check whether MBA bandwidth percentage value is correct. The value is
>    * checked against the minimum and max bandwidth values specified by the
> @@ -293,6 +361,8 @@ static int rdtgroup_parse_resource(char *resname, char *tok,
>   ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
>   				char *buf, size_t nbytes, loff_t off)
>   {
> +	char *dup_buf = kstrdup(buf, GFP_KERNEL);
> +	struct rdt_parse_data data;
>   	struct resctrl_schema *s;
>   	struct rdtgroup *rdtgrp;
>   	struct rdt_domain *dom;
> @@ -354,10 +424,32 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
>   		if (is_mba_sc(r))
>   			continue;
>   
> +		if (r->priority_cap)
> +			r->dspri_store = false;
> +
>   		if (!strcmp(resname, s->name)) {
>   			ret = resctrl_arch_update_domains(r, rdtgrp->closid);
>   			if (ret)
>   				goto out;
> +
> +			if (r->priority_cap) {
> +				r->dspri_store = true;
> +				list_for_each_entry(dom, &r->domains, list) {
> +					ctrlval_parser_t *parse_ctrlval = &parse_dspri;
> +					char *dom_data = NULL;
> +
> +					dom_data = strsep(&dup_buf, ";");
> +					dom_data = strim(dom_data);
> +					data.buf = dom_data;
> +					data.rdtgrp = rdtgrp;
> +					if (parse_ctrlval(&data, s, dom))

Can this parse be called within parse_line()? So parsing and validation 
are before real write. If any parsing error, no write. Otherwise, 
partial write or roll back may happen.

> +						return -EINVAL;
> +				}
> +
> +				ret = resctrl_arch_update_domains(r, rdtgrp->closid);
> +				if (ret)
> +					goto out;

r->dspri_store = false here?

> +			}
>   		}
>   	}
>   

Thanks.

-Fenghua

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [RFC 05/12] fs/resctrl: Set-up downstream priority partition resources
  2023-08-15 15:27 ` [RFC 05/12] fs/resctrl: Set-up downstream priority partition resources Amit Singh Tomar
@ 2023-08-17 17:39   ` Fenghua Yu
  0 siblings, 0 replies; 39+ messages in thread
From: Fenghua Yu @ 2023-08-17 17:39 UTC (permalink / raw)
  To: Amit Singh Tomar, linux-kernel, linux-arm-kernel
  Cc: reinette.chatre, james.morse, gcherian, robh, peternewman

Hi, Amit,

On 8/15/23 08:27, Amit Singh Tomar wrote:
> Upon resource control group creation, Cache portion bitmap, and Memory
> bandwidth allocation gets initialized to the default/maximum values,
> obtained from resource control code.
> 
> Let's replicate it for priority partition resource, and setup the default
> downstream priority value.
> 
> Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
> ---
>   drivers/platform/mpam/mpam_resctrl.c |  4 +++-
>   fs/resctrl/rdtgroup.c                | 30 ++++++++++++++++++++++++++++
>   include/linux/resctrl.h              |  4 ++++
>   3 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/mpam/mpam_resctrl.c b/drivers/platform/mpam/mpam_resctrl.c
> index 1081ceac41a8..cc843f1b0fb7 100644
> --- a/drivers/platform/mpam/mpam_resctrl.c
> +++ b/drivers/platform/mpam/mpam_resctrl.c
> @@ -733,8 +733,10 @@ static int mpam_resctrl_resource_init(struct mpam_resctrl_res *res)
>   			exposed_alloc_capable = true;
>   		}
>   
> -		if (has_ppart)
> +		if (has_ppart) {
>   			r->priority_cap = true;
> +			r->dspri_default_ctrl = BIT_MASK(class->props.dspri_wd) - 1;
> +		}
>   
>   		/*
>   		 * MBWU counters may be 'local' or 'total' depending on where
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> index 9c5dfaaa7fa2..bc5fb246ba68 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c
> @@ -3045,6 +3045,21 @@ static int rdtgroup_init_cat(struct resctrl_schema *s, u32 closid)
>   	return 0;
>   }
>   
> +static int rdtgroup_init_dspri(struct rdt_resource *r, u32 closid)
> +{
> +	struct resctrl_staged_config *cfg;
> +	struct rdt_domain *d;
> +
> +	list_for_each_entry(d, &r->domains, list) {
> +		cfg = &d->staged_config[CDP_NONE];
> +		cfg->new_ctrl = r->dspri_default_ctrl;
> +		cfg->have_new_ctrl = true;
> +		r->dspri_store = true;
> +	}
> +
> +	return 0;
> +}
> +
>   /* Initialize MBA resource with default values. */
>   static void rdtgroup_init_mba(struct rdt_resource *r, u32 closid)
>   {
> @@ -3082,12 +3097,27 @@ static int rdtgroup_init_alloc(struct rdtgroup *rdtgrp)
>   				return ret;
>   		}
>   
> +		if (r->priority_cap)
> +			r->dspri_store = false;
> +
>   		ret = resctrl_arch_update_domains(r, rdtgrp->closid);
>   		if (ret < 0) {
>   			rdt_last_cmd_puts("Failed to initialize allocations\n");
>   			return ret;
>   		}
>   
> +		if (r->priority_cap) {
> +			ret = rdtgroup_init_dspri(r, rdtgrp->closid);
> +			if (ret < 0)
> +				return ret;
> +
> +			ret = resctrl_arch_update_domains(r, rdtgrp->closid);
> +			if (ret < 0) {
> +				rdt_last_cmd_puts("Failed to initialize allocations\n");
> +				return ret;
> +			}
> +			r->dspri_store = false;
> +		}
>   	}
>   
>   	rdtgrp->mode = RDT_MODE_SHAREABLE;
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index a98ba5828211..d5b0661c0f70 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -198,6 +198,7 @@ struct resctrl_membw {
>    * @alloc_capable:	Is allocation available on this machine
>    * @mon_capable:	Is monitor feature available on this machine
>    * @priority_capable:   Is priority partitioning feature available on this machine
> + * @dspri_store:

Please add description for this new field.

>    * @num_rmid:		Number of RMIDs available
>    * @cache_level:	Which cache level defines scope of this resource
>    * @cache:		Cache allocation related data
> @@ -206,6 +207,7 @@ struct resctrl_membw {
>    * @name:		Name to use in "schemata" file.
>    * @data_width:		Character width of data when displaying
>    * @default_ctrl:	Specifies default cache cbm or memory B/W percent.
> + * @dspri_default_ctrl: Specifies default downstream priority value.
>    * @format_str:		Per resource format string to show domain value
>    * @evt_list:		List of monitoring events
>    * @fflags:		flags to choose base and info files
> @@ -216,6 +218,7 @@ struct rdt_resource {
>   	bool			alloc_capable;
>   	bool			mon_capable;
>   	bool                    priority_cap;
> +	bool			dspri_store;
>   	int			num_rmid;
>   	int			cache_level;
>   	struct resctrl_cache	cache;
> @@ -224,6 +227,7 @@ struct rdt_resource {
>   	char			*name;
>   	int			data_width;
>   	u32			default_ctrl;
> +	u32			dspri_default_ctrl;
>   	const char		*format_str;
>   	struct list_head	evt_list;
>   	unsigned long		fflags;

Thanks.

-Fenghua

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [RFC 06/12] fs/resctrl: Extend schemata read for priority partition control
  2023-08-15 15:27 ` [RFC 06/12] fs/resctrl: Extend schemata read for priority partition control Amit Singh Tomar
@ 2023-08-17 17:42   ` Fenghua Yu
  0 siblings, 0 replies; 39+ messages in thread
From: Fenghua Yu @ 2023-08-17 17:42 UTC (permalink / raw)
  To: Amit Singh Tomar, linux-kernel, linux-arm-kernel
  Cc: reinette.chatre, james.morse, gcherian, robh, peternewman

Hi, Amit,

On 8/15/23 08:27, Amit Singh Tomar wrote:
> At present, "schemata" file under resource control group reveals
> information about Cache portion bitmap and Memory Bandwidth allocation.
> With the introduction of priority partition control "schemata" is updated
> to adopt priority value.
> 
> Let's enable support for reading the priority values for "schemata" file.
> 
> Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
> ---
>   drivers/platform/mpam/mpam_resctrl.c |  4 ++++
>   fs/resctrl/ctrlmondata.c             | 15 ++++++++++++---
>   include/linux/resctrl.h              |  4 ++++
>   3 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/mpam/mpam_resctrl.c b/drivers/platform/mpam/mpam_resctrl.c
> index cc843f1b0fb7..b491a0f897fd 100644
> --- a/drivers/platform/mpam/mpam_resctrl.c
> +++ b/drivers/platform/mpam/mpam_resctrl.c
> @@ -735,7 +735,11 @@ static int mpam_resctrl_resource_init(struct mpam_resctrl_res *res)
>   
>   		if (has_ppart) {
>   			r->priority_cap = true;
> +			if (class->props.dspri_wd > 0x10)
> +				class->props.dspri_wd = 0x10;
> +
>   			r->dspri_default_ctrl = BIT_MASK(class->props.dspri_wd) - 1;
> +			r->dspri_data_width = (class->props.dspri_wd + 3) / 4;
>   		}
>   
>   		/*
> diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
> index b19ac2509e38..8c8a4d09d22c 100644
> --- a/fs/resctrl/ctrlmondata.c
> +++ b/fs/resctrl/ctrlmondata.c
> @@ -379,7 +379,7 @@ static void show_doms(struct seq_file *s, struct resctrl_schema *schema, int clo
>   	struct rdt_resource *r = schema->res;
>   	struct rdt_domain *dom;
>   	bool sep = false;
> -	u32 ctrl_val;
> +	u32 ctrl_val, dspri_ctrl_val;
>   
>   	/* Walking r->domains, ensure it can't race with cpuhp */
>   	lockdep_assert_cpus_held();
> @@ -395,9 +395,18 @@ static void show_doms(struct seq_file *s, struct resctrl_schema *schema, int clo
>   			ctrl_val = resctrl_arch_get_config(r, dom, closid,
>   							   schema->conf_type);
>   
> -		seq_printf(s, r->format_str, dom->id, max_data_width,
> -			   ctrl_val);
> +		if (r->priority_cap) {
> +			r->dspri_show = true;
> +			dspri_ctrl_val = resctrl_arch_get_config(r, dom, closid,
> +								 CDP_NONE);
> +			seq_printf(s, r->format_str, dom->id, max_data_width, ctrl_val,
> +					r->dspri_data_width, dspri_ctrl_val);

Move r->dspri_show = false to here.

> +		} else
> +			seq_printf(s, r->format_str, dom->id, max_data_width,
> +				   ctrl_val);
> +
>   		sep = true;
> +		r->dspri_show = false;

dspri_show is irrelevant without priority_cap.

>   	}
>   	seq_puts(s, "\n");
>   }
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index d5b0661c0f70..d7100c330945 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -199,12 +199,14 @@ struct resctrl_membw {
>    * @mon_capable:	Is monitor feature available on this machine
>    * @priority_capable:   Is priority partitioning feature available on this machine
>    * @dspri_store:
> + * @dspri_show:		flag to indicate downstream priority read
>    * @num_rmid:		Number of RMIDs available
>    * @cache_level:	Which cache level defines scope of this resource
>    * @cache:		Cache allocation related data
>    * @membw:		If the component has bandwidth controls, their properties.
>    * @domains:		RCU list of all domains for this resource
>    * @name:		Name to use in "schemata" file.
> + * @dspri_data_width    Character width of dspri value when displaying
>    * @data_width:		Character width of data when displaying
>    * @default_ctrl:	Specifies default cache cbm or memory B/W percent.
>    * @dspri_default_ctrl: Specifies default downstream priority value.
> @@ -219,6 +221,7 @@ struct rdt_resource {
>   	bool			mon_capable;
>   	bool                    priority_cap;
>   	bool			dspri_store;
> +	bool                    dspri_show;
>   	int			num_rmid;
>   	int			cache_level;
>   	struct resctrl_cache	cache;
> @@ -226,6 +229,7 @@ struct rdt_resource {
>   	struct list_head	domains;
>   	char			*name;
>   	int			data_width;
> +	int			dspri_data_width;
>   	u32			default_ctrl;
>   	u32			dspri_default_ctrl;
>   	const char		*format_str;

Thanks.

-Fenghua

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [RFC 09/12] fs/resctrl: Extend schemata write for priority partition control
  2023-08-15 15:27 ` [RFC 09/12] fs/resctrl: Extend schemata write for priority partition control Amit Singh Tomar
  2023-08-17 17:27   ` Fenghua Yu
@ 2023-08-17 17:53   ` Fenghua Yu
  1 sibling, 0 replies; 39+ messages in thread
From: Fenghua Yu @ 2023-08-17 17:53 UTC (permalink / raw)
  To: Amit Singh Tomar, linux-kernel, linux-arm-kernel
  Cc: reinette.chatre, james.morse, gcherian, robh, peternewman

Hi, Amit,

On 8/15/23 08:27, Amit Singh Tomar wrote:
> Currently, Users can pass the configurations for CPBM, and MBA through
> schemata file. For instance, CPBM can be passed using:
> 
> echo L3:0=ffff > schemata
> 
> This change allows, users to pass a new configuration for downstream
> priority along with CPBM. For instance, dspri value of "0xa" can be
> passed as:
> 
> echo L3:0=ffff,a > schemata
> 
> Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
> ---
>   fs/resctrl/ctrlmondata.c | 92 ++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 92 insertions(+)
> 
> diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
> index ffeb68270968..b444adee2002 100644
> --- a/fs/resctrl/ctrlmondata.c
> +++ b/fs/resctrl/ctrlmondata.c
> @@ -30,6 +30,74 @@ typedef int (ctrlval_parser_t)(struct rdt_parse_data *data,
>   			       struct resctrl_schema *s,
>   			       struct rdt_domain *d);
>   
> +static bool dspri_validate(char *buf, unsigned long *data, struct rdt_resource *r)
> +{
> +
> +	char *dup_buf, *dspri_token;
> +	unsigned long dspri_val;
> +	bool success = true;
> +	int ret;
> +
> +	dup_buf = kstrdup(buf, GFP_KERNEL);
> +	if (!dup_buf) {
> +		rdt_last_cmd_printf("Failed to allocate buffer %s\n",
> +					__func__);
> +		success = false;
> +		goto out;
> +	}
> +
> +	strsep(&dup_buf, ",");
> +	if (!dup_buf) {
> +		rdt_last_cmd_printf("Unable to find priority value token %s\n",
> +					__func__);
> +		success = false;
> +		goto out;
> +	}
> +
> +	dspri_token = strsep(&dup_buf, ",");
> +	ret = kstrtoul(dspri_token, 16, &dspri_val);
> +	if (ret) {
> +		rdt_last_cmd_printf("Non-hex character in the mask %s\n", buf);
> +		success = false;
> +		goto out;
> +	}
> +
> +	if (dspri_val > r->dspri_default_ctrl) {
> +		rdt_last_cmd_printf("dspri value %ld out of range [%d-%d]\n", dspri_val,
> +					0, r->dspri_default_ctrl);
> +		success = false;
> +		goto out;
> +	}
> +
> +	*data = dspri_val;
> +
> +out:
> +	kfree(dup_buf);
> +	return success;
> +}
> +
> +static int parse_dspri(struct rdt_parse_data *data, struct resctrl_schema *s,
> +			struct rdt_domain *d)
> +{
> +	struct resctrl_staged_config *cfg;
> +	struct rdt_resource *r = s->res;
> +	unsigned long pri_val;
> +
> +	cfg = &d->staged_config[s->conf_type];
> +	if (cfg->have_new_ctrl) {
> +		rdt_last_cmd_printf("Duplicate domain %d\n", d->id);
> +		return -EINVAL;
> +	}
> +
> +	if (!dspri_validate(data->buf, &pri_val, r))
> +		return -EINVAL;
> +
> +	cfg->new_ctrl = pri_val;
> +	cfg->have_new_ctrl = true;
> +
> +	return 0;
> +}
> +
>   /*
>    * Check whether MBA bandwidth percentage value is correct. The value is
>    * checked against the minimum and max bandwidth values specified by the
> @@ -293,6 +361,8 @@ static int rdtgroup_parse_resource(char *resname, char *tok,
>   ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
>   				char *buf, size_t nbytes, loff_t off)
>   {
> +	char *dup_buf = kstrdup(buf, GFP_KERNEL);

kstrdup() may fail. Need to check NULL for failure.

dup_buf needs to be freed. I don't see it's freed.

> +	struct rdt_parse_data data;
>   	struct resctrl_schema *s;
>   	struct rdtgroup *rdtgrp;
>   	struct rdt_domain *dom;
> @@ -354,10 +424,32 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
>   		if (is_mba_sc(r))
>   			continue;
>   
> +		if (r->priority_cap)
> +			r->dspri_store = false;
> +
>   		if (!strcmp(resname, s->name)) {
>   			ret = resctrl_arch_update_domains(r, rdtgrp->closid);
>   			if (ret)
>   				goto out;
> +
> +			if (r->priority_cap) {
> +				r->dspri_store = true;
> +				list_for_each_entry(dom, &r->domains, list) {
> +					ctrlval_parser_t *parse_ctrlval = &parse_dspri;
> +					char *dom_data = NULL;
> +
> +					dom_data = strsep(&dup_buf, ";");
> +					dom_data = strim(dom_data);
> +					data.buf = dom_data;
> +					data.rdtgrp = rdtgrp;
> +					if (parse_ctrlval(&data, s, dom))
> +						return -EINVAL;
> +				}
> +
> +				ret = resctrl_arch_update_domains(r, rdtgrp->closid);
> +				if (ret)
> +					goto out;
> +			}
>   		}
>   	}
>   

Thanks.

-Fenghua

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
  2023-08-15 15:27 [RFC 00/12] ARM: MPAM: add support for priority partitioning control Amit Singh Tomar
                   ` (11 preceding siblings ...)
  2023-08-15 15:27 ` [RFC 12/12] arm_mpam: Program Downstream priority value Amit Singh Tomar
@ 2023-08-17 19:11 ` Reinette Chatre
  2023-08-17 20:29   ` Reinette Chatre
  2023-08-22 12:44   ` [EXT] " Amit Singh Tomar
  2023-08-22  9:01 ` Peter Newman
                   ` (2 subsequent siblings)
  15 siblings, 2 replies; 39+ messages in thread
From: Reinette Chatre @ 2023-08-17 19:11 UTC (permalink / raw)
  To: Amit Singh Tomar, linux-kernel, linux-arm-kernel
  Cc: fenghua.yu, james.morse, gcherian, robh, peternewman, Luck, Tony

(+Tony)

Hi Amit,

On 8/15/2023 8:27 AM, Amit Singh Tomar wrote:
> Arm Memory System Resource Partitioning and Monitoring (MPAM) supports
> different controls that can be applied to different resources in the system
> For instance, an optional priority partitioning control where priority
> value is generated from one MSC, propagates over interconnect to other MSC
> (known as downstream priority), or can be applied within an MSC for internal
> operations.
> 
> Marvell implementation of ARM MPAM supports priority partitioning control
> that allows LLC MSC to generate priority values that gets propagated (along with
> read/write request from upstream) to DDR Block. Within the DDR block the
> priority values is mapped to different traffic class under DDR QoS strategy.
> The link[1] gives some idea about DDR QoS strategy, and terms like LPR, VPR
> and HPR.
> 
> Setup priority partitioning control under Resource control
> ----------------------------------------------------------
> At present, resource control (resctrl) provides basic interface to configure/set-up
> CAT (Cache Allocation Technology) and MBA (Memory Bandwidth Allocation) capabilities.
> ARM MPAM uses it to support controls like Cache portion partition (CPOR), and 
> MPAM bandwidth partitioning.
> 
> As an example, "schemata" file under resource control group contains information about
> cache portion bitmaps, and memory bandwidth allocation, and these are used to configure
> Cache portion partition (CPOR), and MPAM bandwidth partitioning controls.
> 
> MB:0=0100
> L3:0=ffff
> 
> But resctrl doesn't provide a way to set-up other control that ARM MPAM provides
> (For instance, Priority partitioning control as mentioned above). To support this,
> James has suggested to use already existing schemata to be compatible with 
> portable software, and this is the main idea behind this RFC is to have some kind
> of discussion on how resctrl can be extended to support priority partitioning control.
> 
> To support Priority partitioning control, "schemata" file is updated to accommodate
> priority field (upon priority partitioning capability detection), separated from CPBM
> using delimiter ",".
> 
> L3:0=ffff,f where f indicates downstream priority max value.
> 
> These dspri value gets programmed per partition, that can be used to override 
> QoS value coming from upstream (CPU).
> 
> RFC patch-set[2] is based on James Morse's MPAM snapshot[3] for 6.2, and ACPI
> table is based on DEN0065A_MPAM_ACPI_2.0.
>

There are some aspects of this that I think we should be cautious about. First,
there may inevitably be more properties in the future that need to be associated with
a resource allocation, these may indeed be different between architectures
and individual platforms. Second, user space need a way to know which properties
are supported and what valid parameters may be. 

On a high level I thus understand the goal be to add support for assigning a
property to a resource allocation with "Priority partitioning control" being
the first property.

To that end, I have a few questions:
* How can this interface be expanded to support more properties with the
  expectation that a system/architecture may not support all resctrl supported
  properties?
* Is it possible for support for properties to vary between, for example, different
  MSCs in the system? From resctrl side it may mean that there would be a resource,
  for example "L3", with multiple instances, for example, cache with id #0, cache
  with id#1, etc. but the supported properties or valid values of properties
  may vary between the instances?
* How can user space know that a system supports "Priority partitioning control"?
  User space needs to know when/if it can attempt to write a priority to the
  schemata.
* How can user space know what priority values are valid for a particular system?

> Test set-up and results:
> ------------------------
> 
> The downstream priority value feeds into DRAM controller, and one of the important
> thing that it does with this value is to service the requests sooner (based on the 
> traffic class), hence reducing latency without affecting performance.

Could you please elaborate here? I expected reduced latency to have a big impact
on performance.

> 
> Within the DDR QoS traffic class.
> 
> 0--5 ----> Low priority value
> 6-10 ----> Medium priority value
> 11-15 ----> High priority value
> 
> Benchmark[4] used is multichase.
> 
> Two partition P1 and P2:
> 
> Partition P1:
> -------------
> Assigned core 0
> 100% BW assignment
> 
> Partition P2:
> -------------
> Assigned cores 1-79
> 100% BW assignment
> 
> Test Script:
> -----------
> mkdir p1
> cd p1
> echo 1 > cpus
> echo L3:1=8000,5 > schemata   ##### DSPRI set as 5 (lpr)
> echo "MB:0=100" > schemata
> 
> mkdir p2
> cd p2
> echo ffff,ffffffff,fffffffe > cpus
> echo L3:1=8000,0 > schemata
> echo "MB:0=100" > schemata
> 
> ### Loaded latency run, core 0 does chaseload (pointer chase) with low priority value 5, and cores 1-79 does memory bandwidth run ###

Could you please elaborate what is meant with a "memory bandwidth run"?

> ./multiload -v -n 10 -t 80 -m 1G -c chaseload  
> 
> cd /sys/fs/resctrl/p1
> 
> echo L3:1=8000,a > schemata  ##### DSPRI set as 0xa (vpr)
> 
> ### Loaded latency run, core 0 does chaseload (pointer chase) with medium priority value a, and cores 1-79 does memory bandwidth run ###
> ./multiload -v -n 10 -t 80 -m 1G -c chaseload
> 
> cd /sys/fs/resctrl/p1
> 
> echo L3:1=8000,f > schemata  ##### DSPRI set as 0xf (hpr)
> 
> ### Loaded latency run where core 0 does chaseload (pointer chase) with high priority value f, and cores 1-79 does memory bandwidth run ###
> ./multiload -v -n 10 -t 80 -m 1G -c chaseload
> 
> Results[5]:
> 
> LPR average latency is 204.862(ns) vs VPR average latency is 161.018(ns) vs HPR average latency is 134.210(ns).

Reinette

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
  2023-08-17 19:11 ` [RFC 00/12] ARM: MPAM: add support for priority partitioning control Reinette Chatre
@ 2023-08-17 20:29   ` Reinette Chatre
  2023-08-22 12:44   ` [EXT] " Amit Singh Tomar
  1 sibling, 0 replies; 39+ messages in thread
From: Reinette Chatre @ 2023-08-17 20:29 UTC (permalink / raw)
  To: Amit Singh Tomar, linux-kernel, linux-arm-kernel
  Cc: fenghua.yu, james.morse, gcherian, robh, peternewman, Luck, Tony

Hi Amit,

On 8/17/2023 12:11 PM, Reinette Chatre wrote:
> On 8/15/2023 8:27 AM, Amit Singh Tomar wrote:

>>
>> Within the DDR QoS traffic class.
>>
>> 0--5 ----> Low priority value
>> 6-10 ----> Medium priority value
>> 11-15 ----> High priority value
>>
>> Benchmark[4] used is multichase.
>>
>> Two partition P1 and P2:
>>
>> Partition P1:
>> -------------
>> Assigned core 0
>> 100% BW assignment
>>
>> Partition P2:
>> -------------
>> Assigned cores 1-79
>> 100% BW assignment
>>
>> Test Script:
>> -----------
>> mkdir p1
>> cd p1
>> echo 1 > cpus
>> echo L3:1=8000,5 > schemata   ##### DSPRI set as 5 (lpr)
>> echo "MB:0=100" > schemata

I peeked at the next commit and I am missing something. 

It looks like indeed resource instances need to
support different controls, so that seems to answer my earlier
question. How to let user know what is supported where
remains an open, now with understanding that the information
is required to be per resource instance.

The first commit mentions that #0 has the Priority
partitioning feature but in these examples the schemata
of #1 is updated to modify the priority. Also, if I
understand correctly CPOR and priority partitioning
are mutually exclusive so I find it confusing to
see a bitmap and a priority written to a single resource.


Reinette

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
  2023-08-15 15:27 [RFC 00/12] ARM: MPAM: add support for priority partitioning control Amit Singh Tomar
                   ` (12 preceding siblings ...)
  2023-08-17 19:11 ` [RFC 00/12] ARM: MPAM: add support for priority partitioning control Reinette Chatre
@ 2023-08-22  9:01 ` Peter Newman
  2023-09-01 14:42 ` Jonathan Cameron
  2023-09-01 15:04 ` Jonathan Cameron
  15 siblings, 0 replies; 39+ messages in thread
From: Peter Newman @ 2023-08-22  9:01 UTC (permalink / raw)
  To: Amit Singh Tomar
  Cc: linux-kernel, linux-arm-kernel, fenghua.yu, reinette.chatre,
	james.morse, gcherian, robh

Hi Amit,

On Tue, Aug 15, 2023 at 5:27 PM Amit Singh Tomar <amitsinght@marvell.com> wrote:
> As an example, "schemata" file under resource control group contains information about
> cache portion bitmaps, and memory bandwidth allocation, and these are used to configure
> Cache portion partition (CPOR), and MPAM bandwidth partitioning controls.
>
> MB:0=0100
> L3:0=ffff
>
> But resctrl doesn't provide a way to set-up other control that ARM MPAM provides
> (For instance, Priority partitioning control as mentioned above). To support this,
> James has suggested to use already existing schemata to be compatible with
> portable software, and this is the main idea behind this RFC is to have some kind
> of discussion on how resctrl can be extended to support priority partitioning control.
>
> To support Priority partitioning control, "schemata" file is updated to accommodate
> priority field (upon priority partitioning capability detection), separated from CPBM
> using delimiter ",".
>
> L3:0=ffff,f where f indicates downstream priority max value.

Do we really have to mash two controls into the same schema? In the
CDP example, the code/data controls are presented as multiple schema,
for example: "L3CODE, L3DATA"

Especially when reading back the schemata file, it seems like it would
be simpler for existing software to ignore unfamiliar schema lines in
the schemata file than to overlook the introduction of a comma to the
CBM in the existing "L3" schema.

Thanks!
-Peter

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
  2023-08-17 19:11 ` [RFC 00/12] ARM: MPAM: add support for priority partitioning control Reinette Chatre
  2023-08-17 20:29   ` Reinette Chatre
@ 2023-08-22 12:44   ` Amit Singh Tomar
  2023-08-23 19:06     ` Reinette Chatre
  1 sibling, 1 reply; 39+ messages in thread
From: Amit Singh Tomar @ 2023-08-22 12:44 UTC (permalink / raw)
  To: Reinette Chatre, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
  Cc: fenghua.yu@intel.com, james.morse@arm.com, George Cherian,
	robh@kernel.org, peternewman@google.com, Luck, Tony

Hi Reinette,

Thanks for having a look!

-----Original Message-----
From: Reinette Chatre <reinette.chatre@intel.com> 
Sent: Friday, August 18, 2023 12:41 AM
To: Amit Singh Tomar <amitsinght@marvell.com>; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
Cc: fenghua.yu@intel.com; james.morse@arm.com; George Cherian <gcherian@marvell.com>; robh@kernel.org; peternewman@google.com; Luck, Tony <tony.luck@intel.com>
Subject: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control

External Email

----------------------------------------------------------------------
(+Tony)

Hi Amit,

On 8/15/2023 8:27 AM, Amit Singh Tomar wrote:
> Arm Memory System Resource Partitioning and Monitoring (MPAM) supports 
> different controls that can be applied to different resources in the 
> system For instance, an optional priority partitioning control where 
> priority value is generated from one MSC, propagates over interconnect 
> to other MSC (known as downstream priority), or can be applied within 
> an MSC for internal operations.
> 
> Marvell implementation of ARM MPAM supports priority partitioning 
> control that allows LLC MSC to generate priority values that gets 
> propagated (along with read/write request from upstream) to DDR Block. 
> Within the DDR block the priority values is mapped to different traffic class under DDR QoS strategy.
> The link[1] gives some idea about DDR QoS strategy, and terms like 
> LPR, VPR and HPR.
> 
> Setup priority partitioning control under Resource control
> ----------------------------------------------------------
> At present, resource control (resctrl) provides basic interface to 
> configure/set-up CAT (Cache Allocation Technology) and MBA (Memory Bandwidth Allocation) capabilities.
> ARM MPAM uses it to support controls like Cache portion partition 
> (CPOR), and MPAM bandwidth partitioning.
> 
> As an example, "schemata" file under resource control group contains 
> information about cache portion bitmaps, and memory bandwidth 
> allocation, and these are used to configure Cache portion partition (CPOR), and MPAM bandwidth partitioning controls.
> 
> MB:0=0100
> L3:0=ffff
> 
> But resctrl doesn't provide a way to set-up other control that ARM 
> MPAM provides (For instance, Priority partitioning control as 
> mentioned above). To support this, James has suggested to use already 
> existing schemata to be compatible with portable software, and this is 
> the main idea behind this RFC is to have some kind of discussion on how resctrl can be extended to support priority partitioning control.
> 
> To support Priority partitioning control, "schemata" file is updated 
> to accommodate priority field (upon priority partitioning capability 
> detection), separated from CPBM using delimiter ",".
> 
> L3:0=ffff,f where f indicates downstream priority max value.
> 
> These dspri value gets programmed per partition, that can be used to 
> override QoS value coming from upstream (CPU).
> 
> RFC patch-set[2] is based on James Morse's MPAM snapshot[3] for 6.2, 
> and ACPI table is based on DEN0065A_MPAM_ACPI_2.0.
>

There are some aspects of this that I think we should be cautious about. First, there may inevitably be more properties in the future that need to be associated with a resource allocation, these may indeed be different between architectures and individual platforms. Second, user space need a way to know which properties are supported and what valid parameters may be. 

On a high level I thus understand the goal be to add support for assigning a property to a resource allocation with "Priority partitioning control" being the first property.

To that end, I have a few questions:
* How can this interface be expanded to support more properties with the
  expectation that a system/architecture may not support all resctrl supported
  properties?
[>>] All these new controls ("Priority partitioning is one of them) detected as resource capabilities (via Features Identification Register), and these control will not be probed, if system/architecture
        doesn't support it. From resource control side, this means that users will never get to know about the controls from schemata file. For instance, the platform that supports Priority partitioning control
        schemata file looks like:

       # cat schemata 
           L3:1=ffff

        As oppose to when system has Priority partitioning control
        # cat schemata 
           L3:1=ffff,f
      
        
* Is it possible for support for properties to vary between, for example, different
  MSCs in the system? From resctrl side it may mean that there would be a resource,
  for example "L3", with multiple instances, for example, cache with id #0, cache
  with id#1, etc. but the supported properties or valid values of properties
  may vary between the instances?
[>>] This is really implementation dependent but we would expect, if multiple L3 instances
        across multiple dies implements this control, it should be uniform across, but let's take a case
        where L3 MSC instance on one socket has this control, and other L3 MSC instance on another 
        socket doesn't have this control. From resctrl perspective, one would see this control
        only for L3 instance that has this control, and programmed only for that L3 instance.

       L3:0=XXXX,X;L3:1=XXXX

       And as per proposed format:
   
       L3:0=XXXX,PPART=X, L3:1=XXXX
       
* How can user space know that a system supports "Priority partitioning control"?
  User space needs to know when/if it can attempt to write a priority to the
  schemata.
[>>] At the moment, we label only the resource class, and would like to propose we should
        label newly added controls (under a resource class) as well so that user can easily identify 
        which control to program. For instance, the schemata file with this proposed changes
        will look like this:
        
        L3:0=XXXX,PPART=X

       where PPART=Priority partitioning control, Similarly, if L3 resource class has one more capability, say cache capacity partitioning.

       L3:0=XXXX,PPART=X,CCAP=X

      Very first control always be CAT/CPOR (with no labels)
      

        
* How can user space know what priority values are valid for a particular system?
[>>] Supported priority values are read from one of the MPAM Priority Partitioning register, and in the
        Schemata file, it is set to Maximum value just like Cache portion bitmaps or Memory bandwidth allocation.
        For instance:
   
        L3:0=ffff,f, max priority values is f, and user can program/set from 0-15
      

> Test set-up and results:
> ------------------------
> 
> The downstream priority value feeds into DRAM controller, and one of 
> the important thing that it does with this value is to service the 
> requests sooner (based on the traffic class), hence reducing latency without affecting performance.

Could you please elaborate here? I expected reduced latency to have a big impact on performance.
[>>] To be clear, by performance, it meant Memory bandwidth, and with this  specific configuration/test
       We see priority partitioning as a utility to guarantee lower latency. We are yet to explore its affect
       On memory bandwidth side.
       
> 
> Within the DDR QoS traffic class.
> 
> 0--5 ----> Low priority value
> 6-10 ----> Medium priority value
> 11-15 ----> High priority value
> 
> Benchmark[4] used is multichase.
> 
> Two partition P1 and P2:
> 
> Partition P1:
> -------------
> Assigned core 0
> 100% BW assignment
> 
> Partition P2:
> -------------
> Assigned cores 1-79
> 100% BW assignment
> 
> Test Script:
> -----------
> mkdir p1
> cd p1
> echo 1 > cpus
> echo L3:1=8000,5 > schemata   ##### DSPRI set as 5 (lpr)
> echo "MB:0=100" > schemata
> 
> mkdir p2
> cd p2
> echo ffff,ffffffff,fffffffe > cpus
> echo L3:1=8000,0 > schemata
> echo "MB:0=100" > schemata
> 
> ### Loaded latency run, core 0 does chaseload (pointer chase) with low 
> priority value 5, and cores 1-79 does memory bandwidth run ###

Could you please elaborate what is meant with a "memory bandwidth run"?
[>>] By memory bandwidth run, it meant memory bandwidth test that measure data transfer rate between CPU cores , and Main memory (The 1G size we choose, make sure that it hits DDR , and not     constrained to Caches).

> ./multiload -v -n 10 -t 80 -m 1G -c chaseload
> 
> cd /sys/fs/resctrl/p1
> 
> echo L3:1=8000,a > schemata  ##### DSPRI set as 0xa (vpr)
> 
> ### Loaded latency run, core 0 does chaseload (pointer chase) with 
> medium priority value a, and cores 1-79 does memory bandwidth run ### 
> ./multiload -v -n 10 -t 80 -m 1G -c chaseload
> 
> cd /sys/fs/resctrl/p1
> 
> echo L3:1=8000,f > schemata  ##### DSPRI set as 0xf (hpr)
> 
> ### Loaded latency run where core 0 does chaseload (pointer chase) 
> with high priority value f, and cores 1-79 does memory bandwidth run 
> ### ./multiload -v -n 10 -t 80 -m 1G -c chaseload
> 
> Results[5]:
> 
> LPR average latency is 204.862(ns) vs VPR average latency is 161.018(ns) vs HPR average latency is 134.210(ns).

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
  2023-08-22 12:44   ` [EXT] " Amit Singh Tomar
@ 2023-08-23 19:06     ` Reinette Chatre
  2023-08-23 21:33       ` Amit Singh Tomar
  0 siblings, 1 reply; 39+ messages in thread
From: Reinette Chatre @ 2023-08-23 19:06 UTC (permalink / raw)
  To: Amit Singh Tomar, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
  Cc: fenghua.yu@intel.com, james.morse@arm.com, George Cherian,
	robh@kernel.org, peternewman@google.com, Luck, Tony

Hi Amit,

On 8/22/2023 5:44 AM, Amit Singh Tomar wrote:
> Hi Reinette,
> 
> Thanks for having a look!
> 
> -----Original Message-----
> From: Reinette Chatre <reinette.chatre@intel.com> 
> Sent: Friday, August 18, 2023 12:41 AM
> To: Amit Singh Tomar <amitsinght@marvell.com>; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Cc: fenghua.yu@intel.com; james.morse@arm.com; George Cherian <gcherian@marvell.com>; robh@kernel.org; peternewman@google.com; Luck, Tony <tony.luck@intel.com>
> Subject: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
> 
> External Email
> 
> ----------------------------------------------------------------------
> (+Tony)
> 
> Hi Amit,
> 
> On 8/15/2023 8:27 AM, Amit Singh Tomar wrote:
>> Arm Memory System Resource Partitioning and Monitoring (MPAM) supports 
>> different controls that can be applied to different resources in the 
>> system For instance, an optional priority partitioning control where 
>> priority value is generated from one MSC, propagates over interconnect 
>> to other MSC (known as downstream priority), or can be applied within 
>> an MSC for internal operations.
>>
>> Marvell implementation of ARM MPAM supports priority partitioning 
>> control that allows LLC MSC to generate priority values that gets 
>> propagated (along with read/write request from upstream) to DDR Block. 
>> Within the DDR block the priority values is mapped to different traffic class under DDR QoS strategy.
>> The link[1] gives some idea about DDR QoS strategy, and terms like 
>> LPR, VPR and HPR.
>>
>> Setup priority partitioning control under Resource control
>> ----------------------------------------------------------
>> At present, resource control (resctrl) provides basic interface to 
>> configure/set-up CAT (Cache Allocation Technology) and MBA (Memory Bandwidth Allocation) capabilities.
>> ARM MPAM uses it to support controls like Cache portion partition 
>> (CPOR), and MPAM bandwidth partitioning.
>>
>> As an example, "schemata" file under resource control group contains 
>> information about cache portion bitmaps, and memory bandwidth 
>> allocation, and these are used to configure Cache portion partition (CPOR), and MPAM bandwidth partitioning controls.
>>
>> MB:0=0100
>> L3:0=ffff
>>
>> But resctrl doesn't provide a way to set-up other control that ARM 
>> MPAM provides (For instance, Priority partitioning control as 
>> mentioned above). To support this, James has suggested to use already 
>> existing schemata to be compatible with portable software, and this is 
>> the main idea behind this RFC is to have some kind of discussion on how resctrl can be extended to support priority partitioning control.
>>
>> To support Priority partitioning control, "schemata" file is updated 
>> to accommodate priority field (upon priority partitioning capability 
>> detection), separated from CPBM using delimiter ",".
>>
>> L3:0=ffff,f where f indicates downstream priority max value.
>>
>> These dspri value gets programmed per partition, that can be used to 
>> override QoS value coming from upstream (CPU).
>>
>> RFC patch-set[2] is based on James Morse's MPAM snapshot[3] for 6.2, 
>> and ACPI table is based on DEN0065A_MPAM_ACPI_2.0.
>>
> 
> There are some aspects of this that I think we should be cautious
> about. First, there may inevitably be more properties in the future
> that need to be associated with a resource allocation, these may
> indeed be different between architectures and individual platforms.
> Second, user space need a way to know which properties are supported
> and what valid parameters may be. 
> 
> On a high level I thus understand the goal be to add support for
> assigning a property to a resource allocation with "Priority
> partitioning control" being the first property.

> To that end, I have a few questions:
> * How can this interface be expanded to support more properties with the
>   expectation that a system/architecture may not support all resctrl supported
>   properties?
> [>>] All these new controls ("Priority partitioning is one of them) detected as resource capabilities (via Features Identification Register), and these control will not be probed, if system/architecture
>         doesn't support it. From resource control side, this means that users will never get to know about the controls from schemata file. For instance, the platform that supports Priority partitioning control
>         schemata file looks like:
> 
>        # cat schemata 
>            L3:1=ffff
> 
>         As oppose to when system has Priority partitioning control
>         # cat schemata 
>            L3:1=ffff,f
>

Right, but my question is "How can this interface be expanded ...".
Consider a future L3 resource that has a new and different property
("new_property") that is independent from "Priority partitioning". 
If "L3:1=ffff,f" means "Priority partitioning" == 0xf, how can
a value be assigned to "new_property" if the system's L3 supports
it but not "Priority partitioning"?
If I understand correctly the proposed interface is a positional
interface and "Priority partitioning" is always in second field ...
but a system may or may not support this property so does it require
an empty second field to be able to use other properties?

(fyi ... the quoting used in your response does not make it
obvious what you are responding to)

     
>         
> * Is it possible for support for properties to vary between, for example, different
>   MSCs in the system? From resctrl side it may mean that there would be a resource,
>   for example "L3", with multiple instances, for example, cache with id #0, cache
>   with id#1, etc. but the supported properties or valid values of properties
>   may vary between the instances?
> [>>] This is really implementation dependent but we would expect, if multiple L3 instances
>         across multiple dies implements this control, it should be uniform across, but let's take a case
>         where L3 MSC instance on one socket has this control, and other L3 MSC instance on another 
>         socket doesn't have this control. From resctrl perspective, one would see this control
>         only for L3 instance that has this control, and programmed only for that L3 instance.
> 
>        L3:0=XXXX,X;L3:1=XXXX
> 
>        And as per proposed format:
>    
>        L3:0=XXXX,PPART=X, L3:1=XXXX

I'm a bit lost ... what proposed format?

>        
> * How can user space know that a system supports "Priority partitioning control"?
>   User space needs to know when/if it can attempt to write a priority to the
>   schemata.
> [>>] At the moment, we label only the resource class, and would like to propose we should
>         label newly added controls (under a resource class) as well so that user can easily identify 
>         which control to program. For instance, the schemata file with this proposed changes
>         will look like this:
>         
>         L3:0=XXXX,PPART=X
> 
>        where PPART=Priority partitioning control, Similarly, if L3 resource class has one more capability, say cache capacity partitioning.
> 
>        L3:0=XXXX,PPART=X,CCAP=X
> 
>       Very first control always be CAT/CPOR (with no labels)
>       

Is your response intended to be read from bottom to top?

> * How can user space know what priority values are valid for a particular system?
> [>>] Supported priority values are read from one of the MPAM Priority Partitioning register, and in the
>         Schemata file, it is set to Maximum value just like Cache portion bitmaps or Memory bandwidth allocation.
>         For instance:
>    
>         L3:0=ffff,f, max priority values is f, and user can program/set from 0-15

Doing so would require user space to (a) be running from the
time resctrl is mounted, and (b) maintain state about all
resctrl resources, properties, and supported values.

I think that this is risky and places a burden on user space that
in some scenarios would be impossible to achieve. Consider the
scenario when user space starts running after resctrl has been
in use for a while or if user space loses its state. The
info directory is where information about enabled resources
are located.

>       
> 
>> Test set-up and results:
>> ------------------------
>>
>> The downstream priority value feeds into DRAM controller, and one of 
>> the important thing that it does with this value is to service the 
>> requests sooner (based on the traffic class), hence reducing latency without affecting performance.
> 
> Could you please elaborate here? I expected reduced latency to have a big impact on performance.
> [>>] To be clear, by performance, it meant Memory bandwidth, and with this  specific configuration/test
>        We see priority partitioning as a utility to guarantee lower latency. We are yet to explore its affect
>        On memory bandwidth side.

Please be careful about claims because the above sounds to me as though
this work claims to not affect memory bandwidth but it is also
states that the impact on memory bandwidth has not yet been explored.

Reinette

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
  2023-08-23 19:06     ` Reinette Chatre
@ 2023-08-23 21:33       ` Amit Singh Tomar
  2023-08-23 22:20         ` Reinette Chatre
  0 siblings, 1 reply; 39+ messages in thread
From: Amit Singh Tomar @ 2023-08-23 21:33 UTC (permalink / raw)
  To: Reinette Chatre, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
  Cc: fenghua.yu@intel.com, james.morse@arm.com, George Cherian,
	robh@kernel.org, peternewman@google.com, Luck, Tony

Hi Reinette,

(Kindly follow the responses in a top-to-bottom sequence).

-----Original Message-----
From: Reinette Chatre <reinette.chatre@intel.com> 
Sent: Thursday, August 24, 2023 12:37 AM
To: Amit Singh Tomar <amitsinght@marvell.com>; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
Cc: fenghua.yu@intel.com; james.morse@arm.com; George Cherian <gcherian@marvell.com>; robh@kernel.org; peternewman@google.com; Luck, Tony <tony.luck@intel.com>
Subject: Re: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control

Hi Amit,

On 8/22/2023 5:44 AM, Amit Singh Tomar wrote:
> Hi Reinette,
> 
> Thanks for having a look!
> 
> -----Original Message-----
> From: Reinette Chatre <reinette.chatre@intel.com>
> Sent: Friday, August 18, 2023 12:41 AM
> To: Amit Singh Tomar <amitsinght@marvell.com>; 
> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Cc: fenghua.yu@intel.com; james.morse@arm.com; George Cherian 
> <gcherian@marvell.com>; robh@kernel.org; peternewman@google.com; Luck, 
> Tony <tony.luck@intel.com>
> Subject: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority 
> partitioning control
> 
> External Email
> 
> ----------------------------------------------------------------------
> (+Tony)
> 
> Hi Amit,
> 
> On 8/15/2023 8:27 AM, Amit Singh Tomar wrote:
>> Arm Memory System Resource Partitioning and Monitoring (MPAM) 
>> supports different controls that can be applied to different 
>> resources in the system For instance, an optional priority 
>> partitioning control where priority value is generated from one MSC, 
>> propagates over interconnect to other MSC (known as downstream 
>> priority), or can be applied within an MSC for internal operations.
>>
>> Marvell implementation of ARM MPAM supports priority partitioning 
>> control that allows LLC MSC to generate priority values that gets 
>> propagated (along with read/write request from upstream) to DDR Block.
>> Within the DDR block the priority values is mapped to different traffic class under DDR QoS strategy.
>> The link[1] gives some idea about DDR QoS strategy, and terms like 
>> LPR, VPR and HPR.
>>
>> Setup priority partitioning control under Resource control
>> ----------------------------------------------------------
>> At present, resource control (resctrl) provides basic interface to 
>> configure/set-up CAT (Cache Allocation Technology) and MBA (Memory Bandwidth Allocation) capabilities.
>> ARM MPAM uses it to support controls like Cache portion partition 
>> (CPOR), and MPAM bandwidth partitioning.
>>
>> As an example, "schemata" file under resource control group contains 
>> information about cache portion bitmaps, and memory bandwidth 
>> allocation, and these are used to configure Cache portion partition (CPOR), and MPAM bandwidth partitioning controls.
>>
>> MB:0=0100
>> L3:0=ffff
>>
>> But resctrl doesn't provide a way to set-up other control that ARM 
>> MPAM provides (For instance, Priority partitioning control as 
>> mentioned above). To support this, James has suggested to use already 
>> existing schemata to be compatible with portable software, and this 
>> is the main idea behind this RFC is to have some kind of discussion on how resctrl can be extended to support priority partitioning control.
>>
>> To support Priority partitioning control, "schemata" file is updated 
>> to accommodate priority field (upon priority partitioning capability 
>> detection), separated from CPBM using delimiter ",".
>>
>> L3:0=ffff,f where f indicates downstream priority max value.
>>
>> These dspri value gets programmed per partition, that can be used to 
>> override QoS value coming from upstream (CPU).
>>
>> RFC patch-set[2] is based on James Morse's MPAM snapshot[3] for 6.2, 
>> and ACPI table is based on DEN0065A_MPAM_ACPI_2.0.
>>
> 
> There are some aspects of this that I think we should be cautious 
> about. First, there may inevitably be more properties in the future 
> that need to be associated with a resource allocation, these may 
> indeed be different between architectures and individual platforms.
> Second, user space need a way to know which properties are supported 
> and what valid parameters may be.
> 
> On a high level I thus understand the goal be to add support for 
> assigning a property to a resource allocation with "Priority 
> partitioning control" being the first property.

> To that end, I have a few questions:
> * How can this interface be expanded to support more properties with the
>   expectation that a system/architecture may not support all resctrl supported
>   properties?
> [>>] All these new controls ("Priority partitioning is one of them) detected as resource capabilities (via Features Identification Register), and these control will not be probed, if system/architecture
>         doesn't support it. From resource control side, this means that users will never get to know about the controls from schemata file. For instance, the platform that supports Priority partitioning control
>         schemata file looks like:
> 
>        # cat schemata 
>            L3:1=ffff
> 
>         As oppose to when system has Priority partitioning control
>         # cat schemata 
>            L3:1=ffff,f
>

Right, but my question is "How can this interface be expanded ...".
Consider a future L3 resource that has a new and different property
("new_property") that is independent from "Priority partitioning". 
If "L3:1=ffff,f" means "Priority partitioning" == 0xf, how can a value be assigned to "new_property" if the system's L3 supports it but not "Priority partitioning"?
If I understand correctly the proposed interface is a positional interface and "Priority partitioning" is always in second field ...

[>>] Yes, "Priority partitioning" will always be the second field.

but a system may or may not support this property so does it require an empty second field to be able to use other properties?

[>>] Yes, in the absence of this control ("Priority partitioning"), second field will be taken by other control (if supported).

So, for example, if L3 resource is equipped with two controls, .i.e. CPOR and PPART, schemata will look like:

         L3:0=XXXX,PPART=X

and, if same resource is equipped with another set of controls, .i.e. CPOR and CCAP (cache capacity partitioning), schemata will look like:

         L3:0=XXXX,CCAP=X

and, in case resource is equipped with all three controls, schemata will look like:

        L3:0=XXXX,PPART=X,CCAP=X

Each of these combinations, features its own format specifier.
    
>         
> * Is it possible for support for properties to vary between, for example, different
>   MSCs in the system? From resctrl side it may mean that there would be a resource,
>   for example "L3", with multiple instances, for example, cache with id #0, cache
>   with id#1, etc. but the supported properties or valid values of properties
>   may vary between the instances?
> [>>] This is really implementation dependent but we would expect, if multiple L3 instances
>         across multiple dies implements this control, it should be uniform across, but let's take a case
>         where L3 MSC instance on one socket has this control, and other L3 MSC instance on another 
>         socket doesn't have this control. From resctrl perspective, one would see this control
>         only for L3 instance that has this control, and programmed only for that L3 instance.
> 
>        L3:0=XXXX,X;L3:1=XXXX
> 
>        And as per proposed format:
>    
>        L3:0=XXXX,PPART=X, L3:1=XXXX

I'm a bit lost ... what proposed format?
[>>] Sorry about that, I should have indicated the proposed format is in the point below.

>        
> * How can user space know that a system supports "Priority partitioning control"?
>   User space needs to know when/if it can attempt to write a priority to the
>   schemata.
> [>>] At the moment, we label only the resource class, and would like to propose we should
>         label newly added controls (under a resource class) as well so that user can easily identify 
>         which control to program. For instance, the schemata file with this proposed changes
>         will look like this:
>         
>         L3:0=XXXX,PPART=X
> 
>        where PPART=Priority partitioning control, Similarly, if L3 resource class has one more capability, say cache capacity partitioning.
> 
>        L3:0=XXXX,PPART=X,CCAP=X
> 
>       Very first control always be CAT/CPOR (with no labels)
>       

Is your response intended to be read from bottom to top?

> * How can user space know what priority values are valid for a particular system?
> [>>] Supported priority values are read from one of the MPAM Priority Partitioning register, and in the
>         Schemata file, it is set to Maximum value just like Cache portion bitmaps or Memory bandwidth allocation.
>         For instance:
>    
>         L3:0=ffff,f, max priority values is f, and user can 
> program/set from 0-15

Doing so would require user space to (a) be running from the time resctrl is mounted, and (b) maintain state about all resctrl resources, properties, and supported values.

I think that this is risky and places a burden on user space that in some scenarios would be impossible to achieve. Consider the scenario when user space starts running after resctrl has been in use for a while or if user space loses its state. The info directory is where information about enabled resources are located.

[>>] Thanks for point it out, will export this information to info directory.

>       
> 
>> Test set-up and results:
>> ------------------------
>>
>> The downstream priority value feeds into DRAM controller, and one of 
>> the important thing that it does with this value is to service the 
>> requests sooner (based on the traffic class), hence reducing latency without affecting performance.
> 
> Could you please elaborate here? I expected reduced latency to have a big impact on performance.
> [>>] To be clear, by performance, it meant Memory bandwidth, and with this  specific configuration/test
>        We see priority partitioning as a utility to guarantee lower latency. We are yet to explore its affect
>        On memory bandwidth side.

Please be careful about claims because the above sounds to me as though this work claims to not affect memory bandwidth but it is also states that the impact on memory bandwidth has not yet been explored.
[>>] Sure, will be more careful with my wording but the previous statement "hence reducing latency without affecting performance" is based on
test result we presented. For instance, if we look at Bandwidth numbers across the priority values, it's almost the same ~345 GB/s.

Thanks
-Amit

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
  2023-08-23 21:33       ` Amit Singh Tomar
@ 2023-08-23 22:20         ` Reinette Chatre
  2023-08-23 22:36           ` Luck, Tony
  2023-08-24  8:52           ` Amit Singh Tomar
  0 siblings, 2 replies; 39+ messages in thread
From: Reinette Chatre @ 2023-08-23 22:20 UTC (permalink / raw)
  To: Amit Singh Tomar, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
  Cc: fenghua.yu@intel.com, james.morse@arm.com, George Cherian,
	robh@kernel.org, peternewman@google.com, Luck, Tony

Hi Amit,

On 8/23/2023 2:33 PM, Amit Singh Tomar wrote:
> Hi Reinette,
> 
> (Kindly follow the responses in a top-to-bottom sequence).
> 
> -----Original Message-----
> From: Reinette Chatre <reinette.chatre@intel.com> 
> Sent: Thursday, August 24, 2023 12:37 AM
> To: Amit Singh Tomar <amitsinght@marvell.com>; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Cc: fenghua.yu@intel.com; james.morse@arm.com; George Cherian <gcherian@marvell.com>; robh@kernel.org; peternewman@google.com; Luck, Tony <tony.luck@intel.com>
> Subject: Re: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
> 
> Hi Amit,
> 
> On 8/22/2023 5:44 AM, Amit Singh Tomar wrote:
>> Hi Reinette,
>>
>> Thanks for having a look!
>>
>> -----Original Message-----
>> From: Reinette Chatre <reinette.chatre@intel.com>
>> Sent: Friday, August 18, 2023 12:41 AM
>> To: Amit Singh Tomar <amitsinght@marvell.com>; 
>> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
>> Cc: fenghua.yu@intel.com; james.morse@arm.com; George Cherian 
>> <gcherian@marvell.com>; robh@kernel.org; peternewman@google.com; Luck, 
>> Tony <tony.luck@intel.com>
>> Subject: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority 
>> partitioning control
>>
>> External Email
>>
>> ----------------------------------------------------------------------
>> (+Tony)
>>
>> Hi Amit,
>>
>> On 8/15/2023 8:27 AM, Amit Singh Tomar wrote:
>>> Arm Memory System Resource Partitioning and Monitoring (MPAM) 
>>> supports different controls that can be applied to different 
>>> resources in the system For instance, an optional priority 
>>> partitioning control where priority value is generated from one MSC, 
>>> propagates over interconnect to other MSC (known as downstream 
>>> priority), or can be applied within an MSC for internal operations.
>>>
>>> Marvell implementation of ARM MPAM supports priority partitioning 
>>> control that allows LLC MSC to generate priority values that gets 
>>> propagated (along with read/write request from upstream) to DDR Block.
>>> Within the DDR block the priority values is mapped to different traffic class under DDR QoS strategy.
>>> The link[1] gives some idea about DDR QoS strategy, and terms like 
>>> LPR, VPR and HPR.
>>>
>>> Setup priority partitioning control under Resource control
>>> ----------------------------------------------------------
>>> At present, resource control (resctrl) provides basic interface to 
>>> configure/set-up CAT (Cache Allocation Technology) and MBA (Memory Bandwidth Allocation) capabilities.
>>> ARM MPAM uses it to support controls like Cache portion partition 
>>> (CPOR), and MPAM bandwidth partitioning.
>>>
>>> As an example, "schemata" file under resource control group contains 
>>> information about cache portion bitmaps, and memory bandwidth 
>>> allocation, and these are used to configure Cache portion partition (CPOR), and MPAM bandwidth partitioning controls.
>>>
>>> MB:0=0100
>>> L3:0=ffff
>>>
>>> But resctrl doesn't provide a way to set-up other control that ARM 
>>> MPAM provides (For instance, Priority partitioning control as 
>>> mentioned above). To support this, James has suggested to use already 
>>> existing schemata to be compatible with portable software, and this 
>>> is the main idea behind this RFC is to have some kind of discussion on how resctrl can be extended to support priority partitioning control.
>>>
>>> To support Priority partitioning control, "schemata" file is updated 
>>> to accommodate priority field (upon priority partitioning capability 
>>> detection), separated from CPBM using delimiter ",".
>>>
>>> L3:0=ffff,f where f indicates downstream priority max value.
>>>
>>> These dspri value gets programmed per partition, that can be used to 
>>> override QoS value coming from upstream (CPU).
>>>
>>> RFC patch-set[2] is based on James Morse's MPAM snapshot[3] for 6.2, 
>>> and ACPI table is based on DEN0065A_MPAM_ACPI_2.0.
>>>
>>
>> There are some aspects of this that I think we should be cautious 
>> about. First, there may inevitably be more properties in the future 
>> that need to be associated with a resource allocation, these may 
>> indeed be different between architectures and individual platforms.
>> Second, user space need a way to know which properties are supported 
>> and what valid parameters may be.
>>
>> On a high level I thus understand the goal be to add support for 
>> assigning a property to a resource allocation with "Priority 
>> partitioning control" being the first property.
> 
>> To that end, I have a few questions:
>> * How can this interface be expanded to support more properties with the
>>   expectation that a system/architecture may not support all resctrl supported
>>   properties?
>> [>>] All these new controls ("Priority partitioning is one of them) detected as resource capabilities (via Features Identification Register), and these control will not be probed, if system/architecture
>>         doesn't support it. From resource control side, this means that users will never get to know about the controls from schemata file. For instance, the platform that supports Priority partitioning control
>>         schemata file looks like:
>>
>>        # cat schemata 
>>            L3:1=ffff
>>
>>         As oppose to when system has Priority partitioning control
>>         # cat schemata 
>>            L3:1=ffff,f
>>
> 
> Right, but my question is "How can this interface be expanded ...".
> Consider a future L3 resource that has a new and different property
> ("new_property") that is independent from "Priority partitioning". 
> If "L3:1=ffff,f" means "Priority partitioning" == 0xf, how can a value be assigned to "new_property" if the system's L3 supports it but not "Priority partitioning"?
> If I understand correctly the proposed interface is a positional interface and "Priority partitioning" is always in second field ...
> 
> [>>] Yes, "Priority partitioning" will always be the second field.
> 
> but a system may or may not support this property so does it require an empty second field to be able to use other properties?
> 
> [>>] Yes, in the absence of this control ("Priority partitioning"), second field will be taken by other control (if supported).
> 
> So, for example, if L3 resource is equipped with two controls, .i.e. CPOR and PPART, schemata will look like:
> 
>          L3:0=XXXX,PPART=X
> 
> and, if same resource is equipped with another set of controls, .i.e. CPOR and CCAP (cache capacity partitioning), schemata will look like:
> 
>          L3:0=XXXX,CCAP=X
> 
> and, in case resource is equipped with all three controls, schemata will look like:
> 
>         L3:0=XXXX,PPART=X,CCAP=X
> 
> Each of these combinations, features its own format specifier.
>     

I see. I do have a similar concern as Peter regarding the impact of
this change on parsing of the schemata file. I peeked at intel-cmt-cat's
implementation [1] and if I understand it correctly these changes will
break it. This is just one example but I do think this will have
significant impact on user space that should be avoided.

Apart from this this discussion focused on the display of properties when
user views the schemata file. We also need to consider
how the user will provide new data by writing to the schemata file.
For example, I do not think it is convenient for the user to
have to provide the allocation bitmask every time the
"Priority partitioning" value needs to be changed for a resource
instance. This may also be solved when considering Peter's idea but
since this work depends on other work that is not upstream it
is difficult to envision the impact of any suggestions.

Reinette

[1] https://github.com/intel/intel-cmt-cat/blob/master/lib/resctrl_schemata.c#L495


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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
  2023-08-23 22:20         ` Reinette Chatre
@ 2023-08-23 22:36           ` Luck, Tony
  2023-08-24  8:52           ` Amit Singh Tomar
  1 sibling, 0 replies; 39+ messages in thread
From: Luck, Tony @ 2023-08-23 22:36 UTC (permalink / raw)
  To: Chatre, Reinette, Amit Singh Tomar, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
  Cc: Yu, Fenghua, james.morse@arm.com, George Cherian, robh@kernel.org,
	peternewman@google.com

> I see. I do have a similar concern as Peter regarding the impact of
> this change on parsing of the schemata file. I peeked at intel-cmt-cat's
> implementation [1] and if I understand it correctly these changes will
> break it. This is just one example but I do think this will have
> significant impact on user space that should be avoided.
>
> Apart from this this discussion focused on the display of properties when
> user views the schemata file. We also need to consider
> how the user will provide new data by writing to the schemata file.
> For example, I do not think it is convenient for the user to
> have to provide the allocation bitmask every time the
> "Priority partitioning" value needs to be changed for a resource
> instance. This may also be solved when considering Peter's idea but
> since this work depends on other work that is not upstream it
> is difficult to envision the impact of any suggestions.

Would if be better to add additional files? E.g. keep the syntax of
the schemata file the same. Just specifying the cache allocation
bitmask for each cache instance.

Then have a separate file (or files) for these additional attributes
like PPART and CCAP.

How are these likely to be used in practice? Would a user need to
update all of these at once (in which case separate files would be
inconvenient). Or is is likely that updates to mask, PPART, CCAP
are orthogonal, and so updates are not usually done together?

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
  2023-08-23 22:20         ` Reinette Chatre
  2023-08-23 22:36           ` Luck, Tony
@ 2023-08-24  8:52           ` Amit Singh Tomar
  2023-08-24 15:30             ` Luck, Tony
                               ` (2 more replies)
  1 sibling, 3 replies; 39+ messages in thread
From: Amit Singh Tomar @ 2023-08-24  8:52 UTC (permalink / raw)
  To: Reinette Chatre, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
  Cc: fenghua.yu@intel.com, james.morse@arm.com, George Cherian,
	robh@kernel.org, peternewman@google.com, Luck, Tony

Hi Reinette,

Thanks for your prompt response.

-----Original Message-----
From: Reinette Chatre <reinette.chatre@intel.com> 
Sent: Thursday, August 24, 2023 3:50 AM
To: Amit Singh Tomar <amitsinght@marvell.com>; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
Cc: fenghua.yu@intel.com; james.morse@arm.com; George Cherian <gcherian@marvell.com>; robh@kernel.org; peternewman@google.com; Luck, Tony <tony.luck@intel.com>
Subject: Re: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control

Hi Amit,

On 8/23/2023 2:33 PM, Amit Singh Tomar wrote:
> Hi Reinette,
> 
> (Kindly follow the responses in a top-to-bottom sequence).
> 
> -----Original Message-----
> From: Reinette Chatre <reinette.chatre@intel.com>
> Sent: Thursday, August 24, 2023 12:37 AM
> To: Amit Singh Tomar <amitsinght@marvell.com>; 
> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Cc: fenghua.yu@intel.com; james.morse@arm.com; George Cherian 
> <gcherian@marvell.com>; robh@kernel.org; peternewman@google.com; Luck, 
> Tony <tony.luck@intel.com>
> Subject: Re: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority 
> partitioning control
> 
> Hi Amit,
> 
> On 8/22/2023 5:44 AM, Amit Singh Tomar wrote:
>> Hi Reinette,
>>
>> Thanks for having a look!
>>
>> -----Original Message-----
>> From: Reinette Chatre <reinette.chatre@intel.com>
>> Sent: Friday, August 18, 2023 12:41 AM
>> To: Amit Singh Tomar <amitsinght@marvell.com>; 
>> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
>> Cc: fenghua.yu@intel.com; james.morse@arm.com; George Cherian 
>> <gcherian@marvell.com>; robh@kernel.org; peternewman@google.com; 
>> Luck, Tony <tony.luck@intel.com>
>> Subject: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority 
>> partitioning control
>>
>> External Email
>>
>> ---------------------------------------------------------------------
>> -
>> (+Tony)
>>
>> Hi Amit,
>>
>> On 8/15/2023 8:27 AM, Amit Singh Tomar wrote:
>>> Arm Memory System Resource Partitioning and Monitoring (MPAM) 
>>> supports different controls that can be applied to different 
>>> resources in the system For instance, an optional priority 
>>> partitioning control where priority value is generated from one MSC, 
>>> propagates over interconnect to other MSC (known as downstream 
>>> priority), or can be applied within an MSC for internal operations.
>>>
>>> Marvell implementation of ARM MPAM supports priority partitioning 
>>> control that allows LLC MSC to generate priority values that gets 
>>> propagated (along with read/write request from upstream) to DDR Block.
>>> Within the DDR block the priority values is mapped to different traffic class under DDR QoS strategy.
>>> The link[1] gives some idea about DDR QoS strategy, and terms like 
>>> LPR, VPR and HPR.
>>>
>>> Setup priority partitioning control under Resource control
>>> ----------------------------------------------------------
>>> At present, resource control (resctrl) provides basic interface to 
>>> configure/set-up CAT (Cache Allocation Technology) and MBA (Memory Bandwidth Allocation) capabilities.
>>> ARM MPAM uses it to support controls like Cache portion partition 
>>> (CPOR), and MPAM bandwidth partitioning.
>>>
>>> As an example, "schemata" file under resource control group contains 
>>> information about cache portion bitmaps, and memory bandwidth 
>>> allocation, and these are used to configure Cache portion partition (CPOR), and MPAM bandwidth partitioning controls.
>>>
>>> MB:0=0100
>>> L3:0=ffff
>>>
>>> But resctrl doesn't provide a way to set-up other control that ARM 
>>> MPAM provides (For instance, Priority partitioning control as 
>>> mentioned above). To support this, James has suggested to use 
>>> already existing schemata to be compatible with portable software, 
>>> and this is the main idea behind this RFC is to have some kind of discussion on how resctrl can be extended to support priority partitioning control.
>>>
>>> To support Priority partitioning control, "schemata" file is updated 
>>> to accommodate priority field (upon priority partitioning capability 
>>> detection), separated from CPBM using delimiter ",".
>>>
>>> L3:0=ffff,f where f indicates downstream priority max value.
>>>
>>> These dspri value gets programmed per partition, that can be used to 
>>> override QoS value coming from upstream (CPU).
>>>
>>> RFC patch-set[2] is based on James Morse's MPAM snapshot[3] for 6.2, 
>>> and ACPI table is based on DEN0065A_MPAM_ACPI_2.0.
>>>
>>
>> There are some aspects of this that I think we should be cautious 
>> about. First, there may inevitably be more properties in the future 
>> that need to be associated with a resource allocation, these may 
>> indeed be different between architectures and individual platforms.
>> Second, user space need a way to know which properties are supported 
>> and what valid parameters may be.
>>
>> On a high level I thus understand the goal be to add support for 
>> assigning a property to a resource allocation with "Priority 
>> partitioning control" being the first property.
> 
>> To that end, I have a few questions:
>> * How can this interface be expanded to support more properties with the
>>   expectation that a system/architecture may not support all resctrl supported
>>   properties?
>> [>>] All these new controls ("Priority partitioning is one of them) detected as resource capabilities (via Features Identification Register), and these control will not be probed, if system/architecture
>>         doesn't support it. From resource control side, this means that users will never get to know about the controls from schemata file. For instance, the platform that supports Priority partitioning control
>>         schemata file looks like:
>>
>>        # cat schemata 
>>            L3:1=ffff
>>
>>         As oppose to when system has Priority partitioning control
>>         # cat schemata 
>>            L3:1=ffff,f
>>
> 
> Right, but my question is "How can this interface be expanded ...".
> Consider a future L3 resource that has a new and different property
> ("new_property") that is independent from "Priority partitioning". 
> If "L3:1=ffff,f" means "Priority partitioning" == 0xf, how can a value be assigned to "new_property" if the system's L3 supports it but not "Priority partitioning"?
> If I understand correctly the proposed interface is a positional interface and "Priority partitioning" is always in second field ...
> 
> [>>] Yes, "Priority partitioning" will always be the second field.
> 
> but a system may or may not support this property so does it require an empty second field to be able to use other properties?
> 
> [>>] Yes, in the absence of this control ("Priority partitioning"), second field will be taken by other control (if supported).
> 
> So, for example, if L3 resource is equipped with two controls, .i.e. CPOR and PPART, schemata will look like:
> 
>          L3:0=XXXX,PPART=X
> 
> and, if same resource is equipped with another set of controls, .i.e. CPOR and CCAP (cache capacity partitioning), schemata will look like:
> 
>          L3:0=XXXX,CCAP=X
> 
> and, in case resource is equipped with all three controls, schemata will look like:
> 
>         L3:0=XXXX,PPART=X,CCAP=X
> 
> Each of these combinations, features its own format specifier.
>     

I see. I do have a similar concern as Peter regarding the impact of this change on parsing of the schemata file. I peeked at intel-cmt-cat's implementation [1] and if I understand it correctly these changes will break it. This is just one example but I do think this will have significant impact on user space that should be avoided.[>>] 

[>>] To be honest, I don't see how it breaks things on x86 side. None of these new controls (PPART, or CCAP) exist for intel platform, and in absence of these control, schemata file remains the same, .i.e.
        L3:0=ffff

       Or you're talking about the situation when intel may have similar control, and this proposed approach would break intel-cmt-cat then?

Apart from this this discussion focused on the display of properties when user views the schemata file. We also need to consider how the user will provide new data by writing to the schemata file.
For example, I do not think it is convenient for the user to have to provide the allocation bitmask every time the "Priority partitioning" value needs to be changed for a resource instance. 

[>>] This is something, I was pondering about, not to provide allocation bitmask while changing "Priority partitioning" values or vice-versa but ARM MPAM device driver
run through all the resource instances (learned from ACPI table) and program the ris_idx (along with partid) into MPAMCFG_PART_SEL_NS[RIS].
After that, programs the portion bit map (related to CPOR), or Priority value (depends on the ris_idx) into MPAMCFG_CPBM_NS or MPAMCFG_PRI_NS[dspri].

As example, for resource index 0 (MPAMCFG_PART_SEL_NS[0]), it programs Priority value, and for resource index 1 ( MPAMCFG_PART_SEL_NS[1]), it programs portion bitmap value. In a way, Driver[1]
Expects both these values to be supplied. May be James can correct me here.

This may also be solved when considering Peter's idea but since this work depends on other work that is not upstream it is difficult to envision the impact of any suggestions.

[>>] Initially, we have thought about these three approaches:

1) Populate the resource control filesystem[2] with a new file that corresponds to new control. It requires Priority value to be encoded around portion bitmaps, and James has suggested we should go via
   "schemata" file approach.

   I think, this is something Tony has pointed out in other thread.

2) Second approach that we discussed internally is to have schemata for CPOR, and PPART separated by new line as mentioned/suggested by Peter, But it may require to tweak
   the ARM MPAM device driver a bit. It was kind of toss-up between 2nd and 3nd approach :), and we went with the 3rd one.

   L3:0=XXXX
   L3:0=PPART=X

   Will look into it again.

3) This is the approach we presented in this RFC.

Thanks,
-Amit

 [1]: https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git/tree/drivers/platform/mpam/mpam_devices.c?h=mpam/snapshot/v6.5-rc1#n1175
[2]: https://github.com/Amit-Radur/linux/commit/5b603e282c6e15a79ae03b2ed4882b672724c018

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
  2023-08-24  8:52           ` Amit Singh Tomar
@ 2023-08-24 15:30             ` Luck, Tony
  2023-08-24 18:00             ` Reinette Chatre
  2024-01-11 20:56             ` Peter Newman
  2 siblings, 0 replies; 39+ messages in thread
From: Luck, Tony @ 2023-08-24 15:30 UTC (permalink / raw)
  To: Amit Singh Tomar, Chatre, Reinette, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
  Cc: Yu, Fenghua, james.morse@arm.com, George Cherian, robh@kernel.org,
	peternewman@google.com

> 2) Second approach that we discussed internally is to have schemata for CPOR, and PPART separated by new line as mentioned/suggested by Peter, But it may require to tweak
>    the ARM MPAM device driver a bit. It was kind of toss-up between 2nd and 3nd approach :), and we went with the 3rd one.
>
>    L3:0=XXXX
>    L3:0=PPART=X
>
>    Will look into it again.

That looks hard to parse. How about:

L3:0=XXX;1=YYY
L3PPART:0=AAA;1=BBB
L3CPOR:0=MMM;1=NNN

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
  2023-08-24  8:52           ` Amit Singh Tomar
  2023-08-24 15:30             ` Luck, Tony
@ 2023-08-24 18:00             ` Reinette Chatre
  2024-01-11 20:56             ` Peter Newman
  2 siblings, 0 replies; 39+ messages in thread
From: Reinette Chatre @ 2023-08-24 18:00 UTC (permalink / raw)
  To: Amit Singh Tomar, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
  Cc: fenghua.yu@intel.com, james.morse@arm.com, George Cherian,
	robh@kernel.org, peternewman@google.com, Luck, Tony

Hi Amit,

On 8/24/2023 1:52 AM, Amit Singh Tomar wrote:
> Hi Reinette,
> 
> Thanks for your prompt response.
> 
> -----Original Message-----
> From: Reinette Chatre <reinette.chatre@intel.com> 
> Sent: Thursday, August 24, 2023 3:50 AM
> To: Amit Singh Tomar <amitsinght@marvell.com>; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Cc: fenghua.yu@intel.com; james.morse@arm.com; George Cherian <gcherian@marvell.com>; robh@kernel.org; peternewman@google.com; Luck, Tony <tony.luck@intel.com>
> Subject: Re: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
> 
> Hi Amit,
> 
> On 8/23/2023 2:33 PM, Amit Singh Tomar wrote:
>> Hi Reinette,
>>
>> (Kindly follow the responses in a top-to-bottom sequence).
>>
>> -----Original Message-----
>> From: Reinette Chatre <reinette.chatre@intel.com>
>> Sent: Thursday, August 24, 2023 12:37 AM
>> To: Amit Singh Tomar <amitsinght@marvell.com>; 
>> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
>> Cc: fenghua.yu@intel.com; james.morse@arm.com; George Cherian 
>> <gcherian@marvell.com>; robh@kernel.org; peternewman@google.com; Luck, 
>> Tony <tony.luck@intel.com>
>> Subject: Re: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority 
>> partitioning control
>>
>> Hi Amit,
>>
>> On 8/22/2023 5:44 AM, Amit Singh Tomar wrote:
>>> Hi Reinette,
>>>
>>> Thanks for having a look!
>>>
>>> -----Original Message-----
>>> From: Reinette Chatre <reinette.chatre@intel.com>
>>> Sent: Friday, August 18, 2023 12:41 AM
>>> To: Amit Singh Tomar <amitsinght@marvell.com>; 
>>> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
>>> Cc: fenghua.yu@intel.com; james.morse@arm.com; George Cherian 
>>> <gcherian@marvell.com>; robh@kernel.org; peternewman@google.com; 
>>> Luck, Tony <tony.luck@intel.com>
>>> Subject: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority 
>>> partitioning control
>>>
>>> External Email
>>>
>>> ---------------------------------------------------------------------
>>> -
>>> (+Tony)
>>>
>>> Hi Amit,
>>>
>>> On 8/15/2023 8:27 AM, Amit Singh Tomar wrote:
>>>> Arm Memory System Resource Partitioning and Monitoring (MPAM) 
>>>> supports different controls that can be applied to different 
>>>> resources in the system For instance, an optional priority 
>>>> partitioning control where priority value is generated from one MSC, 
>>>> propagates over interconnect to other MSC (known as downstream 
>>>> priority), or can be applied within an MSC for internal operations.
>>>>
>>>> Marvell implementation of ARM MPAM supports priority partitioning 
>>>> control that allows LLC MSC to generate priority values that gets 
>>>> propagated (along with read/write request from upstream) to DDR Block.
>>>> Within the DDR block the priority values is mapped to different traffic class under DDR QoS strategy.
>>>> The link[1] gives some idea about DDR QoS strategy, and terms like 
>>>> LPR, VPR and HPR.
>>>>
>>>> Setup priority partitioning control under Resource control
>>>> ----------------------------------------------------------
>>>> At present, resource control (resctrl) provides basic interface to 
>>>> configure/set-up CAT (Cache Allocation Technology) and MBA (Memory Bandwidth Allocation) capabilities.
>>>> ARM MPAM uses it to support controls like Cache portion partition 
>>>> (CPOR), and MPAM bandwidth partitioning.
>>>>
>>>> As an example, "schemata" file under resource control group contains 
>>>> information about cache portion bitmaps, and memory bandwidth 
>>>> allocation, and these are used to configure Cache portion partition (CPOR), and MPAM bandwidth partitioning controls.
>>>>
>>>> MB:0=0100
>>>> L3:0=ffff
>>>>
>>>> But resctrl doesn't provide a way to set-up other control that ARM 
>>>> MPAM provides (For instance, Priority partitioning control as 
>>>> mentioned above). To support this, James has suggested to use 
>>>> already existing schemata to be compatible with portable software, 
>>>> and this is the main idea behind this RFC is to have some kind of discussion on how resctrl can be extended to support priority partitioning control.
>>>>
>>>> To support Priority partitioning control, "schemata" file is updated 
>>>> to accommodate priority field (upon priority partitioning capability 
>>>> detection), separated from CPBM using delimiter ",".
>>>>
>>>> L3:0=ffff,f where f indicates downstream priority max value.
>>>>
>>>> These dspri value gets programmed per partition, that can be used to 
>>>> override QoS value coming from upstream (CPU).
>>>>
>>>> RFC patch-set[2] is based on James Morse's MPAM snapshot[3] for 6.2, 
>>>> and ACPI table is based on DEN0065A_MPAM_ACPI_2.0.
>>>>
>>>
>>> There are some aspects of this that I think we should be cautious 
>>> about. First, there may inevitably be more properties in the future 
>>> that need to be associated with a resource allocation, these may 
>>> indeed be different between architectures and individual platforms.
>>> Second, user space need a way to know which properties are supported 
>>> and what valid parameters may be.
>>>
>>> On a high level I thus understand the goal be to add support for 
>>> assigning a property to a resource allocation with "Priority 
>>> partitioning control" being the first property.
>>
>>> To that end, I have a few questions:
>>> * How can this interface be expanded to support more properties with the
>>>   expectation that a system/architecture may not support all resctrl supported
>>>   properties?
>>> [>>] All these new controls ("Priority partitioning is one of them) detected as resource capabilities (via Features Identification Register), and these control will not be probed, if system/architecture
>>>         doesn't support it. From resource control side, this means that users will never get to know about the controls from schemata file. For instance, the platform that supports Priority partitioning control
>>>         schemata file looks like:
>>>
>>>        # cat schemata 
>>>            L3:1=ffff
>>>
>>>         As oppose to when system has Priority partitioning control
>>>         # cat schemata 
>>>            L3:1=ffff,f
>>>
>>
>> Right, but my question is "How can this interface be expanded ...".
>> Consider a future L3 resource that has a new and different property
>> ("new_property") that is independent from "Priority partitioning". 
>> If "L3:1=ffff,f" means "Priority partitioning" == 0xf, how can a value be assigned to "new_property" if the system's L3 supports it but not "Priority partitioning"?
>> If I understand correctly the proposed interface is a positional interface and "Priority partitioning" is always in second field ...
>>
>> [>>] Yes, "Priority partitioning" will always be the second field.
>>
>> but a system may or may not support this property so does it require an empty second field to be able to use other properties?
>>
>> [>>] Yes, in the absence of this control ("Priority partitioning"), second field will be taken by other control (if supported).
>>
>> So, for example, if L3 resource is equipped with two controls, .i.e. CPOR and PPART, schemata will look like:
>>
>>          L3:0=XXXX,PPART=X
>>
>> and, if same resource is equipped with another set of controls, .i.e. CPOR and CCAP (cache capacity partitioning), schemata will look like:
>>
>>          L3:0=XXXX,CCAP=X
>>
>> and, in case resource is equipped with all three controls, schemata will look like:
>>
>>         L3:0=XXXX,PPART=X,CCAP=X
>>
>> Each of these combinations, features its own format specifier.
>>     
> 
> I see. I do have a similar concern as Peter regarding the impact of this change on parsing of the schemata file. I peeked at intel-cmt-cat's implementation [1] and if I understand it correctly these changes will break it. This is just one example but I do think this will have significant impact on user space that should be avoided.[>>] 
> 
> [>>] To be honest, I don't see how it breaks things on x86 side. None of these new controls (PPART, or CCAP) exist for intel platform, and in absence of these control, schemata file remains the same, .i.e.
>         L3:0=ffff
> 
>        Or you're talking about the situation when intel may have similar control, and this proposed approach would break intel-cmt-cat then?

There are indeed two parts to this. First, I still consider
this as breaking user space because user space interacts with
"resctrl" that should be a generic interface. Second, yes, any
"resctrl" interface is available to every vendor. It is not expected
that all systems support all features but resctrl is the interface
with which user space can query what features are supported in
order to interact with the features.

> Apart from this this discussion focused on the display of properties when user views the schemata file. We also need to consider how the user will provide new data by writing to the schemata file.
> For example, I do not think it is convenient for the user to have to provide the allocation bitmask every time the "Priority partitioning" value needs to be changed for a resource instance. 
> 
> [>>] This is something, I was pondering about, not to provide allocation bitmask while changing "Priority partitioning" values or vice-versa but ARM MPAM device driver
> run through all the resource instances (learned from ACPI table) and program the ris_idx (along with partid) into MPAMCFG_PART_SEL_NS[RIS].
> After that, programs the portion bit map (related to CPOR), or Priority value (depends on the ris_idx) into MPAMCFG_CPBM_NS or MPAMCFG_PRI_NS[dspri].
> 
> As example, for resource index 0 (MPAMCFG_PART_SEL_NS[0]), it programs Priority value, and for resource index 1 ( MPAMCFG_PART_SEL_NS[1]), it programs portion bitmap value. In a way, Driver[1]
> Expects both these values to be supplied. May be James can correct me here.

I see obtaining the data from user space as separate from
writing the data to the hardware. resctrl maintains the
hardware configuration internally so it is possible to
have user space modify a portion of the configuration
while still being able to write the entire configuration
to hardware if that is required.


> This may also be solved when considering Peter's idea but since this work depends on other work that is not upstream it is difficult to envision the impact of any suggestions.
> 
> [>>] Initially, we have thought about these three approaches:
> 
> 1) Populate the resource control filesystem[2] with a new file that corresponds to new control. It requires Priority value to be encoded around portion bitmaps, and James has suggested we should go via
>    "schemata" file approach.
> 
>    I think, this is something Tony has pointed out in other thread.

Synchronizing writes to hardware with updates to separate
files may be a challenge.

> 
> 2) Second approach that we discussed internally is to have schemata for CPOR, and PPART separated by new line as mentioned/suggested by Peter, But it may require to tweak
>    the ARM MPAM device driver a bit. It was kind of toss-up between 2nd and 3nd approach :), and we went with the 3rd one.
> 
>    L3:0=XXXX
>    L3:0=PPART=X
> 
>    Will look into it again.

Tony has suggestions here. I think it would be a good exercise to
write a user space client to explore how the interface
can be made most convenient.

Reinette

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [RFC 02/12] arm_mpam: resctrl: Detect priority partitioning capability
  2023-08-15 15:27 ` [RFC 02/12] arm_mpam: resctrl: Detect priority partitioning capability Amit Singh Tomar
@ 2023-09-01 12:30   ` Jonathan Cameron
  0 siblings, 0 replies; 39+ messages in thread
From: Jonathan Cameron @ 2023-09-01 12:30 UTC (permalink / raw)
  To: Amit Singh Tomar
  Cc: linux-kernel, linux-arm-kernel, fenghua.yu, reinette.chatre,
	james.morse, gcherian, robh, peternewman

On Tue, 15 Aug 2023 20:57:02 +0530
Amit Singh Tomar <amitsinght@marvell.com> wrote:

> ARM MPAM supports different control that can be applied to different
> resources in the system, for instance priority partitioning control
> where priority value is generated from one MSC, propagates over
> interconnect to other MSC (known as downstream priority), or can be
> applied within an MSC for internal operations.

Hi Amit,

This talks about INTPRI as well as DSPRI, but only checks for DSPRI.
I couldn't work out why my INTPRI test wasn't resulting in anything
new turning up in schemata.

Should make that clear  However even better would be to enable both
and we definitely want an interface that allows for either or both.

Jonathan

> 
> This change lets the resctrl know the about MSC's priority partitioning
> capability.
> 
> Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
> ---
>  drivers/platform/mpam/mpam_resctrl.c | 14 ++++++++++++++
>  include/linux/resctrl.h              |  4 ++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/platform/mpam/mpam_resctrl.c b/drivers/platform/mpam/mpam_resctrl.c
> index 1dbfb6f6dd34..09618d9ceb1d 100644
> --- a/drivers/platform/mpam/mpam_resctrl.c
> +++ b/drivers/platform/mpam/mpam_resctrl.c
> @@ -435,6 +435,16 @@ static bool cache_has_usable_cpor(struct mpam_class *class)
>  	return (class->props.cpbm_wd <= RESCTRL_MAX_CBM);
>  }
>  
> +static bool cache_has_usable_priority_part(struct mpam_class *class)
> +{
> +	struct mpam_props *cprops = &class->props;
> +
> +	if (!mpam_has_feature(mpam_feat_dspri_part, cprops))
> +		return false;
> +
> +	return (class->props.dspri_wd <= RESCTRL_MAX_DSPRI);
> +}
> +
>  static bool cache_has_usable_csu(struct mpam_class *class)
>  {
>  	struct mpam_props *cprops;
> @@ -691,6 +701,7 @@ static int mpam_resctrl_resource_init(struct mpam_resctrl_res *res)
>  	    res->resctrl_res.rid == RDT_RESOURCE_L3) {
>  		bool has_csu = cache_has_usable_csu(class);
>  		bool has_mbwu = class_has_usable_mbwu(class);
> +		bool has_ppart = cache_has_usable_priority_part(class);
>  
>  		/* TODO: Scaling is not yet supported */
>  		r->cache.cbm_len = class->props.cpbm_wd;
> @@ -718,6 +729,9 @@ static int mpam_resctrl_resource_init(struct mpam_resctrl_res *res)
>  			exposed_alloc_capable = true;
>  		}
>  
> +		if (has_ppart)
> +			r->priority_cap = true;
> +
>  		/*
>  		 * MBWU counters may be 'local' or 'total' depending on where
>  		 * they are in the topology. If The counter is on the L3, its
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index 3ad308e9e226..a98ba5828211 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -38,6 +38,8 @@ int proc_resctrl_show(struct seq_file *m,
>   */
>  #define RESCTRL_MAX_CBM			32
>  
> +#define RESCTRL_MAX_DSPRI               63
> +
>  /* The format for packing fields into the u64 'id' exposed to user-space */
>  #define RESCTRL_ID_CLOSID	GENMASK_ULL(31, 0)
>  #define RESCTRL_ID_RMID		GENMASK_ULL(63, 32)
> @@ -195,6 +197,7 @@ struct resctrl_membw {
>   * @rid:		The index of the resource
>   * @alloc_capable:	Is allocation available on this machine
>   * @mon_capable:	Is monitor feature available on this machine
> + * @priority_capable:   Is priority partitioning feature available on this machine
>   * @num_rmid:		Number of RMIDs available
>   * @cache_level:	Which cache level defines scope of this resource
>   * @cache:		Cache allocation related data
> @@ -212,6 +215,7 @@ struct rdt_resource {
>  	int			rid;
>  	bool			alloc_capable;
>  	bool			mon_capable;
> +	bool                    priority_cap;
>  	int			num_rmid;
>  	int			cache_level;
>  	struct resctrl_cache	cache;


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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [RFC 01/12] arm_mpam: Handle resource instances mapped to different controls
  2023-08-15 15:27 ` [RFC 01/12] arm_mpam: Handle resource instances mapped to different controls Amit Singh Tomar
@ 2023-09-01 12:39   ` Jonathan Cameron
  0 siblings, 0 replies; 39+ messages in thread
From: Jonathan Cameron @ 2023-09-01 12:39 UTC (permalink / raw)
  To: Amit Singh Tomar
  Cc: linux-kernel, linux-arm-kernel, fenghua.yu, reinette.chatre,
	james.morse, gcherian, robh, peternewman

On Tue, 15 Aug 2023 20:57:01 +0530
Amit Singh Tomar <amitsinght@marvell.com> wrote:

Hi Amit,

My comments are likely to be fairly superficial, at least partly because
we haven't yet had a good opportunity for a review of James' MPAM series
on top of which this sits. (side note that I'm keen that James posts
that so we can have at it properly)

Note James has a new version of the tree out so applying this was
a little messy.

> At the moment, configuring multiple resource instances (mapped to same
> control) under a resource class is not supported. For instance, on MARVELL
> SoC MPAMF_IDR_NS[RIS_MAX] (under LLC MSC) is 0x2, and there are three
> different resource at index 0,1,2. These are enumerated in
> TAD_CMN_MPAM_RIS_E:
> 
> 0: MSC
> 1: LTG
> 2: DTG

Sometimes I think the MPAM spec is just too flexible. We are going to see
an awful lot of corner cases.

> 
> LLC MSC resource at index 1, and 2 have 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.

This patch description seems to first describe a clash between LTG and DTG
before going on to what the patch actually changes.
Do we need that initial description or can it be simplified
to just talk about...
> 
> LLC MSC resource at index 0 has the Priority partitioning features.
> If MPAMCFG_PART_SEL_NS[RIS] is set to 0 (MSC), then MPAMF_IDR_NS[HAS_PRI_PART]
> is set to 1, leaving HAS_CPOR_PART bit to 0. CPOR and PRI_PART are
> mutually exclusive resources as far configuration is concerned.
> 
> With this change, multiple resource instances that maps to different
> control, say LTG for CPOR, and MSC for PRI_PART is handled properly.
this second part?

> 
> Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
> ---
>  drivers/platform/mpam/mpam_devices.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/mpam/mpam_devices.c b/drivers/platform/mpam/mpam_devices.c
> index 589ff1ef2b6b..137cbff925ba 100644
> --- a/drivers/platform/mpam/mpam_devices.c
> +++ b/drivers/platform/mpam/mpam_devices.c
> @@ -1829,6 +1829,19 @@ static void mpam_enable_init_class_features(struct mpam_class *class)
>  	class->props = ris->props;
>  }
>  
> +/* 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.

Perhaps highlight that this case only matters if the two properties
are under different RIS.  I'm fairly sure that a valid implementation would
export both CPOR and DSPRI on the same RIS.


> + */
> +static void mpam_enable_club_class_features(struct mpam_class *class,
> +					    struct mpam_msc_ris *ris)
> +{
> +	class->props.features |= ris->props.features;
> +	class->props.cpbm_wd |= ris->props.cpbm_wd;
> +	class->props.dspri_wd |= ris->props.dspri_wd;
> +	class->props.num_csu_mon |= ris->props.num_csu_mon;
> +}
> +
>  /* Merge all the common resource features into class. */
>  static void mpam_enable_merge_features(void)
>  {
> @@ -1843,7 +1856,16 @@ static void mpam_enable_merge_features(void)
>  
>  		list_for_each_entry(comp, &class->components, class_list) {
>  			list_for_each_entry(ris, &comp->ris, comp_list) {
> -				__resource_props_mismatch(ris, class);
> +				/* There can be multiple resources under a class which is
> +				 * mapped to different controls, for instance L3 cache
> +				 * can have both CPOR and DSPRI implemented, and following

implemented with different RIS

> +				 * would avoid property mismatch later on when different
> +				 * resources are present.
> +				 */
> +				if (class->props.features != ris->props.features)
> +					mpam_enable_club_class_features(class, ris);

I think the __resource_props_mismatch still needs calling.  Consider
a pair or RIS A and B
A supports CPOR only with 12 partitions.
B supports CPOR with 10 partitions and DSPRI

This will expand the set to include both CPOR and DSPRI but
fail to catch the mismatch in partitions.

I'll hack some tests on top of my QEMU code to see if this works as
I think it does.

Jonathn

> +				else
> +					__resource_props_mismatch(ris, class);
>  
>  				class->nrdy_usec = max(class->nrdy_usec,
>  						     ris->msc->nrdy_usec);


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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [RFC 12/12] arm_mpam: Program Downstream priority value
  2023-08-15 15:27 ` [RFC 12/12] arm_mpam: Program Downstream priority value Amit Singh Tomar
@ 2023-09-01 13:17   ` Jonathan Cameron
  0 siblings, 0 replies; 39+ messages in thread
From: Jonathan Cameron @ 2023-09-01 13:17 UTC (permalink / raw)
  To: Amit Singh Tomar
  Cc: linux-kernel, linux-arm-kernel, fenghua.yu, reinette.chatre,
	james.morse, gcherian, robh, peternewman

On Tue, 15 Aug 2023 20:57:12 +0530
Amit Singh Tomar <amitsinght@marvell.com> wrote:

> Now that Downstream priorities values can be passed from resource control
> schemata file, let's program it into memory mapped Priority Partition
> Configuration Register.
> 
> Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
> ---
> TODO:
> 	* Invert Priority value based on DSPRI_0_IS_LOW, suggested
>           by James. 

Ah. In testing I just hit this as well as I was expecting the default
to change depending on 0_IS_LOW.



> ---
>  drivers/platform/mpam/mpam_devices.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/mpam/mpam_devices.c b/drivers/platform/mpam/mpam_devices.c
> index 59022e42920c..8af6424bb27b 100644
> --- a/drivers/platform/mpam/mpam_devices.c
> +++ b/drivers/platform/mpam/mpam_devices.c
> @@ -1153,8 +1153,12 @@ static void mpam_reprogram_ris_partid(struct mpam_msc_ris *ris, u16 partid,
>  
>  		if (mpam_has_feature(mpam_feat_intpri_part, rprops))
>  			pri_val |= FIELD_PREP(MPAMCFG_PRI_INTPRI, intpri);
> -		if (mpam_has_feature(mpam_feat_dspri_part, rprops))
> -			pri_val |= FIELD_PREP(MPAMCFG_PRI_DSPRI, dspri);
> +		if (mpam_has_feature(mpam_feat_dspri_part, rprops)) {
> +			if (mpam_has_feature(mpam_feat_dspri_part, cfg)) {
> +				pri_val |= FIELD_PREP(MPAMCFG_PRI_DSPRI, cfg->dspri & dspri);

Note that it's not as simple as inverting the value for DSPRI_0_IS_LOW setting being the opposite
as in that case dspri is st to 0 a few lines up... So this always ends up 0.

> +			} else
> +				pri_val |= FIELD_PREP(MPAMCFG_PRI_DSPRI, dspri);
> +		}
>  
>  		mpam_write_partsel_reg(msc, PRI, pri_val);
>  	}


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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [RFC 11/12] arm_mpam: Fix Downstream priority mask
  2023-08-15 15:27 ` [RFC 11/12] arm_mpam: Fix Downstream priority mask Amit Singh Tomar
@ 2023-09-01 13:32   ` Jonathan Cameron
  0 siblings, 0 replies; 39+ messages in thread
From: Jonathan Cameron @ 2023-09-01 13:32 UTC (permalink / raw)
  To: Amit Singh Tomar
  Cc: linux-kernel, linux-arm-kernel, fenghua.yu, reinette.chatre,
	james.morse, gcherian, robh, peternewman

On Tue, 15 Aug 2023 20:57:11 +0530
Amit Singh Tomar <amitsinght@marvell.com> wrote:

> MPAMF_PRI_IDR_NS[DSPRI_WD] determines the number of implemented bits in
> the downstream priority field (MPAMCFG_PRI_NS). For instance, if the value
> of DSPRI_WD is 4, then the maximum value for dspri is 0xf, and mask should
> be GENMASK(3,0).
> 
> But with current implementation, it turned out to be GENMASK(4,0) .i.e.
> 0x1f instead of 0xf.
> 
> u16 dspri = GENMASK(rprops->dspri_wd, 0);
> 
> Let's fix it, by subtracting 1 from DSPRI_WD value.
> 
> Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>

> ---
>  drivers/platform/mpam/mpam_devices.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/mpam/mpam_devices.c b/drivers/platform/mpam/mpam_devices.c
> index c0c83c04c77c..59022e42920c 100644
> --- a/drivers/platform/mpam/mpam_devices.c
> +++ b/drivers/platform/mpam/mpam_devices.c
> @@ -1099,7 +1099,7 @@ static void mpam_reprogram_ris_partid(struct mpam_msc_ris *ris, u16 partid,
>  	struct mpam_msc *msc = ris->msc;
>  	u16 bwa_fract = MPAMCFG_MBW_MAX_MAX;
>  	struct mpam_props *rprops = &ris->props;
> -	u16 dspri = GENMASK(rprops->dspri_wd, 0);
> +	u16 dspri = GENMASK((rprops->dspri_wd-1), 0);

Spaces around the -

>  	u16 intpri = GENMASK(rprops->intpri_wd, 0);

Please fix intpri as well. Argument is the same.

I'm assuming / hoping James will squash this into relevant patch in his tree.

>  
>  	lockdep_assert_held(&msc->lock);


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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
  2023-08-15 15:27 [RFC 00/12] ARM: MPAM: add support for priority partitioning control Amit Singh Tomar
                   ` (13 preceding siblings ...)
  2023-08-22  9:01 ` Peter Newman
@ 2023-09-01 14:42 ` Jonathan Cameron
  2023-09-01 15:04 ` Jonathan Cameron
  15 siblings, 0 replies; 39+ messages in thread
From: Jonathan Cameron @ 2023-09-01 14:42 UTC (permalink / raw)
  To: Amit Singh Tomar
  Cc: linux-kernel, linux-arm-kernel, fenghua.yu, reinette.chatre,
	james.morse, gcherian, robh, peternewman

On Tue, 15 Aug 2023 20:57:00 +0530
Amit Singh Tomar <amitsinght@marvell.com> wrote:

FWIW I've pushed out a QEMU tree with the MPAM patches posted previously
and an additional one enabling DSPRI on all the caches +
introspection and some additional sanity checks to pick up on the width
of DSPRI bug Amit fixed.

I used that to test this series and it seems fine subject to the TODO
on the final patch.

Note that's a simple model and doesn't actually do anything but is easy
to modify to poke corner cases / features you don't hardware for etc.

gitlab.com/jic23/qemu 

More info in the qemu patch series RFC cover letter:
https://lore.kernel.org/qemu-devel/20230808115713.2613-1-Jonathan.Cameron@huawei.com/#t
(there is an outstanding build issue for arm32, so don't build that :)

Jonathan

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
  2023-08-15 15:27 [RFC 00/12] ARM: MPAM: add support for priority partitioning control Amit Singh Tomar
                   ` (14 preceding siblings ...)
  2023-09-01 14:42 ` Jonathan Cameron
@ 2023-09-01 15:04 ` Jonathan Cameron
  15 siblings, 0 replies; 39+ messages in thread
From: Jonathan Cameron @ 2023-09-01 15:04 UTC (permalink / raw)
  To: Amit Singh Tomar
  Cc: linux-kernel, linux-arm-kernel, fenghua.yu, reinette.chatre,
	james.morse, gcherian, robh, peternewman, Wang ShaoBo

On Tue, 15 Aug 2023 20:57:00 +0530
Amit Singh Tomar <amitsinght@marvell.com> wrote:

> Arm Memory System Resource Partitioning and Monitoring (MPAM) supports
> different controls that can be applied to different resources in the system
> For instance, an optional priority partitioning control where priority
> value is generated from one MSC, propagates over interconnect to other MSC
> (known as downstream priority), or can be applied within an MSC for internal
> operations.

Hi Amit,

I'll most leave side commenting on the actual interface as lots of discussion has
occurred on that already so I'll wait for the next version and see where things
ended up :)

As a side note, openEuler has been carrying MPAM patches out of tree for a
long time now and have supported various features that align with available hardware.

The interface is partly described in. 
https://github.com/openeuler-mirror/kernel/commit/8139268b70398c37843a38bf8c7b243ad1f20c97

e.g.
   > mount -t resctrl resctrl /sys/fs/resctrl -o mbMax,mbMin,caPrio
   > cd /sys/fs/resctrl && cat schemata
     L3:0=0x7fff;1=0x7fff;2=0x7fff;3=0x7fff #default select cpbm as basic ctrl feature
     L3PRI:0=3;1=3;2=3;3=3
     MBMAX:0=100;1=100;2=100;3=100
     MBMIN:0=0;1=0;2=0;3=0

I'm not sure if this is the latest or not.
> 
> Marvell implementation of ARM MPAM supports priority partitioning control
> that allows LLC MSC to generate priority values that gets propagated (along with
> read/write request from upstream) to DDR Block.

This raises an interesting question of whether we should present these as controls
on the cache, or on the Memory controllers.  This is unlike INTPRI controls which
if present on the caches would definitely make sense presented there in resctrl.

If it were the case that downstream priority controls always just applied to one
block then listing them there (as DDR resource controls) might make sense -
however the section in the spec on "Through priorities" blocks that option as
these apply to everything downstream of which ever blocks set the priorities.

So whilst it's confusing I think you are right in presenting this as part of
the cache resource controls.  For the OpenEuler kernel that problem hasn't
arisen as focus is internal priority in the caches rather than downstream.


> Within the DDR block the
> priority values is mapped to different traffic class under DDR QoS strategy.
> The link[1] gives some idea about DDR QoS strategy, and terms like LPR, VPR
> and HPR.
> 

Jonathan


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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
  2023-08-24  8:52           ` Amit Singh Tomar
  2023-08-24 15:30             ` Luck, Tony
  2023-08-24 18:00             ` Reinette Chatre
@ 2024-01-11 20:56             ` Peter Newman
  2024-01-11 21:40               ` Tony Luck
  2 siblings, 1 reply; 39+ messages in thread
From: Peter Newman @ 2024-01-11 20:56 UTC (permalink / raw)
  To: Amit Singh Tomar
  Cc: Reinette Chatre, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, fenghua.yu@intel.com,
	james.morse@arm.com, George Cherian, robh@kernel.org, Luck, Tony

Hi Amit,

On Thu, Aug 24, 2023 at 1:52 AM Amit Singh Tomar <amitsinght@marvell.com> wrote:

> 2) Second approach that we discussed internally is to have schemata for CPOR, and PPART separated by new line as mentioned/suggested by Peter, But it may require to tweak
>    the ARM MPAM device driver a bit. It was kind of toss-up between 2nd and 3nd approach :), and we went with the 3rd one.
>
>    L3:0=XXXX
>    L3:0=PPART=X
>
>    Will look into it again.

I've been looking into the structure of the MPAM driver to understand
the difficulties here.

It seems the challenge with DSPRI is trying to stuff two different
control schema (partitioning, prioritization) into the L3
rdt_resource. The rdt_resource is still a mix of a hardware component
and user interface (schema line), which leads to  the
__resource_props_mismatch() function[1], which makes an arbitrary
choice (driven by resource index order) of which feature should be the
single control presented for each of the rdt_resources, the properties
of which the fields of its rdt_resource entry should describe.

It only seemed to work out for CDP emulation because the properties of
the two schema (L3CODE, L3DATA) for the L3 resource have the same CBM
properties.

My opinion is that the rdt_resource needs to be removed from
fs/resctrl and replaced with a structure to represent a control schema
and another to represent a monitor so that we don't find ourselves
unable to enumerate controls or monitors because a control or monitor
from the same hardware component has already been enumerated.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git/tree/drivers/platform/mpam/mpam_devices.c?h=mpam/snapshot/v6.7-rc2#n1810

-Peter

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
  2024-01-11 20:56             ` Peter Newman
@ 2024-01-11 21:40               ` Tony Luck
  2024-01-11 22:01                 ` Reinette Chatre
  0 siblings, 1 reply; 39+ messages in thread
From: Tony Luck @ 2024-01-11 21:40 UTC (permalink / raw)
  To: Peter Newman
  Cc: Amit Singh Tomar, Reinette Chatre, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, fenghua.yu@intel.com,
	james.morse@arm.com, George Cherian, robh@kernel.org

On Thu, Jan 11, 2024 at 12:56:34PM -0800, Peter Newman wrote:
> Hi Amit,
> 
> On Thu, Aug 24, 2023 at 1:52 AM Amit Singh Tomar <amitsinght@marvell.com> wrote:
> 
> > 2) Second approach that we discussed internally is to have schemata for CPOR, and PPART separated by new line as mentioned/suggested by Peter, But it may require to tweak
> >    the ARM MPAM device driver a bit. It was kind of toss-up between 2nd and 3nd approach :), and we went with the 3rd one.
> >
> >    L3:0=XXXX
> >    L3:0=PPART=X

I'm not sure having multiple lines for the same resource makes anything
clearer.  I preferred one of the earlier proposals like this one:

	L3:0=XXXX,PPART=X,CCAP=X;1=YYYY,CCAP=Y

This makes the schemata file self enumerate which optional capabilities
are present for each L3 instance (in the above example the second
instance doesn't support PPART, but does support CCAP).

Writes to the schemata file already accept partial information, so
the resctrl schemata_write() function should be coded to allow any of:

Just update CCAP for L3 instance 1":
	# echo "L3:1=CCAP=Z" > schemata

Update mask and CCAP for instance 0:
	# echo "L3:0=ABCD,CCAP=Q" > schemata

Update PPART on all instances:
	# echo "L3:0=PPART=M;1=PPART=N" > schemata

Legacy app that only comprehends partioning updates instance 1:
	# echo "L3:1=FFFF" > schemata

-Tony

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
  2024-01-11 21:40               ` Tony Luck
@ 2024-01-11 22:01                 ` Reinette Chatre
  2024-01-11 23:14                   ` Luck, Tony
  0 siblings, 1 reply; 39+ messages in thread
From: Reinette Chatre @ 2024-01-11 22:01 UTC (permalink / raw)
  To: Tony Luck, Peter Newman
  Cc: Amit Singh Tomar, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, fenghua.yu@intel.com,
	james.morse@arm.com, George Cherian, robh@kernel.org



On 1/11/2024 1:40 PM, Tony Luck wrote:
> On Thu, Jan 11, 2024 at 12:56:34PM -0800, Peter Newman wrote:
>> Hi Amit,
>>
>> On Thu, Aug 24, 2023 at 1:52 AM Amit Singh Tomar <amitsinght@marvell.com> wrote:
>>
>>> 2) Second approach that we discussed internally is to have schemata for CPOR, and PPART separated by new line as mentioned/suggested by Peter, But it may require to tweak
>>>    the ARM MPAM device driver a bit. It was kind of toss-up between 2nd and 3nd approach :), and we went with the 3rd one.
>>>
>>>    L3:0=XXXX
>>>    L3:0=PPART=X
> 
> I'm not sure having multiple lines for the same resource makes anything
> clearer.  I preferred one of the earlier proposals like this one:
> 
> 	L3:0=XXXX,PPART=X,CCAP=X;1=YYYY,CCAP=Y

This assumes that all tools (public and private) that currently parse the schemata
file will be able to handle this additional information seamlessly.

Reinette


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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
  2024-01-11 22:01                 ` Reinette Chatre
@ 2024-01-11 23:14                   ` Luck, Tony
  2024-01-11 23:31                     ` Reinette Chatre
  0 siblings, 1 reply; 39+ messages in thread
From: Luck, Tony @ 2024-01-11 23:14 UTC (permalink / raw)
  To: Chatre, Reinette, Peter Newman
  Cc: Amit Singh Tomar, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, Yu, Fenghua,
	james.morse@arm.com, George Cherian, robh@kernel.org

>> I'm not sure having multiple lines for the same resource makes anything
>> clearer.  I preferred one of the earlier proposals like this one:
>> 
>> 	L3:0=XXXX,PPART=X,CCAP=X;1=YYYY,CCAP=Y
>
> This assumes that all tools (public and private) that currently parse the schemata
> file will be able to handle this additional information seamlessly.

Reinette,

Yes. If there are tools that *read* schemata files, they will be surprised by this extra information.

But that also applies if the "extra" information is moved to a second line that also begins with "L3:".

Tools that *write* schemata files should be OK as long as the kernel will still accept:

  # echo "L3:1=fff" > schemata

E.g. the Linux selftests in tools/testing/selftests/resctrl/ should still run without
any modification.

The "separate line" option could work if the prefix isn't "L3:".  E.g.

L3:0=XXXX;1=YYYY
L3PPART:0=X
L3CCAP:0=X;1=Y

If these options are asymmetrically available on cache instances, these extra
lines won't have every L3 cache instance listed.

-Tony

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control
  2024-01-11 23:14                   ` Luck, Tony
@ 2024-01-11 23:31                     ` Reinette Chatre
  0 siblings, 0 replies; 39+ messages in thread
From: Reinette Chatre @ 2024-01-11 23:31 UTC (permalink / raw)
  To: Luck, Tony, Peter Newman
  Cc: Amit Singh Tomar, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, Yu, Fenghua,
	james.morse@arm.com, George Cherian, robh@kernel.org

Hi Tony,

On 1/11/2024 3:14 PM, Luck, Tony wrote:
>>> I'm not sure having multiple lines for the same resource makes anything
>>> clearer.  I preferred one of the earlier proposals like this one:
>>>
>>> 	L3:0=XXXX,PPART=X,CCAP=X;1=YYYY,CCAP=Y
>>
>> This assumes that all tools (public and private) that currently parse the schemata
>> file will be able to handle this additional information seamlessly.
> 
> Reinette,
> 
> Yes. If there are tools that *read* schemata files, they will be surprised by this extra information.
> 
> But that also applies if the "extra" information is moved to a second line that also begins with "L3:".
> 
> Tools that *write* schemata files should be OK as long as the kernel will still accept:
> 
>   # echo "L3:1=fff" > schemata
> 
> E.g. the Linux selftests in tools/testing/selftests/resctrl/ should still run without
> any modification.
> 
> The "separate line" option could work if the prefix isn't "L3:".  E.g.
> 
> L3:0=XXXX;1=YYYY
> L3PPART:0=X
> L3CCAP:0=X;1=Y
> 
> If these options are asymmetrically available on cache instances, these extra
> lines won't have every L3 cache instance listed.

I think we are going in circles here. I shared my concern about user space
breakage a while ago [1] in response to your previous proposal and this new proposal
seems to match where this thread ended [2] last year.

Reinette

[1] https://lore.kernel.org/lkml/be51596e-2e62-2fb9-4176-b0b2a2abb1d3@intel.com/
[2] https://lore.kernel.org/lkml/20230901160451.00001f75@Huawei.com/

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

end of thread, other threads:[~2024-01-11 23:32 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-15 15:27 [RFC 00/12] ARM: MPAM: add support for priority partitioning control Amit Singh Tomar
2023-08-15 15:27 ` [RFC 01/12] arm_mpam: Handle resource instances mapped to different controls Amit Singh Tomar
2023-09-01 12:39   ` Jonathan Cameron
2023-08-15 15:27 ` [RFC 02/12] arm_mpam: resctrl: Detect priority partitioning capability Amit Singh Tomar
2023-09-01 12:30   ` Jonathan Cameron
2023-08-15 15:27 ` [RFC 03/12] arm_mpam: resctrl: Define new schemata format for priority partition Amit Singh Tomar
2023-08-15 15:27 ` [RFC 04/12] fs/resctrl: Obtain CPBM upon priority partition presence Amit Singh Tomar
2023-08-15 15:27 ` [RFC 05/12] fs/resctrl: Set-up downstream priority partition resources Amit Singh Tomar
2023-08-17 17:39   ` Fenghua Yu
2023-08-15 15:27 ` [RFC 06/12] fs/resctrl: Extend schemata read for priority partition control Amit Singh Tomar
2023-08-17 17:42   ` Fenghua Yu
2023-08-15 15:27 ` [RFC 07/12] arm_mpam: resctrl: Retrieve priority values from arch code Amit Singh Tomar
2023-08-15 15:27 ` [RFC 08/12] fs/resctrl: Schemata write only for intended resource Amit Singh Tomar
2023-08-15 15:27 ` [RFC 09/12] fs/resctrl: Extend schemata write for priority partition control Amit Singh Tomar
2023-08-17 17:27   ` Fenghua Yu
2023-08-17 17:53   ` Fenghua Yu
2023-08-15 15:27 ` [RFC 10/12] arm_mpam: resctrl: Facilitate writing downstream priority value Amit Singh Tomar
2023-08-15 15:27 ` [RFC 11/12] arm_mpam: Fix Downstream priority mask Amit Singh Tomar
2023-09-01 13:32   ` Jonathan Cameron
2023-08-15 15:27 ` [RFC 12/12] arm_mpam: Program Downstream priority value Amit Singh Tomar
2023-09-01 13:17   ` Jonathan Cameron
2023-08-17 19:11 ` [RFC 00/12] ARM: MPAM: add support for priority partitioning control Reinette Chatre
2023-08-17 20:29   ` Reinette Chatre
2023-08-22 12:44   ` [EXT] " Amit Singh Tomar
2023-08-23 19:06     ` Reinette Chatre
2023-08-23 21:33       ` Amit Singh Tomar
2023-08-23 22:20         ` Reinette Chatre
2023-08-23 22:36           ` Luck, Tony
2023-08-24  8:52           ` Amit Singh Tomar
2023-08-24 15:30             ` Luck, Tony
2023-08-24 18:00             ` Reinette Chatre
2024-01-11 20:56             ` Peter Newman
2024-01-11 21:40               ` Tony Luck
2024-01-11 22:01                 ` Reinette Chatre
2024-01-11 23:14                   ` Luck, Tony
2024-01-11 23:31                     ` Reinette Chatre
2023-08-22  9:01 ` Peter Newman
2023-09-01 14:42 ` Jonathan Cameron
2023-09-01 15:04 ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).