On Wed, Sep 7, 2022 at 18:18 Ani Sinha <a...@anisinha.ca> wrote:
> On Wed, Aug 31, 2022 at 10:23 AM Ani Sinha <a...@anisinha.ca> wrote:
> >
> > e820 reserved entries were used before the dynamic entries with fw
> config files
> > were intoduced. Please see the following change:
> > 7d67110f2d9a6("pc: add etc/e820 fw_cfg file")
> >
> > Identical support was introduced into seabios as well with the following
> commit:
> > ce39bd4031820 ("Add support for etc/e820 fw_cfg file")
> >
> > Both the above commits are now quite old. QEMU machines 1.7 and newer no
> longer
> > use the reserved entries. Seabios uses fw config files and
> > dynamic e820 entries by default and only falls back to using reserved
> entries
> > when it has to work with old qemu (versions earlier than 1.7). Please see
> > functions qemu_cfg_e820() and qemu_early_e820(). It is safe to remove
> legacy
> > FW_CFG_E820_TABLE and associated code now as QEMU 7.0 has deprecated
> i440fx
> > machines 1.7 and older. It would be incredibly rare to run the latest
> qemu
> > version with a very old version of seabios that did not support fw
> config files
> > for e820.
> >
> > As far as I could see, edk2/ovfm never supported reserved entries and
> uses fw
> > config files from the beginning. So there should be no incompatibilities
> with
> > ovfm as well.
> >
> > CC: Gerd Hoffmann <kra...@redhat.com>
> > Signed-off-by: Ani Sinha <a...@anisinha.ca>
> > Acked-by: Gerd Hoffmann <kra...@redhat.com>
>
> michael, please pick this one as well for the next pull. thanks.
Michael, seems you kissed this one.
>
> > ---
> > hw/i386/e820_memory_layout.c | 20 +-------------------
> > hw/i386/e820_memory_layout.h | 8 --------
> > hw/i386/fw_cfg.c | 3 ---
> > hw/i386/fw_cfg.h | 1 -
> > hw/i386/microvm.c | 2 --
> > 5 files changed, 1 insertion(+), 33 deletions(-)
> >
> > Please see:
> >
> https://patchwork.ozlabs.org/project/qemu-devel/patch/20220420043904.1225153-1-...@anisinha.ca/
> > for the previous post. Now that we are in 7.2 devel cycle, time to push
> > this patch.
> >
> > diff --git a/hw/i386/e820_memory_layout.c b/hw/i386/e820_memory_layout.c
> > index bcf9eaf837..06970ac44a 100644
> > --- a/hw/i386/e820_memory_layout.c
> > +++ b/hw/i386/e820_memory_layout.c
> > @@ -11,29 +11,11 @@
> > #include "e820_memory_layout.h"
> >
> > static size_t e820_entries;
> > -struct e820_table e820_reserve;
> > struct e820_entry *e820_table;
> >
> > int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
> > {
> > - int index = le32_to_cpu(e820_reserve.count);
> > - struct e820_entry *entry;
> > -
> > - if (type != E820_RAM) {
> > - /* old FW_CFG_E820_TABLE entry -- reservations only */
> > - if (index >= E820_NR_ENTRIES) {
> > - return -EBUSY;
> > - }
> > - entry = &e820_reserve.entry[index++];
> > -
> > - entry->address = cpu_to_le64(address);
> > - entry->length = cpu_to_le64(length);
> > - entry->type = cpu_to_le32(type);
> > -
> > - e820_reserve.count = cpu_to_le32(index);
> > - }
> > -
> > - /* new "etc/e820" file -- include ram too */
> > + /* new "etc/e820" file -- include ram and reserved entries */
> > e820_table = g_renew(struct e820_entry, e820_table, e820_entries +
> 1);
> > e820_table[e820_entries].address = cpu_to_le64(address);
> > e820_table[e820_entries].length = cpu_to_le64(length);
> > diff --git a/hw/i386/e820_memory_layout.h b/hw/i386/e820_memory_layout.h
> > index 04f93780f9..7c239aa033 100644
> > --- a/hw/i386/e820_memory_layout.h
> > +++ b/hw/i386/e820_memory_layout.h
> > @@ -16,20 +16,12 @@
> > #define E820_NVS 4
> > #define E820_UNUSABLE 5
> >
> > -#define E820_NR_ENTRIES 16
> > -
> > struct e820_entry {
> > uint64_t address;
> > uint64_t length;
> > uint32_t type;
> > } QEMU_PACKED __attribute((__aligned__(4)));
> >
> > -struct e820_table {
> > - uint32_t count;
> > - struct e820_entry entry[E820_NR_ENTRIES];
> > -} QEMU_PACKED __attribute((__aligned__(4)));
> > -
> > -extern struct e820_table e820_reserve;
> > extern struct e820_entry *e820_table;
> >
> > int e820_add_entry(uint64_t address, uint64_t length, uint32_t type);
> > diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
> > index a283785a8d..72a42f3c66 100644
> > --- a/hw/i386/fw_cfg.c
> > +++ b/hw/i386/fw_cfg.c
> > @@ -36,7 +36,6 @@ const char *fw_cfg_arch_key_name(uint16_t key)
> > {FW_CFG_ACPI_TABLES, "acpi_tables"},
> > {FW_CFG_SMBIOS_ENTRIES, "smbios_entries"},
> > {FW_CFG_IRQ0_OVERRIDE, "irq0_override"},
> > - {FW_CFG_E820_TABLE, "e820_table"},
> > {FW_CFG_HPET, "hpet"},
> > };
> >
> > @@ -127,8 +126,6 @@ FWCfgState *fw_cfg_arch_create(MachineState *ms,
> > #endif
> > fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, 1);
> >
> > - fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE,
> > - &e820_reserve, sizeof(e820_reserve));
> > fw_cfg_add_file(fw_cfg, "etc/e820", e820_table,
> > sizeof(struct e820_entry) * e820_get_num_entries());
> >
> > diff --git a/hw/i386/fw_cfg.h b/hw/i386/fw_cfg.h
> > index 275f15c1c5..86ca7c1c0c 100644
> > --- a/hw/i386/fw_cfg.h
> > +++ b/hw/i386/fw_cfg.h
> > @@ -17,7 +17,6 @@
> > #define FW_CFG_ACPI_TABLES (FW_CFG_ARCH_LOCAL + 0)
> > #define FW_CFG_SMBIOS_ENTRIES (FW_CFG_ARCH_LOCAL + 1)
> > #define FW_CFG_IRQ0_OVERRIDE (FW_CFG_ARCH_LOCAL + 2)
> > -#define FW_CFG_E820_TABLE (FW_CFG_ARCH_LOCAL + 3)
> > #define FW_CFG_HPET (FW_CFG_ARCH_LOCAL + 4)
> >
> > FWCfgState *fw_cfg_arch_create(MachineState *ms,
> > diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
> > index 52cafa003d..a591161c02 100644
> > --- a/hw/i386/microvm.c
> > +++ b/hw/i386/microvm.c
> > @@ -324,8 +324,6 @@ static void microvm_memory_init(MicrovmMachineState
> *mms)
> > fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, machine->smp.max_cpus);
> > fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE,
> (uint64_t)machine->ram_size);
> > fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, 1);
> > - fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE,
> > - &e820_reserve, sizeof(e820_reserve));
> > fw_cfg_add_file(fw_cfg, "etc/e820", e820_table,
> > sizeof(struct e820_entry) * e820_get_num_entries());
> >
> > --
> > 2.34.1
> >
>
