From mboxrd@z Thu Jan  1 00:00:00 1970
From: David Vrabel <david.vrabel@citrix.com>
Subject: Re: [PATCH 8/8] evtchn: add FIFO-based event channel
 hypercalls and port ops
Date: Fri, 23 Aug 2013 12:00:32 +0100
Message-ID: <521740D0.2040904@citrix.com>
References: <1376071720-17644-1-git-send-email-david.vrabel@citrix.com>
	<1376071720-17644-9-git-send-email-david.vrabel@citrix.com>
	<521756B402000078000EDF7A@nat28.tlf.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta5.messagelabs.com ([195.245.231.135])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <david.vrabel@citrix.com>) id 1VCp6a-0004rP-Oq
	for xen-devel@lists.xenproject.org; Fri, 23 Aug 2013 11:00:36 +0000
In-Reply-To: <521756B402000078000EDF7A@nat28.tlf.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>, Keir Fraser <keir@xen.org>, Wei Liu <wei.liu2@citrix.com>
List-Id: xen-devel@lists.xenproject.org

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