xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tools/hvmloader: move shared_info to reserved memory area
@ 2012-10-24 17:57 Olaf Hering
  2012-10-24 19:05 ` Keir Fraser
  0 siblings, 1 reply; 13+ messages in thread
From: Olaf Hering @ 2012-10-24 17:57 UTC (permalink / raw)
  To: xen-devel

# HG changeset patch
# User Olaf Hering <olaf@aepfle.de>
# Date 1351101387 -7200
# Node ID 6a0c73ae9ce5cca72f788c0e0f8fd6872010d83e
# Parent  22e08c9ac770db07c3c3e7c844aa7153050939f3
tools/hvmloader: move shared_info to reserved memory area

Reserve a range of 1MB for the HVM shared info page at 0xFE700000. This
area is already marked as reserved in the E820 map. The purpose of this
change is to provide Linux PVonHVM guests with a fixed page outside of
ordinary RAM. If the shared page is located in RAM it would be
overwritten during a kexec boot.

Signed-off-by: Olaf Hering <olaf@aepfle.de>

diff -r 22e08c9ac770 -r 6a0c73ae9ce5 tools/firmware/hvmloader/config.h
--- a/tools/firmware/hvmloader/config.h
+++ b/tools/firmware/hvmloader/config.h
@@ -68,6 +68,8 @@ extern unsigned long pci_mem_start, pci_
 /* NB. ACPI_INFO_PHYSICAL_ADDRESS *MUST* match definition in acpi/dsdt.asl! */
 #define ACPI_INFO_PHYSICAL_ADDRESS    0xFC000000
 #define RESERVED_MEMORY_DYNAMIC       0xFC001000
+#define HVM_SHARED_INFO_AREA          0xFE700000
+#define HVM_SHARED_INFO_SIZE          0x00100000
 
 extern unsigned long scratch_start;
 
diff -r 22e08c9ac770 -r 6a0c73ae9ce5 tools/firmware/hvmloader/util.c
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -433,11 +433,18 @@ void *mem_alloc(uint32_t size, uint32_t 
     if ( align < 16 )
         align = 16;
 
+retry:
     s = (reserve + align) & ~(align - 1);
     e = s + size - 1;
 
     BUG_ON((e < s) || (e >> PAGE_SHIFT) >= hvm_info->reserved_mem_pgstart);
 
+    /* Skip the shared info region */
+    if (s <= HVM_SHARED_INFO_AREA && e >= HVM_SHARED_INFO_AREA) {
+	    reserve = HVM_SHARED_INFO_AREA + HVM_SHARED_INFO_SIZE - 1;
+	    goto retry;
+    }
+
     while ( (reserve >> PAGE_SHIFT) != (e >> PAGE_SHIFT) )
     {
         reserve += PAGE_SIZE;
@@ -765,7 +772,7 @@ struct shared_info *get_shared_info(void
     xatp.domid = DOMID_SELF;
     xatp.space = XENMAPSPACE_shared_info;
     xatp.idx   = 0;
-    xatp.gpfn  = mem_hole_alloc(1);
+    xatp.gpfn  = HVM_SHARED_INFO_AREA >> PAGE_SHIFT;
     shared_info = (struct shared_info *)(xatp.gpfn << PAGE_SHIFT);
     if ( hypercall_memory_op(XENMEM_add_to_physmap, &xatp) != 0 )
         BUG();

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area
  2012-10-24 17:57 [PATCH] tools/hvmloader: move shared_info to reserved memory area Olaf Hering
@ 2012-10-24 19:05 ` Keir Fraser
  2012-10-25  7:42   ` Olaf Hering
  0 siblings, 1 reply; 13+ messages in thread
From: Keir Fraser @ 2012-10-24 19:05 UTC (permalink / raw)
  To: Olaf Hering, xen-devel

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

On 24/10/2012 10:57, "Olaf Hering" <olaf@aepfle.de> wrote:

> # HG changeset patch
> # User Olaf Hering <olaf@aepfle.de>
> # Date 1351101387 -7200
> # Node ID 6a0c73ae9ce5cca72f788c0e0f8fd6872010d83e
> # Parent  22e08c9ac770db07c3c3e7c844aa7153050939f3
> tools/hvmloader: move shared_info to reserved memory area
> 
> Reserve a range of 1MB for the HVM shared info page at 0xFE700000. This
> area is already marked as reserved in the E820 map. The purpose of this
> change is to provide Linux PVonHVM guests with a fixed page outside of
> ordinary RAM. If the shared page is located in RAM it would be
> overwritten during a kexec boot.

I don't think hvmloader actually needs to map shared-info to the new
location, it justs needs to guarantee that this location is unused, and
document it so that it never *becomes* used in future.

Which can be as simple as the attached patch (in fact all the changes apart
from introducing GUEST_RESERVED_{START,END} are really cleaning up and
bug-fixing the out-of-space checks in the mem_hole_alloc/mem_alloc
functions).

This then just requires that the guest maps shared-info to FE700000 itself.
Should be quite easy. :)

 -- Keir

> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> diff -r 22e08c9ac770 -r 6a0c73ae9ce5 tools/firmware/hvmloader/config.h
> --- a/tools/firmware/hvmloader/config.h
> +++ b/tools/firmware/hvmloader/config.h
> @@ -68,6 +68,8 @@ extern unsigned long pci_mem_start, pci_
>  /* NB. ACPI_INFO_PHYSICAL_ADDRESS *MUST* match definition in acpi/dsdt.asl!
> */
>  #define ACPI_INFO_PHYSICAL_ADDRESS    0xFC000000
>  #define RESERVED_MEMORY_DYNAMIC       0xFC001000
> +#define HVM_SHARED_INFO_AREA          0xFE700000
> +#define HVM_SHARED_INFO_SIZE          0x00100000
>  
>  extern unsigned long scratch_start;
>  
> diff -r 22e08c9ac770 -r 6a0c73ae9ce5 tools/firmware/hvmloader/util.c
> --- a/tools/firmware/hvmloader/util.c
> +++ b/tools/firmware/hvmloader/util.c
> @@ -433,11 +433,18 @@ void *mem_alloc(uint32_t size, uint32_t
>      if ( align < 16 )
>          align = 16;
>  
> +retry:
>      s = (reserve + align) & ~(align - 1);
>      e = s + size - 1;
>  
>      BUG_ON((e < s) || (e >> PAGE_SHIFT) >= hvm_info->reserved_mem_pgstart);
>  
> +    /* Skip the shared info region */
> +    if (s <= HVM_SHARED_INFO_AREA && e >= HVM_SHARED_INFO_AREA) {
> +     reserve = HVM_SHARED_INFO_AREA + HVM_SHARED_INFO_SIZE - 1;
> +     goto retry;
> +    }
> +
>      while ( (reserve >> PAGE_SHIFT) != (e >> PAGE_SHIFT) )
>      {
>          reserve += PAGE_SIZE;
> @@ -765,7 +772,7 @@ struct shared_info *get_shared_info(void
>      xatp.domid = DOMID_SELF;
>      xatp.space = XENMAPSPACE_shared_info;
>      xatp.idx   = 0;
> -    xatp.gpfn  = mem_hole_alloc(1);
> +    xatp.gpfn  = HVM_SHARED_INFO_AREA >> PAGE_SHIFT;
>      shared_info = (struct shared_info *)(xatp.gpfn << PAGE_SHIFT);
>      if ( hypercall_memory_op(XENMEM_add_to_physmap, &xatp) != 0 )
>          BUG();
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


[-- Attachment #2: 00-hvmloader-reserved-mem --]
[-- Type: application/octet-stream, Size: 2494 bytes --]

diff -r 22e08c9ac770 tools/firmware/hvmloader/config.h
--- a/tools/firmware/hvmloader/config.h	Wed Oct 24 17:51:48 2012 +0200
+++ b/tools/firmware/hvmloader/config.h	Wed Oct 24 04:01:24 2012 -0700
@@ -67,7 +67,15 @@ extern unsigned long pci_mem_start, pci_
 #define RESERVED_MEMBASE              0xFC000000
 /* NB. ACPI_INFO_PHYSICAL_ADDRESS *MUST* match definition in acpi/dsdt.asl! */
 #define ACPI_INFO_PHYSICAL_ADDRESS    0xFC000000
-#define RESERVED_MEMORY_DYNAMIC       0xFC001000
+#define RESERVED_MEMORY_DYNAMIC_START 0xFC001000
+#define RESERVED_MEMORY_DYNAMIC_END   0xFE000000
+/*
+ * GUEST_RESERVED: Physical address space reserved for guest use.
+ * This is not dynamically advertised to guests, so this range must *never*
+ * be used for any purpose by us, in future.
+ */
+#define GUEST_RESERVED_START          0xFE700000
+#define GUEST_RESERVED_END            0xFE800000
 
 extern unsigned long scratch_start;
 
diff -r 22e08c9ac770 tools/firmware/hvmloader/util.c
--- a/tools/firmware/hvmloader/util.c	Wed Oct 24 17:51:48 2012 +0200
+++ b/tools/firmware/hvmloader/util.c	Wed Oct 24 04:01:24 2012 -0700
@@ -416,13 +416,14 @@ void mem_hole_populate_ram(xen_pfn_t mfn
     }
 }
 
-static uint32_t reserve = RESERVED_MEMORY_DYNAMIC - 1;
+static uint32_t alloc_up = RESERVED_MEMORY_DYNAMIC_START - 1;
+static uint32_t alloc_down = RESERVED_MEMORY_DYNAMIC_END;
 
 xen_pfn_t mem_hole_alloc(uint32_t nr_mfns)
 {
-    hvm_info->reserved_mem_pgstart -= nr_mfns;
-    BUG_ON(hvm_info->reserved_mem_pgstart <= (reserve >> PAGE_SHIFT));
-    return hvm_info->reserved_mem_pgstart;
+    alloc_down -= nr_mfns << PAGE_SHIFT;
+    BUG_ON(alloc_up >= alloc_down);
+    return alloc_down >> PAGE_SHIFT;
 }
 
 void *mem_alloc(uint32_t size, uint32_t align)
@@ -433,18 +434,18 @@ void *mem_alloc(uint32_t size, uint32_t 
     if ( align < 16 )
         align = 16;
 
-    s = (reserve + align) & ~(align - 1);
+    s = (alloc_up + align) & ~(align - 1);
     e = s + size - 1;
 
-    BUG_ON((e < s) || (e >> PAGE_SHIFT) >= hvm_info->reserved_mem_pgstart);
+    BUG_ON((e < s) || (e >= alloc_down));
 
-    while ( (reserve >> PAGE_SHIFT) != (e >> PAGE_SHIFT) )
+    while ( (alloc_up >> PAGE_SHIFT) != (e >> PAGE_SHIFT) )
     {
-        reserve += PAGE_SIZE;
-        mem_hole_populate_ram(reserve >> PAGE_SHIFT, 1);
+        alloc_up += PAGE_SIZE;
+        mem_hole_populate_ram(alloc_up >> PAGE_SHIFT, 1);
     }
 
-    reserve = e;
+    alloc_up = e;
 
     return (void *)(unsigned long)s;
 }

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

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area
  2012-10-24 19:05 ` Keir Fraser
