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 12:31:55 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0

On 10/08/2018 10:44 AM, Thomas Huth wrote:
On 2018-10-08 16:20, Tony Krowiak wrote:
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.

Fine for me, if you replace the DO_UPCAST in patch 5 with AP_DEVICE().

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?

As long as you don't really need them, I'd simply remove them. They can
be added back when some code really needs them.

That is the plan


  Thomas





reply via email to

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