[prev in list] [next in list] [prev in thread] [next in thread] 

List:       xen-ppc-devel
Subject:    Re: [XenPPC] [PATCH 2/2] linux: build start_info_t from devtree only
From:       Jimi Xenidis <jimix () watson ! ibm ! com>
Date:       2007-02-08 14:09:58
Message-ID: DE9F668A-88D8-467B-A405-F895CD6BE9A9 () watson ! ibm ! com
[Download RAW message or body]


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

> * Jimi Xenidis <jimix@watson.ibm.com> [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@us.ibm.com
>>>
>>>
>>> diffstat output:
>>> setup.c |   33 +++++++++++++++------------------
>>> 1 files changed, 15 insertions(+), 18 deletions(-)
>>>
>>> Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
>>> ---
>>> 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@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel
[prev in list] [next in list] [prev in thread] [next in thread]