| Message ID | 20231005092926.56231-3-vsementsov@yandex-team.ru |
|---|---|
| State | New |
| Headers | show |
| Series | pci hotplug tracking | expand |
On Thu, Oct 05, 2023 at 12:29:24PM +0300, Vladimir Sementsov-Ogievskiy wrote: > We have DEVICE_DELETED event, that signals that device_del command is > actually completed. But we don't have a counter-part for device_add. > Still it's sensible for SHPC and PCIe-native hotplug, as there are time > when the device in some intermediate state. Let's add an event that say > that the device is finally powered on, power indicator is on and > everything is OK for next manipulation on that device. > > Motivations: > 1. To be sure that device is "accepted" by guest. Guest may ignore > hotplugged device for some reason (for example during OS booting). > Management wants to catch this and handle the problem, instead of > silent assume that everything is OK. So, if we don't get the event by > some timeout, we can report an error, try to unplug/plug the disk again > or do some other things to handle the problem. > > 2. The device can't be removed (by blockdev-del) while power indicator > of hotplug controller is blinking (QEMU reports "guest is busy (power > indicator blinking)"). So, management should avoid removing the device > until it gets the DEVICE_ON event. > (Probably, better solution for this point is to automatically postpone > deletion until power indicator stops blinking) > > 3. Also, management tool may make a GUI visualization of power > indicator with help of this event. > > New query-hotplug command in additon to "device-on" state also provides > SHPC/PCIe-native specific hotplug controller properties (like leds) > that may help to determine real state of hotplug controller. That may > help to get additional information for further debugging when DEVICE_ON > / DEVICE_DELETED not come in time as expected. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > --- > hw/core/hotplug.c | 12 ++++ > include/hw/hotplug.h | 11 ++++ > include/hw/qdev-core.h | 7 +++ > include/monitor/qdev.h | 2 + > qapi/qdev.json | 135 +++++++++++++++++++++++++++++++++++++++++ > softmmu/qdev-monitor.c | 41 +++++++++++++ > 6 files changed, 208 insertions(+) > > diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c > index 17ac986685..9651fe7c85 100644 > --- a/hw/core/hotplug.c > +++ b/hw/core/hotplug.c > @@ -57,6 +57,18 @@ void hotplug_handler_unplug(HotplugHandler *plug_handler, > } > } > > +HotplugInfo *hotplug_handler_get_state(HotplugHandler *plug_handler, > + DeviceState *plugged_dev, Error **errp) > +{ > + HotplugHandlerClass *hdc = HOTPLUG_HANDLER_GET_CLASS(plug_handler); > + > + if (hdc->get_hotplug_state) { > + return hdc->get_hotplug_state(plug_handler, plugged_dev, errp); > + } > + > + return NULL; > +} > + > static const TypeInfo hotplug_handler_info = { > .name = TYPE_HOTPLUG_HANDLER, > .parent = TYPE_INTERFACE, > diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h > index a9840ed485..23c57a0967 100644 > --- a/include/hw/hotplug.h > +++ b/include/hw/hotplug.h > @@ -13,6 +13,7 @@ > #define HOTPLUG_H > > #include "qom/object.h" > +#include "qapi/qapi-types-qdev.h" > > #define TYPE_HOTPLUG_HANDLER "hotplug-handler" > > @@ -60,6 +61,8 @@ struct HotplugHandlerClass { > hotplug_fn unplug_request; > hotplug_fn unplug; > bool (*is_hotpluggable_bus)(HotplugHandler *plug_handler, BusState *bus); > + HotplugInfo *(*get_hotplug_state)(HotplugHandler *plug_handler, > + DeviceState *plugged_dev, Error **errp); > }; > > /** > @@ -96,4 +99,12 @@ void hotplug_handler_unplug_request(HotplugHandler *plug_handler, > void hotplug_handler_unplug(HotplugHandler *plug_handler, > DeviceState *plugged_dev, > Error **errp); > + > +/** > + * hotplug_handler_get_state: > + * > + * Calls #HotplugHandlerClass.get_hotplug_state callback of @plug_handler. > + */ > +HotplugInfo *hotplug_handler_get_state(HotplugHandler *plug_handler, > + DeviceState *plugged_dev, Error **errp); > #endif > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index 151d968238..a27ef2eb24 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -229,6 +229,13 @@ struct DeviceState { > * @realized: has device been realized? > */ > bool realized; > + > + /** > + * @device_on_event_sent: When true means that DEVICE_ON event was already > + * sent. > + */ > + bool device_on_event_sent; > + > /** > * @pending_deleted_event: track pending deletion events during unplug > */ > diff --git a/include/monitor/qdev.h b/include/monitor/qdev.h > index 1d57bf6577..c1c8798e89 100644 > --- a/include/monitor/qdev.h > +++ b/include/monitor/qdev.h > @@ -36,4 +36,6 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts, > */ > const char *qdev_set_id(DeviceState *dev, char *id, Error **errp); > > +void qdev_hotplug_device_on_event(DeviceState *dev); > + > #endif > diff --git a/qapi/qdev.json b/qapi/qdev.json > index 2899fac837..fa80694735 100644 > --- a/qapi/qdev.json > +++ b/qapi/qdev.json > @@ -180,3 +180,138 @@ > # "timestamp": { "seconds": 1615570772, "microseconds": 202844 } } > ## > { 'event': 'DEVICE_UNPLUG_GUEST_ERROR', 'data': 'DeviceAndPath' } > + > +## > +# @LedStatus: > +# > +# @on: LED is on. > +# > +# @blinking: LED is blinking. > +# > +# @off: LED is off. > +# > +# Since: 8.2 > +## > +{ 'enum': 'LedStatus', > + 'data': [ 'on', 'blinking', 'off' ] } > + Querying LED status is by itself a reasonable thing. It's under guest control though - how do we avoid a flood of events? > +## > +# @HotplugSHPCSlotState: > +# > +# Standard Hot-Plug Controller slot state. > +# > +# @power-only: Slot is powered on, but neither clock nor bus are > +# connected. > +# > +# @enabled: Slot is powered on, clock and bus are connected, and the > +# card is fully functional from a hardware standpoint. > +# > +# @disabled: Slot is disabled, card is safe to be removed. > +# Are these really the only 3 states a slot can be in? And do we care about power-only? > +# Since: 8.2 > +## > +{ 'enum': 'HotplugSHPCSlotState', > + 'data': [ 'power-only', 'enabled', 'disabled' ] } > + > +## > +# @HotplugSHPCState: > +# > +# Standard Hot Plug Controller state. > +# > +# @slot-state: The slot state field of SHPC Slot Status structure. > +# > +# Since: 8.2 > +## > +{ 'struct': 'HotplugSHPCState', > + 'data': { '*slot-state': 'HotplugSHPCSlotState' } } > + > +## > +# @HotplugPCIeNativeState: > +# > +# PCIe Native hotplug slot state. > +# > +# @power-on: PCIe Power Controller Control of Slot Control Register. > +# True means Power On (Power Controller Control bit is 0), False > +# means Power Off (Power Controller Control bit is 1). > +# > +# Since: 8.2 > +## > +{ 'struct': 'HotplugPCIeNativeState', > + 'data': { '*power-on': 'bool' } } and here we don't care about enable? > + > +## > +# @HotplugType: > +# > +# Type of hotplug controller / provider. > +# > +# @shpc: PCI Standard Hot-plug Controller > +# > +# @pcie-native: PCIe Native hotplug > +# > +# TODO: add @acpi type > +# > +# Since: 8.2 > +## > +{ 'enum': 'HotplugType', > + 'data': ['shpc', 'pcie-native'] } > + > +## > +# @HotplugInfo: > +# > +# Hotplug slot state. > +# > +# @type: type of the hotplug > +# > +# @bus: The QOM path of the parent bus where device is plugged into. > +# > +# @addr: The bus address for hotplugged device if applicable. > +# > +# @child: the hotplugged device > +# > +# @device-on: Device is powered-on by guest. This state changes at > +# most once for the device and corresponds to DEVICE_ON event. > +# > +# @power-led: Power indicator. When power LED is on the device is > +# ready and accepted by guest. When it is off, device is safe to > +# remove. It is blinking while hot plug or unplug is in progress. > +# > +# @attention-led: The attention LED is normally off. It is on to > +# signal a problem. Blinking is for helping users to locate the > +# slot. > +# > +# Single: 8.2 > +## > +{ 'union': 'HotplugInfo', > + 'base': { 'type': 'HotplugType', > + 'bus': 'DeviceAndPath', > + '*addr': 'str', > + 'child': 'DeviceAndPath', > + 'device-on': 'bool', > + '*power-led': 'LedStatus', > + '*attention-led': 'LedStatus'}, > + 'discriminator': 'type', > + 'data': { 'shpc': 'HotplugSHPCState', > + 'pcie-native': 'HotplugPCIeNativeState' } } > + What about ACPI hotplug? > +## > +# @x-query-hotplug: > +# > +# Query the state of hotplug controller. > +# > +# @id: the device's ID or QOM path > +# > +# Since: 8.2 > +## > +{ 'command': 'x-query-hotplug', > + 'data': { 'id': 'str' }, > + 'returns': 'HotplugInfo' } > + > +## > +# @X_DEVICE_ON: > +# > +# Emitted whenever the device insertion completion is acknowledged by > +# the guest. For now only emitted for SHPC and PCIe-native hotplug. > +# > +# Since: 8.2 > +## > +{ 'event': 'X_DEVICE_ON', 'data': 'DeviceAndPath' } > diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c > index 74f4e41338..483da40cb9 100644 > --- a/softmmu/qdev-monitor.c > +++ b/softmmu/qdev-monitor.c > @@ -25,6 +25,7 @@ > #include "sysemu/arch_init.h" > #include "qapi/error.h" > #include "qapi/qapi-commands-qdev.h" > +#include "qapi/qapi-events-qdev.h" > #include "qapi/qmp/dispatch.h" > #include "qapi/qmp/qdict.h" > #include "qapi/qmp/qerror.h" > @@ -956,6 +957,36 @@ void qmp_device_del(const char *id, Error **errp) > } > } > > +HotplugInfo *qmp_x_query_hotplug(const char *id, Error **errp) > +{ > + DeviceState *dev = find_device_state(id, errp); > + HotplugHandler *hotplug_ctrl; > + > + if (!dev) { > + return NULL; > + } > + > + if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) { > + error_setg(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name); > + return NULL; > + } > + > + if (!DEVICE_GET_CLASS(dev)->hotpluggable) { > + error_setg(errp, QERR_DEVICE_NO_HOTPLUG, > + object_get_typename(OBJECT(dev))); > + return NULL; > + } > + > + hotplug_ctrl = qdev_get_hotplug_handler(dev); > + /* > + * hotpluggable device MUST have HotplugHandler, if it doesn't > + * then something is very wrong with it. > + */ > + g_assert(hotplug_ctrl); > + > + return hotplug_handler_get_state(hotplug_ctrl, dev, errp); > +} > + > void hmp_device_add(Monitor *mon, const QDict *qdict) > { > Error *err = NULL; > @@ -1146,3 +1177,13 @@ bool qmp_command_available(const QmpCommand *cmd, Error **errp) > } > return true; > } > + > +void qdev_hotplug_device_on_event(DeviceState *dev) > +{ > + if (dev->device_on_event_sent) { > + return; > + } > + > + dev->device_on_event_sent = true; > + qapi_event_send_x_device_on(dev->id, dev->canonical_path); > +} > -- > 2.34.1
diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c index 17ac986685..9651fe7c85 100644 --- a/hw/core/hotplug.c +++ b/hw/core/hotplug.c @@ -57,6 +57,18 @@ void hotplug_handler_unplug(HotplugHandler *plug_handler, } } +HotplugInfo *hotplug_handler_get_state(HotplugHandler *plug_handler, + DeviceState *plugged_dev, Error **errp) +{ + HotplugHandlerClass *hdc = HOTPLUG_HANDLER_GET_CLASS(plug_handler); + + if (hdc->get_hotplug_state) { + return hdc->get_hotplug_state(plug_handler, plugged_dev, errp); + } + + return NULL; +} + static const TypeInfo hotplug_handler_info = { .name = TYPE_HOTPLUG_HANDLER, .parent = TYPE_INTERFACE, diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h index a9840ed485..23c57a0967 100644 --- a/include/hw/hotplug.h +++ b/include/hw/hotplug.h @@ -13,6 +13,7 @@ #define HOTPLUG_H #include "qom/object.h" +#include "qapi/qapi-types-qdev.h" #define TYPE_HOTPLUG_HANDLER "hotplug-handler" @@ -60,6 +61,8 @@ struct HotplugHandlerClass { hotplug_fn unplug_request; hotplug_fn unplug; bool (*is_hotpluggable_bus)(HotplugHandler *plug_handler, BusState *bus); + HotplugInfo *(*get_hotplug_state)(HotplugHandler *plug_handler, + DeviceState *plugged_dev, Error **errp); }; /** @@ -96,4 +99,12 @@ void hotplug_handler_unplug_request(HotplugHandler *plug_handler, void hotplug_handler_unplug(HotplugHandler *plug_handler, DeviceState *plugged_dev, Error **errp); + +/** + * hotplug_handler_get_state: + * + * Calls #HotplugHandlerClass.get_hotplug_state callback of @plug_handler. + */ +HotplugInfo *hotplug_handler_get_state(HotplugHandler *plug_handler, + DeviceState *plugged_dev, Error **errp); #endif diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 151d968238..a27ef2eb24 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -229,6 +229,13 @@ struct DeviceState { * @realized: has device been realized? */ bool realized; + + /** + * @device_on_event_sent: When true means that DEVICE_ON event was already + * sent. + */ + bool device_on_event_sent; + /** * @pending_deleted_event: track pending deletion events during unplug */ diff --git a/include/monitor/qdev.h b/include/monitor/qdev.h index 1d57bf6577..c1c8798e89 100644 --- a/include/monitor/qdev.h +++ b/include/monitor/qdev.h @@ -36,4 +36,6 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts, */ const char *qdev_set_id(DeviceState *dev, char *id, Error **errp); +void qdev_hotplug_device_on_event(DeviceState *dev); + #endif diff --git a/qapi/qdev.json b/qapi/qdev.json index 2899fac837..fa80694735 100644 --- a/qapi/qdev.json +++ b/qapi/qdev.json @@ -180,3 +180,138 @@ # "timestamp": { "seconds": 1615570772, "microseconds": 202844 } } ## { 'event': 'DEVICE_UNPLUG_GUEST_ERROR', 'data': 'DeviceAndPath' } + +## +# @LedStatus: +# +# @on: LED is on. +# +# @blinking: LED is blinking. +# +# @off: LED is off. +# +# Since: 8.2 +## +{ 'enum': 'LedStatus', + 'data': [ 'on', 'blinking', 'off' ] } + +## +# @HotplugSHPCSlotState: +# +# Standard Hot-Plug Controller slot state. +# +# @power-only: Slot is powered on, but neither clock nor bus are +# connected. +# +# @enabled: Slot is powered on, clock and bus are connected, and the +# card is fully functional from a hardware standpoint. +# +# @disabled: Slot is disabled, card is safe to be removed. +# +# Since: 8.2 +## +{ 'enum': 'HotplugSHPCSlotState', + 'data': [ 'power-only', 'enabled', 'disabled' ] } + +## +# @HotplugSHPCState: +# +# Standard Hot Plug Controller state. +# +# @slot-state: The slot state field of SHPC Slot Status structure. +# +# Since: 8.2 +## +{ 'struct': 'HotplugSHPCState', + 'data': { '*slot-state': 'HotplugSHPCSlotState' } } + +## +# @HotplugPCIeNativeState: +# +# PCIe Native hotplug slot state. +# +# @power-on: PCIe Power Controller Control of Slot Control Register. +# True means Power On (Power Controller Control bit is 0), False +# means Power Off (Power Controller Control bit is 1). +# +# Since: 8.2 +## +{ 'struct': 'HotplugPCIeNativeState', + 'data': { '*power-on': 'bool' } } + +## +# @HotplugType: +# +# Type of hotplug controller / provider. +# +# @shpc: PCI Standard Hot-plug Controller +# +# @pcie-native: PCIe Native hotplug +# +# TODO: add @acpi type +# +# Since: 8.2 +## +{ 'enum': 'HotplugType', + 'data': ['shpc', 'pcie-native'] } + +## +# @HotplugInfo: +# +# Hotplug slot state. +# +# @type: type of the hotplug +# +# @bus: The QOM path of the parent bus where device is plugged into. +# +# @addr: The bus address for hotplugged device if applicable. +# +# @child: the hotplugged device +# +# @device-on: Device is powered-on by guest. This state changes at +# most once for the device and corresponds to DEVICE_ON event. +# +# @power-led: Power indicator. When power LED is on the device is +# ready and accepted by guest. When it is off, device is safe to +# remove. It is blinking while hot plug or unplug is in progress. +# +# @attention-led: The attention LED is normally off. It is on to +# signal a problem. Blinking is for helping users to locate the +# slot. +# +# Single: 8.2 +## +{ 'union': 'HotplugInfo', + 'base': { 'type': 'HotplugType', + 'bus': 'DeviceAndPath', + '*addr': 'str', + 'child': 'DeviceAndPath', + 'device-on': 'bool', + '*power-led': 'LedStatus', + '*attention-led': 'LedStatus'}, + 'discriminator': 'type', + 'data': { 'shpc': 'HotplugSHPCState', + 'pcie-native': 'HotplugPCIeNativeState' } } + +## +# @x-query-hotplug: +# +# Query the state of hotplug controller. +# +# @id: the device's ID or QOM path +# +# Since: 8.2 +## +{ 'command': 'x-query-hotplug', + 'data': { 'id': 'str' }, + 'returns': 'HotplugInfo' } + +## +# @X_DEVICE_ON: +# +# Emitted whenever the device insertion completion is acknowledged by +# the guest. For now only emitted for SHPC and PCIe-native hotplug. +# +# Since: 8.2 +## +{ 'event': 'X_DEVICE_ON', 'data': 'DeviceAndPath' } diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c index 74f4e41338..483da40cb9 100644 --- a/softmmu/qdev-monitor.c +++ b/softmmu/qdev-monitor.c @@ -25,6 +25,7 @@ #include "sysemu/arch_init.h" #include "qapi/error.h" #include "qapi/qapi-commands-qdev.h" +#include "qapi/qapi-events-qdev.h" #include "qapi/qmp/dispatch.h" #include "qapi/qmp/qdict.h" #include "qapi/qmp/qerror.h" @@ -956,6 +957,36 @@ void qmp_device_del(const char *id, Error **errp) } } +HotplugInfo *qmp_x_query_hotplug(const char *id, Error **errp) +{ + DeviceState *dev = find_device_state(id, errp); + HotplugHandler *hotplug_ctrl; + + if (!dev) { + return NULL; + } + + if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) { + error_setg(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name); + return NULL; + } + + if (!DEVICE_GET_CLASS(dev)->hotpluggable) { + error_setg(errp, QERR_DEVICE_NO_HOTPLUG, + object_get_typename(OBJECT(dev))); + return NULL; + } + + hotplug_ctrl = qdev_get_hotplug_handler(dev); + /* + * hotpluggable device MUST have HotplugHandler, if it doesn't + * then something is very wrong with it. + */ + g_assert(hotplug_ctrl); + + return hotplug_handler_get_state(hotplug_ctrl, dev, errp); +} + void hmp_device_add(Monitor *mon, const QDict *qdict) { Error *err = NULL; @@ -1146,3 +1177,13 @@ bool qmp_command_available(const QmpCommand *cmd, Error **errp) } return true; } + +void qdev_hotplug_device_on_event(DeviceState *dev) +{ + if (dev->device_on_event_sent) { + return; + } + + dev->device_on_event_sent = true; + qapi_event_send_x_device_on(dev->id, dev->canonical_path); +}
We have DEVICE_DELETED event, that signals that device_del command is actually completed. But we don't have a counter-part for device_add. Still it's sensible for SHPC and PCIe-native hotplug, as there are time when the device in some intermediate state. Let's add an event that say that the device is finally powered on, power indicator is on and everything is OK for next manipulation on that device. Motivations: 1. To be sure that device is "accepted" by guest. Guest may ignore hotplugged device for some reason (for example during OS booting). Management wants to catch this and handle the problem, instead of silent assume that everything is OK. So, if we don't get the event by some timeout, we can report an error, try to unplug/plug the disk again or do some other things to handle the problem. 2. The device can't be removed (by blockdev-del) while power indicator of hotplug controller is blinking (QEMU reports "guest is busy (power indicator blinking)"). So, management should avoid removing the device until it gets the DEVICE_ON event. (Probably, better solution for this point is to automatically postpone deletion until power indicator stops blinking) 3. Also, management tool may make a GUI visualization of power indicator with help of this event. New query-hotplug command in additon to "device-on" state also provides SHPC/PCIe-native specific hotplug controller properties (like leds) that may help to determine real state of hotplug controller. That may help to get additional information for further debugging when DEVICE_ON / DEVICE_DELETED not come in time as expected. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> --- hw/core/hotplug.c | 12 ++++ include/hw/hotplug.h | 11 ++++ include/hw/qdev-core.h | 7 +++ include/monitor/qdev.h | 2 + qapi/qdev.json | 135 +++++++++++++++++++++++++++++++++++++++++ softmmu/qdev-monitor.c | 41 +++++++++++++ 6 files changed, 208 insertions(+)