Re: [PATCH] SeaBIOS/Xen: Compute the low RAM memory size in the BDA according to the e820

Re: [PATCH] SeaBIOS/Xen: Compute the low RAM memory size in the BDA according to the e820

[Kevin O'Connor]

On Mon, Nov 14, 2011 at 01:50:05AM +0000, Julian Pidancet wrote:
> Some programs or ROMs like iPXE rely on the memory size field present
> in the BDA instead of e820 to relocate sections at the end of the low
> memory.
> Xen's hvmloader places an ACPI info table at 0x9F000 at the end of the
> usable RAM in low memory (just before the EBDA). The e820 table gets
> altered accordingly to protect the area where the table lives, but a ROM
> like iPXE only takes into account the memory size field in the BDA and
> consequently corrupts the table when relocating itself.

That wont work reliably - SeaBIOS (and option roms) can grow the EBDA.
It's not realistic to put a whole in the middle of the first 640k.

[...]
> diff --git a/src/memmap.c b/src/memmap.c
> index 20ccae0..36750f5 100644
> --- a/src/memmap.c
> +++ b/src/memmap.c
> @@ -7,6 +7,7 @@
>  #include "memmap.h" // struct e820entry
>  #include "util.h" // dprintf.h
>  #include "biosvar.h" // SET_EBDA
> +#include "config.h" // CONFIG_*
>  
>  
>  /****************************************************************
> @@ -133,6 +134,34 @@ add_e820(u64 start, u64 size, u32 type)
>      //dump_map();
>  }
>  
> +// Try to find the size of the usable RAM memory in the
> +// first megabyte of RAM.
> +u32
> +get_lowram_size(void)
> +{
> +    u32 lowram_end = 0;
> +
> +    int i;
> +    for (i=0; i<e820_count; i++) {
> +        struct e820entry *e = &e820_list[i];
> +        u64 e_end = e->start + e->size;
> +
> +        if (e->type != E820_RAM)
> +            continue;
> +
> +        if (e_end <= 0x100000 && /* 1M */
> +            e_end > lowram_end)

Shouldn't this read (e_end <= BUILD_LOWRAM_END)?  Otherwise, if for
some strange reason an e820 entry pointed to ram above 0xa0000 but
below 1meg, it would break things.

[...]
> --- a/src/post.c
> +++ b/src/post.c
> @@ -82,18 +82,21 @@ init_bda(void)
>      struct bios_data_area_s *bda = MAKE_FLATPTR(SEG_BDA, 0);
>      memset(bda, 0, sizeof(*bda));
>  
> -    int esize = EBDA_SIZE_START;
> -    SET_BDA(mem_size_kb, BUILD_LOWRAM_END/1024 - esize);
>      u16 ebda_seg = EBDA_SEGMENT_START;
>      SET_BDA(ebda_seg, ebda_seg);

As above, this doesn't look right - SeaBIOS will still locate the EBDA
at 9fc00 and nothing will stop it from growing it over the 9f000 area.

-Kevin

[PATCH] SeaBIOS/Xen: Compute the low RAM memory size in the BDA according to the e820

[Julian Pidancet]

Some programs or ROMs like iPXE rely on the memory size field present
in the BDA instead of e820 to relocate sections at the end of the low
memory.
Xen's hvmloader places an ACPI info table at 0x9F000 at the end of the
usable RAM in low memory (just before the EBDA). The e820 table gets
altered accordingly to protect the area where the table lives, but a ROM
like iPXE only takes into account the memory size field in the BDA and
consequently corrupts the table when relocating itself.

This patch computes the size of the usable RAM in low memory by walking
the e820 table and updates the BDA accordingly. It defaults to 640KB if
it fails to guess that value.

With Xen, the issue can be reproduced by pxe booting a linux kernel
through iPXE using the pci=use_crs command line option. Without this
patch the last of the following lines doesn't appear:

[    0.326967] PCI: Using host bridge windows from ACPI; if necessary, use "pci=nocrs" and report a bug
[    0.327999] ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-ff])
[    0.329028] pci_root PNP0A03:00: host bridge window [io  0x0000-0x0cf7]
[    0.329967] pci_root PNP0A03:00: host bridge window [io  0x0d00-0xffff]
[    0.330966] pci_root PNP0A03:00: host bridge window [mem 0x000a0000-0x000bffff]
[    0.331966] pci_root PNP0A03:00: host bridge window [mem 0xf0000000-0xfbffffff]

Signed-off-by: Julian Pidancet <julian.pidancet@gmail.com>
---
 src/memmap.c |   29 +++++++++++++++++++++++++++++
 src/memmap.h |    1 +
 src/post.c   |    9 ++++++---
 3 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/src/memmap.c b/src/memmap.c
