Re: [Qemu-devel] [PATCH v2 30/31] pc: ACPI BIOS: reserve SRAT entry for hotplug mem hole

[Michael S. Tsirkin]

On Tue, May 20, 2014 at 05:15:33PM +0200, Igor Mammedov wrote:
> Needed for Windows to use hotplugged memory device, otherwise
> it complains that server is not configured for memory hotplug.
> Tests shows that aftewards it uses dynamically provided
> proximity value from _PXM() method if available.
> 
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
>  hw/i386/acpi-build.c |   14 ++++++++++++++
>  1 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 58e7306..97e3a82 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1199,6 +1199,8 @@ build_srat(GArray *table_data, GArray *linker,
>      uint64_t curnode;
>      int srat_start, numa_start, slots;
>      uint64_t mem_len, mem_base, next_base;
> +    PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> +    ram_addr_t hotplug_as_size = memory_region_size(&pcms->hotplug_memory);
>  
>      srat_start = table_data->len;
>

Please don't abbreviate address space as "as". If you abbreviate as as
as it can be misunderstood for the English as which stands for as.
You see how confusing this can be :)
How about hotplug_memory_max_size?

Also, it would be a bit more elegant to make this a read-only property
of the machine.
  
> @@ -1263,6 +1265,18 @@ build_srat(GArray *table_data, GArray *linker,
>          acpi_build_srat_memory(numamem, 0, 0, 0, MEM_AFFINITY_NOFLAGS);
>      }
>  
> +    /*
> +     * Fake entry required by Windows to enable memory hotplug in OS.
> +     * Individual DIMM devices override proximity set here via _PXM method,
> +     * which returns associated with it NUMA node id.
> +     */
> +    if (hotplug_as_size) {
> +        numamem = acpi_data_push(table_data, sizeof *numamem);
> +        acpi_build_srat_memory(numamem, pcms->hotplug_memory_base,
> +                               hotplug_as_size, 0, MEM_AFFINITY_HOTPLUGGABLE 
> |
> +                               MEM_AFFINITY_ENABLED);
> +    }
> +
>      build_header(linker, table_data,
>                   (void *)(table_data->data + srat_start),
>                   "SRAT",
> -- 
> 1.7.1