Re: [Xen-devel] [PATCH 0/3] xen: support of large memory maps

2017-03-22 Thread Alex Thorlton 
On Tue, Mar 21, 2017 at 02:10:20PM +0100, Juergen Gross wrote:
> This patch series is the first part for adding support of large EFI
> memory maps (> the current limit of 128 entries) while reducing
> trampoline size.
> 
> I'm not posting the final patch for making the trampoline size
> reduction effective in order not to add major rebase work to Daniel's
> multiboot2 series.
> 
> Juergen Gross (3):
>   xen/x86: split boot trampoline into permanent and temporary part
>   xen/x86: use trampoline e820 buffer for BIOS interface only
>   xen/x86: support larger memory map from EFI

I have tested these patches on a system with a large E820 table (~700
entries) and everything appears to work fine.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] Supporting systems with large E820 maps

2017-03-20 Thread Alex Thorlton 
Hey everyone,

Recently, I've been working with Boris Ostrovsky to get Xen running on
some of our larger systems, and we've run into a few problems with the
amount of space that Xen sets aside for the E820 map.

The first problem that I hit was that E820MAX is far too small, at 128
entries, for the system that we're testing with.  The EFI memory map
handed up from the boot loader tops out at 783 entries, which far
exceeds the amount of space allocated for the memory map in
arch/x86/boot/mem.S.  I was able to get past this problem by bumping
E820MAX up to 1024 in arch/x86/boot/mem.S and include/asm-x86/e820.h.

The second problem that I encountered was that Xen uses a signed char to
store the number of entries in the memory map in a few places, which is
too small to hold the number of entries after bumping E820MAX up to
1024.  I made the following changes to get past this:

8<---
---
 arch/x86/e820.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--- xen.orig/arch/x86/e820.c
+++ xen/arch/x86/e820.c
@@ -134,7 +134,7 @@ static struct change_member *change_poin
 static struct e820entry *overlap_list[E820MAX] __initdata;
 static struct e820entry new_bios[E820MAX] __initdata;

-static int __init sanitize_e820_map(struct e820entry * biosmap, char * pnr_map)
+static int __init sanitize_e820_map(struct e820entry * biosmap, unsigned int * 
pnr_map)
 {
 struct change_member *change_tmp;
 unsigned long current_type, last_type;
@@ -509,13 +509,13 @@ static void __init reserve_dmi_region(vo
 }
 }

-static void __init machine_specific_memory_setup(struct e820entry *raw, char 
*raw_nr)
+static void __init machine_specific_memory_setup(struct e820entry *raw, 
unsigned int *raw_nr)
 {
 unsigned long mpt_limit, ro_mpt_limit;
 uint64_t top_of_ram, size;
 int i;

-char nr = (char)*raw_nr;
+unsigned int nr = (unsigned int)*raw_nr;
 sanitize_e820_map(raw, );
 *raw_nr = nr;
 (void)copy_e820_map(raw, nr);
--->8

I didn't need to go all the way up to unsigned int here, but I did this
as a quick/dirty test to see if it got things working.

These small changes get our large machine to boot up and recognize all
32TB of available RAM.  I know that these changes are probably not what
we'll want to go with in the end, but I wanted to get them sent upstream
to get a dialogue started.

So, what do others think here?  How do we want to handle a large E820
map?  Boris mentioned to me that we might want to attempt to do a
dynamic allocation scheme, where we reserve more space for the memory
map when we detect that E820 is large.

Any comments/suggestions are greatly appreciated!

- Alex

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 1/2] x86: Make E820_X_MAX unconditionally larger than E820MAX

2016-12-05 Thread Alex Thorlton 
It's really not necessary to limit E820_X_MAX to 128 in the non-EFI
case.  This commit drops E820_X_MAX's dependency on CONFIG_EFI, so that
E820_X_MAX is always at least slightly larger than E820MAX.

The real motivation behind this is actually to prevent some issues in
the Xen kernel, where the XENMEM_machine_memory_map hypercall can
produce an e820 map larger than 128 entries, even on systems where the
original e820 table was quite a bit smaller than that, depending on how
many IOAPICs are installed on the system.

