[Qemu-devel] [RfC PATCH] e820: pass high memory too.

[Qemu-devel] [RfC PATCH] e820: pass high memory too.

[Gerd Hoffmann]

We have a fw_cfg entry to pass e820 entries from qemu to the firmware.
Today it's used to pass reservations only.  This patch makes qemu pass
entries for RAM too.

This allows to pass RAM sizes larger than 1TB to the firmware and it
will also allow to pass non-contignous memory ramges should we decide
to implement that some day, say for our virtual numa nodes.

Obviously this needs some extra care to not break existing firware.

SeaBIOS loads the entries and happily adds them without looking at the
type.  Which is problematic for memory below 4g as this will overwrite
reservations added for bios memory etc.  For memory above 4g it works
just fine, seabios will merge the entry derived from cmos with the one
loaded from fw_cfg.

OVMF doesn't look at the fw_cfg e820 table.
coreboot doesn't look at the fw_cfg e820 table.

Cc: Andrea Arcangeli <aarcange@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/i386/pc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0c313fe..ec5508b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1134,12 +1134,20 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,
     memory_region_init_alias(ram_below_4g, NULL, "ram-below-4g", ram,
                              0, below_4g_mem_size);
     memory_region_add_subregion(system_memory, 0, ram_below_4g);
+    if (0) {
+        /*
+         * Ideally we should do that too, but that would ruin the e820
+         * reservations added by seabios before initializing fw_cfg.
+         */
+        e820_add_entry(0, below_4g_mem_size, E820_RAM);
+    }
     if (above_4g_mem_size > 0) {
         ram_above_4g = g_malloc(sizeof(*ram_above_4g));
         memory_region_init_alias(ram_above_4g, NULL, "ram-above-4g", ram,
                                  below_4g_mem_size, above_4g_mem_size);
         memory_region_add_subregion(system_memory, 0x100000000ULL,
                                     ram_above_4g);
+        e820_add_entry(0x100000000ULL, above_4g_mem_size, E820_RAM);
     }
 
 
-- 
1.8.3.1

Re: [Qemu-devel] [RfC PATCH] e820: pass high memory too.

[Igor Mammedov]

On Thu, 10 Oct 2013 14:54:29 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> We have a fw_cfg entry to pass e820 entries from qemu to the firmware.
> Today it's used to pass reservations only.  This patch makes qemu pass
> entries for RAM too.
> 
> This allows to pass RAM sizes larger than 1TB to the firmware and it
> will also allow to pass non-contignous memory ramges should we decide
> to implement that some day, say for our virtual numa nodes.
> 
> Obviously this needs some extra care to not break existing firware.
> 
> SeaBIOS loads the entries and happily adds them without looking at the
> type.  Which is problematic for memory below 4g as this will overwrite
> reservations added for bios memory etc.  For memory above 4g it works
> just fine, seabios will merge the entry derived from cmos with the one
> loaded from fw_cfg.
It will make amount of available memory in e820 table more than described
in smbios and could break MS's SMBIOS HCT test. Perhaps related smbios info
also should be picked up from QEMU.

> 
> OVMF doesn't look at the fw_cfg e820 table.
> coreboot doesn't look at the fw_cfg e820 table.
> 
> Cc: Andrea Arcangeli <aarcange@redhat.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/i386/pc.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 0c313fe..ec5508b 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1134,12 +1134,20 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,
>      memory_region_init_alias(ram_below_4g, NULL, "ram-below-4g", ram,
>                               0, below_4g_mem_size);
>      memory_region_add_subregion(system_memory, 0, ram_below_4g);
> +    if (0) {
> +        /*
> +         * Ideally we should do that too, but that would ruin the e820
> +         * reservations added by seabios before initializing fw_cfg.
> +         */
> +        e820_add_entry(0, below_4g_mem_size, E820_RAM);
> +    }
>      if (above_4g_mem_size > 0) {
>          ram_above_4g = g_malloc(sizeof(*ram_above_4g));
>          memory_region_init_alias(ram_above_4g, NULL, "ram-above-4g", ram,
>                                   below_4g_mem_size, above_4g_mem_size);
>          memory_region_add_subregion(system_memory, 0x100000000ULL,
>                                      ram_above_4g);
> +        e820_add_entry(0x100000000ULL, above_4g_mem_size, E820_RAM);
>      }
>  
>  

