Re: [Xen-devel] [PATCH v2 04/14] x86/cpuid: Handle more simple Intel leaves in guest_cpuid()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 24/01/17 11:20, Jan Beulich wrote:
>>>> On 23.01.17 at 15:39, <andrew.cooper3@xxxxxxxxxx> wrote:
>> @@ -153,21 +158,31 @@ static void recalculate_xstate(struct cpuid_policy *p)
>>  
>>  /*
>>   * Misc adjustments to the policy.  Mostly clobbering reserved fields and
>> - * duplicating shared fields.
>> + * duplicating shared fields.  Intentionally hidden fields are annotated.
>>   */
>>  static void recalculate_misc(struct cpuid_policy *p)
>>  {
>> +    p->basic.raw[0x8] = EMPTY_LEAF;
>> +    p->basic.raw[0xc] = EMPTY_LEAF;
>> +
>>      p->extd.e1d &= ~CPUID_COMMON_1D_FEATURES;
>>  
>>      switch ( p->x86_vendor )
>>      {
>>      case X86_VENDOR_INTEL:
>> +        p->basic.l2_nr_queries = 1; /* Fixed to 1 query. */
>> +        p->basic.raw[0x3] = EMPTY_LEAF; /* PSN - always hidden. */
>> +        p->basic.raw[0x9] = EMPTY_LEAF; /* DCA - always hidden. */
> Hmm, for one this isn't very useful without also faking zero output
> for the respective MSR read. And then I think this might still be
> useful for pinned domains, so I'd view this as temporary state only
> (same for the un-exposed CPUID bit), yet the comment makes me
> assume this is intended to be permanent.

Well, thus far the toolstack has always explicitly zeroed this leaf. 
From that point of view, it is no change.

The MSR side of things is a separate can of worms which is following
this cpuid work on my todo list.

As for potential exposure, it should be fine to expose to
non-migrateable domains, but I'd consider that a new feature (and far
easier to implement once the default vs max cpuid_policy infrastructure
is in place).

>
>> @@ -694,8 +709,9 @@ static void pv_cpuid(uint32_t leaf, uint32_t subleaf, 
>> struct cpuid_leaf *res)
>>          break;
>>  
>>      case 0x0:
>> -    case 0x7:
>> -    case XSTATE_CPUID:
>> +    case 0x2 ... 0x3:
>> +    case 0x7 ... 0x9:
>> +    case 0xc ... XSTATE_CPUID:
> I can see how using a mix of literal numbers and constants ends up
> ugly here. I think we have two options: Either use only numbers,
> but then preferably include the constant(s) covered in a comment.
> Or only use ranges both ends of which are literal numbers.

These lists are just temporary (and this one specifically is only to
check I don't have duplicate case statements).  When the legacy path
disappears (i.e. I fill in all the holes in that numbering),
{pv,hvm}_cpuid() will disappear, and the similar list in guest_cpuid()
will be dropped and become the default case.

I don't mind them staying as they are.  They won't be around for long.

~Andrew

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