[PATCH 0/9] x86/intel_rdt: MBA integration fixes

[PATCH 0/9] x86/intel_rdt: MBA integration fixes

[Fenghua Yu]

Chen Yu reported an issue where reading the resctrl "size" file results in
a divide-by-zero issue on a system with a MBA resource. Further
investigation revealed more issues where the recent RDT features are not
well integrated with the MBA resource handling.

This series consists out of:
 - One helper function in support of fixes that need to know the number of
   supported CLOSIDs on the system (the minimum of all CLOSIDs of all
   resources).
 - The fix to the issue reported by Chen Yu  - now reading a resource
   group's "size" file will show a MB resource's allocation as its size.
 - A fix from Xiaochen Shen to the MB parsing callback that was never
   changed to accept a new parameter format.
 - Functions that iterate over the number of CLOSIDs need to take care
   whether it is using a particular resource's supported CLOSIDs or the
   number of CLOSIDs supported by the system. This was done incorrectly in
   a few places and fixed here.
 - When a new resource group is created it is intended to be configured
   with sane defaults. This new feature blindly assumed that the resource
   group only consists out of cache resources - make this explicit to not
   cause invalid register writes on a system with MBA resources.
 - The new "exclusive" mode assumes that all resources support a CBM, while
   a MBA resource does not. Since the MBA resource allocations cannot be
   done in a way to specify whether allocations can overlap or not the
   "exclusive" mode of a resource group will only apply to the cache
   resources within the group, if only a MBA resource is present then
   "exclusive" mode will not be allowed.

Reinette Chatre (8):
  x86/intel_rdt: Fix size reporting of MBA resource
  x86/intel_rdt: Global closid helper to support future fixes
  x86/intel_rdt: Fix invalid mode warning when
  x86/intel_rdt: Fix unchecked MSR access
  x86/intel_rdt: Do not allow pseudo-locking of MBA resource
  x86/intel_rdt: Fix incorrect loop end condition
  x86/intel_rdt: Fix exclusive mode handling of MBA resource
  x86/intel_rdt: Fix incorrect loop end condition

Xiaochen Shen (1):
  x86/intel_rdt: Fix MBA parsing callback

 arch/x86/kernel/cpu/intel_rdt.h             |  3 +-
 arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c | 30 +++++++-----
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c    | 53 +++++++++++++++++----
 3 files changed, 64 insertions(+), 22 deletions(-)

-- 
2.19.0

[PATCH 1/9] x86/intel_rdt: Fix MBA parsing callback

[Fenghua Yu]

From: Xiaochen Shen <xiaochen.shen@intel.com>

Each resource is associated with a parsing callback to parse the
data provided from user space when writing schemata file.

