2018-10-05 20:59:10

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v2 00/11] arch/x86: AMD QoS support

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 for this feature is available at
https://www.amd.com/system/files/TechDocs/56375_Quality_of_Service_Extensions.pdf

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-11) adds
support for AMD QoS features.

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

Changes from v1 -> v2:
a. Removed RFC from subject header. Based on the discussion so far,
plan is to go ahead with these patches and eventually re-structure
the code to make arch and non-arch separate.
b. Addressed comments from Reinette Chatre and Fenghua Yu.
c. Separated quirks and MBA from rdt init code. Kept the rest of the
code as is.
d. Added _intel suffixes all the Intel only code just like AMD code.
e. Added one more patch to bring the macros into header file.
f. Few minor text changes.

v1:
https://lore.kernel.org/lkml/[email protected]/

Babu Moger (10):
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: Bring all the macros to rdt.h
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} | 167 +++++++++++++++---
arch/x86/kernel/cpu/{intel_rdt.h => rdt.h} | 41 +++--
...el_rdt_ctrlmondata.c => rdt_ctrlmondata.c} | 78 +++++++-
.../{intel_rdt_monitor.c => rdt_monitor.c} | 29 +--
...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 | 7 +-
arch/x86/kernel/process_32.c | 4 +-
arch/x86/kernel/process_64.c | 4 +-
include/linux/sched.h | 2 +-
14 files changed, 316 insertions(+), 89 deletions(-)
rename arch/x86/include/asm/{intel_rdt_sched.h => rdt_sched.h} (80%)
rename arch/x86/kernel/cpu/{intel_rdt.c => rdt.c} (85%)
rename arch/x86/kernel/cpu/{intel_rdt.h => rdt.h} (92%)
rename arch/x86/kernel/cpu/{intel_rdt_ctrlmondata.c => rdt_ctrlmondata.c} (86%)
rename arch/x86/kernel/cpu/{intel_rdt_monitor.c => rdt_monitor.c} (96%)
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



2018-10-05 20:56:39

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v2 01/11] arch/x86: Start renaming the rdt files to more generic names

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 <[email protected]>
---
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 0f53049719cd..812cc5c5e39e 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 1b8e86a5d5e1..5ecf73c833d3 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


2018-10-05 20:56:45

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v2 05/11] arch/x86: Introduce a new config parameter PLATFORM_QOS

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 <[email protected]>
---
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


2018-10-05 20:56:48

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v2 06/11] arch/x86: Use new config parameter PLATFORM_QOS for compilation

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

Signed-off-by: Babu Moger <[email protected]>
---
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


2018-10-05 20:56:51

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v2 07/11] arch/x86: Initialize the resource functions that are different

Initialize the resource functions that are different between the
vendors. Some features are initialized differently between the vendors.
Add _intel suffix to Intel specific functions.

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 <[email protected]>
---
arch/x86/kernel/cpu/rdt.c | 34 +++++++++++++++++++++++----
arch/x86/kernel/cpu/rdt.h | 8 +++++--
arch/x86/kernel/cpu/rdt_ctrlmondata.c | 2 +-
3 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/cpu/rdt.c b/arch/x86/kernel/cpu/rdt.c
index 87fe073a0571..9680a43d9485 100644
--- a/arch/x86/kernel/cpu/rdt.c
+++ b/arch/x86/kernel/cpu/rdt.c
@@ -57,7 +57,8 @@ int max_name_width, max_data_width;
bool rdt_alloc_capable;

static void
-mba_wrmsr(struct rdt_domain *d, struct msr_param *m, struct rdt_resource *r);
+mba_wrmsr_intel(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);

@@ -171,10 +172,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,
},
@@ -356,7 +354,8 @@ u32 delay_bw_map(unsigned long bw, struct rdt_resource *r)
}

static void
-mba_wrmsr(struct rdt_domain *d, struct msr_param *m, struct rdt_resource *r)
+mba_wrmsr_intel(struct rdt_domain *d, struct msr_param *m,
+ struct rdt_resource *r)
{
unsigned int i;

@@ -870,6 +869,25 @@ static __init bool get_rdt_resources(void)
return (rdt_mon_capable || rdt_alloc_capable);
}

+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_intel;
+ r->parse_ctrlval = 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)
@@ -877,6 +895,12 @@ static int __init rdt_late_init(void)
struct rdt_resource *r;
int state, ret;

+ /*
+ * Initialize functions(or definitions) that are different
+ * between vendors here.
+ */
+ rdt_init_res_defs();
+
/* Run quirks first */
rdt_quirks();

diff --git a/arch/x86/kernel/cpu/rdt.h b/arch/x86/kernel/cpu/rdt.h
index 8431af5c6825..42bf239313a0 100644
--- a/arch/x86/kernel/cpu/rdt.h
+++ b/arch/x86/kernel/cpu/rdt.h
@@ -444,8 +444,8 @@ struct rdt_resource {

int parse_cbm(struct rdt_parse_data *data, struct rdt_resource *r,
struct rdt_domain *d);
-int parse_bw(struct rdt_parse_data *data, struct rdt_resource *r,
- struct rdt_domain *d);
+int parse_bw_intel(struct rdt_parse_data *data, struct rdt_resource *r,
+ struct rdt_domain *d);

extern struct mutex rdtgroup_mutex;

@@ -468,6 +468,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++) \
diff --git a/arch/x86/kernel/cpu/rdt_ctrlmondata.c b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
index 812cc5c5e39e..ee3e8389d8d2 100644
--- a/arch/x86/kernel/cpu/rdt_ctrlmondata.c
+++ b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
@@ -64,7 +64,7 @@ static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
return true;
}

