[PATCH v3 0/5] test and QEMU fixes to ensure proper PCIE device usage

[PATCH v3 0/5] test and QEMU fixes to ensure proper PCIE device usage

[Ani Sinha]

Patches 1-4:
Fix tests so that devices do not use non-zero slots on the pcie root
ports. PCIE ports only have one slot, so PCIE devices can only be
plugged into slot 0 on a PCIE port.

Patch 5:
Enforce only one slot on PCIE port.

The test fixes must be applied before the QEMU change that checks for use
of a single slot in PCIE port.

CC: mst@redhat.com
CC: imammedo@redhat.com
CC: jusual@redhat.com
CC: thuth@redhat.com
CC: lvivier@redhat.com
CC: michael.labiuk@virtuozzo.com

Changelog:
v3: tags added. reword the error description in patch 5. Reword commit log in patch 4. 
v2: add hd-geo-test fix as well as the actual QEMU code fix to the patchset.
The patches are added in the right order.


Ani Sinha (5):
  tests/acpi: allow changes in DSDT.noacpihp table blob
  tests/acpi/bios-tables-test: use the correct slot on the
    pcie-root-port
  tests/acpi/bios-tables-test: update acpi blob q35/DSDT.noacpihp
  tests/qtest/hd-geo-test: fix test by removing unnecessary
    pcie-root-port
  hw/pci: ensure PCIE devices are plugged into only slot 0 of PCIE port

 hw/pci/pci.c                      |   6 ++++++
 tests/data/acpi/q35/DSDT.noacpihp | Bin 8248 -> 8241 bytes
 tests/qtest/bios-tables-test.c    |   4 ++--
 tests/qtest/hd-geo-test.c         |  18 ++++++++----------
 4 files changed, 16 insertions(+), 12 deletions(-)

-- 
2.39.1

[PATCH v3 1/5] tests/acpi: allow changes in DSDT.noacpihp table blob

[Ani Sinha]

We are going to fix bio-tables-test in the next patch and hence need to
make sure the acpi tests continue to pass.

Signed-off-by: Ani Sinha <anisinha@redhat.com>
Acked-by: Igor Mammedov <imammedo@redhat.com>
---
 tests/qtest/bios-tables-test-allowed-diff.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..31df9c6187 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.noacpihp",
-- 
2.39.1

[PATCH v3 2/5] tests/acpi/bios-tables-test: use the correct slot on the pcie-root-port

[Ani Sinha]

PCIE ports only have one slot, slot 0. Hence, non-zero slots are not available
for PCIE devices on PCIE root ports. Fix test_acpi_q35_tcg_no_acpi_hotplug()
so that the test does not use them.

Signed-off-by: Ani Sinha <anisinha@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
 tests/qtest/bios-tables-test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index ed1c69cf01..47ba20b957 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1020,9 +1020,9 @@ static void test_acpi_q35_tcg_no_acpi_hotplug(void)
         " -device pci-testdev,bus=nohprp,acpi-index=501"
         " -device pcie-root-port,id=nohprpint,port=0x0,chassis=3,hotplug=off,"
                                  "multifunction=on,addr=8.0"
-        " -device pci-testdev,bus=nohprpint,acpi-index=601,addr=8.1"
+        " -device pci-testdev,bus=nohprpint,acpi-index=601,addr=0.1"
         " -device pcie-root-port,id=hprp2,port=0x0,chassis=4,bus=nohprpint,"
-                                 "addr=9.0"
+                                 "addr=0.2"
         " -device pci-testdev,bus=hprp2,acpi-index=602"
         , &data);
     free_test_data(&data);
-- 
2.39.1

[PATCH v3 3/5] tests/acpi/bios-tables-test: update acpi blob q35/DSDT.noacpihp

[Ani Sinha]

Some fixes were committed in bios-tables-test in the previous commit. Update
the acpi blob and clear bios-tables-test-allowed-diff.h so that the test
continues to pass with the changes in the bios-tables-test.

Following is the asl diff between the old and the newly updated blob:

