[PATCH 1/4] kvm: vmx: add nested virtualization support for xsaves

[PATCH 1/4] kvm: vmx: add nested virtualization support for xsaves

[Wanpeng Li]

Add nested virtualization support for xsaves.

Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
---
 arch/x86/kvm/vmx.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 6e3a448..e5bc349 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -216,6 +216,7 @@ struct __packed vmcs12 {
 	u64 virtual_apic_page_addr;
 	u64 apic_access_addr;
 	u64 ept_pointer;
+	u64 xss_exit_bitmap;
 	u64 guest_physical_address;
 	u64 vmcs_link_pointer;
 	u64 guest_ia32_debugctl;
@@ -618,6 +619,7 @@ static const unsigned short vmcs_field_to_offset_table[] = {
 	FIELD64(VIRTUAL_APIC_PAGE_ADDR, virtual_apic_page_addr),
 	FIELD64(APIC_ACCESS_ADDR, apic_access_addr),
 	FIELD64(EPT_POINTER, ept_pointer),
+	FIELD64(XSS_EXIT_BITMAP, xss_exit_bitmap),
 	FIELD64(GUEST_PHYSICAL_ADDRESS, guest_physical_address),
 	FIELD64(VMCS_LINK_POINTER, vmcs_link_pointer),
 	FIELD64(GUEST_IA32_DEBUGCTL, guest_ia32_debugctl),
@@ -1104,6 +1106,12 @@ static inline int nested_cpu_has_ept(struct vmcs12 *vmcs12)
 	return nested_cpu_has2(vmcs12, SECONDARY_EXEC_ENABLE_EPT);
 }
 
+static inline bool nested_cpu_has_xsaves(struct vmcs12 *vmcs12)
+{
+	return nested_cpu_has2(vmcs12, SECONDARY_EXEC_XSAVES) &&
+		vmx_xsaves_supported();
+}
+
 static inline bool is_exception(u32 intr_info)
 {
 	return (intr_info & (INTR_INFO_INTR_TYPE_MASK | INTR_INFO_VALID_MASK))
@@ -2392,7 +2400,8 @@ static __init void nested_vmx_setup_ctls_msrs(void)
 	nested_vmx_secondary_ctls_high &=
 		SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
 		SECONDARY_EXEC_UNRESTRICTED_GUEST |
-		SECONDARY_EXEC_WBINVD_EXITING;
+		SECONDARY_EXEC_WBINVD_EXITING |
+		SECONDARY_EXEC_XSAVES;
 
 	if (enable_ept) {
 		/* nested EPT: emulate EPT also to L1 */
@@ -7285,6 +7294,8 @@ static bool nested_vmx_exit_handled(struct kvm_vcpu *vcpu)
 		return nested_cpu_has2(vmcs12, SECONDARY_EXEC_WBINVD_EXITING);
 	case EXIT_REASON_XSETBV:
 		return 1;
+	case EXIT_REASON_XSAVES: case EXIT_REASON_XRSTORS:
+		return nested_cpu_has2(vmcs12, SECONDARY_EXEC_XSAVES);
 	default:
 		return 1;
 	}
@@ -8341,6 +8352,8 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
 	vmcs_writel(GUEST_SYSENTER_ESP, vmcs12->guest_sysenter_esp);
 	vmcs_writel(GUEST_SYSENTER_EIP, vmcs12->guest_sysenter_eip);
 
+	if (nested_cpu_has_xsaves(vmcs12))
+		vmcs_write64(XSS_EXIT_BITMAP, vmcs12->xss_exit_bitmap);
 	vmcs_write64(VMCS_LINK_POINTER, -1ull);
 
 	exec_control = vmcs12->pin_based_vm_exec_control;
@@ -8981,6 +8994,8 @@ static void prepare_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
 	vmcs12->guest_sysenter_eip = vmcs_readl(GUEST_SYSENTER_EIP);
 	if (vmx_mpx_supported())
 		vmcs12->guest_bndcfgs = vmcs_read64(GUEST_BNDCFGS);
+	if (nested_cpu_has_xsaves(vmcs12))
+		vmcs12->xss_exit_bitmap = vmcs_read64(XSS_EXIT_BITMAP);
 
 	/* update exit information fields: */
 
-- 
1.9.1

[PATCH 2/4] kvm: cpuid: fix the size of xsaves area

[Wanpeng Li]

The section of CPUID(EAX=0xd, ECX=1) in the spec which commit 
f5c2290cd01e (KVM: cpuid: mask more bits in leaf 0xd and subleaves)
mentioned is older than SDM. 

EBX: Bits 31-00: The size in bytes of the XSAVE area containing all 
states enabled by XCR0|IA32_XSS.

The the value of EBX should represent the size of XCR0 related XSAVE 
area since IA32_XSS is not used currently.

Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
---
 arch/x86/kvm/cpuid.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 646e6e8..70f0fa1 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -477,7 +477,10 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 			do_cpuid_1_ent(&entry[i], function, idx);
 			if (idx == 1) {
 				entry[i].eax &= kvm_supported_word10_x86_features;
-				entry[i].ebx = 0;
+				if (entry[i].eax & F(XSAVES))
+					entry[i].ebx = xstate_required_size(supported, true);
+				else
+					entry[i].ebx = xstate_required_size(supported, false);
 			} else {
 				if (entry[i].eax == 0 || !(supported & mask))
 					continue;
-- 
1.9.1

[PATCH 3/4] kvm: cpuid: fix xsave area size of XSAVEC

[Wanpeng Li]

XSAVEC also use the compacted format for the extended region
of the XSAVE area. This patch fix it by caculate the size of 
XSAVEC extended region of XSAVE area as compact format.

Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
---
 arch/x86/kvm/cpuid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 70f0fa1..e16a0c7 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -92,7 +92,7 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
 	}
 
 	best = kvm_find_cpuid_entry(vcpu, 0xD, 1);
-	if (best && (best->eax & F(XSAVES)))
+	if (best && (best->eax & (F(XSAVES) || F(XSAVEC))))
 		best->ebx = xstate_required_size(vcpu->arch.xcr0, true);
 
 	/*
-- 
1.9.1

[PATCH 4/4] kvm: vmx: fix VMfailValid when write vmcs02/vmcs01

[Wanpeng Li]

SDM 30.3 VMWRITE

ELSIF secondary source operand does not correspond to any VMCS field
   THEN VMfailValid(VMREAD/VMWRITE from/to unsupported VMCS component);

We can't suppose L1 VMM expose MPX to L2 just if L0 support MPX. There 
will be VMfailValid if L0 doesn't support MPX and L1 expose MPX to L2
when L0 writes vmcs02/vmcs01, in addition, there is no need to read 
GUEST_BNDCFGS if L1 VMM doesn't expose it to L2. This patch fix it by 
both check L0 support xsaves and L1 expose MPX to L2.

Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
---
 arch/x86/kvm/vmx.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index e5bc349..1233159 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -8496,7 +8496,8 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
 
 	set_cr4_guest_host_mask(vmx);
 
-	if (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_BNDCFGS)
+	if (vmx_mpx_supported() &&
+		(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_BNDCFGS))
 		vmcs_write64(GUEST_BNDCFGS, vmcs12->guest_bndcfgs);
 
 	if (vmcs12->cpu_based_vm_exec_control & CPU_BASED_USE_TSC_OFFSETING)
@@ -8992,7 +8993,8 @@ static void prepare_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
 	vmcs12->guest_sysenter_cs = vmcs_read32(GUEST_SYSENTER_CS);
 	vmcs12->guest_sysenter_esp = vmcs_readl(GUEST_SYSENTER_ESP);
 	vmcs12->guest_sysenter_eip = vmcs_readl(GUEST_SYSENTER_EIP);
-	if (vmx_mpx_supported())
+	if (vmx_mpx_supported() &&
+		(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_BNDCFGS))
 		vmcs12->guest_bndcfgs = vmcs_read64(GUEST_BNDCFGS);
 	if (nested_cpu_has_xsaves(vmcs12))
 		vmcs12->xss_exit_bitmap = vmcs_read64(XSS_EXIT_BITMAP);
@@ -9106,7 +9108,8 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
 	vmcs_writel(GUEST_GDTR_BASE, vmcs12->host_gdtr_base);
 
 	/* If not VM_EXIT_CLEAR_BNDCFGS, the L2 value propagates to L1.  */
-	if (vmcs12->vm_exit_controls & VM_EXIT_CLEAR_BNDCFGS)
+	if (vmx_mpx_supported() &&
+		(vmcs12->vm_exit_controls & VM_EXIT_CLEAR_BNDCFGS))
 		vmcs_write64(GUEST_BNDCFGS, 0);
 
 	if (vmcs12->vm_exit_controls & VM_EXIT_LOAD_IA32_PAT) {
-- 
1.9.1

Re: [PATCH 3/4] kvm: cpuid: fix xsave area size of XSAVEC

[Nadav Amit]

Wanpeng Li <wanpeng.li@linux.intel.com> wrote:

> XSAVEC also use the compacted format for the extended region
> of the XSAVE area. This patch fix it by caculate the size of 
> XSAVEC extended region of XSAVE area as compact format.
> 
> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
> ---
> arch/x86/kvm/cpuid.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 70f0fa1..e16a0c7 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -92,7 +92,7 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
> 	}
> 
> 	best = kvm_find_cpuid_entry(vcpu, 0xD, 1);
> -	if (best && (best->eax & F(XSAVES)))
> +	if (best && (best->eax & (F(XSAVES) || F(XSAVEC))))
Did you want to use | (bitwise or) instead of II (logical or) ?

> 		best->ebx = xstate_required_size(vcpu->arch.xcr0, true);
> 
> 	/*
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Nadav

Re: [PATCH 3/4] kvm: cpuid: fix xsave area size of XSAVEC

[Wanpeng Li]

Hi Nadav,
On 12/4/14, 4:30 PM, Nadav Amit wrote:
> Wanpeng Li <wanpeng.li@linux.intel.com> wrote:
>
>> XSAVEC also use the compacted format for the extended region
>> of the XSAVE area. This patch fix it by caculate the size of
>> XSAVEC extended region of XSAVE area as compact format.
>>
>> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
>> ---
>> arch/x86/kvm/cpuid.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
>> index 70f0fa1..e16a0c7 100644
>> --- a/arch/x86/kvm/cpuid.c
>> +++ b/arch/x86/kvm/cpuid.c
>> @@ -92,7 +92,7 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
>> 	}
>>
>> 	best = kvm_find_cpuid_entry(vcpu, 0xD, 1);
>> -	if (best && (best->eax & F(XSAVES)))
>> +	if (best && (best->eax & (F(XSAVES) || F(XSAVEC))))
> Did you want to use | (bitwise or) instead of II (logical or) ?

I make a mistake here, I will fix it in next version.

Regards,
Wanpeng Li

>
>> 		best->ebx = xstate_required_size(vcpu->arch.xcr0, true);
>>
>> 	/*
>> -- 
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Nadav
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/4] kvm: cpuid: fix the size of xsaves area

[Paolo Bonzini]

On 04/12/2014 09:24, Wanpeng Li wrote:
> The section of CPUID(EAX=0xd, ECX=1) in the spec which commit 
> f5c2290cd01e (KVM: cpuid: mask more bits in leaf 0xd and subleaves)
> mentioned is older than SDM. 
> 
> EBX: Bits 31-00: The size in bytes of the XSAVE area containing all 
> states enabled by XCR0|IA32_XSS.
> 
> The the value of EBX should represent the size of XCR0 related XSAVE 
> area since IA32_XSS is not used currently.
> 
> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
> ---
>  arch/x86/kvm/cpuid.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 646e6e8..70f0fa1 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -477,7 +477,10 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  			do_cpuid_1_ent(&entry[i], function, idx);
>  			if (idx == 1) {
>  				entry[i].eax &= kvm_supported_word10_x86_features;
> -				entry[i].ebx = 0;
> +				if (entry[i].eax & F(XSAVES))
> +					entry[i].ebx = xstate_required_size(supported, true);
> +				else
> +					entry[i].ebx = xstate_required_size(supported, false);

On machines without XSAVES, this value is zero though.

Paolo

>  			} else {
>  				if (entry[i].eax == 0 || !(supported & mask))
>  					continue;
> 

Re: [PATCH 3/4] kvm: cpuid: fix xsave area size of XSAVEC

[Paolo Bonzini]

On 04/12/2014 09:24, Wanpeng Li wrote:
> XSAVEC also use the compacted format for the extended region
> of the XSAVE area. This patch fix it by caculate the size of 
> XSAVEC extended region of XSAVE area as compact format.
> 
> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
> ---
>  arch/x86/kvm/cpuid.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 70f0fa1..e16a0c7 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -92,7 +92,7 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
>  	}
>  
>  	best = kvm_find_cpuid_entry(vcpu, 0xD, 1);
> -	if (best && (best->eax & F(XSAVES)))
> +	if (best && (best->eax & (F(XSAVES) || F(XSAVEC))))

"|" instead of "||" as mentioned by Nadav.

Is the Intel manual implicitly relying on the fact that there is no
processor that has XSAVES but not XSAVEC?

Paolo

>  		best->ebx = xstate_required_size(vcpu->arch.xcr0, true);
>  
>  	/*
> 

Re: [PATCH 2/4] kvm: cpuid: fix the size of xsaves area

[Paolo Bonzini]

On 04/12/2014 09:24, Wanpeng Li wrote:
> The section of CPUID(EAX=0xd, ECX=1) in the spec which commit 
> f5c2290cd01e (KVM: cpuid: mask more bits in leaf 0xd and subleaves)
> mentioned is older than SDM. 
> 
> EBX: Bits 31-00: The size in bytes of the XSAVE area containing all 
> states enabled by XCR0|IA32_XSS.
> 
> The the value of EBX should represent the size of XCR0 related XSAVE 
> area since IA32_XSS is not used currently.
> 
> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
> ---
>  arch/x86/kvm/cpuid.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 646e6e8..70f0fa1 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -477,7 +477,10 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  			do_cpuid_1_ent(&entry[i], function, idx);
>  			if (idx == 1) {
>  				entry[i].eax &= kvm_supported_word10_x86_features;
> -				entry[i].ebx = 0;
> +				if (entry[i].eax & F(XSAVES))

Also, either you have F(XSAVES)|F(XSAVEC) both here and in patch 3, or
you have F(XSAVES).  Making it inconsistent is definitely wrong.

Paolo

> +					entry[i].ebx = xstate_required_size(supported, true);
> +				else
> +					entry[i].ebx = xstate_required_size(supported, false);
>  			} else {
>  				if (entry[i].eax == 0 || !(supported & mask))
>  					continue;
>