qemu-s390x
[Top][All Lists]
Advanced

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

Re: [qemu-s390x] [PATCH v5] vfio-ccw: support async command subregion


From: Farhan Ali
Subject: Re: [qemu-s390x] [PATCH v5] vfio-ccw: support async command subregion
Date: Tue, 11 Jun 2019 15:37:12 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0



On 06/11/2019 07:37 AM, Cornelia Huck wrote:
On Fri, 7 Jun 2019 11:19:09 -0400
Farhan Ali <address@hidden> wrote:

On 06/07/2019 11:09 AM, Cornelia Huck wrote:
On Fri, 7 Jun 2019 11:02:36 -0400
Farhan Ali <address@hidden> wrote:
On 06/07/2019 10:53 AM, Cornelia Huck wrote:

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index ad310b9f94bc..b92395f165e6 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -22,6 +22,7 @@
    #include "trace.h"
    #include "hw/s390x/s390_flic.h"
    #include "hw/s390x/s390-virtio-ccw.h"
+#include "hw/s390x/s390-ccw.h"
typedef struct CrwContainer {
        CRW crw;
@@ -1205,6 +1206,26 @@ static void sch_handle_start_func_virtual(SubchDev *sch)
} +static void sch_handle_halt_func_passthrough(SubchDev *sch)
+{
+    int ret;
+
+    ret = s390_ccw_halt(sch);
+    if (ret == -ENOSYS) {
+        sch_handle_halt_func(sch);
+    }
+}
+
+static void sch_handle_clear_func_passthrough(SubchDev *sch)
+{
+    int ret;
+
+    ret = s390_ccw_clear(sch);
+    if (ret == -ENOSYS) {
+        sch_handle_clear_func(sch);
+    }
+}
+

do we need an extra s390_ccw_clear/halt functions? can't we just call
cdc->clear/halt in the passthrough functions?

I mostly added them for symmetry reasons... we still need to check for
presence of the callback in any case, though.

(vfio is not always built, e.g. on windows or os x.)


right, but if we are calling do_subchannel_work_passthrough, then we
know for sure we are building the S390CCWDevice which is the vfio
device, no?

So we could just add checks for callbacks in
sch_handle_clear/halt_func_passthrough, no?

I would even like to get rid of the s390_ccw_cmd_request if we can, but
that is out of scope for this patch. :)

Ok, I just walked through various source files (some of which are a bit
confusingly named) again and now I understand again why it was done
that way in the first place.

- hw/s390x/s390-ccw.c provides some interfaces for pass-through ccw
   devices. It is built unconditionally, and its interfaces are called
   unconditionally from the css code.
   It also provides a device class where code can hook up callbacks.
- hw/vfio/ccw.c (which is not built for !KVM) actually hooks up
   callbacks in that device class.

So, s390-ccw.c (not to be confused with ccw-device.c...) provides a
layer that makes it possible to call things unconditionally, regardless
whether we have vfio-ccw available or not. Not that the code ends up
being called without vfio-ccw support; but the class indirection
enables the code to be built.


Okay, now I get it. Thanks for the explanation, I really do appreciate it! :)

There's possibly a way to make this work without the class indirection
as well, but I think we'd end up doing code juggling before ending up
with something that's not really nicer than the code we have now.
Therefore, I'd prefer to keep the class handling and hook up the
halt/clear callbacks there as well.



Yeah I agree, given the constraints I don't think any code juggling would look any prettier.

Thanks
Farhan




reply via email to

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