[RFC PATCH 00/10] arch/x86: AMD QoS support

[RFC PATCH 00/10] arch/x86: AMD QoS support

[Moger, Babu]

This series adds support for AMD64 architectural extensions for Platform
Quality of Service. These extensions are intended to provide for the
monitoring of the usage of certain system resources by one or more
processors and for the separate allocation and enforcement of limits on
the use of certain system resources by one or more processors.

The monitoring and enforcement are not necessarily applied across the
entire system, but in general apply to a QOS domain which corresponds to
some shared system resource.  The set of resources which are monitored and
the set for which the enforcement of limits is provided are implementation
dependent. Platform QOS features are implemented on a logical processor basis.
Therefore, multiple hardware threads of a single physical CPU core may have
independent resource monitoring and enforcement configurations.

AMD's next generation of processors support following QoS sub-features.
- L3 Cache allocation enforcement
- L3 Cache occupancy monitoring
- L3 Code-Data Prioritization support
- Memory Bandwidth Enforcement(Allocation)

The public specification is still in works. Will add the link when it is
available.

Obviously, there are multiple ways we can go about these changes. We felt
it is appropriate to rename and re-organize the code little bit before
making the functional changes. The first few patches(1-6) renames and
re-organizes the sources in preparation. Rest of the patches(7-10) adds
support for AMD QoS features.

Please review and provide me feedback. If you think of better way to
approach this, please let us know. 

Babu Moger (9):
  arch/x86: Start renaming the rdt files to more generic names
  arch/x86: Rename the RDT functions and definitions
  arch/x86: Re-arrange RDT init code
  arch/x86: Introduce a new config parameter PLATFORM_QOS
  arch/x86: Use new config parameter PLATFORM_QOS for compilation
  arch/x86: Initialize the resource functions that are different
  arch/x86: Bring few more functions into the resource structure
  arch/x86: Introduce new config parameter AMD_QOS
  arch/x86: Introduce QOS feature for AMD

Sherry Hurwitz (1):
  arch/x86: Add AMD feature bit X86_FEATURE_MBA in cpuid bits array

 arch/x86/Kconfig                              |  19 ++
 .../asm/{intel_rdt_sched.h => rdt_sched.h}    |  26 +--
 arch/x86/kernel/cpu/Makefile                  |   6 +-
 arch/x86/kernel/cpu/{intel_rdt.c => rdt.c}    | 175 +++++++++++++++---
 arch/x86/kernel/cpu/{intel_rdt.h => rdt.h}    |  34 +++-
 ...el_rdt_ctrlmondata.c => rdt_ctrlmondata.c} |  87 ++++++++-
 .../{intel_rdt_monitor.c => rdt_monitor.c}    |  22 ++-
 ...el_rdt_pseudo_lock.c => rdt_pseudo_lock.c} |   6 +-
 ...o_lock_event.h => rdt_pseudo_lock_event.h} |   2 +-
 .../{intel_rdt_rdtgroup.c => rdt_rdtgroup.c}  |  14 +-
 arch/x86/kernel/cpu/scattered.c               |   1 +
 arch/x86/kernel/process_32.c                  |   4 +-
 arch/x86/kernel/process_64.c                  |   4 +-
 include/linux/sched.h                         |   2 +-
 14 files changed, 319 insertions(+), 83 deletions(-)
 rename arch/x86/include/asm/{intel_rdt_sched.h => rdt_sched.h} (80%)
 rename arch/x86/kernel/cpu/{intel_rdt.c => rdt.c} (84%)
 rename arch/x86/kernel/cpu/{intel_rdt.h => rdt.h} (93%)
 rename arch/x86/kernel/cpu/{intel_rdt_ctrlmondata.c => rdt_ctrlmondata.c} (84%)
 rename arch/x86/kernel/cpu/{intel_rdt_monitor.c => rdt_monitor.c} (97%)
 rename arch/x86/kernel/cpu/{intel_rdt_pseudo_lock.c => rdt_pseudo_lock.c} (99%)
 rename arch/x86/kernel/cpu/{intel_rdt_pseudo_lock_event.h => rdt_pseudo_lock_event.h} (95%)
 rename arch/x86/kernel/cpu/{intel_rdt_rdtgroup.c => rdt_rdtgroup.c} (99%)

-- 
2.17.1

[RFC PATCH 01/10] arch/x86: Start renaming the rdt files to more generic names

[Moger, Babu]

New generation of AMD processors start support RDT(or QOS) features.
With more than one vendors supporting these features, it seems more
appropriate to rename these files.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/include/asm/{intel_rdt_sched.h => rdt_sched.h}     | 0
 arch/x86/kernel/cpu/Makefile                                | 6 +++---
 arch/x86/kernel/cpu/{intel_rdt.c => rdt.c}                  | 4 ++--
 arch/x86/kernel/cpu/{intel_rdt.h => rdt.h}                  | 0
 .../cpu/{intel_rdt_ctrlmondata.c => rdt_ctrlmondata.c}      | 2 +-
 arch/x86/kernel/cpu/{intel_rdt_monitor.c => rdt_monitor.c}  | 2 +-
 .../cpu/{intel_rdt_pseudo_lock.c => rdt_pseudo_lock.c}      | 6 +++---
 ...ntel_rdt_pseudo_lock_event.h => rdt_pseudo_lock_event.h} | 2 +-
 .../x86/kernel/cpu/{intel_rdt_rdtgroup.c => rdt_rdtgroup.c} | 4 ++--
 arch/x86/kernel/process_32.c                                | 2 +-
 arch/x86/kernel/process_64.c                                | 2 +-
 11 files changed, 15 insertions(+), 15 deletions(-)
 rename arch/x86/include/asm/{intel_rdt_sched.h => rdt_sched.h} (100%)
 rename arch/x86/kernel/cpu/{intel_rdt.c => rdt.c} (99%)
 rename arch/x86/kernel/cpu/{intel_rdt.h => rdt.h} (100%)
 rename arch/x86/kernel/cpu/{intel_rdt_ctrlmondata.c => rdt_ctrlmondata.c} (99%)
 rename arch/x86/kernel/cpu/{intel_rdt_monitor.c => rdt_monitor.c} (99%)
 rename arch/x86/kernel/cpu/{intel_rdt_pseudo_lock.c => rdt_pseudo_lock.c} (99%)
 rename arch/x86/kernel/cpu/{intel_rdt_pseudo_lock_event.h => rdt_pseudo_lock_event.h} (95%)
 rename arch/x86/kernel/cpu/{intel_rdt_rdtgroup.c => rdt_rdtgroup.c} (99%)

diff --git a/arch/x86/include/asm/intel_rdt_sched.h b/arch/x86/include/asm/rdt_sched.h
similarity index 100%
rename from arch/x86/include/asm/intel_rdt_sched.h
rename to arch/x86/include/asm/rdt_sched.h
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index 347137e80bf5..6c35d89f174f 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -35,9 +35,9 @@ obj-$(CONFIG_CPU_SUP_CENTAUR)		+= centaur.o
 obj-$(CONFIG_CPU_SUP_TRANSMETA_32)	+= transmeta.o
 obj-$(CONFIG_CPU_SUP_UMC_32)		+= umc.o
 
-obj-$(CONFIG_INTEL_RDT)	+= intel_rdt.o intel_rdt_rdtgroup.o intel_rdt_monitor.o
-obj-$(CONFIG_INTEL_RDT)	+= intel_rdt_ctrlmondata.o intel_rdt_pseudo_lock.o
-CFLAGS_intel_rdt_pseudo_lock.o = -I$(src)
+obj-$(CONFIG_INTEL_RDT)	+= rdt.o rdt_rdtgroup.o rdt_monitor.o
+obj-$(CONFIG_INTEL_RDT)	+= rdt_ctrlmondata.o rdt_pseudo_lock.o
+CFLAGS_rdt_pseudo_lock.o = -I$(src)
 
 obj-$(CONFIG_X86_MCE)			+= mcheck/
 obj-$(CONFIG_MTRR)			+= mtrr/
diff --git a/arch/x86/kernel/cpu/intel_rdt.c b/arch/x86/kernel/cpu/rdt.c
similarity index 99%
rename from arch/x86/kernel/cpu/intel_rdt.c
rename to arch/x86/kernel/cpu/rdt.c
index abb71ac70443..28d6cd254ba9 100644
--- a/arch/x86/kernel/cpu/intel_rdt.c
+++ b/arch/x86/kernel/cpu/rdt.c
@@ -30,8 +30,8 @@
 #include <linux/cpuhotplug.h>
 
 #include <asm/intel-family.h>
-#include <asm/intel_rdt_sched.h>
-#include "intel_rdt.h"
+#include <asm/rdt_sched.h>
+#include "rdt.h"
 
 #define MBA_IS_LINEAR	0x4
 #define MBA_MAX_MBPS	U32_MAX
diff --git a/arch/x86/kernel/cpu/intel_rdt.h b/arch/x86/kernel/cpu/rdt.h
similarity index 100%
rename from arch/x86/kernel/cpu/intel_rdt.h
rename to arch/x86/kernel/cpu/rdt.h
diff --git a/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
similarity index 99%
rename from arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c
rename to arch/x86/kernel/cpu/rdt_ctrlmondata.c
index af358ca05160..0565c564b297 100644
--- a/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c
+++ b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
@@ -26,7 +26,7 @@
 #include <linux/kernfs.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
-#include "intel_rdt.h"
+#include "rdt.h"
 
 /*
  * Check whether MBA bandwidth percentage value is correct. The value is
diff --git a/arch/x86/kernel/cpu/intel_rdt_monitor.c b/arch/x86/kernel/cpu/rdt_monitor.c
similarity index 99%
rename from arch/x86/kernel/cpu/intel_rdt_monitor.c
rename to arch/x86/kernel/cpu/rdt_monitor.c
index b0f3aed76b75..2898a61cbdd9 100644
--- a/arch/x86/kernel/cpu/intel_rdt_monitor.c
+++ b/arch/x86/kernel/cpu/rdt_monitor.c
@@ -26,7 +26,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <asm/cpu_device_id.h>
-#include "intel_rdt.h"
+#include "rdt.h"
 
 #define MSR_IA32_QM_CTR		0x0c8e
 #define MSR_IA32_QM_EVTSEL		0x0c8d
diff --git a/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c b/arch/x86/kernel/cpu/rdt_pseudo_lock.c
similarity index 99%
rename from arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
rename to arch/x86/kernel/cpu/rdt_pseudo_lock.c
index 40f3903ae5d9..6105a2af3216 100644
--- a/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
+++ b/arch/x86/kernel/cpu/rdt_pseudo_lock.c
@@ -23,13 +23,13 @@
 
 #include <asm/cacheflush.h>
 #include <asm/intel-family.h>
-#include <asm/intel_rdt_sched.h>
+#include <asm/rdt_sched.h>
 #include <asm/perf_event.h>
 
-#include "intel_rdt.h"
+#include "rdt.h"
 
 #define CREATE_TRACE_POINTS
-#include "intel_rdt_pseudo_lock_event.h"
+#include "rdt_pseudo_lock_event.h"
 
 /*
  * MSR_MISC_FEATURE_CONTROL register enables the modification of hardware
diff --git a/arch/x86/kernel/cpu/intel_rdt_pseudo_lock_event.h b/arch/x86/kernel/cpu/rdt_pseudo_lock_event.h
similarity index 95%
rename from arch/x86/kernel/cpu/intel_rdt_pseudo_lock_event.h
rename to arch/x86/kernel/cpu/rdt_pseudo_lock_event.h
index 2c041e6d9f05..5c6eda48bdc1 100644
--- a/arch/x86/kernel/cpu/intel_rdt_pseudo_lock_event.h
+++ b/arch/x86/kernel/cpu/rdt_pseudo_lock_event.h
@@ -39,5 +39,5 @@ TRACE_EVENT(pseudo_lock_l3,
 
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
-#define TRACE_INCLUDE_FILE intel_rdt_pseudo_lock_event
+#define TRACE_INCLUDE_FILE rdt_pseudo_lock_event
 #include <trace/define_trace.h>
diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/rdt_rdtgroup.c
similarity index 99%
rename from arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
rename to arch/x86/kernel/cpu/rdt_rdtgroup.c
index b799c00bef09..c4a226ebf4e9 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/rdt_rdtgroup.c
@@ -35,8 +35,8 @@
 
 #include <uapi/linux/magic.h>
 
-#include <asm/intel_rdt_sched.h>
-#include "intel_rdt.h"
+#include <asm/rdt_sched.h>
+#include "rdt.h"
 
 DEFINE_STATIC_KEY_FALSE(rdt_enable_key);
 DEFINE_STATIC_KEY_FALSE(rdt_mon_enable_key);
diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c
index 5046a3c9dec2..931b2d0cb95e 100644
--- a/arch/x86/kernel/process_32.c
+++ b/arch/x86/kernel/process_32.c
@@ -56,7 +56,7 @@
 #include <asm/debugreg.h>
 #include <asm/switch_to.h>
 #include <asm/vm86.h>
-#include <asm/intel_rdt_sched.h>
+#include <asm/rdt_sched.h>
 #include <asm/proto.h>
 
 void __show_regs(struct pt_regs *regs, enum show_regs_mode mode)
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index ea5ea850348d..c029782a9216 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -52,7 +52,7 @@
 #include <asm/switch_to.h>
 #include <asm/xen/hypervisor.h>
 #include <asm/vdso.h>
-#include <asm/intel_rdt_sched.h>
+#include <asm/rdt_sched.h>
 #include <asm/unistd.h>
 #ifdef CONFIG_IA32_EMULATION
 /* Not included via unistd.h */
-- 
2.17.1

[RFC PATCH 02/10] arch/x86: Rename the RDT functions and definitions

[Moger, Babu]

As AMD is starting to support RDT(or QOS) features, rename
the RDT functions and definitions to more generic names.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/include/asm/rdt_sched.h   | 22 +++++++++++-----------
 arch/x86/kernel/cpu/rdt.c          | 24 ++++++++++++------------
 arch/x86/kernel/cpu/rdt.h          |  8 ++++----
 arch/x86/kernel/cpu/rdt_monitor.c  | 10 +++++-----
 arch/x86/kernel/cpu/rdt_rdtgroup.c | 10 +++++-----
 arch/x86/kernel/process_32.c       |  2 +-
 arch/x86/kernel/process_64.c       |  2 +-
 7 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/arch/x86/include/asm/rdt_sched.h b/arch/x86/include/asm/rdt_sched.h
index 9acb06b6f81e..666bf9acb41d 100644
--- a/arch/x86/include/asm/rdt_sched.h
+++ b/arch/x86/include/asm/rdt_sched.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _ASM_X86_INTEL_RDT_SCHED_H
-#define _ASM_X86_INTEL_RDT_SCHED_H
+#ifndef _ASM_X86_RDT_SCHED_H
+#define _ASM_X86_RDT_SCHED_H
 
 #ifdef CONFIG_INTEL_RDT
 
@@ -24,21 +24,21 @@
  * The cache also helps to avoid pointless updates if the value does
  * not change.
  */
