* [PATCH v3 0/5] test and QEMU fixes to ensure proper PCIE device usage
@ 2023-06-26 12:43 Ani Sinha
2023-06-26 12:43 ` [PATCH v3 1/5] tests/acpi: allow changes in DSDT.noacpihp table blob Ani Sinha
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Ani Sinha @ 2023-06-26 12:43 UTC (permalink / raw)
To: qemu-devel
Cc: Ani Sinha, mst, imammedo, jusual, thuth, lvivier, michael.labiuk
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
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v3 1/5] tests/acpi: allow changes in DSDT.noacpihp table blob 2023-06-26 12:43 [PATCH v3 0/5] test and QEMU fixes to ensure proper PCIE device usage Ani Sinha @ 2023-06-26 12:43 ` Ani Sinha 2023-06-26 12:43 ` [PATCH v3 2/5] tests/acpi/bios-tables-test: use the correct slot on the pcie-root-port Ani Sinha ` (3 subsequent siblings) 4 siblings, 0 replies; 9+ messages in thread From: Ani Sinha @ 2023-06-26 12:43 UTC (permalink / raw) To: qemu-devel, Michael S. Tsirkin, Igor Mammedov, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 2/5] tests/acpi/bios-tables-test: use the correct slot on the pcie-root-port 2023-06-26 12:43 [PATCH v3 0/5] test and QEMU fixes to ensure proper PCIE device usage Ani Sinha 2023-06-26 12:43 ` [PATCH v3 1/5] tests/acpi: allow changes in DSDT.noacpihp table blob Ani Sinha @ 2023-06-26 12:43 ` Ani Sinha 2023-06-26 12:43 ` [PATCH v3 3/5] tests/acpi/bios-tables-test: update acpi blob q35/DSDT.noacpihp Ani Sinha ` (2 subsequent siblings) 4 siblings, 0 replies; 9+ messages in thread From: Ani Sinha @ 2023-06-26 12:43 UTC (permalink / raw) To: qemu-devel, Michael S. Tsirkin, Igor Mammedov, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 3/5] tests/acpi/bios-tables-test: update acpi blob q35/DSDT.noacpihp 2023-06-26 12:43 [PATCH v3 0/5] test and QEMU fixes to ensure proper PCIE device usage Ani Sinha 2023-06-26 12:43 ` [PATCH v3 1/5] tests/acpi: allow changes in DSDT.noacpihp table blob Ani Sinha 2023-06-26 12:43 ` [PATCH v3 2/5] tests/acpi/bios-tables-test: use the correct slot on the pcie-root-port Ani Sinha @ 2023-06-26 12:43 ` Ani Sinha 2023-06-26 12:43 ` [PATCH v3 4/5] tests/qtest/hd-geo-test: fix test by removing unnecessary pcie-root-port Ani Sinha 2023-06-26 12:43 ` [PATCH v3 5/5] hw/pci: ensure PCIE devices are plugged into only slot 0 of PCIE port Ani Sinha 4 siblings, 0 replies; 9+ messages in thread From: Ani Sinha @ 2023-06-26 12:43 UTC (permalink / raw) To: qemu-devel, Michael S. Tsirkin, Igor Mammedov, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 4/5] tests/qtest/hd-geo-test: fix test by removing unnecessary pcie-root-port 2023-06-26 12:43 [PATCH v3 0/5] test and QEMU fixes to ensure proper PCIE device usage Ani Sinha ` (2 preceding siblings ...) 2023-06-26 12:43 ` [PATCH v3 3/5] tests/acpi/bios-tables-test: update acpi blob q35/DSDT.noacpihp Ani Sinha @ 2023-06-26 12:43 ` Ani Sinha 2023-06-26 13:30 ` Igor Mammedov 2023-06-26 12:43 ` [PATCH v3 5/5] hw/pci: ensure PCIE devices are plugged into only slot 0 of PCIE port Ani Sinha 4 siblings, 1 reply; 9+ messages in thread From: Ani Sinha @ 2023-06-26 12:43 UTC (permalink / raw) To: qemu-devel, Thomas Huth, Laurent Vivier, Paolo Bonzini Cc: Ani Sinha, mst, imammedo, Michael Labiuk 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 4/5] tests/qtest/hd-geo-test: fix test by removing unnecessary pcie-root-port 2023-06-26 12:43 ` [PATCH v3 4/5] tests/qtest/hd-geo-test: fix test by removing unnecessary pcie-root-port Ani Sinha @ 2023-06-26 13:30 ` Igor Mammedov 2023-06-26 13:53 ` Michael S. Tsirkin 0 siblings, 1 reply; 9+ messages in thread From: Igor Mammedov @ 2023-06-26 13:30 UTC (permalink / raw) To: Ani Sinha Cc: qemu-devel, Thomas Huth, Laurent Vivier, Paolo Bonzini, mst, Michael Labiuk 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); > } > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 4/5] tests/qtest/hd-geo-test: fix test by removing unnecessary pcie-root-port 2023-06-26 13:30 ` Igor Mammedov @ 2023-06-26 13:53 ` Michael S. Tsirkin 2023-06-26 14:09 ` Igor Mammedov 0 siblings, 1 reply; 9+ messages in thread From: Michael S. Tsirkin @ 2023-06-26 13:53 UTC (permalink / raw) To: Igor Mammedov Cc: Ani Sinha, qemu-devel, Thomas Huth, Laurent Vivier, Paolo Bonzini, Michael Labiuk 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); > > } > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 4/5] tests/qtest/hd-geo-test: fix test by removing unnecessary pcie-root-port 2023-06-26 13:53 ` Michael S. Tsirkin @ 2023-06-26 14:09 ` Igor Mammedov 0 siblings, 0 replies; 9+ messages in thread From: Igor Mammedov @ 2023-06-26 14:09 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Ani Sinha, qemu-devel, Thomas Huth, Laurent Vivier, Paolo Bonzini, Michael Labiuk 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); > > > } > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 5/5] hw/pci: ensure PCIE devices are plugged into only slot 0 of PCIE port 2023-06-26 12:43 [PATCH v3 0/5] test and QEMU fixes to ensure proper PCIE device usage Ani Sinha ` (3 preceding siblings ...) 2023-06-26 12:43 ` [PATCH v3 4/5] tests/qtest/hd-geo-test: fix test by removing unnecessary pcie-root-port Ani Sinha @ 2023-06-26 12:43 ` Ani Sinha 4 siblings, 0 replies; 9+ messages in thread From: Ani Sinha @ 2023-06-26 12:43 UTC (permalink / raw) To: qemu-devel, Michael S. Tsirkin, Marcel Apfelbaum Cc: Ani Sinha, jusual, imammedo 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-06-26 14:09 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-06-26 12:43 [PATCH v3 0/5] test and QEMU fixes to ensure proper PCIE device usage Ani Sinha 2023-06-26 12:43 ` [PATCH v3 1/5] tests/acpi: allow changes in DSDT.noacpihp table blob Ani Sinha 2023-06-26 12:43 ` [PATCH v3 2/5] tests/acpi/bios-tables-test: use the correct slot on the pcie-root-port Ani Sinha 2023-06-26 12:43 ` [PATCH v3 3/5] tests/acpi/bios-tables-test: update acpi blob q35/DSDT.noacpihp Ani Sinha 2023-06-26 12:43 ` [PATCH v3 4/5] tests/qtest/hd-geo-test: fix test by removing unnecessary pcie-root-port Ani Sinha 2023-06-26 13:30 ` Igor Mammedov 2023-06-26 13:53 ` Michael S. Tsirkin 2023-06-26 14:09 ` Igor Mammedov 2023-06-26 12:43 ` [PATCH v3 5/5] hw/pci: ensure PCIE devices are plugged into only slot 0 of PCIE port Ani Sinha
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.