Re: [win-pv-devel] [PATCH 10/10] Add support for the FIFO event channel ABI

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: David Vrabel
> Sent: 10 November 2014 15:29
> To: Paul Durrant; win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH 10/10] Add support for the FIFO event channel ABI
> 
> On 07/11/14 14:47, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: David Vrabel
> >> Sent: 07 November 2014 14:37
> >> To: Paul Durrant; win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> >> Subject: Re: [PATCH 10/10] Add support for the FIFO event channel ABI
> >>
> >> On 07/11/14 14:02, Paul Durrant wrote:
> >>>> -----Original Message-----
> >>>> From: Paul Durrant [mailto:paul.durrant@xxxxxxxxxx]
> >>>> +static BOOLEAN
> >>>> +EvtchnFifoPortUnmask(
> >>>> +    IN  PXENBUS_EVTCHN_ABI_CONTEXT  _Context,
> >>>> +    IN  ULONG                       Port
> >>>> +    )
> >>>> +{
> >>>> +    PXENBUS_EVTCHN_FIFO_CONTEXT     Context = (PVOID)_Context;
> >>>> +    event_word_t                    *EventWord;
> >>>> +    LONG                            Old;
> >>>> +    LONG                            New;
> >>>> +
> >>>> +    EventWord = EvtchnFifoEventWord(Context, Port);
> >>>> +
> >>>> +    // Clear masked bit, spinning if busy
> >>>> +    do {
> >>>> +        Old = *EventWord & ~(1 << EVTCHN_FIFO_BUSY);
> >>>> +        New = Old & ~(1 << EVTCHN_FIFO_MASKED);
> >>>> +    } while (InterlockedCompareExchange((LONG *)EventWord, New,
> >> Old) !=
> >>>> Old);
> >>>> +
> >>>> +    // Check whether the port was masked
> >>>> +    if (~Old & (1 << EVTCHN_FIFO_MASKED))
> >>>> +        return FALSE;
> >>>> +
> >>>> +    // If we cleared the mask then check whether something is pending
> >>>> +    if (!__EvtchnFifoTestAndClearFlag(EventWord,
> >> EVTCHN_FIFO_PENDING))
> >>>> +        return FALSE;
> >>
> >> The doc says you should do an EVTCHNOP_unmask hypercall here, but
> >> picking up any pending event here is fine.
> >>
> >> You may want to be more careful if you unmask an event while running
> on
> >> a VCPU that is not the one the event is bound to.
> >>
> >
> > Yes - I do need to allow for that. It won't happen at the moment but
> nothing in the API prevents it.
> 
> Actually, picking up the PENDING bit on unmask like this will prioritise
> this event over any other pending events.  This subvert the ABI's FIFO
> properties and will also result in lower priority events being handled
> before higher priority ones.
> 

That is true if the unmask is done in the event handler. If the unmask is done 
from a DPC then a higher priority event can still come in at any time, since 
the evtchn upcall will be at > DISPATCH_LEVEL. So, I'll add the hypercall for 
the former case.

Cheers,

  Paul

> I would recommend: do not clear PENDING, do the unmask hypercall and
> always return FALSE.
> 
> David

_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
http://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel