Mailing List Archive: [PATCH] tools/hvmloader: move shared

[PATCH] tools/hvmloader: move shared_info to reserved memory area

Oct 24, 2012, 10:57 AM

Post #1 of 13 (304 views)

# 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();

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

Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area [ In reply to ]

keir.xen at gmail

Oct 24, 2012, 12:05 PM

Post #2 of 13 (287 views)

Permalink

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

Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area [ In reply to ]

olaf at aepfle

Oct 25, 2012, 12:42 AM

Post #3 of 13 (278 views)

Permalink

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

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

Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area [ In reply to ]

olaf at aepfle

Oct 25, 2012, 12:51 AM

Post #4 of 13 (280 views)

Permalink

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

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

Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area [ In reply to ]

keir.xen at gmail

Oct 25, 2012, 4:33 AM

Post #5 of 13 (280 views)

Permalink

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

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

Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area [ In reply to ]

keir.xen at gmail

Oct 25, 2012, 4:39 AM

Post #6 of 13 (279 views)

Permalink

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

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

Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area [ In reply to ]

olaf at aepfle

Oct 25, 2012, 4:46 AM

Post #7 of 13 (278 views)

Permalink

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

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

Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area [ In reply to ]

Ian.Campbell at citrix

Oct 25, 2012, 5:10 AM

Post #8 of 13 (283 views)

Permalink

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.

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

Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area [ In reply to ]

olaf at aepfle

Oct 25, 2012, 5:16 AM

Post #9 of 13 (281 views)

Permalink

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

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

Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area [ In reply to ]

konrad at kernel

Oct 26, 2012, 4:58 AM

Post #10 of 13 (277 views)

Permalink

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

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

Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area [ In reply to ]

olaf at aepfle

Oct 26, 2012, 7:08 AM

Post #11 of 13 (278 views)

Permalink

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

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

Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area [ In reply to ]

olaf at aepfle

Oct 26, 2012, 8:51 AM

Post #12 of 13 (278 views)

Permalink

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)

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

Re: [PATCH] tools/hvmloader: move shared_info to reserved memory area [ In reply to ]

keir.xen at gmail

Oct 26, 2012, 9:08 AM

Post #13 of 13 (278 views)

Permalink

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)

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

Mailing List Archive

Mailing List Archive

Attached Files: