| Message ID | 20241014105124.24473-3-adrian.hunter@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [V13,01/14] perf/x86/intel/pt: Fix buffer full but size is 0 case | expand |
"KVM: VMX:" for the scope. And I would much prefer to actually state what is changing. "Fix XYZ" isn't helpful in understanding what's actually broken, fallout from the bug, etc. It's never easy to describe bugs where the logic is flat out busted, but I think we can at least capture the basic gist, and allude to the badness being a wrongly disallowed write. On Mon, Oct 14, 2024, Adrian Hunter wrote: > Fix KVM IA32_RTIT_CTL MSR validation logic so that if RTIT_CTL_TRACEEN > bit is cleared, then other bits are allowed to change also. For example, > writing 0 to IA32_RTIT_CTL in order to stop tracing, is valid. There's a fair amount of extraneous and disctracting information in both the shortlog and changelog. E.g. "Intel PT IA32_RTIT_CTL MSR" can simply be MSR_IA32_RTIT_CTL. And the I'll fix up to the below when applying; AFAICT, this fix is completely independent of the rest of the series. KVM: VMX: Allow toggling bits in MSR_IA32_RTIT_CTL when enable bit is cleared Allow toggling other bits in MSR_IA32_RTIT_CTL if the enable bit is being cleared, the existing logic simply ignores the enable bit. E.g. KVM will incorrectly reject a write of '0' to stop tracing. > Fixes: bf8c55d8dc09 ("KVM: x86: Implement Intel PT MSRs read/write emulation") > Cc: stable@vger.kernel.org > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > --- > arch/x86/kvm/vmx/vmx.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 1a4438358c5e..eaf4965ac6df 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -1635,7 +1635,8 @@ static int vmx_rtit_ctl_check(struct kvm_vcpu *vcpu, u64 data) > * result in a #GP unless the same write also clears TraceEn. > */ > if ((vmx->pt_desc.guest.ctl & RTIT_CTL_TRACEEN) && > - ((vmx->pt_desc.guest.ctl ^ data) & ~RTIT_CTL_TRACEEN)) > + (data & RTIT_CTL_TRACEEN) && > + data != vmx->pt_desc.guest.ctl) > return 1; > > /* > -- > 2.43.0 >
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 1a4438358c5e..eaf4965ac6df 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -1635,7 +1635,8 @@ static int vmx_rtit_ctl_check(struct kvm_vcpu *vcpu, u64 data) * result in a #GP unless the same write also clears TraceEn. */ if ((vmx->pt_desc.guest.ctl & RTIT_CTL_TRACEEN) && - ((vmx->pt_desc.guest.ctl ^ data) & ~RTIT_CTL_TRACEEN)) + (data & RTIT_CTL_TRACEEN) && + data != vmx->pt_desc.guest.ctl) return 1; /*
Fix KVM IA32_RTIT_CTL MSR validation logic so that if RTIT_CTL_TRACEEN bit is cleared, then other bits are allowed to change also. For example, writing 0 to IA32_RTIT_CTL in order to stop tracing, is valid. Fixes: bf8c55d8dc09 ("KVM: x86: Implement Intel PT MSRs read/write emulation") Cc: stable@vger.kernel.org Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- arch/x86/kvm/vmx/vmx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)