-int parse_bw(struct rdt_parse_data *data, struct rdt_resource *r,
+int parse_bw_intel(struct rdt_parse_data *data, struct rdt_resource *r,
struct rdt_domain *d)
{
unsigned long bw_val;
--
2.17.1


2018-10-05 20:56:54

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v2 10/11] arch/x86: Add AMD feature bit X86_FEATURE_MBA in cpuid bits array

From: Sherry Hurwitz <[email protected]>

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.

This feature is supported by both Intel and AMD. But they are detected
different CPUID leaves.

Signed-off-by: Babu Moger <[email protected]>
Signed-off-by: Sherry Hurwitz <[email protected]>
---
arch/x86/kernel/cpu/scattered.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 772c219b6889..bd7853334b27 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -17,7 +17,11 @@ struct cpuid_bit {
u32 sub_leaf;
};

-/* Please keep the leaf sorted by cpuid_bit.level for faster search. */
+/*
+ * Please keep the leaf sorted by cpuid_bit.level for faster search.
+ * X86_FEATURE_MBA supported by both Intel and AMD. But the cpuid
+ * levels are different. Add a separate enty for each.
+ */
static const struct cpuid_bit cpuid_bits[] = {
{ X86_FEATURE_APERFMPERF, CPUID_ECX, 0, 0x00000006, 0 },
{ X86_FEATURE_EPB, CPUID_ECX, 3, 0x00000006, 0 },
@@ -29,6 +33,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


2018-10-05 20:56:57

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v2 11/11] arch/x86: Introduce QOS feature for AMD

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 <[email protected]>
---
arch/x86/kernel/cpu/rdt.c | 69 +++++++++++++++++++++++++-
arch/x86/kernel/cpu/rdt.h | 5 ++
arch/x86/kernel/cpu/rdt_ctrlmondata.c | 70 +++++++++++++++++++++++++++
3 files changed, 142 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/rdt.c b/arch/x86/kernel/cpu/rdt.c
index c7c2dbaae7bb..99b3a69457c7 100644
--- a/arch/x86/kernel/cpu/rdt.c
+++ b/arch/x86/kernel/cpu/rdt.c
@@ -61,6 +61,9 @@ mba_wrmsr_intel(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)

@@ -280,6 +283,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;
@@ -339,6 +367,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.
@@ -788,8 +826,13 @@ static bool __init rdt_cpu_has(int flag)
static __init bool rdt_mba_config(void)
{
if (rdt_cpu_has(X86_FEATURE_MBA)) {
- if (rdt_get_mem_config(&rdt_resources_all[RDT_RESOURCE_MBA]))
- return true;
+ if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) {
+ if (rdt_get_mem_config(&rdt_resources_all[RDT_RESOURCE_MBA]))
+ return true;
+ } else if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) {
+ if (rdt_get_mem_config_amd(&rdt_resources_all[RDT_RESOURCE_MBA]))
+ return true;
+ }
}

return false;
@@ -890,10 +933,32 @@ 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 cb7e5a4739fc..4bc8c5594650 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 IA32_QM_CTR 0x0c8e
#define IA32_QM_EVTSEL 0x0c8d
@@ -34,6 +35,7 @@
#define MAX_MBA_BW 100u
#define MBA_IS_LINEAR 0x4
#define MBA_MAX_MBPS U32_MAX
+#define MAX_MBA_BW_AMD 0x800

#define RMID_VAL_ERROR BIT_ULL(63)
#define RMID_VAL_UNAVAIL BIT_ULL(62)
@@ -451,6 +453,8 @@ int parse_cbm(struct rdt_parse_data *data, struct rdt_resource *r,
struct rdt_domain *d);
int parse_bw_intel(struct rdt_parse_data *data, struct rdt_resource *r,
struct rdt_domain *d);
+int parse_bw_amd(struct rdt_parse_data *data, struct rdt_resource *r,
+ struct rdt_domain *d);

extern struct mutex rdtgroup_mutex;

@@ -583,5 +587,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_intel(struct rdtgroup *rgrp, struct rdt_domain *dom_mbm);
bool cbm_validate_intel(char *buf, u32 *data, struct rdt_resource *r);
+bool cbm_validate_amd(char *buf, u32 *data, struct rdt_resource *r);

