[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] pc: memhotplug: keep reserved-memory-end br
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] pc: memhotplug: keep reserved-memory-end broken on 2.4 and earlier machines |
Date: |
Wed, 9 Sep 2015 17:14:38 -0300 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Mon, Sep 07, 2015 at 01:55:32PM +0200, Igor Mammedov wrote:
> it will prevent guests on old machines from seeing
> inconsistent memory mapping in firmware/ACPI views.
>
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
> PS: this patch probably clashes with Eduardo's
> pc_compat refactoring, so I've split it from
> main fix, if applied via Eduardo's it could
> be squashed in main fix after merging on top
> of his patches or as is if goes in first.
It conflicts with my "pc: Initialization and compat function cleanup"
series, but my series needs to be rebased because of smbios changes
anyway. I can rebase my series after your fix gets in.
[...]
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 354e1b3..b5107f7 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1412,8 +1412,12 @@ FWCfgState *pc_memory_init(PCMachineState *pcms,
>
> if (guest_info->has_reserved_memory && pcms->hotplug_memory.base) {
> uint64_t *val = g_malloc(sizeof(*val));
> - uint64_t res_mem_end = pcms->hotplug_memory.base +
> - memory_region_size(&pcms->hotplug_memory.mr);
> + PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
Style suggestion: move pcmc to the beginning of the function, because
other blocks inside the function are likely to look at PCMachineClass
fields in the future.
> + uint64_t res_mem_end = pcms->hotplug_memory.base;
> +
> + if (!pcmc->broken_reserved_end) {
> + res_mem_end += memory_region_size(&pcms->hotplug_memory.mr);
> + }
> *val = cpu_to_le64(ROUND_UP(res_mem_end, 0x1ULL << 30));
> fw_cfg_add_file(fw_cfg, "etc/reserved-memory-end", val,
> sizeof(*val));
> }
[...]
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index d0cad87..ff0b48b 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -59,6 +59,7 @@ struct PCMachineClass {
> MachineClass parent_class;
>
> /*< public >*/
> + bool broken_reserved_end;
A comment describing what exactly the new field does would be welcome.
I don't think my suggestions should block the patch, so:
Reviewed-by: Eduardo Habkost <address@hidden>
--
Eduardo