Signed-off-by: Alex Thorlton <athorl...@sgi.com>
Suggested-by: Ingo Molnar <mi...@redhat.com>
Cc: Russ Anderson <r...@sgi.com>
Cc: David Vrabel <david.vra...@citrix.com>
Cc: Ingo Molnar <mi...@redhat.com>
Cc: Juergen Gross <jgr...@suse.com>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: "H. Peter Anvin" <h...@zytor.com>
Cc: Denys Vlasenko <dvlas...@redhat.com>
Cc: Boris Ostrovsky <boris.ostrov...@oracle.com>
Cc: x...@kernel.org
Cc: xen-de...@lists.xenproject.org
---
 arch/x86/include/asm/e820.h | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/e820.h b/arch/x86/include/asm/e820.h
index 476b574..ec23d8e 100644
--- a/arch/x86/include/asm/e820.h
+++ b/arch/x86/include/asm/e820.h
@@ -1,13 +1,17 @@
 #ifndef _ASM_X86_E820_H
 #define _ASM_X86_E820_H
 
-#ifdef CONFIG_EFI
+/*
+ * E820_X_MAX is the maximum size of the extended E820 table.  The extended
+ * table may contain up to 3 extra E820 entries per possible NUMA node, so we
+ * make room for 3 * MAX_NUMNODES possible entries, beyond the standard 128.
+ * Also note that E820_X_MAX *must* be defined before we include 
uapi/asm/e820.h.
+ */
 #include 
 #define E820_X_MAX (E820MAX + 3 * MAX_NUMNODES)
-#else  /* ! CONFIG_EFI */
-#define E820_X_MAX E820MAX
-#endif
+
 #include 
+
 #ifndef __ASSEMBLY__
 /* see comment in arch/x86/kernel/e820.c */
 extern struct e820map *e820;
-- 
1.8.5.6


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [RFC PATCH v3] xen/x86: Increase xen_e820_map to E820_X_MAX possible entries

2016-12-05 Thread Alex Thorlton 
This is the third pass at my patchset to fix up our problems with
XENMEM_machine_memory_map on large systems.  The only changes on this
pass were to flesh out the comment above the E820_X_MAX definition, and
to add Juergen's Reviewed-by to the second patch.

Let me know if anyone has any questions/comments!

Alex Thorlton (2):
  x86: Make E820_X_MAX unconditionally larger than E820MAX
  xen/x86: Increase xen_e820_map to E820_X_MAX possible entries

 arch/x86/include/asm/e820.h | 12 
 arch/x86/xen/setup.c|  6 +++---
 2 files changed, 11 insertions(+), 7 deletions(-)

-- 
1.8.5.6


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 2/2] xen/x86: Increase xen_e820_map to E820_X_MAX possible entries

2016-12-05 Thread Alex Thorlton 
On systems with sufficiently large e820 tables, and several IOAPICs, it
is possible for the XENMEM_machine_memory_map callback (and its
counterpart, XENMEM_memory_map) to attempt to return an e820 table with
more than 128 entries.  This callback adds entries to the BIOS-provided
e820 table to account for IOAPIC registers, which, on sufficiently large
systems, can result in an e820 table that is too large to copy back into
xen_e820_map.

This change simply increases the size of xen_e820_map to E820_X_MAX to
ensure that there is enough room to store the entire e820 map returned
from this callback.

Signed-off-by: Alex Thorlton <athorl...@sgi.com>
Suggested-by: Boris Ostrovsky <boris.ostrov...@oracle.com>
Reviewed-by: Juergen Gross <jgr...@suse.com>
Cc: Russ Anderson <r...@sgi.com>
Cc: David Vrabel <david.vra...@citrix.com>
Cc: Ingo Molnar <mi...@redhat.com>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: "H. Peter Anvin" <h...@zytor.com>
Cc: Denys Vlasenko <dvlas...@redhat.com>
Cc: Boris Ostrovsky <boris.ostrov...@oracle.com>
Cc: x...@kernel.org
Cc: xen-de...@lists.xenproject.org
---
 arch/x86/xen/setup.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index f8960fc..8c394e3 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -41,7 +41,7 @@
 unsigned long xen_released_pages;
 
 /* E820 map used during setting up memory. */
