[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 2/3] tests/acpi/pcihp: add unit tests for hotplug on multi
|
From: |
Ani Sinha |
|
Subject: |
Re: [PATCH v3 2/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35 |
|
Date: |
Thu, 7 Oct 2021 13:45:36 +0530 (IST) |
|
User-agent: |
Alpine 2.22 (DEB 394 2020-01-19) |
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);
>
>