index 20ccae0..36750f5 100644
--- a/src/memmap.c
+++ b/src/memmap.c
@@ -7,6 +7,7 @@
 #include "memmap.h" // struct e820entry
 #include "util.h" // dprintf.h
 #include "biosvar.h" // SET_EBDA
+#include "config.h" // CONFIG_*
 
 
 /****************************************************************
@@ -133,6 +134,34 @@ add_e820(u64 start, u64 size, u32 type)
     //dump_map();
 }
 
+// Try to find the size of the usable RAM memory in the
+// first megabyte of RAM.
+u32
+get_lowram_size(void)
+{
+    u32 lowram_end = 0;
+
+    int i;
+    for (i=0; i<e820_count; i++) {
+        struct e820entry *e = &e820_list[i];
+        u64 e_end = e->start + e->size;
+
+        if (e->type != E820_RAM)
+            continue;
+
+        if (e_end <= 0x100000 && /* 1M */
+            e_end > lowram_end)
+            lowram_end = (u32)e_end;
+    }
+
+    // No RAM entry ending before the first megabyte of memory
+    // found, default to 0xA0000
+    if (!lowram_end)
+        lowram_end = BUILD_LOWRAM_END;
+
+    return lowram_end;
+}
+
 // Report on final memory locations.
 void
 memmap_finalize(void)
diff --git a/src/memmap.h b/src/memmap.h
index 01c7ddb..e65d090 100644
--- a/src/memmap.h
+++ b/src/memmap.h
@@ -18,6 +18,7 @@ struct e820entry {
 
 void add_e820(u64 start, u64 size, u32 type);
 void memmap_finalize(void);
+u32 get_lowram_size(void);
 
 // A typical OS page size
 #define PAGE_SIZE 4096
diff --git a/src/post.c b/src/post.c
index b4ad1fa..9f0cb63 100644
--- a/src/post.c
+++ b/src/post.c
@@ -82,18 +82,21 @@ init_bda(void)
     struct bios_data_area_s *bda = MAKE_FLATPTR(SEG_BDA, 0);
     memset(bda, 0, sizeof(*bda));
 
-    int esize = EBDA_SIZE_START;
-    SET_BDA(mem_size_kb, BUILD_LOWRAM_END/1024 - esize);
     u16 ebda_seg = EBDA_SEGMENT_START;
     SET_BDA(ebda_seg, ebda_seg);
 
     // Init ebda
     struct extended_bios_data_area_s *ebda = get_ebda_ptr();
     memset(ebda, 0, sizeof(*ebda));
-    ebda->size = esize;
+    ebda->size = EBDA_SIZE_START;
 
     add_e820((u32)MAKE_FLATPTR(ebda_seg, 0), GET_EBDA2(ebda_seg, size) * 1024
              , E820_RESERVED);
+
+    u32 lowram_size = get_lowram_size();
+
+    dprintf(8, "End of usable low memory RAM: %08x\n", lowram_size);
+    SET_BDA(mem_size_kb, lowram_size / 1024);
 }
 
 static void
-- 
Julian Pidancet

Re: [PATCH] SeaBIOS/Xen: Compute the low RAM memory size in the BDA according to the e820

[Julian Pidancet]

On Mon, Nov 14, 2011 at 1:11 AM, Kevin O'Connor <kevin@koconnor.net> wrote:
>
> That wont work reliably - SeaBIOS (and option roms) can grow the EBDA.
> It's not realistic to put a whole in the middle of the first 640k.
>

When you say "grow" do you mean grow downwards ? Because if not, the
ACPI info structure lives in 9F000, so there shouldn't be any problem
reporting the 0-9F000 range as the memory size in the BDA.

> [...]
>> diff --git a/src/memmap.c b/src/memmap.c
>> index 20ccae0..36750f5 100644
>> --- a/src/memmap.c
>> +++ b/src/memmap.c
>> @@ -7,6 +7,7 @@
>>  #include "memmap.h" // struct e820entry
>>  #include "util.h" // dprintf.h
>>  #include "biosvar.h" // SET_EBDA
>> +#include "config.h" // CONFIG_*
>>
>>
>>  /****************************************************************
>> @@ -133,6 +134,34 @@ add_e820(u64 start, u64 size, u32 type)
>>      //dump_map();
>>  }
>>
>> +// Try to find the size of the usable RAM memory in the
>> +// first megabyte of RAM.
>> +u32
>> +get_lowram_size(void)
>> +{
>> +    u32 lowram_end = 0;
>> +
>> +    int i;
>> +    for (i=0; i<e820_count; i++) {
>> +        struct e820entry *e = &e820_list[i];
>> +        u64 e_end = e->start + e->size;
>> +
>> +        if (e->type != E820_RAM)
>> +            continue;
>> +
>> +        if (e_end <= 0x100000 && /* 1M */
>> +            e_end > lowram_end)
>
> Shouldn't this read (e_end <= BUILD_LOWRAM_END)?  Otherwise, if for
> some strange reason an e820 entry pointed to ram above 0xa0000 but
> below 1meg, it would break things.
>

