Message ID | 20180927212438.32024-3-lersek@redhat.com |
---|---|
State | New |
Headers | show |
Series | hw/pci-host/x86: extend the 64-bit PCI hole relative to the fw-assigned base | expand |
On 9/28/18 12:24 AM, Laszlo Ersek wrote: > In commit 9fa99d2519cb ("hw/pci-host: Fix x86 Host Bridges 64bit PCI > hole", 2017-11-16), we meant to expose such a 64-bit PCI MMIO aperture in > the ACPI DSDT that would be at least as large as the new "pci-hole64-size" > property (2GB on i440fx, 32GB on q35). The goal was to offer "enough" > 64-bit MMIO aperture to the guest OS for hotplug purposes. > > Currently the aperture is extended relative to a possibly incorrect base. > This may result in an aperture size that is smaller than the intent of > commit 9fa99d2519cb. > > We're going to fix the error in a later patch in this series; now we just > add a test case that reproduces and captures the problem. In the fix, the > test data will be updated as well. > > In the test case being added: > - use 128 MB initial RAM size, > - ask for one DIMM hotplug slot, > - ask for 2 GB maximum RAM size, > - use a pci-testdev with a 64-bit BAR of 2 GB size. > > Consequences: > > (1) In pc_memory_init() [hw/i386/pc.c], the DIMM hotplug area size is > initially set to 2048-128 = 1920 MB. (Maximum RAM size minus initial > RAM size.) > > (2) The DIMM area base is set to 4096 MB (because the initial RAM is only > 128 MB -- there is no initial "high RAM"). > > (3) Due to commit 085f8e88ba73 ("pc: count in 1Gb hugepage alignment when > sizing hotplug-memory container", 2014-11-24), we add 1 GB for the one > DIMM hotplug slot that was specified. This sets the DIMM area size to > 1920+1024 = 2944 MB. > > (4) The reserved-memory-end address (exclusive) is set to 4096 + 2944 = > 7040 MB (DIMM area base plus DIMM area size). > > (5) The reserved-memory-end address is rounded up to GB alignment, > yielding 7 GB (7168 MB). > > (6) Given the 2 GB BAR size of pci-testdev, SeaBIOS allocates said 64-bit > BAR in 64-bit address space. > > (7) Because reserved-memory-end is at 7 GB, it is unaligned for the 2 GB > BAR. Therefore SeaBIOS allocates the BAR at 8 GB. QEMU then > (correctly) assigns the root bridge aperture base this BAR address, to > be exposed in \_SB.PCI0._CRS. > > (8) The intent of commit 9fa99d2519cb dictates that QEMU extend the > aperture size to 32 GB, implying a 40 GB end address. However, QEMU > performs the extension relative to reserved-memory-end (7 GB), not > relative to the bridge aperture base that was correctly deduced from > SeaBIOS's BAR programming (8 GB). Therefore we see 39 GB as the > aperture end address in \_SB.PCI0._CRS: > >> QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, Cacheable, ReadWrite, >> 0x0000000000000000, // Granularity >> 0x0000000200000000, // Range Minimum >> 0x00000009BFFFFFFF, // Range Maximum >> 0x0000000000000000, // Translation Offset >> 0x00000007C0000000, // Length >> ,, , AddressRangeMemory, TypeStatic) > Cc: "Michael S. Tsirkin" <mst@redhat.com> > Cc: Alex Williamson <alex.williamson@redhat.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Igor Mammedov <imammedo@redhat.com> > Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > > Notes: > v2: > - new patch > - rely on Gerd's "[PATCH] pci-testdev: add optional memory bar" at > <https://patchew.org/QEMU/20180927121055.28361-1-kraxel@redhat.com/> > > tests/bios-tables-test.c | 16 ++++++++++++++++ > tests/acpi-test-data/q35/DSDT.mmio64 | Bin 0 -> 8947 bytes > tests/acpi-test-data/q35/SRAT.mmio64 | Bin 0 -> 224 bytes > 3 files changed, 16 insertions(+) > create mode 100644 tests/acpi-test-data/q35/DSDT.mmio64 > create mode 100644 tests/acpi-test-data/q35/SRAT.mmio64 > > diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c > index 4e24930c4bf9..9dd88f9d868a 100644 > --- a/tests/bios-tables-test.c > +++ b/tests/bios-tables-test.c > @@ -708,6 +708,21 @@ static void test_acpi_q35_tcg_bridge(void) > free_test_data(&data); > } > > +static void test_acpi_q35_tcg_mmio64(void) > +{ > + test_data data = { > + .machine = MACHINE_Q35, > + .variant = ".mmio64", > + .required_struct_types = base_required_struct_types, > + .required_struct_types_len = ARRAY_SIZE(base_required_struct_types) > + }; > + > + test_acpi_one("-m 128M,slots=1,maxmem=2G " > + "-device pci-testdev,membar=2G", > + &data); > + free_test_data(&data); > +} > + > static void test_acpi_piix4_tcg_cphp(void) > { > test_data data; > @@ -875,6 +890,7 @@ int main(int argc, char *argv[]) > qtest_add_func("acpi/piix4/bridge", test_acpi_piix4_tcg_bridge); > qtest_add_func("acpi/q35", test_acpi_q35_tcg); > qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge); > + qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64); > qtest_add_func("acpi/piix4/ipmi", test_acpi_piix4_tcg_ipmi); > qtest_add_func("acpi/q35/ipmi", test_acpi_q35_tcg_ipmi); > qtest_add_func("acpi/piix4/cpuhp", test_acpi_piix4_tcg_cphp); > diff --git a/tests/acpi-test-data/q35/DSDT.mmio64 b/tests/acpi-test-data/q35/DSDT.mmio64 > new file mode 100644 > index 0000000000000000000000000000000000000000..498c535f2a85a058b76a0a5ebc67c3d5236c3265 > GIT binary patch > literal 8947 > zcmb7KTW=f38J*=#t05&VrL`<y5))3)v}rDs<T^#$1SWSWk`Y&;NIG#q16;dtD<v(m > zP@E!Z5I|OdT>MZVVG^`JALxMpBmN0_Yk)rWwXZ=DKSe#?%#J*>q<~lt<b1Q|eBYee > z**VKy`CY&F{YA#CL3P_}bW7E1ZO=!a#TcVDeLKy}b=E)dtM!4EPi3tBoxEt{qiVa) > z_|>a*>+ieaS7CVj<IuVhv728zU*5bCefUXulM(2hn-S;Ka5nj%TOITdJ>K@3WqZ(Q > z`i083+_ndvY^Ci%#qThCaJA`MZfj<+-Sske-(;Tu>gs4KJKQZUai<^rd;NzC=dXPA > z>g~$?7ytE#yKnLg0BiWR@x30=NBDloFN7oK^WmQL0nx>=)zQ-DLvdL4Idminspnon > zc2TO;@taOy%dyt%M!VKiW3@0sj1BYAX!inEVC>r;^DNW9<50ZxN^Q$8wLAWx<X#?h > zdleQ@PZ&|xjlYK62}gy`h9Mh<`J<yT8_j<<yurS>QDFc1Uv|vKd;v4j=@yH8{-aq; > zJ(9BwWA8EN%FJ^#9GW&vU#{7$48&An02eTrcC)jS;vp~e4_Y{bDSp6H{X1u<&AjcP > zl4cgZs`zTPdwxowYAJq5s8c}Im{+N7SzZ+%tu{WKE2p1NtBO>ufB0;t=b@-PXL+0> > z8tDo@RktD*F(-AAS#@il?Xy(wJ=Q;bVYBKV!_#SZR?Nq-L_rK$$Skm4XRqV?VhT$8 > zhff9DSC5GcOG%>5vlp2|z?Y!q7?@%1ikxv8e>lovmdET~=D484=jztG>37q%eepmP > zs%0=It4dMUidS8nh9=~sG!N1b(#*c-h%_6NW-XD1r_^w0PEX8}dXPdM8+q(@)7xNf > zVSsNd+NH?(+iMn`0>}HWxLn$uUVCx#?SsX92CIj)wl-@_wk@<b*kXbDc?v;Y1!)8g > zwm6q593A0embf?~7J&2enBW1bLSiDnz@`Kf*f=7lf~J^Y0xFmYiHXq&Y+5kY2x$F@ > ztOA;rOh8pgOjQNzUSdU_j9@BgMl#jl5$Za#hR&>DDri<R)!-58I&+53oM0+w&eVxe > z*EwV8oH2CHm^u;aI&DLzZRoU3od|WE*der;o;7sNnmQ5cI=P{f8#=kE6QQm%Z|KY$ > zI`gJZgt|`bRNDD07&;54PK3J7IYZ~1p>xjEiBQ)$Z|Iyibk3VP5$ZY@44n&x&IMB^ > zLS3h0=yVL7j;Rx&uCr+9EE+nCrcQ*q&P7A#qM>uq)QM2nxn$^EGITDPIuYtRT|=j9 > z=yXk;2z8yO44tP8ou^El2z8yyhR$U}=d!62p|10^q4Tt%^R%fGp{{eq(79shTrqVb > z)ODT_%u2iyo)OGyyxE<ROz&YxOh3k+HJE1&=2?@8P-mVqnCA@UIg^P{XP!5h=MCn0 > zlZjAgK4vf<GnkK=OoTe~aluqB^SEHDJ^pdYR2w%!Vk#%OVAQ-|)VyHUL?~<8Fwlx- > zp!II^*g(-n;n+YCpsWni2vkuRsKg0NvT(m6fXXpb5~@OCDrqoKiB$%wLCHWRR4~av > z5n?pTKqWSgsHXB-GEfOUHc*5Zy1>FfB{q(TsV0{UR6+%l3>2Z#2?Lc_!GwWoP%=;n > z6-+Wvgi0q2RAL1a2C6~HKqXW#$v_b*oiI>|6-*eY1|<WPP{AYvMW}SbKqXc%VW1k6 > z3{*k|lMEE0(g_2VSiyvWYEUvz2^CB-P=rb+3{+wT69%e5$v`DkFv&m>DxEM;i4{y3 > zs0Jkil~BPX14XEG!ayZfFkzq?lnhit1(OUEq0$Khl~}=qfof1PPze=GGEjs{Ck#|# > z1rr9ULCHWRR4~av5h|T9P>B^x7^ns%1C>z0Bm+gLbizO-Rxn|p8k7uFLIsly6rs`y > z1C?08gn?>MGEfN>Ofpb}N+%3dVg(ZhszJ#>B~&oUKoKgPFi?pVOc<yJB?FaE!6XAk > zsC2?WB~~zDpc<46R6+%l3>2Z#2?Lc_!GwWoP%=;n6-+Wvgi0q2RAL1a2C6~HKqXW# > z$v_b*oiI>|6-*eY1|<WPP{AYvMW}SbKqXc%VW1k63{*k|lMEE0(g_1aq!}n8-9QoQ > z28vKKP>l%#)tF?U8j}oEW5Pf+CJa<#l7VVWGEj{P1J#%?P>o3jsxiqxH6{#HW5Pf+ > zCK;&4Bm>o$Fi=G5?7~11;f#sNBm+f=)1X3Ps=0-MB2se;14X3fmJAf3np-kZgg$f6 > z4q%~dR2YgMt7G~>dM`&`mHwUIJfEgtsr0OXo<>;h&Q1l6yHlkPk3M{x;n~PejoKUZ > zQKyfF?JhNUx;<+&`#wL|#Y2$UG0(e~nmb-+7JyE<IM4H+=GZJfjKVMFyVS?C_-sb+ > z@A3skEexWACnK#H+#^)fZ}9>ir_jS18^i7vm-*7C^K3Q^cVpPU8Fpf1a~I@u2^s=M > zQ*ny}zu9FUP@5l!o&|c+!NxQo5uYA(J6-GUZafAmPyKQAYN9$GEBxf@RjqneRIg%u > zdiAP1y?V8O7?5@^QJtqAp}eP+_e6OwQQkYDycd`EljZTa>?f=Dwer3w?<dOpCzSW& > z^0j37%p;VqY2|C8d@WJFc0&1DTz(^2KKlseH?;B_qWngp{Kg68H{$a3Wcl19l&@>$ > z>!N%;QNDgc`FdQwkt{#+2<01E`GzRpNR)4!P`-ik8QRK{<sDgGZ63I-pvP?DsioQH > z=ENnBFS@Sdbjiz@7tM6I*O*8r`=+LwxGopXbhrVTNGJQIrkl9VmdtdxE15_q`=+Lw > zxNclC9d1=7(#gK5=_am&Q)W8czf7c)eN)p-tm|bn9d2qS(#gK5=_c0cX)_(}bSBct > zJ~N$DeL=n<@c8wI&&u1KT34+^jtda}Gv~FhmKtx}j{fw{A4~hU-hS(y{omhuoAoWP > z-P(*-<Q(U9>$}2t1*?EZ+wdJ2V*a}V9_ZhBf`SSwdGuV~Yi@NduM@mtd9|0z7Rz{E > z;M@H>iv&u0-CC;zq+jWk@F=;t)wCl<o&CewGuOo%1dK(FZ?59Co8|IqnMNSrzGU5^ > z*V}k4HR8iv_K{aEm-pO8&O`Ot{u9@qnSq>;vcbQJ9-yHPA+;pprzd4NVA@rAJe > z)Wnh2<~VeNEGc@lLRK;rGEB|yHs-yqw`HFs!wSEy<cA^NWP}k*CdCYo>X`{a;!I$= > zl5kAE6-|r`=*SB1ZDvM|$yDNePEW<tleqv#uIWO9S#P`BE1V>B>iL=&EEGq3Q*=c< > zmFy+x8_7;g!PL?9KYWQ+6)rIRVS!?0*<)H?Sc5^OSIl#p;paTv=(+QF_t(pl7`D6j > z1}@QS0dvn`5!c(bL9M)o(<EL{(SoLPi+ww*U57!VEjP)z_Ex)GDPKkjx)y_5e|~HK > zz0&T)<tUv&T!*+l<ZOkm!$v#kSoj^zSNeyI7v&X%%yKtRWcHguP<s8=U-szYwgZ2w > zgIA@`X;VW(qJ-(7Q*D*-(ZENGS}nBN*9N8jooA-tE9?pnYTbbq(2@&iu>`f32gv^9 > z6m&YEO(58=y@HRfS8|uc4en@mNJGLqG2C7Et)S&?GPjZoFmvqTF|7ao6Ljq8=<DBo > z84@E3DdA{oD63<Y;xZ9KxwkxWhjKX7ODmP4{gBosB^g`pvjOd7TH5FzyG$g^4}~VV > zB?xrrR#pPs>|zg)Lr_>d=v=;3T<7Jxzhta{S4!sJk4N%P^~T{5eku@=&tv2**<3`k > zA)7yI)l~1(XvV$ELFHPEJ&R@|qSHhqYP2X38m$HqsL`S%`_xSh;!+?E6MN)-NquyD > zpJGdN7DcCZ`?!Z%Ejrd3`7=Z6#`@{3uN5!w^B=={9!e&uB$FfxdFyMOBmDrH(3}T1 > zZZ2a$gNQ2A;fRvc!HC95hvG9twBzRGOYW0chk=zm0THcEby7q3=U7<1%miqU7G3lP > ztvZ>~y@+*MPE7#C@|BSO;dU-)m+`*<ba9s7#pa;gvv8p90BKRsam2J&jA>29G3^Rt > z8ncKOY0pTHX?PT4-*BA)`Q(@eBD&mSfW;bH997=aHkEa?sn9h}mxklgb|Xd<`z1CK > zr@kO}0{Kr27+moR=l}d<j-~L7j?NB!;I;OH!h?ckqu-Ke@CO`zP&kg~#JT)lKf@en > zmVTGgUy6w%;#V)i`$tFL{3s5v`!7vi2zbIwtKu8+ZvzrWpZD=x6?ZfEpMea7$bY#= > z>|4~Y-In0r9(om?*S5n_u9UOz#@+eFb)1~;`d~4au`n#OFAWy$3{P=XNAxQ?LRFpL > zKAfQb;d2|Wdh(WVZl@Mpg#oM)I#I>RI$65;akjJ9#ar)sR{pm`zFRKiH9!4dNdNHZ > pZu!?Tz+74k_o>}gADbFA)>LDi8vVnC?rKZ~HpSdr+uQ1~{{x5db9n#& > > literal 0 > HcmV?d00001 > > diff --git a/tests/acpi-test-data/q35/SRAT.mmio64 b/tests/acpi-test-data/q35/SRAT.mmio64 > new file mode 100644 > index 0000000000000000000000000000000000000000..ac35f3dac4f47b86e41c7f35ee40bac14174b37e > GIT binary patch > literal 224 > zcmWFzatwLEz`($0?d0$55v<@85#SsQ6k`O6f!H7#gyBE{mCvXFmw__4-~!0{5bA&i > cfWZfLm_qF8V6xb0gn4lH?0~6chB1IN0P8^t0RR91 > > literal 0 > HcmV?d00001 > Reviewed-by: Marcel Apfelbaum<marcel.apfelbaum@gmail.com> Thanks, Marcel Reviewed-by: Marcel Apfelbaum<marcel.apfelbaum@gmail.com> Thanks, Marcel Reviewed-by: Marcel Apfelbaum<marcel.apfelbaum@gmail.com> Thanks, Marcel
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c index 4e24930c4bf9..9dd88f9d868a 100644 --- a/tests/bios-tables-test.c +++ b/tests/bios-tables-test.c @@ -708,6 +708,21 @@ static void test_acpi_q35_tcg_bridge(void) free_test_data(&data); } +static void test_acpi_q35_tcg_mmio64(void) +{ + test_data data = { + .machine = MACHINE_Q35, + .variant = ".mmio64", + .required_struct_types = base_required_struct_types, + .required_struct_types_len = ARRAY_SIZE(base_required_struct_types) + }; + + test_acpi_one("-m 128M,slots=1,maxmem=2G " + "-device pci-testdev,membar=2G", + &data); + free_test_data(&data); +} + static void test_acpi_piix4_tcg_cphp(void) { test_data data; @@ -875,6 +890,7 @@ int main(int argc, char *argv[]) qtest_add_func("acpi/piix4/bridge", test_acpi_piix4_tcg_bridge); qtest_add_func("acpi/q35", test_acpi_q35_tcg); qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge); + qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64); qtest_add_func("acpi/piix4/ipmi", test_acpi_piix4_tcg_ipmi); qtest_add_func("acpi/q35/ipmi", test_acpi_q35_tcg_ipmi); qtest_add_func("acpi/piix4/cpuhp", test_acpi_piix4_tcg_cphp);