All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35
@ 2021-09-20  7:00 Ani Sinha
  2021-09-20  7:00 ` [PATCH v3 1/3] tests/acpi/bios-tables-test: add and allow changes to a new q35 DSDT table blob Ani Sinha
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Ani Sinha @ 2021-09-20  7:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: Ani Sinha, imammedo, mst

This patchset adds a unit test to exercize acpi hotplug support for multifunction
bridges on q35 machines. This support was added with the commit:

d7346e614f4ec ("acpi: x86: pcihp: add support hotplug on multifunction bridges")

changelist:
v1 : initial RFC patch.
v2: incorporated some of the feedbacks from Igor.
v3: forgot to add the ASL diff for patch 3 in commit log for v2. Added now.

Ani Sinha (3):
  tests/acpi/bios-tables-test: add and allow changes to a new q35 DSDT
    table blob
  tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges
    for q35
  tests/acpi/bios-tables-test: update DSDT blob for multifunction bridge
    test

 tests/data/acpi/q35/DSDT.multi-bridge | Bin 0 -> 8435 bytes
 tests/qtest/bios-tables-test.c        |  18 ++++++++++++++++++
 2 files changed, 18 insertions(+)
 create mode 100644 tests/data/acpi/q35/DSDT.multi-bridge

-- 
2.25.1



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

* [PATCH v3 1/3] tests/acpi/bios-tables-test: add and allow changes to a new q35 DSDT table blob
  2021-09-20  7:00 [PATCH v3 0/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35 Ani Sinha
@ 2021-09-20  7:00 ` Ani Sinha
  2021-10-04 11:34   ` Ani Sinha
  2021-09-20  7:00 ` [PATCH v3 2/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35 Ani Sinha
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Ani Sinha @ 2021-09-20  7:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: Ani Sinha, imammedo, mst

We are adding a new unit test to cover the acpi hotplug support in q35 for
multi-function bridges. This test uses a new table DSDT.multi-bridge.
We need to allow changes in DSDT acpi table for addition of this new
unit test.

Signed-off-by: Ani Sinha <ani@anisinha.ca>
---
 tests/data/acpi/q35/DSDT.multi-bridge       | 0
 tests/qtest/bios-tables-test-allowed-diff.h | 1 +
 2 files changed, 1 insertion(+)
 create mode 100644 tests/data/acpi/q35/DSDT.multi-bridge

diff --git a/tests/data/acpi/q35/DSDT.multi-bridge b/tests/data/acpi/q35/DSDT.multi-bridge
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..dabc024f53 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,2 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/q35/DSDT.multi-bridge",
-- 
2.25.1



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

* [PATCH v3 2/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35
  2021-09-20  7:00 [PATCH v3 0/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35 Ani Sinha
  2021-09-20  7:00 ` [PATCH v3 1/3] tests/acpi/bios-tables-test: add and allow changes to a new q35 DSDT table blob Ani Sinha
@ 2021-09-20  7:00 ` Ani Sinha
  2021-10-07  8:07   ` Igor Mammedov
  2021-09-20  7:00 ` [PATCH v3 3/3] tests/acpi/bios-tables-test: update DSDT blob for multifunction bridge test Ani Sinha
  2021-09-23  7:14 ` [PATCH v3 0/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35 Ani Sinha
  3 siblings, 1 reply; 14+ messages in thread
From: Ani Sinha @ 2021-09-20  7:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: Ani Sinha, imammedo, mst

commit d7346e614f4ec ("acpi: x86: pcihp: add support hotplug on multifunction bridges")
added ACPI hotplug descriptions for cold plugged bridges for functions other
than 0. For all other devices, the ACPI hotplug descriptions are limited to
function 0 only. This change adds unit tests for this feature.

This test adds the following devices to qemu and then checks the changes
introduced in the DSDT table due to the addition of the following devices:

(a) a multifunction bridge device
(b) a bridge device with function 1
(c) a non-bridge device with function 2

In the DSDT table, we should see AML hotplug descriptions for (a) and (b).
For (a) we should find a hotplug AML description for function 0.

The following diff compares the DSDT table AML with the new unit test before
and after the change d7346e614f4ec is introduced. In other words,
this diff reflects the changes that occurs in the DSDT table due to the change
d7346e614f4ec .

@@ -5,13 +5,13 @@
  *
  * Disassembling to symbolic ASL+ operators
  *
- * Disassembly of /tmp/aml-7A7890, Sat Sep 18 13:13:29 2021
+ * Disassembly of /tmp/aml-PE4S90, Sat Sep 18 13:08:54 2021
  *
  * Original Table Header:
  *     Signature        "DSDT"
- *     Length           0x0000206A (8298)
+ *     Length           0x000020F3 (8435)
  *     Revision         0x01 **** 32-bit table (V1), no 64-bit math support
- *     Checksum         0x59
+ *     Checksum         0x1B
  *     OEM ID           "BOCHS "
  *     OEM Table ID     "BXPC    "
  *     OEM Revision     0x00000001 (1)
@@ -20,28 +20,6 @@
  */
 DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPC    ", 0x00000001)
 {
-    /*
-     * iASL Warning: There was 1 external control method found during
-     * disassembly, but only 0 were resolved (1 unresolved). Additional
-     * ACPI tables may be required to properly disassemble the code. This
-     * resulting disassembler output file may not compile because the
-     * disassembler did not know how many arguments to assign to the
-     * unresolved methods. Note: SSDTs can be dynamically loaded at
-     * runtime and may or may not be available via the host OS.
-     *
-     * In addition, the -fe option can be used to specify a file containing
-     * control method external declarations with the associated method
-     * argument counts. Each line of the file must be of the form:
-     *     External (<method pathname>, MethodObj, <argument count>)
-     * Invocation:
-     *     iasl -fe refs.txt -d dsdt.aml
-     *
-     * The following methods were unresolved and many not compile properly
-     * because the disassembler had to guess at the number of arguments
-     * required for each:
-     */
-    External (_SB_.PCI0.S09_.PCNT, MethodObj)    // Warning: Unknown method, guessing 1 arguments
-
     Scope (\)
     {
         OperationRegion (DBG, SystemIO, 0x0402, One)
@@ -3280,9 +3258,45 @@
                 }
             }

+            Device (S09)
+            {
+                Name (_ADR, 0x00010001)  // _ADR: Address
+                Name (BSEL, Zero)
+                Device (S00)
+                {
+                    Name (_SUN, Zero)  // _SUN: Slot User Number
+                    Name (_ADR, Zero)  // _ADR: Address
+                    Method (_EJ0, 1, NotSerialized)  // _EJx: Eject Device, x=0-9
+                    {
+                        PCEJ (BSEL, _SUN)
+                    }
+
+                    Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
+                    {
+                        Return (PDSM (Arg0, Arg1, Arg2, Arg3, BSEL, _SUN))
+                    }
+                }
+
+                Method (DVNT, 2, NotSerialized)
+                {
+                    If ((Arg0 & One))
+                    {
+                        Notify (S00, Arg1)
+                    }
+                }
+
+                Method (PCNT, 0, NotSerialized)
+                {
+                    BNUM = Zero
+                    DVNT (PCIU, One)
+                    DVNT (PCID, 0x03)
+                }
+            }
+
             Method (PCNT, 0, NotSerialized)
             {
-                ^S09.PCNT (^S08.PCNT ())
+                ^S09.PCNT ()
+                ^S08.PCNT ()
             }
         }
     }

Signed-off-by: Ani Sinha <ani@anisinha.ca>
---
 tests/qtest/bios-tables-test.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 4f11d03055..d4cd77ea02 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -859,6 +859,23 @@ static void test_acpi_q35_tcg_bridge(void)
     free_test_data(&data);
 }
 
+static void test_acpi_q35_multif_bridge(void)
+{
+    test_data data = {
+        .machine = MACHINE_Q35,
+        .variant = ".multi-bridge",
+    };
+    test_acpi_one("-nodefaults -device pcie-root-port,id=pcie-root-port-0,"
+                  "multifunction=on,"
+                  "port=0x0,chassis=1,addr=0x1,bus=pcie.0 "
+                  "-device pcie-root-port,id=pcie-root-port-1,"
+                  "port=0x1,chassis=2,addr=0x1.0x1,bus=pcie.0 "
+                  "-device virtio-balloon,id=balloon0,"
+                  "bus=pcie.0,addr=0x1.0x2",
+                  &data);
+    free_test_data(&data);
+}
+
 static void test_acpi_q35_tcg_mmio64(void)
 {
     test_data data = {
@@ -1534,6 +1551,7 @@ int main(int argc, char *argv[])
                        test_acpi_piix4_no_acpi_pci_hotplug);
         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/multif-bridge", test_acpi_q35_multif_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);
-- 
2.25.1



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

* [PATCH v3 3/3] tests/acpi/bios-tables-test: update DSDT blob for multifunction bridge test
  2021-09-20  7:00 [PATCH v3 0/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35 Ani Sinha
  2021-09-20  7:00 ` [PATCH v3 1/3] tests/acpi/bios-tables-test: add and allow changes to a new q35 DSDT table blob Ani Sinha
  2021-09-20  7:00 ` [PATCH v3 2/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35 Ani Sinha
@ 2021-09-20  7:00 ` Ani Sinha
  2021-10-04 11:34   ` Ani Sinha
  2021-09-23  7:14 ` [PATCH v3 0/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35 Ani Sinha
  3 siblings, 1 reply; 14+ messages in thread
From: Ani Sinha @ 2021-09-20  7:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: Ani Sinha, imammedo, mst

We added a new unit test for testing acpi hotplug on multifunction bridges in
q35 machines. Here, we update the DSDT table gloden master blob for this unit
test.

Following is the ASL diff between the original DSDT table and the modified DSDT
table due to the unit test:

@@ -5,13 +5,13 @@
  *
  * Disassembling to symbolic ASL+ operators
  *
- * Disassembly of tests/data/acpi/q35/DSDT, Mon Sep 20 12:22:11 2021
+ * Disassembly of tests/data/acpi/q35/DSDT.multi-bridge, Mon Sep 20 12:22:31 2021
  *
  * Original Table Header:
  *     Signature        "DSDT"
- *     Length           0x00002061 (8289)
+ *     Length           0x000020F3 (8435)
  *     Revision         0x01 **** 32-bit table (V1), no 64-bit math support
- *     Checksum         0xE5
+ *     Checksum         0x1B
  *     OEM ID           "BOCHS "
  *     OEM Table ID     "BXPC    "
  *     OEM Revision     0x00000001 (1)
@@ -184,42 +184,6 @@
             })
         }

-        Device (LPT1)
-        {
-            Name (_HID, EisaId ("PNP0400") /* Standard LPT Parallel Port */)  // _HID: Hardware ID
-            Name (_UID, One)  // _UID: Unique ID
-            Name (_STA, 0x0F)  // _STA: Status
-            Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
-            {
-                IO (Decode16,
-                    0x0378,             // Range Minimum
-                    0x0378,             // Range Maximum
-                    0x08,               // Alignment
-                    0x08,               // Length
-                    )
-                IRQNoFlags ()
-                    {7}
-            })
-        }
-
-        Device (COM1)
-        {
-            Name (_HID, EisaId ("PNP0501") /* 16550A-compatible COM Serial Port */)  // _HID: Hardware ID
-            Name (_UID, One)  // _UID: Unique ID
-            Name (_STA, 0x0F)  // _STA: Status
-            Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
-            {
-                IO (Decode16,
-                    0x03F8,             // Range Minimum
-                    0x03F8,             // Range Maximum
-                    0x00,               // Alignment
-                    0x08,               // Length
-                    )
-                IRQNoFlags ()
-                    {4}
-            })
-        }
-
         Device (RTC)
         {
             Name (_HID, EisaId ("PNP0B00") /* AT Real-Time Clock */)  // _HID: Hardware ID
@@ -3262,24 +3226,77 @@
             Device (S08)
             {
                 Name (_ADR, 0x00010000)  // _ADR: Address
-                Method (_S1D, 0, NotSerialized)  // _S1D: S1 Device State
+                Name (BSEL, One)
+                Device (S00)
                 {
-                    Return (Zero)
+                    Name (_SUN, Zero)  // _SUN: Slot User Number
+                    Name (_ADR, Zero)  // _ADR: Address
+                    Method (_EJ0, 1, NotSerialized)  // _EJx: Eject Device, x=0-9
+                    {
+                        PCEJ (BSEL, _SUN)
+                    }
+
+                    Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
+                    {
+                        Return (PDSM (Arg0, Arg1, Arg2, Arg3, BSEL, _SUN))
+                    }
                 }

-                Method (_S2D, 0, NotSerialized)  // _S2D: S2 Device State
+                Method (DVNT, 2, NotSerialized)
                 {
-                    Return (Zero)
+                    If ((Arg0 & One))
+                    {
+                        Notify (S00, Arg1)
+                    }
                 }

-                Method (_S3D, 0, NotSerialized)  // _S3D: S3 Device State
+                Method (PCNT, 0, NotSerialized)
                 {
-                    Return (Zero)
+                    BNUM = One
+                    DVNT (PCIU, One)
+                    DVNT (PCID, 0x03)
+                }
+            }
+
+            Device (S09)
+            {
+                Name (_ADR, 0x00010001)  // _ADR: Address
+                Name (BSEL, Zero)
+                Device (S00)
+                {
+                    Name (_SUN, Zero)  // _SUN: Slot User Number
+                    Name (_ADR, Zero)  // _ADR: Address
+                    Method (_EJ0, 1, NotSerialized)  // _EJx: Eject Device, x=0-9
+                    {
+                        PCEJ (BSEL, _SUN)
+                    }
+
+                    Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
+                    {
+                        Return (PDSM (Arg0, Arg1, Arg2, Arg3, BSEL, _SUN))
+                    }
+                }
+
+                Method (DVNT, 2, NotSerialized)
+                {
+                    If ((Arg0 & One))
+                    {
+                        Notify (S00, Arg1)
+                    }
+                }
+
+                Method (PCNT, 0, NotSerialized)
+                {
+                    BNUM = Zero
+                    DVNT (PCIU, One)
+                    DVNT (PCID, 0x03)
                 }
             }

             Method (PCNT, 0, NotSerialized)
             {
+                ^S09.PCNT ()
+                ^S08.PCNT ()
             }
         }
     }

Signed-off-by: Ani Sinha <ani@anisinha.ca>
---
 tests/data/acpi/q35/DSDT.multi-bridge       | Bin 0 -> 8435 bytes
 tests/qtest/bios-tables-test-allowed-diff.h |   1 -
 2 files changed, 1 deletion(-)

diff --git a/tests/data/acpi/q35/DSDT.multi-bridge b/tests/data/acpi/q35/DSDT.multi-bridge
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..a6565acc5cc390826f4ac23585912f9cf1d9acb9 100644
GIT binary patch
literal 8435
zcmcIpOKcm*8J;C6X|-HROKN4=vJ(+`^nnx!73bmf5ro|3%NA*hOT`Ijlq<_=B|AkH
zNqk6*0I~ukjt@l>HbIYczyQ6qr(PSNxfJNZhXM`IQxCo58WhPX?Dx;?$TK7bREP4g
z+JE-@=KsIh+5J{Ce&Bb$&NF75E?x9$LB8}-%gf_uFvjSd{yO#4W!ByIOVyr{PNa<P
ztu#N!p=7#0@=IG)<9EB^Ct>*J`=PNNu`8cCA79yy-n|`OVFbEyC9*6!amow5pw#Og
zcvj1+7tCI*?%TzCmTC4pah5cDMZe9=-d5db?#5j2V&J9jTxXttX?@U~>F;KjET`M}
z=h<)8&b;u+r8kS8|Lh;XxO3e~0q`XLn)rJ*qC<EywAR9b^HG1#c!%ix+WKJWqdvbZ
z+8nwPh17D_7ERRE`tW-9;GEwsdc4L=tJx}eg>}^BwS(-Mb>{sQ#%vlK`u%Y<8vR+E
zGUqMkG)$qgY}Q)kj#Ozt8>#B<*IJzhvl+YhU7Dr3w;YO>UMx5Le5>vE^6oRepi^WK
zwS*BhZND8_PB^eX?1!u$rbnY88!UX--)6tvw%Nb_Gc#mEYf;pewHDr+r&{iP$5M=;
zn(sYM)zEl%4_50zvB!rP>DN$7tKQyBSfOP*-ux3u2_#ha;He8GA7F8hlBgb4$uuXe
z<*E^~kQqkRxXP~L??VY>>mEGPxVUx5T3pgtrZw{rjZpXC(RRl>M5W_#b*xO)h*-o7
z^C4!S^k(*c?fWe+=oslQY|F%uQQ$$-RvrgG-`x+&=P^6~W?LB0GzDI%P(F_frDmgT
zr2j5x6nLfPRwLi4`!D6YMrp&lz}!b$Uio~Y`!GeLQM5~l=q+Q9Q6Js!Q}d9yj}^h0
zhN8YAI)pJcbp6?uA2b#Q8}XFA_v~N)9X$PZ<HMD&8mIo^H6rTk)nA2!hx^yq;8cIl
zN?>Nb!|$(+=`>=)Y|^+z2X=R5u-w09%|*GH1k14PD7?MR*l^oy)x8RH?H>Lb(Jn>K
zuU|IkCOY0%`~hpVJFVP>H}-Ss6lM+EXkI8Yac<ze!g4mXTL}bJ86*+ZST2)79hl()
z7e~Yba6w$-JV0egO!UvO3C;vIj)*Ct3C5X#awbAzq8g4(a;B^x$%rYTNx=k^GZ7L~
zX2E<HSl&;HGbJ=7n6h|;s?Hfr=L}~`=!{^>;t{GkXEmL(oGGERx=w_u&N)rzoThV5
z*NIToX=*x6O{b~rM5yY-n?V`rc}?fMt`niE)6#TWnodjCiBQ#<)^w&dooQVsLRBZ;
zQOf<aHJ!Gu6QQazqv_0OIy1UXgsRR3P3MB9b3xaMP}RAp>0H!wF6ufFsyZD_r=#g~
zbe#xQomow1R@0f)bs|)C<}{r-O=nKmiBQ$Kr0HDJbS~*S5vn>}O{c5rbakBwRh`S4
z&Sg#KvaS=Ms`G@V^Mt1Jgsu~zsuK>6*(cvaE1J#~T_-|S=SfZHNloWTT_-|S=PAxC
z#&5z?oLP!rcBcf>yB`wMkFl#7b5&!m>P&<x^R&i1tuasQOoS@)jK(~pG0*5sgevm^
zjroAad_ZR+RGANQrgWJHIa9vl9~4aa;zmeJc?Z|DoNHRnH9aRnk<)~M78L`n26d$7
zW@_LZiDLsrfV3h=B9KX8paRD&$za5i^%+Pipoa#EP)dV=3am6x1xf}gpoa#E5Y<Qn
z71%hUoT`Be=%IlkL`5760~OdfiZya58K{8DP}K<o72R?s3{-)Vfg+TyNCt{f>V$y`
zEN8+%6(|{~fbyJVpa`W-7^uK<CJa=8l7R{+XOe*;lsaLc0?U~&Pz6c`DxjQ628vML
zPZ+4cawZH^fs%m=C})y^B9uB|paRR8Fi-_b1}dPONd}5g>V$y`EN8+%6(|{~fN~}o
zC_<?d1}d<e2?JH2WS|1dnPi{{rA`>Az;Y%GRDqI#3Mglifg+STVW0xbnJ`cVN(L&R
zoJj_XQ0jz%3M^;BKouw%sDN@N87M-j69y`<oCyO}pk$x|%9&)K2&GOKsK9b23{-)V
zfeI*Rl7S+WI$@v!%b74x1xf}gpqxnticso=feI{V!ax-$8K{7ACK)I~sS^e&u$&14
zRiI>`0?L_Wpa`W-7^uK<CJa=8l7R{+XOe*;lsaLc0?U~&Pz6c`DxjQ628vMXgn<ey
zXTm@gC>f}LawZulLa7r5DzKah16818paRO7WS|J8P8cX6#Xu3M28vKMP=tzsDohxt
z!XyJ#m}H;|69%d<VW0|=3{+u~fhtTGsKSJSDoip^g-HggFkzqy69%d<$v_n*8K}a9
zfg+N37Y2$5cZ?4v87M;B1{o4ljx7unksMnXC?YwwWS|J;*ph)F#1hVG4;IP>cAx*V
zI;4L{@6OVnV)xdMpGeYwskGWa>kvk(y;+3gZkFia(ZM$v)<ibTbY7uDl@2x2U8-+l
zHDEA*)7sy~;>Y~ZO1n?hH@)^e0PR9{!AgHH%jWqC&0FjawOKPb)}x!d)*_=b6w$_#
z$i^JL5sGq|!^ScSEz+1MJIJ1}mOfZu^GUcH!~2`zO^h`Tcag8p45<WEQ*^UEzaFr6
z=-k@pEsL~H#DMVHJYMMq?ZCLR8`nVTsXfkK8q01RBYR27UgFtHs87vaa;Il6bq^Y(
zT`X~787|gtC61BaQ__1py*HNLJ0iUor}xLiTUcoNpU2);()&EUKbGD<BE27{FOR2B
z9V2~NNnhsa%VX)wN2D*u=_}*uXO5A+qNK0z^p&ynl_Szu;`G(=^s~oEUscjqdHU*D
z`sxwst8x0;c>1|xq^~LIYdn2zEPd^W^fjbU(W`7cy(7}gmj^y8@VJY0RlUuf<&Ql6
z&~qJkH~tt~S-m@aYfN+}+NO3l@m%Kg?(hLJ(Vb|U+TFx+hNW40&ZIDWl}vOe+NO3l
z@!Yt2clcD9=uWgv?QY^ZSk}A4_sc|gqHSt-6Z86n-W@(_Cb|=CQ@fj(rz?7Q_|lo^
zPPFOWIj8ThXCEvEjS{v%u-#!Nu@>)Kd|vDt)U3<*SK?dkav*1^g@+T~l<`_y`Pyr5
zMz7xZW&ZV7-gxcC>pQQ!!McXm+PDzU){JFUjW6vlZNtWDJ3NV&&cC!RyL;;g6ckzB
zYn7XZS8oP}*KWLMU;(^9hl;_Hp4afP$etr)(hJHPdBFW*Cy%w}db4guj2bcP&tB$w
zP>?7xQ}kN(LSel?Wrz)(88_>7DlaDnR)3eh=M@TtJ-0TC)%)Un_u<P=q~w+Z2_*rv
z{65}{WDyGotNrl{Bq6dEvFtu^W#hsybQ?1y>v)Nb@vhLr)c)Pd2`})P<`FVXuUWp7
zON=766B$=O^>h8J*xR@o8DYe>Ki#l@(+`c0chD3@JL4(2;PFP>l)=i!<E^+YJD&~a
z``gwCX+DdlT^~F$F>-X@qn+DS<2JjojjHZGJ9v5m#%Bg@-;R}xCI$u<!Mm|T8-C@K
z*zuzdudp4u<<Pd%U7s#)qA%2Q<I415Zp?cZgN}W~Fi)Q?6a8cGqP;0&6IqGzR)Ss|
zZ^ST9t*-m-IhxaW{_(@(fq88XX?EiYXcRlyv}H1^HqdvdyMWEiPMXB9-Mwq>X*@jD
zR=HO$JjvoGo*pEcQ_jT{UE&XZ{7e;|VeWIlt}_~l!<{;@W2H2-ye6+yLpY6P@uc!i
zA-;!sK5hN!c7`ReM1bFj4%o^2*8bKuOtc$#C*k)EE`Mtu#u@A(G4?gz{N&f^vv@S3
z&tm!|n6@oyVjJrZ;tKn{cI9*ImwqR%eE$5M$FP+e_a@qCv}g|u1E>8crp-eyg~1WK
z$9!kDh`|xBMrv2yiDL6HEgYP`6Z5nAS%dFDo=>%Et+p6q+TOk22?lA(`*pIfFYGUD
z<0Q5ze|Q<`gR{NdY|23GIDe{_GgDT=!Xh4`r|AkQaei|DHgpdjuUztk?`1a2jV(;0
zCkc&BR<&Yh#+jkFxKnd4^R2xXT)K<hgLF_Rh`n44N!1Jr=R|~CW-zmacL+XAekwr0
zRuf&*&bpiH(VLz&JPFdn6?`ROuhJgUjwL4M^~3g@|6;R~7^H9R8tgslXwO}v`NblG
zc1Y<eO4%~v6W>aTLY|8Y!E&c41O@(|3SmbS;;!;SfBtw-cn*8t%?_QN<7fW`tJddg

literal 0
HcmV?d00001

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dabc024f53..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,2 +1 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/q35/DSDT.multi-bridge",
-- 
2.25.1



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

* Re: [PATCH v3 0/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35
  2021-09-20  7:00 [PATCH v3 0/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35 Ani Sinha
                   ` (2 preceding siblings ...)
  2021-09-20  7:00 ` [PATCH v3 3/3] tests/acpi/bios-tables-test: update DSDT blob for multifunction bridge test Ani Sinha
@ 2021-09-23  7:14 ` Ani Sinha
  2021-10-04 11:35   ` Ani Sinha
  3 siblings, 1 reply; 14+ messages in thread
From: Ani Sinha @ 2021-09-23  7:14 UTC (permalink / raw)
  To: Ani Sinha; +Cc: imammedo, qemu-devel, mst

ping ...

On Mon, 20 Sep 2021, Ani Sinha wrote:

> This patchset adds a unit test to exercize acpi hotplug support for multifunction
> bridges on q35 machines. This support was added with the commit:
>
> d7346e614f4ec ("acpi: x86: pcihp: add support hotplug on multifunction bridges")
>
> changelist:
> v1 : initial RFC patch.
> v2: incorporated some of the feedbacks from Igor.
> v3: forgot to add the ASL diff for patch 3 in commit log for v2. Added now.
>
> Ani Sinha (3):
>   tests/acpi/bios-tables-test: add and allow changes to a new q35 DSDT
>     table blob
>   tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges
>     for q35
>   tests/acpi/bios-tables-test: update DSDT blob for multifunction bridge
>     test
>
>  tests/data/acpi/q35/DSDT.multi-bridge | Bin 0 -> 8435 bytes
>  tests/qtest/bios-tables-test.c        |  18 ++++++++++++++++++
>  2 files changed, 18 insertions(+)
>  create mode 100644 tests/data/acpi/q35/DSDT.multi-bridge
>
> --
> 2.25.1
>
>


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

* Re: [PATCH v3 1/3] tests/acpi/bios-tables-test: add and allow changes to a new q35 DSDT table blob
  2021-09-20  7:00 ` [PATCH v3 1/3] tests/acpi/bios-tables-test: add and allow changes to a new q35 DSDT table blob Ani Sinha
@ 2021-10-04 11:34   ` Ani Sinha
  0 siblings, 0 replies; 14+ messages in thread
From: Ani Sinha @ 2021-10-04 11:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: imammedo, mst

ping ...

On Mon, Sep 20, 2021 at 12:31 PM Ani Sinha <ani@anisinha.ca> wrote:
>
> We are adding a new unit test to cover the acpi hotplug support in q35 for
> multi-function bridges. This test uses a new table DSDT.multi-bridge.
> We need to allow changes in DSDT acpi table for addition of this new
> unit test.
>
> Signed-off-by: Ani Sinha <ani@anisinha.ca>
> ---
>  tests/data/acpi/q35/DSDT.multi-bridge       | 0
>  tests/qtest/bios-tables-test-allowed-diff.h | 1 +
>  2 files changed, 1 insertion(+)
>  create mode 100644 tests/data/acpi/q35/DSDT.multi-bridge
>
> diff --git a/tests/data/acpi/q35/DSDT.multi-bridge b/tests/data/acpi/q35/DSDT.multi-bridge
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index dfb8523c8b..dabc024f53 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1 +1,2 @@
>  /* List of comma-separated changed AML files to ignore */
> +"tests/data/acpi/q35/DSDT.multi-bridge",
> --
> 2.25.1
>


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

* Re: [PATCH v3 3/3] tests/acpi/bios-tables-test: update DSDT blob for multifunction bridge test
  2021-09-20  7:00 ` [PATCH v3 3/3] tests/acpi/bios-tables-test: update DSDT blob for multifunction bridge test Ani Sinha
@ 2021-10-04 11:34   ` Ani Sinha
  0 siblings, 0 replies; 14+ messages in thread
From: Ani Sinha @ 2021-10-04 11:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: imammedo, mst

ping

On Mon, Sep 20, 2021 at 12:31 PM Ani Sinha <ani@anisinha.ca> wrote:
>
> We added a new unit test for testing acpi hotplug on multifunction bridges in
> q35 machines. Here, we update the DSDT table gloden master blob for this unit
> test.
>
> Following is the ASL diff between the original DSDT table and the modified DSDT
> table due to the unit test:
>
> @@ -5,13 +5,13 @@
>   *
>   * Disassembling to symbolic ASL+ operators
>   *
> - * Disassembly of tests/data/acpi/q35/DSDT, Mon Sep 20 12:22:11 2021
> + * Disassembly of tests/data/acpi/q35/DSDT.multi-bridge, Mon Sep 20 12:22:31 2021
>   *
>   * Original Table Header:
>   *     Signature        "DSDT"
> - *     Length           0x00002061 (8289)
> + *     Length           0x000020F3 (8435)
>   *     Revision         0x01 **** 32-bit table (V1), no 64-bit math support
> - *     Checksum         0xE5
> + *     Checksum         0x1B
>   *     OEM ID           "BOCHS "
>   *     OEM Table ID     "BXPC    "
>   *     OEM Revision     0x00000001 (1)
> @@ -184,42 +184,6 @@
>              })
>          }
>
> -        Device (LPT1)
> -        {
> -            Name (_HID, EisaId ("PNP0400") /* Standard LPT Parallel Port */)  // _HID: Hardware ID
> -            Name (_UID, One)  // _UID: Unique ID
> -            Name (_STA, 0x0F)  // _STA: Status
> -            Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
> -            {
> -                IO (Decode16,
> -                    0x0378,             // Range Minimum
> -                    0x0378,             // Range Maximum
> -                    0x08,               // Alignment
> -                    0x08,               // Length
> -                    )
> -                IRQNoFlags ()
> -                    {7}
> -            })
> -        }
> -
> -        Device (COM1)
> -        {
> -            Name (_HID, EisaId ("PNP0501") /* 16550A-compatible COM Serial Port */)  // _HID: Hardware ID
> -            Name (_UID, One)  // _UID: Unique ID
> -            Name (_STA, 0x0F)  // _STA: Status
> -            Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
> -            {
> -                IO (Decode16,
> -                    0x03F8,             // Range Minimum
> -                    0x03F8,             // Range Maximum
> -                    0x00,               // Alignment
> -                    0x08,               // Length
> -                    )
> -                IRQNoFlags ()
> -                    {4}
> -            })
> -        }
> -
>          Device (RTC)
>          {
>              Name (_HID, EisaId ("PNP0B00") /* AT Real-Time Clock */)  // _HID: Hardware ID
> @@ -3262,24 +3226,77 @@
>              Device (S08)
>              {
>                  Name (_ADR, 0x00010000)  // _ADR: Address
> -                Method (_S1D, 0, NotSerialized)  // _S1D: S1 Device State
> +                Name (BSEL, One)
> +                Device (S00)
>                  {
> -                    Return (Zero)
> +                    Name (_SUN, Zero)  // _SUN: Slot User Number
> +                    Name (_ADR, Zero)  // _ADR: Address
> +                    Method (_EJ0, 1, NotSerialized)  // _EJx: Eject Device, x=0-9
> +                    {
> +                        PCEJ (BSEL, _SUN)
> +                    }
> +
> +                    Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
> +                    {
> +                        Return (PDSM (Arg0, Arg1, Arg2, Arg3, BSEL, _SUN))
> +                    }
>                  }
>
> -                Method (_S2D, 0, NotSerialized)  // _S2D: S2 Device State
> +                Method (DVNT, 2, NotSerialized)
>                  {
> -                    Return (Zero)
> +                    If ((Arg0 & One))
> +                    {
> +                        Notify (S00, Arg1)
> +                    }
>                  }
>
> -                Method (_S3D, 0, NotSerialized)  // _S3D: S3 Device State
> +                Method (PCNT, 0, NotSerialized)
>                  {
> -                    Return (Zero)
> +                    BNUM = One
> +                    DVNT (PCIU, One)
> +                    DVNT (PCID, 0x03)
> +                }
> +            }
> +
> +            Device (S09)
> +            {
> +                Name (_ADR, 0x00010001)  // _ADR: Address
> +                Name (BSEL, Zero)
> +                Device (S00)
> +                {
> +                    Name (_SUN, Zero)  // _SUN: Slot User Number
> +                    Name (_ADR, Zero)  // _ADR: Address
> +                    Method (_EJ0, 1, NotSerialized)  // _EJx: Eject Device, x=0-9
> +                    {
> +                        PCEJ (BSEL, _SUN)
> +                    }
> +
> +                    Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
> +                    {
> +                        Return (PDSM (Arg0, Arg1, Arg2, Arg3, BSEL, _SUN))
> +                    }
> +                }
> +
> +                Method (DVNT, 2, NotSerialized)
> +                {
> +                    If ((Arg0 & One))
> +                    {
> +                        Notify (S00, Arg1)
> +                    }
> +                }
> +
> +                Method (PCNT, 0, NotSerialized)
> +                {
> +                    BNUM = Zero
> +                    DVNT (PCIU, One)
> +                    DVNT (PCID, 0x03)
>                  }
>              }
>
>              Method (PCNT, 0, NotSerialized)
>              {
> +                ^S09.PCNT ()
> +                ^S08.PCNT ()
>              }
>          }
>      }
>
> Signed-off-by: Ani Sinha <ani@anisinha.ca>
> ---
>  tests/data/acpi/q35/DSDT.multi-bridge       | Bin 0 -> 8435 bytes
>  tests/qtest/bios-tables-test-allowed-diff.h |   1 -
>  2 files changed, 1 deletion(-)
>
> diff --git a/tests/data/acpi/q35/DSDT.multi-bridge b/tests/data/acpi/q35/DSDT.multi-bridge
> index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..a6565acc5cc390826f4ac23585912f9cf1d9acb9 100644
> GIT binary patch
> literal 8435
> zcmcIpOKcm*8J;C6X|-HROKN4=vJ(+`^nnx!73bmf5ro|3%NA*hOT`Ijlq<_=B|AkH
> zNqk6*0I~ukjt@l>HbIYczyQ6qr(PSNxfJNZhXM`IQxCo58WhPX?Dx;?$TK7bREP4g
> z+JE-@=KsIh+5J{Ce&Bb$&NF75E?x9$LB8}-%gf_uFvjSd{yO#4W!ByIOVyr{PNa<P
> ztu#N!p=7#0@=IG)<9EB^Ct>*J`=PNNu`8cCA79yy-n|`OVFbEyC9*6!amow5pw#Og
> zcvj1+7tCI*?%TzCmTC4pah5cDMZe9=-d5db?#5j2V&J9jTxXttX?@U~>F;KjET`M}
> z=h<)8&b;u+r8kS8|Lh;XxO3e~0q`XLn)rJ*qC<EywAR9b^HG1#c!%ix+WKJWqdvbZ
> z+8nwPh17D_7ERRE`tW-9;GEwsdc4L=tJx}eg>}^BwS(-Mb>{sQ#%vlK`u%Y<8vR+E
> zGUqMkG)$qgY}Q)kj#Ozt8>#B<*IJzhvl+YhU7Dr3w;YO>UMx5Le5>vE^6oRepi^WK
> zwS*BhZND8_PB^eX?1!u$rbnY88!UX--)6tvw%Nb_Gc#mEYf;pewHDr+r&{iP$5M=;
> zn(sYM)zEl%4_50zvB!rP>DN$7tKQyBSfOP*-ux3u2_#ha;He8GA7F8hlBgb4$uuXe
> z<*E^~kQqkRxXP~L??VY>>mEGPxVUx5T3pgtrZw{rjZpXC(RRl>M5W_#b*xO)h*-o7
> z^C4!S^k(*c?fWe+=oslQY|F%uQQ$$-RvrgG-`x+&=P^6~W?LB0GzDI%P(F_frDmgT
> zr2j5x6nLfPRwLi4`!D6YMrp&lz}!b$Uio~Y`!GeLQM5~l=q+Q9Q6Js!Q}d9yj}^h0
> zhN8YAI)pJcbp6?uA2b#Q8}XFA_v~N)9X$PZ<HMD&8mIo^H6rTk)nA2!hx^yq;8cIl
> zN?>Nb!|$(+=`>=)Y|^+z2X=R5u-w09%|*GH1k14PD7?MR*l^oy)x8RH?H>Lb(Jn>K
> zuU|IkCOY0%`~hpVJFVP>H}-Ss6lM+EXkI8Yac<ze!g4mXTL}bJ86*+ZST2)79hl()
> z7e~Yba6w$-JV0egO!UvO3C;vIj)*Ct3C5X#awbAzq8g4(a;B^x$%rYTNx=k^GZ7L~
> zX2E<HSl&;HGbJ=7n6h|;s?Hfr=L}~`=!{^>;t{GkXEmL(oGGERx=w_u&N)rzoThV5
> z*NIToX=*x6O{b~rM5yY-n?V`rc}?fMt`niE)6#TWnodjCiBQ#<)^w&dooQVsLRBZ;
> zQOf<aHJ!Gu6QQazqv_0OIy1UXgsRR3P3MB9b3xaMP}RAp>0H!wF6ufFsyZD_r=#g~
> zbe#xQomow1R@0f)bs|)C<}{r-O=nKmiBQ$Kr0HDJbS~*S5vn>}O{c5rbakBwRh`S4
> z&Sg#KvaS=Ms`G@V^Mt1Jgsu~zsuK>6*(cvaE1J#~T_-|S=SfZHNloWTT_-|S=PAxC
> z#&5z?oLP!rcBcf>yB`wMkFl#7b5&!m>P&<x^R&i1tuasQOoS@)jK(~pG0*5sgevm^
> zjroAad_ZR+RGANQrgWJHIa9vl9~4aa;zmeJc?Z|DoNHRnH9aRnk<)~M78L`n26d$7
> zW@_LZiDLsrfV3h=B9KX8paRD&$za5i^%+Pipoa#EP)dV=3am6x1xf}gpoa#E5Y<Qn
> z71%hUoT`Be=%IlkL`5760~OdfiZya58K{8DP}K<o72R?s3{-)Vfg+TyNCt{f>V$y`
> zEN8+%6(|{~fbyJVpa`W-7^uK<CJa=8l7R{+XOe*;lsaLc0?U~&Pz6c`DxjQ628vML
> zPZ+4cawZH^fs%m=C})y^B9uB|paRR8Fi-_b1}dPONd}5g>V$y`EN8+%6(|{~fN~}o
> zC_<?d1}d<e2?JH2WS|1dnPi{{rA`>Az;Y%GRDqI#3Mglifg+STVW0xbnJ`cVN(L&R
> zoJj_XQ0jz%3M^;BKouw%sDN@N87M-j69y`<oCyO}pk$x|%9&)K2&GOKsK9b23{-)V
> zfeI*Rl7S+WI$@v!%b74x1xf}gpqxnticso=feI{V!ax-$8K{7ACK)I~sS^e&u$&14
> zRiI>`0?L_Wpa`W-7^uK<CJa=8l7R{+XOe*;lsaLc0?U~&Pz6c`DxjQ628vMXgn<ey
> zXTm@gC>f}LawZulLa7r5DzKah16818paRO7WS|J8P8cX6#Xu3M28vKMP=tzsDohxt
> z!XyJ#m}H;|69%d<VW0|=3{+u~fhtTGsKSJSDoip^g-HggFkzqy69%d<$v_n*8K}a9
> zfg+N37Y2$5cZ?4v87M;B1{o4ljx7unksMnXC?YwwWS|J;*ph)F#1hVG4;IP>cAx*V
> zI;4L{@6OVnV)xdMpGeYwskGWa>kvk(y;+3gZkFia(ZM$v)<ibTbY7uDl@2x2U8-+l
> zHDEA*)7sy~;>Y~ZO1n?hH@)^e0PR9{!AgHH%jWqC&0FjawOKPb)}x!d)*_=b6w$_#
> z$i^JL5sGq|!^ScSEz+1MJIJ1}mOfZu^GUcH!~2`zO^h`Tcag8p45<WEQ*^UEzaFr6
> z=-k@pEsL~H#DMVHJYMMq?ZCLR8`nVTsXfkK8q01RBYR27UgFtHs87vaa;Il6bq^Y(
> zT`X~787|gtC61BaQ__1py*HNLJ0iUor}xLiTUcoNpU2);()&EUKbGD<BE27{FOR2B
> z9V2~NNnhsa%VX)wN2D*u=_}*uXO5A+qNK0z^p&ynl_Szu;`G(=^s~oEUscjqdHU*D
> z`sxwst8x0;c>1|xq^~LIYdn2zEPd^W^fjbU(W`7cy(7}gmj^y8@VJY0RlUuf<&Ql6
> z&~qJkH~tt~S-m@aYfN+}+NO3l@m%Kg?(hLJ(Vb|U+TFx+hNW40&ZIDWl}vOe+NO3l
> z@!Yt2clcD9=uWgv?QY^ZSk}A4_sc|gqHSt-6Z86n-W@(_Cb|=CQ@fj(rz?7Q_|lo^
> zPPFOWIj8ThXCEvEjS{v%u-#!Nu@>)Kd|vDt)U3<*SK?dkav*1^g@+T~l<`_y`Pyr5
> zMz7xZW&ZV7-gxcC>pQQ!!McXm+PDzU){JFUjW6vlZNtWDJ3NV&&cC!RyL;;g6ckzB
> zYn7XZS8oP}*KWLMU;(^9hl;_Hp4afP$etr)(hJHPdBFW*Cy%w}db4guj2bcP&tB$w
> zP>?7xQ}kN(LSel?Wrz)(88_>7DlaDnR)3eh=M@TtJ-0TC)%)Un_u<P=q~w+Z2_*rv
> z{65}{WDyGotNrl{Bq6dEvFtu^W#hsybQ?1y>v)Nb@vhLr)c)Pd2`})P<`FVXuUWp7
> zON=766B$=O^>h8J*xR@o8DYe>Ki#l@(+`c0chD3@JL4(2;PFP>l)=i!<E^+YJD&~a
> z``gwCX+DdlT^~F$F>-X@qn+DS<2JjojjHZGJ9v5m#%Bg@-;R}xCI$u<!Mm|T8-C@K
> z*zuzdudp4u<<Pd%U7s#)qA%2Q<I415Zp?cZgN}W~Fi)Q?6a8cGqP;0&6IqGzR)Ss|
> zZ^ST9t*-m-IhxaW{_(@(fq88XX?EiYXcRlyv}H1^HqdvdyMWEiPMXB9-Mwq>X*@jD
> zR=HO$JjvoGo*pEcQ_jT{UE&XZ{7e;|VeWIlt}_~l!<{;@W2H2-ye6+yLpY6P@uc!i
> zA-;!sK5hN!c7`ReM1bFj4%o^2*8bKuOtc$#C*k)EE`Mtu#u@A(G4?gz{N&f^vv@S3
> z&tm!|n6@oyVjJrZ;tKn{cI9*ImwqR%eE$5M$FP+e_a@qCv}g|u1E>8crp-eyg~1WK
> z$9!kDh`|xBMrv2yiDL6HEgYP`6Z5nAS%dFDo=>%Et+p6q+TOk22?lA(`*pIfFYGUD
> z<0Q5ze|Q<`gR{NdY|23GIDe{_GgDT=!Xh4`r|AkQaei|DHgpdjuUztk?`1a2jV(;0
> zCkc&BR<&Yh#+jkFxKnd4^R2xXT)K<hgLF_Rh`n44N!1Jr=R|~CW-zmacL+XAekwr0
> zRuf&*&bpiH(VLz&JPFdn6?`ROuhJgUjwL4M^~3g@|6;R~7^H9R8tgslXwO}v`NblG
> zc1Y<eO4%~v6W>aTLY|8Y!E&c41O@(|3SmbS;;!;SfBtw-cn*8t%?_QN<7fW`tJddg
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index dabc024f53..dfb8523c8b 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1,2 +1 @@
>  /* List of comma-separated changed AML files to ignore */
> -"tests/data/acpi/q35/DSDT.multi-bridge",
> --
> 2.25.1
>


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

* Re: [PATCH v3 0/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35
  2021-09-23  7:14 ` [PATCH v3 0/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35 Ani Sinha
@ 2021-10-04 11:35   ` Ani Sinha
  0 siblings, 0 replies; 14+ messages in thread
From: Ani Sinha @ 2021-10-04 11:35 UTC (permalink / raw)
  To: Ani Sinha; +Cc: imammedo, qemu-devel, mst

ping

On Thu, Sep 23, 2021 at 12:44 PM Ani Sinha <ani@anisinha.ca> wrote:
>
> ping ...
>
> On Mon, 20 Sep 2021, Ani Sinha wrote:
>
> > This patchset adds a unit test to exercize acpi hotplug support for multifunction
> > bridges on q35 machines. This support was added with the commit:
> >
> > d7346e614f4ec ("acpi: x86: pcihp: add support hotplug on multifunction bridges")
> >
> > changelist:
> > v1 : initial RFC patch.
> > v2: incorporated some of the feedbacks from Igor.
> > v3: forgot to add the ASL diff for patch 3 in commit log for v2. Added now.
> >
> > Ani Sinha (3):
> >   tests/acpi/bios-tables-test: add and allow changes to a new q35 DSDT
> >     table blob
> >   tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges
> >     for q35
> >   tests/acpi/bios-tables-test: update DSDT blob for multifunction bridge
> >     test
> >
> >  tests/data/acpi/q35/DSDT.multi-bridge | Bin 0 -> 8435 bytes
> >  tests/qtest/bios-tables-test.c        |  18 ++++++++++++++++++
> >  2 files changed, 18 insertions(+)
> >  create mode 100644 tests/data/acpi/q35/DSDT.multi-bridge
> >
> > --
> > 2.25.1
> >
> >


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

* Re: [PATCH v3 2/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35
  2021-09-20  7:00 ` [PATCH v3 2/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35 Ani Sinha
@ 2021-10-07  8:07   ` Igor Mammedov
  2021-10-07  8:15     ` Ani Sinha
  0 siblings, 1 reply; 14+ messages in thread
From: Igor Mammedov @ 2021-10-07  8:07 UTC (permalink / raw)
  To: Ani Sinha; +Cc: qemu-devel, mst

On Mon, 20 Sep 2021 12:30:46 +0530
Ani Sinha <ani@anisinha.ca> wrote:

> commit d7346e614f4ec ("acpi: x86: pcihp: add support hotplug on multifunction bridges")
> added ACPI hotplug descriptions for cold plugged bridges for functions other
> than 0. For all other devices, the ACPI hotplug descriptions are limited to
> function 0 only. This change adds unit tests for this feature.
> 
> This test adds the following devices to qemu and then checks the changes
> introduced in the DSDT table due to the addition of the following devices:
> 
> (a) a multifunction bridge device
> (b) a bridge device with function 1
> (c) a non-bridge device with function 2
> 
> In the DSDT table, we should see AML hotplug descriptions for (a) and (b).
> For (a) we should find a hotplug AML description for function 0.
> 
> The following diff compares the DSDT table AML with the new unit test before
> and after the change d7346e614f4ec is introduced. In other words,
> this diff reflects the changes that occurs in the DSDT table due to the change
> d7346e614f4ec .
> 
> @@ -5,13 +5,13 @@
>   *
>   * Disassembling to symbolic ASL+ operators
>   *
> - * Disassembly of /tmp/aml-7A7890, Sat Sep 18 13:13:29 2021
> + * Disassembly of /tmp/aml-PE4S90, Sat Sep 18 13:08:54 2021
>   *
>   * Original Table Header:
>   *     Signature        "DSDT"
> - *     Length           0x0000206A (8298)
> + *     Length           0x000020F3 (8435)
>   *     Revision         0x01 **** 32-bit table (V1), no 64-bit math support
> - *     Checksum         0x59
> + *     Checksum         0x1B
>   *     OEM ID           "BOCHS "
>   *     OEM Table ID     "BXPC    "
>   *     OEM Revision     0x00000001 (1)
> @@ -20,28 +20,6 @@
>   */
>  DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPC    ", 0x00000001)
>  {
> -    /*
> -     * iASL Warning: There was 1 external control method found during
> -     * disassembly, but only 0 were resolved (1 unresolved). Additional
> -     * ACPI tables may be required to properly disassemble the code. This
> -     * resulting disassembler output file may not compile because the
> -     * disassembler did not know how many arguments to assign to the
> -     * unresolved methods. Note: SSDTs can be dynamically loaded at
> -     * runtime and may or may not be available via the host OS.
> -     *
> -     * In addition, the -fe option can be used to specify a file containing
> -     * control method external declarations with the associated method
> -     * argument counts. Each line of the file must be of the form:
> -     *     External (<method pathname>, MethodObj, <argument count>)
> -     * Invocation:
> -     *     iasl -fe refs.txt -d dsdt.aml
> -     *
> -     * The following methods were unresolved and many not compile properly
> -     * because the disassembler had to guess at the number of arguments
> -     * required for each:
> -     */
> -    External (_SB_.PCI0.S09_.PCNT, MethodObj)    // Warning: Unknown method, guessing 1 arguments
> -
>      Scope (\)
>      {
>          OperationRegion (DBG, SystemIO, 0x0402, One)
> @@ -3280,9 +3258,45 @@
>                  }
>              }
> 
> +            Device (S09)
> +            {
> +                Name (_ADR, 0x00010001)  // _ADR: Address
> +                Name (BSEL, Zero)
> +                Device (S00)
> +                {
> +                    Name (_SUN, Zero)  // _SUN: Slot User Number
> +                    Name (_ADR, Zero)  // _ADR: Address
> +                    Method (_EJ0, 1, NotSerialized)  // _EJx: Eject Device, x=0-9
> +                    {
> +                        PCEJ (BSEL, _SUN)
> +                    }
> +
> +                    Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
> +                    {
> +                        Return (PDSM (Arg0, Arg1, Arg2, Arg3, BSEL, _SUN))
> +                    }
> +                }
> +
> +                Method (DVNT, 2, NotSerialized)
> +                {
> +                    If ((Arg0 & One))
> +                    {
> +                        Notify (S00, Arg1)
> +                    }
> +                }
> +
> +                Method (PCNT, 0, NotSerialized)
> +                {
> +                    BNUM = Zero
> +                    DVNT (PCIU, One)
> +                    DVNT (PCID, 0x03)
> +                }
> +            }
> +
>              Method (PCNT, 0, NotSerialized)
>              {
> -                ^S09.PCNT (^S08.PCNT ())
> +                ^S09.PCNT ()
> +                ^S08.PCNT ()
>              }
>          }
>      }
> 
> Signed-off-by: Ani Sinha <ani@anisinha.ca>
> ---
>  tests/qtest/bios-tables-test.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index 4f11d03055..d4cd77ea02 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -859,6 +859,23 @@ static void test_acpi_q35_tcg_bridge(void)
>      free_test_data(&data);
>  }
>  
> +static void test_acpi_q35_multif_bridge(void)
> +{
> +    test_data data = {
> +        .machine = MACHINE_Q35,
> +        .variant = ".multi-bridge",
> +    };
> +    test_acpi_one("-nodefaults -device pcie-root-port,id=pcie-root-port-0,"

what's the reason for using "-nodefaults" here?

> +                  "multifunction=on,"
> +                  "port=0x0,chassis=1,addr=0x1,bus=pcie.0 "
> +                  "-device pcie-root-port,id=pcie-root-port-1,"
> +                  "port=0x1,chassis=2,addr=0x1.0x1,bus=pcie.0 "
> +                  "-device virtio-balloon,id=balloon0,"
> +                  "bus=pcie.0,addr=0x1.0x2",
> +                  &data);
> +    free_test_data(&data);
> +}
> +
>  static void test_acpi_q35_tcg_mmio64(void)
>  {
>      test_data data = {
> @@ -1534,6 +1551,7 @@ int main(int argc, char *argv[])
>                         test_acpi_piix4_no_acpi_pci_hotplug);
>          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/multif-bridge", test_acpi_q35_multif_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);



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

* Re: [PATCH v3 2/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35
  2021-10-07  8:07   ` Igor Mammedov
@ 2021-10-07  8:15     ` Ani Sinha
  2021-10-07 11:07       ` Igor Mammedov
  0 siblings, 1 reply; 14+ messages in thread
From: Ani Sinha @ 2021-10-07  8:15 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: Ani Sinha, qemu-devel, mst



On Thu, 7 Oct 2021, Igor Mammedov wrote:

> On Mon, 20 Sep 2021 12:30:46 +0530
> Ani Sinha <ani@anisinha.ca> wrote:
>
> > commit d7346e614f4ec ("acpi: x86: pcihp: add support hotplug on multifunction bridges")
> > added ACPI hotplug descriptions for cold plugged bridges for functions other
> > than 0. For all other devices, the ACPI hotplug descriptions are limited to
> > function 0 only. This change adds unit tests for this feature.
> >
> > This test adds the following devices to qemu and then checks the changes
> > introduced in the DSDT table due to the addition of the following devices:
> >
> > (a) a multifunction bridge device
> > (b) a bridge device with function 1
> > (c) a non-bridge device with function 2
> >
> > In the DSDT table, we should see AML hotplug descriptions for (a) and (b).
> > For (a) we should find a hotplug AML description for function 0.
> >
> > The following diff compares the DSDT table AML with the new unit test before
> > and after the change d7346e614f4ec is introduced. In other words,
> > this diff reflects the changes that occurs in the DSDT table due to the change
> > d7346e614f4ec .
> >
> > @@ -5,13 +5,13 @@
> >   *
> >   * Disassembling to symbolic ASL+ operators
> >   *
> > - * Disassembly of /tmp/aml-7A7890, Sat Sep 18 13:13:29 2021
> > + * Disassembly of /tmp/aml-PE4S90, Sat Sep 18 13:08:54 2021
> >   *
> >   * Original Table Header:
> >   *     Signature        "DSDT"
> > - *     Length           0x0000206A (8298)
> > + *     Length           0x000020F3 (8435)
> >   *     Revision         0x01 **** 32-bit table (V1), no 64-bit math support
> > - *     Checksum         0x59
> > + *     Checksum         0x1B
> >   *     OEM ID           "BOCHS "
> >   *     OEM Table ID     "BXPC    "
> >   *     OEM Revision     0x00000001 (1)
> > @@ -20,28 +20,6 @@
> >   */
> >  DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPC    ", 0x00000001)
> >  {
> > -    /*
> > -     * iASL Warning: There was 1 external control method found during
> > -     * disassembly, but only 0 were resolved (1 unresolved). Additional
> > -     * ACPI tables may be required to properly disassemble the code. This
> > -     * resulting disassembler output file may not compile because the
> > -     * disassembler did not know how many arguments to assign to the
> > -     * unresolved methods. Note: SSDTs can be dynamically loaded at
> > -     * runtime and may or may not be available via the host OS.
> > -     *
> > -     * In addition, the -fe option can be used to specify a file containing
> > -     * control method external declarations with the associated method
> > -     * argument counts. Each line of the file must be of the form:
> > -     *     External (<method pathname>, MethodObj, <argument count>)
> > -     * Invocation:
> > -     *     iasl -fe refs.txt -d dsdt.aml
> > -     *
> > -     * The following methods were unresolved and many not compile properly
> > -     * because the disassembler had to guess at the number of arguments
> > -     * required for each:
> > -     */
> > -    External (_SB_.PCI0.S09_.PCNT, MethodObj)    // Warning: Unknown method, guessing 1 arguments
> > -
> >      Scope (\)
> >      {
> >          OperationRegion (DBG, SystemIO, 0x0402, One)
> > @@ -3280,9 +3258,45 @@
> >                  }
> >              }
> >
> > +            Device (S09)
> > +            {
> > +                Name (_ADR, 0x00010001)  // _ADR: Address
> > +                Name (BSEL, Zero)
> > +                Device (S00)
> > +                {
> > +                    Name (_SUN, Zero)  // _SUN: Slot User Number
> > +                    Name (_ADR, Zero)  // _ADR: Address
> > +                    Method (_EJ0, 1, NotSerialized)  // _EJx: Eject Device, x=0-9
> > +                    {
> > +                        PCEJ (BSEL, _SUN)
> > +                    }
> > +
> > +                    Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
> > +                    {
> > +                        Return (PDSM (Arg0, Arg1, Arg2, Arg3, BSEL, _SUN))
> > +                    }
> > +                }
> > +
> > +                Method (DVNT, 2, NotSerialized)
> > +                {
> > +                    If ((Arg0 & One))
> > +                    {
> > +                        Notify (S00, Arg1)
> > +                    }
> > +                }
> > +
> > +                Method (PCNT, 0, NotSerialized)
> > +                {
> > +                    BNUM = Zero
> > +                    DVNT (PCIU, One)
> > +                    DVNT (PCID, 0x03)
> > +                }
> > +            }
> > +
> >              Method (PCNT, 0, NotSerialized)
> >              {
> > -                ^S09.PCNT (^S08.PCNT ())
> > +                ^S09.PCNT ()
> > +                ^S08.PCNT ()
> >              }
> >          }
> >      }
> >
> > Signed-off-by: Ani Sinha <ani@anisinha.ca>
> > ---
> >  tests/qtest/bios-tables-test.c | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> > index 4f11d03055..d4cd77ea02 100644
> > --- a/tests/qtest/bios-tables-test.c
> > +++ b/tests/qtest/bios-tables-test.c
> > @@ -859,6 +859,23 @@ static void test_acpi_q35_tcg_bridge(void)
> >      free_test_data(&data);
> >  }
> >
> > +static void test_acpi_q35_multif_bridge(void)
> > +{
> > +    test_data data = {
> > +        .machine = MACHINE_Q35,
> > +        .variant = ".multi-bridge",
> > +    };
> > +    test_acpi_one("-nodefaults -device pcie-root-port,id=pcie-root-port-0,"
>
> what's the reason for using "-nodefaults" here?

I have tried to use the same commandline that commit d7346e614f4ec
mentions as the repro case for the issue.


>
> > +                  "multifunction=on,"
> > +                  "port=0x0,chassis=1,addr=0x1,bus=pcie.0 "
> > +                  "-device pcie-root-port,id=pcie-root-port-1,"
> > +                  "port=0x1,chassis=2,addr=0x1.0x1,bus=pcie.0 "
> > +                  "-device virtio-balloon,id=balloon0,"
> > +                  "bus=pcie.0,addr=0x1.0x2",
> > +                  &data);
> > +    free_test_data(&data);
> > +}
> > +
> >  static void test_acpi_q35_tcg_mmio64(void)
> >  {
> >      test_data data = {
> > @@ -1534,6 +1551,7 @@ int main(int argc, char *argv[])
> >                         test_acpi_piix4_no_acpi_pci_hotplug);
> >          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/multif-bridge", test_acpi_q35_multif_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);
>
>


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

* Re: [PATCH v3 2/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35
  2021-10-07  8:15     ` Ani Sinha
@ 2021-10-07 11:07       ` Igor Mammedov
  2021-10-07 15:22         ` Ani Sinha
  0 siblings, 1 reply; 14+ messages in thread
From: Igor Mammedov @ 2021-10-07 11:07 UTC (permalink / raw)
  To: Ani Sinha; +Cc: qemu-devel, mst

On Thu, 7 Oct 2021 13:45:36 +0530 (IST)
Ani Sinha <ani@anisinha.ca> wrote:

> On Thu, 7 Oct 2021, Igor Mammedov wrote:
> 
> > On Mon, 20 Sep 2021 12:30:46 +0530
> > Ani Sinha <ani@anisinha.ca> wrote:
> >  
> > > commit d7346e614f4ec ("acpi: x86: pcihp: add support hotplug on multifunction bridges")
> > > added ACPI hotplug descriptions for cold plugged bridges for functions other
> > > than 0. For all other devices, the ACPI hotplug descriptions are limited to
> > > function 0 only. This change adds unit tests for this feature.
> > >
> > > This test adds the following devices to qemu and then checks the changes
> > > introduced in the DSDT table due to the addition of the following devices:
> > >
> > > (a) a multifunction bridge device
> > > (b) a bridge device with function 1
> > > (c) a non-bridge device with function 2
> > >
> > > In the DSDT table, we should see AML hotplug descriptions for (a) and (b).
> > > For (a) we should find a hotplug AML description for function 0.
> > >
> > > The following diff compares the DSDT table AML with the new unit test before
> > > and after the change d7346e614f4ec is introduced. In other words,
> > > this diff reflects the changes that occurs in the DSDT table due to the change
> > > d7346e614f4ec .
> > >
> > > @@ -5,13 +5,13 @@
> > >   *
> > >   * Disassembling to symbolic ASL+ operators
> > >   *
> > > - * Disassembly of /tmp/aml-7A7890, Sat Sep 18 13:13:29 2021
> > > + * Disassembly of /tmp/aml-PE4S90, Sat Sep 18 13:08:54 2021
> > >   *
> > >   * Original Table Header:
> > >   *     Signature        "DSDT"
> > > - *     Length           0x0000206A (8298)
> > > + *     Length           0x000020F3 (8435)
> > >   *     Revision         0x01 **** 32-bit table (V1), no 64-bit math support
> > > - *     Checksum         0x59
> > > + *     Checksum         0x1B
> > >   *     OEM ID           "BOCHS "
> > >   *     OEM Table ID     "BXPC    "
> > >   *     OEM Revision     0x00000001 (1)
> > > @@ -20,28 +20,6 @@
> > >   */
> > >  DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPC    ", 0x00000001)
> > >  {
> > > -    /*
> > > -     * iASL Warning: There was 1 external control method found during
> > > -     * disassembly, but only 0 were resolved (1 unresolved). Additional
> > > -     * ACPI tables may be required to properly disassemble the code. This
> > > -     * resulting disassembler output file may not compile because the
> > > -     * disassembler did not know how many arguments to assign to the
> > > -     * unresolved methods. Note: SSDTs can be dynamically loaded at
> > > -     * runtime and may or may not be available via the host OS.
> > > -     *
> > > -     * In addition, the -fe option can be used to specify a file containing
> > > -     * control method external declarations with the associated method
> > > -     * argument counts. Each line of the file must be of the form:
> > > -     *     External (<method pathname>, MethodObj, <argument count>)
> > > -     * Invocation:
> > > -     *     iasl -fe refs.txt -d dsdt.aml
> > > -     *
> > > -     * The following methods were unresolved and many not compile properly
> > > -     * because the disassembler had to guess at the number of arguments
> > > -     * required for each:
> > > -     */
> > > -    External (_SB_.PCI0.S09_.PCNT, MethodObj)    // Warning: Unknown method, guessing 1 arguments
> > > -
> > >      Scope (\)
> > >      {
> > >          OperationRegion (DBG, SystemIO, 0x0402, One)
> > > @@ -3280,9 +3258,45 @@
> > >                  }
> > >              }
> > >
> > > +            Device (S09)
> > > +            {
> > > +                Name (_ADR, 0x00010001)  // _ADR: Address
> > > +                Name (BSEL, Zero)
> > > +                Device (S00)
> > > +                {
> > > +                    Name (_SUN, Zero)  // _SUN: Slot User Number
> > > +                    Name (_ADR, Zero)  // _ADR: Address
> > > +                    Method (_EJ0, 1, NotSerialized)  // _EJx: Eject Device, x=0-9
> > > +                    {
> > > +                        PCEJ (BSEL, _SUN)
> > > +                    }
> > > +
> > > +                    Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
> > > +                    {
> > > +                        Return (PDSM (Arg0, Arg1, Arg2, Arg3, BSEL, _SUN))
> > > +                    }
> > > +                }
> > > +
> > > +                Method (DVNT, 2, NotSerialized)
> > > +                {
> > > +                    If ((Arg0 & One))
> > > +                    {
> > > +                        Notify (S00, Arg1)
> > > +                    }
> > > +                }
> > > +
> > > +                Method (PCNT, 0, NotSerialized)
> > > +                {
> > > +                    BNUM = Zero
> > > +                    DVNT (PCIU, One)
> > > +                    DVNT (PCID, 0x03)
> > > +                }
> > > +            }
> > > +
> > >              Method (PCNT, 0, NotSerialized)
> > >              {
> > > -                ^S09.PCNT (^S08.PCNT ())
> > > +                ^S09.PCNT ()
> > > +                ^S08.PCNT ()
> > >              }
> > >          }
> > >      }
> > >
> > > Signed-off-by: Ani Sinha <ani@anisinha.ca>
> > > ---
> > >  tests/qtest/bios-tables-test.c | 18 ++++++++++++++++++
> > >  1 file changed, 18 insertions(+)
> > >
> > > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> > > index 4f11d03055..d4cd77ea02 100644
> > > --- a/tests/qtest/bios-tables-test.c
> > > +++ b/tests/qtest/bios-tables-test.c
> > > @@ -859,6 +859,23 @@ static void test_acpi_q35_tcg_bridge(void)
> > >      free_test_data(&data);
> > >  }
> > >
> > > +static void test_acpi_q35_multif_bridge(void)
> > > +{
> > > +    test_data data = {
> > > +        .machine = MACHINE_Q35,
> > > +        .variant = ".multi-bridge",
> > > +    };
> > > +    test_acpi_one("-nodefaults -device pcie-root-port,id=pcie-root-port-0,"  
> >
> > what's the reason for using "-nodefaults" here?  
> 
> I have tried to use the same commandline that commit d7346e614f4ec
> mentions as the repro case for the issue.

if test can work without 'nodefaults', it would be better as it
introduces not related diff hunk in the following patch.

Other than that patch looks good to me.

> 
> 
> >  
> > > +                  "multifunction=on,"
> > > +                  "port=0x0,chassis=1,addr=0x1,bus=pcie.0 "
> > > +                  "-device pcie-root-port,id=pcie-root-port-1,"
> > > +                  "port=0x1,chassis=2,addr=0x1.0x1,bus=pcie.0 "
> > > +                  "-device virtio-balloon,id=balloon0,"
> > > +                  "bus=pcie.0,addr=0x1.0x2",
> > > +                  &data);
> > > +    free_test_data(&data);
> > > +}
> > > +
> > >  static void test_acpi_q35_tcg_mmio64(void)
> > >  {
> > >      test_data data = {
> > > @@ -1534,6 +1551,7 @@ int main(int argc, char *argv[])
> > >                         test_acpi_piix4_no_acpi_pci_hotplug);
> > >          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/multif-bridge", test_acpi_q35_multif_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);  
> >
> >  
> 



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

* Re: [PATCH v3 2/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35
  2021-10-07 11:07       ` Igor Mammedov
@ 2021-10-07 15:22         ` Ani Sinha
  0 siblings, 0 replies; 14+ messages in thread
From: Ani Sinha @ 2021-10-07 15:22 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: Ani Sinha, qemu-devel, mst



On Thu, 7 Oct 2021, Igor Mammedov wrote:

> On Thu, 7 Oct 2021 13:45:36 +0530 (IST)
> Ani Sinha <ani@anisinha.ca> wrote:
>
> > On Thu, 7 Oct 2021, Igor Mammedov wrote:
> >
> > > On Mon, 20 Sep 2021 12:30:46 +0530
> > > Ani Sinha <ani@anisinha.ca> wrote:
> > >
> > > > commit d7346e614f4ec ("acpi: x86: pcihp: add support hotplug on multifunction bridges")
> > > > added ACPI hotplug descriptions for cold plugged bridges for functions other
> > > > than 0. For all other devices, the ACPI hotplug descriptions are limited to
> > > > function 0 only. This change adds unit tests for this feature.
> > > >
> > > > This test adds the following devices to qemu and then checks the changes
> > > > introduced in the DSDT table due to the addition of the following devices:
> > > >
> > > > (a) a multifunction bridge device
> > > > (b) a bridge device with function 1
> > > > (c) a non-bridge device with function 2
> > > >
> > > > In the DSDT table, we should see AML hotplug descriptions for (a) and (b).
> > > > For (a) we should find a hotplug AML description for function 0.
> > > >
> > > > The following diff compares the DSDT table AML with the new unit test before
> > > > and after the change d7346e614f4ec is introduced. In other words,
> > > > this diff reflects the changes that occurs in the DSDT table due to the change
> > > > d7346e614f4ec .
> > > >
> > > > @@ -5,13 +5,13 @@
> > > >   *
> > > >   * Disassembling to symbolic ASL+ operators
> > > >   *
> > > > - * Disassembly of /tmp/aml-7A7890, Sat Sep 18 13:13:29 2021
> > > > + * Disassembly of /tmp/aml-PE4S90, Sat Sep 18 13:08:54 2021
> > > >   *
> > > >   * Original Table Header:
> > > >   *     Signature        "DSDT"
> > > > - *     Length           0x0000206A (8298)
> > > > + *     Length           0x000020F3 (8435)
> > > >   *     Revision         0x01 **** 32-bit table (V1), no 64-bit math support
> > > > - *     Checksum         0x59
> > > > + *     Checksum         0x1B
> > > >   *     OEM ID           "BOCHS "
> > > >   *     OEM Table ID     "BXPC    "
> > > >   *     OEM Revision     0x00000001 (1)
> > > > @@ -20,28 +20,6 @@
> > > >   */
> > > >  DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPC    ", 0x00000001)
> > > >  {
> > > > -    /*
> > > > -     * iASL Warning: There was 1 external control method found during
> > > > -     * disassembly, but only 0 were resolved (1 unresolved). Additional
> > > > -     * ACPI tables may be required to properly disassemble the code. This
> > > > -     * resulting disassembler output file may not compile because the
> > > > -     * disassembler did not know how many arguments to assign to the
> > > > -     * unresolved methods. Note: SSDTs can be dynamically loaded at
> > > > -     * runtime and may or may not be available via the host OS.
> > > > -     *
> > > > -     * In addition, the -fe option can be used to specify a file containing
> > > > -     * control method external declarations with the associated method
> > > > -     * argument counts. Each line of the file must be of the form:
> > > > -     *     External (<method pathname>, MethodObj, <argument count>)
> > > > -     * Invocation:
> > > > -     *     iasl -fe refs.txt -d dsdt.aml
> > > > -     *
> > > > -     * The following methods were unresolved and many not compile properly
> > > > -     * because the disassembler had to guess at the number of arguments
> > > > -     * required for each:
> > > > -     */
> > > > -    External (_SB_.PCI0.S09_.PCNT, MethodObj)    // Warning: Unknown method, guessing 1 arguments
> > > > -
> > > >      Scope (\)
> > > >      {
> > > >          OperationRegion (DBG, SystemIO, 0x0402, One)
> > > > @@ -3280,9 +3258,45 @@
> > > >                  }
> > > >              }
> > > >
> > > > +            Device (S09)
> > > > +            {
> > > > +                Name (_ADR, 0x00010001)  // _ADR: Address
> > > > +                Name (BSEL, Zero)
> > > > +                Device (S00)
> > > > +                {
> > > > +                    Name (_SUN, Zero)  // _SUN: Slot User Number
> > > > +                    Name (_ADR, Zero)  // _ADR: Address
> > > > +                    Method (_EJ0, 1, NotSerialized)  // _EJx: Eject Device, x=0-9
> > > > +                    {
> > > > +                        PCEJ (BSEL, _SUN)
> > > > +                    }
> > > > +
> > > > +                    Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
> > > > +                    {
> > > > +                        Return (PDSM (Arg0, Arg1, Arg2, Arg3, BSEL, _SUN))
> > > > +                    }
> > > > +                }
> > > > +
> > > > +                Method (DVNT, 2, NotSerialized)
> > > > +                {
> > > > +                    If ((Arg0 & One))
> > > > +                    {
> > > > +                        Notify (S00, Arg1)
> > > > +                    }
> > > > +                }
> > > > +
> > > > +                Method (PCNT, 0, NotSerialized)
> > > > +                {
> > > > +                    BNUM = Zero
> > > > +                    DVNT (PCIU, One)
> > > > +                    DVNT (PCID, 0x03)
> > > > +                }
> > > > +            }
> > > > +
> > > >              Method (PCNT, 0, NotSerialized)
> > > >              {
> > > > -                ^S09.PCNT (^S08.PCNT ())
> > > > +                ^S09.PCNT ()
> > > > +                ^S08.PCNT ()
> > > >              }
> > > >          }
> > > >      }
> > > >
> > > > Signed-off-by: Ani Sinha <ani@anisinha.ca>
> > > > ---
> > > >  tests/qtest/bios-tables-test.c | 18 ++++++++++++++++++
> > > >  1 file changed, 18 insertions(+)
> > > >
> > > > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> > > > index 4f11d03055..d4cd77ea02 100644
> > > > --- a/tests/qtest/bios-tables-test.c
> > > > +++ b/tests/qtest/bios-tables-test.c
> > > > @@ -859,6 +859,23 @@ static void test_acpi_q35_tcg_bridge(void)
> > > >      free_test_data(&data);
> > > >  }
> > > >
> > > > +static void test_acpi_q35_multif_bridge(void)
> > > > +{
> > > > +    test_data data = {
> > > > +        .machine = MACHINE_Q35,
> > > > +        .variant = ".multi-bridge",
> > > > +    };
> > > > +    test_acpi_one("-nodefaults -device pcie-root-port,id=pcie-root-port-0,"
> > >
> > > what's the reason for using "-nodefaults" here?
> >
> > I have tried to use the same commandline that commit d7346e614f4ec
> > mentions as the repro case for the issue.
>
> if test can work without 'nodefaults', it would be better as it
> introduces not related diff hunk in the following patch.
>
> Other than that patch looks good to me.

yes that is a fair comment. I have sent a v3 with nodefaults removed. I
had to adjust the test a bit because we now have VGA and other devices
occupying certain slots.



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

* Re: [PATCH v3 3/3] tests/acpi/bios-tables-test: update DSDT blob for multifunction bridge test
  2021-10-07 13:57 ` [PATCH v3 3/3] tests/acpi/bios-tables-test: update DSDT blob for multifunction bridge test Ani Sinha
@ 2021-10-11 13:54   ` Igor Mammedov
  0 siblings, 0 replies; 14+ messages in thread
From: Igor Mammedov @ 2021-10-11 13:54 UTC (permalink / raw)
  To: Ani Sinha; +Cc: qemu-devel, Michael S. Tsirkin

On Thu,  7 Oct 2021 19:27:50 +0530
Ani Sinha <ani@anisinha.ca> wrote:

> We added a new unit test for testing acpi hotplug on multifunction bridges in
> q35 machines. Here, we update the DSDT table gloden master blob for this unit
> test.
> 
> The test adds the following devices to qemu and then checks the changes
> introduced in the DSDT table due to the addition of the following devices:
> 
> (a) a multifunction bridge device
> (b) a bridge device with function 1
> (c) a non-bridge device with function 2
> 
> In the DSDT table, we should see AML hotplug descriptions for (a) and (b).
> For (a) we should find a hotplug AML description for function 0.
> 
> Following is the ASL diff between the original DSDT table and the modified DSDT
> table due to the unit test. We see that multifunction bridge on bus 2 and single
> function bridge on bus 3 function 1 are described, not the non-bridge balloon
> device on bus 4, function 2.
> 
> @@ -1,30 +1,30 @@
>  /*
>   * Intel ACPI Component Architecture
>   * AML/ASL+ Disassembler version 20190509 (64-bit version)
>   * Copyright (c) 2000 - 2019 Intel Corporation
>   *
>   * Disassembling to symbolic ASL+ operators
>   *
> - * Disassembly of tests/data/acpi/q35/DSDT, Thu Oct  7 18:29:19 2021
> + * Disassembly of /tmp/aml-C7JCA1, Thu Oct  7 18:29:19 2021
>   *
>   * Original Table Header:
>   *     Signature        "DSDT"
> - *     Length           0x00002061 (8289)
> + *     Length           0x00002187 (8583)
>   *     Revision         0x01 **** 32-bit table (V1), no 64-bit math support
> - *     Checksum         0xF9
> + *     Checksum         0x8D
>   *     OEM ID           "BOCHS "
>   *     OEM Table ID     "BXPC    "
>   *     OEM Revision     0x00000001 (1)
>   *     Compiler ID      "BXPC"
>   *     Compiler Version 0x00000001 (1)
>   */
>  DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPC    ", 0x00000001)
>  {
>      Scope (\)
>      {
>          OperationRegion (DBG, SystemIO, 0x0402, One)
>          Field (DBG, ByteAcc, NoLock, Preserve)
>          {
>              DBGB,   8
>          }
> 
> @@ -3265,23 +3265,95 @@
>                  Method (_S1D, 0, NotSerialized)  // _S1D: S1 Device State
>                  {
>                      Return (Zero)
>                  }
> 
>                  Method (_S2D, 0, NotSerialized)  // _S2D: S2 Device State
>                  {
>                      Return (Zero)
>                  }
> 
>                  Method (_S3D, 0, NotSerialized)  // _S3D: S3 Device State
>                  {
>                      Return (Zero)
>                  }
>              }
> 
> +            Device (S10)
> +            {
> +                Name (_ADR, 0x00020000)  // _ADR: Address
> +                Name (BSEL, One)
> +                Device (S00)
> +                {
> +                    Name (_SUN, Zero)  // _SUN: Slot User Number
> +                    Name (_ADR, Zero)  // _ADR: Address
> +                    Method (_EJ0, 1, NotSerialized)  // _EJx: Eject Device, x=0-9
> +                    {
> +                        PCEJ (BSEL, _SUN)
> +                    }
> +
> +                    Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
> +                    {
> +                        Return (PDSM (Arg0, Arg1, Arg2, Arg3, BSEL, _SUN))
> +                    }
> +                }
> +
> +                Method (DVNT, 2, NotSerialized)
> +                {
> +                    If ((Arg0 & One))
> +                    {
> +                        Notify (S00, Arg1)
> +                    }
> +                }
> +
> +                Method (PCNT, 0, NotSerialized)
> +                {
> +                    BNUM = One
> +                    DVNT (PCIU, One)
> +                    DVNT (PCID, 0x03)
> +                }
> +            }
> +
> +            Device (S19)
> +            {
> +                Name (_ADR, 0x00030001)  // _ADR: Address
> +                Name (BSEL, Zero)
> +                Device (S00)
> +                {
> +                    Name (_SUN, Zero)  // _SUN: Slot User Number
> +                    Name (_ADR, Zero)  // _ADR: Address
> +                    Method (_EJ0, 1, NotSerialized)  // _EJx: Eject Device, x=0-9
> +                    {
> +                        PCEJ (BSEL, _SUN)
> +                    }
> +
> +                    Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
> +                    {
> +                        Return (PDSM (Arg0, Arg1, Arg2, Arg3, BSEL, _SUN))
> +                    }
> +                }
> +
> +                Method (DVNT, 2, NotSerialized)
> +                {
> +                    If ((Arg0 & One))
> +                    {
> +                        Notify (S00, Arg1)
> +                    }
> +                }
> +
> +                Method (PCNT, 0, NotSerialized)
> +                {
> +                    BNUM = Zero
> +                    DVNT (PCIU, One)
> +                    DVNT (PCID, 0x03)
> +                }
> +            }
> +
>              Method (PCNT, 0, NotSerialized)
>              {
> +                ^S19.PCNT ()
> +                ^S10.PCNT ()
>              }
>          }
>      }
>  }
> 
> Signed-off-by: Ani Sinha <ani@anisinha.ca>

Acked-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  tests/data/acpi/q35/DSDT.multi-bridge       | Bin 0 -> 8583 bytes
>  tests/qtest/bios-tables-test-allowed-diff.h |   1 -
>  2 files changed, 1 deletion(-)
> 
> diff --git a/tests/data/acpi/q35/DSDT.multi-bridge b/tests/data/acpi/q35/DSDT.multi-bridge
> index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..a24c713d22102a1a1583b5c902edffe1694e5cfe 100644
> GIT binary patch
> literal 8583
> zcmcIpOKcm*8J^`sS}j-7l3Gc&>_n`S^pzr^>^%DjO78Myi4?`9;si8G%5qxCPLV|t
> z14)bkvH~QIfd)<31U=FL1N7FOdaZ$8+M90;&{Ge+<XRN*DeU*p?8q}D1;mH)u-bq2
> z`{w_@*`57XGk)N=KKKq}#%~InUM0vDUTSz*{0znzozq{Znz+c?2Y#X4F;cOF(Y}%5
> z=QtEh_eXwVyKMY^ulrfI`{oB-V<%*nK6gI7v=hE}vwMjV=-Q>wvgpJq&UJ!9r+w&I
> z4X>IrJC&+$=kHpk+400#-0bB2CNn$RRiC*V)1A%0OWeB3JpaO4zn<*vr57xxUHj*`  
> zuUAk1{Id&h=I{LMAHTeH)k*+x7Jp6rJr~lUd%bI|cKgmJy?x_dqVsEO{e@3@{IY0s
> z=t|h7mfN;yqOR5kSKEir`OUn?Yn*M=8#ynxhPu3FkY2S;f3VD$O@l+fKMjY&zlc-j
> zyv>}NDO48CN~744Dh+5ORqcaHqg7)zV|Twvu|)fZL-E3k#k!wuH2qH2eWnw%@+_p5  
> zZb(f#?{qDv+qXaNby=^Q8V(1nKlgEOhy8BHX8-zca=-@Gyr?a0&AmTEwcP!NB^X0B
> z-+h9rq491Xu2h43hYvB*ucDMjwYe3ux|Z#%&p#QLKti<-pIR^Y0E>GRNA;*mra56P
> zmW?j!GQ%hvm)T|fJrhH=_Ti(o&Fv%B{DQ_Zt>hURq4wd~jYext_MH&-+t9F%HCr~`
> zioX>%%`*qQ=d<2s?TC{E8lyRZb1!}`&df6o=RDSIc}LZxqWj0Y4q3<y^9(aEMD_Ij
> zF*;~?LCZ+pu`Ls~h5`>`x1Gho&$bVO;(5%1zu6XUT#5p(kSm_Yg+jg7G*Wj3jRLPw  
> z->zjFRsW@I+bC>!>&$(u;T6xv+K*5)9EN*@2;VmL8THZr9yJe``*<FlNhs<oq(e8t
> zc3pp};Rm(3{zf!4?mqkXe+N&$Q~P-NtJ=xGdbN=HdiB@c{v*9Btbek%Z^bYJ-sR7c
> zjmLJ#2I;tQgAVNWa(}UR#hMN?$rzhuJ7M?c4r7BIvr+X*%(XlCYlM3gIUl`j(8J<*
> zU-60DXto-e^*0VOsRW)*wozX%GI4I;yu>m#wOcU+Wf{Z~R9I#<fjTgg1TKn*1>l^x
> z#(99skeKM7V`H2NY!newLSu|G0p(1D#6&e58|O?}L7Wj&LgRu7C}$!hrp$tQC9u4o
> z1ZPTULNI0V2vwa)O=prbB{V6RvUr55&M8gj6lY54l&%w@s&iV?Ij!lO)^#FOb()$^
> zQ`2ecIuWWm@di>xdPdVZqw7Sd>a;YSmZsCvbs|)CrZk-?O=n8iiBQ#vccF4WZB3`G  
> z>qMyPoYi#BYC30iod{K(bDGXMP3N4h6QQbeUeh_R>73VfB2;xcnodX4>F7EUsyfq}  
> z&a|d8t?NXn>da_5Gn&qft`niEb3xO&py^!Dbs|)Cx|&W`)9LCu5vn>DHJyu^&P81(  
> zLRIIIrgKTtxuokvsOrR*1fD+m9$MCPF6%lGsya_-I!|aiPv|-ksya_{W<Giop5)9z
> z^s+lCnBM)6n0|y^(U>b5b46z&RGFtV<|&PNN@pTenWr`8X^nYWXChRY4{6MYH0DD(  
> z6QRm{m@}ozJj|K$9sjUk$`?06V#+(Xs^whOa<1w*5sI893^cD8XgR1NH8)cO=SUnG
> zC<3GvK^%ch3Ii25Zb=3smaNY}QUN_OP=rz%3{+sHfhtfkPyszMP=u&P8mPcV5#>}3
> zR6vgm6d@|&SQx0lMp2}ZL&-n|REDZf7^vu$Ghv_#lnfN1bVV{ygi<FARA4z12C6{G
> zKn0ZNBm+e#b;3XemNQ|X3X}{~Ksl2P6rt1!0~J`#gn=qhGEf2KOfpb}@_xcV1(q{m
> zpbC@>R6seC3>2Z%2?G^a&V+#~P%=;f<xDbAgi<FARA4z12C6{GKn0XD$v_cGoiI>=  
> z<xCi;0wn_#P|hR+MJRQ`Kn0dFVW0|>3{*fllMEE0)CmI>Sk8ojDo`>|0p(0GP=rz^
> z3{+q_69%e4$v_2^Gs!>^N}VuJf#pmXr~)Me6;RG314Srx!axO<Ghv_#lnhirIg<<&  
> zq0|Wj6<E%MfhtfkPyyvkGEjt4Ck#|zITHq|K*>M_lrzad5lWpfP=V!47^ngz0~Jut
> zBm+e#b;3XemNQ|X3X}{~Ksl2P6rt1!0~J`#gn=qhGEf2KOfpb}QYQ>lU^x>8szAv=  
> z1(Y+%KoLrvFi?TzOc<yFB?A>u&LjgxD0RX>5h(_WNHtJ|s(~U@3{+vlKouq#sKO)z
> zRhTePg$V;ym}H;|lMGa0!ax-!3{+u~fhtTgP=yHtRhTePg-HggFv&m_CJYpjyt^<^
> zM7U#oFv&m>;x@>Tm~w1kporwy!axzpu_XgVD94rz6d{(1Rywdy*0+27pVa~VLwb9P  
> z{^Z*?e*9#d{!67*4O;Io8qKXd9Cxcg2agWE$*}gaRiyJ09m;g5nC?P#3#$SBnd{cU
> z9u`?<23E>_s=DPhX8>sC(sNeo!znhySAyPVx2VlZ;#du@?^*MV&QL@XOD`MK_(sUf
> zB@Y|RHMAIHqU<2OWG#F+$7bSiH-`5&!<!hZ7w$Y?2O3Zbs3z~GJAO4_@6x$-z+2{N
> zJ&FP0HF><;37Ua%YcHyS(o=hsy)cp;iznYYdqK%w;MohPPt9I%Cuc9T4{M}dETv(&  
> zFVb$s9w5D^r1yAwZzR2UOnNU$?~jPLu%Pv=)%!|%pQran()-7x_oMX1(e#N2NMBUa
> z7kT>PNc!S2>5EbN(rEhR1Eeo0=}SC)X(WB=nDnJ6eR(wf)B~h1E9uKTeR(8(`Iz+O  
> zD1Bu#{qzH*uPEs&Jbh&(edU<+6{Jtlt86sABht&42R<t>-NpK?-sVp6DUUxiucPip
> zr?HjRyTiA}Sa+grVs~TnGNX5g50J6$MBBvf#^xE8fN7pdVfZQ;>rS*y>~3t{xO#W^  
> zR2l0|v`y@8Y#uD?-QoLXtUJ**vAeP7dP(mNA2nm$iMEN|jXkH!dUyEJ8S758>D@V}
> z?yqMb&Ih#uwpy^QV#l%O?`*yxb|)&<#rrG%jb<^BPpO5;32&P98r#{*Yj1|HUi(${
> z^;h0_?b_?Rue`z9hS%6wkDk_9%PJdR+F#m+ja7Yk5-pv7X<K&t#`h`6v#i%B)(x*(
> z4-Btad(prGc#aMwgT+0s=3|jPL&&%n6gRSf`}tNDYt7Yq)eIRm;;BD(k?TQ0qR832
> z*Qn-lYdI=IY#b%swAU)V9P3-XJ@&qr%jNdn$`n@b^E2&7E<Tx%TN@;l1km#PXfu+9
> ztlMAdjb0!Lp*4?X_pvJ*>w~UaOOmVyOJt08g&rpM@0OOlz^j|b$S}Qn@j@mx4B2jI
> zT>jk8^e!7AyBr$bknMcFVgI(*H9p-%Q#aflP00n1HsYrAmp>hC#ckRBqCeByv4%+V  
> zML6kt|Ix9Lqx&B2-lQ6L*tH#0b^F==(_=9HWZ?GgTJdmfU~mz<YrC|uSWJkWMe6Vp
> z+of9$ZM*HY$>PTPLOs_mO&;djjJFxI>|=&`^3yWbKL#({pD;F|6&r0O=#|k%4D-b5  
> z+V4F_&ot&gewZG3uFU~G-IxKjd@G%@Oor74`VMvHuxZ*#kr=kOf5kn8$y06=JH^~t
> z7R`8?Nc2oOn+dwaCw?@ia?ddLd0^KV4aCuopxCie8X8`mSE?bL+M<}Ld{c<;m7Y&o
> zf4(`(Vpt-;Z%7C17~Zq**@lUB1MejKzQ*Ny_E9{GJtW5d!#7#^b^0tG4(YR)elezP
> zzna*_`lGnQ{-9mCgMHa=#FaZQ+<F{awoz}QjYf<1z%X#yk7C-S^b!~xu}jT&fb$p}
> z@oJ=Y<(()tE>pt6`8zQ?m7Oy9PUrbVqta-KA*OBQ`<-BrCcIzA`?_P_v5gbhru@N0
> zqz}$@GE)fywd4G$PR2}FF$;@$h@Pe^q{R8j{oBw!e4=#06TUaQRjh5}F*-|VZ04#J
> zJ4q)=Z*j+8zR0)sHkRlvwhvQ5E+_VkF(f|>a?gnXcg$cg<6zBa*tEpSS$T4fPTGfM  
> zGgyn50LPeF!21RtF+UX@2HSO%LA(8KrbDhko1+Bj;0nH|u&-$kXm=A2@ztaDjQ?W2
> z73-(2?-}fU>S*6xrDu_a2JNQORg}GLL?^x#7lk|@RSnw_g`mLyQz7h_LfjQz=r5iK
> P)}F_He7!|yIezv(me~h+
> 
> literal 0
> HcmV?d00001
> 
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index dabc024f53..dfb8523c8b 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1,2 +1 @@
>  /* List of comma-separated changed AML files to ignore */
> -"tests/data/acpi/q35/DSDT.multi-bridge",



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

* [PATCH v3 3/3] tests/acpi/bios-tables-test: update DSDT blob for multifunction bridge test
  2021-10-07 13:57 Ani Sinha
@ 2021-10-07 13:57 ` Ani Sinha
  2021-10-11 13:54   ` Igor Mammedov
  0 siblings, 1 reply; 14+ messages in thread
From: Ani Sinha @ 2021-10-07 13:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Ani Sinha, Igor Mammedov, Michael S. Tsirkin

We added a new unit test for testing acpi hotplug on multifunction bridges in
q35 machines. Here, we update the DSDT table gloden master blob for this unit
test.

The test adds the following devices to qemu and then checks the changes
introduced in the DSDT table due to the addition of the following devices:

(a) a multifunction bridge device
(b) a bridge device with function 1
(c) a non-bridge device with function 2

In the DSDT table, we should see AML hotplug descriptions for (a) and (b).
For (a) we should find a hotplug AML description for function 0.

Following is the ASL diff between the original DSDT table and the modified DSDT
table due to the unit test. We see that multifunction bridge on bus 2 and single
function bridge on bus 3 function 1 are described, not the non-bridge balloon
device on bus 4, function 2.

@@ -1,30 +1,30 @@
 /*
  * Intel ACPI Component Architecture
  * AML/ASL+ Disassembler version 20190509 (64-bit version)
  * Copyright (c) 2000 - 2019 Intel Corporation
  *
  * Disassembling to symbolic ASL+ operators
  *
- * Disassembly of tests/data/acpi/q35/DSDT, Thu Oct  7 18:29:19 2021
+ * Disassembly of /tmp/aml-C7JCA1, Thu Oct  7 18:29:19 2021
  *
  * Original Table Header:
  *     Signature        "DSDT"
- *     Length           0x00002061 (8289)
+ *     Length           0x00002187 (8583)
  *     Revision         0x01 **** 32-bit table (V1), no 64-bit math support
- *     Checksum         0xF9
+ *     Checksum         0x8D
  *     OEM ID           "BOCHS "
  *     OEM Table ID     "BXPC    "
  *     OEM Revision     0x00000001 (1)
  *     Compiler ID      "BXPC"
  *     Compiler Version 0x00000001 (1)
  */
 DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPC    ", 0x00000001)
 {
     Scope (\)
     {
         OperationRegion (DBG, SystemIO, 0x0402, One)
         Field (DBG, ByteAcc, NoLock, Preserve)
         {
             DBGB,   8
         }

@@ -3265,23 +3265,95 @@
                 Method (_S1D, 0, NotSerialized)  // _S1D: S1 Device State
                 {
                     Return (Zero)
                 }

                 Method (_S2D, 0, NotSerialized)  // _S2D: S2 Device State
                 {
                     Return (Zero)
                 }

                 Method (_S3D, 0, NotSerialized)  // _S3D: S3 Device State
                 {
                     Return (Zero)
                 }
             }

+            Device (S10)
+            {
+                Name (_ADR, 0x00020000)  // _ADR: Address
+                Name (BSEL, One)
+                Device (S00)
+                {
+                    Name (_SUN, Zero)  // _SUN: Slot User Number
+                    Name (_ADR, Zero)  // _ADR: Address
+                    Method (_EJ0, 1, NotSerialized)  // _EJx: Eject Device, x=0-9
+                    {
+                        PCEJ (BSEL, _SUN)
+                    }
+
+                    Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
+                    {
+                        Return (PDSM (Arg0, Arg1, Arg2, Arg3, BSEL, _SUN))
+                    }
+                }
+
+                Method (DVNT, 2, NotSerialized)
+                {
+                    If ((Arg0 & One))
+                    {
+                        Notify (S00, Arg1)
+                    }
+                }
+
+                Method (PCNT, 0, NotSerialized)
+                {
+                    BNUM = One
+                    DVNT (PCIU, One)
+                    DVNT (PCID, 0x03)
+                }
+            }
+
+            Device (S19)
+            {
+                Name (_ADR, 0x00030001)  // _ADR: Address
+                Name (BSEL, Zero)
+                Device (S00)
+                {
+                    Name (_SUN, Zero)  // _SUN: Slot User Number
+                    Name (_ADR, Zero)  // _ADR: Address
+                    Method (_EJ0, 1, NotSerialized)  // _EJx: Eject Device, x=0-9
+                    {
+                        PCEJ (BSEL, _SUN)
+                    }
+
+                    Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
+                    {
+                        Return (PDSM (Arg0, Arg1, Arg2, Arg3, BSEL, _SUN))
+                    }
+                }
+
+                Method (DVNT, 2, NotSerialized)
+                {
+                    If ((Arg0 & One))
+                    {
+                        Notify (S00, Arg1)
+                    }
+                }
+
+                Method (PCNT, 0, NotSerialized)
+                {
+                    BNUM = Zero
+                    DVNT (PCIU, One)
+                    DVNT (PCID, 0x03)
+                }
+            }
+
             Method (PCNT, 0, NotSerialized)
             {
+                ^S19.PCNT ()
+                ^S10.PCNT ()
             }
         }
     }
 }

Signed-off-by: Ani Sinha <ani@anisinha.ca>
---
 tests/data/acpi/q35/DSDT.multi-bridge       | Bin 0 -> 8583 bytes
 tests/qtest/bios-tables-test-allowed-diff.h |   1 -
 2 files changed, 1 deletion(-)

diff --git a/tests/data/acpi/q35/DSDT.multi-bridge b/tests/data/acpi/q35/DSDT.multi-bridge
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..a24c713d22102a1a1583b5c902edffe1694e5cfe 100644
GIT binary patch
literal 8583
zcmcIpOKcm*8J^`sS}j-7l3Gc&>_n`S^pzr^>^%DjO78Myi4?`9;si8G%5qxCPLV|t
z14)bkvH~QIfd)<31U=FL1N7FOdaZ$8+M90;&{Ge+<XRN*DeU*p?8q}D1;mH)u-bq2
z`{w_@*`57XGk)N=KKKq}#%~InUM0vDUTSz*{0znzozq{Znz+c?2Y#X4F;cOF(Y}%5
z=QtEh_eXwVyKMY^ulrfI`{oB-V<%*nK6gI7v=hE}vwMjV=-Q>wvgpJq&UJ!9r+w&I
z4X>IrJC&+$=kHpk+400#-0bB2CNn$RRiC*V)1A%0OWeB3JpaO4zn<*vr57xxUHj*`
zuUAk1{Id&h=I{LMAHTeH)k*+x7Jp6rJr~lUd%bI|cKgmJy?x_dqVsEO{e@3@{IY0s
z=t|h7mfN;yqOR5kSKEir`OUn?Yn*M=8#ynxhPu3FkY2S;f3VD$O@l+fKMjY&zlc-j
zyv>}NDO48CN~744Dh+5ORqcaHqg7)zV|Twvu|)fZL-E3k#k!wuH2qH2eWnw%@+_p5
zZb(f#?{qDv+qXaNby=^Q8V(1nKlgEOhy8BHX8-zca=-@Gyr?a0&AmTEwcP!NB^X0B
z-+h9rq491Xu2h43hYvB*ucDMjwYe3ux|Z#%&p#QLKti<-pIR^Y0E>GRNA;*mra56P
zmW?j!GQ%hvm)T|fJrhH=_Ti(o&Fv%B{DQ_Zt>hURq4wd~jYext_MH&-+t9F%HCr~`
zioX>%%`*qQ=d<2s?TC{E8lyRZb1!}`&df6o=RDSIc}LZxqWj0Y4q3<y^9(aEMD_Ij
zF*;~?LCZ+pu`Ls~h5`>`x1Gho&$bVO;(5%1zu6XUT#5p(kSm_Yg+jg7G*Wj3jRLPw
z->zjFRsW@I+bC>!>&$(u;T6xv+K*5)9EN*@2;VmL8THZr9yJe``*<FlNhs<oq(e8t
zc3pp};Rm(3{zf!4?mqkXe+N&$Q~P-NtJ=xGdbN=HdiB@c{v*9Btbek%Z^bYJ-sR7c
zjmLJ#2I;tQgAVNWa(}UR#hMN?$rzhuJ7M?c4r7BIvr+X*%(XlCYlM3gIUl`j(8J<*
zU-60DXto-e^*0VOsRW)*wozX%GI4I;yu>m#wOcU+Wf{Z~R9I#<fjTgg1TKn*1>l^x
z#(99skeKM7V`H2NY!newLSu|G0p(1D#6&e58|O?}L7Wj&LgRu7C}$!hrp$tQC9u4o
z1ZPTULNI0V2vwa)O=prbB{V6RvUr55&M8gj6lY54l&%w@s&iV?Ij!lO)^#FOb()$^
zQ`2ecIuWWm@di>xdPdVZqw7Sd>a;YSmZsCvbs|)CrZk-?O=n8iiBQ#vccF4WZB3`G
z>qMyPoYi#BYC30iod{K(bDGXMP3N4h6QQbeUeh_R>73VfB2;xcnodX4>F7EUsyfq}
z&a|d8t?NXn>da_5Gn&qft`niEb3xO&py^!Dbs|)Cx|&W`)9LCu5vn>DHJyu^&P81(
zLRIIIrgKTtxuokvsOrR*1fD+m9$MCPF6%lGsya_-I!|aiPv|-ksya_{W<Giop5)9z
z^s+lCnBM)6n0|y^(U>b5b46z&RGFtV<|&PNN@pTenWr`8X^nYWXChRY4{6MYH0DD(
z6QRm{m@}ozJj|K$9sjUk$`?06V#+(Xs^whOa<1w*5sI893^cD8XgR1NH8)cO=SUnG
zC<3GvK^%ch3Ii25Zb=3smaNY}QUN_OP=rz%3{+sHfhtfkPyszMP=u&P8mPcV5#>}3
zR6vgm6d@|&SQx0lMp2}ZL&-n|REDZf7^vu$Ghv_#lnfN1bVV{ygi<FARA4z12C6{G
zKn0ZNBm+e#b;3XemNQ|X3X}{~Ksl2P6rt1!0~J`#gn=qhGEf2KOfpb}@_xcV1(q{m
zpbC@>R6seC3>2Z%2?G^a&V+#~P%=;f<xDbAgi<FARA4z12C6{GKn0XD$v_cGoiI>=
z<xCi;0wn_#P|hR+MJRQ`Kn0dFVW0|>3{*fllMEE0)CmI>Sk8ojDo`>|0p(0GP=rz^
z3{+q_69%e4$v_2^Gs!>^N}VuJf#pmXr~)Me6;RG314Srx!axO<Ghv_#lnhirIg<<&
zq0|Wj6<E%MfhtfkPyyvkGEjt4Ck#|zITHq|K*>M_lrzad5lWpfP=V!47^ngz0~Jut
zBm+e#b;3XemNQ|X3X}{~Ksl2P6rt1!0~J`#gn=qhGEf2KOfpb}QYQ>lU^x>8szAv=
z1(Y+%KoLrvFi?TzOc<yFB?A>u&LjgxD0RX>5h(_WNHtJ|s(~U@3{+vlKouq#sKO)z
zRhTePg$V;ym}H;|lMGa0!ax-!3{+u~fhtTgP=yHtRhTePg-HggFv&m_CJYpjyt^<^
zM7U#oFv&m>;x@>Tm~w1kporwy!axzpu_XgVD94rz6d{(1Rywdy*0+27pVa~VLwb9P
z{^Z*?e*9#d{!67*4O;Io8qKXd9Cxcg2agWE$*}gaRiyJ09m;g5nC?P#3#$SBnd{cU
z9u`?<23E>_s=DPhX8>sC(sNeo!znhySAyPVx2VlZ;#du@?^*MV&QL@XOD`MK_(sUf
zB@Y|RHMAIHqU<2OWG#F+$7bSiH-`5&!<!hZ7w$Y?2O3Zbs3z~GJAO4_@6x$-z+2{N
zJ&FP0HF><;37Ua%YcHyS(o=hsy)cp;iznYYdqK%w;MohPPt9I%Cuc9T4{M}dETv(&
zFVb$s9w5D^r1yAwZzR2UOnNU$?~jPLu%Pv=)%!|%pQran()-7x_oMX1(e#N2NMBUa
z7kT>PNc!S2>5EbN(rEhR1Eeo0=}SC)X(WB=nDnJ6eR(wf)B~h1E9uKTeR(8(`Iz+O
zD1Bu#{qzH*uPEs&Jbh&(edU<+6{Jtlt86sABht&42R<t>-NpK?-sVp6DUUxiucPip
zr?HjRyTiA}Sa+grVs~TnGNX5g50J6$MBBvf#^xE8fN7pdVfZQ;>rS*y>~3t{xO#W^
zR2l0|v`y@8Y#uD?-QoLXtUJ**vAeP7dP(mNA2nm$iMEN|jXkH!dUyEJ8S758>D@V}
z?yqMb&Ih#uwpy^QV#l%O?`*yxb|)&<#rrG%jb<^BPpO5;32&P98r#{*Yj1|HUi(${
z^;h0_?b_?Rue`z9hS%6wkDk_9%PJdR+F#m+ja7Yk5-pv7X<K&t#`h`6v#i%B)(x*(
z4-Btad(prGc#aMwgT+0s=3|jPL&&%n6gRSf`}tNDYt7Yq)eIRm;;BD(k?TQ0qR832
z*Qn-lYdI=IY#b%swAU)V9P3-XJ@&qr%jNdn$`n@b^E2&7E<Tx%TN@;l1km#PXfu+9
ztlMAdjb0!Lp*4?X_pvJ*>w~UaOOmVyOJt08g&rpM@0OOlz^j|b$S}Qn@j@mx4B2jI
zT>jk8^e!7AyBr$bknMcFVgI(*H9p-%Q#aflP00n1HsYrAmp>hC#ckRBqCeByv4%+V
zML6kt|Ix9Lqx&B2-lQ6L*tH#0b^F==(_=9HWZ?GgTJdmfU~mz<YrC|uSWJkWMe6Vp
z+of9$ZM*HY$>PTPLOs_mO&;djjJFxI>|=&`^3yWbKL#({pD;F|6&r0O=#|k%4D-b5
z+V4F_&ot&gewZG3uFU~G-IxKjd@G%@Oor74`VMvHuxZ*#kr=kOf5kn8$y06=JH^~t
z7R`8?Nc2oOn+dwaCw?@ia?ddLd0^KV4aCuopxCie8X8`mSE?bL+M<}Ld{c<;m7Y&o
zf4(`(Vpt-;Z%7C17~Zq**@lUB1MejKzQ*Ny_E9{GJtW5d!#7#^b^0tG4(YR)elezP
zzna*_`lGnQ{-9mCgMHa=#FaZQ+<F{awoz}QjYf<1z%X#yk7C-S^b!~xu}jT&fb$p}
z@oJ=Y<(()tE>pt6`8zQ?m7Oy9PUrbVqta-KA*OBQ`<-BrCcIzA`?_P_v5gbhru@N0
zqz}$@GE)fywd4G$PR2}FF$;@$h@Pe^q{R8j{oBw!e4=#06TUaQRjh5}F*-|VZ04#J
zJ4q)=Z*j+8zR0)sHkRlvwhvQ5E+_VkF(f|>a?gnXcg$cg<6zBa*tEpSS$T4fPTGfM
zGgyn50LPeF!21RtF+UX@2HSO%LA(8KrbDhko1+Bj;0nH|u&-$kXm=A2@ztaDjQ?W2
z73-(2?-}fU>S*6xrDu_a2JNQORg}GLL?^x#7lk|@RSnw_g`mLyQz7h_LfjQz=r5iK
P)}F_He7!|yIezv(me~h+

literal 0
HcmV?d00001

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dabc024f53..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,2 +1 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/q35/DSDT.multi-bridge",
-- 
2.25.1



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

end of thread, other threads:[~2021-10-11 13:57 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-20  7:00 [PATCH v3 0/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35 Ani Sinha
2021-09-20  7:00 ` [PATCH v3 1/3] tests/acpi/bios-tables-test: add and allow changes to a new q35 DSDT table blob Ani Sinha
2021-10-04 11:34   ` Ani Sinha
2021-09-20  7:00 ` [PATCH v3 2/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35 Ani Sinha
2021-10-07  8:07   ` Igor Mammedov
2021-10-07  8:15     ` Ani Sinha
2021-10-07 11:07       ` Igor Mammedov
2021-10-07 15:22         ` Ani Sinha
2021-09-20  7:00 ` [PATCH v3 3/3] tests/acpi/bios-tables-test: update DSDT blob for multifunction bridge test Ani Sinha
2021-10-04 11:34   ` Ani Sinha
2021-09-23  7:14 ` [PATCH v3 0/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35 Ani Sinha
2021-10-04 11:35   ` Ani Sinha
2021-10-07 13:57 Ani Sinha
2021-10-07 13:57 ` [PATCH v3 3/3] tests/acpi/bios-tables-test: update DSDT blob for multifunction bridge test Ani Sinha
2021-10-11 13:54   ` Igor Mammedov

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