My bad, I just realized that. Maybe a more correct (and simpler)
approach would be to look for the range marked as RAM which starts at
0x0 and simply use its size (and check that it doesn't go past
0xA0000). Would that sound right ?

> [...]
>> --- a/src/post.c
>> +++ b/src/post.c
>> @@ -82,18 +82,21 @@ init_bda(void)
>>      struct bios_data_area_s *bda = MAKE_FLATPTR(SEG_BDA, 0);
>>      memset(bda, 0, sizeof(*bda));
>>
>> -    int esize = EBDA_SIZE_START;
>> -    SET_BDA(mem_size_kb, BUILD_LOWRAM_END/1024 - esize);
>>      u16 ebda_seg = EBDA_SEGMENT_START;
>>      SET_BDA(ebda_seg, ebda_seg);
>
> As above, this doesn't look right - SeaBIOS will still locate the EBDA
> at 9fc00 and nothing will stop it from growing it over the 9f000 area.
>

The EBDA is a standard bios structure, can't we safely assume that
it's size will hardly change ? If not, would you suggest that this
ACPI info structure should be placed at a different location ? The
problem with this is that it has to be in a location with a fixed
address, and preferably in a location which is accessible by any AML
interpreter.

-- 
Julian

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

Re: [PATCH] SeaBIOS/Xen: Compute the low RAM memory size in the BDA according to the e820