-struct intel_pqr_state {
+struct rdt_pqr_state {
 	u32			cur_rmid;
 	u32			cur_closid;
 	u32			default_rmid;
 	u32			default_closid;
 };
 
-DECLARE_PER_CPU(struct intel_pqr_state, pqr_state);
+DECLARE_PER_CPU(struct rdt_pqr_state, pqr_state);
 
 DECLARE_STATIC_KEY_FALSE(rdt_enable_key);
 DECLARE_STATIC_KEY_FALSE(rdt_alloc_enable_key);
 DECLARE_STATIC_KEY_FALSE(rdt_mon_enable_key);
 
 /*
- * __intel_rdt_sched_in() - Writes the task's CLOSid/RMID to IA32_PQR_MSR
+ * __rdt_sched_in() - Writes the task's CLOSid/RMID to IA32_PQR_MSR
  *
  * Following considerations are made so that this has minimal impact
  * on scheduler hot path:
@@ -51,9 +51,9 @@ DECLARE_STATIC_KEY_FALSE(rdt_mon_enable_key);
  *   simple as possible.
  * Must be called with preemption disabled.
  */
-static void __intel_rdt_sched_in(void)
+static void __rdt_sched_in(void)
 {
-	struct intel_pqr_state *state = this_cpu_ptr(&pqr_state);
+	struct rdt_pqr_state *state = this_cpu_ptr(&pqr_state);
 	u32 closid = state->default_closid;
 	u32 rmid = state->default_rmid;
 
@@ -78,16 +78,16 @@ static void __intel_rdt_sched_in(void)
 	}
 }
 
-static inline void intel_rdt_sched_in(void)
+static inline void rdt_sched_in(void)
 {
 	if (static_branch_likely(&rdt_enable_key))
-		__intel_rdt_sched_in();
+		__rdt_sched_in();
 }
 
 #else
 
-static inline void intel_rdt_sched_in(void) {}
+static inline void rdt_sched_in(void) {}
 
 #endif /* CONFIG_INTEL_RDT */
 
-#endif /* _ASM_X86_INTEL_RDT_SCHED_H */
+#endif /* _ASM_X86_RDT_SCHED_H */
diff --git a/arch/x86/kernel/cpu/rdt.c b/arch/x86/kernel/cpu/rdt.c
index 28d6cd254ba9..b361c63170d7 100644
--- a/arch/x86/kernel/cpu/rdt.c
+++ b/arch/x86/kernel/cpu/rdt.c
@@ -40,12 +40,12 @@
 DEFINE_MUTEX(rdtgroup_mutex);
 
 /*
- * The cached intel_pqr_state is strictly per CPU and can never be
+ * The cached rdt_pqr_state is strictly per CPU and can never be
  * updated from a remote CPU. Functions which modify the state
  * are called with interrupts disabled and no preemption, which
  * is sufficient for the protection.
  */
-DEFINE_PER_CPU(struct intel_pqr_state, pqr_state);
+DEFINE_PER_CPU(struct rdt_pqr_state, pqr_state);
 
 /*
  * Used to store the max resource name width and max resource data width
@@ -634,7 +634,7 @@ static void domain_remove_cpu(int cpu, struct rdt_resource *r)
 
 static void clear_closid_rmid(int cpu)
 {
-	struct intel_pqr_state *state = this_cpu_ptr(&pqr_state);
+	struct rdt_pqr_state *state = this_cpu_ptr(&pqr_state);
 
 	state->default_closid = 0;
 	state->default_rmid = 0;
@@ -643,7 +643,7 @@ static void clear_closid_rmid(int cpu)
 	wrmsr(IA32_PQR_ASSOC, 0, 0);
 }
 
-static int intel_rdt_online_cpu(unsigned int cpu)
+static int rdt_online_cpu(unsigned int cpu)
 {
 	struct rdt_resource *r;
 
@@ -669,7 +669,7 @@ static void clear_childcpus(struct rdtgroup *r, unsigned int cpu)
 	}
 }
 
-static int intel_rdt_offline_cpu(unsigned int cpu)
+static int rdt_offline_cpu(unsigned int cpu)
 {
 	struct rdtgroup *rdtgrp;
 	struct rdt_resource *r;
@@ -861,7 +861,7 @@ static __init bool get_rdt_resources(void)
 
 static enum cpuhp_state rdt_online;
 
-static int __init intel_rdt_late_init(void)
+static int __init rdt_late_init(void)
 {
 	struct rdt_resource *r;
 	int state, ret;
@@ -873,7 +873,7 @@ static int __init intel_rdt_late_init(void)
 
 	state = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
 				  "x86/rdt/cat:online:",
-				  intel_rdt_online_cpu, intel_rdt_offline_cpu);
+				  rdt_online_cpu, rdt_offline_cpu);
 	if (state < 0)
 		return state;
 
@@ -885,20 +885,20 @@ static int __init intel_rdt_late_init(void)
 	rdt_online = state;
 
 	for_each_alloc_capable_rdt_resource(r)
-		pr_info("Intel RDT %s allocation detected\n", r->name);
+		pr_info("RDT %s allocation detected\n", r->name);
 
 	for_each_mon_capable_rdt_resource(r)
-		pr_info("Intel RDT %s monitoring detected\n", r->name);
+		pr_info("RDT %s monitoring detected\n", r->name);
 
 	return 0;
 }
 
-late_initcall(intel_rdt_late_init);
+late_initcall(rdt_late_init);
 
-static void __exit intel_rdt_exit(void)
+static void __exit rdt_exit(void)
 {
 	cpuhp_remove_state(rdt_online);
 	rdtgroup_exit();
 }
 
-__exitcall(intel_rdt_exit);
+__exitcall(rdt_exit);
diff --git a/arch/x86/kernel/cpu/rdt.h b/arch/x86/kernel/cpu/rdt.h
index 4e588f36228f..c15417a6b1af 100644
--- a/arch/x86/kernel/cpu/rdt.h
+++ b/arch/x86/kernel/cpu/rdt.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _ASM_X86_INTEL_RDT_H
-#define _ASM_X86_INTEL_RDT_H
+#ifndef _ASM_X86_RDT_H
+#define _ASM_X86_RDT_H
 
 #include <linux/sched.h>
 #include <linux/kernfs.h>
@@ -69,7 +69,7 @@ struct rmid_read {
 	u64			val;
 };
 
-extern unsigned int intel_cqm_threshold;
+extern unsigned int rdt_cqm_threshold;
 extern bool rdt_alloc_capable;
 extern bool rdt_mon_capable;
 extern unsigned int rdt_mon_features;
@@ -559,4 +559,4 @@ 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);
 
-#endif /* _ASM_X86_INTEL_RDT_H */
+#endif /* _ASM_X86_RDT_H */
diff --git a/arch/x86/kernel/cpu/rdt_monitor.c b/arch/x86/kernel/cpu/rdt_monitor.c
index 2898a61cbdd9..577514cd4a71 100644
--- a/arch/x86/kernel/cpu/rdt_monitor.c
+++ b/arch/x86/kernel/cpu/rdt_monitor.c
@@ -73,7 +73,7 @@ unsigned int rdt_mon_features;
  * This is the threshold cache occupancy at which we will consider an
  * RMID available for re-allocation.
  */
-unsigned int intel_cqm_threshold;
+unsigned int rdt_cqm_threshold;
 
 static inline struct rmid_entry *__rmid_entry(u32 rmid)
 {
@@ -107,7 +107,7 @@ static bool rmid_dirty(struct rmid_entry *entry)
 {
 	u64 val = __rmid_read(entry->rmid, QOS_L3_OCCUP_EVENT_ID);
 
-	return val >= intel_cqm_threshold;
+	return val >= rdt_cqm_threshold;
 }
 
 /*
@@ -187,7 +187,7 @@ static void add_rmid_to_limbo(struct rmid_entry *entry)
 	list_for_each_entry(d, &r->domains, list) {
 		if (cpumask_test_cpu(cpu, &d->cpu_mask)) {
 			val = __rmid_read(entry->rmid, QOS_L3_OCCUP_EVENT_ID);
-			if (val <= intel_cqm_threshold)
+			if (val <= rdt_cqm_threshold)
 				continue;
 		}
 
@@ -637,10 +637,10 @@ int rdt_get_mon_l3_config(struct rdt_resource *r)
 	 *
 	 * For a 35MB LLC and 56 RMIDs, this is ~1.8% of the LLC.
 	 */
-	intel_cqm_threshold = boot_cpu_data.x86_cache_size * 1024 / r->num_rmid;
+	rdt_cqm_threshold = boot_cpu_data.x86_cache_size * 1024 / r->num_rmid;
 
 	/* h/w works in units of "boot_cpu_data.x86_cache_occ_scale" */
-	intel_cqm_threshold /= r->mon_scale;
+	rdt_cqm_threshold /= r->mon_scale;
 
 	ret = dom_data_init(r);
 	if (ret)
diff --git a/arch/x86/kernel/cpu/rdt_rdtgroup.c b/arch/x86/kernel/cpu/rdt_rdtgroup.c
index c4a226ebf4e9..159b56980715 100644
--- a/arch/x86/kernel/cpu/rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/rdt_rdtgroup.c
@@ -281,7 +281,7 @@ static int rdtgroup_cpus_show(struct kernfs_open_file *of,
 }
 
 /*
- * This is safe against intel_rdt_sched_in() called from __switch_to()
+ * This is safe against rdt_sched_in() called from __switch_to()
  * because __switch_to() is executed with interrupts disabled. A local call
  * from update_closid_rmid() is proteced against __switch_to() because
  * preemption is disabled.
@@ -300,7 +300,7 @@ static void update_cpu_closid_rmid(void *info)
 	 * executing task might have its own closid selected. Just reuse
 	 * the context switch code.
 	 */
-	intel_rdt_sched_in();
+	rdt_sched_in();
 }
 
 /*
@@ -525,7 +525,7 @@ static void move_myself(struct callback_head *head)
 
 	preempt_disable();
 	/* update PQR_ASSOC MSR to make resource group go into effect */
-	intel_rdt_sched_in();
+	rdt_sched_in();
 	preempt_enable();
 
 	kfree(callback);
@@ -909,7 +909,7 @@ static int max_threshold_occ_show(struct kernfs_open_file *of,
 {
 	struct rdt_resource *r = of->kn->parent->priv;
 
-	seq_printf(seq, "%u\n", intel_cqm_threshold * r->mon_scale);
+	seq_printf(seq, "%u\n", rdt_cqm_threshold * r->mon_scale);
 
 	return 0;
 }
@@ -928,7 +928,7 @@ static ssize_t max_threshold_occ_write(struct kernfs_open_file *of,
 	if (bytes > (boot_cpu_data.x86_cache_size * 1024))
 		return -EINVAL;
 
-	intel_cqm_threshold = bytes / r->mon_scale;
+	rdt_cqm_threshold = bytes / r->mon_scale;
 
 	return nbytes;
 }
diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c
index 931b2d0cb95e..d9e7e5668fe1 100644
--- a/arch/x86/kernel/process_32.c
+++ b/arch/x86/kernel/process_32.c
@@ -302,7 +302,7 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
 	this_cpu_write(current_task, next_p);
 
 	/* Load the Intel cache allocation PQR MSR. */
-	intel_rdt_sched_in();
+	rdt_sched_in();
 
 	return prev_p;
 }
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index c029782a9216..3b38d37b7742 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -536,7 +536,7 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
 	}
 
 	/* Load the Intel cache allocation PQR MSR. */
-	intel_rdt_sched_in();
+	rdt_sched_in();
 
 	return prev_p;
 }
-- 
2.17.1

[RFC PATCH 03/10] arch/x86: Re-arrange RDT init code

[Moger, Babu]

Re-organize the RDT init code. Separate the call sequence for each
feature. That way, it is easy to call quirks or features separately
for each vendor if there are differences.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/rdt.c | 44 ++++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/cpu/rdt.c b/arch/x86/kernel/cpu/rdt.c
index b361c63170d7..736715b81fd8 100644
--- a/arch/x86/kernel/cpu/rdt.c
+++ b/arch/x86/kernel/cpu/rdt.c
@@ -813,10 +813,6 @@ static __init bool get_rdt_alloc_resources(void)
 		ret = true;
 	}
 
-	if (rdt_cpu_has(X86_FEATURE_MBA)) {
-		if (rdt_get_mem_config(&rdt_resources_all[RDT_RESOURCE_MBA]))
-			ret = true;
-	}
 	return ret;
 }
 
@@ -831,11 +827,12 @@ static __init bool get_rdt_mon_resources(void)
 
 	if (!rdt_mon_features)
 		return false;
+	else
+		return true;
 
-	return !rdt_get_mon_l3_config(&rdt_resources_all[RDT_RESOURCE_L3]);
 }
 
-static __init void rdt_quirks(void)
+static __init void rdt_quirks_intel(void)
 {
 	switch (boot_cpu_data.x86_model) {
 	case INTEL_FAM6_HASWELL_X:
@@ -850,13 +847,22 @@ static __init void rdt_quirks(void)
 	}
 }
 
-static __init bool get_rdt_resources(void)
+static __init void rdt_quirks(void)
 {
-	rdt_quirks();
-	rdt_alloc_capable = get_rdt_alloc_resources();
-	rdt_mon_capable = get_rdt_mon_resources();
+	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
+		rdt_quirks_intel();
+}
+
+static __init void rdt_detect_l3_mon(void)
+{
+	if (rdt_mon_capable)
+		rdt_get_mon_l3_config(&rdt_resources_all[RDT_RESOURCE_L3]);
+}
 
-	return (rdt_mon_capable || rdt_alloc_capable);
+static __init void rdt_check_mba(void)
+{
+	if (rdt_cpu_has(X86_FEATURE_MBA))
+		rdt_get_mem_config(&rdt_resources_all[RDT_RESOURCE_MBA]);
 }
 
 static enum cpuhp_state rdt_online;
@@ -866,8 +872,22 @@ static int __init rdt_late_init(void)
 	struct rdt_resource *r;
 	int state, ret;
 
-	if (!get_rdt_resources())
+	/* Run quirks first */
+	rdt_quirks();
+
+	rdt_alloc_capable = get_rdt_alloc_resources();
+	rdt_mon_capable = get_rdt_mon_resources();
+
+	if (!(rdt_alloc_capable || rdt_mon_capable)) {
+		pr_info("RDT allocation or monitoring not detected\n");
 		return -ENODEV;
+	}
+
+	/* Detect l3 monitoring resources */
+	rdt_detect_l3_mon();
+
+	/* Check for Memory Bandwidth Allocation */
+	rdt_check_mba();
 
 	rdt_init_padding();
 
-- 
2.17.1

[RFC PATCH 04/10] arch/x86: Introduce a new config parameter PLATFORM_QOS

[Moger, Babu]

Introduces a new config parameter PLATFORM_QOS.

This will be used as a common config parameter for both Intel and AMD.
Each vendor will have their own config parameter to enable RDT feature.
One for Intel(INTEL_RDT) and one for AMD(AMD_QOS). It can be enabled or
disabled separately. The new parameter PLATFORM_QOS will be dependent
on INTEL_RDT or AMD_QOS.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 1a0be022f91d..7f2da780a327 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -458,6 +458,10 @@ config INTEL_RDT
 
 	  Say N if unsure.
 
+config PLATFORM_QOS
+	def_bool y
+	depends on X86 && INTEL_RDT
+
 if X86_32
 config X86_BIGSMP
 	bool "Support for big SMP systems with more than 8 CPUs"
-- 
2.17.1

[RFC PATCH 05/10] arch/x86: Use new config parameter PLATFORM_QOS for compilation

[Moger, Babu]

Use newly added config parameter PLATFORM_QOS to compile sources.
This is common parameter across both Intel and AMD.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/include/asm/rdt_sched.h | 4 ++--
 arch/x86/kernel/cpu/Makefile     | 4 ++--
 include/linux/sched.h            | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/rdt_sched.h b/arch/x86/include/asm/rdt_sched.h
index 666bf9acb41d..6018a362d1cf 100644
--- a/arch/x86/include/asm/rdt_sched.h
+++ b/arch/x86/include/asm/rdt_sched.h
@@ -2,7 +2,7 @@
 #ifndef _ASM_X86_RDT_SCHED_H
 #define _ASM_X86_RDT_SCHED_H
 
-#ifdef CONFIG_INTEL_RDT
+#ifdef CONFIG_PLATFORM_QOS
 
 #include <linux/sched.h>
 #include <linux/jump_label.h>
@@ -88,6 +88,6 @@ static inline void rdt_sched_in(void)
 
 static inline void rdt_sched_in(void) {}
 
-#endif /* CONFIG_INTEL_RDT */
+#endif /* CONFIG_PLATFORM_QOS */
 
 #endif /* _ASM_X86_RDT_SCHED_H */
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index 6c35d89f174f..8655adc84f11 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -35,8 +35,8 @@ obj-$(CONFIG_CPU_SUP_CENTAUR)		+= centaur.o
 obj-$(CONFIG_CPU_SUP_TRANSMETA_32)	+= transmeta.o
 obj-$(CONFIG_CPU_SUP_UMC_32)		+= umc.o
 
-obj-$(CONFIG_INTEL_RDT)	+= rdt.o rdt_rdtgroup.o rdt_monitor.o
-obj-$(CONFIG_INTEL_RDT)	+= rdt_ctrlmondata.o rdt_pseudo_lock.o
+obj-$(CONFIG_PLATFORM_QOS)	+= rdt.o rdt_rdtgroup.o rdt_monitor.o
+obj-$(CONFIG_PLATFORM_QOS)	+= rdt_ctrlmondata.o rdt_pseudo_lock.o
 CFLAGS_rdt_pseudo_lock.o = -I$(src)
 
 obj-$(CONFIG_X86_MCE)			+= mcheck/
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 977cb57d7bc9..1a4d00b7a5b1 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -985,7 +985,7 @@ struct task_struct {
 	/* cg_list protected by css_set_lock and tsk->alloc_lock: */
 	struct list_head		cg_list;
 #endif
-#ifdef CONFIG_INTEL_RDT
+#ifdef CONFIG_PLATFORM_QOS
 	u32				closid;
 	u32				rmid;
 #endif
-- 
2.17.1

[RFC PATCH 06/10] arch/x86: Initialize the resource functions that are different

[Moger, Babu]

Initialize the resource functions that are different between the
vendors. Some features are initialized differently between the vendors.

For example, MBA feature varies significantly between Intel and AMD.
Separate the initialization of these resource functions. That way we
can easily add AMD's functions later.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/rdt.c | 28 +++++++++++++++++++++++++---
 arch/x86/kernel/cpu/rdt.h |  4 ++++
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/rdt.c b/arch/x86/kernel/cpu/rdt.c
index 736715b81fd8..6dec45bf81d6 100644
--- a/arch/x86/kernel/cpu/rdt.c
+++ b/arch/x86/kernel/cpu/rdt.c
@@ -174,10 +174,7 @@ struct rdt_resource rdt_resources_all[] = {
 		.rid			= RDT_RESOURCE_MBA,
 		.name			= "MB",
 		.domains		= domain_init(RDT_RESOURCE_MBA),
-		.msr_base		= IA32_MBA_THRTL_BASE,
-		.msr_update		= mba_wrmsr,
 		.cache_level		= 3,
-		.parse_ctrlval		= parse_bw,
 		.format_str		= "%d=%*u",
 		.fflags			= RFTYPE_RES_MB,
 	},
@@ -865,6 +862,25 @@ static __init void rdt_check_mba(void)
 		rdt_get_mem_config(&rdt_resources_all[RDT_RESOURCE_MBA]);
 }
 
+static __init void rdt_init_res_defs_intel(void)
+{
+	struct rdt_resource *r;
+
+	for_each_rdt_resource(r) {
+		if (r->rid == RDT_RESOURCE_MBA) {
+			r->msr_base = IA32_MBA_THRTL_BASE;
+			r->msr_update = mba_wrmsr;
+			r->parse_ctrlval = parse_bw;
+		}
+	}
+}
+
+static __init void rdt_init_res_defs(void)
+{
+	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
+		rdt_init_res_defs_intel();
+}
+
 static enum cpuhp_state rdt_online;
 
 static int __init rdt_late_init(void)
@@ -875,6 +891,12 @@ static int __init rdt_late_init(void)
 	/* Run quirks first */
 	rdt_quirks();
 
+	/*
+	 * Initialize functions(or definitions) that are different
+	 * between vendors here.
+	 */
+	rdt_init_res_defs();
+
 	rdt_alloc_capable = get_rdt_alloc_resources();
 	rdt_mon_capable = get_rdt_mon_resources();
 
diff --git a/arch/x86/kernel/cpu/rdt.h b/arch/x86/kernel/cpu/rdt.h
index c15417a6b1af..2569c10c37f4 100644
--- a/arch/x86/kernel/cpu/rdt.h
+++ b/arch/x86/kernel/cpu/rdt.h
@@ -455,6 +455,10 @@ enum {
 	RDT_NUM_RESOURCES,
 };
 
+#define for_each_rdt_resource(r)					      \
+	for (r = rdt_resources_all; r < rdt_resources_all + RDT_NUM_RESOURCES;\
+	     r++)
+
 #define for_each_capable_rdt_resource(r)				      \
 	for (r = rdt_resources_all; r < rdt_resources_all + RDT_NUM_RESOURCES;\
 	     r++)							      \
-- 
2.17.1

[RFC PATCH 07/10] arch/x86: Bring few more functions into the resource structure

[Moger, Babu]

Bring all resource functions that are different between the vendors
into resource structure and initialize them dynamically.

Implement these functions separately for each vendors.
update_mba_bw : Feedback loop bandwidth update functionality is not
                needed for AMD.
cbm_validate  : Cache bitmask validate function. AMD allows
                non-contiguous masks. So, use separate functions for
                Intel and AMD.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/rdt.c             | 17 +++++++++++++----
 arch/x86/kernel/cpu/rdt.h             | 19 +++++++++++++------
 arch/x86/kernel/cpu/rdt_ctrlmondata.c |  4 ++--
 arch/x86/kernel/cpu/rdt_monitor.c     | 10 +++++++---
 4 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/arch/x86/kernel/cpu/rdt.c b/arch/x86/kernel/cpu/rdt.c
index 6dec45bf81d6..ae26b9b3fafa 100644
--- a/arch/x86/kernel/cpu/rdt.c
+++ b/arch/x86/kernel/cpu/rdt.c
@@ -867,10 +867,19 @@ static __init void rdt_init_res_defs_intel(void)
 	struct rdt_resource *r;
 
 	for_each_rdt_resource(r) {
-		if (r->rid == RDT_RESOURCE_MBA) {
-			r->msr_base = IA32_MBA_THRTL_BASE;
-			r->msr_update = mba_wrmsr;
-			r->parse_ctrlval = parse_bw;
+		if ((r->rid == RDT_RESOURCE_L3) ||
+		    (r->rid == RDT_RESOURCE_L3DATA) ||
+		    (r->rid == RDT_RESOURCE_L3CODE) ||
+		    (r->rid == RDT_RESOURCE_L2) ||
+		    (r->rid == RDT_RESOURCE_L2DATA) ||
+		    (r->rid == RDT_RESOURCE_L2CODE))
+			r->cbm_validate = cbm_validate;
+
+		else if (r->rid == RDT_RESOURCE_MBA) {
+			 r->msr_base = IA32_MBA_THRTL_BASE;
+			 r->msr_update = mba_wrmsr;
+			 r->parse_ctrlval = parse_bw;
+			 r->update_mba_bw = update_mba_bw;
 		}
 	}
 }
diff --git a/arch/x86/kernel/cpu/rdt.h b/arch/x86/kernel/cpu/rdt.h
index 2569c10c37f4..7205157d359b 100644
--- a/arch/x86/kernel/cpu/rdt.h
+++ b/arch/x86/kernel/cpu/rdt.h
@@ -386,9 +386,9 @@ static inline bool is_mbm_event(int e)
  * struct rdt_resource - attributes of an RDT resource
  * @rid:		The index of the resource
  * @alloc_enabled:	Is allocation enabled on this machine
- * @mon_enabled:		Is monitoring enabled for this feature
+ * @mon_enabled:	Is monitoring enabled for this feature
  * @alloc_capable:	Is allocation available on this machine
- * @mon_capable:		Is monitor feature available on this machine
+ * @mon_capable:	Is monitor feature available on this machine
  * @name:		Name to use in "schemata" file
  * @num_closid:		Number of CLOSIDs available
  * @cache_level:	Which cache level defines scope of this resource
@@ -400,10 +400,12 @@ static inline bool is_mbm_event(int e)
  * @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
- * @evt_list:			List of monitoring events
- * @num_rmid:			Number of RMIDs available
- * @mon_scale:			cqm counter * mon_scale = occupancy in bytes
- * @fflags:			flags to choose base and info files
+ * @update_mba_bw:	Feedback loop for MBA software controllerer function
+ * @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
+ * @fflags:		flags to choose base and info files
  */
 struct rdt_resource {
 	int			rid;
@@ -425,6 +427,9 @@ struct rdt_resource {
 	const char		*format_str;
 	int (*parse_ctrlval)	(void *data, struct rdt_resource *r,
 				 struct rdt_domain *d);
+	void (*update_mba_bw)   (struct rdtgroup *rgrp,
+				 struct rdt_domain *dom_mbm);
+	bool (*cbm_validate)    (char *buf, u32 *data, struct rdt_resource *r);
 	struct list_head	evt_list;
 	int			num_rmid;
 	unsigned int		mon_scale;
@@ -562,5 +567,7 @@ 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);
+void update_mba_bw(struct rdtgroup *rgrp, struct rdt_domain *dom_mbm);
+bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r);
 
 #endif /* _ASM_X86_RDT_H */
diff --git a/arch/x86/kernel/cpu/rdt_ctrlmondata.c b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
index 0565c564b297..5a282b6c4bd7 100644
--- a/arch/x86/kernel/cpu/rdt_ctrlmondata.c
+++ b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
@@ -88,7 +88,7 @@ int parse_bw(void *_buf, struct rdt_resource *r, struct rdt_domain *d)
  *	are allowed (e.g. FFFFH, 0FF0H, 003CH, etc.).
  * Additionally Haswell requires at least two bits set.
  */
-static bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r)
+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;
@@ -153,7 +153,7 @@ int parse_cbm(void *_data, struct rdt_resource *r, struct rdt_domain *d)
 		return -EINVAL;
 	}
 
