Re: [Xen-devel] [PATCH 3/4] xen: make start_info x86 specific.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-02-14 at 15:20 +0000, Jan Beulich wrote:
> >>> On 14.02.13 at 15:46, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> > On Thu, 2013-02-14 at 14:43 +0000, Jan Beulich wrote:
> >> >>> On 14.02.13 at 15:16, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
> >> > Most of this struct is PV MMU specific and it is not used on ARM at all.
> >> 
> >> I'm not convinced this is the right move.
> >> 
> >> > --- a/xen/include/public/xen.h
> >> > +++ b/xen/include/public/xen.h
> >> > @@ -706,79 +706,6 @@ typedef struct shared_info shared_info_t;
> >> >   *     extended by an extra 4MB to ensure this.
> >> >   */
> >> >  
> >> > -#define MAX_GUEST_CMDLINE 1024
> >> > -struct start_info {
> >> > -    /* THE FOLLOWING ARE FILLED IN BOTH ON INITIAL BOOT AND ON RESUME.  
> >> >   
> > */
> >> > -    char magic[32];             /* "xen-<version>-<platform>".          
> >> >   */
> >> > -    unsigned long nr_pages;     /* Total pages allocated to this 
> >> > domain.  
> > */
> >> > -    unsigned long shared_info;  /* MACHINE address of shared info 
> >> > struct. 
> > */
> >> > -    uint32_t flags;             /* SIF_xxx flags.                       
> >> >   
> > */
> >> > -    xen_pfn_t store_mfn;        /* MACHINE page number of shared page.  
> >> >   
> > */
> >> > -    uint32_t store_evtchn;      /* Event channel for store 
> >> > communication. 
> > */
> >> > -    union {
> >> > -        struct {
> >> > -            xen_pfn_t mfn;      /* MACHINE page number of console page. 
> >> >   
> > */
> >> > -            uint32_t  evtchn;   /* Event channel for console page.      
> >> >   
> > */
> >> > -        } domU;
> >> > -        struct {
> >> > -            uint32_t info_off;  /* Offset of console_info struct.       
> >> >   
> > */
> >> > -            uint32_t info_size; /* Size of console_info struct from 
> > start.*/
> >> > -        } dom0;
> >> > -    } console;
> >> 
> >> What is PV MMU related up to here?
> > 
> > Hrm, perhaps categorising this as PV MMU was a mistake on my part.
> > 
> > These are all unused on ARM in terms of the hypervisor ABI since it uses
> > HVM like mechanisms for those which are appropriate and doesn't use a
> > bunch of the others at all.
> 
> But nothing here makes this structure x86 specific, so I'd really like
> to keep it that way. If ARM doesn't use the structure at all, then
> I don't see the point in removing it from the common headers. If
> part of it (the flags come to mind) are being used by ARM, let's
> find a reasonable abstraction so that ARM doesn't need to carry
> useless baggage (e.g. a XEN_ARCH_HVM_ONLY #define).

Even the flags are unused on ARM (as part of the hypervisor ABI), which
uses XENFEAT_dom0 and not SIF_*.

I'd be as happy to ifdef this in the public headers instead of moving
them if that is what people prefer. I don't think XEN_ARCH_HVM_ONLY is
quite the right name though. XEN_HAVE_PV_GUEST_ENTRY perhaps? This makes
some sense by virtue of start_info not being useful on ARM partly
because we use the normal native entry points which don't allow for it.

Do we think that it is likely that a new architecture port which doesn't
use hardware virtualisation extensions instead of PV is very likely?

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel