* [PATCH] KVM: nVMX: Fix L2 guest hang if shadow page tables on EPT
@ 2017-03-17 14:41 Wanpeng Li
2017-03-17 14:47 ` Paolo Bonzini
2017-03-17 17:28 ` Ladi Prosek
0 siblings, 2 replies; 8+ messages in thread
From: Wanpeng Li @ 2017-03-17 14:41 UTC (permalink / raw)
To: linux-kernel, kvm
Cc: Paolo Bonzini, Radim Krčmář, Wanpeng Li, Ladi Prosek
From: Wanpeng Li <wanpeng.li@hotmail.com>
The L2 guest hang if shadow page tables on EPT, the trace on L1 shows that
L2 kvm_exit reason EXCEPTION_NMI and page fault repeatedly:
qemu-system-x86-2821 [003] d..2 45.848814: kvm_entry: vcpu 0
qemu-system-x86-2821 [003] ...1 45.848827: kvm_exit: reason EXCEPTION_NMI rip 0xe05b info fe05b 80000b0e
qemu-system-x86-2821 [003] ...1 45.848827: kvm_page_fault: address fe05b error_code 14
Commit 7ca29de21362 (KVM: nVMX: fix CR3 load if L2 uses PAE paging and EPT)
prevents to load L2's PDPTRs according to dereferencing L2's CR3 since it is
uninitialized in real mode. Hyper-V L1 will emulate L2 real mode with PAE
paging and EPT enabled. However, there is a progress to switch from Legacy
mode's such-mode Protected mode to Long mode during system boot, the check
in nested_vmx_load_cr3() will prevent to load PDPTRs if it is still in
Protected mode w/ PAE paging and nested EPT/shadow page tables on EPT. Actually
the original commit should just intended to prevent to dereference L2's CR3
if the L1 hypervisor emulates L2's real mode through vm8086.
This patch fixes it by allowing load PDPTRs if PAE paing, EPT enabled and
!vm86_active.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Ladi Prosek <lprosek@redhat.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
arch/x86/kvm/vmx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index c664365..2b2a05f 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -9933,7 +9933,7 @@ static bool nested_cr3_valid(struct kvm_vcpu *vcpu, unsigned long val)
static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3, bool nested_ept,
u32 *entry_failure_code)
{
- if (cr3 != kvm_read_cr3(vcpu) || (!nested_ept && pdptrs_changed(vcpu))) {
+ if (cr3 != kvm_read_cr3(vcpu) || pdptrs_changed(vcpu)) {
if (!nested_cr3_valid(vcpu, cr3)) {
*entry_failure_code = ENTRY_FAIL_DEFAULT;
return 1;
@@ -9944,7 +9944,7 @@ static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3, bool ne
* must not be dereferenced.
*/
if (!is_long_mode(vcpu) && is_pae(vcpu) && is_paging(vcpu) &&
- !nested_ept) {
+ !(nested_ept && to_vmx(vcpu)->rmode.vm86_active)) {
if (!load_pdptrs(vcpu, vcpu->arch.walk_mmu, cr3)) {
*entry_failure_code = ENTRY_FAIL_PDPTE;
return 1;
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: nVMX: Fix L2 guest hang if shadow page tables on EPT
2017-03-17 14:41 [PATCH] KVM: nVMX: Fix L2 guest hang if shadow page tables on EPT Wanpeng Li
@ 2017-03-17 14:47 ` Paolo Bonzini
2017-03-17 17:28 ` Ladi Prosek
1 sibling, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2017-03-17 14:47 UTC (permalink / raw)
To: Wanpeng Li, linux-kernel, kvm
Cc: Radim Krčmář, Wanpeng Li, Ladi Prosek
On 17/03/2017 15:41, Wanpeng Li wrote:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
>
> The L2 guest hang if shadow page tables on EPT, the trace on L1 shows that
> L2 kvm_exit reason EXCEPTION_NMI and page fault repeatedly:
>
> qemu-system-x86-2821 [003] d..2 45.848814: kvm_entry: vcpu 0
> qemu-system-x86-2821 [003] ...1 45.848827: kvm_exit: reason EXCEPTION_NMI rip 0xe05b info fe05b 80000b0e
> qemu-system-x86-2821 [003] ...1 45.848827: kvm_page_fault: address fe05b error_code 14
>
> Commit 7ca29de21362 (KVM: nVMX: fix CR3 load if L2 uses PAE paging and EPT)
> prevents to load L2's PDPTRs according to dereferencing L2's CR3 since it is
> uninitialized in real mode. Hyper-V L1 will emulate L2 real mode with PAE
> paging and EPT enabled. However, there is a progress to switch from Legacy
> mode's such-mode Protected mode to Long mode during system boot, the check
> in nested_vmx_load_cr3() will prevent to load PDPTRs if it is still in
> Protected mode w/ PAE paging and nested EPT/shadow page tables on EPT. Actually
> the original commit should just intended to prevent to dereference L2's CR3
> if the L1 hypervisor emulates L2's real mode through vm8086.
>
> This patch fixes it by allowing load PDPTRs if PAE paing, EPT enabled and
> !vm86_active.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Ladi Prosek <lprosek@redhat.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
Please provide a testcase. I know this is a regression, but I'm not
going to merge the fix without a corresponding patch to kvm-unit-tests.
Paolo
> ---
> arch/x86/kvm/vmx.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index c664365..2b2a05f 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -9933,7 +9933,7 @@ static bool nested_cr3_valid(struct kvm_vcpu *vcpu, unsigned long val)
> static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3, bool nested_ept,
> u32 *entry_failure_code)
> {
> - if (cr3 != kvm_read_cr3(vcpu) || (!nested_ept && pdptrs_changed(vcpu))) {
> + if (cr3 != kvm_read_cr3(vcpu) || pdptrs_changed(vcpu)) {
> if (!nested_cr3_valid(vcpu, cr3)) {
> *entry_failure_code = ENTRY_FAIL_DEFAULT;
> return 1;
> @@ -9944,7 +9944,7 @@ static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3, bool ne
> * must not be dereferenced.
> */
> if (!is_long_mode(vcpu) && is_pae(vcpu) && is_paging(vcpu) &&
> - !nested_ept) {
> + !(nested_ept && to_vmx(vcpu)->rmode.vm86_active)) {
> if (!load_pdptrs(vcpu, vcpu->arch.walk_mmu, cr3)) {
> *entry_failure_code = ENTRY_FAIL_PDPTE;
> return 1;
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: nVMX: Fix L2 guest hang if shadow page tables on EPT
2017-03-17 14:41 [PATCH] KVM: nVMX: Fix L2 guest hang if shadow page tables on EPT Wanpeng Li
2017-03-17 14:47 ` Paolo Bonzini
@ 2017-03-17 17:28 ` Ladi Prosek
2017-03-17 17:33 ` Paolo Bonzini
2017-03-18 6:37 ` Wanpeng Li
1 sibling, 2 replies; 8+ messages in thread
From: Ladi Prosek @ 2017-03-17 17:28 UTC (permalink / raw)
To: Wanpeng Li
Cc: linux-kernel, KVM list, Paolo Bonzini,
Radim Krčmář,
Wanpeng Li
On Fri, Mar 17, 2017 at 3:41 PM, Wanpeng Li <kernellwp@gmail.com> wrote:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
>
> The L2 guest hang if shadow page tables on EPT, the trace on L1 shows that
> L2 kvm_exit reason EXCEPTION_NMI and page fault repeatedly:
>
> qemu-system-x86-2821 [003] d..2 45.848814: kvm_entry: vcpu 0
> qemu-system-x86-2821 [003] ...1 45.848827: kvm_exit: reason EXCEPTION_NMI rip 0xe05b info fe05b 80000b0e
> qemu-system-x86-2821 [003] ...1 45.848827: kvm_page_fault: address fe05b error_code 14
>
> Commit 7ca29de21362 (KVM: nVMX: fix CR3 load if L2 uses PAE paging and EPT)
> prevents to load L2's PDPTRs according to dereferencing L2's CR3 since it is
> uninitialized in real mode. Hyper-V L1 will emulate L2 real mode with PAE
> paging and EPT enabled. However, there is a progress to switch from Legacy
> mode's such-mode Protected mode to Long mode during system boot, the check
> in nested_vmx_load_cr3() will prevent to load PDPTRs if it is still in
> Protected mode w/ PAE paging and nested EPT/shadow page tables on EPT. Actually
> the original commit should just intended to prevent to dereference L2's CR3
> if the L1 hypervisor emulates L2's real mode through vm8086.
>
> This patch fixes it by allowing load PDPTRs if PAE paing, EPT enabled and
> !vm86_active.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Ladi Prosek <lprosek@redhat.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
> arch/x86/kvm/vmx.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index c664365..2b2a05f 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -9933,7 +9933,7 @@ static bool nested_cr3_valid(struct kvm_vcpu *vcpu, unsigned long val)
> static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3, bool nested_ept,
> u32 *entry_failure_code)
> {
> - if (cr3 != kvm_read_cr3(vcpu) || (!nested_ept && pdptrs_changed(vcpu))) {
> + if (cr3 != kvm_read_cr3(vcpu) || pdptrs_changed(vcpu)) {
> if (!nested_cr3_valid(vcpu, cr3)) {
> *entry_failure_code = ENTRY_FAIL_DEFAULT;
> return 1;
> @@ -9944,7 +9944,7 @@ static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3, bool ne
> * must not be dereferenced.
> */
> if (!is_long_mode(vcpu) && is_pae(vcpu) && is_paging(vcpu) &&
> - !nested_ept) {
> + !(nested_ept && to_vmx(vcpu)->rmode.vm86_active)) {
This change breaks Hyper-V on KVM. L2 hangs on start-up, same symptoms
as before 7ca29de21362.
I'll take a closer look next week. Is there an easy way for me to
reproduce the issue you're seeing?
> if (!load_pdptrs(vcpu, vcpu->arch.walk_mmu, cr3)) {
> *entry_failure_code = ENTRY_FAIL_PDPTE;
> return 1;
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: nVMX: Fix L2 guest hang if shadow page tables on EPT
2017-03-17 17:28 ` Ladi Prosek
@ 2017-03-17 17:33 ` Paolo Bonzini
2017-03-18 6:37 ` Wanpeng Li
1 sibling, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2017-03-17 17:33 UTC (permalink / raw)
To: Ladi Prosek, Wanpeng Li
Cc: linux-kernel, KVM list, Radim Krčmář, Wanpeng Li
On 17/03/2017 18:28, Ladi Prosek wrote:
> On Fri, Mar 17, 2017 at 3:41 PM, Wanpeng Li <kernellwp@gmail.com> wrote:
>> From: Wanpeng Li <wanpeng.li@hotmail.com>
>>
>> The L2 guest hang if shadow page tables on EPT, the trace on L1 shows that
>> L2 kvm_exit reason EXCEPTION_NMI and page fault repeatedly:
>>
>> qemu-system-x86-2821 [003] d..2 45.848814: kvm_entry: vcpu 0
>> qemu-system-x86-2821 [003] ...1 45.848827: kvm_exit: reason EXCEPTION_NMI rip 0xe05b info fe05b 80000b0e
>> qemu-system-x86-2821 [003] ...1 45.848827: kvm_page_fault: address fe05b error_code 14
>>
>> Commit 7ca29de21362 (KVM: nVMX: fix CR3 load if L2 uses PAE paging and EPT)
>> prevents to load L2's PDPTRs according to dereferencing L2's CR3 since it is
>> uninitialized in real mode. Hyper-V L1 will emulate L2 real mode with PAE
>> paging and EPT enabled. However, there is a progress to switch from Legacy
>> mode's such-mode Protected mode to Long mode during system boot, the check
>> in nested_vmx_load_cr3() will prevent to load PDPTRs if it is still in
>> Protected mode w/ PAE paging and nested EPT/shadow page tables on EPT. Actually
>> the original commit should just intended to prevent to dereference L2's CR3
>> if the L1 hypervisor emulates L2's real mode through vm8086.
>>
>> This patch fixes it by allowing load PDPTRs if PAE paing, EPT enabled and
>> !vm86_active.
>>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> Cc: Ladi Prosek <lprosek@redhat.com>
>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> ---
>> arch/x86/kvm/vmx.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index c664365..2b2a05f 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -9933,7 +9933,7 @@ static bool nested_cr3_valid(struct kvm_vcpu *vcpu, unsigned long val)
>> static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3, bool nested_ept,
>> u32 *entry_failure_code)
>> {
>> - if (cr3 != kvm_read_cr3(vcpu) || (!nested_ept && pdptrs_changed(vcpu))) {
>> + if (cr3 != kvm_read_cr3(vcpu) || pdptrs_changed(vcpu)) {
>> if (!nested_cr3_valid(vcpu, cr3)) {
>> *entry_failure_code = ENTRY_FAIL_DEFAULT;
>> return 1;
>> @@ -9944,7 +9944,7 @@ static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3, bool ne
>> * must not be dereferenced.
>> */
>> if (!is_long_mode(vcpu) && is_pae(vcpu) && is_paging(vcpu) &&
>> - !nested_ept) {
>> + !(nested_ept && to_vmx(vcpu)->rmode.vm86_active)) {
>
> This change breaks Hyper-V on KVM. L2 hangs on start-up, same symptoms
> as before 7ca29de21362.
Looks like we need _two_ testcases then... :)
Paolo
> I'll take a closer look next week. Is there an easy way for me to
> reproduce the issue you're seeing?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: nVMX: Fix L2 guest hang if shadow page tables on EPT
2017-03-17 17:28 ` Ladi Prosek
2017-03-17 17:33 ` Paolo Bonzini
@ 2017-03-18 6:37 ` Wanpeng Li
2017-03-22 12:00 ` Ladi Prosek
1 sibling, 1 reply; 8+ messages in thread
From: Wanpeng Li @ 2017-03-18 6:37 UTC (permalink / raw)
To: Ladi Prosek
Cc: linux-kernel, KVM list, Paolo Bonzini,
Radim Krčmář,
Wanpeng Li
2017-03-18 1:28 GMT+08:00 Ladi Prosek <lprosek@redhat.com>:
> On Fri, Mar 17, 2017 at 3:41 PM, Wanpeng Li <kernellwp@gmail.com> wrote:
>> From: Wanpeng Li <wanpeng.li@hotmail.com>
>>
>> The L2 guest hang if shadow page tables on EPT, the trace on L1 shows that
>> L2 kvm_exit reason EXCEPTION_NMI and page fault repeatedly:
>>
>> qemu-system-x86-2821 [003] d..2 45.848814: kvm_entry: vcpu 0
>> qemu-system-x86-2821 [003] ...1 45.848827: kvm_exit: reason EXCEPTION_NMI rip 0xe05b info fe05b 80000b0e
>> qemu-system-x86-2821 [003] ...1 45.848827: kvm_page_fault: address fe05b error_code 14
>>
>> Commit 7ca29de21362 (KVM: nVMX: fix CR3 load if L2 uses PAE paging and EPT)
>> prevents to load L2's PDPTRs according to dereferencing L2's CR3 since it is
>> uninitialized in real mode. Hyper-V L1 will emulate L2 real mode with PAE
>> paging and EPT enabled. However, there is a progress to switch from Legacy
>> mode's such-mode Protected mode to Long mode during system boot, the check
>> in nested_vmx_load_cr3() will prevent to load PDPTRs if it is still in
>> Protected mode w/ PAE paging and nested EPT/shadow page tables on EPT. Actually
>> the original commit should just intended to prevent to dereference L2's CR3
>> if the L1 hypervisor emulates L2's real mode through vm8086.
>>
>> This patch fixes it by allowing load PDPTRs if PAE paing, EPT enabled and
>> !vm86_active.
>>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> Cc: Ladi Prosek <lprosek@redhat.com>
>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> ---
>> arch/x86/kvm/vmx.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index c664365..2b2a05f 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -9933,7 +9933,7 @@ static bool nested_cr3_valid(struct kvm_vcpu *vcpu, unsigned long val)
>> static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3, bool nested_ept,
>> u32 *entry_failure_code)
>> {
>> - if (cr3 != kvm_read_cr3(vcpu) || (!nested_ept && pdptrs_changed(vcpu))) {
>> + if (cr3 != kvm_read_cr3(vcpu) || pdptrs_changed(vcpu)) {
>> if (!nested_cr3_valid(vcpu, cr3)) {
>> *entry_failure_code = ENTRY_FAIL_DEFAULT;
>> return 1;
>> @@ -9944,7 +9944,7 @@ static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3, bool ne
>> * must not be dereferenced.
>> */
>> if (!is_long_mode(vcpu) && is_pae(vcpu) && is_paging(vcpu) &&
>> - !nested_ept) {
>> + !(nested_ept && to_vmx(vcpu)->rmode.vm86_active)) {
>
> This change breaks Hyper-V on KVM. L2 hangs on start-up, same symptoms
> as before 7ca29de21362.
Hmm, I miss the function pdptrs_changed() will also dereference CR3.
How about something like this:
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index c664365..d7ebf03 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -9933,7 +9933,9 @@ static bool nested_cr3_valid(struct kvm_vcpu
*vcpu, unsigned long val)
static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long
cr3, bool nested_ept,
u32 *entry_failure_code)
{
- if (cr3 != kvm_read_cr3(vcpu) || (!nested_ept && pdptrs_changed(vcpu))) {
+ if (cr3 != kvm_read_cr3(vcpu) ||
+ (!(nested_ept && to_vmx(vcpu)->rmode.vm86_active) &&
+ pdptrs_changed(vcpu))) {
if (!nested_cr3_valid(vcpu, cr3)) {
*entry_failure_code = ENTRY_FAIL_DEFAULT;
return 1;
@@ -9944,7 +9946,7 @@ static int nested_vmx_load_cr3(struct kvm_vcpu
*vcpu, unsigned long cr3, bool ne
* must not be dereferenced.
*/
if (!is_long_mode(vcpu) && is_pae(vcpu) && is_paging(vcpu) &&
- !nested_ept) {
+ !(nested_ept && to_vmx(vcpu)->rmode.vm86_active)) {
if (!load_pdptrs(vcpu, vcpu->arch.walk_mmu, cr3)) {
*entry_failure_code = ENTRY_FAIL_PDPTE;
return 1;
>
> I'll take a closer look next week. Is there an easy way for me to
> reproduce the issue you're seeing?
L1 KVM w/ EPT = 0 and L0 KVM w/ EPT = 1.
Regards,
Wanpeng Li
>
>> if (!load_pdptrs(vcpu, vcpu->arch.walk_mmu, cr3)) {
>> *entry_failure_code = ENTRY_FAIL_PDPTE;
>> return 1;
>> --
>> 2.7.4
>>
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: nVMX: Fix L2 guest hang if shadow page tables on EPT
2017-03-18 6:37 ` Wanpeng Li
@ 2017-03-22 12:00 ` Ladi Prosek
2017-03-22 13:44 ` Wanpeng Li
0 siblings, 1 reply; 8+ messages in thread
From: Ladi Prosek @ 2017-03-22 12:00 UTC (permalink / raw)
To: Wanpeng Li
Cc: linux-kernel, KVM list, Paolo Bonzini,
Radim Krčmář,
Wanpeng Li
On Sat, Mar 18, 2017 at 7:37 AM, Wanpeng Li <kernellwp@gmail.com> wrote:
> 2017-03-18 1:28 GMT+08:00 Ladi Prosek <lprosek@redhat.com>:
>> On Fri, Mar 17, 2017 at 3:41 PM, Wanpeng Li <kernellwp@gmail.com> wrote:
>>> From: Wanpeng Li <wanpeng.li@hotmail.com>
>>>
>>> The L2 guest hang if shadow page tables on EPT, the trace on L1 shows that
>>> L2 kvm_exit reason EXCEPTION_NMI and page fault repeatedly:
>>>
>>> qemu-system-x86-2821 [003] d..2 45.848814: kvm_entry: vcpu 0
>>> qemu-system-x86-2821 [003] ...1 45.848827: kvm_exit: reason EXCEPTION_NMI rip 0xe05b info fe05b 80000b0e
>>> qemu-system-x86-2821 [003] ...1 45.848827: kvm_page_fault: address fe05b error_code 14
>>>
>>> Commit 7ca29de21362 (KVM: nVMX: fix CR3 load if L2 uses PAE paging and EPT)
>>> prevents to load L2's PDPTRs according to dereferencing L2's CR3 since it is
>>> uninitialized in real mode. Hyper-V L1 will emulate L2 real mode with PAE
>>> paging and EPT enabled. However, there is a progress to switch from Legacy
>>> mode's such-mode Protected mode to Long mode during system boot, the check
>>> in nested_vmx_load_cr3() will prevent to load PDPTRs if it is still in
>>> Protected mode w/ PAE paging and nested EPT/shadow page tables on EPT. Actually
>>> the original commit should just intended to prevent to dereference L2's CR3
>>> if the L1 hypervisor emulates L2's real mode through vm8086.
>>>
>>> This patch fixes it by allowing load PDPTRs if PAE paing, EPT enabled and
>>> !vm86_active.
>>>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>>> Cc: Ladi Prosek <lprosek@redhat.com>
>>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>>> ---
>>> arch/x86/kvm/vmx.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>> index c664365..2b2a05f 100644
>>> --- a/arch/x86/kvm/vmx.c
>>> +++ b/arch/x86/kvm/vmx.c
>>> @@ -9933,7 +9933,7 @@ static bool nested_cr3_valid(struct kvm_vcpu *vcpu, unsigned long val)
>>> static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3, bool nested_ept,
>>> u32 *entry_failure_code)
>>> {
>>> - if (cr3 != kvm_read_cr3(vcpu) || (!nested_ept && pdptrs_changed(vcpu))) {
>>> + if (cr3 != kvm_read_cr3(vcpu) || pdptrs_changed(vcpu)) {
>>> if (!nested_cr3_valid(vcpu, cr3)) {
>>> *entry_failure_code = ENTRY_FAIL_DEFAULT;
>>> return 1;
>>> @@ -9944,7 +9944,7 @@ static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3, bool ne
>>> * must not be dereferenced.
>>> */
>>> if (!is_long_mode(vcpu) && is_pae(vcpu) && is_paging(vcpu) &&
>>> - !nested_ept) {
>>> + !(nested_ept && to_vmx(vcpu)->rmode.vm86_active)) {
>>
>> This change breaks Hyper-V on KVM. L2 hangs on start-up, same symptoms
>> as before 7ca29de21362.
>
> Hmm, I miss the function pdptrs_changed() will also dereference CR3.
> How about something like this:
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index c664365..d7ebf03 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -9933,7 +9933,9 @@ static bool nested_cr3_valid(struct kvm_vcpu
> *vcpu, unsigned long val)
> static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long
> cr3, bool nested_ept,
> u32 *entry_failure_code)
> {
> - if (cr3 != kvm_read_cr3(vcpu) || (!nested_ept && pdptrs_changed(vcpu))) {
> + if (cr3 != kvm_read_cr3(vcpu) ||
> + (!(nested_ept && to_vmx(vcpu)->rmode.vm86_active) &&
> + pdptrs_changed(vcpu))) {
> if (!nested_cr3_valid(vcpu, cr3)) {
> *entry_failure_code = ENTRY_FAIL_DEFAULT;
> return 1;
> @@ -9944,7 +9946,7 @@ static int nested_vmx_load_cr3(struct kvm_vcpu
> *vcpu, unsigned long cr3, bool ne
> * must not be dereferenced.
> */
> if (!is_long_mode(vcpu) && is_pae(vcpu) && is_paging(vcpu) &&
> - !nested_ept) {
> + !(nested_ept && to_vmx(vcpu)->rmode.vm86_active)) {
> if (!load_pdptrs(vcpu, vcpu->arch.walk_mmu, cr3)) {
> *entry_failure_code = ENTRY_FAIL_PDPTE;
> return 1;
Still the same, Hyper-V is broken. The problem is not in real vs.
protected mode. The way nested_ept_enabled is computed is incorrect.
I can run both Hyper-V and KVM with EPT = 0 in L1 with this patch. Can
you please give it a try?
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 98e82ee..9145c94 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -10121,7 +10121,7 @@ static int prepare_vmcs02(struct kvm_vcpu
*vcpu, struct vmcs12 *vmcs12,
vmcs12->guest_intr_status);
}
- nested_ept_enabled = (exec_control &
SECONDARY_EXEC_ENABLE_EPT) != 0;
+ nested_ept_enabled =
(vmcs12->secondary_vm_exec_control & SECONDARY_EXEC_ENABLE_EPT) != 0;
/*
* Write an illegal value to APIC_ACCESS_ADDR. Later,
Thanks!
Ladi
>> I'll take a closer look next week. Is there an easy way for me to
>> reproduce the issue you're seeing?
>
> L1 KVM w/ EPT = 0 and L0 KVM w/ EPT = 1.
>
> Regards,
> Wanpeng Li
>
>>
>>> if (!load_pdptrs(vcpu, vcpu->arch.walk_mmu, cr3)) {
>>> *entry_failure_code = ENTRY_FAIL_PDPTE;
>>> return 1;
>>> --
>>> 2.7.4
>>>
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: nVMX: Fix L2 guest hang if shadow page tables on EPT
2017-03-22 12:00 ` Ladi Prosek
@ 2017-03-22 13:44 ` Wanpeng Li
2017-03-22 14:17 ` Paolo Bonzini
0 siblings, 1 reply; 8+ messages in thread
From: Wanpeng Li @ 2017-03-22 13:44 UTC (permalink / raw)
To: Ladi Prosek
Cc: linux-kernel, KVM list, Paolo Bonzini,
Radim Krčmář,
Wanpeng Li
2017-03-22 20:00 GMT+08:00 Ladi Prosek <lprosek@redhat.com>:
> On Sat, Mar 18, 2017 at 7:37 AM, Wanpeng Li <kernellwp@gmail.com> wrote:
>> 2017-03-18 1:28 GMT+08:00 Ladi Prosek <lprosek@redhat.com>:
>>> On Fri, Mar 17, 2017 at 3:41 PM, Wanpeng Li <kernellwp@gmail.com> wrote:
>>>> From: Wanpeng Li <wanpeng.li@hotmail.com>
>>>>
>>>> The L2 guest hang if shadow page tables on EPT, the trace on L1 shows that
>>>> L2 kvm_exit reason EXCEPTION_NMI and page fault repeatedly:
>>>>
>>>> qemu-system-x86-2821 [003] d..2 45.848814: kvm_entry: vcpu 0
>>>> qemu-system-x86-2821 [003] ...1 45.848827: kvm_exit: reason EXCEPTION_NMI rip 0xe05b info fe05b 80000b0e
>>>> qemu-system-x86-2821 [003] ...1 45.848827: kvm_page_fault: address fe05b error_code 14
>>>>
>>>> Commit 7ca29de21362 (KVM: nVMX: fix CR3 load if L2 uses PAE paging and EPT)
>>>> prevents to load L2's PDPTRs according to dereferencing L2's CR3 since it is
>>>> uninitialized in real mode. Hyper-V L1 will emulate L2 real mode with PAE
>>>> paging and EPT enabled. However, there is a progress to switch from Legacy
>>>> mode's such-mode Protected mode to Long mode during system boot, the check
>>>> in nested_vmx_load_cr3() will prevent to load PDPTRs if it is still in
>>>> Protected mode w/ PAE paging and nested EPT/shadow page tables on EPT. Actually
>>>> the original commit should just intended to prevent to dereference L2's CR3
>>>> if the L1 hypervisor emulates L2's real mode through vm8086.
>>>>
>>>> This patch fixes it by allowing load PDPTRs if PAE paing, EPT enabled and
>>>> !vm86_active.
>>>>
>>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>>>> Cc: Ladi Prosek <lprosek@redhat.com>
>>>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>>>> ---
>>>> arch/x86/kvm/vmx.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>>> index c664365..2b2a05f 100644
>>>> --- a/arch/x86/kvm/vmx.c
>>>> +++ b/arch/x86/kvm/vmx.c
>>>> @@ -9933,7 +9933,7 @@ static bool nested_cr3_valid(struct kvm_vcpu *vcpu, unsigned long val)
>>>> static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3, bool nested_ept,
>>>> u32 *entry_failure_code)
>>>> {
>>>> - if (cr3 != kvm_read_cr3(vcpu) || (!nested_ept && pdptrs_changed(vcpu))) {
>>>> + if (cr3 != kvm_read_cr3(vcpu) || pdptrs_changed(vcpu)) {
>>>> if (!nested_cr3_valid(vcpu, cr3)) {
>>>> *entry_failure_code = ENTRY_FAIL_DEFAULT;
>>>> return 1;
>>>> @@ -9944,7 +9944,7 @@ static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3, bool ne
>>>> * must not be dereferenced.
>>>> */
>>>> if (!is_long_mode(vcpu) && is_pae(vcpu) && is_paging(vcpu) &&
>>>> - !nested_ept) {
>>>> + !(nested_ept && to_vmx(vcpu)->rmode.vm86_active)) {
>>>
>>> This change breaks Hyper-V on KVM. L2 hangs on start-up, same symptoms
>>> as before 7ca29de21362.
>>
>> Hmm, I miss the function pdptrs_changed() will also dereference CR3.
>> How about something like this:
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index c664365..d7ebf03 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -9933,7 +9933,9 @@ static bool nested_cr3_valid(struct kvm_vcpu
>> *vcpu, unsigned long val)
>> static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long
>> cr3, bool nested_ept,
>> u32 *entry_failure_code)
>> {
>> - if (cr3 != kvm_read_cr3(vcpu) || (!nested_ept && pdptrs_changed(vcpu))) {
>> + if (cr3 != kvm_read_cr3(vcpu) ||
>> + (!(nested_ept && to_vmx(vcpu)->rmode.vm86_active) &&
>> + pdptrs_changed(vcpu))) {
>> if (!nested_cr3_valid(vcpu, cr3)) {
>> *entry_failure_code = ENTRY_FAIL_DEFAULT;
>> return 1;
>> @@ -9944,7 +9946,7 @@ static int nested_vmx_load_cr3(struct kvm_vcpu
>> *vcpu, unsigned long cr3, bool ne
>> * must not be dereferenced.
>> */
>> if (!is_long_mode(vcpu) && is_pae(vcpu) && is_paging(vcpu) &&
>> - !nested_ept) {
>> + !(nested_ept && to_vmx(vcpu)->rmode.vm86_active)) {
>> if (!load_pdptrs(vcpu, vcpu->arch.walk_mmu, cr3)) {
>> *entry_failure_code = ENTRY_FAIL_PDPTE;
>> return 1;
>
> Still the same, Hyper-V is broken. The problem is not in real vs.
> protected mode. The way nested_ept_enabled is computed is incorrect.
>
> I can run both Hyper-V and KVM with EPT = 0 in L1 with this patch. Can
> you please give it a try?
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 98e82ee..9145c94 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -10121,7 +10121,7 @@ static int prepare_vmcs02(struct kvm_vcpu
> *vcpu, struct vmcs12 *vmcs12,
> vmcs12->guest_intr_status);
> }
>
> - nested_ept_enabled = (exec_control &
> SECONDARY_EXEC_ENABLE_EPT) != 0;
> + nested_ept_enabled =
> (vmcs12->secondary_vm_exec_control & SECONDARY_EXEC_ENABLE_EPT) != 0;
>
> /*
> * Write an illegal value to APIC_ACCESS_ADDR. Later,
You are right, it works. Please send out a formal patch and add the
kvm-unit-tests as Paolo mentioned.
Regards,
Wanpeng Li
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: nVMX: Fix L2 guest hang if shadow page tables on EPT
2017-03-22 13:44 ` Wanpeng Li
@ 2017-03-22 14:17 ` Paolo Bonzini
0 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2017-03-22 14:17 UTC (permalink / raw)
To: Wanpeng Li, Ladi Prosek
Cc: linux-kernel, KVM list, Radim Krčmář, Wanpeng Li
On 22/03/2017 14:44, Wanpeng Li wrote:
> 2017-03-22 20:00 GMT+08:00 Ladi Prosek <lprosek@redhat.com>:
>> On Sat, Mar 18, 2017 at 7:37 AM, Wanpeng Li <kernellwp@gmail.com> wrote:
>>> 2017-03-18 1:28 GMT+08:00 Ladi Prosek <lprosek@redhat.com>:
>>>> On Fri, Mar 17, 2017 at 3:41 PM, Wanpeng Li <kernellwp@gmail.com> wrote:
>>>>> From: Wanpeng Li <wanpeng.li@hotmail.com>
>>>>>
>>>>> The L2 guest hang if shadow page tables on EPT, the trace on L1 shows that
>>>>> L2 kvm_exit reason EXCEPTION_NMI and page fault repeatedly:
>>>>>
>>>>> qemu-system-x86-2821 [003] d..2 45.848814: kvm_entry: vcpu 0
>>>>> qemu-system-x86-2821 [003] ...1 45.848827: kvm_exit: reason EXCEPTION_NMI rip 0xe05b info fe05b 80000b0e
>>>>> qemu-system-x86-2821 [003] ...1 45.848827: kvm_page_fault: address fe05b error_code 14
>>>>>
>>>>> Commit 7ca29de21362 (KVM: nVMX: fix CR3 load if L2 uses PAE paging and EPT)
>>>>> prevents to load L2's PDPTRs according to dereferencing L2's CR3 since it is
>>>>> uninitialized in real mode. Hyper-V L1 will emulate L2 real mode with PAE
>>>>> paging and EPT enabled. However, there is a progress to switch from Legacy
>>>>> mode's such-mode Protected mode to Long mode during system boot, the check
>>>>> in nested_vmx_load_cr3() will prevent to load PDPTRs if it is still in
>>>>> Protected mode w/ PAE paging and nested EPT/shadow page tables on EPT. Actually
>>>>> the original commit should just intended to prevent to dereference L2's CR3
>>>>> if the L1 hypervisor emulates L2's real mode through vm8086.
>>>>>
>>>>> This patch fixes it by allowing load PDPTRs if PAE paing, EPT enabled and
>>>>> !vm86_active.
>>>>>
>>>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>>>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>>>>> Cc: Ladi Prosek <lprosek@redhat.com>
>>>>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>>>>> ---
>>>>> arch/x86/kvm/vmx.c | 4 ++--
>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>>>> index c664365..2b2a05f 100644
>>>>> --- a/arch/x86/kvm/vmx.c
>>>>> +++ b/arch/x86/kvm/vmx.c
>>>>> @@ -9933,7 +9933,7 @@ static bool nested_cr3_valid(struct kvm_vcpu *vcpu, unsigned long val)
>>>>> static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3, bool nested_ept,
>>>>> u32 *entry_failure_code)
>>>>> {
>>>>> - if (cr3 != kvm_read_cr3(vcpu) || (!nested_ept && pdptrs_changed(vcpu))) {
>>>>> + if (cr3 != kvm_read_cr3(vcpu) || pdptrs_changed(vcpu)) {
>>>>> if (!nested_cr3_valid(vcpu, cr3)) {
>>>>> *entry_failure_code = ENTRY_FAIL_DEFAULT;
>>>>> return 1;
>>>>> @@ -9944,7 +9944,7 @@ static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3, bool ne
>>>>> * must not be dereferenced.
>>>>> */
>>>>> if (!is_long_mode(vcpu) && is_pae(vcpu) && is_paging(vcpu) &&
>>>>> - !nested_ept) {
>>>>> + !(nested_ept && to_vmx(vcpu)->rmode.vm86_active)) {
>>>>
>>>> This change breaks Hyper-V on KVM. L2 hangs on start-up, same symptoms
>>>> as before 7ca29de21362.
>>>
>>> Hmm, I miss the function pdptrs_changed() will also dereference CR3.
>>> How about something like this:
>>>
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>> index c664365..d7ebf03 100644
>>> --- a/arch/x86/kvm/vmx.c
>>> +++ b/arch/x86/kvm/vmx.c
>>> @@ -9933,7 +9933,9 @@ static bool nested_cr3_valid(struct kvm_vcpu
>>> *vcpu, unsigned long val)
>>> static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long
>>> cr3, bool nested_ept,
>>> u32 *entry_failure_code)
>>> {
>>> - if (cr3 != kvm_read_cr3(vcpu) || (!nested_ept && pdptrs_changed(vcpu))) {
>>> + if (cr3 != kvm_read_cr3(vcpu) ||
>>> + (!(nested_ept && to_vmx(vcpu)->rmode.vm86_active) &&
>>> + pdptrs_changed(vcpu))) {
>>> if (!nested_cr3_valid(vcpu, cr3)) {
>>> *entry_failure_code = ENTRY_FAIL_DEFAULT;
>>> return 1;
>>> @@ -9944,7 +9946,7 @@ static int nested_vmx_load_cr3(struct kvm_vcpu
>>> *vcpu, unsigned long cr3, bool ne
>>> * must not be dereferenced.
>>> */
>>> if (!is_long_mode(vcpu) && is_pae(vcpu) && is_paging(vcpu) &&
>>> - !nested_ept) {
>>> + !(nested_ept && to_vmx(vcpu)->rmode.vm86_active)) {
>>> if (!load_pdptrs(vcpu, vcpu->arch.walk_mmu, cr3)) {
>>> *entry_failure_code = ENTRY_FAIL_PDPTE;
>>> return 1;
>>
>> Still the same, Hyper-V is broken. The problem is not in real vs.
>> protected mode. The way nested_ept_enabled is computed is incorrect.
>>
>> I can run both Hyper-V and KVM with EPT = 0 in L1 with this patch. Can
>> you please give it a try?
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 98e82ee..9145c94 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -10121,7 +10121,7 @@ static int prepare_vmcs02(struct kvm_vcpu
>> *vcpu, struct vmcs12 *vmcs12,
>> vmcs12->guest_intr_status);
>> }
>>
>> - nested_ept_enabled = (exec_control &
>> SECONDARY_EXEC_ENABLE_EPT) != 0;
>> + nested_ept_enabled =
>> (vmcs12->secondary_vm_exec_control & SECONDARY_EXEC_ENABLE_EPT) != 0;
>>
>> /*
>> * Write an illegal value to APIC_ACCESS_ADDR. Later,
>
> You are right, it works. Please send out a formal patch and add the
> kvm-unit-tests as Paolo mentioned.
I do believe it would be more fair if _you_ send out the kvm-unit-tests
patch.
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-03-22 14:17 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-17 14:41 [PATCH] KVM: nVMX: Fix L2 guest hang if shadow page tables on EPT Wanpeng Li
2017-03-17 14:47 ` Paolo Bonzini
2017-03-17 17:28 ` Ladi Prosek
2017-03-17 17:33 ` Paolo Bonzini
2017-03-18 6:37 ` Wanpeng Li
2017-03-22 12:00 ` Ladi Prosek
2017-03-22 13:44 ` Wanpeng Li
2017-03-22 14:17 ` 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.