[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] x86/hvm: more sure APIC assist is aborted if guest EOIs APIC



> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 18 January 2018 08:33
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; xen-
> devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH] x86/hvm: more sure APIC assist is aborted if guest EOIs
> APIC
> 
> >>> On 17.01.18 at 13:53, <paul.durrant@xxxxxxxxxx> wrote:
> > --- a/xen/arch/x86/hvm/vlapic.c
> > +++ b/xen/arch/x86/hvm/vlapic.c
> > @@ -422,6 +422,13 @@ void vlapic_EOI_set(struct vlapic *vlapic)
> >      if ( vector == -1 )
> >          return;
> >
> > +    /*
> > +     * It is possible that APIC assist has been enabled by the guest but
> > +     * it has chosen not to use it, by EOIing normally. It is therefore
> > +     * necessary to abort any APIC assist that may have been started
> > +     * to avoid confusing the state machine.
> > +     */
> > +    viridian_abort_apic_assist(vlapic_vcpu(vlapic));
> >      vlapic_clear_vector(vector, &vlapic->regs->data[APIC_ISR]);
> 
> Fundamentally fine, but is clearing bit 0 in the shared location a
> valid thing to do in this case? Plus shouldn't you pass in the vector,
> so the abort would only happen for the very vector that is pending?

I believe that it is ok.

> Iirc with nested interrupts normal EOI might still need to be used
> by Windows (as only a single vector can be in flight in the assist), so
> clearing the assist for the wrong vector might actually cause harm.
> 

If there are nested interrupts then the assist should be aborted anyway. Assist 
only helps if there is one interrupt pending so should not have been started if 
something higher priority already in the ISR so, if we see an EOI for a vector 
that is not the one latched in assist I think this is also a bug in the state 
machine. I can add verification of that, if you'd like.

  Paul

> Jan


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.