#endif /* _ASM_X86_RDT_H */
diff --git a/arch/x86/kernel/cpu/rdt_ctrlmondata.c b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
index af8506003ee8..f9eb0de5ba96 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(struct rdt_parse_data *data, struct rdt_resource *r,
+ struct rdt_domain *d)
+{
+ unsigned long bw_val;
+
+ if (d->have_new_ctrl) {
+ rdt_last_cmd_printf("duplicate domain %d\n", d->id);
+ return -EINVAL;
+ }
+
+ if (!bw_validate_amd(data->buf, &bw_val, r))
+ return -EINVAL;
+ d->new_ctrl = bw_val;
+ 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,30 @@ bool cbm_validate_intel(char *buf, u32 *data, struct rdt_resource *r)
return true;
}

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


2018-10-05 20:57:01

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v2 09/11] arch/x86: Introduce new config parameter AMD_QOS

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 <[email protected]>
---
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


2018-10-05 20:58:01

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v2 04/11] arch/x86: Bring all the macros to rdt.h

Bring all the macros to rdt.h and rename for consistency.

Signed-off-by: Babu Moger <[email protected]>
---
arch/x86/kernel/cpu/rdt.c | 3 ---
arch/x86/kernel/cpu/rdt.h | 5 +++++
arch/x86/kernel/cpu/rdt_monitor.c | 7 ++-----
3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/cpu/rdt.c b/arch/x86/kernel/cpu/rdt.c
index c3ac7f9a3a0f..87fe073a0571 100644
--- a/arch/x86/kernel/cpu/rdt.c
+++ b/arch/x86/kernel/cpu/rdt.c
@@ -33,9 +33,6 @@
#include <asm/rdt_sched.h>
#include "rdt.h"

-#define MBA_IS_LINEAR 0x4
-#define MBA_MAX_MBPS U32_MAX
-
/* Mutex to protect rdtgroup access. */
DEFINE_MUTEX(rdtgroup_mutex);

diff --git a/arch/x86/kernel/cpu/rdt.h b/arch/x86/kernel/cpu/rdt.h
index 1d7aa7e266af..8431af5c6825 100644
--- a/arch/x86/kernel/cpu/rdt.h
+++ b/arch/x86/kernel/cpu/rdt.h
@@ -12,6 +12,9 @@
#define IA32_L2_CBM_BASE 0xd10
#define IA32_MBA_THRTL_BASE 0xd50

+#define IA32_QM_CTR 0x0c8e
+#define IA32_QM_EVTSEL 0x0c8d
+
#define L3_QOS_CDP_ENABLE 0x01ULL

#define L2_QOS_CDP_ENABLE 0x01ULL
@@ -29,6 +32,8 @@
#define MBM_CNTR_WIDTH 24
#define MBM_OVERFLOW_INTERVAL 1000
#define MAX_MBA_BW 100u
+#define MBA_IS_LINEAR 0x4
+#define MBA_MAX_MBPS U32_MAX

#define RMID_VAL_ERROR BIT_ULL(63)
#define RMID_VAL_UNAVAIL BIT_ULL(62)
diff --git a/arch/x86/kernel/cpu/rdt_monitor.c b/arch/x86/kernel/cpu/rdt_monitor.c
index 577514cd4a71..c8b95561f5be 100644
--- a/arch/x86/kernel/cpu/rdt_monitor.c
+++ b/arch/x86/kernel/cpu/rdt_monitor.c
@@ -28,9 +28,6 @@
#include <asm/cpu_device_id.h>
#include "rdt.h"

-#define MSR_IA32_QM_CTR 0x0c8e
-#define MSR_IA32_QM_EVTSEL 0x0c8d
-
struct rmid_entry {
u32 rmid;
int busy;
@@ -97,8 +94,8 @@ static u64 __rmid_read(u32 rmid, u32 eventid)
* IA32_QM_CTR.Error (bit 63) and IA32_QM_CTR.Unavailable (bit 62)
* are error bits.
*/
- wrmsr(MSR_IA32_QM_EVTSEL, eventid, rmid);
- rdmsrl(MSR_IA32_QM_CTR, val);
+ wrmsr(IA32_QM_EVTSEL, eventid, rmid);
+ rdmsrl(IA32_QM_CTR, val);

return val;
}
--
2.17.1


2018-10-05 20:58:06

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v2 03/11] arch/x86: Re-arrange RDT init code

Separate the call sequence for rdt_quirks and MBA feature.
This is in preparation to handle vendor differences in these
call sequences.

Signed-off-by: Babu Moger <[email protected]>
---
arch/x86/kernel/cpu/rdt.c | 29 +++++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/rdt.c b/arch/x86/kernel/cpu/rdt.c
index b361c63170d7..c3ac7f9a3a0f 100644
--- a/arch/x86/kernel/cpu/rdt.c
+++ b/arch/x86/kernel/cpu/rdt.c
@@ -789,6 +789,16 @@ static bool __init rdt_cpu_has(int flag)
return ret;
}

+static __init bool rdt_mba_config(void)
+{
+ if (rdt_cpu_has(X86_FEATURE_MBA)) {
+ if (rdt_get_mem_config(&rdt_resources_all[RDT_RESOURCE_MBA]))
+ return true;
+ }
+
+ return false;
+}
+
static __init bool get_rdt_alloc_resources(void)
{
bool ret = false;
@@ -813,10 +823,9 @@ 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;
- }
+ if (rdt_mba_config())
+ ret = true;
+
return ret;
}