-	if (!cbm_validate(data->buf, &cbm_val, r))
+	if ((r->cbm_validate) && !(r->cbm_validate(data->buf, &cbm_val, r)))
 		return -EINVAL;
 
 	if ((rdtgrp->mode == RDT_MODE_EXCLUSIVE ||
diff --git a/arch/x86/kernel/cpu/rdt_monitor.c b/arch/x86/kernel/cpu/rdt_monitor.c
index 577514cd4a71..0dc0260f10d9 100644
--- a/arch/x86/kernel/cpu/rdt_monitor.c
+++ b/arch/x86/kernel/cpu/rdt_monitor.c
@@ -361,7 +361,7 @@ void mon_event_count(void *info)
  * throttle MSRs already have low percentage values.  To avoid
  * unnecessarily restricting such rdtgroups, we also increase the bandwidth.
  */
-static void update_mba_bw(struct rdtgroup *rgrp, struct rdt_domain *dom_mbm)
+void update_mba_bw(struct rdtgroup *rgrp, struct rdt_domain *dom_mbm)
 {
 	u32 closid, rmid, cur_msr, cur_msr_val, new_msr_val;
 	struct mbm_state *pmbm_data, *cmbm_data;
@@ -520,6 +520,7 @@ void mbm_handle_overflow(struct work_struct *work)
 	unsigned long delay = msecs_to_jiffies(MBM_OVERFLOW_INTERVAL);
 	struct rdtgroup *prgrp, *crgrp;
 	int cpu = smp_processor_id();
+	struct rdt_resource *r_mba;
 	struct list_head *head;
 	struct rdt_domain *d;
 
@@ -539,8 +540,11 @@ void mbm_handle_overflow(struct work_struct *work)
 		list_for_each_entry(crgrp, head, mon.crdtgrp_list)
 			mbm_update(d, crgrp->mon.rmid);
 
-		if (is_mba_sc(NULL))
-			update_mba_bw(prgrp, d);
+		if (is_mba_sc(NULL)) {
+			r_mba = &rdt_resources_all[RDT_RESOURCE_MBA];
+			if (r_mba->update_mba_bw)
+				r_mba->update_mba_bw(prgrp, d);
+		}
 	}
 
 	schedule_delayed_work_on(cpu, &d->mbm_over, delay);
-- 
2.17.1

[RFC PATCH 08/10] arch/x86: Introduce new config parameter AMD_QOS

[Moger, Babu]

Introduces the new config parameter AMD_QOS. This parameter will be
used to enable cache and memory bandwidth allocation and monitoring
features on AMD processors. This will enable common config parameter
PLATFORM_QOS if selected.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/Kconfig | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 7f2da780a327..0bfb5f4f32f2 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -458,9 +458,24 @@ config INTEL_RDT
 
 	  Say N if unsure.
 
+config AMD_QOS
+	bool "AMD Quality of Service support"
+	default n
+	depends on X86 && CPU_SUP_AMD
+	select KERNFS
+	help
+	  Select to enable cache and memory bandwidth enforcement and monitoring
+	  features of AMD processors. These features are intended to provide
+	  support for the monitoring of the usage of certain system resources
+	  by one or more processors and for the separate allocation and
+	  enforcement of limits on the use of certain system resources by one or
+	  more processors.
+
+	  Say N if unsure.
+
 config PLATFORM_QOS
 	def_bool y
-	depends on X86 && INTEL_RDT
+	depends on X86 && (INTEL_RDT || AMD_QOS)
 
 if X86_32
 config X86_BIGSMP
-- 
2.17.1

[RFC PATCH 09/10] arch/x86: Add AMD feature bit X86_FEATURE_MBA in cpuid bits array

[Moger, Babu]

From: Sherry Hurwitz <sherry.hurwitz@amd.com>

The feature bit X86_FEATURE_MBA is detected via CPUID leaf 0x80000008
EBX Bit 06. This bit indicates the support of AMD's MBA feature.

Signed-off-by: Babu Moger <babu.moger@amd.com>
Signed-off-by: Sherry Hurwitz <sherry.hurwitz@amd.com>
---
 arch/x86/kernel/cpu/scattered.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 772c219b6889..602f3725f6c4 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -29,6 +29,7 @@ static const struct cpuid_bit cpuid_bits[] = {
 	{ X86_FEATURE_HW_PSTATE,	CPUID_EDX,  7, 0x80000007, 0 },
 	{ X86_FEATURE_CPB,		CPUID_EDX,  9, 0x80000007, 0 },
 	{ X86_FEATURE_PROC_FEEDBACK,    CPUID_EDX, 11, 0x80000007, 0 },
+	{ X86_FEATURE_MBA,		CPUID_EBX,  6, 0x80000008, 0 },
 	{ X86_FEATURE_SME,		CPUID_EAX,  0, 0x8000001f, 0 },
 	{ X86_FEATURE_SEV,		CPUID_EAX,  1, 0x8000001f, 0 },
 	{ 0, 0, 0, 0, 0 }
-- 
2.17.1

[RFC PATCH 10/10] arch/x86: Introduce QOS feature for AMD

[Moger, Babu]

Enables QOS feature on AMD.
Following QoS sub-features are supported in AMD if the underlying
hardware supports it.
 - L3 Cache allocation enforcement
 - L3 Cache occupancy monitoring
 - L3 Code-Data Prioritization support
 - Memory Bandwidth Enforcement(Allocation)

There are differences in the way some of the features are implemented.
Separate those functions and add those as vendor specific functions.
The major difference is in MBA feature.
 - AMD uses CPUID leaf 0x80000020 to initialize the MBA features.
 - AMD uses direct bandwidth value instead of delay based on bandwidth
   values.
 - MSR register base addresses are different for MBA.
 - Also AMD allows non-contiguous L3 cache bit masks.

Adds following functions to take care of the differences.
rdt_get_mem_config_amd : MBA initialization function
parse_bw_amd : Bandwidth parsing
mba_wrmsr_amd: Writes bandwidth value
cbm_validate_amd : L3 cache bitmask validation

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/rdt.c             | 70 ++++++++++++++++++++++-
 arch/x86/kernel/cpu/rdt.h             |  3 +
 arch/x86/kernel/cpu/rdt_ctrlmondata.c | 81 +++++++++++++++++++++++++++
 3 files changed, 152 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/rdt.c b/arch/x86/kernel/cpu/rdt.c
index ae26b9b3fafa..21c17893cc0a 100644
--- a/arch/x86/kernel/cpu/rdt.c
+++ b/arch/x86/kernel/cpu/rdt.c
@@ -35,6 +35,7 @@
 
 #define MBA_IS_LINEAR	0x4
 #define MBA_MAX_MBPS	U32_MAX
+#define MAX_MBA_BW_AMD 0x800
 
 /* Mutex to protect rdtgroup access. */
 DEFINE_MUTEX(rdtgroup_mutex);
@@ -63,6 +64,9 @@ static void
 mba_wrmsr(struct rdt_domain *d, struct msr_param *m, struct rdt_resource *r);
 static void
 cat_wrmsr(struct rdt_domain *d, struct msr_param *m, struct rdt_resource *r);
+static void
+mba_wrmsr_amd(struct rdt_domain *d, struct msr_param *m,
+	      struct rdt_resource *r);
 
 #define domain_init(id) LIST_HEAD_INIT(rdt_resources_all[id].domains)
 
@@ -282,6 +286,31 @@ static bool rdt_get_mem_config(struct rdt_resource *r)
 	return true;
 }
 
+static bool rdt_get_mem_config_amd(struct rdt_resource *r)
+{
+	union cpuid_0x10_3_eax eax;
+	union cpuid_0x10_x_edx edx;
+	u32 ebx, ecx;
+
+	cpuid_count(0x80000020, 1, &eax.full, &ebx, &ecx, &edx.full);
+	r->num_closid = edx.split.cos_max + 1;
+	r->default_ctrl = MAX_MBA_BW_AMD;
+
+	/* AMD does not use delay. Set delay_linear to false by default */
+	r->membw.delay_linear = false;
+
+	/* FIX ME - May need to be read from MSR */
+	r->membw.min_bw = 0;
+	r->membw.bw_gran = 1;
+	/* Max value is 2048, Data width should be 4 in decimal */
+	r->data_width = 4;
+
+	r->alloc_capable = true;
+	r->alloc_enabled = true;
+
+	return true;
+}
+
 static void rdt_get_cache_alloc_cfg(int idx, struct rdt_resource *r)
 {
 	union cpuid_0x10_1_eax eax;
@@ -341,6 +370,16 @@ static int get_cache_id(int cpu, int level)
 	return -1;
 }
 
+static void
+mba_wrmsr_amd(struct rdt_domain *d, struct msr_param *m, struct rdt_resource *r)
+{
+	unsigned int i;
+
+	/*  Write the bw values for mba. */
+	for (i = m->low; i < m->high; i++)
+		wrmsrl(r->msr_base + i, d->ctrl_val[i]);
+}
+
 /*
  * Map the memory b/w percentage value to delay values
  * that can be written to QOS_MSRs.
@@ -858,8 +897,12 @@ static __init void rdt_detect_l3_mon(void)
 
 static __init void rdt_check_mba(void)
 {
-	if (rdt_cpu_has(X86_FEATURE_MBA))
-		rdt_get_mem_config(&rdt_resources_all[RDT_RESOURCE_MBA]);
+	if (rdt_cpu_has(X86_FEATURE_MBA)) {
+		if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
+			rdt_get_mem_config(&rdt_resources_all[RDT_RESOURCE_MBA]);
+		else if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
+			rdt_get_mem_config_amd(&rdt_resources_all[RDT_RESOURCE_MBA]);
+	}
 }
 
 static __init void rdt_init_res_defs_intel(void)
@@ -884,10 +927,33 @@ static __init void rdt_init_res_defs_intel(void)
 	}
 }
 
+static __init void rdt_init_res_defs_amd(void)
+{
+	struct rdt_resource *r;
+
+	for_each_rdt_resource(r) {
+		if ((r->rid == RDT_RESOURCE_L3) ||
+		    (r->rid == RDT_RESOURCE_L3DATA) ||
+		    (r->rid == RDT_RESOURCE_L3CODE) ||
+		    (r->rid == RDT_RESOURCE_L2) ||
+		    (r->rid == RDT_RESOURCE_L2DATA) ||
+		    (r->rid == RDT_RESOURCE_L2CODE))
+			r->cbm_validate = cbm_validate_amd;
+
+		else if (r->rid == RDT_RESOURCE_MBA) {
+			 r->msr_base = IA32_MBA_BW_BASE;
+			 r->msr_update = mba_wrmsr_amd;
+			 r->parse_ctrlval = parse_bw_amd;
+		}
+	}
+}
+
 static __init void rdt_init_res_defs(void)
 {
 	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
 		rdt_init_res_defs_intel();
+	else if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
+		rdt_init_res_defs_amd();
 }
 
 static enum cpuhp_state rdt_online;
diff --git a/arch/x86/kernel/cpu/rdt.h b/arch/x86/kernel/cpu/rdt.h
index 7205157d359b..e0244ac071e4 100644
--- a/arch/x86/kernel/cpu/rdt.h
+++ b/arch/x86/kernel/cpu/rdt.h
@@ -11,6 +11,7 @@
 #define IA32_L3_CBM_BASE	0xc90
 #define IA32_L2_CBM_BASE	0xd10
 #define IA32_MBA_THRTL_BASE	0xd50
+#define IA32_MBA_BW_BASE	0xc0000200
 
 #define L3_QOS_CDP_ENABLE	0x01ULL
 
@@ -438,6 +439,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_amd(void *_buf, struct rdt_resource *r,  struct rdt_domain *d);
 
 extern struct mutex rdtgroup_mutex;
 
@@ -569,5 +571,6 @@ bool has_busy_rmid(struct rdt_resource *r, struct rdt_domain *d);
 void __check_limbo(struct rdt_domain *d, bool force_free);
 void update_mba_bw(struct rdtgroup *rgrp, struct rdt_domain *dom_mbm);
 bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r);
+bool cbm_validate_amd(char *buf, u32 *data, struct rdt_resource *r);
 
 #endif /* _ASM_X86_RDT_H */
diff --git a/arch/x86/kernel/cpu/rdt_ctrlmondata.c b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
index 5a282b6c4bd7..1e4631f88696 100644
--- a/arch/x86/kernel/cpu/rdt_ctrlmondata.c
+++ b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
@@ -28,6 +28,52 @@
 #include <linux/slab.h>
 #include "rdt.h"
 
+/*
+ * Check whether MBA bandwidth percentage value is correct. The value is
+ * checked against the minimum and max bandwidth values specified by the
+ * hardware. The allocated bandwidth percentage is rounded to the next
+ * control step available on the hardware.
+ */
+static bool bw_validate_amd(char *buf, unsigned long *data,
+			    struct rdt_resource *r)
+{
+	unsigned long bw;
+	int ret;
+
+	ret = kstrtoul(buf, 10, &bw);
+	if (ret) {
+		rdt_last_cmd_printf("Non-decimal digit in MB value %s\n", buf);
+		return false;
+	}
+
+	if (bw < r->membw.min_bw || bw > r->default_ctrl) {
+		rdt_last_cmd_printf("MB value %ld out of range [%d,%d]\n", bw,
+				    r->membw.min_bw, r->default_ctrl);
+		return false;
+	}
+
+	*data = roundup(bw, (unsigned long)r->membw.bw_gran);
+	return true;
+}
+
+int parse_bw_amd(void *_buf, struct rdt_resource *r, struct rdt_domain *d)
+{
+	unsigned long data;
+	char *buf = _buf;
+
+	if (d->have_new_ctrl) {
+		rdt_last_cmd_printf("duplicate domain %d\n", d->id);
+		return -EINVAL;
+	}
+
+	if (!bw_validate_amd(buf, &data, r))
+		return -EINVAL;
+	d->new_ctrl = data;
+	d->have_new_ctrl = true;
+
+	return 0;
+}
+
 /*
  * Check whether MBA bandwidth percentage value is correct. The value is
  * checked against the minimum and max bandwidth values specified by the
@@ -123,6 +169,41 @@ bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r)
 	return true;
 }
 
+/*
+ * Check whether a cache bit mask is valid. AMD allows
+ * non-contiguous masks.
+ */
+bool cbm_validate_amd(char *buf, u32 *data, struct rdt_resource *r)
+{
+	unsigned long first_bit, zero_bit, val;
+	unsigned int cbm_len = r->cache.cbm_len;
+	int ret;
+
+	ret = kstrtoul(buf, 16, &val);
+	if (ret) {
+		rdt_last_cmd_printf("non-hex character in mask %s\n", buf);
+		return false;
+	}
+
+	if (val == 0 || val > r->default_ctrl) {
+		rdt_last_cmd_puts("mask out of range\n");
+		return false;
+	}
+
+	first_bit = find_first_bit(&val, cbm_len);
+	zero_bit = find_next_zero_bit(&val, cbm_len, first_bit);
+
+
+	if ((zero_bit - first_bit) < r->cache.min_cbm_bits) {
+		rdt_last_cmd_printf("Need at least %d bits in mask\n",
+				    r->cache.min_cbm_bits);
+		return false;
+	}
+
+	*data = val;
+	return true;
+}
+
 struct rdt_cbm_parse_data {
 	struct rdtgroup		*rdtgrp;
 	char			*buf;
-- 
2.17.1

Re: [RFC PATCH 00/10] arch/x86: AMD QoS support

[Thomas Gleixner]

Babu,

On Mon, 24 Sep 2018, Moger, Babu wrote:

> This series adds support for AMD64 architectural extensions for Platform
> Quality of Service. These extensions are intended to provide for the
> monitoring of the usage of certain system resources by one or more
> processors and for the separate allocation and enforcement of limits on
> the use of certain system resources by one or more processors.
> 
> The monitoring and enforcement are not necessarily applied across the
> entire system, but in general apply to a QOS domain which corresponds to
> some shared system resource.  The set of resources which are monitored and
> the set for which the enforcement of limits is provided are implementation
> dependent. Platform QOS features are implemented on a logical processor basis.
> Therefore, multiple hardware threads of a single physical CPU core may have
> independent resource monitoring and enforcement configurations.
> 
> AMD's next generation of processors support following QoS sub-features.
> - L3 Cache allocation enforcement
> - L3 Cache occupancy monitoring
> - L3 Code-Data Prioritization support
> - Memory Bandwidth Enforcement(Allocation)
> 
> The public specification is still in works. Will add the link when it is
> available.
> 
> Obviously, there are multiple ways we can go about these changes. We felt
> it is appropriate to rename and re-organize the code little bit before
> making the functional changes. The first few patches(1-6) renames and
> re-organizes the sources in preparation. Rest of the patches(7-10) adds
> support for AMD QoS features.

On the first glance this all looks sensible, but there is work in progress
by James Morse (Cc'ed), who wants to generalize the resctrl filesystem so
it can be reused by ARM. I just want to make sure that your reorganization
is not colliding or creating duplicate effort.

https://lkml.kernel.org/r/20180824104519.11203-1-james.morse@arm.com

Thanks,

	tglx

RE: [RFC PATCH 00/10] arch/x86: AMD QoS support

[Moger, Babu]

Thomas, 

> -----Original Message-----
> From: Thomas Gleixner <tglx@linutronix.de>
> Sent: Thursday, September 27, 2018 3:14 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: mingo@redhat.com; hpa@zytor.com; fenghua.yu@intel.com;
> reinette.chatre@intel.com; vikas.shivappa@linux.intel.com;
> tony.luck@intel.com; x86@kernel.org; peterz@infradead.org;
> pombredanne@nexb.com; gregkh@linuxfoundation.org;
> kstewart@linuxfoundation.org; bp@suse.de; rafael.j.wysocki@intel.com;
> ak@linux.intel.com; kirill.shutemov@linux.intel.com;
> xiaochen.shen@intel.com; colin.king@canonical.com; Hurwitz, Sherry
> <sherry.hurwitz@amd.com>; Lendacky, Thomas
> <Thomas.Lendacky@amd.com>; pbonzini@redhat.com;
> dwmw@amazon.co.uk; luto@kernel.org; jroedel@suse.de;
> jannh@google.com; dima@arista.com; jpoimboe@redhat.com;
> vkuznets@redhat.com; linux-kernel@vger.kernel.org; James Morse
> <james.morse@arm.com>
> Subject: Re: [RFC PATCH 00/10] arch/x86: AMD QoS support
> 
> Babu,
> 
> On Mon, 24 Sep 2018, Moger, Babu wrote:
> 
> > This series adds support for AMD64 architectural extensions for Platform
> > Quality of Service. These extensions are intended to provide for the
> > monitoring of the usage of certain system resources by one or more
> > processors and for the separate allocation and enforcement of limits on
> > the use of certain system resources by one or more processors.
> >
> > The monitoring and enforcement are not necessarily applied across the
> > entire system, but in general apply to a QOS domain which corresponds to
> > some shared system resource.  The set of resources which are monitored
> and
> > the set for which the enforcement of limits is provided are implementation
> > dependent. Platform QOS features are implemented on a logical processor
> basis.
> > Therefore, multiple hardware threads of a single physical CPU core may
> have
> > independent resource monitoring and enforcement configurations.
> >
> > AMD's next generation of processors support following QoS sub-features.
> > - L3 Cache allocation enforcement
> > - L3 Cache occupancy monitoring
> > - L3 Code-Data Prioritization support
> > - Memory Bandwidth Enforcement(Allocation)
> >
> > The public specification is still in works. Will add the link when it is
> > available.
> >
> > Obviously, there are multiple ways we can go about these changes. We felt
> > it is appropriate to rename and re-organize the code little bit before
> > making the functional changes. The first few patches(1-6) renames and
> > re-organizes the sources in preparation. Rest of the patches(7-10) adds
> > support for AMD QoS features.
> 
> On the first glance this all looks sensible, but there is work in progress
> by James Morse (Cc'ed), who wants to generalize the resctrl filesystem so
> it can be reused by ARM. I just want to make sure that your reorganization
> is not colliding or creating duplicate effort.
> 
> https://lkml.kernel.org/r/20180824104519.11203-1-james.morse@arm.com

Thanks for pointing this out. I have looked thru James patches. It appears this series is only a small part of his much bigger change. Don't know the timeframe of his overall changes.
I will let him speak on that.  That being said, I don't consider our efforts as duplicate. He is touching the resource structures, and trying to separate arch, non-arch components.
My changes are mostly inside the resource structures(mostly resource handlers) and trying to accommodate minor differences within the architecture.  It will be mostly involve rebase
effort on either side in the end whoever goes first.

James, What are your thoughts? 

> 
> Thanks,
> 
> 	tglx

Re: [RFC PATCH 00/10] arch/x86: AMD QoS support

[Fenghua Yu]

On Mon, Sep 24, 2018 at 07:18:54PM +0000, Moger, Babu wrote:
> The public specification is still in works. Will add the link when it is
> available.

Is this the public AMD QoS spec?
https://support.amd.com/TechDocs/56375_Quality_of_Service_Extensions.pdf

Thanks.

-Fenghua

RE: [RFC PATCH 00/10] arch/x86: AMD QoS support

[Moger, Babu]

Hi Fenghua,

> -----Original Message-----
> From: Fenghua Yu <fenghua.yu@intel.com>
> Sent: Tuesday, October 2, 2018 12:07 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> fenghua.yu@intel.com; reinette.chatre@intel.com;
> vikas.shivappa@linux.intel.com; tony.luck@intel.com; x86@kernel.org;
> peterz@infradead.org; pombredanne@nexb.com;
> gregkh@linuxfoundation.org; kstewart@linuxfoundation.org; bp@suse.de;
> rafael.j.wysocki@intel.com; ak@linux.intel.com;
> kirill.shutemov@linux.intel.com; xiaochen.shen@intel.com;
> colin.king@canonical.com; Hurwitz, Sherry <sherry.hurwitz@amd.com>;
> Lendacky, Thomas <Thomas.Lendacky@amd.com>; pbonzini@redhat.com;
> dwmw@amazon.co.uk; luto@kernel.org; jroedel@suse.de;
> jannh@google.com; dima@arista.com; jpoimboe@redhat.com;
> vkuznets@redhat.com; linux-kernel@vger.kernel.org
> Subject: Re: [RFC PATCH 00/10] arch/x86: AMD QoS support
> 
> On Mon, Sep 24, 2018 at 07:18:54PM +0000, Moger, Babu wrote:
> > The public specification is still in works. Will add the link when it is
> > available.
> 
> Is this the public AMD QoS spec?
> https://support.amd.com/TechDocs/56375_Quality_of_Service_Extensions.
> pdf
> 

Yes. That is the one. 
I was going to send the link after refreshing the patches on 4-19-rc5.  You already got it. Please let know if you have any feedback on the patches. 

> Thanks.
> 
> -Fenghua

Re: [RFC PATCH 10/10] arch/x86: Introduce QOS feature for AMD

[Fenghua Yu]

On Mon, Sep 24, 2018 at 07:19:16PM +0000, Moger, Babu wrote:
>  int parse_bw(void *_buf, struct rdt_resource *r,  struct rdt_domain *d);
> +int parse_bw_amd(void *_buf, struct rdt_resource *r,  struct rdt_domain *d);

Please note the type of _buf in parse_bw() is changed in latest kernel
to fix some issues. Please follow the same definition of parse_bw() in
parse_bw_amd().

Thanks.

-Fenghua

Re: [RFC PATCH 00/10] arch/x86: AMD QoS support

[Fenghua Yu]

On Tue, Oct 02, 2018 at 05:44:47PM +0000, Moger, Babu wrote:
> Hi Fenghua,
> 
> > -----Original Message-----
> > From: Fenghua Yu <fenghua.yu@intel.com>
> > Sent: Tuesday, October 2, 2018 12:07 PM
> > On Mon, Sep 24, 2018 at 07:18:54PM +0000, Moger, Babu wrote:
> > > The public specification is still in works. Will add the link when it is
> > > available.
> > 
> > Is this the public AMD QoS spec?
> > https://support.amd.com/TechDocs/56375_Quality_of_Service_Extensions.
> > pdf
> > 
> 
> Yes. That is the one. 
> I was going to send the link after refreshing the patches on 4-19-rc5.  You already got it. Please let know if you have any feedback on the patches. 

I don't see the name 'RDT' (Resource Director Technology) in the
spec "AMD64 Technology Platform Quality of Service Extensions".
Does AMD use the same name 'RDT' as Intel?

Thanks.

-Fenghua

Re: [RFC PATCH 00/10] arch/x86: AMD QoS support

[Moger, Babu]

On 10/02/2018 01:46 PM, Fenghua Yu wrote:
> On Tue, Oct 02, 2018 at 05:44:47PM +0000, Moger, Babu wrote:
>> Hi Fenghua,
>>
>>> -----Original Message-----
>>> From: Fenghua Yu <fenghua.yu@intel.com>
>>> Sent: Tuesday, October 2, 2018 12:07 PM
>>> On Mon, Sep 24, 2018 at 07:18:54PM +0000, Moger, Babu wrote:
>>>> The public specification is still in works. Will add the link when it is
>>>> available.
>>>
>>> Is this the public AMD QoS spec?
>>> https://support.amd.com/TechDocs/56375_Quality_of_Service_Extensions.
>>> pdf
>>>
>>
>> Yes. That is the one. 
>> I was going to send the link after refreshing the patches on 4-19-rc5.  You already got it. Please let know if you have any feedback on the patches. 
> 
> I don't see the name 'RDT' (Resource Director Technology) in the
> spec "AMD64 Technology Platform Quality of Service Extensions".
> Does AMD use the same name 'RDT' as Intel?

No. AMD uses the word "Platform Quality of Service"(or in short QoS) to
refer this feature.

> 
> Thanks.
> 
> -Fenghua
> 

Re: [RFC PATCH 03/10] arch/x86: Re-arrange RDT init code

[Reinette Chatre]

Hi Babu,

On 9/24/2018 12:19 PM, Moger, Babu wrote:
> Re-organize the RDT init code. Separate the call sequence for each
> feature. That way, it is easy to call quirks or features separately
> for each vendor if there are differences.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  arch/x86/kernel/cpu/rdt.c | 44 ++++++++++++++++++++++++++++-----------
>  1 file changed, 32 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/rdt.c b/arch/x86/kernel/cpu/rdt.c
> index b361c63170d7..736715b81fd8 100644
> --- a/arch/x86/kernel/cpu/rdt.c
> +++ b/arch/x86/kernel/cpu/rdt.c
> @@ -813,10 +813,6 @@ static __init bool get_rdt_alloc_resources(void)
>  		ret = true;
>  	}
>  
> -	if (rdt_cpu_has(X86_FEATURE_MBA)) {
> -		if (rdt_get_mem_config(&rdt_resources_all[RDT_RESOURCE_MBA]))
> -			ret = true;
> -	}

The commit message mentions that the call sequence for each feature is
separated, but here only the MBA feature is separated.

The MBA feature detection is removed above .... (more later)

>  	return ret;
>  }
>  
> @@ -831,11 +827,12 @@ static __init bool get_rdt_mon_resources(void)
>  
>  	if (!rdt_mon_features)
>  		return false;
> +	else
> +		return true;
>  
> -	return !rdt_get_mon_l3_config(&rdt_resources_all[RDT_RESOURCE_L3]);
>  }
>  
> -static __init void rdt_quirks(void)
> +static __init void rdt_quirks_intel(void)
>  {
>  	switch (boot_cpu_data.x86_model) {
>  	case INTEL_FAM6_HASWELL_X:
> @@ -850,13 +847,22 @@ static __init void rdt_quirks(void)
>  	}
>  }
>  
> -static __init bool get_rdt_resources(void)
> +static __init void rdt_quirks(void)
>  {
> -	rdt_quirks();
> -	rdt_alloc_capable = get_rdt_alloc_resources();
> -	rdt_mon_capable = get_rdt_mon_resources();
> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
> +		rdt_quirks_intel();
> +}
> +
> +static __init void rdt_detect_l3_mon(void)
> +{
> +	if (rdt_mon_capable)
> +		rdt_get_mon_l3_config(&rdt_resources_all[RDT_RESOURCE_L3]);

The possible errors from this configuration is now lost.

> +}
>  
> -	return (rdt_mon_capable || rdt_alloc_capable);
> +static __init void rdt_check_mba(void)
> +{
> +	if (rdt_cpu_has(X86_FEATURE_MBA))
> +		rdt_get_mem_config(&rdt_resources_all[RDT_RESOURCE_MBA]);

Here too the possible failure of this configuration is now lost.

>  }
>  
>  static enum cpuhp_state rdt_online;
> @@ -866,8 +872,22 @@ static int __init rdt_late_init(void)
>  	struct rdt_resource *r;
>  	int state, ret;
>  
> -	if (!get_rdt_resources())
> +	/* Run quirks first */
> +	rdt_quirks();
> +
> +	rdt_alloc_capable = get_rdt_alloc_resources();
> +	rdt_mon_capable = get_rdt_mon_resources();
> +
> +	if (!(rdt_alloc_capable || rdt_mon_capable)) {
> +		pr_info("RDT allocation or monitoring not detected\n");

This function ends with a log entry for every resource discovered. Is
this new log entry needed to indicate that such resources have not been
found? Could it not just be the absence of the other message?

>  		return -ENODEV;
> +	}

... (continued from above) ... since the MBA feature detection was
removed from get_rdt_alloc_resources() would the above not cause failure
on systems that only support MBA?

> +
> +	/* Detect l3 monitoring resources */

I do not think this comment is accurate ... has the monitoring resources
not been detected earlier in get_rdt_mon_resources() and now they will
be configured?

> +	rdt_detect_l3_mon();
> +
> +	/* Check for Memory Bandwidth Allocation */
> +	rdt_check_mba();

To follow up on above .. the potential failure of these configurations
are now lost here. Initialization should not continue if these
configurations failed.

>  
>  	rdt_init_padding();
>  
> 

Reinette

Re: [RFC PATCH 06/10] arch/x86: Initialize the resource functions that are different

[Reinette Chatre]

Hi Babu,

On 9/24/2018 12:19 PM, Moger, Babu wrote:
> Initialize the resource functions that are different between the
> vendors. Some features are initialized differently between the vendors.
> 
> For example, MBA feature varies significantly between Intel and AMD.
> Separate the initialization of these resource functions. That way we
> can easily add AMD's functions later.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  arch/x86/kernel/cpu/rdt.c | 28 +++++++++++++++++++++++++---
>  arch/x86/kernel/cpu/rdt.h |  4 ++++
>  2 files changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/rdt.c b/arch/x86/kernel/cpu/rdt.c
> index 736715b81fd8..6dec45bf81d6 100644
> --- a/arch/x86/kernel/cpu/rdt.c
> +++ b/arch/x86/kernel/cpu/rdt.c
> @@ -174,10 +174,7 @@ struct rdt_resource rdt_resources_all[] = {
>  		.rid			= RDT_RESOURCE_MBA,
>  		.name			= "MB",
>  		.domains		= domain_init(RDT_RESOURCE_MBA),
> -		.msr_base		= IA32_MBA_THRTL_BASE,
> -		.msr_update		= mba_wrmsr,
>  		.cache_level		= 3,
> -		.parse_ctrlval		= parse_bw,
>  		.format_str		= "%d=%*u",
>  		.fflags			= RFTYPE_RES_MB,
>  	},
> @@ -865,6 +862,25 @@ static __init void rdt_check_mba(void)
>  		rdt_get_mem_config(&rdt_resources_all[RDT_RESOURCE_MBA]);
>  }
>  
> +static __init void rdt_init_res_defs_intel(void)
> +{
> +	struct rdt_resource *r;
> +
> +	for_each_rdt_resource(r) {
> +		if (r->rid == RDT_RESOURCE_MBA) {
> +			r->msr_base = IA32_MBA_THRTL_BASE;
> +			r->msr_update = mba_wrmsr;
> +			r->parse_ctrlval = parse_bw;
> +		}
> +	}
> +}

Patch 10 introduces parse_bw_amd and mba_wrmsr_amd as you prepare us for
in the commit message. I think it would reduce confusion if these
functions, mba_wrmsr and parse_bw, also follow this pattern to contain
the vendor name. So, mba_wrmsr -> mba_wrmsr_intel, parse_bw ->
parse_bw_intel.

> +
> +static __init void rdt_init_res_defs(void)
> +{
> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
> +		rdt_init_res_defs_intel();
> +}
> +
>  static enum cpuhp_state rdt_online;
>  
>  static int __init rdt_late_init(void)
> @@ -875,6 +891,12 @@ static int __init rdt_late_init(void)
>  	/* Run quirks first */
>  	rdt_quirks();
>  
> +	/*
> +	 * Initialize functions(or definitions) that are different
> +	 * between vendors here.
> +	 */
> +	rdt_init_res_defs();
> +

While it does seem as though currently rdt_quirks() is not using any of
the settings made in rdt_init_res_defs() it (rdt_quirks()) does use the
partially initialized resources structure and this may in the future
include using parameters that have not been initialized yet.

I thus think it would be safer to do this initialization before
rdt_quirks().

Reinette

Re: [RFC PATCH 07/10] arch/x86: Bring few more functions into the resource structure

[Reinette Chatre]

Hi Babu,

On 9/24/2018 12:19 PM, Moger, Babu wrote:
> Bring all resource functions that are different between the vendors
> into resource structure and initialize them dynamically.
> 
> Implement these functions separately for each vendors.
> update_mba_bw : Feedback loop bandwidth update functionality is not
>                 needed for AMD.
> cbm_validate  : Cache bitmask validate function. AMD allows
>                 non-contiguous masks. So, use separate functions for
>                 Intel and AMD.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  arch/x86/kernel/cpu/rdt.c             | 17 +++++++++++++----
>  arch/x86/kernel/cpu/rdt.h             | 19 +++++++++++++------
>  arch/x86/kernel/cpu/rdt_ctrlmondata.c |  4 ++--
>  arch/x86/kernel/cpu/rdt_monitor.c     | 10 +++++++---
>  4 files changed, 35 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/rdt.c b/arch/x86/kernel/cpu/rdt.c
> index 6dec45bf81d6..ae26b9b3fafa 100644
> --- a/arch/x86/kernel/cpu/rdt.c
> +++ b/arch/x86/kernel/cpu/rdt.c
> @@ -867,10 +867,19 @@ static __init void rdt_init_res_defs_intel(void)
>  	struct rdt_resource *r;
>  
>  	for_each_rdt_resource(r) {
> -		if (r->rid == RDT_RESOURCE_MBA) {
> -			r->msr_base = IA32_MBA_THRTL_BASE;
> -			r->msr_update = mba_wrmsr;
> -			r->parse_ctrlval = parse_bw;
> +		if ((r->rid == RDT_RESOURCE_L3) ||
> +		    (r->rid == RDT_RESOURCE_L3DATA) ||
> +		    (r->rid == RDT_RESOURCE_L3CODE) ||
> +		    (r->rid == RDT_RESOURCE_L2) ||
> +		    (r->rid == RDT_RESOURCE_L2DATA) ||
> +		    (r->rid == RDT_RESOURCE_L2CODE))
> +			r->cbm_validate = cbm_validate;

Same comment here about naming as in patch 6. Later cbm_validate_amd
would appear while this remains - to help reduce confusion it may help
to rename this function to cbm_validate_intel at this time.

> +
> +		else if (r->rid == RDT_RESOURCE_MBA) {
> +			 r->msr_base = IA32_MBA_THRTL_BASE;
> +			 r->msr_update = mba_wrmsr;
> +			 r->parse_ctrlval = parse_bw;
> +			 r->update_mba_bw = update_mba_bw;

Same comment about naming.

>  		}
>  	}
>  }
> diff --git a/arch/x86/kernel/cpu/rdt.h b/arch/x86/kernel/cpu/rdt.h
> index 2569c10c37f4..7205157d359b 100644
> --- a/arch/x86/kernel/cpu/rdt.h
> +++ b/arch/x86/kernel/cpu/rdt.h
> @@ -386,9 +386,9 @@ static inline bool is_mbm_event(int e)
>   * struct rdt_resource - attributes of an RDT resource
>   * @rid:		The index of the resource
>   * @alloc_enabled:	Is allocation enabled on this machine
> - * @mon_enabled:		Is monitoring enabled for this feature
> + * @mon_enabled:	Is monitoring enabled for this feature
>   * @alloc_capable:	Is allocation available on this machine
> - * @mon_capable:		Is monitor feature available on this machine
> + * @mon_capable:	Is monitor feature available on this machine
>   * @name:		Name to use in "schemata" file
>   * @num_closid:		Number of CLOSIDs available
>   * @cache_level:	Which cache level defines scope of this resource
> @@ -400,10 +400,12 @@ static inline bool is_mbm_event(int e)
>   * @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
> - * @evt_list:			List of monitoring events
> - * @num_rmid:			Number of RMIDs available
> - * @mon_scale:			cqm counter * mon_scale = occupancy in bytes
> - * @fflags:			flags to choose base and info files
> + * @update_mba_bw:	Feedback loop for MBA software controllerer function

controllerer -> controller ?

> + * @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
> + * @fflags:		flags to choose base and info files
>   */
>  struct rdt_resource {
>  	int			rid;
> @@ -425,6 +427,9 @@ struct rdt_resource {
>  	const char		*format_str;
>  	int (*parse_ctrlval)	(void *data, struct rdt_resource *r,
>  				 struct rdt_domain *d);
> +	void (*update_mba_bw)   (struct rdtgroup *rgrp,
> +				 struct rdt_domain *dom_mbm);
> +	bool (*cbm_validate)    (char *buf, u32 *data, struct rdt_resource *r);
>  	struct list_head	evt_list;
>  	int			num_rmid;
>  	unsigned int		mon_scale;
> @@ -562,5 +567,7 @@ 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);
> +void update_mba_bw(struct rdtgroup *rgrp, struct rdt_domain *dom_mbm);
> +bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r);
>  
>  #endif /* _ASM_X86_RDT_H */
> diff --git a/arch/x86/kernel/cpu/rdt_ctrlmondata.c b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
> index 0565c564b297..5a282b6c4bd7 100644
> --- a/arch/x86/kernel/cpu/rdt_ctrlmondata.c
> +++ b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
> @@ -88,7 +88,7 @@ int parse_bw(void *_buf, struct rdt_resource *r, struct rdt_domain *d)
>   *	are allowed (e.g. FFFFH, 0FF0H, 003CH, etc.).
>   * Additionally Haswell requires at least two bits set.
>   */
> -static bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r)
> +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;
> @@ -153,7 +153,7 @@ int parse_cbm(void *_data, struct rdt_resource *r, struct rdt_domain *d)
>  		return -EINVAL;
>  	}
>  
> -	if (!cbm_validate(data->buf, &cbm_val, r))
> +	if ((r->cbm_validate) && !(r->cbm_validate(data->buf, &cbm_val, r)))

