W dniu 30.08.2023 o 23:48, Michael S. Tsirkin pisze:
current code sets PCI_SEC_LATENCY_TIMER to WO, but for
pcie to pcie bridges it must be RO 0 according to
pci express spec which says:
This register does not apply to PCI Express. It must be read-only
and hardwired to 00h. For PCI Express to PCI/PCI-X Bridges, refer to the
[PCIe-to-PCI-PCI-X-Bridge] for requirements for this register.
also, fix typo in comment where it's make writeable - this typo
is likely what prevented us noticing we violate this requirement
in the 1st place.
Reported-by: Marcin Juszkiewicz <marcin.juszkiew...@linaro.org>
Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
---
Marcin, could you pls test this patch with virt-8.1 and latest?
Thanks a lot!
Tested-by: Marcin Juszkiewicz <marcin.juszkiew...@linaro.org>
sbsa-ref: PASS
virt: PASS
virt-8.1: FAIL (as expected)
virt-8.0: FAIL (as expected)
./code/qemu/build/aarch64-softmmu/qemu-system-aarch64 \
-machine virt \
-m 4096 \
-cpu neoverse-n1 \
-smp 2 \
-drive if=pflash,format=raw,file=QEMU_EFI.fd \
-drive if=pflash,format=raw,file=QEMU_EFI-vars.fd \
-drive file=fat:rw:$PWD/disks/virtual/,format=raw \
-device pcie-root-port,id=root30,chassis=30,slot=0 \
-device igb,bus=root30 \
-device qemu-xhci,id=usb \
-device bochs-display \
-device e1000e \
-nographic \
-usb \
-device usb-kbd \
-device usb-tablet \
-monitor telnet::45454,server,nowait \
-serial stdio
include/hw/pci/pci_bridge.h | 3 +++
hw/core/machine.c | 5 ++++-
hw/pci/pci.c | 2 +-
hw/pci/pci_bridge.c | 14 ++++++++++++++
4 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
index ea54a81a15..5cd452115a 100644
--- a/include/hw/pci/pci_bridge.h
+++ b/include/hw/pci/pci_bridge.h
@@ -77,6 +77,9 @@ struct PCIBridge {
pci_map_irq_fn map_irq;
const char *bus_name;
+
+ /* SLT is RO for PCIE to PCIE bridges, but old QEMU versions had it RW */
+ bool pcie_writeable_slt_bug;
};
#define PCI_BRIDGE_DEV_PROP_CHASSIS_NR "chassis_nr"
diff --git a/hw/core/machine.c b/hw/core/machine.c
index da699cf4e1..76743e3b31 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -32,6 +32,7 @@
#include "qemu/error-report.h"
#include "sysemu/qtest.h"
#include "hw/pci/pci.h"
+#include "hw/pci/pci_bridge.h"
#include "hw/mem/nvdimm.h"
#include "migration/global_state.h"
#include "migration/vmstate.h"
@@ -39,7 +40,9 @@
#include "hw/virtio/virtio.h"
#include "hw/virtio/virtio-pci.h"
-GlobalProperty hw_compat_8_1[] = {};
+GlobalProperty hw_compat_8_1[] = {
+ { TYPE_PCI_BRIDGE, "x-pci-express-writeable-slt-bug", "true" },
+};
const size_t hw_compat_8_1_len = G_N_ELEMENTS(hw_compat_8_1);
GlobalProperty hw_compat_8_0[] = {
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 881d774fb6..b0d21bf43a 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -893,7 +893,7 @@ static void pci_init_w1cmask(PCIDevice *dev)
static void pci_init_mask_bridge(PCIDevice *d)
{
/* PCI_PRIMARY_BUS, PCI_SECONDARY_BUS, PCI_SUBORDINATE_BUS and
- PCI_SEC_LETENCY_TIMER */
+ PCI_SEC_LATENCY_TIMER */
memset(d->wmask + PCI_PRIMARY_BUS, 0xff, 4);
/* base and limit */
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index e7b9345615..6a4e38856d 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -38,6 +38,7 @@
#include "qapi/error.h"
#include "hw/acpi/acpi_aml_interface.h"
#include "hw/acpi/pci.h"
+#include "hw/qdev-properties.h"
/* PCI bridge subsystem vendor ID helper functions */
#define PCI_SSVID_SIZEOF 8
@@ -385,6 +386,11 @@ void pci_bridge_initfn(PCIDevice *dev, const char
*typename)
pci_bridge_region_init(br);
QLIST_INIT(&sec_bus->child);
QLIST_INSERT_HEAD(&parent->child, sec_bus, sibling);
+
+ /* For express secondary buses, secondary latency timer is RO 0 */
+ if (pci_bus_is_express(sec_bus) && !br->pcie_writeable_slt_bug) {
+ dev->wmask[PCI_SEC_LATENCY_TIMER] = 0;
+ }
}
/* default qdev clean up function for PCI-to-PCI bridge */
@@ -466,10 +472,18 @@ int pci_bridge_qemu_reserve_cap_init(PCIDevice *dev, int
cap_offset,
return 0;
}
+static Property pci_bridge_properties[] = {
+ DEFINE_PROP_BOOL("x-pci-express-writeable-slt-bug", PCIBridge,
+ pcie_writeable_slt_bug, false),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
static void pci_bridge_class_init(ObjectClass *klass, void *data)
{
AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass);
+ DeviceClass *k = DEVICE_CLASS(klass);
+ device_class_set_props(k, pci_bridge_properties);
adevc->build_dev_aml = build_pci_bridge_aml;
}
