On 31/8/23 08:45, Michael S. Tsirkin wrote:
On Thu, Aug 31, 2023 at 08:22:34AM +0200, Philippe Mathieu-Daudé wrote:
Hi Michael,
On 30/8/23 23:48, Michael S. Tsirkin wrote:
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!
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;
};
Patch LGTM, so:
Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
-GlobalProperty hw_compat_8_1[] = {};
+GlobalProperty hw_compat_8_1[] = {
+ { TYPE_PCI_BRIDGE, "x-pci-express-writeable-slt-bug", "true" },
However I don't understand why we can't clear the config register and
must use a compat flag, since per the spec it is hardwired to 0.
Because historically we didn't. If we make it RO and migrate from
VM where guest wrote into this register, migration will fail
as we check that RO fields are unchanged.
Another trick would be to pretend it's hardware driven
and set cmask. Compat machinery is more straight-forward though.
I see.
Do we need the "x-" compat prefix? This is not an experimental property.
It's an internal one, we don't want users to tweak it.
OK, thanks!
