Re: [RFC PATCH 1/3] Add a field in struct domain to indicate evtchn level.

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

On 31/12/12 18:22, Wei Liu wrote:
> From: Wei Liu <liuw@liuw.name>

The first patch needs to add the ABI documentation.

It's not clear what this is all for from just the changeset descriptions.

> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  xen/common/event_channel.c |    1 +
>  xen/include/xen/sched.h    |   15 ++++++++++++++-
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
> index 89f0ca7..87e422e 100644
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -1173,6 +1173,7 @@ void notify_via_xen_event_channel(struct domain *ld, int lport)
>  int evtchn_init(struct domain *d)
>  {
>      spin_lock_init(&d->event_lock);
> +    d->evtchn_level = 2;

I'd be inclined to have something like:

/* Original ABI: 2 level event channels */
#define EVTCHN_LEVEL_DEFAULT 2
...
    d->evtchn_level = EVTCHN_LEVEL_DEFAULT;


>      if ( get_free_port(d) != 0 )
>          return -EINVAL;
>      evtchn_from_port(d, 0)->state = ECS_RESERVED;
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 6c55039..1c43e0a 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -50,7 +50,19 @@ extern struct domain *dom0;
>  #else
>  #define BITS_PER_EVTCHN_WORD(d) (has_32bit_shinfo(d) ? 32 : BITS_PER_LONG)
>  #endif
> -#define MAX_EVTCHNS(d) (BITS_PER_EVTCHN_WORD(d) * BITS_PER_EVTCHN_WORD(d))
> +#define MAX_EVTCHNS_L2(d) (BITS_PER_EVTCHN_WORD(d) * BITS_PER_EVTCHN_WORD(d))
> +#define MAX_EVTCHNS_L3(d) (MAX_EVTCHNS_L2(d) * BITS_PER_EVTCHN_WORD(d))
> +#define MAX_EVTCHNS(d) ({ int __v = 0;          \
> +            switch ( d->evtchn_level ) {        \
> +            case 2:                             \
> +                __v = MAX_EVTCHNS_L2(d); break; \
> +            case 3:                             \
> +                __v = MAX_EVTCHNS_L3(d); break; \
> +            default:                            \
> +                BUG();                          \
> +            };                                  \
> +            __v;})

Here you BUG if d->evtchn_level is bad, but in other places in later
patches with similar you have a printk.  Need to be more consistent here.

On the Linux side you use a set of event channel ops instead of
repeatedly testing d->evtchn_level.  Would this also be a better
approach for the Xen side?

David

>  #define EVTCHNS_PER_BUCKET 128
>  #define NR_EVTCHN_BUCKETS  (NR_EVENT_CHANNELS / EVTCHNS_PER_BUCKET)
>  
> @@ -262,6 +274,7 @@ struct domain
>      /* Event channel information. */
>      struct evtchn   *evtchn[NR_EVTCHN_BUCKETS];
>      spinlock_t       event_lock;
> +    unsigned int     evtchn_level;
>  
>      struct grant_table *grant_table;
>