All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] KVM: VMX: Don't expose unrestricted_guest is enabled if ept is disabled
@ 2017-10-08  2:35 Wanpeng Li
  2017-10-08  2:35 ` [PATCH 2/2] KVM: X86: XCR0 should be set to the fixed value on vCPU reset Wanpeng Li
  2017-10-08 22:35 ` [PATCH 1/2] KVM: VMX: Don't expose unrestricted_guest is enabled if ept is disabled Jim Mattson
  0 siblings, 2 replies; 6+ messages in thread
From: Wanpeng Li @ 2017-10-08  2:35 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: Paolo Bonzini, Radim Krčmář, Wanpeng Li

From: Wanpeng Li <wanpeng.li@hotmail.com>

SDM mentioned: 

 "If either the “unrestricted guest” VM-execution control or the “mode-based 
  execute control for EPT” VM- execution control is 1, the “enable EPT” 
  VM-execution control must also be 1."

However, we can still observe unrestricted_guest is Y after inserting the kvm-intel.ko 
w/ ept=N. It depends on later starts a guest in order that the function 
vmx_compute_secondary_exec_control() can be executed, then both the module parameter 
and exec control fields will be amended.

This patch fixes it by amending module parameter immediately during vmcs data setup.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 arch/x86/kvm/vmx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 244e366..3e664ca 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -6737,7 +6737,7 @@ static __init int hardware_setup(void)
 	if (!cpu_has_vmx_ept_ad_bits() || !enable_ept)
 		enable_ept_ad_bits = 0;
 
-	if (!cpu_has_vmx_unrestricted_guest())
+	if (!cpu_has_vmx_unrestricted_guest() || !enable_ept)
 		enable_unrestricted_guest = 0;
 
 	if (!cpu_has_vmx_flexpriority())
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] KVM: X86: XCR0 should be set to the fixed value on vCPU reset
  2017-10-08  2:35 [PATCH 1/2] KVM: VMX: Don't expose unrestricted_guest is enabled if ept is disabled Wanpeng Li
@ 2017-10-08  2:35 ` Wanpeng Li
  2017-10-08 22:36   ` Jim Mattson
  2017-10-09  8:04   ` Paolo Bonzini
  2017-10-08 22:35 ` [PATCH 1/2] KVM: VMX: Don't expose unrestricted_guest is enabled if ept is disabled Jim Mattson
  1 sibling, 2 replies; 6+ messages in thread
From: Wanpeng Li @ 2017-10-08  2:35 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: Paolo Bonzini, Radim Krčmář, Wanpeng Li

From: Wanpeng Li <wanpeng.li@hotmail.com>

SDM section 2.6 mentioned: 

After reset, all bits (except bit 0) in XCR0 are cleared to zero; XCR0[0] is set to 1.

This patch sets XCRO to the 0x1 after vCPU reset.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 arch/x86/kvm/x86.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index b0d2915..c784cd6 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7816,6 +7816,8 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
 	vcpu->arch.regs_avail = ~0;
 	vcpu->arch.regs_dirty = ~0;
 
