* [Qemu-devel] [PATCH v4 01/10] tests: pc: acpi: piix4: add sparse CPU hotplug case
2016-02-26 13:59 [Qemu-devel] [PATCH v4 00/10] pc: do not create invalid MADT.LAPIC/Processor entries Igor Mammedov
@ 2016-02-26 13:59 ` Igor Mammedov
2016-03-03 11:36 ` Marcel Apfelbaum
` (2 more replies)
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 02/10] pc: init pcms->apic_id_limit once and use it throughout pc.c Igor Mammedov
` (8 subsequent siblings)
9 siblings, 3 replies; 25+ messages in thread
From: Igor Mammedov @ 2016-02-26 13:59 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, ehabkost, mst
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
tests/acpi-test-data/pc/APIC.cpuhp_sparse | Bin 0 -> 168 bytes
tests/acpi-test-data/pc/DSDT.cpuhp_sparse | Bin 0 -> 5889 bytes
tests/acpi-test-data/pc/SRAT.cpuhp_sparse | Bin 0 -> 280 bytes
tests/bios-tables-test.c | 16 ++++++++++++++++
4 files changed, 16 insertions(+)
create mode 100644 tests/acpi-test-data/pc/APIC.cpuhp_sparse
create mode 100644 tests/acpi-test-data/pc/DSDT.cpuhp_sparse
create mode 100644 tests/acpi-test-data/pc/SRAT.cpuhp_sparse
diff --git a/tests/acpi-test-data/pc/APIC.cpuhp_sparse b/tests/acpi-test-data/pc/APIC.cpuhp_sparse
new file mode 100644
index 0000000000000000000000000000000000000000..d921ee1fa15fd6a549d8f12eb5f15682f5d5a8f3
GIT binary patch
literal 168
zcmXAh!4ZHU3<LuLgP=Xw3h>dF%=qzdbCzK#R)XfpBiYT(O;X*;O+@O-ds&YqpS2{;
z6lZXd=xv>zdpNz4*lUfG{4fZO5n3yB4pbBN)39~mfxvoghK(a9-2xZHV(ezz$1i^D
C5DftU
literal 0
HcmV?d00001
diff --git a/tests/acpi-test-data/pc/DSDT.cpuhp_sparse b/tests/acpi-test-data/pc/DSDT.cpuhp_sparse
new file mode 100644
index 0000000000000000000000000000000000000000..0dafa1037da867521d69d0ee0a5d2df07ac5a419
GIT binary patch
literal 5889
zcmb7I-EZ606+c&`Wa>(?DVs^0q-_NEBWt!K`mmh@7=}osY{?O2R+NJb3`WX!FlE>l
z!cBlCMghkKoV0J9mZ5tYlt9nPpRhgdv42AMFLc<Nr+Vj*x{~aKKqtWD`S_i4?$<fz
z(uQrc*(m^)sI2QXJ6E}7=?2LFfM|NFb@?_pdq!o|WvQ6VoX4qv#z#fbeqmHLR@paQ
z?=PNr|B1&sK5V{FpKW&hFP?gvK%fVkKIcTJ3$9&po&B?xUe7CTt!|{t-*QQD%SH<n
zccX4F)l@=ERjFCkwg52%p&Ex#v)U%40lxh)1+w#4C4TN^wQ1z6mf_~KW!G+(!6%95
zlc@8^<EqzBf8Fz-=cNu02hgAXy4Qg}chc~$|HKC{;4_M=x9n_|Pah<4#^@YmfQO)d
zc!@?s!*%vgTek|OV615g7!JiMl=K+zLv<xS0jtY(yX=NITv);_=Jgo&I1cxt-&`Uu
zWC>8a3{wrO+GKDQB)v?8q*w5ztQh!I8)q*|B0TwY_Aj>DI=T)QE`>`*qn_s{R+$ez
zsIdiLt852$AeMLt&i<7f6=MXKBrO@N&1kxTp$8r?wAzMkd@sas*3SNUbA4lEI5T(L
zP~q_fPzksuuNeAhE2lu450{@N;Pn%@<O6svk0PGs^Q-K8>3a#}RrZ7Q1Fqx3b+{l0
zL4Vb-+M}TI$Q7;7TU@C;v&ue`K9dw>VK1<+%KExl5UUes;eiq(qV<h(^ICc>fwG_m
zZdS-m@t7MP8~MQviu4nE9Ya3~`cZ4j_)P3sy4_|ec*zw!vBc<jq&9N+FmleGU0uV>
zQeM&lNfD!0^3^p|RGMasrPNSCj9zJOm^p`48u|^;KD6}eTFiN$xWhxgOG^JCy9;FQ
zoG0-B)Fx&`#Z_-`I>nm7U8~gwe>S$~`OtY$Nj~fFJ^xXM$6(Xn-RW!smtd=}_O=j|
z#4KnuZy2!M|Dd<U`q^HG%OkN3+s`v2u8g2;Z;Qu9lFCpbjU+m3`=3AD<SJku!qaU4
zf4772o^CVnx1V|)c+eSm&f^a*OEg+K^prfzYJHMd@KK;gksc*Qo2!3<tJ+WQ^1UwR
zaB{#?+H(C9y_G~jE1#X_sV@_dq!NZ3@)gN=9G~@m>RA`(+$DNIVrRcGh0`pHO;2Mz
zplYMwTG`o?eD2F>NJ??^!|^W__|s;>WTHXP@U*OJ)NS~jXucOnW+**jKtl@nQ?A{z
z*;n0g6j+>Eb&?OGSB^!;^ZS#d7bc<?g6IVt-(>WH_V(xnXWyi`>&K$=*#8l}I1#=W
zgfAWoUyKM}9ELZJhsS#VlV@L=2ww`qmyU%mMT9Ro`|C2!-aZ~)4#SHhL`~r#jLKk2
zO9Y#(rNT9)p`wU(S2fd=ZrYM=nPrAOP`B2bUGV&DiYuU?AN+Ov?o&#*;3a|YAt%G2
zqn*($Jkn#Ue%!;_`CJ-+#&wI5eS!Z|TT5U(9q<6z?j3BmK6o!y;?KM%6l-YgL8s4m
zJw64GHreL$G>Y$FBb0!$e3M~@FZ`+3+48r70eaBqnBUFz%C9~4TnXM>df=(ei{mP`
zU%qJ8tZ=6?R@2It^UGn^&X{-paBt^fu6ysk|N95O&F$Rz^xlJ=tvjEBHWy?`KiM;l
zW(%7ECaJ^BpnR1RrmZ(&tiyRY@00HE@Wn5md1T>xrd@6H=Xz$nX)`kzpU;SSezLFi
z%$9jGlyV8h$nAK{bIy~K0n;v;En3d8TmP?J2ppz*X4Q6?&u0YM)5`NEW=v8HL0}cL
zwY-=u@%&dm2f)sM?Dykg_!HtGdx#Z>4EZz;wh@xENY+B~qefLoFCaOTo;1t1hS)Pm
z)_k*7sgV&84RVA;qeccqG|0&giHRUvVgfZ;_qAsvBmEBXED&Y`A-g}45Y-@Eqi;<0
zNDMgTQZajuzw-pIhT|#IRB4(t#bA1+BIoGd#W^84jkZJbBo4^+DKe>ja!yL0#!0H-
z4w2c2Twc^ZFs)lAJH7!vO5O1`{mfjq+wvQryXJcJrvE!s>Y+<*!K|+3u|0+-N^B<{
z6YQ`dM6G7rRjV2#f2OQk^?V+WG-h%mp9k%lTg?~1dH?qK78Z8e34x_I+s^q30U<OM
zEsJnFOdU*B?KBEXaQm_>u5*+~bozr>i($y6@HHI3m5{p{zOIF@a`>u*FFkx2;j0?H
zRzR(0hoOU8HeC@bQ~ZxlGZ4djGwmNe@XGf({W{GQl(TS;e?ax?^oY*{yZ!IUGr)20
zsr}I}27xAMugEHghkySuP{2R_OIJf(#-$CfhR3P}$tP+AUijN(o(eCs8O~SPcdx#C
z#pLjQY1OP&xIsrt4&D!PwKwX`o4(*Bykr_)$UeS}#mX+Zvk95uppjm5XBC;pun-fs
zL>iia+W6aT+9zlKhJ1;xAy%t8Y7jgTTn#e1wei!!zk*jRhf~2TL2w6r#dFT%bih#k
zzOFi7lynQvTkzA;u32uwfVu}DL*FvU*lo}<E*@uwOmz*qp-~}R{7EA=X~ZUt7#b~O
zq#{lif2bNo!rN_T916MVaV#Di+Lgl6cf?LQVkaH3lZJMS!J;0GCm*qskJ!mWyCu4B
z9UEu-h+X`MU3_S_oOshNal|fh#4gd*>T&9LG3<d|IH1kAOC%$*aJQf8b$N{FVn|Cw
zmqJ=5dNHKqL@$MOg6QSD3U%b!<kT=p@Z)x(R0Pd5Xp0j^A8(i(=IOlDEV}#y=kw0v
zbHqs01aj08@X;WuffUw8JW%fvpbXb}(AKc%VzUf-BYj#pk57d5*h+I$Ch=kGIW0l`
zG^U#O=#VQF)MDDE?0g=-!sv_K*}q^Hh76X57-*O#1p~V{5k*0ih;a5lwhLF$(B5bc
zzv$6K0&O-pO7x{d0Z2{IBDp0fUbHY2Lh*wy0xG=51vDN7EeWrfq@mYDBvg2v5>Whj
zeft0wg;zyDl}M=YniSAv6m&^=<pRnhp~5TH$Z%SzDCn~AnikM>Bvg3K2xuk>dPR7h
z7SQQPsPKxPP~li-qM%oWS5-jONT~3d70_%H^qTNGE1<KHP~mk>K<A>MW#JW1emE^H
z5-Pl&5YQ7*(2DSSQb12eLWS2;0(vS6std340y-ZF6<$vZ=;<h^A-tXu&@+)x;q|P5
zo{fT5h1Yik^qokk@cOQRz8eKy5nkUD(Dx#t!s|H!J@+=KRy%9lY_?<l^llfw;|{3S
z-3=HO8LVa@lOslGt|NmLDb(x{Lz>KPH--NdJg~8>47nI_Bb|&K7KfpZVl9ny6bw^*
zsH14hBOQgqL>=lV-WAf#>~^ubVgov`G2sM{cL3zadhF2$Hl~iy<A*BpW4%f|CV|lB
z0)DK&PCOoGq2CMmu|AkF9!#OfW^!Qb6MC989<h<0=50*qY36uXM*1A_6Z-4O<F=3V
cd&Hw&J4aV^{O2hBbBwkSd))>b5(bz42U8HCOaK4?
literal 0
HcmV?d00001
diff --git a/tests/acpi-test-data/pc/SRAT.cpuhp_sparse b/tests/acpi-test-data/pc/SRAT.cpuhp_sparse
new file mode 100644
index 0000000000000000000000000000000000000000..5377019cca32fb7a97111ea79d96d68a52627cd8
GIT binary patch
literal 280
zcmWFzatx7RWME*-aq@Te2v%^42yhMtiUEZfKx_~V!f+sf!DmF1XF}sMqw!hL_^fDr
gHe^1d2Ha*Sg9|QzT^-PYVDNz*rVzV2m@IZ00Pd>^0RR91
literal 0
HcmV?d00001
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 0a80ddf..15fa3c9 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -781,6 +781,20 @@ static void test_acpi_q35_tcg_bridge(void)
free_test_data(&data);
}
+static void test_acpi_piix4_tcg_cpuhp_sparse(void)
+{
+ test_data data;
+
+ memset(&data, 0, sizeof(data));
+ data.machine = MACHINE_PC;
+ data.variant = ".cpuhp_sparse";
+ test_acpi_one("-machine accel=tcg"
+ " -numa node -smp 1,sockets=2,cores=3,maxcpus=6",
+ &data);
+ free_test_data(&data);
+}
+
+
int main(int argc, char *argv[])
{
const char *arch = qtest_get_arch();
@@ -797,6 +811,8 @@ int main(int argc, char *argv[])
qtest_add_func("acpi/piix4/tcg/bridge", test_acpi_piix4_tcg_bridge);
qtest_add_func("acpi/q35/tcg", test_acpi_q35_tcg);
qtest_add_func("acpi/q35/tcg/bridge", test_acpi_q35_tcg_bridge);
+ qtest_add_func("acpi/piix4/tcg/cpuhp_sparse",
+ test_acpi_piix4_tcg_cpuhp_sparse);
}
ret = g_test_run();
boot_sector_cleanup(disk);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH v4 01/10] tests: pc: acpi: piix4: add sparse CPU hotplug case
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 01/10] tests: pc: acpi: piix4: add sparse CPU hotplug case Igor Mammedov
@ 2016-03-03 11:36 ` Marcel Apfelbaum
2016-03-08 14:05 ` Marcel Apfelbaum
2016-03-11 13:48 ` Michael S. Tsirkin
2 siblings, 0 replies; 25+ messages in thread
From: Marcel Apfelbaum @ 2016-03-03 11:36 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel; +Cc: ehabkost, mst
On 02/26/2016 03:59 PM, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> tests/acpi-test-data/pc/APIC.cpuhp_sparse | Bin 0 -> 168 bytes
> tests/acpi-test-data/pc/DSDT.cpuhp_sparse | Bin 0 -> 5889 bytes
> tests/acpi-test-data/pc/SRAT.cpuhp_sparse | Bin 0 -> 280 bytes
> tests/bios-tables-test.c | 16 ++++++++++++++++
> 4 files changed, 16 insertions(+)
> create mode 100644 tests/acpi-test-data/pc/APIC.cpuhp_sparse
> create mode 100644 tests/acpi-test-data/pc/DSDT.cpuhp_sparse
> create mode 100644 tests/acpi-test-data/pc/SRAT.cpuhp_sparse
>
> diff --git a/tests/acpi-test-data/pc/APIC.cpuhp_sparse b/tests/acpi-test-data/pc/APIC.cpuhp_sparse
> new file mode 100644
> index 0000000000000000000000000000000000000000..d921ee1fa15fd6a549d8f12eb5f15682f5d5a8f3
> GIT binary patch
> literal 168
> zcmXAh!4ZHU3<LuLgP=Xw3h>dF%=qzdbCzK#R)XfpBiYT(O;X*;O+@O-ds&YqpS2{;
> z6lZXd=xv>zdpNz4*lUfG{4fZO5n3yB4pbBN)39~mfxvoghK(a9-2xZHV(ezz$1i^D
> C5DftU
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/acpi-test-data/pc/DSDT.cpuhp_sparse b/tests/acpi-test-data/pc/DSDT.cpuhp_sparse
> new file mode 100644
> index 0000000000000000000000000000000000000000..0dafa1037da867521d69d0ee0a5d2df07ac5a419
> GIT binary patch
> literal 5889
> zcmb7I-EZ606+c&`Wa>(?DVs^0q-_NEBWt!K`mmh@7=}osY{?O2R+NJb3`WX!FlE>l
> z!cBlCMghkKoV0J9mZ5tYlt9nPpRhgdv42AMFLc<Nr+Vj*x{~aKKqtWD`S_i4?$<fz
> z(uQrc*(m^)sI2QXJ6E}7=?2LFfM|NFb@?_pdq!o|WvQ6VoX4qv#z#fbeqmHLR@paQ
> z?=PNr|B1&sK5V{FpKW&hFP?gvK%fVkKIcTJ3$9&po&B?xUe7CTt!|{t-*QQD%SH<n
> zccX4F)l@=ERjFCkwg52%p&Ex#v)U%40lxh)1+w#4C4TN^wQ1z6mf_~KW!G+(!6%95
> zlc@8^<EqzBf8Fz-=cNu02hgAXy4Qg}chc~$|HKC{;4_M=x9n_|Pah<4#^@YmfQO)d
> zc!@?s!*%vgTek|OV615g7!JiMl=K+zLv<xS0jtY(yX=NITv);_=Jgo&I1cxt-&`Uu
> zWC>8a3{wrO+GKDQB)v?8q*w5ztQh!I8)q*|B0TwY_Aj>DI=T)QE`>`*qn_s{R+$ez
> zsIdiLt852$AeMLt&i<7f6=MXKBrO@N&1kxTp$8r?wAzMkd@sas*3SNUbA4lEI5T(L
> zP~q_fPzksuuNeAhE2lu450{@N;Pn%@<O6svk0PGs^Q-K8>3a#}RrZ7Q1Fqx3b+{l0
> zL4Vb-+M}TI$Q7;7TU@C;v&ue`K9dw>VK1<+%KExl5UUes;eiq(qV<h(^ICc>fwG_m
> zZdS-m@t7MP8~MQviu4nE9Ya3~`cZ4j_)P3sy4_|ec*zw!vBc<jq&9N+FmleGU0uV>
> zQeM&lNfD!0^3^p|RGMasrPNSCj9zJOm^p`48u|^;KD6}eTFiN$xWhxgOG^JCy9;FQ
> zoG0-B)Fx&`#Z_-`I>nm7U8~gwe>S$~`OtY$Nj~fFJ^xXM$6(Xn-RW!smtd=}_O=j|
> z#4KnuZy2!M|Dd<U`q^HG%OkN3+s`v2u8g2;Z;Qu9lFCpbjU+m3`=3AD<SJku!qaU4
> zf4772o^CVnx1V|)c+eSm&f^a*OEg+K^prfzYJHMd@KK;gksc*Qo2!3<tJ+WQ^1UwR
> zaB{#?+H(C9y_G~jE1#X_sV@_dq!NZ3@)gN=9G~@m>RA`(+$DNIVrRcGh0`pHO;2Mz
> zplYMwTG`o?eD2F>NJ??^!|^W__|s;>WTHXP@U*OJ)NS~jXucOnW+**jKtl@nQ?A{z
> z*;n0g6j+>Eb&?OGSB^!;^ZS#d7bc<?g6IVt-(>WH_V(xnXWyi`>&K$=*#8l}I1#=W
> zgfAWoUyKM}9ELZJhsS#VlV@L=2ww`qmyU%mMT9Ro`|C2!-aZ~)4#SHhL`~r#jLKk2
> zO9Y#(rNT9)p`wU(S2fd=ZrYM=nPrAOP`B2bUGV&DiYuU?AN+Ov?o&#*;3a|YAt%G2
> zqn*($Jkn#Ue%!;_`CJ-+#&wI5eS!Z|TT5U(9q<6z?j3BmK6o!y;?KM%6l-YgL8s4m
> zJw64GHreL$G>Y$FBb0!$e3M~@FZ`+3+48r70eaBqnBUFz%C9~4TnXM>df=(ei{mP`
> zU%qJ8tZ=6?R@2It^UGn^&X{-paBt^fu6ysk|N95O&F$Rz^xlJ=tvjEBHWy?`KiM;l
> zW(%7ECaJ^BpnR1RrmZ(&tiyRY@00HE@Wn5md1T>xrd@6H=Xz$nX)`kzpU;SSezLFi
> z%$9jGlyV8h$nAK{bIy~K0n;v;En3d8TmP?J2ppz*X4Q6?&u0YM)5`NEW=v8HL0}cL
> zwY-=u@%&dm2f)sM?Dykg_!HtGdx#Z>4EZz;wh@xENY+B~qefLoFCaOTo;1t1hS)Pm
> z)_k*7sgV&84RVA;qeccqG|0&giHRUvVgfZ;_qAsvBmEBXED&Y`A-g}45Y-@Eqi;<0
> zNDMgTQZajuzw-pIhT|#IRB4(t#bA1+BIoGd#W^84jkZJbBo4^+DKe>ja!yL0#!0H-
> z4w2c2Twc^ZFs)lAJH7!vO5O1`{mfjq+wvQryXJcJrvE!s>Y+<*!K|+3u|0+-N^B<{
> z6YQ`dM6G7rRjV2#f2OQk^?V+WG-h%mp9k%lTg?~1dH?qK78Z8e34x_I+s^q30U<OM
> zEsJnFOdU*B?KBEXaQm_>u5*+~bozr>i($y6@HHI3m5{p{zOIF@a`>u*FFkx2;j0?H
> zRzR(0hoOU8HeC@bQ~ZxlGZ4djGwmNe@XGf({W{GQl(TS;e?ax?^oY*{yZ!IUGr)20
> zsr}I}27xAMugEHghkySuP{2R_OIJf(#-$CfhR3P}$tP+AUijN(o(eCs8O~SPcdx#C
> z#pLjQY1OP&xIsrt4&D!PwKwX`o4(*Bykr_)$UeS}#mX+Zvk95uppjm5XBC;pun-fs
> zL>iia+W6aT+9zlKhJ1;xAy%t8Y7jgTTn#e1wei!!zk*jRhf~2TL2w6r#dFT%bih#k
> zzOFi7lynQvTkzA;u32uwfVu}DL*FvU*lo}<E*@uwOmz*qp-~}R{7EA=X~ZUt7#b~O
> zq#{lif2bNo!rN_T916MVaV#Di+Lgl6cf?LQVkaH3lZJMS!J;0GCm*qskJ!mWyCu4B
> z9UEu-h+X`MU3_S_oOshNal|fh#4gd*>T&9LG3<d|IH1kAOC%$*aJQf8b$N{FVn|Cw
> zmqJ=5dNHKqL@$MOg6QSD3U%b!<kT=p@Z)x(R0Pd5Xp0j^A8(i(=IOlDEV}#y=kw0v
> zbHqs01aj08@X;WuffUw8JW%fvpbXb}(AKc%VzUf-BYj#pk57d5*h+I$Ch=kGIW0l`
> zG^U#O=#VQF)MDDE?0g=-!sv_K*}q^Hh76X57-*O#1p~V{5k*0ih;a5lwhLF$(B5bc
> zzv$6K0&O-pO7x{d0Z2{IBDp0fUbHY2Lh*wy0xG=51vDN7EeWrfq@mYDBvg2v5>Whj
> zeft0wg;zyDl}M=YniSAv6m&^=<pRnhp~5TH$Z%SzDCn~AnikM>Bvg3K2xuk>dPR7h
> z7SQQPsPKxPP~li-qM%oWS5-jONT~3d70_%H^qTNGE1<KHP~mk>K<A>MW#JW1emE^H
> z5-Pl&5YQ7*(2DSSQb12eLWS2;0(vS6std340y-ZF6<$vZ=;<h^A-tXu&@+)x;q|P5
> zo{fT5h1Yik^qokk@cOQRz8eKy5nkUD(Dx#t!s|H!J@+=KRy%9lY_?<l^llfw;|{3S
> z-3=HO8LVa@lOslGt|NmLDb(x{Lz>KPH--NdJg~8>47nI_Bb|&K7KfpZVl9ny6bw^*
> zsH14hBOQgqL>=lV-WAf#>~^ubVgov`G2sM{cL3zadhF2$Hl~iy<A*BpW4%f|CV|lB
> z0)DK&PCOoGq2CMmu|AkF9!#OfW^!Qb6MC989<h<0=50*qY36uXM*1A_6Z-4O<F=3V
> cd&Hw&J4aV^{O2hBbBwkSd))>b5(bz42U8HCOaK4?
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/acpi-test-data/pc/SRAT.cpuhp_sparse b/tests/acpi-test-data/pc/SRAT.cpuhp_sparse
> new file mode 100644
> index 0000000000000000000000000000000000000000..5377019cca32fb7a97111ea79d96d68a52627cd8
> GIT binary patch
> literal 280
> zcmWFzatx7RWME*-aq@Te2v%^42yhMtiUEZfKx_~V!f+sf!DmF1XF}sMqw!hL_^fDr
> gHe^1d2Ha*Sg9|QzT^-PYVDNz*rVzV2m@IZ00Pd>^0RR91
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> index 0a80ddf..15fa3c9 100644
> --- a/tests/bios-tables-test.c
> +++ b/tests/bios-tables-test.c
> @@ -781,6 +781,20 @@ static void test_acpi_q35_tcg_bridge(void)
> free_test_data(&data);
> }
>
> +static void test_acpi_piix4_tcg_cpuhp_sparse(void)
> +{
> + test_data data;
> +
> + memset(&data, 0, sizeof(data));
> + data.machine = MACHINE_PC;
> + data.variant = ".cpuhp_sparse";
> + test_acpi_one("-machine accel=tcg"
> + " -numa node -smp 1,sockets=2,cores=3,maxcpus=6",
> + &data);
> + free_test_data(&data);
> +}
> +
> +
> int main(int argc, char *argv[])
> {
> const char *arch = qtest_get_arch();
> @@ -797,6 +811,8 @@ int main(int argc, char *argv[])
> qtest_add_func("acpi/piix4/tcg/bridge", test_acpi_piix4_tcg_bridge);
> qtest_add_func("acpi/q35/tcg", test_acpi_q35_tcg);
> qtest_add_func("acpi/q35/tcg/bridge", test_acpi_q35_tcg_bridge);
> + qtest_add_func("acpi/piix4/tcg/cpuhp_sparse",
> + test_acpi_piix4_tcg_cpuhp_sparse);
> }
> ret = g_test_run();
> boot_sector_cleanup(disk);
>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Thanks,
Marcel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH v4 01/10] tests: pc: acpi: piix4: add sparse CPU hotplug case
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 01/10] tests: pc: acpi: piix4: add sparse CPU hotplug case Igor Mammedov
2016-03-03 11:36 ` Marcel Apfelbaum
@ 2016-03-08 14:05 ` Marcel Apfelbaum
2016-03-11 13:48 ` Michael S. Tsirkin
2 siblings, 0 replies; 25+ messages in thread
From: Marcel Apfelbaum @ 2016-03-08 14:05 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel; +Cc: ehabkost, mst
On 02/26/2016 03:59 PM, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> tests/acpi-test-data/pc/APIC.cpuhp_sparse | Bin 0 -> 168 bytes
> tests/acpi-test-data/pc/DSDT.cpuhp_sparse | Bin 0 -> 5889 bytes
> tests/acpi-test-data/pc/SRAT.cpuhp_sparse | Bin 0 -> 280 bytes
> tests/bios-tables-test.c | 16 ++++++++++++++++
> 4 files changed, 16 insertions(+)
> create mode 100644 tests/acpi-test-data/pc/APIC.cpuhp_sparse
> create mode 100644 tests/acpi-test-data/pc/DSDT.cpuhp_sparse
> create mode 100644 tests/acpi-test-data/pc/SRAT.cpuhp_sparse
>
> diff --git a/tests/acpi-test-data/pc/APIC.cpuhp_sparse b/tests/acpi-test-data/pc/APIC.cpuhp_sparse
> new file mode 100644
> index 0000000000000000000000000000000000000000..d921ee1fa15fd6a549d8f12eb5f15682f5d5a8f3
> GIT binary patch
> literal 168
> zcmXAh!4ZHU3<LuLgP=Xw3h>dF%=qzdbCzK#R)XfpBiYT(O;X*;O+@O-ds&YqpS2{;
> z6lZXd=xv>zdpNz4*lUfG{4fZO5n3yB4pbBN)39~mfxvoghK(a9-2xZHV(ezz$1i^D
> C5DftU
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/acpi-test-data/pc/DSDT.cpuhp_sparse b/tests/acpi-test-data/pc/DSDT.cpuhp_sparse
> new file mode 100644
> index 0000000000000000000000000000000000000000..0dafa1037da867521d69d0ee0a5d2df07ac5a419
> GIT binary patch
> literal 5889
> zcmb7I-EZ606+c&`Wa>(?DVs^0q-_NEBWt!K`mmh@7=}osY{?O2R+NJb3`WX!FlE>l
> z!cBlCMghkKoV0J9mZ5tYlt9nPpRhgdv42AMFLc<Nr+Vj*x{~aKKqtWD`S_i4?$<fz
> z(uQrc*(m^)sI2QXJ6E}7=?2LFfM|NFb@?_pdq!o|WvQ6VoX4qv#z#fbeqmHLR@paQ
> z?=PNr|B1&sK5V{FpKW&hFP?gvK%fVkKIcTJ3$9&po&B?xUe7CTt!|{t-*QQD%SH<n
> zccX4F)l@=ERjFCkwg52%p&Ex#v)U%40lxh)1+w#4C4TN^wQ1z6mf_~KW!G+(!6%95
> zlc@8^<EqzBf8Fz-=cNu02hgAXy4Qg}chc~$|HKC{;4_M=x9n_|Pah<4#^@YmfQO)d
> zc!@?s!*%vgTek|OV615g7!JiMl=K+zLv<xS0jtY(yX=NITv);_=Jgo&I1cxt-&`Uu
> zWC>8a3{wrO+GKDQB)v?8q*w5ztQh!I8)q*|B0TwY_Aj>DI=T)QE`>`*qn_s{R+$ez
> zsIdiLt852$AeMLt&i<7f6=MXKBrO@N&1kxTp$8r?wAzMkd@sas*3SNUbA4lEI5T(L
> zP~q_fPzksuuNeAhE2lu450{@N;Pn%@<O6svk0PGs^Q-K8>3a#}RrZ7Q1Fqx3b+{l0
> zL4Vb-+M}TI$Q7;7TU@C;v&ue`K9dw>VK1<+%KExl5UUes;eiq(qV<h(^ICc>fwG_m
> zZdS-m@t7MP8~MQviu4nE9Ya3~`cZ4j_)P3sy4_|ec*zw!vBc<jq&9N+FmleGU0uV>
> zQeM&lNfD!0^3^p|RGMasrPNSCj9zJOm^p`48u|^;KD6}eTFiN$xWhxgOG^JCy9;FQ
> zoG0-B)Fx&`#Z_-`I>nm7U8~gwe>S$~`OtY$Nj~fFJ^xXM$6(Xn-RW!smtd=}_O=j|
> z#4KnuZy2!M|Dd<U`q^HG%OkN3+s`v2u8g2;Z;Qu9lFCpbjU+m3`=3AD<SJku!qaU4
> zf4772o^CVnx1V|)c+eSm&f^a*OEg+K^prfzYJHMd@KK;gksc*Qo2!3<tJ+WQ^1UwR
> zaB{#?+H(C9y_G~jE1#X_sV@_dq!NZ3@)gN=9G~@m>RA`(+$DNIVrRcGh0`pHO;2Mz
> zplYMwTG`o?eD2F>NJ??^!|^W__|s;>WTHXP@U*OJ)NS~jXucOnW+**jKtl@nQ?A{z
> z*;n0g6j+>Eb&?OGSB^!;^ZS#d7bc<?g6IVt-(>WH_V(xnXWyi`>&K$=*#8l}I1#=W
> zgfAWoUyKM}9ELZJhsS#VlV@L=2ww`qmyU%mMT9Ro`|C2!-aZ~)4#SHhL`~r#jLKk2
> zO9Y#(rNT9)p`wU(S2fd=ZrYM=nPrAOP`B2bUGV&DiYuU?AN+Ov?o&#*;3a|YAt%G2
> zqn*($Jkn#Ue%!;_`CJ-+#&wI5eS!Z|TT5U(9q<6z?j3BmK6o!y;?KM%6l-YgL8s4m
> zJw64GHreL$G>Y$FBb0!$e3M~@FZ`+3+48r70eaBqnBUFz%C9~4TnXM>df=(ei{mP`
> zU%qJ8tZ=6?R@2It^UGn^&X{-paBt^fu6ysk|N95O&F$Rz^xlJ=tvjEBHWy?`KiM;l
> zW(%7ECaJ^BpnR1RrmZ(&tiyRY@00HE@Wn5md1T>xrd@6H=Xz$nX)`kzpU;SSezLFi
> z%$9jGlyV8h$nAK{bIy~K0n;v;En3d8TmP?J2ppz*X4Q6?&u0YM)5`NEW=v8HL0}cL
> zwY-=u@%&dm2f)sM?Dykg_!HtGdx#Z>4EZz;wh@xENY+B~qefLoFCaOTo;1t1hS)Pm
> z)_k*7sgV&84RVA;qeccqG|0&giHRUvVgfZ;_qAsvBmEBXED&Y`A-g}45Y-@Eqi;<0
> zNDMgTQZajuzw-pIhT|#IRB4(t#bA1+BIoGd#W^84jkZJbBo4^+DKe>ja!yL0#!0H-
> z4w2c2Twc^ZFs)lAJH7!vO5O1`{mfjq+wvQryXJcJrvE!s>Y+<*!K|+3u|0+-N^B<{
> z6YQ`dM6G7rRjV2#f2OQk^?V+WG-h%mp9k%lTg?~1dH?qK78Z8e34x_I+s^q30U<OM
> zEsJnFOdU*B?KBEXaQm_>u5*+~bozr>i($y6@HHI3m5{p{zOIF@a`>u*FFkx2;j0?H
> zRzR(0hoOU8HeC@bQ~ZxlGZ4djGwmNe@XGf({W{GQl(TS;e?ax?^oY*{yZ!IUGr)20
> zsr}I}27xAMugEHghkySuP{2R_OIJf(#-$CfhR3P}$tP+AUijN(o(eCs8O~SPcdx#C
> z#pLjQY1OP&xIsrt4&D!PwKwX`o4(*Bykr_)$UeS}#mX+Zvk95uppjm5XBC;pun-fs
> zL>iia+W6aT+9zlKhJ1;xAy%t8Y7jgTTn#e1wei!!zk*jRhf~2TL2w6r#dFT%bih#k
> zzOFi7lynQvTkzA;u32uwfVu}DL*FvU*lo}<E*@uwOmz*qp-~}R{7EA=X~ZUt7#b~O
> zq#{lif2bNo!rN_T916MVaV#Di+Lgl6cf?LQVkaH3lZJMS!J;0GCm*qskJ!mWyCu4B
> z9UEu-h+X`MU3_S_oOshNal|fh#4gd*>T&9LG3<d|IH1kAOC%$*aJQf8b$N{FVn|Cw
> zmqJ=5dNHKqL@$MOg6QSD3U%b!<kT=p@Z)x(R0Pd5Xp0j^A8(i(=IOlDEV}#y=kw0v
> zbHqs01aj08@X;WuffUw8JW%fvpbXb}(AKc%VzUf-BYj#pk57d5*h+I$Ch=kGIW0l`
> zG^U#O=#VQF)MDDE?0g=-!sv_K*}q^Hh76X57-*O#1p~V{5k*0ih;a5lwhLF$(B5bc
> zzv$6K0&O-pO7x{d0Z2{IBDp0fUbHY2Lh*wy0xG=51vDN7EeWrfq@mYDBvg2v5>Whj
> zeft0wg;zyDl}M=YniSAv6m&^=<pRnhp~5TH$Z%SzDCn~AnikM>Bvg3K2xuk>dPR7h
> z7SQQPsPKxPP~li-qM%oWS5-jONT~3d70_%H^qTNGE1<KHP~mk>K<A>MW#JW1emE^H
> z5-Pl&5YQ7*(2DSSQb12eLWS2;0(vS6std340y-ZF6<$vZ=;<h^A-tXu&@+)x;q|P5
> zo{fT5h1Yik^qokk@cOQRz8eKy5nkUD(Dx#t!s|H!J@+=KRy%9lY_?<l^llfw;|{3S
> z-3=HO8LVa@lOslGt|NmLDb(x{Lz>KPH--NdJg~8>47nI_Bb|&K7KfpZVl9ny6bw^*
> zsH14hBOQgqL>=lV-WAf#>~^ubVgov`G2sM{cL3zadhF2$Hl~iy<A*BpW4%f|CV|lB
> z0)DK&PCOoGq2CMmu|AkF9!#OfW^!Qb6MC989<h<0=50*qY36uXM*1A_6Z-4O<F=3V
> cd&Hw&J4aV^{O2hBbBwkSd))>b5(bz42U8HCOaK4?
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/acpi-test-data/pc/SRAT.cpuhp_sparse b/tests/acpi-test-data/pc/SRAT.cpuhp_sparse
> new file mode 100644
> index 0000000000000000000000000000000000000000..5377019cca32fb7a97111ea79d96d68a52627cd8
> GIT binary patch
> literal 280
> zcmWFzatx7RWME*-aq@Te2v%^42yhMtiUEZfKx_~V!f+sf!DmF1XF}sMqw!hL_^fDr
> gHe^1d2Ha*Sg9|QzT^-PYVDNz*rVzV2m@IZ00Pd>^0RR91
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> index 0a80ddf..15fa3c9 100644
> --- a/tests/bios-tables-test.c
> +++ b/tests/bios-tables-test.c
> @@ -781,6 +781,20 @@ static void test_acpi_q35_tcg_bridge(void)
> free_test_data(&data);
> }
>
> +static void test_acpi_piix4_tcg_cpuhp_sparse(void)
> +{
> + test_data data;
> +
> + memset(&data, 0, sizeof(data));
> + data.machine = MACHINE_PC;
> + data.variant = ".cpuhp_sparse";
> + test_acpi_one("-machine accel=tcg"
> + " -numa node -smp 1,sockets=2,cores=3,maxcpus=6",
> + &data);
> + free_test_data(&data);
> +}
> +
> +
> int main(int argc, char *argv[])
> {
> const char *arch = qtest_get_arch();
> @@ -797,6 +811,8 @@ int main(int argc, char *argv[])
> qtest_add_func("acpi/piix4/tcg/bridge", test_acpi_piix4_tcg_bridge);
> qtest_add_func("acpi/q35/tcg", test_acpi_q35_tcg);
> qtest_add_func("acpi/q35/tcg/bridge", test_acpi_q35_tcg_bridge);
> + qtest_add_func("acpi/piix4/tcg/cpuhp_sparse",
> + test_acpi_piix4_tcg_cpuhp_sparse);
> }
> ret = g_test_run();
> boot_sector_cleanup(disk);
>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH v4 01/10] tests: pc: acpi: piix4: add sparse CPU hotplug case
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 01/10] tests: pc: acpi: piix4: add sparse CPU hotplug case Igor Mammedov
2016-03-03 11:36 ` Marcel Apfelbaum
2016-03-08 14:05 ` Marcel Apfelbaum
@ 2016-03-11 13:48 ` Michael S. Tsirkin
2 siblings, 0 replies; 25+ messages in thread
From: Michael S. Tsirkin @ 2016-03-11 13:48 UTC (permalink / raw)
To: Igor Mammedov; +Cc: marcel, qemu-devel, ehabkost
On Fri, Feb 26, 2016 at 02:59:19PM +0100, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Seems to give warnings likely because expected
files changed upstream.
I skipped this patch in series, pls rebase it.
We also really need to find a better way to test this than just
adding more variants that have to be
updated each time.
> ---
> tests/acpi-test-data/pc/APIC.cpuhp_sparse | Bin 0 -> 168 bytes
> tests/acpi-test-data/pc/DSDT.cpuhp_sparse | Bin 0 -> 5889 bytes
> tests/acpi-test-data/pc/SRAT.cpuhp_sparse | Bin 0 -> 280 bytes
> tests/bios-tables-test.c | 16 ++++++++++++++++
> 4 files changed, 16 insertions(+)
> create mode 100644 tests/acpi-test-data/pc/APIC.cpuhp_sparse
> create mode 100644 tests/acpi-test-data/pc/DSDT.cpuhp_sparse
> create mode 100644 tests/acpi-test-data/pc/SRAT.cpuhp_sparse
>
> diff --git a/tests/acpi-test-data/pc/APIC.cpuhp_sparse b/tests/acpi-test-data/pc/APIC.cpuhp_sparse
> new file mode 100644
> index 0000000000000000000000000000000000000000..d921ee1fa15fd6a549d8f12eb5f15682f5d5a8f3
> GIT binary patch
> literal 168
> zcmXAh!4ZHU3<LuLgP=Xw3h>dF%=qzdbCzK#R)XfpBiYT(O;X*;O+@O-ds&YqpS2{;
> z6lZXd=xv>zdpNz4*lUfG{4fZO5n3yB4pbBN)39~mfxvoghK(a9-2xZHV(ezz$1i^D
> C5DftU
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/acpi-test-data/pc/DSDT.cpuhp_sparse b/tests/acpi-test-data/pc/DSDT.cpuhp_sparse
> new file mode 100644
> index 0000000000000000000000000000000000000000..0dafa1037da867521d69d0ee0a5d2df07ac5a419
> GIT binary patch
> literal 5889
> zcmb7I-EZ606+c&`Wa>(?DVs^0q-_NEBWt!K`mmh@7=}osY{?O2R+NJb3`WX!FlE>l
> z!cBlCMghkKoV0J9mZ5tYlt9nPpRhgdv42AMFLc<Nr+Vj*x{~aKKqtWD`S_i4?$<fz
> z(uQrc*(m^)sI2QXJ6E}7=?2LFfM|NFb@?_pdq!o|WvQ6VoX4qv#z#fbeqmHLR@paQ
> z?=PNr|B1&sK5V{FpKW&hFP?gvK%fVkKIcTJ3$9&po&B?xUe7CTt!|{t-*QQD%SH<n
> zccX4F)l@=ERjFCkwg52%p&Ex#v)U%40lxh)1+w#4C4TN^wQ1z6mf_~KW!G+(!6%95
> zlc@8^<EqzBf8Fz-=cNu02hgAXy4Qg}chc~$|HKC{;4_M=x9n_|Pah<4#^@YmfQO)d
> zc!@?s!*%vgTek|OV615g7!JiMl=K+zLv<xS0jtY(yX=NITv);_=Jgo&I1cxt-&`Uu
> zWC>8a3{wrO+GKDQB)v?8q*w5ztQh!I8)q*|B0TwY_Aj>DI=T)QE`>`*qn_s{R+$ez
> zsIdiLt852$AeMLt&i<7f6=MXKBrO@N&1kxTp$8r?wAzMkd@sas*3SNUbA4lEI5T(L
> zP~q_fPzksuuNeAhE2lu450{@N;Pn%@<O6svk0PGs^Q-K8>3a#}RrZ7Q1Fqx3b+{l0
> zL4Vb-+M}TI$Q7;7TU@C;v&ue`K9dw>VK1<+%KExl5UUes;eiq(qV<h(^ICc>fwG_m
> zZdS-m@t7MP8~MQviu4nE9Ya3~`cZ4j_)P3sy4_|ec*zw!vBc<jq&9N+FmleGU0uV>
> zQeM&lNfD!0^3^p|RGMasrPNSCj9zJOm^p`48u|^;KD6}eTFiN$xWhxgOG^JCy9;FQ
> zoG0-B)Fx&`#Z_-`I>nm7U8~gwe>S$~`OtY$Nj~fFJ^xXM$6(Xn-RW!smtd=}_O=j|
> z#4KnuZy2!M|Dd<U`q^HG%OkN3+s`v2u8g2;Z;Qu9lFCpbjU+m3`=3AD<SJku!qaU4
> zf4772o^CVnx1V|)c+eSm&f^a*OEg+K^prfzYJHMd@KK;gksc*Qo2!3<tJ+WQ^1UwR
> zaB{#?+H(C9y_G~jE1#X_sV@_dq!NZ3@)gN=9G~@m>RA`(+$DNIVrRcGh0`pHO;2Mz
> zplYMwTG`o?eD2F>NJ??^!|^W__|s;>WTHXP@U*OJ)NS~jXucOnW+**jKtl@nQ?A{z
> z*;n0g6j+>Eb&?OGSB^!;^ZS#d7bc<?g6IVt-(>WH_V(xnXWyi`>&K$=*#8l}I1#=W
> zgfAWoUyKM}9ELZJhsS#VlV@L=2ww`qmyU%mMT9Ro`|C2!-aZ~)4#SHhL`~r#jLKk2
> zO9Y#(rNT9)p`wU(S2fd=ZrYM=nPrAOP`B2bUGV&DiYuU?AN+Ov?o&#*;3a|YAt%G2
> zqn*($Jkn#Ue%!;_`CJ-+#&wI5eS!Z|TT5U(9q<6z?j3BmK6o!y;?KM%6l-YgL8s4m
> zJw64GHreL$G>Y$FBb0!$e3M~@FZ`+3+48r70eaBqnBUFz%C9~4TnXM>df=(ei{mP`
> zU%qJ8tZ=6?R@2It^UGn^&X{-paBt^fu6ysk|N95O&F$Rz^xlJ=tvjEBHWy?`KiM;l
> zW(%7ECaJ^BpnR1RrmZ(&tiyRY@00HE@Wn5md1T>xrd@6H=Xz$nX)`kzpU;SSezLFi
> z%$9jGlyV8h$nAK{bIy~K0n;v;En3d8TmP?J2ppz*X4Q6?&u0YM)5`NEW=v8HL0}cL
> zwY-=u@%&dm2f)sM?Dykg_!HtGdx#Z>4EZz;wh@xENY+B~qefLoFCaOTo;1t1hS)Pm
> z)_k*7sgV&84RVA;qeccqG|0&giHRUvVgfZ;_qAsvBmEBXED&Y`A-g}45Y-@Eqi;<0
> zNDMgTQZajuzw-pIhT|#IRB4(t#bA1+BIoGd#W^84jkZJbBo4^+DKe>ja!yL0#!0H-
> z4w2c2Twc^ZFs)lAJH7!vO5O1`{mfjq+wvQryXJcJrvE!s>Y+<*!K|+3u|0+-N^B<{
> z6YQ`dM6G7rRjV2#f2OQk^?V+WG-h%mp9k%lTg?~1dH?qK78Z8e34x_I+s^q30U<OM
> zEsJnFOdU*B?KBEXaQm_>u5*+~bozr>i($y6@HHI3m5{p{zOIF@a`>u*FFkx2;j0?H
> zRzR(0hoOU8HeC@bQ~ZxlGZ4djGwmNe@XGf({W{GQl(TS;e?ax?^oY*{yZ!IUGr)20
> zsr}I}27xAMugEHghkySuP{2R_OIJf(#-$CfhR3P}$tP+AUijN(o(eCs8O~SPcdx#C
> z#pLjQY1OP&xIsrt4&D!PwKwX`o4(*Bykr_)$UeS}#mX+Zvk95uppjm5XBC;pun-fs
> zL>iia+W6aT+9zlKhJ1;xAy%t8Y7jgTTn#e1wei!!zk*jRhf~2TL2w6r#dFT%bih#k
> zzOFi7lynQvTkzA;u32uwfVu}DL*FvU*lo}<E*@uwOmz*qp-~}R{7EA=X~ZUt7#b~O
> zq#{lif2bNo!rN_T916MVaV#Di+Lgl6cf?LQVkaH3lZJMS!J;0GCm*qskJ!mWyCu4B
> z9UEu-h+X`MU3_S_oOshNal|fh#4gd*>T&9LG3<d|IH1kAOC%$*aJQf8b$N{FVn|Cw
> zmqJ=5dNHKqL@$MOg6QSD3U%b!<kT=p@Z)x(R0Pd5Xp0j^A8(i(=IOlDEV}#y=kw0v
> zbHqs01aj08@X;WuffUw8JW%fvpbXb}(AKc%VzUf-BYj#pk57d5*h+I$Ch=kGIW0l`
> zG^U#O=#VQF)MDDE?0g=-!sv_K*}q^Hh76X57-*O#1p~V{5k*0ih;a5lwhLF$(B5bc
> zzv$6K0&O-pO7x{d0Z2{IBDp0fUbHY2Lh*wy0xG=51vDN7EeWrfq@mYDBvg2v5>Whj
> zeft0wg;zyDl}M=YniSAv6m&^=<pRnhp~5TH$Z%SzDCn~AnikM>Bvg3K2xuk>dPR7h
> z7SQQPsPKxPP~li-qM%oWS5-jONT~3d70_%H^qTNGE1<KHP~mk>K<A>MW#JW1emE^H
> z5-Pl&5YQ7*(2DSSQb12eLWS2;0(vS6std340y-ZF6<$vZ=;<h^A-tXu&@+)x;q|P5
> zo{fT5h1Yik^qokk@cOQRz8eKy5nkUD(Dx#t!s|H!J@+=KRy%9lY_?<l^llfw;|{3S
> z-3=HO8LVa@lOslGt|NmLDb(x{Lz>KPH--NdJg~8>47nI_Bb|&K7KfpZVl9ny6bw^*
> zsH14hBOQgqL>=lV-WAf#>~^ubVgov`G2sM{cL3zadhF2$Hl~iy<A*BpW4%f|CV|lB
> z0)DK&PCOoGq2CMmu|AkF9!#OfW^!Qb6MC989<h<0=50*qY36uXM*1A_6Z-4O<F=3V
> cd&Hw&J4aV^{O2hBbBwkSd))>b5(bz42U8HCOaK4?
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/acpi-test-data/pc/SRAT.cpuhp_sparse b/tests/acpi-test-data/pc/SRAT.cpuhp_sparse
> new file mode 100644
> index 0000000000000000000000000000000000000000..5377019cca32fb7a97111ea79d96d68a52627cd8
> GIT binary patch
> literal 280
> zcmWFzatx7RWME*-aq@Te2v%^42yhMtiUEZfKx_~V!f+sf!DmF1XF}sMqw!hL_^fDr
> gHe^1d2Ha*Sg9|QzT^-PYVDNz*rVzV2m@IZ00Pd>^0RR91
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> index 0a80ddf..15fa3c9 100644
> --- a/tests/bios-tables-test.c
> +++ b/tests/bios-tables-test.c
> @@ -781,6 +781,20 @@ static void test_acpi_q35_tcg_bridge(void)
> free_test_data(&data);
> }
>
> +static void test_acpi_piix4_tcg_cpuhp_sparse(void)
> +{
> + test_data data;
> +
> + memset(&data, 0, sizeof(data));
> + data.machine = MACHINE_PC;
> + data.variant = ".cpuhp_sparse";
> + test_acpi_one("-machine accel=tcg"
> + " -numa node -smp 1,sockets=2,cores=3,maxcpus=6",
> + &data);
> + free_test_data(&data);
> +}
> +
> +
> int main(int argc, char *argv[])
> {
> const char *arch = qtest_get_arch();
> @@ -797,6 +811,8 @@ int main(int argc, char *argv[])
> qtest_add_func("acpi/piix4/tcg/bridge", test_acpi_piix4_tcg_bridge);
> qtest_add_func("acpi/q35/tcg", test_acpi_q35_tcg);
> qtest_add_func("acpi/q35/tcg/bridge", test_acpi_q35_tcg_bridge);
> + qtest_add_func("acpi/piix4/tcg/cpuhp_sparse",
> + test_acpi_piix4_tcg_cpuhp_sparse);
> }
> ret = g_test_run();
> boot_sector_cleanup(disk);
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v4 02/10] pc: init pcms->apic_id_limit once and use it throughout pc.c
2016-02-26 13:59 [Qemu-devel] [PATCH v4 00/10] pc: do not create invalid MADT.LAPIC/Processor entries Igor Mammedov
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 01/10] tests: pc: acpi: piix4: add sparse CPU hotplug case Igor Mammedov
@ 2016-02-26 13:59 ` Igor Mammedov
2016-02-28 20:11 ` Marcel Apfelbaum
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 03/10] machine: introduce MachineClass.possible_cpu_arch_ids() hook Igor Mammedov
` (7 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Igor Mammedov @ 2016-02-26 13:59 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, ehabkost, mst
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/i386/pc.c | 45 +++++++++++++++++++--------------------------
1 file changed, 19 insertions(+), 26 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0aeefd2..151a64c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -700,18 +700,6 @@ static uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
}
}
-/* Calculates the limit to CPU APIC ID values
- *
- * This function returns the limit for the APIC ID value, so that all
- * CPU APIC IDs are < pc_apic_id_limit().
- *
- * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
- */
-static unsigned int pc_apic_id_limit(unsigned int max_cpus)
-{
- return x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
-}
-
static void pc_build_smbios(FWCfgState *fw_cfg)
{
uint8_t *smbios_tables, *smbios_anchor;
@@ -749,12 +737,11 @@ static void pc_build_smbios(FWCfgState *fw_cfg)
}
}
-static FWCfgState *bochs_bios_init(AddressSpace *as)
+static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
{
FWCfgState *fw_cfg;
uint64_t *numa_fw_cfg;
int i, j;
- unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
fw_cfg = fw_cfg_init_io_dma(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 4, as);
@@ -772,7 +759,7 @@ static FWCfgState *bochs_bios_init(AddressSpace *as)
* [1] The only kind of "CPU identifier" used between SeaBIOS and QEMU is
* the APIC ID, not the "CPU index"
*/
- fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)apic_id_limit);
+ fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)pcms->apic_id_limit);
fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES,
acpi_tables, acpi_tables_len);
@@ -790,11 +777,11 @@ static FWCfgState *bochs_bios_init(AddressSpace *as)
* of nodes, one word for each VCPU->node and one word for each node to
* hold the amount of memory.
*/
- numa_fw_cfg = g_new0(uint64_t, 1 + apic_id_limit + nb_numa_nodes);
+ numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes);
numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
for (i = 0; i < max_cpus; i++) {
unsigned int apic_id = x86_cpu_apic_id_from_index(i);
- assert(apic_id < apic_id_limit);
+ assert(apic_id < pcms->apic_id_limit);
for (j = 0; j < nb_numa_nodes; j++) {
if (test_bit(i, numa_info[j].node_cpu)) {
numa_fw_cfg[apic_id + 1] = cpu_to_le64(j);
@@ -803,10 +790,11 @@ static FWCfgState *bochs_bios_init(AddressSpace *as)
}
}
for (i = 0; i < nb_numa_nodes; i++) {
- numa_fw_cfg[apic_id_limit + 1 + i] = cpu_to_le64(numa_info[i].node_mem);
+ numa_fw_cfg[pcms->apic_id_limit + 1 + i] =
+ cpu_to_le64(numa_info[i].node_mem);
}
fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, numa_fw_cfg,
- (1 + apic_id_limit + nb_numa_nodes) *
+ (1 + pcms->apic_id_limit + nb_numa_nodes) *
sizeof(*numa_fw_cfg));
return fw_cfg;
@@ -1120,7 +1108,6 @@ void pc_cpus_init(PCMachineState *pcms)
int i;
X86CPU *cpu = NULL;
MachineState *machine = MACHINE(pcms);
- unsigned long apic_id_limit;
/* init CPUs */
if (machine->cpu_model == NULL) {
@@ -1131,10 +1118,17 @@ void pc_cpus_init(PCMachineState *pcms)
#endif
}
- apic_id_limit = pc_apic_id_limit(max_cpus);
- if (apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
- error_report("max_cpus is too large. APIC ID of last CPU is %lu",
- apic_id_limit - 1);
+ /* Calculates the limit to CPU APIC ID values
+ *
+ * Limit for the APIC ID value, so that all
+ * CPU APIC IDs are < pcms->apic_id_limit.
+ *
+ * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
+ */
+ pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
+ if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
+ error_report("max_cpus is too large. APIC ID of last CPU is %u",
+ pcms->apic_id_limit - 1);
exit(1);
}
@@ -1187,7 +1181,6 @@ void pc_guest_info_init(PCMachineState *pcms)
{
int i, j;
- pcms->apic_id_limit = pc_apic_id_limit(max_cpus);
pcms->apic_xrupt_override = kvm_allows_irq0_override();
pcms->numa_nodes = nb_numa_nodes;
pcms->node_mem = g_malloc0(pcms->numa_nodes *
@@ -1372,7 +1365,7 @@ void pc_memory_init(PCMachineState *pcms,
option_rom_mr,
1);
- fw_cfg = bochs_bios_init(&address_space_memory);
+ fw_cfg = bochs_bios_init(&address_space_memory, pcms);
rom_set_fw(fw_cfg);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH v4 02/10] pc: init pcms->apic_id_limit once and use it throughout pc.c
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 02/10] pc: init pcms->apic_id_limit once and use it throughout pc.c Igor Mammedov
@ 2016-02-28 20:11 ` Marcel Apfelbaum
0 siblings, 0 replies; 25+ messages in thread
From: Marcel Apfelbaum @ 2016-02-28 20:11 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel; +Cc: marcel, ehabkost, mst
On 02/26/2016 03:59 PM, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/i386/pc.c | 45 +++++++++++++++++++--------------------------
> 1 file changed, 19 insertions(+), 26 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 0aeefd2..151a64c 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -700,18 +700,6 @@ static uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
> }
> }
>
> -/* Calculates the limit to CPU APIC ID values
> - *
> - * This function returns the limit for the APIC ID value, so that all
> - * CPU APIC IDs are < pc_apic_id_limit().
> - *
> - * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
> - */
> -static unsigned int pc_apic_id_limit(unsigned int max_cpus)
> -{
> - return x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
> -}
> -
> static void pc_build_smbios(FWCfgState *fw_cfg)
> {
> uint8_t *smbios_tables, *smbios_anchor;
> @@ -749,12 +737,11 @@ static void pc_build_smbios(FWCfgState *fw_cfg)
> }
> }
>
> -static FWCfgState *bochs_bios_init(AddressSpace *as)
> +static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
> {
> FWCfgState *fw_cfg;
> uint64_t *numa_fw_cfg;
> int i, j;
> - unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
>
> fw_cfg = fw_cfg_init_io_dma(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 4, as);
>
> @@ -772,7 +759,7 @@ static FWCfgState *bochs_bios_init(AddressSpace *as)
> * [1] The only kind of "CPU identifier" used between SeaBIOS and QEMU is
> * the APIC ID, not the "CPU index"
> */
> - fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)apic_id_limit);
> + fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)pcms->apic_id_limit);
> fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES,
> acpi_tables, acpi_tables_len);
> @@ -790,11 +777,11 @@ static FWCfgState *bochs_bios_init(AddressSpace *as)
> * of nodes, one word for each VCPU->node and one word for each node to
> * hold the amount of memory.
> */
> - numa_fw_cfg = g_new0(uint64_t, 1 + apic_id_limit + nb_numa_nodes);
> + numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes);
> numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
> for (i = 0; i < max_cpus; i++) {
> unsigned int apic_id = x86_cpu_apic_id_from_index(i);
> - assert(apic_id < apic_id_limit);
> + assert(apic_id < pcms->apic_id_limit);
> for (j = 0; j < nb_numa_nodes; j++) {
> if (test_bit(i, numa_info[j].node_cpu)) {
> numa_fw_cfg[apic_id + 1] = cpu_to_le64(j);
> @@ -803,10 +790,11 @@ static FWCfgState *bochs_bios_init(AddressSpace *as)
> }
> }
> for (i = 0; i < nb_numa_nodes; i++) {
> - numa_fw_cfg[apic_id_limit + 1 + i] = cpu_to_le64(numa_info[i].node_mem);
> + numa_fw_cfg[pcms->apic_id_limit + 1 + i] =
> + cpu_to_le64(numa_info[i].node_mem);
> }
> fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, numa_fw_cfg,
> - (1 + apic_id_limit + nb_numa_nodes) *
> + (1 + pcms->apic_id_limit + nb_numa_nodes) *
> sizeof(*numa_fw_cfg));
>
> return fw_cfg;
> @@ -1120,7 +1108,6 @@ void pc_cpus_init(PCMachineState *pcms)
> int i;
> X86CPU *cpu = NULL;
> MachineState *machine = MACHINE(pcms);
> - unsigned long apic_id_limit;
>
> /* init CPUs */
> if (machine->cpu_model == NULL) {
> @@ -1131,10 +1118,17 @@ void pc_cpus_init(PCMachineState *pcms)
> #endif
> }
>
> - apic_id_limit = pc_apic_id_limit(max_cpus);
> - if (apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
> - error_report("max_cpus is too large. APIC ID of last CPU is %lu",
> - apic_id_limit - 1);
> + /* Calculates the limit to CPU APIC ID values
> + *
> + * Limit for the APIC ID value, so that all
> + * CPU APIC IDs are < pcms->apic_id_limit.
> + *
> + * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
> + */
> + pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
> + if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
> + error_report("max_cpus is too large. APIC ID of last CPU is %u",
> + pcms->apic_id_limit - 1);
> exit(1);
> }
>
> @@ -1187,7 +1181,6 @@ void pc_guest_info_init(PCMachineState *pcms)
> {
> int i, j;
>
> - pcms->apic_id_limit = pc_apic_id_limit(max_cpus);
> pcms->apic_xrupt_override = kvm_allows_irq0_override();
> pcms->numa_nodes = nb_numa_nodes;
> pcms->node_mem = g_malloc0(pcms->numa_nodes *
> @@ -1372,7 +1365,7 @@ void pc_memory_init(PCMachineState *pcms,
> option_rom_mr,
> 1);
>
> - fw_cfg = bochs_bios_init(&address_space_memory);
> + fw_cfg = bochs_bios_init(&address_space_memory, pcms);
>
> rom_set_fw(fw_cfg);
>
>
Looks OK to me.
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Thanks,
Marcel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v4 03/10] machine: introduce MachineClass.possible_cpu_arch_ids() hook
2016-02-26 13:59 [Qemu-devel] [PATCH v4 00/10] pc: do not create invalid MADT.LAPIC/Processor entries Igor Mammedov
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 01/10] tests: pc: acpi: piix4: add sparse CPU hotplug case Igor Mammedov
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 02/10] pc: init pcms->apic_id_limit once and use it throughout pc.c Igor Mammedov
@ 2016-02-26 13:59 ` Igor Mammedov
2016-03-03 11:27 ` Marcel Apfelbaum
2016-03-03 14:28 ` [Qemu-devel] [PATCH v5 " Igor Mammedov
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 04/10] pc: acpi: cleanup qdev_get_machine() calls Igor Mammedov
` (6 subsequent siblings)
9 siblings, 2 replies; 25+ messages in thread
From: Igor Mammedov @ 2016-02-26 13:59 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, ehabkost, mst
on x86 currently range 0..max_cpus is used to generate
architecture-dependent CPU ID (APIC Id) for each present
and possible CPUs. However architecture-dependent CPU IDs
list could be sparse and code that needs to enumerate
all IDs (ACPI) ended up doing guess work enumerating all
possible and impossible IDs up to
apic_id_limit = x86_cpu_apic_id_from_index(max_cpus).
That leads to creation of MADT entries and Processor
objects in ACPI tables for not possible CPUs.
Fix it by allowing board specify a concrete list of
CPU IDs accourding its own rules (which for x86 depends
on topology). So that code that needs this list could
request it from board instead of trying to guess
what IDs are correct on its own.
This interface will also allow to help making AML
part of CPU hotplug target independent so it could
be reused for ARM target.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/i386/pc.c | 44 ++++++++++++++++++++++++++++++++++++++++----
include/hw/boards.h | 26 ++++++++++++++++++++++++++
include/hw/i386/pc.h | 1 +
3 files changed, 67 insertions(+), 4 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 151a64c..5ce9295 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1132,10 +1132,17 @@ void pc_cpus_init(PCMachineState *pcms)
exit(1);
}
- for (i = 0; i < smp_cpus; i++) {
- cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i),
- &error_fatal);
- object_unref(OBJECT(cpu));
+ pcms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
+ sizeof(CPUArchId) * (max_cpus - 1));
+ for (i = 0; i < max_cpus; i++) {
+ pcms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
+ pcms->possible_cpus->len++;
+ if (i < smp_cpus) {
+ cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i),
+ &error_fatal);
+ pcms->possible_cpus->cpus[i].cpu = CPU(cpu);
+ object_unref(OBJECT(cpu));
+ }
}
/* tell smbios about cpuid version and features */
@@ -1658,9 +1665,19 @@ static void pc_dimm_unplug(HotplugHandler *hotplug_dev,
error_propagate(errp, local_err);
}
+static int pc_apic_cmp(const void *a, const void *b)
+{
+ CPUArchId *apic_a = (CPUArchId *)a;
+ CPUArchId *apic_b = (CPUArchId *)b;
+
+ return apic_a->arch_id - apic_b->arch_id;
+}
+
static void pc_cpu_plug(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
+ CPUClass *cc = CPU_GET_CLASS(dev);
+ CPUArchId apic_id, *found_cpu;
HotplugHandlerClass *hhc;
Error *local_err = NULL;
PCMachineState *pcms = PC_MACHINE(hotplug_dev);
@@ -1683,6 +1700,13 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
/* increment the number of CPUs */
rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) + 1);
+
+ apic_id.arch_id = cc->get_arch_id(CPU(dev));
+ found_cpu = bsearch(&apic_id, pcms->possible_cpus->cpus,
+ pcms->possible_cpus->len, sizeof(*pcms->possible_cpus->cpus),
+ pc_apic_cmp);
+ assert(found_cpu);
+ found_cpu->cpu = CPU(dev);
out:
error_propagate(errp, local_err);
}
@@ -1925,6 +1949,17 @@ static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index)
return topo.pkg_id;
}
+static CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *machine)
+{
+ PCMachineState *pcms = PC_MACHINE(machine);
+ int len = sizeof(CPUArchIdList) +
+ sizeof(CPUArchId) * (pcms->possible_cpus->len - 1);
+ CPUArchIdList *list = g_malloc(len);
+
+ memcpy(list, pcms->possible_cpus, len);
+ return list;
+}
+
static void pc_machine_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
@@ -1947,6 +1982,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
pcmc->save_tsc_khz = true;
mc->get_hotplug_handler = pc_get_hotpug_handler;
mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
+ mc->possible_cpu_arch_ids = pc_possible_cpu_arch_ids;
mc->default_boot_order = "cad";
mc->hot_add_cpu = pc_hot_add_cpu;
mc->max_cpus = 255;
diff --git a/include/hw/boards.h b/include/hw/boards.h
index de3b3bd..c9b11d4 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -8,6 +8,7 @@
#include "sysemu/accel.h"
#include "hw/qdev.h"
#include "qom/object.h"
+#include "qom/cpu.h"
void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
const char *name,
@@ -42,6 +43,26 @@ bool machine_dump_guest_core(MachineState *machine);
bool machine_mem_merge(MachineState *machine);
/**
+ * CPUArchId:
+ * @arch_id - architecture-dependent CPU ID of present or possible CPU
+ * @cpu - pointer to corresponding CPU object if it's present on NULL otherwise
+ */
+typedef struct {
+ uint64_t arch_id;
+ struct CPUState *cpu;
+} CPUArchId;
+
+/**
+ * CPUArchIdList:
+ * @len - number of @CPUArchId items in @cpus array
+ * @cpus - array of present or possible CPUs for current machine configuration
+ */
+typedef struct {
+ int len;
+ CPUArchId cpus[1];
+} CPUArchIdList;
+
+/**
* MachineClass:
* @get_hotplug_handler: this function is called during bus-less
* device hotplug. If defined it returns pointer to an instance
@@ -57,6 +78,10 @@ bool machine_mem_merge(MachineState *machine);
* Set only by old machines because they need to keep
* compatibility on code that exposed QEMU_VERSION to guests in
* the past (and now use qemu_hw_version()).
+ * @possible_cpu_arch_ids:
+ * Returns an array of @CPUArchId architecture-dependent CPU IDs
+ * which includes CPU IDs for present and possible to hotplug CPUs.
+ * Caller is responsible for freeing returned list.
*/
struct MachineClass {
/*< private >*/
@@ -98,6 +123,7 @@ struct MachineClass {
HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
DeviceState *dev);
unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
+ CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
};
/**
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 8b3546e..3e09232 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -65,6 +65,7 @@ struct PCMachineState {
/* CPU and apic information: */
bool apic_xrupt_override;
unsigned apic_id_limit;
+ CPUArchIdList *possible_cpus;
/* NUMA information: */
uint64_t numa_nodes;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH v4 03/10] machine: introduce MachineClass.possible_cpu_arch_ids() hook
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 03/10] machine: introduce MachineClass.possible_cpu_arch_ids() hook Igor Mammedov
@ 2016-03-03 11:27 ` Marcel Apfelbaum
2016-03-03 14:13 ` Igor Mammedov
2016-03-03 14:28 ` [Qemu-devel] [PATCH v5 " Igor Mammedov
1 sibling, 1 reply; 25+ messages in thread
From: Marcel Apfelbaum @ 2016-03-03 11:27 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel; +Cc: ehabkost, mst
On 02/26/2016 03:59 PM, Igor Mammedov wrote:
> on x86 currently range 0..max_cpus is used to generate
> architecture-dependent CPU ID (APIC Id) for each present
> and possible CPUs. However architecture-dependent CPU IDs
> list could be sparse and code that needs to enumerate
> all IDs (ACPI) ended up doing guess work enumerating all
> possible and impossible IDs up to
> apic_id_limit = x86_cpu_apic_id_from_index(max_cpus).
>
> That leads to creation of MADT entries and Processor
> objects in ACPI tables for not possible CPUs.
> Fix it by allowing board specify a concrete list of
> CPU IDs accourding its own rules (which for x86 depends
> on topology). So that code that needs this list could
> request it from board instead of trying to guess
> what IDs are correct on its own.
>
> This interface will also allow to help making AML
> part of CPU hotplug target independent so it could
> be reused for ARM target.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/i386/pc.c | 44 ++++++++++++++++++++++++++++++++++++++++----
> include/hw/boards.h | 26 ++++++++++++++++++++++++++
> include/hw/i386/pc.h | 1 +
> 3 files changed, 67 insertions(+), 4 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 151a64c..5ce9295 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1132,10 +1132,17 @@ void pc_cpus_init(PCMachineState *pcms)
> exit(1);
> }
>
> - for (i = 0; i < smp_cpus; i++) {
> - cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i),
> - &error_fatal);
> - object_unref(OBJECT(cpu));
> + pcms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
> + sizeof(CPUArchId) * (max_cpus - 1));
> + for (i = 0; i < max_cpus; i++) {
> + pcms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
> + pcms->possible_cpus->len++;
> + if (i < smp_cpus) {
> + cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i),
> + &error_fatal);
> + pcms->possible_cpus->cpus[i].cpu = CPU(cpu);
> + object_unref(OBJECT(cpu));
> + }
> }
>
> /* tell smbios about cpuid version and features */
> @@ -1658,9 +1665,19 @@ static void pc_dimm_unplug(HotplugHandler *hotplug_dev,
> error_propagate(errp, local_err);
> }
>
> +static int pc_apic_cmp(const void *a, const void *b)
> +{
> + CPUArchId *apic_a = (CPUArchId *)a;
> + CPUArchId *apic_b = (CPUArchId *)b;
> +
> + return apic_a->arch_id - apic_b->arch_id;
> +}
> +
> static void pc_cpu_plug(HotplugHandler *hotplug_dev,
> DeviceState *dev, Error **errp)
> {
> + CPUClass *cc = CPU_GET_CLASS(dev);
> + CPUArchId apic_id, *found_cpu;
> HotplugHandlerClass *hhc;
> Error *local_err = NULL;
> PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> @@ -1683,6 +1700,13 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
>
> /* increment the number of CPUs */
> rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) + 1);
> +
> + apic_id.arch_id = cc->get_arch_id(CPU(dev));
> + found_cpu = bsearch(&apic_id, pcms->possible_cpus->cpus,
> + pcms->possible_cpus->len, sizeof(*pcms->possible_cpus->cpus),
> + pc_apic_cmp);
> + assert(found_cpu);
> + found_cpu->cpu = CPU(dev);
> out:
> error_propagate(errp, local_err);
> }
> @@ -1925,6 +1949,17 @@ static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index)
> return topo.pkg_id;
> }
>
> +static CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *machine)
> +{
> + PCMachineState *pcms = PC_MACHINE(machine);
> + int len = sizeof(CPUArchIdList) +
> + sizeof(CPUArchId) * (pcms->possible_cpus->len - 1);
> + CPUArchIdList *list = g_malloc(len);
> +
> + memcpy(list, pcms->possible_cpus, len);
> + return list;
> +}
> +
> static void pc_machine_class_init(ObjectClass *oc, void *data)
> {
> MachineClass *mc = MACHINE_CLASS(oc);
> @@ -1947,6 +1982,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
> pcmc->save_tsc_khz = true;
> mc->get_hotplug_handler = pc_get_hotpug_handler;
> mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
> + mc->possible_cpu_arch_ids = pc_possible_cpu_arch_ids;
> mc->default_boot_order = "cad";
> mc->hot_add_cpu = pc_hot_add_cpu;
> mc->max_cpus = 255;
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index de3b3bd..c9b11d4 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -8,6 +8,7 @@
> #include "sysemu/accel.h"
> #include "hw/qdev.h"
> #include "qom/object.h"
> +#include "qom/cpu.h"
>
> void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
> const char *name,
> @@ -42,6 +43,26 @@ bool machine_dump_guest_core(MachineState *machine);
> bool machine_mem_merge(MachineState *machine);
>
> /**
> + * CPUArchId:
> + * @arch_id - architecture-dependent CPU ID of present or possible CPU
> + * @cpu - pointer to corresponding CPU object if it's present on NULL otherwise
> + */
> +typedef struct {
> + uint64_t arch_id;
> + struct CPUState *cpu;
> +} CPUArchId;
> +
> +/**
> + * CPUArchIdList:
> + * @len - number of @CPUArchId items in @cpus array
> + * @cpus - array of present or possible CPUs for current machine configuration
> + */
> +typedef struct {
> + int len;
> + CPUArchId cpus[1];
Hi Igor,
The patch looks good to me,
I just have one question. Why do you have cpus[1] in CPUArchIdList and not cpus[0]?
I suppose is to say that every machine has at least one cpu..., on the other
hand that leads to "not standard" usage like when allocating cpus you need a "-1" and so on.
Thanks,
Marcel
> +} CPUArchIdList;
> +
> +/**
> * MachineClass:
> * @get_hotplug_handler: this function is called during bus-less
> * device hotplug. If defined it returns pointer to an instance
> @@ -57,6 +78,10 @@ bool machine_mem_merge(MachineState *machine);
> * Set only by old machines because they need to keep
> * compatibility on code that exposed QEMU_VERSION to guests in
> * the past (and now use qemu_hw_version()).
> + * @possible_cpu_arch_ids:
> + * Returns an array of @CPUArchId architecture-dependent CPU IDs
> + * which includes CPU IDs for present and possible to hotplug CPUs.
> + * Caller is responsible for freeing returned list.
> */
> struct MachineClass {
> /*< private >*/
> @@ -98,6 +123,7 @@ struct MachineClass {
> HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
> DeviceState *dev);
> unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
> + CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
> };
>
> /**
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 8b3546e..3e09232 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -65,6 +65,7 @@ struct PCMachineState {
> /* CPU and apic information: */
> bool apic_xrupt_override;
> unsigned apic_id_limit;
> + CPUArchIdList *possible_cpus;
>
> /* NUMA information: */
> uint64_t numa_nodes;
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH v4 03/10] machine: introduce MachineClass.possible_cpu_arch_ids() hook
2016-03-03 11:27 ` Marcel Apfelbaum
@ 2016-03-03 14:13 ` Igor Mammedov
0 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2016-03-03 14:13 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: qemu-devel, ehabkost, mst
On Thu, 3 Mar 2016 13:27:27 +0200
Marcel Apfelbaum <marcel@redhat.com> wrote:
> On 02/26/2016 03:59 PM, Igor Mammedov wrote:
> > on x86 currently range 0..max_cpus is used to generate
> > architecture-dependent CPU ID (APIC Id) for each present
> > and possible CPUs. However architecture-dependent CPU IDs
> > list could be sparse and code that needs to enumerate
> > all IDs (ACPI) ended up doing guess work enumerating all
> > possible and impossible IDs up to
> > apic_id_limit = x86_cpu_apic_id_from_index(max_cpus).
> >
> > That leads to creation of MADT entries and Processor
> > objects in ACPI tables for not possible CPUs.
> > Fix it by allowing board specify a concrete list of
> > CPU IDs accourding its own rules (which for x86 depends
> > on topology). So that code that needs this list could
> > request it from board instead of trying to guess
> > what IDs are correct on its own.
> >
> > This interface will also allow to help making AML
> > part of CPU hotplug target independent so it could
> > be reused for ARM target.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > hw/i386/pc.c | 44 ++++++++++++++++++++++++++++++++++++++++----
> > include/hw/boards.h | 26 ++++++++++++++++++++++++++
> > include/hw/i386/pc.h | 1 +
> > 3 files changed, 67 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 151a64c..5ce9295 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -1132,10 +1132,17 @@ void pc_cpus_init(PCMachineState *pcms)
> > exit(1);
> > }
> >
> > - for (i = 0; i < smp_cpus; i++) {
> > - cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i),
> > - &error_fatal);
> > - object_unref(OBJECT(cpu));
> > + pcms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
> > + sizeof(CPUArchId) * (max_cpus - 1));
> > + for (i = 0; i < max_cpus; i++) {
> > + pcms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
> > + pcms->possible_cpus->len++;
> > + if (i < smp_cpus) {
> > + cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i),
> > + &error_fatal);
> > + pcms->possible_cpus->cpus[i].cpu = CPU(cpu);
> > + object_unref(OBJECT(cpu));
> > + }
> > }
> >
> > /* tell smbios about cpuid version and features */
> > @@ -1658,9 +1665,19 @@ static void pc_dimm_unplug(HotplugHandler *hotplug_dev,
> > error_propagate(errp, local_err);
> > }
> >
> > +static int pc_apic_cmp(const void *a, const void *b)
> > +{
> > + CPUArchId *apic_a = (CPUArchId *)a;
> > + CPUArchId *apic_b = (CPUArchId *)b;
> > +
> > + return apic_a->arch_id - apic_b->arch_id;
> > +}
> > +
> > static void pc_cpu_plug(HotplugHandler *hotplug_dev,
> > DeviceState *dev, Error **errp)
> > {
> > + CPUClass *cc = CPU_GET_CLASS(dev);
> > + CPUArchId apic_id, *found_cpu;
> > HotplugHandlerClass *hhc;
> > Error *local_err = NULL;
> > PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> > @@ -1683,6 +1700,13 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
> >
> > /* increment the number of CPUs */
> > rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) + 1);
> > +
> > + apic_id.arch_id = cc->get_arch_id(CPU(dev));
> > + found_cpu = bsearch(&apic_id, pcms->possible_cpus->cpus,
> > + pcms->possible_cpus->len, sizeof(*pcms->possible_cpus->cpus),
> > + pc_apic_cmp);
> > + assert(found_cpu);
> > + found_cpu->cpu = CPU(dev);
> > out:
> > error_propagate(errp, local_err);
> > }
> > @@ -1925,6 +1949,17 @@ static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index)
> > return topo.pkg_id;
> > }
> >
> > +static CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *machine)
> > +{
> > + PCMachineState *pcms = PC_MACHINE(machine);
> > + int len = sizeof(CPUArchIdList) +
> > + sizeof(CPUArchId) * (pcms->possible_cpus->len - 1);
> > + CPUArchIdList *list = g_malloc(len);
> > +
> > + memcpy(list, pcms->possible_cpus, len);
> > + return list;
> > +}
> > +
> > static void pc_machine_class_init(ObjectClass *oc, void *data)
> > {
> > MachineClass *mc = MACHINE_CLASS(oc);
> > @@ -1947,6 +1982,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
> > pcmc->save_tsc_khz = true;
> > mc->get_hotplug_handler = pc_get_hotpug_handler;
> > mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
> > + mc->possible_cpu_arch_ids = pc_possible_cpu_arch_ids;
> > mc->default_boot_order = "cad";
> > mc->hot_add_cpu = pc_hot_add_cpu;
> > mc->max_cpus = 255;
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index de3b3bd..c9b11d4 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -8,6 +8,7 @@
> > #include "sysemu/accel.h"
> > #include "hw/qdev.h"
> > #include "qom/object.h"
> > +#include "qom/cpu.h"
> >
> > void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
> > const char *name,
> > @@ -42,6 +43,26 @@ bool machine_dump_guest_core(MachineState *machine);
> > bool machine_mem_merge(MachineState *machine);
> >
> > /**
> > + * CPUArchId:
> > + * @arch_id - architecture-dependent CPU ID of present or possible CPU
> > + * @cpu - pointer to corresponding CPU object if it's present on NULL otherwise
> > + */
> > +typedef struct {
> > + uint64_t arch_id;
> > + struct CPUState *cpu;
> > +} CPUArchId;
> > +
> > +/**
> > + * CPUArchIdList:
> > + * @len - number of @CPUArchId items in @cpus array
> > + * @cpus - array of present or possible CPUs for current machine configuration
> > + */
> > +typedef struct {
> > + int len;
> > + CPUArchId cpus[1];
>
> Hi Igor,
>
> The patch looks good to me,
> I just have one question. Why do you have cpus[1] in CPUArchIdList and not cpus[0]?
>
> I suppose is to say that every machine has at least one cpu..., on the other
> hand that leads to "not standard" usage like when allocating cpus you need a "-1" and so on.
I'll fix it up to CPUArchId cpus[0]; and post fixed up path v5
as reply to this patch.
Thanks,
Igor
>
> Thanks,
> Marcel
>
>
>
> > +} CPUArchIdList;
> > +
> > +/**
> > * MachineClass:
> > * @get_hotplug_handler: this function is called during bus-less
> > * device hotplug. If defined it returns pointer to an instance
> > @@ -57,6 +78,10 @@ bool machine_mem_merge(MachineState *machine);
> > * Set only by old machines because they need to keep
> > * compatibility on code that exposed QEMU_VERSION to guests in
> > * the past (and now use qemu_hw_version()).
> > + * @possible_cpu_arch_ids:
> > + * Returns an array of @CPUArchId architecture-dependent CPU IDs
> > + * which includes CPU IDs for present and possible to hotplug CPUs.
> > + * Caller is responsible for freeing returned list.
> > */
> > struct MachineClass {
> > /*< private >*/
> > @@ -98,6 +123,7 @@ struct MachineClass {
> > HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
> > DeviceState *dev);
> > unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
> > + CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
> > };
> >
> > /**
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index 8b3546e..3e09232 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -65,6 +65,7 @@ struct PCMachineState {
> > /* CPU and apic information: */
> > bool apic_xrupt_override;
> > unsigned apic_id_limit;
> > + CPUArchIdList *possible_cpus;
> >
> > /* NUMA information: */
> > uint64_t numa_nodes;
> >
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v5 03/10] machine: introduce MachineClass.possible_cpu_arch_ids() hook
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 03/10] machine: introduce MachineClass.possible_cpu_arch_ids() hook Igor Mammedov
2016-03-03 11:27 ` Marcel Apfelbaum
@ 2016-03-03 14:28 ` Igor Mammedov
[not found] ` <20160308132850.2fbe25bd@nial.brq.redhat.com>
2016-03-08 16:29 ` Igor Mammedov
1 sibling, 2 replies; 25+ messages in thread
From: Igor Mammedov @ 2016-03-03 14:28 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, ehabkost, mst
on x86 currently range 0..max_cpus is used to generate
architecture-dependent CPU ID (APIC Id) for each present
and possible CPUs. However architecture-dependent CPU IDs
list could be sparse and code that needs to enumerate
all IDs (ACPI) ended up doing guess work enumerating all
possible and impossible IDs up to
apic_id_limit = x86_cpu_apic_id_from_index(max_cpus).
That leads to creation of MADT entries and Processor
objects in ACPI tables for not possible CPUs.
Fix it by allowing board specify a concrete list of
CPU IDs accourding its own rules (which for x86 depends
on topology). So that code that needs this list could
request it from board instead of trying to guess
what IDs are correct on its own.
This interface will also allow to help making AML
part of CPU hotplug target independent so it could
be reused for ARM target.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v5:
s/CPUArchId cpus[1]/CPUArchId cpus[0]/ to make length
calculation simpler. Marcel Apfelbaum <marcel@redhat.com>
---
hw/i386/pc.c | 44 ++++++++++++++++++++++++++++++++++++++++----
include/hw/boards.h | 26 ++++++++++++++++++++++++++
include/hw/i386/pc.h | 1 +
3 files changed, 67 insertions(+), 4 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 151a64c..1af95a1 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1132,10 +1132,17 @@ void pc_cpus_init(PCMachineState *pcms)
exit(1);
}
- for (i = 0; i < smp_cpus; i++) {
- cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i),
- &error_fatal);
- object_unref(OBJECT(cpu));
+ pcms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
+ sizeof(CPUArchId) * max_cpus);
+ for (i = 0; i < max_cpus; i++) {
+ pcms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
+ pcms->possible_cpus->len++;
+ if (i < smp_cpus) {
+ cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i),
+ &error_fatal);
+ pcms->possible_cpus->cpus[i].cpu = CPU(cpu);
+ object_unref(OBJECT(cpu));
+ }
}
/* tell smbios about cpuid version and features */
@@ -1658,9 +1665,19 @@ static void pc_dimm_unplug(HotplugHandler *hotplug_dev,
error_propagate(errp, local_err);
}
+static int pc_apic_cmp(const void *a, const void *b)
+{
+ CPUArchId *apic_a = (CPUArchId *)a;
+ CPUArchId *apic_b = (CPUArchId *)b;
+
+ return apic_a->arch_id - apic_b->arch_id;
+}
+
static void pc_cpu_plug(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
+ CPUClass *cc = CPU_GET_CLASS(dev);
+ CPUArchId apic_id, *found_cpu;
HotplugHandlerClass *hhc;
Error *local_err = NULL;
PCMachineState *pcms = PC_MACHINE(hotplug_dev);
@@ -1683,6 +1700,13 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
/* increment the number of CPUs */
rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) + 1);
+
+ apic_id.arch_id = cc->get_arch_id(CPU(dev));
+ found_cpu = bsearch(&apic_id, pcms->possible_cpus->cpus,
+ pcms->possible_cpus->len, sizeof(*pcms->possible_cpus->cpus),
+ pc_apic_cmp);
+ assert(found_cpu);
+ found_cpu->cpu = CPU(dev);
out:
error_propagate(errp, local_err);
}
@@ -1925,6 +1949,17 @@ static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index)
return topo.pkg_id;
}
+static CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *machine)
+{
+ PCMachineState *pcms = PC_MACHINE(machine);
+ int len = sizeof(CPUArchIdList) +
+ sizeof(CPUArchId) * (pcms->possible_cpus->len);
+ CPUArchIdList *list = g_malloc(len);
+
+ memcpy(list, pcms->possible_cpus, len);
+ return list;
+}
+
static void pc_machine_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
@@ -1947,6 +1982,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
pcmc->save_tsc_khz = true;
mc->get_hotplug_handler = pc_get_hotpug_handler;
mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
+ mc->possible_cpu_arch_ids = pc_possible_cpu_arch_ids;
mc->default_boot_order = "cad";
mc->hot_add_cpu = pc_hot_add_cpu;
mc->max_cpus = 255;
diff --git a/include/hw/boards.h b/include/hw/boards.h
index b5d7eae..4b3fdbe 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -8,6 +8,7 @@
#include "sysemu/accel.h"
#include "hw/qdev.h"
#include "qom/object.h"
+#include "qom/cpu.h"
void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
const char *name,
@@ -42,6 +43,26 @@ bool machine_dump_guest_core(MachineState *machine);
bool machine_mem_merge(MachineState *machine);
/**
+ * CPUArchId:
+ * @arch_id - architecture-dependent CPU ID of present or possible CPU
+ * @cpu - pointer to corresponding CPU object if it's present on NULL otherwise
+ */
+typedef struct {
+ uint64_t arch_id;
+ struct CPUState *cpu;
+} CPUArchId;
+
+/**
+ * CPUArchIdList:
+ * @len - number of @CPUArchId items in @cpus array
+ * @cpus - array of present or possible CPUs for current machine configuration
+ */
+typedef struct {
+ int len;
+ CPUArchId cpus[0];
+} CPUArchIdList;
+
+/**
* MachineClass:
* @get_hotplug_handler: this function is called during bus-less
* device hotplug. If defined it returns pointer to an instance
@@ -57,6 +78,10 @@ bool machine_mem_merge(MachineState *machine);
* Set only by old machines because they need to keep
* compatibility on code that exposed QEMU_VERSION to guests in
* the past (and now use qemu_hw_version()).
+ * @possible_cpu_arch_ids:
+ * Returns an array of @CPUArchId architecture-dependent CPU IDs
+ * which includes CPU IDs for present and possible to hotplug CPUs.
+ * Caller is responsible for freeing returned list.
*/
struct MachineClass {
/*< private >*/
@@ -98,6 +123,7 @@ struct MachineClass {
HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
DeviceState *dev);
unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
+ CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
};
/**
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 8b3546e..3e09232 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -65,6 +65,7 @@ struct PCMachineState {
/* CPU and apic information: */
bool apic_xrupt_override;
unsigned apic_id_limit;
+ CPUArchIdList *possible_cpus;
/* NUMA information: */
uint64_t numa_nodes;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread[parent not found: <20160308132850.2fbe25bd@nial.brq.redhat.com>]
* Re: [Qemu-devel] [PATCH v5 03/10] machine: introduce MachineClass.possible_cpu_arch_ids() hook
2016-03-03 14:28 ` [Qemu-devel] [PATCH v5 " Igor Mammedov
[not found] ` <20160308132850.2fbe25bd@nial.brq.redhat.com>
@ 2016-03-08 16:29 ` Igor Mammedov
2016-03-08 17:40 ` Marcel Apfelbaum
1 sibling, 1 reply; 25+ messages in thread
From: Igor Mammedov @ 2016-03-08 16:29 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, ehabkost, mst
On Thu, 3 Mar 2016 15:28:56 +0100
Igor Mammedov <imammedo@redhat.com> wrote:
> on x86 currently range 0..max_cpus is used to generate
> architecture-dependent CPU ID (APIC Id) for each present
> and possible CPUs. However architecture-dependent CPU IDs
> list could be sparse and code that needs to enumerate
> all IDs (ACPI) ended up doing guess work enumerating all
> possible and impossible IDs up to
> apic_id_limit = x86_cpu_apic_id_from_index(max_cpus).
>
> That leads to creation of MADT entries and Processor
> objects in ACPI tables for not possible CPUs.
> Fix it by allowing board specify a concrete list of
> CPU IDs accourding its own rules (which for x86 depends
> on topology). So that code that needs this list could
> request it from board instead of trying to guess
> what IDs are correct on its own.
>
> This interface will also allow to help making AML
> part of CPU hotplug target independent so it could
> be reused for ARM target.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v5:
> s/CPUArchId cpus[1]/CPUArchId cpus[0]/ to make length
> calculation simpler. Marcel Apfelbaum <marcel@redhat.com>
Marcel, does v5 looks good to you?
/me, sorry for replying/asking out of thread the first time./
> ---
> hw/i386/pc.c | 44 ++++++++++++++++++++++++++++++++++++++++----
> include/hw/boards.h | 26 ++++++++++++++++++++++++++
> include/hw/i386/pc.h | 1 +
> 3 files changed, 67 insertions(+), 4 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 151a64c..1af95a1 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1132,10 +1132,17 @@ void pc_cpus_init(PCMachineState *pcms)
> exit(1);
> }
>
> - for (i = 0; i < smp_cpus; i++) {
> - cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i),
> - &error_fatal);
> - object_unref(OBJECT(cpu));
> + pcms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
> + sizeof(CPUArchId) * max_cpus);
> + for (i = 0; i < max_cpus; i++) {
> + pcms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
> + pcms->possible_cpus->len++;
> + if (i < smp_cpus) {
> + cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i),
> + &error_fatal);
> + pcms->possible_cpus->cpus[i].cpu = CPU(cpu);
> + object_unref(OBJECT(cpu));
> + }
> }
>
> /* tell smbios about cpuid version and features */
> @@ -1658,9 +1665,19 @@ static void pc_dimm_unplug(HotplugHandler *hotplug_dev,
> error_propagate(errp, local_err);
> }
>
> +static int pc_apic_cmp(const void *a, const void *b)
> +{
> + CPUArchId *apic_a = (CPUArchId *)a;
> + CPUArchId *apic_b = (CPUArchId *)b;
> +
> + return apic_a->arch_id - apic_b->arch_id;
> +}
> +
> static void pc_cpu_plug(HotplugHandler *hotplug_dev,
> DeviceState *dev, Error **errp)
> {
> + CPUClass *cc = CPU_GET_CLASS(dev);
> + CPUArchId apic_id, *found_cpu;
> HotplugHandlerClass *hhc;
> Error *local_err = NULL;
> PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> @@ -1683,6 +1700,13 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
>
> /* increment the number of CPUs */
> rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) + 1);
> +
> + apic_id.arch_id = cc->get_arch_id(CPU(dev));
> + found_cpu = bsearch(&apic_id, pcms->possible_cpus->cpus,
> + pcms->possible_cpus->len, sizeof(*pcms->possible_cpus->cpus),
> + pc_apic_cmp);
> + assert(found_cpu);
> + found_cpu->cpu = CPU(dev);
> out:
> error_propagate(errp, local_err);
> }
> @@ -1925,6 +1949,17 @@ static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index)
> return topo.pkg_id;
> }
>
> +static CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *machine)
> +{
> + PCMachineState *pcms = PC_MACHINE(machine);
> + int len = sizeof(CPUArchIdList) +
> + sizeof(CPUArchId) * (pcms->possible_cpus->len);
> + CPUArchIdList *list = g_malloc(len);
> +
> + memcpy(list, pcms->possible_cpus, len);
> + return list;
> +}
> +
> static void pc_machine_class_init(ObjectClass *oc, void *data)
> {
> MachineClass *mc = MACHINE_CLASS(oc);
> @@ -1947,6 +1982,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
> pcmc->save_tsc_khz = true;
> mc->get_hotplug_handler = pc_get_hotpug_handler;
> mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
> + mc->possible_cpu_arch_ids = pc_possible_cpu_arch_ids;
> mc->default_boot_order = "cad";
> mc->hot_add_cpu = pc_hot_add_cpu;
> mc->max_cpus = 255;
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index b5d7eae..4b3fdbe 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -8,6 +8,7 @@
> #include "sysemu/accel.h"
> #include "hw/qdev.h"
> #include "qom/object.h"
> +#include "qom/cpu.h"
>
> void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
> const char *name,
> @@ -42,6 +43,26 @@ bool machine_dump_guest_core(MachineState *machine);
> bool machine_mem_merge(MachineState *machine);
>
> /**
> + * CPUArchId:
> + * @arch_id - architecture-dependent CPU ID of present or possible CPU
> + * @cpu - pointer to corresponding CPU object if it's present on NULL otherwise
> + */
> +typedef struct {
> + uint64_t arch_id;
> + struct CPUState *cpu;
> +} CPUArchId;
> +
> +/**
> + * CPUArchIdList:
> + * @len - number of @CPUArchId items in @cpus array
> + * @cpus - array of present or possible CPUs for current machine configuration
> + */
> +typedef struct {
> + int len;
> + CPUArchId cpus[0];
> +} CPUArchIdList;
> +
> +/**
> * MachineClass:
> * @get_hotplug_handler: this function is called during bus-less
> * device hotplug. If defined it returns pointer to an instance
> @@ -57,6 +78,10 @@ bool machine_mem_merge(MachineState *machine);
> * Set only by old machines because they need to keep
> * compatibility on code that exposed QEMU_VERSION to guests in
> * the past (and now use qemu_hw_version()).
> + * @possible_cpu_arch_ids:
> + * Returns an array of @CPUArchId architecture-dependent CPU IDs
> + * which includes CPU IDs for present and possible to hotplug CPUs.
> + * Caller is responsible for freeing returned list.
> */
> struct MachineClass {
> /*< private >*/
> @@ -98,6 +123,7 @@ struct MachineClass {
> HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
> DeviceState *dev);
> unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
> + CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
> };
>
> /**
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 8b3546e..3e09232 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -65,6 +65,7 @@ struct PCMachineState {
> /* CPU and apic information: */
> bool apic_xrupt_override;
> unsigned apic_id_limit;
> + CPUArchIdList *possible_cpus;
>
> /* NUMA information: */
> uint64_t numa_nodes;
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH v5 03/10] machine: introduce MachineClass.possible_cpu_arch_ids() hook
2016-03-08 16:29 ` Igor Mammedov
@ 2016-03-08 17:40 ` Marcel Apfelbaum
0 siblings, 0 replies; 25+ messages in thread
From: Marcel Apfelbaum @ 2016-03-08 17:40 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel; +Cc: marcel, ehabkost, mst
On 03/08/2016 06:29 PM, Igor Mammedov wrote:
> On Thu, 3 Mar 2016 15:28:56 +0100
> Igor Mammedov <imammedo@redhat.com> wrote:
>
>> on x86 currently range 0..max_cpus is used to generate
>> architecture-dependent CPU ID (APIC Id) for each present
>> and possible CPUs. However architecture-dependent CPU IDs
>> list could be sparse and code that needs to enumerate
>> all IDs (ACPI) ended up doing guess work enumerating all
>> possible and impossible IDs up to
>> apic_id_limit = x86_cpu_apic_id_from_index(max_cpus).
>>
>> That leads to creation of MADT entries and Processor
>> objects in ACPI tables for not possible CPUs.
>> Fix it by allowing board specify a concrete list of
>> CPU IDs accourding its own rules (which for x86 depends
>> on topology). So that code that needs this list could
>> request it from board instead of trying to guess
>> what IDs are correct on its own.
>>
>> This interface will also allow to help making AML
>> part of CPU hotplug target independent so it could
>> be reused for ARM target.
>>
>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>> ---
>> v5:
>> s/CPUArchId cpus[1]/CPUArchId cpus[0]/ to make length
>> calculation simpler. Marcel Apfelbaum <marcel@redhat.com>
> Marcel, does v5 looks good to you?
>
> /me, sorry for replying/asking out of thread the first time./
>
No problem, I think I already sent this, but to be sure
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Thanks,
Marcel
>> ---
>> hw/i386/pc.c | 44 ++++++++++++++++++++++++++++++++++++++++----
>> include/hw/boards.h | 26 ++++++++++++++++++++++++++
>> include/hw/i386/pc.h | 1 +
>> 3 files changed, 67 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 151a64c..1af95a1 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1132,10 +1132,17 @@ void pc_cpus_init(PCMachineState *pcms)
>> exit(1);
>> }
>>
>> - for (i = 0; i < smp_cpus; i++) {
>> - cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i),
>> - &error_fatal);
>> - object_unref(OBJECT(cpu));
>> + pcms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
>> + sizeof(CPUArchId) * max_cpus);
>> + for (i = 0; i < max_cpus; i++) {
>> + pcms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
>> + pcms->possible_cpus->len++;
>> + if (i < smp_cpus) {
>> + cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i),
>> + &error_fatal);
>> + pcms->possible_cpus->cpus[i].cpu = CPU(cpu);
>> + object_unref(OBJECT(cpu));
>> + }
>> }
>>
>> /* tell smbios about cpuid version and features */
>> @@ -1658,9 +1665,19 @@ static void pc_dimm_unplug(HotplugHandler *hotplug_dev,
>> error_propagate(errp, local_err);
>> }
>>
>> +static int pc_apic_cmp(const void *a, const void *b)
>> +{
>> + CPUArchId *apic_a = (CPUArchId *)a;
>> + CPUArchId *apic_b = (CPUArchId *)b;
>> +
>> + return apic_a->arch_id - apic_b->arch_id;
>> +}
>> +
>> static void pc_cpu_plug(HotplugHandler *hotplug_dev,
>> DeviceState *dev, Error **errp)
>> {
>> + CPUClass *cc = CPU_GET_CLASS(dev);
>> + CPUArchId apic_id, *found_cpu;
>> HotplugHandlerClass *hhc;
>> Error *local_err = NULL;
>> PCMachineState *pcms = PC_MACHINE(hotplug_dev);
>> @@ -1683,6 +1700,13 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
>>
>> /* increment the number of CPUs */
>> rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) + 1);
>> +
>> + apic_id.arch_id = cc->get_arch_id(CPU(dev));
>> + found_cpu = bsearch(&apic_id, pcms->possible_cpus->cpus,
>> + pcms->possible_cpus->len, sizeof(*pcms->possible_cpus->cpus),
>> + pc_apic_cmp);
>> + assert(found_cpu);
>> + found_cpu->cpu = CPU(dev);
>> out:
>> error_propagate(errp, local_err);
>> }
>> @@ -1925,6 +1949,17 @@ static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index)
>> return topo.pkg_id;
>> }
>>
>> +static CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *machine)
>> +{
>> + PCMachineState *pcms = PC_MACHINE(machine);
>> + int len = sizeof(CPUArchIdList) +
>> + sizeof(CPUArchId) * (pcms->possible_cpus->len);
>> + CPUArchIdList *list = g_malloc(len);
>> +
>> + memcpy(list, pcms->possible_cpus, len);
>> + return list;
>> +}
>> +
>> static void pc_machine_class_init(ObjectClass *oc, void *data)
>> {
>> MachineClass *mc = MACHINE_CLASS(oc);
>> @@ -1947,6 +1982,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
>> pcmc->save_tsc_khz = true;
>> mc->get_hotplug_handler = pc_get_hotpug_handler;
>> mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
>> + mc->possible_cpu_arch_ids = pc_possible_cpu_arch_ids;
>> mc->default_boot_order = "cad";
>> mc->hot_add_cpu = pc_hot_add_cpu;
>> mc->max_cpus = 255;
>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>> index b5d7eae..4b3fdbe 100644
>> --- a/include/hw/boards.h
>> +++ b/include/hw/boards.h
>> @@ -8,6 +8,7 @@
>> #include "sysemu/accel.h"
>> #include "hw/qdev.h"
>> #include "qom/object.h"
>> +#include "qom/cpu.h"
>>
>> void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
>> const char *name,
>> @@ -42,6 +43,26 @@ bool machine_dump_guest_core(MachineState *machine);
>> bool machine_mem_merge(MachineState *machine);
>>
>> /**
>> + * CPUArchId:
>> + * @arch_id - architecture-dependent CPU ID of present or possible CPU
>> + * @cpu - pointer to corresponding CPU object if it's present on NULL otherwise
>> + */
>> +typedef struct {
>> + uint64_t arch_id;
>> + struct CPUState *cpu;
>> +} CPUArchId;
>> +
>> +/**
>> + * CPUArchIdList:
>> + * @len - number of @CPUArchId items in @cpus array
>> + * @cpus - array of present or possible CPUs for current machine configuration
>> + */
>> +typedef struct {
>> + int len;
>> + CPUArchId cpus[0];
>> +} CPUArchIdList;
>> +
>> +/**
>> * MachineClass:
>> * @get_hotplug_handler: this function is called during bus-less
>> * device hotplug. If defined it returns pointer to an instance
>> @@ -57,6 +78,10 @@ bool machine_mem_merge(MachineState *machine);
>> * Set only by old machines because they need to keep
>> * compatibility on code that exposed QEMU_VERSION to guests in
>> * the past (and now use qemu_hw_version()).
>> + * @possible_cpu_arch_ids:
>> + * Returns an array of @CPUArchId architecture-dependent CPU IDs
>> + * which includes CPU IDs for present and possible to hotplug CPUs.
>> + * Caller is responsible for freeing returned list.
>> */
>> struct MachineClass {
>> /*< private >*/
>> @@ -98,6 +123,7 @@ struct MachineClass {
>> HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
>> DeviceState *dev);
>> unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
>> + CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
>> };
>>
>> /**
>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index 8b3546e..3e09232 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -65,6 +65,7 @@ struct PCMachineState {
>> /* CPU and apic information: */
>> bool apic_xrupt_override;
>> unsigned apic_id_limit;
>> + CPUArchIdList *possible_cpus;
>>
>> /* NUMA information: */
>> uint64_t numa_nodes;
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v4 04/10] pc: acpi: cleanup qdev_get_machine() calls
2016-02-26 13:59 [Qemu-devel] [PATCH v4 00/10] pc: do not create invalid MADT.LAPIC/Processor entries Igor Mammedov
` (2 preceding siblings ...)
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 03/10] machine: introduce MachineClass.possible_cpu_arch_ids() hook Igor Mammedov
@ 2016-02-26 13:59 ` Igor Mammedov
2016-02-28 20:05 ` Marcel Apfelbaum
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 05/10] pc: acpi: SRAT: create only valid processor lapic entries Igor Mammedov
` (5 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Igor Mammedov @ 2016-02-26 13:59 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, ehabkost, mst
cache qdev_get_machine() result in acpi_setup/acpi_build_update
time and pass it as an argument to child functions that need it.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/i386/acpi-build.c | 31 +++++++++++++++----------------
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 52c9470..c750435 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -362,9 +362,9 @@ build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm,
}
static void
-build_madt(GArray *table_data, GArray *linker, AcpiCpuInfo *cpu)
+build_madt(GArray *table_data, GArray *linker, PCMachineState *pcms,
+ AcpiCpuInfo *cpu)
{
- PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
int madt_start = table_data->len;
AcpiMultipleApicTable *madt;
@@ -1938,13 +1938,12 @@ static Aml *build_q35_osc_method(void)
static void
build_dsdt(GArray *table_data, GArray *linker,
AcpiCpuInfo *cpu, AcpiPmInfo *pm, AcpiMiscInfo *misc,
- PcPciInfo *pci)
+ PcPciInfo *pci, MachineState *machine)
{
CrsRangeEntry *entry;
Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs;
GPtrArray *mem_ranges = g_ptr_array_new_with_free_func(crs_range_free);
GPtrArray *io_ranges = g_ptr_array_new_with_free_func(crs_range_free);
- MachineState *machine = MACHINE(qdev_get_machine());
PCMachineState *pcms = PC_MACHINE(machine);
uint32_t nr_mem = machine->ram_slots;
int root_bus_limit = 0xFF;
@@ -2367,7 +2366,7 @@ acpi_build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
}
static void
-build_srat(GArray *table_data, GArray *linker)
+build_srat(GArray *table_data, GArray *linker, MachineState *machine)
{
AcpiSystemResourceAffinityTable *srat;
AcpiSratProcessorAffinity *core;
@@ -2377,7 +2376,7 @@ build_srat(GArray *table_data, GArray *linker)
uint64_t curnode;
int srat_start, numa_start, slots;
uint64_t mem_len, mem_base, next_base;
- PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
+ PCMachineState *pcms = PC_MACHINE(machine);
ram_addr_t hotplugabble_address_space_size =
object_property_get_int(OBJECT(pcms), PC_MACHINE_MEMHP_REGION_SIZE,
NULL);
@@ -2581,17 +2580,17 @@ static bool acpi_has_iommu(void)
return intel_iommu && !ambiguous;
}
-static bool acpi_has_nvdimm(void)
+static bool acpi_has_nvdimm(MachineState *machine)
{
- PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
+ PCMachineState *pcms = PC_MACHINE(machine);
return pcms->nvdimm;
}
static
-void acpi_build(AcpiBuildTables *tables)
+void acpi_build(AcpiBuildTables *tables, MachineState *machine)
{
- PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
+ PCMachineState *pcms = PC_MACHINE(machine);
PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
GArray *table_offsets;
unsigned facs, dsdt, rsdt, fadt;
@@ -2629,7 +2628,7 @@ void acpi_build(AcpiBuildTables *tables)
/* DSDT is pointed to by FADT */
dsdt = tables_blob->len;
- build_dsdt(tables_blob, tables->linker, &cpu, &pm, &misc, &pci);
+ build_dsdt(tables_blob, tables->linker, &cpu, &pm, &misc, &pci, machine);
/* Count the size of the DSDT and SSDT, we will need it for legacy
* sizing of ACPI tables.
@@ -2644,7 +2643,7 @@ void acpi_build(AcpiBuildTables *tables)
aml_len += tables_blob->len - fadt;
acpi_add_table(table_offsets, tables_blob);
- build_madt(tables_blob, tables->linker, &cpu);
+ build_madt(tables_blob, tables->linker, pcms, &cpu);
if (misc.has_hpet) {
acpi_add_table(table_offsets, tables_blob);
@@ -2661,7 +2660,7 @@ void acpi_build(AcpiBuildTables *tables)
}
if (pcms->numa_nodes) {
acpi_add_table(table_offsets, tables_blob);
- build_srat(tables_blob, tables->linker);
+ build_srat(tables_blob, tables->linker, machine);
}
if (acpi_get_mcfg(&mcfg)) {
acpi_add_table(table_offsets, tables_blob);
@@ -2672,7 +2671,7 @@ void acpi_build(AcpiBuildTables *tables)
build_dmar_q35(tables_blob, tables->linker);
}
- if (acpi_has_nvdimm()) {
+ if (acpi_has_nvdimm(machine)) {
nvdimm_build_acpi(table_offsets, tables_blob, tables->linker);
}
@@ -2766,7 +2765,7 @@ static void acpi_build_update(void *build_opaque)
acpi_build_tables_init(&tables);
- acpi_build(&tables);
+ acpi_build(&tables, MACHINE(qdev_get_machine()));
acpi_ram_update(build_state->table_mr, tables.table_data);
@@ -2831,7 +2830,7 @@ void acpi_setup(void)
acpi_set_pci_info();
acpi_build_tables_init(&tables);
- acpi_build(&tables);
+ acpi_build(&tables, MACHINE(pcms));
/* Now expose it all to Guest */
build_state->table_mr = acpi_add_rom_blob(build_state, tables.table_data,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH v4 04/10] pc: acpi: cleanup qdev_get_machine() calls
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 04/10] pc: acpi: cleanup qdev_get_machine() calls Igor Mammedov
@ 2016-02-28 20:05 ` Marcel Apfelbaum
0 siblings, 0 replies; 25+ messages in thread
From: Marcel Apfelbaum @ 2016-02-28 20:05 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel; +Cc: marcel, ehabkost, mst
On 02/26/2016 03:59 PM, Igor Mammedov wrote:
> cache qdev_get_machine() result in acpi_setup/acpi_build_update
> time and pass it as an argument to child functions that need it.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/i386/acpi-build.c | 31 +++++++++++++++----------------
> 1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 52c9470..c750435 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -362,9 +362,9 @@ build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm,
> }
>
> static void
> -build_madt(GArray *table_data, GArray *linker, AcpiCpuInfo *cpu)
> +build_madt(GArray *table_data, GArray *linker, PCMachineState *pcms,
> + AcpiCpuInfo *cpu)
> {
> - PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> int madt_start = table_data->len;
>
> AcpiMultipleApicTable *madt;
> @@ -1938,13 +1938,12 @@ static Aml *build_q35_osc_method(void)
> static void
> build_dsdt(GArray *table_data, GArray *linker,
> AcpiCpuInfo *cpu, AcpiPmInfo *pm, AcpiMiscInfo *misc,
> - PcPciInfo *pci)
> + PcPciInfo *pci, MachineState *machine)
> {
> CrsRangeEntry *entry;
> Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs;
> GPtrArray *mem_ranges = g_ptr_array_new_with_free_func(crs_range_free);
> GPtrArray *io_ranges = g_ptr_array_new_with_free_func(crs_range_free);
> - MachineState *machine = MACHINE(qdev_get_machine());
> PCMachineState *pcms = PC_MACHINE(machine);
> uint32_t nr_mem = machine->ram_slots;
> int root_bus_limit = 0xFF;
> @@ -2367,7 +2366,7 @@ acpi_build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
> }
>
> static void
> -build_srat(GArray *table_data, GArray *linker)
> +build_srat(GArray *table_data, GArray *linker, MachineState *machine)
> {
> AcpiSystemResourceAffinityTable *srat;
> AcpiSratProcessorAffinity *core;
> @@ -2377,7 +2376,7 @@ build_srat(GArray *table_data, GArray *linker)
> uint64_t curnode;
> int srat_start, numa_start, slots;
> uint64_t mem_len, mem_base, next_base;
> - PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> + PCMachineState *pcms = PC_MACHINE(machine);
> ram_addr_t hotplugabble_address_space_size =
> object_property_get_int(OBJECT(pcms), PC_MACHINE_MEMHP_REGION_SIZE,
> NULL);
> @@ -2581,17 +2580,17 @@ static bool acpi_has_iommu(void)
> return intel_iommu && !ambiguous;
> }
>
> -static bool acpi_has_nvdimm(void)
> +static bool acpi_has_nvdimm(MachineState *machine)
> {
> - PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> + PCMachineState *pcms = PC_MACHINE(machine);
>
> return pcms->nvdimm;
> }
>
> static
> -void acpi_build(AcpiBuildTables *tables)
> +void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> {
> - PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> + PCMachineState *pcms = PC_MACHINE(machine);
> PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
> GArray *table_offsets;
> unsigned facs, dsdt, rsdt, fadt;
> @@ -2629,7 +2628,7 @@ void acpi_build(AcpiBuildTables *tables)
>
> /* DSDT is pointed to by FADT */
> dsdt = tables_blob->len;
> - build_dsdt(tables_blob, tables->linker, &cpu, &pm, &misc, &pci);
> + build_dsdt(tables_blob, tables->linker, &cpu, &pm, &misc, &pci, machine);
>
> /* Count the size of the DSDT and SSDT, we will need it for legacy
> * sizing of ACPI tables.
> @@ -2644,7 +2643,7 @@ void acpi_build(AcpiBuildTables *tables)
> aml_len += tables_blob->len - fadt;
>
> acpi_add_table(table_offsets, tables_blob);
> - build_madt(tables_blob, tables->linker, &cpu);
> + build_madt(tables_blob, tables->linker, pcms, &cpu);
>
> if (misc.has_hpet) {
> acpi_add_table(table_offsets, tables_blob);
> @@ -2661,7 +2660,7 @@ void acpi_build(AcpiBuildTables *tables)
> }
> if (pcms->numa_nodes) {
> acpi_add_table(table_offsets, tables_blob);
> - build_srat(tables_blob, tables->linker);
> + build_srat(tables_blob, tables->linker, machine);
> }
> if (acpi_get_mcfg(&mcfg)) {
> acpi_add_table(table_offsets, tables_blob);
> @@ -2672,7 +2671,7 @@ void acpi_build(AcpiBuildTables *tables)
> build_dmar_q35(tables_blob, tables->linker);
> }
>
> - if (acpi_has_nvdimm()) {
> + if (acpi_has_nvdimm(machine)) {
> nvdimm_build_acpi(table_offsets, tables_blob, tables->linker);
> }
>
> @@ -2766,7 +2765,7 @@ static void acpi_build_update(void *build_opaque)
>
> acpi_build_tables_init(&tables);
>
> - acpi_build(&tables);
> + acpi_build(&tables, MACHINE(qdev_get_machine()));
>
> acpi_ram_update(build_state->table_mr, tables.table_data);
>
> @@ -2831,7 +2830,7 @@ void acpi_setup(void)
> acpi_set_pci_info();
>
> acpi_build_tables_init(&tables);
> - acpi_build(&tables);
> + acpi_build(&tables, MACHINE(pcms));
>
> /* Now expose it all to Guest */
> build_state->table_mr = acpi_add_rom_blob(build_state, tables.table_data,
>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Thanks,
Marcel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v4 05/10] pc: acpi: SRAT: create only valid processor lapic entries
2016-02-26 13:59 [Qemu-devel] [PATCH v4 00/10] pc: do not create invalid MADT.LAPIC/Processor entries Igor Mammedov
` (3 preceding siblings ...)
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 04/10] pc: acpi: cleanup qdev_get_machine() calls Igor Mammedov
@ 2016-02-26 13:59 ` Igor Mammedov
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 06/10] pc: acpi: create MADT.lapic entries only for valid lapics Igor Mammedov
` (4 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2016-02-26 13:59 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, ehabkost, mst
When APIC IDs are sparse*, in addition to valid LAPIC
entries the SRAT is also filled invalid ones for non
possible APIC IDs.
Fix it by asking machine for all possible APIC IDs
instead of wrongly assuming that all APIC IDs in
range 0..apic_id_limit are possible.
* sparse lapic topology CLI:
-smp x,sockets=2,cores=3,maxcpus=6
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
---
hw/i386/acpi-build.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index c750435..54d2f92 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2376,6 +2376,8 @@ build_srat(GArray *table_data, GArray *linker, MachineState *machine)
uint64_t curnode;
int srat_start, numa_start, slots;
uint64_t mem_len, mem_base, next_base;
+ MachineClass *mc = MACHINE_GET_CLASS(machine);
+ CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine);
PCMachineState *pcms = PC_MACHINE(machine);
ram_addr_t hotplugabble_address_space_size =
object_property_get_int(OBJECT(pcms), PC_MACHINE_MEMHP_REGION_SIZE,
@@ -2387,12 +2389,14 @@ build_srat(GArray *table_data, GArray *linker, MachineState *machine)
srat->reserved1 = cpu_to_le32(1);
core = (void *)(srat + 1);
- for (i = 0; i < pcms->apic_id_limit; ++i) {
+ for (i = 0; i < apic_ids->len; i++) {
+ int apic_id = apic_ids->cpus[i].arch_id;
+
core = acpi_data_push(table_data, sizeof *core);
core->type = ACPI_SRAT_PROCESSOR;
core->length = sizeof(*core);
- core->local_apic_id = i;
- curnode = pcms->node_cpu[i];
+ core->local_apic_id = apic_id;
+ curnode = pcms->node_cpu[apic_id];
core->proximity_lo = curnode;
memset(core->proximity_hi, 0, 3);
core->local_sapic_eid = 0;
@@ -2457,6 +2461,7 @@ build_srat(GArray *table_data, GArray *linker, MachineState *machine)
(void *)(table_data->data + srat_start),
"SRAT",
table_data->len - srat_start, 1, NULL, NULL);
+ g_free(apic_ids);
}
static void
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PATCH v4 06/10] pc: acpi: create MADT.lapic entries only for valid lapics
2016-02-26 13:59 [Qemu-devel] [PATCH v4 00/10] pc: do not create invalid MADT.LAPIC/Processor entries Igor Mammedov
` (4 preceding siblings ...)
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 05/10] pc: acpi: SRAT: create only valid processor lapic entries Igor Mammedov
@ 2016-02-26 13:59 ` Igor Mammedov
2016-03-03 10:35 ` Marcel Apfelbaum
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 07/10] pc: acpi: create Processor and Notify objects " Igor Mammedov
` (3 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Igor Mammedov @ 2016-02-26 13:59 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, ehabkost, mst
do not assume that all lapics in range 0..apic_id_limit
are valid and do not create lapic entries for not
possible lapics in MADT.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/i386/acpi-build.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 54d2f92..c06f3f5 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -362,9 +362,10 @@ build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm,
}
static void
-build_madt(GArray *table_data, GArray *linker, PCMachineState *pcms,
- AcpiCpuInfo *cpu)
+build_madt(GArray *table_data, GArray *linker, PCMachineState *pcms)
{
+ MachineClass *mc = MACHINE_GET_CLASS(pcms);
+ CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(pcms));
int madt_start = table_data->len;
AcpiMultipleApicTable *madt;
@@ -377,18 +378,22 @@ build_madt(GArray *table_data, GArray *linker, PCMachineState *pcms,
madt->local_apic_address = cpu_to_le32(APIC_DEFAULT_ADDRESS);
madt->flags = cpu_to_le32(1);
- for (i = 0; i < pcms->apic_id_limit; i++) {
+ for (i = 0; i < apic_ids->len; i++) {
AcpiMadtProcessorApic *apic = acpi_data_push(table_data, sizeof *apic);
+ int apic_id = apic_ids->cpus[i].arch_id;
+
apic->type = ACPI_APIC_PROCESSOR;
apic->length = sizeof(*apic);
- apic->processor_id = i;
- apic->local_apic_id = i;
- if (test_bit(i, cpu->found_cpus)) {
+ apic->processor_id = apic_id;
+ apic->local_apic_id = apic_id;
+ if (apic_ids->cpus[i].cpu != NULL) {
apic->flags = cpu_to_le32(1);
} else {
apic->flags = cpu_to_le32(0);
}
}
+ g_free(apic_ids);
+
io_apic = acpi_data_push(table_data, sizeof *io_apic);
io_apic->type = ACPI_APIC_IO;
io_apic->length = sizeof(*io_apic);
@@ -2648,7 +2653,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
aml_len += tables_blob->len - fadt;
acpi_add_table(table_offsets, tables_blob);
- build_madt(tables_blob, tables->linker, pcms, &cpu);
+ build_madt(tables_blob, tables->linker, pcms);
if (misc.has_hpet) {
acpi_add_table(table_offsets, tables_blob);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH v4 06/10] pc: acpi: create MADT.lapic entries only for valid lapics
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 06/10] pc: acpi: create MADT.lapic entries only for valid lapics Igor Mammedov
@ 2016-03-03 10:35 ` Marcel Apfelbaum
0 siblings, 0 replies; 25+ messages in thread
From: Marcel Apfelbaum @ 2016-03-03 10:35 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel; +Cc: marcel, ehabkost, mst
On 02/26/2016 03:59 PM, Igor Mammedov wrote:
> do not assume that all lapics in range 0..apic_id_limit
> are valid and do not create lapic entries for not
> possible lapics in MADT.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/i386/acpi-build.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 54d2f92..c06f3f5 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -362,9 +362,10 @@ build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm,
> }
>
> static void
> -build_madt(GArray *table_data, GArray *linker, PCMachineState *pcms,
> - AcpiCpuInfo *cpu)
> +build_madt(GArray *table_data, GArray *linker, PCMachineState *pcms)
> {
> + MachineClass *mc = MACHINE_GET_CLASS(pcms);
> + CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(pcms));
> int madt_start = table_data->len;
>
> AcpiMultipleApicTable *madt;
> @@ -377,18 +378,22 @@ build_madt(GArray *table_data, GArray *linker, PCMachineState *pcms,
> madt->local_apic_address = cpu_to_le32(APIC_DEFAULT_ADDRESS);
> madt->flags = cpu_to_le32(1);
>
> - for (i = 0; i < pcms->apic_id_limit; i++) {
> + for (i = 0; i < apic_ids->len; i++) {
> AcpiMadtProcessorApic *apic = acpi_data_push(table_data, sizeof *apic);
> + int apic_id = apic_ids->cpus[i].arch_id;
> +
> apic->type = ACPI_APIC_PROCESSOR;
> apic->length = sizeof(*apic);
> - apic->processor_id = i;
> - apic->local_apic_id = i;
> - if (test_bit(i, cpu->found_cpus)) {
> + apic->processor_id = apic_id;
> + apic->local_apic_id = apic_id;
> + if (apic_ids->cpus[i].cpu != NULL) {
> apic->flags = cpu_to_le32(1);
> } else {
> apic->flags = cpu_to_le32(0);
> }
> }
> + g_free(apic_ids);
> +
> io_apic = acpi_data_push(table_data, sizeof *io_apic);
> io_apic->type = ACPI_APIC_IO;
> io_apic->length = sizeof(*io_apic);
> @@ -2648,7 +2653,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> aml_len += tables_blob->len - fadt;
>
> acpi_add_table(table_offsets, tables_blob);
> - build_madt(tables_blob, tables->linker, pcms, &cpu);
> + build_madt(tables_blob, tables->linker, pcms);
>
> if (misc.has_hpet) {
> acpi_add_table(table_offsets, tables_blob);
>
Looks OK to me.
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Thanks,
Marcel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v4 07/10] pc: acpi: create Processor and Notify objects only for valid lapics
2016-02-26 13:59 [Qemu-devel] [PATCH v4 00/10] pc: do not create invalid MADT.LAPIC/Processor entries Igor Mammedov
` (5 preceding siblings ...)
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 06/10] pc: acpi: create MADT.lapic entries only for valid lapics Igor Mammedov
@ 2016-02-26 13:59 ` Igor Mammedov
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 08/10] pc: acpi: drop cpu->found_cpus bitmap Igor Mammedov
` (2 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2016-02-26 13:59 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, ehabkost, mst
do not assume that all lapics in range 0..apic_id_limit
are valid and do not create Processor and Notify objects
for not possible lapics.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
---
hw/i386/acpi-build.c | 39 ++++++++++++++++++++++++---------------
1 file changed, 24 insertions(+), 15 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index c06f3f5..df7e6df 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -965,7 +965,7 @@ static Aml *build_crs(PCIHostState *host,
return crs;
}
-static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus,
+static void build_processor_devices(Aml *sb_scope, MachineState *machine,
AcpiCpuInfo *cpu, AcpiPmInfo *pm)
{
int i;
@@ -975,11 +975,14 @@ static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus,
Aml *field;
Aml *ifctx;
Aml *method;
+ MachineClass *mc = MACHINE_GET_CLASS(machine);
+ CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine);
+ PCMachineState *pcms = PC_MACHINE(machine);
/* The current AML generator can cover the APIC ID range [0..255],
* inclusive, for VCPU hotplug. */
QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256);
- g_assert(acpi_cpus <= ACPI_CPU_HOTPLUG_ID_LIMIT);
+ g_assert(pcms->apic_id_limit <= ACPI_CPU_HOTPLUG_ID_LIMIT);
/* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */
dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE));
@@ -1004,22 +1007,26 @@ static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus,
aml_append(sb_scope, field);
/* build Processor object for each processor */
- for (i = 0; i < acpi_cpus; i++) {
- dev = aml_processor(i, 0, 0, "CP%.02X", i);
+ for (i = 0; i < apic_ids->len; i++) {
+ int apic_id = apic_ids->cpus[i].arch_id;
+
+ assert(apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
+ dev = aml_processor(apic_id, 0, 0, "CP%.02X", apic_id);
method = aml_method("_MAT", 0, AML_NOTSERIALIZED);
aml_append(method,
- aml_return(aml_call1(CPU_MAT_METHOD, aml_int(i))));
+ aml_return(aml_call1(CPU_MAT_METHOD, aml_int(apic_id))));
aml_append(dev, method);
method = aml_method("_STA", 0, AML_NOTSERIALIZED);
aml_append(method,
- aml_return(aml_call1(CPU_STATUS_METHOD, aml_int(i))));
+ aml_return(aml_call1(CPU_STATUS_METHOD, aml_int(apic_id))));
aml_append(dev, method);
method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
aml_append(method,
- aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(i), aml_arg(0)))
+ aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(apic_id),
+ aml_arg(0)))
);
aml_append(dev, method);
@@ -1031,10 +1038,12 @@ static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus,
*/
/* Arg0 = Processor ID = APIC ID */
method = aml_method(AML_NOTIFY_METHOD, 2, AML_NOTSERIALIZED);
- for (i = 0; i < acpi_cpus; i++) {
- ifctx = aml_if(aml_equal(aml_arg(0), aml_int(i)));
+ for (i = 0; i < apic_ids->len; i++) {
+ int apic_id = apic_ids->cpus[i].arch_id;
+
+ ifctx = aml_if(aml_equal(aml_arg(0), aml_int(apic_id)));
aml_append(ifctx,
- aml_notify(aml_name("CP%.02X", i), aml_arg(1))
+ aml_notify(aml_name("CP%.02X", apic_id), aml_arg(1))
);
aml_append(method, ifctx);
}
@@ -1047,14 +1056,15 @@ static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus,
* ith up to 255 elements. Windows guests up to win2k8 fail when
* VarPackageOp is used.
*/
- pkg = acpi_cpus <= 255 ? aml_package(acpi_cpus) :
- aml_varpackage(acpi_cpus);
+ pkg = pcms->apic_id_limit <= 255 ? aml_package(pcms->apic_id_limit) :
+ aml_varpackage(pcms->apic_id_limit);
- for (i = 0; i < acpi_cpus; i++) {
+ for (i = 0; i < pcms->apic_id_limit; i++) {
uint8_t b = test_bit(i, cpu->found_cpus) ? 0x01 : 0x00;
aml_append(pkg, aml_int(b));
}
aml_append(sb_scope, aml_name_decl(CPU_ON_BITMAP, pkg));
+ g_free(apic_ids);
}
static void build_memory_devices(Aml *sb_scope, int nr_mem,
@@ -1949,7 +1959,6 @@ build_dsdt(GArray *table_data, GArray *linker,
Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs;
GPtrArray *mem_ranges = g_ptr_array_new_with_free_func(crs_range_free);
GPtrArray *io_ranges = g_ptr_array_new_with_free_func(crs_range_free);
- PCMachineState *pcms = PC_MACHINE(machine);
uint32_t nr_mem = machine->ram_slots;
int root_bus_limit = 0xFF;
PCIBus *bus = NULL;
@@ -2250,7 +2259,7 @@ build_dsdt(GArray *table_data, GArray *linker,
sb_scope = aml_scope("\\_SB");
{
- build_processor_devices(sb_scope, pcms->apic_id_limit, cpu, pm);
+ build_processor_devices(sb_scope, machine, cpu, pm);
build_memory_devices(sb_scope, nr_mem, pm->mem_hp_io_base,
pm->mem_hp_io_len);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PATCH v4 08/10] pc: acpi: drop cpu->found_cpus bitmap
2016-02-26 13:59 [Qemu-devel] [PATCH v4 00/10] pc: do not create invalid MADT.LAPIC/Processor entries Igor Mammedov
` (6 preceding siblings ...)
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 07/10] pc: acpi: create Processor and Notify objects " Igor Mammedov
@ 2016-02-26 13:59 ` Igor Mammedov
2016-03-03 11:33 ` Marcel Apfelbaum
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 09/10] pc: acpi: clarify why possible LAPIC entries must be present in MADT Igor Mammedov
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 10/10] tests: update ACPI tables blobs for cpuhp_sparse case Igor Mammedov
9 siblings, 1 reply; 25+ messages in thread
From: Igor Mammedov @ 2016-02-26 13:59 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, ehabkost, mst
cpu->found_cpus bitmap is used for setting present
flag in CPON AML package. But it takes a bunch of code
to fill bitmap and could be simplified by getting
presense info from possible CPUs list directly.
So drop cpu->found_cpus bitmap and unroll possible
CPUs list into APIC index array at the place where
CPUON AML package is created.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/i386/acpi-build.c | 53 ++++++++++++++--------------------------------------
1 file changed, 14 insertions(+), 39 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index df7e6df..48240e0 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -76,10 +76,6 @@
#define ACPI_BUILD_DPRINTF(fmt, ...)
#endif
-typedef struct AcpiCpuInfo {
- DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
-} AcpiCpuInfo;
-
typedef struct AcpiMcfgInfo {
uint64_t mcfg_base;
uint32_t mcfg_size;
@@ -121,31 +117,6 @@ typedef struct AcpiBuildPciBusHotplugState {
bool pcihp_bridge_en;
} AcpiBuildPciBusHotplugState;
-static
-int acpi_add_cpu_info(Object *o, void *opaque)
-{
- AcpiCpuInfo *cpu = opaque;
- uint64_t apic_id;
-
- if (object_dynamic_cast(o, TYPE_CPU)) {
- apic_id = object_property_get_int(o, "apic-id", NULL);
- assert(apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
-
- set_bit(apic_id, cpu->found_cpus);
- }
-
- object_child_foreach(o, acpi_add_cpu_info, opaque);
- return 0;
-}
-
-static void acpi_get_cpu_info(AcpiCpuInfo *cpu)
-{
- Object *root = object_get_root();
-
- memset(cpu->found_cpus, 0, sizeof cpu->found_cpus);
- object_child_foreach(root, acpi_add_cpu_info, cpu);
-}
-
static void acpi_get_pm_info(AcpiPmInfo *pm)
{
Object *piix = piix4_pm_find();
@@ -966,9 +937,9 @@ static Aml *build_crs(PCIHostState *host,
}
static void build_processor_devices(Aml *sb_scope, MachineState *machine,
- AcpiCpuInfo *cpu, AcpiPmInfo *pm)
+ AcpiPmInfo *pm)
{
- int i;
+ int i, apic_idx;
Aml *dev;
Aml *crs;
Aml *pkg;
@@ -1011,6 +982,7 @@ static void build_processor_devices(Aml *sb_scope, MachineState *machine,
int apic_id = apic_ids->cpus[i].arch_id;
assert(apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
+
dev = aml_processor(apic_id, 0, 0, "CP%.02X", apic_id);
method = aml_method("_MAT", 0, AML_NOTSERIALIZED);
@@ -1059,9 +1031,14 @@ static void build_processor_devices(Aml *sb_scope, MachineState *machine,
pkg = pcms->apic_id_limit <= 255 ? aml_package(pcms->apic_id_limit) :
aml_varpackage(pcms->apic_id_limit);
- for (i = 0; i < pcms->apic_id_limit; i++) {
- uint8_t b = test_bit(i, cpu->found_cpus) ? 0x01 : 0x00;
- aml_append(pkg, aml_int(b));
+ for (i = 0, apic_idx = 0; i < apic_ids->len; i++) {
+ int apic_id = apic_ids->cpus[i].arch_id;
+
+ for (; apic_idx < apic_id; apic_idx++) {
+ aml_append(pkg, aml_int(0));
+ }
+ aml_append(pkg, aml_int(apic_ids->cpus[i].cpu ? 1 : 0));
+ apic_idx = apic_id + 1;
}
aml_append(sb_scope, aml_name_decl(CPU_ON_BITMAP, pkg));
g_free(apic_ids);
@@ -1952,7 +1929,7 @@ static Aml *build_q35_osc_method(void)
static void
build_dsdt(GArray *table_data, GArray *linker,
- AcpiCpuInfo *cpu, AcpiPmInfo *pm, AcpiMiscInfo *misc,
+ AcpiPmInfo *pm, AcpiMiscInfo *misc,
PcPciInfo *pci, MachineState *machine)
{
CrsRangeEntry *entry;
@@ -2259,7 +2236,7 @@ build_dsdt(GArray *table_data, GArray *linker,
sb_scope = aml_scope("\\_SB");
{
- build_processor_devices(sb_scope, machine, cpu, pm);
+ build_processor_devices(sb_scope, machine, pm);
build_memory_devices(sb_scope, nr_mem, pm->mem_hp_io_base,
pm->mem_hp_io_len);
@@ -2613,7 +2590,6 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
GArray *table_offsets;
unsigned facs, dsdt, rsdt, fadt;
- AcpiCpuInfo cpu;
AcpiPmInfo pm;
AcpiMiscInfo misc;
AcpiMcfgInfo mcfg;
@@ -2623,7 +2599,6 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
GArray *tables_blob = tables->table_data;
AcpiSlicOem slic_oem = { .id = NULL, .table_id = NULL };
- acpi_get_cpu_info(&cpu);
acpi_get_pm_info(&pm);
acpi_get_misc_info(&misc);
acpi_get_pci_info(&pci);
@@ -2647,7 +2622,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
/* DSDT is pointed to by FADT */
dsdt = tables_blob->len;
- build_dsdt(tables_blob, tables->linker, &cpu, &pm, &misc, &pci, machine);
+ build_dsdt(tables_blob, tables->linker, &pm, &misc, &pci, machine);
/* Count the size of the DSDT and SSDT, we will need it for legacy
* sizing of ACPI tables.
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH v4 08/10] pc: acpi: drop cpu->found_cpus bitmap
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 08/10] pc: acpi: drop cpu->found_cpus bitmap Igor Mammedov
@ 2016-03-03 11:33 ` Marcel Apfelbaum
2016-03-03 14:24 ` Igor Mammedov
0 siblings, 1 reply; 25+ messages in thread
From: Marcel Apfelbaum @ 2016-03-03 11:33 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel; +Cc: ehabkost, mst
On 02/26/2016 03:59 PM, Igor Mammedov wrote:
> cpu->found_cpus bitmap is used for setting present
> flag in CPON AML package. But it takes a bunch of code
> to fill bitmap and could be simplified by getting
> presense info from possible CPUs list directly.
>
> So drop cpu->found_cpus bitmap and unroll possible
> CPUs list into APIC index array at the place where
> CPUON AML package is created.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/i386/acpi-build.c | 53 ++++++++++++++--------------------------------------
> 1 file changed, 14 insertions(+), 39 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index df7e6df..48240e0 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -76,10 +76,6 @@
> #define ACPI_BUILD_DPRINTF(fmt, ...)
> #endif
>
> -typedef struct AcpiCpuInfo {
> - DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
> -} AcpiCpuInfo;
> -
> typedef struct AcpiMcfgInfo {
> uint64_t mcfg_base;
> uint32_t mcfg_size;
> @@ -121,31 +117,6 @@ typedef struct AcpiBuildPciBusHotplugState {
> bool pcihp_bridge_en;
> } AcpiBuildPciBusHotplugState;
>
> -static
> -int acpi_add_cpu_info(Object *o, void *opaque)
> -{
> - AcpiCpuInfo *cpu = opaque;
> - uint64_t apic_id;
> -
> - if (object_dynamic_cast(o, TYPE_CPU)) {
> - apic_id = object_property_get_int(o, "apic-id", NULL);
> - assert(apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
> -
> - set_bit(apic_id, cpu->found_cpus);
> - }
> -
> - object_child_foreach(o, acpi_add_cpu_info, opaque);
> - return 0;
> -}
> -
> -static void acpi_get_cpu_info(AcpiCpuInfo *cpu)
> -{
> - Object *root = object_get_root();
> -
> - memset(cpu->found_cpus, 0, sizeof cpu->found_cpus);
> - object_child_foreach(root, acpi_add_cpu_info, cpu);
> -}
> -
> static void acpi_get_pm_info(AcpiPmInfo *pm)
> {
> Object *piix = piix4_pm_find();
> @@ -966,9 +937,9 @@ static Aml *build_crs(PCIHostState *host,
> }
>
> static void build_processor_devices(Aml *sb_scope, MachineState *machine,
> - AcpiCpuInfo *cpu, AcpiPmInfo *pm)
> + AcpiPmInfo *pm)
> {
> - int i;
> + int i, apic_idx;
> Aml *dev;
> Aml *crs;
> Aml *pkg;
> @@ -1011,6 +982,7 @@ static void build_processor_devices(Aml *sb_scope, MachineState *machine,
> int apic_id = apic_ids->cpus[i].arch_id;
>
> assert(apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
> +
Unneeded chunk, not really a big issue :)
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Thanks,
Marcel
> dev = aml_processor(apic_id, 0, 0, "CP%.02X", apic_id);
>
> method = aml_method("_MAT", 0, AML_NOTSERIALIZED);
> @@ -1059,9 +1031,14 @@ static void build_processor_devices(Aml *sb_scope, MachineState *machine,
> pkg = pcms->apic_id_limit <= 255 ? aml_package(pcms->apic_id_limit) :
> aml_varpackage(pcms->apic_id_limit);
>
> - for (i = 0; i < pcms->apic_id_limit; i++) {
> - uint8_t b = test_bit(i, cpu->found_cpus) ? 0x01 : 0x00;
> - aml_append(pkg, aml_int(b));
> + for (i = 0, apic_idx = 0; i < apic_ids->len; i++) {
> + int apic_id = apic_ids->cpus[i].arch_id;
> +
> + for (; apic_idx < apic_id; apic_idx++) {
> + aml_append(pkg, aml_int(0));
> + }
> + aml_append(pkg, aml_int(apic_ids->cpus[i].cpu ? 1 : 0));
> + apic_idx = apic_id + 1;
> }
> aml_append(sb_scope, aml_name_decl(CPU_ON_BITMAP, pkg));
> g_free(apic_ids);
> @@ -1952,7 +1929,7 @@ static Aml *build_q35_osc_method(void)
>
> static void
> build_dsdt(GArray *table_data, GArray *linker,
> - AcpiCpuInfo *cpu, AcpiPmInfo *pm, AcpiMiscInfo *misc,
> + AcpiPmInfo *pm, AcpiMiscInfo *misc,
> PcPciInfo *pci, MachineState *machine)
> {
> CrsRangeEntry *entry;
> @@ -2259,7 +2236,7 @@ build_dsdt(GArray *table_data, GArray *linker,
>
> sb_scope = aml_scope("\\_SB");
> {
> - build_processor_devices(sb_scope, machine, cpu, pm);
> + build_processor_devices(sb_scope, machine, pm);
>
> build_memory_devices(sb_scope, nr_mem, pm->mem_hp_io_base,
> pm->mem_hp_io_len);
> @@ -2613,7 +2590,6 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
> GArray *table_offsets;
> unsigned facs, dsdt, rsdt, fadt;
> - AcpiCpuInfo cpu;
> AcpiPmInfo pm;
> AcpiMiscInfo misc;
> AcpiMcfgInfo mcfg;
> @@ -2623,7 +2599,6 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> GArray *tables_blob = tables->table_data;
> AcpiSlicOem slic_oem = { .id = NULL, .table_id = NULL };
>
> - acpi_get_cpu_info(&cpu);
> acpi_get_pm_info(&pm);
> acpi_get_misc_info(&misc);
> acpi_get_pci_info(&pci);
> @@ -2647,7 +2622,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>
> /* DSDT is pointed to by FADT */
> dsdt = tables_blob->len;
> - build_dsdt(tables_blob, tables->linker, &cpu, &pm, &misc, &pci, machine);
> + build_dsdt(tables_blob, tables->linker, &pm, &misc, &pci, machine);
>
> /* Count the size of the DSDT and SSDT, we will need it for legacy
> * sizing of ACPI tables.
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH v4 08/10] pc: acpi: drop cpu->found_cpus bitmap
2016-03-03 11:33 ` Marcel Apfelbaum
@ 2016-03-03 14:24 ` Igor Mammedov
0 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2016-03-03 14:24 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: qemu-devel, ehabkost, mst
On Thu, 3 Mar 2016 13:33:29 +0200
Marcel Apfelbaum <marcel@redhat.com> wrote:
> On 02/26/2016 03:59 PM, Igor Mammedov wrote:
> > cpu->found_cpus bitmap is used for setting present
> > flag in CPON AML package. But it takes a bunch of code
> > to fill bitmap and could be simplified by getting
> > presense info from possible CPUs list directly.
> >
> > So drop cpu->found_cpus bitmap and unroll possible
> > CPUs list into APIC index array at the place where
> > CPUON AML package is created.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > hw/i386/acpi-build.c | 53 ++++++++++++++--------------------------------------
> > 1 file changed, 14 insertions(+), 39 deletions(-)
> >
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index df7e6df..48240e0 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -76,10 +76,6 @@
> > #define ACPI_BUILD_DPRINTF(fmt, ...)
> > #endif
> >
> > -typedef struct AcpiCpuInfo {
> > - DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
> > -} AcpiCpuInfo;
> > -
> > typedef struct AcpiMcfgInfo {
> > uint64_t mcfg_base;
> > uint32_t mcfg_size;
> > @@ -121,31 +117,6 @@ typedef struct AcpiBuildPciBusHotplugState {
> > bool pcihp_bridge_en;
> > } AcpiBuildPciBusHotplugState;
> >
> > -static
> > -int acpi_add_cpu_info(Object *o, void *opaque)
> > -{
> > - AcpiCpuInfo *cpu = opaque;
> > - uint64_t apic_id;
> > -
> > - if (object_dynamic_cast(o, TYPE_CPU)) {
> > - apic_id = object_property_get_int(o, "apic-id", NULL);
> > - assert(apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
> > -
> > - set_bit(apic_id, cpu->found_cpus);
> > - }
> > -
> > - object_child_foreach(o, acpi_add_cpu_info, opaque);
> > - return 0;
> > -}
> > -
> > -static void acpi_get_cpu_info(AcpiCpuInfo *cpu)
> > -{
> > - Object *root = object_get_root();
> > -
> > - memset(cpu->found_cpus, 0, sizeof cpu->found_cpus);
> > - object_child_foreach(root, acpi_add_cpu_info, cpu);
> > -}
> > -
> > static void acpi_get_pm_info(AcpiPmInfo *pm)
> > {
> > Object *piix = piix4_pm_find();
> > @@ -966,9 +937,9 @@ static Aml *build_crs(PCIHostState *host,
> > }
> >
> > static void build_processor_devices(Aml *sb_scope, MachineState *machine,
> > - AcpiCpuInfo *cpu, AcpiPmInfo *pm)
> > + AcpiPmInfo *pm)
> > {
> > - int i;
> > + int i, apic_idx;
> > Aml *dev;
> > Aml *crs;
> > Aml *pkg;
> > @@ -1011,6 +982,7 @@ static void build_processor_devices(Aml *sb_scope, MachineState *machine,
> > int apic_id = apic_ids->cpus[i].arch_id;
> >
> > assert(apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
> > +
>
> Unneeded chunk, not really a big issue :)
assert has been separated from following statement originally,
so I've just kept the current style.
>
>
> Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Thanks
>
> Thanks,
> Marcel
>
> > dev = aml_processor(apic_id, 0, 0, "CP%.02X", apic_id);
> >
> > method = aml_method("_MAT", 0, AML_NOTSERIALIZED);
> > @@ -1059,9 +1031,14 @@ static void build_processor_devices(Aml *sb_scope, MachineState *machine,
> > pkg = pcms->apic_id_limit <= 255 ? aml_package(pcms->apic_id_limit) :
> > aml_varpackage(pcms->apic_id_limit);
> >
> > - for (i = 0; i < pcms->apic_id_limit; i++) {
> > - uint8_t b = test_bit(i, cpu->found_cpus) ? 0x01 : 0x00;
> > - aml_append(pkg, aml_int(b));
> > + for (i = 0, apic_idx = 0; i < apic_ids->len; i++) {
> > + int apic_id = apic_ids->cpus[i].arch_id;
> > +
> > + for (; apic_idx < apic_id; apic_idx++) {
> > + aml_append(pkg, aml_int(0));
> > + }
> > + aml_append(pkg, aml_int(apic_ids->cpus[i].cpu ? 1 : 0));
> > + apic_idx = apic_id + 1;
> > }
> > aml_append(sb_scope, aml_name_decl(CPU_ON_BITMAP, pkg));
> > g_free(apic_ids);
> > @@ -1952,7 +1929,7 @@ static Aml *build_q35_osc_method(void)
> >
> > static void
> > build_dsdt(GArray *table_data, GArray *linker,
> > - AcpiCpuInfo *cpu, AcpiPmInfo *pm, AcpiMiscInfo *misc,
> > + AcpiPmInfo *pm, AcpiMiscInfo *misc,
> > PcPciInfo *pci, MachineState *machine)
> > {
> > CrsRangeEntry *entry;
> > @@ -2259,7 +2236,7 @@ build_dsdt(GArray *table_data, GArray *linker,
> >
> > sb_scope = aml_scope("\\_SB");
> > {
> > - build_processor_devices(sb_scope, machine, cpu, pm);
> > + build_processor_devices(sb_scope, machine, pm);
> >
> > build_memory_devices(sb_scope, nr_mem, pm->mem_hp_io_base,
> > pm->mem_hp_io_len);
> > @@ -2613,7 +2590,6 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> > PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
> > GArray *table_offsets;
> > unsigned facs, dsdt, rsdt, fadt;
> > - AcpiCpuInfo cpu;
> > AcpiPmInfo pm;
> > AcpiMiscInfo misc;
> > AcpiMcfgInfo mcfg;
> > @@ -2623,7 +2599,6 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> > GArray *tables_blob = tables->table_data;
> > AcpiSlicOem slic_oem = { .id = NULL, .table_id = NULL };
> >
> > - acpi_get_cpu_info(&cpu);
> > acpi_get_pm_info(&pm);
> > acpi_get_misc_info(&misc);
> > acpi_get_pci_info(&pci);
> > @@ -2647,7 +2622,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> >
> > /* DSDT is pointed to by FADT */
> > dsdt = tables_blob->len;
> > - build_dsdt(tables_blob, tables->linker, &cpu, &pm, &misc, &pci, machine);
> > + build_dsdt(tables_blob, tables->linker, &pm, &misc, &pci, machine);
> >
> > /* Count the size of the DSDT and SSDT, we will need it for legacy
> > * sizing of ACPI tables.
> >
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v4 09/10] pc: acpi: clarify why possible LAPIC entries must be present in MADT
2016-02-26 13:59 [Qemu-devel] [PATCH v4 00/10] pc: do not create invalid MADT.LAPIC/Processor entries Igor Mammedov
` (7 preceding siblings ...)
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 08/10] pc: acpi: drop cpu->found_cpus bitmap Igor Mammedov
@ 2016-02-26 13:59 ` Igor Mammedov
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 10/10] tests: update ACPI tables blobs for cpuhp_sparse case Igor Mammedov
9 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2016-02-26 13:59 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, ehabkost, mst
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
---
hw/i386/acpi-build.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 48240e0..155198b 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -360,6 +360,12 @@ build_madt(GArray *table_data, GArray *linker, PCMachineState *pcms)
if (apic_ids->cpus[i].cpu != NULL) {
apic->flags = cpu_to_le32(1);
} else {
+ /* ACPI spec says that LAPIC entry for non present
+ * CPU may be omitted from MADT or it must be marked
+ * as disabled. However omitting non present CPU from
+ * MADT breaks hotplug on linux. So possible CPUs
+ * should be put in MADT but kept disabled.
+ */
apic->flags = cpu_to_le32(0);
}
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PATCH v4 10/10] tests: update ACPI tables blobs for cpuhp_sparse case
2016-02-26 13:59 [Qemu-devel] [PATCH v4 00/10] pc: do not create invalid MADT.LAPIC/Processor entries Igor Mammedov
` (8 preceding siblings ...)
2016-02-26 13:59 ` [Qemu-devel] [PATCH v4 09/10] pc: acpi: clarify why possible LAPIC entries must be present in MADT Igor Mammedov
@ 2016-02-26 13:59 ` Igor Mammedov
9 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2016-02-26 13:59 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, ehabkost, mst
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
tests/acpi-test-data/pc/APIC.cpuhp_sparse | Bin 168 -> 160 bytes
tests/acpi-test-data/pc/DSDT.cpuhp_sparse | Bin 5889 -> 5821 bytes
tests/acpi-test-data/pc/SRAT.cpuhp_sparse | Bin 280 -> 264 bytes
3 files changed, 0 insertions(+), 0 deletions(-)
diff --git a/tests/acpi-test-data/pc/APIC.cpuhp_sparse b/tests/acpi-test-data/pc/APIC.cpuhp_sparse
index d921ee1fa15fd6a549d8f12eb5f15682f5d5a8f3..67918d51b7b7fbd26aff57802b3cf358864fb36a 100644
GIT binary patch
delta 21
ccmZ3%xPXz%F~HM#0RsaAW939Hn~8o=06e+{oB#j-
delta 24
fcmZ3$xPp<(F~HM#1p@;EWAa2U8zyGvi4F+>Nq_~8
diff --git a/tests/acpi-test-data/pc/DSDT.cpuhp_sparse b/tests/acpi-test-data/pc/DSDT.cpuhp_sparse
index 0dafa1037da867521d69d0ee0a5d2df07ac5a419..fab936a3cd242b89ff168a0a92b5196838101339 100644
GIT binary patch
delta 43
zcmZqF+pEjv66_MPSB!yy@yA9kZeAvjS)0XqMHnY1^DSobWtq&uud~^K{~$X64bcnS
delta 100
zcmdn1+o;Fo66_MfD9*sZ$hMJ-o0rLF-ez%L5k^B}W(E)t;f?oo3}IN}9N_E7#SG*H
whd6?G!685%U%acA0V9y->cz!8S%q&ElPBxs3w%0k%x%sA2F9De@f~Le01(d?1poj5
diff --git a/tests/acpi-test-data/pc/SRAT.cpuhp_sparse b/tests/acpi-test-data/pc/SRAT.cpuhp_sparse
index 5377019cca32fb7a97111ea79d96d68a52627cd8..8deeaee3bde46a891d8946b21368c4cef55d7d4f 100644
GIT binary patch
delta 21
ccmbQi)WO6R9OM|n!N|bCIAJ1J<iz5o05%8(pa1{>
delta 23
ecmeBRn!&^s9OM`x!N|bCm@|<pl9730$QA%SR0V(l
--
1.8.3.1
^ permalink raw reply [flat|nested] 25+ messages in thread