[Xen-ia64-devel] RE: [PATCH] pv_ops/xen: elf note based xen startup

["He, Qing" <qing.he@xxxxxxxxx>]

>-----Original Message-----
>From: Isaku Yamahata [mailto:yamahata@xxxxxxxxxxxxx]
>Sent: 2008年5月7日 10:17
>To: He, Qing
>Cc: xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
>Subject: Re: [PATCH] pv_ops/xen: elf note based xen startup
>
>On Tue, May 06, 2008 at 04:37:11PM +0800, He, Qing wrote:
>> This patch enables elf note based xen startup for IA-64, which gives the
>> kernel an early hint for running on xen instead of relying on psr.cpl.
>> It is also an extension for possible support of future hypervisors.
>
>Thank you for the patch.
>Basically it looks good, but some comments.
>
>- It looks unnecessary to define PARAVIRT_HYPERVISOR_TYPE_xxx,
>  hypervisor_type and hypervisor_setup_hooks array.
>  Only just one hook looks necessary.
>  Do you intended to use hypervisor type later?
>  If so, probably pv_info->name should be used or add a new member
>  to pv_info.
>
>  C-like pseudo code:
>  head.S
>  void (*paravirt_setup_hook) = NULL
>  ...
>  _start:
>
>  if (*paravirt_setup_hook != NULL)
>    (*paravirt_setup_hook)()
>  ...
>
>  xensetup.S
>  startup_xen:
>  *paravirt_setup_hook = &xen_setup_hook
>  ...
>  branch _start
>
>  ...
>  xen_setup_hook:
>  ...
>
>  Thus the weak symbol trick also goes away.

The patch uses a similar way as x86 pv_ops does. One advantage of using a 
branch table is that it is viable for a future `single entry, CPUID based pv 
setup branching'. Anyway, both approaches are fine for the current code.

With this idea, the xen specific entry is just a stub. It is one of (future 
possible) ways of letting the kernel know what hypervisor it runs on. Also, at 
the very early stage like startup_xen (psr.dt==0), I'd think avoid touching 
structs using asm-offset would be preferable.

I don't really like the macro PARAVIRT_HYPERVISOR_TYPE_xxx very much, since any 
modification requires manual sync up in two files, but I just hate magic 
numbers more...

>
>- Please split it into two parts, xen specific part
>  (modification to xensetup.S) and common part (modification to head.S).
>  If you don't mind, I'll do it keeping your signed off by.

I'm OK with that. Thank you for the comments.

>
>thanks.
>--
>yamahata

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