-static struct e820entry xen_e820_map[E820MAX] __initdata;
+static struct e820entry xen_e820_map[E820_X_MAX] __initdata;
 static u32 xen_e820_map_entries __initdata;
 
 /*
@@ -750,7 +750,7 @@ char * __init xen_memory_setup(void)
max_pfn = min(max_pfn, xen_start_info->nr_pages);
mem_end = PFN_PHYS(max_pfn);
 
-   memmap.nr_entries = E820MAX;
+   memmap.nr_entries = ARRAY_SIZE(xen_e820_map);
set_xen_guest_handle(memmap.buffer, xen_e820_map);
 
op = xen_initial_domain() ?
@@ -923,7 +923,7 @@ char * __init xen_auto_xlated_memory_setup(void)
int i;
int rc;
 
-   memmap.nr_entries = E820MAX;
+   memmap.nr_entries = ARRAY_SIZE(xen_e820_map);
set_xen_guest_handle(memmap.buffer, xen_e820_map);
 
rc = HYPERVISOR_memory_op(XENMEM_memory_map, );
-- 
1.8.5.6


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 1/2] x86: Make E820_X_MAX unconditionally larger than E820MAX

2016-12-01 Thread Alex Thorlton 
On Wed, Nov 30, 2016 at 07:21:48AM +0100, Ingo Molnar wrote:
> 
> * Alex Thorlton <athorl...@sgi.com> wrote:
> 
> > It's really not necessary to limit E820_X_MAX to 128 in the non-EFI
> > case.  This commit drops E820_X_MAX's dependency on CONFIG_EFI, so that
> > E820_X_MAX is always at least slightly larger than E820MAX.
> > 
> > The real motivation behind this is actually to prevent some issues in
> > the Xen kernel, where the XENMEM_machine_memory_map hypercall can
> > produce an e820 map larger than 128 entries, even on systems where the
> > original e820 table was quite a bit smaller than that, depending on how
> > many IOAPICs are installed on the system.
> > 
> > Signed-off-by: Alex Thorlton <athorl...@sgi.com>
> > Suggested-by: Ingo Molnar <mi...@redhat.com>
> > Cc: Russ Anderson <r...@sgi.com>
> > Cc: David Vrabel <david.vra...@citrix.com>
> > Cc: Ingo Molnar <mi...@redhat.com>
> > Cc: Juergen Gross <jgr...@suse.com>
> > Cc: Thomas Gleixner <t...@linutronix.de>
> > Cc: "H. Peter Anvin" <h...@zytor.com>
> > Cc: Denys Vlasenko <dvlas...@redhat.com>
> > Cc: Boris Ostrovsky <boris.ostrov...@oracle.com>
> > Cc: x...@kernel.org
> > Cc: xen-de...@lists.xenproject.org
> > ---
> >  arch/x86/include/asm/e820.h | 10 ++
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/e820.h b/arch/x86/include/asm/e820.h
> > index 476b574..aa00d33 100644
> > --- a/arch/x86/include/asm/e820.h
> > +++ b/arch/x86/include/asm/e820.h
> > @@ -1,13 +1,15 @@
> >  #ifndef _ASM_X86_E820_H
> >  #define _ASM_X86_E820_H
> >  
> > -#ifdef CONFIG_EFI
> > +/*
> > + * We need to make sure that E820_X_MAX is defined
> > + * before we include uapi/asm/e820.h
> > + */
> >  #include 
> >  #define E820_X_MAX (E820MAX + 3 * MAX_NUMNODES)
> 
> What we need an explanation for in the comment is what does this stand for 
> (what 
> does the 'X' mean?), and what is the magic 3*MAX_NUMNODES about?

