This series adds a bug fix for AMD and a minor code cleanup.
---
v5:
This is bug. So, submitting these patches as a separate series from the old v4 series.
This was a comment from James Morse.
v4
https://lore.kernel.org/lkml/166257348081.1043018.11227924488792315932.stgit@bmoger-ubuntu/
Babu Moger (2):
x86/resctrl: Fix min_cbm_bits for AMD
x86/resctrl: Remove arch_has_empty_bitmaps
arch/x86/kernel/cpu/resctrl/core.c | 10 ++--------
arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 3 +--
include/linux/resctrl.h | 6 +++---
3 files changed, 6 insertions(+), 13 deletions(-)
--
AMD systems support zero CBM (capacity bit mask) for cache allocation.
That is reflected in rdt_init_res_defs_amd() by:
r->cache.arch_has_empty_bitmaps = true;
However given the unified code in cbm_validate(), checking for:
val == 0 && !arch_has_empty_bitmaps
is not enough because of another check in cbm_validate():
if ((zero_bit - first_bit) < r->cache.min_cbm_bits)
The default value of r->cache.min_cbm_bits = 1.
Leading to:
$ cd /sys/fs/resctrl
$ mkdir foo
$ cd foo
$ echo L3:0=0 > schemata
-bash: echo: write error: Invalid argument
$ cat /sys/fs/resctrl/info/last_cmd_status
Need at least 1 bits in the mask
Fix the issue by initializing the min_cbm_bits to 0 for AMD.
Also, remove the default setting of min_cbm_bits and initialize it
separately.
After the fix
$ cd /sys/fs/resctrl
$ mkdir foo
$ cd foo
$ echo L3:0=0 > schemata
$ cat /sys/fs/resctrl/info/last_cmd_status
ok
Link: https://lore.kernel.org/lkml/[email protected]/
Fixes: 316e7f901f5a ("x86/resctrl: Add struct rdt_cache::arch_has_{sparse, empty}_bitmaps")
Co-developed-by: Stephane Eranian <[email protected]>
Signed-off-by: Stephane Eranian <[email protected]>
Signed-off-by: Babu Moger <[email protected]>
Reviewed-by: Ingo Molnar <[email protected]>
Reviewed-by: James Morse <[email protected]>
Reviewed-by: Reinette Chatre <[email protected]>
---
arch/x86/kernel/cpu/resctrl/core.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index bb1c3f5f60c8..a5c51a14fbce 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -66,9 +66,6 @@ struct rdt_hw_resource rdt_resources_all[] = {
.rid = RDT_RESOURCE_L3,
.name = "L3",
.cache_level = 3,
- .cache = {
- .min_cbm_bits = 1,
- },
.domains = domain_init(RDT_RESOURCE_L3),
.parse_ctrlval = parse_cbm,
.format_str = "%d=%0*x",
@@ -83,9 +80,6 @@ struct rdt_hw_resource rdt_resources_all[] = {
.rid = RDT_RESOURCE_L2,
.name = "L2",
.cache_level = 2,
- .cache = {
- .min_cbm_bits = 1,
- },
.domains = domain_init(RDT_RESOURCE_L2),
.parse_ctrlval = parse_cbm,
.format_str = "%d=%0*x",
@@ -877,6 +871,7 @@ static __init void rdt_init_res_defs_intel(void)
r->cache.arch_has_sparse_bitmaps = false;
r->cache.arch_has_empty_bitmaps = false;
r->cache.arch_has_per_cpu_cfg = false;
+ r->cache.min_cbm_bits = 1;
} else if (r->rid == RDT_RESOURCE_MBA) {
hw_res->msr_base = MSR_IA32_MBA_THRTL_BASE;
hw_res->msr_update = mba_wrmsr_intel;
@@ -897,6 +892,7 @@ static __init void rdt_init_res_defs_amd(void)
r->cache.arch_has_sparse_bitmaps = true;
r->cache.arch_has_empty_bitmaps = true;
r->cache.arch_has_per_cpu_cfg = true;
+ r->cache.min_cbm_bits = 0;
} else if (r->rid == RDT_RESOURCE_MBA) {
hw_res->msr_base = MSR_IA32_MBA_BW_BASE;
hw_res->msr_update = mba_wrmsr_amd;
The field arch_has_empty_bitmaps is not required anymore. The field
min_cbm_bits is enough to validate the CBM (capacity bit mask) if the
architecture can support the zero CBM or not.
Suggested-by: Reinette Chatre <[email protected]>
Signed-off-by: Babu Moger <[email protected]>
Reviewed-by: Reinette Chatre <[email protected]>
---
arch/x86/kernel/cpu/resctrl/core.c | 2 --
arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 3 +--
include/linux/resctrl.h | 6 +++---
3 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index a5c51a14fbce..c2657754672e 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -869,7 +869,6 @@ static __init void rdt_init_res_defs_intel(void)
if (r->rid == RDT_RESOURCE_L3 ||
r->rid == RDT_RESOURCE_L2) {
r->cache.arch_has_sparse_bitmaps = false;
- r->cache.arch_has_empty_bitmaps = false;
r->cache.arch_has_per_cpu_cfg = false;
r->cache.min_cbm_bits = 1;
} else if (r->rid == RDT_RESOURCE_MBA) {
@@ -890,7 +889,6 @@ static __init void rdt_init_res_defs_amd(void)
if (r->rid == RDT_RESOURCE_L3 ||
r->rid == RDT_RESOURCE_L2) {
r->cache.arch_has_sparse_bitmaps = true;
- r->cache.arch_has_empty_bitmaps = true;
r->cache.arch_has_per_cpu_cfg = true;
r->cache.min_cbm_bits = 0;
} else if (r->rid == RDT_RESOURCE_MBA) {
diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index 87666275eed9..7f38c8bd8429 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -98,8 +98,7 @@ static bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r)
return false;
}
- if ((!r->cache.arch_has_empty_bitmaps && val == 0) ||
- val > r->default_ctrl) {
+ if ((r->cache.min_cbm_bits > 0 && val == 0) || val > r->default_ctrl) {
rdt_last_cmd_puts("Mask out of range\n");
return false;
}
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index 21deb5212bbd..46ed8589857c 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -72,11 +72,12 @@ struct rdt_domain {
/**
* struct resctrl_cache - Cache allocation related data
* @cbm_len: Length of the cache bit mask
- * @min_cbm_bits: Minimum number of consecutive bits to be set
+ * @min_cbm_bits: Minimum number of consecutive bits to be set.
+ * The value 0 means the architecture can support
+ * zero CBM.
* @shareable_bits: Bitmask of shareable resource with other
* executing entities
* @arch_has_sparse_bitmaps: True if a bitmap like f00f is valid.
- * @arch_has_empty_bitmaps: True if the '0' bitmap is valid.
* @arch_has_per_cpu_cfg: True if QOS_CFG register for this cache
* level has CPU scope.
*/
@@ -85,7 +86,6 @@ struct resctrl_cache {
unsigned int min_cbm_bits;
unsigned int shareable_bits;
bool arch_has_sparse_bitmaps;
- bool arch_has_empty_bitmaps;
bool arch_has_per_cpu_cfg;
};
> AMD systems support zero CBM (capacity bit mask) for cache allocation.
> That is reflected in rdt_init_res_defs_amd() by:
>
> r->cache.arch_has_empty_bitmaps = true;
>
> However given the unified code in cbm_validate(), checking for:
> val == 0 && !arch_has_empty_bitmaps
>
> is not enough because of another check in cbm_validate():
>
> if ((zero_bit - first_bit) < r->cache.min_cbm_bits)
>
> The default value of r->cache.min_cbm_bits = 1.
>
> Leading to:
>
> $ cd /sys/fs/resctrl
> $ mkdir foo
> $ cd foo
> $ echo L3:0=0 > schemata
> -bash: echo: write error: Invalid argument
> $ cat /sys/fs/resctrl/info/last_cmd_status
> Need at least 1 bits in the mask
>
> Fix the issue by initializing the min_cbm_bits to 0 for AMD.
> Also, remove the default setting of min_cbm_bits and initialize it separately.
>
> After the fix
> $ cd /sys/fs/resctrl
> $ mkdir foo
> $ cd foo
> $ echo L3:0=0 > schemata
> $ cat /sys/fs/resctrl/info/last_cmd_status
> ok
>
> Link: https://lore.kernel.org/lkml/20220517001234.3137157-1-
> [email protected]/
> Fixes: 316e7f901f5a ("x86/resctrl: Add struct rdt_cache::arch_has_{sparse,
> empty}_bitmaps")
> Co-developed-by: Stephane Eranian <[email protected]>
> Signed-off-by: Stephane Eranian <[email protected]>
> Signed-off-by: Babu Moger <[email protected]>
> Reviewed-by: Ingo Molnar <[email protected]>
> Reviewed-by: James Morse <[email protected]>
> Reviewed-by: Reinette Chatre <[email protected]>
Reviewed-by: Fenghua Yu <[email protected]>
> The field arch_has_empty_bitmaps is not required anymore. The field
> min_cbm_bits is enough to validate the CBM (capacity bit mask) if the
> architecture can support the zero CBM or not.
>
> Suggested-by: Reinette Chatre <[email protected]>
> Signed-off-by: Babu Moger <[email protected]>
> Reviewed-by: Reinette Chatre <[email protected]>
Reviewed-by: Fenghua Yu <[email protected]>
Hi X86 Maintainers,
Could you please consider this fix and cleanup for inclusion?
Thank you very much
Reinette
On 9/27/2022 1:16 PM, Babu Moger wrote:
> This series adds a bug fix for AMD and a minor code cleanup.
>
> ---
> v5:
> This is bug. So, submitting these patches as a separate series from the old v4 series.
> This was a comment from James Morse.
>
> v4
> https://lore.kernel.org/lkml/166257348081.1043018.11227924488792315932.stgit@bmoger-ubuntu/
>
> Babu Moger (2):
> x86/resctrl: Fix min_cbm_bits for AMD
> x86/resctrl: Remove arch_has_empty_bitmaps
>
>
> arch/x86/kernel/cpu/resctrl/core.c | 10 ++--------
> arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 3 +--
> include/linux/resctrl.h | 6 +++---
> 3 files changed, 6 insertions(+), 13 deletions(-)
>
> --
>
>
On Tue, Oct 18, 2022 at 09:26:41AM -0700, Reinette Chatre wrote:
> Hi X86 Maintainers,
>
> Could you please consider this fix and cleanup for inclusion?
Sure.
From the looks of it, the first one needs to go to stable, right?
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Tue, Oct 18, 2022 at 09:43:15AM -0700, Reinette Chatre wrote:
> Correct. Apologies for missing the "Cc: [email protected]" in
> that one.
Nothing to apologize, all good.
Lemme take care of them.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Hi Boris,
On 10/18/2022 9:35 AM, Borislav Petkov wrote:
> On Tue, Oct 18, 2022 at 09:26:41AM -0700, Reinette Chatre wrote:
>> Hi X86 Maintainers,
>>
>> Could you please consider this fix and cleanup for inclusion?
>
> Sure.
Thank you very much.
>
> From the looks of it, the first one needs to go to stable, right?
Correct. Apologies for missing the "Cc: [email protected]" in
that one.
Reinette
On 10/18/22 11:47, Borislav Petkov wrote:
> On Tue, Oct 18, 2022 at 09:43:15AM -0700, Reinette Chatre wrote:
>> Correct. Apologies for missing the "Cc: [email protected]" in
>> that one.
> Nothing to apologize, all good.
>
> Lemme take care of them.
>
Thank you Reinette, Boris.
The following commit has been merged into the x86/cache branch of tip:
Commit-ID: 2d4daa549c17b6ba4845a751c7a78d3b2419d78f
Gitweb: https://git.kernel.org/tip/2d4daa549c17b6ba4845a751c7a78d3b2419d78f
Author: Babu Moger <[email protected]>
AuthorDate: Tue, 27 Sep 2022 15:16:36 -05:00
Committer: Borislav Petkov <[email protected]>
CommitterDate: Mon, 24 Oct 2022 10:30:29 +02:00
x86/resctrl: Remove arch_has_empty_bitmaps
The field arch_has_empty_bitmaps is not required anymore. The field
min_cbm_bits is enough to validate the CBM (capacity bit mask) if the
architecture can support the zero CBM or not.
Suggested-by: Reinette Chatre <[email protected]>
Signed-off-by: Babu Moger <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Reviewed-by: Reinette Chatre <[email protected]>
Reviewed-by: Fenghua Yu <[email protected]>
Link: https://lore.kernel.org/r/166430979654.372014.615622285687642644.stgit@bmoger-ubuntu
---
arch/x86/kernel/cpu/resctrl/core.c | 2 --
arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 3 +--
include/linux/resctrl.h | 6 +++---
3 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 3266ea3..03cfbf0 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -828,7 +828,6 @@ static __init void rdt_init_res_defs_intel(void)
if (r->rid == RDT_RESOURCE_L3 ||
r->rid == RDT_RESOURCE_L2) {
r->cache.arch_has_sparse_bitmaps = false;
- r->cache.arch_has_empty_bitmaps = false;
r->cache.arch_has_per_cpu_cfg = false;
r->cache.min_cbm_bits = 1;
} else if (r->rid == RDT_RESOURCE_MBA) {
@@ -849,7 +848,6 @@ static __init void rdt_init_res_defs_amd(void)
if (r->rid == RDT_RESOURCE_L3 ||
r->rid == RDT_RESOURCE_L2) {
r->cache.arch_has_sparse_bitmaps = true;
- r->cache.arch_has_empty_bitmaps = true;
r->cache.arch_has_per_cpu_cfg = true;
r->cache.min_cbm_bits = 0;
} else if (r->rid == RDT_RESOURCE_MBA) {
diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index 1dafbdc..1df0e32 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -105,8 +105,7 @@ static bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r)
return false;
}
- if ((!r->cache.arch_has_empty_bitmaps && val == 0) ||
- val > r->default_ctrl) {
+ if ((r->cache.min_cbm_bits > 0 && val == 0) || val > r->default_ctrl) {
rdt_last_cmd_puts("Mask out of range\n");
return false;
}
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index 0cf5b20..0cee154 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -89,11 +89,12 @@ struct rdt_domain {
/**
* struct resctrl_cache - Cache allocation related data
* @cbm_len: Length of the cache bit mask
- * @min_cbm_bits: Minimum number of consecutive bits to be set
+ * @min_cbm_bits: Minimum number of consecutive bits to be set.
+ * The value 0 means the architecture can support
+ * zero CBM.
* @shareable_bits: Bitmask of shareable resource with other
* executing entities
* @arch_has_sparse_bitmaps: True if a bitmap like f00f is valid.
- * @arch_has_empty_bitmaps: True if the '0' bitmap is valid.
* @arch_has_per_cpu_cfg: True if QOS_CFG register for this cache
* level has CPU scope.
*/
@@ -102,7 +103,6 @@ struct resctrl_cache {
unsigned int min_cbm_bits;
unsigned int shareable_bits;
bool arch_has_sparse_bitmaps;
- bool arch_has_empty_bitmaps;
bool arch_has_per_cpu_cfg;
};