[Kevin O'Connor]

On Mon, Nov 14, 2011 at 02:03:38AM +0000, Julian Pidancet wrote:
> On Mon, Nov 14, 2011 at 1:11 AM, Kevin O'Connor <kevin@koconnor.net> wrote:
> >
> > That wont work reliably - SeaBIOS (and option roms) can grow the EBDA.
> > It's not realistic to put a whole in the middle of the first 640k.
> >
> 
> When you say "grow" do you mean grow downwards ? Because if not, the
> ACPI info structure lives in 9F000, so there shouldn't be any problem
> reporting the 0-9F000 range as the memory size in the BDA.

Yes - the EBDA grows down.  Both SeaBIOS itself can grow the EBDA (see
pmm.c:zonelow_expand) and option roms can grow the EBDA.

> > As above, this doesn't look right - SeaBIOS will still locate the EBDA
> > at 9fc00 and nothing will stop it from growing it over the 9f000 area.
> >
> 
> The EBDA is a standard bios structure, can't we safely assume that
> it's size will hardly change ? If not, would you suggest that this
> ACPI info structure should be placed at a different location ? The
> problem with this is that it has to be in a location with a fixed
> address, and preferably in a location which is accessible by any AML
> interpreter.

Why does it have to be at a fixed location?  What structure is
actually placed at this address?

The AML interpreter should be able to see all of ram, so that doesn't
seem like an issue.

-Kevin

Re: [PATCH] SeaBIOS/Xen: Compute the low RAM memory size in the BDA according to the e820

[Julian Pidancet]

On Mon, Nov 14, 2011 at 2:09 AM, Kevin O'Connor <kevin@koconnor.net> wrote:
>
> Yes - the EBDA grows down.  Both SeaBIOS itself can grow the EBDA (see
> pmm.c:zonelow_expand) and option roms can grow the EBDA.
>

I ignored that. Thanks for the info.

>> > As above, this doesn't look right - SeaBIOS will still locate the EBDA
>> > at 9fc00 and nothing will stop it from growing it over the 9f000 area.
>> >
>>
>> The EBDA is a standard bios structure, can't we safely assume that
>> it's size will hardly change ? If not, would you suggest that this
>> ACPI info structure should be placed at a different location ? The
>> problem with this is that it has to be in a location with a fixed
>> address, and preferably in a location which is accessible by any AML
>> interpreter.
>
> Why does it have to be at a fixed location?  What structure is
> actually placed at this address?
>

Xen's hvmloader automatically computes the size of the PCI memory and
stores the PCI memory range adresses in that structure. It also
provide information about wether some devices are present (uart, hpet,
ect...). The ACPI code that we inject in the guest has the address of
this structure hardcoded, and it contains some tricks to make the AML
interpreter go read the values contained in it, and take action to
expose the right information to the guest OS. Like the right PCI root
window for example.

I'm not at all an ACPI expert, I don't know if there's a better way to
expose to the guest the right information.

> The AML interpreter should be able to see all of ram, so that doesn't
> seem like an issue.
>

Like I said, I'm not an ACPI expert. But let say we decide to move
this ACPI info structure to some other area, where there's less risk
for it to be overwritten, like somewhere above 0xFC000000, wouldn't
that prevent some Operating System with limited memory capabilities to
access it ?

Besides, it seems that SeaBIOS manages itself the space between
0xFC000000 and 4G, so it seems difficult to imagine to have a reserved
space with a fixed address in there.

-- 
Julian

Re: [PATCH] SeaBIOS/Xen: Compute the low RAM memory size in the BDA according to the e820

[Kevin O'Connor]

On Mon, Nov 14, 2011 at 02:48:28AM +0000, Julian Pidancet wrote:
> On Mon, Nov 14, 2011 at 2:09 AM, Kevin O'Connor <kevin@koconnor.net> wrote:
> > Why does it have to be at a fixed location?  What structure is
> > actually placed at this address?
> 
> Xen's hvmloader automatically computes the size of the PCI memory and
> stores the PCI memory range adresses in that structure. It also
> provide information about wether some devices are present (uart, hpet,
> ect...). The ACPI code that we inject in the guest has the address of
> this structure hardcoded, and it contains some tricks to make the AML
> interpreter go read the values contained in it, and take action to
> expose the right information to the guest OS. Like the right PCI root
> window for example.
> 
> I'm not at all an ACPI expert, I don't know if there's a better way to
> expose to the guest the right information.

Unfortunately, there aren't very many places to put a hardcoded
address.  The safest thing is probably to dynamically generate an SSDT
with a pointer - then the DSDT can use the pointer instead of a
hardcoded address.  This is more work, however.

> > The AML interpreter should be able to see all of ram, so that doesn't
> > seem like an issue.
> 
> Like I said, I'm not an ACPI expert. But let say we decide to move
> this ACPI info structure to some other area, where there's less risk
> for it to be overwritten, like somewhere above 0xFC000000, wouldn't
> that prevent some Operating System with limited memory capabilities to
> access it ?

If the OS can handle AML it can handle 32bit addresses.

> Besides, it seems that SeaBIOS manages itself the space between
> 0xFC000000 and 4G, so it seems difficult to imagine to have a reserved
> space with a fixed address in there.

On Xen, the PCI init code isn't used, so assuming this struct doesn't
need to live in real "ram", I think it could live just about anywhere
past the end of ram.  Even with pciinit.c, addresses over 0xfc00000
(with the exception of a few bytes for hpet, apic, ioapic, and bios
image) could be used.

-Kevin

Re: [PATCH] SeaBIOS/Xen: Compute the low RAM memory size in the BDA according to the e820

[Rudolf Marek]

> Unfortunately, there aren't very many places to put a hardcoded
> address.  The safest thing is probably to dynamically generate an SSDT
> with a pointer - then the DSDT can use the pointer instead of a
> hardcoded address.  This is more work, however.

You can even create an "OEM" ACPI table, with std ACPI header, but with custom 
data inside. OS will ignore it and as bonus you will have it with checksum.

Thanks
Rudolf

Re: Re: [PATCH] SeaBIOS/Xen: Compute the low RAM memory size in the BDA according to the e820

[Keir Fraser]

On 14/11/2011 03:36, "Kevin O'Connor" <kevin@koconnor.net> wrote:

>> Like I said, I'm not an ACPI expert. But let say we decide to move
>> this ACPI info structure to some other area, where there's less risk
>> for it to be overwritten, like somewhere above 0xFC000000, wouldn't
>> that prevent some Operating System with limited memory capabilities to
>> access it ?
> 
> If the OS can handle AML it can handle 32bit addresses.
> 
>> Besides, it seems that SeaBIOS manages itself the space between
>> 0xFC000000 and 4G, so it seems difficult to imagine to have a reserved
>> space with a fixed address in there.
> 
> On Xen, the PCI init code isn't used, so assuming this struct doesn't
> need to live in real "ram", I think it could live just about anywhere
> past the end of ram.  Even with pciinit.c, addresses over 0xfc00000
> (with the exception of a few bytes for hpet, apic, ioapic, and bios
> image) could be used.

I suggest we stick it at FC000000, and shift hvmloader's mem_alloc()
starting address up by one page to FC001000. The acpi build code will have
to manually mem_hole_populate_ram() that one page before writing to it. This
can then be documented in hvmloader/config.h which contains a description
of, and defines for, the system memory map. This is by far the easiest
solution to this problem; manually crafting an SSDT is a right pain in the
arse, whereas this is maybe a 5-line patch.

 -- Keir

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

Re: Re: [PATCH] SeaBIOS/Xen: Compute the low RAM memory size in the BDA according to the e820

[Keir Fraser]

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

On 14/11/2011 08:53, "Keir Fraser" <keir.xen@gmail.com> wrote:

> On 14/11/2011 03:36, "Kevin O'Connor" <kevin@koconnor.net> wrote:
> 
>> On Xen, the PCI init code isn't used, so assuming this struct doesn't
>> need to live in real "ram", I think it could live just about anywhere
>> past the end of ram.  Even with pciinit.c, addresses over 0xfc00000
>> (with the exception of a few bytes for hpet, apic, ioapic, and bios
>> image) could be used.
> 
> I suggest we stick it at FC000000, and shift hvmloader's mem_alloc()
> starting address up by one page to FC001000. The acpi build code will have
> to manually mem_hole_populate_ram() that one page before writing to it. This
> can then be documented in hvmloader/config.h which contains a description
> of, and defines for, the system memory map. This is by far the easiest
> solution to this problem; manually crafting an SSDT is a right pain in the
> arse, whereas this is maybe a 5-line patch.

Like the attached patch (untested), which is a bit larger than anticipated,
but actually allows code to be net deleted. :-)

 -- Keir

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


[-- Attachment #2: 00-relocate-acpi-info --]
[-- Type: application/octet-stream, Size: 6105 bytes --]

diff -r 0844b17df7a9 tools/firmware/hvmloader/acpi/build.c
--- a/tools/firmware/hvmloader/acpi/build.c	Fri Nov 11 18:14:35 2011 +0000
+++ b/tools/firmware/hvmloader/acpi/build.c	Mon Nov 14 13:20:26 2011 +0000
@@ -276,7 +276,7 @@ static int construct_secondary_tables(un
 
 void acpi_build_tables(struct acpi_config *config, unsigned int physical)
 {
-    struct acpi_info *acpi_info = (struct acpi_info *)ACPI_INFO_PHYSICAL_ADDRESS;
+    struct acpi_info *acpi_info;
     struct acpi_20_rsdp *rsdp;
     struct acpi_20_rsdt *rsdt;
     struct acpi_20_xsdt *xsdt;
@@ -287,6 +287,9 @@ void acpi_build_tables(struct acpi_confi
     unsigned long        secondary_tables[16];
     int                  nr_secondaries, i;
 
+    /* Allocate and initialise the acpi info area. */
+    mem_hole_populate_ram(ACPI_INFO_PHYSICAL_ADDRESS >> PAGE_SHIFT, 1);
+    acpi_info = (struct acpi_info *)ACPI_INFO_PHYSICAL_ADDRESS;
     memset(acpi_info, 0, sizeof(*acpi_info));
 
     /*
diff -r 0844b17df7a9 tools/firmware/hvmloader/acpi/dsdt.asl
--- a/tools/firmware/hvmloader/acpi/dsdt.asl	Fri Nov 11 18:14:35 2011 +0000
+++ b/tools/firmware/hvmloader/acpi/dsdt.asl	Mon Nov 14 13:20:26 2011 +0000
@@ -61,8 +61,8 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, 
 
     Scope (\_SB)
     {
-       /* ACPI_INFO_PHYSICAL_ADDRESS == 0x9F000 */
-       OperationRegion(BIOS, SystemMemory, 0x9F000, 24)
+       /* ACPI_INFO_PHYSICAL_ADDRESS == 0xFC000000 */
+       OperationRegion(BIOS, SystemMemory, 0xFC000000, 24)
        Field(BIOS, ByteAcc, NoLock, Preserve) {
            UAR1, 1,
            UAR2, 1,
diff -r 0844b17df7a9 tools/firmware/hvmloader/config.h
--- a/tools/firmware/hvmloader/config.h	Fri Nov 11 18:14:35 2011 +0000
+++ b/tools/firmware/hvmloader/config.h	Mon Nov 14 13:20:26 2011 +0000
@@ -54,18 +54,17 @@ extern struct bios_config seabios_config
 #define PCI_MEM_END         0xfc000000
 extern unsigned long pci_mem_start, pci_mem_end;
 
-/* Reserved for special BIOS mappings, etc. */
-#define RESERVED_MEMBASE    0xfc000000
 
 /* Memory map. */
 #define SCRATCH_PHYSICAL_ADDRESS      0x00010000
 #define HYPERCALL_PHYSICAL_ADDRESS    0x00080000
-#define ACPI_INFO_PHYSICAL_ADDRESS    0x0009F000
 #define VGABIOS_PHYSICAL_ADDRESS      0x000C0000
 #define HVMLOADER_PHYSICAL_ADDRESS    0x00100000
-
-#define ACPI_INFO_SIZE                     0xC00
-#define ACPI_INFO_PHYSICAL_END (ACPI_INFO_PHYSICAL_ADDRESS + ACPI_INFO_SIZE)
+/* Special BIOS mappings, etc. are allocated from here upwards... */
+#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
 
 extern unsigned long scratch_start;
 
diff -r 0844b17df7a9 tools/firmware/hvmloader/e820.c
--- a/tools/firmware/hvmloader/e820.c	Fri Nov 11 18:14:35 2011 +0000
+++ b/tools/firmware/hvmloader/e820.c	Mon Nov 14 13:20:26 2011 +0000
@@ -81,55 +81,24 @@ int build_e820_table(struct e820entry *e
     /* Lowmem must be at least 512K to keep Windows happy) */
     ASSERT ( lowmem_reserved_base > 512<<10 );
 
-    /*
-     * Lowmem reservation must either cover the ACPI info region
-     * entirely or not at all. Sitting half way through suggests
-     * something funny is going on.
-     */
-    ASSERT ( lowmem_reserved_base < ACPI_INFO_PHYSICAL_ADDRESS ||
-             lowmem_reserved_base > ACPI_INFO_PHYSICAL_END );
-
     ASSERT ( bios_image_base < 0x100000 );
 
-    if ( lowmem_reserved_base < ACPI_INFO_PHYSICAL_ADDRESS )
+    /*
+     * 0x0-lowmem_reserved_base: Ordinary RAM.
+     */
+    e820[nr].addr = 0x00000;
+    e820[nr].size = lowmem_reserved_base;
+    e820[nr].type = E820_RAM;
+    nr++;
+
+    /* lowmem_reserved_base-0xA0000: reserved by BIOS implementation. */
+    if ( lowmem_reserved_base < 0xA0000 )
     {
-        /*
-         * 0x0-lowmem_reserved_base: Ordinary RAM.
-         */
-        e820[nr].addr = 0x00000;
-        e820[nr].size = lowmem_reserved_base;
-        e820[nr].type = E820_RAM;
-        nr++;
-    }
-    else
-    {
-        /* 0x0-ACPI_INFO: Ordinary RAM. */
-        e820[nr].addr = 0x00000;
-        e820[nr].size = ACPI_INFO_PHYSICAL_ADDRESS;
-        e820[nr].type = E820_RAM;
-        nr++;
-
-        /* ACPI INFO: Reserved. */
-        e820[nr].addr = ACPI_INFO_PHYSICAL_ADDRESS;
-        e820[nr].size = ACPI_INFO_SIZE;
+        /* Reserved for internal use. */
+        e820[nr].addr = lowmem_reserved_base;
+        e820[nr].size = 0xA0000-lowmem_reserved_base;
         e820[nr].type = E820_RESERVED;
         nr++;
-
-        /* ACPI_INFO-lowmem_reserved_base: Ordinary RAM. */
-        e820[nr].addr = ACPI_INFO_PHYSICAL_END;
-        e820[nr].size = lowmem_reserved_base - ACPI_INFO_PHYSICAL_END;
-        e820[nr].type = E820_RAM;
-        nr++;
-    }
-
-    /* lowmem_reserved_base-0xa00000: reserved by BIOS implementation. */
-    if ( lowmem_reserved_base < 0xA0000 )
-    {
-            /* Reserved for internal use. */
-            e820[nr].addr = lowmem_reserved_base;
-            e820[nr].size = 0xA0000-lowmem_reserved_base;
-            e820[nr].type = E820_RESERVED;
-            nr++;
     }
 
     /*
diff -r 0844b17df7a9 tools/firmware/hvmloader/rombios.c
--- a/tools/firmware/hvmloader/rombios.c	Fri Nov 11 18:14:35 2011 +0000
+++ b/tools/firmware/hvmloader/rombios.c	Mon Nov 14 13:20:26 2011 +0000
@@ -47,7 +47,6 @@ static void rombios_setup_e820(void)
 {
     /*
      * 0x9E000-0x09F000: Stack.
-     * 0x9F000-0x09C000: ACPI info.
      * 0x9FC00-0x0A0000: Extended BIOS Data Area (EBDA).
      * ...
      * 0xE0000-0x0F0000: PC-specific area. We place various tables here.
diff -r 0844b17df7a9 tools/firmware/hvmloader/util.c
--- a/tools/firmware/hvmloader/util.c	Fri Nov 11 18:14:35 2011 +0000
+++ b/tools/firmware/hvmloader/util.c	Mon Nov 14 13:20:26 2011 +0000
@@ -343,7 +343,7 @@ void mem_hole_populate_ram(xen_pfn_t mfn
     }
 }
 
-static uint32_t reserve = RESERVED_MEMBASE - 1;
+static uint32_t reserve = RESERVED_MEMORY_DYNAMIC - 1;
 
 xen_pfn_t mem_hole_alloc(uint32_t nr_mfns)
 {

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

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

Re: Re: [PATCH] SeaBIOS/Xen: Compute the low RAM memory size in the BDA according to the e820

[Julian Pidancet]

On Mon, Nov 14, 2011 at 1:23 PM, Keir Fraser <keir@xen.org> wrote:
> On 14/11/2011 08:53, "Keir Fraser" <keir.xen@gmail.com> wrote:
>
>> On 14/11/2011 03:36, "Kevin O'Connor" <kevin@koconnor.net> wrote:
>>
>>> On Xen, the PCI init code isn't used, so assuming this struct doesn't
>>> need to live in real "ram", I think it could live just about anywhere
>>> past the end of ram.  Even with pciinit.c, addresses over 0xfc00000
>>> (with the exception of a few bytes for hpet, apic, ioapic, and bios
>>> image) could be used.
>>
>> I suggest we stick it at FC000000, and shift hvmloader's mem_alloc()
>> starting address up by one page to FC001000. The acpi build code will have
>> to manually mem_hole_populate_ram() that one page before writing to it. This
>> can then be documented in hvmloader/config.h which contains a description
>> of, and defines for, the system memory map. This is by far the easiest
>> solution to this problem; manually crafting an SSDT is a right pain in the
>> arse, whereas this is maybe a 5-line patch.
>
> Like the attached patch (untested), which is a bit larger than anticipated,
> but actually allows code to be net deleted. :-)
>

That was quick. I will give it a try shortly.

-- 
Julian

Re: Re: [PATCH] SeaBIOS/Xen: Compute the low RAM memory size in the BDA according to the e820

[Kevin O'Connor]

On Mon, Nov 14, 2011 at 01:23:41PM +0000, Keir Fraser wrote:
> On 14/11/2011 08:53, "Keir Fraser" <keir.xen@gmail.com> wrote:
> 
> > On 14/11/2011 03:36, "Kevin O'Connor" <kevin@koconnor.net> wrote:
> > 
> >> On Xen, the PCI init code isn't used, so assuming this struct doesn't
> >> need to live in real "ram", I think it could live just about anywhere
> >> past the end of ram.  Even with pciinit.c, addresses over 0xfc00000

Oops - that should have read 0xfec00000.

> >> (with the exception of a few bytes for hpet, apic, ioapic, and bios
> >> image) could be used.
> > 
> > I suggest we stick it at FC000000, and shift hvmloader's mem_alloc()

Since Xen lays out the PCI space (and assuming it wont put a PCI
device at that address) that should be fine.

> > starting address up by one page to FC001000. The acpi build code will have
> > to manually mem_hole_populate_ram() that one page before writing to it. This
> > can then be documented in hvmloader/config.h which contains a description
> > of, and defines for, the system memory map. This is by far the easiest
> > solution to this problem; manually crafting an SSDT is a right pain in the
> > arse, whereas this is maybe a 5-line patch.
> 
> Like the attached patch (untested), which is a bit larger than anticipated,
> but actually allows code to be net deleted. :-)

-Kevin

Re: [Xen-devel] Re: [PATCH] SeaBIOS/Xen: Compute the low RAM memory size in the BDA according to the e820

[Julian Pidancet]

On Mon, Nov 14, 2011 at 1:23 PM, Keir Fraser <keir@xen.org> wrote:
> On 14/11/2011 08:53, "Keir Fraser" <keir.xen@gmail.com> wrote:
>
>> On 14/11/2011 03:36, "Kevin O'Connor" <kevin@koconnor.net> wrote:
>>
>>> On Xen, the PCI init code isn't used, so assuming this struct doesn't
>>> need to live in real "ram", I think it could live just about anywhere
>>> past the end of ram.  Even with pciinit.c, addresses over 0xfc00000
>>> (with the exception of a few bytes for hpet, apic, ioapic, and bios
>>> image) could be used.
>>
>> I suggest we stick it at FC000000, and shift hvmloader's mem_alloc()
>> starting address up by one page to FC001000. The acpi build code will have
>> to manually mem_hole_populate_ram() that one page before writing to it. This
>> can then be documented in hvmloader/config.h which contains a description
>> of, and defines for, the system memory map. This is by far the easiest
>> solution to this problem; manually crafting an SSDT is a right pain in the
>> arse, whereas this is maybe a 5-line patch.
>
> Like the attached patch (untested), which is a bit larger than anticipated,
> but actually allows code to be net deleted. :-)
>

I just tested your patch with Windows 7 and Linux guest booted from iPXE.

Everything seems to work fine. SeaBIOS reports the following e820:

(XEN) HVM23: e820 map has 6 items:
(XEN) HVM23:   0: 0000000000000000 - 000000000009f400 = 1 RAM
(XEN) HVM23:   1: 000000000009f400 - 00000000000a0000 = 2 RESERVED
(XEN) HVM23:   2: 00000000000f0000 - 0000000000100000 = 2 RESERVED
(XEN) HVM23:   3: 0000000000100000 - 000000003f7ff000 = 1 RAM
(XEN) HVM23:   4: 000000003f7ff000 - 000000003f800000 = 2 RESERVED
(XEN) HVM23:   5: 00000000fc000000 - 0000000100000000 = 2 RESERVED

The ACPI code in Linux reports the right PCI memory window:

[    0.338966] PCI: Using host bridge windows from ACPI; if necessary,
use "pci=nocrs" and report a bug
[    0.340000] ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-ff])
[    0.341029] pci_root PNP0A03:00: host bridge window [io  0x0000-0x0cf7]
[    0.341965] pci_root PNP0A03:00: host bridge window [io  0x0d00-0xffff]
[    0.342965] pci_root PNP0A03:00: host bridge window [mem
0x000a0000-0x000bffff]
[    0.343965] pci_root PNP0A03:00: host bridge window [mem
0xf0000000-0xfbffffff]

Can you ship it ?

-- 
Julian

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

Re: [Xen-devel] Re: [PATCH] SeaBIOS/Xen: Compute the low RAM memory size in the BDA according to the e820

[Keir Fraser]

On 14/11/2011 19:25, "Julian Pidancet" <julian.pidancet@gmail.com> wrote:

> On Mon, Nov 14, 2011 at 1:23 PM, Keir Fraser <keir@xen.org> wrote:
>> On 14/11/2011 08:53, "Keir Fraser" <keir.xen@gmail.com> wrote:
>> 
>>> On 14/11/2011 03:36, "Kevin O'Connor" <kevin@koconnor.net> wrote:
>>> 
>>>> On Xen, the PCI init code isn't used, so assuming this struct doesn't
>>>> need to live in real "ram", I think it could live just about anywhere
>>>> past the end of ram.  Even with pciinit.c, addresses over 0xfc00000
>>>> (with the exception of a few bytes for hpet, apic, ioapic, and bios
>>>> image) could be used.
>>> 
>>> I suggest we stick it at FC000000, and shift hvmloader's mem_alloc()
>>> starting address up by one page to FC001000. The acpi build code will have
>>> to manually mem_hole_populate_ram() that one page before writing to it. This
>>> can then be documented in hvmloader/config.h which contains a description
>>> of, and defines for, the system memory map. This is by far the easiest
>>> solution to this problem; manually crafting an SSDT is a right pain in the
>>> arse, whereas this is maybe a 5-line patch.
>> 
>> Like the attached patch (untested), which is a bit larger than anticipated,
>> but actually allows code to be net deleted. :-)
>> 
> 
> I just tested your patch with Windows 7 and Linux guest booted from iPXE.
> 
> Everything seems to work fine. SeaBIOS reports the following e820:
> 
> (XEN) HVM23: e820 map has 6 items:
> (XEN) HVM23:   0: 0000000000000000 - 000000000009f400 = 1 RAM
> (XEN) HVM23:   1: 000000000009f400 - 00000000000a0000 = 2 RESERVED
> (XEN) HVM23:   2: 00000000000f0000 - 0000000000100000 = 2 RESERVED
> (XEN) HVM23:   3: 0000000000100000 - 000000003f7ff000 = 1 RAM
> (XEN) HVM23:   4: 000000003f7ff000 - 000000003f800000 = 2 RESERVED
> (XEN) HVM23:   5: 00000000fc000000 - 0000000100000000 = 2 RESERVED
> 
> The ACPI code in Linux reports the right PCI memory window:
> 
> [    0.338966] PCI: Using host bridge windows from ACPI; if necessary,
> use "pci=nocrs" and report a bug
> [    0.340000] ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-ff])
> [    0.341029] pci_root PNP0A03:00: host bridge window [io  0x0000-0x0cf7]
> [    0.341965] pci_root PNP0A03:00: host bridge window [io  0x0d00-0xffff]
> [    0.342965] pci_root PNP0A03:00: host bridge window [mem
> 0x000a0000-0x000bffff]
> [    0.343965] pci_root PNP0A03:00: host bridge window [mem
> 0xf0000000-0xfbffffff]
> 
> Can you ship it ?

Done. Xen-unstable:24143

 Thanks,
 Keir