I do not think you need all of those brackets.

Reinette

Re: [RFC PATCH 10/10] arch/x86: Introduce QOS feature for AMD

[Reinette Chatre]

Hi Babu,

On 9/24/2018 12:19 PM, Moger, Babu wrote:
> +/*
> + * Check whether a cache bit mask is valid. AMD allows
> + * non-contiguous masks.
> + */
> +bool cbm_validate_amd(char *buf, u32 *data, struct rdt_resource *r)
> +{
> +	unsigned long first_bit, zero_bit, val;
> +	unsigned int cbm_len = r->cache.cbm_len;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 16, &val);
> +	if (ret) {
> +		rdt_last_cmd_printf("non-hex character in mask %s\n", buf);
> +		return false;
> +	}
> +
> +	if (val == 0 || val > r->default_ctrl) {
> +		rdt_last_cmd_puts("mask out of range\n");
> +		return false;
> +	}

According to
https://www.amd.com/system/files/TechDocs/56375_Quality_of_Service_Extensions.pdf
"If an L3_MASK_n register is programmed with all 0’s, that COS will be
prevented from allocating any lines in the L3 cache."

The "val == 0" test thus does not seem necessary.

> +
> +	first_bit = find_first_bit(&val, cbm_len);
> +	zero_bit = find_next_zero_bit(&val, cbm_len, first_bit);
> +
> +
> +	if ((zero_bit - first_bit) < r->cache.min_cbm_bits) {
> +		rdt_last_cmd_printf("Need at least %d bits in mask\n",
> +				    r->cache.min_cbm_bits);
> +		return false;
> +	}

