Re: [V2 PATCH 1/9] x86/hvm: pkeys, add pkeys support for cpuid handling

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 27/11/15 09:51, Huaitong Han wrote:
> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
> index e60929d..84d3a10 100644
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -264,8 +264,9 @@ static void __cpuinit generic_identify(struct cpuinfo_x86 *c)
>  	/* Intel-defined flags: level 0x00000007 */
>  	if ( c->cpuid_level >= 0x00000007 )
>  		cpuid_count(0x00000007, 0, &tmp,
> -			    &c->x86_capability[cpufeat_word(X86_FEATURE_FSGSBASE)],
> -			    &tmp, &tmp);
> +                &c->x86_capability[cpufeat_word(X86_FEATURE_FSGSBASE)],
> +                &c->x86_capability[cpufeat_word(X86_FEATURE_PKU)],
> +                &tmp);

You have some indentation issues here.

>  }
>  
>  /*
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index ea982e2..0adafe9 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4582,6 +4582,18 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
>          /* Don't expose INVPCID to non-hap hvm. */
>          if ( (count == 0) && !hap_enabled(d) )
>              *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID);
> +
> +        /* X86_FEATURE_PKU is not yet implemented for shadow paging
> +         *
> +         * Hypervisor gets guest pkru value from XSAVE state, because
> +         * Hypervisor CR4 without X86_CR4_PKE disables RDPKRU instruction.
> +         */
> +        if ( (count == 0) && (!hap_enabled(d) || !cpu_has_xsave) )
> +            *ecx &= ~cpufeat_mask(X86_FEATURE_PKU);
> +
> +        if ( (count == 0) && cpu_has_pku )
> +            *ecx |= (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_PKE) ?
> +                     cpufeat_mask(X86_FEATURE_OSPKE) : 0;

This is still buggy.  cpu_has_pku has no relevance to whether OSPKE
becomes visible.

Visibility of OSPKE is determined solely by v->arch.hvm_vcpu.guest_cr[4]
& X86_CR4_PKE and nothing else.

~Andrew