@ 2012-10-25  7:42   ` Olaf Hering
  2012-10-25  7:51     ` Olaf Hering
  0 siblings, 1 reply; 13+ messages in thread
From: Olaf Hering @ 2012-10-25  7:42 UTC (permalink / raw)
  To: Keir Fraser; +Cc: xen-devel

On Wed, Oct 24, Keir Fraser wrote:

> Which can be as simple as the attached patch (in fact all the changes apart
> from introducing GUEST_RESERVED_{START,END} are really cleaning up and
> bug-fixing the out-of-space checks in the mem_hole_alloc/mem_alloc
> functions).
> 
> This then just requires that the guest maps shared-info to FE700000 itself.
> Should be quite easy. :)

The patch works for me. And the kernel patch I sent yesterday works as
well.
Is the memory area starting from 0xFC000000 also reserved in older
versions, such as Xen3?

Olaf

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area
  2012-10-25  7:42   ` Olaf Hering
@ 2012-10-25  7:51     ` Olaf Hering
  2012-10-25 11:33       ` Keir Fraser
  0 siblings, 1 reply; 13+ messages in thread
From: Olaf Hering @ 2012-10-25  7:51 UTC (permalink / raw)
  To: Keir Fraser; +Cc: xen-devel

On Thu, Oct 25, Olaf Hering wrote:

> On Wed, Oct 24, Keir Fraser wrote:
> 
> > Which can be as simple as the attached patch (in fact all the changes apart
> > from introducing GUEST_RESERVED_{START,END} are really cleaning up and
> > bug-fixing the out-of-space checks in the mem_hole_alloc/mem_alloc
> > functions).
> > 
> > This then just requires that the guest maps shared-info to FE700000 itself.
> > Should be quite easy. :)
> 
> The patch works for me. And the kernel patch I sent yesterday works as
> well.
> Is the memory area starting from 0xFC000000 also reserved in older
> versions, such as Xen3?

And if the guest runs on an older tool stack, is there a slim chance
that something allocated memory up to 0xFE700000?

Olaf

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area
  2012-10-25  7:51     ` Olaf Hering