Re: [Qemu-devel] [RfC PATCH] e820: pass high memory too.

[Gerd Hoffmann]

On Fr, 2013-10-11 at 13:20 +0200, Igor Mammedov wrote:
> On Thu, 10 Oct 2013 14:54:29 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> > We have a fw_cfg entry to pass e820 entries from qemu to the firmware.
> > Today it's used to pass reservations only.  This patch makes qemu pass
> > entries for RAM too.
> > 
> > This allows to pass RAM sizes larger than 1TB to the firmware and it
> > will also allow to pass non-contignous memory ramges should we decide
> > to implement that some day, say for our virtual numa nodes.
> > 
> > Obviously this needs some extra care to not break existing firware.
> > 
> > SeaBIOS loads the entries and happily adds them without looking at the
> > type.  Which is problematic for memory below 4g as this will overwrite
> > reservations added for bios memory etc.  For memory above 4g it works
> > just fine, seabios will merge the entry derived from cmos with the one
> > loaded from fw_cfg.
> It will make amount of available memory in e820 table more than described
> in smbios and could break MS's SMBIOS HCT test.

Happens only in case the installed amount of memory is larger than 1TB
(which is the maximum the cmos can describe).  Such a setup doesn't work
correctly without the patch anyway, so it isn't a regression IMHO.  And
with an additional patch on the seabios side we can fix the smbios info
even for the >1TB case.

>  Perhaps related smbios info
> also should be picked up from QEMU.

Worth investigating, but lets concentrate on the acpi table merge first.

cheers,
  Gerd

Re: [Qemu-devel] [RfC PATCH] e820: pass high memory too.

[Igor Mammedov]

On Fri, 11 Oct 2013 14:27:12 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> On Fr, 2013-10-11 at 13:20 +0200, Igor Mammedov wrote:
> > On Thu, 10 Oct 2013 14:54:29 +0200
> > Gerd Hoffmann <kraxel@redhat.com> wrote:
> > 
> > > We have a fw_cfg entry to pass e820 entries from qemu to the firmware.
> > > Today it's used to pass reservations only.  This patch makes qemu pass
> > > entries for RAM too.
> > > 
> > > This allows to pass RAM sizes larger than 1TB to the firmware and it
> > > will also allow to pass non-contignous memory ramges should we decide
> > > to implement that some day, say for our virtual numa nodes.
> > > 
> > > Obviously this needs some extra care to not break existing firware.
> > > 
> > > SeaBIOS loads the entries and happily adds them without looking at the
> > > type.  Which is problematic for memory below 4g as this will overwrite
> > > reservations added for bios memory etc.  For memory above 4g it works
> > > just fine, seabios will merge the entry derived from cmos with the one
> > > loaded from fw_cfg.
> > It will make amount of available memory in e820 table more than described
> > in smbios and could break MS's SMBIOS HCT test.
> 
> Happens only in case the installed amount of memory is larger than 1TB
> (which is the maximum the cmos can describe).  Such a setup doesn't work
> correctly without the patch anyway, so it isn't a regression IMHO.  And
> with an additional patch on the seabios side we can fix the smbios info
> even for the >1TB case.
True, and it looks better then adding extra CMOS ports.

BTW: what about OVMF and core boot, how would they know that there is more
then 1Tb RAM?

> 
> >  Perhaps related smbios info
> > also should be picked up from QEMU.
> 
> Worth investigating, but lets concentrate on the acpi table merge first.
> 
> cheers,
>   Gerd
> 
> 

Re: [Qemu-devel] [RfC PATCH] e820: pass high memory too.

[Gerd Hoffmann]

  Hi,

> BTW: what about OVMF and core boot, how would they know that there is more
> then 1Tb RAM?

They should look at fw_cfg too.  Both have support for fw_cfg already,
but don't parse the e820 table today.  Shouldn't be that hard to add
though.

cheers,
  Gerd