@@ -1,30 +1,30 @@
 /*
  * Intel ACPI Component Architecture
  * AML/ASL+ Disassembler version 20210604 (64-bit version)
  * Copyright (c) 2000 - 2021 Intel Corporation
  *
  * Disassembling to symbolic ASL+ operators
  *
- * Disassembly of tests/data/acpi/q35/DSDT.noacpihp, Wed Jun 21 18:26:52 2023
+ * Disassembly of /tmp/aml-O8SU61, Wed Jun 21 18:26:52 2023
  *
  * Original Table Header:
  *     Signature        "DSDT"
- *     Length           0x00002038 (8248)
+ *     Length           0x00002031 (8241)
  *     Revision         0x01 **** 32-bit table (V1), no 64-bit math support
- *     Checksum         0x4A
+ *     Checksum         0x89
  *     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
         }

@@ -3148,48 +3148,48 @@
                 {
                     Name (_ADR, Zero)  // _ADR: Address
                     Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
                     {
                         Local0 = Package (0x01)
                             {
                                 0x01F5
                             }
                         Return (EDSM (Arg0, Arg1, Arg2, Arg3, Local0))
                     }
                 }
             }

             Device (S40)
             {
                 Name (_ADR, 0x00080000)  // _ADR: Address
-                Device (S41)
+                Device (S01)
                 {
-                    Name (_ADR, 0x00080001)  // _ADR: Address
+                    Name (_ADR, One)  // _ADR: Address
                     Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
                     {
                         Local0 = Package (0x01)
                             {
                                 0x0259
                             }
                         Return (EDSM (Arg0, Arg1, Arg2, Arg3, Local0))
                     }
                 }

-                Device (S48)
+                Device (S02)
                 {
-                    Name (_ADR, 0x00090000)  // _ADR: Address
+                    Name (_ADR, 0x02)  // _ADR: Address
                     Device (S00)
                     {
                         Name (_ADR, Zero)  // _ADR: Address
                     }
                 }
             }

             Device (SF8)
             {
                 Name (_ADR, 0x001F0000)  // _ADR: Address
                 OperationRegion (PIRQ, PCI_Config, 0x60, 0x0C)
                 Scope (\_SB)
                 {
                     Field (PCI0.SF8.PIRQ, ByteAcc, NoLock, Preserve)
                     {
                         PRQA,   8,

Signed-off-by: Ani Sinha <anisinha@redhat.com>
Acked-by: Igor Mammedov <imammedo@redhat.com>
---
 tests/data/acpi/q35/DSDT.noacpihp           | Bin 8248 -> 8241 bytes
 tests/qtest/bios-tables-test-allowed-diff.h |   1 -
 2 files changed, 1 deletion(-)

diff --git a/tests/data/acpi/q35/DSDT.noacpihp b/tests/data/acpi/q35/DSDT.noacpihp
index 6ab1f0e52543fcb7f84a7fd1327fe5aa42010565..8cab2f8eb9ae94e0165f3f17857ec7d080fb0e13 100644
GIT binary patch
delta 109
zcmdntu+f3bCD<jzP=SGgv2!Dri!7J3UQB$jQ@nt;?&b(tDMlAZ)?gEZc#e2SmmnSn
z1`dYkCY4|VLx=#Qh(x?gurE)65Gx~hBvZl?S0FDVGb=kGx=AwFzzCv>i)r&-xoSoL
DyqFtK

delta 94
zcmdn!u)~4NCD<jzLV<yS(Q6}@i!7IyUQB$jQ@nta-sT8dDMm#P)?gEZc#e2SmmnSn
k1`dYkCXHYdL#O~FP+)SuoHV~ou!#j+5huguZF1F&02bsG6#xJL

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 31df9c6187..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.noacpihp",
-- 
2.39.1

[PATCH v3 4/5] tests/qtest/hd-geo-test: fix test by removing unnecessary pcie-root-port

[Ani Sinha]

The test attaches both a SCSI controller and a pcie-to-pci bridge on the same
pcie-root-port. This is incorrect since only one downstream device can be
attached to a pcie-root-port. Further, in the test scenario, there is no need
to attach a pcie-root-port to the root complex. A SCSI controller can be
attached to a pcie-to-pci bridge which in turn can be directly attached to the
root bus (peie.0). Fix the test and simplify it.

CC: mst@redhat.com
CC: imammedo@redhat.com
CC: Michael Labiuk <michael.labiuk@virtuozzo.com>

Signed-off-by: Ani Sinha <anisinha@redhat.com>
---
 tests/qtest/hd-geo-test.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
index 5aa258a2b3..d08bffad91 100644
--- a/tests/qtest/hd-geo-test.c
+++ b/tests/qtest/hd-geo-test.c
@@ -784,14 +784,12 @@ static void test_override_scsi(void)
     test_override(args, "pc", expected);
 }
 
-static void setup_pci_bridge(TestArgs *args, const char *id, const char *rootid)
+static void setup_pci_bridge(TestArgs *args, const char *id)
 {
 
-    char *root, *br;
-    root = g_strdup_printf("-device pcie-root-port,id=%s", rootid);
-    br = g_strdup_printf("-device pcie-pci-bridge,bus=%s,id=%s", rootid, id);
+    char *br;
+    br = g_strdup_printf("-device pcie-pci-bridge,bus=pcie.0,id=%s", id);
 
-    args->argc = append_arg(args->argc, args->argv, ARGV_SIZE, root);
     args->argc = append_arg(args->argc, args->argv, ARGV_SIZE, br);
 }
 
@@ -811,8 +809,8 @@ static void test_override_scsi_q35(void)
     add_drive_with_mbr(args, empty_mbr, 1);
     add_drive_with_mbr(args, empty_mbr, 1);
     add_drive_with_mbr(args, empty_mbr, 1);
-    setup_pci_bridge(args, "pcie.0", "br");
-    add_scsi_controller(args, "lsi53c895a", "br", 3);
+    setup_pci_bridge(args, "pcie-pci-br");
+    add_scsi_controller(args, "lsi53c895a", "pcie-pci-br", 3);
     add_scsi_disk(args, 0, 0, 0, 0, 0, 10000, 120, 30);
     add_scsi_disk(args, 1, 0, 0, 1, 0, 9000, 120, 30);
     add_scsi_disk(args, 2, 0, 0, 2, 0, 1, 0, 0);
@@ -868,9 +866,9 @@ static void test_override_virtio_blk_q35(void)
     };
     add_drive_with_mbr(args, empty_mbr, 1);
     add_drive_with_mbr(args, empty_mbr, 1);
-    setup_pci_bridge(args, "pcie.0", "br");
-    add_virtio_disk(args, 0, "br", 3, 10000, 120, 30);
-    add_virtio_disk(args, 1, "br", 4, 9000, 120, 30);
+    setup_pci_bridge(args, "pcie-pci-br");
+    add_virtio_disk(args, 0, "pcie-pci-br", 3, 10000, 120, 30);
+    add_virtio_disk(args, 1, "pcie-pci-br", 4, 9000, 120, 30);
     test_override(args, "q35", expected);
 }
 
-- 
2.39.1

[PATCH v3 5/5] hw/pci: ensure PCIE devices are plugged into only slot 0 of PCIE port

[Ani Sinha]

PCI Express ports only have one slot, so PCI Express devices can only be
plugged into slot 0 on a PCIE port. Enforce it.

CC: jusual@redhat.com
CC: imammedo@redhat.com
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2128929
Signed-off-by: Ani Sinha <anisinha@redhat.com>
Reviewed-by: Julia Suvorova <jusual@redhat.com>
---
 hw/pci/pci.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index bf38905b7d..426af133b0 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -64,6 +64,7 @@ bool pci_available = true;
 static char *pcibus_get_dev_path(DeviceState *dev);
 static char *pcibus_get_fw_dev_path(DeviceState *dev);
 static void pcibus_reset(BusState *qbus);
+static bool pcie_has_upstream_port(PCIDevice *dev);
 
 static Property pci_props[] = {
     DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
@@ -1189,6 +1190,11 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev,
                    name);
 
        return NULL;
+    } else if (pcie_has_upstream_port(pci_dev) && PCI_SLOT(devfn)) {
+        error_setg(errp, "PCI: slot %d is not valid for %s,"
+                   " parent device only allows plugging into slot 0.",
+                   PCI_SLOT(devfn), name);
+        return NULL;
     }
 
     pci_dev->devfn = devfn;
-- 
2.39.1

Re: [PATCH v3 4/5] tests/qtest/hd-geo-test: fix test by removing unnecessary pcie-root-port

[Igor Mammedov]

On Mon, 26 Jun 2023 18:13:05 +0530
Ani Sinha <anisinha@redhat.com> wrote:

> The test attaches both a SCSI controller and a pcie-to-pci bridge on the same
> pcie-root-port.

at slot addresses ...
> This is incorrect since only one downstream device can be
> attached to a pcie-root-port.
not true in case of multifunction 
perhaps
  s/only one downstream device/a downstream device/
  s/can be attached to/can be attached to slot 0/

also point out mess with pcie.0 bus name used for as id for bridge.

> Further, in the test scenario, there is no need
> to attach a pcie-root-port to the root complex. A SCSI controller can be
> attached to a pcie-to-pci bridge which in turn can be directly attached to the
> root bus (peie.0). Fix the test and simplify it.
> 
> CC: mst@redhat.com
> CC: imammedo@redhat.com
> CC: Michael Labiuk <michael.labiuk@virtuozzo.com>
> 
> Signed-off-by: Ani Sinha <anisinha@redhat.com>
> ---
>  tests/qtest/hd-geo-test.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
> index 5aa258a2b3..d08bffad91 100644
> --- a/tests/qtest/hd-geo-test.c
> +++ b/tests/qtest/hd-geo-test.c
> @@ -784,14 +784,12 @@ static void test_override_scsi(void)
>      test_override(args, "pc", expected);
>  }
>  
> -static void setup_pci_bridge(TestArgs *args, const char *id, const char *rootid)
> +static void setup_pci_bridge(TestArgs *args, const char *id)
>  {
>  
> -    char *root, *br;
> -    root = g_strdup_printf("-device pcie-root-port,id=%s", rootid);
> -    br = g_strdup_printf("-device pcie-pci-bridge,bus=%s,id=%s", rootid, id);
> +    char *br;
> +    br = g_strdup_printf("-device pcie-pci-bridge,bus=pcie.0,id=%s", id);
>  
> -    args->argc = append_arg(args->argc, args->argv, ARGV_SIZE, root);
>      args->argc = append_arg(args->argc, args->argv, ARGV_SIZE, br);
>  }
>  
> @@ -811,8 +809,8 @@ static void test_override_scsi_q35(void)
>      add_drive_with_mbr(args, empty_mbr, 1);
>      add_drive_with_mbr(args, empty_mbr, 1);
>      add_drive_with_mbr(args, empty_mbr, 1);
> -    setup_pci_bridge(args, "pcie.0", "br");
> -    add_scsi_controller(args, "lsi53c895a", "br", 3);
> +    setup_pci_bridge(args, "pcie-pci-br");
> +    add_scsi_controller(args, "lsi53c895a", "pcie-pci-br", 3);
>      add_scsi_disk(args, 0, 0, 0, 0, 0, 10000, 120, 30);
>      add_scsi_disk(args, 1, 0, 0, 1, 0, 9000, 120, 30);
>      add_scsi_disk(args, 2, 0, 0, 2, 0, 1, 0, 0);
> @@ -868,9 +866,9 @@ static void test_override_virtio_blk_q35(void)
>      };
>      add_drive_with_mbr(args, empty_mbr, 1);
>      add_drive_with_mbr(args, empty_mbr, 1);
> -    setup_pci_bridge(args, "pcie.0", "br");
> -    add_virtio_disk(args, 0, "br", 3, 10000, 120, 30);
> -    add_virtio_disk(args, 1, "br", 4, 9000, 120, 30);
> +    setup_pci_bridge(args, "pcie-pci-br");
> +    add_virtio_disk(args, 0, "pcie-pci-br", 3, 10000, 120, 30);
> +    add_virtio_disk(args, 1, "pcie-pci-br", 4, 9000, 120, 30);
>      test_override(args, "q35", expected);
>  }
>  

Re: [PATCH v3 4/5] tests/qtest/hd-geo-test: fix test by removing unnecessary pcie-root-port

[Michael S. Tsirkin]

On Mon, Jun 26, 2023 at 03:30:14PM +0200, Igor Mammedov wrote:
> On Mon, 26 Jun 2023 18:13:05 +0530
> Ani Sinha <anisinha@redhat.com> wrote:
> 
> > The test attaches both a SCSI controller and a pcie-to-pci bridge on the same
> > pcie-root-port.
> 
> at slot addresses ...
> > This is incorrect since only one downstream device can be
> > attached to a pcie-root-port.
> not true in case of multifunction 
> perhaps
>   s/only one downstream device/a downstream device/
>   s/can be attached to/can be attached to slot 0/

The limitation is that devices can only be attached to slot 0.

BTW once we have ARI all these checks will have to be rewritten.



> also point out mess with pcie.0 bus name used for as id for bridge.
> 
> > Further, in the test scenario, there is no need
> > to attach a pcie-root-port to the root complex. A SCSI controller can be
> > attached to a pcie-to-pci bridge which in turn can be directly attached to the
> > root bus (peie.0). Fix the test and simplify it.
> > 
> > CC: mst@redhat.com
> > CC: imammedo@redhat.com
> > CC: Michael Labiuk <michael.labiuk@virtuozzo.com>
> > 
> > Signed-off-by: Ani Sinha <anisinha@redhat.com>
> > ---
> >  tests/qtest/hd-geo-test.c | 18 ++++++++----------
> >  1 file changed, 8 insertions(+), 10 deletions(-)
> > 
> > diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
> > index 5aa258a2b3..d08bffad91 100644
> > --- a/tests/qtest/hd-geo-test.c
> > +++ b/tests/qtest/hd-geo-test.c
> > @@ -784,14 +784,12 @@ static void test_override_scsi(void)
> >      test_override(args, "pc", expected);
> >  }
> >  
> > -static void setup_pci_bridge(TestArgs *args, const char *id, const char *rootid)
> > +static void setup_pci_bridge(TestArgs *args, const char *id)
> >  {
> >  
> > -    char *root, *br;
> > -    root = g_strdup_printf("-device pcie-root-port,id=%s", rootid);
> > -    br = g_strdup_printf("-device pcie-pci-bridge,bus=%s,id=%s", rootid, id);
> > +    char *br;
> > +    br = g_strdup_printf("-device pcie-pci-bridge,bus=pcie.0,id=%s", id);
> >  
> > -    args->argc = append_arg(args->argc, args->argv, ARGV_SIZE, root);
> >      args->argc = append_arg(args->argc, args->argv, ARGV_SIZE, br);
> >  }
> >  
> > @@ -811,8 +809,8 @@ static void test_override_scsi_q35(void)
> >      add_drive_with_mbr(args, empty_mbr, 1);
> >      add_drive_with_mbr(args, empty_mbr, 1);
> >      add_drive_with_mbr(args, empty_mbr, 1);
> > -    setup_pci_bridge(args, "pcie.0", "br");
> > -    add_scsi_controller(args, "lsi53c895a", "br", 3);
> > +    setup_pci_bridge(args, "pcie-pci-br");
> > +    add_scsi_controller(args, "lsi53c895a", "pcie-pci-br", 3);
> >      add_scsi_disk(args, 0, 0, 0, 0, 0, 10000, 120, 30);
> >      add_scsi_disk(args, 1, 0, 0, 1, 0, 9000, 120, 30);
> >      add_scsi_disk(args, 2, 0, 0, 2, 0, 1, 0, 0);
> > @@ -868,9 +866,9 @@ static void test_override_virtio_blk_q35(void)
> >      };
> >      add_drive_with_mbr(args, empty_mbr, 1);
> >      add_drive_with_mbr(args, empty_mbr, 1);
> > -    setup_pci_bridge(args, "pcie.0", "br");
> > -    add_virtio_disk(args, 0, "br", 3, 10000, 120, 30);
> > -    add_virtio_disk(args, 1, "br", 4, 9000, 120, 30);
> > +    setup_pci_bridge(args, "pcie-pci-br");
> > +    add_virtio_disk(args, 0, "pcie-pci-br", 3, 10000, 120, 30);
> > +    add_virtio_disk(args, 1, "pcie-pci-br", 4, 9000, 120, 30);
> >      test_override(args, "q35", expected);
> >  }
> >  

Re: [PATCH v3 4/5] tests/qtest/hd-geo-test: fix test by removing unnecessary pcie-root-port

[Igor Mammedov]

On Mon, 26 Jun 2023 09:53:40 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Jun 26, 2023 at 03:30:14PM +0200, Igor Mammedov wrote:
> > On Mon, 26 Jun 2023 18:13:05 +0530
> > Ani Sinha <anisinha@redhat.com> wrote:
> >   
> > > The test attaches both a SCSI controller and a pcie-to-pci bridge on the same
> > > pcie-root-port.  
> > 
> > at slot addresses ...  
> > > This is incorrect since only one downstream device can be
> > > attached to a pcie-root-port.  
> > not true in case of multifunction 
> > perhaps
> >   s/only one downstream device/a downstream device/
> >   s/can be attached to/can be attached to slot 0/  
> 
> The limitation is that devices can only be attached to slot 0.
> 
> BTW once we have ARI all these checks will have to be rewritten.

I'm under impression that we already support ARI (pcie_ari_init)

> 
> 
> 
> > also point out mess with pcie.0 bus name used for as id for bridge.
> >   
> > > Further, in the test scenario, there is no need
> > > to attach a pcie-root-port to the root complex. A SCSI controller can be
> > > attached to a pcie-to-pci bridge which in turn can be directly attached to the
> > > root bus (peie.0). Fix the test and simplify it.
> > > 
> > > CC: mst@redhat.com
> > > CC: imammedo@redhat.com
> > > CC: Michael Labiuk <michael.labiuk@virtuozzo.com>
> > > 
> > > Signed-off-by: Ani Sinha <anisinha@redhat.com>
> > > ---
> > >  tests/qtest/hd-geo-test.c | 18 ++++++++----------
> > >  1 file changed, 8 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
> > > index 5aa258a2b3..d08bffad91 100644
> > > --- a/tests/qtest/hd-geo-test.c
> > > +++ b/tests/qtest/hd-geo-test.c
> > > @@ -784,14 +784,12 @@ static void test_override_scsi(void)
> > >      test_override(args, "pc", expected);
> > >  }
> > >  
> > > -static void setup_pci_bridge(TestArgs *args, const char *id, const char *rootid)
> > > +static void setup_pci_bridge(TestArgs *args, const char *id)
> > >  {
> > >  
> > > -    char *root, *br;
> > > -    root = g_strdup_printf("-device pcie-root-port,id=%s", rootid);
> > > -    br = g_strdup_printf("-device pcie-pci-bridge,bus=%s,id=%s", rootid, id);
> > > +    char *br;
> > > +    br = g_strdup_printf("-device pcie-pci-bridge,bus=pcie.0,id=%s", id);
> > >  
> > > -    args->argc = append_arg(args->argc, args->argv, ARGV_SIZE, root);
> > >      args->argc = append_arg(args->argc, args->argv, ARGV_SIZE, br);
> > >  }
> > >  
> > > @@ -811,8 +809,8 @@ static void test_override_scsi_q35(void)
> > >      add_drive_with_mbr(args, empty_mbr, 1);
> > >      add_drive_with_mbr(args, empty_mbr, 1);
> > >      add_drive_with_mbr(args, empty_mbr, 1);
> > > -    setup_pci_bridge(args, "pcie.0", "br");
> > > -    add_scsi_controller(args, "lsi53c895a", "br", 3);
> > > +    setup_pci_bridge(args, "pcie-pci-br");
> > > +    add_scsi_controller(args, "lsi53c895a", "pcie-pci-br", 3);
> > >      add_scsi_disk(args, 0, 0, 0, 0, 0, 10000, 120, 30);
> > >      add_scsi_disk(args, 1, 0, 0, 1, 0, 9000, 120, 30);
> > >      add_scsi_disk(args, 2, 0, 0, 2, 0, 1, 0, 0);
> > > @@ -868,9 +866,9 @@ static void test_override_virtio_blk_q35(void)
> > >      };
> > >      add_drive_with_mbr(args, empty_mbr, 1);
> > >      add_drive_with_mbr(args, empty_mbr, 1);
> > > -    setup_pci_bridge(args, "pcie.0", "br");
> > > -    add_virtio_disk(args, 0, "br", 3, 10000, 120, 30);
> > > -    add_virtio_disk(args, 1, "br", 4, 9000, 120, 30);
> > > +    setup_pci_bridge(args, "pcie-pci-br");
> > > +    add_virtio_disk(args, 0, "pcie-pci-br", 3, 10000, 120, 30);
> > > +    add_virtio_disk(args, 1, "pcie-pci-br", 4, 9000, 120, 30);
> > >      test_override(args, "q35", expected);
> > >  }
> > >    
>