Commit 9ab9aa15c309 ("x86/intel_rdt: Ensure requested schemata
respects mode") changes the first parameter 'data' of parse_ctrlval()
from string to a pointer to a structure. parse_ctrlval() could point
to two functions parse_cbm() and parse_bw() and both of the
functions should be changed to take the different type of parameter
'data'. But the commit only changes parse_cbm(), not parse_bw().
Thus, parse_bw() takes wrong 'data' parameter and causes failure
of parsing MBA throttle value.

To fix the issue, parse_bw() should handle its first parameter as
right structure instead of a string.

Fixes: 9ab9aa15c309 ("x86/intel_rdt: Ensure requested schemata respects mode")

Signed-off-by: Xiaochen Shen <xiaochen.shen@intel.com>
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt.h             |  2 +-
 arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c | 24 ++++++++++-----------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_rdt.h b/arch/x86/kernel/cpu/intel_rdt.h
index 4e588f36228f..8c30772e37ed 100644
--- a/arch/x86/kernel/cpu/intel_rdt.h
+++ b/arch/x86/kernel/cpu/intel_rdt.h
@@ -432,7 +432,7 @@ struct rdt_resource {
 };
 
 int parse_cbm(void *_data, struct rdt_resource *r, struct rdt_domain *d);
-int parse_bw(void *_buf, struct rdt_resource *r,  struct rdt_domain *d);
+int parse_bw(void *_data, struct rdt_resource *r, struct rdt_domain *d);
 
 extern struct mutex rdtgroup_mutex;
 
diff --git a/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c b/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c
index af358ca05160..d427c86e7cd0 100644
--- a/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c
+++ b/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c
@@ -28,6 +28,11 @@
 #include <linux/slab.h>
 #include "intel_rdt.h"
 
+struct rdt_parse_data {
+	struct rdtgroup		*rdtgrp;
+	char			*buf;
+};
+
 /*
  * Check whether MBA bandwidth percentage value is correct. The value is
  * checked against the minimum and max bandwidth values specified by the
@@ -64,19 +69,19 @@ static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
 	return true;
 }
 
-int parse_bw(void *_buf, struct rdt_resource *r, struct rdt_domain *d)
+int parse_bw(void *_data, struct rdt_resource *r, struct rdt_domain *d)
 {
-	unsigned long data;
-	char *buf = _buf;
+	struct rdt_parse_data *data = _data;
+	unsigned long bw_val;
 
 	if (d->have_new_ctrl) {
 		rdt_last_cmd_printf("duplicate domain %d\n", d->id);
 		return -EINVAL;
 	}
 
-	if (!bw_validate(buf, &data, r))
+	if (!bw_validate(data->buf, &bw_val, r))
 		return -EINVAL;
-	d->new_ctrl = data;
+	d->new_ctrl = bw_val;
 	d->have_new_ctrl = true;
 
 	return 0;
@@ -123,18 +128,13 @@ static bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r)
 	return true;
 }
 
-struct rdt_cbm_parse_data {
-	struct rdtgroup		*rdtgrp;
-	char			*buf;
-};
-
 /*
  * Read one cache bit mask (hex). Check that it is valid for the current
  * resource type.
  */
 int parse_cbm(void *_data, struct rdt_resource *r, struct rdt_domain *d)
 {
-	struct rdt_cbm_parse_data *data = _data;
+	struct rdt_parse_data *data = _data;
 	struct rdtgroup *rdtgrp = data->rdtgrp;
 	u32 cbm_val;
 
@@ -195,7 +195,7 @@ int parse_cbm(void *_data, struct rdt_resource *r, struct rdt_domain *d)
 static int parse_line(char *line, struct rdt_resource *r,
 		      struct rdtgroup *rdtgrp)
 {
-	struct rdt_cbm_parse_data data;
+	struct rdt_parse_data data;
 	char *dom = NULL, *id;
 	struct rdt_domain *d;
 	unsigned long dom_id;
-- 
2.19.0

[PATCH 2/9] x86/intel_rdt: Fix size reporting of MBA resource

[Fenghua Yu]

From: Reinette Chatre <reinette.chatre@intel.com>

Chen Yu reported a divide-by-zero error when accessing the 'size'
resctrl file when a MBA resource is enabled.

[  151.193447] divide error: 0000 [#1] SMP PTI
[  151.197743] CPU: 93 PID: 1929 Comm: cat Not tainted 4.19.0-rc2-debug-rdt+ #25
[  151.205070] Hardware name: Dell Inc. PowerEdge R640/0CRT1G, BIOS 1.3.7 02/08/2018
[  151.212783] RIP: 0010:rdtgroup_cbm_to_size+0x7e/0xa0
[  151.237172] RSP: 0018:ffffb3454f90bd88 EFLAGS: 00010246
[  151.242538] RAX: 00000000023c0000 RBX: 0000000000000000 RCX: 0000000000000003
[  151.249878] RDX: 0000000000000000 RSI: 0000000000000004 RDI: 0000000000000003
[  151.257213] RBP: ffff96ff0089e000 R08: 0000000000000000 R09: 0000000000aaaaaa
[  151.264544] R10: ffffb3454f90bd8c R11: 00000000ffffffff R12: ffffffffb5028910
[  151.271887] R13: ffffffffb5028910 R14: 0000000000000064 R15: ffff96ff0089e000
[  151.279217] FS:  00007f95a623a500(0000) GS:ffff97170f9c0000(0000) knlGS:0000000000000000
[  151.287532] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  151.293432] CR2: 00007f95a6217000 CR3: 00000023f696c003 CR4: 00000000007606e0
[  151.300766] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  151.308094] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[  151.315426] PKRU: 55555554
[  151.318212] Call Trace:
[  151.320732]  rdtgroup_size_show+0x11a/0x1d0
[  151.325039]  seq_read+0xd8/0x3b0
[  151.328363]  __vfs_read+0x36/0x170
[  151.331857]  vfs_read+0x89/0x130
[  151.335179]  ksys_read+0x52/0xc0
[  151.338500]  do_syscall_64+0x5b/0x180
[  151.342261]  entry_SYSCALL_64_after_hwframe+0x44/0xa9

Quoting Chen Yu's report: This is because for MB resource,
the r->cache.cbm_len is zero, thus calculating size in
rdtgroup_cbm_to_size() will trigger the exception.

Fix this issue in the 'size' file by getting correct memory bandwidth
value which is in MBps when MBA software controller is enabled or in
percentage when MBA software controller is disabled.

Fixes: d9b48c86eb38 ("x86/intel_rdt: Display resource groups' allocations in bytes")
Link: https://lkml.kernel.org/r/20180904174614.26682-1-yu.c.chen@intel.com
Reported-by: Chen Yu <yu.c.chen@intel.com>
Tested-by: Chen Yu <yu.c.chen@intel.com>
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index b799c00bef09..32e8bbdf2400 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -1155,8 +1155,8 @@ static int rdtgroup_size_show(struct kernfs_open_file *of,
 	struct rdt_resource *r;
 	struct rdt_domain *d;
 	unsigned int size;
-	bool sep = false;
-	u32 cbm;
+	bool sep;
+	u32 ctrl;
 
 	rdtgrp = rdtgroup_kn_lock_live(of->kn);
 	if (!rdtgrp) {
@@ -1174,6 +1174,7 @@ static int rdtgroup_size_show(struct kernfs_open_file *of,
 	}
 
 	for_each_alloc_enabled_rdt_resource(r) {
+		sep = false;
 		seq_printf(s, "%*s:", max_name_width, r->name);
 		list_for_each_entry(d, &r->domains, list) {
 			if (sep)
@@ -1181,8 +1182,13 @@ static int rdtgroup_size_show(struct kernfs_open_file *of,
 			if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP) {
 				size = 0;
 			} else {
-				cbm = d->ctrl_val[rdtgrp->closid];
-				size = rdtgroup_cbm_to_size(r, d, cbm);
+				ctrl = (!is_mba_sc(r) ?
+						d->ctrl_val[rdtgrp->closid] :
+						d->mbps_val[rdtgrp->closid]);
+				if (r->rid == RDT_RESOURCE_MBA)
+					size = ctrl;
+				else
+					size = rdtgroup_cbm_to_size(r, d, ctrl);
 			}
 			seq_printf(s, "%d=%u", d->id, size);
 			sep = true;
-- 
2.19.0

[PATCH 3/9] x86/intel_rdt: Global closid helper to support future fixes

[Fenghua Yu]

From: Reinette Chatre <reinette.chatre@intel.com>

The number of CLOSIDs supported by a system is the minimum number of
CLOSIDs supported by any of its resources. Care should be taken when
iterating over the CLOSIDs of a resource since it may be that the number
of CLOSIDs supported on the system is less than the number of CLOSIDs
supported by the resource.

Introduce a helper function that can be used to query the number of
CLOSIDs that is supported by all resources, irrespective of how many
CLOSIDs are supported by a particular resource.

Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt.h          | 1 +
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/arch/x86/kernel/cpu/intel_rdt.h b/arch/x86/kernel/cpu/intel_rdt.h
index 8c30772e37ed..b2bbba3c4c7e 100644
--- a/arch/x86/kernel/cpu/intel_rdt.h
+++ b/arch/x86/kernel/cpu/intel_rdt.h
@@ -536,6 +536,7 @@ int rdtgroup_pseudo_lock_create(struct rdtgroup *rdtgrp);
 void rdtgroup_pseudo_lock_remove(struct rdtgroup *rdtgrp);
 struct rdt_domain *get_domain_from_cpu(int cpu, struct rdt_resource *r);
 int update_domains(struct rdt_resource *r, int closid);
+int closids_supported(void);
 void closid_free(int closid);
 int alloc_rmid(void);
 void free_rmid(u32 rmid);
diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 32e8bbdf2400..b372923eb209 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -97,6 +97,12 @@ void rdt_last_cmd_printf(const char *fmt, ...)
  *   limited as the number of resources grows.
  */
 static int closid_free_map;
+static int closid_free_map_len;
+
+int closids_supported(void)
+{
+	return closid_free_map_len;
+}
 
 static void closid_init(void)
 {
@@ -111,6 +117,7 @@ static void closid_init(void)
 
 	/* CLOSID 0 is always reserved for the default group */
 	closid_free_map &= ~1;
+	closid_free_map_len = rdt_min_closid;
 }
 
 static int closid_alloc(void)
-- 
2.19.0

[PATCH 4/9] x86/intel_rdt: Fix invalid mode warning when

[Fenghua Yu]

From: Reinette Chatre <reinette.chatre@intel.com>

multiple resources are managed

When multiple resources are managed by RDT, the number of CLOSIDs used
is the minimum of the CLOSIDs supported by each resource. In the function
rdt_bit_usage_show(), the annotated bitmask is created to depict how the
CAT supporting caches are being used. During this annotated bitmask
creation, each resource group is queried for its mode that is used as a
label in the annotated bitmask.

The maximum number of resource groups is currently assumed to be the
number of CLOSIDs supported by the resource for which the information is
being displayed. This is incorrect since the number of active CLOSIDs is
the minimum across all resources.

If information for a cache instance with more CLOSIDs than another is
being generated we thus encounter a warning like:

[  130.010591] ------------[ cut here ]------------
[  130.016680] invalid mode for closid 8
[  130.021791] WARNING: CPU: 88 PID: 1791 at [SNIP]/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
:827 rdt_bit_usage_show+0x221/0x2b0
[  130.037974] Modules linked in: intel_rapl x86_pkg_temp_thermal kvm ipmi_ssif iTCO_wdt vfat fat iTCO_vendor_support pcspkr irqbyp
ass lpc_ich ipmi_si ioatdma joydev i2c_i801 mfd_core ipmi_devintf dca wmi ipmi_msghandler acpi_pad i40e crct10dif_pclmul crc32_pclm
ul nvme crc32c_intel nvme_core sunrpc scsi_transport_iscsi
[  130.067581] CPU: 88 PID: 1791 Comm: grep Not tainted 4.18.0-rc1+ #18
[  130.075065] Hardware name: Intel Corporation S2600WFD/S2600WFD, BIOS SE5C620.86B.01.00.0833.051120182255 05/11/2018
[  130.087022] RIP: 0010:rdt_bit_usage_show+0x221/0x2b0
[  130.093365] Code: 45 84 c0 75 58 84 c0 74 42 be 50 00 00 00 4c 89 e7 e8 53 18 26 00 e9 30 ff ff ff 44 89 fe 48 c7 c7 ed f6 e4 95
 e8 4f d4 06 00 <0f> 0b e9 e3 fe ff ff 41 8b 45 00 09 44 24 08 e9 d6 fe ff ff 41 8b
[  130.113872] RSP: 0018:ffffa7874f147d88 EFLAGS: 00010286
[  130.120347] RAX: 0000000000000000 RBX: ffff8975434e0400 RCX: 0000000000000000
[  130.128963] RDX: ffff898d5f41dbd8 RSI: ffff898d5f415c78 RDI: ffff898d5f415c78
[  130.137562] RBP: ffffffff960285a0 R08: 00000000000005a4 R09: 0000000000aaaaaa
[  130.146158] R10: ffff898d3e040600 R11: 00000000ffffffff R12: ffff898d4b592000
[  130.154755] R13: ffff897543e62de0 R14: 0000000000000001 R15: 0000000000000008
[  130.163355] FS:  00007ff6f43dd740(0000) GS:ffff898d5f400000(0000) knlGS:0000000000000000
[  130.172920] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  130.180093] CR2: 000055b84a20f4d8 CR3: 0000002fcb5c6003 CR4: 00000000007606e0
[  130.187846] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  130.195091] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[  130.202336] PKRU: 55555554
[  130.205093] Call Trace:
[  130.207588]  seq_read+0xee/0x460
[  130.210866]  __vfs_read+0x36/0x170
[  130.214324]  vfs_read+0x89/0x130
[  130.217605]  ksys_read+0x52/0xc0
[  130.220885]  do_syscall_64+0x5b/0x180
[  130.224609]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  130.229733] RIP: 0033:0x7ff6f3c88701

Fix this by ensuring that only the number of supported CLOSIDs are
considered.

Fixes: e651901187ab8 ("x86/intel_rdt: Introduce "bit_usage" to display cache allocations details")
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index b372923eb209..ea91750ba27f 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -809,7 +809,7 @@ static int rdt_bit_usage_show(struct kernfs_open_file *of,
 		sw_shareable = 0;
 		exclusive = 0;
 		seq_printf(seq, "%d=", dom->id);
-		for (i = 0; i < r->num_closid; i++, ctrl++) {
+		for (i = 0; i < closids_supported(); i++, ctrl++) {
 			if (!closid_allocated(i))
 				continue;
 			mode = rdtgroup_mode_by_closid(i);
-- 
2.19.0

[PATCH 5/9] x86/intel_rdt: Fix unchecked MSR access

[Fenghua Yu]

From: Reinette Chatre <reinette.chatre@intel.com>

When a new resource group is created, it is initialized with sane
defaults that currently assume the resource being initialized is a CAT
resource. This code path is also followed by a MBA resource that is not
allocated the same as a CAT resource and as a result we encounter the
following unchecked MSR access error:

[ 6944.864724] unchecked MSR access error: WRMSR to 0xd51 (tried to write 0x0000
000000000064) at rIP: 0xffffffffae059994 (native_write_msr+0x4/0x20)
[ 6944.877967] Call Trace:
[ 6944.880472]  mba_wrmsr+0x41/0x80
[ 6944.883762]  update_domains+0x125/0x130
[ 6944.887667]  rdtgroup_mkdir+0x270/0x500
[ 6944.891572]  kernfs_iop_mkdir+0x5d/0x80
[ 6944.895475]  vfs_mkdir+0x101/0x1b0
[ 6944.898934]  do_mkdirat+0x7b/0xf0
[ 6944.902308]  do_syscall_64+0x5b/0x180
[ 6944.906036]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 6944.911171] RIP: 0033:0x7f5a452b5377
[ 6944.914803] Code: 00 b8 ff ff ff ff c3 0f 1f 40 00 48 8b 05 21 9b 2c 00 64 c7
 00 5f 00 00 00 b8 ff ff ff ff c3 0f 1f 40 00 b8 53 00 00 00 0f 05 <48> 3d 01 f0
 ff ff 73 01 c3 48 8b 0d f9 9a 2c 00 f7 d8 64 89 01 48
[ 6944.933942] RSP: 002b:00007ffe18172f78 EFLAGS: 00000246 ORIG_RAX: 00000000000
00053
[ 6944.941628] RAX: ffffffffffffffda RBX: 00007ffe18174cff RCX: 00007f5a452b5377
[ 6944.948929] RDX: 0000000000000001 RSI: 00000000000001ff RDI: 00007ffe18174cff
[ 6944.956168] RBP: 00007ffe18174cff R08: 00000000000001ff R09: 000056126dad8d30
[ 6944.963407] R10: 0000000000000000 R11: 0000000000000246 R12: 00000000000001ff
[ 6944.970651] R13: 00007ffe181730f0 R14: 0000000000000000 R15: 00007ffe18173140

Fix the above by ensuring the initial allocation is only attempted on a
CAT resource.

Fixes: 95f0b77ef ("x86/intel_rdt: Initialize new resource group with sane defaults")
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index ea91750ba27f..74821bc457c0 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -2349,6 +2349,12 @@ static int rdtgroup_init_alloc(struct rdtgroup *rdtgrp)
 	u32 *ctrl;
 
 	for_each_alloc_enabled_rdt_resource(r) {
+		/*
+		 * Only initialize default allocations for CBM cache
+		 * resources
+		 */
+		if (r->rid == RDT_RESOURCE_MBA)
+			continue;
 		list_for_each_entry(d, &r->domains, list) {
 			d->have_new_ctrl = false;
 			d->new_ctrl = r->cache.shareable_bits;
@@ -2386,6 +2392,12 @@ static int rdtgroup_init_alloc(struct rdtgroup *rdtgrp)
 	}
 
 	for_each_alloc_enabled_rdt_resource(r) {
+		/*
+		 * Only initialize default allocations for CBM cache
+		 * resources
+		 */
+		if (r->rid == RDT_RESOURCE_MBA)
+			continue;
 		ret = update_domains(r, rdtgrp->closid);
 		if (ret < 0) {
 			rdt_last_cmd_puts("failed to initialize allocations\n");
-- 
2.19.0

[PATCH 6/9] x86/intel_rdt: Do not allow pseudo-locking of MBA resource

[Fenghua Yu]

From: Reinette Chatre <reinette.chatre@intel.com>

A system supporting pseudo-locking may have MBA as well as CAT
resources of which only the CAT resources could support cache
pseudo-locking. When the schemata to be pseudo-locked is provided it
should be checked that that schemata does not attempt to pseudo-lock a
MBA resource.

Fixes: e0bdfe8e3 ("x86/intel_rdt: Support creation/removal of pseudo-locked region")
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c b/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c
index d427c86e7cd0..44b7a689d083 100644
--- a/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c
+++ b/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c
@@ -200,6 +200,12 @@ static int parse_line(char *line, struct rdt_resource *r,
 	struct rdt_domain *d;
 	unsigned long dom_id;
 
+	if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP &&
+	    r->rid == RDT_RESOURCE_MBA) {
+		rdt_last_cmd_puts("Cannot pseudo-lock MBA resource\n");
+		return -EINVAL;
+	}
+
 next:
 	if (!line || line[0] == '\0')
 		return 0;
-- 
2.19.0

[PATCH 7/9] x86/intel_rdt: Fix incorrect loop end condition

[Fenghua Yu]

From: Reinette Chatre <reinette.chatre@intel.com>

A loop is used to check if a CAT resource's CBM of one CLOSID
overlaps with the CBM of another CLOSID of the same resource. The loop
is run over all CLOSIDs supported by the resource.

The problem with running the loop over all CLOSIDs supported by the
resource is that its number of supported CLOSIDs may be more than the
number of supported CLOSIDs on the system, which is the minimum number of
CLOSIDs supported across all resources.

Fix the loop to only consider the number of system supported CLOSIDs,
not all that are supported by the resource.

Fixes: 49f7b4efa ("x86/intel_rdt: Enable setting of exclusive mode")
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 74821bc457c0..afd93d45e21b 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -996,7 +996,7 @@ bool rdtgroup_cbm_overlaps(struct rdt_resource *r, struct rdt_domain *d,
 
 	/* Check for overlap with other resource groups */
 	ctrl = d->ctrl_val;
-	for (i = 0; i < r->num_closid; i++, ctrl++) {
+	for (i = 0; i < closids_supported(); i++, ctrl++) {
 		ctrl_b = (unsigned long *)ctrl;
 		mode = rdtgroup_mode_by_closid(i);
 		if (closid_allocated(i) && i != closid &&
-- 
2.19.0

[PATCH 8/9] x86/intel_rdt: Fix exclusive mode handling of MBA resource

[Fenghua Yu]

From: Reinette Chatre <reinette.chatre@intel.com>

It is possible for a resource group to consist out of MBA as well as
CAT/CDP resources. The "exclusive" resource mode only applies to the
CAT/CDP resources since MBA allocations cannot be specified to overlap
or not. When a user requests a resource group to become "exclusive" then it
can only be successful if there are CAT/CDP resources in the group
and none of their CBMs associated with the group's CLOSID overlaps with
any other resource group.

Fix the "exclusive" mode setting by failing if there isn't any CAT/CDP
resource in the group and ensuring that the CBM checking is only done on
CAT/CDP resources.

Fixes: 49f7b4efa ("x86/intel_rdt: Enable setting of exclusive mode")
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index afd93d45e21b..fac99f81d02f 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -1031,16 +1031,27 @@ static bool rdtgroup_mode_test_exclusive(struct rdtgroup *rdtgrp)
 {
 	int closid = rdtgrp->closid;
 	struct rdt_resource *r;
+	bool has_cache = false;
 	struct rdt_domain *d;
 
 	for_each_alloc_enabled_rdt_resource(r) {
+		if (r->rid == RDT_RESOURCE_MBA)
+			continue;
+		has_cache = true;
 		list_for_each_entry(d, &r->domains, list) {
 			if (rdtgroup_cbm_overlaps(r, d, d->ctrl_val[closid],
-						  rdtgrp->closid, false))
+						  rdtgrp->closid, false)) {
+				rdt_last_cmd_puts("schemata overlaps\n");
 				return false;
+			}
 		}
 	}
 
+	if (!has_cache) {
+		rdt_last_cmd_puts("cannot be exclusive without CAT/CDP\n");
+		return false;
+	}
+
 	return true;
 }
 
@@ -1092,7 +1103,6 @@ static ssize_t rdtgroup_mode_write(struct kernfs_open_file *of,
 		rdtgrp->mode = RDT_MODE_SHAREABLE;
 	} else if (!strcmp(buf, "exclusive")) {
 		if (!rdtgroup_mode_test_exclusive(rdtgrp)) {
-			rdt_last_cmd_printf("schemata overlaps\n");
 			ret = -EINVAL;
 			goto out;
 		}
-- 
2.19.0

[PATCH 9/9] x86/intel_rdt: Fix incorrect loop end condition

[Fenghua Yu]

From: Reinette Chatre <reinette.chatre@intel.com>

In order to determine a sane default cache allocation for a new CAT/CDP
resource group, all resource groups are checked to determine which cache
portions are available to share. At this time all possible CLOSIDs
that can be supported by the resource is checked. This is problematic
if the resource supports more CLOSIDs than another CAT/CDP resource. In
this case, the number of CLOSIDs that could be allocated are fewer than
the number of CLOSIDs that can be supported by the resource.

Limit the check of closids to that what is supported by the system based
on the minimum across all resources.

Fixes: 95f0b77ef ("x86/intel_rdt: Initialize new resource group with sane defaults")
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index fac99f81d02f..90b76c1ebd70 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -2370,7 +2370,7 @@ static int rdtgroup_init_alloc(struct rdtgroup *rdtgrp)
 			d->new_ctrl = r->cache.shareable_bits;
 			used_b = r->cache.shareable_bits;
 			ctrl = d->ctrl_val;
-			for (i = 0; i < r->num_closid; i++, ctrl++) {
+			for (i = 0; i < closids_supported(); i++, ctrl++) {
 				if (closid_allocated(i) && i != closid) {
 					mode = rdtgroup_mode_by_closid(i);
 					if (mode == RDT_MODE_PSEUDO_LOCKSETUP)
-- 
2.19.0

Re: [PATCH 4/9] x86/intel_rdt: Fix invalid mode warning when

[Reinette Chatre]

On 9/14/2018 1:32 PM, Fenghua Yu wrote:
> From: Reinette Chatre <reinette.chatre@intel.com>
> 
> multiple resources are managed

The above snippet is redundant. We can remove it in the next version.

Reinette

Re: [PATCH 4/9] x86/intel_rdt: Fix invalid mode warning when

[Reinette Chatre]

On 9/14/2018 9:36 PM, Reinette Chatre wrote:
> On 9/14/2018 1:32 PM, Fenghua Yu wrote:
>> From: Reinette Chatre <reinette.chatre@intel.com>
>>
>> multiple resources are managed
> 
> The above snippet is redundant. We can remove it in the next version.

Apologies, I responded too fast. The subject intended to read:
x86/intel_rdt: Fix invalid mode warning when multiple resources are managed

Reinette

Re: [PATCH 1/9] x86/intel_rdt: Fix MBA parsing callback

[Thomas Gleixner]

On Fri, 14 Sep 2018, Fenghua Yu wrote:
>  int parse_cbm(void *_data, struct rdt_resource *r, struct rdt_domain *d);
> -int parse_bw(void *_buf, struct rdt_resource *r,  struct rdt_domain *d);
> +int parse_bw(void *_data, struct rdt_resource *r, struct rdt_domain *d);

Sorry no. This keeps the code equally error prone as it was. Why is that
argument a void pointer in the first place? 

>  extern struct mutex rdtgroup_mutex;
>  
> diff --git a/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c b/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c
> index af358ca05160..d427c86e7cd0 100644
> --- a/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c
> +++ b/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c
> @@ -28,6 +28,11 @@
>  #include <linux/slab.h>
>  #include "intel_rdt.h"
>  
> +struct rdt_parse_data {
> +	struct rdtgroup		*rdtgrp;
> +	char			*buf;
> +};

This is a copy of rdt_cbm_parse_data. Sigh.

The right thing to do here is

    1) rename struct rdt_cbm_parse_data to struct rdt_parse_data

    2) Move it to a header file

    3) Change the argument of parse_ctrlval from void * to struct
       rdt_parse_data *

Everything else is just proliferating the initial underlying problem of
having a void pointer in those callbacks for no reason at all.

Thanks,

	tglx

Re: [PATCH 4/9] x86/intel_rdt: Fix invalid mode warning when

[Thomas Gleixner]

On Fri, 14 Sep 2018, Fenghua Yu wrote:
> If information for a cache instance with more CLOSIDs than another is
> being generated we thus encounter a warning like:
> 
> [  130.010591] ------------[ cut here ]------------
> [  130.016680] invalid mode for closid 8
> [  130.021791] WARNING: CPU: 88 PID: 1791 at [SNIP]/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> :827 rdt_bit_usage_show+0x221/0x2b0
> [  130.037974] Modules linked in: intel_rapl x86_pkg_temp_thermal kvm ipmi_ssif iTCO_wdt vfat fat iTCO_vendor_support pcspkr irqbyp
> ass lpc_ich ipmi_si ioatdma joydev i2c_i801 mfd_core ipmi_devintf dca wmi ipmi_msghandler acpi_pad i40e crct10dif_pclmul crc32_pclm
> ul nvme crc32c_intel nvme_core sunrpc scsi_transport_iscsi
> [  130.067581] CPU: 88 PID: 1791 Comm: grep Not tainted 4.18.0-rc1+ #18
> [  130.075065] Hardware name: Intel Corporation S2600WFD/S2600WFD, BIOS SE5C620.86B.01.00.0833.051120182255 05/11/2018
> [  130.087022] RIP: 0010:rdt_bit_usage_show+0x221/0x2b0
> [  130.093365] Code: 45 84 c0 75 58 84 c0 74 42 be 50 00 00 00 4c 89 e7 e8 53 18 26 00 e9 30 ff ff ff 44 89 fe 48 c7 c7 ed f6 e4 95
>  e8 4f d4 06 00 <0f> 0b e9 e3 fe ff ff 41 8b 45 00 09 44 24 08 e9 d6 fe ff ff 41 8b
> [  130.113872] RSP: 0018:ffffa7874f147d88 EFLAGS: 00010286
> [  130.120347] RAX: 0000000000000000 RBX: ffff8975434e0400 RCX: 0000000000000000
> [  130.128963] RDX: ffff898d5f41dbd8 RSI: ffff898d5f415c78 RDI: ffff898d5f415c78
> [  130.137562] RBP: ffffffff960285a0 R08: 00000000000005a4 R09: 0000000000aaaaaa
> [  130.146158] R10: ffff898d3e040600 R11: 00000000ffffffff R12: ffff898d4b592000
> [  130.154755] R13: ffff897543e62de0 R14: 0000000000000001 R15: 0000000000000008
> [  130.163355] FS:  00007ff6f43dd740(0000) GS:ffff898d5f400000(0000) knlGS:0000000000000000
> [  130.172920] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  130.180093] CR2: 000055b84a20f4d8 CR3: 0000002fcb5c6003 CR4: 00000000007606e0
> [  130.187846] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [  130.195091] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [  130.202336] PKRU: 55555554
> [  130.205093] Call Trace:
> [  130.207588]  seq_read+0xee/0x460
> [  130.210866]  __vfs_read+0x36/0x170
> [  130.214324]  vfs_read+0x89/0x130
> [  130.217605]  ksys_read+0x52/0xc0
> [  130.220885]  do_syscall_64+0x5b/0x180
> [  130.224609]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [  130.229733] RIP: 0033:0x7ff6f3c88701

Can you please condense all these backtraces to the absolute minimum?

In this case the only interresting part is:

 invalid mode for closid 8
 WARNING: CPU: 88 PID: 1791 at [SNIP]/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c:827
 rdt_bit_usage_show

and if at all a stripped stack trace:

 seq_read
 __vfs_read
 vfs_read
 ksys_read
 do_syscall_64

But think about it whether the stack trace provides actually useful
information or not. It's a well defined call chain.

The rest of the stuff I stripped is really completely irrelevant for
describing the problem. It just occupies space and distracts.

Thanks,

	tglx

Re: [PATCH 1/9] x86/intel_rdt: Fix MBA parsing callback

[Fenghua Yu]

On Sat, Sep 15, 2018 at 12:13:53PM +0200, Thomas Gleixner wrote:
> On Fri, 14 Sep 2018, Fenghua Yu wrote:
> > +int parse_bw(void *_data, struct rdt_resource *r, struct rdt_domain *d);
> 
> Sorry no. This keeps the code equally error prone as it was. Why is that
> argument a void pointer in the first place? 
> 
> >  extern struct mutex rdtgroup_mutex;
> 
> This is a copy of rdt_cbm_parse_data. Sigh.
> 
> The right thing to do here is
> 
>     1) rename struct rdt_cbm_parse_data to struct rdt_parse_data
> 
>     2) Move it to a header file
> 
>     3) Change the argument of parse_ctrlval from void * to struct
>        rdt_parse_data *
> 
> Everything else is just proliferating the initial underlying problem of
> having a void pointer in those callbacks for no reason at all.

Sure. I have updated this patch and patch 2, 4, 5 based on your comments.

Thanks.

-Fenghua