I'm not actually certain why 3*MAX_NUMNODES was chosen as the max size
for this table, but it was written by somebody here at SGI, so I'm sure
I can find out :)

As for the 'X,' I'm assuming that's meant to imply that this is the
maxmimum size for the 'eXtended' table, but that could be made more
clear in the comment as well.

I'll come up with a better comment for this and submit a v3.

Thanks, Ingo!

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 2/2] xen/x86: Increase xen_e820_map to E820_X_MAX possible entries

2016-11-29 Thread Alex Thorlton 
On systems with sufficiently large e820 tables, and several IOAPICs, it
is possible for the XENMEM_machine_memory_map callback (and its
counterpart, XENMEM_memory_map) to attempt to return an e820 table with
more than 128 entries.  This callback adds entries to the BIOS-provided
e820 table to account for IOAPIC registers, which, on sufficiently large
systems, can result in an e820 table that is too large to copy back into
xen_e820_map.

This change simply increases the size of xen_e820_map to E820_X_MAX to
ensure that there is enough room to store the entire e820 map returned
from this callback.

Signed-off-by: Alex Thorlton <athorl...@sgi.com>
Suggested-by: Boris Ostrovsky <boris.ostrov...@oracle.com>
Cc: Russ Anderson <r...@sgi.com>
Cc: David Vrabel <david.vra...@citrix.com>
Cc: Ingo Molnar <mi...@redhat.com>
Cc: Juergen Gross <jgr...@suse.com>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: "H. Peter Anvin" <h...@zytor.com>
Cc: Denys Vlasenko <dvlas...@redhat.com>
Cc: Boris Ostrovsky <boris.ostrov...@oracle.com>
Cc: x...@kernel.org
Cc: xen-de...@lists.xenproject.org
---
 arch/x86/xen/setup.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index f8960fc..8c394e3 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -41,7 +41,7 @@
 unsigned long xen_released_pages;
 
 /* E820 map used during setting up memory. */
-static struct e820entry xen_e820_map[E820MAX] __initdata;
+static struct e820entry xen_e820_map[E820_X_MAX] __initdata;
 static u32 xen_e820_map_entries __initdata;
 
 /*
@@ -750,7 +750,7 @@ char * __init xen_memory_setup(void)
max_pfn = min(max_pfn, xen_start_info->nr_pages);
mem_end = PFN_PHYS(max_pfn);
 
-   memmap.nr_entries = E820MAX;
+   memmap.nr_entries = ARRAY_SIZE(xen_e820_map);
set_xen_guest_handle(memmap.buffer, xen_e820_map);
 
op = xen_initial_domain() ?
@@ -923,7 +923,7 @@ char * __init xen_auto_xlated_memory_setup(void)
int i;
int rc;
 
-   memmap.nr_entries = E820MAX;
+   memmap.nr_entries = ARRAY_SIZE(xen_e820_map);
set_xen_guest_handle(memmap.buffer, xen_e820_map);
 
rc = HYPERVISOR_memory_op(XENMEM_memory_map, );
-- 
1.8.5.6


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 1/2] x86: Make E820_X_MAX unconditionally larger than E820MAX

2016-11-29 Thread Alex Thorlton 
It's really not necessary to limit E820_X_MAX to 128 in the non-EFI
case.  This commit drops E820_X_MAX's dependency on CONFIG_EFI, so that
E820_X_MAX is always at least slightly larger than E820MAX.

The real motivation behind this is actually to prevent some issues in
the Xen kernel, where the XENMEM_machine_memory_map hypercall can
produce an e820 map larger than 128 entries, even on systems where the
original e820 table was quite a bit smaller than that, depending on how
many IOAPICs are installed on the system.

Signed-off-by: Alex Thorlton <athorl...@sgi.com>
Suggested-by: Ingo Molnar <mi...@redhat.com>
Cc: Russ Anderson <r...@sgi.com>
Cc: David Vrabel <david.vra...@citrix.com>
Cc: Ingo Molnar <mi...@redhat.com>
Cc: Juergen Gross <jgr...@suse.com>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: "H. Peter Anvin" <h...@zytor.com>
Cc: Denys Vlasenko <dvlas...@redhat.com>
Cc: Boris Ostrovsky <boris.ostrov...@oracle.com>
Cc: x...@kernel.org
Cc: xen-de...@lists.xenproject.org
---
 arch/x86/include/asm/e820.h | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/e820.h b/arch/x86/include/asm/e820.h
index 476b574..aa00d33 100644
--- a/arch/x86/include/asm/e820.h
+++ b/arch/x86/include/asm/e820.h
@@ -1,13 +1,15 @@
 #ifndef _ASM_X86_E820_H
 #define _ASM_X86_E820_H
 
-#ifdef CONFIG_EFI
+/*
+ * We need to make sure that E820_X_MAX is defined
+ * before we include uapi/asm/e820.h
+ */
 #include 
 #define E820_X_MAX (E820MAX + 3 * MAX_NUMNODES)
