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

["Jan Beulich" <JBeulich@xxxxxxxx>]

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

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

Jan


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