2020-02-14 18:26:10

by James Morse

[permalink] [raw]
Subject: [PATCH 00/10] x86/resctrl: Misc cleanup

Hello!

These are the miscellaneous cleanup patches that floated to the top of
the MPAM tree. (more on that story later).

The only interesting thing are the patches to make the AMD/Intel
differences something resctrl understands, instead of just 'happening'
because of the different function pointers.
This will become more important once MPAM support is added. parse_bw()
and friends are what enforces resctrl's ABI resctrl. Allowing an
architecture/platform to provide a subtly different function here would
be bad for user-space.

MPAM would set arch_has_sparse_bitmaps, but not arch_needs_linear.

Nothing in this series should change any behaviour.
This series is based on v5.6-rc1.


Thanks,

James Morse (10):
x86/resctrl: Nothing uses struct mbm_state chunks_bw
x86/resctrl: Remove max_delay
x86/resctrl: Fix stale comment
x86/resctrl: use container_of() in delayed_work handlers
x86/resctrl: Include pid.h
x86/resctrl: Use is_closid_match() in more places
x86/resctrl: Add arch_needs_linear to explain AMD/Intel MBA difference
x86/resctrl: Merge AMD/Intel parse_bw() calls
x86/resctrl: Add arch_has_sparse_bitmaps to explain AMD/Intel CAT
difference
cacheinfo: Move resctrl's get_cache_id() to the cacheinfo header file

arch/x86/kernel/cpu/resctrl/core.c | 36 ++++------
arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 87 +++--------------------
arch/x86/kernel/cpu/resctrl/internal.h | 19 ++---
arch/x86/kernel/cpu/resctrl/monitor.c | 15 +---
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 32 ++++-----
include/linux/cacheinfo.h | 18 +++++
include/linux/resctrl.h | 2 +
7 files changed, 67 insertions(+), 142 deletions(-)

--
2.24.1


2020-02-14 18:26:12

by James Morse

[permalink] [raw]
Subject: [PATCH 05/10] x86/resctrl: Include pid.h

We are about to disturb the header soup. This header uses struct pid
and struct pid_namespace. Include their header.

Signed-off-by: James Morse <[email protected]>
---
include/linux/resctrl.h | 2 ++
1 file changed, 2 insertions(+)

diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index daf5cf64c6a6..9b05af9b3e28 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -2,6 +2,8 @@
#ifndef _RESCTRL_H
#define _RESCTRL_H

+#include <linux/pid.h>
+
#ifdef CONFIG_PROC_CPU_RESCTRL

int proc_resctrl_show(struct seq_file *m,
--
2.24.1

2020-02-14 18:26:20

by James Morse

[permalink] [raw]
Subject: [PATCH 03/10] x86/resctrl: Fix stale comment

The comment in rdtgroup_init() refers to the non existent function
rdt_mount(), which has now been renamed rdt_get_tree(). Fix the
comment.

Signed-off-by: James Morse <[email protected]>
---
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 064e9ef44cd6..fef09105cbe4 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -3181,7 +3181,7 @@ int __init rdtgroup_init(void)
* It may also be ok since that would enable debugging of RDT before
* resctrl is mounted.
* The reason why the debugfs directory is created here and not in
- * rdt_mount() is because rdt_mount() takes rdtgroup_mutex and
+ * rdt_get_tree() is because rdt_get_tree() takes rdtgroup_mutex and
* during the debugfs directory creation also &sb->s_type->i_mutex_key
* (the lockdep class of inode->i_rwsem). Other filesystem
* interactions (eg. SyS_getdents) have the lock ordering:
--
2.24.1

2020-02-14 18:26:36

by James Morse

[permalink] [raw]
Subject: [PATCH 06/10] x86/resctrl: Use is_closid_match() in more places

rdtgroup_tasks_assigned() and show_rdt_tasks() loop over threads testing
for a CTRL/MON group match by closid/rmid with the provided rdtgrp.
Further down the file are helpers to do this, move these further up and
make use of them here.

These helpers additionally check for alloc/mon capable. This is harmless
as rdtgroup_mkdir() tests these capable flags before allowing the config
directories to be created.

Signed-off-by: James Morse <[email protected]>
---
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 30 ++++++++++++--------------
1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index fef09105cbe4..c84b1f355a9a 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -592,6 +592,18 @@ static int __rdtgroup_move_task(struct task_struct *tsk,
return ret;
}

+static bool is_closid_match(struct task_struct *t, struct rdtgroup *r)
+{
+ return (rdt_alloc_capable &&
+ (r->type == RDTCTRL_GROUP) && (t->closid == r->closid));
+}
+
+static bool is_rmid_match(struct task_struct *t, struct rdtgroup *r)
+{
+ return (rdt_mon_capable &&
+ (r->type == RDTMON_GROUP) && (t->rmid == r->mon.rmid));
+}
+
/**
* rdtgroup_tasks_assigned - Test if tasks have been assigned to resource group
* @r: Resource group
@@ -607,8 +619,7 @@ int rdtgroup_tasks_assigned(struct rdtgroup *r)

rcu_read_lock();
for_each_process_thread(p, t) {
- if ((r->type == RDTCTRL_GROUP && t->closid == r->closid) ||
- (r->type == RDTMON_GROUP && t->rmid == r->mon.rmid)) {
+ if (is_closid_match(t, r) || is_rmid_match(t, r)) {
ret = 1;
break;
}
@@ -706,8 +717,7 @@ static void show_rdt_tasks(struct rdtgroup *r, struct seq_file *s)

rcu_read_lock();
for_each_process_thread(p, t) {
- if ((r->type == RDTCTRL_GROUP && t->closid == r->closid) ||
- (r->type == RDTMON_GROUP && t->rmid == r->mon.rmid))
+ if (is_closid_match(t, r) || is_rmid_match(t, r))
seq_printf(s, "%d\n", t->pid);
}
rcu_read_unlock();
@@ -2231,18 +2241,6 @@ static int reset_all_ctrls(struct rdt_resource *r)
return 0;
}

-static bool is_closid_match(struct task_struct *t, struct rdtgroup *r)
-{
- return (rdt_alloc_capable &&
- (r->type == RDTCTRL_GROUP) && (t->closid == r->closid));
-}
-
-static bool is_rmid_match(struct task_struct *t, struct rdtgroup *r)
-{
- return (rdt_mon_capable &&
- (r->type == RDTMON_GROUP) && (t->rmid == r->mon.rmid));
-}
-
/*
* Move tasks from one to the other group. If @from is NULL, then all tasks
* in the systems are moved unconditionally (used for teardown).
--
2.24.1

2020-02-14 18:26:47

by James Morse

[permalink] [raw]
Subject: [PATCH 09/10] x86/resctrl: Add arch_has_sparse_bitmaps to explain AMD/Intel CAT difference

Intel expects the cache bitmap provided by user-space to have on a
single span of 1s, whereas AMD can support bitmaps like 0xf00f.
Arm's MPAM support also allows sparse bitmaps.

To move resctrl out to /fs/ we need to explain platform differences
like this. Add a resource property arch_has_sparse_bitmaps. Test this
around the 'non-consecutive' test in cbm_validate().

Merging the validate calls causes AMD top gain the min_cbm_bits test
needed for Haswell, but as it always sets this value to 1, it will
never match.

Signed-off-by: James Morse <[email protected]>
---
arch/x86/kernel/cpu/resctrl/core.c | 4 +--
arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 36 +++++------------------
arch/x86/kernel/cpu/resctrl/internal.h | 6 ++--
3 files changed, 12 insertions(+), 34 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index e90c10ca85f4..7c9c4bd5fd32 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -920,7 +920,7 @@ static __init void rdt_init_res_defs_intel(void)
r->rid == RDT_RESOURCE_L2 ||
r->rid == RDT_RESOURCE_L2DATA ||
r->rid == RDT_RESOURCE_L2CODE)
- r->cbm_validate = cbm_validate_intel;
+ r->cache.arch_has_sparse_bitmaps = false;
else if (r->rid == RDT_RESOURCE_MBA) {
r->msr_base = MSR_IA32_MBA_THRTL_BASE;
r->msr_update = mba_wrmsr_intel;
@@ -940,7 +940,7 @@ static __init void rdt_init_res_defs_amd(void)
r->rid == RDT_RESOURCE_L2 ||
r->rid == RDT_RESOURCE_L2DATA ||
r->rid == RDT_RESOURCE_L2CODE)
- r->cbm_validate = cbm_validate_amd;
+ r->cache.arch_has_sparse_bitmaps = true;
else if (r->rid == RDT_RESOURCE_MBA) {
r->msr_base = MSR_IA32_MBA_BW_BASE;
r->msr_update = mba_wrmsr_amd;
diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index 416becb591d1..38df876feb54 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -76,12 +76,14 @@ int parse_bw(struct rdt_parse_data *data, struct rdt_resource *r,
}

/*
- * Check whether a cache bit mask is valid. The SDM says:
+ * Check whether a cache bit mask is valid.
+ * For Intel The SDM says:
* Please note that all (and only) contiguous '1' combinations
* are allowed (e.g. FFFFH, 0FF0H, 003CH, etc.).
* Additionally Haswell requires at least two bits set.
+ * AMD allows non-contiguous bitmasks.
*/
-bool cbm_validate_intel(char *buf, u32 *data, struct rdt_resource *r)
+static bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r)
{
unsigned long first_bit, zero_bit, val;
unsigned int cbm_len = r->cache.cbm_len;
@@ -101,7 +103,9 @@ bool cbm_validate_intel(char *buf, u32 *data, struct rdt_resource *r)
first_bit = find_first_bit(&val, cbm_len);
zero_bit = find_next_zero_bit(&val, cbm_len, first_bit);

- if (find_next_bit(&val, cbm_len, zero_bit) < cbm_len) {
+ /* Are non-contiguous bitmaps allowed? */
+ if (!r->cache.arch_has_sparse_bitmaps &&
+ (find_next_bit(&val, cbm_len, zero_bit) < cbm_len)) {
rdt_last_cmd_printf("The mask %lx has non-consecutive 1-bits\n", val);
return false;
}
@@ -116,30 +120,6 @@ bool cbm_validate_intel(char *buf, u32 *data, struct rdt_resource *r)
return true;
}

-/*
- * Check whether a cache bit mask is valid. AMD allows non-contiguous
- * bitmasks
- */
-bool cbm_validate_amd(char *buf, u32 *data, struct rdt_resource *r)
-{
- unsigned long val;
- int ret;
-
- ret = kstrtoul(buf, 16, &val);
- if (ret) {
- rdt_last_cmd_printf("Non-hex character in the mask %s\n", buf);
- return false;
- }
-
- if (val > r->default_ctrl) {
- rdt_last_cmd_puts("Mask out of range\n");
- return false;
- }
-
- *data = val;
- return true;
-}
-
/*
* Read one cache bit mask (hex). Check that it is valid for the current
* resource type.
@@ -165,7 +145,7 @@ int parse_cbm(struct rdt_parse_data *data, struct rdt_resource *r,
return -EINVAL;
}

- if (!r->cbm_validate(data->buf, &cbm_val, r))
+ if (!cbm_validate(data->buf, &cbm_val, r))
return -EINVAL;

if ((rdtgrp->mode == RDT_MODE_EXCLUSIVE ||
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 45fc695081d1..0172a87de814 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -350,6 +350,7 @@ struct msr_param {
* in a cache bit mask
* @shareable_bits: Bitmask of shareable resource with other
* executing entities
+ * @arch_has_sparse_bitmaps: True if a bitmap like f00f is valid.
*/
struct rdt_cache {
unsigned int cbm_len;
@@ -357,6 +358,7 @@ struct rdt_cache {
unsigned int cbm_idx_mult;
unsigned int cbm_idx_offset;
unsigned int shareable_bits;
+ bool arch_has_sparse_bitmaps;
};

/**
@@ -426,7 +428,6 @@ struct rdt_parse_data {
* @cache: Cache allocation related data
* @format_str: Per resource format string to show domain value
* @parse_ctrlval: Per resource function pointer to parse control values
- * @cbm_validate Cache bitmask validate function
* @evt_list: List of monitoring events
* @num_rmid: Number of RMIDs available
* @mon_scale: cqm counter * mon_scale = occupancy in bytes
@@ -453,7 +454,6 @@ struct rdt_resource {
int (*parse_ctrlval)(struct rdt_parse_data *data,
struct rdt_resource *r,
struct rdt_domain *d);
- bool (*cbm_validate)(char *buf, u32 *data, struct rdt_resource *r);
struct list_head evt_list;
int num_rmid;
unsigned int mon_scale;
@@ -594,7 +594,5 @@ void cqm_setup_limbo_handler(struct rdt_domain *dom, unsigned long delay_ms);
void cqm_handle_limbo(struct work_struct *work);
bool has_busy_rmid(struct rdt_resource *r, struct rdt_domain *d);
void __check_limbo(struct rdt_domain *d, bool force_free);
-bool cbm_validate_intel(char *buf, u32 *data, struct rdt_resource *r);
-bool cbm_validate_amd(char *buf, u32 *data, struct rdt_resource *r);

#endif /* _ASM_X86_RESCTRL_INTERNAL_H */
--
2.24.1

2020-02-14 18:27:12

by James Morse

[permalink] [raw]
Subject: [PATCH 01/10] x86/resctrl: Nothing uses struct mbm_state chunks_bw

Nothing reads struct mbm_states's chunks_bw value, its a copy of
chunks. Remove it.

Signed-off-by: James Morse <[email protected]>
---
arch/x86/kernel/cpu/resctrl/internal.h | 2 --
arch/x86/kernel/cpu/resctrl/monitor.c | 3 +--
2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 181c992f448c..90ca6a090c77 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -275,7 +275,6 @@ struct rftype {
* struct mbm_state - status for each MBM counter in each domain
* @chunks: Total data moved (multiply by rdt_group.mon_scale to get bytes)
* @prev_msr Value of IA32_QM_CTR for this RMID last time we read it
- * @chunks_bw Total local data moved. Used for bandwidth calculation
* @prev_bw_msr:Value of previous IA32_QM_CTR for bandwidth counting
* @prev_bw The most recent bandwidth in MBps
* @delta_bw Difference between the current and previous bandwidth
@@ -284,7 +283,6 @@ struct rftype {
struct mbm_state {
u64 chunks;
u64 prev_msr;
- u64 chunks_bw;
u64 prev_bw_msr;
u32 prev_bw;
u32 delta_bw;
diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
index 773124b0e18a..af549df38ec6 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -279,8 +279,7 @@ static void mbm_bw_count(u32 rmid, struct rmid_read *rr)
return;

chunks = mbm_overflow_count(m->prev_bw_msr, tval);
- m->chunks_bw += chunks;
- m->chunks = m->chunks_bw;
+ m->chunks += chunks;
cur_bw = (chunks * r->mon_scale) >> 20;

if (m->delta_comp)
--
2.24.1

2020-03-16 18:09:46

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH 01/10] x86/resctrl: Nothing uses struct mbm_state chunks_bw

Hi James,

On 2/14/2020 10:23 AM, James Morse wrote:
> Nothing reads struct mbm_states's chunks_bw value, its a copy of
> chunks. Remove it.
>
> Signed-off-by: James Morse <[email protected]>
> ---

Reviewed-by: Reinette Chatre <[email protected]>

Thank you

Reinette


2020-03-16 18:13:25

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH 03/10] x86/resctrl: Fix stale comment

Hi James,

On 2/14/2020 10:23 AM, James Morse wrote:
> The comment in rdtgroup_init() refers to the non existent function
> rdt_mount(), which has now been renamed rdt_get_tree(). Fix the
> comment.
>
> Signed-off-by: James Morse <[email protected]>
> ---

Reviewed-by: Reinette Chatre <[email protected]>

Thank you

Reinette

2020-03-16 18:15:25

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH 05/10] x86/resctrl: Include pid.h

Hi James,

On 2/14/2020 10:23 AM, James Morse wrote:
> We are about to disturb the header soup. This header uses struct pid
> and struct pid_namespace. Include their header.
>
> Signed-off-by: James Morse <[email protected]>
> ---

Reviewed-by: Reinette Chatre <[email protected]>

Thank you

Reinette

2020-03-16 18:16:27

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH 06/10] x86/resctrl: Use is_closid_match() in more places

Hi James,

On 2/14/2020 10:23 AM, James Morse wrote:
> rdtgroup_tasks_assigned() and show_rdt_tasks() loop over threads testing
> for a CTRL/MON group match by closid/rmid with the provided rdtgrp.
> Further down the file are helpers to do this, move these further up and
> make use of them here.
>
> These helpers additionally check for alloc/mon capable. This is harmless
> as rdtgroup_mkdir() tests these capable flags before allowing the config
> directories to be created.
>
> Signed-off-by: James Morse <[email protected]>

Reviewed-by: Reinette Chatre <[email protected]>

Thank you

Reinette

2020-03-16 18:59:05

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH 09/10] x86/resctrl: Add arch_has_sparse_bitmaps to explain AMD/Intel CAT difference