If AMD platforms accept CBM of all zeroes then it seems that the
platforms would not require a minimum number of set bits?

Reinette

Re: [RFC PATCH 03/10] arch/x86: Re-arrange RDT init code

[Moger, Babu]

Hi Reinette,
Thanks for the review. My response below.

On 10/02/2018 02:21 PM, Reinette Chatre wrote:
> Hi Babu,
> 
> On 9/24/2018 12:19 PM, Moger, Babu wrote:
>> Re-organize the RDT init code. Separate the call sequence for each
>> feature. That way, it is easy to call quirks or features separately
>> for each vendor if there are differences.
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>>  arch/x86/kernel/cpu/rdt.c | 44 ++++++++++++++++++++++++++++-----------
>>  1 file changed, 32 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/rdt.c b/arch/x86/kernel/cpu/rdt.c
>> index b361c63170d7..736715b81fd8 100644
>> --- a/arch/x86/kernel/cpu/rdt.c
>> +++ b/arch/x86/kernel/cpu/rdt.c
>> @@ -813,10 +813,6 @@ static __init bool get_rdt_alloc_resources(void)
>>  		ret = true;
>>  	}
>>  
>> -	if (rdt_cpu_has(X86_FEATURE_MBA)) {
>> -		if (rdt_get_mem_config(&rdt_resources_all[RDT_RESOURCE_MBA]))
>> -			ret = true;
>> -	}
> 
> The commit message mentions that the call sequence for each feature is
> separated, but here only the MBA feature is separated.

Yes. MBA and quirks are separated. I will fix the commit message. I
overlooked some of the errors returned by these functions. Let me go back
and update this patch. Will keep mostly as is. Only separate MBA and
quirks which are important. Will make sure errors are propagated properly.

> 
> The MBA feature detection is removed above .... (more later)
> 
>>  	return ret;
>>  }
>>  
>> @@ -831,11 +827,12 @@ static __init bool get_rdt_mon_resources(void)
>>  
>>  	if (!rdt_mon_features)
>>  		return false;
>> +	else
>> +		return true;
>>  
>> -	return !rdt_get_mon_l3_config(&rdt_resources_all[RDT_RESOURCE_L3]);
>>  }
>>  
>> -static __init void rdt_quirks(void)
>> +static __init void rdt_quirks_intel(void)
>>  {
>>  	switch (boot_cpu_data.x86_model) {
>>  	case INTEL_FAM6_HASWELL_X:
>> @@ -850,13 +847,22 @@ static __init void rdt_quirks(void)
>>  	}
>>  }
>>  
>> -static __init bool get_rdt_resources(void)
>> +static __init void rdt_quirks(void)
>>  {
>> -	rdt_quirks();
>> -	rdt_alloc_capable = get_rdt_alloc_resources();
>> -	rdt_mon_capable = get_rdt_mon_resources();
>> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
>> +		rdt_quirks_intel();
>> +}
>> +
>> +static __init void rdt_detect_l3_mon(void)
>> +{
>> +	if (rdt_mon_capable)
>> +		rdt_get_mon_l3_config(&rdt_resources_all[RDT_RESOURCE_L3]);
> 
> The possible errors from this configuration is now lost.

Yes. I overlooked it. Same comment as above. Let me go back and update
this patch.
> 
>> +}
>>  
>> -	return (rdt_mon_capable || rdt_alloc_capable);
>> +static __init void rdt_check_mba(void)
>> +{
>> +	if (rdt_cpu_has(X86_FEATURE_MBA))
>> +		rdt_get_mem_config(&rdt_resources_all[RDT_RESOURCE_MBA]);
> 
> Here too the possible failure of this configuration is now lost.

Ditto.. Let me go back and update this patch.

> 
>>  }
>>  
>>  static enum cpuhp_state rdt_online;
>> @@ -866,8 +872,22 @@ static int __init rdt_late_init(void)
>>  	struct rdt_resource *r;
>>  	int state, ret;
>>  
>> -	if (!get_rdt_resources())
>> +	/* Run quirks first */
>> +	rdt_quirks();
>> +
>> +	rdt_alloc_capable = get_rdt_alloc_resources();
>> +	rdt_mon_capable = get_rdt_mon_resources();
>> +
>> +	if (!(rdt_alloc_capable || rdt_mon_capable)) {
>> +		pr_info("RDT allocation or monitoring not detected\n");
> 
> This function ends with a log entry for every resource discovered. Is
> this new log entry needed to indicate that such resources have not been
> found? Could it not just be the absence of the other message?

As this is relatively new feature, so I added this info message. It helped
me debug what went wrong. Otherwise, I don't see anything. I can remove it
if the message is too annoying to the user.

> 
>>  		return -ENODEV;
>> +	}
> 
> ... (continued from above) ... since the MBA feature detection was
> removed from get_rdt_alloc_resources() would the above not cause failure
> on systems that only support MBA?

yes. Let me go back and update this patch.
> 
>> +
>> +	/* Detect l3 monitoring resources */
> 
> I do not think this comment is accurate ... has the monitoring resources
> not been detected earlier in get_rdt_mon_resources() and now they will
> be configured?
> 
>> +	rdt_detect_l3_mon();
>> +
>> +	/* Check for Memory Bandwidth Allocation */
>> +	rdt_check_mba();
> 
> To follow up on above .. the potential failure of these configurations
> are now lost here. Initialization should not continue if these
> configurations failed.

Yes. Let me go back and update this patch.

> 
>>  
>>  	rdt_init_padding();
>>  
>>
> 
> Reinette
> 

Re: [RFC PATCH 06/10] arch/x86: Initialize the resource functions that are different

[Moger, Babu]

On 10/02/2018 05:06 PM, Reinette Chatre wrote:
> Hi Babu,
> 
> On 9/24/2018 12:19 PM, Moger, Babu wrote:
>> Initialize the resource functions that are different between the
>> vendors. Some features are initialized differently between the vendors.
>>
>> For example, MBA feature varies significantly between Intel and AMD.
>> Separate the initialization of these resource functions. That way we
>> can easily add AMD's functions later.
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>>  arch/x86/kernel/cpu/rdt.c | 28 +++++++++++++++++++++++++---
>>  arch/x86/kernel/cpu/rdt.h |  4 ++++
>>  2 files changed, 29 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/rdt.c b/arch/x86/kernel/cpu/rdt.c
>> index 736715b81fd8..6dec45bf81d6 100644
>> --- a/arch/x86/kernel/cpu/rdt.c
>> +++ b/arch/x86/kernel/cpu/rdt.c
>> @@ -174,10 +174,7 @@ struct rdt_resource rdt_resources_all[] = {
>>  		.rid			= RDT_RESOURCE_MBA,
>>  		.name			= "MB",
>>  		.domains		= domain_init(RDT_RESOURCE_MBA),
>> -		.msr_base		= IA32_MBA_THRTL_BASE,
>> -		.msr_update		= mba_wrmsr,
>>  		.cache_level		= 3,
>> -		.parse_ctrlval		= parse_bw,
>>  		.format_str		= "%d=%*u",
>>  		.fflags			= RFTYPE_RES_MB,
>>  	},
>> @@ -865,6 +862,25 @@ static __init void rdt_check_mba(void)
>>  		rdt_get_mem_config(&rdt_resources_all[RDT_RESOURCE_MBA]);
>>  }
>>  
>> +static __init void rdt_init_res_defs_intel(void)
>> +{
>> +	struct rdt_resource *r;
>> +
>> +	for_each_rdt_resource(r) {
>> +		if (r->rid == RDT_RESOURCE_MBA) {
>> +			r->msr_base = IA32_MBA_THRTL_BASE;
>> +			r->msr_update = mba_wrmsr;
>> +			r->parse_ctrlval = parse_bw;
>> +		}
>> +	}
>> +}
> 
> Patch 10 introduces parse_bw_amd and mba_wrmsr_amd as you prepare us for
> in the commit message. I think it would reduce confusion if these
> functions, mba_wrmsr and parse_bw, also follow this pattern to contain
> the vendor name. So, mba_wrmsr -> mba_wrmsr_intel, parse_bw ->
> parse_bw_intel.

Yes. Sure. Will make this change.

> 
>> +
>> +static __init void rdt_init_res_defs(void)
>> +{
>> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
>> +		rdt_init_res_defs_intel();
>> +}
>> +
>>  static enum cpuhp_state rdt_online;
>>  
>>  static int __init rdt_late_init(void)
>> @@ -875,6 +891,12 @@ static int __init rdt_late_init(void)
>>  	/* Run quirks first */
>>  	rdt_quirks();
>>  
>> +	/*
>> +	 * Initialize functions(or definitions) that are different
>> +	 * between vendors here.
>> +	 */
>> +	rdt_init_res_defs();
>> +
> 
> While it does seem as though currently rdt_quirks() is not using any of
> the settings made in rdt_init_res_defs() it (rdt_quirks()) does use the
> partially initialized resources structure and this may in the future
> include using parameters that have not been initialized yet.
> 
> I thus think it would be safer to do this initialization before
> rdt_quirks().

Yes.  Makes sense.

> 
> Reinette
> 

Re: [RFC PATCH 07/10] arch/x86: Bring few more functions into the resource structure

[Moger, Babu]