-#else  /* ! CONFIG_EFI */
-#define E820_X_MAX E820MAX
-#endif
+
 #include 
+
 #ifndef __ASSEMBLY__
 /* see comment in arch/x86/kernel/e820.c */
 extern struct e820map *e820;
-- 
1.8.5.6


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [RFC PATCH v2] xen/x86: Increase xen_e820_map to E820_X_MAX possible entries

2016-11-29 Thread Alex Thorlton 
Here's the second round of my patches to fix up the problems that we're
seeing with the XENMEM_machine_memory_map hypercall.  These few simple
changes, to give xen_e820_map some extra space, get things working fine
on our large machine.

This version of the patchset adds a patch to remove the #ifdef
CONFIG_EFI conditional around the definition of E820_X_MAX, so that it's
always slightly larger than E820MAX.  I've also tweaked the code that
works on xen_e820_map to use ARRAY_SIZE(xen_e820_map) instead of using
E820_X_MAX directly, as suggested by Boris.

As always, I appreciate any input that others can give!

- Alex

Alex Thorlton (2):
  x86: Make E820_X_MAX unconditionally larger than E820MAX
  xen/x86: Increase xen_e820_map to E820_X_MAX possible entries

 arch/x86/include/asm/e820.h | 8 +++-
 arch/x86/xen/setup.c| 6 +++---
 2 files changed, 6 insertions(+), 8 deletions(-)

-- 
1.8.5.6


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] ACPI fixmap overflow

2016-11-17 Thread Alex Thorlton 
On Thu, Nov 17, 2016 at 10:01:15AM -0500, Boris Ostrovsky wrote:
> On 11/16/2016 05:08 AM, Jan Beulich wrote:
>  On 15.11.16 at 21:51,  wrote:
> >> On 11/15/2016 03:44 PM, Andrew Cooper wrote:
> >>> On 15/11/2016 20:39, Boris Ostrovsky wrote:
>  On 11/15/2016 02:45 PM, Andrew Cooper wrote:
> > On 15/11/16 19:34, Boris Ostrovsky wrote:
> >> In addition to running out of e820 entries on that large machine that
> >> Alex was referring to in [0] he is also running out of ACPI fixmap 
> >> space
> >> while parsing MADT (the box has *lots* of processors). The
> >> quick-and-dirty solution is to increase NUM_FIXMAP_ACPI_PAGES but I
> >> wonder whether we should move to dynamic memory allocation.
> > Why do we use fixmap anyway?  It doesn't look too hard to reorder
> > vm_init() slightly higher, and be able to use ioremap() for all APCI 
> > tables.
>  Hmm... Let me see how possible this is. Just moving it up won't work
>  since heap allocator is initialized after ACPI tables.
> >>> We have plenty of usable PTEs already allocated at boot, mainly from the
> >>> init pagetables.  Given a static __init vm_bitmap, a new boot-time-only
> >>> vm range should be usable without any heap allocations at all.
> >> Wouldn't that (using pre-allocated PTEs), in a way, be equivalent to
> >> increasing fixmap size?
> > Indeed. For the time being I think growing the fixmap should be
> > fine. Clearly it being a fixed 4 pages has been wrong for a long
> > time - there's no point in it being smaller than MAX_LOCAL_APIC
> > x2apic entries, plus min(MAX_LOCAL_APIC, 256) lapic ones, plus
> > MAX_IO_APICS ioapic ones etc.
> >
> > Otoh the actual parsing of MADT happens after the heap allocator
> > has been initialized. The only earlier use is via acpi_table_init() ->
> > check_multiple_madt(); acpi_initialize_tables() doesn't appear to
> > map full tables. And check_multiple_madt() not being able to map
> > the full table would not prevent boot from continuing afaics. So
> > Andrew's suggestion to switch to dynamic mapping earlier would
> > still seem to be possible (and then preferred). In fact
> > acpi_boot_init() already gets called after vm_init(), so switching
> > acpi_os_map_memory() to use dynamic mappings when
> >> = SYS_STATE_boot should already work today (on x86 at least,
> > not sure about ARM).
> 
> Yes, switching to ioremap on SYS_STATE_boot seems to work. I asked Alex
> to test this on OVM (which is 4.4-based).

