| Message ID | 20210531174628.10265-1-jiangshanlai@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: X86: reset and read st->preempted in atomic way | expand |
Nit: the shortlog is somewhat inaccurate now, maybe just: KVM: x86: Ensure PV TLB flush tracepoint reflects KVM behavior or something along those lines. Not sure what the best wording is :-/ On Tue, Jun 01, 2021, Lai Jiangshan wrote: > From: Lai Jiangshan <laijs@linux.alibaba.com> > > In record_steal_time(), st->preempted is read twice, and > trace_kvm_pv_tlb_flush() might output result inconsistent if > kvm_vcpu_flush_tlb_guest() see a different st->preempted later. > > It is a very trivial problem and hardly has actual harm and can be > avoided by reseting and reading st->preempted in atomic way via xchg(). > > Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com> I saw this quirk too, but couldn't quite bring myself to care enought to test a patch :-) Reviewed-by: Sean Christopherson <seanjc@google.com>
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 0087d3532c98..fba39fe162da 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3117,9 +3117,11 @@ static void record_steal_time(struct kvm_vcpu *vcpu) * expensive IPIs. */ if (guest_pv_has(vcpu, KVM_FEATURE_PV_TLB_FLUSH)) { + u8 st_preempted = xchg(&st->preempted, 0); + trace_kvm_pv_tlb_flush(vcpu->vcpu_id, - st->preempted & KVM_VCPU_FLUSH_TLB); - if (xchg(&st->preempted, 0) & KVM_VCPU_FLUSH_TLB) + st_preempted & KVM_VCPU_FLUSH_TLB); + if (st_preempted & KVM_VCPU_FLUSH_TLB) kvm_vcpu_flush_tlb_guest(vcpu); } else { st->preempted = 0;