@ 2012-10-25 11:33       ` Keir Fraser
  2012-10-25 11:39         ` Keir Fraser
  0 siblings, 1 reply; 13+ messages in thread
From: Keir Fraser @ 2012-10-25 11:33 UTC (permalink / raw)
  To: Olaf Hering; +Cc: xen-devel

On 25/10/2012 00:51, "Olaf Hering" <olaf@aepfle.de> wrote:

> On Thu, Oct 25, Olaf Hering wrote:
> 
>> On Wed, Oct 24, Keir Fraser wrote:
>> 
>>> Which can be as simple as the attached patch (in fact all the changes apart
>>> from introducing GUEST_RESERVED_{START,END} are really cleaning up and
>>> bug-fixing the out-of-space checks in the mem_hole_alloc/mem_alloc
>>> functions).
>>> 
>>> This then just requires that the guest maps shared-info to FE700000 itself.
>>> Should be quite easy. :)
>> 
>> The patch works for me. And the kernel patch I sent yesterday works as
>> well.
>> Is the memory area starting from 0xFC000000 also reserved in older
>> versions, such as Xen3?

It is marked as E820_RESERVED in the e820 map as far back as Xen-3.4.0
(released Spring 2009). Before that it was not covered by an e820 entry, and
there is a slim chance your guest kernel may decide to map something else at
FE700000 (PCI BAR remapping f.ex)?

> And if the guest runs on an older tool stack, is there a slim chance
> that something allocated memory up to 0xFE700000?

Again, back as far as at least Xen-3.4.0, nothing would ever have got mapped
at FE700000. Earlier than that, can't be as authoritative, but I think it's
very unlikely.

 -- Keir

> Olaf

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area
  2012-10-25 11:33       ` Keir Fraser
@ 2012-10-25 11:39         ` Keir Fraser
  2012-10-25 11:46           ` Olaf Hering
  0 siblings, 1 reply; 13+ messages in thread
From: Keir Fraser @ 2012-10-25 11:39 UTC (permalink / raw)
  To: Olaf Hering; +Cc: xen-devel

On 25/10/2012 04:33, "Keir Fraser" <keir.xen@gmail.com> wrote:

> On 25/10/2012 00:51, "Olaf Hering" <olaf@aepfle.de> wrote:
> 
>> On Thu, Oct 25, Olaf Hering wrote:
>> 
>>> On Wed, Oct 24, Keir Fraser wrote:
>>> 
>>>> Which can be as simple as the attached patch (in fact all the changes apart
>>>> from introducing GUEST_RESERVED_{START,END} are really cleaning up and
>>>> bug-fixing the out-of-space checks in the mem_hole_alloc/mem_alloc
>>>> functions).
>>>> 
>>>> This then just requires that the guest maps shared-info to FE700000 itself.
>>>> Should be quite easy. :)
>>> 
>>> The patch works for me. And the kernel patch I sent yesterday works as
>>> well.
>>> Is the memory area starting from 0xFC000000 also reserved in older
>>> versions, such as Xen3?
> 
> It is marked as E820_RESERVED in the e820 map as far back as Xen-3.4.0
> (released Spring 2009). Before that it was not covered by an e820 entry, and
> there is a slim chance your guest kernel may decide to map something else at
> FE700000 (PCI BAR remapping f.ex)?
> 
>> And if the guest runs on an older tool stack, is there a slim chance
>> that something allocated memory up to 0xFE700000?
> 
> Again, back as far as at least Xen-3.4.0, nothing would ever have got mapped
> at FE700000. Earlier than that, can't be as authoritative, but I think it's
> very unlikely.

To be honest, given that the XenPVHVM extensions to Linux won't have been
tested on such old hypervisors, it wouldn't be a bad thing to bail on
setting up the extensions when you detect running on a really old Xen
version (e.g., earlier than 3.4.0) anyway. There's a fair chance of doing
more harm than good?

 -- Keir

>  -- Keir
> 
>> Olaf
> 
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area
  2012-10-25 11:39         ` Keir Fraser
