* [PATCH 0/5] buses: switch to 3-phase-reset
@ 2024-01-19 16:35 Peter Maydell
2024-01-19 16:35 ` [PATCH 1/5] pci: Switch bus reset " Peter Maydell
` (7 more replies)
0 siblings, 8 replies; 21+ messages in thread
From: Peter Maydell @ 2024-01-19 16:35 UTC (permalink / raw)
To: qemu-devel
Cc: Maciej S. Szmigiero, Mark Cave-Ayland, Michael S. Tsirkin,
Marcel Apfelbaum, Halil Pasic, Christian Borntraeger,
Eric Farman, Peter Xu, Philippe Mathieu-Daudé
This patchset switches the handful of bus types that implement a
reset method over to using the 3-phase-reset APIs, and then removes
the transitional infrastructure from the core bus class that was
supporting the ability to have bus types that use old-style reset.
I wrote this ages ago and recently picked it back up because of a
recent PCI related reset ordering problem noted by Peter Xu. I'm not
sure if this patchset is necessary as a part of fixing that ordering
problem (it might even be possible now to have the intel_iommu device
use 3-phase reset and put the relevant parts of its reset into the
'exit' phase), but either way we really ought to do this cleanup
to reduce the amount of legacy/transitional handling we have.
In theory this patchset should be fine and shouldn't be changing
behaviour. On the other hand the reason I never sent it out when I
originally wrote it was that I ran into a test failure in the
BootLinuxConsole.test_sh4_r2d avocado test. Rerunning all the
avocado tests I don't see that failing now, so maybe I was just
confused by a flaky test back then.
In any case, this could probably use a thorough soak testing with
workloads that do resets of the PCI bus; I've only done 'make check'
and 'make check-avocado' on it. But I wanted to get it out onto the
list anyway.
thanks
-- PMM
Peter Maydell (5):
pci: Switch bus reset to 3-phase-reset
vmbus: Switch bus reset to 3-phase-reset
adb: Switch bus reset to 3-phase-reset
hw/s390x/css-bridge: switch virtual-css bus to 3-phase-reset
hw/core: Remove transitional infrastructure from BusClass
include/hw/qdev-core.h | 2 --
hw/core/bus.c | 67 ------------------------------------------
hw/hyperv/vmbus.c | 7 +++--
hw/input/adb.c | 7 +++--
hw/pci/pci.c | 10 ++++---
hw/s390x/css-bridge.c | 5 ++--
6 files changed, 17 insertions(+), 81 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/5] pci: Switch bus reset to 3-phase-reset
2024-01-19 16:35 [PATCH 0/5] buses: switch to 3-phase-reset Peter Maydell
@ 2024-01-19 16:35 ` Peter Maydell
2024-01-31 3:52 ` Zhao Liu
2024-01-19 16:35 ` [PATCH 2/5] vmbus: " Peter Maydell
` (6 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Peter Maydell @ 2024-01-19 16:35 UTC (permalink / raw)
To: qemu-devel
Cc: Maciej S. Szmigiero, Mark Cave-Ayland, Michael S. Tsirkin,
Marcel Apfelbaum, Halil Pasic, Christian Borntraeger,
Eric Farman, Peter Xu, Philippe Mathieu-Daudé
Switch the PCI bus from using BusClass::reset to the Resettable
interface.
This has no behavioural change, because the BusClass code to support
subclasses that use the legacy BusClass::reset will call that method
in the hold phase of 3-phase reset.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/pci/pci.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 76080af580d..05c2e46bda5 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -64,7 +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 void pcibus_reset_hold(Object *obj);
static bool pcie_has_upstream_port(PCIDevice *dev);
static Property pci_props[] = {
@@ -202,13 +202,15 @@ static void pci_bus_class_init(ObjectClass *klass, void *data)
{
BusClass *k = BUS_CLASS(klass);
PCIBusClass *pbc = PCI_BUS_CLASS(klass);
+ ResettableClass *rc = RESETTABLE_CLASS(klass);
k->print_dev = pcibus_dev_print;
k->get_dev_path = pcibus_get_dev_path;
k->get_fw_dev_path = pcibus_get_fw_dev_path;
k->realize = pci_bus_realize;
k->unrealize = pci_bus_unrealize;
- k->reset = pcibus_reset;
+
+ rc->phases.hold = pcibus_reset_hold;
pbc->bus_num = pcibus_num;
pbc->numa_node = pcibus_numa_node;
@@ -424,9 +426,9 @@ void pci_device_reset(PCIDevice *dev)
* Called via bus_cold_reset on RST# assert, after the devices
* have been reset device_cold_reset-ed already.
*/
-static void pcibus_reset(BusState *qbus)
+static void pcibus_reset_hold(Object *obj)
{
- PCIBus *bus = DO_UPCAST(PCIBus, qbus, qbus);
+ PCIBus *bus = PCI_BUS(obj);
int i;
for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/5] vmbus: Switch bus reset to 3-phase-reset
2024-01-19 16:35 [PATCH 0/5] buses: switch to 3-phase-reset Peter Maydell
2024-01-19 16:35 ` [PATCH 1/5] pci: Switch bus reset " Peter Maydell
@ 2024-01-19 16:35 ` Peter Maydell
2024-01-22 14:30 ` Maciej S. Szmigiero
2024-01-31 3:53 ` Zhao Liu
2024-01-19 16:35 ` [PATCH 3/5] adb: " Peter Maydell
` (5 subsequent siblings)
7 siblings, 2 replies; 21+ messages in thread
From: Peter Maydell @ 2024-01-19 16:35 UTC (permalink / raw)
To: qemu-devel
Cc: Maciej S. Szmigiero, Mark Cave-Ayland, Michael S. Tsirkin,
Marcel Apfelbaum, Halil Pasic, Christian Borntraeger,
Eric Farman, Peter Xu, Philippe Mathieu-Daudé
Switch vmbus from using BusClass::reset to the Resettable interface.
This has no behavioural change, because the BusClass code to support
subclasses that use the legacy BusClass::reset will call that method
in the hold phase of 3-phase reset.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/hyperv/vmbus.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/hyperv/vmbus.c b/hw/hyperv/vmbus.c
index c86d1895bae..380239af2c7 100644
--- a/hw/hyperv/vmbus.c
+++ b/hw/hyperv/vmbus.c
@@ -2453,9 +2453,9 @@ static void vmbus_unrealize(BusState *bus)
qemu_mutex_destroy(&vmbus->rx_queue_lock);
}
-static void vmbus_reset(BusState *bus)
+static void vmbus_reset_hold(Object *obj)
{
- vmbus_deinit(VMBUS(bus));
+ vmbus_deinit(VMBUS(obj));
}
static char *vmbus_get_dev_path(DeviceState *dev)
@@ -2476,12 +2476,13 @@ static char *vmbus_get_fw_dev_path(DeviceState *dev)
static void vmbus_class_init(ObjectClass *klass, void *data)
{
BusClass *k = BUS_CLASS(klass);
+ ResettableClass *rc = RESETTABLE_CLASS(klass);
k->get_dev_path = vmbus_get_dev_path;
k->get_fw_dev_path = vmbus_get_fw_dev_path;
k->realize = vmbus_realize;
k->unrealize = vmbus_unrealize;
- k->reset = vmbus_reset;
+ rc->phases.hold = vmbus_reset_hold;
}
static int vmbus_pre_load(void *opaque)
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/5] adb: Switch bus reset to 3-phase-reset
2024-01-19 16:35 [PATCH 0/5] buses: switch to 3-phase-reset Peter Maydell
2024-01-19 16:35 ` [PATCH 1/5] pci: Switch bus reset " Peter Maydell
2024-01-19 16:35 ` [PATCH 2/5] vmbus: " Peter Maydell
@ 2024-01-19 16:35 ` Peter Maydell
2024-01-29 21:42 ` Mark Cave-Ayland
2024-01-31 3:53 ` Zhao Liu
2024-01-19 16:35 ` [PATCH 4/5] hw/s390x/css-bridge: switch virtual-css bus " Peter Maydell
` (4 subsequent siblings)
7 siblings, 2 replies; 21+ messages in thread
From: Peter Maydell @ 2024-01-19 16:35 UTC (permalink / raw)
To: qemu-devel
Cc: Maciej S. Szmigiero, Mark Cave-Ayland, Michael S. Tsirkin,
Marcel Apfelbaum, Halil Pasic, Christian Borntraeger,
Eric Farman, Peter Xu, Philippe Mathieu-Daudé
Switch the ADB bus from using BusClass::reset to the Resettable
interface.
This has no behavioural change, because the BusClass code to support
subclasses that use the legacy BusClass::reset will call that method
in the hold phase of 3-phase reset.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/input/adb.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/input/adb.c b/hw/input/adb.c
index 0f3c73d6d00..98f39b4281a 100644
--- a/hw/input/adb.c
+++ b/hw/input/adb.c
@@ -231,9 +231,9 @@ static const VMStateDescription vmstate_adb_bus = {
}
};
-static void adb_bus_reset(BusState *qbus)
+static void adb_bus_reset_hold(Object *obj)
{
- ADBBusState *adb_bus = ADB_BUS(qbus);
+ ADBBusState *adb_bus = ADB_BUS(obj);
adb_bus->autopoll_enabled = false;
adb_bus->autopoll_mask = 0xffff;
@@ -262,10 +262,11 @@ static void adb_bus_unrealize(BusState *qbus)
static void adb_bus_class_init(ObjectClass *klass, void *data)
{
BusClass *k = BUS_CLASS(klass);
+ ResettableClass *rc = RESETTABLE_CLASS(klass);
k->realize = adb_bus_realize;
k->unrealize = adb_bus_unrealize;
- k->reset = adb_bus_reset;
+ rc->phases.hold = adb_bus_reset_hold;
}
static const TypeInfo adb_bus_type_info = {
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/5] hw/s390x/css-bridge: switch virtual-css bus to 3-phase-reset
2024-01-19 16:35 [PATCH 0/5] buses: switch to 3-phase-reset Peter Maydell
` (2 preceding siblings ...)
2024-01-19 16:35 ` [PATCH 3/5] adb: " Peter Maydell
@ 2024-01-19 16:35 ` Peter Maydell
2024-01-22 9:18 ` Halil Pasic
` (2 more replies)
2024-01-19 16:35 ` [PATCH 5/5] hw/core: Remove transitional infrastructure from BusClass Peter Maydell
` (3 subsequent siblings)
7 siblings, 3 replies; 21+ messages in thread
From: Peter Maydell @ 2024-01-19 16:35 UTC (permalink / raw)
To: qemu-devel
Cc: Maciej S. Szmigiero, Mark Cave-Ayland, Michael S. Tsirkin,
Marcel Apfelbaum, Halil Pasic, Christian Borntraeger,
Eric Farman, Peter Xu, Philippe Mathieu-Daudé
Switch the s390x virtual-css bus from using BusClass::reset to the
Resettable interface.
This has no behavioural change, because the BusClass code to support
subclasses that use the legacy BusClass::reset will call that method
in the hold phase of 3-phase reset.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/s390x/css-bridge.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/s390x/css-bridge.c b/hw/s390x/css-bridge.c
index 15d26efc951..34639f21435 100644
--- a/hw/s390x/css-bridge.c
+++ b/hw/s390x/css-bridge.c
@@ -56,7 +56,7 @@ static void ccw_device_unplug(HotplugHandler *hotplug_dev,
qdev_unrealize(dev);
}
-static void virtual_css_bus_reset(BusState *qbus)
+static void virtual_css_bus_reset_hold(Object *obj)
{
/* This should actually be modelled via the generic css */
css_reset();
@@ -81,8 +81,9 @@ static char *virtual_css_bus_get_dev_path(DeviceState *dev)
static void virtual_css_bus_class_init(ObjectClass *klass, void *data)
{
BusClass *k = BUS_CLASS(klass);
+ ResettableClass *rc = RESETTABLE_CLASS(klass);
- k->reset = virtual_css_bus_reset;
+ rc->phases.hold = virtual_css_bus_reset_hold;
k->get_dev_path = virtual_css_bus_get_dev_path;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 5/5] hw/core: Remove transitional infrastructure from BusClass
2024-01-19 16:35 [PATCH 0/5] buses: switch to 3-phase-reset Peter Maydell
` (3 preceding siblings ...)
2024-01-19 16:35 ` [PATCH 4/5] hw/s390x/css-bridge: switch virtual-css bus " Peter Maydell
@ 2024-01-19 16:35 ` Peter Maydell
2024-01-31 3:58 ` Zhao Liu
2024-01-21 11:37 ` [PATCH 0/5] buses: switch to 3-phase-reset Michael S. Tsirkin
` (2 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Peter Maydell @ 2024-01-19 16:35 UTC (permalink / raw)
To: qemu-devel
Cc: Maciej S. Szmigiero, Mark Cave-Ayland, Michael S. Tsirkin,
Marcel Apfelbaum, Halil Pasic, Christian Borntraeger,
Eric Farman, Peter Xu, Philippe Mathieu-Daudé
BusClass currently has transitional infrastructure to support
subclasses which implement the legacy BusClass::reset method rather
than the Resettable interface. We have now removed all the users of
BusClass::reset in the tree, so we can remove the transitional
infrastructure.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
include/hw/qdev-core.h | 2 --
hw/core/bus.c | 67 ------------------------------------------
2 files changed, 69 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 151d9682380..986c924fa55 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -329,8 +329,6 @@ struct BusClass {
*/
char *(*get_fw_dev_path)(DeviceState *dev);
- void (*reset)(BusState *bus);
-
/*
* Return whether the device can be added to @bus,
* based on the address that was set (via device properties)
diff --git a/hw/core/bus.c b/hw/core/bus.c
index c7831b5293b..b9d89495cdf 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -232,57 +232,6 @@ static char *default_bus_get_fw_dev_path(DeviceState *dev)
return g_strdup(object_get_typename(OBJECT(dev)));
}
-/**
- * bus_phases_reset:
- * Transition reset method for buses to allow moving
- * smoothly from legacy reset method to multi-phases
- */
-static void bus_phases_reset(BusState *bus)
-{
- ResettableClass *rc = RESETTABLE_GET_CLASS(bus);
-
- if (rc->phases.enter) {
- rc->phases.enter(OBJECT(bus), RESET_TYPE_COLD);
- }
- if (rc->phases.hold) {
- rc->phases.hold(OBJECT(bus));
- }
- if (rc->phases.exit) {
- rc->phases.exit(OBJECT(bus));
- }
-}
-
-static void bus_transitional_reset(Object *obj)
-{
- BusClass *bc = BUS_GET_CLASS(obj);
-
- /*
- * This will call either @bus_phases_reset (for multi-phases transitioned
- * buses) or a bus's specific method for not-yet transitioned buses.
- * In both case, it does not reset children.
- */
- if (bc->reset) {
- bc->reset(BUS(obj));
- }
-}
-
-/**
- * bus_get_transitional_reset:
- * check if the bus's class is ready for multi-phase
- */
-static ResettableTrFunction bus_get_transitional_reset(Object *obj)
-{
- BusClass *dc = BUS_GET_CLASS(obj);
- if (dc->reset != bus_phases_reset) {
- /*
- * dc->reset has been overridden by a subclass,
- * the bus is not ready for multi phase yet.
- */
- return bus_transitional_reset;
- }
- return NULL;
-}
-
static void bus_class_init(ObjectClass *class, void *data)
{
BusClass *bc = BUS_CLASS(class);
@@ -293,22 +242,6 @@ static void bus_class_init(ObjectClass *class, void *data)
rc->get_state = bus_get_reset_state;
rc->child_foreach = bus_reset_child_foreach;
-
- /*
- * @bus_phases_reset is put as the default reset method below, allowing
- * to do the multi-phase transition from base classes to leaf classes. It
- * allows a legacy-reset Bus class to extend a multi-phases-reset
- * Bus class for the following reason:
- * + If a base class B has been moved to multi-phase, then it does not
- * override this default reset method and may have defined phase methods.
- * + A child class C (extending class B) which uses
- * bus_class_set_parent_reset() (or similar means) to override the
- * reset method will still work as expected. @bus_phases_reset function
- * will be registered as the parent reset method and effectively call
- * parent reset phases.
- */
- bc->reset = bus_phases_reset;
- rc->get_transitional_function = bus_get_transitional_reset;
}
static void qbus_finalize(Object *obj)
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 0/5] buses: switch to 3-phase-reset
2024-01-19 16:35 [PATCH 0/5] buses: switch to 3-phase-reset Peter Maydell
` (4 preceding siblings ...)
2024-01-19 16:35 ` [PATCH 5/5] hw/core: Remove transitional infrastructure from BusClass Peter Maydell
@ 2024-01-21 11:37 ` Michael S. Tsirkin
2024-01-22 2:06 ` Peter Xu
2024-02-01 13:20 ` Peter Maydell
7 siblings, 0 replies; 21+ messages in thread
From: Michael S. Tsirkin @ 2024-01-21 11:37 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, Maciej S. Szmigiero, Mark Cave-Ayland,
Marcel Apfelbaum, Halil Pasic, Christian Borntraeger,
Eric Farman, Peter Xu, Philippe Mathieu-Daudé
On Fri, Jan 19, 2024 at 04:35:07PM +0000, Peter Maydell wrote:
> This patchset switches the handful of bus types that implement a
> reset method over to using the 3-phase-reset APIs, and then removes
> the transitional infrastructure from the core bus class that was
> supporting the ability to have bus types that use old-style reset.
>
> I wrote this ages ago and recently picked it back up because of a
> recent PCI related reset ordering problem noted by Peter Xu. I'm not
> sure if this patchset is necessary as a part of fixing that ordering
> problem (it might even be possible now to have the intel_iommu device
> use 3-phase reset and put the relevant parts of its reset into the
> 'exit' phase), but either way we really ought to do this cleanup
> to reduce the amount of legacy/transitional handling we have.
>
> In theory this patchset should be fine and shouldn't be changing
> behaviour. On the other hand the reason I never sent it out when I
> originally wrote it was that I ran into a test failure in the
> BootLinuxConsole.test_sh4_r2d avocado test. Rerunning all the
> avocado tests I don't see that failing now, so maybe I was just
> confused by a flaky test back then.
>
> In any case, this could probably use a thorough soak testing with
> workloads that do resets of the PCI bus; I've only done 'make check'
> and 'make check-avocado' on it. But I wanted to get it out onto the
> list anyway.
>
> thanks
> -- PMM
From a quick look, we need this cleanup
Acked-by: Michael S. Tsirkin <mst@redhat.com>
I'll try some tests too, and report.
> Peter Maydell (5):
> pci: Switch bus reset to 3-phase-reset
> vmbus: Switch bus reset to 3-phase-reset
> adb: Switch bus reset to 3-phase-reset
> hw/s390x/css-bridge: switch virtual-css bus to 3-phase-reset
> hw/core: Remove transitional infrastructure from BusClass
>
> include/hw/qdev-core.h | 2 --
> hw/core/bus.c | 67 ------------------------------------------
> hw/hyperv/vmbus.c | 7 +++--
> hw/input/adb.c | 7 +++--
> hw/pci/pci.c | 10 ++++---
> hw/s390x/css-bridge.c | 5 ++--
> 6 files changed, 17 insertions(+), 81 deletions(-)
>
> --
> 2.34.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/5] buses: switch to 3-phase-reset
2024-01-19 16:35 [PATCH 0/5] buses: switch to 3-phase-reset Peter Maydell
` (5 preceding siblings ...)
2024-01-21 11:37 ` [PATCH 0/5] buses: switch to 3-phase-reset Michael S. Tsirkin
@ 2024-01-22 2:06 ` Peter Xu
2024-01-22 14:19 ` Cédric Le Goater
2024-02-01 13:20 ` Peter Maydell
7 siblings, 1 reply; 21+ messages in thread
From: Peter Xu @ 2024-01-22 2:06 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, Maciej S. Szmigiero, Mark Cave-Ayland,
Michael S. Tsirkin, Marcel Apfelbaum, Halil Pasic,
Christian Borntraeger, Eric Farman, Philippe Mathieu-Daudé,
Alex Williamson, Cédric Le Goater
Hi, Peter,
On Fri, Jan 19, 2024 at 04:35:07PM +0000, Peter Maydell wrote:
> I wrote this ages ago and recently picked it back up because of a
> recent PCI related reset ordering problem noted by Peter Xu. I'm not
> sure if this patchset is necessary as a part of fixing that ordering
> problem (it might even be possible now to have the intel_iommu device
> use 3-phase reset and put the relevant parts of its reset into the
> 'exit' phase), but either way we really ought to do this cleanup
> to reduce the amount of legacy/transitional handling we have.
The VFIO issue I was working on may not directly benefit from this series
iiuc, as it's more of an special ordering on both (1) VFIO special case
reset path using qemu_register_reset(), and (2) VT-d device is not put at
the right place in the QOM hierachy [1].
Said that, thanks a lot for posting the patches; they all look reasonable
and good cleanups to the reset infrastructure, afaict.
[1] https://lore.kernel.org/r/ZapYii9nr5Tj9ClE@x1n
--
Peter Xu
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/5] hw/s390x/css-bridge: switch virtual-css bus to 3-phase-reset
2024-01-19 16:35 ` [PATCH 4/5] hw/s390x/css-bridge: switch virtual-css bus " Peter Maydell
@ 2024-01-22 9:18 ` Halil Pasic
2024-01-22 21:49 ` Eric Farman
2024-01-31 3:54 ` Zhao Liu
2 siblings, 0 replies; 21+ messages in thread
From: Halil Pasic @ 2024-01-22 9:18 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, Maciej S. Szmigiero, Mark Cave-Ayland,
Michael S. Tsirkin, Marcel Apfelbaum, Christian Borntraeger,
Eric Farman, Peter Xu, Philippe Mathieu-Daudé,
Halil Pasic
On Fri, 19 Jan 2024 16:35:11 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:
> Switch the s390x virtual-css bus from using BusClass::reset to the
> Resettable interface.
>
> This has no behavioural change, because the BusClass code to support
> subclasses that use the legacy BusClass::reset will call that method
> in the hold phase of 3-phase reset.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Halil Pasic <pasic@linux.ibm.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/5] buses: switch to 3-phase-reset
2024-01-22 2:06 ` Peter Xu
@ 2024-01-22 14:19 ` Cédric Le Goater
2024-01-31 17:36 ` Cédric Le Goater
0 siblings, 1 reply; 21+ messages in thread
From: Cédric Le Goater @ 2024-01-22 14:19 UTC (permalink / raw)
To: Peter Xu, Peter Maydell
Cc: qemu-devel, Maciej S. Szmigiero, Mark Cave-Ayland,
Michael S. Tsirkin, Marcel Apfelbaum, Halil Pasic,
Christian Borntraeger, Eric Farman, Philippe Mathieu-Daudé,
Alex Williamson
Hello,
On 1/22/24 03:06, Peter Xu wrote:
> Hi, Peter,
>
> On Fri, Jan 19, 2024 at 04:35:07PM +0000, Peter Maydell wrote:
>> I wrote this ages ago and recently picked it back up because of a
>> recent PCI related reset ordering problem noted by Peter Xu. I'm not
>> sure if this patchset is necessary as a part of fixing that ordering
>> problem (it might even be possible now to have the intel_iommu device
>> use 3-phase reset and put the relevant parts of its reset into the
>> 'exit' phase), but either way we really ought to do this cleanup
>> to reduce the amount of legacy/transitional handling we have.
>
> The VFIO issue I was working on may not directly benefit from this series
> iiuc, as it's more of an special ordering on both (1) VFIO special case
> reset path using qemu_register_reset(), and (2) VT-d device is not put at
> the right place in the QOM hierachy [1].
>
> Said that, thanks a lot for posting the patches; they all look reasonable
> and good cleanups to the reset infrastructure, afaict.
Yes. I took the series in my vfio testing environment (x86_64 and s390x) and
didn't see any issue. I will keep it for further testing.
Thanks,
C.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] vmbus: Switch bus reset to 3-phase-reset
2024-01-19 16:35 ` [PATCH 2/5] vmbus: " Peter Maydell
@ 2024-01-22 14:30 ` Maciej S. Szmigiero
2024-01-31 3:53 ` Zhao Liu
1 sibling, 0 replies; 21+ messages in thread
From: Maciej S. Szmigiero @ 2024-01-22 14:30 UTC (permalink / raw)
To: Peter Maydell
Cc: Mark Cave-Ayland, Michael S. Tsirkin, Marcel Apfelbaum,
Halil Pasic, Christian Borntraeger, Eric Farman, Peter Xu,
Philippe Mathieu-Daudé,
qemu-devel
On 19.01.2024 17:35, Peter Maydell wrote:
> Switch vmbus from using BusClass::reset to the Resettable interface.
>
> This has no behavioural change, because the BusClass code to support
> subclasses that use the legacy BusClass::reset will call that method
> in the hold phase of 3-phase reset.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
Acked-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
Thanks,
Maciej
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/5] hw/s390x/css-bridge: switch virtual-css bus to 3-phase-reset
2024-01-19 16:35 ` [PATCH 4/5] hw/s390x/css-bridge: switch virtual-css bus " Peter Maydell
2024-01-22 9:18 ` Halil Pasic
@ 2024-01-22 21:49 ` Eric Farman
2024-01-31 3:54 ` Zhao Liu
2 siblings, 0 replies; 21+ messages in thread
From: Eric Farman @ 2024-01-22 21:49 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Maciej S. Szmigiero, Mark Cave-Ayland, Michael S. Tsirkin,
Marcel Apfelbaum, Halil Pasic, Christian Borntraeger, Peter Xu,
Philippe Mathieu-Daudé
On Fri, 2024-01-19 at 16:35 +0000, Peter Maydell wrote:
> Switch the s390x virtual-css bus from using BusClass::reset to the
> Resettable interface.
>
> This has no behavioural change, because the BusClass code to support
> subclasses that use the legacy BusClass::reset will call that method
> in the hold phase of 3-phase reset.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/s390x/css-bridge.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
Reviewed-by: Eric Farman <farman@linux.ibm.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/5] adb: Switch bus reset to 3-phase-reset
2024-01-19 16:35 ` [PATCH 3/5] adb: " Peter Maydell
@ 2024-01-29 21:42 ` Mark Cave-Ayland
2024-01-31 3:53 ` Zhao Liu
1 sibling, 0 replies; 21+ messages in thread
From: Mark Cave-Ayland @ 2024-01-29 21:42 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Maciej S. Szmigiero, Michael S. Tsirkin, Marcel Apfelbaum,
Halil Pasic, Christian Borntraeger, Eric Farman, Peter Xu,
Philippe Mathieu-Daudé
On 19/01/2024 16:35, Peter Maydell wrote:
> Switch the ADB bus from using BusClass::reset to the Resettable
> interface.
>
> This has no behavioural change, because the BusClass code to support
> subclasses that use the legacy BusClass::reset will call that method
> in the hold phase of 3-phase reset.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/input/adb.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/hw/input/adb.c b/hw/input/adb.c
> index 0f3c73d6d00..98f39b4281a 100644
> --- a/hw/input/adb.c
> +++ b/hw/input/adb.c
> @@ -231,9 +231,9 @@ static const VMStateDescription vmstate_adb_bus = {
> }
> };
>
> -static void adb_bus_reset(BusState *qbus)
> +static void adb_bus_reset_hold(Object *obj)
> {
> - ADBBusState *adb_bus = ADB_BUS(qbus);
> + ADBBusState *adb_bus = ADB_BUS(obj);
>
> adb_bus->autopoll_enabled = false;
> adb_bus->autopoll_mask = 0xffff;
> @@ -262,10 +262,11 @@ static void adb_bus_unrealize(BusState *qbus)
> static void adb_bus_class_init(ObjectClass *klass, void *data)
> {
> BusClass *k = BUS_CLASS(klass);
> + ResettableClass *rc = RESETTABLE_CLASS(klass);
>
> k->realize = adb_bus_realize;
> k->unrealize = adb_bus_unrealize;
> - k->reset = adb_bus_reset;
> + rc->phases.hold = adb_bus_reset_hold;
> }
>
> static const TypeInfo adb_bus_type_info = {
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
ATB,
Mark.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/5] pci: Switch bus reset to 3-phase-reset
2024-01-19 16:35 ` [PATCH 1/5] pci: Switch bus reset " Peter Maydell
@ 2024-01-31 3:52 ` Zhao Liu
0 siblings, 0 replies; 21+ messages in thread
From: Zhao Liu @ 2024-01-31 3:52 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, Maciej S. Szmigiero, Mark Cave-Ayland,
Michael S. Tsirkin, Marcel Apfelbaum, Halil Pasic,
Christian Borntraeger, Eric Farman, Peter Xu,
Philippe Mathieu-Daudé
On Fri, Jan 19, 2024 at 04:35:08PM +0000, Peter Maydell wrote:
> Date: Fri, 19 Jan 2024 16:35:08 +0000
> From: Peter Maydell <peter.maydell@linaro.org>
> Subject: [PATCH 1/5] pci: Switch bus reset to 3-phase-reset
> X-Mailer: git-send-email 2.34.1
>
> Switch the PCI bus from using BusClass::reset to the Resettable
> interface.
>
> This has no behavioural change, because the BusClass code to support
> subclasses that use the legacy BusClass::reset will call that method
> in the hold phase of 3-phase reset.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/pci/pci.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 76080af580d..05c2e46bda5 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -64,7 +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 void pcibus_reset_hold(Object *obj);
> static bool pcie_has_upstream_port(PCIDevice *dev);
>
> static Property pci_props[] = {
> @@ -202,13 +202,15 @@ static void pci_bus_class_init(ObjectClass *klass, void *data)
> {
> BusClass *k = BUS_CLASS(klass);
> PCIBusClass *pbc = PCI_BUS_CLASS(klass);
> + ResettableClass *rc = RESETTABLE_CLASS(klass);
>
> k->print_dev = pcibus_dev_print;
> k->get_dev_path = pcibus_get_dev_path;
> k->get_fw_dev_path = pcibus_get_fw_dev_path;
> k->realize = pci_bus_realize;
> k->unrealize = pci_bus_unrealize;
> - k->reset = pcibus_reset;
> +
> + rc->phases.hold = pcibus_reset_hold;
>
> pbc->bus_num = pcibus_num;
> pbc->numa_node = pcibus_numa_node;
> @@ -424,9 +426,9 @@ void pci_device_reset(PCIDevice *dev)
> * Called via bus_cold_reset on RST# assert, after the devices
> * have been reset device_cold_reset-ed already.
> */
> -static void pcibus_reset(BusState *qbus)
> +static void pcibus_reset_hold(Object *obj)
> {
> - PCIBus *bus = DO_UPCAST(PCIBus, qbus, qbus);
> + PCIBus *bus = PCI_BUS(obj);
> int i;
>
> for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] vmbus: Switch bus reset to 3-phase-reset
2024-01-19 16:35 ` [PATCH 2/5] vmbus: " Peter Maydell
2024-01-22 14:30 ` Maciej S. Szmigiero
@ 2024-01-31 3:53 ` Zhao Liu
1 sibling, 0 replies; 21+ messages in thread
From: Zhao Liu @ 2024-01-31 3:53 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, Maciej S. Szmigiero, Mark Cave-Ayland,
Michael S. Tsirkin, Marcel Apfelbaum, Halil Pasic,
Christian Borntraeger, Eric Farman, Peter Xu,
Philippe Mathieu-Daudé
On Fri, Jan 19, 2024 at 04:35:09PM +0000, Peter Maydell wrote:
> Date: Fri, 19 Jan 2024 16:35:09 +0000
> From: Peter Maydell <peter.maydell@linaro.org>
> Subject: [PATCH 2/5] vmbus: Switch bus reset to 3-phase-reset
> X-Mailer: git-send-email 2.34.1
>
> Switch vmbus from using BusClass::reset to the Resettable interface.
>
> This has no behavioural change, because the BusClass code to support
> subclasses that use the legacy BusClass::reset will call that method
> in the hold phase of 3-phase reset.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/hyperv/vmbus.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
>
> diff --git a/hw/hyperv/vmbus.c b/hw/hyperv/vmbus.c
> index c86d1895bae..380239af2c7 100644
> --- a/hw/hyperv/vmbus.c
> +++ b/hw/hyperv/vmbus.c
> @@ -2453,9 +2453,9 @@ static void vmbus_unrealize(BusState *bus)
> qemu_mutex_destroy(&vmbus->rx_queue_lock);
> }
>
> -static void vmbus_reset(BusState *bus)
> +static void vmbus_reset_hold(Object *obj)
> {
> - vmbus_deinit(VMBUS(bus));
> + vmbus_deinit(VMBUS(obj));
> }
>
> static char *vmbus_get_dev_path(DeviceState *dev)
> @@ -2476,12 +2476,13 @@ static char *vmbus_get_fw_dev_path(DeviceState *dev)
> static void vmbus_class_init(ObjectClass *klass, void *data)
> {
> BusClass *k = BUS_CLASS(klass);
> + ResettableClass *rc = RESETTABLE_CLASS(klass);
>
> k->get_dev_path = vmbus_get_dev_path;
> k->get_fw_dev_path = vmbus_get_fw_dev_path;
> k->realize = vmbus_realize;
> k->unrealize = vmbus_unrealize;
> - k->reset = vmbus_reset;
> + rc->phases.hold = vmbus_reset_hold;
> }
>
> static int vmbus_pre_load(void *opaque)
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/5] adb: Switch bus reset to 3-phase-reset
2024-01-19 16:35 ` [PATCH 3/5] adb: " Peter Maydell
2024-01-29 21:42 ` Mark Cave-Ayland
@ 2024-01-31 3:53 ` Zhao Liu
1 sibling, 0 replies; 21+ messages in thread
From: Zhao Liu @ 2024-01-31 3:53 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, Maciej S. Szmigiero, Mark Cave-Ayland,
Michael S. Tsirkin, Marcel Apfelbaum, Halil Pasic,
Christian Borntraeger, Eric Farman, Peter Xu,
Philippe Mathieu-Daudé
On Fri, Jan 19, 2024 at 04:35:10PM +0000, Peter Maydell wrote:
> Date: Fri, 19 Jan 2024 16:35:10 +0000
> From: Peter Maydell <peter.maydell@linaro.org>
> Subject: [PATCH 3/5] adb: Switch bus reset to 3-phase-reset
> X-Mailer: git-send-email 2.34.1
>
> Switch the ADB bus from using BusClass::reset to the Resettable
> interface.
>
> This has no behavioural change, because the BusClass code to support
> subclasses that use the legacy BusClass::reset will call that method
> in the hold phase of 3-phase reset.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/input/adb.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
>
> diff --git a/hw/input/adb.c b/hw/input/adb.c
> index 0f3c73d6d00..98f39b4281a 100644
> --- a/hw/input/adb.c
> +++ b/hw/input/adb.c
> @@ -231,9 +231,9 @@ static const VMStateDescription vmstate_adb_bus = {
> }
> };
>
> -static void adb_bus_reset(BusState *qbus)
> +static void adb_bus_reset_hold(Object *obj)
> {
> - ADBBusState *adb_bus = ADB_BUS(qbus);
> + ADBBusState *adb_bus = ADB_BUS(obj);
>
> adb_bus->autopoll_enabled = false;
> adb_bus->autopoll_mask = 0xffff;
> @@ -262,10 +262,11 @@ static void adb_bus_unrealize(BusState *qbus)
> static void adb_bus_class_init(ObjectClass *klass, void *data)
> {
> BusClass *k = BUS_CLASS(klass);
> + ResettableClass *rc = RESETTABLE_CLASS(klass);
>
> k->realize = adb_bus_realize;
> k->unrealize = adb_bus_unrealize;
> - k->reset = adb_bus_reset;
> + rc->phases.hold = adb_bus_reset_hold;
> }
>
> static const TypeInfo adb_bus_type_info = {
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/5] hw/s390x/css-bridge: switch virtual-css bus to 3-phase-reset
2024-01-19 16:35 ` [PATCH 4/5] hw/s390x/css-bridge: switch virtual-css bus " Peter Maydell
2024-01-22 9:18 ` Halil Pasic
2024-01-22 21:49 ` Eric Farman
@ 2024-01-31 3:54 ` Zhao Liu
2 siblings, 0 replies; 21+ messages in thread
From: Zhao Liu @ 2024-01-31 3:54 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, Maciej S. Szmigiero, Mark Cave-Ayland,
Michael S. Tsirkin, Marcel Apfelbaum, Halil Pasic,
Christian Borntraeger, Eric Farman, Peter Xu,
Philippe Mathieu-Daudé
On Fri, Jan 19, 2024 at 04:35:11PM +0000, Peter Maydell wrote:
> Date: Fri, 19 Jan 2024 16:35:11 +0000
> From: Peter Maydell <peter.maydell@linaro.org>
> Subject: [PATCH 4/5] hw/s390x/css-bridge: switch virtual-css bus to
> 3-phase-reset
> X-Mailer: git-send-email 2.34.1
>
> Switch the s390x virtual-css bus from using BusClass::reset to the
> Resettable interface.
>
> This has no behavioural change, because the BusClass code to support
> subclasses that use the legacy BusClass::reset will call that method
> in the hold phase of 3-phase reset.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/s390x/css-bridge.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
>
> diff --git a/hw/s390x/css-bridge.c b/hw/s390x/css-bridge.c
> index 15d26efc951..34639f21435 100644
> --- a/hw/s390x/css-bridge.c
> +++ b/hw/s390x/css-bridge.c
> @@ -56,7 +56,7 @@ static void ccw_device_unplug(HotplugHandler *hotplug_dev,
> qdev_unrealize(dev);
> }
>
> -static void virtual_css_bus_reset(BusState *qbus)
> +static void virtual_css_bus_reset_hold(Object *obj)
> {
> /* This should actually be modelled via the generic css */
> css_reset();
> @@ -81,8 +81,9 @@ static char *virtual_css_bus_get_dev_path(DeviceState *dev)
> static void virtual_css_bus_class_init(ObjectClass *klass, void *data)
> {
> BusClass *k = BUS_CLASS(klass);
> + ResettableClass *rc = RESETTABLE_CLASS(klass);
>
> - k->reset = virtual_css_bus_reset;
> + rc->phases.hold = virtual_css_bus_reset_hold;
> k->get_dev_path = virtual_css_bus_get_dev_path;
> }
>
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/5] hw/core: Remove transitional infrastructure from BusClass
2024-01-19 16:35 ` [PATCH 5/5] hw/core: Remove transitional infrastructure from BusClass Peter Maydell
@ 2024-01-31 3:58 ` Zhao Liu
2024-02-01 13:31 ` Peter Maydell
0 siblings, 1 reply; 21+ messages in thread
From: Zhao Liu @ 2024-01-31 3:58 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, Maciej S. Szmigiero, Mark Cave-Ayland,
Michael S. Tsirkin, Marcel Apfelbaum, Halil Pasic,
Christian Borntraeger, Eric Farman, Peter Xu,
Philippe Mathieu-Daudé
On Fri, Jan 19, 2024 at 04:35:12PM +0000, Peter Maydell wrote:
> Date: Fri, 19 Jan 2024 16:35:12 +0000
> From: Peter Maydell <peter.maydell@linaro.org>
> Subject: [PATCH 5/5] hw/core: Remove transitional infrastructure from
> BusClass
> X-Mailer: git-send-email 2.34.1
>
> BusClass currently has transitional infrastructure to support
> subclasses which implement the legacy BusClass::reset method rather
> than the Resettable interface. We have now removed all the users of
> BusClass::reset in the tree, so we can remove the transitional
> infrastructure.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> include/hw/qdev-core.h | 2 --
> hw/core/bus.c | 67 ------------------------------------------
> 2 files changed, 69 deletions(-)
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
It seems the similar cleanup for DeviceClass needs a lot of effort.
>
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 151d9682380..986c924fa55 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -329,8 +329,6 @@ struct BusClass {
> */
> char *(*get_fw_dev_path)(DeviceState *dev);
>
> - void (*reset)(BusState *bus);
> -
> /*
> * Return whether the device can be added to @bus,
> * based on the address that was set (via device properties)
> diff --git a/hw/core/bus.c b/hw/core/bus.c
> index c7831b5293b..b9d89495cdf 100644
> --- a/hw/core/bus.c
> +++ b/hw/core/bus.c
> @@ -232,57 +232,6 @@ static char *default_bus_get_fw_dev_path(DeviceState *dev)
> return g_strdup(object_get_typename(OBJECT(dev)));
> }
>
> -/**
> - * bus_phases_reset:
> - * Transition reset method for buses to allow moving
> - * smoothly from legacy reset method to multi-phases
> - */
> -static void bus_phases_reset(BusState *bus)
> -{
> - ResettableClass *rc = RESETTABLE_GET_CLASS(bus);
> -
> - if (rc->phases.enter) {
> - rc->phases.enter(OBJECT(bus), RESET_TYPE_COLD);
> - }
> - if (rc->phases.hold) {
> - rc->phases.hold(OBJECT(bus));
> - }
> - if (rc->phases.exit) {
> - rc->phases.exit(OBJECT(bus));
> - }
> -}
> -
> -static void bus_transitional_reset(Object *obj)
> -{
> - BusClass *bc = BUS_GET_CLASS(obj);
> -
> - /*
> - * This will call either @bus_phases_reset (for multi-phases transitioned
> - * buses) or a bus's specific method for not-yet transitioned buses.
> - * In both case, it does not reset children.
> - */
> - if (bc->reset) {
> - bc->reset(BUS(obj));
> - }
> -}
> -
> -/**
> - * bus_get_transitional_reset:
> - * check if the bus's class is ready for multi-phase
> - */
> -static ResettableTrFunction bus_get_transitional_reset(Object *obj)
> -{
> - BusClass *dc = BUS_GET_CLASS(obj);
> - if (dc->reset != bus_phases_reset) {
> - /*
> - * dc->reset has been overridden by a subclass,
> - * the bus is not ready for multi phase yet.
> - */
> - return bus_transitional_reset;
> - }
> - return NULL;
> -}
> -
> static void bus_class_init(ObjectClass *class, void *data)
> {
> BusClass *bc = BUS_CLASS(class);
> @@ -293,22 +242,6 @@ static void bus_class_init(ObjectClass *class, void *data)
>
> rc->get_state = bus_get_reset_state;
> rc->child_foreach = bus_reset_child_foreach;
> -
> - /*
> - * @bus_phases_reset is put as the default reset method below, allowing
> - * to do the multi-phase transition from base classes to leaf classes. It
> - * allows a legacy-reset Bus class to extend a multi-phases-reset
> - * Bus class for the following reason:
> - * + If a base class B has been moved to multi-phase, then it does not
> - * override this default reset method and may have defined phase methods.
> - * + A child class C (extending class B) which uses
> - * bus_class_set_parent_reset() (or similar means) to override the
> - * reset method will still work as expected. @bus_phases_reset function
> - * will be registered as the parent reset method and effectively call
> - * parent reset phases.
> - */
> - bc->reset = bus_phases_reset;
> - rc->get_transitional_function = bus_get_transitional_reset;
> }
>
> static void qbus_finalize(Object *obj)
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/5] buses: switch to 3-phase-reset
2024-01-22 14:19 ` Cédric Le Goater
@ 2024-01-31 17:36 ` Cédric Le Goater
0 siblings, 0 replies; 21+ messages in thread
From: Cédric Le Goater @ 2024-01-31 17:36 UTC (permalink / raw)
To: Peter Xu, Peter Maydell
Cc: qemu-devel, Maciej S. Szmigiero, Mark Cave-Ayland,
Michael S. Tsirkin, Marcel Apfelbaum, Halil Pasic,
Christian Borntraeger, Eric Farman, Philippe Mathieu-Daudé,
Alex Williamson
On 1/22/24 15:19, Cédric Le Goater wrote:
> Hello,
>
> On 1/22/24 03:06, Peter Xu wrote:
>> Hi, Peter,
>>
>> On Fri, Jan 19, 2024 at 04:35:07PM +0000, Peter Maydell wrote:
>>> I wrote this ages ago and recently picked it back up because of a
>>> recent PCI related reset ordering problem noted by Peter Xu. I'm not
>>> sure if this patchset is necessary as a part of fixing that ordering
>>> problem (it might even be possible now to have the intel_iommu device
>>> use 3-phase reset and put the relevant parts of its reset into the
>>> 'exit' phase), but either way we really ought to do this cleanup
>>> to reduce the amount of legacy/transitional handling we have.
>>
>> The VFIO issue I was working on may not directly benefit from this series
>> iiuc, as it's more of an special ordering on both (1) VFIO special case
>> reset path using qemu_register_reset(), and (2) VT-d device is not put at
>> the right place in the QOM hierachy [1].
>>
>> Said that, thanks a lot for posting the patches; they all look reasonable
>> and good cleanups to the reset infrastructure, afaict.
>
>
> Yes. I took the series in my vfio testing environment (x86_64 and s390x) and
> didn't see any issue. I will keep it for further testing.
Acked-by: Cédric Le Goater <clg@redhat.com>
Tested-by: Cédric Le Goater <clg@redhat.com>
Thanks,
C.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/5] buses: switch to 3-phase-reset
2024-01-19 16:35 [PATCH 0/5] buses: switch to 3-phase-reset Peter Maydell
` (6 preceding siblings ...)
2024-01-22 2:06 ` Peter Xu
@ 2024-02-01 13:20 ` Peter Maydell
7 siblings, 0 replies; 21+ messages in thread
From: Peter Maydell @ 2024-02-01 13:20 UTC (permalink / raw)
To: qemu-devel
Cc: Maciej S. Szmigiero, Mark Cave-Ayland, Michael S. Tsirkin,
Marcel Apfelbaum, Halil Pasic, Christian Borntraeger,
Eric Farman, Peter Xu, Philippe Mathieu-Daudé
On Fri, 19 Jan 2024 at 16:35, Peter Maydell <peter.maydell@linaro.org> wrote:
> This patchset switches the handful of bus types that implement a
> reset method over to using the 3-phase-reset APIs, and then removes
> the transitional infrastructure from the core bus class that was
> supporting the ability to have bus types that use old-style reset.
Thanks all for the review and testing. It seems like there
weren't any negative reports, so I'm going to take this via
the target-arm.next queue. We're still early in the 9.0
release cycle so we have plenty of time for debugging or
(if necessary) reverting if any unexpected consequences
turn up...
-- PMM
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/5] hw/core: Remove transitional infrastructure from BusClass
2024-01-31 3:58 ` Zhao Liu
@ 2024-02-01 13:31 ` Peter Maydell
0 siblings, 0 replies; 21+ messages in thread
From: Peter Maydell @ 2024-02-01 13:31 UTC (permalink / raw)
To: Zhao Liu
Cc: qemu-devel, Maciej S. Szmigiero, Mark Cave-Ayland,
Michael S. Tsirkin, Marcel Apfelbaum, Halil Pasic,
Christian Borntraeger, Eric Farman, Peter Xu,
Philippe Mathieu-Daudé
On Wed, 31 Jan 2024 at 03:44, Zhao Liu <zhao1.liu@intel.com> wrote:
>
> On Fri, Jan 19, 2024 at 04:35:12PM +0000, Peter Maydell wrote:
> > Date: Fri, 19 Jan 2024 16:35:12 +0000
> > From: Peter Maydell <peter.maydell@linaro.org>
> > Subject: [PATCH 5/5] hw/core: Remove transitional infrastructure from
> > BusClass
> > X-Mailer: git-send-email 2.34.1
> >
> > BusClass currently has transitional infrastructure to support
> > subclasses which implement the legacy BusClass::reset method rather
> > than the Resettable interface. We have now removed all the users of
> > BusClass::reset in the tree, so we can remove the transitional
> > infrastructure.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > include/hw/qdev-core.h | 2 --
> > hw/core/bus.c | 67 ------------------------------------------
> > 2 files changed, 69 deletions(-)
>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
>
> It seems the similar cleanup for DeviceClass needs a lot of effort.
Yes and no. It's true that there are a lot of DeviceClass
subclasses that implement the legacy reset method still.
However, they are all "leaf" classes which provide a single
reset method, and don't need to either chain up to a parent
class reset implementation or allow a child class to
inherit from them. So they don't affect any other classes
in the system or block any other classes from being able
to use a full three-phase reset. So I'm not too worried
if we continue to have a lot of these leaf classes: the
old reset method is then just a different way of having
a device with a reset 'hold' method and no others.
I did an earlier round of refactoring back in 2022 that did
the cleanup of the non-leaf classes and the classes that
need to chain up to their parent class reset, which mostly
updated all of that kind of class to 3-phase. The one
exception is that we still have a single user of
device_class_set_parent_reset() in the s390 CPU, which
is awkward to deal with because it interacts with an
s390-specific handling of different levels of reset.
I also want to look at providing a 3-phase equivalent
of the qdev_register_reset() API at some point: that one
is definitely blocking some useful improvements, including
dealing with the vfio ordering issue, and also would let
me clean up some M-profile ARM CPU reset hacks.
thanks
-- PMM
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2024-02-01 13:32 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-19 16:35 [PATCH 0/5] buses: switch to 3-phase-reset Peter Maydell
2024-01-19 16:35 ` [PATCH 1/5] pci: Switch bus reset " Peter Maydell
2024-01-31 3:52 ` Zhao Liu
2024-01-19 16:35 ` [PATCH 2/5] vmbus: " Peter Maydell
2024-01-22 14:30 ` Maciej S. Szmigiero
2024-01-31 3:53 ` Zhao Liu
2024-01-19 16:35 ` [PATCH 3/5] adb: " Peter Maydell
2024-01-29 21:42 ` Mark Cave-Ayland
2024-01-31 3:53 ` Zhao Liu
2024-01-19 16:35 ` [PATCH 4/5] hw/s390x/css-bridge: switch virtual-css bus " Peter Maydell
2024-01-22 9:18 ` Halil Pasic
2024-01-22 21:49 ` Eric Farman
2024-01-31 3:54 ` Zhao Liu
2024-01-19 16:35 ` [PATCH 5/5] hw/core: Remove transitional infrastructure from BusClass Peter Maydell
2024-01-31 3:58 ` Zhao Liu
2024-02-01 13:31 ` Peter Maydell
2024-01-21 11:37 ` [PATCH 0/5] buses: switch to 3-phase-reset Michael S. Tsirkin
2024-01-22 2:06 ` Peter Xu
2024-01-22 14:19 ` Cédric Le Goater
2024-01-31 17:36 ` Cédric Le Goater
2024-02-01 13:20 ` Peter Maydell
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.