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

[Jimi Xenidis <jimix@xxxxxxxxxxxxxx>]

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.

> +
> +       /* 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.
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.
  if (!store && !(xen_start_info->flags | SIF_INITDOMAIN) panic();
  if (!console) panic();
  if (get_property(console, "reg", NULL) == NULL &&
      !(xen_start_info->flags | SIF_INITDOMAIN)) panic

> +       if (xen_start_info->flags == 0) {
> +               struct device_node *console = 
> of_find_node_by_path("/xen/console");
> +               struct device_node *store = of_find_node_by_path("/xen/store");
> +
>                 xen_start_info->store_mfn = (*((u64 *)get_property(store,
>                    "reg", NULL))) >> PAGE_SHIFT;
>                 xen_start_info->store_evtchn = *((u32 *)get_property(store,
>
> _______________________________________________
> Xen-ppc-devel mailing list
> Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-ppc-devel

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