* [PATCH] KVM: x86: Fix the intel_pt PMI handling wrongly considered from guest
@ 2022-05-15 17:16 Yanfei Xu
2022-05-16 13:58 ` Sean Christopherson
0 siblings, 1 reply; 6+ messages in thread
From: Yanfei Xu @ 2022-05-15 17:16 UTC (permalink / raw)
To: pbonzini, seanjc, vkuznets, wanpengli, jmattson, joro, tglx,
mingo, bp, dave.hansen
Cc: x86, kvm, linux-kernel
When kernel handles the vm-exit caused by external interrupts and PMI,
it always set a type of kvm_intr_type to handling_intr_from_guest to
tell if it's dealing an IRQ or NMI.
However, the further type judgment is missing in kvm_arch_pmi_in_guest().
It could make the PMI of intel_pt wrongly considered it comes from a
guest once the PMI breaks the handling of vm-exit of external interrupts.
Fixes: db215756ae59 ("KVM: x86: More precisely identify NMI from guest when handling PMI")
Signed-off-by: Yanfei Xu <yanfei.xu@intel.com>
---
arch/x86/include/asm/kvm_host.h | 8 +++++++-
arch/x86/kvm/x86.h | 6 ------
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 4ff36610af6a..308cf19f123d 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1582,8 +1582,14 @@ static inline int kvm_arch_flush_remote_tlb(struct kvm *kvm)
return -ENOTSUPP;
}
+enum kvm_intr_type {
+ /* Values are arbitrary, but must be non-zero. */
+ KVM_HANDLING_IRQ = 1,
+ KVM_HANDLING_NMI,
+};
+
#define kvm_arch_pmi_in_guest(vcpu) \
- ((vcpu) && (vcpu)->arch.handling_intr_from_guest)
+ ((vcpu) && (vcpu)->arch.handling_intr_from_guest == KVM_HANDLING_NMI)
void kvm_mmu_x86_module_init(void);
int kvm_mmu_vendor_module_init(void);
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 588792f00334..3bdf1bc76863 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -344,12 +344,6 @@ static inline bool kvm_cstate_in_guest(struct kvm *kvm)
return kvm->arch.cstate_in_guest;
}
-enum kvm_intr_type {
- /* Values are arbitrary, but must be non-zero. */
- KVM_HANDLING_IRQ = 1,
- KVM_HANDLING_NMI,
-};
-
static inline void kvm_before_interrupt(struct kvm_vcpu *vcpu,
enum kvm_intr_type intr)
{
--
2.32.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: Fix the intel_pt PMI handling wrongly considered from guest
2022-05-15 17:16 [PATCH] KVM: x86: Fix the intel_pt PMI handling wrongly considered from guest Yanfei Xu
@ 2022-05-16 13:58 ` Sean Christopherson
2022-05-18 10:01 ` Yanfei Xu
2022-05-20 9:54 ` Xu, Yanfei
0 siblings, 2 replies; 6+ messages in thread
From: Sean Christopherson @ 2022-05-16 13:58 UTC (permalink / raw)
To: Yanfei Xu
Cc: pbonzini, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp,
dave.hansen, x86, kvm, linux-kernel
On Mon, May 16, 2022, Yanfei Xu wrote:
> When kernel handles the vm-exit caused by external interrupts and PMI,
> it always set a type of kvm_intr_type to handling_intr_from_guest to
> tell if it's dealing an IRQ or NMI.
> However, the further type judgment is missing in kvm_arch_pmi_in_guest().
> It could make the PMI of intel_pt wrongly considered it comes from a
> guest once the PMI breaks the handling of vm-exit of external interrupts.
>
> Fixes: db215756ae59 ("KVM: x86: More precisely identify NMI from guest when handling PMI")
> Signed-off-by: Yanfei Xu <yanfei.xu@intel.com>
> ---
> arch/x86/include/asm/kvm_host.h | 8 +++++++-
> arch/x86/kvm/x86.h | 6 ------
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 4ff36610af6a..308cf19f123d 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1582,8 +1582,14 @@ static inline int kvm_arch_flush_remote_tlb(struct kvm *kvm)
> return -ENOTSUPP;
> }
>
> +enum kvm_intr_type {
> + /* Values are arbitrary, but must be non-zero. */
> + KVM_HANDLING_IRQ = 1,
> + KVM_HANDLING_NMI,
> +};
> +
> #define kvm_arch_pmi_in_guest(vcpu) \
> - ((vcpu) && (vcpu)->arch.handling_intr_from_guest)
> + ((vcpu) && (vcpu)->arch.handling_intr_from_guest == KVM_HANDLING_NMI)
My understanding is that this isn't correct as a general change, as perf events
can use regular IRQs in some cases. See commit dd60d217062f4 ("KVM: x86: Fix perf
timer mode IP reporting").
I assume there's got to be a way to know which mode perf is using, e.g. we should
be able to make this look something like:
((vcpu) && (vcpu)->arch.handling_intr_from_guest == kvm_pmi_vector)
> void kvm_mmu_x86_module_init(void);
> int kvm_mmu_vendor_module_init(void);
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index 588792f00334..3bdf1bc76863 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -344,12 +344,6 @@ static inline bool kvm_cstate_in_guest(struct kvm *kvm)
> return kvm->arch.cstate_in_guest;
> }
>
> -enum kvm_intr_type {
> - /* Values are arbitrary, but must be non-zero. */
> - KVM_HANDLING_IRQ = 1,
> - KVM_HANDLING_NMI,
> -};
> -
> static inline void kvm_before_interrupt(struct kvm_vcpu *vcpu,
> enum kvm_intr_type intr)
> {
> --
> 2.32.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: Fix the intel_pt PMI handling wrongly considered from guest
2022-05-16 13:58 ` Sean Christopherson
@ 2022-05-18 10:01 ` Yanfei Xu
2022-05-20 9:54 ` Xu, Yanfei
1 sibling, 0 replies; 6+ messages in thread
From: Yanfei Xu @ 2022-05-18 10:01 UTC (permalink / raw)
To: Sean Christopherson
Cc: pbonzini, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp,
dave.hansen, x86, kvm, linux-kernel
On 2022/5/16 21:58, Sean Christopherson wrote:
> On Mon, May 16, 2022, Yanfei Xu wrote:
>> When kernel handles the vm-exit caused by external interrupts and PMI,
>> it always set a type of kvm_intr_type to handling_intr_from_guest to
>> tell if it's dealing an IRQ or NMI.
>> However, the further type judgment is missing in kvm_arch_pmi_in_guest().
>> It could make the PMI of intel_pt wrongly considered it comes from a
>> guest once the PMI breaks the handling of vm-exit of external interrupts.
>>
>> Fixes: db215756ae59 ("KVM: x86: More precisely identify NMI from guest when handling PMI")
>> Signed-off-by: Yanfei Xu <yanfei.xu@intel.com>
>> ---
>> arch/x86/include/asm/kvm_host.h | 8 +++++++-
>> arch/x86/kvm/x86.h | 6 ------
>> 2 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index 4ff36610af6a..308cf19f123d 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -1582,8 +1582,14 @@ static inline int kvm_arch_flush_remote_tlb(struct kvm *kvm)
>> return -ENOTSUPP;
>> }
>>
>> +enum kvm_intr_type {
>> + /* Values are arbitrary, but must be non-zero. */
>> + KVM_HANDLING_IRQ = 1,
>> + KVM_HANDLING_NMI,
>> +};
>> +
>> #define kvm_arch_pmi_in_guest(vcpu) \
>> - ((vcpu) && (vcpu)->arch.handling_intr_from_guest)
>> + ((vcpu) && (vcpu)->arch.handling_intr_from_guest == KVM_HANDLING_NMI)
> My understanding is that this isn't correct as a general change, as perf events
> can use regular IRQs in some cases. See commit dd60d217062f4 ("KVM: x86: Fix perf
> timer mode IP reporting").
>
> I assume there's got to be a way to know which mode perf is using, e.g. we should
> be able to make this look something like:
>
> ((vcpu) && (vcpu)->arch.handling_intr_from_guest == kvm_pmi_vector)
Thanks for your comments, I am going to understand these clearly and
then reply or give a next version.
Regards,
Yanfei
>
>> void kvm_mmu_x86_module_init(void);
>> int kvm_mmu_vendor_module_init(void);
>> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
>> index 588792f00334..3bdf1bc76863 100644
>> --- a/arch/x86/kvm/x86.h
>> +++ b/arch/x86/kvm/x86.h
>> @@ -344,12 +344,6 @@ static inline bool kvm_cstate_in_guest(struct kvm *kvm)
>> return kvm->arch.cstate_in_guest;
>> }
>>
>> -enum kvm_intr_type {
>> - /* Values are arbitrary, but must be non-zero. */
>> - KVM_HANDLING_IRQ = 1,
>> - KVM_HANDLING_NMI,
>> -};
>> -
>> static inline void kvm_before_interrupt(struct kvm_vcpu *vcpu,
>> enum kvm_intr_type intr)
>> {
>> --
>> 2.32.0
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] KVM: x86: Fix the intel_pt PMI handling wrongly considered from guest
2022-05-16 13:58 ` Sean Christopherson
2022-05-18 10:01 ` Yanfei Xu
@ 2022-05-20 9:54 ` Xu, Yanfei
2022-05-20 16:31 ` Sean Christopherson
1 sibling, 1 reply; 6+ messages in thread
From: Xu, Yanfei @ 2022-05-20 9:54 UTC (permalink / raw)
To: Christopherson,, Sean
Cc: pbonzini, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp,
dave.hansen, x86, kvm, linux-kernel, Wang, Wei W, Liang, Kan
Hi Sean,
You are right, the change of kvm_arch_pmi_in_guest is incorrect, because it should cover two cases of PMI.
For the PMI of intel pt, it certainly is the NMI PMI. So how about fixing it like below?
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 610355b9ccce..378036c1cf94 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7856,7 +7856,7 @@ static unsigned int vmx_handle_intel_pt_intr(void)
struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
/* '0' on failure so that the !PT case can use a RET0 static call. */
- if (!kvm_arch_pmi_in_guest(vcpu))
+ if (!kvm_handling_nmi_from_guest(vcpu))
return 0;
kvm_make_request(KVM_REQ_PMI, vcpu);
Thanks,
Yanfei
-----Original Message-----
From: Sean Christopherson <seanjc@google.com>
Sent: Monday, May 16, 2022 9:58 PM
To: Xu, Yanfei <yanfei.xu@intel.com>
Cc: pbonzini@redhat.com; vkuznets@redhat.com; wanpengli@tencent.com; jmattson@google.com; joro@8bytes.org; tglx@linutronix.de; mingo@redhat.com; bp@alien8.de; dave.hansen@linux.intel.com; x86@kernel.org; kvm@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] KVM: x86: Fix the intel_pt PMI handling wrongly considered from guest
On Mon, May 16, 2022, Yanfei Xu wrote:
> When kernel handles the vm-exit caused by external interrupts and PMI,
> it always set a type of kvm_intr_type to handling_intr_from_guest to
> tell if it's dealing an IRQ or NMI.
> However, the further type judgment is missing in kvm_arch_pmi_in_guest().
> It could make the PMI of intel_pt wrongly considered it comes from a
> guest once the PMI breaks the handling of vm-exit of external interrupts.
>
> Fixes: db215756ae59 ("KVM: x86: More precisely identify NMI from guest
> when handling PMI")
> Signed-off-by: Yanfei Xu <yanfei.xu@intel.com>
> ---
> arch/x86/include/asm/kvm_host.h | 8 +++++++-
> arch/x86/kvm/x86.h | 6 ------
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h
> b/arch/x86/include/asm/kvm_host.h index 4ff36610af6a..308cf19f123d
> 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1582,8 +1582,14 @@ static inline int kvm_arch_flush_remote_tlb(struct kvm *kvm)
> return -ENOTSUPP;
> }
>
> +enum kvm_intr_type {
> + /* Values are arbitrary, but must be non-zero. */
> + KVM_HANDLING_IRQ = 1,
> + KVM_HANDLING_NMI,
> +};
> +
> #define kvm_arch_pmi_in_guest(vcpu) \
> - ((vcpu) && (vcpu)->arch.handling_intr_from_guest)
> + ((vcpu) && (vcpu)->arch.handling_intr_from_guest ==
> +KVM_HANDLING_NMI)
My understanding is that this isn't correct as a general change, as perf events can use regular IRQs in some cases. See commit dd60d217062f4 ("KVM: x86: Fix perf timer mode IP reporting").
I assume there's got to be a way to know which mode perf is using, e.g. we should be able to make this look something like:
((vcpu) && (vcpu)->arch.handling_intr_from_guest == kvm_pmi_vector)
> void kvm_mmu_x86_module_init(void);
> int kvm_mmu_vendor_module_init(void); diff --git a/arch/x86/kvm/x86.h
> b/arch/x86/kvm/x86.h index 588792f00334..3bdf1bc76863 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -344,12 +344,6 @@ static inline bool kvm_cstate_in_guest(struct kvm *kvm)
> return kvm->arch.cstate_in_guest;
> }
>
> -enum kvm_intr_type {
> - /* Values are arbitrary, but must be non-zero. */
> - KVM_HANDLING_IRQ = 1,
> - KVM_HANDLING_NMI,
> -};
> -
> static inline void kvm_before_interrupt(struct kvm_vcpu *vcpu,
> enum kvm_intr_type intr)
> {
> --
> 2.32.0
>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: Fix the intel_pt PMI handling wrongly considered from guest
2022-05-20 9:54 ` Xu, Yanfei
@ 2022-05-20 16:31 ` Sean Christopherson
2022-05-21 4:31 ` Xu, Yanfei
0 siblings, 1 reply; 6+ messages in thread
From: Sean Christopherson @ 2022-05-20 16:31 UTC (permalink / raw)
To: Xu, Yanfei
Cc: pbonzini, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp,
dave.hansen, x86, kvm, linux-kernel, Wang, Wei W, Liang, Kan
Please don't top-post.
On Fri, May 20, 2022, Xu, Yanfei wrote:
> From: Sean Christopherson <seanjc@google.com>
> On Mon, May 16, 2022, Yanfei Xu wrote:
> > diff --git a/arch/x86/include/asm/kvm_host.h
> > b/arch/x86/include/asm/kvm_host.h index 4ff36610af6a..308cf19f123d
> > 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -1582,8 +1582,14 @@ static inline int kvm_arch_flush_remote_tlb(struct kvm *kvm)
> > return -ENOTSUPP;
> > }
> >
> > +enum kvm_intr_type {
> > + /* Values are arbitrary, but must be non-zero. */
> > + KVM_HANDLING_IRQ = 1,
> > + KVM_HANDLING_NMI,
> > +};
> > +
> > #define kvm_arch_pmi_in_guest(vcpu) \
> > - ((vcpu) && (vcpu)->arch.handling_intr_from_guest)
> > + ((vcpu) && (vcpu)->arch.handling_intr_from_guest ==
> > +KVM_HANDLING_NMI)
>
> My understanding is that this isn't correct as a general change, as perf
> events can use regular IRQs in some cases. See commit dd60d217062f4 ("KVM:
> x86: Fix perf timer mode IP reporting").
>
> I assume there's got to be a way to know which mode perf is using, e.g. we
> should be able to make this look something like:
>
> ((vcpu) && (vcpu)->arch.handling_intr_from_guest == kvm_pmi_vector)
> Hi Sean,
> You are right, the change of kvm_arch_pmi_in_guest is incorrect, because it should cover two cases of PMI.
> For the PMI of intel pt, it certainly is the NMI PMI. So how about fixing it like below?
Yep, that works. I did enough spelunking to figure out how we can fix the generic
issue, but it's per-event and requires a decent amount of plumbing in perf.
perf_guest_handle_intel_pt_intr() doesn't bother with perf_guest_state() since it's
such a specialized event, so fixing this in vmx_handle_intel_pt_intr() would likely
be the long-term solution even if/when the generic case is fixed.
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] KVM: x86: Fix the intel_pt PMI handling wrongly considered from guest
2022-05-20 16:31 ` Sean Christopherson
@ 2022-05-21 4:31 ` Xu, Yanfei
0 siblings, 0 replies; 6+ messages in thread
From: Xu, Yanfei @ 2022-05-21 4:31 UTC (permalink / raw)
To: Christopherson,, Sean
Cc: pbonzini, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp,
dave.hansen, x86, kvm, linux-kernel, Wang, Wei W, Liang, Kan
> -----Original Message-----
> From: Sean Christopherson <seanjc@google.com>
> Sent: Saturday, May 21, 2022 12:32 AM
> To: Xu, Yanfei <yanfei.xu@intel.com>
> Cc: pbonzini@redhat.com; vkuznets@redhat.com; wanpengli@tencent.com;
> jmattson@google.com; joro@8bytes.org; tglx@linutronix.de;
> mingo@redhat.com; bp@alien8.de; dave.hansen@linux.intel.com;
> x86@kernel.org; kvm@vger.kernel.org; linux-kernel@vger.kernel.org; Wang,
> Wei W <wei.w.wang@intel.com>; Liang, Kan <kan.liang@intel.com>
> Subject: Re: [PATCH] KVM: x86: Fix the intel_pt PMI handling wrongly considered
> from guest
>
> Please don't top-post.
Got it, just correctly configured my mailbox with prefix each line of original message.
>
> On Fri, May 20, 2022, Xu, Yanfei wrote:
> > From: Sean Christopherson <seanjc@google.com> On Mon, May 16, 2022,
> > Yanfei Xu wrote:
> > > diff --git a/arch/x86/include/asm/kvm_host.h
> > > b/arch/x86/include/asm/kvm_host.h index 4ff36610af6a..308cf19f123d
> > > 100644
> > > --- a/arch/x86/include/asm/kvm_host.h
> > > +++ b/arch/x86/include/asm/kvm_host.h
> > > @@ -1582,8 +1582,14 @@ static inline int
> kvm_arch_flush_remote_tlb(struct kvm *kvm)
> > > return -ENOTSUPP;
> > > }
> > >
> > > +enum kvm_intr_type {
> > > + /* Values are arbitrary, but must be non-zero. */
> > > + KVM_HANDLING_IRQ = 1,
> > > + KVM_HANDLING_NMI,
> > > +};
> > > +
> > > #define kvm_arch_pmi_in_guest(vcpu) \
> > > - ((vcpu) && (vcpu)->arch.handling_intr_from_guest)
> > > + ((vcpu) && (vcpu)->arch.handling_intr_from_guest ==
> > > +KVM_HANDLING_NMI)
> >
> > My understanding is that this isn't correct as a general change, as
> > perf events can use regular IRQs in some cases. See commit dd60d217062f4
> ("KVM:
> > x86: Fix perf timer mode IP reporting").
> >
> > I assume there's got to be a way to know which mode perf is using,
> > e.g. we should be able to make this look something like:
> >
> > ((vcpu) && (vcpu)->arch.handling_intr_from_guest == kvm_pmi_vector)
>
> > Hi Sean,
> > You are right, the change of kvm_arch_pmi_in_guest is incorrect, because it
> should cover two cases of PMI.
> > For the PMI of intel pt, it certainly is the NMI PMI. So how about fixing it like
> below?
>
> Yep, that works. I did enough spelunking to figure out how we can fix the
> generic issue, but it's per-event and requires a decent amount of plumbing in
> perf.
Agree, it's per-event for the generic issue...
>
> perf_guest_handle_intel_pt_intr() doesn't bother with perf_guest_state() since
> it's such a specialized event, so fixing this in vmx_handle_intel_pt_intr() would
> likely be the long-term solution even if/when the generic case is fixed.
Will send the v2.
Thanks,
Yanfei
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-21 4:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-15 17:16 [PATCH] KVM: x86: Fix the intel_pt PMI handling wrongly considered from guest Yanfei Xu
2022-05-16 13:58 ` Sean Christopherson
2022-05-18 10:01 ` Yanfei Xu
2022-05-20 9:54 ` Xu, Yanfei
2022-05-20 16:31 ` Sean Christopherson
2022-05-21 4:31 ` Xu, Yanfei
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.