Re: [PATCH 04/13] xen: sync public headers

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Feb 05, 2013 at 05:23:40PM +0000, Wei Liu wrote:
> On Tue, 2013-02-05 at 17:00 +0000, Konrad Rzeszutek Wilk wrote:
> > On Thu, Jan 31, 2013 at 02:46:58PM +0000, Wei Liu wrote:
> > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > > ---
> > >  include/xen/interface/event_channel.h |   33 +++++++++++++++++++++++++++++++++
> > >  include/xen/interface/xen.h           |    9 ++++++++-
> > >  2 files changed, 41 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/include/xen/interface/event_channel.h b/include/xen/interface/event_channel.h
> > > index f494292..9d8b9e7 100644
> > > --- a/include/xen/interface/event_channel.h
> > > +++ b/include/xen/interface/event_channel.h
> > > @@ -190,6 +190,39 @@ struct evtchn_reset {
> > >  };
> > >  typedef struct evtchn_reset evtchn_reset_t;
> > >  
> > > +/*
> > > + * EVTCHNOP_register_nlevel: Register N-level event channel
> > > + * NOTES:
> > > + *  1. Currently only 3-level is supported.
> > > + *  2. Should fall back to 2-level if this call fails.
> > > + */
> > > +#define EVTCHNOP_register_nlevel 11
> > > +/* 64 bit guests need 8 pages for evtchn_pending and evtchn_mask for
> > > + * 256k event channels while 32 bit ones only need 1 page for 32k
> > > + * event channels. */
> > > +#define EVTCHN_MAX_L3_PAGES 8
> > > +struct evtchn_register_3level {
> > > +	/* IN parameters. */
> > > +	uint32_t nr_pages; /* for evtchn_{pending,mask} */
> > > +	uint32_t nr_vcpus; /* for l2sel_{mfns,mask} */
> > > +	GUEST_HANDLE(ulong) evtchn_pending;
> > > +	GUEST_HANDLE(ulong) evtchn_mask;
> > > +	GUEST_HANDLE(ulong) l2sel_mfns;
> > > +	GUEST_HANDLE(ulong) l2sel_offsets;
> > > +};
> > > +typedef struct evtchn_register_3level evtchn_register_3level_t;

Please please not put them in. I know that there are some typdefs in
there but they should actually be removed.

> > > +DEFINE_GUEST_HANDLE(evtchn_register_3level_t);
> > > +
> > > +struct evtchn_register_nlevel {
> > > +	/* IN parameters. */
> > > +	uint32_t level;
> > > +	union {
> > > +		evtchn_register_3level_t l3;
> > > +	} u;
> > > +};
> > > +typedef struct evtchn_register_nlevel evtchn_register_nlevel_t;
> > > +DEFINE_GUEST_HANDLE(evtchn_register_nlevel_t);
> > > +
> > >  struct evtchn_op {
> > >  	uint32_t cmd; /* EVTCHNOP_* */
> > >  	union {
> > > diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
> > > index a890804..5220e33 100644
> > > --- a/include/xen/interface/xen.h
> > > +++ b/include/xen/interface/xen.h
> > > @@ -283,9 +283,16 @@ DEFINE_GUEST_HANDLE_STRUCT(multicall_entry);
> > >  
> > >  /*
> > >   * Event channel endpoints per domain:
> > > + * 2-level:
> > >   *  1024 if a long is 32 bits; 4096 if a long is 64 bits.
> > > + * 3-level:
> > > + *  32k if a long is 32 bits; 256k if a long is 64 bits.
> > >   */
> > > -#define NR_EVENT_CHANNELS (sizeof(unsigned long) * sizeof(unsigned long) * 64)
> > > +#define NR_EVENT_CHANNELS_L2 (sizeof(unsigned long) * sizeof(unsigned long) * 64)
> > 
> > We did a bit of header change to make the ARM code always use 'unsinged
> > long long' on 32-bit (so it is a nice 8-bytes) and 'unsigned long' on
> > 64-bit. This was all done using the xen_pfn_t and xen_mfn_t typdefs.
> > 
> > Any chance you can do that here? That way on ARM - irregardless if it is
> > 32-bit or 64-bit it is always of the 64-bit size?
> > 
> 
> I don't think so. The reason to use unsigned long here is to guarantee
> each selector (in 2-level case there is only L1 selector) fits into a
> word.

OK, so we do depend on this being of different size on 32-bit and
64-bit.
> 
> > Or do we not care about the ARM case here?
> > 
> 
> What do you mean? How would this definition affect ARM case?
> 

Xen ARM uses 64-bit values even if the host/guest is 32-bit.

> 
> Wei.
> 
> > > +#define NR_EVENT_CHANNELS_L3 (NR_EVENT_CHANNELS_L2 * 64)
> > > +#if !defined(__XEN__) && !defined(__XEN_TOOLS__)
> > > +#define NR_EVENT_CHANNELS NR_EVENT_CHANNELS_L2 /* for compatibility */
> > > +#endif
> > >  
> > >  struct vcpu_time_info {
> > >  	/*
> > > -- 
> > > 1.7.10.4
> > > 
> 
>