| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20190614165528.GE198207@romley-ivt3.sc.intel.com>
Date: Fri, 14 Jun 2019 09:55:28 -0700
From: Fenghua Yu <fenghua.yu@...el.com>
To: Borislav Petkov <bp@...en8.de>
Cc: Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, H Peter Anvin <hpa@...or.com>,
Christopherson Sean J <sean.j.christopherson@...el.com>,
Ravi V Shankar <ravi.v.shankar@...el.com>,
linux-kernel <linux-kernel@...r.kernel.org>, x86 <x86@...nel.org>
Subject: Re: [RFC PATCH 1/3] x86/resctrl: Get max rmid and occupancy scale
directly from CPUID instead of cpuinfo_x86
On Fri, Jun 14, 2019 at 01:16:33PM +0200, Borislav Petkov wrote:
> On Thu, Jun 13, 2019 at 01:51:02PM -0700, Fenghua Yu wrote:
> > Although x86_cache_max_rmid and x86_cache_occ_scale are read only once
> > during resctrl initialization, they are always stored in cpuinfo_x86 on
> > each CPU during run time even if resctrl is not configured.
> >
> > To save cpuinfo_x86 space and make CPU and resctrl initialization simpler,
> > remove the two fields from cpuinfo_x86 and get max rmid and occupancy
> > scale directly from CPUID during resctrl initialization. And since each
> > known platform that supports resctrl has same max rmid on all CPUs, no
> > need to scan all CPUs to find minimum of max rmid values, i.e. getting
> > max rmid from CPUID on the current CPU is fine.
> >
> > Signed-off-by: Fenghua Yu <fenghua.yu@...el.com>
> > ---
> > arch/x86/include/asm/processor.h | 3 ---
> > arch/x86/kernel/cpu/common.c | 28 --------------------------
> > arch/x86/kernel/cpu/resctrl/internal.h | 2 +-
> > arch/x86/kernel/cpu/resctrl/monitor.c | 28 +++++++++++++++++++++++---
> > 4 files changed, 26 insertions(+), 35 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> > index c34a35c78618..27e875d4ca7d 100644
> > --- a/arch/x86/include/asm/processor.h
> > +++ b/arch/x86/include/asm/processor.h
> > @@ -99,9 +99,6 @@ struct cpuinfo_x86 {
> > /* in KB - valid for CPUS which support this call: */
> > unsigned int x86_cache_size;
> > int x86_cache_alignment; /* In bytes */
> > - /* Cache QoS architectural values: */
> > - int x86_cache_max_rmid; /* max index */
> > - int x86_cache_occ_scale; /* scale to bytes */
> > int x86_power;
> > unsigned long loops_per_jiffy;
> > /* cpuid returned max cores value: */
> > diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> > index 2c57fffebf9b..38e4b1a9005e 100644
> > --- a/arch/x86/kernel/cpu/common.c
> > +++ b/arch/x86/kernel/cpu/common.c
> > @@ -840,22 +840,9 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
> > c->x86_capability[CPUID_F_0_EDX] = edx;
> >
> > if (cpu_has(c, X86_FEATURE_CQM_LLC)) {
> > - /* will be overridden if occupancy monitoring exists */
> > - c->x86_cache_max_rmid = ebx;
> > -
> > /* QoS sub-leaf, EAX=0Fh, ECX=1 */
> > cpuid_count(0x0000000F, 1, &eax, &ebx, &ecx, &edx);
> > c->x86_capability[CPUID_F_1_EDX] = edx;
> > -
> > - if ((cpu_has(c, X86_FEATURE_CQM_OCCUP_LLC)) ||
> > - ((cpu_has(c, X86_FEATURE_CQM_MBM_TOTAL)) ||
> > - (cpu_has(c, X86_FEATURE_CQM_MBM_LOCAL)))) {
> > - c->x86_cache_max_rmid = ecx;
> > - c->x86_cache_occ_scale = ebx;
> > - }
> > - } else {
> > - c->x86_cache_max_rmid = -1;
> > - c->x86_cache_occ_scale = -1;
> > }
> > }
> >
> > @@ -1269,20 +1256,6 @@ static void generic_identify(struct cpuinfo_x86 *c)
> > #endif
> > }
> >
> > -static void x86_init_cache_qos(struct cpuinfo_x86 *c)
> > -{
> > - /*
> > - * The heavy lifting of max_rmid and cache_occ_scale are handled
> > - * in get_cpu_cap(). Here we just set the max_rmid for the boot_cpu
> > - * in case CQM bits really aren't there in this CPU.
> > - */
> > - if (c != &boot_cpu_data) {
> > - boot_cpu_data.x86_cache_max_rmid =
> > - min(boot_cpu_data.x86_cache_max_rmid,
> > - c->x86_cache_max_rmid);
> > - }
> > -}
> > -
> > /*
> > * Validate that ACPI/mptables have the same information about the
> > * effective APIC id and update the package map.
> > @@ -1391,7 +1364,6 @@ static void identify_cpu(struct cpuinfo_x86 *c)
> > #endif
> >
> > x86_init_rdrand(c);
> > - x86_init_cache_qos(c);
> > setup_pku(c);
> >
> > /*
> > diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> > index e49b77283924..474a7090d2dd 100644
> > --- a/arch/x86/kernel/cpu/resctrl/internal.h
> > +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> > @@ -579,7 +579,7 @@ int closids_supported(void);
> > void closid_free(int closid);
> > int alloc_rmid(void);
> > void free_rmid(u32 rmid);
> > -int rdt_get_mon_l3_config(struct rdt_resource *r);
> > +int __init rdt_get_mon_l3_config(struct rdt_resource *r);
> > void mon_event_count(void *info);
> > int rdtgroup_mondata_show(struct seq_file *m, void *arg);
> > void rmdir_mondata_subdir_allrdtgrp(struct rdt_resource *r,
> > diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
> > index 1573a0a6b525..e9d876c25703 100644
> > --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> > +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> > @@ -617,13 +617,35 @@ static void l3_mon_evt_init(struct rdt_resource *r)
> > list_add_tail(&mbm_local_event.list, &r->evt_list);
> > }
> >
> > -int rdt_get_mon_l3_config(struct rdt_resource *r)
> > +static void __init get_cqm_info(struct rdt_resource *r)
> > +{
> > + u32 eax, ebx, ecx, edx;
> > +
> > + /*
> > + * At this point, CQM LLC and one of occupancy, MBM total, and
> > + * MBM local monitoring features must be supported.
> > + */
> > + cpuid_count(0x0000000F, 0, &eax, &ebx, &ecx, &edx);
> > + /* will be overridden if occupancy monitoring exists */
> > + r->num_rmid = ebx + 1;
> > +
> > + cpuid_count(0x0000000F, 1, &eax, &ebx, &ecx, &edx);
>
> Those CPUID accesses should be done *after* testing features, not
> before.
>
> > +
> > + if (boot_cpu_has(X86_FEATURE_CQM_OCCUP_LLC))
>
> That is already done in get_rdt_mon_resources() and rdt_mon_features
> caches those bits. I think you wanna test QOS_L3_OCCUP_EVENT_ID in there
Sure. I can test QOS_L3_OCCUP_EVENT_ID instead of cpuinfo_x86.
> and then read CPUID 0xf and set ->num_rmid.
When this function is called, X86_FEATURE_CQM_LLC must be supported and
one of X86_FEATURE_CQM_OCCUP_LLC, X86_FEATURE_CQM_MBM_LOCAL, and
X86_FEATURE_CQM_MBM_TOTAL must be supported. Otherwise,
get_rdt_mon_resource() is returned before calling rdt_get_mon_l3_config().
So CPUID.f.0 and CPUID.f.1 must be readable and return meaningful
data without testing the features.
But the problem is it's unknown which ones of X86_FEATURE_CQM_OCCUP_LLC,
X86_FEATURE_CQM_MBM_LOCAL, and X86_FEATURE_CQM_MBM_TOTAL are supported.
So after reading CPUID.f.1, test X86_FEATURE_CQM_OCCUP_LLC in order to
override r->num_rmid from ecx. And test X86_FEATURE_CQM_MBM_TOTAL or
X86_FEATURE_CQM_MBM_LOCAL in order to get r>mon_scale from ebx.
Is this sequence OK? If the current sequence is OK, I only need to
change code to test the QOS_L3_*_EVENT_ID, right?
>
> > + r->num_rmid = ecx + 1;
> > +
> > + if (boot_cpu_has(X86_FEATURE_CQM_MBM_TOTAL) || boot_cpu_has(X86_FEATURE_CQM_MBM_LOCAL))
>
> Ditto.
Thanks.
-Fenghua
Powered by blists - more mailing lists