[Xen-devel] Re: [RFC, PATCH 9/24] i386 Vmi smp support

[Pavel Machek <pavel@xxxxxx>]

On Po 13-03-06 10:05:58, Zachary Amsden wrote:
> SMP bootstrapping support.  Just as in the physical platform model,
> the BSP is responsible for initializing the AP state prior to execution.
> The dependence on lots of processor state information is a design choice
> of our implementation.  Conceivably, this could be a hypercall that
> awakens the same start of day state on APs as on the BSP.
> 
> It is likely the AP startup and the start-of-day model will eventually
> merge into a more common interface.
> 
> Signed-off-by: Zachary Amsden <zach@xxxxxxxxxx>
> Signed-off-by: Daniel Arai <arai@xxxxxxxxxx>

I have to admit booting virtual CPUs is easy compared to booting real
ones :-).

> +#include <asm/io.h>
> +#include <asm/highmem.h>
> +#include <asm/pgtable.h>
> +#include <vmi.h>

How it is possible that vmi.h is included without path?

> +APState ap;

Please don't hide structs like this.
> +static __init int no_ipi_broadcast(char *str)
> +{
> +     get_option(&str, &no_broadcast);
> +     printk ("Using %s mode\n", no_broadcast ? "No IPI Broadcast" :
> +                                                                             
>         "IPI Broadcast");

Excesive number of spaces,             I'          d            say .

> @@ -0,0 +1,51 @@
> +/* 
> + * include/asm-i386/mach-default/smpboot_hooks.h
> + *
> + * Portions Copyright 2005 VMware, Inc.
> + */

Whose are the other portions?


> +static inline void smpboot_restore_warm_reset_vector(void)
> +{
> +     /*
> +      * Install writable page 0 entry to set BIOS data area.
> +      */
> +     local_flush_tlb();

Code does not seem to match the comment.

> +/*
> + * The following vectors are part of the Linux architecture, there
> + * is no hardware IRQ pin equivalent for them, they are triggered
> + * through the ICC by us (IPIs)
> + */
> +#ifdef CONFIG_X86_SMP
> +BUILD_INTERRUPT(reschedule_interrupt,RESCHEDULE_VECTOR)
> +BUILD_INTERRUPT(invalidate_interrupt,INVALIDATE_TLB_VECTOR)
> +BUILD_INTERRUPT(call_function_interrupt,CALL_FUNCTION_VECTOR)
> +#endif

How is it different from CONFIG_SMP? Also please add " " after ",".

> +/*
> + * every pentium local APIC has two 'local interrupts', with a

"Every Pentium"

> + * soft-definable vector attached to both interrupts, one of
> + * which is a timer interrupt, the other one is error counter
> + * overflow. Linux uses the local APIC timer interrupt to get
> + * a much simpler SMP time architecture:
> + */

                                                        Pavel

-- 
184:        br = new BinaryReader( fs );

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