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

[Thomas Gleixner <tglx@linutronix.de>]

On Mon, 17 Oct 2016, Luck, Tony wrote:
> 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.

That's my understanding.

> 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).

Cute. Welcome to configuration hell!

So how are we going to deal with that in the schematas? Assume the L3=16
and L2=8 case(no CDP). So effectively any write of L2 to CLOSID=0 will
affect the setting of L2 in CLOSID=8.

Will the code tell the user that L2 cannot be set for CLOSID >= 8? 

Will it print the setting of CLOSID - 8 for CLOSID >= 8 when you read the
schemata file of CLOSID >= 8?

And of course this becomes very interesting when CLOSID 1 is deleted, then
what happens to CLOSID 9? Not to talk about the case when CLOSID 1 is
reused again.

Seems to be a well thought out hardware feature once again.

> 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.

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

Whatever the outcome will be :)

Thanks,

	tglx