Re: [Xen-devel] [PATCH for-4.13] x86/vmx: always sync PIR to IRR before vmentry

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Nov 27, 2019 at 12:16:37PM +0100, Jan Beulich wrote:
> On 27.11.2019 12:03, Roger Pau Monné  wrote:
> > On Wed, Nov 27, 2019 at 02:07:16AM +0000, Tian, Kevin wrote:
> >> Then what's the difference from original logic?
> > 
> > The original logic is:
> > 
> > if ( running && (in_irq() || (v != current)) )
> > {
> >         unsigned int cpu = v->processor;
> > 
> >         if ( cpu != smp_processor_id() )
> >             send_IPI_mask(cpumask_of(cpu), posted_intr_vector);
> >         else if ( !softirq_pending(cpu) )
> >             raise_softirq(VCPU_KICK_SOFTIRQ);
> > }
> > 
> > Which I find much harder to understand. For example I'm not sure of
> > what's the benefit of doing the cpu != smp_processor_id() check
> > instead of simply doing v != current (like in the outer if condition).
> 
> There are two aspects to consider: One is that v->processor
> may equal smp_processor_id() also for v != current. The other
> is that without this check in the if() it would need adding
> to the else-if(). I'm not sure to what degree which of the
> two matters functionality wise.

Since the vCPU is running v->processor can only equal smp_processor_id
if v == current, and hence I think both checks achieve exactly the
same end result, it's just that IMO doing the outer one with v !=
current and the inner one with cpu != smp_processor_id() is confusing.

Maybe I'm missing something else that actually requires doing the
inner check with v->processor and smp_processor_id().

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel