All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v4 0/2] hw/acpi-build: Fix ACPI SRAT Memory Affinity building
@ 2017-08-22  3:24 Dou Liyang
  2017-08-22  3:24 ` [Qemu-devel] [PATCH v4 1/2] hw/acpi-build: Fix SRAT memory building when there is no memory in node0 Dou Liyang
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Dou Liyang @ 2017-08-22  3:24 UTC (permalink / raw)
  To: qemu-devel, ehabkost, imammedo; +Cc: pbonzini, rth, mst, Dou Liyang

V3 --> v4:
  -add a new testcase.

This patchset fixs an ACPI building bug which caused by no RAM
in the first NUAM node. and also add a new testcase for the bug.

Dou Liyang (2):
  hw/acpi-build: Fix SRAT memory building when there is no memory in
    node0
  ACPI/unit-test: Add a new testcase for RAM allocation in numa node

 hw/i386/acpi-build.c                  |  78 ++++++++++++++++++++++------------
 tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
 tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
 tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
 tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
 tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
 tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
 tests/bios-tables-test.c              |  30 +++++++++++++
 8 files changed, 80 insertions(+), 28 deletions(-)
 create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
 create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
 create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
 create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
 create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
 create mode 100644 tests/acpi-test-data/q35/SRAT.numamem

-- 
2.5.5

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v4 1/2] hw/acpi-build: Fix SRAT memory building when there is no memory in node0
  2017-08-22  3:24 [Qemu-devel] [PATCH v4 0/2] hw/acpi-build: Fix ACPI SRAT Memory Affinity building Dou Liyang
@ 2017-08-22  3:24 ` Dou Liyang
  2017-08-30 20:55   ` Eduardo Habkost
  2017-08-22  3:24 ` [Qemu-devel] [PATCH v4 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node Dou Liyang
  2017-08-23  2:48 ` [Qemu-devel] [PATCH v4 0/2] hw/acpi-build: Fix ACPI SRAT Memory Affinity building Michael S. Tsirkin
  2 siblings, 1 reply; 19+ messages in thread
From: Dou Liyang @ 2017-08-22  3:24 UTC (permalink / raw)
  To: qemu-devel, ehabkost, imammedo; +Cc: pbonzini, rth, mst, Dou Liyang

Currently, Using the fisrt node without memory on the machine makes
QEMU unhappy. With this example command line:
  ... \
  -m 1024M,slots=4,maxmem=32G \
  -numa node,nodeid=0 \
  -numa node,mem=1024M,nodeid=1 \
  -numa node,nodeid=2 \
  -numa node,nodeid=3 \
Guest reports "No NUMA configuration found" and the NUMA topology is
wrong.

This is because when QEMU builds ACPI SRAT, it regards node0 as the
default node to deal with the memory hole(640K-1M). this means the
node0 must have some memory(>1M), but, actually it can have no
memory.

Fix this problem by replace the node0 with the first node which has
memory on it. Add a new function for each node. Also do some cleanup.

Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
 hw/i386/acpi-build.c | 78 +++++++++++++++++++++++++++++++++-------------------
 1 file changed, 50 insertions(+), 28 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 98dd424..f93d712 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2318,15 +2318,43 @@ build_tpm2(GArray *table_data, BIOSLinker *linker)
                  (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, NULL, NULL);
 }
 
+static uint64_t
+build_srat_node_entry(GArray *table_data, PCMachineState *pcms,
+                                int i, uint64_t mem_base, uint64_t mem_len)
+{
+    AcpiSratMemoryAffinity *numamem;
+    uint64_t next_base;
+
+    next_base = mem_base + mem_len;
+
+    /* Cut out the ACPI_PCI hole */
+    if (mem_base <= pcms->below_4g_mem_size &&
+        next_base > pcms->below_4g_mem_size) {
+        mem_len -= next_base - pcms->below_4g_mem_size;
+        if (mem_len > 0) {
+            numamem = acpi_data_push(table_data, sizeof *numamem);
+            build_srat_memory(numamem, mem_base, mem_len, i,
+                              MEM_AFFINITY_ENABLED);
+        }
+        mem_base = 1ULL << 32;
+        mem_len = next_base - pcms->below_4g_mem_size;
+        next_base += (1ULL << 32) - pcms->below_4g_mem_size;
+    }
+    numamem = acpi_data_push(table_data, sizeof *numamem);
+    build_srat_memory(numamem, mem_base, mem_len, i,
+                      MEM_AFFINITY_ENABLED);
+    return next_base;
+}
+
 static void
 build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
 {
     AcpiSystemResourceAffinityTable *srat;
     AcpiSratMemoryAffinity *numamem;
 
-    int i;
+    int i, node;
     int srat_start, numa_start, slots;
-    uint64_t mem_len, mem_base, next_base;
+    uint64_t mem_len, mem_base;
     MachineClass *mc = MACHINE_GET_CLASS(machine);
     const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine);
     PCMachineState *pcms = PC_MACHINE(machine);
@@ -2370,36 +2398,30 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
     /* the memory map is a bit tricky, it contains at least one hole
      * from 640k-1M and possibly another one from 3.5G-4G.
      */
-    next_base = 0;
+
     numa_start = table_data->len;
 
-    numamem = acpi_data_push(table_data, sizeof *numamem);
-    build_srat_memory(numamem, 0, 640 * 1024, 0, MEM_AFFINITY_ENABLED);
-    next_base = 1024 * 1024;
-    for (i = 1; i < pcms->numa_nodes + 1; ++i) {
-        mem_base = next_base;
-        mem_len = pcms->node_mem[i - 1];
-        if (i == 1) {
-            mem_len -= 1024 * 1024;
+    /* get the first node which has memory and map the hole from 640K-1M */
+    for (node = 0; node < pcms->numa_nodes; node++) {
+        if (pcms->node_mem[node] != 0) {
+            break;
         }
-        next_base = mem_base + mem_len;
-
-        /* Cut out the ACPI_PCI hole */
-        if (mem_base <= pcms->below_4g_mem_size &&
-            next_base > pcms->below_4g_mem_size) {
-            mem_len -= next_base - pcms->below_4g_mem_size;
-            if (mem_len > 0) {
-                numamem = acpi_data_push(table_data, sizeof *numamem);
-                build_srat_memory(numamem, mem_base, mem_len, i - 1,
-                                  MEM_AFFINITY_ENABLED);
-            }
-            mem_base = 1ULL << 32;
-            mem_len = next_base - pcms->below_4g_mem_size;
-            next_base += (1ULL << 32) - pcms->below_4g_mem_size;
+    }
+    numamem = acpi_data_push(table_data, sizeof *numamem);
+    build_srat_memory(numamem, 0, 640 * 1024, node, MEM_AFFINITY_ENABLED);
+
+    /* map the rest of memory from 1M */
+    mem_base = 1024 * 1024;
+    mem_len = pcms->node_mem[node] - mem_base;
+    mem_base = build_srat_node_entry(table_data, pcms, node,
+                                            mem_base, mem_len);
+
+    for (i = 0; i < pcms->numa_nodes; i++) {
+        if (i == node) {
+            continue;
         }
-        numamem = acpi_data_push(table_data, sizeof *numamem);
-        build_srat_memory(numamem, mem_base, mem_len, i - 1,
-                          MEM_AFFINITY_ENABLED);
+        mem_base = build_srat_node_entry(table_data, pcms, i,
+                                            mem_base, pcms->node_mem[i]);
     }
     slots = (table_data->len - numa_start) / sizeof *numamem;
     for (; slots < pcms->numa_nodes + 2; slots++) {
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v4 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node
  2017-08-22  3:24 [Qemu-devel] [PATCH v4 0/2] hw/acpi-build: Fix ACPI SRAT Memory Affinity building Dou Liyang
  2017-08-22  3:24 ` [Qemu-devel] [PATCH v4 1/2] hw/acpi-build: Fix SRAT memory building when there is no memory in node0 Dou Liyang
@ 2017-08-22  3:24 ` Dou Liyang
  2017-08-23  8:40   ` Igor Mammedov
  2017-08-24 13:50   ` [Qemu-devel] [PATCH v5 " Dou Liyang
  2017-08-23  2:48 ` [Qemu-devel] [PATCH v4 0/2] hw/acpi-build: Fix ACPI SRAT Memory Affinity building Michael S. Tsirkin
  2 siblings, 2 replies; 19+ messages in thread
From: Dou Liyang @ 2017-08-22  3:24 UTC (permalink / raw)
  To: qemu-devel, ehabkost, imammedo; +Cc: pbonzini, rth, mst, Dou Liyang

As QEMU supports the memory-less node, it is possible that there is
no RAM in the first numa node(also be called as node0). eg:
  ... \
  -m 128,slots=3,maxmem=1G \
  -numa node -numa node,mem=128M \

But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
table. Only fixing it is not enough.

Add a testcase for this situation to make sure the ACPI table is
correct for guest.

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
 tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
 tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
 tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
 tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
 tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
 tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
 tests/bios-tables-test.c              |  30 ++++++++++++++++++++++++++++++
 7 files changed, 30 insertions(+)
 create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
 create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
 create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
 create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
 create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
 create mode 100644 tests/acpi-test-data/q35/SRAT.numamem

diff --git a/tests/acpi-test-data/pc/DSDT.numamem b/tests/acpi-test-data/pc/DSDT.numamem
new file mode 100644
index 0000000000000000000000000000000000000000..53f6d5824359ff0ca20179b19b3b5cb79f017f82
GIT binary patch
literal 6463
zcmcgw-EJGl6`tiTrR9*4meR(St)wR8{4{A|^T&}Bv}jH4QX(xbwdPW;i#5unETdGA
z)(a&rq6h(G22h;b1+0tqjy8C0KSKKmd4%*8QZ(^Z)bH$aD2CDk$wf;*t2uMN{mwZv
zXU@#5>6p#moMTMdFKrkVCsVp*8z%ZB#u&BfzgAUlGxxwOt+k|NOp)9N$)Jr#N!8yp
zOPg!b-#Xr3J@3QMJgM!ottZ-}t+xN^LvM=_=>C?^IW@H9mQ!lE-6h+oX4O`uYNm=`
zaanB@%?49jn^jZNEH%WG)rwti3XlX4)NrF>H!YT8?5ppSOmQD*Brn`7*UgOGFk2aY
zrR6k>%%>jDr>^$L9@o5n>dT(TdS3GAXu$fjU-sJUFYOfj*MH&zHsEt=%V;?1G@m_8
zAYza+g|R)Ry>f}XA$Q&GIr~<w5crysnFLqtT)~JjexzNCkHHM3>J(cc4g*WLD_JAP
zJq~wY{rx3kp*I<;TxQcXyIhypJ4`l;)R2u5{%OXA%d#*`Y;O0hM$-UkIAlo-7Wuo#
zUs#iT<})p}%%nAGm+i9H)E;xYSJzAC2rkQdA{doXpuvkC^O%IUw%IoRFUJtC+kMU2
z*c`n$w=nsl%HzvSBVbWoHI30EP7gg=;)`q2H}W?!Q`V&K<nJ(0%h%W&O2D04t=UB}
z*{f!CKBF-n=NmjL@n<~7U%}rzU*bpnef}+<<Eq+PHOuL<T&FY~|HUA<-Cx)2=Ezrt
z>5Jji(~x@oDNOf(Jyu3BYkX!+`bhprR@LQ$z@M^WY*;xlsBtOGKtV$j5=HY|el9b0
zSqo@Zi6%cm!($^J%xEM}?0F14DtNStdYnWDw&66TBzwwLq&Nu%a&R+)hnaB?obq)n
zMfE9VRFVXvlr3L}qExpUQc?>QBp9Xorj>D}Qq5Rr`YX0kz8-Tgl5}+BcSz~)Njr>Q
zcVDCK0n_SOLZz0r&MED6leg?fllk+p1J7seC#A%rHb3w`Zu1!1^7nV!Ta3%>PG9TY
z0VsiGQ>(9=Y`gz*?~c?@_u5<;bvtbP@ytjn0+jCE;jvLqY1ku=dJMMhf3mm5HHL-D
z9&R({@3&#^;kLy5?T22Q-ER*(_rc4TW%8ER3avKDZrsSKc;x7jr$<567pgZfs{O=1
zKj>i5Ck8yJuT*asjRXLVY<iX_Kc8ZWAldfV7u3h&cvkz#M;*l6q4t2f-EM6fVIrBs
z1(`wtq4K0+teo^2zVP`hOUQ9#DaT(De|t=y3)dhyLGJ14mRWV!C)DN#LC+kmYBnI3
z0{LvqX*kjsozM$ZvqqKrht^9I){(zI+InuxdM>b@gMZ`JbNcDlb8gq7*o_J6JoXdJ
z=f}+F1M~R_^Z5w#`Js7p(mXbZA02&R%zPm*Uzjjoh%jGpyBi9kcP7m%p?Q%)w2NTM
zMcGa2Q^92G$#9IxRdmtpY8zL>4bR}WkBh$DD346=sUW##40|(Ex%Z*}{{25>cJF?0
z@BZ$cyB{!DGVI#=u>8(&z9z9}?3t88jYd)hRpc2@xeqRqP-Gc{HXNf`cO;`>-IP!e
zvJz7a!!l87<_RPlPPvu=(kwPJda6~eS5==;r`s*Pbvvk1@FnzbrfAsJY&MrA55oOC
zuBVOW>hEHG-s`YKBb&|c=#?qduwuf!c>A&<Ha3cpe5Xn@*$rbp^ZMs|lM3?Q=X0L>
z+6hf<ec<U<oFe*ZhNMX<Fr3P-S7r^zsH;(7NZylTvgct-MyZ@6#R&J;loJ63GlAin
zgk$g}|Ac2i`;zx~UFla&QU&vQIu(~sasdvm(*v~<#)i{OMFo25e4TI>5&Ju*#O3qY
zWG_MAo9x68oa&wX>AFH`g*ApBG8Cz$4k*EpgjTVcPI8s0boPQa*`f@mq+t4N3qd6*
zGS=C74=eVT#q=d)^qO66m9u%wS#a#4?52rEt%;8<N~>arWoO2&+u34v1?EuoXRNz_
zytliT>6}=b;VCU#lpI^ZA}z{_O?x!sd=bmZ3YQa@!+JWwY*sBRvwio^JG9bO%d9uJ
z>^`_OHc;dfG0Ik>RLkH|!J|g48d~<PR>pns<|+6hyT+}u(~>MoK#NkxqEiL7(o@i~
zMJ2)7DBr}xF*5o>u$T1{J@N_XENrE&WCe$e;*^D<W5+tI2KOTAj*g!E>XAop_#P4V
z7kZ&}<SZ6V;L77OeZ3dDL%o!@B<*^XsYEi6^tUXkYh$$j-#aWIO!k79!U7SXJ-xVS
zVP8W<2wf0Z)oQF<Nf&tbi(fE?1O3GB`=jtDx`*Ns&IR-$pM~eOusM%rC2YQ0D~sMY
z&>Z%jv5L2b*z;&se3~YDi5fK`LZeop7pPGql3nVi2E9u{pr*I``XlNi{VvH;(3uW8
zr4L6v)T)uL);Fhn)Qtpc6!Pf{{Ke1Cj)zR4QkWEC5MCkAIp|x63C&r&J8YhTLoJ^y
zliDY8Qu^d4sfIH|uZ@~3SM-;Whn6%s0X}7?m{d>yITEX>oB-`m()sUE)`gV%GV(Nw
zL+lTy6KR6<4I$X0rz|^*%L%Q{>_?~+qwV@YZ1j{Aq=!s}V?a4Il&LHX$mc+wr=lSb
z$UTx#ICL$jDFbpO=o%WtRbpr`_x~}7OUKY){(o!`(}U$W+|p@mcW8H)4$;i*^pm{~
zk5N0nGgeLuqMTB7qne;?C{$^Kxfq@sP($^1QF?QXwwlqMH@u*Xb=XEd*b9Du(8}qs
zeA3ZBthJ?S{-=jCEQSvRl+pCS4flEKc}h~zFNF*JJE)(hM)FLM|NkCe6&$O9j_ve|
zu|Om4pz6`lKfVt&|NNG2!iIvN!kh3!`KJD{qZ+<!G>berwkv%7?CWQeLL}D}yJ9!O
zq50wNg5XX$_MPNnPuWvR$3^n>+eikd(3+o8Bsh)smDaqf;Bt*?Go-gkLjh>NJRUx`
zdujEz#%k!mjk2|g{gskx(qk>I#p!UVmHpenHPN~kjD89A9APA#Y!+8?`kv!<-*9q6
z0plHGI@oe1HVbOxMWa9sxBI%2dj~H%oAu%MAiN?oeLh%ebZsRAq$X&dXbFl_H%tMF
z+kXU9u*L;69tAB3R-Cs()~QISV4W7w=_u$G!HTaLA*&h*6|4yXO+-Q87OY%Ac_dV@
zBJn~8l2OnV!I~1#R3uce&IssC6!fZKofXj8NT^`NeLD1YE(-dNVATXvi-Zc+w1B3g
zpzjLSc>$e|gbLOL0bPiK76mIZKMYHcgbLOd1oVX{Xi2c15zsS{P{DduK+i@&4Z*r7
zpo@`E!Fo<W&qYB^!Fpam&qqQ9>ym&jMM2Ag^+f@FF%l|RUlP!lqM)mS^=AV5vq-35
zy&#|$PJ`-|CG%#z8SAI^JGh!0Qmxww1PubJStw*k5SnWcP?18-0STm;+3zHAuQ_y3
zR)$iHq>)a6gyJyNk*&8!Ix>bOKGc!5m649jVWAFnWbZ2J=Jq?NuGmEm9V|E@V;_Nh
ztjBJ0=wRsxJ?>tRkM$bKSOh|!3FKq_HIk9eLVpm*$NC^*B&N_~GdXmOF+D|%6dUO&
uZgWgekt10~`V7fq`fHFe?IZmG$<XT;=-`15?DVCSrVxAGrZjArT>dYwdQV#b

literal 0
HcmV?d00001

diff --git a/tests/acpi-test-data/pc/SLIT.numamem b/tests/acpi-test-data/pc/SLIT.numamem
new file mode 100644
index 0000000000000000000000000000000000000000..74ec3b4b461ffecca36d8537975c202a5f011185
GIT binary patch
literal 48
scmWIc@eDCwU|?X>aq@Te2v%^42yhMtiZKGkKx`1r1jHb~B`V4V0NaKK0RR91

literal 0
HcmV?d00001

diff --git a/tests/acpi-test-data/pc/SRAT.numamem b/tests/acpi-test-data/pc/SRAT.numamem
new file mode 100644
index 0000000000000000000000000000000000000000..f8ec8fa242adc6132ba033bfc13cf7e858ad22cc
GIT binary patch
literal 264
zcmWFzatz^MWME*N>*Vk35v<@85#SsQ6axw|fY=}!gyBE{mCvYwA`4W;1y_nJgHQ)F
m01Q5`!xW;bgNZV5z*$fR4t40tQ1}RY;qLwcQ@{*k0BHdBCkg=o

literal 0
HcmV?d00001

diff --git a/tests/acpi-test-data/q35/DSDT.numamem b/tests/acpi-test-data/q35/DSDT.numamem
new file mode 100644
index 0000000000000000000000000000000000000000..1b7c484a5e31ec456685bc246197252cb48a3adf
GIT binary patch
literal 9147
zcmcgyO>Y~=8J;C6YBi*!rL>mik3@tWG;Nv>N^+f|AA!kTie$u<D3VSb&;ZwN+)7E?
zEEK0m8U&COARj&yNSFlejSl!PdT4$^ZVk{=ue}yUe2V(KGduFkk^<sGs{=Lf?DM?u
zJhQX&EO+U5{O(^bGiJR~+4kz4V&z85^U-H9#;8sIokr#+>mB%&THng0GFI;}C))U^
z*zVJQ<$BHf^KSS>7~cIbv~ET0_Lt7*w{J!7e-z$k1bXXs#5pybP2TTR`n@BMx4cHl
z?$;ZBzWg<}?Y<{lX}e$c+sy7?Z}^tmobGRTyv)7VndiT}I^3EX>=qWe(+mEw{_V=e
zYhS#4xBS&F|NiTHuk#E5Yxr;D|9V6p;kzMU35U+7gFWj#qKjjz!^KYr;;`&<=tvY&
z&;7jYqExG+*PXzYW3AftR<*0fYGH&J8|I_l>IN*&*w^3XSf+RAP`vn3b;~cd+J3+2
zUhQ|fWfoCS7*W@)KZe{1hxtziAsd9b<Kqz<&V4et#Xh^0XaD@q)QFAvJZ7TZDHQnJ
z2Q!#@Bxf1M-eJzQ>1Sy;G;Nr^Y@<`|i>bl@Rxp@WqrH>jA<y>?nmB_ge!x?`!*kST
z-gZz<GYkJJ_}6T8{ggmeQ~ZcfXMn0QuUy@-yb3;=EqplF&OVn`6{%Y9=$UrcLs5Co
zayUmc(q(?8W<@MwPU;e~YSup6XQ}KvtatSMX2m~&r_=6?n2!^Qf*7)pSzx`&Ud8`Q
zDJbn7JsE6YKOxR9CW$tmy2Km;z5p#JzzlPj<&4w#!(kS)JYx4VCj}ioTeIFyzn!-2
zO9!G*HG?@>Rf?vpc-6&eXhKd)^C0~o&Fo8#NV7p{))HxWN)3nR^wd152Px#Sk;iT|
zybb2&`}l7~yA(Npdc~qs;CTNMmrJYNZ7poRd9aYnVD+%()@GH-wuSZvTgX#CPa&wO
zAdR5T7G^W~<6~UR5*J6r0&q?q6FfjwNKE7x*py%b8%M-c&=eC)Km`*aF)<o}O$(+P
z0j)ogRY22{38)H*sj6VzORUI~5ljWmNTwP*LS5&Sp>s+w6?956)!-58I<tn(tY9i=
z*3^km*EwzIoHle$n>rEdI&DLzZRoU3od|WE*der;o-uUJm^u;aI=P{f8#=kE6QQm%
zXXwlsI&-E@gt|`bRNDE>8#?o*PK3J7SwrWnp>x*MiBQ)$XXu<Wbk3PN5$ZbU4W09b
z&UsTOLS3h0=yVL7j;Rx&uCrk1EEqZqrcQ*q&ILo~f}wN4)QM2nxoGHIG;}VSIuYtR
zT|=j9=yXk;2z8xj44r2Too7s)2z8xHhR!8J=aQ)tp|10+q4TVv^Q@^8p{{e;(79~r
zTsCzg)ODT{%yPUFo)gSUyxE<TOz&YxOh3k+H<;%Q=6REeP-k8+m=_G@1(S(TXI?az
z7Y*h`lZjAgK4vf<GnkK=OoTe~aluqB^SEHDJ^pdYR2w%!Vk#$DF>0<DHCN1<2xUzh
z23pn(wAN{y7%18(92+PCl$Ajmfhr0El{jHZ7VdWhP&sBwLRCmiB@G5DvC2R-C>f}P
z3MLsSLX1WksKmw*)l^<f1}dQ^28s|v7g!jm#KsXZ)#Q?aN~mCxfg)5oVW1K#m@rTc
zN(L&Sf=LF7Q0at$N~~bQKs6{CsDuh887M-f69y`=f(ZlFpk$yDDwt%T2$fD4sKg2;
z3{-=Xfl8=gl7S*rI$@v^E0{1)4N3+op@K;Uicsl<fl91k!ay}B8K{H`CK)I~r4t4!
zv4RN$)u3dc5-ONvpa_*t7^uVwCJa=Al7UL7V3L6%R61dx5-XT6Pz_23Dxrc&28vMW
zgn>$|V8TE(C>f}P3MLsSLZuT1DzSnI1J$5ppb{#WWS|I@P8g`f3MLFxgOY(ts9=(T
zB2+qIpb{&XFi;Ij1}dR~Nd}5g>4bqwtYE@GH7FUVgbF4ZC_<$Z1}d?F2?N!jWS|l%
zm}H;`l};F_#0n-1RD+U%N~mCxfg)5oVW1K#m@rTcN(L&Sf=LF7Q0at$N~~bQKs6{C
zsDuh887M-f69y`=f(ZlFpk$yDDwt%T2$fD4sKg2;3{-=Xfl8=gl7S*rI$@xQGy_GX
z8z@5EKoM#Nsxe`p8j}oEW0HYtOc<!fgn?>IGEj|42C6Y(pc)efsxiqxH6|IT#)N@t
zOc<!fBm>o$WS|-o28u|XT^J}LoG~$(WS|If8dOM3HMcNOL~3qfporAml7S*rb4vz_
z&}Z)XJ}i_C^8@i^bwpoC?`P>>xp(-p=hE~om7W#Q(+I28-YLUzcPjMZ(T8s{JR8}m
zQhS3wYV=XJ-NnXEr)v#o-sK0ocnC5x;yL$9W5;XH0MITK=6LSoESsT+QTV2OkNWr&
zJ{!@yyL_He3xjCm$w+e=_XuV6T|AG+DfF<$#;`kuCBFFa9GgkQ-5B<7hMgGM+<Ez2
zf`)+6l-)w#Z*<su)aD1GXP%yPun`SN#Ao}RcE`H68;^m?Q-55&lBkZy3g5eWMXO#B
z)higEUcKT@u3qUK1*F|eROhKjDDP?IJyG6El=n_4@5SZ)WO+O;``+q(t-LSF`-$@Y
zDdqjRd^K4<^9bduTKTFdUrm&+o>IOVm)}U1pL&Gy8(R4dQGO#)e&dw#8*%wsvV8Ut
z%Gb2=HBr8nC|^6Jd@U|tPnMs4gz|N*d|i~UC(74PDPKqV3~gn}@{TO8HV@oZ&|^07
z)Y9y8v*MD+Kf12tbjiz@7tD0H*BDDD`zEFvyDk^ZbhrT-ODFp#rW?D?7R_|HD;Y~C
z`zEFvyKY=F9d1>|(#gJw>Bg>uGiEy6zl^1meG}7-t?MN-9d2sI(#gJw>BiRSSu-8(
zbjH%jJ~N%O{k(ib;PIOepOv@T)s9+;92X$`XVz<7FV^3<8~ygJ-xT-ny!pmk`@g#L
zChJ*VtGOAk$XU*7);IZY@>U*?w&6Q4#N0P|JkUS9OhK6yJ$f$hHMTmI*A8B?yy}Z3
zi)B17@a^8=0)f(Ar`jw6>6g1jJW6hCHSCB{XYZ);%uVqI0b`Nln=N~-Mya$~q7lfq
zFH>&8>u$V~8uGy|`@k!eN_%cS>!JEg@AA#(GU|N*X{50qA4j{%ZWN1HI6OZ{9-z@j
zd_L?wId-JEISSoiiWEIsAuE{*87Ag;>vLYm+p<rSVTE6obAu3XGQx-@lVXO4wal0x
zaV9WbNjN6oipE9;bYzA1H#5Wfcq(x|C#T}+@mzo-*JPo=jJMtC=1&tk@qCR97K)?2
z3A!SlO7;@;wPYuzVB+X{?{8*kS>X!9A66)qmOY{chD8{ZyM-LL8NSccot~TP(+a0W
z!Q45#2<+xa8Qa}^4cF_9fVmg2qHC>czgk+uITLTIXm!(x#wI?cU6et+C3no()>f-j
zE?q@Ax-f$~zk6f<o#O7;wJB{vT$H$1<ZPKP%6cnkTliAWmvK1-@^V6ExwR)U`;8zd
zzIx{mdvvASfxp$}>E7Y9+C<TqC}TQkSDHn9)bY`zRuiq(jefCr`1AyPnO);SwbQo(
zT7Us9ouK++AFpMfn1D_Pv?T=F)tB(m@rv%ExZNGj3}{SvLxy|po)t8`P3D%f0j7>U
zJcg^GcbSeIAAk9a&qHEFAtf9x4rFz#SzI(?DEF6!?m!NQdTHHKv>(z+r6eQEeI}rN
zO-md7WtWMBxq;9mHwb|a-STpPTVL!5atI1*2kom@3hTUd@5hYg@q)?x`|eo&sopp|
z#uo=7@;QvWDVqyu)@Abt&8q5s3eC88DJb8FvFFjOM|7HqM2#jTLZewH0yUbHWS_dJ
zL0k&NVPX&6&#8}&?^A4v&VuN)?w<5et4YV2Lw|Ze-B>{F^|itZzxW}H=%HkiN-{~J
zkhi|ZInwu`3C%fh<K_|u)Q_k#9gZkD9gJw4bSOSUL_29-z2ZKBbr@L56A;nrR3|lb
ze~5+E&5VKeXwgNl(W;Xv-Ah=frPLTuEME!f9c^cWRtdihpsTa=Hg*c-rTGJOKS+y$
zjw7axV?=8zj%iyM(U?WNK$}K-M8l&P8;9$NJ!M1#5nXXXyabL5uKs@t;&pLcaP5Cw
z5T^&1V{GX(w|nI7)(2>=?hSK;U7n)$+Mc$ZuB+{ob~oA+9G5m)aVp~525g~DBQN(r
z`CAU0l|p5j|Nf&{mcpY-T9)*IxBCzB5Av3ceoHRhZ*llR{v@6ii}>IA(&kvw^!1$n
zQcN5XU;Yt(b$tA{@5BN2)eH11055}~$={MB_F(GQ?zixJj&7Ofw5`3EEoLqJl)(AX
zO`L_!dVe9Cu}}!@EBysK!&4j&?h*Zjj!;qO=MN?Kj-K6k*^~FWvpdz`I*g6ZX0nPi
z<xJ6CowJp>DPHGqWaQ5{a-C8Mzh9sqy!4Kq>Xd#e1I&)aa0A|1^|3!wV@);IMI+nt
Hwz}-UIV`hz

literal 0
HcmV?d00001

diff --git a/tests/acpi-test-data/q35/SLIT.numamem b/tests/acpi-test-data/q35/SLIT.numamem
new file mode 100644
index 0000000000000000000000000000000000000000..74ec3b4b461ffecca36d8537975c202a5f011185
GIT binary patch
literal 48
scmWIc@eDCwU|?X>aq@Te2v%^42yhMtiZKGkKx`1r1jHb~B`V4V0NaKK0RR91

literal 0
HcmV?d00001

diff --git a/tests/acpi-test-data/q35/SRAT.numamem b/tests/acpi-test-data/q35/SRAT.numamem
new file mode 100644
index 0000000000000000000000000000000000000000..f8ec8fa242adc6132ba033bfc13cf7e858ad22cc
GIT binary patch
literal 264
zcmWFzatz^MWME*N>*Vk35v<@85#SsQ6axw|fY=}!gyBE{mCvYwA`4W;1y_nJgHQ)F
m01Q5`!xW;bgNZV5z*$fR4t40tQ1}RY;qLwcQ@{*k0BHdBCkg=o

literal 0
HcmV?d00001

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 564da45..79728c6 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -808,6 +808,34 @@ static void test_acpi_piix4_tcg_memhp(void)
     free_test_data(&data);
 }
 
+static void test_acpi_q35_tcg_numamem(void)
+{
+    test_data data;
+
+    memset(&data, 0, sizeof(data));
+    data.machine = MACHINE_Q35;
+    data.variant = ".numamem";
+    test_acpi_one(" -m 128,slots=3,maxmem=1G"
+                  " -numa node -numa node,mem=128"
+                  " -numa dist,src=0,dst=1,val=21",
+                  &data);
+    free_test_data(&data);
+}
+
+static void test_acpi_piix4_tcg_numamem(void)
+{
+    test_data data;
+
+    memset(&data, 0, sizeof(data));
+    data.machine = MACHINE_PC;
+    data.variant = ".numamem";
+    test_acpi_one(" -m 128,slots=3,maxmem=1G"
+                  " -numa node -numa node,mem=128"
+                  " -numa dist,src=0,dst=1,val=21",
+                  &data);
+    free_test_data(&data);
+}
+
 int main(int argc, char *argv[])
 {
     const char *arch = qtest_get_arch();
@@ -830,6 +858,8 @@ int main(int argc, char *argv[])
         qtest_add_func("acpi/q35/cpuhp", test_acpi_q35_tcg_cphp);
         qtest_add_func("acpi/piix4/memhp", test_acpi_piix4_tcg_memhp);
         qtest_add_func("acpi/q35/memhp", test_acpi_q35_tcg_memhp);
+        qtest_add_func("acpi/piix4/numamem", test_acpi_piix4_tcg_numamem);
+        qtest_add_func("acpi/q35/numamem", test_acpi_q35_tcg_numamem);
     }
     ret = g_test_run();
     boot_sector_cleanup(disk);
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 0/2] hw/acpi-build: Fix ACPI SRAT Memory Affinity building
  2017-08-22  3:24 [Qemu-devel] [PATCH v4 0/2] hw/acpi-build: Fix ACPI SRAT Memory Affinity building Dou Liyang
  2017-08-22  3:24 ` [Qemu-devel] [PATCH v4 1/2] hw/acpi-build: Fix SRAT memory building when there is no memory in node0 Dou Liyang
  2017-08-22  3:24 ` [Qemu-devel] [PATCH v4 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node Dou Liyang
@ 2017-08-23  2:48 ` Michael S. Tsirkin
  2017-08-23  3:47   ` Dou Liyang
  2 siblings, 1 reply; 19+ messages in thread
From: Michael S. Tsirkin @ 2017-08-23  2:48 UTC (permalink / raw)
  To: Dou Liyang; +Cc: qemu-devel, ehabkost, imammedo, pbonzini, rth

On Tue, Aug 22, 2017 at 11:24:08AM +0800, Dou Liyang wrote:
> V3 --> v4:
>   -add a new testcase.
> 
> This patchset fixs an ACPI building bug which caused by no RAM
> in the first NUAM node. and also add a new testcase for the bug.

thanks!
Pls remember to ping or repost after release is out.

> Dou Liyang (2):
>   hw/acpi-build: Fix SRAT memory building when there is no memory in
>     node0
>   ACPI/unit-test: Add a new testcase for RAM allocation in numa node
> 
>  hw/i386/acpi-build.c                  |  78 ++++++++++++++++++++++------------
>  tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
>  tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
>  tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
>  tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
>  tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
>  tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
>  tests/bios-tables-test.c              |  30 +++++++++++++
>  8 files changed, 80 insertions(+), 28 deletions(-)
>  create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
>  create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
>  create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
>  create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
>  create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
>  create mode 100644 tests/acpi-test-data/q35/SRAT.numamem
> 
> -- 
> 2.5.5
> 
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 0/2] hw/acpi-build: Fix ACPI SRAT Memory Affinity building
  2017-08-23  2:48 ` [Qemu-devel] [PATCH v4 0/2] hw/acpi-build: Fix ACPI SRAT Memory Affinity building Michael S. Tsirkin
@ 2017-08-23  3:47   ` Dou Liyang
  0 siblings, 0 replies; 19+ messages in thread
From: Dou Liyang @ 2017-08-23  3:47 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel, ehabkost, imammedo, pbonzini, rth

Hi Michael,

At 08/23/2017 10:48 AM, Michael S. Tsirkin wrote:
> On Tue, Aug 22, 2017 at 11:24:08AM +0800, Dou Liyang wrote:
>> V3 --> v4:
>>   -add a new testcase.
>>
>> This patchset fixs an ACPI building bug which caused by no RAM
>> in the first NUAM node. and also add a new testcase for the bug.
>
> thanks!
> Pls remember to ping or repost after release is out.
>

OK, I will.

Thanks,
	dou.

>> Dou Liyang (2):
>>   hw/acpi-build: Fix SRAT memory building when there is no memory in
>>     node0
>>   ACPI/unit-test: Add a new testcase for RAM allocation in numa node
>>
>>  hw/i386/acpi-build.c                  |  78 ++++++++++++++++++++++------------
>>  tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
>>  tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
>>  tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
>>  tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
>>  tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
>>  tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
>>  tests/bios-tables-test.c              |  30 +++++++++++++
>>  8 files changed, 80 insertions(+), 28 deletions(-)
>>  create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
>>  create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
>>  create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
>>  create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
>>  create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
>>  create mode 100644 tests/acpi-test-data/q35/SRAT.numamem
>>
>> --
>> 2.5.5
>>
>>
>
>
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node
  2017-08-22  3:24 ` [Qemu-devel] [PATCH v4 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node Dou Liyang
@ 2017-08-23  8:40   ` Igor Mammedov
  2017-08-23 12:12     ` Dou Liyang
  2017-08-24 13:50   ` [Qemu-devel] [PATCH v5 " Dou Liyang
  1 sibling, 1 reply; 19+ messages in thread
From: Igor Mammedov @ 2017-08-23  8:40 UTC (permalink / raw)
  To: Dou Liyang; +Cc: qemu-devel, ehabkost, pbonzini, rth, mst

On Tue, 22 Aug 2017 11:24:10 +0800
Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:

> As QEMU supports the memory-less node, it is possible that there is
> no RAM in the first numa node(also be called as node0). eg:
>   ... \
>   -m 128,slots=3,maxmem=1G \
>   -numa node -numa node,mem=128M \
> 
> But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
> table. Only fixing it is not enough.
> 
> Add a testcase for this situation to make sure the ACPI table is
> correct for guest.
> 
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
> ---
>  tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
>  tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
>  tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
>  tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
>  tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
>  tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
>  tests/bios-tables-test.c              |  30 ++++++++++++++++++++++++++++++
>  7 files changed, 30 insertions(+)
>  create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
>  create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
>  create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
>  create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
>  create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
>  create mode 100644 tests/acpi-test-data/q35/SRAT.numamem


considering only SRAT table has been changed and the other
tables match with default blobs, I'd suggest to keep only
  tests/acpi-test-data/[pc|q35]/SRAT.numamem
in this patch and drop the rest of *.numamem tables
as test case should fallback to default tables
when .numamem variant doesn't exists

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node
  2017-08-23  8:40   ` Igor Mammedov
@ 2017-08-23 12:12     ` Dou Liyang
  2017-08-23 12:45       ` Igor Mammedov
  0 siblings, 1 reply; 19+ messages in thread
From: Dou Liyang @ 2017-08-23 12:12 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: qemu-devel, ehabkost, pbonzini, rth, mst

Hi Igor,

At 08/23/2017 04:40 PM, Igor Mammedov wrote:
> On Tue, 22 Aug 2017 11:24:10 +0800
> Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
>
>> As QEMU supports the memory-less node, it is possible that there is
>> no RAM in the first numa node(also be called as node0). eg:
>>   ... \
>>   -m 128,slots=3,maxmem=1G \
>>   -numa node -numa node,mem=128M \
>>
>> But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
>> table. Only fixing it is not enough.
>>
>> Add a testcase for this situation to make sure the ACPI table is
>> correct for guest.
>>
>> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
>> ---
>>  tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
>>  tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
>>  tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
>>  tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
>>  tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
>>  tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
>>  tests/bios-tables-test.c              |  30 ++++++++++++++++++++++++++++++
>>  7 files changed, 30 insertions(+)
>>  create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
>>  create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
>>  create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
>>  create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
>>  create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
>>  create mode 100644 tests/acpi-test-data/q35/SRAT.numamem
>
>
> considering only SRAT table has been changed and the other
> tables match with default blobs, I'd suggest to keep only


Our testcase is:

+    test_acpi_one(" -m 128,slots=3,maxmem=1G"
+                  " -numa node -numa node,mem=128"
+                  " -numa dist,src=0,dst=1,val=21",
+                  &data);

The DSDT and SLIT don't match with default blobs.

So, they can't be dropped.

Thanks,
	dou.

>   tests/acpi-test-data/[pc|q35]/SRAT.numamem
> in this patch and drop the rest of *.numamem tables
> as test case should fallback to default tables
> when .numamem variant doesn't exists
>
>
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node
  2017-08-23 12:12     ` Dou Liyang
@ 2017-08-23 12:45       ` Igor Mammedov
  2017-08-23 13:35         ` Dou Liyang
  0 siblings, 1 reply; 19+ messages in thread
From: Igor Mammedov @ 2017-08-23 12:45 UTC (permalink / raw)
  To: Dou Liyang; +Cc: qemu-devel, ehabkost, pbonzini, rth, mst

On Wed, 23 Aug 2017 20:12:51 +0800
Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:

> Hi Igor,
> 
> At 08/23/2017 04:40 PM, Igor Mammedov wrote:
> > On Tue, 22 Aug 2017 11:24:10 +0800
> > Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
> >  
> >> As QEMU supports the memory-less node, it is possible that there is
> >> no RAM in the first numa node(also be called as node0). eg:
> >>   ... \
> >>   -m 128,slots=3,maxmem=1G \
> >>   -numa node -numa node,mem=128M \
> >>
> >> But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
> >> table. Only fixing it is not enough.
> >>
> >> Add a testcase for this situation to make sure the ACPI table is
> >> correct for guest.
> >>
> >> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> >> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
> >> ---
> >>  tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
> >>  tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
> >>  tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
> >>  tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
> >>  tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
> >>  tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
> >>  tests/bios-tables-test.c              |  30 ++++++++++++++++++++++++++++++
> >>  7 files changed, 30 insertions(+)
> >>  create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
> >>  create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
> >>  create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
> >>  create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
> >>  create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
> >>  create mode 100644 tests/acpi-test-data/q35/SRAT.numamem  
> >
> >
> > considering only SRAT table has been changed and the other
> > tables match with default blobs, I'd suggest to keep only  
> 
> 
> Our testcase is:
> 
> +    test_acpi_one(" -m 128,slots=3,maxmem=1G"
> +                  " -numa node -numa node,mem=128"
> +                  " -numa dist,src=0,dst=1,val=21",
> +                  &data);
> 
> The DSDT and SLIT don't match with default blobs.
do you actually need SLIT table /i.e. -numa dist/ for test at all?
it looks not relevant for the test case at the hand,
I'd suggest to drop '-numa dist' option for the test.

> 
> So, they can't be dropped.

I wonder what's changed, could you post DSDT diff here?

(to get diff run 'make V=1 check' without DSDT.numamem being present)


> 
> Thanks,
> 	dou.
> 
> >   tests/acpi-test-data/[pc|q35]/SRAT.numamem
> > in this patch and drop the rest of *.numamem tables
> > as test case should fallback to default tables
> > when .numamem variant doesn't exists
> >
> >
> >  
> 
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node
  2017-08-23 12:45       ` Igor Mammedov
@ 2017-08-23 13:35         ` Dou Liyang
  2017-08-23 17:25           ` Eduardo Habkost
  2017-08-23 17:47           ` Igor Mammedov
  0 siblings, 2 replies; 19+ messages in thread
From: Dou Liyang @ 2017-08-23 13:35 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: qemu-devel, ehabkost, pbonzini, rth, mst

Hi Igor,

At 08/23/2017 08:45 PM, Igor Mammedov wrote:
> On Wed, 23 Aug 2017 20:12:51 +0800
> Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
>
>> Hi Igor,
>>
>> At 08/23/2017 04:40 PM, Igor Mammedov wrote:
>>> On Tue, 22 Aug 2017 11:24:10 +0800
>>> Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
>>>
>>>> As QEMU supports the memory-less node, it is possible that there is
>>>> no RAM in the first numa node(also be called as node0). eg:
>>>>   ... \
>>>>   -m 128,slots=3,maxmem=1G \
>>>>   -numa node -numa node,mem=128M \
>>>>
>>>> But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
>>>> table. Only fixing it is not enough.
>>>>
>>>> Add a testcase for this situation to make sure the ACPI table is
>>>> correct for guest.
>>>>
>>>> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
>>>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
>>>> ---
>>>>  tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
>>>>  tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
>>>>  tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
>>>>  tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
>>>>  tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
>>>>  tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
>>>>  tests/bios-tables-test.c              |  30 ++++++++++++++++++++++++++++++
>>>>  7 files changed, 30 insertions(+)
>>>>  create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
>>>>  create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
>>>>  create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
>>>>  create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
>>>>  create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
>>>>  create mode 100644 tests/acpi-test-data/q35/SRAT.numamem
>>>
>>>
>>> considering only SRAT table has been changed and the other
>>> tables match with default blobs, I'd suggest to keep only
>>
>>
>> Our testcase is:
>>
>> +    test_acpi_one(" -m 128,slots=3,maxmem=1G"
>> +                  " -numa node -numa node,mem=128"
>> +                  " -numa dist,src=0,dst=1,val=21",
>> +                  &data);
>>
>> The DSDT and SLIT don't match with default blobs.
> do you actually need SLIT table /i.e. -numa dist/ for test at all?
> it looks not relevant for the test case at the hand,
> I'd suggest to drop '-numa dist' option for the test.
>

OK, Got it, will drop '-numa dist' option in next version.

>>
>> So, they can't be dropped.
>
> I wonder what's changed, could you post DSDT diff here?
>

Just like memory hot-plug cases, when we use the '-m
128,slots=3,maxmem=1G' option, As the ACPI spec said, There may be some
Memory Device in the DSDT table.

> (to get diff run 'make V=1 check' without DSDT.numamem being present)

diff --git a/asl-YJ034Y.dsl b/asl-JLX34Y.dsl
index c7b187b..6cd9e5d 100644
--- a/asl-YJ034Y.dsl
+++ b/asl-JLX34Y.dsl
@@ -5,13 +5,13 @@
   *
   * Disassembling to symbolic ASL+ operators
   *
- * Disassembly of tests/acpi-test-data/pc/DSDT, Wed Aug 23 21:22:56 2017
+ * Disassembly of /tmp/aml-8IX34Y, Wed Aug 23 21:22:56 2017
   *
   * Original Table Header:
   *     Signature        "DSDT"
- *     Length           0x000013EA (5098)
+ *     Length           0x0000193F (6463)
   *     Revision         0x01 **** 32-bit table (V1), no 64-bit math 
support
- *     Checksum         0x78
+ *     Checksum         0x7B
   *     OEM ID           "BOCHS "
   *     OEM Table ID     "BXPCDSDT"
   *     OEM Revision     0x00000001 (1)
@@ -783,6 +783,8 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", 
"BXPCDSDT", 0x00000001)
                  {
                      COST (Zero, Arg0, Arg1, Arg2)
                  }
+
+                Name (_PXM, Zero)  // _PXM: Device Proximity
              }
          }
      }
@@ -792,6 +794,310 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", 
"BXPCDSDT", 0x00000001)
          \_SB.CPUS.CSCN ()
      }

+    Device (\_SB.PCI0.MHPD)
+    {
+        Name (_HID, "PNP0A06" /* Generic Container Device */)  // _HID: 
Hardware ID
+        Name (_UID, "Memory hotplug resources")  // _UID: Unique ID
+        Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
+        {
+            IO (Decode16,
+                0x0A00,             // Range Minimum
+                0x0A00,             // Range Maximum
+                0x00,               // Alignment
+                0x18,               // Length
+                )
+        })
+        OperationRegion (HPMR, SystemIO, 0x0A00, 0x18)
+    }
+
+    Device (\_SB.MHPC)
+    {
+        Name (_HID, "PNP0A06" /* Generic Container Device */)  // _HID: 
Hardware ID
+        Name (_UID, "DIMM devices")  // _UID: Unique ID
+        Name (MDNR, 0x03)
+        Field (\_SB.PCI0.MHPD.HPMR, DWordAcc, NoLock, Preserve)
+        {
+            MRBL,   32,
+            MRBH,   32,
+            MRLL,   32,
+            MRLH,   32,
+            MPX,    32
+        }
+
+        Field (\_SB.PCI0.MHPD.HPMR, ByteAcc, NoLock, WriteAsZeros)
+        {
+            Offset (0x14),
+            MES,    1,
+            MINS,   1,
+            MRMV,   1,
+            MEJ,    1
+        }
+
+        Field (\_SB.PCI0.MHPD.HPMR, DWordAcc, NoLock, Preserve)
+        {
+            MSEL,   32,
+            MOEV,   32,
+            MOSC,   32
+        }
+
+        Method (_STA, 0, NotSerialized)  // _STA: Status
+        {
+            If (MDNR == Zero)
+            {
+                Return (Zero)
+            }
+
+            Return (0x0B)
+        }
+
+        Mutex (MLCK, 0x00)
+        Method (MSCN, 0, NotSerialized)
+        {
+            If (MDNR == Zero)
+            {
+                Return (Zero)
+            }
+
+            Local0 = Zero
+            Acquire (MLCK, 0xFFFF)
+            While (Local0 < MDNR)
+            {
+                MSEL = Local0
+                If (MINS == One)
+                {
+                    MTFY (Local0, One)
+                    MINS = One
+                }
+                ElseIf (MRMV == One)
+                {
+                    MTFY (Local0, 0x03)
+                    MRMV = One
+                }
+
+                Local0 += One
+            }
+
+            Release (MLCK)
+            Return (One)
+        }
+
+        Method (MRST, 1, NotSerialized)
+        {
+            Local0 = Zero
+            Acquire (MLCK, 0xFFFF)
+            MSEL = ToInteger (Arg0)
+            If (MES == One)
+            {
+                Local0 = 0x0F
+            }
+
+            Release (MLCK)
+            Return (Local0)
+        }
+
+        Method (MCRS, 1, Serialized)
+        {
+            Acquire (MLCK, 0xFFFF)
+            MSEL = ToInteger (Arg0)
+            Name (MR64, ResourceTemplate ()
+            {
+                QWordMemory (ResourceProducer, PosDecode, MinFixed, 
MaxFixed, Cacheable, ReadWrite,
+                    0x0000000000000000, // Granularity
+                    0x0000000000000000, // Range Minimum
+                    0xFFFFFFFFFFFFFFFE, // Range Maximum
+                    0x0000000000000000, // Translation Offset
+                    0xFFFFFFFFFFFFFFFF, // Length
+                    ,, _Y01, AddressRangeMemory, TypeStatic)
+            })
+            CreateDWordField (MR64, \_SB.MHPC.MCRS._Y01._MIN, MINL)  // 
_MIN: Minimum Base Address
+            CreateDWordField (MR64, 0x12, MINH)
+            CreateDWordField (MR64, \_SB.MHPC.MCRS._Y01._LEN, LENL)  // 
_LEN: Length
+            CreateDWordField (MR64, 0x2A, LENH)
+            CreateDWordField (MR64, \_SB.MHPC.MCRS._Y01._MAX, MAXL)  // 
_MAX: Maximum Base Address
+            CreateDWordField (MR64, 0x1A, MAXH)
+            MINH = MRBH /* \_SB_.MHPC.MRBH */
+            MINL = MRBL /* \_SB_.MHPC.MRBL */
+            LENH = MRLH /* \_SB_.MHPC.MRLH */
+            LENL = MRLL /* \_SB_.MHPC.MRLL */
+            MAXL = (MINL + LENL) /* \_SB_.MHPC.MCRS.LENL */
+            MAXH = (MINH + LENH) /* \_SB_.MHPC.MCRS.LENH */
+            If (MAXL < MINL)
+            {
+                MAXH += One
+            }
+
+            If (MAXL < One)
+            {
+                MAXH -= One
+            }
+
+            MAXL -= One
+            If (MAXH == Zero)
+            {
+                Name (MR32, ResourceTemplate ()
+                {
+                    DWordMemory (ResourceProducer, PosDecode, MinFixed, 
MaxFixed, Cacheable, ReadWrite,
+                        0x00000000,         // Granularity
+                        0x00000000,         // Range Minimum
+                        0xFFFFFFFE,         // Range Maximum
+                        0x00000000,         // Translation Offset
+                        0xFFFFFFFF,         // Length
+                        ,, _Y02, AddressRangeMemory, TypeStatic)
+                })
+                CreateDWordField (MR32, \_SB.MHPC.MCRS._Y02._MIN, MIN) 
// _MIN: Minimum Base Address
+                CreateDWordField (MR32, \_SB.MHPC.MCRS._Y02._MAX, MAX) 
// _MAX: Maximum Base Address
+                CreateDWordField (MR32, \_SB.MHPC.MCRS._Y02._LEN, LEN) 
// _LEN: Length
+                MIN = MINL /* \_SB_.MHPC.MCRS.MINL */
+                MAX = MAXL /* \_SB_.MHPC.MCRS.MAXL */
+                LEN = LENL /* \_SB_.MHPC.MCRS.LENL */
+                Release (MLCK)
+                Return (MR32) /* \_SB_.MHPC.MCRS.MR32 */
+            }
+
+            Release (MLCK)
+            Return (MR64) /* \_SB_.MHPC.MCRS.MR64 */
+        }
+
+        Method (MPXM, 1, NotSerialized)
+        {
+            Acquire (MLCK, 0xFFFF)
+            MSEL = ToInteger (Arg0)
+            Local0 = MPX /* \_SB_.MHPC.MPX_ */
+            Release (MLCK)
+            Return (Local0)
+        }
+
+        Method (MOST, 4, NotSerialized)
+        {
+            Acquire (MLCK, 0xFFFF)
+            MSEL = ToInteger (Arg0)
+            MOEV = Arg1
+            MOSC = Arg2
+            Release (MLCK)
+        }
+
+        Method (MEJ0, 2, NotSerialized)
+        {
+            Acquire (MLCK, 0xFFFF)
+            MSEL = ToInteger (Arg0)
+            MEJ = One
+            Release (MLCK)
+        }
+
+        Device (MP00)
+        {
+            Name (_UID, "0x00")  // _UID: Unique ID
+            Name (_HID, EisaId ("PNP0C80") /* Memory Device */)  // 
_HID: Hardware ID
+            Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource 
Settings
+            {
+                Return (MCRS (_UID))
+            }
+
+            Method (_STA, 0, NotSerialized)  // _STA: Status
+            {
+                Return (MRST (_UID))
+            }
+
+            Method (_PXM, 0, NotSerialized)  // _PXM: Device Proximity
+            {
+                Return (MPXM (_UID))
+            }
+
+            Method (_OST, 3, NotSerialized)  // _OST: OSPM Status 
Indication
+            {
+                Return (MOST (_UID, Arg0, Arg1, Arg2))
+            }
+
+            Method (_EJ0, 1, NotSerialized)  // _EJx: Eject Device
+            {
+                Return (MEJ0 (_UID, Arg0))
+            }
+        }
+
+        Device (MP01)
+        {
+            Name (_UID, "0x01")  // _UID: Unique ID
+            Name (_HID, EisaId ("PNP0C80") /* Memory Device */)  // 
_HID: Hardware ID
+            Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource 
Settings
+            {
+                Return (MCRS (_UID))
+            }
+
+            Method (_STA, 0, NotSerialized)  // _STA: Status
+            {
+                Return (MRST (_UID))
+            }
+
+            Method (_PXM, 0, NotSerialized)  // _PXM: Device Proximity
+            {
+                Return (MPXM (_UID))
+            }
+
+            Method (_OST, 3, NotSerialized)  // _OST: OSPM Status 
Indication
+            {
+                Return (MOST (_UID, Arg0, Arg1, Arg2))
+            }
+
+            Method (_EJ0, 1, NotSerialized)  // _EJx: Eject Device
+            {
+                Return (MEJ0 (_UID, Arg0))
+            }
+        }
+
+        Device (MP02)
+        {
+            Name (_UID, "0x02")  // _UID: Unique ID
+            Name (_HID, EisaId ("PNP0C80") /* Memory Device */)  // 
_HID: Hardware ID
+            Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource 
Settings
+            {
+                Return (MCRS (_UID))
+            }
+
+            Method (_STA, 0, NotSerialized)  // _STA: Status
+            {
+                Return (MRST (_UID))
+            }
+
+            Method (_PXM, 0, NotSerialized)  // _PXM: Device Proximity
+            {
+                Return (MPXM (_UID))
+            }
+
+            Method (_OST, 3, NotSerialized)  // _OST: OSPM Status 
Indication
+            {
+                Return (MOST (_UID, Arg0, Arg1, Arg2))
+            }
+
+            Method (_EJ0, 1, NotSerialized)  // _EJx: Eject Device
+            {
+                Return (MEJ0 (_UID, Arg0))
+            }
+        }
+
+        Method (MTFY, 2, NotSerialized)
+        {
+            If (Arg0 == Zero)
+            {
+                Notify (MP00, Arg1)
+            }
+
+            If (Arg0 == One)
+            {
+                Notify (MP01, Arg1)
+            }
+
+            If (Arg0 == 0x02)
+            {
+                Notify (MP02, Arg1)
+            }
+        }
+    }
+
+    Method (\_GPE._E03, 0, NotSerialized)  // _Exx: Edge-Triggered GPE
+    {
+        \_SB.MHPC.MSCN ()
+    }
+
      Scope (_GPE)
      {
          Name (_HID, "ACPI0006" /* GPE Block Device */)  // _HID: 
Hardware ID

>
>
>>
>> Thanks,
>> 	dou.
>>
>>>   tests/acpi-test-data/[pc|q35]/SRAT.numamem
>>> in this patch and drop the rest of *.numamem tables
>>> as test case should fallback to default tables
>>> when .numamem variant doesn't exists
>>>
>>>
>>>
>>
>>
>
>
>
>

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node
  2017-08-23 13:35         ` Dou Liyang
@ 2017-08-23 17:25           ` Eduardo Habkost
  2017-08-24  1:30             ` Dou Liyang
  2017-08-23 17:47           ` Igor Mammedov
  1 sibling, 1 reply; 19+ messages in thread
From: Eduardo Habkost @ 2017-08-23 17:25 UTC (permalink / raw)
  To: Dou Liyang; +Cc: Igor Mammedov, qemu-devel, pbonzini, rth, mst

On Wed, Aug 23, 2017 at 09:35:29PM +0800, Dou Liyang wrote:
> Hi Igor,
> 
> At 08/23/2017 08:45 PM, Igor Mammedov wrote:
> > On Wed, 23 Aug 2017 20:12:51 +0800
> > Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
> > 
> > > Hi Igor,
> > > 
> > > At 08/23/2017 04:40 PM, Igor Mammedov wrote:
> > > > On Tue, 22 Aug 2017 11:24:10 +0800
> > > > Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
> > > > 
> > > > > As QEMU supports the memory-less node, it is possible that there is
> > > > > no RAM in the first numa node(also be called as node0). eg:
> > > > >   ... \
> > > > >   -m 128,slots=3,maxmem=1G \
> > > > >   -numa node -numa node,mem=128M \
> > > > > 
> > > > > But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
> > > > > table. Only fixing it is not enough.
> > > > > 
> > > > > Add a testcase for this situation to make sure the ACPI table is
> > > > > correct for guest.
> > > > > 
> > > > > Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> > > > > Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
> > > > > ---
> > > > >  tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
> > > > >  tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
> > > > >  tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
> > > > >  tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
> > > > >  tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
> > > > >  tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
> > > > >  tests/bios-tables-test.c              |  30 ++++++++++++++++++++++++++++++
> > > > >  7 files changed, 30 insertions(+)
> > > > >  create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
> > > > >  create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
> > > > >  create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
> > > > >  create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
> > > > >  create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
> > > > >  create mode 100644 tests/acpi-test-data/q35/SRAT.numamem
> > > > 
> > > > 
> > > > considering only SRAT table has been changed and the other
> > > > tables match with default blobs, I'd suggest to keep only
> > > 
> > > 
> > > Our testcase is:
> > > 
> > > +    test_acpi_one(" -m 128,slots=3,maxmem=1G"
> > > +                  " -numa node -numa node,mem=128"
> > > +                  " -numa dist,src=0,dst=1,val=21",
> > > +                  &data);
> > > 
> > > The DSDT and SLIT don't match with default blobs.
> > do you actually need SLIT table /i.e. -numa dist/ for test at all?
> > it looks not relevant for the test case at the hand,
> > I'd suggest to drop '-numa dist' option for the test.
> > 
> 
> OK, Got it, will drop '-numa dist' option in next version.
> 
> > > 
> > > So, they can't be dropped.
> > 
> > I wonder what's changed, could you post DSDT diff here?
> > 
> 
> Just like memory hot-plug cases, when we use the '-m
> 128,slots=3,maxmem=1G' option, As the ACPI spec said, There may be some
> Memory Device in the DSDT table.

Do you really need to use -m 128,slots=3,maxmem=1G to test your
bug fix?

-- 
Eduardo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node
  2017-08-23 13:35         ` Dou Liyang
  2017-08-23 17:25           ` Eduardo Habkost
@ 2017-08-23 17:47           ` Igor Mammedov
  2017-08-24  1:42             ` Dou Liyang
  1 sibling, 1 reply; 19+ messages in thread
From: Igor Mammedov @ 2017-08-23 17:47 UTC (permalink / raw)
  To: Dou Liyang; +Cc: qemu-devel, ehabkost, pbonzini, rth, mst

On Wed, 23 Aug 2017 21:35:29 +0800
Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:

> Hi Igor,
> 
> At 08/23/2017 08:45 PM, Igor Mammedov wrote:
> > On Wed, 23 Aug 2017 20:12:51 +0800
> > Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
> >
> >> Hi Igor,
> >>
> >> At 08/23/2017 04:40 PM, Igor Mammedov wrote:
> >>> On Tue, 22 Aug 2017 11:24:10 +0800
> >>> Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
> >>>
> >>>> As QEMU supports the memory-less node, it is possible that there is
> >>>> no RAM in the first numa node(also be called as node0). eg:
> >>>>   ... \
> >>>>   -m 128,slots=3,maxmem=1G \
> >>>>   -numa node -numa node,mem=128M \
> >>>>
> >>>> But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
> >>>> table. Only fixing it is not enough.
> >>>>
> >>>> Add a testcase for this situation to make sure the ACPI table is
> >>>> correct for guest.
> >>>>
> >>>> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> >>>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
> >>>> ---
> >>>>  tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
> >>>>  tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
> >>>>  tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
> >>>>  tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
> >>>>  tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
> >>>>  tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
> >>>>  tests/bios-tables-test.c              |  30 ++++++++++++++++++++++++++++++
> >>>>  7 files changed, 30 insertions(+)
> >>>>  create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
> >>>>  create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
> >>>>  create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
> >>>>  create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
> >>>>  create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
> >>>>  create mode 100644 tests/acpi-test-data/q35/SRAT.numamem
> >>>
> >>>
> >>> considering only SRAT table has been changed and the other
> >>> tables match with default blobs, I'd suggest to keep only
> >>
> >>
> >> Our testcase is:
> >>
> >> +    test_acpi_one(" -m 128,slots=3,maxmem=1G"
> >> +                  " -numa node -numa node,mem=128"
> >> +                  " -numa dist,src=0,dst=1,val=21",
> >> +                  &data);
> >>
> >> The DSDT and SLIT don't match with default blobs.
> > do you actually need SLIT table /i.e. -numa dist/ for test at all?
> > it looks not relevant for the test case at the hand,
> > I'd suggest to drop '-numa dist' option for the test.
> >
> 
> OK, Got it, will drop '-numa dist' option in next version.
> 
> >>
> >> So, they can't be dropped.
> >
> > I wonder what's changed, could you post DSDT diff here?
> >
> 
> Just like memory hot-plug cases, when we use the '-m 128
> 128,slots=3,maxmem=1G' option, As the ACPI spec said, There may be some
> Memory Device in the DSDT table.
for your case '-numa node -numa node,mem=128', there is no need in enabling memory hotplug.
If I recall it correctly the default memory for x86 is 128Mb,
hence removing "-m" would probably make DSDT match default one.

[...] 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node
  2017-08-23 17:25           ` Eduardo Habkost
@ 2017-08-24  1:30             ` Dou Liyang
  0 siblings, 0 replies; 19+ messages in thread
From: Dou Liyang @ 2017-08-24  1:30 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: Igor Mammedov, qemu-devel, pbonzini, rth, mst

Hi Eduardo,

At 08/24/2017 01:25 AM, Eduardo Habkost wrote:
> On Wed, Aug 23, 2017 at 09:35:29PM +0800, Dou Liyang wrote:
>> Hi Igor,
>>
>> At 08/23/2017 08:45 PM, Igor Mammedov wrote:
>>> On Wed, 23 Aug 2017 20:12:51 +0800
>>> Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
>>>
>>>> Hi Igor,
>>>>
>>>> At 08/23/2017 04:40 PM, Igor Mammedov wrote:
>>>>> On Tue, 22 Aug 2017 11:24:10 +0800
>>>>> Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
>>>>>
>>>>>> As QEMU supports the memory-less node, it is possible that there is
>>>>>> no RAM in the first numa node(also be called as node0). eg:
>>>>>>   ... \
>>>>>>   -m 128,slots=3,maxmem=1G \
>>>>>>   -numa node -numa node,mem=128M \
>>>>>>
>>>>>> But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
>>>>>> table. Only fixing it is not enough.
>>>>>>
>>>>>> Add a testcase for this situation to make sure the ACPI table is
>>>>>> correct for guest.
>>>>>>
>>>>>> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
>>>>>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
>>>>>> ---
>>>>>>  tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
>>>>>>  tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
>>>>>>  tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
>>>>>>  tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
>>>>>>  tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
>>>>>>  tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
>>>>>>  tests/bios-tables-test.c              |  30 ++++++++++++++++++++++++++++++
>>>>>>  7 files changed, 30 insertions(+)
>>>>>>  create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
>>>>>>  create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
>>>>>>  create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
>>>>>>  create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
>>>>>>  create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
>>>>>>  create mode 100644 tests/acpi-test-data/q35/SRAT.numamem
>>>>>
>>>>>
>>>>> considering only SRAT table has been changed and the other
>>>>> tables match with default blobs, I'd suggest to keep only
>>>>
>>>>
>>>> Our testcase is:
>>>>
>>>> +    test_acpi_one(" -m 128,slots=3,maxmem=1G"
>>>> +                  " -numa node -numa node,mem=128"
>>>> +                  " -numa dist,src=0,dst=1,val=21",
>>>> +                  &data);
>>>>
>>>> The DSDT and SLIT don't match with default blobs.
>>> do you actually need SLIT table /i.e. -numa dist/ for test at all?
>>> it looks not relevant for the test case at the hand,
>>> I'd suggest to drop '-numa dist' option for the test.
>>>
>>
>> OK, Got it, will drop '-numa dist' option in next version.
>>
>>>>
>>>> So, they can't be dropped.
>>>
>>> I wonder what's changed, could you post DSDT diff here?
>>>
>>
>> Just like memory hot-plug cases, when we use the '-m
>> 128,slots=3,maxmem=1G' option, As the ACPI spec said, There may be some
>> Memory Device in the DSDT table.
>
> Do you really need to use -m 128,slots=3,maxmem=1G to test your
> bug fix?
>

I was wrong, As the default memory for x86 is 128Mb, I will remove this 
option to make one case just do one thing.

Thanks,
	dou.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node
  2017-08-23 17:47           ` Igor Mammedov
@ 2017-08-24  1:42             ` Dou Liyang
  2017-08-24 12:33               ` Igor Mammedov
  0 siblings, 1 reply; 19+ messages in thread
From: Dou Liyang @ 2017-08-24  1:42 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: qemu-devel, ehabkost, pbonzini, rth, mst

Hi Igor,

At 08/24/2017 01:47 AM, Igor Mammedov wrote:
> On Wed, 23 Aug 2017 21:35:29 +0800
> Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
>
>> Hi Igor,
>>
>> At 08/23/2017 08:45 PM, Igor Mammedov wrote:
>>> On Wed, 23 Aug 2017 20:12:51 +0800
>>> Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
>>>
>>>> Hi Igor,
>>>>
>>>> At 08/23/2017 04:40 PM, Igor Mammedov wrote:
>>>>> On Tue, 22 Aug 2017 11:24:10 +0800
>>>>> Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
>>>>>
>>>>>> As QEMU supports the memory-less node, it is possible that there is
>>>>>> no RAM in the first numa node(also be called as node0). eg:
>>>>>>   ... \
>>>>>>   -m 128,slots=3,maxmem=1G \
>>>>>>   -numa node -numa node,mem=128M \
>>>>>>
>>>>>> But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
>>>>>> table. Only fixing it is not enough.
>>>>>>
>>>>>> Add a testcase for this situation to make sure the ACPI table is
>>>>>> correct for guest.
>>>>>>
>>>>>> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
>>>>>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
>>>>>> ---
>>>>>>  tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
>>>>>>  tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
>>>>>>  tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
>>>>>>  tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
>>>>>>  tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
>>>>>>  tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
>>>>>>  tests/bios-tables-test.c              |  30 ++++++++++++++++++++++++++++++
>>>>>>  7 files changed, 30 insertions(+)
>>>>>>  create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
>>>>>>  create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
>>>>>>  create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
>>>>>>  create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
>>>>>>  create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
>>>>>>  create mode 100644 tests/acpi-test-data/q35/SRAT.numamem
>>>>>
>>>>>
>>>>> considering only SRAT table has been changed and the other
>>>>> tables match with default blobs, I'd suggest to keep only
>>>>
>>>>
>>>> Our testcase is:
>>>>
>>>> +    test_acpi_one(" -m 128,slots=3,maxmem=1G"
>>>> +                  " -numa node -numa node,mem=128"
>>>> +                  " -numa dist,src=0,dst=1,val=21",
>>>> +                  &data);
>>>>
>>>> The DSDT and SLIT don't match with default blobs.
>>> do you actually need SLIT table /i.e. -numa dist/ for test at all?
>>> it looks not relevant for the test case at the hand,
>>> I'd suggest to drop '-numa dist' option for the test.
>>>
>>
>> OK, Got it, will drop '-numa dist' option in next version.
>>
>>>>
>>>> So, they can't be dropped.
>>>
>>> I wonder what's changed, could you post DSDT diff here?
>>>
>>
>> Just like memory hot-plug cases, when we use the '-m 128
>> 128,slots=3,maxmem=1G' option, As the ACPI spec said, There may be some
>> Memory Device in the DSDT table.
> for your case '-numa node -numa node,mem=128', there is no need in enabling memory hotplug.

Thank you very much for your kind explanation.

Yes, I understood.

> If I recall it correctly the default memory for x86 is 128Mb,
> hence removing "-m" would probably make DSDT match default one.

Yes, I removed the "-m":

test_acpi_one(" -numa node -numa node,mem=128", &data);

but, the DSDT didn't match the default one. because, if we support
NUMA, the DSDT will give us "_PXM" to map the CPU to node.


--- a/tmp/asl-7QO54Y.dsl
+++ b/tmp/asl-EWM54Y.dsl
@@ -5,13 +5,13 @@
   *
   * Disassembling to symbolic ASL+ operators
   *
- * Disassembly of tests/acpi-test-data/pc/DSDT, Thu Aug 24 09:20:29 2017
+ * Disassembly of /tmp/aml-1YM54Y, Thu Aug 24 09:20:29 2017
   *
   * Original Table Header:
   *     Signature        "DSDT"
- *     Length           0x000013EA (5098)
+ *     Length           0x000013F0 (5104)
   *     Revision         0x01 **** 32-bit table (V1), no 64-bit math 
support
- *     Checksum         0x78
+ *     Checksum         0x13
   *     OEM ID           "BOCHS "
   *     OEM Table ID     "BXPCDSDT"
   *     OEM Revision     0x00000001 (1)
@@ -783,6 +783,8 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", 
"BXPCDSDT", 0x00000001)
                  {
                      COST (Zero, Arg0, Arg1, Arg2)
                  }
+
+                Name (_PXM, Zero)  // _PXM: Device Proximity
              }
          }
      }


Thanks,
	dou.
>

> [...]
>
>
>
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node
  2017-08-24  1:42             ` Dou Liyang
@ 2017-08-24 12:33               ` Igor Mammedov
  2017-08-24 13:20                 ` Dou Liyang
  0 siblings, 1 reply; 19+ messages in thread
From: Igor Mammedov @ 2017-08-24 12:33 UTC (permalink / raw)
  To: Dou Liyang; +Cc: pbonzini, rth, mst, qemu-devel, ehabkost

On Thu, 24 Aug 2017 09:42:28 +0800
Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:

> Hi Igor,
> 
> At 08/24/2017 01:47 AM, Igor Mammedov wrote:
> > On Wed, 23 Aug 2017 21:35:29 +0800
> > Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
> >  
> >> Hi Igor,
> >>
> >> At 08/23/2017 08:45 PM, Igor Mammedov wrote:  
> >>> On Wed, 23 Aug 2017 20:12:51 +0800
> >>> Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
> >>>  
> >>>> Hi Igor,
> >>>>
> >>>> At 08/23/2017 04:40 PM, Igor Mammedov wrote:  
> >>>>> On Tue, 22 Aug 2017 11:24:10 +0800
> >>>>> Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
> >>>>>  
> >>>>>> As QEMU supports the memory-less node, it is possible that there is
> >>>>>> no RAM in the first numa node(also be called as node0). eg:
> >>>>>>   ... \
> >>>>>>   -m 128,slots=3,maxmem=1G \
> >>>>>>   -numa node -numa node,mem=128M \
> >>>>>>
> >>>>>> But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
> >>>>>> table. Only fixing it is not enough.
> >>>>>>
> >>>>>> Add a testcase for this situation to make sure the ACPI table is
> >>>>>> correct for guest.
> >>>>>>
> >>>>>> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> >>>>>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
> >>>>>> ---
> >>>>>>  tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
> >>>>>>  tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
> >>>>>>  tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
> >>>>>>  tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
> >>>>>>  tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
> >>>>>>  tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
> >>>>>>  tests/bios-tables-test.c              |  30 ++++++++++++++++++++++++++++++
> >>>>>>  7 files changed, 30 insertions(+)
> >>>>>>  create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
> >>>>>>  create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
> >>>>>>  create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
> >>>>>>  create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
> >>>>>>  create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
> >>>>>>  create mode 100644 tests/acpi-test-data/q35/SRAT.numamem  
> >>>>>
> >>>>>
> >>>>> considering only SRAT table has been changed and the other
> >>>>> tables match with default blobs, I'd suggest to keep only  
> >>>>
> >>>>
> >>>> Our testcase is:
> >>>>
> >>>> +    test_acpi_one(" -m 128,slots=3,maxmem=1G"
> >>>> +                  " -numa node -numa node,mem=128"
> >>>> +                  " -numa dist,src=0,dst=1,val=21",
> >>>> +                  &data);
> >>>>
> >>>> The DSDT and SLIT don't match with default blobs.  
> >>> do you actually need SLIT table /i.e. -numa dist/ for test at all?
> >>> it looks not relevant for the test case at the hand,
> >>> I'd suggest to drop '-numa dist' option for the test.
> >>>  
> >>
> >> OK, Got it, will drop '-numa dist' option in next version.
> >>  
> >>>>
> >>>> So, they can't be dropped.  
> >>>
> >>> I wonder what's changed, could you post DSDT diff here?
> >>>  
> >>
> >> Just like memory hot-plug cases, when we use the '-m 128
> >> 128,slots=3,maxmem=1G' option, As the ACPI spec said, There may be some
> >> Memory Device in the DSDT table.  
> > for your case '-numa node -numa node,mem=128', there is no need in enabling memory hotplug.  
> 
> Thank you very much for your kind explanation.
> 
> Yes, I understood.
> 
> > If I recall it correctly the default memory for x86 is 128Mb,
> > hence removing "-m" would probably make DSDT match default one.  
> 
> Yes, I removed the "-m":
> 
> test_acpi_one(" -numa node -numa node,mem=128", &data);
> 
> but, the DSDT didn't match the default one. because, if we support
> NUMA, the DSDT will give us "_PXM" to map the CPU to node.

Ok, looks like you'll have to include your variant of DSDT along with SRAT

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node
  2017-08-24 12:33               ` Igor Mammedov
@ 2017-08-24 13:20                 ` Dou Liyang
  0 siblings, 0 replies; 19+ messages in thread
From: Dou Liyang @ 2017-08-24 13:20 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: pbonzini, rth, mst, qemu-devel, ehabkost

Hi Igor,

At 08/24/2017 08:33 PM, Igor Mammedov wrote:
>> > test_acpi_one(" -numa node -numa node,mem=128", &data);
>> >
>> > but, the DSDT didn't match the default one. because, if we support
>> > NUMA, the DSDT will give us "_PXM" to map the CPU to node.
> Ok, looks like you'll have to include your variant of DSDT along with SRAT
>

Yes, got it.

I will modify and post this patch first.

Thanks,
	dou.

>


>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v5 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node
  2017-08-22  3:24 ` [Qemu-devel] [PATCH v4 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node Dou Liyang
  2017-08-23  8:40   ` Igor Mammedov
@ 2017-08-24 13:50   ` Dou Liyang
  1 sibling, 0 replies; 19+ messages in thread
From: Dou Liyang @ 2017-08-24 13:50 UTC (permalink / raw)
  To: qemu-devel, ehabkost, imammedo; +Cc: mst, Dou Liyang

As QEMU supports the memory-less node, it is possible that there is
no RAM in the first numa node(also be called as node0). eg:
  ... \
  -m 128,slots=3,maxmem=1G \
  -numa node -numa node,mem=128M \

But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
table. Only fixing it is not enough.

Add a testcase for this situation to make sure the ACPI table is
correct for guest.

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
v4 --> v5:
  - rewrite the testcase.
  - Drop the SLIT date

 tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 5104 bytes
 tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 224 bytes
 tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 7788 bytes
 tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 224 bytes
 tests/bios-tables-test.c              |  24 ++++++++++++++++++++++++
 5 files changed, 24 insertions(+)
 create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
 create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
 create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
 create mode 100644 tests/acpi-test-data/q35/SRAT.numamem

diff --git a/tests/acpi-test-data/pc/DSDT.numamem b/tests/acpi-test-data/pc/DSDT.numamem
new file mode 100644
index 0000000000000000000000000000000000000000..bc703d77ee48720af1b801c518b43511c95c852d
GIT binary patch
literal 5104
zcmb7IZExE~5}qX~n_5aXWizjC8^LKVXmL&SlDFc3gC$b76k9SQ%DEh9oG8b|DMfK1
z(gvt&6i`&ab^ERG<!~RBK%e8E(EADf3B7-C=%I~2Rh=QVvMQTEE5Ovw&ckPBc4uct
zwj8VRzj**QUtBlKPP+KOHZ7cE06=5<)+@>;xE-sw(qx*XF!z}jjPX%ajXzq&jTQFq
zw)Zd3`{YZHwS3rmsXyOp`CsgNTR@<vTR!JB&=;FdvFUafY_pP4o8^j?D166dwOO$0
zpf)!u7SpS0h$*RMyVMXMh9Fd<8)dsug#^HNKWm`4&vcUK@0V&;+OAv8v~jEHGz#F;
zjOWu->obq*UN7;@fd>bkHW>7wH}lOw3;x+kz<+;?^`Xya)uvf@QYk*O7srT^929^Z
z(63x0Z^&J@d%=E?%?G|FWMJruoz0sP@PWP*8-tlj#VIsH94;*3Ze&b}dmP>U!{4qE
z3%v<Yz7ErCcB#hT4#;MK2C`YiFWtO!T^5Fk&F#KdZ<t6LE}4dlMgE@WXI7XGKIqaS
zuod<Mo`5v918(>BYS9|OW!Z=at1=2Sc+mq77{0X)+xWdGVZ3hlYIS{M^y2LN<clhg
zErL$Kg1T&(qfMM1dbGgLudpBFA7oHg*lYPUF4W=@ysHG<+2u96AU1p1s?4Qz;4!|=
zGmIZ{iC@9LIljOL{3HGopXI9BT((N7bKIab9REltxZOXm*^QB}3K-|Zt*0gRdQ-UF
zeR!daV%B+bg?%c2Dy!;ZC-A4FnCsQ7SkxE`Gf>dbhD6aE$wxA%i+Vt_Of<O(8ZM1^
za7QC~q$3G=ROHdi>TwcvY}08l4PJ2-Q=9}7b8sV#4=e3<oYGx9is~!MD2)WOm?_;w
zMX^?`GffW_B$&n8Mm6oS;+nY%#ueKv-Id(SBn<|Bo0R?zdj#~ldzGg9px5vS6`T4h
zr?fXL-n8ot@aLqC=R@mdG5);8JN{=aF2R<+`=qr6T!x1|{oo;j;&^P<YO5A(_uf8u
z$a<-R7FR~o4s5?jj-&#DQU?#YG@2<6XXMe03ETeXJ6l`_Jh-sG4dCy#(BA$w1Alwp
zYr)f2-*cb6eO)GR>8#LcV|M*sM#V>#9yxmCRb#$#4_CDp-{qY)9{PBnYsRh0J+mH1
zKs}S1;o4VI5D$`V2fn5`9>Zs)r#)|D%xxO?Y1-|sO=Fmt%;AAdU;&}>q~cmRsk40k
zs~L#PG0akqe;WSnfH51ML2`oJGg3{f;=t!L=AB?>mQFSF$)!L(*L3O*`??)^fz_;D
zq4}Zp;)Hd~-`{LKJ7zr_SkIz=<JPmr>DIGuw@R^_6V|!(JIv?C%;y60xe4>R2=lq2
zd27-<UJ$=I`uv#rd|*C5VLl&WKJRwd6^!1QG_Qo_MGDa^f?F=iu4YUHn{8;}8k4Iy
zMZ2r7-wrQ4lW!gueY;*7nc!1FawScBBVB&{iT}~lzo(yk@bTlPPab~oF}TdM*H(w+
zH_7=5gF`rE39QkWR6!Lv<O%oLWfBUIHtE7KD>a9i_3C|w1tG&gG0m!lrDl#mvgwr8
z(ulMQjkJ+yR%#X12by%d#rHOYDuup;{v`{hUCCs!8S)^!&tpc)Y%Kp(>hXg%?3tNN
z=8;jJ!WveHyO%ewE8=3K7|D04M3d8K%m=S`@nBLx-urykbFZGztgZGvqZ*@#exD&W
zNreoj@*CwD(=lsmR2a;AS<ntVyppk0PLkpZ_g0h>0R=mO%QXqd#b^Er&k*f1@5QRp
zE1#qa_VaWqE}!H=IC7mHXf<xGJB>tCpr`KF31=~4|IsON`COWuCFqBfleh$@dgp$z
zs!&?t8N&}|D5jR$rv$?!tQHz6jjNzi?}b{eNf}N_0me)dgVHE6Xg~T8Pway#7z>!u
zD|V?_%H(j*g0_pYn>JcsS4b6{^<koCZh`SWVzWSNGiuVp8+B~LHfw?Nze4DxRCs*o
zt_oYY;xzxqei9_?Xz??k2exEKiK7Hlah_qTiJ#Y~K1z(_B;JJp|NrtFK&YpCW-y?8
zhCUJm8qqYVgTc2yhnoNVM6H{tU{GP}mb7bjrhq|(C5Gn5Y74gFjRMy~PlMX>{o(hA
zOd*oHie0wr;nMt?1cN)JPMzd}SMZ7%*!jG(iRPVrb8bpu=roRRH0M+WyFu*pP`XJP
z3PAtU@$$LdYs-HzmqQ2cm8u(<5II6mc&x|t7*#{P<ZlLjP4#iG`b8`;7>4F#GT1}X
z4-mKeu9F=KxZV;N$d<LRi=;tL4DvK^yYD#JJNTlrQ5*h%!B=E3=7MyiK8p;HnxHwN
zB`97KXbPd&u0}uwYfM06QP8|##S0>2or;7C)@cEqj)LA0tk`~stZF1wu*L;69tFKA
zSh;}mNT^`N#0wqJqM)|~YeGO1kx;>!6wqW8^tND~5zv`Ps9?ngF!Xgc3VKJd>H?}q
zLIrC|KvPlB_XX>mfX+oi1?#+k&PPEDf)z7A49kdw3f40MdL{~56s%_j^lT(lu$~jp
zb5T%Ju$~vt^N~=&dO<)hL_saVdQm_xMnVPaf`BeWK}&-54FP>45-M2V6wo)Lpv!{w
zEdhNi5-M0P3FxKMphkJYx?gKZy~J)C@6tW0b&mo;M**u@D5Oacnk5vlB88d`38YEx
zwl%zn_Z%!MLrEfOq*GAB;xN>at(zkq8N(w!)RDDYBORH;gF4iaz1yUl-ECua#am?0
z!2>5`yhl(z*5hrm=it#1dTbU@KGy3b;~^0GbRZw=mq^BR7Wz&gAM1mNF)@W6FOxmT
z9Me<Om|`P6#chr0DRNAfkv>iGn0^Um-1d>aLo(`(OVoJae}w3#J#8W0bsKCru(<pS
DEH%hI

literal 0
HcmV?d00001

diff --git a/tests/acpi-test-data/pc/SRAT.numamem b/tests/acpi-test-data/pc/SRAT.numamem
new file mode 100644
index 0000000000000000000000000000000000000000..7d0dec06a2cca14e68b70ccf56d55f161c5e4447
GIT binary patch
literal 224
zcmWFzatwLEz`(%x#>wB=BUr&HBEUHqD8>jB1F=Cg2*ZH@DxXmUMHZ-x3$7Gd2B8jU
a02q8=hbcr=2NPxBfU}?s9O}@Oq3{884+;SQ

literal 0
HcmV?d00001

diff --git a/tests/acpi-test-data/q35/DSDT.numamem b/tests/acpi-test-data/q35/DSDT.numamem
new file mode 100644
index 0000000000000000000000000000000000000000..fcb18d947b28c09c8b5a4fa65692efb41ef3a5d9
GIT binary patch
literal 7788
zcmb7JTW=f38J#7U)M~kumeR`dU4ruvv}j_=PLrZV1CzVFRiZ?Zbew<!xOS6Pj)R8q
z1xRWHkQ5*nKNLvR27ROh{g3!3<gEt!)Ym>0D2nJQ>iNFm$TLd{i1<M6nLTH|IkT6u
z+=)6-_k+tqSpO++g!RsH`R!I1q0JIP&^G=04SP%UcA|2vZ{<?9)!WVSHonSE@QbK?
zvu6E$JN_n)AATNNw+G_RxBlPm+#Y=TMSMpP>Cv5m>(Yk5>h?S3es3>yTVbQ<^y`ht
zD}C=ePCt}eX{TR`+QRAIY(!SDHr3zgg!YqrB8+aW4A(RJ+l6`8?=}B<{fDKCH@>;`
zu=MP=|M<g`d#(+_HT*jGy*{9?_;Ktm#Y6w={kzsDB<FoA!}+iGdADryY0n_0mZzR<
zqOMjB?)gnej<w>{Ta~UFtA!C_Y?zOFtJ@Tw5a0ip6LxRcCp`aFWj$JMwWI!W@Or<~
zEr|iO!~<%&{pZ;A<DvJ}ek}Im+|kj27|wjPe_Q<ZwkQ7ezs!L+aAz?S?M|WK&U`kF
zsYmC+7UE;!-<W!phC|ba>B}}cr9Mv;2C#&|v>NTrlpDKVZ)XihFy-#Jsow56+7{tP
zvy>JVe#-b+YjvX(vnnZfk5~&}RYX{-tXp9jUu!LV`8O7?rd36%*4ulf-3?Jx9<v;d
z5smb$yHK+RVj%p~6=Bt^`{KSxWj_|Zy*F0N(J?)p4yJiNjxDk<7O}9PdRM%Q-zzCN
z?d`qX+_-rxoSjb!9XE4D_(XgYUXF>j2+qnGr}4+bEN1yYJhhJt`fk2veU$zv?KoF<
zxKPE$oUABE8EbUbjk=);IjNh&^kG^!SA6cKO5I#bbmOMfaA;0X>?d`YLO%}raaxVA
zDgv*MUu&>U$p6bb799dV{5M}Nt#-FHxB6gbE@$KH5o_zK6(P4RY*)pcN9}G3pr$|?
zpf2X}ws&-di&^qUh*TiX$bGB>RY6jsf99rG3GN7yQc+Wam7uZ`kdzn=bJMI;BcSt-
zdu3``DnV6{l&T8Od&%W~Y*s3&EtP8UfV$6&;WNWZMa@X18a$xxGi&(FvQkmArcXfK
z=ak`d%J4a5`UKQ{I)+cj@adR70d=3aLufNSZTOrveFEw}UBjnq_;gL5fV$6|;WKCW
z%$Ys`b)UFXX~)wue0ru&K;38F@R>J!=1re~y3ZNI=ZxWV#`FoO`<yj=&Kf>vO`m|e
zPv7wA8$Ny0C!p@LVE8N;J`1K#K;7q@;d9RLIcNF=)P2qyKIaXe^QKQg-DhC<3=E%v
z=@U@*xnTHQFnlhUJ^^)~XAGZb44-FApMbj0MZ@Q!;d9aS38?!#Yxq2C_&jU+1k`<=
zV`XV{Cp^c>^5|xFPAbC}kd)Dgd)`o<H<ag1C7`anU??vb$_u6vP*+|wlot)<MN<i=
zD=!(!ONR22sRY!Oc=FWl371)^?(vtUQr)-#NvV=#$*8$x)Lb%a0?L{W0<@$BXsy#Y
z4p3~P@F+k5P*#F8fGUarl{}6l3(q@DREe3AR0T<?1*qhz0M)1zppwc;3Q$0dMg^$k
zju6#USxW&bsmB2dh@mqV0V=s8L`pTe6rhsIN(xXw<r4uaxvWHhYE%kPNo6GkD4_C*
z0F_);B0x1N1*oL5k^&S^`9y$9E-MkB8kGW6Qdvm>3aES{KqZ%z2vChm0V=7iqyPm}
zJ`tdj%Sr^OMx_9iR8~@e0xF*fP|0N_0#u_?fJ!PWDL?_0PXwssvJwHRQ7J$rm6a5r
zfXXKVRB~B~0M)1zppwc;3Q$1h69Fo@tVDonR0>c@WhDhDpz?_Tm0VULKs71_sHC!z
z0u)gBM1V>zD-oa?l>$^!SxEs3sC*(oC6|>5P>o6fDygib00mS&5ulRGN(87zr2v&w
zR#JchDxU~Y$z>%1RHIUWN-8TUKmnCc1gPY)5&^1FDL^Hal@y?W$|nL;a#@K0)u<Gp
zlFCX7P(bAq0V=tyM1X2k3Q$R9B?Ty;@`(VITvj4LH7W(Dq_UC%6j1p@fJ!ba5uh5C
z0#s62NdXF|d?G+4mz4-mjY<J3sjQ>`1ynu}pg>xH0_gz?s0S#Z7ND9E0jen}Ks6-=
zsHQ}KYDxsCrlbJXloX(v5&^0y5uloq0#s8{fNDwvsHQ}KYDx-FO-TW&DG{JR>gXas
zfpEllGATd-aTpXNrJ7p=D3F?41SpW2TMAG>HMbO?fRPB!_Yt9D=<V~L)dTuN`ZP;F
zrQYtZucql=D!nV9w-HvWy;(xyZkFjQq_4;kcsH_Hq3tSt)#$741oMr}PS+YvKX!Mv
z@e*YEz|94(H8#WcGzjftVaCmUnHAIYG7A5cpHQ2d!FOZuc-x&7w1q*m@n&Rg3eN~7
z^<Uh>>lAuf<6zjG!Wnn|%Na49M!E^yzXk5Z=q;F)?<Hsm7)>cC^rJ>cd_vpq4!6wG
zn+|b617f_`@3cGClkL$Ms64fgs+SYh@mk?0S1)VT%Ur#T@#)pe!Q|@Y-d>Zuhl%QL
z>J;Tet$fJk!$kS;g!192e3UGY*JVH1eWaC-xO|i-ADvJ>8kMgk%iE_YU(w1}xO^p1
zzH&nO%BXxbSw3@$@>Q*TmCIKX<*O%@ua3&slI63fC|}dc*SLHwQNDIU`P!&_Jz0M0
z6y@t$`8t=cC(74PC|^f;n{H*v^1dvuZXS57px12t*3xVXvV6(ohpy{Ucgf4xEtuWm
zS!1j_**3AevFmcq><$kgW8KNNiQSD|XY*!vcq$p|PPR?#ZtS`V%<k~0GS;1Jo7mmh
zb+BM|hv%2E?qu7<?#9mRGiG;qs2S@{woUAA>^xmGyTg;tSa-6`?9ShKL#_~n?$(QU
z<*jz5qs~MZ7a;N{AGU5T*FStX_~6kWm+!y#;KN7vfB)VC(X+zV+Un?x%)4&QdhR{<
zEDx`?kvj;)+;a~v^mkt(P!h`_y_XLg>m4g>H{Y_t%I}Jnu*0w!IlbLEBBjGlWo;SE
zsMKA?tK`Od!x;!_?Cq6b+2R!hj75$wUkY1|VsWKNBao{vnV=AMtM8<S?*6v;EG!m_
zcY}I1MD^+3OIufMwH`noY3xTwgY9H9iVZ|OJinjZL8BkIvvKd`u{~?62XWBMkfYNT
zvXWh)hl%|M^_j2}t~)2$u;On^x&0U`8Sy|QJH-qSYxbBSJ`$L&Bps7)4aP<W_P~mt
zuG+);cvpNpCwJwh$NK^ueN8snoDMfSUGF5L6OY%}VBvUhcY?2hn@YA4_5EZcreNae
zdY`V^bXwsG!v|+5PA%tvPB5H=%~H3Ja~*;I^Yo+_<oa}m(@7zM87u^LbL32H-+dp~
z>)TBcT)-J!YgPJ{;x!yIUag|Dn+~)e&>p@HN9TF*bu46h@*0qfkF<PMo^t>6MP8)v
z-x{AI`oeP5p?Bz64%#gb>lZ@&fZd1QG0tQ0MTq}?j+=tqPtTJ_NAx^NpOoM|{Im-A
z?C9vbA9;s(_9iV`g*JvJ7eDi@^;WTjp~?FMwQDyBEI@WkZcauP-yoK=%UKI+U;fXw
zAn9E1&t+{3g|Pivf6lSpl#8VrpkLA+D(e5{h2`GftJPbfygBDLE6tlY64!{GsN!e*
z4BdwOb$g4~pQ^T8cg%H)MJ%|{3T<!i=bhrOB*5%g0*TRCiLm5G8`spvb>7If!u78B
EKkzjyY5)KL

literal 0
HcmV?d00001

diff --git a/tests/acpi-test-data/q35/SRAT.numamem b/tests/acpi-test-data/q35/SRAT.numamem
new file mode 100644
index 0000000000000000000000000000000000000000..7d0dec06a2cca14e68b70ccf56d55f161c5e4447
GIT binary patch
literal 224
zcmWFzatwLEz`(%x#>wB=BUr&HBEUHqD8>jB1F=Cg2*ZH@DxXmUMHZ-x3$7Gd2B8jU
a02q8=hbcr=2NPxBfU}?s9O}@Oq3{884+;SQ

literal 0
HcmV?d00001

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 564da45..f092315 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -808,6 +808,28 @@ static void test_acpi_piix4_tcg_memhp(void)
     free_test_data(&data);
 }
 
+static void test_acpi_q35_tcg_numamem(void)
+{
+    test_data data;
+
+    memset(&data, 0, sizeof(data));
+    data.machine = MACHINE_Q35;
+    data.variant = ".numamem";
+    test_acpi_one(" -numa node -numa node,mem=128", &data);
+    free_test_data(&data);
+}
+
+static void test_acpi_piix4_tcg_numamem(void)
+{
+    test_data data;
+
+    memset(&data, 0, sizeof(data));
+    data.machine = MACHINE_PC;
+    data.variant = ".numamem";
+    test_acpi_one(" -numa node -numa node,mem=128", &data);
+    free_test_data(&data);
+}
+
 int main(int argc, char *argv[])
 {
     const char *arch = qtest_get_arch();
@@ -830,6 +852,8 @@ int main(int argc, char *argv[])
         qtest_add_func("acpi/q35/cpuhp", test_acpi_q35_tcg_cphp);
         qtest_add_func("acpi/piix4/memhp", test_acpi_piix4_tcg_memhp);
         qtest_add_func("acpi/q35/memhp", test_acpi_q35_tcg_memhp);
+        qtest_add_func("acpi/piix4/numamem", test_acpi_piix4_tcg_numamem);
+        qtest_add_func("acpi/q35/numamem", test_acpi_q35_tcg_numamem);
     }
     ret = g_test_run();
     boot_sector_cleanup(disk);
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 1/2] hw/acpi-build: Fix SRAT memory building when there is no memory in node0
  2017-08-22  3:24 ` [Qemu-devel] [PATCH v4 1/2] hw/acpi-build: Fix SRAT memory building when there is no memory in node0 Dou Liyang
@ 2017-08-30 20:55   ` Eduardo Habkost
  2017-08-31 10:38     ` Dou Liyang
  0 siblings, 1 reply; 19+ messages in thread
From: Eduardo Habkost @ 2017-08-30 20:55 UTC (permalink / raw)
  To: Dou Liyang; +Cc: qemu-devel, imammedo, pbonzini, mst, rth

On Tue, Aug 22, 2017 at 11:24:09AM +0800, Dou Liyang wrote:
> Currently, Using the fisrt node without memory on the machine makes
> QEMU unhappy. With this example command line:
>   ... \
>   -m 1024M,slots=4,maxmem=32G \
>   -numa node,nodeid=0 \
>   -numa node,mem=1024M,nodeid=1 \
>   -numa node,nodeid=2 \
>   -numa node,nodeid=3 \
> Guest reports "No NUMA configuration found" and the NUMA topology is
> wrong.
> 
> This is because when QEMU builds ACPI SRAT, it regards node0 as the
> default node to deal with the memory hole(640K-1M). this means the
> node0 must have some memory(>1M), but, actually it can have no
> memory.
> 
> Fix this problem by replace the node0 with the first node which has
> memory on it. Add a new function for each node. Also do some cleanup.
> 
> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
> ---
>  hw/i386/acpi-build.c | 78 +++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 50 insertions(+), 28 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 98dd424..f93d712 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2318,15 +2318,43 @@ build_tpm2(GArray *table_data, BIOSLinker *linker)
>                   (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, NULL, NULL);
>  }
>  
> +static uint64_t
> +build_srat_node_entry(GArray *table_data, PCMachineState *pcms,
> +                                int i, uint64_t mem_base, uint64_t mem_len)
> +{
> +    AcpiSratMemoryAffinity *numamem;
> +    uint64_t next_base;
> +
> +    next_base = mem_base + mem_len;
> +
> +    /* Cut out the ACPI_PCI hole */
> +    if (mem_base <= pcms->below_4g_mem_size &&
> +        next_base > pcms->below_4g_mem_size) {
> +        mem_len -= next_base - pcms->below_4g_mem_size;
> +        if (mem_len > 0) {
> +            numamem = acpi_data_push(table_data, sizeof *numamem);
> +            build_srat_memory(numamem, mem_base, mem_len, i,
> +                              MEM_AFFINITY_ENABLED);
> +        }
> +        mem_base = 1ULL << 32;
> +        mem_len = next_base - pcms->below_4g_mem_size;
> +        next_base += (1ULL << 32) - pcms->below_4g_mem_size;
> +    }
> +    numamem = acpi_data_push(table_data, sizeof *numamem);
> +    build_srat_memory(numamem, mem_base, mem_len, i,
> +                      MEM_AFFINITY_ENABLED);
> +    return next_base;
> +}
> +
>  static void
>  build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>  {
>      AcpiSystemResourceAffinityTable *srat;
>      AcpiSratMemoryAffinity *numamem;
>  
> -    int i;
> +    int i, node;
>      int srat_start, numa_start, slots;
> -    uint64_t mem_len, mem_base, next_base;
> +    uint64_t mem_len, mem_base;
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
>      const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine);
>      PCMachineState *pcms = PC_MACHINE(machine);
> @@ -2370,36 +2398,30 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>      /* the memory map is a bit tricky, it contains at least one hole
>       * from 640k-1M and possibly another one from 3.5G-4G.
>       */
> -    next_base = 0;
> +
>      numa_start = table_data->len;
>  
> -    numamem = acpi_data_push(table_data, sizeof *numamem);
> -    build_srat_memory(numamem, 0, 640 * 1024, 0, MEM_AFFINITY_ENABLED);
> -    next_base = 1024 * 1024;
> -    for (i = 1; i < pcms->numa_nodes + 1; ++i) {
> -        mem_base = next_base;
> -        mem_len = pcms->node_mem[i - 1];
> -        if (i == 1) {
> -            mem_len -= 1024 * 1024;
> +    /* get the first node which has memory and map the hole from 640K-1M */
> +    for (node = 0; node < pcms->numa_nodes; node++) {
> +        if (pcms->node_mem[node] != 0) {
> +            break;
>          }
> -        next_base = mem_base + mem_len;
> -
> -        /* Cut out the ACPI_PCI hole */
> -        if (mem_base <= pcms->below_4g_mem_size &&
> -            next_base > pcms->below_4g_mem_size) {
> -            mem_len -= next_base - pcms->below_4g_mem_size;
> -            if (mem_len > 0) {
> -                numamem = acpi_data_push(table_data, sizeof *numamem);
> -                build_srat_memory(numamem, mem_base, mem_len, i - 1,
> -                                  MEM_AFFINITY_ENABLED);
> -            }
> -            mem_base = 1ULL << 32;
> -            mem_len = next_base - pcms->below_4g_mem_size;
> -            next_base += (1ULL << 32) - pcms->below_4g_mem_size;
> +    }
> +    numamem = acpi_data_push(table_data, sizeof *numamem);
> +    build_srat_memory(numamem, 0, 640 * 1024, node, MEM_AFFINITY_ENABLED);
> +
> +    /* map the rest of memory from 1M */
> +    mem_base = 1024 * 1024;
> +    mem_len = pcms->node_mem[node] - mem_base;
> +    mem_base = build_srat_node_entry(table_data, pcms, node,
> +                                            mem_base, mem_len);
> +
> +    for (i = 0; i < pcms->numa_nodes; i++) {
> +        if (i == node) {
> +            continue;
>          }

Now the nodes will be out of order, if node 0 has no RAM.  Why?

Why not implement this in the same way the PCI 4GB hole is
already implemented.  e.g.:

#define HOLE_640K_START  (640 * 1024)
#define HOLE_640K_END   (1024 * 1024)

    for (node = 0; node < pcms->numa_nodes; node++) {
        mem_base = next_base;
        mem_len = pcms->node_mem[node];
        next_base = mem_base + mem_len;

        /* Cut out the 640K hole */
        if (mem_base <= HOLE_640K_START &&
            next_base > HOLE_640K_START) {
            mem_len -= next_base - HOLE_640K;
            if (mem_len > 0) {
                numamem = acpi_data_push(table_data, sizeof *numamem);
                build_srat_memory(numamem, mem_base, mem_len, node,
                                  MEM_AFFINITY_ENABLED);
            }
            mem_base = HOLE_640K_END;
            /* ... */
        }
        /* Cut out the ACPI_PCI hole */
        if (mem_base <= pcms->below_4g_mem_size &&
            next_base > pcms->below_4g_mem_size) {
            mem_len -= next_base - pcms->below_4g_mem_size;
            /* ... *]
        }
        numamem = acpi_data_push(table_data, sizeof *numamem);
        build_srat_memory(numamem, mem_base, mem_len, node,
                          MEM_AFFINITY_ENABLED);
        /* ... */
    }

> -        numamem = acpi_data_push(table_data, sizeof *numamem);
> -        build_srat_memory(numamem, mem_base, mem_len, i - 1,
> -                          MEM_AFFINITY_ENABLED);
> +        mem_base = build_srat_node_entry(table_data, pcms, i,
> +                                            mem_base, pcms->node_mem[i]);
>      }
>      slots = (table_data->len - numa_start) / sizeof *numamem;
>      for (; slots < pcms->numa_nodes + 2; slots++) {
> -- 
> 2.5.5
> 
> 
> 
> 

-- 
Eduardo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 1/2] hw/acpi-build: Fix SRAT memory building when there is no memory in node0
  2017-08-30 20:55   ` Eduardo Habkost
@ 2017-08-31 10:38     ` Dou Liyang
  2017-08-31 12:09       ` Dou Liyang
  0 siblings, 1 reply; 19+ messages in thread
From: Dou Liyang @ 2017-08-31 10:38 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: qemu-devel, imammedo, pbonzini, mst, rth

Hi Eduardo,

[...]
>> +            continue;
>>          }
>
> Now the nodes will be out of order, if node 0 has no RAM.  Why?
>

Because the code parsed the other node with RAM first, then parsed the 
node 0.

> Why not implement this in the same way the PCI 4GB hole is
> already implemented.  e.g.:
>

Indeed, it is better and more refined. I will do it in the next version.

> #define HOLE_640K_START  (640 * 1024)
> #define HOLE_640K_END   (1024 * 1024)
>
>     for (node = 0; node < pcms->numa_nodes; node++) {
>         mem_base = next_base;
>         mem_len = pcms->node_mem[node];
>         next_base = mem_base + mem_len;
>
>         /* Cut out the 640K hole */
>         if (mem_base <= HOLE_640K_START &&
>             next_base > HOLE_640K_START) {
>             mem_len -= next_base - HOLE_640K;
>             if (mem_len > 0) {
>                 numamem = acpi_data_push(table_data, sizeof *numamem);
>                 build_srat_memory(numamem, mem_base, mem_len, node,
>                                   MEM_AFFINITY_ENABLED);
>             }
>             mem_base = HOLE_640K_END;
>             /* ... */

BTW, in case

    0
    |-----------|-------|--------|-------
    |           |       |        |
mem_base      640K   next_base   1M


  I wanna add a check here,

             /* Check for the rare case: 640K < RAM < 1M */
             if (next_base <= HOLE_640K_END) {
                 next_base = HOLE_640K_END;
                 continue;
             }
             mem_base = HOLE_640K_END;
             mem_len = next_base - HOLE_640K_END;

I guess no one would set a node with this less RAM,
So, Is that necessary?

Thanks,
	dou.

>         }
>         /* Cut out the ACPI_PCI hole */
>         if (mem_base <= pcms->below_4g_mem_size &&
>             next_base > pcms->below_4g_mem_size) {
>             mem_len -= next_base - pcms->below_4g_mem_size;
>             /* ... *]
>         }
>         numamem = acpi_data_push(table_data, sizeof *numamem);
>         build_srat_memory(numamem, mem_base, mem_len, node,
>                           MEM_AFFINITY_ENABLED);
>         /* ... */
>     }
>
>> -        numamem = acpi_data_push(table_data, sizeof *numamem);
>> -        build_srat_memory(numamem, mem_base, mem_len, i - 1,
>> -                          MEM_AFFINITY_ENABLED);
>> +        mem_base = build_srat_node_entry(table_data, pcms, i,
>> +                                            mem_base, pcms->node_mem[i]);
>>      }
>>      slots = (table_data->len - numa_start) / sizeof *numamem;
>>      for (; slots < pcms->numa_nodes + 2; slots++) {
>> --
>> 2.5.5
>>
>>
>>
>>
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 1/2] hw/acpi-build: Fix SRAT memory building when there is no memory in node0
  2017-08-31 10:38     ` Dou Liyang
@ 2017-08-31 12:09       ` Dou Liyang
  0 siblings, 0 replies; 19+ messages in thread
From: Dou Liyang @ 2017-08-31 12:09 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: qemu-devel, imammedo, pbonzini, mst, rth

Hi Eduardo,

At 08/31/2017 06:38 PM, Dou Liyang wrote:
> Hi Eduardo,
>
> [...]
>>> +            continue;
>>>          }
>>
>> Now the nodes will be out of order, if node 0 has no RAM.  Why?
>>
>
> Because the code parsed the other node with RAM first, then parsed the
> node 0.
>
>> Why not implement this in the same way the PCI 4GB hole is
>> already implemented.  e.g.:
>>
>
> Indeed, it is better and more refined. I will do it in the next version.
>
>> #define HOLE_640K_START  (640 * 1024)
>> #define HOLE_640K_END   (1024 * 1024)
>>
>>     for (node = 0; node < pcms->numa_nodes; node++) {
>>         mem_base = next_base;
>>         mem_len = pcms->node_mem[node];
>>         next_base = mem_base + mem_len;
>>
>>         /* Cut out the 640K hole */
>>         if (mem_base <= HOLE_640K_START &&
>>             next_base > HOLE_640K_START) {
>>             mem_len -= next_base - HOLE_640K;
>>             if (mem_len > 0) {
>>                 numamem = acpi_data_push(table_data, sizeof *numamem);
>>                 build_srat_memory(numamem, mem_base, mem_len, node,
>>                                   MEM_AFFINITY_ENABLED);
>>             }
>>             mem_base = HOLE_640K_END;
>>             /* ... */
>
> BTW, in case
>
>    0
>    |-----------|-------|--------|-------
>    |           |       |        |
> mem_base      640K   next_base   1M
>
>
>  I wanna add a check here,
>
>             /* Check for the rare case: 640K < RAM < 1M */
>             if (next_base <= HOLE_640K_END) {
>                 next_base = HOLE_640K_END;
>                 continue;
>             }
>             mem_base = HOLE_640K_END;
>             mem_len = next_base - HOLE_640K_END;
>
> I guess no one would set a node with this less RAM,
> So, Is that necessary?
>

I post the V5 patches, and use your code.

It may be not very clearly, for the detail, Please see the new version.

Thanks,
	dou.

> Thanks,
>     dou.
>
>>         }
>>         /* Cut out the ACPI_PCI hole */
>>         if (mem_base <= pcms->below_4g_mem_size &&
>>             next_base > pcms->below_4g_mem_size) {
>>             mem_len -= next_base - pcms->below_4g_mem_size;
>>             /* ... *]
>>         }
>>         numamem = acpi_data_push(table_data, sizeof *numamem);
>>         build_srat_memory(numamem, mem_base, mem_len, node,
>>                           MEM_AFFINITY_ENABLED);
>>         /* ... */
>>     }
>>
>>> -        numamem = acpi_data_push(table_data, sizeof *numamem);
>>> -        build_srat_memory(numamem, mem_base, mem_len, i - 1,
>>> -                          MEM_AFFINITY_ENABLED);
>>> +        mem_base = build_srat_node_entry(table_data, pcms, i,
>>> +                                            mem_base,
>>> pcms->node_mem[i]);
>>>      }
>>>      slots = (table_data->len - numa_start) / sizeof *numamem;
>>>      for (; slots < pcms->numa_nodes + 2; slots++) {
>>> --
>>> 2.5.5
>>>
>>>
>>>
>>>
>>

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2017-08-31 12:10 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-22  3:24 [Qemu-devel] [PATCH v4 0/2] hw/acpi-build: Fix ACPI SRAT Memory Affinity building Dou Liyang
2017-08-22  3:24 ` [Qemu-devel] [PATCH v4 1/2] hw/acpi-build: Fix SRAT memory building when there is no memory in node0 Dou Liyang
2017-08-30 20:55   ` Eduardo Habkost
2017-08-31 10:38     ` Dou Liyang
2017-08-31 12:09       ` Dou Liyang
2017-08-22  3:24 ` [Qemu-devel] [PATCH v4 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node Dou Liyang
2017-08-23  8:40   ` Igor Mammedov
2017-08-23 12:12     ` Dou Liyang
2017-08-23 12:45       ` Igor Mammedov
2017-08-23 13:35         ` Dou Liyang
2017-08-23 17:25           ` Eduardo Habkost
2017-08-24  1:30             ` Dou Liyang
2017-08-23 17:47           ` Igor Mammedov
2017-08-24  1:42             ` Dou Liyang
2017-08-24 12:33               ` Igor Mammedov
2017-08-24 13:20                 ` Dou Liyang
2017-08-24 13:50   ` [Qemu-devel] [PATCH v5 " Dou Liyang
2017-08-23  2:48 ` [Qemu-devel] [PATCH v4 0/2] hw/acpi-build: Fix ACPI SRAT Memory Affinity building Michael S. Tsirkin
2017-08-23  3:47   ` Dou Liyang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.