@@ -835,7 +844,7 @@ static __init bool get_rdt_mon_resources(void)
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,9 +859,14 @@ static __init void rdt_quirks(void)
}
}

+static __init void rdt_quirks(void)
+{
+ if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
+ rdt_quirks_intel();
+}
+
static __init bool get_rdt_resources(void)
{
- rdt_quirks();
rdt_alloc_capable = get_rdt_alloc_resources();
rdt_mon_capable = get_rdt_mon_resources();

@@ -866,6 +880,9 @@ static int __init rdt_late_init(void)
struct rdt_resource *r;
int state, ret;

+ /* Run quirks first */
+ rdt_quirks();
+
if (!get_rdt_resources())
return -ENODEV;

--
2.17.1


2018-10-05 20:58:28

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v2 08/11] arch/x86: Bring few more functions into the resource structure

Bring all resource functions that are different between the vendors
into resource structure and initialize them dynamically.
Add _intel suffix to Intel specific functions.

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 <[email protected]>
---
arch/x86/kernel/cpu/rdt.c | 10 +++++++++-
arch/x86/kernel/cpu/rdt.h | 15 +++++++++++----
arch/x86/kernel/cpu/rdt_ctrlmondata.c | 4 ++--
arch/x86/kernel/cpu/rdt_monitor.c | 10 +++++++---
4 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/cpu/rdt.c b/arch/x86/kernel/cpu/rdt.c
index 9680a43d9485..c7c2dbaae7bb 100644
--- a/arch/x86/kernel/cpu/rdt.c
+++ b/arch/x86/kernel/cpu/rdt.c
@@ -874,10 +874,18 @@ static __init void rdt_init_res_defs_intel(void)
struct rdt_resource *r;