FYI, I'm going to get this done a little later today.  I'll let you know
how it worked out this evening!

- Alex

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen/x86: Increase xen_e820_map to E820_X_MAX possible entries

2016-11-16 Thread Alex Thorlton 
On Wed, Nov 16, 2016 at 07:09:01AM +0100, Juergen Gross wrote:
> On 15/11/16 01:11, Alex Thorlton wrote:
> > On systems with sufficiently large e820 tables, and several IOAPICs, it
> > is possible for the XENMEM_machine_memory_map callback (and its
> > counterpart, XENMEM_memory_map) to attempt to return an e820 table with
> > more than 128 entries.  This callback adds entries to the BIOS-provided
> > e820 table to account for IOAPIC registers, which, on sufficiently large
> > systems, can result in an e820 table that is too large to copy back into
> > xen_e820_map.
> > 
> > This change simply increases the size of xen_e820_map to E820_X_MAX to
> > ensure that there is enough room to store the entire e820 map returned
> > from this callback.
> > 
> > Signed-off-by: Alex Thorlton <athorl...@sgi.com>
> > Suggested-by: Boris Ostrovsky <boris.ostrov...@oracle.com>
> > Cc: Russ Anderson <r...@sgi.com>
> > Cc: David Vrabel <david.vra...@citrix.com>
> > Cc: Juergen Gross <jgr...@suse.com>
> > Cc: Thomas Gleixner <t...@linutronix.de>
> > Cc: Ingo Molnar <mi...@redhat.com>
> > Cc: "H. Peter Anvin" <h...@zytor.com>
> > Cc: x...@kernel.org
> > Cc: xen-de...@lists.xenproject.org
> > ---
> >  arch/x86/xen/setup.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> > index f8960fc..5e1ecc7 100644
> > --- a/arch/x86/xen/setup.c
> > +++ b/arch/x86/xen/setup.c
> > @@ -41,7 +41,7 @@
> >  unsigned long xen_released_pages;
> >  
> >  /* E820 map used during setting up memory. */
> > -static struct e820entry xen_e820_map[E820MAX] __initdata;
> > +static struct e820entry xen_e820_map[E820_X_MAX] __initdata;
> >  static u32 xen_e820_map_entries __initdata;
> >  
> >  /*
> > @@ -750,7 +750,7 @@ char * __init xen_memory_setup(void)
> > max_pfn = min(max_pfn, xen_start_info->nr_pages);
> > mem_end = PFN_PHYS(max_pfn);
> >  
> > -   memmap.nr_entries = E820MAX;
> > +   memmap.nr_entries = E820_X_MAX;
> 
> Please use ARRAY_SIZE(xen_e820_map) here and ...
> 
> > set_xen_guest_handle(memmap.buffer, xen_e820_map);
> >  
> > op = xen_initial_domain() ?
> > @@ -923,7 +923,7 @@ char * __init xen_auto_xlated_memory_setup(void)
> > int i;
> > int rc;
> >  
> > -   memmap.nr_entries = E820MAX;
> > +   memmap.nr_entries = E820_X_MAX;
> 
> ... here.

Got it.  Thanks, Juergen!

- Alex

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen/x86: Increase xen_e820_map to E820_X_MAX possible entries

2016-11-16 Thread Alex Thorlton 
On Tue, Nov 15, 2016 at 07:30:35AM +0100, Ingo Molnar wrote:
> 
> * Alex Thorlton <athorl...@sgi.com> wrote:
> 
> > On systems with sufficiently large e820 tables, and several IOAPICs, it
> > is possible for the XENMEM_machine_memory_map callback (and its
> > counterpart, XENMEM_memory_map) to attempt to return an e820 table with
> > more than 128 entries.  This callback adds entries to the BIOS-provided
> > e820 table to account for IOAPIC registers, which, on sufficiently large
> > systems, can result in an e820 table that is too large to copy back into
> > xen_e820_map.
> > 
> > This change simply increases the size of xen_e820_map to E820_X_MAX to
> > ensure that there is enough room to store the entire e820 map returned
> > from this callback.
> 
> This means:
> 
>  #ifdef CONFIG_EFI
>  #include 
>  #define E820_X_MAX (E820MAX + 3 * MAX_NUMNODES)
>  #else   /* ! CONFIG_EFI */
>  #define E820_X_MAX E820MAX
>  #endif
> 
> It's a bit weird to key it off of CONFIG_EFI - why isn't it unconditional?
> 
> But I have no objections to the principle if it fixes a crash.

