Am 21.09.26 um 20:43 schrieb Eric Farman:
The channel subsystem will reject channel programs that incorporate
256 CCWs without a data transfer, but the existing check to account
for this only does so for CCWs with a data address of zero. Other
valid CCWs that wouldn't transfer data are not accounted for.
Rather than re-inventing the wheel, use the output of the datastream
at the end of the entire CCW parsing tree to indicate whether data
was moved or not.
Cc: [email protected]
Fixes: e8601dd5d0 ("s390x/css: catch ccw sequence errors")
Signed-off-by: Eric Farman <[email protected]>
---
hw/s390x/css.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 9da9128d88..10bebdcb1a 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -1016,13 +1016,6 @@ static int css_interpret_ccw(SubchDev *sch, hwaddr
ccw_addr,
check_len = !((ccw.flags & CCW_FLAG_SLI) && !(ccw.flags & CCW_FLAG_DC));
- if (!ccw.cda) {
- if (sch->ccw_no_data_cnt == 255) {
- return -EINVAL;
- }
- sch->ccw_no_data_cnt++;
- }
-
/* Look at the command. */
ccw_dstream_init(&sch->cds, &ccw, &(sch->orb));
switch (ccw.cmd_code) {
@@ -1110,6 +1103,19 @@ static int css_interpret_ccw(SubchDev *sch, hwaddr
ccw_addr,
}
}
These two context lines are the end of
if (ret == 0) {
if (ccw.flags & CCW_FLAG_CC) {
sch->channel_prog += 8;
ret = -EAGAIN;
}
}
so by the time we get here, every successfully executed CCW that has
command chaining set already has ret == -EAGAIN.
+ /*
+ * A CCW that transfers no data is allowed, but ensure an upper limit
+ * is established to prevent long-running channel programs that don't
+ * move actual data.
+ */
+ if (ret == 0 && sch->cds.at_byte == 0) {
+ if (sch->ccw_no_data_cnt == 255) {
+ ret = -EINVAL;
+ } else {
+ sch->ccw_no_data_cnt++;
+ }
+ }
+
return ret;
}
Which means this never fires for a chained CCW. The only CCW that can
still be counted is the last one of the program (no CC), and
ccw_no_data_cnt is reset on every SSCH, so the counter can never reach
255. A chain of 100000 NOOPs with CC now runs to completion, while HEAD
rejects it at the 256th CCW when cda is zero. As posted this removes the
protection from e8601dd5d0 rather than extending it.
Moving the check above the CC block (right after last_cmd_valid = true)
should do it, no?