for_each_rdt_resource(r) {
- if (r->rid == RDT_RESOURCE_MBA) {
+ 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_intel;
+ else if (r->rid == RDT_RESOURCE_MBA) {
r->msr_base = IA32_MBA_THRTL_BASE;
r->msr_update = mba_wrmsr_intel;
r->parse_ctrlval = parse_bw_intel;
+ r->update_mba_bw = update_mba_bw_intel;
}
}
}
diff --git a/arch/x86/kernel/cpu/rdt.h b/arch/x86/kernel/cpu/rdt.h
index 42bf239313a0..cb7e5a4739fc 100644
--- a/arch/x86/kernel/cpu/rdt.h
+++ b/arch/x86/kernel/cpu/rdt.h
@@ -410,10 +410,12 @@ struct rdt_parse_data {
* @cache: Cache allocation related data
* @format_str: Per resource format string to show domain value
* @parse_ctrlval: Per resource function pointer to parse control values
- * @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 controller 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;
@@ -436,6 +438,9 @@ struct rdt_resource {
int (*parse_ctrlval)(struct rdt_parse_data *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;
@@ -576,5 +581,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_intel(struct rdtgroup *rgrp, struct rdt_domain *dom_mbm);
+bool cbm_validate_intel(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 ee3e8389d8d2..af8506003ee8 100644
--- a/arch/x86/kernel/cpu/rdt_ctrlmondata.c
+++ b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
@@ -88,7 +88,7 @@ int parse_bw_intel(struct rdt_parse_data *data, struct rdt_resource *r,
* 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_intel(char *buf, u32 *data, struct rdt_resource *r)
{
unsigned long first_bit, zero_bit, val;
unsigned int cbm_len = r->cache.cbm_len;
@@ -148,7 +148,7 @@ int parse_cbm(struct rdt_parse_data *data, struct rdt_resource *r,
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 c8b95561f5be..cd621237a6bb 100644
--- a/arch/x86/kernel/cpu/rdt_monitor.c
+++ b/arch/x86/kernel/cpu/rdt_monitor.c
@@ -358,7 +358,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_intel(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;
@@ -517,6 +517,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;

@@ -536,8 +537,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


2018-10-05 20:59:15

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v2 02/11] arch/x86: Rename the RDT functions and definitions

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 <[email protected]>
---
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 285eb3ec4200..1d7aa7e266af 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;
@@ -568,4 +568,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 5ecf73c833d3..bd8d03bad4aa 100644
--- a/arch/x86/kernel/cpu/rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/rdt_rdtgroup.c
@@ -288,7 +288,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.
@@ -307,7 +307,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();
}

/*
@@ -532,7 +532,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);
@@ -916,7 +916,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;
}
@@ -935,7 +935,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


2018-10-05 21:31:45

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v2 10/11] arch/x86: Add AMD feature bit X86_FEATURE_MBA in cpuid bits array

On Fri, Oct 05, 2018 at 08:56:09PM +0000, Moger, Babu wrote:
> From: Sherry Hurwitz <[email protected]>
>
> 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.
>
> This feature is supported by both Intel and AMD. But they are detected
> different CPUID leaves.
>
> Signed-off-by: Babu Moger <[email protected]>
> Signed-off-by: Sherry Hurwitz <[email protected]>

This SOB chain should be the other way around - first Sherry, then you.

> ---
> arch/x86/kernel/cpu/scattered.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
> index 772c219b6889..bd7853334b27 100644
> --- a/arch/x86/kernel/cpu/scattered.c
> +++ b/arch/x86/kernel/cpu/scattered.c
> @@ -17,7 +17,11 @@ struct cpuid_bit {
> u32 sub_leaf;
> };
>
> -/* Please keep the leaf sorted by cpuid_bit.level for faster search. */
> +/*
> + * Please keep the leaf sorted by cpuid_bit.level for faster search.
> + * X86_FEATURE_MBA supported by both Intel and AMD. But the cpuid
> + * levels are different. Add a separate enty for each.
> + */
> static const struct cpuid_bit cpuid_bits[] = {
> { X86_FEATURE_APERFMPERF, CPUID_ECX, 0, 0x00000006, 0 },
> { X86_FEATURE_EPB, CPUID_ECX, 3, 0x00000006, 0 },
> @@ -29,6 +33,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 }
> --

With that fixed:

Reviewed-by: Borislav Petkov <[email protected]>

--
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

2018-10-05 23:42:37

by Fenghua Yu

[permalink] [raw]
Subject: Re: [PATCH v2 05/11] arch/x86: Introduce a new config parameter PLATFORM_QOS

On Fri, Oct 05, 2018 at 08:55:52PM +0000, Moger, Babu wrote:
> 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 <[email protected]>
> ---
> 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
> +

Can change "PLATFORM_QOS" to a more neutral name "RESCTRL"?

Thanks.

-Fenghua

2018-10-08 02:01:59

by Moger, Babu

[permalink] [raw]
Subject: RE: [PATCH v2 05/11] arch/x86: Introduce a new config parameter PLATFORM_QOS

Hi Fenghua,

> -----Original Message-----
> From: Fenghua Yu <[email protected]>
> Sent: Friday, October 5, 2018 6:39 PM
> To: Moger, Babu <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; Hurwitz, Sherry <[email protected]>;
> Lendacky, Thomas <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 05/11] arch/x86: Introduce a new config parameter
> PLATFORM_QOS
>
> On Fri, Oct 05, 2018 at 08:55:52PM +0000, Moger, Babu wrote:
> > 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 <[email protected]>
> > ---
> > 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
> > +
>
> Can change "PLATFORM_QOS" to a more neutral name "RESCTRL"?

Yes. Will change it.
>
> Thanks.
>
> -Fenghua

2018-10-08 02:06:58

by Moger, Babu

[permalink] [raw]
Subject: RE: [PATCH v2 10/11] arch/x86: Add AMD feature bit X86_FEATURE_MBA in cpuid bits array



> -----Original Message-----
> From: Borislav Petkov <[email protected]>
> Sent: Friday, October 5, 2018 4:31 PM
> To: Moger, Babu <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; Hurwitz, Sherry <[email protected]>;
> Lendacky, Thomas <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 10/11] arch/x86: Add AMD feature bit
> X86_FEATURE_MBA in cpuid bits array
>
> On Fri, Oct 05, 2018 at 08:56:09PM +0000, Moger, Babu wrote:
> > From: Sherry Hurwitz <[email protected]>
> >
> > 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.
> >
> > This feature is supported by both Intel and AMD. But they are detected
> > different CPUID leaves.
> >
> > Signed-off-by: Babu Moger <[email protected]>
> > Signed-off-by: Sherry Hurwitz <[email protected]>
>
> This SOB chain should be the other way around - first Sherry, then you.

Sure. Will change it.
>
> > ---
> > arch/x86/kernel/cpu/scattered.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kernel/cpu/scattered.c
> b/arch/x86/kernel/cpu/scattered.c
> > index 772c219b6889..bd7853334b27 100644
> > --- a/arch/x86/kernel/cpu/scattered.c
> > +++ b/arch/x86/kernel/cpu/scattered.c
> > @@ -17,7 +17,11 @@ struct cpuid_bit {
> > u32 sub_leaf;
> > };
> >
> > -/* Please keep the leaf sorted by cpuid_bit.level for faster search. */
> > +/*
> > + * Please keep the leaf sorted by cpuid_bit.level for faster search.
> > + * X86_FEATURE_MBA supported by both Intel and AMD. But the cpuid
> > + * levels are different. Add a separate enty for each.
> > + */
> > static const struct cpuid_bit cpuid_bits[] = {
> > { X86_FEATURE_APERFMPERF, CPUID_ECX, 0, 0x00000006, 0 },
> > { X86_FEATURE_EPB, CPUID_ECX, 3, 0x00000006, 0 },
> > @@ -29,6 +33,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 }
> > --
>
> With that fixed:
>
> Reviewed-by: Borislav Petkov <[email protected]>
>

Thanks

> --
> Regards/Gruss,
> Boris.
>
> Good mailing practices for 400: avoid top-posting and trim the reply.

2018-10-09 16:40:32

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH v2 01/11] arch/x86: Start renaming the rdt files to more generic names

Hi Babu,

On 10/5/2018 1:55 PM, Moger, Babu wrote:
> 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 <[email protected]>
> ---
> 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%)

During the RFC it was agreed that "resctrl" will be the neutral name and
"intel_rdt", "amd_qos", or "arm mpam" would be the vendor specific names.

It is ok to delay that renaming but I think any renaming done from this
point should respect this agreement.

For example, if you want to rename intel_rdt.c then please rename it to
resctrl.c instead of just rdt.c which does not represent a generic name
as expressed as a goal in the subject of this patch.

Reinette

2018-10-09 16:43:33

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH v2 02/11] arch/x86: Rename the RDT functions and definitions

Hi Babu,

On 10/5/2018 1:55 PM, Moger, Babu wrote:
> As AMD is starting to support RDT(or QOS) features, rename
> the RDT functions and definitions to more generic names.

I have the same comment here. If the goal is to start with the renaming
to generic names then could you please use the "resctrl" generic name
that was agreed on during the RFC?

Reinette

2018-10-09 17:19:49

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH v2 07/11] arch/x86: Initialize the resource functions that are different