I originally thought that Juergen and Jan's alternative solution
eliminated the need for my change, but it appears that I was incorrect,
at least if we want to get the kernel booting on the older HV
without having to modify it.

I'll remove the ifdef here to make this unconditional.

Thanks, Ingo!

- Alex

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [RFC PATCH] xen/x86: Increase xen_e820_map to E820_X_MAX possible entries

2016-11-16 Thread Alex Thorlton 
On Wed, Nov 16, 2016 at 05:42:11PM +0100, Juergen Gross wrote:
> On 15/11/16 08:15, Jan Beulich wrote:
> >>>> On 15.11.16 at 07:33, <jgr...@suse.com> wrote:
> >> On 15/11/16 01:11, Alex Thorlton wrote:
> >>> Hey everyone,
> >>>
> >>> We're having problems with large systems hitting a BUG in
> >>> xen_memory_setup, due to extra e820 entries created in the
> >>> XENMEM_machine_memory_map callback.  The change in the patch gets things
> >>> working, but Boris and I wanted to get opinions on whether or not this
> >>> is the appropriate/entire solution, which is why I've sent it as an RFC
> >>> for now.
> 
> >> While I think extending the e820 table is the right thing to do I'm
> >> questioning the assumptions here.
> >>
> >> Looking briefly through the Xen hypervisor sources I think it isn't
> >> yet ready for such large machines: the hypervisor's e820 map seems to
> >> be still limited to 128 e820 entries. Jan, did I overlook an EFI
> >> specific path extending this limitation?
> > 
> > No, you didn't. I do question the correlation with "large machines"
> > here though: The issue isn't with large machines afaict, but with
> > ones having very many entries (i.e. heavily fragmented).
> 
> Alex, I would appreciate if you could send me the E820 map printed
> at a bare metal Linux boot. I suspect it is already larger than
> 128 entries and the hypervisor is just cutting it off at the end.

No problem!  I'll get this to you today.

- Alex

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [RFC PATCH] xen/x86: Increase xen_e820_map to E820_X_MAX possible entries

2016-11-15 Thread Alex Thorlton 
On Tue, Nov 15, 2016 at 10:55:49AM +0100, Juergen Gross wrote:
> I'd go with the new error code. What about E2BIG or ENOSPC?
> 
> I think the hypervisor should fill in the number of entries required
> in this case.
> 
> In case nobody objects I can post patches for this purpose (both Xen
> and Linux).

