[Qemu-devel] Re: [PATCH 2/3] NUMA: promoting NUMA topology to BIOS and pin guest memory

[Andre Przywara <andre.przywara@amd.com>]

Anthony Liguori wrote:
> Andre Przywara wrote:
>> This patch pushes the parsed NUMA topology via the firmware 
>> configuration interface to the BIOS and pins the guest memory (if 
>> desired).
>>
>> Signed-off-by: Andre Przywara <andre.przywara@amd.com>
>>
>> # HG changeset patch
>> # User Andre Przywara <andre.przywara@amd.com>
>> # Date 1228992161 -3600
>> # Node ID 0501b7490a00ef7a77e69f846d332f797162052a
>> # Parent  394d02758aa4358be3bcd14f9d59efaf42e89328
>> promoting NUMA topology to BIOS and pin guest memory
>>   
> 
> Do you have a BIOS patch too?
Actually I was waiting with this part as you said you wanted to sync the 
QEMU's BIOS with the upstream BOCHS one, which would made my patch a lot 
easier. I can prepare a patch based on the current version in QEMU, but 
that diff would include code which is already in upstream BOCHS, which 
would complicate the next merge.

>>  
>>  ifdef CONFIG_VNC_TLS
>> diff -r 394d02758aa4 -r 0501b7490a00 configure
>> --- a/configure    Thu Dec 11 11:36:21 2008 +0100
>> +++ b/configure    Thu Dec 11 11:42:41 2008 +0100
>> @@ -368,6 +368,8 @@ for opt do
>>    ;;
>>    --enable-mixemu) mixemu="yes"
>>    ;;
>> +  --disable-numa) numa="no"
>> +  ;;
>>    --disable-aio) aio="no"
>>    ;;
>>    --disable-blobs) blobs="no"
>>   
> 
> Need to set numa="yes" as a default.
Well, it seems there are two ways to do this in QEMU's configure:
1. (as in aio): Default to yes, optionally disable, if still set to yes 
compile check
2. (as in brlapi): no default, optionally set to "no", if empty string 
compile check and set to yes or no accordingly
Seems like I copied the wrong version ;-)

> 
>> diff -r 394d02758aa4 -r 0501b7490a00 hw/pc.c
>> --- a/hw/pc.c    Thu Dec 11 11:36:21 2008 +0100
>> +++ b/hw/pc.c    Thu Dec 11 11:42:41 2008 +0100
>> @@ -436,6 +436,12 @@ static void bochs_bios_init(void)
>>      fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
>>      fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
>>      fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
>> +    fw_cfg_add_i16(fw_cfg, FW_CFG_NUMA_NODES, numnumanodes);
>> +
>> +    fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA_NODE_MEM, (uint8_t*)node_mem,
>> +        sizeof(node_mem[0]) * numnumanodes);
>> +    fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA_NODE_CPUS, 
>> (uint8_t*)node_to_cpus,
>> +        sizeof(node_to_cpus[0]) * numnumanodes);
>>  }
> 
> This stuff (the firmware awareness) should be independent of the libnuma 
> support.
Moved to the first patch.

Thanks for the detailed review. I will address your other comments as well.

Regards,
Andre.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 277-84917
----to satisfy European Law for business letters:
AMD Saxony Limited Liability Company & Co. KG,
Wilschdorfer Landstr. 101, 01109 Dresden, Germany
Register Court Dresden: HRA 4896, General Partner authorized
to represent: AMD Saxony LLC (Wilmington, Delaware, US)
General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy