Re: [PATCH v8 next 04/10] arm_mpam: Refactor rmid to reqPARTID/PMG mapping

[Zeng Heng <zengheng4@huawei.com>]

Hi James,

>> @@ -478,6 +518,7 @@ static int __read_mon(struct mpam_resctrl_mon *mon, struct mpam_component *mon_c
>>   		      enum resctrl_conf_type cdp_type, u32 closid, u32 rmid, u64 *val)
>>   {
>>   	struct mon_cfg cfg;
>> +	u32 reqpartid = rmid2reqpartid(rmid);
>>   
>>   	if (!mpam_is_enabled())
>>   		return -EINVAL;
>> @@ -493,8 +534,8 @@ static int __read_mon(struct mpam_resctrl_mon *mon, struct mpam_component *mon_c
>>   	cfg = (struct mon_cfg) {
>>   		.mon = mon_idx,
>>   		.match_pmg = true,
>> -		.partid = closid,
>> -		.pmg = rmid,
>> +		.partid = (cdp_type == CDP_CODE) ? reqpartid + 1 : reqpartid,
>> +		.pmg = rmid2pmg(rmid),
> 
> Not using the CLOSID here breaks multiple control groups.
> 


After carefully reviewing your comments and Martin's patch series,
I tried to understand why there is insistence that CLOSID information
is necessary to support multiple control groups, but that is actually
not the case.

Before proceeding, please allow me to refer to base_partid as CPARTID
(control partition ID; I'm no longer borrowing the intPARTID concept
here). The associated partids_per_closid all share the same control
scheme.

The partids derived from partids_per_closid under a given CPARTID,
I will call MPARTID (monitor partition ID; no longer borrowing the
reqPARTID concept). These represent the PARTIDs used for different
monitoring groups under the same control scheme.


I've summarized the ID translation schemes from James and Martin as
follows:

  +-------------------------------+------------------+
  |            CLOSID       |{CDP}|       RMID       |
  +-------------------------------+------------+-----+
  |            MPARTID                         | PMG |
  |      CPARTID(or MPARTID_hi)   : MPARTID_lo |     |
  +--------------------------------------------+-----+

Where closid = cpartid (base_partid or mpartid_hi),
       rmid   = mpartid_lo * pmg_num + pmg,
with mpartid_lo in the range [0, partids_per_closid).

In this scheme, CLOSID and RMID are coupled together to form MPARTID,
which represents the monitor group PARTID.


In current patchset design, decoupling CLOSID and RMID, letting them
represent CPARTID and MPARTID respectively:

  +---------------------------------------------+
  |                    CLOSID             |{CDP}|
  +---------------------------------------------+
  |                    CPARTID                  |
  +---------------------------------------------+

  +---------------------------------------------+
  |                     RMID                    |
  +---------------------------------------+-----+
  |                   MPARTID             | PMG |
  |  MPARTID_hi(or CPARTID)  : MPARTID_lo |     |
  +---------------------------------------+-----+

Where closid  = cpartid (base_partid or mpartid_hi),
       rmid    = mpartid * pmg_num + pmg,
   and mpartid = mpartid_hi * partids_per_closid + mpartid_lo .

The design intent is to decouple CLOSID and RMID, rather than having
RMID depend on CLOSID to derive MPARTID. This decoupling is essential
for future dynamic allocation, because the relationship between MPARTID
and CPARTID must rely on table lookup rather than linear mapping. If
don't decouple in the static allocation design, we would need another
refactor when considering dynamic allocation extensibility.

The control path uses CLOSID alone (CPARTID), and the monitor path uses
RMID alone (the (MPARTID, PMG) pair). This definition also aligns
closely with the native resctrl concepts: CLOSID (Class of Service ID,
corresponding to CPARTID) and RMID (Resource Monitor ID, corresponding
to the (MPARTID, PMG) pair).


In the end, the number of control groups is determined by the number of
CPARTIDs. Both of these ID translation schemes support multiple control
groups.

Please allow me to rework the patch series into v9 based on my current
patches, incorporating your review feedback.


Best Regards,
Zeng Heng