qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [qemu-s390x] [PATCH v9 4/6] s390x/ap: base Adjunct Proc


From: Tony Krowiak
Subject: Re: [Qemu-devel] [qemu-s390x] [PATCH v9 4/6] s390x/ap: base Adjunct Processor (AP) object model
Date: Mon, 8 Oct 2018 10:20:43 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0

On 09/27/2018 08:52 AM, Cornelia Huck wrote:
On Thu, 27 Sep 2018 14:29:01 +0200
Thomas Huth <address@hidden> wrote:

On 2018-09-27 00:54, Tony Krowiak wrote:
From: Tony Krowiak <address@hidden>

Introduces the base object model for virtualizing AP devices.

Signed-off-by: Tony Krowiak <address@hidden>
---

+typedef struct APBridge {
+    SysBusDevice sysbus_dev;
+    bool css_dev_path;

What is this css_dev_path variable good for? I don't see it used in any
of the other patches?
If you don't need it, I think you could get rid of this struct completely?

Huh, now I remember complaining about it before. Looks like a
copy-and-paste from the css bridge; that variable is used for compat
handling there (and should be ditched here).


+} APBridge;
+
+#define TYPE_AP_BRIDGE "ap-bridge"
+#define AP_BRIDGE(obj) \
+    OBJECT_CHECK(APBridge, (obj), TYPE_AP_BRIDGE)
+
+typedef struct APBus {
+    BusState parent_obj;
+} APBus;
+
+#define TYPE_AP_BUS "ap-bus"
+#define AP_BUS(obj) \
+     OBJECT_CHECK(APBus, (obj), TYPE_AP_BUS)

I think you could also get rid of AP_BRIDGE(), AP_BUS() and maybe even
struct APBus.

If there's nothing interesting to put in these inherited structures,
probably yes.


+void s390_init_ap(void);
+
+#endif
diff --git a/include/hw/s390x/ap-device.h b/include/hw/s390x/ap-device.h
new file mode 100644
index 000000000000..693df90cc041
--- /dev/null
+++ b/include/hw/s390x/ap-device.h
@@ -0,0 +1,38 @@
+/*
+ * Adjunct Processor (AP) matrix device interfaces
+ *
+ * Copyright 2018 IBM Corp.
+ * Author(s): Tony Krowiak <address@hidden>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+#ifndef HW_S390X_AP_DEVICE_H
+#define HW_S390X_AP_DEVICE_H
+
+#define AP_DEVICE_TYPE       "ap-device"
+
+typedef struct APDevice {
+    DeviceState parent_obj;
+} APDevice;
+
+typedef struct APDeviceClass {
+    DeviceClass parent_class;
+} APDeviceClass;
+
+static inline APDevice *to_ap_dev(DeviceState *dev)
+{
+    return container_of(dev, APDevice, parent_obj);
+}
+
+#define AP_DEVICE(obj) \
+    OBJECT_CHECK(APDevice, (obj), AP_DEVICE_TYPE)
+
+#define AP_DEVICE_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(APDeviceClass, (obj), AP_DEVICE_TYPE)
+
+#define AP_DEVICE_CLASS(klass) \
+    OBJECT_CLASS_CHECK(APDeviceClass, (klass), AP_DEVICE_TYPE)

Do you really need any of these definitions except AP_DEVICE_TYPE ?

Yes, we need AP_DEVICE(obj) and struct APDevice; they are both used in
patch 5/6. We can probably get rid of AP_DEVICE_GET_CLASS(obj) and
AP_DEVICE_CLASS(klass), but aren't those typically included in all
QOM definitions?


Same here, I think.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]