Re: [PATCH 1/8] hmat acpi: Don't require initiator value in -numa

2022-07-17 Thread Brice Goglin 


Le 18/07/2022 à 05:17, Liu, Jingqi a écrit :


On 7/11/2022 6:44 PM, Hesham Almatary via wrote:

From: Brice Goglin 

The "Memory Proximity Domain Attributes" structure of the ACPI HMAT
has a "Processor Proximity Domain Valid" flag that is currently
always set because Qemu -numa requires an initiator=X value
when hmat=on. Unsetting this flag allows to create more complex
memory topologies by having multiple best initiators for a single
memory target.


Do you mean the memory-only numa node requires an initiator=X value ?

It would be better if you can explicitly provide cases in the description

for creating more complex memory topologies without initiator=X.



Hello

I wrote this text so I am going to reply. All nodes currently require an 
initiator= attribute. For CPU-node, that's usually OK. However it forces 
all nodes to have a best initiator that is exactly one single node. This 
prevents creating a topology where some memory has for best initiator 
the sum of 2 nodes for instance. There's an example below in the 
description. A real-world example would be a Xeon server with SNC 
enabled: there are 2 DDR per socket, one per SNC, but a single NVDIMM 
per socket covering both SNC. That NVDIMM node cannot have a single 
initiator value since its best initiator is both SNC. That's similar to 
the example below in the description


Brice





Thanks,

Jingqi


This patch allows -numa without initiator=X when hmat=on by keeping
the default value MAX_NODES in numa_state->nodes[i].initiator.
All places reading numa_state->nodes[i].initiator already check
whether it's different from MAX_NODES before using it.

Tested with
qemu-system-x86_64 -accel kvm \
  -machine pc,hmat=on \
  -drive if=pflash,format=raw,file=./OVMF.fd \
  -drive media=disk,format=qcow2,file=efi.qcow2 \
  -smp 4 \
  -m 3G \
  -object memory-backend-ram,size=1G,id=ram0 \
  -object memory-backend-ram,size=1G,id=ram1 \
  -object memory-backend-ram,size=1G,id=ram2 \
  -numa node,nodeid=0,memdev=ram0,cpus=0-1 \
  -numa node,nodeid=1,memdev=ram1,cpus=2-3 \
  -numa node,nodeid=2,memdev=ram2 \
  -numa 
hmat-lb,initiator=0,target=0,hierarchy=memory,data-type=access-latency,latency=10 
\
  -numa 
hmat-lb,initiator=0,target=0,hierarchy=memory,data-type=access-bandwidth,bandwidth=10485760 
\
  -numa 
hmat-lb,initiator=0,target=1,hierarchy=memory,data-type=access-latency,latency=20 
\
  -numa 
hmat-lb,initiator=0,target=1,hierarchy=memory,data-type=access-bandwidth,bandwidth=5242880 
\
  -numa 
hmat-lb,initiator=0,target=2,hierarchy=memory,data-type=access-latency,latency=30 
\
  -numa 
hmat-lb,initiator=0,target=2,hierarchy=memory,data-type=access-bandwidth,bandwidth=1048576 
\
  -numa 
hmat-lb,initiator=1,target=0,hierarchy=memory,data-type=access-latency,latency=20 
\
  -numa 
hmat-lb,initiator=1,target=0,hierarchy=memory,data-type=access-bandwidth,bandwidth=5242880 
\
  -numa 
hmat-lb,initiator=1,target=1,hierarchy=memory,data-type=access-latency,latency=10 
\
  -numa 
hmat-lb,initiator=1,target=1,hierarchy=memory,data-type=access-bandwidth,bandwidth=10485760 
\
  -numa 
hmat-lb,initiator=1,target=2,hierarchy=memory,data-type=access-latency,latency=30 
\
  -numa 
hmat-lb,initiator=1,target=2,hierarchy=memory,data-type=access-bandwidth,bandwidth=1048576
which reports NUMA node2 at same distance from both node0 and node1 
as seen in lstopo:

Machine (2966MB total) + Package P#0
   NUMANode P#2 (979MB)
   Group0
 NUMANode P#0 (980MB)
 Core P#0 + PU P#0
 Core P#1 + PU P#1
   Group0
 NUMANode P#1 (1007MB)
 Core P#2 + PU P#2
 Core P#3 + PU P#3

Before this patch, we had to add ",initiator=X" to "-numa 
node,nodeid=2,memdev=ram2".

The lstopo output difference between initiator=1 and no initiator is:
@@ -1,10 +1,10 @@
  Machine (2966MB total) + Package P#0
+  NUMANode P#2 (979MB)
    Group0
  NUMANode P#0 (980MB)
  Core P#0 + PU P#0
  Core P#1 + PU P#1
    Group0
  NUMANode P#1 (1007MB)
-    NUMANode P#2 (979MB)
  Core P#2 + PU P#2
  Core P#3 + PU P#3

Corresponding changes in the HMAT MPDA structure:
@@ -49,10 +49,10 @@
  [078h 0120   2]   Structure Type :  [Memory 
Proximity Domain Attributes]

  [07Ah 0122   2] Reserved : 
  [07Ch 0124   4]   Length : 0028
-[080h 0128   2]    Flags (decoded below) : 0001
-    Processor Proximity Domain Valid : 1
+[080h 0128   2]    Flags (decoded below) : 
+    Processor Proximity Domain Valid : 0
  [082h 0130   2]    Reserved1 : 
-[084h 0132   4] Attached Initiator Proximity Domain : 0001
+[084h 0132   4] Attached Initiator Proximity Domain : 0080
  [088h 0136   4]  Memory Proximity Domain : 0002
  [08Ch 0140   4]    Reserved2 : 
  [090h 0144   8]    Reserved3 : 

Final HMAT SLLB structures:
[0A0h 0160   2]   Structure Type

Re: [PATCH v4 0/4] hmat acpi: Don't require initiator value in -numa

2022-06-30 Thread Brice Goglin 


Le 30/06/2022 à 14:23, Igor Mammedov a écrit :

On Thu, 30 Jun 2022 09:36:47 +0200
Brice Goglin  wrote:


Allow -numa without initiator value when hmat=on so that we may
build more complex topologies, e.g. NUMA nodes whose best initiators
are not just another single node.


patches looks fine code-wise,
however something wrong with them, i.e. 'git am' doesn't like them
nor ./scripts/checkpatch (which one should use before sending patches).

I've checked it's not my mail server/client issue(or whatever)
that corrupts them (ones downloaded from patchew are broken as well)



I don't know what's going on. These 4 patches are in 
https://github.com/bgoglin/qemu/commits/hmat-noinitiator (rebased on 
master 10mn ago).


Do whatever you want with them. I am not allowed to spend more time on this.

Brice





OpenPGP_signature
Description: OpenPGP digital signature

[PATCH v4 3/4] tests: acpi: q35: add test for hmat nodes without initiators

2022-06-30 Thread Brice Goglin 
000
[10Ch 0268   4] Target Proximity Domain List : 0001
[110h 0272   4] Target Proximity Domain List : 0002
[114h 0276   2]Entry : 000A
[116h 0278   2]Entry : 0005
[118h 0280   2]Entry : 0001
[11Ah 0282   2]Entry : 0005
[11Ch 0284   2]Entry : 000A
[11Eh 0286   2]Entry : 0001

Raw Table Data: Length 288 (0x120)

: 48 4D 41 54 20 01 00 00 02 4F 42 4F 43 48 53 20  // HMAT OBOCHS
0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43  // BXPCBXPC
0020: 01 00 00 00 00 00 00 00 00 00 00 00 28 00 00 00  // (...
0030: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
0040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
0050: 00 00 00 00 28 00 00 00 01 00 00 00 01 00 00 00  // (...
0060: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
0070: 00 00 00 00 00 00 00 00 00 00 00 00 28 00 00 00  // (...
0080: 00 00 00 00 80 00 00 00 02 00 00 00 00 00 00 00  // 
0090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
00A0: 01 00 00 00 40 00 00 00 00 00 00 00 02 00 00 00  // @...
00B0: 03 00 00 00 00 00 00 00 10 27 00 00 00 00 00 00  // .'..
00C0: 00 00 00 00 01 00 00 00 00 00 00 00 01 00 00 00  // 
00D0: 02 00 00 00 01 00 02 00 03 00 02 00 01 00 03 00  // 
00E0: 01 00 00 00 40 00 00 00 00 03 00 00 02 00 00 00  // @...
00F0: 03 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00  // 
0100: 00 00 00 00 01 00 00 00 00 00 00 00 01 00 00 00  // 
0110: 02 00 00 00 0A 00 05 00 01 00 05 00 0A 00 01 00  // ....

Signed-off-by: Brice Goglin 
Reviewed-by: Jonathan Cameron 
---
 tests/qtest/bios-tables-test.c | 49 ++
 1 file changed, 49 insertions(+)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 359916c228..0cea6684dc 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1461,6 +1461,54 @@ static void test_acpi_piix4_tcg_acpi_hmat(void)
 test_acpi_tcg_acpi_hmat(MACHINE_PC);
 }
 
+static void test_acpi_q35_tcg_acpi_hmat_noinitiator(void)

+{
+test_data data;
+
+memset(, 0, sizeof(data));
+data.machine = MACHINE_Q35;
+data.variant = ".acpihmat-noinitiator";
+test_acpi_one(" -machine hmat=on"
+  " -smp 4,sockets=2"
+  " -m 128M"
+  " -object memory-backend-ram,size=32M,id=ram0"
+  " -object memory-backend-ram,size=32M,id=ram1"
+  " -object memory-backend-ram,size=64M,id=ram2"
+  " -numa node,nodeid=0,memdev=ram0"
+  " -numa node,nodeid=1,memdev=ram1"
+  " -numa node,nodeid=2,memdev=ram2"
+  " -numa cpu,node-id=0,socket-id=0"
+  " -numa cpu,node-id=0,socket-id=0"
+  " -numa cpu,node-id=1,socket-id=1"
+  " -numa cpu,node-id=1,socket-id=1"
+  " -numa hmat-lb,initiator=0,target=0,hierarchy=memory,"
+  "data-type=access-latency,latency=10"
+  " -numa hmat-lb,initiator=0,target=0,hierarchy=memory,"
+  "data-type=access-bandwidth,bandwidth=10485760"
+  " -numa hmat-lb,initiator=0,target=1,hierarchy=memory,"
+  "data-type=access-latency,latency=20"
+  " -numa hmat-lb,initiator=0,target=1,hierarchy=memory,"
+  "data-type=access-bandwidth,bandwidth=5242880"
+  " -numa hmat-lb,initiator=0,target=2,hierarchy=memory,"
+  "data-type=access-latency,latency=30"
+  " -numa hmat-lb,initiator=0,target=2,hierarchy=memory,"
+  "data-type=access-bandwidth,bandwidth=1048576"
+  " -numa hmat-lb,initiator=1,target=0,hierarchy=memory,"
+  "data-type=access-latency,latency=20"
+  " -numa hmat-lb,initiator=1,target=0,hierarchy=memory,"
+  "data-type=access-bandwidth,bandwidth=5242880"
+  " -numa hmat-lb,initiator=1,target=1,hierarchy=memory,"
+  "data-type=access-latency,latency=10"
+  " -numa hmat-lb,initiator=1,target=1,hierarchy=memory,"
+  "data-type=access-bandwidth,bandwidth=10485760"
+  " -numa hmat-lb,init

[PATCH v4 4/4] tests: acpi: q35: update expected blobs *.hmat-noinitiators

2022-06-30 Thread Brice Goglin 
 Entry : 0005
[11Ch 0284   2]Entry : 000A
[11Eh 0286   2]Entry : 0001

Raw Table Data: Length 288 (0x120)

: 48 4D 41 54 20 01 00 00 02 4F 42 4F 43 48 53 20  // HMAT OBOCHS
0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43  // BXPCBXPC
0020: 01 00 00 00 00 00 00 00 00 00 00 00 28 00 00 00  // (...
0030: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
0040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
0050: 00 00 00 00 28 00 00 00 01 00 00 00 01 00 00 00  // (...
0060: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
0070: 00 00 00 00 00 00 00 00 00 00 00 00 28 00 00 00  // (...
0080: 00 00 00 00 80 00 00 00 02 00 00 00 00 00 00 00  // 
0090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
00A0: 01 00 00 00 40 00 00 00 00 00 00 00 02 00 00 00  // @...
00B0: 03 00 00 00 00 00 00 00 10 27 00 00 00 00 00 00  // .'..
00C0: 00 00 00 00 01 00 00 00 00 00 00 00 01 00 00 00  // 
00D0: 02 00 00 00 01 00 02 00 03 00 02 00 01 00 03 00  // 
00E0: 01 00 00 00 40 00 00 00 00 03 00 00 02 00 00 00  // @...
00F0: 03 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00  // 
0100: 00 00 00 00 01 00 00 00 00 00 00 00 01 00 00 00  // 
0110: 02 00 00 00 0A 00 05 00 01 00 05 00 0A 00 01 00  // ....

Signed-off-by: Brice Goglin 
Reviewed-by: Jonathan Cameron 
---
 tests/data/acpi/q35/APIC.acpihmat-noinitiator | Bin 0 -> 144 bytes
 tests/data/acpi/q35/DSDT.acpihmat-noinitiator | Bin 0 -> 8553 bytes
 tests/data/acpi/q35/FACP.acpihmat-noinitiator | Bin 0 -> 244 bytes
 tests/data/acpi/q35/HMAT.acpihmat-noinitiator | Bin 0 -> 288 bytes
 tests/data/acpi/q35/SRAT.acpihmat-noinitiator | Bin 0 -> 312 bytes
 tests/qtest/bios-tables-test-allowed-diff.h   |   5 -
 6 files changed, 5 deletions(-)

diff --git a/tests/data/acpi/q35/APIC.acpihmat-noinitiator 
b/tests/data/acpi/q35/APIC.acpihmat-noinitiator
index 
e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..d904d4a70ddecbb79a83a267af8e26f925e9f4c6
 100644
GIT binary patch
literal 144
zcmZ<^@N}NQz`(%h?d0$55v<@85#X!<1dKp25F11@Fg*ANra6G>KwJ(+MhMNs1fiLk
tK{O)|Nb|r~o3y%?)O;u>A)b0RWi;3;_TD

literal 0
HcmV?d1

diff --git a/tests/data/acpi/q35/DSDT.acpihmat-noinitiator 
b/tests/data/acpi/q35/DSDT.acpihmat-noinitiator
index 
e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..c767d11cb1d088f613c49e55a7139cccababf66c
 100644
GIT binary patch
literal 8553
zcmb7JOKcm*8J^`sS}m8-lA^$0{LCIadERkMusn`Jpkg}Xsa#CcG
z#6TVhAj?4F_)s)q67)z13ea17>a_-XX>YzYKu;$gY0nd~tOIw46@kU+2F&
zd-_*jUVf)|@0b7l^_|zP1OTh}HSv2sq_5rwJ?l(w;C$BKGd?6bKesYi{H)JUi#CVO
zggt7xYl|jIwQ~4+=inu;RdjhD(|*%0xP=w;%WVajv)1X4ml?BZaOm^r;c)m@ambwa
znA0$Y%CcGW%WbLBfHqRq*{}KS2D2Hv|1iZ8otqBDi?5WMUfyqc-F)taZqP2WkXm{n
zHEqA*?*{?=OI4%Cddx7Y
z#x-^gzvp5|-#K`yacOhJT3FOrrjbzjgZa7)e2M&7S`SMnXB
zwC=94+|#~Wz8LE~Nzrf^?h+z=&)8#>qw_;*K4iIPir`E`QLd1_dJ(pl^JaW6Xv`1R
zqkG~0i~smf@cjFYPnW-KocNpD2r1VazwHg4>|bYt6a76ahAICcpEx$=>5v^};>Jz-
zVt1DZOa1HCY?w{PSek8zz1!Q29d4U`-L0^k-NmmF?o#CZ?p1?kk>h^L?_9st_Ookm
z?`Km9Oh>lfTq`qiY~Z-UvNp9_F$7f^#1Yh3Hl08}aO((M6cG!+d2x>O0F@yzk)LB@
zoC$0c5mQ2Aj57h{OoYTlZ#Xv2nX(6QMobBf3nrkPiIAAG3T`BU<#`gEDWM6$l$}SY
z>P%`nlbk7`Nx_tzN2uzY(R9vmri9MuIuWWmXEmL(n$B5WCqh-Hsp|ou;l6p{f&)
z9A%{EG@Wz0PK2sXOVep-IxSr%LRDu<)0xtArgWVMRh@YBDc951blSR3gsRT8rZcVS
zOzS!ksygR2o%5Q`d0i($Rp)}Hb3xO>U1=nj;7Pmbs|)CW;C4{O=m{eiBQ#<
z)pTYxompKcLRIIYrgKr#xv1+zsOrpVI&+%NoURk0s%xxuoe_(sd$Kbsp1n9@BIl
z({&h_lvrj=w_u}WS`n-#`zR!)
z7hz9o%##}Pq|QXBGEZsDQyTM>(PixH68uPTyM5r>K(3nqX%qMgvLX~-jGo{O%
z;Y|6AKO>m(!HtlZ@(P~SYM#|`0y;8Kgy@YlP=Spis;L^NfQ}3lA$r8I
zFi?SwqDUi$l7R}S3{{;lP?451VW0|>3>2YsMKVx?QYQ>lU^x>8szAv=1(eq$14Srx
z!axOlU^x>8szAv=1(Y+%KoLrvFi?TzOcu
z>1(q{mpbC@>R6seC3>2Z%2?G^a+#~P%=;f=3{*fllMEE0)CmI>Sk8oj
zDo`>|0p(0GP=rz^3{+q_69%e4$v_2^Gs!>^N}VuJf#pmXr~)Me6;RG314Srx!axO<
zGhv_#lnhirIg<<|Wj6M_lrzad5lWpf
zP=V!47^ngz0~JutBm+e#b;3XqDF%v2HBf}Afg)54RAItE6($*|!XyJ#m@rU<2?JG_
zWS|O@3{+vlKouqoRAG{VDoip^g$V;ym@rU1;Px-d{gxMF-T$v_d}
zGRTmaa%^Fsh~(J9KoQBYB?Cn$$CeBfA=ZXYc447xVE6f-)kFG2dUu9?ik+K3e=bh{
zQfaA!mNyK)wNZrQZj|WDr7zE9SZvuS({Y8qs`OPebBpy2tOg9`KCt$8v8pn6Xr*!&
z>Kkrr4uDo6GjF9nnPGE$>E}ImhuW+pzU$!!yVe4uBXpvLrIz(sydxCll824;8Cv}@
z(d{5}%v$_pp3TMKZVbVJjUA?4KFY)Rn^iQo`%1y6c>KrskyI4EJ`d*~niakbo
zS1IrE^6pr9_lWXtRNfmCZ(%{}d#m@9@*Xemjg|L~DDOq(%j4w}k5Rs?lrQu0<+1YR
zBg@|E%O$;T*PQOZ|%`N~-N$`R!`I*NkUscLidHL#C`RWnnt5Nydc=_4K
zC|^^`*LeBbSozu!YH+~yi89g1|
zH73%DwyEhR?#ry64lf`R=|tPqbQAX(mVoI#lfv*;GLcTSO-(m(-{kakcvYE5C)%c_
zo45~_^mKTCnMfzvrly;i*T?j9c)%c_o0zA|dOE!6Or#TSdOGKnvc|scf+0>HHfTy9YOaL_v||UBBEk+uZO-Tq8-!AFq%xo(dVJ=Fe4+xq;g>kC0)y
z}JY#6eg(75(hIorQxgzQ>q^g_1%Rm1*$zh`{0Vot5e0CUT)aH?B?}=f&l

[PATCH v4 2/4] tests: acpi: add and whitelist *.hmat-noinitiator expected blobs

2022-06-30 Thread Brice Goglin 

.. which will be used by follow up hmat-noinitiator test-case.

Signed-off-by: Brice Goglin 
Acked-by: Igor Mammedov 
Reviewed-by: Jonathan Cameron 
---
 tests/data/acpi/q35/APIC.acpihmat-noinitiator | 0
 tests/data/acpi/q35/DSDT.acpihmat-noinitiator | 0
 tests/data/acpi/q35/FACP.acpihmat-noinitiator | 0
 tests/data/acpi/q35/HMAT.acpihmat-noinitiator | 0
 tests/data/acpi/q35/SRAT.acpihmat-noinitiator | 0
 tests/qtest/bios-tables-test-allowed-diff.h   | 5 +
 6 files changed, 5 insertions(+)
 create mode 100644 tests/data/acpi/q35/APIC.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/DSDT.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/FACP.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/HMAT.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/SRAT.acpihmat-noinitiator

diff --git a/tests/data/acpi/q35/APIC.acpihmat-noinitiator 
b/tests/data/acpi/q35/APIC.acpihmat-noinitiator
new file mode 100644
index 00..e69de29bb2
diff --git a/tests/data/acpi/q35/DSDT.acpihmat-noinitiator 
b/tests/data/acpi/q35/DSDT.acpihmat-noinitiator
new file mode 100644
index 00..e69de29bb2
diff --git a/tests/data/acpi/q35/FACP.acpihmat-noinitiator 
b/tests/data/acpi/q35/FACP.acpihmat-noinitiator
new file mode 100644
index 00..e69de29bb2
diff --git a/tests/data/acpi/q35/HMAT.acpihmat-noinitiator 
b/tests/data/acpi/q35/HMAT.acpihmat-noinitiator
new file mode 100644
index 00..e69de29bb2
diff --git a/tests/data/acpi/q35/SRAT.acpihmat-noinitiator 
b/tests/data/acpi/q35/SRAT.acpihmat-noinitiator
new file mode 100644
index 00..e69de29bb2
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h 
b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..ae025e3a3e 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,6 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/q35/APIC.acpihmat-noinitiator",
+"tests/data/acpi/q35/DSDT.acpihmat-noinitiator",
+"tests/data/acpi/q35/FACP.acpihmat-noinitiator",
+"tests/data/acpi/q35/HMAT.acpihmat-noinitiator",
+"tests/data/acpi/q35/SRAT.acpihmat-noinitiator",
--
2.30.2





OpenPGP_signature
Description: OpenPGP digital signature

[PATCH v4 1/4] hmat acpi: Don't require initiator value in -numa

2022-06-30 Thread Brice Goglin 
 0220   2]Entry : 0001
[0DEh 0222   2]Entry : 0003

[0E0h 0224   2]   Structure Type : 0001 [System Locality Latency 
and Bandwidth Information]
[0E2h 0226   2] Reserved : 
[0E4h 0228   4]   Length : 0040
[0E8h 0232   1]Flags (decoded below) : 00
Memory Hierarchy : 0
[0E9h 0233   1]Data Type : 03
[0EAh 0234   2]Reserved1 : 
[0ECh 0236   4] Initiator Proximity Domains # : 0002
[0F0h 0240   4]   Target Proximity Domains # : 0003
[0F4h 0244   4]Reserved2 : 
[0F8h 0248   8]  Entry Base Unit : 0001
[100h 0256   4] Initiator Proximity Domain List : 
[104h 0260   4] Initiator Proximity Domain List : 0001
[108h 0264   4] Target Proximity Domain List : 
[10Ch 0268   4] Target Proximity Domain List : 0001
[110h 0272   4] Target Proximity Domain List : 0002
[114h 0276   2]Entry : 000A
[116h 0278   2]Entry : 0005
[118h 0280   2]Entry : 0001
[11Ah 0282   2]Entry : 0005
[11Ch 0284   2]Entry : 000A
[11Eh 0286   2]        Entry : 0001

Signed-off-by: Brice Goglin 
Reviewed-by: Jonathan Cameron 
---
 hw/core/machine.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index a673302cce..d4d7e77401 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1173,9 +1173,7 @@ static void numa_validate_initiator(NumaState *numa_state)
 
 for (i = 0; i < numa_state->num_nodes; i++) {

 if (numa_info[i].initiator == MAX_NODES) {
-error_report("The initiator of NUMA node %d is missing, use "
- "'-numa node,initiator' option to declare it", i);
-exit(1);
+continue;
 }
 
 if (!numa_info[numa_info[i].initiator].present) {

--
2.30.2





OpenPGP_signature
Description: OpenPGP digital signature

[PATCH v4 0/4] hmat acpi: Don't require initiator value in -numa

2022-06-30 Thread Brice Goglin 

Allow -numa without initiator value when hmat=on so that we may
build more complex topologies, e.g. NUMA nodes whose best initiators
are not just another single node.

changes v3->v4
* use -numa cpu instead of legacy cpus=
changes v2->v3:
* improve messages for patches 0/4 and 3/4
changes v1->v2:
* add q35 acpi test

Brice Goglin (4):
  hmat acpi: Don't require initiator value in -numa
  tests: acpi: add and whitelist *.hmat-noinitiator expected blobs
  tests: acpi: q35: add test for hmat nodes without initiators
  tests: acpi: q35: update expected blobs *.hmat-noinitiators

 hw/core/machine.c |   4 +-
 tests/data/acpi/q35/APIC.acpihmat-noinitiator | Bin 0 -> 144 bytes
 tests/data/acpi/q35/DSDT.acpihmat-noinitiator | Bin 0 -> 8553 bytes
 tests/data/acpi/q35/FACP.acpihmat-noinitiator | Bin 0 -> 244 bytes
 tests/data/acpi/q35/HMAT.acpihmat-noinitiator | Bin 0 -> 288 bytes
 tests/data/acpi/q35/SRAT.acpihmat-noinitiator | Bin 0 -> 312 bytes
 tests/qtest/bios-tables-test.c|  49 ++
 7 files changed, 50 insertions(+), 3 deletions(-)
 create mode 100644 tests/data/acpi/q35/APIC.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/DSDT.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/FACP.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/HMAT.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/SRAT.acpihmat-noinitiator

--
2.30.2





OpenPGP_signature
Description: OpenPGP digital signature

[PATCH v3 1/4] hmat acpi: Don't require initiator value in -numa

2022-06-29 Thread Brice Goglin 
 0220   2]Entry : 0001
[0DEh 0222   2]Entry : 0003

[0E0h 0224   2]   Structure Type : 0001 [System Locality Latency 
and Bandwidth Information]
[0E2h 0226   2] Reserved : 
[0E4h 0228   4]   Length : 0040
[0E8h 0232   1]Flags (decoded below) : 00
Memory Hierarchy : 0
[0E9h 0233   1]Data Type : 03
[0EAh 0234   2]Reserved1 : 
[0ECh 0236   4] Initiator Proximity Domains # : 0002
[0F0h 0240   4]   Target Proximity Domains # : 0003
[0F4h 0244   4]Reserved2 : 
[0F8h 0248   8]  Entry Base Unit : 0001
[100h 0256   4] Initiator Proximity Domain List : 
[104h 0260   4] Initiator Proximity Domain List : 0001
[108h 0264   4] Target Proximity Domain List : 
[10Ch 0268   4] Target Proximity Domain List : 0001
[110h 0272   4] Target Proximity Domain List : 0002
[114h 0276   2]Entry : 000A
[116h 0278   2]Entry : 0005
[118h 0280   2]Entry : 0001
[11Ah 0282   2]Entry : 0005
[11Ch 0284   2]Entry : 000A
[11Eh 0286   2]        Entry : 0001

Signed-off-by: Brice Goglin 
Reviewed-by: Jonathan Cameron 
---
 hw/core/machine.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index a673302cce..d4d7e77401 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1173,9 +1173,7 @@ static void numa_validate_initiator(NumaState *numa_state)
 
 for (i = 0; i < numa_state->num_nodes; i++) {

 if (numa_info[i].initiator == MAX_NODES) {
-error_report("The initiator of NUMA node %d is missing, use "
- "'-numa node,initiator' option to declare it", i);
-exit(1);
+continue;
 }
 
 if (!numa_info[numa_info[i].initiator].present) {

--
2.30.2





OpenPGP_signature
Description: OpenPGP digital signature

[PATCH v3 4/4] tests: acpi: q35: update expected blobs *.hmat-noinitiators

2022-06-29 Thread Brice Goglin 
 Entry : 0005
[11Ch 0284   2]Entry : 000A
[11Eh 0286   2]Entry : 0001

Raw Table Data: Length 288 (0x120)

: 48 4D 41 54 20 01 00 00 02 4F 42 4F 43 48 53 20  // HMAT OBOCHS
0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43  // BXPCBXPC
0020: 01 00 00 00 00 00 00 00 00 00 00 00 28 00 00 00  // (...
0030: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
0040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
0050: 00 00 00 00 28 00 00 00 01 00 00 00 01 00 00 00  // (...
0060: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
0070: 00 00 00 00 00 00 00 00 00 00 00 00 28 00 00 00  // (...
0080: 00 00 00 00 80 00 00 00 02 00 00 00 00 00 00 00  // 
0090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
00A0: 01 00 00 00 40 00 00 00 00 00 00 00 02 00 00 00  // @...
00B0: 03 00 00 00 00 00 00 00 10 27 00 00 00 00 00 00  // .'..
00C0: 00 00 00 00 01 00 00 00 00 00 00 00 01 00 00 00  // 
00D0: 02 00 00 00 01 00 02 00 03 00 02 00 01 00 03 00  // 
00E0: 01 00 00 00 40 00 00 00 00 03 00 00 02 00 00 00  // @...
00F0: 03 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00  // 
0100: 00 00 00 00 01 00 00 00 00 00 00 00 01 00 00 00  // 
0110: 02 00 00 00 0A 00 05 00 01 00 05 00 0A 00 01 00  // ....

Signed-off-by: Brice Goglin 
Reviewed-by: Jonathan Cameron 
---
 tests/data/acpi/q35/APIC.acpihmat-noinitiator | Bin 0 -> 144 bytes
 tests/data/acpi/q35/DSDT.acpihmat-noinitiator | Bin 0 -> 8553 bytes
 tests/data/acpi/q35/FACP.acpihmat-noinitiator | Bin 0 -> 244 bytes
 tests/data/acpi/q35/HMAT.acpihmat-noinitiator | Bin 0 -> 288 bytes
 tests/data/acpi/q35/SRAT.acpihmat-noinitiator | Bin 0 -> 312 bytes
 tests/qtest/bios-tables-test-allowed-diff.h   |   5 -
 6 files changed, 5 deletions(-)

diff --git a/tests/data/acpi/q35/APIC.acpihmat-noinitiator 
b/tests/data/acpi/q35/APIC.acpihmat-noinitiator
index 
e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..d904d4a70ddecbb79a83a267af8e26f925e9f4c6
 100644
GIT binary patch
literal 144
zcmZ<^@N}NQz`(%h?d0$55v<@85#X!<1dKp25F11@Fg*ANra6G>KwJ(+MhMNs1fiLk
tK{O)|Nb|r~o3y%?)O;u>A)b0RWi;3;_TD

literal 0
HcmV?d1

diff --git a/tests/data/acpi/q35/DSDT.acpihmat-noinitiator 
b/tests/data/acpi/q35/DSDT.acpihmat-noinitiator
index 
e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..c767d11cb1d088f613c49e55a7139cccababf66c
 100644
GIT binary patch
literal 8553
zcmb7JOKcm*8J^`sS}m8-lA^$0{LCIadERkMusn`Jpkg}Xsa#CcG
z#6TVhAj?4F_)s)q67)z13ea17>a_-XX>YzYKu;$gY0nd~tOIw46@kU+2F&
zd-_*jUVf)|@0b7l^_|zP1OTh}HSv2sq_5rwJ?l(w;C$BKGd?6bKesYi{H)JUi#CVO
zggt7xYl|jIwQ~4+=inu;RdjhD(|*%0xP=w;%WVajv)1X4ml?BZaOm^r;c)m@ambwa
znA0$Y%CcGW%WbLBfHqRq*{}KS2D2Hv|1iZ8otqBDi?5WMUfyqc-F)taZqP2WkXm{n
zHEqA*?*{?=OI4%Cddx7Y
z#x-^gzvp5|-#K`yacOhJT3FOrrjbzjgZa7)e2M&7S`SMnXB
zwC=94+|#~Wz8LE~Nzrf^?h+z=&)8#>qw_;*K4iIPir`E`QLd1_dJ(pl^JaW6Xv`1R
zqkG~0i~smf@cjFYPnW-KocNpD2r1VazwHg4>|bYt6a76ahAICcpEx$=>5v^};>Jz-
zVt1DZOa1HCY?w{PSek8zz1!Q29d4U`-L0^k-NmmF?o#CZ?p1?kk>h^L?_9st_Ookm
z?`Km9Oh>lfTq`qiY~Z-UvNp9_F$7f^#1Yh3Hl08}aO((M6cG!+d2x>O0F@yzk)LB@
zoC$0c5mQ2Aj57h{OoYTlZ#Xv2nX(6QMobBf3nrkPiIAAG3T`BU<#`gEDWM6$l$}SY
z>P%`nlbk7`Nx_tzN2uzY(R9vmri9MuIuWWmXEmL(n$B5WCqh-Hsp|ou;l6p{f&)
z9A%{EG@Wz0PK2sXOVep-IxSr%LRDu<)0xtArgWVMRh@YBDc951blSR3gsRT8rZcVS
zOzS!ksygR2o%5Q`d0i($Rp)}Hb3xO>U1=nj;7Pmbs|)CW;C4{O=m{eiBQ#<
z)pTYxompKcLRIIYrgKr#xv1+zsOrpVI&+%NoURk0s%xxuoe_(sd$Kbsp1n9@BIl
z({&h_lvrj=w_u}WS`n-#`zR!)
z7hz9o%##}Pq|QXBGEZsDQyTM>(PixH68uPTyM5r>K(3nqX%qMgvLX~-jGo{O%
z;Y|6AKO>m(!HtlZ@(P~SYM#|`0y;8Kgy@YlP=Spis;L^NfQ}3lA$r8I
zFi?SwqDUi$l7R}S3{{;lP?451VW0|>3>2YsMKVx?QYQ>lU^x>8szAv=1(eq$14Srx
z!axOlU^x>8szAv=1(Y+%KoLrvFi?TzOcu
z>1(q{mpbC@>R6seC3>2Z%2?G^a+#~P%=;f=3{*fllMEE0)CmI>Sk8oj
zDo`>|0p(0GP=rz^3{+q_69%e4$v_2^Gs!>^N}VuJf#pmXr~)Me6;RG314Srx!axO<
zGhv_#lnhirIg<<|Wj6M_lrzad5lWpf
zP=V!47^ngz0~JutBm+e#b;3XqDF%v2HBf}Afg)54RAItE6($*|!XyJ#m@rU<2?JG_
zWS|O@3{+vlKouqoRAG{VDoip^g$V;ym@rU1;Px-d{gxMF-T$v_d}
zGRTmaa%^Fsh~(J9KoQBYB?Cn$$CeBfA=ZXYc447xVE6f-)kFG2dUu9?ik+K3e=bh{
zQfaA!mNyK)wNZrQZj|WDr7zE9SZvuS({Y8qs`OPebBpy2tOg9`KCt$8v8pn6Xr*!&
z>Kkrr4uDo6GjF9nnPGE$>E}ImhuW+pzU$!!yVe4uBXpvLrIz(sydxCll824;8Cv}@
z(d{5}%v$_pp3TMKZVbVJjUA?4KFY)Rn^iQo`%1y6c>KrskyI4EJ`d*~niakbo
zS1IrE^6pr9_lWXtRNfmCZ(%{}d#m@9@*Xemjg|L~DDOq(%j4w}k5Rs?lrQu0<+1YR
zBg@|E%O$;T*PQOZ|%`N~-N$`R!`I*NkUscLidHL#C`RWnnt5Nydc=_4K
zC|^^`*LeBbSozu!YH+~yi89g1|
zH73%DwyEhR?#ry64lf`R=|tPqbQAX(mVoI#lfv*;GLcTSO-(m(-{kakcvYE5C)%c_
zo45~_^mKTCnMfzvrly;i*T?j9c)%c_o0zA|dOE!6Or#TSdOGKnvc|scf+0>HHfTy9YOaL_v||UBBEk+uZO-Tq8-!AFq%xo(dVJ=Fe4+xq;g>kC0)y
z}JY#6eg(75(hIorQxgzQ>q^g_1%Rm1*$zh`{0Vot5e0CUT)aH?B?}=f&l

[PATCH v3 3/4] tests: acpi: q35: add test for hmat nodes without initiators

2022-06-29 Thread Brice Goglin 
n List : 0001
[110h 0272   4] Target Proximity Domain List : 0002
[114h 0276   2]Entry : 000A
[116h 0278   2]Entry : 0005
[118h 0280   2]Entry : 0001
[11Ah 0282   2]Entry : 0005
[11Ch 0284   2]Entry : 000A
[11Eh 0286   2]Entry : 0001

Raw Table Data: Length 288 (0x120)

: 48 4D 41 54 20 01 00 00 02 4F 42 4F 43 48 53 20  // HMAT OBOCHS
0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43  // BXPCBXPC
0020: 01 00 00 00 00 00 00 00 00 00 00 00 28 00 00 00  // (...
0030: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
0040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
0050: 00 00 00 00 28 00 00 00 01 00 00 00 01 00 00 00  // (...
0060: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
0070: 00 00 00 00 00 00 00 00 00 00 00 00 28 00 00 00  // (...
0080: 00 00 00 00 80 00 00 00 02 00 00 00 00 00 00 00  // 
0090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
00A0: 01 00 00 00 40 00 00 00 00 00 00 00 02 00 00 00  // @...
00B0: 03 00 00 00 00 00 00 00 10 27 00 00 00 00 00 00  // .'..
00C0: 00 00 00 00 01 00 00 00 00 00 00 00 01 00 00 00  // 
00D0: 02 00 00 00 01 00 02 00 03 00 02 00 01 00 03 00  // 
00E0: 01 00 00 00 40 00 00 00 00 03 00 00 02 00 00 00  // @...
00F0: 03 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00  // 
0100: 00 00 00 00 01 00 00 00 00 00 00 00 01 00 00 00  // 
0110: 02 00 00 00 0A 00 05 00 01 00 05 00 0A 00 01 00  // ....

Signed-off-by: Brice Goglin 
Reviewed-by: Jonathan Cameron 
---
 tests/qtest/bios-tables-test.c | 45 ++
 1 file changed, 45 insertions(+)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 359916c228..1252b166ff 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1461,6 +1461,50 @@ static void test_acpi_piix4_tcg_acpi_hmat(void)
 test_acpi_tcg_acpi_hmat(MACHINE_PC);
 }
 
+static void test_acpi_q35_tcg_acpi_hmat_noinitiator(void)

+{
+test_data data;
+
+memset(, 0, sizeof(data));
+data.machine = MACHINE_Q35;
+data.variant = ".acpihmat-noinitiator";
+test_acpi_one(" -machine hmat=on"
+  " -smp 4"
+  " -m 128M"
+  " -object memory-backend-ram,size=32M,id=ram0"
+  " -object memory-backend-ram,size=32M,id=ram1"
+  " -object memory-backend-ram,size=64M,id=ram2"
+  " -numa node,nodeid=0,memdev=ram0,cpus=0-1"
+  " -numa node,nodeid=1,memdev=ram1,cpus=2-3"
+  " -numa node,nodeid=2,memdev=ram2"
+  " -numa hmat-lb,initiator=0,target=0,hierarchy=memory,"
+  "data-type=access-latency,latency=10"
+  " -numa hmat-lb,initiator=0,target=0,hierarchy=memory,"
+  "data-type=access-bandwidth,bandwidth=10485760"
+  " -numa hmat-lb,initiator=0,target=1,hierarchy=memory,"
+  "data-type=access-latency,latency=20"
+  " -numa hmat-lb,initiator=0,target=1,hierarchy=memory,"
+  "data-type=access-bandwidth,bandwidth=5242880"
+  " -numa hmat-lb,initiator=0,target=2,hierarchy=memory,"
+  "data-type=access-latency,latency=30"
+  " -numa hmat-lb,initiator=0,target=2,hierarchy=memory,"
+  "data-type=access-bandwidth,bandwidth=1048576"
+  " -numa hmat-lb,initiator=1,target=0,hierarchy=memory,"
+  "data-type=access-latency,latency=20"
+  " -numa hmat-lb,initiator=1,target=0,hierarchy=memory,"
+  "data-type=access-bandwidth,bandwidth=5242880"
+  " -numa hmat-lb,initiator=1,target=1,hierarchy=memory,"
+  "data-type=access-latency,latency=10"
+  " -numa hmat-lb,initiator=1,target=1,hierarchy=memory,"
+  "data-type=access-bandwidth,bandwidth=10485760"
+  " -numa hmat-lb,initiator=1,target=2,hierarchy=memory,"
+  "data-type=access-latency,latency=30"
+  " -numa hmat-lb,initiator=1,target=2,hierarchy=memory,"
+  "data-type=access-bandwidth,bandwidth=1048576",
+  );
+f

[PATCH v3 2/4] tests: acpi: add and whitelist *.hmat-noinitiator expected blobs

2022-06-29 Thread Brice Goglin 

.. which will be used by follow up hmat-noinitiator test-case.

Signed-off-by: Brice Goglin 
Reviewed-by: Jonathan Cameron 
---
 tests/data/acpi/q35/APIC.acpihmat-noinitiator | 0
 tests/data/acpi/q35/DSDT.acpihmat-noinitiator | 0
 tests/data/acpi/q35/FACP.acpihmat-noinitiator | 0
 tests/data/acpi/q35/HMAT.acpihmat-noinitiator | 0
 tests/data/acpi/q35/SRAT.acpihmat-noinitiator | 0
 tests/qtest/bios-tables-test-allowed-diff.h   | 5 +
 6 files changed, 5 insertions(+)
 create mode 100644 tests/data/acpi/q35/APIC.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/DSDT.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/FACP.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/HMAT.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/SRAT.acpihmat-noinitiator

diff --git a/tests/data/acpi/q35/APIC.acpihmat-noinitiator 
b/tests/data/acpi/q35/APIC.acpihmat-noinitiator
new file mode 100644
index 00..e69de29bb2
diff --git a/tests/data/acpi/q35/DSDT.acpihmat-noinitiator 
b/tests/data/acpi/q35/DSDT.acpihmat-noinitiator
new file mode 100644
index 00..e69de29bb2
diff --git a/tests/data/acpi/q35/FACP.acpihmat-noinitiator 
b/tests/data/acpi/q35/FACP.acpihmat-noinitiator
new file mode 100644
index 00..e69de29bb2
diff --git a/tests/data/acpi/q35/HMAT.acpihmat-noinitiator 
b/tests/data/acpi/q35/HMAT.acpihmat-noinitiator
new file mode 100644
index 00..e69de29bb2
diff --git a/tests/data/acpi/q35/SRAT.acpihmat-noinitiator 
b/tests/data/acpi/q35/SRAT.acpihmat-noinitiator
new file mode 100644
index 00..e69de29bb2
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h 
b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..ae025e3a3e 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,6 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/q35/APIC.acpihmat-noinitiator",
+"tests/data/acpi/q35/DSDT.acpihmat-noinitiator",
+"tests/data/acpi/q35/FACP.acpihmat-noinitiator",
+"tests/data/acpi/q35/HMAT.acpihmat-noinitiator",
+"tests/data/acpi/q35/SRAT.acpihmat-noinitiator",
--
2.30.2





OpenPGP_signature
Description: OpenPGP digital signature

[PATCH v3 0/4] hmat acpi: Don't require initiator value in -numa

2022-06-29 Thread Brice Goglin 

Allow -numa without initiator value when hmat=on so that we may
build more complex topologies, e.g. NUMA nodes whose best initiators
are not just another single node.

changes v2->v3:
* improve messages for patches 0/4 and 3/4
changes v1->v2:
* add q35 acpi test

Brice Goglin (4):
  hmat acpi: Don't require initiator value in -numa
  tests: acpi: add and whitelist *.hmat-noinitiator expected blobs
  tests: acpi: q35: add test for hmat nodes without initiators
  tests: acpi: q35: update expected blobs *.hmat-noinitiators

 hw/core/machine.c |   4 +-
 tests/data/acpi/q35/APIC.acpihmat-noinitiator | Bin 0 -> 144 bytes
 tests/data/acpi/q35/DSDT.acpihmat-noinitiator | Bin 0 -> 8553 bytes
 tests/data/acpi/q35/FACP.acpihmat-noinitiator | Bin 0 -> 244 bytes
 tests/data/acpi/q35/HMAT.acpihmat-noinitiator | Bin 0 -> 288 bytes
 tests/data/acpi/q35/SRAT.acpihmat-noinitiator | Bin 0 -> 312 bytes
 tests/qtest/bios-tables-test.c|  45 ++
 7 files changed, 46 insertions(+), 3 deletions(-)
 create mode 100644 tests/data/acpi/q35/APIC.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/DSDT.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/FACP.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/HMAT.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/SRAT.acpihmat-noinitiator

--
2.30.2

Re: [PATCH 1/4] hmat acpi: Don't require initiator value in -numa

2022-06-28 Thread Brice Goglin 

Le 28/06/2022 à 16:19, Igor Mammedov a écrit :

On Thu, 23 Jun 2022 16:58:28 +0200
Brice Goglin  wrote:


The "Memory Proximity Domain Attributes" structure of the ACPI HMAT
has a "Processor Proximity Domain Valid" flag that is currently
always set because Qemu -numa requires an initiator=X value
when hmat=on. Unsetting this flag allows to create more complex
memory topologies by having multiple best initiators for a single
memory target.

This patch allows -numa without initiator=X when hmat=on by keeping
the default value MAX_NODES in numa_state->nodes[i].initiator.
All places reading numa_state->nodes[i].initiator already check
whether it's different from MAX_NODES before using it.

Tested with
qemu-system-x86_64 -accel kvm \
   -machine pc,hmat=on \
   -drive if=pflash,format=raw,file=./OVMF.fd \
   -drive media=disk,format=qcow2,file=efi.qcow2 \
   -smp 4 \
   -m 3G \
   -object memory-backend-ram,size=1G,id=ram0 \
   -object memory-backend-ram,size=1G,id=ram1 \
   -object memory-backend-ram,size=1G,id=ram2 \
   -numa node,nodeid=0,memdev=ram0,cpus=0-1 \
   -numa node,nodeid=1,memdev=ram1,cpus=2-3 \
   -numa node,nodeid=2,memdev=ram2 \
   -numa 
hmat-lb,initiator=0,target=0,hierarchy=memory,data-type=access-latency,latency=10
 \
   -numa 
hmat-lb,initiator=0,target=0,hierarchy=memory,data-type=access-bandwidth,bandwidth=10485760
 \
   -numa 
hmat-lb,initiator=0,target=1,hierarchy=memory,data-type=access-latency,latency=20
 \
   -numa 
hmat-lb,initiator=0,target=1,hierarchy=memory,data-type=access-bandwidth,bandwidth=5242880
 \
   -numa 
hmat-lb,initiator=0,target=2,hierarchy=memory,data-type=access-latency,latency=30
 \
   -numa 
hmat-lb,initiator=0,target=2,hierarchy=memory,data-type=access-bandwidth,bandwidth=1048576
 \
   -numa 
hmat-lb,initiator=1,target=0,hierarchy=memory,data-type=access-latency,latency=20
 \
   -numa 
hmat-lb,initiator=1,target=0,hierarchy=memory,data-type=access-bandwidth,bandwidth=5242880
 \
   -numa 
hmat-lb,initiator=1,target=1,hierarchy=memory,data-type=access-latency,latency=10
 \
   -numa 
hmat-lb,initiator=1,target=1,hierarchy=memory,data-type=access-bandwidth,bandwidth=10485760
 \
   -numa 
hmat-lb,initiator=1,target=2,hierarchy=memory,data-type=access-latency,latency=30
 \
   -numa 
hmat-lb,initiator=1,target=2,hierarchy=memory,data-type=access-bandwidth,bandwidth=1048576
which reports NUMA node2 at same distance from both node0 and node1 as seen in 
lstopo:
Machine (2966MB total) + Package P#0
NUMANode P#2 (979MB)
Group0
  NUMANode P#0 (980MB)
  Core P#0 + PU P#0
  Core P#1 + PU P#1
Group0
  NUMANode P#1 (1007MB)
  Core P#2 + PU P#2
  Core P#3 + PU P#3

Before this patch, we had to add ",initiator=X" to "-numa 
node,nodeid=2,memdev=ram2".
The lstopo output difference between initiator=1 and no initiator is:
@@ -1,10 +1,10 @@
   Machine (2966MB total) + Package P#0
+  NUMANode P#2 (979MB)
 Group0
   NUMANode P#0 (980MB)
   Core P#0 + PU P#0
   Core P#1 + PU P#1
 Group0
   NUMANode P#1 (1007MB)
-NUMANode P#2 (979MB)
   Core P#2 + PU P#2
   Core P#3 + PU P#3

Corresponding changes in the HMAT MPDA structure:
@@ -49,10 +49,10 @@
   [078h 0120   2]   Structure Type :  [Memory Proximity Domain 
Attributes]
   [07Ah 0122   2] Reserved : 
   [07Ch 0124   4]   Length : 0028
-[080h 0128   2]Flags (decoded below) : 0001
-Processor Proximity Domain Valid : 1
+[080h 0128   2]Flags (decoded below) : 
+Processor Proximity Domain Valid : 0
   [082h 0130   2]Reserved1 : 
-[084h 0132   4] Attached Initiator Proximity Domain : 0001
+[084h 0132   4] Attached Initiator Proximity Domain : 0080

  
where does this value come from?



This is #define MAX_NODES 128, the default value for initiator field in 
Qemu. But it's meaningless here because "Processor Proximity Domain 
Valid" flag above is 0.


Brice





OpenPGP_signature
Description: OpenPGP digital signature

[PATCH 4/4] tests: acpi: q35: update expected blobs *.hmat-noinitiators

2022-06-23 Thread Brice Goglin 
 Entry : 0005
[11Ch 0284   2]Entry : 000A
[11Eh 0286   2]Entry : 0001

Raw Table Data: Length 288 (0x120)

: 48 4D 41 54 20 01 00 00 02 4F 42 4F 43 48 53 20  // HMAT OBOCHS
0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43  // BXPCBXPC
0020: 01 00 00 00 00 00 00 00 00 00 00 00 28 00 00 00  // (...
0030: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
0040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
0050: 00 00 00 00 28 00 00 00 01 00 00 00 01 00 00 00  // (...
0060: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
0070: 00 00 00 00 00 00 00 00 00 00 00 00 28 00 00 00  // (...
0080: 00 00 00 00 80 00 00 00 02 00 00 00 00 00 00 00  // 
0090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
00A0: 01 00 00 00 40 00 00 00 00 00 00 00 02 00 00 00  // @...
00B0: 03 00 00 00 00 00 00 00 10 27 00 00 00 00 00 00  // .'..
00C0: 00 00 00 00 01 00 00 00 00 00 00 00 01 00 00 00  // 
00D0: 02 00 00 00 01 00 02 00 03 00 02 00 01 00 03 00  // 
00E0: 01 00 00 00 40 00 00 00 00 03 00 00 02 00 00 00  // @...
00F0: 03 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00  // 
0100: 00 00 00 00 01 00 00 00 00 00 00 00 01 00 00 00  // 
0110: 02 00 00 00 0A 00 05 00 01 00 05 00 0A 00 01 00  // ....

Signed-off-by: Brice Goglin 
---
 tests/data/acpi/q35/APIC.acpihmat-noinitiator | Bin 0 -> 144 bytes
 tests/data/acpi/q35/DSDT.acpihmat-noinitiator | Bin 0 -> 8553 bytes
 tests/data/acpi/q35/FACP.acpihmat-noinitiator | Bin 0 -> 244 bytes
 tests/data/acpi/q35/HMAT.acpihmat-noinitiator | Bin 0 -> 288 bytes
 tests/data/acpi/q35/SRAT.acpihmat-noinitiator | Bin 0 -> 312 bytes
 tests/qtest/bios-tables-test-allowed-diff.h   |   5 -
 6 files changed, 5 deletions(-)

diff --git a/tests/data/acpi/q35/APIC.acpihmat-noinitiator 
b/tests/data/acpi/q35/APIC.acpihmat-noinitiator
index 
e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..d904d4a70ddecbb79a83a267af8e26f925e9f4c6
 100644
GIT binary patch
literal 144
zcmZ<^@N}NQz`(%h?d0$55v<@85#X!<1dKp25F11@Fg*ANra6G>KwJ(+MhMNs1fiLk
tK{O)|Nb|r~o3y%?)O;u>A)b0RWi;3;_TD

literal 0
HcmV?d1

diff --git a/tests/data/acpi/q35/DSDT.acpihmat-noinitiator 
b/tests/data/acpi/q35/DSDT.acpihmat-noinitiator
index 
e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..c767d11cb1d088f613c49e55a7139cccababf66c
 100644
GIT binary patch
literal 8553
zcmb7JOKcm*8J^`sS}m8-lA^$0{LCIadERkMusn`Jpkg}Xsa#CcG
z#6TVhAj?4F_)s)q67)z13ea17>a_-XX>YzYKu;$gY0nd~tOIw46@kU+2F&
zd-_*jUVf)|@0b7l^_|zP1OTh}HSv2sq_5rwJ?l(w;C$BKGd?6bKesYi{H)JUi#CVO
zggt7xYl|jIwQ~4+=inu;RdjhD(|*%0xP=w;%WVajv)1X4ml?BZaOm^r;c)m@ambwa
znA0$Y%CcGW%WbLBfHqRq*{}KS2D2Hv|1iZ8otqBDi?5WMUfyqc-F)taZqP2WkXm{n
zHEqA*?*{?=OI4%Cddx7Y
z#x-^gzvp5|-#K`yacOhJT3FOrrjbzjgZa7)e2M&7S`SMnXB
zwC=94+|#~Wz8LE~Nzrf^?h+z=&)8#>qw_;*K4iIPir`E`QLd1_dJ(pl^JaW6Xv`1R
zqkG~0i~smf@cjFYPnW-KocNpD2r1VazwHg4>|bYt6a76ahAICcpEx$=>5v^};>Jz-
zVt1DZOa1HCY?w{PSek8zz1!Q29d4U`-L0^k-NmmF?o#CZ?p1?kk>h^L?_9st_Ookm
z?`Km9Oh>lfTq`qiY~Z-UvNp9_F$7f^#1Yh3Hl08}aO((M6cG!+d2x>O0F@yzk)LB@
zoC$0c5mQ2Aj57h{OoYTlZ#Xv2nX(6QMobBf3nrkPiIAAG3T`BU<#`gEDWM6$l$}SY
z>P%`nlbk7`Nx_tzN2uzY(R9vmri9MuIuWWmXEmL(n$B5WCqh-Hsp|ou;l6p{f&)
z9A%{EG@Wz0PK2sXOVep-IxSr%LRDu<)0xtArgWVMRh@YBDc951blSR3gsRT8rZcVS
zOzS!ksygR2o%5Q`d0i($Rp)}Hb3xO>U1=nj;7Pmbs|)CW;C4{O=m{eiBQ#<
z)pTYxompKcLRIIYrgKr#xv1+zsOrpVI&+%NoURk0s%xxuoe_(sd$Kbsp1n9@BIl
z({&h_lvrj=w_u}WS`n-#`zR!)
z7hz9o%##}Pq|QXBGEZsDQyTM>(PixH68uPTyM5r>K(3nqX%qMgvLX~-jGo{O%
z;Y|6AKO>m(!HtlZ@(P~SYM#|`0y;8Kgy@YlP=Spis;L^NfQ}3lA$r8I
zFi?SwqDUi$l7R}S3{{;lP?451VW0|>3>2YsMKVx?QYQ>lU^x>8szAv=1(eq$14Srx
z!axOlU^x>8szAv=1(Y+%KoLrvFi?TzOcu
z>1(q{mpbC@>R6seC3>2Z%2?G^a+#~P%=;f=3{*fllMEE0)CmI>Sk8oj
zDo`>|0p(0GP=rz^3{+q_69%e4$v_2^Gs!>^N}VuJf#pmXr~)Me6;RG314Srx!axO<
zGhv_#lnhirIg<<|Wj6M_lrzad5lWpf
zP=V!47^ngz0~JutBm+e#b;3XqDF%v2HBf}Afg)54RAItE6($*|!XyJ#m@rU<2?JG_
zWS|O@3{+vlKouqoRAG{VDoip^g$V;ym@rU1;Px-d{gxMF-T$v_d}
zGRTmaa%^Fsh~(J9KoQBYB?Cn$$CeBfA=ZXYc447xVE6f-)kFG2dUu9?ik+K3e=bh{
zQfaA!mNyK)wNZrQZj|WDr7zE9SZvuS({Y8qs`OPebBpy2tOg9`KCt$8v8pn6Xr*!&
z>Kkrr4uDo6GjF9nnPGE$>E}ImhuW+pzU$!!yVe4uBXpvLrIz(sydxCll824;8Cv}@
z(d{5}%v$_pp3TMKZVbVJjUA?4KFY)Rn^iQo`%1y6c>KrskyI4EJ`d*~niakbo
zS1IrE^6pr9_lWXtRNfmCZ(%{}d#m@9@*Xemjg|L~DDOq(%j4w}k5Rs?lrQu0<+1YR
zBg@|E%O$;T*PQOZ|%`N~-N$`R!`I*NkUscLidHL#C`RWnnt5Nydc=_4K
zC|^^`*LeBbSozu!YH+~yi89g1|
zH73%DwyEhR?#ry64lf`R=|tPqbQAX(mVoI#lfv*;GLcTSO-(m(-{kakcvYE5C)%c_
zo45~_^mKTCnMfzvrly;i*T?j9c)%c_o0zA|dOE!6Or#TSdOGKnvc|scf+0>HHfTy9YOaL_v||UBBEk+uZO-Tq8-!AFq%xo(dVJ=Fe4+xq;g>kC0)y
z}JY#6eg(75(hIorQxgzQ>q^g_1%Rm1*$zh`{0Vot5e0CUT)aH?B?}=f<3SDQMeAjPvyAGLavH7w%0No6w4l
zw-

[PATCH 2/4] tests: acpi: add and whitelist *.hmat-noinitiator expected blobs

2022-06-23 Thread Brice Goglin 

.. which will be used by follow up hmat-noinitiator test-case.

Signed-off-by: Brice Goglin 
---
 tests/data/acpi/q35/APIC.acpihmat-noinitiator | 0
 tests/data/acpi/q35/DSDT.acpihmat-noinitiator | 0
 tests/data/acpi/q35/FACP.acpihmat-noinitiator | 0
 tests/data/acpi/q35/HMAT.acpihmat-noinitiator | 0
 tests/data/acpi/q35/SRAT.acpihmat-noinitiator | 0
 tests/qtest/bios-tables-test-allowed-diff.h   | 5 +
 6 files changed, 5 insertions(+)
 create mode 100644 tests/data/acpi/q35/APIC.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/DSDT.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/FACP.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/HMAT.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/SRAT.acpihmat-noinitiator

diff --git a/tests/data/acpi/q35/APIC.acpihmat-noinitiator 
b/tests/data/acpi/q35/APIC.acpihmat-noinitiator
new file mode 100644
index 00..e69de29bb2
diff --git a/tests/data/acpi/q35/DSDT.acpihmat-noinitiator 
b/tests/data/acpi/q35/DSDT.acpihmat-noinitiator
new file mode 100644
index 00..e69de29bb2
diff --git a/tests/data/acpi/q35/FACP.acpihmat-noinitiator 
b/tests/data/acpi/q35/FACP.acpihmat-noinitiator
new file mode 100644
index 00..e69de29bb2
diff --git a/tests/data/acpi/q35/HMAT.acpihmat-noinitiator 
b/tests/data/acpi/q35/HMAT.acpihmat-noinitiator
new file mode 100644
index 00..e69de29bb2
diff --git a/tests/data/acpi/q35/SRAT.acpihmat-noinitiator 
b/tests/data/acpi/q35/SRAT.acpihmat-noinitiator
new file mode 100644
index 00..e69de29bb2
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h 
b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..ae025e3a3e 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,6 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/q35/APIC.acpihmat-noinitiator",
+"tests/data/acpi/q35/DSDT.acpihmat-noinitiator",
+"tests/data/acpi/q35/FACP.acpihmat-noinitiator",
+"tests/data/acpi/q35/HMAT.acpihmat-noinitiator",
+"tests/data/acpi/q35/SRAT.acpihmat-noinitiator",
--
2.30.2





OpenPGP_signature
Description: OpenPGP digital signature

[PATCH 3/4] tests: acpi: q35: add test for hmat nodes without initiators

2022-06-23 Thread Brice Goglin 
 Entry : 0005
[11Ch 0284   2]Entry : 000A
[11Eh 0286   2]Entry : 0001

Raw Table Data: Length 288 (0x120)

: 48 4D 41 54 20 01 00 00 02 4F 42 4F 43 48 53 20  // HMAT OBOCHS
0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43  // BXPCBXPC
0020: 01 00 00 00 00 00 00 00 00 00 00 00 28 00 00 00  // (...
0030: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
0040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
0050: 00 00 00 00 28 00 00 00 01 00 00 00 01 00 00 00  // (...
0060: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
0070: 00 00 00 00 00 00 00 00 00 00 00 00 28 00 00 00  // (...
0080: 00 00 00 00 80 00 00 00 02 00 00 00 00 00 00 00  // 
0090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // 
00A0: 01 00 00 00 40 00 00 00 00 00 00 00 02 00 00 00  // @...
00B0: 03 00 00 00 00 00 00 00 10 27 00 00 00 00 00 00  // .'..
00C0: 00 00 00 00 01 00 00 00 00 00 00 00 01 00 00 00  // 
00D0: 02 00 00 00 01 00 02 00 03 00 02 00 01 00 03 00  // 
00E0: 01 00 00 00 40 00 00 00 00 03 00 00 02 00 00 00  // @...
00F0: 03 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00  // 
0100: 00 00 00 00 01 00 00 00 00 00 00 00 01 00 00 00  // 
0110: 02 00 00 00 0A 00 05 00 01 00 05 00 0A 00 01 00  // ....

Signed-off-by: Brice Goglin 
---
 tests/qtest/bios-tables-test.c | 45 ++
 1 file changed, 45 insertions(+)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 359916c228..1252b166ff 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1461,6 +1461,50 @@ static void test_acpi_piix4_tcg_acpi_hmat(void)
 test_acpi_tcg_acpi_hmat(MACHINE_PC);
 }
 
+static void test_acpi_q35_tcg_acpi_hmat_noinitiator(void)

+{
+test_data data;
+
+memset(, 0, sizeof(data));
+data.machine = MACHINE_Q35;
+data.variant = ".acpihmat-noinitiator";
+test_acpi_one(" -machine hmat=on"
+  " -smp 4"
+  " -m 128M"
+  " -object memory-backend-ram,size=32M,id=ram0"
+  " -object memory-backend-ram,size=32M,id=ram1"
+  " -object memory-backend-ram,size=64M,id=ram2"
+  " -numa node,nodeid=0,memdev=ram0,cpus=0-1"
+  " -numa node,nodeid=1,memdev=ram1,cpus=2-3"
+  " -numa node,nodeid=2,memdev=ram2"
+  " -numa hmat-lb,initiator=0,target=0,hierarchy=memory,"
+  "data-type=access-latency,latency=10"
+  " -numa hmat-lb,initiator=0,target=0,hierarchy=memory,"
+  "data-type=access-bandwidth,bandwidth=10485760"
+  " -numa hmat-lb,initiator=0,target=1,hierarchy=memory,"
+  "data-type=access-latency,latency=20"
+  " -numa hmat-lb,initiator=0,target=1,hierarchy=memory,"
+  "data-type=access-bandwidth,bandwidth=5242880"
+  " -numa hmat-lb,initiator=0,target=2,hierarchy=memory,"
+  "data-type=access-latency,latency=30"
+  " -numa hmat-lb,initiator=0,target=2,hierarchy=memory,"
+  "data-type=access-bandwidth,bandwidth=1048576"
+  " -numa hmat-lb,initiator=1,target=0,hierarchy=memory,"
+  "data-type=access-latency,latency=20"
+  " -numa hmat-lb,initiator=1,target=0,hierarchy=memory,"
+  "data-type=access-bandwidth,bandwidth=5242880"
+  " -numa hmat-lb,initiator=1,target=1,hierarchy=memory,"
+  "data-type=access-latency,latency=10"
+  " -numa hmat-lb,initiator=1,target=1,hierarchy=memory,"
+  "data-type=access-bandwidth,bandwidth=10485760"
+  " -numa hmat-lb,initiator=1,target=2,hierarchy=memory,"
+  "data-type=access-latency,latency=30"
+  " -numa hmat-lb,initiator=1,target=2,hierarchy=memory,"
+  "data-type=access-bandwidth,bandwidth=1048576",
+  );
+free_test_data();
+}
+
 static void test_acpi_erst(const char *machine)
 {
 gchar *tmp_path = g_dir_make_tmp("qemu-test-erst.XX", NULL);
@@ -1803,6 +1847,7 @@ int main(int argc, char *argv[])
 qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm

[PATCH 1/4] hmat acpi: Don't require initiator value in -numa

2022-06-23 Thread Brice Goglin 
 0220   2]Entry : 0001
[0DEh 0222   2]Entry : 0003

[0E0h 0224   2]   Structure Type : 0001 [System Locality Latency 
and Bandwidth Information]
[0E2h 0226   2] Reserved : 
[0E4h 0228   4]   Length : 0040
[0E8h 0232   1]Flags (decoded below) : 00
Memory Hierarchy : 0
[0E9h 0233   1]Data Type : 03
[0EAh 0234   2]Reserved1 : 
[0ECh 0236   4] Initiator Proximity Domains # : 0002
[0F0h 0240   4]   Target Proximity Domains # : 0003
[0F4h 0244   4]Reserved2 : 
[0F8h 0248   8]  Entry Base Unit : 0001
[100h 0256   4] Initiator Proximity Domain List : 
[104h 0260   4] Initiator Proximity Domain List : 0001
[108h 0264   4] Target Proximity Domain List : 
[10Ch 0268   4] Target Proximity Domain List : 0001
[110h 0272   4] Target Proximity Domain List : 0002
[114h 0276   2]Entry : 000A
[116h 0278   2]Entry : 0005
[118h 0280   2]Entry : 0001
[11Ah 0282   2]Entry : 0005
[11Ch 0284   2]Entry : 000A
[11Eh 0286   2]        Entry : 0001

Signed-off-by: Brice Goglin 
---
 hw/core/machine.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index a673302cce..d4d7e77401 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1173,9 +1173,7 @@ static void numa_validate_initiator(NumaState *numa_state)
 
 for (i = 0; i < numa_state->num_nodes; i++) {

 if (numa_info[i].initiator == MAX_NODES) {
-error_report("The initiator of NUMA node %d is missing, use "
- "'-numa node,initiator' option to declare it", i);
-exit(1);
+continue;
 }
 
 if (!numa_info[numa_info[i].initiator].present) {

--
2.30.2





OpenPGP_signature
Description: OpenPGP digital signature

[PATCH 0/4] hmat acpi: Don't require initiator value in -numa

2022-06-23 Thread Brice Goglin 

Brice Goglin (4):
  hmat acpi: Don't require initiator value in -numa
  tests: acpi: add and whitelist *.hmat-noinitiator expected blobs
  tests: acpi: q35: add test for hmat nodes without initiators
  tests: acpi: q35: update expected blobs *.hmat-noinitiators

 hw/core/machine.c |   4 +-
 tests/data/acpi/q35/APIC.acpihmat-noinitiator | Bin 0 -> 144 bytes
 tests/data/acpi/q35/DSDT.acpihmat-noinitiator | Bin 0 -> 8553 bytes
 tests/data/acpi/q35/FACP.acpihmat-noinitiator | Bin 0 -> 244 bytes
 tests/data/acpi/q35/HMAT.acpihmat-noinitiator | Bin 0 -> 288 bytes
 tests/data/acpi/q35/SRAT.acpihmat-noinitiator | Bin 0 -> 312 bytes
 tests/qtest/bios-tables-test.c|  45 ++
 7 files changed, 46 insertions(+), 3 deletions(-)
 create mode 100644 tests/data/acpi/q35/APIC.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/DSDT.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/FACP.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/HMAT.acpihmat-noinitiator
 create mode 100644 tests/data/acpi/q35/SRAT.acpihmat-noinitiator

--
2.30.2





OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH] hmat acpi: Don't require initiator value in -numa when hmat=on

2022-06-21 Thread Brice Goglin 


Le 20/06/2022 à 18:05, Igor Mammedov a écrit :

On Mon, 20 Jun 2022 17:24:18 +0200
Brice Goglin  wrote:


Le 20/06/2022 à 15:27, Igor Mammedov a écrit Machine (2966MB total) +
Package P#0

 NUMANode P#2 (979MB)
 Group0
   NUMANode P#0 (980MB)
   Core P#0 + PU P#0
   Core P#1 + PU P#1
 Group0
   NUMANode P#1 (1007MB)
   Core P#2 + PU P#2
   Core P#3 + PU P#3

here should be a dis-assembled dump of generated HMAT table


Hello

Like what I added at the end of
https://github.com/bgoglin/qemu/commit/d9b3f5cb1514adafa644afcc2a363f2dc9795a32
?

yep, only full version including headers.



Thanks,

I split the patch adding the new test in 3 patches, reduced memory to 
128M total, etc.


I am not sure I understood what I am supposed to put in commit messages. 
Can you check the 4 patches on top of 
https://github.com/bgoglin/qemu/commits/hmat-noinitiator before I resend 
them to the list?


Brice





OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH] hmat acpi: Don't require initiator value in -numa when hmat=on

2022-06-20 Thread Brice Goglin 
Le 20/06/2022 à 15:27, Igor Mammedov a écrit Machine (2966MB total) + 
Package P#0

NUMANode P#2 (979MB)
Group0
  NUMANode P#0 (980MB)
  Core P#0 + PU P#0
  Core P#1 + PU P#1
Group0
  NUMANode P#1 (1007MB)
  Core P#2 + PU P#2
  Core P#3 + PU P#3

here should be a dis-assembled dump of generated HMAT table



Hello

Like what I added at the end of 
https://github.com/bgoglin/qemu/commit/d9b3f5cb1514adafa644afcc2a363f2dc9795a32 
?




+ a test case, see tests/qtest/bios-tables-test.c
for the process (at tho top) and test examples



https://github.com/bgoglin/qemu/commit/643dfa2de8b3e1f5b5675825e5d1be5c93a9549c

This passes make check V=1 but I am really not sure about what I did. 
The doc is far from easy for new contributors. Only HMAT matters here, 
but it looks like it wanted some other tables too. Also I don't know 
about pc vs piix4 vs q35, what "tcg" is, etc.


Advices appreciated.


How are we supposed to send patches that contain binary changes?

Brice





OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH] hmat acpi: Don't require initiator value in -numa when hmat=on

2022-06-17 Thread Brice Goglin 

Hello

Could somebody please apply (or reject) this commit?

Thanks

Brice


Le 06/04/2022 à 14:29, Brice Goglin a écrit :

From: Brice Goglin

The "Memory Proximity Domain Attributes" structure of the ACPI HMAT
has a "Processor Proximity Domain Valid" flag that is currently
always set because Qemu -numa requires initiator=X when hmat=on.

Unsetting this flag allows to create more complex memory topologies
by having multiple best initiators for a single memory target.

This patch allows -numa with initiator=X when hmat=on by keeping
the default value MAX_NODES in numa_state->nodes[i].initiator.
All places reading numa_state->nodes[i].initiator already check
whether it's different from MAX_NODES before using it. And
hmat_build_table_structs() already unset the Valid flag when needed.

Tested with
qemu-system-x86_64 -accel kvm \
  -machine pc,hmat=on \
  -drive if=pflash,format=raw,file=./OVMF.fd \
  -drive media=disk,format=qcow2,file=efi.qcow2 \
  -smp 4 \
  -m 3G \
  -object memory-backend-ram,size=1G,id=ram0 \
  -object memory-backend-ram,size=1G,id=ram1 \
  -object memory-backend-ram,size=1G,id=ram2 \
  -numa node,nodeid=0,memdev=ram0,cpus=0-1 \
  -numa node,nodeid=1,memdev=ram1,cpus=2-3 \
  -numa node,nodeid=2,memdev=ram2 \
  -numa 
hmat-lb,initiator=0,target=0,hierarchy=memory,data-type=access-latency,latency=10
 \
  -numa 
hmat-lb,initiator=0,target=0,hierarchy=memory,data-type=access-bandwidth,bandwidth=10485760
 \
  -numa 
hmat-lb,initiator=0,target=1,hierarchy=memory,data-type=access-latency,latency=20
 \
  -numa 
hmat-lb,initiator=0,target=1,hierarchy=memory,data-type=access-bandwidth,bandwidth=5242880
 \
  -numa 
hmat-lb,initiator=0,target=2,hierarchy=memory,data-type=access-latency,latency=30
 \
  -numa 
hmat-lb,initiator=0,target=2,hierarchy=memory,data-type=access-bandwidth,bandwidth=1048576
 \
  -numa 
hmat-lb,initiator=1,target=0,hierarchy=memory,data-type=access-latency,latency=20
 \
  -numa 
hmat-lb,initiator=1,target=0,hierarchy=memory,data-type=access-bandwidth,bandwidth=5242880
 \
  -numa 
hmat-lb,initiator=1,target=1,hierarchy=memory,data-type=access-latency,latency=10
 \
  -numa 
hmat-lb,initiator=1,target=1,hierarchy=memory,data-type=access-bandwidth,bandwidth=10485760
 \
  -numa 
hmat-lb,initiator=1,target=2,hierarchy=memory,data-type=access-latency,latency=30
 \
  -numa 
hmat-lb,initiator=1,target=2,hierarchy=memory,data-type=access-bandwidth,bandwidth=1048576
 \

This exposes NUMA node2 at same distance from both node0 and node1 as seen in 
lstopo:

Machine (2966MB total) + Package P#0
   NUMANode P#2 (979MB)
   Group0
 NUMANode P#0 (980MB)
 Core P#0 + PU P#0
 Core P#1 + PU P#1
   Group0
 NUMANode P#1 (1007MB)
 Core P#2 + PU P#2
 Core P#3 + PU P#3

Signed-off-by: Brice Goglin
---
  hw/core/machine.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index d856485cb4..9884ef7ac6 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1012,9 +1012,7 @@ static void numa_validate_initiator(NumaState *numa_state)
  
  for (i = 0; i < numa_state->num_nodes; i++) {

  if (numa_info[i].initiator == MAX_NODES) {
-error_report("The initiator of NUMA node %d is missing, use "
- "'-numa node,initiator' option to declare it", i);
-exit(1);
+continue;
  }
  
  if (!numa_info[numa_info[i].initiator].present) {

--
2.30.2






OpenPGP_signature
Description: OpenPGP digital signature

[PATCH] hmat acpi: Don't require initiator value in -numa when hmat=on

2022-04-06 Thread Brice Goglin 

From: Brice Goglin 

The "Memory Proximity Domain Attributes" structure of the ACPI HMAT
has a "Processor Proximity Domain Valid" flag that is currently
always set because Qemu -numa requires initiator=X when hmat=on.

Unsetting this flag allows to create more complex memory topologies
by having multiple best initiators for a single memory target.

This patch allows -numa with initiator=X when hmat=on by keeping
the default value MAX_NODES in numa_state->nodes[i].initiator.
All places reading numa_state->nodes[i].initiator already check
whether it's different from MAX_NODES before using it. And
hmat_build_table_structs() already unset the Valid flag when needed.

Tested with
qemu-system-x86_64 -accel kvm \
 -machine pc,hmat=on \
 -drive if=pflash,format=raw,file=./OVMF.fd \
 -drive media=disk,format=qcow2,file=efi.qcow2 \
 -smp 4 \
 -m 3G \
 -object memory-backend-ram,size=1G,id=ram0 \
 -object memory-backend-ram,size=1G,id=ram1 \
 -object memory-backend-ram,size=1G,id=ram2 \
 -numa node,nodeid=0,memdev=ram0,cpus=0-1 \
 -numa node,nodeid=1,memdev=ram1,cpus=2-3 \
 -numa node,nodeid=2,memdev=ram2 \
 -numa 
hmat-lb,initiator=0,target=0,hierarchy=memory,data-type=access-latency,latency=10
 \
 -numa 
hmat-lb,initiator=0,target=0,hierarchy=memory,data-type=access-bandwidth,bandwidth=10485760
 \
 -numa 
hmat-lb,initiator=0,target=1,hierarchy=memory,data-type=access-latency,latency=20
 \
 -numa 
hmat-lb,initiator=0,target=1,hierarchy=memory,data-type=access-bandwidth,bandwidth=5242880
 \
 -numa 
hmat-lb,initiator=0,target=2,hierarchy=memory,data-type=access-latency,latency=30
 \
 -numa 
hmat-lb,initiator=0,target=2,hierarchy=memory,data-type=access-bandwidth,bandwidth=1048576
 \
 -numa 
hmat-lb,initiator=1,target=0,hierarchy=memory,data-type=access-latency,latency=20
 \
 -numa 
hmat-lb,initiator=1,target=0,hierarchy=memory,data-type=access-bandwidth,bandwidth=5242880
 \
 -numa 
hmat-lb,initiator=1,target=1,hierarchy=memory,data-type=access-latency,latency=10
 \
 -numa 
hmat-lb,initiator=1,target=1,hierarchy=memory,data-type=access-bandwidth,bandwidth=10485760
 \
 -numa 
hmat-lb,initiator=1,target=2,hierarchy=memory,data-type=access-latency,latency=30
 \
 -numa 
hmat-lb,initiator=1,target=2,hierarchy=memory,data-type=access-bandwidth,bandwidth=1048576
 \

This exposes NUMA node2 at same distance from both node0 and node1 as seen in 
lstopo:

Machine (2966MB total) + Package P#0
  NUMANode P#2 (979MB)
  Group0
NUMANode P#0 (980MB)
Core P#0 + PU P#0
Core P#1 + PU P#1
  Group0
NUMANode P#1 (1007MB)
Core P#2 + PU P#2
Core P#3 + PU P#3

Signed-off-by: Brice Goglin 
---
 hw/core/machine.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index d856485cb4..9884ef7ac6 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1012,9 +1012,7 @@ static void numa_validate_initiator(NumaState *numa_state)
 
 for (i = 0; i < numa_state->num_nodes; i++) {

 if (numa_info[i].initiator == MAX_NODES) {
-error_report("The initiator of NUMA node %d is missing, use "
- "'-numa node,initiator' option to declare it", i);
-exit(1);
+continue;
 }
 
 if (!numa_info[numa_info[i].initiator].present) {

--
2.30.2




OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH v1 10/11] linux-user: fix /proc/self/stat handling

2020-04-10 Thread Brice Goglin 
Le 10/04/2020 à 14:33, Alex Bennée a écrit :
> That was by inspection on my system which seems to truncate a lot
> earlier. It would be nice to find where in the Linux kernel it is
> output but I failed to grep the relevant function last night.


It's in proc/array.c, do_task_stat() calls proc_task_name(). In the end,
it seems to use task->tcomm or task->comm which is limited by

#define TASK_COMM_LEN   16

Brice



>
> On Fri, 10 Apr 2020, 12:11 Philippe Mathieu-Daudé,  <mailto:phi...@redhat.com>> wrote:
>
> Cc'ing Ludovic in case he can test with Guix-HPC.
>
> On 4/9/20 11:15 PM, Alex Bennée wrote:
> > In the original bug report long files names in Guix caused
> > /proc/self/stat be truncated without the trailing ") " as
> specified in
> > proc manpage which says:
> >      (2) comm  %s
> >             The  filename of the executable, in parentheses.  This
> >             is visible whether or not the  executable  is  swapped
> >             out.
> >
> > Additionally it should only be reporting the executable name rather
> > than the full path. Fix both these failings while cleaning up
> the code
> > to use GString to build up the reported values. As the whole
> function
> > is cleaned up also adjust the white space to the current coding
> style.
> >
> > Message-ID:  <mailto:fb4c55fa-d539-67ee-c6c9-de8fb63c8...@inria.fr>>
> > Reported-by: Brice Goglin  <mailto:brice.gog...@inria.fr>>
> > Cc: Philippe_Mathieu-Daudé  <mailto:phi...@redhat.com>>
> > Signed-off-by: Alex Bennée  <mailto:alex.ben...@linaro.org>>
> > ---
> >   linux-user/syscall.c | 43
> +++
> >   1 file changed, 19 insertions(+), 24 deletions(-)
> >
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index 6495ddc4cda..674f70e70a5 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -7295,34 +7295,29 @@ static int open_self_stat(void *cpu_env,
> int fd)
> >   {
> >       CPUState *cpu = env_cpu((CPUArchState *)cpu_env);
> >       TaskState *ts = cpu->opaque;
> > -    abi_ulong start_stack = ts->info->start_stack;
> > +    g_autoptr(GString) buf = g_string_new(NULL);
> >       int i;
> >   
> >       for (i = 0; i < 44; i++) {
> > -      char buf[128];
> > -      int len;
> > -      uint64_t val = 0;
> > -
> > -      if (i == 0) {
> > -        /* pid */
> > -        val = getpid();
> > -        snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
> > -      } else if (i == 1) {
> > -        /* app name */
> > -        snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
> > -      } else if (i == 27) {
> > -        /* stack bottom */
> > -        val = start_stack;
> > -        snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
> > -      } else {
> > -        /* for the rest, there is MasterCard */
> > -        snprintf(buf, sizeof(buf), "0%c", i == 43 ? '\n' : ' ');
> > -      }
> > +        if (i == 0) {
> > +            /* pid */
> > +            g_string_printf(buf, FMT_pid " ", getpid());
> > +        } else if (i == 1) {
> > +            /* app name */
> > +            gchar *bin = g_strrstr(ts->bprm->argv[0], "/");
> > +            bin = bin ? bin + 1 : ts->bprm->argv[0];
> > +            g_string_printf(buf, "(%.15s) ", bin);
>
> 15 or 125? 15 seems short. From your previous test I understood it
> was
> 124, for
> 
> sizeof("cat_with9_12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890___40").
>
> > +        } else if (i == 27) {
> > +            /* stack bottom */
> > +            g_string_printf(buf, TARGET_ABI_FMT_ld " ",
> ts->info->start_stack);
> > +        } else {
> > +            /* for the rest, there is MasterCard */
> > +            g_string_printf(buf, "0%c", i == 43 ? '\n' : ' ');
> > +        }
> >   
> > -      len = strlen(buf);
> > -      if (write(fd, buf, len) != len) {
> > -          return -1;
> > -      }
> > +        if (write(fd, buf->str, buf->len) != buf->len) {
> > +            return -1;
> > +        }
> >       }
> >   
> >       return 0;
> >
>

Re: linux-user: keep the name-ending parenthesis in /proc/self/stat

2020-04-09 Thread Brice Goglin 
Le 09/04/2020 à 17:27, Alex Bennée a écrit :
> Brice Goglin  writes:
>
>> When the program name is very long, qemu-user may truncate it in
>> /proc/self/stat. However the truncation must keep the ending ") "
>> to conform to the proc manpage which says:
>> (2) comm  %s
>>The  filename of the executable, in parentheses.  This
>>is visible whether or not the  executable  is  swapped
>>out.
>>
>> To reproduce:
>> $ ln -s /bin/cat 
>> $ qemu-x86_64 ./ /proc/self/stat
>>
>> Before the patch, you get:
>> 1134631 (0 0 0 0 0 0 0 0 ...
>> After the patch:
>> 1134631 () 0 0 0 0 0 0 0 0 ...
>>
>> This fixes an issue with hwloc failing to parse /proc/self/stat
>> when Ludovic Courtes was testing it in Guix over qemu-aarch64.
>>
>> Signed-off-by: Philippe_Mathieu-Daudé 
>> Signed-off-by: Brice Goglin 
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -7305,7 +7305,11 @@ static int open_self_stat(void *cpu_env, int fd)
>>  snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>>} else if (i == 1) {
>>  /* app name */
>> -snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>> +char *ptr = buf;
>> +
>> +*ptr++ = '(';
>> +ptr = stpncpy(ptr, ts->bprm->argv[0], sizeof(buf) - 3);
>> +strcpy(ptr, ") ");
> why not just use a format string:
>
>   snprintf(buf, sizeof(buf), "(%.125s) ", ts->bprm->argv[0]);
>

Go ahead and apply what you want (maybe 124 instead of 125 because of
the ending \0).

My commit message above explains how to test things very quickly.

I don't use qemu-user or Guix myself, and I can't spend time
debugging/testing this further.

Thank you

Brice

Re: linux-user: keep the name-ending parenthesis in /proc/self/stat

2020-04-08 Thread Brice Goglin 
Please apply my first patch if you believe Philippe's patch is wrong. I
can't spend more time debugging this trivial issue unfortunately.

Brice



Le 08/04/2020 à 17:48, Laurent Vivier a écrit :
> Le 08/04/2020 à 10:24, Brice Goglin a écrit :
>> When the program name is very long, qemu-user may truncate it in
>> /proc/self/stat. However the truncation must keep the ending ") "
>> to conform to the proc manpage which says:
>> (2) comm  %s
>>The  filename of the executable, in parentheses.  This
>>is visible whether or not the  executable  is  swapped
>>out.
>>
>> To reproduce:
>> $ ln -s /bin/cat 
>> $ qemu-x86_64 ./ /proc/self/stat
>>
>> Before the patch, you get:
>> 1134631 (0 0 0 0 0 0 0 0 ...
>> After the patch:
>> 1134631 () 0 0 0 0 0 0 0 0 ...
>>
>> This fixes an issue with hwloc failing to parse /proc/self/stat
>> when Ludovic Courtes was testing it in Guix over qemu-aarch64.
>>
>> Signed-off-by: Philippe_Mathieu-Daudé 
> You can't add "Signed-off-by" of someone else, in this case you could
> add "Suggested-by:".
>
> The subject of your patch should include "[PATCH]" (and the version of
> the patch, "[PATCH v2]").
>
> https://wiki.qemu.org/Contribute/SubmitAPatch
>
>> Signed-off-by: Brice Goglin 
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -7305,7 +7305,11 @@ static int open_self_stat(void *cpu_env, int fd)
>>  snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>>    } else if (i == 1) {
>>  /* app name */
>> -    snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>> +    char *ptr = buf;
>> +
>> +    *ptr++ = '(';
>> +    ptr = stpncpy(ptr, ts->bprm->argv[0], sizeof(buf) - 3);
> To have space for the NUL character I think it should be "sizeof(buf) - 4".
>
>> +    strcpy(ptr, ") ");
>>    } else if (i == 27) {
>>  /* stack bottom */
>>  val = start_stack;
>>
> Thanks,
> Laurent

linux-user: keep the name-ending parenthesis in /proc/self/stat

2020-04-08 Thread Brice Goglin 
When the program name is very long, qemu-user may truncate it in
/proc/self/stat. However the truncation must keep the ending ") "
to conform to the proc manpage which says:
(2) comm  %s
   The  filename of the executable, in parentheses.  This
   is visible whether or not the  executable  is  swapped
   out.

To reproduce:
$ ln -s /bin/cat 
$ qemu-x86_64 ./ /proc/self/stat

Before the patch, you get:
1134631 (0 0 0 0 0 0 0 0 ...
After the patch:
1134631 () 0 0 0 0 0 0 0 0 ...

This fixes an issue with hwloc failing to parse /proc/self/stat
when Ludovic Courtes was testing it in Guix over qemu-aarch64.

Signed-off-by: Philippe_Mathieu-Daudé 
Signed-off-by: Brice Goglin 

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7305,7 +7305,11 @@ static int open_self_stat(void *cpu_env, int fd)
 snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
   } else if (i == 1) {
 /* app name */
-    snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
+    char *ptr = buf;
+
+    *ptr++ = '(';
+    ptr = stpncpy(ptr, ts->bprm->argv[0], sizeof(buf) - 3);
+    strcpy(ptr, ") ");
   } else if (i == 27) {
 /* stack bottom */
 val = start_stack;

Re: linux-user: keep the name-ending parenthesis in /proc/self/stat

2020-04-08 Thread Brice Goglin 
Le 31/03/2020 à 00:29, Brice Goglin a écrit :
> Le 31/03/2020 à 00:05, Philippe Mathieu-Daudé a écrit :
>> On 3/30/20 9:07 PM, Brice Goglin wrote:
>>> When the program name is very long, qemu-user may truncate it in
>>> /proc/self/stat. However the truncation must keep the ending ") "
>>> to conform to the proc manpage which says:
>>>  (2) comm  %s
>>>     The  filename of the executable, in parentheses.  This
>>>     is visible whether or not the  executable  is  swapped
>>>     out.
>>>
>>> To reproduce:
>>> $ ln -s /bin/cat 
>>> $ qemu-x86_64 ./ /proc/self/stat
>>>
>>> Before the patch, you get:
>>> 1134631 (0 0 0 0 0 0 0 0 ...
>>> After the patch:
>>> 1134631 () 0 0 0 0 0 0 0 0 ...
>>>
>>> This fixes an issue with hwloc failing to parse /proc/self/stat
>>> when Ludovic Courtes was testing it in guix over qemu-aarch64.
>>>
>>> Signed-off-by: Brice Goglin
>>>
>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>> index 5af55fca78..a1126dcf5b 100644
>>> --- a/linux-user/syscall.c
>>> +++ b/linux-user/syscall.c
>>> @@ -7305,7 +7305,10 @@ static int open_self_stat(void *cpu_env, int fd)
>>>   snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>>>     } else if (i == 1) {
>>>   /* app name */
>>> -    snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>>> +    len = snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>>> +    if (len >= sizeof(buf))
>>> +  /* bring back the ending ") " that was truncated */
>>> +  strcpy(buf+sizeof(buf)-3, ") ");
>> Maybe we can avoid the sprintf() call:
>>
>> -- >8 --
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -7305,7 +7305,11 @@ static int open_self_stat(void *cpu_env, int fd)
>>  snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>>    } else if (i == 1) {
>>  /* app name */
>> -    snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>> +    char *ptr = buf;
>> +
>> +    *ptr++ = '(';
>> +    ptr = stpncpy(ptr, ts->bprm->argv[0], sizeof(buf) - 3);
>> +    strcpy(ptr, ") ");
>>    } else if (i == 27) {
>>  /* stack bottom */
>>  val = start_stack;
>>
> This works too.


Hello

Is anybody going to fix this anyhow for the next release?

Thank you

Brice

Re: linux-user: keep the name-ending parenthesis in /proc/self/stat

2020-03-30 Thread Brice Goglin 
Le 31/03/2020 à 00:05, Philippe Mathieu-Daudé a écrit :
> On 3/30/20 9:07 PM, Brice Goglin wrote:
>> When the program name is very long, qemu-user may truncate it in
>> /proc/self/stat. However the truncation must keep the ending ") "
>> to conform to the proc manpage which says:
>>  (2) comm  %s
>>     The  filename of the executable, in parentheses.  This
>>     is visible whether or not the  executable  is  swapped
>>     out.
>>
>> To reproduce:
>> $ ln -s /bin/cat 
>> $ qemu-x86_64 ./ /proc/self/stat
>>
>> Before the patch, you get:
>> 1134631 (0 0 0 0 0 0 0 0 ...
>> After the patch:
>> 1134631 () 0 0 0 0 0 0 0 0 ...
>>
>> This fixes an issue with hwloc failing to parse /proc/self/stat
>> when Ludovic Courtes was testing it in guix over qemu-aarch64.
>>
>> Signed-off-by: Brice Goglin
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 5af55fca78..a1126dcf5b 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -7305,7 +7305,10 @@ static int open_self_stat(void *cpu_env, int fd)
>>   snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>>     } else if (i == 1) {
>>   /* app name */
>> -    snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>> +    len = snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>> +    if (len >= sizeof(buf))
>> +  /* bring back the ending ") " that was truncated */
>> +  strcpy(buf+sizeof(buf)-3, ") ");
>
> Maybe we can avoid the sprintf() call:
>
> -- >8 --
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7305,7 +7305,11 @@ static int open_self_stat(void *cpu_env, int fd)
>  snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>    } else if (i == 1) {
>  /* app name */
> -    snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
> +    char *ptr = buf;
> +
> +    *ptr++ = '(';
> +    ptr = stpncpy(ptr, ts->bprm->argv[0], sizeof(buf) - 3);
> +    strcpy(ptr, ") ");
>    } else if (i == 27) {
>  /* stack bottom */
>  val = start_stack;
>

This works too.

Brice

linux-user: keep the name-ending parenthesis in /proc/self/stat

2020-03-30 Thread Brice Goglin 
When the program name is very long, qemu-user may truncate it in
/proc/self/stat. However the truncation must keep the ending ") "
to conform to the proc manpage which says:
(2) comm  %s
   The  filename of the executable, in parentheses.  This
   is visible whether or not the  executable  is  swapped
   out.

To reproduce:
$ ln -s /bin/cat 
$ qemu-x86_64 ./ /proc/self/stat

Before the patch, you get:
1134631 (0 0 0 0 0 0 0 0 ...
After the patch:
1134631 () 0 0 0 0 0 0 0 0 ...

This fixes an issue with hwloc failing to parse /proc/self/stat
when Ludovic Courtes was testing it in guix over qemu-aarch64.

Signed-off-by: Brice Goglin 

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 5af55fca78..a1126dcf5b 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7305,7 +7305,10 @@ static int open_self_stat(void *cpu_env, int fd)
 snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
   } else if (i == 1) {
 /* app name */
-snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
+len = snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
+if (len >= sizeof(buf))
+  /* bring back the ending ") " that was truncated */
+  strcpy(buf+sizeof(buf)-3, ") ");
   } else if (i == 27) {
 /* stack bottom */
 val = start_stack;