Xen devel - May 2007 - [PATCH] 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 <jbeulich@novell.com>

Index: 2007-05-14/xen/arch/x86/hvm/hvm.c
==================================================================---
2007-05-14.orig/xen/arch/x86/hvm/hvm.c	2007-05-14 08:28:38.000000000 +0200
+++ 2007-05-14/xen/arch/x86/hvm/hvm.c	2007-05-14 13:47:02.000000000 +0200
@@ -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;




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

On 14/5/07 16:23, "Jan Beulich" <jbeulich@novell.com> wrote:
> Latch guest shared info format when hypercall page is being set up rather
> than during an arbitrarily selected hypercall.
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
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



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

>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 15.05.07 08:59
>>>
>On 14/5/07 16:23, "Jan Beulich" <jbeulich@novell.com> wrote:
>
>> Latch guest shared info format when hypercall page is being set up
rather
>> than during an arbitrarily selected hypercall.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>
>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.
>We could sample the execution mode in *both* places (new and old)?
Even if it gets fixed in the drivers?

Jan


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

On 15/5/07 08:13, "Jan Beulich" <jbeulich@novell.com> wrote:
>> 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.
>> We could sample the execution mode in *both* places (new and old)?
> 
> Even if it gets fixed in the drivers?
Is that unreasonable?

 -- Keir



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

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


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

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 <raj.subrahmanian@unisys.com>

Thanks
Raj


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

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

On 18/5/07 20:20, "Subrahmanian, Raj"
<raj.subrahmanian@unisys.com> wrote:
> 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 <raj.subrahmanian@unisys.com>
> 
> Thanks
> Raj


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

>-----Original Message-----
>From: Keir Fraser [mailto:Keir.Fraser@cl.cam.ac.uk] 
>Sent: Saturday, May 19, 2007 4:20 AM
>To: Subrahmanian, Raj; xen-devel@lists.xensource.com
>Subject: Re: [PATCH] calling zap_low_mappings breaks xen boot 
>on the es7000
>> 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
<raj.subrahmanian@unisys.com>
>> 
>> 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 


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

On 23/5/07 22:16, "Subrahmanian, Raj"
<raj.subrahmanian@unisys.com> wrote:
>> 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


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

Keir,
>>> 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.
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

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

On 25/5/07 13:24, "Subrahmanian, Raj"
<raj.subrahmanian@unisys.com> wrote:
>> 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


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

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


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

On 14/6/07 18:58, "Subrahmanian, Raj"
<raj.subrahmanian@unisys.com> wrote:
> 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


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