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

[David Vrabel <david.vrabel@xxxxxxxxxx>]

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.

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