resctrl has three types of control, these emerge from the way the
architecture initialises a number of properties in struct rdt_resource.

A group of these properties need to be set the same on all architectures,
it would be better to specify the format the schema entry should use, and
allow resctrl to generate all the other properties it needs. This avoids
architectures having divergant behaviour here.

Add a schema format enum, and as a first use, replace the fflags member
of struct rdt_resource.

The MBA schema has a different format between AMD and Intel systems.
The schema_fmt property is changed by __rdt_get_mem_config_amd() to
enable the MBPS format.

Signed-off-by: James Morse <james.morse@arm.com>
---
Changes since v2:
 * This patch is new
---
 arch/x86/kernel/cpu/resctrl/core.c     | 13 +++++++++----
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 17 +++++++++++++++--
 include/linux/resctrl.h                | 16 ++++++++++++++--
 3 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index d04e157dd7f7..a72fd53e0ffe 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -68,11 +68,11 @@ struct rdt_hw_resource rdt_resources_all[] = {
 		.r_resctrl = {
 			.rid			= RDT_RESOURCE_L3,
 			.name			= "L3",
+			.schema_fmt		= RESCTRL_SCHEMA_BITMAP,
 			.cache_level		= 3,
 			.domains		= domain_init(RDT_RESOURCE_L3),
 			.parse_ctrlval		= parse_cbm,
 			.format_str		= "%d=%0*x",
-			.fflags			= RFTYPE_RES_CACHE,
 		},
 		.msr_base		= MSR_IA32_L3_CBM_BASE,
 		.msr_update		= cat_wrmsr,
@@ -82,11 +82,11 @@ struct rdt_hw_resource rdt_resources_all[] = {
 		.r_resctrl = {
 			.rid			= RDT_RESOURCE_L2,
 			.name			= "L2",
+			.schema_fmt		= RESCTRL_SCHEMA_BITMAP,
 			.cache_level		= 2,
 			.domains		= domain_init(RDT_RESOURCE_L2),
 			.parse_ctrlval		= parse_cbm,
 			.format_str		= "%d=%0*x",
-			.fflags			= RFTYPE_RES_CACHE,
 		},
 		.msr_base		= MSR_IA32_L2_CBM_BASE,
 		.msr_update		= cat_wrmsr,
@@ -96,11 +96,15 @@ struct rdt_hw_resource rdt_resources_all[] = {
 		.r_resctrl = {
 			.rid			= RDT_RESOURCE_MBA,
 			.name			= "MB",
+			/*
+			 * MBA schema_fmt is modified by
+			 * __rdt_get_mem_config_amd()
+			 */
+			.schema_fmt		= RESCTRL_SCHEMA_PERCENTAGE,
 			.cache_level		= 3,
 			.domains		= domain_init(RDT_RESOURCE_MBA),
 			.parse_ctrlval		= parse_bw,
 			.format_str		= "%d=%*u",
-			.fflags			= RFTYPE_RES_MB,
 		},
 	},
 	[RDT_RESOURCE_SMBA] =
@@ -108,11 +112,11 @@ struct rdt_hw_resource rdt_resources_all[] = {
 		.r_resctrl = {
 			.rid			= RDT_RESOURCE_SMBA,
 			.name			= "SMBA",
+			.schema_fmt		= RESCTRL_SCHEMA_MBPS,
 			.cache_level		= 3,
 			.domains		= domain_init(RDT_RESOURCE_SMBA),
 			.parse_ctrlval		= parse_bw,
 			.format_str		= "%d=%*u",
-			.fflags			= RFTYPE_RES_MB,
 		},
 	},
 };
@@ -253,6 +257,7 @@ static bool __rdt_get_mem_config_amd(struct rdt_resource *r)
 	cpuid_count(0x80000020, subleaf, &eax, &ebx, &ecx, &edx);
 	hw_res->num_closid = edx + 1;
 	r->default_ctrl = 1 << eax;
+	r->schema_fmt = RESCTRL_SCHEMA_MBPS;
 
 	/* AMD does not use delay */
 	r->membw.delay_linear = false;
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index e3edc41882dc..b12307d465bc 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -2162,6 +2162,19 @@ static int rdtgroup_mkdir_info_resdir(void *priv, char *name,
 	return ret;
 }
 
+static u32 fflags_from_resource(struct rdt_resource *r)
+{
+	switch (r->schema_fmt) {
+	case RESCTRL_SCHEMA_BITMAP:
+		return RFTYPE_RES_CACHE;
+	case RESCTRL_SCHEMA_PERCENTAGE:
+	case RESCTRL_SCHEMA_MBPS:
+		return RFTYPE_RES_MB;
+	}
+
+	return WARN_ON_ONCE(1);
+}
+
 static int rdtgroup_create_info_dir(struct kernfs_node *parent_kn)
 {
 	struct resctrl_schema *s;
@@ -2182,14 +2195,14 @@ static int rdtgroup_create_info_dir(struct kernfs_node *parent_kn)
 	/* loop over enabled controls, these are all alloc_capable */
 	list_for_each_entry(s, &resctrl_schema_all, list) {
 		r = s->res;
-		fflags = r->fflags | RFTYPE_CTRL_INFO;
+		fflags =  fflags_from_resource(r) | RFTYPE_CTRL_INFO;
 		ret = rdtgroup_mkdir_info_resdir(s, s->name, fflags);
 		if (ret)
 			goto out_destroy;
 	}
 
 	for_each_mon_capable_rdt_resource(r) {
-		fflags = r->fflags | RFTYPE_MON_INFO;
+		fflags =  fflags_from_resource(r) | RFTYPE_MON_INFO;
 		sprintf(name, "%s_MON", r->name);
 		ret = rdtgroup_mkdir_info_resdir(r, name, fflags);
 		if (ret)
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index 168cc9510069..4822abbc08c8 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -160,6 +160,18 @@ struct resctrl_membw {
 struct rdt_parse_data;
 struct resctrl_schema;
 
+/**
+ * enum resctrl_schema_fmt - The format user-space provides for a schema.
+ * @RESCTRL_SCHEMA_BITMAP:	The schema is a bitmap in hex.
+ * @RESCTRL_SCHEMA_PERCENTAGE:	The schema is a decimal percentage value.
+ * @RESCTRL_SCHEMA_MBPS:	The schema is a decimal MBps value.
+ */
+enum resctrl_schema_fmt {
+	RESCTRL_SCHEMA_BITMAP,
+	RESCTRL_SCHEMA_PERCENTAGE,
+	RESCTRL_SCHEMA_MBPS,
+};
+
 /**
  * struct rdt_resource - attributes of a resctrl resource
  * @rid:		The index of the resource
@@ -175,8 +187,8 @@ struct resctrl_schema;
  * @default_ctrl:	Specifies default cache cbm or memory B/W percent.
  * @format_str:		Per resource format string to show domain value
  * @parse_ctrlval:	Per resource function pointer to parse control values
+ * @schema_fmt:	Which format string and parser is used for this schema.
  * @evt_list:		List of monitoring events
- * @fflags:		flags to choose base and info files
  * @cdp_capable:	Is the CDP feature available on this resource
  */
 struct rdt_resource {
@@ -195,8 +207,8 @@ struct rdt_resource {
 	int			(*parse_ctrlval)(struct rdt_parse_data *data,
 						 struct resctrl_schema *s,
 						 struct rdt_domain *d);
+	enum resctrl_schema_fmt	schema_fmt;
 	struct list_head	evt_list;
-	unsigned long		fflags;
 	bool			cdp_capable;
 };
 
-- 
2.39.2

Hi James,

On 6/14/24 7:59 AM, James Morse wrote:
> resctrl has three types of control, these emerge from the way the
> architecture initialises a number of properties in struct rdt_resource.
> 
> A group of these properties need to be set the same on all architectures,
> it would be better to specify the format the schema entry should use, and
> allow resctrl to generate all the other properties it needs. This avoids
> architectures having divergant behaviour here.

divergant -> divergent ?

> 
> Add a schema format enum, and as a first use, replace the fflags member
> of struct rdt_resource.
> 
> The MBA schema has a different format between AMD and Intel systems.
> The schema_fmt property is changed by __rdt_get_mem_config_amd() to
> enable the MBPS format.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> ---

...

> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index e3edc41882dc..b12307d465bc 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -2162,6 +2162,19 @@ static int rdtgroup_mkdir_info_resdir(void *priv, char *name,
>   	return ret;
>   }
>   
> +static u32 fflags_from_resource(struct rdt_resource *r)
> +{
> +	switch (r->schema_fmt) {
> +	case RESCTRL_SCHEMA_BITMAP:
> +		return RFTYPE_RES_CACHE;
> +	case RESCTRL_SCHEMA_PERCENTAGE:
> +	case RESCTRL_SCHEMA_MBPS:
> +		return RFTYPE_RES_MB;
> +	}
> +
> +	return WARN_ON_ONCE(1);
> +}
> +

The fflags returned specifies which files will be associated with the resource
in the "info" directory. Basing this on a property of the schema does not look
right to me. I understand that many of the info files relate to, for example,
information related to the bitmap used by the cache, but that is not the same for
info files related to the MBA resource (all info files related to MBA resource
are not about the schema property format).

I do not think the type of values of a schema should dictate which files
appear in the info directory. Doesn't MPAM support percentage for cache resources
and bitmaps for memory resources?

Can the fflags rather depend on the resource type itself, by using the rid?

>   static int rdtgroup_create_info_dir(struct kernfs_node *parent_kn)
>   {
>   	struct resctrl_schema *s;
> @@ -2182,14 +2195,14 @@ static int rdtgroup_create_info_dir(struct kernfs_node *parent_kn)
>   	/* loop over enabled controls, these are all alloc_capable */
>   	list_for_each_entry(s, &resctrl_schema_all, list) {
>   		r = s->res;
> -		fflags = r->fflags | RFTYPE_CTRL_INFO;
> +		fflags =  fflags_from_resource(r) | RFTYPE_CTRL_INFO;

(please watch for extra spaces)

>   		ret = rdtgroup_mkdir_info_resdir(s, s->name, fflags);
>   		if (ret)
>   			goto out_destroy;
>   	}
>   
>   	for_each_mon_capable_rdt_resource(r) {
> -		fflags = r->fflags | RFTYPE_MON_INFO;
> +		fflags =  fflags_from_resource(r) | RFTYPE_MON_INFO;

(please watch for extra spaces)

Reinette

Hi Reinette,

On 28/06/2024 17:43, Reinette Chatre wrote:
> On 6/14/24 7:59 AM, James Morse wrote:
>> resctrl has three types of control, these emerge from the way the
>> architecture initialises a number of properties in struct rdt_resource.
>>
>> A group of these properties need to be set the same on all architectures,
>> it would be better to specify the format the schema entry should use, and
>> allow resctrl to generate all the other properties it needs. This avoids
>> architectures having divergant behaviour here.
> 
> divergant -> divergent ?
> 
>>
>> Add a schema format enum, and as a first use, replace the fflags member
>> of struct rdt_resource.
>>
>> The MBA schema has a different format between AMD and Intel systems.
>> The schema_fmt property is changed by __rdt_get_mem_config_amd() to
>> enable the MBPS format.

>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index e3edc41882dc..b12307d465bc 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -2162,6 +2162,19 @@ static int rdtgroup_mkdir_info_resdir(void *priv, char *name,
>>       return ret;
>>   }
>>   +static u32 fflags_from_resource(struct rdt_resource *r)
>> +{
>> +    switch (r->schema_fmt) {
>> +    case RESCTRL_SCHEMA_BITMAP:
>> +        return RFTYPE_RES_CACHE;
>> +    case RESCTRL_SCHEMA_PERCENTAGE:
>> +    case RESCTRL_SCHEMA_MBPS:
>> +        return RFTYPE_RES_MB;
>> +    }
>> +
>> +    return WARN_ON_ONCE(1);
>> +}
>> +
> 
> The fflags returned specifies which files will be associated with the resource
> in the "info" directory. Basing this on a property of the schema does not look
> right to me. I understand that many of the info files relate to, for example,
> information related to the bitmap used by the cache,

Do we agree that some of them are?

One reason for doing this is it decouples the parsing and management of bitmaps from "this
is the L3 cache", which will make it much easier to support bitmaps on some other kind of
resource.

Ultimately I'd like to expose these to user-space, so that user-space can work out how to
configure resources it doesn't recognise. Today '100' could be a percentage, a bitmap, or
a value in MB/s. Today some knowledge of the control type is needed to work this out.


> but that is not the same for
> info files related to the MBA resource (all info files related to MBA resource
> are not about the schema property format).

Hmmm, because the files min_bandwidth and bandwidth_gran both have bandwidth in their name?

I agree 'delay_linear' and 'thread_throttle_mode' are a bit strange.


> I do not think the type of values of a schema should dictate which files
> appear in the info directory.

Longer term I think this will be a problem. We probably only have 3 types of control:
percentage, bitmap and MB/s... but if each resource on each architecture adds files here
the list will quickly grow. User-space won't be able to work out how to configure a
resource type it hadn't seen before.

This may not be the time - but I think eventually resctrl shouldn't have to care about
what resources the architecture is presenting.
For these files, we may need to duplicate 'min_bandwidth' as 'min_percentage'. MBA would
have both, but any new controls using percentage wouldn't expose them.


> Doesn't MPAM support percentage for cache resources
> and bitmaps for memory resources?

It can have fixed-point-fractions and bitmaps for both caches and memory. Unfortunately
everything in MPAM is optional - the driver converts whatever it finds for memory
bandwidth to a percentage as that is what resctrl and user-space expect.
I can't do the same for cache controls as bitmaps implicitly isolate portions, something
that can't be done with the fractional control. So far everyone has built the bitmaps
because its the easiest implementation - but I have had requests to support the cache
fixed-point-fraction. Doing it as a percentage is least invasive to resctrl...


> Can the fflags rather depend on the resource type itself, by using the rid?

Sure.



Thanks,

James

>> @@ -2182,14 +2195,14 @@ static int rdtgroup_create_info_dir(struct kernfs_node *parent_kn)
>>       /* loop over enabled controls, these are all alloc_capable */
>>       list_for_each_entry(s, &resctrl_schema_all, list) {
>>           r = s->res;
>> -        fflags = r->fflags | RFTYPE_CTRL_INFO;
>> +        fflags =  fflags_from_resource(r) | RFTYPE_CTRL_INFO;
> 
> (please watch for extra spaces)
> 
>>           ret = rdtgroup_mkdir_info_resdir(s, s->name, fflags);
>>           if (ret)
>>               goto out_destroy;
>>       }
>>         for_each_mon_capable_rdt_resource(r) {
>> -        fflags = r->fflags | RFTYPE_MON_INFO;
>> +        fflags =  fflags_from_resource(r) | RFTYPE_MON_INFO;
> 
> (please watch for extra spaces)
> 
> Reinette

Hi James,

On 7/1/24 11:17 AM, James Morse wrote:
> Hi Reinette,
> 
> On 28/06/2024 17:43, Reinette Chatre wrote:
>> On 6/14/24 7:59 AM, James Morse wrote:
>>> resctrl has three types of control, these emerge from the way the
>>> architecture initialises a number of properties in struct rdt_resource.
>>>
>>> A group of these properties need to be set the same on all architectures,
>>> it would be better to specify the format the schema entry should use, and
>>> allow resctrl to generate all the other properties it needs. This avoids
>>> architectures having divergant behaviour here.
>>
>> divergant -> divergent ?
>>
>>>
>>> Add a schema format enum, and as a first use, replace the fflags member
>>> of struct rdt_resource.
>>>
>>> The MBA schema has a different format between AMD and Intel systems.
>>> The schema_fmt property is changed by __rdt_get_mem_config_amd() to
>>> enable the MBPS format.
> 
>>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>>> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>>> index e3edc41882dc..b12307d465bc 100644
>>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>>> @@ -2162,6 +2162,19 @@ static int rdtgroup_mkdir_info_resdir(void *priv, char *name,
>>>        return ret;
>>>    }
>>>    +static u32 fflags_from_resource(struct rdt_resource *r)
>>> +{
>>> +    switch (r->schema_fmt) {
>>> +    case RESCTRL_SCHEMA_BITMAP:
>>> +        return RFTYPE_RES_CACHE;
>>> +    case RESCTRL_SCHEMA_PERCENTAGE:
>>> +    case RESCTRL_SCHEMA_MBPS:
>>> +        return RFTYPE_RES_MB;
>>> +    }
>>> +
>>> +    return WARN_ON_ONCE(1);
>>> +}
>>> +
>>
>> The fflags returned specifies which files will be associated with the resource
>> in the "info" directory. Basing this on a property of the schema does not look
>> right to me. I understand that many of the info files relate to, for example,
>> information related to the bitmap used by the cache,
> 
> Do we agree that some of them are?
> 
> One reason for doing this is it decouples the parsing and management of bitmaps from "this
> is the L3 cache", which will make it much easier to support bitmaps on some other kind of
> resource.

The way I see it is that it changes the meaning of the RFTYPE_RES_CACHE flag from "this is a
file related to the cache resource" to "this is a file containing a bitmap property".
It prevents us from easily adding a file related to the cache resource, which
the info directory is intended to contain.

> 
> Ultimately I'd like to expose these to user-space, so that user-space can work out how to
> configure resources it doesn't recognise. Today '100' could be a percentage, a bitmap, or
> a value in MB/s. Today some knowledge of the control type is needed to work this out.
> 
> 
>> but that is not the same for
>> info files related to the MBA resource (all info files related to MBA resource
>> are not about the schema property format).
> 
> Hmmm, because the files min_bandwidth and bandwidth_gran both have bandwidth in their name?
> 
> I agree 'delay_linear' and 'thread_throttle_mode' are a bit strange.

Right. This is not a clean association.

> 
> 
>> I do not think the type of values of a schema should dictate which files
>> appear in the info directory.
> 
> Longer term I think this will be a problem. We probably only have 3 types of control:
> percentage, bitmap and MB/s... but if each resource on each architecture adds files here
> the list will quickly grow. User-space won't be able to work out how to configure a
> resource type it hadn't seen before.

That is fair. This makes the type of control a property of the resource as is done in this
series. Perhaps this can be exposed to user space via the info directory?

Possibly the files related to control can have new flags that that reflect the control type
instead of the resource. For example, "bit_usage" currently has
"RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE" and that could be (for lack of better
term) "RFTYPE_CTRL_INFO | RFTYPE_CTRL_BITMAP" to disconnect the control type from the
resource. Doing so may then map nicely to the fflags_from_resource() in this patch that
connects the schema format to the _control_ type flag. As we have found there is not
a clear mapping between the control type and the resource type so I expect RFTYPE_RES_CACHE
and RFTYPE_RES_MB to remain and be associated with files that contain information
specific to that resource. This enables future additions of files containing cache specific
(non-bitmap) properties to still be added (with RFTYPE_RES_CACHE flag) without impacting
everything that uses a bitmap.

What do you think?

> 
> This may not be the time - but I think eventually resctrl shouldn't have to care about
> what resources the architecture is presenting.
> For these files, we may need to duplicate 'min_bandwidth' as 'min_percentage'. MBA would
> have both, but any new controls using percentage wouldn't expose them.
> 
> 
>> Doesn't MPAM support percentage for cache resources
>> and bitmaps for memory resources?
> 
> It can have fixed-point-fractions and bitmaps for both caches and memory. Unfortunately
> everything in MPAM is optional - the driver converts whatever it finds for memory
> bandwidth to a percentage as that is what resctrl and user-space expect.
> I can't do the same for cache controls as bitmaps implicitly isolate portions, something
> that can't be done with the fractional control. So far everyone has built the bitmaps
> because its the easiest implementation - but I have had requests to support the cache
> fixed-point-fraction. Doing it as a percentage is least invasive to resctrl...
> 
> 
>> Can the fflags rather depend on the resource type itself, by using the rid?
> 
> Sure.
> 

Reinette

Hi Reinette,

On 01/07/2024 22:09, Reinette Chatre wrote:
> On 7/1/24 11:17 AM, James Morse wrote:
>> On 28/06/2024 17:43, Reinette Chatre wrote:
>>> On 6/14/24 7:59 AM, James Morse wrote:
>>>> resctrl has three types of control, these emerge from the way the
>>>> architecture initialises a number of properties in struct rdt_resource.
>>>>
>>>> A group of these properties need to be set the same on all architectures,
>>>> it would be better to specify the format the schema entry should use, and
>>>> allow resctrl to generate all the other properties it needs. This avoids
>>>> architectures having divergant behaviour here.
>>>
>>> divergant -> divergent ?
>>>
>>>>
>>>> Add a schema format enum, and as a first use, replace the fflags member
>>>> of struct rdt_resource.
>>>>
>>>> The MBA schema has a different format between AMD and Intel systems.
>>>> The schema_fmt property is changed by __rdt_get_mem_config_amd() to
>>>> enable the MBPS format.
>>
>>>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>>>> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>>>> index e3edc41882dc..b12307d465bc 100644
>>>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>>>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>>>> @@ -2162,6 +2162,19 @@ static int rdtgroup_mkdir_info_resdir(void *priv, char *name,
>>>>        return ret;
>>>>    }
>>>>    +static u32 fflags_from_resource(struct rdt_resource *r)
>>>> +{
>>>> +    switch (r->schema_fmt) {
>>>> +    case RESCTRL_SCHEMA_BITMAP:
>>>> +        return RFTYPE_RES_CACHE;
>>>> +    case RESCTRL_SCHEMA_PERCENTAGE:
>>>> +    case RESCTRL_SCHEMA_MBPS:
>>>> +        return RFTYPE_RES_MB;
>>>> +    }
>>>> +
>>>> +    return WARN_ON_ONCE(1);
>>>> +}
>>>> +
>>>
>>> The fflags returned specifies which files will be associated with the resource
>>> in the "info" directory. Basing this on a property of the schema does not look
>>> right to me. I understand that many of the info files relate to, for example,
>>> information related to the bitmap used by the cache,
>>
>> Do we agree that some of them are?
>>
>> One reason for doing this is it decouples the parsing and management of bitmaps from "this
>> is the L3 cache", which will make it much easier to support bitmaps on some other kind of
>> resource.

> The way I see it is that it changes the meaning of the RFTYPE_RES_CACHE flag from "this is a
> file related to the cache resource" to "this is a file containing a bitmap property".
> It prevents us from easily adding a file related to the cache resource, which
> the info directory is intended to contain.

I struggled to find something that is a property of a "cache control", but is neither a
property of the control (e.g. bitmap size) or the cache. I guess the 'bit_usage' stuff is
the best example.

Maybe we end up with two sets of flags - this will be for the distant future. Currently I
taking your 'base fflags on resource id'.


>> Ultimately I'd like to expose these to user-space, so that user-space can work out how to
>> configure resources it doesn't recognise. Today '100' could be a percentage, a bitmap, or
>> a value in MB/s. Today some knowledge of the control type is needed to work this out.
>>
>>
>>> but that is not the same for
>>> info files related to the MBA resource (all info files related to MBA resource
>>> are not about the schema property format).
>>
>> Hmmm, because the files min_bandwidth and bandwidth_gran both have bandwidth in their name?
>>
>> I agree 'delay_linear' and 'thread_throttle_mode' are a bit strange.
> 
> Right. This is not a clean association.
> 
>>
>>
>>> I do not think the type of values of a schema should dictate which files
>>> appear in the info directory.
>>
>> Longer term I think this will be a problem. We probably only have 3 types of control:
>> percentage, bitmap and MB/s... but if each resource on each architecture adds files here
>> the list will quickly grow. User-space won't be able to work out how to configure a
>> resource type it hadn't seen before.
> 
> That is fair. This makes the type of control a property of the resource as is done in this
> series. Perhaps this can be exposed to user space via the info directory?

Yes, that is something I intend to look at. I eventually need to get MPAM's "cache
capacity" controls working as there are a number of hardware platforms that have it. This
would probably be a percentage control for 'L2' or 'L3', exposing an "info/schema_format"
file makes the most sense. I can't convert the existing bitmap as it implies isolation,
which this control format can't do, so it does need to be separate.
But! - to prevent confusing existing software, I don't think the L2/L3 should be touched -
those will forever have to be implicitly a bitmap, so anything in this area would have to
be an additional schema.


> Possibly the files related to control can have new flags that that reflect the control type
> instead of the resource. For example, "bit_usage" currently has
> "RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE" and that could be (for lack of better
> term) "RFTYPE_CTRL_INFO | RFTYPE_CTRL_BITMAP" to disconnect the control type from the
> resource. Doing so may then map nicely to the fflags_from_resource() in this patch that
> connects the schema format to the _control_ type flag. As we have found there is not
> a clear mapping between the control type and the resource type so I expect RFTYPE_RES_CACHE
> and RFTYPE_RES_MB to remain and be associated with files that contain information
> specific to that resource. This enables future additions of files containing cache specific
> (non-bitmap) properties to still be added (with RFTYPE_RES_CACHE flag) without impacting
> everything that uses a bitmap.
> 
> What do you think?

Makes sense!


Thanks,

James