On Thu, 2021-10-28 at 14:28 +0300, Maxim Levitsky wrote: > On Fri, 2021-10-08 at 19:12 -0700, Sean Christopherson wrote: > > Hoist the CPU => APIC ID conversion for the Posted Interrupt descriptor > > out of the loop to write the descriptor, preemption is disabled so the > > CPU won't change, and if the APIC ID changes KVM has bigger problems. > > > > No functional change intended. > > Is preemption always disabled in vmx_vcpu_pi_load? vmx_vcpu_pi_load is called from vmx_vcpu_load, > which is called indirectly from vcpu_load which is called from many ioctls, > which userspace does. In these places I don't think that preemption is disabled. You can disregard this, I missed the fact that we have 'int cpu = get_cpu();' which disables preemption in 'vcpu_load' Thus, Reviewed-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx> Best regards, Maxim Levitsky > > Best regards, > Maxim Levitsky > > > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx> > > --- > > arch/x86/kvm/vmx/posted_intr.c | 25 +++++++++++-------------- > > 1 file changed, 11 insertions(+), 14 deletions(-) > > > > diff --git a/arch/x86/kvm/vmx/posted_intr.c b/arch/x86/kvm/vmx/posted_intr.c > > index fea343dcc011..2b2206339174 100644 > > --- a/arch/x86/kvm/vmx/posted_intr.c > > +++ b/arch/x86/kvm/vmx/posted_intr.c > > @@ -51,17 +51,15 @@ void vmx_vcpu_pi_load(struct kvm_vcpu *vcpu, int cpu) > > goto after_clear_sn; > > } > > > > - /* The full case. */ > > + /* The full case. Set the new destination and clear SN. */ > > + dest = cpu_physical_id(cpu); > > + if (!x2apic_mode) > > + dest = (dest << 8) & 0xFF00; > > + > > do { > > old.control = new.control = READ_ONCE(pi_desc->control); > > > > - dest = cpu_physical_id(cpu); > > - > > - if (x2apic_mode) > > - new.ndst = dest; > > - else > > - new.ndst = (dest << 8) & 0xFF00; > > - > > + new.ndst = dest; > > new.sn = 0; > > } while (cmpxchg64(&pi_desc->control, old.control, > > new.control) != old.control); > > @@ -103,15 +101,14 @@ static void __pi_post_block(struct kvm_vcpu *vcpu) > > WARN(pi_desc->nv != POSTED_INTR_WAKEUP_VECTOR, > > "Wakeup handler not enabled while the vCPU was blocking"); > > > > + dest = cpu_physical_id(vcpu->cpu); > > + if (!x2apic_mode) > > + dest = (dest << 8) & 0xFF00; > > + > > do { > > old.control = new.control = READ_ONCE(pi_desc->control); > > > > - dest = cpu_physical_id(vcpu->cpu); > > - > > - if (x2apic_mode) > > - new.ndst = dest; > > - else > > - new.ndst = (dest << 8) & 0xFF00; > > + new.ndst = dest; > > > > /* set 'NV' to 'notification vector' */ > > new.nv = POSTED_INTR_VECTOR;