@ 2012-10-25 11:46           ` Olaf Hering
  2012-10-25 12:10             ` Ian Campbell
  2012-10-26 11:58             ` Konrad Rzeszutek Wilk
  0 siblings, 2 replies; 13+ messages in thread
From: Olaf Hering @ 2012-10-25 11:46 UTC (permalink / raw)
  To: Keir Fraser, Konrad Rzeszutek Wilk; +Cc: xen-devel

On Thu, Oct 25, Keir Fraser wrote:

> To be honest, given that the XenPVHVM extensions to Linux won't have been
> tested on such old hypervisors, it wouldn't be a bad thing to bail on
> setting up the extensions when you detect running on a really old Xen
> version (e.g., earlier than 3.4.0) anyway. There's a fair chance of doing
> more harm than good?

I could stick such a check into
arch/x86/xen/enlighten.c:x86_hyper_xen_hvm->detect, by rearranging the
code of xen_hvm_platform and init_hvm_pv_info.  Konrad, what do you
think about that? Recent changes indicate that you did some testing on
3.4 based hosts.

Olaf

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area
  2012-10-25 11:46           ` Olaf Hering
@ 2012-10-25 12:10             ` Ian Campbell
  2012-10-25 12:16               ` Olaf Hering
  2012-10-26 11:58             ` Konrad Rzeszutek Wilk
  1 sibling, 1 reply; 13+ messages in thread
From: Ian Campbell @ 2012-10-25 12:10 UTC (permalink / raw)
  To: Olaf Hering; +Cc: Konrad Rzeszutek Wilk, Keir Fraser, xen-devel@lists.xen.org

On Thu, 2012-10-25 at 12:46 +0100, Olaf Hering wrote:
> On Thu, Oct 25, Keir Fraser wrote:
> 
> > To be honest, given that the XenPVHVM extensions to Linux won't have been
> > tested on such old hypervisors, it wouldn't be a bad thing to bail on
> > setting up the extensions when you detect running on a really old Xen
> > version (e.g., earlier than 3.4.0) anyway. There's a fair chance of doing
> > more harm than good?
> 
> I could stick such a check into
> arch/x86/xen/enlighten.c:x86_hyper_xen_hvm->detect, by rearranging the
> code of xen_hvm_platform and init_hvm_pv_info.  Konrad, what do you
> think about that?

If you do something like this please could you add a command line option
to allow it to be forced both off and on?

I suppose this relates a bit to xen_emul_unplug too?

> Recent changes indicate that you did some testing on
> 3.4 based hosts.

NB Keir said "earlier than 3.4". I think we would want to try and
support 3.4, but not 3.3.

Ian.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area
  2012-10-25 12:10             ` Ian Campbell
@ 2012-10-25 12:16               ` Olaf Hering
  0 siblings, 0 replies; 13+ messages in thread
From: Olaf Hering @ 2012-10-25 12:16 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Konrad Rzeszutek Wilk, Keir Fraser, xen-devel@lists.xen.org

On Thu, Oct 25, Ian Campbell wrote:

> NB Keir said "earlier than 3.4". I think we would want to try and
> support 3.4, but not 3.3.

Absolutely.

Olaf

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area
  2012-10-25 11:46           ` Olaf Hering
  2012-10-25 12:10             ` Ian Campbell
@ 2012-10-26 11:58             ` Konrad Rzeszutek Wilk
  2012-10-26 14:08               ` Olaf Hering
  1 sibling, 1 reply; 13+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-10-26 11:58 UTC (permalink / raw)
  To: Olaf Hering; +Cc: Keir Fraser, xen-devel

On Thu, Oct 25, 2012 at 01:46:45PM +0200, Olaf Hering wrote:
> On Thu, Oct 25, Keir Fraser wrote:
> 
> > To be honest, given that the XenPVHVM extensions to Linux won't have been
> > tested on such old hypervisors, it wouldn't be a bad thing to bail on
> > setting up the extensions when you detect running on a really old Xen
> > version (e.g., earlier than 3.4.0) anyway. There's a fair chance of doing
> > more harm than good?
> 
> I could stick such a check into
> arch/x86/xen/enlighten.c:x86_hyper_xen_hvm->detect, by rearranging the
> code of xen_hvm_platform and init_hvm_pv_info.  Konrad, what do you
> think about that? Recent changes indicate that you did some testing on
> 3.4 based hosts.

Sure. It might make sense to streamline this a bit. Meaning after the
version check, have an int (or function) called 'xen_old_hypervisor()'
which your code and the XenBus code could call?

That way on ARM that function can just become a nop while on X86 we
do the cpuids (or if those had already been done - we just return
true or false).

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area
  2012-10-26 11:58             ` Konrad Rzeszutek Wilk
@ 2012-10-26 14:08               ` Olaf Hering
  2012-10-26 15:51                 ` Olaf Hering
  0 siblings, 1 reply; 13+ messages in thread
From: Olaf Hering @ 2012-10-26 14:08 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: Keir Fraser, xen-devel

On Fri, Oct 26, Konrad Rzeszutek Wilk wrote:

> On Thu, Oct 25, 2012 at 01:46:45PM +0200, Olaf Hering wrote:
> > On Thu, Oct 25, Keir Fraser wrote:
> > 
> > > To be honest, given that the XenPVHVM extensions to Linux won't have been
> > > tested on such old hypervisors, it wouldn't be a bad thing to bail on
> > > setting up the extensions when you detect running on a really old Xen
> > > version (e.g., earlier than 3.4.0) anyway. There's a fair chance of doing
> > > more harm than good?
> > 
> > I could stick such a check into
> > arch/x86/xen/enlighten.c:x86_hyper_xen_hvm->detect, by rearranging the
> > code of xen_hvm_platform and init_hvm_pv_info.  Konrad, what do you
> > think about that? Recent changes indicate that you did some testing on
> > 3.4 based hosts.
> 
> Sure. It might make sense to streamline this a bit. Meaning after the
> version check, have an int (or function) called 'xen_old_hypervisor()'
> which your code and the XenBus code could call?

Oh, my take would be to return false in xen_hvm_platform if the
hypervisor is 3.3 or older, so that the guest does not use the PVonHVM
extensions.

Olaf

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area
  2012-10-26 14:08               ` Olaf Hering
@ 2012-10-26 15:51                 ` Olaf Hering
  2012-10-26 16:08                   ` Keir Fraser
  0 siblings, 1 reply; 13+ messages in thread
From: Olaf Hering @ 2012-10-26 15:51 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: Keir Fraser, xen-devel

On Fri, Oct 26, Olaf Hering wrote:

> Oh, my take would be to return false in xen_hvm_platform if the
> hypervisor is 3.3 or older, so that the guest does not use the PVonHVM
> extensions.

Like this untested patch:

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 0cc41f8..8566fa8 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1543,17 +1543,10 @@ static void __init xen_hvm_init_shared_info(void)
 
 static void __init init_hvm_pv_info(void)
 {
-	int major, minor;
 	uint32_t eax, ebx, ecx, edx, pages, msr, base;
 	u64 pfn;
 
 	base = xen_cpuid_base();
-	cpuid(base + 1, &eax, &ebx, &ecx, &edx);
-
-	major = eax >> 16;
-	minor = eax & 0xffff;
-	printk(KERN_INFO "Xen version %d.%d.\n", major, minor);
-
 	cpuid(base + 2, &pages, &msr, &ecx, &edx);
 
 	pfn = __pa(hypercall_page);
@@ -1604,13 +1597,29 @@ static void __init xen_hvm_guest_init(void)
 
 static bool __init xen_hvm_platform(void)
 {
+	int major, minor, old = 0;
+	uint32_t eax, ebx, ecx, edx, base;
+	bool usable = true;
+
 	if (xen_pv_domain())
 		return false;
 
-	if (!xen_cpuid_base())
+	base = xen_cpuid_base();
+	if (!base)
 		return false;
 
-	return true;
+	cpuid(base + 1, &eax, &ebx, &ecx, &edx);
+
+	major = eax >> 16;
+	minor = eax & 0xffff;
+
+	/* Require at least Xen 3.4 */
+	if (major < 3 || (major == 3 && minor < 4))
+		usable = false;
+	printk(KERN_INFO "Xen version %d.%d.%s\n",
+		major, minor, usable ? "" : " (too old)");
+
+	return usable;
 }
 
 bool xen_hvm_need_lapic(void)

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area
  2012-10-26 15:51                 ` Olaf Hering
