s390x/css: ccw interpretation fixes

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.

---

Was posted as stand alone patch. See:
http://patchwork.ozlabs.org/patch/810995/

If you are going to, *please review there*.

Signed-off-by Halil Pasic <[email protected]>

---
 hw/s390x/css.c | 9 ---------
 1 file changed, 9 deletions(-)

hw/s390x/css.c +0 -2

index 901dc6a0f3..09f6ba0310 100644
@@ -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 */

Quoted 21 lines · On Fri, 8 Sep 2017 17:24:43 +0200 Halil Pasic <[email protected]> wrote:

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.

Quoted 6 lines

---

Was posted as stand alone patch. See:
http://patchwork.ozlabs.org/patch/810995/

If you are going to, *please review there*.

Countered. *Please review here*

(It makes it easier for me.)

Quoted 2 lines

Signed-off-by Halil Pasic <[email protected]>

Your s-o-b should go before the triple-dash delimiter. I can fix that up.

Quoted 19 lines

---
 hw/s390x/css.c | 9 ---------
 1 file changed, 9 deletions(-)

hw/s390x/css.c +0 -6

index 901dc6a0f3..09f6ba0310 100644
@@ -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 |

hw/s390x/css.c static void sch_handle_start_func_virtual(SubchDev *sch) · line 985

Quoted 6 lines

hw/s390x/css.c +0 -3

-                    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;

Looks reasonable. Queued.

Back then in the time of df1fe5bb49 ("s390: Virtual channel subsystem support.", 2013-01-24) -EIO used to map to a channel-program check (via the default label of the switch statement). Then 2dc95b4cac ("s390x/3270: 3270 data stream handling", 2016-04-01) came along and that changed dramatically.

Let us roll back this undesired side effect, and go back to channel-program check.

Signed-off-by Halil Pasic <[email protected]>

Fixes 2dc95b4cac "s390x/3270: 3270 data stream handling"

---

I'm not sure 0 as CCW address it's strictly illegal. Yes 0 is an unlikely address for a CCW but I would appreciate a PoP reference clarifying this...

Another reason to not use Unix/POSIX error codes like this.

---
 hw/s390x/css.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

hw/s390x/css.c +0 -0

index 09f6ba0310..a44d87ab3e 100644

The case in question actually never happens. Let us get rid of the dead code.

Signed-off-by Halil Pasic <[email protected]>

---
 hw/s390x/css.c | 7 -------
 1 file changed, 7 deletions(-)

hw/s390x/css.c +0 -0

index a44d87ab3e..a9cdd54efc 100644
@@ -980,13 +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;

On Fri, 8 Sep 2017 17:24:45 +0200
Halil Pasic <[email protected]> wrote:

s/dubious/unused/

Quoted 2 lines

The case in question actually never happens. Let us get rid of the dead code.

I had tried to be complete in my initial implementation. With the current implementation, it can never happen, yes. We can easily resurrect it from git history should we need it in the future.

Quoted 18 lines

Signed-off-by Halil Pasic <[email protected]>

---
 hw/s390x/css.c | 7 -------
 1 file changed, 7 deletions(-)

hw/s390x/css.c +0 -4

index a44d87ab3e..a9cdd54efc 100644
@@ -980,13 +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 -EBUSY:
-            /* subchannel busy, generate deferred cc 1 */
-            s->flags &= ~SCSW_FLAGS_MASK_CC;
-            s->flags |= (1 << 8);

hw/s390x/css.c static void sch_handle_start_func_virtual(SubchDev *sch) · line 985

Quoted 6 lines

hw/s390x/css.c +0 -3

-            s->ctrl &= ~SCSW_CTRL_MASK_STCTL;
-            s->ctrl |= SCSW_STCTL_ALERT | SCSW_STCTL_STATUS_PEND;
-            break;
         case -EINPROGRESS:
             /* channel program has been suspended */
             s->ctrl &= ~SCSW_ACTL_START_PEND;

Thanks, queued.

Quoted 10 lines · Cornelia Huck

Quoted 2 lines · On Fri, 8 Sep 2017 17:24:45 +0200 Halil Pasic <[email protected]> wrote:

The case in question actually never happens. Let us get rid of the dead code.

I had tried to be complete in my initial implementation. With the current implementation, it can never happen, yes. We can easily resurrect it from git history should we need it in the future.

I've tried to understand the branch but I failed. Weather the subchannel is busy or not should be already decided, and if the device is busy that is probably supposed to be indicated via device status (word 2 bit 3).

So I wrote dubious because I could not figure it out. But unused is certainly nicer.

I think shall the need emerge the preferred way of handling would be to use the custom handling (-EIO) branch.

We report incorrect length via SCSW program check instead of incorrect length check (SCWS word 2 bit 10 instead of bit 9). Since we have there is no fitting errno for incorrect length, and since I don't like what we do with the errno's, as part of the fix, errnos used for control flow in ccw interpretation are replaced with an enum using more speaking names.

For virtio, if incorrect length checking is suppressed we keep the current behavior (channel-program check).

Signed-off-by Halil Pasic <[email protected]>

---
 hw/s390x/3270-ccw.c    |  24 +++++-----
 hw/s390x/css.c         |  67 +++++++++++++++-----------
 hw/s390x/virtio-ccw.c  | 128 ++++++++++++++++++++++++-------------------------
 include/hw/s390x/css.h |  13 ++++-
 4 files changed, 127 insertions(+), 105 deletions(-)

Quoted 5 lines · On Fri, 8 Sep 2017 17:24:46 +0200 Halil Pasic <[email protected]> wrote:

We report incorrect length via SCSW program check instead of incorrect length check (SCWS word 2 bit 10 instead of bit 9). Since we have there is no fitting errno for incorrect length, and since I don't like what we do with the errno's, as part of the fix, errnos used for control flow in ccw interpretation are replaced with an enum using more speaking names.

I'm not sure whether this is the way to go. I mainly dislike the size of the patch (and the fact that it mixes a fix and a change of function signature).

Can we instead choose a mapping for incorrect length, and defer a possible rework?

