Re: [Xen-devel] Fwd: [PATCH RFC 3/4] x86/AMD: support further feature masking MSRs

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.04.14 at 18:21, <aravind.gopalakrishnan@xxxxxxx> wrote:
> On 4/4/2014 4:37 AM, Jan Beulich wrote:
>>>>> On 04.04.14 at 00:44, <aravind.gopalakrishnan@xxxxxxx> wrote:
>>> On 4/1/2014 6:10 PM, Aravind Gopalakrishnan wrote:
>>>>> Newer AMD CPUs also allow masking CPUID leaf 6 ECX and CPUID leaf 7
>>>>> sub-leaf 0 EAX and EBX.
>>>>>
>>>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx 
>>>>> <mailto:jbeulich@xxxxxxxx>>
>>>>>
>>>>> TBD:
>>>>> - Fam15M0x is documented to not have MSR C0011002 despite
>>>>> CPUID[7,0].EBX != 0 from model 02
>>>>>    onwards, and contrary to what I see on the system I have access to
>>>>> (which is model 02)
>>>>> - Fam12, Fam14, and Fam16 are documented to not have MSR C0011003
>>>>> despite CPUID[6].ECX != 0
>>>> Fam10 too has cpuid[6].ecx != 0 but no MSR C0011003
>>>>
>>>>> - Fam11 is documented to not even have MSR C0011004 and C0011005
>>>>>
>>>> I am still trying to get some clarity on this;
>>> Here's more info regarding your questions:
>>> 1. This is a documentation error.
>>> 2. We cannot mask cpuid[6].ecx on any of these families: 0x10, 0x11,
>>> 0x12,0x14,0x16 as feature is not supported..
>>> 3. We cannot mask standard,extended cpuid feature bits on Fam0x11 as
>>> feature is not supported.
>>>
>>> So simple enough additions to your patch to include some family checks:
>>>
>>> +    if (c->cpuid_level >= 7)
>>> +        cpuid_count(7, 0, &eax, &ebx, &ecx, &edx);
>>> +    else
>>> +        ebx = eax = 0;
>>> +    if ((eax | ebx) && ~(l7s0_eax & l7s0_ebx) &&
>>> +         c->x86 == 0x15 && c->x86_model >= 0x2) {
>> Also, is Fam16 indeed not capable of masking features here too?
>>
>> Assuming "yes" and "no", I'd rather use
>>
>>      if ((eax | ebx) && ~(l7s0_eax & l7s0_ebx) &&
>>           (c->x86 != 0x15 || c->x86_model >= 0x2)) {
>>
>> But then again models 0 and 1 are documented to have this CPUID
>> leaf blank, so we shouldn't need a family/model check here at all.
> 
> Yes, you are right. No need for above family check at all.
> But for fam16h, I was not able to access the MSR on my machine.
> And here's why:
> fam16h can use the MSR to mask the bit only if microcode patch level >= 
> 0x700010a

I think that's going too far - no-one is required to pass these command
line options, and all that would happen if someone did was that the
system would crash during boot. The one thing we may want to check
(and adjust if necessary) is that this code only runs after an eventual
microcode update was already applied, to increase the chances of
things working. After all, if a minimum microcode level is known, I
suppose respective microcode updates are or will be made available
publicly.

Andrew otoh, in his attempt to make the feature masking per-VM,
will clearly need to take this into consideration, or else he'd be
risking runtime crashes.

So I'll repost the patch set with the adjustments done so far, and
we'll go from there.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel