diff mbox series

[1/1] s390x/css: drop data-check in interpretation

Message ID 20170907115700.69911-1-pasic@linux.vnet.ibm.com
State New
Headers show
Series [1/1] s390x/css: drop data-check in interpretation | expand

Commit Message

Halil Pasic Sept. 7, 2017, 11:57 a.m. UTC 
The architecture says that channel-data check is indicating that
an uncorrected storage (memory) error has been detected in regard
to the data residing in main storage (memory) that is currently
used for an I/O operation. The described detection is done using
the CBC technology.

The ccw interpretation code is however generating a channel-data check
effectively when the (device specific) ccw_cb returns -EFAULT.  In case
of virtio-ccw devices this happens when mapping memory fails, or when a
NULL pointer is encountered. So this behavior is not architecture
conform.

Furthermore the best fit for these situations (null pointer, mapping a
piece of guest memory fails) from architectural perspective the condition
described as the channel subsystem refers to a location that is not
available, which when encountered shall result in a channel-program
check.

To fix this, all we have to do is to get rid of the switch case matching
-EFAULT: the default is generating a channel-program check.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---

References
-----------

1. paragraph: PoP page 16-42 "Channel-Data Check".
3. paragraph: PoP page 15-59 "Designation of Storage Area"

Side note on usage of error codes
----------------------------------

I'm currently having a patch set under discussion which aims to move away
from mapping CSS conditions to error codes and back for the IO
instruction handlers. IMHO as of today the given usage of the error codes
is rather obfuscating the source code than contributing to clarity. I've
experimented with a bunch of changes to improve on the readability of the
code, but did not settle about if and what to do yet (the main problem is
the gains not solid and quite limited -- it's hard to justify churn).

If there is interest I would be happy to write up an RFC and start a
discussion on what makes sense and what does not.
---
 hw/s390x/css.c | 9 ---------
 1 file changed, 9 deletions(-)
diff mbox series

Patch

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 1880b1a0ff..72fb193676 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -980,15 +980,6 @@  static void sch_handle_start_func_virtual(SubchDev *sch)
                     SCSW_STCTL_ALERT | SCSW_STCTL_STATUS_PEND;
             s->cpa = sch->channel_prog + 8;
             break;
-        case -EFAULT:
-            /* memory problem, generate channel data check */
-            s->ctrl &= ~SCSW_ACTL_START_PEND;
-            s->cstat = SCSW_CSTAT_DATA_CHECK;
-            s->ctrl &= ~SCSW_CTRL_MASK_STCTL;
-            s->ctrl |= SCSW_STCTL_PRIMARY | SCSW_STCTL_SECONDARY |
-                    SCSW_STCTL_ALERT | SCSW_STCTL_STATUS_PEND;
-            s->cpa = sch->channel_prog + 8;
-            break;
         case -EBUSY:
             /* subchannel busy, generate deferred cc 1 */
             s->flags &= ~SCSW_FLAGS_MASK_CC;