This sounds like a good solution to me.  I think it's definitely more
appropriate than simply bumping up the size of xen_e820_map, especially
considering the fact that it's theoretically possible for the e820 map
generated by the hypercall to grow too large, even on a non-EFI machine,
where my change would have no effect.

Thanks for your input!

- Alex

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [RFC PATCH] xen/x86: Increase xen_e820_map to E820_X_MAX possible entries

2016-11-14 Thread Alex Thorlton 
Hey everyone,

We're having problems with large systems hitting a BUG in
xen_memory_setup, due to extra e820 entries created in the
XENMEM_machine_memory_map callback.  The change in the patch gets things
working, but Boris and I wanted to get opinions on whether or not this
is the appropriate/entire solution, which is why I've sent it as an RFC
for now.

Boris pointed out to me that E820_X_MAX is only large when CONFIG_EFI=y,
which is a detail worth discussig.  He proposed possibly adding
CONFIG_XEN to the conditions under which we set E820_X_MAX to a larger
value than E820MAX, since the Xen e820 table isn't bound by the
zero-page memory limitations.

I do *slightly* question the use of E820_X_MAX here, only from a
cosmetic prospective, as I believe this macro is intended to describe
the maximum size of the extended e820 table, which, AFAIK, is not used
by the Xen HV.  That being said, there isn't exactly a "more
appropriate" macro/variable to use, so this may not really be an issue.

Any input on the patch, or the questions I've raised above is greatly
appreciated!

- Alex

Alex Thorlton (1):
  xen/x86: Increase xen_e820_map to E820_X_MAX possible entries

 arch/x86/xen/setup.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
1.8.5.6


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH] xen/x86: Increase xen_e820_map to E820_X_MAX possible entries

2016-11-14 Thread Alex Thorlton 
On systems with sufficiently large e820 tables, and several IOAPICs, it
is possible for the XENMEM_machine_memory_map callback (and its
counterpart, XENMEM_memory_map) to attempt to return an e820 table with
more than 128 entries.  This callback adds entries to the BIOS-provided
e820 table to account for IOAPIC registers, which, on sufficiently large
systems, can result in an e820 table that is too large to copy back into
xen_e820_map.

This change simply increases the size of xen_e820_map to E820_X_MAX to
ensure that there is enough room to store the entire e820 map returned
from this callback.

Signed-off-by: Alex Thorlton <athorl...@sgi.com>
Suggested-by: Boris Ostrovsky <boris.ostrov...@oracle.com>
Cc: Russ Anderson <r...@sgi.com>
Cc: David Vrabel <david.vra...@citrix.com>
Cc: Juergen Gross <jgr...@suse.com>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Ingo Molnar <mi...@redhat.com>
Cc: "H. Peter Anvin" <h...@zytor.com>
Cc: x...@kernel.org
Cc: xen-de...@lists.xenproject.org
---
 arch/x86/xen/setup.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index f8960fc..5e1ecc7 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -41,7 +41,7 @@
 unsigned long xen_released_pages;
 
 /* E820 map used during setting up memory. */
-static struct e820entry xen_e820_map[E820MAX] __initdata;
+static struct e820entry xen_e820_map[E820_X_MAX] __initdata;
 static u32 xen_e820_map_entries __initdata;
 
 /*
@@ -750,7 +750,7 @@ char * __init xen_memory_setup(void)
max_pfn = min(max_pfn, xen_start_info->nr_pages);
mem_end = PFN_PHYS(max_pfn);
 
-   memmap.nr_entries = E820MAX;
+   memmap.nr_entries = E820_X_MAX;
set_xen_guest_handle(memmap.buffer, xen_e820_map);
 
op = xen_initial_domain() ?
@@ -923,7 +923,7 @@ char * __init xen_auto_xlated_memory_setup(void)
int i;
int rc;
 
-   memmap.nr_entries = E820MAX;
+   memmap.nr_entries = E820_X_MAX;
set_xen_guest_handle(memmap.buffer, xen_e820_map);
 
rc = HYPERVISOR_memory_op(XENMEM_memory_map, );
-- 
1.8.5.6


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel