From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [V2 PATCH 1/9] x86/hvm: pkeys,
	add pkeys support for cpuid handling
Date: Tue, 1 Dec 2015 20:03:37 +0000
Message-ID: <565DFD19.9080002@citrix.com>
References: <1448617923-10884-1-git-send-email-huaitong.han@intel.com>
	<1448617923-10884-2-git-send-email-huaitong.han@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1448617923-10884-2-git-send-email-huaitong.han@intel.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Huaitong Han <huaitong.han@intel.com>, jbeulich@suse.com
Cc: xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

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