Re: [Qemu-devel] [PATCH 05/15] target-i386: introduce ICC bus/device/bridge

[Andreas Färber]

Am 25.04.2013 16:05, schrieb Igor Mammedov:
> Provides hotplug-able bus for APIC and CPU.
> 
> * icc-bridge will serve as a parent for icc-bus and provide
>   mmio mapping services to child icc-devices.
> * icc-device will replace SysBusDevice as a parent of APIC
>   and IOAPIC devices.
> 
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
> v3:
>   * mv include/hw/i386/icc_bus.h into include/hw/cpu/
>   * various style fixes
>   * embed icc-bus inside icc-bridge and use qbus_create_inplace()
>   * update MAINTAINERS with new files
> v2:
>   * Rebase on top the last HW reorganization series.
>   * Move hw/icc_bus.c into hw/cpu/ and hw/icc_bus.h include/hw/i386/
> ---
>  MAINTAINERS                        |    6 ++
>  default-configs/i386-softmmu.mak   |    1 +
>  default-configs/x86_64-softmmu.mak |    1 +
>  hw/cpu/Makefile.objs               |    1 +
>  hw/cpu/icc_bus.c                   |  104 
> ++++++++++++++++++++++++++++++++++++
>  hw/i386/pc_piix.c                  |    7 +++
>  hw/i386/pc_q35.c                   |    7 +++
>  include/hw/cpu/icc_bus.h           |   58 ++++++++++++++++++++
>  8 files changed, 185 insertions(+), 0 deletions(-)
>  create mode 100644 hw/cpu/icc_bus.c
>  create mode 100644 include/hw/cpu/icc_bus.h

Thanks, queued on qom-cpu-next (with changes below):
https://github.com/afaerber/qemu-cpu/commits/qom-cpu-next

In particular since this is a new file I've taken the liberty to tidy
the header a bit; and since you had changed initfn -> init I renamed
realizefn alongside and normalized Error** argument.
When I converted the CPUs to QOM there were protests against the use of
k / klass and the compromise was to use oc, dc, etc., thus idc here.
Doing this temporarily on -next in case I run into non-trivial rebase
problems later on.

Andreas

diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
index f6091b9..2db42b1 100644
--- a/hw/cpu/icc_bus.c
+++ b/hw/cpu/icc_bus.c
@@ -1,5 +1,5 @@
 /* icc_bus.c
- * emulate x86 ICC(Interrupt Controller Communications) bus
+ * emulate x86 ICC (Interrupt Controller Communications) bus
  *
  * Copyright (c) 2013 Red Hat, Inc
  *
@@ -23,6 +23,7 @@
 #include "hw/sysbus.h"

 /* icc-bridge implementation */
+
 static void icc_bus_init(Object *obj)
 {
     BusState *b = BUS(obj);
@@ -37,18 +38,18 @@ static const TypeInfo icc_bus_info = {
     .instance_init = icc_bus_init,
 };

+
 /* icc-device implementation */
-static void icc_device_realizefn(DeviceState *dev, Error **err)
+
+static void icc_device_realize(DeviceState *dev, Error **errp)
 {
     ICCDevice *id = ICC_DEVICE(dev);
     ICCDeviceClass *k = ICC_DEVICE_GET_CLASS(id);
-    Error *local_err = NULL;

     if (k->init) {
         if (k->init(id) < 0) {
-            error_setg(&local_err, "%s initialization failed.",
+            error_setg(errp, "%s initialization failed.",
                        object_get_typename(OBJECT(dev)));
-            error_propagate(err, local_err);
         }
     }
 }
@@ -57,7 +58,7 @@ static void icc_device_class_init(ObjectClass *klass,
void *da
ta)
 {
     DeviceClass *k = DEVICE_CLASS(klass);

-    k->realize = icc_device_realizefn;
+    k->realize = icc_device_realize;
     k->bus_type = TYPE_ICC_BUS;
 }

@@ -70,10 +71,14 @@ static const TypeInfo icc_device_info = {
     .class_init = icc_device_class_init,
 };

+
 /*  icc-bridge implementation */
+
 typedef struct ICCBridgeState {
     /*< private >*/
     SysBusDevice parent_obj;
+    /*< public >*/
+
     ICCBus icc_bus;
 } ICCBridgeState;

diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
index a0abc21..db252c7 100644
--- a/include/hw/cpu/icc_bus.h
+++ b/include/hw/cpu/icc_bus.h
@@ -1,5 +1,5 @@
 /* icc_bus.h
- * emulate x86 ICC(Interrupt Controller Communications) bus
+ * emulate x86 ICC (Interrupt Controller Communications) bus
  *
  * Copyright (c) 2013 Red Hat, Inc
  *
@@ -28,21 +28,42 @@

 #ifndef CONFIG_USER_ONLY

+/**
+ * ICCBus:
+ *
+ * ICC bus
+ */
 typedef struct ICCBus {
     /*< private >*/
     BusState parent_obj;
+    /*< public >*/
 } ICCBus;

 #define ICC_BUS(obj) OBJECT_CHECK(ICCBus, (obj), TYPE_ICC_BUS)

+/**
+ * ICCDevice:
+ *
+ * ICC device
+ */
 typedef struct ICCDevice {
+    /*< private >*/
+    DeviceState parent_obj;
     /*< public >*/
-    DeviceState qdev;
 } ICCDevice;

+/**
+ * ICCDeviceClass:
+ * @init: Initialization callback for derived classes.
+ *
+ * ICC device class
+ */
 typedef struct ICCDeviceClass {
+    /*< private >*/
     DeviceClass parent_class;
-    int (*init)(ICCDevice *dev);
+    /*< public >*/
+
+    int (*init)(ICCDevice *dev); /* TODO replace with QOM realize */
 } ICCDeviceClass;

 #define TYPE_ICC_DEVICE "icc-device"



diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
index 2db42b1..a89bbb3 100644
--- a/hw/cpu/icc_bus.c
+++ b/hw/cpu/icc_bus.c
@@ -56,10 +56,10 @@ static void icc_device_realize(DeviceState *dev,
Error **errp)

 static void icc_device_class_init(ObjectClass *klass, void *data)
 {
-    DeviceClass *k = DEVICE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);

-    k->realize = icc_device_realize;
-    k->bus_type = TYPE_ICC_BUS;
+    dc->realize = icc_device_realize;
+    dc->bus_type = TYPE_ICC_BUS;
 }

 static const TypeInfo icc_device_info = {



diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
index a89bbb3..1d5e646 100644
--- a/hw/cpu/icc_bus.c
+++ b/hw/cpu/icc_bus.c
@@ -44,19 +44,19 @@ static const TypeInfo icc_bus_info = {
 static void icc_device_realize(DeviceState *dev, Error **errp)
 {
     ICCDevice *id = ICC_DEVICE(dev);
-    ICCDeviceClass *k = ICC_DEVICE_GET_CLASS(id);
+    ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(id);

-    if (k->init) {
-        if (k->init(id) < 0) {
+    if (idc->init) {
+        if (idc->init(id) < 0) {
             error_setg(errp, "%s initialization failed.",
                        object_get_typename(OBJECT(dev)));
         }
     }
 }

-static void icc_device_class_init(ObjectClass *klass, void *data)
+static void icc_device_class_init(ObjectClass *oc, void *data)
 {
-    DeviceClass *dc = DEVICE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(oc);

     dc->realize = icc_device_realize;
     dc->bus_type = TYPE_ICC_BUS;

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg