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

From: "Jan Beulich" <JBeulich@suse.com>
To: Huaitong Han <huaitong.han@intel.com>
Cc: kevin.tian@intel.com, wei.liu2@citrix.com,
	ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com,
	george.dunlap@eu.citrix.com, andrew.cooper3@citrix.com,
	ian.jackson@eu.citrix.com, xen-devel@lists.xen.org,
	jun.nakajima@intel.com, keir@xen.org
Subject: Re: [V3 PATCH 9/9] x86/hvm: pkeys, add pkeys support for cpuid handling
Date: Fri, 11 Dec 2015 02:47:56 -0700	[thread overview]
Message-ID: <566AA9DC02000078000BE8A1@prv-mh.provo.novell.com> (raw)
In-Reply-To: <1449479780-19146-10-git-send-email-huaitong.han@intel.com>

>>> On 07.12.15 at 10:16, <huaitong.han@intel.com> wrote:
> @@ -4605,6 +4605,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

Coding style.

> +         *
> +         * 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) )

I has been said before that you should check the guest property
here, not the host one. Without you doing so I can't even see the
point of you adjusting the logic to set OSXSAVE above.

Jan