On 07/26/2017 05:01 AM, Dong Jia Shi wrote: > Hello Halil, > > * Halil Pasic <pa...@linux.vnet.ibm.com> [2017-07-26 00:44:42 +0200]: > >> According to the PoP bit positions 0-3 and 8-32 of the format-1 CCW must >> contain zeros. Bits 0-3 are already covered by cmd_code validity >> checking, and bit 32 is covered by the CCW address checking. >> >> Bits 8-31 correspond to CCW1.flags and CCW1.count. Currently we only >> check for the absence of certain flags. Let's fix this. >> >> Signed-off-by: Halil Pasic <pa...@linux.vnet.ibm.com> >> --- >> hw/s390x/css.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/hw/s390x/css.c b/hw/s390x/css.c >> index d17e21b7af..1f04ce4a1b 100644 >> --- a/hw/s390x/css.c >> +++ b/hw/s390x/css.c >> @@ -884,7 +884,8 @@ static int css_interpret_ccw(SubchDev *sch, hwaddr >> ccw_addr, >> ret = -EINVAL; >> break; >> } >> - if (ccw.flags & (CCW_FLAG_CC | CCW_FLAG_DC)) { >> + if (ccw.flags || ccw.count) { >> + /* We have already sanitized these if fmt 0. */ > ccw0 does not have the same restrictions as ccw1. We don't sanitize > these for ccw0. >
Yes you have misunderstood. For fmt 1 these bits have to be zero otherwise a program-check condition is to be recognized. Thus we don't sanitize for fmt 1. For fmt 0 these bits are ignored. We sanitize them in for some time now by setting them to zero when making a CCW1 out of an CCW0. If we would recognize a program-check for fmt 0 that would be wrong. The comment tells us why this code is good for CCW0 too, and why can we omit sch->ccw_fmt_1 from the conditon. Regards, Halil > (This comment is still here. Did I misunderstand things? :) > >> ret = -EINVAL; >> break; >> } >> -- >> 2.11.2 >> > > With the comment removed: > Reviewed-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> >