Re: [PATCH 8/8] evtchn: add FIFO-based event channel hypercalls and port ops

[David Vrabel <david.vrabel@citrix.com>]

On 23/08/13 11:33, Jan Beulich wrote:
>>>> On 09.08.13 at 20:08, David Vrabel <david.vrabel@citrix.com> wrote:
>> +static void evtchn_fifo_bind_to_vcpu(struct domain *d, struct evtchn *evtchn,
>> +                                     struct vcpu *v)
>> +{
>> +    unsigned priority;
>> +
>> +    /* Keep the same priority if possible, otherwise use the
>> +       default. */
>> +    if ( evtchn->queue )
>> +        priority = evtchn->queue->priority;
>> +    else
>> +        priority = EVTCHN_FIFO_PRIORITY_DEFAULT;
>> +
>> +    evtchn->queue = &v->evtchn_fifo->queue[priority];
> 
> Why not simply
> 
>     if ( !evtchn->queue )
>         evtchn->queue = &v->evtchn_fifo->queue[EVTCHN_FIFO_PRIORITY_DEFAULT];
> 
> ?

event->queue might point to a queue on VCPU w, and we want it to point
to one (of the same priority) on VCPU v.

But as you said later on, maybe it would be better to just store the
priority in the struct evtchn.  This would avoid the need to this
function/hook entirely.

>> +int evtchn_fifo_init_control(struct evtchn_init_control *init_control)
>> +{
[...]
>> +
>> +    /* Must be 8-bytes aligned. */
>> +    if ( offset & (8 - 1) )
> 
> This would better use __alignof() to document where the requirement
> stems from.

There's no specific requirement for it to be 8 byte aligned at this time
(the ready word is only 32-bits), this is to make it more future proof
if the structure is extended later.

David