On 03.05.2018 17:49, David Hildenbrand wrote:
> Hotplug handlers usually do 3 things:
> 1. Allocate some resources for a new device
> 2. Make the new device visible for the guest
> 3. Notify the guest about the new device
>
> While 2. and 3. are only ever performed once for a device, there
> might be various resources to allocate. E.g. a MemoryDevice could be
> exposed via Virtio. The proxy device hotplug handler (e.g. virtio-pci,
> virtio-ccw) takes care of exposing the device to the guest. However,
> we also have to allocate system resources, like a portion in guest
> physical address space, which is to be handled by the machine.
>
> One key difference between a hotplug handler and a resource handler is
> that resource handlers have no "unplug_request". There is no
> communication with the guest (this is done by a "hotplug" handler). So
> conceptually, they are different.
>
> For now we live with the assumption, that - in additon to the existing
> hotplug handler which can do some resource asignment - at most one
> resource handler is necessary.
>
> We might even later decide to have multiple resource handlers for a
> specific device (e.g. one for each interface they implement). For now,
> this design is sufficient.
>
> Resource handlers have to run before the hotplug handler runs (esp
> before exposing the device to the guest).
>
> Signed-off-by: David Hildenbrand <da...@redhat.com>
> ---
> hw/core/Makefile.objs | 1 +
> hw/core/qdev.c | 41 ++++++++++++++++++++++++++++++-
> hw/core/resource-handler.c | 57
> +++++++++++++++++++++++++++++++++++++++++++
> include/hw/boards.h | 9 +++++++
> include/hw/resource-handler.h | 46 ++++++++++++++++++++++++++++++++++
> 5 files changed, 153 insertions(+), 1 deletion(-)
> create mode 100644 hw/core/resource-handler.c
> create mode 100644 include/hw/resource-handler.h
>
> diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
> index eb88ca979e..7474387fcd 100644
> --- a/hw/core/Makefile.objs
> +++ b/hw/core/Makefile.objs
> @@ -6,6 +6,7 @@ common-obj-$(CONFIG_SOFTMMU) += fw-path-provider.o
> # irq.o needed for qdev GPIO handling:
> common-obj-y += irq.o
> common-obj-y += hotplug.o
> +common-obj-y += resource-handler.o
> common-obj-$(CONFIG_SOFTMMU) += nmi.o
>
> common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index f6f92473b8..042e275884 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -35,6 +35,7 @@
> #include "qemu/error-report.h"
> #include "qemu/option.h"
> #include "hw/hotplug.h"
> +#include "hw/resource-handler.h"
> #include "hw/boards.h"
> #include "hw/sysbus.h"
>
> @@ -271,6 +272,17 @@ HotplugHandler *qdev_get_hotplug_handler(DeviceState
> *dev)
> return hotplug_ctrl;
> }
>
> +static ResourceHandler *qdev_get_resource_handler(DeviceState *dev)
> +{
> + MachineState *ms = MACHINE(qdev_get_machine());
> + MachineClass *mc = MACHINE_GET_CLASS(ms);
> +
> + if (mc->get_resource_handler) {
> + return mc->get_resource_handler(ms, dev);
> + }
> + return NULL;
> +}
> +
> static int qdev_reset_one(DeviceState *dev, void *opaque)
> {
> device_reset(dev);
> @@ -814,6 +826,7 @@ static void device_set_realized(Object *obj, bool value,
> Error **errp)
> {
> DeviceState *dev = DEVICE(obj);
> DeviceClass *dc = DEVICE_GET_CLASS(dev);
> + ResourceHandler *resource_ctrl;
> HotplugHandler *hotplug_ctrl;
> BusState *bus;
> Error *local_err = NULL;
> @@ -825,6 +838,8 @@ static void device_set_realized(Object *obj, bool value,
> Error **errp)
> return;
> }
>
> + resource_ctrl = qdev_get_resource_handler(dev);
> +
> if (value && !dev->realized) {
> if (!check_only_migratable(obj, &local_err)) {
> goto fail;
> @@ -840,6 +855,13 @@ static void device_set_realized(Object *obj, bool value,
> Error **errp)
> g_free(name);
> }
>
> + if (resource_ctrl) {
> + resource_handler_pre_assign(resource_ctrl, dev, &local_err);
> + if (local_err != NULL) {
> + goto fail;
> + }
> + }
> +
> hotplug_ctrl = qdev_get_hotplug_handler(dev);
> if (hotplug_ctrl) {
> hotplug_handler_pre_plug(hotplug_ctrl, dev, &local_err);
> @@ -858,12 +880,19 @@ static void device_set_realized(Object *obj, bool
> value, Error **errp)
>
> DEVICE_LISTENER_CALL(realize, Forward, dev);
>
> + if (resource_ctrl) {
> + resource_handler_assign(resource_ctrl, dev, &local_err);
> + if (local_err != NULL) {
> + goto post_realize_fail;
> + }
> + }
> +
> if (hotplug_ctrl) {
> hotplug_handler_plug(hotplug_ctrl, dev, &local_err);
> }
>
> if (local_err != NULL) {
> - goto post_realize_fail;
> + goto post_assign_fail;
> }
>
> /*
> @@ -903,6 +932,11 @@ static void device_set_realized(Object *obj, bool value,
> Error **errp)
> if (qdev_get_vmsd(dev)) {
> vmstate_unregister(dev, qdev_get_vmsd(dev), dev);
> }
> +
> + if (resource_ctrl) {
> + resource_handler_unassign(resource_ctrl, dev);
> + }
> +
> if (dc->unrealize) {
> local_errp = local_err ? NULL : &local_err;
> dc->unrealize(dev, local_errp);
> @@ -928,6 +962,11 @@ child_realize_fail:
> vmstate_unregister(dev, qdev_get_vmsd(dev), dev);
> }
>
> +post_assign_fail:
> + if (resource_ctrl) {
> + resource_handler_unassign(resource_ctrl, dev);
> + }
> +
> post_realize_fail:
> g_free(dev->canonical_path);
> dev->canonical_path = NULL;
> diff --git a/hw/core/resource-handler.c b/hw/core/resource-handler.c
> new file mode 100644
> index 0000000000..0a1ff0e66a
> --- /dev/null
> +++ b/hw/core/resource-handler.c
> @@ -0,0 +1,57 @@
> +/*
> + * Resource handler interface.
> + *
> + * Copyright (c) 2018 Red Hat Inc.
> + *
> + * Authors:
> + * David Hildenbrand <da...@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/resource-handler.h"
> +#include "qemu/module.h"
> +
> +void resource_handler_pre_assign(ResourceHandler *rh,
> + const DeviceState *dev, Error **errp)
> +{
> + ResourceHandlerClass *rhc = RESOURCE_HANDLER_GET_CLASS(rh);
> +
> + if (rhc->pre_assign) {
> + rhc->pre_assign(rh, dev, errp);
> + }
> +}
> +
> +void resource_handler_assign(ResourceHandler *rh, DeviceState *dev,
> + Error **errp)
> +{
> + ResourceHandlerClass *rhc = RESOURCE_HANDLER_GET_CLASS(rh);
> +
> + if (rhc->assign) {
> + rhc->assign(rh, dev, errp);
> + }
> +}
> +
> +void resource_handler_unassign(ResourceHandler *rh, DeviceState *dev)
> +{
> + ResourceHandlerClass *rhc = RESOURCE_HANDLER_GET_CLASS(rh);
> +
> + if (rhc->unassign) {
> + rhc->unassign(rh, dev);
> + }
> +}
> +
> +static const TypeInfo resource_handler_info = {
> + .name = TYPE_RESOURCE_HANDLER,
> + .parent = TYPE_INTERFACE,
> + .class_size = sizeof(ResourceHandlerClass),
> +};
> +
> +static void resource_handler_register_types(void)
> +{
> + type_register_static(&resource_handler_info);
> +}
> +
> +type_init(resource_handler_register_types)
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 8748964e6f..ff5142d7c2 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -8,6 +8,7 @@
> #include "hw/qdev.h"
> #include "qom/object.h"
> #include "qom/cpu.h"
> +#include "hw/resource-handler.h"
>
> /**
> * memory_region_allocate_system_memory - Allocate a board's main memory
> @@ -115,6 +116,12 @@ typedef struct {
> * of HotplugHandler object, which handles hotplug operation
> * for a given @dev. It may return NULL if @dev doesn't require
> * any actions to be performed by hotplug handler.
> + * @get_resource_handler: this function is called when a new device is
> + * about to be hotplugged. If defined, it returns
> pointer
> + * to an instance of ResourceHandler object, which
> + * handles resource asignment for a given @dev. It
> + * may return NULL if @dev doesn't require any actions
> + * to be performed by a resource handler.
> * @cpu_index_to_instance_props:
> * used to provide @cpu_index to socket/core/thread number mapping,
> allowing
> * legacy code to perform maping from cpu_index to topology properties
> @@ -208,6 +215,8 @@ struct MachineClass {
>
> HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
> DeviceState *dev);
> + ResourceHandler *(*get_resource_handler)(MachineState *machine,
> + const DeviceState *dev);
> CpuInstanceProperties (*cpu_index_to_instance_props)(MachineState
> *machine,
> unsigned cpu_index);
> const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
> diff --git a/include/hw/resource-handler.h b/include/hw/resource-handler.h
> new file mode 100644
> index 0000000000..3eda1bec5c
> --- /dev/null
> +++ b/include/hw/resource-handler.h
> @@ -0,0 +1,46 @@
> +/*
> + * Resource handler interface.
> + *
> + * Copyright (c) 2018 Red Hat Inc.
> + *
> + * Authors:
> + * David Hildenbrand <da...@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef RESOURCE_HANDLER_H
> +#define RESOURCE_HANDLER_H
> +
> +#include "qom/object.h"
> +
> +#define TYPE_RESOURCE_HANDLER "resource-handler"
> +
> +#define RESOURCE_HANDLER_CLASS(klass) \
> + OBJECT_CLASS_CHECK(ResourceHandlerClass, (klass), TYPE_RESOURCE_HANDLER)
> +#define RESOURCE_HANDLER_GET_CLASS(obj) \
> + OBJECT_GET_CLASS(ResourceHandlerClass, (obj), TYPE_RESOURCE_HANDLER)
> +#define RESOURCE_HANDLER(obj) \
> + INTERFACE_CHECK(ResourceHandler, (obj), TYPE_RESOURCE_HANDLER)
> +
> +typedef struct ResourceHandler {
> + Object Parent;
> +} ResourceHandler;
> +
> +typedef struct ResourceHandlerClass {
> + InterfaceClass parent;
> +
> + void (*pre_assign)(ResourceHandler *rh, const DeviceState *dev,
> + Error **errp);
> + void (*assign)(ResourceHandler *rh, DeviceState *dev, Error **errp);
> + void (*unassign)(ResourceHandler *rh, DeviceState *dev);
> +} ResourceHandlerClass;
> +
> +void resource_handler_pre_assign(ResourceHandler *rh, const DeviceState *dev,
> + Error **errp);
> +void resource_handler_assign(ResourceHandler *rh, DeviceState *dev,
> + Error **errp);
> +void resource_handler_unassign(ResourceHandler *rh, DeviceState *dev);
> +
> +#endif /* RESOURCE_HANDLER_H */
>
I did the following changes to make it pass "make check".
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 042e275884..efa59b9d1e 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -274,12 +274,18 @@ HotplugHandler
*qdev_get_hotplug_handler(DeviceState *dev)
static ResourceHandler *qdev_get_resource_handler(DeviceState *dev)
{
- MachineState *ms = MACHINE(qdev_get_machine());
- MachineClass *mc = MACHINE_GET_CLASS(ms);
+ MachineState *ms;
+ MachineClass *mc;
+ Object *m_obj = qdev_get_machine();
- if (mc->get_resource_handler) {
- return mc->get_resource_handler(ms, dev);
+ if (object_dynamic_cast(m_obj, TYPE_MACHINE)) {
+ ms = MACHINE(m_obj);
+ mc = MACHINE_GET_CLASS(ms);
+ if (mc->get_resource_handler) {
+ return mc->get_resource_handler(ms, dev);
+ }
}
+
return NULL;
}
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3b9a5e31a2..45c64bee64 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -645,6 +645,7 @@ tests/atomic_add-bench$(EXESUF):
tests/atomic_add-bench.o $(test-util-obj-y)
tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
hw/core/qdev.o hw/core/qdev-properties.o hw/core/hotplug.o\
+ hw/core/resource-handler.o \
hw/core/bus.o \
hw/core/irq.o \
hw/core/fw-path-provider.o \
--
Thanks,
David / dhildenb
