* [PATCH] KVM: vmx: hyper-v: don't pass EPT configuration info to vmx_hv_remote_flush_tlb()
@ 2018-10-11 10:03 Vitaly Kuznetsov
2018-10-11 12:07 ` Tianyu Lan
0 siblings, 1 reply; 7+ messages in thread
From: Vitaly Kuznetsov @ 2018-10-11 10:03 UTC (permalink / raw)
To: kvm
Cc: Paolo Bonzini, Radim Krčmář,
K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Michael Kelley (EOSG),
Tianyu Lan, linux-kernel
I'm observing random crashes in multi-vCPU L2 guests running on KVM on
Hyper-V. I bisected the issue to the commit 877ad952be3d ("KVM: vmx: Add
tlb_remote_flush callback support"). Hyper-V TLFS states:
"AddressSpace specifies an address space ID (an EPT PML4 table pointer)"
So apparently, Hyper-V doesn't expect us to pass naked EPTP, only PML4
pointer should be used. Strip off EPT configuration information before
calling into vmx_hv_remote_flush_tlb().
Fixes: 877ad952be3d ("KVM: vmx: Add tlb_remote_flush callback support")
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
arch/x86/kvm/vmx.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 612fd17be635..e665aa7167cf 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1572,8 +1572,12 @@ static int vmx_hv_remote_flush_tlb(struct kvm *kvm)
goto out;
}
+ /*
+ * FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE hypercall needs the address of the
+ * base of EPT PML4 table, strip off EPT configuration information.
+ */
ret = hyperv_flush_guest_mapping(
- to_vmx(kvm_get_vcpu(kvm, 0))->ept_pointer);
+ to_vmx(kvm_get_vcpu(kvm, 0))->ept_pointer & PAGE_MASK);
out:
spin_unlock(&to_kvm_vmx(kvm)->ept_pointer_lock);
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: vmx: hyper-v: don't pass EPT configuration info to vmx_hv_remote_flush_tlb()
2018-10-11 10:03 [PATCH] KVM: vmx: hyper-v: don't pass EPT configuration info to vmx_hv_remote_flush_tlb() Vitaly Kuznetsov
@ 2018-10-11 12:07 ` Tianyu Lan
2018-10-11 12:18 ` Vitaly Kuznetsov
0 siblings, 1 reply; 7+ messages in thread
From: Tianyu Lan @ 2018-10-11 12:07 UTC (permalink / raw)
To: vkuznets
Cc: kvm, Paolo Bonzini, Radim Krcmar, kys, haiyangz, sthemmin,
Michael.H.Kelley, Tianyu Lan, linux-kernel@vger kernel org
On Thu, Oct 11, 2018 at 6:32 PM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
> I'm observing random crashes in multi-vCPU L2 guests running on KVM on
> Hyper-V. I bisected the issue to the commit 877ad952be3d ("KVM: vmx: Add
> tlb_remote_flush callback support"). Hyper-V TLFS states:
>
> "AddressSpace specifies an address space ID (an EPT PML4 table pointer)"
>
> So apparently, Hyper-V doesn't expect us to pass naked EPTP, only PML4
> pointer should be used. Strip off EPT configuration information before
> calling into vmx_hv_remote_flush_tlb().
Hi Vitaly:
: Thanks to fix this. Sorry, I didn't meet the issue..
I think we may just store EPT PML4 table pointer without EPT
configuration information
in the ept_pointer field for this case.
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 619307b3e6bb..e316058b41a6 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -5379,7 +5379,7 @@ static void vmx_set_cr3(struct kvm_vcpu *vcpu,
unsigned long cr3)
if (kvm_x86_ops->tlb_remote_flush) {
spin_lock(&to_kvm_vmx(kvm)->ept_pointer_lock);
- to_vmx(vcpu)->ept_pointer = eptp;
+ to_vmx(vcpu)->ept_pointer = cr3;
to_kvm_vmx(kvm)->ept_pointers_match
= EPT_POINTERS_CHECK;
spin_unlock(&to_kvm_vmx(kvm)->ept_pointer_lock);
>
> Fixes: 877ad952be3d ("KVM: vmx: Add tlb_remote_flush callback support")
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> arch/x86/kvm/vmx.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 612fd17be635..e665aa7167cf 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -1572,8 +1572,12 @@ static int vmx_hv_remote_flush_tlb(struct kvm *kvm)
> goto out;
> }
>
> + /*
> + * FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE hypercall needs the address of the
> + * base of EPT PML4 table, strip off EPT configuration information.
> + */
> ret = hyperv_flush_guest_mapping(
> - to_vmx(kvm_get_vcpu(kvm, 0))->ept_pointer);
> + to_vmx(kvm_get_vcpu(kvm, 0))->ept_pointer & PAGE_MASK);
>
> out:
> spin_unlock(&to_kvm_vmx(kvm)->ept_pointer_lock);
> --
> 2.17.1
>
--
Best regards
Tianyu Lan
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: vmx: hyper-v: don't pass EPT configuration info to vmx_hv_remote_flush_tlb()
2018-10-11 12:07 ` Tianyu Lan
@ 2018-10-11 12:18 ` Vitaly Kuznetsov
2018-10-11 12:41 ` Tianyu Lan
0 siblings, 1 reply; 7+ messages in thread
From: Vitaly Kuznetsov @ 2018-10-11 12:18 UTC (permalink / raw)
To: Tianyu Lan
Cc: kvm, Paolo Bonzini, Radim Krcmar, kys, haiyangz, sthemmin,
Michael.H.Kelley, Tianyu Lan, linux-kernel@vger kernel org
Tianyu Lan <lantianyu1986@gmail.com> writes:
> On Thu, Oct 11, 2018 at 6:32 PM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>>
>> I'm observing random crashes in multi-vCPU L2 guests running on KVM on
>> Hyper-V. I bisected the issue to the commit 877ad952be3d ("KVM: vmx: Add
>> tlb_remote_flush callback support"). Hyper-V TLFS states:
>>
>> "AddressSpace specifies an address space ID (an EPT PML4 table pointer)"
>>
>> So apparently, Hyper-V doesn't expect us to pass naked EPTP, only PML4
>> pointer should be used. Strip off EPT configuration information before
>> calling into vmx_hv_remote_flush_tlb().
>
> Hi Vitaly:
> : Thanks to fix this. Sorry, I didn't meet the issue..
> I think we may just store EPT PML4 table pointer without EPT
> configuration information
> in the ept_pointer field for this case.
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 619307b3e6bb..e316058b41a6 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -5379,7 +5379,7 @@ static void vmx_set_cr3(struct kvm_vcpu *vcpu,
> unsigned long cr3)
>
> if (kvm_x86_ops->tlb_remote_flush) {
> spin_lock(&to_kvm_vmx(kvm)->ept_pointer_lock);
> - to_vmx(vcpu)->ept_pointer = eptp;
> + to_vmx(vcpu)->ept_pointer = cr3;
True, we can do that (and I even had a version of my patch doing so)
but 'ept_pointer' will likely need to be renamed as it's not obvious
why vcpu->ept_pointer != eptp.
Alternatively, we can filter lower 12 bits out in
hyperv_flush_guest_mapping() but I would rename 'as' parameter to eptp
then.
[snip]
--
Vitaly
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: vmx: hyper-v: don't pass EPT configuration info to vmx_hv_remote_flush_tlb()
2018-10-11 12:18 ` Vitaly Kuznetsov
@ 2018-10-11 12:41 ` Tianyu Lan
2018-10-11 13:00 ` Vitaly Kuznetsov
0 siblings, 1 reply; 7+ messages in thread
From: Tianyu Lan @ 2018-10-11 12:41 UTC (permalink / raw)
To: vkuznets
Cc: kvm, Paolo Bonzini, Radim Krcmar, kys, haiyangz, sthemmin,
Michael.H.Kelley, Tianyu Lan, linux-kernel@vger kernel org
On Thu, Oct 11, 2018 at 8:18 PM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
> Tianyu Lan <lantianyu1986@gmail.com> writes:
>
> > On Thu, Oct 11, 2018 at 6:32 PM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
> >>
> >> I'm observing random crashes in multi-vCPU L2 guests running on KVM on
> >> Hyper-V. I bisected the issue to the commit 877ad952be3d ("KVM: vmx: Add
> >> tlb_remote_flush callback support"). Hyper-V TLFS states:
> >>
> >> "AddressSpace specifies an address space ID (an EPT PML4 table pointer)"
> >>
> >> So apparently, Hyper-V doesn't expect us to pass naked EPTP, only PML4
> >> pointer should be used. Strip off EPT configuration information before
> >> calling into vmx_hv_remote_flush_tlb().
> >
> > Hi Vitaly:
> > : Thanks to fix this. Sorry, I didn't meet the issue..
> > I think we may just store EPT PML4 table pointer without EPT
> > configuration information
> > in the ept_pointer field for this case.
> >
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index 619307b3e6bb..e316058b41a6 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -5379,7 +5379,7 @@ static void vmx_set_cr3(struct kvm_vcpu *vcpu,
> > unsigned long cr3)
> >
> > if (kvm_x86_ops->tlb_remote_flush) {
> > spin_lock(&to_kvm_vmx(kvm)->ept_pointer_lock);
> > - to_vmx(vcpu)->ept_pointer = eptp;
> > + to_vmx(vcpu)->ept_pointer = cr3;
>
> True, we can do that (and I even had a version of my patch doing so)
> but 'ept_pointer' will likely need to be renamed as it's not obvious
> why vcpu->ept_pointer != eptp.
>
Yes. that need to rename ept_pointer.
> Alternatively, we can filter lower 12 bits out in
> hyperv_flush_guest_mapping() but I would rename 'as' parameter to eptp
> then.
OK. I got it. Thanks.
--
Best regards
Tianyu Lan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: vmx: hyper-v: don't pass EPT configuration info to vmx_hv_remote_flush_tlb()
2018-10-11 12:41 ` Tianyu Lan
@ 2018-10-11 13:00 ` Vitaly Kuznetsov
2018-10-11 13:12 ` Tianyu Lan
2018-10-13 9:39 ` Paolo Bonzini
0 siblings, 2 replies; 7+ messages in thread
From: Vitaly Kuznetsov @ 2018-10-11 13:00 UTC (permalink / raw)
To: Tianyu Lan
Cc: kvm, Paolo Bonzini, Radim Krcmar, kys, haiyangz, sthemmin,
Michael.H.Kelley, Tianyu Lan, linux-kernel@vger kernel org
Tianyu Lan <lantianyu1986@gmail.com> writes:
> On Thu, Oct 11, 2018 at 8:18 PM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>>
>> Tianyu Lan <lantianyu1986@gmail.com> writes:
>>
>> > On Thu, Oct 11, 2018 at 6:32 PM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>> >>
>> >> I'm observing random crashes in multi-vCPU L2 guests running on KVM on
>> >> Hyper-V. I bisected the issue to the commit 877ad952be3d ("KVM: vmx: Add
>> >> tlb_remote_flush callback support"). Hyper-V TLFS states:
>> >>
>> >> "AddressSpace specifies an address space ID (an EPT PML4 table pointer)"
>> >>
>> >> So apparently, Hyper-V doesn't expect us to pass naked EPTP, only PML4
>> >> pointer should be used. Strip off EPT configuration information before
>> >> calling into vmx_hv_remote_flush_tlb().
>> >
>> > Hi Vitaly:
>> > : Thanks to fix this. Sorry, I didn't meet the issue..
>> > I think we may just store EPT PML4 table pointer without EPT
>> > configuration information
>> > in the ept_pointer field for this case.
>> >
>> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> > index 619307b3e6bb..e316058b41a6 100644
>> > --- a/arch/x86/kvm/vmx.c
>> > +++ b/arch/x86/kvm/vmx.c
>> > @@ -5379,7 +5379,7 @@ static void vmx_set_cr3(struct kvm_vcpu *vcpu,
>> > unsigned long cr3)
>> >
>> > if (kvm_x86_ops->tlb_remote_flush) {
>> > spin_lock(&to_kvm_vmx(kvm)->ept_pointer_lock);
>> > - to_vmx(vcpu)->ept_pointer = eptp;
>> > + to_vmx(vcpu)->ept_pointer = cr3;
>>
>> True, we can do that (and I even had a version of my patch doing so)
>> but 'ept_pointer' will likely need to be renamed as it's not obvious
>> why vcpu->ept_pointer != eptp.
>>
>
> Yes. that need to rename ept_pointer.
>
Honestly, I would prefer to keep more information cached, e.g. if
someone needs EPT configuration data later he can easily get it from
ept_pointer and by putting raw cr3 there we'll just keep less.
But I don't have a strong opinion, I'll leave it up to the maintainers
to tell us how to proceed)
>> Alternatively, we can filter lower 12 bits out in
>> hyperv_flush_guest_mapping() but I would rename 'as' parameter to eptp
>> then.
>
> OK. I got it. Thanks.
--
Vitaly
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: vmx: hyper-v: don't pass EPT configuration info to vmx_hv_remote_flush_tlb()
2018-10-11 13:00 ` Vitaly Kuznetsov
@ 2018-10-11 13:12 ` Tianyu Lan
2018-10-13 9:39 ` Paolo Bonzini
1 sibling, 0 replies; 7+ messages in thread
From: Tianyu Lan @ 2018-10-11 13:12 UTC (permalink / raw)
To: vkuznets
Cc: kvm, Paolo Bonzini, Radim Krcmar, kys, haiyangz, sthemmin,
Michael.H.Kelley, Tianyu Lan, linux-kernel@vger kernel org
On Thu, Oct 11, 2018 at 9:00 PM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
> Tianyu Lan <lantianyu1986@gmail.com> writes:
>
> > On Thu, Oct 11, 2018 at 8:18 PM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
> >>
> >> Tianyu Lan <lantianyu1986@gmail.com> writes:
> >>
> >> > On Thu, Oct 11, 2018 at 6:32 PM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
> >> >>
> >> >> I'm observing random crashes in multi-vCPU L2 guests running on KVM on
> >> >> Hyper-V. I bisected the issue to the commit 877ad952be3d ("KVM: vmx: Add
> >> >> tlb_remote_flush callback support"). Hyper-V TLFS states:
> >> >>
> >> >> "AddressSpace specifies an address space ID (an EPT PML4 table pointer)"
> >> >>
> >> >> So apparently, Hyper-V doesn't expect us to pass naked EPTP, only PML4
> >> >> pointer should be used. Strip off EPT configuration information before
> >> >> calling into vmx_hv_remote_flush_tlb().
> >> >
> >> > Hi Vitaly:
> >> > : Thanks to fix this. Sorry, I didn't meet the issue..
> >> > I think we may just store EPT PML4 table pointer without EPT
> >> > configuration information
> >> > in the ept_pointer field for this case.
> >> >
> >> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> >> > index 619307b3e6bb..e316058b41a6 100644
> >> > --- a/arch/x86/kvm/vmx.c
> >> > +++ b/arch/x86/kvm/vmx.c
> >> > @@ -5379,7 +5379,7 @@ static void vmx_set_cr3(struct kvm_vcpu *vcpu,
> >> > unsigned long cr3)
> >> >
> >> > if (kvm_x86_ops->tlb_remote_flush) {
> >> > spin_lock(&to_kvm_vmx(kvm)->ept_pointer_lock);
> >> > - to_vmx(vcpu)->ept_pointer = eptp;
> >> > + to_vmx(vcpu)->ept_pointer = cr3;
> >>
> >> True, we can do that (and I even had a version of my patch doing so)
> >> but 'ept_pointer' will likely need to be renamed as it's not obvious
> >> why vcpu->ept_pointer != eptp.
> >>
> >
> > Yes. that need to rename ept_pointer.
> >
>
> Honestly, I would prefer to keep more information cached, e.g. if
> someone needs EPT configuration data later he can easily get it from
> ept_pointer and by putting raw cr3 there we'll just keep less.
>
Yes, that makes sense.
--
Best regards
Tianyu Lan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: vmx: hyper-v: don't pass EPT configuration info to vmx_hv_remote_flush_tlb()
2018-10-11 13:00 ` Vitaly Kuznetsov
2018-10-11 13:12 ` Tianyu Lan
@ 2018-10-13 9:39 ` Paolo Bonzini
1 sibling, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2018-10-13 9:39 UTC (permalink / raw)
To: Vitaly Kuznetsov, Tianyu Lan
Cc: kvm, Radim Krcmar, kys, haiyangz, sthemmin, Michael.H.Kelley,
Tianyu Lan, linux-kernel@vger kernel org
On 11/10/2018 15:00, Vitaly Kuznetsov wrote:
>> Yes. that need to rename ept_pointer.
>>
> Honestly, I would prefer to keep more information cached, e.g. if
> someone needs EPT configuration data later he can easily get it from
> ept_pointer and by putting raw cr3 there we'll just keep less.
>
> But I don't have a strong opinion, I'll leave it up to the maintainers
> to tell us how to proceed)
>
I have (re)applied your patch (I had queued it and tested it earlier,
now I've added it back).
Paolo
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-10-13 9:39 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-11 10:03 [PATCH] KVM: vmx: hyper-v: don't pass EPT configuration info to vmx_hv_remote_flush_tlb() Vitaly Kuznetsov
2018-10-11 12:07 ` Tianyu Lan
2018-10-11 12:18 ` Vitaly Kuznetsov
2018-10-11 12:41 ` Tianyu Lan
2018-10-11 13:00 ` Vitaly Kuznetsov
2018-10-11 13:12 ` Tianyu Lan
2018-10-13 9:39 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).