* [PATCH 1/2] Revert "hw/pxb-cxl: Support passthrough HDM Decoders unless overridden"
2023-04-17 13:00 [PATCH 0/2] Re-enable qom-cast-debug by default Thomas Huth
@ 2023-04-17 13:00 ` Thomas Huth
2023-04-17 13:00 ` [PATCH 2/2] meson_options.txt: Enable qom-cast-debug by default again Thomas Huth
1 sibling, 0 replies; 5+ messages in thread
From: Thomas Huth @ 2023-04-17 13:00 UTC (permalink / raw)
To: peter.maydell, qemu-devel, Michael S. Tsirkin, Jonathan Cameron
Cc: Fan Ni, Paolo Bonzini
This reverts commit 154070eaf6597c47f64c3ea917bcba62427ae61f.
The pxb_cxl_dev_reset() function tries to cast the device via
PXB_DEV(), however the function belongs to TYPE_PXB_CXL_DEVICE
which is not derived from TYPE_PXB_DEVICE. So this causes QEMU
to abort in case the QOM checks have been enabled with the
"--enable-qom-cast-debug" configure option. Thus revert this
faulty patch so we can enable the qom cast debug switch by de-
fault again - and the pxb-cxl code should be redone in a proper
way later.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
include/hw/cxl/cxl.h | 1 -
include/hw/cxl/cxl_component.h | 1 -
include/hw/pci/pci_bridge.h | 1 -
hw/cxl/cxl-host.c | 31 ++++++++------------
hw/pci-bridge/pci_expander_bridge.c | 44 ++++-------------------------
5 files changed, 17 insertions(+), 61 deletions(-)
diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
index b2cffbb364..b161be59b7 100644
--- a/include/hw/cxl/cxl.h
+++ b/include/hw/cxl/cxl.h
@@ -49,7 +49,6 @@ struct CXLHost {
PCIHostState parent_obj;
CXLComponentState cxl_cstate;
- bool passthrough;
};
#define TYPE_PXB_CXL_HOST "pxb-cxl-host"
diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h
index 42c7e581a7..ec4203b83f 100644
--- a/include/hw/cxl/cxl_component.h
+++ b/include/hw/cxl/cxl_component.h
@@ -247,7 +247,6 @@ static inline hwaddr cxl_decode_ig(int ig)
}
CXLComponentState *cxl_get_hb_cstate(PCIHostState *hb);
-bool cxl_get_hb_passthrough(PCIHostState *hb);
void cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp);
void cxl_doe_cdat_release(CXLComponentState *cxl_cstate);
diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
index 1677176b2a..68b88ec5e4 100644
--- a/include/hw/pci/pci_bridge.h
+++ b/include/hw/pci/pci_bridge.h
@@ -92,7 +92,6 @@ struct PXBDev {
uint8_t bus_nr;
uint16_t numa_node;
bool bypass_iommu;
- bool hdm_for_passthrough;
struct cxl_dev {
CXLHost *cxl_host_bridge; /* Pointer to a CXLHost */
} cxl;
diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
index 6e923ceeaf..3c1ec8732a 100644
--- a/hw/cxl/cxl-host.c
+++ b/hw/cxl/cxl-host.c
@@ -146,28 +146,21 @@ static PCIDevice *cxl_cfmws_find_device(CXLFixedWindow *fw, hwaddr addr)
return NULL;
}
- if (cxl_get_hb_passthrough(hb)) {
- rp = pcie_find_port_first(hb->bus);
- if (!rp) {
- return NULL;
- }
- } else {
- hb_cstate = cxl_get_hb_cstate(hb);
- if (!hb_cstate) {
- return NULL;
- }
+ hb_cstate = cxl_get_hb_cstate(hb);
+ if (!hb_cstate) {
+ return NULL;
+ }
- cache_mem = hb_cstate->crb.cache_mem_registers;
+ cache_mem = hb_cstate->crb.cache_mem_registers;
- target_found = cxl_hdm_find_target(cache_mem, addr, &target);
- if (!target_found) {
- return NULL;
- }
+ target_found = cxl_hdm_find_target(cache_mem, addr, &target);
+ if (!target_found) {
+ return NULL;
+ }
- rp = pcie_find_port_by_pn(hb->bus, target);
- if (!rp) {
- return NULL;
- }
+ rp = pcie_find_port_by_pn(hb->bus, target);
+ if (!rp) {
+ return NULL;
}
d = pci_bridge_get_sec_bus(PCI_BRIDGE(rp))->devices[0];
diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index ead33f0c05..e752a21292 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -15,7 +15,6 @@
#include "hw/pci/pci.h"
#include "hw/pci/pci_bus.h"
#include "hw/pci/pci_host.h"
-#include "hw/pci/pcie_port.h"
#include "hw/qdev-properties.h"
#include "hw/pci/pci_bridge.h"
#include "hw/pci-bridge/pci_expander_bridge.h"
@@ -80,13 +79,6 @@ CXLComponentState *cxl_get_hb_cstate(PCIHostState *hb)
return &host->cxl_cstate;
}
-bool cxl_get_hb_passthrough(PCIHostState *hb)
-{
- CXLHost *host = PXB_CXL_HOST(hb);
-
- return host->passthrough;
-}
-
static int pxb_bus_num(PCIBus *bus)
{
PXBDev *pxb = convert_to_pxb(bus->parent_dev);
@@ -297,32 +289,15 @@ static int pxb_map_irq_fn(PCIDevice *pci_dev, int pin)
return pin - PCI_SLOT(pxb->devfn);
}
-static void pxb_cxl_dev_reset(DeviceState *dev)
+static void pxb_dev_reset(DeviceState *dev)
{
CXLHost *cxl = PXB_CXL_DEV(dev)->cxl.cxl_host_bridge;
CXLComponentState *cxl_cstate = &cxl->cxl_cstate;
- PCIHostState *hb = PCI_HOST_BRIDGE(cxl);
uint32_t *reg_state = cxl_cstate->crb.cache_mem_registers;
uint32_t *write_msk = cxl_cstate->crb.cache_mem_regs_write_mask;
- int dsp_count = 0;
cxl_component_register_init_common(reg_state, write_msk, CXL2_ROOT_PORT);
- /*
- * The CXL specification allows for host bridges with no HDM decoders
- * if they only have a single root port.
- */
- if (!PXB_DEV(dev)->hdm_for_passthrough) {
- dsp_count = pcie_count_ds_ports(hb->bus);
- }
- /* Initial reset will have 0 dsp so wait until > 0 */
- if (dsp_count == 1) {
- cxl->passthrough = true;
- /* Set Capability ID in header to NONE */
- ARRAY_FIELD_DP32(reg_state, CXL_HDM_CAPABILITY_HEADER, ID, 0);
- } else {
- ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, TARGET_COUNT,
- 8);
- }
+ ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, TARGET_COUNT, 8);
}
static gint pxb_compare(gconstpointer a, gconstpointer b)
@@ -506,18 +481,9 @@ static void pxb_cxl_dev_realize(PCIDevice *dev, Error **errp)
}
pxb_dev_realize_common(dev, CXL, errp);
- pxb_cxl_dev_reset(DEVICE(dev));
+ pxb_dev_reset(DEVICE(dev));
}
-static Property pxb_cxl_dev_properties[] = {
- /* Note: 0 is not a legal PXB bus number. */
- DEFINE_PROP_UINT8("bus_nr", PXBDev, bus_nr, 0),
- DEFINE_PROP_UINT16("numa_node", PXBDev, numa_node, NUMA_NODE_UNASSIGNED),
- DEFINE_PROP_BOOL("bypass_iommu", PXBDev, bypass_iommu, false),
- DEFINE_PROP_BOOL("hdm_for_passthrough", PXBDev, hdm_for_passthrough, false),
- DEFINE_PROP_END_OF_LIST(),
-};
-
static void pxb_cxl_dev_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -531,12 +497,12 @@ static void pxb_cxl_dev_class_init(ObjectClass *klass, void *data)
*/
dc->desc = "CXL Host Bridge";
- device_class_set_props(dc, pxb_cxl_dev_properties);
+ device_class_set_props(dc, pxb_dev_properties);
set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
/* Host bridges aren't hotpluggable. FIXME: spec reference */
dc->hotpluggable = false;
- dc->reset = pxb_cxl_dev_reset;
+ dc->reset = pxb_dev_reset;
}
static const TypeInfo pxb_cxl_dev_info = {
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] meson_options.txt: Enable qom-cast-debug by default again
2023-04-17 13:00 [PATCH 0/2] Re-enable qom-cast-debug by default Thomas Huth
2023-04-17 13:00 ` [PATCH 1/2] Revert "hw/pxb-cxl: Support passthrough HDM Decoders unless overridden" Thomas Huth
@ 2023-04-17 13:00 ` Thomas Huth
2023-04-17 13:13 ` Peter Maydell
2023-04-20 9:26 ` Philippe Mathieu-Daudé
1 sibling, 2 replies; 5+ messages in thread
From: Thomas Huth @ 2023-04-17 13:00 UTC (permalink / raw)
To: peter.maydell, qemu-devel, Michael S. Tsirkin, Jonathan Cameron
Cc: Fan Ni, Paolo Bonzini
This switch had been disabled by default by accident in commit
c55cf6ab03f. But we should enable it by default instead to avoid
regressions in the QOM device hierarchy.
Fixes: c55cf6ab03 ("configure, meson: move some default-disabled options to meson_options.txt")
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
meson_options.txt | 2 +-
scripts/meson-buildoptions.sh | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/meson_options.txt b/meson_options.txt
index fc9447d267..2471dd02da 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -315,7 +315,7 @@ option('debug_mutex', type: 'boolean', value: false,
description: 'mutex debugging support')
option('debug_stack_usage', type: 'boolean', value: false,
description: 'measure coroutine stack usage')
-option('qom_cast_debug', type: 'boolean', value: false,
+option('qom_cast_debug', type: 'boolean', value: true,
description: 'cast debugging support')
option('gprof', type: 'boolean', value: false,
description: 'QEMU profiling with gprof',
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 009fab1515..d4369a3ad8 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -14,6 +14,7 @@ meson_options_help() {
printf "%s\n" ' use idef-parser to automatically generate TCG'
printf "%s\n" ' code for the Hexagon frontend'
printf "%s\n" ' --disable-install-blobs install provided firmware blobs'
+ printf "%s\n" ' --disable-qom-cast-debug cast debugging support'
printf "%s\n" ' --docdir=VALUE Base directory for documentation installation'
printf "%s\n" ' (can be empty) [share/doc]'
printf "%s\n" ' --enable-block-drv-whitelist-in-tools'
@@ -35,7 +36,6 @@ meson_options_help() {
printf "%s\n" ' --enable-module-upgrades try to load modules from alternate paths for'
printf "%s\n" ' upgrades'
printf "%s\n" ' --enable-profiler profiler support'
- printf "%s\n" ' --enable-qom-cast-debug cast debugging support'
printf "%s\n" ' --enable-rng-none dummy RNG, avoid using /dev/(u)random and'
printf "%s\n" ' getrandom()'
printf "%s\n" ' --enable-strip Strip targets on install'
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread