linux-kernel - Re: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 17 Oct 2016 14:50:26 -0700
From:   "Luck, Tony" <tony.luck@...el.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Fenghua Yu <fenghua.yu@...el.com>,
        "H. Peter Anvin" <h.peter.anvin@...el.com>,
        Ingo Molnar <mingo@...e.hu>,
        Peter Zijlstra <peterz@...radead.org>,
        Stephane Eranian <eranian@...gle.com>,
        Borislav Petkov <bp@...e.de>,
        Dave Hansen <dave.hansen@...el.com>,
        Nilay Vaish <nilayvaish@...il.com>, Shaohua Li <shli@...com>,
        David Carrillo-Cisneros <davidcc@...gle.com>,
        Ravi V Shankar <ravi.v.shankar@...el.com>,
        Sai Prakhya <sai.praneeth.prakhya@...el.com>,
        Vikas Shivappa <vikas.shivappa@...ux.intel.com>,
        linux-kernel <linux-kernel@...r.kernel.org>, x86 <x86@...nel.org>
Subject: Re: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system

On Mon, Oct 17, 2016 at 11:14:55PM +0200, Thomas Gleixner wrote:
> > +	/* Compute rdt_max_closid across all resources */
> > +	rdt_max_closid = 0;
> > +	for_each_rdt_resource(r)
> > +		rdt_max_closid = max(rdt_max_closid, r->num_closid);
> 
> Oh no! This needs to be min().
> 
> Assume you have a system with L3 and L2 CAT. L2 reports COS_MAX=16, L3
> reports COS_MAX=16 as well. Then you enabled CDP which cuts L3 COS_MAX in
> half. So the real usable number of CLOSIDs is going to be 8 for both L2 and
> L3 simply because you do not have a seperation of L2 and L3 in
> MSR_PQR_ASSOC. And if you allow 16 then any CLOSID > 8 will result in
> undefined behaviour. See SDM:
> 
>  "When CDP is enabled, specifying a COS value in IA32_PQR_ASSOC.COS outside
>   of the lower half of the COS space will cause undefined performance impact
>   to code and data fetches due to MSR space re-indexing into code/data masks
>   when CDP is enabled."

Bother.  The SDM also has this gem:

17.17.5.1 Cache Allocation Technology Dynamic Configuration
Both the CAT masks and CQM registers are accessible and modifiable at
any time during execution using RDMSR/WRMSR unless otherwise noted. When
writing to these MSRs a #GP(0) will be generated if any of the following
conditions occur:

 * Writing a COS greater than the supported maximum (specified as the
   maximum value of CPUID.(EAX=10H, ECX=ResID):EDX[15:0] for all valid
   ResID values) is written to the IA32_PQR_ASSOC.CLOS field.

With the intent here being that if you have more of one resource than
another, you can use all of the resources in the larger (with the
resource with fewer mask registers defaulting to the maximum value
when PQR_ASSOC.COS is too large [and I can't find the text that talks
about that default behaviour :-( ]

I think this all means that L3/CDP is "special". If CDP is on, we can't
use the top half of the CLOSID space that CPUID.(EAX=10H, ECX=1):EDX[15:0]
told us is present. So we can't exceed the half-way point. But in other
cases like L3 with max=16 and L2 with max=8 we should allow 16 groups,
but 0-7 allow control of L3 and L2, while 8-15 only allow L3 control (the
schemata code enforces this when you get to that part).

Perhaps we can encode this in another field in the rdt_resource structure
that says that some maximums globally override all others, while some
can be legitimately exceeded.

I'll have some words with the h/w architect, and get the SDM fixed in
the next edition.

-Tony