Skip to contentSkip to conversation

x86/hvm: determine shared info format earlier

PATCH In review Jan Beulich 4 participants 13 messages May 14, 2007 – Jun 14, 2007

lore
Share & tools
lore

Jan Beulich

author
#0 lore raw

PATCH 1/1 x86/hvm: determine shared info format earlier

Latch guest shared info format when hypercall page is being set up rather than during an arbitrarily selected hypercall.

Signed-off-by Jan Beulich <[email protected]>
xen/arch/x86/hvm/hvm.c +9 -7
Index: 2007-05-14/xen/arch/x86/hvm/hvm.c
===================================================================
@@ -824,6 +824,15 @@ void hvm_update_guest_cr3(struct vcpu *v
 void hvm_hypercall_page_initialise(struct domain *d,
                                    void *hypercall_page)
 {
+#ifdef __x86_64__
+    /*
+     * Since this operation is one of the very first executed by PV drivers
+     * on initialisation or after save/restore, it is a sensible point at
+     * which to sample the execution mode of the guest and latch 32- or 64-
+     * bit format for shared state.
+     */
+    d->arch.has_32bit_shinfo = (hvm_guest_x86_mode(current) != 8);
+#endif
     hvm_funcs.init_hypercall_page(d, hypercall_page);
 }
 

@@ -1053,13 +1062,6 @@ long do_hvm_op(unsigned long op, XEN_GUE
                 break;
             case HVM_PARAM_CALLBACK_IRQ:
                 hvm_set_callback_via(d, a.value);
-                /*
-                 * Since this operation is one of the very first executed
-                 * by PV drivers on initialisation or after save/restore, it
-                 * is a sensible point at which to sample the execution mode of
-                 * the guest and latch 32- or 64-bit format for shared state.
-                 */
-                d->arch.has_32bit_shinfo = (hvm_guest_x86_mode(current) != 8);
                 break;
             }
             d->arch.hvm_domain.params[a.index] = a.value;
#1 lore raw
Quoted 4 lines · Jan Beulich

Latch guest shared info format when hypercall page is being set up rather than during an arbitrarily selected hypercall.

Signed-off-by Jan Beulich <[email protected]>

I don't think this works since the PV drivers don't re-create the hypercall page after save/restore (bad us!).

We could sample the execution mode in *both* places (new and old)?

-- Keir

author
#2 lore raw
Quoted 10 lines
Quoted 1 line
Quoted 1 line

Keir Fraser <[email protected]> 15.05.07 08:59 >>>

Quoted 4 lines · Jan Beulich

Latch guest shared info format when hypercall page is being set up rather than during an arbitrarily selected hypercall.

Signed-off-by Jan Beulich <[email protected]>

I don't think this works since the PV drivers don't re-create the hypercall page after save/restore (bad us!).

This must be fixed then. I don't think it's appropriate for them to assume the hypercall page doesn't change its layout across suspend.

Quoted 1 line

We could sample the execution mode in *both* places (new and old)?

Even if it gets fixed in the drivers?

Jan

#3 lore raw
Quoted 5 lines · Jan Beulich
Quoted 2 lines

I don't think this works since the PV drivers don't re-create the hypercall page after save/restore (bad us!).

This must be fixed then. I don't think it's appropriate for them to assume the hypercall page doesn't change its layout across suspend.

I agree.

Quoted 3 lines
Quoted 1 line

We could sample the execution mode in *both* places (new and old)?

Even if it gets fixed in the drivers?

Is that unreasonable?

-- Keir

author
#4 lore raw
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

We could sample the execution mode in *both* places (new and old)?

Even if it gets fixed in the drivers?

Is that unreasonable?

No, the current placement just looks very arbitrary, hence I would rather like to see it go away.

Jan

#5 lore raw

PATCH 1/1 calling zap_low_mappings breaks xen boot on the es7000

In reply to Keir Fraser

All,
Changeset 15044 fixes zap_low_mappings, and calls it from setup.c if on a x86_64 box.
This zaps some of the memory the ES7000 uses to talk to the Server Control (to setup APICs, for instance).
This patch executes the zap_low_mappings only if not on an ES7000.

Signed-off-by Rajagopalan Subrahmanian <[email protected]>

Thanks
Raj

# HG changeset patch
# User [email protected]
# Node ID 30b90beeb7d16e7134336abb07170ad07e765647
# Parent 089696e0c60379e2bb9cfcc988d35aa1bdc261de
Changeset 15044 fixes zap_low_mappings, and calls it from setup.c if on a x86_64 box. This zaps some of the memory the ES7000 uses to talk to the Server Control (to setup APICs, for instance). This patch executes the zap_low_mappings only if not on an ES7000.

Signed-off-by Rajagopalan Subrahmanian <[email protected]>
xen/arch/x86/setup.c +1 -1
diff -r 089696e0c603 -r 30b90beeb7d1 xen/arch/x86/setup.c
@@ -781,7 +781,7 @@ void __init __start_xen(multiboot_info_t
 
 #ifdef CONFIG_X86_64
     /* Low mappings were only needed for some BIOS table parsing. */
-    zap_low_mappings();
+    if(!es7000_plat) zap_low_mappings();/* ES7000 needs some of these mappings*/
 #endif
 
     init_apic_mappings();

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#6 lore raw

Re: [PATCH] calling zap_low_mappings breaks xen boot on the es7000

Either we should just delay the zap_low_mappings() call, or fix your code to map_domain_page()/unmap_domain_page(). How late in boot do you need the low mappings?

-- Keir

Quoted 10 lines · Raj

All,
Changeset 15044 fixes zap_low_mappings, and calls it from setup.c if on a x86_64 box.
This zaps some of the memory the ES7000 uses to talk to the Server Control (to setup APICs, for instance).
This patch executes the zap_low_mappings only if not on an ES7000.

Signed-off-by Rajagopalan Subrahmanian <[email protected]>

Thanks
Raj

#7 lore raw

RE: [PATCH] calling zap_low_mappings breaks xen boot on the es7000

Quoted 14 lines

-----Original Message-----
From: Keir Fraser [mailto:[email protected]]
Sent: Saturday, May 19, 2007 4:20 AM
To: Subrahmanian, Raj; [email protected]
Subject: Re: [PATCH] calling zap_low_mappings breaks xen boot on the es7000

Quoted 8 lines

All,
Changeset 15044 fixes zap_low_mappings, and calls it from setup.c if on a x86_64 box.
This zaps some of the memory the ES7000 uses to talk to the Server Control (to setup APICs, for instance).
This patch executes the zap_low_mappings only if not on an ES7000.

Signed-off-by Rajagopalan Subrahmanian <[email protected]>
Quoted 5 lines
Quoted 2 lines

Thanks
Raj

Either we should just delay the zap_low_mappings() call, or fix your code to map_domain_page()/unmap_domain_page(). How late in boot do you need the low mappings?

Keir.

This patch delays the zap_low_mappings() call until after the smp_prepare_cpus call. With this patch, I am able to run Xen on the ES7000 and a Dell SC 1430.

Raj

# HG changeset patch
# User [email protected]
# Date 1179926604 14400
# Node ID 0d8b1ec0486e36ae7029edeb65e9e3e8a9e40691
# Parent 089696e0c60379e2bb9cfcc988d35aa1bdc261de
Changeset 15044 fixes zap_low_mappings and calls it from setup.c. This zaps some of the low memory that the ES7000 uses to talk to server control. This patch postpones the calling of zap_low_mappings when xen will no longer need to talk to server control.

Signed-off-by Rajagopalan Subrahmanian <[email protected]>
xen/arch/x86/setup.c +4 -6
diff -r 089696e0c603 -r 0d8b1ec0486e xen/arch/x86/setup.c
@@ -779,11 +779,6 @@ void __init __start_xen(multiboot_info_t
     if ( smp_found_config )
         get_smp_config();
 
-#ifdef CONFIG_X86_64
-    /* Low mappings were only needed for some BIOS table parsing. */
-    zap_low_mappings();
-#endif
-
     init_apic_mappings();
 
     init_IRQ();

@@ -812,7 +807,10 @@ void __init __start_xen(multiboot_info_t
         max_cpus = 0;
 
     smp_prepare_cpus(max_cpus);
-
+#ifdef CONFIG_X86_64
+    /* Low mappings were only needed for some BIOS table parsing. */
+    zap_low_mappings();
+#endif
     /*
      * Initialise higher-level timer functions. We do this fairly late
      * (post-SMP) because the time bases and scale factors need to be updated

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#8 lore raw

Re: [PATCH] calling zap_low_mappings breaks xen boot on the es7000

Quoted 7 lines · Raj
Quoted 3 lines

Either we should just delay the zap_low_mappings() call, or fix your code to map_domain_page()/unmap_domain_page(). How late in boot do you need the low mappings?

This patch delays the zap_low_mappings() call until after the smp_prepare_cpus call. With this patch, I am able to run Xen on the ES7000 and a Dell SC 1430.

So where is the late low-memory access happening? There's not that much es7000-specific code. It can't be hard to fix properly. If the physical pointer is known to be below 1MB you can just __va() it. If not you may need to use a fixmap, or acpi_map_table, or just __va() only on x86_64. I'd like to keep the zap as early as possible to catch truly erroneous low pointers.

-- Keir

#9 lore raw

RE: [PATCH] calling zap_low_mappings breaks xen boot on the es7000

Keir,

Quoted 9 lines
Quoted 1 line
Quoted 1 line

Either we should just delay the zap_low_mappings() call, or

fix your

Quoted 6 lines
Quoted 2 lines

code to map_domain_page()/unmap_domain_page(). How late in boot do you need the low mappings?

This patch delays the zap_low_mappings() call until after the smp_prepare_cpus call. With this patch, I am able to run Xen on the ES7000 and a Dell SC 1430.

Quoted 7 lines

So where is the late low-memory access happening? There's not that much es7000-specific code. It can't be hard to fix properly. If the physical pointer is known to be below 1MB you can just __va() it. If not you may need to use a fixmap, or acpi_map_table, or just __va() only on x86_64. I'd like to keep the zap as early as possible to catch truly erroneous low pointers.

I will look into that code and fix it.
If you don't mind me asking, what exactly does zap_low_mappings do? I am not able to figure it out looking at the code.
Thanks
Raj

#10 lore raw

Re: RE: [PATCH] calling zap_low_mappings breaks xen boot on the es7000

Quoted 10 lines · Raj
Quoted 7 lines

So where is the late low-memory access happening? There's not that much es7000-specific code. It can't be hard to fix properly. If the physical pointer is known to be below 1MB you can just __va() it. If not you may need to use a fixmap, or acpi_map_table, or just __va() only on x86_64. I'd like to keep the zap as early as possible to catch truly erroneous low pointers.

I will look into that code and fix it.
If you don't mind me asking, what exactly does zap_low_mappings do? I am not able to figure it out looking at the code.

It removes the 1:1 mapping of physical memory at virtual address 0x0.

-- Keir

#11 lore raw

RE: [PATCH] calling zap_low_mappings breaks xen boot on the es7000

In reply to Keir Fraser

This patch tries to switch into APIC mode only on classic ES7000s. This is not required on the ES7000 Zorros.
With this patch, I can once again boot Xen on an ES7000, on the unstable tip.

Raj

# HG changeset patch
# User [email protected]
# Date 1181813822 14400
# Node ID 7bbb7b45a91cef067d3804615ff3c6e2f193d4b0 # Parent 93f77a5a8437df0b34919a6987f48e84342b4a43 This patch cleans up some of the ES7000 specific APIC code.

Signed-off-by Raj Subrahmanian <[email protected]>
xen/arch/x86/genapic/es7000plat.c +3 -3
diff -r 93f77a5a8437 -r 7bbb7b45a91c xen/arch/x86/genapic/es7000plat.c
@@ -298,13 +298,13 @@ es7000_stop_cpu(int cpu)
 
 void __init
 es7000_sw_apic()
-{
-	if (es7000_plat) {
+{	
+	if ((es7000_plat)&&es7000_plat!=ES7000_ZORRO) {
 		int mip_status;
 		struct mip_reg es7000_mip_reg;
 
 		printk("ES7000: Enabling APIC mode.\n");
-        	memset(&es7000_mip_reg, 0, sizeof(struct mip_reg));
+		memset(&es7000_mip_reg, 0, sizeof(struct mip_reg));
         	es7000_mip_reg.off_0 = MIP_SW_APIC;
         	es7000_mip_reg.off_38 = (MIP_VALID);
         	while ((mip_status = es7000_mip_write(&es7000_mip_reg)) != 0)

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#12 lore raw

Re: RE: [PATCH] calling zap_low_mappings breaks xen boot on the es7000

Quoted 4 lines · Raj

This patch tries to switch into APIC mode only on classic ES7000s. This is not required on the ES7000 Zorros.
With this patch, I can once again boot Xen on an ES7000, on the unstable tip.

Does this leave unstable tip broken on the older ES7000 though?

The proper fix would seem to be to find out which access is faulting, and to obtain the required virtual address by __va() or by map_domain_page() rather than relying on the phys==virt mapping. Or is the problem more tricky than that?

-- Keir