+	vcpu->arch.xcr0 = XFEATURE_MASK_FP;
+
 	kvm_x86_ops->vcpu_reset(vcpu, init_event);
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] KVM: VMX: Don't expose unrestricted_guest is enabled if ept is disabled
  2017-10-08  2:35 [PATCH 1/2] KVM: VMX: Don't expose unrestricted_guest is enabled if ept is disabled Wanpeng Li
  2017-10-08  2:35 ` [PATCH 2/2] KVM: X86: XCR0 should be set to the fixed value on vCPU reset Wanpeng Li
@ 2017-10-08 22:35 ` Jim Mattson
  2017-10-09  7:54   ` Paolo Bonzini
  1 sibling, 1 reply; 6+ messages in thread
From: Jim Mattson @ 2017-10-08 22:35 UTC (permalink / raw)
  To: Wanpeng Li
  Cc: LKML, kvm list, Paolo Bonzini, Radim Krčmář, Wanpeng Li

If it were me, I'd apply De Morgan to that expression, but the logic looks fine.

Reviewed-by: Jim Mattson <jmattson@google.com>

On Sat, Oct 7, 2017 at 7:35 PM, Wanpeng Li <kernellwp@gmail.com> wrote:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
>
> SDM mentioned:
>
>  "If either the “unrestricted guest” VM-execution control or the “mode-based
>   execute control for EPT” VM- execution control is 1, the “enable EPT”
>   VM-execution control must also be 1."
>
> However, we can still observe unrestricted_guest is Y after inserting the kvm-intel.ko
> w/ ept=N. It depends on later starts a guest in order that the function
> vmx_compute_secondary_exec_control() can be executed, then both the module parameter
> and exec control fields will be amended.
>
> This patch fixes it by amending module parameter immediately during vmcs data setup.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
>  arch/x86/kvm/vmx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 244e366..3e664ca 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -6737,7 +6737,7 @@ static __init int hardware_setup(void)
>         if (!cpu_has_vmx_ept_ad_bits() || !enable_ept)
>                 enable_ept_ad_bits = 0;
>
> -       if (!cpu_has_vmx_unrestricted_guest())
> +       if (!cpu_has_vmx_unrestricted_guest() || !enable_ept)
>                 enable_unrestricted_guest = 0;
>
>         if (!cpu_has_vmx_flexpriority())
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] KVM: X86: XCR0 should be set to the fixed value on vCPU reset
  2017-10-08  2:35 ` [PATCH 2/2] KVM: X86: XCR0 should be set to the fixed value on vCPU reset Wanpeng Li
@ 2017-10-08 22:36   ` Jim Mattson
  2017-10-09  8:04   ` Paolo Bonzini
  1 sibling, 0 replies; 6+ messages in thread
From: Jim Mattson @ 2017-10-08 22:36 UTC (permalink / raw)
  To: Wanpeng Li
  Cc: LKML, kvm list, Paolo Bonzini, Radim Krčmář, Wanpeng Li

Reviewed-by: Jim Mattson <jmattson@google.com>

On Sat, Oct 7, 2017 at 7:35 PM, Wanpeng Li <kernellwp@gmail.com> wrote:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
>
> SDM section 2.6 mentioned:
>
> After reset, all bits (except bit 0) in XCR0 are cleared to zero; XCR0[0] is set to 1.
>
> This patch sets XCRO to the 0x1 after vCPU reset.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
>  arch/x86/kvm/x86.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index b0d2915..c784cd6 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -7816,6 +7816,8 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>         vcpu->arch.regs_avail = ~0;
>         vcpu->arch.regs_dirty = ~0;
>
> +       vcpu->arch.xcr0 = XFEATURE_MASK_FP;
> +
>         kvm_x86_ops->vcpu_reset(vcpu, init_event);
>  }
>
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] KVM: VMX: Don't expose unrestricted_guest is enabled if ept is disabled
  2017-10-08 22:35 ` [PATCH 1/2] KVM: VMX: Don't expose unrestricted_guest is enabled if ept is disabled Jim Mattson
@ 2017-10-09  7:54   ` Paolo Bonzini
  0 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2017-10-09  7:54 UTC (permalink / raw)
  To: Jim Mattson, Wanpeng Li
  Cc: LKML, kvm list, Radim Krčmář, Wanpeng Li

On 09/10/2017 00:35, Jim Mattson wrote:
> If it were me, I'd apply De Morgan to that expression, but the logic looks fine.
> 
> Reviewed-by: Jim Mattson <jmattson@google.com>

I'm okay with the way Wanpeng wrote it, but as a follow up this:

        if (!cpu_has_vmx_ept() ||
            !cpu_has_vmx_ept_4levels() ||
            !cpu_has_vmx_ept_mt_wb()) {
                enable_ept = 0;
                enable_unrestricted_guest = 0;
                enable_ept_ad_bits = 0;
        }

