Re: [XenPPC] [PATCH 2/2] linux: build start_info_t from devtree only

[Jimi Xenidis <jimix@xxxxxxxxxxxxxx>]

On Feb 8, 2007, at 8:45 AM, Ryan Harper wrote:

> * Jimi Xenidis <jimix@xxxxxxxxxxxxxx> [2007-02-08 06:48]:
> > some comments
> > On Feb 7, 2007, at 6:34 PM, Ryan Harper wrote:
> >
> > > This patch cleans up xen_init_early() to construct a start_info_t  
> > > only
> > > from the devtree as Patch1 fixes xen to no longer create a
> > > start_info_t
> > > for dom0.
> > >
> > > --
> > > Ryan Harper
> > > Software Engineer; Linux Technology Center
> > > IBM Corp., Austin, Tx
> > > (512) 838-9253   T/L: 678-9253
> > > ryanh@xxxxxxxxxx
> > >
> > >
> > > diffstat output:
> > > setup.c |   33 +++++++++++++++------------------
> > > 1 files changed, 15 insertions(+), 18 deletions(-)
> > >
> > > Signed-off-by: Ryan Harper <ryanh@xxxxxxxxxx>
> > > ---
> > > diff -r a6adf094e08e arch/powerpc/platforms/xen/setup.c
> > > --- a/arch/powerpc/platforms/xen/setup.c        Tue Feb 06 13:55:48 2007
> > > -0600
> > > +++ b/arch/powerpc/platforms/xen/setup.c        Wed Feb 07 11:33:10 2007
> > > -0600
> > > @@ -88,29 +88,26 @@ static void __init xen_init_early(void)
> > > static void __init xen_init_early(void)
> > > {
> > >         struct device_node *xen;
> > > -       u64 *si;
> > >
> > >         DBG(" -> %s\n", __func__);
> > >
> > >         xen = of_find_node_by_path("/xen");
> > >
> > > -       si = (u64 *)get_property(xen, "start-info", NULL);
> > > -
> > > -       /* build our own start_info_t if start-info property is not
> > > present */
> > > -       if (si != NULL) {
> > > -               xen_start_info = (start_info_t *)__va(*si);
> > > -       } else {
> > > -               struct device_node *console;
> > > -               struct device_node *store;
> > > -
> > > -               console = of_find_node_by_path("/xen/console");
> > > -               store = of_find_node_by_path("/xen/store");
> > > -
> > > -               xen_start_info = &xsi;
> > > -
> > > -               /* fill out start_info_t from devtree */
> > > -               xen_start_info->shared_info = *((u64 *)get_property(xen,
> > > -                  "shared-info", NULL));
> > > +       xen_start_info = &xsi;
> > Please make xsi static in its declaration earlier in the file.
> Sure.  And while I wanted to access xsi vai xen_start_info,
> the
>
>    xen_start_info = &xsi
>
> seemed a bit awkward.  Not sure if I should switch to xsi.  Thoughts?
There is too much code referring to xen_start_info as a pointer.
perhaps making that static __xen_start_info, so its obvious that we  
are just suing it for memory allocation, at some point when all  
references are gone we can flatten this out.
> > > +
> > > +       /* fill out start_info_t from devtree */
> > > +       if ((char *)get_property(xen, "privileged", NULL))
> > > +               xen_start_info->flags |= SIF_PRIVILEGED;
> > > +       if ((char *)get_property(xen, "initdomain", NULL))
> > > +               xen_start_info->flags |= SIF_INITDOMAIN;
> > > +       xen_start_info->shared_info = *((u64 *)get_property(xen,
> > > +          "shared-info", NULL));
> > > +
> > > +       /* only look for store and console for guest domains */
> > Hmm, I think you need to look for them always.
> > You _at_least_ need the console evtchn, which may not be zero and we
> > create the node/prop anyway.
> Hrm, you may be right.  I know that dom0 boots fine with this, but  
> that
> maybe because it defaults those values to 0.  I'll kill the if().
>
> > Feel free to "panic()" more:
> > NOTE: this is early so a "udbg_printf(); HYPERVISOR_shutdown
> > (SHUTDOWN_poweroff);" will do cuz panic() is no available yet.
> Yeah, good idea though none of the messages get out if our shared_info
> page isn't setup correctly, which I learned during my testing of this
> patch, was the value most likely to get hosed.
Oh yeah, you probably want to:
        HYPERVISOR_shared_info = ...;
        xen_start_info->flags |= SIF_INITDOMAIN; # or not
        udbg_init_xen();

As soon as you can, before you check everything else.

If you want more info earlier you can always build with:
  CONFIG_PPC_EARLY_DEBUG_XEN_DOM0
xor
  CONFIG_PPC_EARLY_DEBUG_XEN_DOMU

-JX






_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel