paravirt & xen & SMP

paravirt & xen & SMP

[Gerd Hoffmann]

  Hi,

Anyone has this working?  Looks like there is a chicken-and-egg issue
with pda setup:

  * xen_load_gdt() uses multicalls, thus depends on cpu-specific
    variables (per-cpu mc buffer) which in turn requires pda being
    setup already.
  * pda setup can't be done before xen_load_gdt() ...

Next question while looking at xen_load_gdt(): why does it use
multicalls in the first place?  Seems that function puts only one
hypercall into the multicall array, so there is no obvious point in
using multicalls ...

cheers,
  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>

Re: paravirt & xen & SMP

[Gerd Hoffmann]

[-- Attachment #1: Type: text/plain, Size: 372 bytes --]

Chuck Short wrote:
> Hi Gerd,
> 
> I was wondering how you got it to work this far?

It's almost at userspace now (virtual machine is UP only, kernel built
with CONFIG_SMP=y though), just by avoiding multicalls in the boot path
before pda being setup.  Patch attached, goes on top of the complete
paravirt patch queue.

cheers,

  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>

[-- Attachment #2: kraxel.buildfix --]
[-- Type: text/plain, Size: 3854 bytes --]

---
 arch/i386/paravirt-xen/enlighten.c |   28 +++++++++++++++++++++++++++-
 drivers/Makefile                   |    2 +-
 include/asm-i386/paravirt.h        |   20 ++++++++++----------
 3 files changed, 38 insertions(+), 12 deletions(-)

Index: paravirt-2.6.20-rc4-hg691/include/asm-i386/paravirt.h
===================================================================
--- paravirt-2.6.20-rc4-hg691.orig/include/asm-i386/paravirt.h
+++ paravirt-2.6.20-rc4-hg691/include/asm-i386/paravirt.h
@@ -493,6 +493,16 @@ static inline void apic_write_atomic(uns
 	paravirt_ops.apic_write_atomic(reg,v);
 }
 unsigned long apic_read(unsigned long reg);
+
+static inline void setup_boot_clock(void)
+{
+	paravirt_ops.setup_boot_clock();
+}
+
+static inline void setup_secondary_clock(void)
+{
+	paravirt_ops.setup_secondary_clock();
+}
 #endif
 
 /* These will be unexported once raid6 is fixed... */
@@ -525,16 +535,6 @@ static inline void pmd_clear(pmd_t *pmdp
 {
 	paravirt_ops.pmd_clear(pmdp);
 }
-
-static inline void setup_boot_clock(void)
-{
-	paravirt_ops.setup_boot_clock();
-}
-
-static inline void setup_secondary_clock(void)
-{
-	paravirt_ops.setup_secondary_clock();
-}
 #endif
 
 /* Lazy mode for batching updates / context switch */
Index: paravirt-2.6.20-rc4-hg691/arch/i386/paravirt-xen/enlighten.c
===================================================================
--- paravirt-2.6.20-rc4-hg691.orig/arch/i386/paravirt-xen/enlighten.c
+++ paravirt-2.6.20-rc4-hg691/arch/i386/paravirt-xen/enlighten.c
@@ -240,6 +240,25 @@ static fastcall void xen_load_gdt(const 
 		xen_mc_flush();
 }
 
+static fastcall void xen_load_gdt_boot(const struct Xgt_desc_struct *dtr)
+{
+        unsigned long va;
+        int f;
+	unsigned size = dtr->size + 1;
+	unsigned long frames[16];
+
+	BUG_ON(size > 16*PAGE_SIZE);
+
+        for (va = dtr->address, f = 0;
+             va < dtr->address + size;
+             va += PAGE_SIZE, f++) {
+                frames[f] = virt_to_mfn(va);
+		make_lowmem_page_readonly((void *)va);
+        }
+
+	HYPERVISOR_set_gdt(frames, size/8);
+}
+
 static void load_TLS_descriptor(struct thread_struct *t,
 				unsigned int cpu, unsigned int i)
 {
@@ -522,6 +541,7 @@ static fastcall void xen_alloc_pd(u32 pf
 static fastcall void xen_release_pd(u32 pfn)
 {
 	make_lowmem_page_readwrite(__va(PFN_PHYS(pfn)));
+	memset(__va(PFN_PHYS(pfn)), 0, PAGE_SIZE);
 }
 
 static fastcall void xen_release_pt(u32 pfn)
@@ -757,12 +777,18 @@ static asmlinkage void __init xen_start_
 	/* set up the boot-time gdt and segments */
 	init_mm.pgd = pgd; /* use the Xen pagetables to start */
 
-	xen_load_gdt(&cpu_gdt_descr);
+	xen_load_gdt_boot(&cpu_gdt_descr);
 
 	/* set up PDA descriptor */
 	pack_descriptor(&low, &high, (unsigned)&boot_pda, sizeof(boot_pda)-1,
 			0x80 | DESCTYPE_S | 0x02, 0);
+#if 0
 	xen_write_gdt_entry(cpu_gdt_table, GDT_ENTRY_PDA, low, high);
+#else
+	if (HYPERVISOR_update_descriptor(virt_to_machine(cpu_gdt_table + GDT_ENTRY_PDA).maddr,
+					 (u64)high << 32 | low))
+		BUG();
+#endif
 
 	/* set up %gs and init Xen parts of the PDA */
 	asm volatile("mov %0, %%gs" : : "r" (__KERNEL_PDA) : "memory");
Index: paravirt-2.6.20-rc4-hg691/drivers/Makefile
===================================================================
--- paravirt-2.6.20-rc4-hg691.orig/drivers/Makefile
+++ paravirt-2.6.20-rc4-hg691/drivers/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_ARM_AMBA)		+= amba/
 # char/ comes before serial/ etc so that the VT console is the boot-time
 # default.
 obj-y				+= char/
+obj-$(CONFIG_XEN)		+= xen/
 
 obj-$(CONFIG_CONNECTOR)		+= connector/
 
@@ -31,7 +32,6 @@ obj-y				+= base/ block/ misc/ mfd/ net/
 obj-$(CONFIG_NUBUS)		+= nubus/
 obj-$(CONFIG_ATM)		+= atm/
 obj-$(CONFIG_PPC_PMAC)		+= macintosh/
-obj-$(CONFIG_XEN)		+= xen/
 obj-$(CONFIG_IDE)		+= ide/
 obj-$(CONFIG_FC4)		+= fc4/
 obj-$(CONFIG_SCSI)		+= scsi/

[-- Attachment #3: Type: text/plain, Size: 165 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/virtualization

Re: paravirt & xen & SMP

[Jeremy Fitzhardinge]

Gerd Hoffmann wrote:
> Anyone has this working?  Looks like there is a chicken-and-egg issue
> with pda setup:
>
>   * xen_load_gdt() uses multicalls, thus depends on cpu-specific
>     variables (per-cpu mc buffer) which in turn requires pda being
>     setup already.
>   * pda setup can't be done before xen_load_gdt() ...
>   

Good point.  I haven't done any SMP work yet, so if anything works at
all it is pure luck.

> Next question while looking at xen_load_gdt(): why does it use
> multicalls in the first place?  Seems that function puts only one
> hypercall into the multicall array, so there is no obvious point in
> using multicalls ...
>   

I think you're right; I probably converted it when I swept through and
did the others without really thinking about the bootstrap consequences.

    J

Re: paravirt & xen & SMP

[Gerd Hoffmann]

Jeremy Fitzhardinge wrote:
>> Next question while looking at xen_load_gdt(): why does it use
>> multicalls in the first place?  Seems that function puts only one
>> hypercall into the multicall array, so there is no obvious point in
>> using multicalls ...
> 
> I think you're right; I probably converted it when I swept through and
> did the others without really thinking about the bootstrap consequences.

Well, after reading the code a bit more it seems the intention is to
call the function in a row with others, have stuff queued up (in case
lazy_mode == PARAVIRT_LAZY_CPU is set) and update everything in one go
then.  I don't think that makes sense for that function as it isn't
performance-critical.  It is called on boot and cpu bringup (well, not
yet) only, right?

We have a simliar issue for xen_write_gdt_entry() btw, it calls
mc_flush() which doesn't work too.  I've fixed it that way:

@@ -761,7 +762,13 @@ static asmlinkage void __init xen_start_
        /* set up PDA descriptor */
        pack_descriptor(&low, &high, (unsigned)&boot_pda,
sizeof(boot_pda)-1,
                        0x80 | DESCTYPE_S | 0x02, 0);
+#if 0
        xen_write_gdt_entry(cpu_gdt_table, GDT_ENTRY_PDA, low, high);
+#else
+       if (HYPERVISOR_update_descriptor(virt_to_machine(cpu_gdt_table +
GDT_ENTRY_PDA).maddr,
+                                        (u64)high << 32 | low))
+               BUG();
+#endif

        /* set up %gs and init Xen parts of the PDA */
        asm volatile("mov %0, %%gs" : : "r" (__KERNEL_PDA) : "memory");

[ gna, thunderbird isn't great for sending patches inline, you should
  get the idea though ... ]

cheers,
  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>

Re: paravirt & xen & SMP

[Jeremy Fitzhardinge]

Gerd Hoffmann wrote:
> Well, after reading the code a bit more it seems the intention is to
> call the function in a row with others, have stuff queued up (in case
> lazy_mode == PARAVIRT_LAZY_CPU is set) and update everything in one go
> then.  I don't think that makes sense for that function as it isn't
> performance-critical.  It is called on boot and cpu bringup (well, not
> yet) only, right?
>   

Yes, that's right.  While its not performance critical in itself, the
idea was that if it were in a stream of other deferred cpu-state
updates, then it should happen in the appropriate order.   But in
practice this is mainly to make a context switch a single hypercall, so
the most important ones are stack-switch followed by 3x
update_descriptor (for TLS).

> We have a simliar issue for xen_write_gdt_entry() btw, it calls
> mc_flush() which doesn't work too.  I've fixed it that way:
>
> @@ -761,7 +762,13 @@ static asmlinkage void __init xen_start_
>         /* set up PDA descriptor */
>         pack_descriptor(&low, &high, (unsigned)&boot_pda,
> sizeof(boot_pda)-1,
>                         0x80 | DESCTYPE_S | 0x02, 0);
> +#if 0
>         xen_write_gdt_entry(cpu_gdt_table, GDT_ENTRY_PDA, low, high);
> +#else
> +       if (HYPERVISOR_update_descriptor(virt_to_machine(cpu_gdt_table +
> GDT_ENTRY_PDA).maddr,
> +                                        (u64)high << 32 | low))
> +               BUG();
> +#endif
>
>         /* set up %gs and init Xen parts of the PDA */
>         asm volatile("mov %0, %%gs" : : "r" (__KERNEL_PDA) : "memory");
>
> [ gna, thunderbird isn't great for sending patches inline, you should
>   get the idea though ... ]
>   

Yeah, that's ugly, but I guess it will do for now.

(You can inline patches without damage in thunderbird if you paste them
as "preformat")

    J