(Another idea would be to have the callback prepare the scsw via helper functions. We'd just keep -EAGAIN to keep processing a chain and 0 to stop.)

Quoted 3 lines

For virtio, if incorrect length checking is suppressed we keep the current behavior (channel-program check).

Confused. If it is suppressed, there should not be an error, no?

From VIRTIO 1.0 4.3.1.2 Device Requirements: Basic Concepts

Quoted 9 lines

Signed-off-by Halil Pasic <[email protected]>

---
 hw/s390x/3270-ccw.c    |  24 +++++-----
 hw/s390x/css.c         |  67 +++++++++++++++-----------
 hw/s390x/virtio-ccw.c  | 128 ++++++++++++++++++++++++-------------------------
 include/hw/s390x/css.h |  13 ++++-
 4 files changed, 127 insertions(+), 105 deletions(-)

Quoted 9 lines · Cornelia Huck

Quoted 5 lines · On Fri, 8 Sep 2017 17:24:46 +0200 Halil Pasic <[email protected]> wrote:

We report incorrect length via SCSW program check instead of incorrect length check (SCWS word 2 bit 10 instead of bit 9). Since we have there is no fitting errno for incorrect length, and since I don't like what we do with the errno's, as part of the fix, errnos used for control flow in ccw interpretation are replaced with an enum using more speaking names.

Quoted 3 lines

I'm not sure whether this is the way to go. I mainly dislike the size of the patch (and the fact that it mixes a fix and a change of function signature).

Do you agree that we should move away from POSIX errno codes? I think if we do, this cant' get much smaller.

Quoted 5 lines

Can we instead choose a mapping for incorrect length, and defer a possible rework?

In the commit message, I say that I don't have a fitting errno. If you tell me which one to use, I would be glad to split this up. I don't like mixing re-factoring and changing behavior myself.

Can I have your position on the re-factoring (that is let us imagine I did not change handling for incorrect length)?

Quoted 5 lines

(Another idea would be to have the callback prepare the scsw via helper functions. We'd just keep -EAGAIN to keep processing a chain and 0 to stop.)

That was my first idea how to improve on this. I should still have the code (patches), but I'm not sure whether it's clean or lumped together with other experiments.

After pushing the handling down the call chain (caller would use inline functions to manipulate SCSW), I've realized that it does not buy us much/anything expect the better names, while we get the machine code manipulating the SCSW generated in multiple instead of in one place. I also showed the results to Dong Jia and he was ambivalent too: said something like it does look better, but it ain't better enough to make it worthwhile.

This is why I've decided to go with a less intrusive approach:
just change the names so that it's obvious what's happening.

Quoted 4 lines

Quoted 2 lines

For virtio, if incorrect length checking is suppressed we keep the current behavior (channel-program check).

Confused. If it is suppressed, there should not be an error, no?

No.

From VIRTIO 1.0 4.3.1.2 Device Requirements: Basic Concepts

"If a driver did suppress length checks for a channel command, the device MUST present a check condition if the transmitted data does not contain enough data to process the command."
(http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-1230001)

So for virtio we have to present a check condition. Architecturally it might look better if the one refusing is the device and not the CSS, but for that we would have to change the VIRTIO spec. With the given constraints a program check is IMHO the best fit.

Quoted 10 lines · On Mon, 11 Sep 2017 13:36:29 +0200 Halil Pasic <[email protected]> wrote:

Quoted 9 lines · Cornelia Huck

Quoted 5 lines · On Fri, 8 Sep 2017 17:24:46 +0200 Halil Pasic <[email protected]> wrote:

We report incorrect length via SCSW program check instead of incorrect length check (SCWS word 2 bit 10 instead of bit 9). Since we have there is no fitting errno for incorrect length, and since I don't like what we do with the errno's, as part of the fix, errnos used for control flow in ccw interpretation are replaced with an enum using more speaking names.

Quoted 6 lines

Quoted 3 lines

I'm not sure whether this is the way to go. I mainly dislike the size of the patch (and the fact that it mixes a fix and a change of function signature).

Do you agree that we should move away from POSIX errno codes? I think if we do, this cant' get much smaller.

I'm not really a fan of defining our own return values, tbh.

Quoted 10 lines

Quoted 5 lines

Can we instead choose a mapping for incorrect length, and defer a possible rework?

In the commit message, I say that I don't have a fitting errno. If you tell me which one to use, I would be glad to split this up. I don't like mixing re-factoring and changing behavior myself.

Quoted 2 lines

Can I have your position on the re-factoring (that is let us imagine I did not change handling for incorrect length)?

If there is no return code that can be made to fit, we probably won't be able to get around some kind of refactoring... but then I'd prefer to do the refactoring first and the fix second.

Quoted 18 lines

Quoted 5 lines

(Another idea would be to have the callback prepare the scsw via helper functions. We'd just keep -EAGAIN to keep processing a chain and 0 to stop.)

That was my first idea how to improve on this. I should still have the code (patches), but I'm not sure whether it's clean or lumped together with other experiments.

After pushing the handling down the call chain (caller would use inline functions to manipulate SCSW), I've realized that it does not buy us much/anything expect the better names, while we get the machine code manipulating the SCSW generated in multiple instead of in one place. I also showed the results to Dong Jia and he was ambivalent too: said something like it does look better, but it ain't better enough to make it worthwhile.

Quoted 2 lines

This is why I've decided to go with a less intrusive approach:
just change the names so that it's obvious what's happening.

Something like return channel_program_check(...); or so would be quite obvious, I think.

But yes, it will be easier to evaluate this for an actual patch ;)

Quoted 14 lines

Quoted 4 lines

Quoted 2 lines

For virtio, if incorrect length checking is suppressed we keep the current behavior (channel-program check).

Confused. If it is suppressed, there should not be an error, no?

No.

From VIRTIO 1.0 4.3.1.2 Device Requirements: Basic Concepts

"If a driver did suppress length checks for a channel command, the device MUST present a check condition if the transmitted data does not contain enough data to process the command."
(http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-1230001)

Quoted 4 lines

So for virtio we have to present a check condition. Architecturally it might look better if the one refusing is the device and not the CSS, but for that we would have to change the VIRTIO spec. With the given constraints a program check is IMHO the best fit.

Ah, but that's not general length checking for virtio-ccw :)

Reword the sentence to use 'short data with incorrect length checking suppressed' or so?

Quoted 13 lines · Cornelia Huck

Quoted 10 lines · On Mon, 11 Sep 2017 13:36:29 +0200 Halil Pasic <[email protected]> wrote:

Quoted 9 lines · Cornelia Huck

Quoted 5 lines · On Fri, 8 Sep 2017 17:24:46 +0200 Halil Pasic <[email protected]> wrote:

We report incorrect length via SCSW program check instead of incorrect length check (SCWS word 2 bit 10 instead of bit 9). Since we have there is no fitting errno for incorrect length, and since I don't like what we do with the errno's, as part of the fix, errnos used for control flow in ccw interpretation are replaced with an enum using more speaking names.

Quoted 9 lines

Quoted 6 lines

Quoted 3 lines

I'm not sure whether this is the way to go. I mainly dislike the size of the patch (and the fact that it mixes a fix and a change of function signature).

Do you agree that we should move away from POSIX errno codes? I think if we do, this cant' get much smaller.

I'm not really a fan of defining our own return values, tbh.

I've suspected. But your statement, although being useful, does not answer my question. I think we need to agree on this question before proceeding.

In my opinion both the EIO bug and this bug are great examples why the POSIX errno codes are sub-optimal and misleading, but that's my opinion.

Quoted 11 lines

Quoted 11 lines

Quoted 6 lines

Can we instead choose a mapping for incorrect length, and defer a possible rework?

In the commit message, I say that I don't have a fitting errno. If you tell me which one to use, I would be glad to split this up. I don't like mixing re-factoring and changing behavior myself.

Quoted 7 lines

Quoted 2 lines

Can I have your position on the re-factoring (that is let us imagine I did not change handling for incorrect length)?

If there is no return code that can be made to fit, we probably won't be able to get around some kind of refactoring... but then I'd prefer to do the refactoring first and the fix second.

That is a can do. I dislike refactoring known bugs, because fixing bugs is usually higher priority than making the code nicer, or even marginally faster. (Btw I found these while trying to refactor.) This however is a weak principle of mine and can be easily overpowered by a maintainer request for example.

Quoted 19 lines

Quoted 18 lines

Quoted 4 lines

Quoted 2 lines

For virtio, if incorrect length checking is suppressed we keep the current behavior (channel-program check).

Confused. If it is suppressed, there should not be an error, no?

No.

From VIRTIO 1.0 4.3.1.2 Device Requirements: Basic Concepts

"If a driver did suppress length checks for a channel command, the device MUST present a check condition if the transmitted data does not contain enough data to process the command."
(http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-1230001)

So for virtio we have to present a check condition. Architecturally it might look better if the one refusing is the device and not the CSS, but for that we would have to change the VIRTIO spec. With the given constraints a program check is IMHO the best fit.

Quoted 1 line

Ah, but that's not general length checking for virtio-ccw :)

What is general length checking for virtio-ccw? Did I say it was general length checking for virtio-ccw?

Quoted 28 lines · On Tue, 12 Sep 2017 17:43:03 +0200 Halil Pasic <[email protected]> wrote:

Quoted 13 lines · Cornelia Huck

Quoted 10 lines · On Mon, 11 Sep 2017 13:36:29 +0200 Halil Pasic <[email protected]> wrote:

Quoted 9 lines · Cornelia Huck

Quoted 5 lines · On Fri, 8 Sep 2017 17:24:46 +0200 Halil Pasic <[email protected]> wrote:

We report incorrect length via SCSW program check instead of incorrect length check (SCWS word 2 bit 10 instead of bit 9). Since we have there is no fitting errno for incorrect length, and since I don't like what we do with the errno's, as part of the fix, errnos used for control flow in ccw interpretation are replaced with an enum using more speaking names.

Quoted 9 lines

Quoted 6 lines

Quoted 3 lines

I'm not sure whether this is the way to go. I mainly dislike the size of the patch (and the fact that it mixes a fix and a change of function signature).

Do you agree that we should move away from POSIX errno codes? I think if we do, this cant' get much smaller.

I'm not really a fan of defining our own return values, tbh.

I've suspected. But your statement, although being useful, does not answer my question. I think we need to agree on this question before proceeding.

Quoted 3 lines

In my opinion both the EIO bug and this bug are great examples why the POSIX errno codes are sub-optimal and misleading, but that's my opinion.

It depends. I prefer them over home-grown ones.

(And I tend to dislike absolute statements.)

Quoted 19 lines

Quoted 11 lines

Quoted 11 lines

Quoted 6 lines

Can we instead choose a mapping for incorrect length, and defer a possible rework?

In the commit message, I say that I don't have a fitting errno. If you tell me which one to use, I would be glad to split this up. I don't like mixing re-factoring and changing behavior myself.

Quoted 7 lines

Quoted 2 lines

Can I have your position on the re-factoring (that is let us imagine I did not change handling for incorrect length)?

If there is no return code that can be made to fit, we probably won't be able to get around some kind of refactoring... but then I'd prefer to do the refactoring first and the fix second.

Quoted 5 lines

That is a can do. I dislike refactoring known bugs, because fixing bugs is usually higher priority than making the code nicer, or even marginally faster. (Btw I found these while trying to refactor.) This however is a weak principle of mine and can be easily overpowered by a maintainer request for example.

If a good fix requires refactoring, I'd prefer to do the refactoring first. I'd prefer an ugly fix first only for serious issues (and I don't think that one counts as one.)

Quoted 19 lines

Quoted 19 lines

Quoted 18 lines

Quoted 4 lines

Quoted 2 lines

For virtio, if incorrect length checking is suppressed we keep the current behavior (channel-program check).

Confused. If it is suppressed, there should not be an error, no?

No.

From VIRTIO 1.0 4.3.1.2 Device Requirements: Basic Concepts

"If a driver did suppress length checks for a channel command, the device MUST present a check condition if the transmitted data does not contain enough data to process the command."
(http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-1230001)

So for virtio we have to present a check condition. Architecturally it might look better if the one refusing is the device and not the CSS, but for that we would have to change the VIRTIO spec. With the given constraints a program check is IMHO the best fit.

Quoted 4 lines

Quoted 1 line

Ah, but that's not general length checking for virtio-ccw :)

What is general length checking for virtio-ccw? Did I say it was general length checking for virtio-ccw?

Hm? Generally, suppressing is supposed to allow incorrect length specifications. For virtio-ccw, that only applies to 'too much' and not 'not enough'.

Also, reading the statement in the spec: It only talks about a 'check condition', not _which_ one - so there's no requirement to keep a channel-program check (other than possibly confusing guests)?

Quoted 31 lines · Cornelia Huck

Quoted 28 lines · On Tue, 12 Sep 2017 17:43:03 +0200 Halil Pasic <[email protected]> wrote:

Quoted 13 lines · Cornelia Huck

Quoted 10 lines · On Mon, 11 Sep 2017 13:36:29 +0200 Halil Pasic <[email protected]> wrote:

Quoted 9 lines · Cornelia Huck

Quoted 5 lines · On Fri, 8 Sep 2017 17:24:46 +0200 Halil Pasic <[email protected]> wrote:

We report incorrect length via SCSW program check instead of incorrect length check (SCWS word 2 bit 10 instead of bit 9). Since we have there is no fitting errno for incorrect length, and since I don't like what we do with the errno's, as part of the fix, errnos used for control flow in ccw interpretation are replaced with an enum using more speaking names.

Quoted 9 lines

Quoted 6 lines

Quoted 3 lines

I'm not sure whether this is the way to go. I mainly dislike the size of the patch (and the fact that it mixes a fix and a change of function signature).

Do you agree that we should move away from POSIX errno codes? I think if we do, this cant' get much smaller.

I'm not really a fan of defining our own return values, tbh.

I've suspected. But your statement, although being useful, does not answer my question. I think we need to agree on this question before proceeding.

Quoted 8 lines

Quoted 3 lines

In my opinion both the EIO bug and this bug are great examples why the POSIX errno codes are sub-optimal and misleading, but that's my opinion.

It depends. I prefer them over home-grown ones.

(And I tend to dislike absolute statements.)

Ah, we may have a misunderstanding here. POSIX errno codes are great if they are used for what they are supposed to. The context was meant to be implicitly included in the statement limiting it's scope.

Other than spotting a possible misunderstanding (I hope I did not misunderstand what do you mean by absolute statements myself) this did not bring me any further.

Quoted 24 lines

Quoted 19 lines

Quoted 19 lines

Quoted 18 lines

Quoted 4 lines

Quoted 2 lines

For virtio, if incorrect length checking is suppressed we keep the current behavior (channel-program check).

Confused. If it is suppressed, there should not be an error, no?

No.

From VIRTIO 1.0 4.3.1.2 Device Requirements: Basic Concepts

"If a driver did suppress length checks for a channel command, the device MUST present a check condition if the transmitted data does not contain enough data to process the command."
(http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-1230001)

So for virtio we have to present a check condition. Architecturally it might look better if the one refusing is the device and not the CSS, but for that we would have to change the VIRTIO spec. With the given constraints a program check is IMHO the best fit.

Quoted 4 lines

Quoted 1 line

Ah, but that's not general length checking for virtio-ccw :)

What is general length checking for virtio-ccw? Did I say it was general length checking for virtio-ccw?

Quoted 8 lines

Hm? Generally, suppressing is supposed to allow incorrect length specifications. For virtio-ccw, that only applies to 'too much' and not 'not enough'.

Also, reading the statement in the spec: It only talks about a 'check condition', not _which_ one - so there's no requirement to keep a channel-program check (other than possibly confusing guests)?

.
You are right, and I was wrong. We could also present an unit-check (that's also check -- and is the only one in device status. The spec even says the 'device must present', although I device is in virtio sense and not in PoP sense here, and does not use 'present subchannel status' as in the previous sentence. For a unit check I would have expected the some sense bit specified to though (like it's specified that under certain conditions we have to do an unit check with a command reject (that is sense bit 0). What shall we do in your opinion?