@ 2012-10-26 16:08                   ` Keir Fraser
  0 siblings, 0 replies; 13+ messages in thread
From: Keir Fraser @ 2012-10-26 16:08 UTC (permalink / raw)
  To: Olaf Hering, Konrad Rzeszutek Wilk; +Cc: xen-devel

On 26/10/2012 08:51, "Olaf Hering" <olaf@aepfle.de> wrote:

> On Fri, Oct 26, Olaf Hering wrote:
> 
>> Oh, my take would be to return false in xen_hvm_platform if the
>> hypervisor is 3.3 or older, so that the guest does not use the PVonHVM
>> extensions.
> 
> Like this untested patch:

Looks okay to me.

 -- Keir

> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 0cc41f8..8566fa8 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1543,17 +1543,10 @@ static void __init xen_hvm_init_shared_info(void)
>  
>  static void __init init_hvm_pv_info(void)
>  {
> - int major, minor;
> uint32_t eax, ebx, ecx, edx, pages, msr, base;
> u64 pfn;
>  
> base = xen_cpuid_base();
> - cpuid(base + 1, &eax, &ebx, &ecx, &edx);
> -
> - major = eax >> 16;
> - minor = eax & 0xffff;
> - printk(KERN_INFO "Xen version %d.%d.\n", major, minor);
> -
> cpuid(base + 2, &pages, &msr, &ecx, &edx);
>  
> pfn = __pa(hypercall_page);
> @@ -1604,13 +1597,29 @@ static void __init xen_hvm_guest_init(void)
>  
>  static bool __init xen_hvm_platform(void)
>  {
> + int major, minor, old = 0;
> + uint32_t eax, ebx, ecx, edx, base;
> + bool usable = true;
> +
> if (xen_pv_domain())
> return false;
>  
> - if (!xen_cpuid_base())
> + base = xen_cpuid_base();
> + if (!base)
> return false;
>  
> - return true;
> + cpuid(base + 1, &eax, &ebx, &ecx, &edx);
> +
> + major = eax >> 16;
> + minor = eax & 0xffff;
> +
> + /* Require at least Xen 3.4 */
> + if (major < 3 || (major == 3 && minor < 4))
> +  usable = false;
> + printk(KERN_INFO "Xen version %d.%d.%s\n",
> +  major, minor, usable ? "" : " (too old)");
> +
> + return usable;
>  }
>  
>  bool xen_hvm_need_lapic(void)

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2012-10-26 16:08 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-24 17:57 [PATCH] tools/hvmloader: move shared_info to reserved memory area Olaf Hering
2012-10-24 19:05 ` Keir Fraser
2012-10-25  7:42   ` Olaf Hering
2012-10-25  7:51     ` Olaf Hering
2012-10-25 11:33       ` Keir Fraser
2012-10-25 11:39         ` Keir Fraser
2012-10-25 11:46           ` Olaf Hering
2012-10-25 12:10             ` Ian Campbell
2012-10-25 12:16               ` Olaf Hering
2012-10-26 11:58             ` Konrad Rzeszutek Wilk
2012-10-26 14:08               ` Olaf Hering
2012-10-26 15:51                 ` Olaf Hering
2012-10-26 16:08                   ` Keir Fraser

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).