paravirt & xen & SMP

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 <[email protected]>

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 <[email protected]>

---
 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
===================================================================
@@ -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
===================================================================
@@ -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
===================================================================
@@ -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/

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/virtualization

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

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

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 <[email protected]>

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).

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