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

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

[Andre Przywara]

[-- Attachment #1: Type: text/plain, Size: 629 bytes --]

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>

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

[-- Attachment #2: qemunuma_hostalloc.patch --]
[-- Type: text/x-patch, Size: 4688 bytes --]

# 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

diff -r 394d02758aa4 -r 0501b7490a00 Makefile.target
--- a/Makefile.target	Thu Dec 11 11:36:21 2008 +0100
+++ b/Makefile.target	Thu Dec 11 11:42:41 2008 +0100
@@ -600,6 +600,10 @@ endif
 endif
 ifdef CONFIG_CS4231A
 SOUND_HW += cs4231a.o
+endif
+
+ifdef CONFIG_NUMA
+LIBS += -lnuma
 endif
 
 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"
@@ -462,6 +464,7 @@ echo "  --audio-card-list=LIST   set lis
 echo "  --audio-card-list=LIST   set list of additional emulated audio cards"
 echo "                           Available cards: ac97 adlib cs4231a gus"
 echo "  --enable-mixemu          enable mixer emulation"
+echo "  --disable-numa           disable NUMA support (host side)"
 echo "  --disable-brlapi         disable BrlAPI"
 echo "  --disable-vnc-tls        disable TLS encryption for VNC server"
 echo "  --disable-curses         disable curses output"
@@ -877,6 +880,21 @@ done
 done
 
 ##########################################
+# libnuma probe
+
+if test -z "$numa" ; then
+    numa=no
+
+    cat > $TMPC << EOF
+#include <numa.h>
+int main(void) { return numa_available(); }
+EOF
+    if $cc ${ARCH_CFLAGS} -o $TMPE ${OS_CFLAGS} $TMPC -lnuma > /dev/null 2> /dev/null ; then
+	    numa=yes
+    fi
+fi
+
+##########################################
 # BrlAPI probe
 
 if test -z "$brlapi" ; then
@@ -1033,6 +1051,7 @@ echo "Audio drivers     $audio_drv_list"
 echo "Audio drivers     $audio_drv_list"
 echo "Extra audio cards $audio_card_list"
 echo "Mixer emulation   $mixemu"
+echo "NUMA support      $numa"
 echo "VNC TLS support   $vnc_tls"
 if test "$vnc_tls" = "yes" ; then
     echo "    TLS CFLAGS    $vnc_tls_cflags"
@@ -1272,6 +1291,10 @@ if test "$mixemu" = "yes" ; then
 if test "$mixemu" = "yes" ; then
   echo "CONFIG_MIXEMU=yes" >> $config_mak
   echo "#define CONFIG_MIXEMU 1" >> $config_h
+fi
+if test "$numa" = "yes" ; then
+  echo "CONFIG_NUMA=yes" >> $config_mak
+  echo "#define CONFIG_NUMA 1" >> $config_h
 fi
 if test "$vnc_tls" = "yes" ; then
   echo "CONFIG_VNC_TLS=yes" >> $config_mak
diff -r 394d02758aa4 -r 0501b7490a00 hw/fw_cfg.h
--- a/hw/fw_cfg.h	Thu Dec 11 11:36:21 2008 +0100
+++ b/hw/fw_cfg.h	Thu Dec 11 11:42:41 2008 +0100
@@ -8,6 +8,9 @@
 #define FW_CFG_NOGRAPHIC        0x04
 #define FW_CFG_NB_CPUS          0x05
 #define FW_CFG_MACHINE_ID       0x06
+#define FW_CFG_NUMA_NODES       0x07
+#define FW_CFG_NUMA_NODE_CPUS   0x08
+#define FW_CFG_NUMA_NODE_MEM    0x09
 #define FW_CFG_MAX_ENTRY        0x10
 
 #define FW_CFG_WRITE_CHANNEL    0x4000
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);
 }
 
 /* Generate an initial boot sector which sets state and jump to
diff -r 394d02758aa4 -r 0501b7490a00 vl.c
--- a/vl.c	Thu Dec 11 11:36:21 2008 +0100
+++ b/vl.c	Thu Dec 11 11:42:41 2008 +0100
@@ -93,6 +93,9 @@
 
 #include <linux/ppdev.h>
 #include <linux/parport.h>
+#ifdef CONFIG_NUMA
+#include <numa.h>
+#endif
 #endif
 #ifdef __sun__
 #include <sys/stat.h>
@@ -5449,6 +5452,21 @@ int main(int argc, char **argv, char **e
         exit(1);
     }
 
+#ifdef CONFIG_NUMA
+    if (numnumanodes > 0 && numa_available() != -1) {
+        unsigned long offset = 0;
+        int i;
+
+        for (i = 0; i < numnumanodes; ++i) {
+            if (hostnodes[i] != (uint64_t)-1) {
+                numa_tonode_memory (phys_ram_base + offset, node_mem[i],
+                    hostnodes[i] % (numa_max_node() + 1));
+            }
+            offset += node_mem[i];
+        }
+    }
+#endif
+
     /* init the dynamic translator */
     cpu_exec_init_all(tb_size * 1024 * 1024);
 

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

[Anthony Liguori]

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?

> diff -r 394d02758aa4 -r 0501b7490a00 Makefile.target
> --- a/Makefile.target	Thu Dec 11 11:36:21 2008 +0100
> +++ b/Makefile.target	Thu Dec 11 11:42:41 2008 +0100
> @@ -600,6 +600,10 @@ endif
>  endif
>  ifdef CONFIG_CS4231A
>  SOUND_HW += cs4231a.o
> +endif
> +
> +ifdef CONFIG_NUMA
> +LIBS += -lnuma
>  endif

Should introduce NUMALIBS and set this from the configure as the rest of 
the library dependencies work.

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

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

>  
>  /* Generate an initial boot sector which sets state and jump to
> diff -r 394d02758aa4 -r 0501b7490a00 vl.c
> --- a/vl.c	Thu Dec 11 11:36:21 2008 +0100
> +++ b/vl.c	Thu Dec 11 11:42:41 2008 +0100
> @@ -93,6 +93,9 @@
>  
>  #include <linux/ppdev.h>
>  #include <linux/parport.h>
> +#ifdef CONFIG_NUMA
> +#include <numa.h>
> +#endif
>  #endif
>  #ifdef __sun__
>  #include <sys/stat.h>
> @@ -5449,6 +5452,21 @@ int main(int argc, char **argv, char **e
>          exit(1);
>      }
>  
> +#ifdef CONFIG_NUMA
> +    if (numnumanodes > 0 && numa_available() != -1) {
> +        unsigned long offset = 0;
> +        int i;
> +
> +        for (i = 0; i < numnumanodes; ++i) {
> +            if (hostnodes[i] != (uint64_t)-1) {
> +                numa_tonode_memory (phys_ram_base + offset, node_mem[i],
> +                    hostnodes[i] % (numa_max_node() + 1));
> +            }
> +            offset += node_mem[i];
> +        }
> +    }
> +#endif
>   

At this point, I'm okay with introducing the libnuma dependency for 
memory pinning.  I'm not sure I think we should even present this as a 
command line option though.  I think the command line option should just 
specify the NUMA topology and then we should use a monitor command for 
pinning (as you do on your next patch).

I'd like to see some more documentation of this functionality.

Regards,

Anthony Liguori

>      /* init the dynamic translator */
>      cpu_exec_init_all(tb_size * 1024 * 1024);
>   

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

[Avi Kivity]

Anthony Liguori wrote:
>
> At this point, I'm okay with introducing the libnuma dependency for 
> memory pinning.  I'm not sure I think we should even present this as a 
> command line option though.  I think the command line option should 
> just specify the NUMA topology and then we should use a monitor 
> command for pinning (as you do on your next patch).

It's helpful to have static pinning from the command line.  That's 
useful for quick tests for those of us who don't use management interfaces.

Since the monitor and command line can share code, there shouldn't be 
any cost to this.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

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

[Andre Przywara]

Avi Kivity wrote:
> Anthony Liguori wrote:
>>
>> At this point, I'm okay with introducing the libnuma dependency for 
>> memory pinning.  I'm not sure I think we should even present this as a 
>> command line option though.  I think the command line option should 
>> just specify the NUMA topology and then we should use a monitor 
>> command for pinning (as you do on your next patch).
> 
> It's helpful to have static pinning from the command line.  That's 
> useful for quick tests for those of us who don't use management interfaces.
> 
> Since the monitor and command line can share code, there shouldn't be 
> any cost to this.
ACK. Actually I'd had to add code to prevent pinning from the command 
line. I think this doesn't hurt, if you use virtualization for 
partitioning (where a NUMA architecture can actually help you, because 
guests don't compete will all other guests for the memory bandwidth), it 
is quite helpful to specify everything at the beginning. Here even UP 
guest can take advantage:
-numa 1,pin:1

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

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

[Anthony Liguori]

Andre Przywara wrote:
> Avi Kivity wrote:
>> Anthony Liguori wrote:
>>>
>>> At this point, I'm okay with introducing the libnuma dependency for 
>>> memory pinning.  I'm not sure I think we should even present this as 
>>> a command line option though.  I think the command line option 
>>> should just specify the NUMA topology and then we should use a 
>>> monitor command for pinning (as you do on your next patch).
>>
>> It's helpful to have static pinning from the command line.  That's 
>> useful for quick tests for those of us who don't use management 
>> interfaces.
>>
>> Since the monitor and command line can share code, there shouldn't be 
>> any cost to this.
> ACK. Actually I'd had to add code to prevent pinning from the command 
> line. I think this doesn't hurt, if you use virtualization for 
> partitioning (where a NUMA architecture can actually help you, because 
> guests don't compete will all other guests for the memory bandwidth), 
> it is quite helpful to specify everything at the beginning.

The thing that makes me uneasy about it is the fact that it's part of 
the base numa syntax.  This combination of guest configuration and host 
configuration seems less than optimal to me.  This isn't something that 
bothers me that much and won't keep me from applying the patches.

Regards,

Anthony Liguori

> Here even UP guest can take advantage:
> -numa 1,pin:1
>
> Regards,
> Andre.
>

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

[Avi Kivity]

Anthony Liguori wrote:
>> ACK. Actually I'd had to add code to prevent pinning from the command 
>> line. I think this doesn't hurt, if you use virtualization for 
>> partitioning (where a NUMA architecture can actually help you, 
>> because guests don't compete will all other guests for the memory 
>> bandwidth), it is quite helpful to specify everything at the beginning.
>
> The thing that makes me uneasy about it is the fact that it's part of 
> the base numa syntax.  This combination of guest configuration and 
> host configuration seems less than optimal to me.  This isn't 
> something that bothers me that much and won't keep me from applying 
> the patches.

I also think that it would be good to separate guest and host 
configuration, like -net nic and -net tap.

-- 
error compiling committee.c: too many arguments to function

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

[Andre Przywara]

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