Re: [Xen-devel] [PATCH v5 2/2] x86/hvm/viridian: Enable APIC assist enlightenment

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.03.16 at 11:06, <Paul.Durrant@xxxxxxxxxx> wrote:
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: 17 March 2016 16:43
>> >>> On 17.03.16 at 13:13, <paul.durrant@xxxxxxxxxx> wrote:
>> > @@ -1170,10 +1183,27 @@ int vlapic_has_pending_irq(struct vcpu *v)
>> >           !nestedhvm_vcpu_in_guestmode(v) )
>> >          return irr;
>> >
>> > +    /*
>> > +     * If APIC assist was used then there may have been no EOI so
>> > +     * we need to clear the requisite bit from the ISR here, before
>> > +     * comparing with the IRR.
>> > +     */
>> > +    if ( viridian_complete_apic_assist(v, &vector) &&
>> > +         vector != -1 )
>> 
>> Afaict "vector" is uninitialized here when initialize_apic_assist()
>> didn't run for that vCPU yet (which includes the case where no
>> Viridian emulation is active at all).
>> 
> 
> Yes, vector will be uninitialized in that case but viridian 
> _complete_apic_assist() will return 0 (because the va will be zero) and so 
> the second clause of the if will not be evaluated.

Ah, true. But raises the question why viridian_complete_apic_assist()
doesn't return the vector then rather then using indirection.

>> > +    /*
>> > +     * This vector is edge triggered and there are no lower priority
>> > +     * vectors pending, so we can use APIC assist to avoid exiting
>> > +     * for EOI.
>> > +     */
>> > +    viridian_start_apic_assist(v, vector);
>> >
>> > +done:
>> 
>> Labels indented by at least one space please.
> 
> OK, sorry, emacs keeps moving them back.

That's very unfriendly of it.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel