[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] hw/pci/pci_bridge: ensure PCIe slots have only one slot
|
From: |
Thomas Huth |
|
Subject: |
Re: [PATCH v2] hw/pci/pci_bridge: ensure PCIe slots have only one slot |
|
Date: |
Tue, 19 Jul 2022 12:42:47 +0200 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.10.0 |
On 19/07/2022 10.01, Roman Kagan wrote:
It's possible to create non-working configurations by attaching a device
to a derivative of PCIe slot (pcie-root-port, ioh3420, etc) and
specifying a slot number other that zero, e.g.:
-device pcie-root-port,id=s0,... \
-device virtio-blk-pci,bus=s0,addr=4,...
Make QEMU reject such configurations and only allow addr=0 on the
secondary bus of a PCIe slot.
To verify this new behavior, add two basic qtests for the PCIe bridges
that may be affected by change: pcie-root-port and x3130. For the
former, two testcases are included, one positive for slot #0 and one
negative for (arbitrary) slot #4; for the latter, only a positive
testcase for slot #4 is included.
Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru>
---
v1 -> v2:
- use object_dynamic_cast (without assert) [Vladimir]
- add explaining comment [Michael]
- add tests
(I've only had a chance to run tests against x86; hope I didn't mess
them up for other arches)
With my s390x hat on: This should not affect s390x - we're not using PCIe there.
hw/pci/pci_bridge.c | 6 +++
tests/qtest/pcie-root-port-test.c | 77 +++++++++++++++++++++++++++++++
tests/qtest/xio3130-test.c | 54 ++++++++++++++++++++++
tests/qtest/meson.build | 2 +
4 files changed, 139 insertions(+)
create mode 100644 tests/qtest/pcie-root-port-test.c
create mode 100644 tests/qtest/xio3130-test.c
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index da34c8ebcd..23e1701d06 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -33,6 +33,7 @@
#include "qemu/units.h"
#include "hw/pci/pci_bridge.h"
#include "hw/pci/pci_bus.h"
+#include "hw/pci/pcie_port.h"
#include "qemu/module.h"
#include "qemu/range.h"
#include "qapi/error.h"
@@ -386,6 +387,11 @@ void pci_bridge_initfn(PCIDevice *dev, const char
*typename)
br->windows = pci_bridge_region_init(br);
QLIST_INIT(&sec_bus->child);
QLIST_INSERT_HEAD(&parent->child, sec_bus, sibling);
+
+ /* PCIe slot derivatives are bridges with a single slot; enforce that */
+ if (object_dynamic_cast(OBJECT(dev), TYPE_PCIE_SLOT)) {
+ sec_bus->slot_reserved_mask = ~1u;
+ }
}
/* default qdev clean up function for PCI-to-PCI bridge */
diff --git a/tests/qtest/pcie-root-port-test.c
b/tests/qtest/pcie-root-port-test.c
new file mode 100644
index 0000000000..a1f3d84d75
--- /dev/null
+++ b/tests/qtest/pcie-root-port-test.c
@@ -0,0 +1,77 @@
+/*
+ * QTest testcase for generic PCIe root port
+ *
+ * Copyright (c) 2022 Yandex N.V.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest-single.h"
Do you really need libqtest-single.h here? libqtest.h should be enough,
shouldn't it?
+/*
+ * Let QEMU choose the bus and slot for the device under test. It may even be
+ * a non-PCIe bus but it's ok for the purpose of the test.
+ */
+static const char *common_args = "-device pcie-root-port,id=s0"
+ ",port=1,chassis=1,multifunction=on";
+
+static void test_slot0(void)
+{
+ QTestState *qts;
+ QDict *resp;
+
+ /* attach a PCIe device into slot0 of the root port */
+ qts = qtest_init(common_args);
+ /* PCIe root port is known to be supported, use it as a leaf device too */
+ resp = qtest_qmp(qts, "{'execute': 'device_add', 'arguments': {"
+ "'driver': 'pcie-root-port', "
+ "'id': 'port1', "
+ "'bus': 's0', "
+ "'chassis': 5, "
+ "'addr': '0'"
+ "} }");
+ g_assert_nonnull(resp);
+ g_assert(!qdict_haskey(resp, "event"));
+ g_assert(!qdict_haskey(resp, "error"));
+ qobject_unref(resp);
+
+ qtest_quit(qts);
+}
+
+static void test_slot4(void)
+{
+ QTestState *qts;
+ QDict *resp;
+
+ /* attach a PCIe device into slot4 of the root port should be rejected */
+ qts = qtest_init(common_args);
+ /* PCIe root port is known to be supported, use it as a leaf device too */
+ resp = qtest_qmp(qts, "{'execute': 'device_add', 'arguments': {"
+ "'driver': 'pcie-root-port', "
+ "'id': 'port1', "
+ "'bus': 's0', "
+ "'chassis': 5, "
+ "'addr': '4'"
+ "} }");
+ qmp_expect_error_and_unref(resp, "GenericError");
+
+ qtest_quit(qts);
+}
+
+int main(int argc, char **argv)
+{
+ int ret;
+
+ g_test_init(&argc, &argv, NULL);
+
+ qtest_add_func("/pcie-root-port/slot0", test_slot0);
+ qtest_add_func("/pcie-root-port/slot4", test_slot4);
+
+ ret = g_test_run();
+
+ qtest_end();
Please drop the qtest_end() - you're already using qtest_quit in the
individual tests.
+ return ret;
+}
diff --git a/tests/qtest/xio3130-test.c b/tests/qtest/xio3130-test.c
new file mode 100644
index 0000000000..3a937889b9
--- /dev/null
+++ b/tests/qtest/xio3130-test.c
@@ -0,0 +1,54 @@
+/*
+ * QTest testcase for TI X3130 PCIe switch
+ *
+ * Copyright (c) 2022 Yandex N.V.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest-single.h"
dito - please switch to libqtest.h
+/*
+ * Let QEMU choose the bus and slot for the device under test. It may even be
+ * a non-PCIe bus but it's ok for the purpose of the test.
+ */
+static const char *common_args = "-device x3130-upstream,id=s0";
+
+static void test_slot4(void)
+{
+ QTestState *qts;
+ QDict *resp;
+
+ /* attach a downstream port into slot4 of the upstream port */
+ qts = qtest_init(common_args);
+ resp = qtest_qmp(qts, "{'execute': 'device_add', 'arguments': {"
+ "'driver': 'xio3130-downstream', "
+ "'id': 'port1', "
+ "'bus': 's0', "
+ "'chassis': 5, "
+ "'addr': '4'"
+ "} }");
+ g_assert_nonnull(resp);
+ g_assert(!qdict_haskey(resp, "event"));
+ g_assert(!qdict_haskey(resp, "error"));
+ qobject_unref(resp);
+
+ qtest_quit(qts);
+}
+
+int main(int argc, char **argv)
+{
+ int ret;
+
+ g_test_init(&argc, &argv, NULL);
+
+ qtest_add_func("/x3130/slot4", test_slot4);
+
+ ret = g_test_run();
+
+ qtest_end();
Drop qtest_end() also here.
+ return ret;
+}
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 31287a9173..19cab1bc35 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -54,6 +54,7 @@ qtests_i386 = \
(config_all_devices.has_key('CONFIG_I82801B11') ? ['i82801b11-test'] : [])
+ \
(config_all_devices.has_key('CONFIG_IOH3420') ? ['ioh3420-test'] : []) +
\
(config_all_devices.has_key('CONFIG_LPC_ICH9') ? ['lpc-ich9-test'] : []) +
\
+ (config_all_devices.has_key('CONFIG_PCIE_PORT') ? ['pcie-root-port-test'] :
[]) + \
(config_all_devices.has_key('CONFIG_USB_UHCI') ? ['usb-hcd-uhci-test'] :
[]) + \
(config_all_devices.has_key('CONFIG_USB_UHCI') and
\
config_all_devices.has_key('CONFIG_USB_EHCI') ? ['usb-hcd-ehci-test'] :
[]) + \
@@ -63,6 +64,7 @@ qtests_i386 = \
(config_all_devices.has_key('CONFIG_TPM_TIS_ISA') ? ['tpm-tis-test'] : [])
+ \
(config_all_devices.has_key('CONFIG_TPM_TIS_ISA') ? ['tpm-tis-swtpm-test']
: []) + \
(config_all_devices.has_key('CONFIG_RTL8139_PCI') ? ['rtl8139-test'] : [])
+ \
+ (config_all_devices.has_key('CONFIG_XIO3130') ? ['xio3130-test'] : []) +
\
(config_all_devices.has_key('CONFIG_E1000E_PCI_EXPRESS') ?
['fuzz-e1000e-test'] : []) + \
(config_all_devices.has_key('CONFIG_MEGASAS_SCSI_PCI') ?
['fuzz-megasas-test'] : []) + \
(config_all_devices.has_key('CONFIG_LSI_SCSI_PCI') ?
['fuzz-lsi53c895a-test'] : []) + \
Thomas
- Re: [PATCH v2] hw/pci/pci_bridge: ensure PCIe slots have only one slot,
Thomas Huth <=