On 10/02/2018 05:07 PM, Reinette Chatre wrote:
> Hi Babu,
> 
> On 9/24/2018 12:19 PM, Moger, Babu wrote:
>> Bring all resource functions that are different between the vendors
>> into resource structure and initialize them dynamically.
>>
>> Implement these functions separately for each vendors.
>> update_mba_bw : Feedback loop bandwidth update functionality is not
>>                 needed for AMD.
>> cbm_validate  : Cache bitmask validate function. AMD allows
>>                 non-contiguous masks. So, use separate functions for
>>                 Intel and AMD.
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>>  arch/x86/kernel/cpu/rdt.c             | 17 +++++++++++++----
>>  arch/x86/kernel/cpu/rdt.h             | 19 +++++++++++++------
>>  arch/x86/kernel/cpu/rdt_ctrlmondata.c |  4 ++--
>>  arch/x86/kernel/cpu/rdt_monitor.c     | 10 +++++++---
>>  4 files changed, 35 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/rdt.c b/arch/x86/kernel/cpu/rdt.c
>> index 6dec45bf81d6..ae26b9b3fafa 100644
>> --- a/arch/x86/kernel/cpu/rdt.c
>> +++ b/arch/x86/kernel/cpu/rdt.c
>> @@ -867,10 +867,19 @@ static __init void rdt_init_res_defs_intel(void)
>>  	struct rdt_resource *r;
>>  
>>  	for_each_rdt_resource(r) {
>> -		if (r->rid == RDT_RESOURCE_MBA) {
>> -			r->msr_base = IA32_MBA_THRTL_BASE;
>> -			r->msr_update = mba_wrmsr;
>> -			r->parse_ctrlval = parse_bw;
>> +		if ((r->rid == RDT_RESOURCE_L3) ||
>> +		    (r->rid == RDT_RESOURCE_L3DATA) ||
>> +		    (r->rid == RDT_RESOURCE_L3CODE) ||
>> +		    (r->rid == RDT_RESOURCE_L2) ||
>> +		    (r->rid == RDT_RESOURCE_L2DATA) ||
>> +		    (r->rid == RDT_RESOURCE_L2CODE))
>> +			r->cbm_validate = cbm_validate;
> 
> Same comment here about naming as in patch 6. Later cbm_validate_amd
> would appear while this remains - to help reduce confusion it may help
> to rename this function to cbm_validate_intel at this time.

Sure.  Will make this change.

> 
>> +
>> +		else if (r->rid == RDT_RESOURCE_MBA) {
>> +			 r->msr_base = IA32_MBA_THRTL_BASE;
>> +			 r->msr_update = mba_wrmsr;
>> +			 r->parse_ctrlval = parse_bw;
>> +			 r->update_mba_bw = update_mba_bw;
> 
> Same comment about naming.

Yes.  Will add _intel to these functions.

> 
>>  		}
>>  	}
>>  }
>> diff --git a/arch/x86/kernel/cpu/rdt.h b/arch/x86/kernel/cpu/rdt.h
>> index 2569c10c37f4..7205157d359b 100644
>> --- a/arch/x86/kernel/cpu/rdt.h
>> +++ b/arch/x86/kernel/cpu/rdt.h
>> @@ -386,9 +386,9 @@ static inline bool is_mbm_event(int e)
>>   * struct rdt_resource - attributes of an RDT resource
>>   * @rid:		The index of the resource
>>   * @alloc_enabled:	Is allocation enabled on this machine
>> - * @mon_enabled:		Is monitoring enabled for this feature
>> + * @mon_enabled:	Is monitoring enabled for this feature
>>   * @alloc_capable:	Is allocation available on this machine
>> - * @mon_capable:		Is monitor feature available on this machine
>> + * @mon_capable:	Is monitor feature available on this machine
>>   * @name:		Name to use in "schemata" file
>>   * @num_closid:		Number of CLOSIDs available
>>   * @cache_level:	Which cache level defines scope of this resource
>> @@ -400,10 +400,12 @@ static inline bool is_mbm_event(int e)
>>   * @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
>> - * @evt_list:			List of monitoring events
>> - * @num_rmid:			Number of RMIDs available
>> - * @mon_scale:			cqm counter * mon_scale = occupancy in bytes
>> - * @fflags:			flags to choose base and info files
>> + * @update_mba_bw:	Feedback loop for MBA software controllerer function
> 
> controllerer -> controller ?

Yes. Will fix it.

> 
>> + * @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
>> + * @fflags:		flags to choose base and info files
>>   */
>>  struct rdt_resource {
>>  	int			rid;
>> @@ -425,6 +427,9 @@ struct rdt_resource {
>>  	const char		*format_str;
>>  	int (*parse_ctrlval)	(void *data, struct rdt_resource *r,
>>  				 struct rdt_domain *d);
>> +	void (*update_mba_bw)   (struct rdtgroup *rgrp,
>> +				 struct rdt_domain *dom_mbm);
>> +	bool (*cbm_validate)    (char *buf, u32 *data, struct rdt_resource *r);
>>  	struct list_head	evt_list;
>>  	int			num_rmid;
>>  	unsigned int		mon_scale;
>> @@ -562,5 +567,7 @@ 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);
>> +void update_mba_bw(struct rdtgroup *rgrp, struct rdt_domain *dom_mbm);
>> +bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r);
>>  
>>  #endif /* _ASM_X86_RDT_H */
>> diff --git a/arch/x86/kernel/cpu/rdt_ctrlmondata.c b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
>> index 0565c564b297..5a282b6c4bd7 100644
>> --- a/arch/x86/kernel/cpu/rdt_ctrlmondata.c
>> +++ b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
>> @@ -88,7 +88,7 @@ int parse_bw(void *_buf, struct rdt_resource *r, struct rdt_domain *d)
>>   *	are allowed (e.g. FFFFH, 0FF0H, 003CH, etc.).
>>   * Additionally Haswell requires at least two bits set.
>>   */
>> -static bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r)
>> +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;
>> @@ -153,7 +153,7 @@ int parse_cbm(void *_data, struct rdt_resource *r, struct rdt_domain *d)
>>  		return -EINVAL;
>>  	}
>>  
>> -	if (!cbm_validate(data->buf, &cbm_val, r))
>> +	if ((r->cbm_validate) && !(r->cbm_validate(data->buf, &cbm_val, r)))
> 
> I do not think you need all of those brackets.

Yes. Will simplify this.

> 
> Reinette
> 

Re: [RFC PATCH 10/10] arch/x86: Introduce QOS feature for AMD

[Moger, Babu]

On 10/02/2018 01:27 PM, Fenghua Yu wrote:
> On Mon, Sep 24, 2018 at 07:19:16PM +0000, Moger, Babu wrote:
>>  int parse_bw(void *_buf, struct rdt_resource *r,  struct rdt_domain *d);
>> +int parse_bw_amd(void *_buf, struct rdt_resource *r,  struct rdt_domain *d);
> 
> Please note the type of _buf in parse_bw() is changed in latest kernel
> to fix some issues. Please follow the same definition of parse_bw() in
> parse_bw_amd().

Yes, I noticed it. Saw the same issue(mba data parsing) and found it fixed
in 4-19-rc5. Will re-base to the latest kernel.
> 
> Thanks.
> 
> -Fenghua
> 

Re: [RFC PATCH 10/10] arch/x86: Introduce QOS feature for AMD

[Moger, Babu]

Hi Reinette,

