On Wed, Apr 17 2024 at 11:01, Jacob Pan wrote: > On Tue, 16 Apr 2024 17:39:42 -0700, Sean Christopherson <seanjc@xxxxxxxxxx> > wrote: >> > diff --git a/arch/x86/kvm/vmx/posted_intr.c >> > b/arch/x86/kvm/vmx/posted_intr.c index af662312fd07..592dbb765675 100644 >> > --- a/arch/x86/kvm/vmx/posted_intr.c >> > +++ b/arch/x86/kvm/vmx/posted_intr.c >> > @@ -107,7 +107,7 @@ void vmx_vcpu_pi_load(struct kvm_vcpu *vcpu, int >> > cpu) >> > * handle task migration (@cpu != vcpu->cpu). >> > */ >> > new.ndst = dest; >> > - new.sn = 0; >> > + new.notif_ctrl &= ~POSTED_INTR_SN; >> >> At the risk of creating confusing, would it make sense to add >> double-underscore, non-atomic versions of the set/clear helpers for ON >> and SN? >> >> I can't tell if that's a net positive versus open coding clear() and >> set() here and below. > IMHO, we can add non-atomic helpers when we have more than one user for > each operation. > > I do have a stupid bug here, it should be: > - new.notif_ctrl &= ~POSTED_INTR_SN; > + new.notif_ctrl &= ~BIT(POSTED_INTR_SN); That's a perfect reason to use a proper helper.