Hi Babu,

On 10/5/2018 1:55 PM, Moger, Babu wrote:
> diff --git a/arch/x86/kernel/cpu/rdt_ctrlmondata.c b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
> index 812cc5c5e39e..ee3e8389d8d2 100644
> --- a/arch/x86/kernel/cpu/rdt_ctrlmondata.c
> +++ b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
> @@ -64,7 +64,7 @@ static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
> return true;
> }
>
> -int parse_bw(struct rdt_parse_data *data, struct rdt_resource *r,
> +int parse_bw_intel(struct rdt_parse_data *data, struct rdt_resource *r,
> struct rdt_domain *d)
> {
> unsigned long bw_val;
>

Just a small nit - could you please check that the alignment matches the
open parenthesis as you have done with all the other similar changes in
this patch?

Reinette

2018-10-09 21:21:48

by Moger, Babu

[permalink] [raw]
Subject: Re: [PATCH v2 01/11] arch/x86: Start renaming the rdt files to more generic names

Hi Reinette,

On 10/09/2018 11:39 AM, Reinette Chatre wrote:
> Hi Babu,
>
> On 10/5/2018 1:55 PM, Moger, Babu wrote:
>> 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 <[email protected]>
>> ---
>> 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%)
>
> During the RFC it was agreed that "resctrl" will be the neutral name and
> "intel_rdt", "amd_qos", or "arm mpam" would be the vendor specific names.
>
> It is ok to delay that renaming but I think any renaming done from this
> point should respect this agreement.
>
> For example, if you want to rename intel_rdt.c then please rename it to
> resctrl.c instead of just rdt.c which does not represent a generic name
> as expressed as a goal in the subject of this patch.

I knew this was going to bit tricky. I can change all the places where I
am touching the code to generic names(change from intel_rdt to "resctrl").
Also lets change the "texts" which are visible to user to make it more
generic.

But "rdt" has been used generously in multiple files(like rdt_resource,
rdt_domain etc). Changing those definitions and functions will be messier.
I will not worry about it now. Thoughts?

>
> Reinette
>

2018-10-09 22:02:34

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH v2 01/11] arch/x86: Start renaming the rdt files to more generic names

Hi Babu,

On 10/9/2018 2:17 PM, Moger, Babu wrote:
> On 10/09/2018 11:39 AM, Reinette Chatre wrote:
>> Hi Babu,
>>
>> On 10/5/2018 1:55 PM, Moger, Babu wrote:
>>> 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 <[email protected]>
>>> ---
>>> 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%)
>>
>> During the RFC it was agreed that "resctrl" will be the neutral name and
>> "intel_rdt", "amd_qos", or "arm mpam" would be the vendor specific names.
>>
>> It is ok to delay that renaming but I think any renaming done from this
>> point should respect this agreement.
>>
>> For example, if you want to rename intel_rdt.c then please rename it to
>> resctrl.c instead of just rdt.c which does not represent a generic name
>> as expressed as a goal in the subject of this patch.
>
> I knew this was going to bit tricky. I can change all the places where I
> am touching the code to generic names(change from intel_rdt to "resctrl").

Yes, "intel_rdt" can be changed to the generic "resctrl" when it is not
vendor specific.

As far as all the code you touch is concerned it may be easier and cause
less confusion for now to just follow the current naming conventions as
you have done in patches 3 onwards and have it be included in the later
larger restructuring.

> Also lets change the "texts" which are visible to user to make it more
> generic.

Could you please elaborate what you mean with "texts" here? Are you
referring to the pr_info() found in intel_rdt_late_init()? Here it may
be good to also change to print "RESCTRL %s allocation
detected"/"RESCTRL %s monitoring detected" - the resource names printed
are already generic.

> But "rdt" has been used generously in multiple files(like rdt_resource,
> rdt_domain etc). Changing those definitions and functions will be messier.
> I will not worry about it now. Thoughts?

I agree. My comments were specific to the first two patches of this
series that started doing the renaming but not using the agreed upon
naming - especially since both those patches claim to transition to
generic names. Could just these two patches be modified to change
"intel_rdt" to "resctrl" instead of "intel_rdt" to "rdt" as it currently
does?

Reinette



2018-10-09 22:50:46

by Moger, Babu