Hi James,

On 2/14/2020 10:24 AM, James Morse wrote:
> Intel expects the cache bitmap provided by user-space to have on a
> single span of 1s, whereas AMD can support bitmaps like 0xf00f.
> Arm's MPAM support also allows sparse bitmaps.
>
> To move resctrl out to /fs/ we need to explain platform differences
> like this. Add a resource property arch_has_sparse_bitmaps. Test this
> around the 'non-consecutive' test in cbm_validate().
>
> Merging the validate calls causes AMD top gain the min_cbm_bits test
> needed for Haswell, but as it always sets this value to 1, it will
> never match.
>
> Signed-off-by: James Morse <[email protected]>
> ---
> arch/x86/kernel/cpu/resctrl/core.c | 4 +--
> arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 36 +++++------------------
> arch/x86/kernel/cpu/resctrl/internal.h | 6 ++--
> 3 files changed, 12 insertions(+), 34 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index e90c10ca85f4..7c9c4bd5fd32 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -920,7 +920,7 @@ static __init void rdt_init_res_defs_intel(void)
> r->rid == RDT_RESOURCE_L2 ||
> r->rid == RDT_RESOURCE_L2DATA ||
> r->rid == RDT_RESOURCE_L2CODE)
> - r->cbm_validate = cbm_validate_intel;
> + r->cache.arch_has_sparse_bitmaps = false;
> else if (r->rid == RDT_RESOURCE_MBA) {
> r->msr_base = MSR_IA32_MBA_THRTL_BASE;
> r->msr_update = mba_wrmsr_intel;
> @@ -940,7 +940,7 @@ static __init void rdt_init_res_defs_amd(void)
> r->rid == RDT_RESOURCE_L2 ||
> r->rid == RDT_RESOURCE_L2DATA ||
> r->rid == RDT_RESOURCE_L2CODE)
> - r->cbm_validate = cbm_validate_amd;
> + r->cache.arch_has_sparse_bitmaps = true;
> else if (r->rid == RDT_RESOURCE_MBA) {
> r->msr_base = MSR_IA32_MBA_BW_BASE;
> r->msr_update = mba_wrmsr_amd;
> diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> index 416becb591d1..38df876feb54 100644
> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> @@ -76,12 +76,14 @@ int parse_bw(struct rdt_parse_data *data, struct rdt_resource *r,
> }
>
> /*
> - * Check whether a cache bit mask is valid. The SDM says:
> + * Check whether a cache bit mask is valid.
> + * For Intel The SDM says:

s/The/the/

> * Please note that all (and only) contiguous '1' combinations
> * are allowed (e.g. FFFFH, 0FF0H, 003CH, etc.).
> * Additionally Haswell requires at least two bits set.
> + * AMD allows non-contiguous bitmasks.
> */
> -bool cbm_validate_intel(char *buf, u32 *data, struct rdt_resource *r)
> +static bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r)
> {
> unsigned long first_bit, zero_bit, val;
> unsigned int cbm_len = r->cache.cbm_len;
> @@ -101,7 +103,9 @@ bool cbm_validate_intel(char *buf, u32 *data, struct rdt_resource *r)
> first_bit = find_first_bit(&val, cbm_len);
> zero_bit = find_next_zero_bit(&val, cbm_len, first_bit);
>
> - if (find_next_bit(&val, cbm_len, zero_bit) < cbm_len) {
> + /* Are non-contiguous bitmaps allowed? */
> + if (!r->cache.arch_has_sparse_bitmaps &&
> + (find_next_bit(&val, cbm_len, zero_bit) < cbm_len)) {
> rdt_last_cmd_printf("The mask %lx has non-consecutive 1-bits\n", val);
> return false;
> }
> @@ -116,30 +120,6 @@ bool cbm_validate_intel(char *buf, u32 *data, struct rdt_resource *r)
> return true;
> }
>
> -/*
> - * Check whether a cache bit mask is valid. AMD allows non-contiguous
> - * bitmasks
> - */
> -bool cbm_validate_amd(char *buf, u32 *data, struct rdt_resource *r)
> -{
> - unsigned long val;
> - int ret;
> -
> - ret = kstrtoul(buf, 16, &val);
> - if (ret) {
> - rdt_last_cmd_printf("Non-hex character in the mask %s\n", buf);
> - return false;
> - }
> -
> - if (val > r->default_ctrl) {
> - rdt_last_cmd_puts("Mask out of range\n");
> - return false;
> - }
> -
> - *data = val;
> - return true;
> -}
> -
> /*
> * Read one cache bit mask (hex). Check that it is valid for the current
> * resource type.
> @@ -165,7 +145,7 @@ int parse_cbm(struct rdt_parse_data *data, struct rdt_resource *r,
> return -EINVAL;
> }
>
> - if (!r->cbm_validate(data->buf, &cbm_val, r))
> + if (!cbm_validate(data->buf, &cbm_val, r))
> return -EINVAL;
>
> if ((rdtgrp->mode == RDT_MODE_EXCLUSIVE ||
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 45fc695081d1..0172a87de814 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -350,6 +350,7 @@ struct msr_param {
> * in a cache bit mask
> * @shareable_bits: Bitmask of shareable resource with other
> * executing entities
> + * @arch_has_sparse_bitmaps: True if a bitmap like f00f is valid.
> */

This area uses tab for spacing.

> struct rdt_cache {
> unsigned int cbm_len;
> @@ -357,6 +358,7 @@ struct rdt_cache {
> unsigned int cbm_idx_mult;
> unsigned int cbm_idx_offset;
> unsigned int shareable_bits;
> + bool arch_has_sparse_bitmaps;
> };
>
> /**
> @@ -426,7 +428,6 @@ struct rdt_parse_data {
> * @cache: Cache allocation related data
> * @format_str: Per resource format string to show domain value
> * @parse_ctrlval: Per resource function pointer to parse control values
> - * @cbm_validate Cache bitmask validate function
> * @evt_list: List of monitoring events
> * @num_rmid: Number of RMIDs available
> * @mon_scale: cqm counter * mon_scale = occupancy in bytes
> @@ -453,7 +454,6 @@ struct rdt_resource {
> int (*parse_ctrlval)(struct rdt_parse_data *data,
> struct rdt_resource *r,
> struct rdt_domain *d);
> - bool (*cbm_validate)(char *buf, u32 *data, struct rdt_resource *r);
> struct list_head evt_list;
> int num_rmid;
> unsigned int mon_scale;
> @@ -594,7 +594,5 @@ void cqm_setup_limbo_handler(struct rdt_domain *dom, unsigned long delay_ms);
> void cqm_handle_limbo(struct work_struct *work);
> bool has_busy_rmid(struct rdt_resource *r, struct rdt_domain *d);
> void __check_limbo(struct rdt_domain *d, bool force_free);
> -bool cbm_validate_intel(char *buf, u32 *data, struct rdt_resource *r);
> -bool cbm_validate_amd(char *buf, u32 *data, struct rdt_resource *r);
>
> #endif /* _ASM_X86_RESCTRL_INTERNAL_H */
>

Just the two small comments from my side. For the rest:

Reviewed-by: Reinette Chatre <[email protected]>

Babu may want to take a look.

Reinette