>-----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
|
Home