Re: [Xen-ia64-devel] [patch] [4/4] Support INIT handler of xen

[Alex Williamson <alex.williamson@xxxxxx>]

Hi Akio,

   A few more comments...

On Mon, 2006-06-12 at 15:08 +0900, Akio Takebe wrote:
> diff -r aa2298739112 xen/arch/ia64/linux-xen/Makefile
> --- a/xen/arch/ia64/linux-xen/Makefile  Fri Jun 09 10:40:31 2006 -0600
> +++ b/xen/arch/ia64/linux-xen/Makefile  Mon Jun 12 03:21:27 2006 +0900
> @@ -3,6 +3,8 @@ obj-y += irq_ia64.o
>  obj-y += irq_ia64.o
>  obj-y += mm_contig.o
>  obj-y += pal.o
> +obj-y += mca_asm.o
> +obj-y += mca.o

   Need a README.origin update too.

> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#ifdef XEN
> +#include <xen/symbols.h>
> +#else
> +#include <linux/kallsyms.h>
> +#endif /* XEN */
> +#include <linux/smp_lock.h>
> +#include <linux/bootmem.h>
> +#include <linux/acpi.h>
> +#include <linux/timer.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/smp.h>
> +#ifndef XEN
> +#include <linux/workqueue.h>
> +#endif /* !XEN */

   I wouldn't bother with the /* XEN */ when there's so little code
between the #ifdef and #endif.

> +#include <asm/delay.h>
> +#include <asm/machvec.h>
> +#include <asm/page.h>
> +#include <asm/ptrace.h>
> +#include <asm/system.h>
> +#include <asm/sal.h>
> +#include <asm/mca.h>
> +
> +#include <asm/irq.h>
> +#include <asm/hw_irq.h>
> +#ifndef XEN
> +#include <asm/crashdump.h>
> +#endif /* !XEN */
> +#ifdef XEN

#else?


> +#ifdef XEN
> +        fetch_min_state(ms, pt, sw);
> +        spin_lock(&show_stack_lock);
> +        show_min_state(ms);
> +        printk("Backtrace of current vcpu (vcpu_id %d)\n",
> current->vcpu_id);
> +        unw_init_from_interruption(&info, current, pt, sw);
> +        ia64_do_show_stack(&info, NULL);
> +        unw_init_running(save_ksp, NULL);
> +        spin_unlock(&show_stack_lock);
> +        wmb();
> +       init_cache_flush();
> +        
> +        if (spin_trylock(&init_dump_lock)){
> +#ifdef CONFIG_SMP
> +                udelay(5*1000000);
> +#endif 
> +                
> +                if(try_crashdump(pt) == 0)
> +                        printk("\nINIT dump complete.  Please reboot
> now.\n");
> +        }
> +       printk("%s: CPU%d init handler done\n",
> __FUNCTION__ ,smp_processor_id());


   I'm not completely sure of the leverage we're getting by wedging this
#ifdef XEN section into the existing init_handler_platform() function.
Maybe we should #ifdef out the whole function and make our own for xen?

  In fact, the number of non-#ifdef'd out code in both mca.c and
mca_asm.S is fairly small.  Are we planning to leverage more code out of
these in the future?  I wonder if we'd be ahead to cut and paste the
little bits we are using into xen-specific files(?).  Thanks,

        Alex

-- 
Alex Williamson                             HP Open Source & Linux Org.


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