Re: [RFC PATCH] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35

[Ani Sinha]

Ping ...

On Mon, Aug 16, 2021 at 9:14 PM Ani Sinha <ani@anisinha.ca> wrote:
>
> Ping ...
>
> Igor, if this looks ok, I will send the entire patch series with the blobs
> updated.
>
> On Fri, 6 Aug 2021, Ani Sinha 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.
> >
> > The diff of ACPI DSDT table before and after the change d7346e614f4e with 
> > the
> > same newly added unit test is provided below:
> >
> > @@ -5,13 +5,13 @@
> >   *
> >   * Disassembling to symbolic ASL+ operators
> >   *
> > - * Disassembly of /tmp/aml-35UR70, Fri Aug  6 21:00:03 2021
> > + * Disassembly of /tmp/aml-GY8760, Fri Aug  6 21:10:31 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 | 28 ++++++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> >
> > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> > index 51d3a4e239..c92b70e8b8 100644
> > --- a/tests/qtest/bios-tables-test.c
> > +++ b/tests/qtest/bios-tables-test.c
> > @@ -859,6 +859,33 @@ 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",
> > +        .required_struct_types = base_required_struct_types,
> > +        .required_struct_types_len = ARRAY_SIZE(base_required_struct_types)
> > +    };
> > +    /*
> > +     * lets try three things:
> > +     * (a) a multifunction bridge device
> > +     * (b) a bridge device with function 1
> > +     * (c) a non-bridge device with function 2
> > +     * We should see AML hotplug descriptions for (a) and (b) in DSDT.
> > +     * For (a) it should have a hotplug AML description for function 0.
> > +     */
> > +    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 = {
> > @@ -1528,6 +1555,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
> >
> >