[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 02/18] xen: introduce new 'XenBus' and 'XenDe
From: |
Paul Durrant |
Subject: |
Re: [Qemu-devel] [PATCH v2 02/18] xen: introduce new 'XenBus' and 'XenDevice' object hierarchy |
Date: |
Fri, 7 Dec 2018 12:57:29 +0000 |
> -----Original Message-----
> From: Anthony PERARD [mailto:address@hidden
> Sent: 07 December 2018 12:15
> To: Paul Durrant <address@hidden>
> Cc: address@hidden; address@hidden; xen-
> address@hidden; Stefano Stabellini <address@hidden>;
> Michael S. Tsirkin <address@hidden>; Marcel Apfelbaum
> <address@hidden>; Paolo Bonzini <address@hidden>; Richard
> Henderson <address@hidden>; Eduardo Habkost <address@hidden>
> Subject: Re: [PATCH v2 02/18] xen: introduce new 'XenBus' and 'XenDevice'
> object hierarchy
>
> On Thu, Dec 06, 2018 at 03:08:28PM +0000, Paul Durrant wrote:
> > This patch adds the basic boilerplate for a 'XenBus' object that will
> act
> > as a parent to 'XenDevice' PV backends.
> > A new 'XenBridge' object is also added to connect XenBus to the system
> bus.
> >
> > The XenBus object is instantiated by a new xen_bus_init() function
> called
> > from the same sites as the legacy xen_be_init() function.
> >
> > Subsequent patches will flesh-out the functionality of these objects.
> >
> > Signed-off-by: Paul Durrant <address@hidden>
> > ---
> > Cc: Stefano Stabellini <address@hidden>
> > Cc: Anthony Perard <address@hidden>
> > Cc: "Michael S. Tsirkin" <address@hidden>
> > Cc: Marcel Apfelbaum <address@hidden>
> > Cc: Paolo Bonzini <address@hidden>
> > Cc: Richard Henderson <address@hidden>
> > Cc: Eduardo Habkost <address@hidden>
> >
> > v2:
> > - Fix boilerplate
> > - Make xen-bus hotplug capable
> > ---
> > hw/i386/xen/xen-hvm.c | 3 ++
> > hw/xen/Makefile.objs | 2 +-
> > hw/xen/trace-events | 6 +++
> > hw/xen/xen-bus.c | 131
> ++++++++++++++++++++++++++++++++++++++++++++++
> > hw/xenpv/xen_machine_pv.c | 3 ++
> > include/hw/xen/xen-bus.h | 55 +++++++++++++++++++
> > 6 files changed, 199 insertions(+), 1 deletion(-)
> > create mode 100644 hw/xen/xen-bus.c
> > create mode 100644 include/hw/xen/xen-bus.h
> >
> > diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
> > index 1d63763..4497f75 100644
> > --- a/hw/i386/xen/xen-hvm.c
> > +++ b/hw/i386/xen/xen-hvm.c
> > @@ -17,6 +17,7 @@
> > #include "hw/i386/apic-msidef.h"
> > #include "hw/xen/xen_common.h"
> > #include "hw/xen/xen-legacy-backend.h"
> > +#include "hw/xen/xen-bus.h"
> > #include "qapi/error.h"
> > #include "qapi/qapi-commands-misc.h"
> > #include "qemu/error-report.h"
> > @@ -1479,6 +1480,8 @@ void xen_hvm_init(PCMachineState *pcms,
> MemoryRegion **ram_memory)
> > QLIST_INIT(&state->dev_list);
> > device_listener_register(&state->device_listener);
> >
> > + xen_bus_init();
> > +
> > /* Initialize backend core & drivers */
> > if (xen_be_init() != 0) {
> > error_report("xen backend core setup failed");
> > diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs
> > index 3f64a44..d9d6d7b 100644
> > --- a/hw/xen/Makefile.objs
> > +++ b/hw/xen/Makefile.objs
> > @@ -1,5 +1,5 @@
> > # xen backend driver support
> > -common-obj-$(CONFIG_XEN) += xen-legacy-backend.o xen_devconfig.o
> xen_pvdev.o xen-common.o
> > +common-obj-$(CONFIG_XEN) += xen-legacy-backend.o xen_devconfig.o
> xen_pvdev.o xen-common.o xen-bus.o
> >
> > obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
> > obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o
> xen_pt_graphics.o xen_pt_msi.o
> > diff --git a/hw/xen/trace-events b/hw/xen/trace-events
> > index c7e7a3b..0172cd4 100644
> > --- a/hw/xen/trace-events
> > +++ b/hw/xen/trace-events
> > @@ -12,3 +12,9 @@ xen_unmap_portio_range(uint32_t id, uint64_t
> start_addr, uint64_t end_addr) "id:
> > xen_map_pcidev(uint32_t id, uint8_t bus, uint8_t dev, uint8_t func)
> "id: %u bdf: %02x.%02x.%02x"
> > xen_unmap_pcidev(uint32_t id, uint8_t bus, uint8_t dev, uint8_t func)
> "id: %u bdf: %02x.%02x.%02x"
> > xen_domid_restrict(int err) "err: %u"
> > +
> > +# include/hw/xen/xen-bus.c
> > +xen_bus_realize(void) ""
> > +xen_bus_unrealize(void) ""
> > +xen_device_realize(const char *type) "type: %s"
> > +xen_device_unrealize(const char *type) "type: %s"
> > diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
> > new file mode 100644
> > index 0000000..1385bab
> > --- /dev/null
> > +++ b/hw/xen/xen-bus.c
> > @@ -0,0 +1,131 @@
> > +/*
> > + * Copyright (c) 2018 Citrix Systems Inc.
> > + *
> > + * 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/hw.h"
> > +#include "hw/sysbus.h"
> > +#include "hw/xen/xen-bus.h"
> > +#include "qapi/error.h"
> > +#include "trace.h"
> > +
> > +static void xen_bus_unrealize(BusState *bus, Error **errp)
> > +{
> > + trace_xen_bus_unrealize();
> > +}
> > +
> > +static void xen_bus_realize(BusState *bus, Error **errp)
> > +{
> > + trace_xen_bus_realize();
> > +}
> > +
> > +static void xen_bus_class_init(ObjectClass *class, void *data)
> > +{
> > + BusClass *bus_class = BUS_CLASS(class);
> > +
> > + bus_class->realize = xen_bus_realize;
> > + bus_class->unrealize = xen_bus_unrealize;
> > +}
> > +
> > +static const TypeInfo xen_bus_type_info = {
> > + .name = TYPE_XEN_BUS,
> > + .parent = TYPE_BUS,
> > + .instance_size = sizeof(XenBus),
> > + .class_size = sizeof(XenBusClass),
> > + .class_init = xen_bus_class_init,
> > + .interfaces = (InterfaceInfo[]) {
> > + { TYPE_HOTPLUG_HANDLER },
> > + { }
> > + },
> > +};
> > +
> > +static void xen_device_unrealize(DeviceState *dev, Error **errp)
> > +{
> > + XenDevice *xendev = XEN_DEVICE(dev);
> > + XenDeviceClass *xendev_class = XEN_DEVICE_GET_CLASS(xendev);
> > + const char *type = object_get_typename(OBJECT(xendev));
> > + Error *local_err = NULL;
> > +
> > + trace_xen_device_unrealize(type);
> > +
> > + if (xendev_class->unrealize) {
> > + xendev_class->unrealize(xendev, &local_err);
>
> Since all you do here is propagate the error, you could even pass `errp'
> to unrealize(), instead of having `local_err'. That "for readability",
> as explained in "qapi/error.h".
Yes, true.
>
> > + if (local_err) {
> > + error_propagate(errp, local_err);
> > + }
> > + }
> > +}
> > +
>
> With that nit fixed:
> Reviewed-by: Anthony PERARD <address@hidden>
>
Thanks :-)
Paul
> Thanks,
>
> --
> Anthony PERARD
- [Qemu-devel] [PATCH v2 00/18] Xen PV backend 'qdevification', Paul Durrant, 2018/12/06
- [Qemu-devel] [PATCH v2 01/18] xen: re-name XenDevice to XenLegacyDevice..., Paul Durrant, 2018/12/06
- [Qemu-devel] [PATCH v2 02/18] xen: introduce new 'XenBus' and 'XenDevice' object hierarchy, Paul Durrant, 2018/12/06
- [Qemu-devel] [PATCH v2 08/18] xen: duplicate xen_disk.c as basis of dataplane/xen-block.c, Paul Durrant, 2018/12/06
- [Qemu-devel] [PATCH v2 05/18] xen: add xenstore watcher infrastructure, Paul Durrant, 2018/12/06
- [Qemu-devel] [PATCH v2 04/18] xen: create xenstore areas for XenDevice-s, Paul Durrant, 2018/12/06
- [Qemu-devel] [PATCH v2 09/18] xen: remove unnecessary code from dataplane/xen-block.c, Paul Durrant, 2018/12/06
- [Qemu-devel] [PATCH v2 07/18] xen: add event channel interface for XenDevice-s, Paul Durrant, 2018/12/06