[permalink] [raw]
Subject: Re: [PATCH v2 07/11] arch/x86: Initialize the resource functions that are different



On 10/09/2018 12:18 PM, Reinette Chatre wrote:
> Hi Babu,
>
> On 10/5/2018 1:55 PM, Moger, Babu wrote:
>> diff --git a/arch/x86/kernel/cpu/rdt_ctrlmondata.c b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
>> index 812cc5c5e39e..ee3e8389d8d2 100644
>> --- a/arch/x86/kernel/cpu/rdt_ctrlmondata.c
>> +++ b/arch/x86/kernel/cpu/rdt_ctrlmondata.c
>> @@ -64,7 +64,7 @@ static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
>> return true;
>> }
>>
>> -int parse_bw(struct rdt_parse_data *data, struct rdt_resource *r,
>> +int parse_bw_intel(struct rdt_parse_data *data, struct rdt_resource *r,
>> struct rdt_domain *d)
>> {
>> unsigned long bw_val;
>>
>
> Just a small nit - could you please check that the alignment matches the
> open parenthesis as you have done with all the other similar changes in
> this patch?

Ok. Sure. will take care of it. Thanks.

>
> Reinette
>

2018-10-10 14:12:26

by Moger, Babu

[permalink] [raw]
Subject: Re: [PATCH v2 01/11] arch/x86: Start renaming the rdt files to more generic names

Hi Reinette,

On 10/09/2018 05:01 PM, Reinette Chatre wrote:
> Hi Babu,
>
> On 10/9/2018 2:17 PM, Moger, Babu wrote:
>> On 10/09/2018 11:39 AM, Reinette Chatre wrote:
>>> Hi Babu,
>>>
>>> On 10/5/2018 1:55 PM, Moger, Babu wrote:
>>>> 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 <[email protected]>
>>>> ---
>>>> 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%)
>>>
>>> During the RFC it was agreed that "resctrl" will be the neutral name and
>>> "intel_rdt", "amd_qos", or "arm mpam" would be the vendor specific names.
>>>
>>> It is ok to delay that renaming but I think any renaming done from this
>>> point should respect this agreement.
>>>
>>> For example, if you want to rename intel_rdt.c then please rename it to
>>> resctrl.c instead of just rdt.c which does not represent a generic name
>>> as expressed as a goal in the subject of this patch.
>>
>> I knew this was going to bit tricky. I can change all the places where I
>> am touching the code to generic names(change from intel_rdt to "resctrl").
>
> Yes, "intel_rdt" can be changed to the generic "resctrl" when it is not
> vendor specific.

Ok. sure.

>
> As far as all the code you touch is concerned it may be easier and cause
> less confusion for now to just follow the current naming conventions as
> you have done in patches 3 onwards and have it be included in the later
> larger restructuring.

Yes. I am making sure first 3 patches are renamed to "resctrl" wherever
applicable. Will send the patches soon.

But I am confused about what you meant by "have it be included in the
later larger restructuring". Can you please elaborate?

>
>> Also lets change the "texts" which are visible to user to make it more
>> generic.
>
> Could you please elaborate what you mean with "texts" here? Are you
> referring to the pr_info() found in intel_rdt_late_init()? Here it may
> be good to also change to print "RESCTRL %s allocation
> detected"/"RESCTRL %s monitoring detected" - the resource names printed
> are already generic.

Yes. I meant pr_info text. Will change it to print "RESCTRL"

>
>> But "rdt" has been used generously in multiple files(like rdt_resource,
>> rdt_domain etc). Changing those definitions and functions will be messier.
>> I will not worry about it now. Thoughts?
>
> I agree. My comments were specific to the first two patches of this
> series that started doing the renaming but not using the agreed upon
> naming - especially since both those patches claim to transition to
> generic names. Could just these two patches be modified to change
> "intel_rdt" to "resctrl" instead of "intel_rdt" to "rdt" as it currently
> does?

Sure. Will take care of that.

>
> Reinette
>
>

2018-10-10 14:17:48

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v2 01/11] arch/x86: Start renaming the rdt files to more generic names

Babu,

On Wed, 10 Oct 2018, Moger, Babu wrote:
> On 10/09/2018 05:01 PM, Reinette Chatre wrote:
> > As far as all the code you touch is concerned it may be easier and cause
> > less confusion for now to just follow the current naming conventions as
> > you have done in patches 3 onwards and have it be included in the later
> > larger restructuring.
>
> Yes. I am making sure first 3 patches are renamed to "resctrl" wherever
> applicable. Will send the patches soon.

Please make sure they apply against

git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/cache

which has all the latest updates and fixes to this code.

Thanks,

tglx

2018-10-10 14:31:52

by Moger, Babu

[permalink] [raw]
Subject: Re: [PATCH v2 01/11] arch/x86: Start renaming the rdt files to more generic names

Thomas,

On 10/10/2018 09:16 AM, Thomas Gleixner wrote:
> Babu,
>
> On Wed, 10 Oct 2018, Moger, Babu wrote:
>> On 10/09/2018 05:01 PM, Reinette Chatre wrote:
>>> As far as all the code you touch is concerned it may be easier and cause
>>> less confusion for now to just follow the current naming conventions as
>>> you have done in patches 3 onwards and have it be included in the later
>>> larger restructuring.
>>
>> Yes. I am making sure first 3 patches are renamed to "resctrl" wherever
>> applicable. Will send the patches soon.
>
> Please make sure they apply against
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/cache
>
> which has all the latest updates and fixes to this code.

Sure. I will base next versions on top of this. Thanks

>
> Thanks,
>
> tglx
>

2018-10-10 17:54:34

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH v2 01/11] arch/x86: Start renaming the rdt files to more generic names

Hi Babu,

On 10/10/2018 7:11 AM, Moger, Babu wrote:
> On 10/09/2018 05:01 PM, Reinette Chatre wrote:
>> On 10/9/2018 2:17 PM, Moger, Babu wrote:
>>> On 10/09/2018 11:39 AM, Reinette Chatre wrote:
>>>> On 10/5/2018 1:55 PM, Moger, Babu wrote:
>>>>> 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 <[email protected]>
>>>>> ---
>>>>> 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%)
>>>>
>>>> During the RFC it was agreed that "resctrl" will be the neutral name and
>>>> "intel_rdt", "amd_qos", or "arm mpam" would be the vendor specific names.
>>>>
>>>> It is ok to delay that renaming but I think any renaming done from this
>>>> point should respect this agreement.
>>>>
>>>> For example, if you want to rename intel_rdt.c then please rename it to
>>>> resctrl.c instead of just rdt.c which does not represent a generic name
>>>> as expressed as a goal in the subject of this patch.
>>>
>>> I knew this was going to bit tricky. I can change all the places where I
>>> am touching the code to generic names(change from intel_rdt to "resctrl").
>>
>> Yes, "intel_rdt" can be changed to the generic "resctrl" when it is not
>> vendor specific.
>
> Ok. sure.
>
>>
>> As far as all the code you touch is concerned it may be easier and cause
>> less confusion for now to just follow the current naming conventions as
>> you have done in patches 3 onwards and have it be included in the later
>> larger restructuring.
>
> Yes. I am making sure first 3 patches are renamed to "resctrl" wherever
> applicable. Will send the patches soon.
>
> But I am confused about what you meant by "have it be included in the
> later larger restructuring". Can you please elaborate?

I was referring to the restructuring that was discussed during your
original RFC submission. Specifically,
http://lkml.kernel.org/r/[email protected]
http://lkml.kernel.org/r/[email protected]

Reinette




2018-10-10 18:07:46

by Moger, Babu

[permalink] [raw]
Subject: Re: [PATCH v2 01/11] arch/x86: Start renaming the rdt files to more generic names



On 10/10/2018 12:53 PM, Reinette Chatre wrote:
> Hi Babu,
>
> On 10/10/2018 7:11 AM, Moger, Babu wrote:
>> On 10/09/2018 05:01 PM, Reinette Chatre wrote:
>>> On 10/9/2018 2:17 PM, Moger, Babu wrote:
>>>> On 10/09/2018 11:39 AM, Reinette Chatre wrote:
>>>>> On 10/5/2018 1:55 PM, Moger, Babu wrote:
>>>>>> 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 <[email protected]>
>>>>>> ---
>>>>>> 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%)
>>>>>
>>>>> During the RFC it was agreed that "resctrl" will be the neutral name and
>>>>> "intel_rdt", "amd_qos", or "arm mpam" would be the vendor specific names.
>>>>>
>>>>> It is ok to delay that renaming but I think any renaming done from this
>>>>> point should respect this agreement.
>>>>>
>>>>> For example, if you want to rename intel_rdt.c then please rename it to
>>>>> resctrl.c instead of just rdt.c which does not represent a generic name
>>>>> as expressed as a goal in the subject of this patch.
>>>>
>>>> I knew this was going to bit tricky. I can change all the places where I
>>>> am touching the code to generic names(change from intel_rdt to "resctrl").
>>>
>>> Yes, "intel_rdt" can be changed to the generic "resctrl" when it is not
>>> vendor specific.
>>
>> Ok. sure.
>>
>>>
>>> As far as all the code you touch is concerned it may be easier and cause
>>> less confusion for now to just follow the current naming conventions as
>>> you have done in patches 3 onwards and have it be included in the later
>>> larger restructuring.
>>
>> Yes. I am making sure first 3 patches are renamed to "resctrl" wherever
>> applicable. Will send the patches soon.
>>
>> But I am confused about what you meant by "have it be included in the
>> later larger restructuring". Can you please elaborate?
>
> I was referring to the restructuring that was discussed during your
> original RFC submission. Specifically,
> http://lkml.kernel.org/r/[email protected]
> http://lkml.kernel.org/r/[email protected]

Ok. Got it. Thanks.

>
> Reinette
>
>
>

2018-11-02 06:45:23

by Jon Masters

[permalink] [raw]
Subject: Re: [PATCH v2 00/11] arch/x86: AMD QoS support

On 10/5/18 4:55 PM, Moger, Babu wrote:

> The public specification for this feature is available at
> https://www.amd.com/system/files/TechDocs/56375_Quality_of_Service_Extensions.pdf

404 error