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