答复: [PATCH][v3] KVM: x86: refine kvm_vcpu_is_preempted

Li,Rongqing
2022-02-06T11:52:56Z

> On Wed, Feb 02, 2022, Peter Zijlstra wrote:
> > On Mon, Jan 17, 2022 at 01:37:22PM +0800, Li RongQing wrote:
> > > After support paravirtualized TLB shootdowns, steal_time.preempted
> > > includes not only KVM_VCPU_PREEMPTED, but also KVM_VCPU_FLUSH_TLB
> > >
> > > and kvm_vcpu_is_preempted should test only with KVM_VCPU_PREEMPTED
> >
> > This still fails to actually explain what the problem is, why did you
> > write this patch?
>
> Ya, definitely is lacking details. I think this captures everything...
>
> Tweak the assembly code for detecting KVM_VCPU_PREEMPTED to future
> proof
> it against new features, code refactorings, and theoretical compiler
> behavior.
>
> Explicitly test only the KVM_VCPU_PREEMPTED flag; steal_time.preempted
> has already been overloaded once for KVM_VCPU_FLUSH_TLB, and checking
> the
> entire byte for a non-zero value could yield a false positive. This
> currently isn't problematic as PREEMPTED and FLUSH_TLB are mutually
> exclusive, but that may not hold true for future flags.
>
> Use AND instead of TEST for querying PREEMPTED to clear RAX[63:8] before
> returning to avoid a potential false postive in the caller due to leaving
> the address (non-zero value) in the upper bits. Compilers are technically
> allowed to use more than a byte for storing _Bool, and it would be all too
> easy for someone to refactor the return type to something larger.
>
> Keep the SETcc (but change it to setnz for sanity's sake) as the fact that
> KVM_VCPU_PREEMPTED happens to be bit 0, i.e. the AND will yield 0/1 as
> needed for _Bool, is pure coincidence.
> cpu_is_preempted;"

Sean:

Could you resubmit this patch, thanks

-Li