can be reduced to just "enable_ept = 0".

Paolo

> On Sat, Oct 7, 2017 at 7:35 PM, Wanpeng Li <kernellwp@gmail.com> wrote:
>> From: Wanpeng Li <wanpeng.li@hotmail.com>
>>
>> SDM mentioned:
>>
>>  "If either the “unrestricted guest” VM-execution control or the “mode-based
>>   execute control for EPT” VM- execution control is 1, the “enable EPT”
>>   VM-execution control must also be 1."
>>
>> However, we can still observe unrestricted_guest is Y after inserting the kvm-intel.ko
>> w/ ept=N. It depends on later starts a guest in order that the function
>> vmx_compute_secondary_exec_control() can be executed, then both the module parameter
>> and exec control fields will be amended.
>>
>> This patch fixes it by amending module parameter immediately during vmcs data setup.
>>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> ---
>>  arch/x86/kvm/vmx.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 244e366..3e664ca 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -6737,7 +6737,7 @@ static __init int hardware_setup(void)
>>         if (!cpu_has_vmx_ept_ad_bits() || !enable_ept)
>>                 enable_ept_ad_bits = 0;
>>
>> -       if (!cpu_has_vmx_unrestricted_guest())
>> +       if (!cpu_has_vmx_unrestricted_guest() || !enable_ept)
>>                 enable_unrestricted_guest = 0;
>>
>>         if (!cpu_has_vmx_flexpriority())
>> --
>> 2.7.4
>>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] KVM: X86: XCR0 should be set to the fixed value on vCPU reset
  2017-10-08  2:35 ` [PATCH 2/2] KVM: X86: XCR0 should be set to the fixed value on vCPU reset Wanpeng Li
  2017-10-08 22:36   ` Jim Mattson
@ 2017-10-09  8:04   ` Paolo Bonzini
  1 sibling, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2017-10-09  8:04 UTC (permalink / raw)
  To: Wanpeng Li, linux-kernel, kvm; +Cc: Radim Krčmář, Wanpeng Li

On 08/10/2017 04:35, Wanpeng Li wrote:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
> 
> SDM section 2.6 mentioned: 
> 
> After reset, all bits (except bit 0) in XCR0 are cleared to zero; XCR0[0] is set to 1.
> 
> This patch sets XCRO to the 0x1 after vCPU reset.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
>  arch/x86/kvm/x86.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index b0d2915..c784cd6 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -7816,6 +7816,8 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>  	vcpu->arch.regs_avail = ~0;
>  	vcpu->arch.regs_dirty = ~0;
>  
> +	vcpu->arch.xcr0 = XFEATURE_MASK_FP;

XCR0 is unchanged by INIT.  There are many other registers that are
zeroed by RESET by not INIT; KVM pretty much relies on userspace doing
that.  On the other hand, it's useful to zero here those registers that
are zeroed by INIT as well:

- XSS

- BNDCFGU, BND0-BND3, BNDCFGS (BNDCFGS probably would go in
vmx_vcpu_reset, the others are in vcpu->arch.guest_fpu.state.xsave).

>  	kvm_x86_ops->vcpu_reset(vcpu, init_event);
>  }
>  
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-10-09  8:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-08  2:35 [PATCH 1/2] KVM: VMX: Don't expose unrestricted_guest is enabled if ept is disabled Wanpeng Li
2017-10-08  2:35 ` [PATCH 2/2] KVM: X86: XCR0 should be set to the fixed value on vCPU reset Wanpeng Li
2017-10-08 22:36   ` Jim Mattson
2017-10-09  8:04   ` Paolo Bonzini
2017-10-08 22:35 ` [PATCH 1/2] KVM: VMX: Don't expose unrestricted_guest is enabled if ept is disabled Jim Mattson
2017-10-09  7:54   ` Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.