diff mbox series

[v3,1/7] s390x/css: be more consistent if broken beyond repair

Message ID 20171017140453.51099-2-pasic@linux.vnet.ibm.com
State New
Headers show
Series improve error handling for IO instr | expand

Commit Message

Halil Pasic Oct. 17, 2017, 2:04 p.m. UTC 
Calling do_subchannel_work with no function control flags set in SCSW is
a programming error. Currently the handle this differently in
do_subchannel_work_virtual and do_subchannel_work_passthrough. Let's be
consistent and guard with a common assert against this programming error.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---
Already applied to Connies s390-next. Included for the sake of completenes
(with) the old typo in the commit message.
---
 hw/s390x/css.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

Comments

Thomas Huth Oct. 18, 2017, 8:13 a.m. UTC | #1 
On 17.10.2017 16:04, Halil Pasic wrote:
> Calling do_subchannel_work with no function control flags set in SCSW is
> a programming error. Currently the handle this differently in
> do_subchannel_work_virtual and do_subchannel_work_passthrough. Let's be
> consistent and guard with a common assert against this programming error.
> 
> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> ---
> Already applied to Connies s390-next. Included for the sake of completenes
> (with) the old typo in the commit message.

Looks like Cornelia fixed it up in s390-next :-)

> ---
>  hw/s390x/css.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)

FWIW:

Reviewed-by: Thomas Huth <thuth@redhat.com>
diff mbox series

Patch

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 35683d7954..aa233d5f8a 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -1245,9 +1245,6 @@  int do_subchannel_work_virtual(SubchDev *sch)
     } else if (s->ctrl & SCSW_FCTL_START_FUNC) {
         /* Triggered by both ssch and rsch. */
         sch_handle_start_func_virtual(sch);
-    } else {
-        /* Cannot happen. */
-        return 0;
     }
     css_inject_io_interrupt(sch);
     return 0;
@@ -1255,22 +1252,17 @@  int do_subchannel_work_virtual(SubchDev *sch)
 
 int do_subchannel_work_passthrough(SubchDev *sch)
 {
-    int ret;
+    int ret = 0;
     SCSW *s = &sch->curr_status.scsw;
 
     if (s->ctrl & SCSW_FCTL_CLEAR_FUNC) {
         /* TODO: Clear handling */
         sch_handle_clear_func(sch);
-        ret = 0;
     } else if (s->ctrl & SCSW_FCTL_HALT_FUNC) {
         /* TODO: Halt handling */
         sch_handle_halt_func(sch);
-        ret = 0;
     } else if (s->ctrl & SCSW_FCTL_START_FUNC) {
         ret = sch_handle_start_func_passthrough(sch);
-    } else {
-        /* Cannot happen. */
-        return -ENODEV;
     }
 
     return ret;
@@ -1278,11 +1270,11 @@  int do_subchannel_work_passthrough(SubchDev *sch)
 
 static int do_subchannel_work(SubchDev *sch)
 {
-    if (sch->do_subchannel_work) {
-        return sch->do_subchannel_work(sch);
-    } else {
+    if (!sch->do_subchannel_work) {
         return -EINVAL;
     }
+    g_assert(sch->curr_status.scsw.ctrl & SCSW_CTRL_MASK_FCTL);
+    return sch->do_subchannel_work(sch);
 }
 
 static void copy_pmcw_to_guest(PMCW *dest, const PMCW *src)