2014-12-04 08:24:36

by Wanpeng Li

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

Add nested virtualization support for xsaves.

Signed-off-by: Wanpeng Li <[email protected]>
---
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


2014-12-04 08:24:43

by Wanpeng Li

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

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 <[email protected]>
---
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

2014-12-04 08:24:40

by Wanpeng Li

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

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 <[email protected]>
---
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

2014-12-04 08:24:38

by Wanpeng Li

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

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 <[email protected]>
---
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

2014-12-04 08:31:04

by Nadav Amit

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

Wanpeng Li <[email protected]> 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 <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

Nadav

2014-12-04 08:55:09

by Wanpeng Li

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

Hi Nadav,
On 12/4/14, 4:30 PM, Nadav Amit wrote:
> Wanpeng Li <[email protected]> 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 <[email protected]>
>> ---
>> 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 [email protected]
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2014-12-04 13:56:04

by Paolo Bonzini

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



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 <[email protected]>
> ---
> 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;
>

2014-12-04 13:57:20

by Paolo Bonzini

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



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 <[email protected]>
> ---
> 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);
>
> /*
>

2014-12-04 13:59:16

by Paolo Bonzini

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



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 <[email protected]>
> ---
> 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;
>