On 10/02/2018 05:13 PM, Reinette Chatre wrote:
> Hi Babu,
> 
> On 9/24/2018 12:19 PM, Moger, Babu wrote:
>> +/*
>> + * Check whether a cache bit mask is valid. AMD allows
>> + * non-contiguous masks.
>> + */
>> +bool cbm_validate_amd(char *buf, u32 *data, struct rdt_resource *r)
>> +{
>> +	unsigned long first_bit, zero_bit, val;
>> +	unsigned int cbm_len = r->cache.cbm_len;
>> +	int ret;
>> +
>> +	ret = kstrtoul(buf, 16, &val);
>> +	if (ret) {
>> +		rdt_last_cmd_printf("non-hex character in mask %s\n", buf);
>> +		return false;
>> +	}
>> +
>> +	if (val == 0 || val > r->default_ctrl) {
>> +		rdt_last_cmd_puts("mask out of range\n");
>> +		return false;
>> +	}
> 
> According to
> https://www.amd.com/system/files/TechDocs/56375_Quality_of_Service_Extensions.pdf
> "If an L3_MASK_n register is programmed with all 0’s, that COS will be
> prevented from allocating any lines in the L3 cache."
> 
> The "val == 0" test thus does not seem necessary.

Yes. Good point. We don't need this test.
> 
>> +
>> +	first_bit = find_first_bit(&val, cbm_len);
>> +	zero_bit = find_next_zero_bit(&val, cbm_len, first_bit);
>> +
>> +
>> +	if ((zero_bit - first_bit) < r->cache.min_cbm_bits) {
>> +		rdt_last_cmd_printf("Need at least %d bits in mask\n",
>> +				    r->cache.min_cbm_bits);
>> +		return false;
>> +	}
> 
> If AMD platforms accept CBM of all zeroes then it seems that the
> platforms would not require a minimum number of set bits?

Yes. We don't need this check as well.  Tested and confirmed.
Thanks
> 
> Reinette
> 

Re: [RFC PATCH 00/10] arch/x86: AMD QoS support

[Fenghua Yu]

On Tue, Oct 02, 2018 at 07:16:23PM +0000, Moger, Babu wrote:
> 
> 
> On 10/02/2018 01:46 PM, Fenghua Yu wrote:
> > On Tue, Oct 02, 2018 at 05:44:47PM +0000, Moger, Babu wrote:
> >> Hi Fenghua,
> >>
> >>> -----Original Message-----
> >>> From: Fenghua Yu <fenghua.yu@intel.com>
> >>> Sent: Tuesday, October 2, 2018 12:07 PM
> >>> On Mon, Sep 24, 2018 at 07:18:54PM +0000, Moger, Babu wrote:
> >>>> The public specification is still in works. Will add the link when it is
> >>>> available.
> >>>
> >>> Is this the public AMD QoS spec?
> >>> https://support.amd.com/TechDocs/56375_Quality_of_Service_Extensions.
> >>> pdf
> >>>
> >>
> >> Yes. That is the one. 
> >> I was going to send the link after refreshing the patches on 4-19-rc5.  You already got it. Please let know if you have any feedback on the patches. 
> > 
> > I don't see the name 'RDT' (Resource Director Technology) in the
> > spec "AMD64 Technology Platform Quality of Service Extensions".
> > Does AMD use the same name 'RDT' as Intel?
> 
> No. AMD uses the word "Platform Quality of Service"(or in short QoS) to
> refer this feature.

Hi, Thomas, Babu, and James,

Can we use "resctrl" to replace "intel_rdt" and "rdt" in kernel? "resctrl"
is a neutral name and has been used in user interface already. Hopefully
"resctrl" can be acceptable by ARM, AMD, and Intel. We only use "intel_rdt",
"amd_qos", or "arm mpam" for vendor specific code.

Can we move all of Intel RDT, AMD QoS, and ARM MPAM code into a generic
place fs/resctrl where both ARM and X86 code can stay?
 
Thanks.

-Fenghua

Re: [RFC PATCH 03/10] arch/x86: Re-arrange RDT init code

[Reinette Chatre]

Hi Babu,

On 10/2/2018 4:41 PM, Moger, Babu wrote:
> On 10/02/2018 02:21 PM, Reinette Chatre wrote:
>> On 9/24/2018 12:19 PM, Moger, Babu wrote:
>>>  static enum cpuhp_state rdt_online;
>>> @@ -866,8 +872,22 @@ static int __init rdt_late_init(void)
>>>  	struct rdt_resource *r;
>>>  	int state, ret;
>>>  
>>> -	if (!get_rdt_resources())
>>> +	/* Run quirks first */
>>> +	rdt_quirks();
>>> +
>>> +	rdt_alloc_capable = get_rdt_alloc_resources();
>>> +	rdt_mon_capable = get_rdt_mon_resources();
>>> +
>>> +	if (!(rdt_alloc_capable || rdt_mon_capable)) {
>>> +		pr_info("RDT allocation or monitoring not detected\n");
>>
>> This function ends with a log entry for every resource discovered. Is
>> this new log entry needed to indicate that such resources have not been
>> found? Could it not just be the absence of the other message?
> 
> As this is relatively new feature, so I added this info message. It helped
> me debug what went wrong. Otherwise, I don't see anything. I can remove it
> if the message is too annoying to the user.

This log entry is made after detection of resources/features supported
by the system. A user would find more information in the
presence/absence of the relevant CPU feature flags in /proc/cpuinfo.

Reinette

Re: [RFC PATCH 00/10] arch/x86: AMD QoS support

[Thomas Gleixner]

On Wed, 3 Oct 2018, Fenghua Yu wrote:
> On Tue, Oct 02, 2018 at 07:16:23PM +0000, Moger, Babu wrote:
> > No. AMD uses the word "Platform Quality of Service"(or in short QoS) to
> > refer this feature.
> 
> Hi, Thomas, Babu, and James,
> 
> Can we use "resctrl" to replace "intel_rdt" and "rdt" in kernel? "resctrl"
> is a neutral name and has been used in user interface already. Hopefully
> "resctrl" can be acceptable by ARM, AMD, and Intel. We only use "intel_rdt",
> "amd_qos", or "arm mpam" for vendor specific code.

Works for me.

> Can we move all of Intel RDT, AMD QoS, and ARM MPAM code into a generic
> place fs/resctrl where both ARM and X86 code can stay?

Moving the filesystem to fs is fine. I'm not convinced that all the
architecture specific code should live there. James's patchset was nicely
splitting this apart.

Thanks,

	tglx

Re: [RFC PATCH 00/10] arch/x86: AMD QoS support

[Moger, Babu]

On 10/03/2018 01:52 PM, Fenghua Yu wrote:
> On Tue, Oct 02, 2018 at 07:16:23PM +0000, Moger, Babu wrote:
>>
>>
>> On 10/02/2018 01:46 PM, Fenghua Yu wrote:
>>> On Tue, Oct 02, 2018 at 05:44:47PM +0000, Moger, Babu wrote:
>>>> Hi Fenghua,
>>>>
>>>>> -----Original Message-----
>>>>> From: Fenghua Yu <fenghua.yu@intel.com>
>>>>> Sent: Tuesday, October 2, 2018 12:07 PM
>>>>> On Mon, Sep 24, 2018 at 07:18:54PM +0000, Moger, Babu wrote:
>>>>>> The public specification is still in works. Will add the link when it is
>>>>>> available.
>>>>>
>>>>> Is this the public AMD QoS spec?
>>>>> https://support.amd.com/TechDocs/56375_Quality_of_Service_Extensions.
>>>>> pdf
>>>>>
>>>>
>>>> Yes. That is the one. 
>>>> I was going to send the link after refreshing the patches on 4-19-rc5.  You already got it. Please let know if you have any feedback on the patches. 
>>>
>>> I don't see the name 'RDT' (Resource Director Technology) in the
>>> spec "AMD64 Technology Platform Quality of Service Extensions".
>>> Does AMD use the same name 'RDT' as Intel?
>>
>> No. AMD uses the word "Platform Quality of Service"(or in short QoS) to
>> refer this feature.
> 
> Hi, Thomas, Babu, and James,
> 
> Can we use "resctrl" to replace "intel_rdt" and "rdt" in kernel? "resctrl"
> is a neutral name and has been used in user interface already. Hopefully
> "resctrl" can be acceptable by ARM, AMD, and Intel. We only use "intel_rdt",
> "amd_qos", or "arm mpam" for vendor specific code.

Yes. resctrl seems like a reasonable name. >
> Can we move all of Intel RDT, AMD QoS, and ARM MPAM code into a generic
> place fs/resctrl where both ARM and X86 code can stay?

Looking at the James patches, I thought he is going in the same(or
similar) direction eventually.


>  
> Thanks.
> 
> -Fenghua
> 

Re: [RFC PATCH 03/10] arch/x86: Re-arrange RDT init code

[Moger, Babu]

On 10/03/2018 01:54 PM, Reinette Chatre wrote:
> Hi Babu,
> 
> On 10/2/2018 4:41 PM, Moger, Babu wrote:
>> On 10/02/2018 02:21 PM, Reinette Chatre wrote:
>>> On 9/24/2018 12:19 PM, Moger, Babu wrote:
>>>>  static enum cpuhp_state rdt_online;
>>>> @@ -866,8 +872,22 @@ static int __init rdt_late_init(void)
>>>>  	struct rdt_resource *r;
>>>>  	int state, ret;
>>>>  
>>>> -	if (!get_rdt_resources())
>>>> +	/* Run quirks first */
>>>> +	rdt_quirks();
>>>> +
>>>> +	rdt_alloc_capable = get_rdt_alloc_resources();
>>>> +	rdt_mon_capable = get_rdt_mon_resources();
>>>> +
>>>> +	if (!(rdt_alloc_capable || rdt_mon_capable)) {
>>>> +		pr_info("RDT allocation or monitoring not detected\n");
>>>
>>> This function ends with a log entry for every resource discovered. Is
>>> this new log entry needed to indicate that such resources have not been
>>> found? Could it not just be the absence of the other message?
>>
>> As this is relatively new feature, so I added this info message. It helped
>> me debug what went wrong. Otherwise, I don't see anything. I can remove it
>> if the message is too annoying to the user.
> 
> This log entry is made after detection of resources/features supported
> by the system. A user would find more information in the
> presence/absence of the relevant CPU feature flags in /proc/cpuinfo.

Ok. Understood. I have dropped this log entry.

> 
> Reinette
> 

Re: [RFC PATCH 00/10] arch/x86: AMD QoS support

[James Morse]

Hi Babu,

(Thanks for looping me in!)

On 28/09/18 02:57, Moger, Babu wrote:
>> On Mon, 24 Sep 2018, Moger, Babu wrote:
>>
>>> This series adds support for AMD64 architectural extensions for Platform
>>> Quality of Service. These extensions are intended to provide for the
>>> monitoring of the usage of certain system resources by one or more
>>> processors and for the separate allocation and enforcement of limits on
>>> the use of certain system resources by one or more processors.
>>>
>>> The monitoring and enforcement are not necessarily applied across the
>>> entire system, but in general apply to a QOS domain which corresponds to
>>> some shared system resource.  The set of resources which are monitored
>> and
>>> the set for which the enforcement of limits is provided are implementation
>>> dependent. Platform QOS features are implemented on a logical processor
>> basis.
>>> Therefore, multiple hardware threads of a single physical CPU core may
>> have
>>> independent resource monitoring and enforcement configurations.
>>>
>>> AMD's next generation of processors support following QoS sub-features.
>>> - L3 Cache allocation enforcement
>>> - L3 Cache occupancy monitoring
>>> - L3 Code-Data Prioritization support
>>> - Memory Bandwidth Enforcement(Allocation)
>>>
>>> The public specification is still in works. Will add the link when it is
>>> available.
>>>
>>> Obviously, there are multiple ways we can go about these changes. We felt
>>> it is appropriate to rename and re-organize the code little bit before
>>> making the functional changes. The first few patches(1-6) renames and
>>> re-organizes the sources in preparation. Rest of the patches(7-10) adds
>>> support for AMD QoS features.
>>
>> On the first glance this all looks sensible, but there is work in progress
>> by James Morse (Cc'ed), who wants to generalize the resctrl filesystem so
>> it can be reused by ARM. I just want to make sure that your reorganization
>> is not colliding or creating duplicate effort.

Aha, some of this makes my life easier. I'm against having the ABI different
between architectures. This meant contiguous bitmaps on arm, which the MPAM
stuff doesn't actually require...


> Thanks for pointing this out. I have looked thru James patches. It appears this
> series is only a small part of his much bigger change. Don't know the timeframe
> of his overall changes.
> I will let him speak on that.

Longer, as its more invasive,


> That being said, I don't consider our efforts as
> duplicate. He is touching the resource structures, and trying to separate arch,
> non-arch components.
> My changes are mostly inside the resource structures(mostly resource handlers)

> and trying to accommodate minor differences within the architecture.  It will
> be mostly involve rebase
> effort on either side in the end whoever goes first.

> James, What are your thoughts? 

I think your stuff should go first. It doesn't look like you are adding new
features/controls, so its normally:painful for me to rebase over it.
(doing it the other-way round would be harder!)


Thanks,

James

Re: [RFC PATCH 00/10] arch/x86: AMD QoS support

[James Morse]

Hi guys,

On 03/10/18 21:09, Moger, Babu wrote:
> On 10/03/2018 01:52 PM, Fenghua Yu wrote:
>> On Tue, Oct 02, 2018 at 07:16:23PM +0000, Moger, Babu wrote:
>>> On 10/02/2018 01:46 PM, Fenghua Yu wrote:
>>>> I don't see the name 'RDT' (Resource Director Technology) in the
>>>> spec "AMD64 Technology Platform Quality of Service Extensions".
>>>> Does AMD use the same name 'RDT' as Intel?
>>>
>>> No. AMD uses the word "Platform Quality of Service"(or in short QoS) to
>>> refer this feature.
>>
>> Hi, Thomas, Babu, and James,
>>
>> Can we use "resctrl" to replace "intel_rdt" and "rdt" in kernel? "resctrl"
>> is a neutral name and has been used in user interface already. Hopefully
>> "resctrl" can be acceptable by ARM, AMD, and Intel. We only use "intel_rdt",
>> "amd_qos", or "arm mpam" for vendor specific code.
> 
> Yes. resctrl seems like a reasonable name. >
>> Can we move all of Intel RDT, AMD QoS, and ARM MPAM code into a generic

all? Hmmm... I wanted to keep anything with an ABI implication under /fs/ so its
definitely the same. Obviously some things are definitely arch specific, e.g.
writing msr-s in intel_rdt_sched_in().


>> place fs/resctrl where both ARM and X86 code can stay?
> 
> Looking at the James patches, I thought he is going in the same(or
> similar) direction eventually.

Yes, I was trying to name stuff 'resctrl' if its moving to /fs/, and
resctrl_arch if its arch specific, thus stays where it is. (and the existing
names if I didn't need to touch it).

The struct names all have rdt in them, I thought it too noisy to change them.



Thanks,

James

Re: [RFC PATCH 10/10] arch/x86: Introduce QOS feature for AMD

[James Morse]

Hi Babu,

On 24/09/18 20:19, Moger, Babu wrote:
> Enables QOS feature on AMD.
> Following QoS sub-features are supported in AMD if the underlying
> hardware supports it.
>  - L3 Cache allocation enforcement
>  - L3 Cache occupancy monitoring
>  - L3 Code-Data Prioritization support
>  - Memory Bandwidth Enforcement(Allocation)
> 
> There are differences in the way some of the features are implemented.
> Separate those functions and add those as vendor specific functions.
> The major difference is in MBA feature.
>  - AMD uses CPUID leaf 0x80000020 to initialize the MBA features.
>  - AMD uses direct bandwidth value instead of delay based on bandwidth
>    values.
>  - MSR register base addresses are different for MBA.

>  - Also AMD allows non-contiguous L3 cache bit masks.

Nice!

This is visible to user-space, the 'Cache Bit Masks (CBM)' section of
Documentation/x86/intel_rdt_ui.txt currently says 'X86 hardware requires ... a
contiguous block'.

Does user-space need to know it can do this in advance, or is it a try-it-and-see?

Arm's MPAM stuff can do this too, but I'm against having the ABI vary between
architectures. If this is going to be discoverable, I'd like it to work on Arm too.


Thanks,

James

> Adds following functions to take care of the differences.
> rdt_get_mem_config_amd : MBA initialization function
> parse_bw_amd : Bandwidth parsing
> mba_wrmsr_amd: Writes bandwidth value
> cbm_validate_amd : L3 cache bitmask validation

> diff --git a/arch/x86/kernel/cpu/rdt_ctrlmondata.c b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
> index 5a282b6c4bd7..1e4631f88696 100644
> --- a/arch/x86/kernel/cpu/rdt_ctrlmondata.c
> +++ b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
> @@ -123,6 +169,41 @@ bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r)
>  	return true;
>  }
>  
> +/*
> + * Check whether a cache bit mask is valid. AMD allows
> + * non-contiguous masks.
> + */
> +bool cbm_validate_amd(char *buf, u32 *data, struct rdt_resource *r)
> +{
> +	unsigned long first_bit, zero_bit, val;
> +	unsigned int cbm_len = r->cache.cbm_len;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 16, &val);
> +	if (ret) {
> +		rdt_last_cmd_printf("non-hex character in mask %s\n", buf);
> +		return false;
> +	}
> +
> +	if (val == 0 || val > r->default_ctrl) {
> +		rdt_last_cmd_puts("mask out of range\n");
> +		return false;
> +	}
> +
> +	first_bit = find_first_bit(&val, cbm_len);
> +	zero_bit = find_next_zero_bit(&val, cbm_len, first_bit);
> +
> +
> +	if ((zero_bit - first_bit) < r->cache.min_cbm_bits) {
> +		rdt_last_cmd_printf("Need at least %d bits in mask\n",
> +				    r->cache.min_cbm_bits);
> +		return false;
> +	}
> +
> +	*data = val;
> +	return true;
> +}
> +
>  struct rdt_cbm_parse_data {
>  	struct rdtgroup		*rdtgrp;
>  	char			*buf;
> 

Re: [RFC PATCH 00/10] arch/x86: AMD QoS support

[Moger, Babu]

Hi James,

On 10/05/2018 11:18 AM, James Morse wrote:
> Hi Babu,
> 
> (Thanks for looping me in!)
> 
> On 28/09/18 02:57, Moger, Babu wrote:
>>> On Mon, 24 Sep 2018, Moger, Babu wrote:
>>>
>>>> This series adds support for AMD64 architectural extensions for Platform
>>>> Quality of Service. These extensions are intended to provide for the
>>>> monitoring of the usage of certain system resources by one or more
>>>> processors and for the separate allocation and enforcement of limits on
>>>> the use of certain system resources by one or more processors.
>>>>
>>>> The monitoring and enforcement are not necessarily applied across the
>>>> entire system, but in general apply to a QOS domain which corresponds to
>>>> some shared system resource.  The set of resources which are monitored
>>> and
>>>> the set for which the enforcement of limits is provided are implementation
>>>> dependent. Platform QOS features are implemented on a logical processor
>>> basis.
>>>> Therefore, multiple hardware threads of a single physical CPU core may
>>> have
>>>> independent resource monitoring and enforcement configurations.
>>>>
>>>> AMD's next generation of processors support following QoS sub-features.
>>>> - L3 Cache allocation enforcement
>>>> - L3 Cache occupancy monitoring
>>>> - L3 Code-Data Prioritization support
>>>> - Memory Bandwidth Enforcement(Allocation)
>>>>
>>>> The public specification is still in works. Will add the link when it is
>>>> available.
>>>>
>>>> Obviously, there are multiple ways we can go about these changes. We felt
>>>> it is appropriate to rename and re-organize the code little bit before
>>>> making the functional changes. The first few patches(1-6) renames and
>>>> re-organizes the sources in preparation. Rest of the patches(7-10) adds
>>>> support for AMD QoS features.
>>>
>>> On the first glance this all looks sensible, but there is work in progress
>>> by James Morse (Cc'ed), who wants to generalize the resctrl filesystem so
>>> it can be reused by ARM. I just want to make sure that your reorganization
>>> is not colliding or creating duplicate effort.
> 
> Aha, some of this makes my life easier. I'm against having the ABI different
> between architectures. This meant contiguous bitmaps on arm, which the MPAM
> stuff doesn't actually require...
> 
> 
>> Thanks for pointing this out. I have looked thru James patches. It appears this
>> series is only a small part of his much bigger change. Don't know the timeframe
>> of his overall changes.
>> I will let him speak on that.
> 
> Longer, as its more invasive,

Ok. Thanks for the heads up.

> 
> 
>> That being said, I don't consider our efforts as
>> duplicate. He is touching the resource structures, and trying to separate arch,
>> non-arch components.
>> My changes are mostly inside the resource structures(mostly resource handlers)
> 
>> and trying to accommodate minor differences within the architecture.  It will
>> be mostly involve rebase
>> effort on either side in the end whoever goes first.
> 
>> James, What are your thoughts? 
> 
> I think your stuff should go first. It doesn't look like you are adding new
> features/controls, so its normally:painful for me to rebase over it.
> (doing it the other-way round would be harder!)

Ok. Sounds good. I will send v2 version soon. Feel free to review.


> 
> 
> Thanks,
> 
> James
> 

Re: [RFC PATCH 10/10] arch/x86: Introduce QOS feature for AMD

[Moger, Babu]

Hi James,

On 10/05/2018 11:20 AM, James Morse wrote:
> Hi Babu,
> 
> On 24/09/18 20:19, Moger, Babu wrote:
>> Enables QOS feature on AMD.
>> Following QoS sub-features are supported in AMD if the underlying
>> hardware supports it.
>>  - L3 Cache allocation enforcement
>>  - L3 Cache occupancy monitoring
>>  - L3 Code-Data Prioritization support
>>  - Memory Bandwidth Enforcement(Allocation)
>>
>> There are differences in the way some of the features are implemented.
>> Separate those functions and add those as vendor specific functions.
>> The major difference is in MBA feature.
>>  - AMD uses CPUID leaf 0x80000020 to initialize the MBA features.
>>  - AMD uses direct bandwidth value instead of delay based on bandwidth
>>    values.
>>  - MSR register base addresses are different for MBA.
> 
>>  - Also AMD allows non-contiguous L3 cache bit masks.
> 
> Nice!
> 
> This is visible to user-space, the 'Cache Bit Masks (CBM)' section of
> Documentation/x86/intel_rdt_ui.txt currently says 'X86 hardware requires ... a
> contiguous block'.
> 
> Does user-space need to know it can do this in advance, or is it a try-it-and-see?

It is try-it-and-see.
> 
> Arm's MPAM stuff can do this too, but I'm against having the ABI vary between
> architectures. If this is going to be discoverable, I'd like it to work on Arm too.

It not discoverable at this point. Mostly predefined. Yes. It will be bit
of a challenge handle these differences. We may have to come up with some
kind of a flag(or something) to make it look similar on the ABI side.

> 
> 
> Thanks,
> 
> James
> 
>> Adds following functions to take care of the differences.
>> rdt_get_mem_config_amd : MBA initialization function
>> parse_bw_amd : Bandwidth parsing
>> mba_wrmsr_amd: Writes bandwidth value
>> cbm_validate_amd : L3 cache bitmask validation
> 
>> diff --git a/arch/x86/kernel/cpu/rdt_ctrlmondata.c b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
>> index 5a282b6c4bd7..1e4631f88696 100644
>> --- a/arch/x86/kernel/cpu/rdt_ctrlmondata.c
>> +++ b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
>> @@ -123,6 +169,41 @@ bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r)
>>  	return true;
>>  }
>>  
>> +/*
>> + * Check whether a cache bit mask is valid. AMD allows
>> + * non-contiguous masks.
>> + */
>> +bool cbm_validate_amd(char *buf, u32 *data, struct rdt_resource *r)
>> +{
>> +	unsigned long first_bit, zero_bit, val;
>> +	unsigned int cbm_len = r->cache.cbm_len;
>> +	int ret;
>> +
>> +	ret = kstrtoul(buf, 16, &val);
>> +	if (ret) {
>> +		rdt_last_cmd_printf("non-hex character in mask %s\n", buf);
>> +		return false;
>> +	}
>> +
>> +	if (val == 0 || val > r->default_ctrl) {
>> +		rdt_last_cmd_puts("mask out of range\n");
>> +		return false;
>> +	}
>> +
>> +	first_bit = find_first_bit(&val, cbm_len);
>> +	zero_bit = find_next_zero_bit(&val, cbm_len, first_bit);
>> +
>> +
>> +	if ((zero_bit - first_bit) < r->cache.min_cbm_bits) {
>> +		rdt_last_cmd_printf("Need at least %d bits in mask\n",
>> +				    r->cache.min_cbm_bits);
>> +		return false;
>> +	}
>> +
>> +	*data = val;
>> +	return true;
>> +}
>> +
>>  struct rdt_cbm_parse_data {
>>  	struct rdtgroup		*rdtgrp;
>>  	char			*buf;
>>
>