[v4,4/5] 390x/css: introduce maximum data address checking

The architecture mandates the addresses to be accessed on the first
indirection level (that is, the data addresses without IDA, and the
(M)IDAW addresses with (M)IDA) to be checked against an CCW format
dependent limit maximum address.  If a violation is detected, the storage
access is not to be performed and a channel program check needs to be
generated. As of today, we fail to do this check.

Let us stick even closer to the architecture specification.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---
 hw/s390x/css.c         | 10 ++++++++++
 include/hw/s390x/css.h |  1 +
 2 files changed, 11 insertions(+)

On 21/09/2017 20:08, Halil Pasic wrote:
> The architecture mandates the addresses to be accessed on the first
> indirection level (that is, the data addresses without IDA, and the
> (M)IDAW addresses with (M)IDA) to be checked against an CCW format
> dependent limit maximum address.  If a violation is detected, the storage
> access is not to be performed and a channel program check needs to be
> generated. As of today, we fail to do this check.
> 
> Let us stick even closer to the architecture specification.
> 
> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> ---
>   hw/s390x/css.c         | 10 ++++++++++
>   include/hw/s390x/css.h |  1 +
>   2 files changed, 11 insertions(+)
> 
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index e0d989829f..cd5580ebb8 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -795,6 +795,11 @@ static inline int cds_check_len(CcwDataStream *cds, int len)
>       return cds->flags & CDS_F_STREAM_BROKEN ? -EINVAL : len;
>   }
> 
> +static inline bool cds_ccw_addrs_ok(hwaddr addr, int len, bool ccw_fmt1)
> +{
> +    return (addr + len) < (ccw_fmt1 ? (1UL << 31) : (1UL << 24));
> +}
> +
>   static int ccw_dstream_rw_noflags(CcwDataStream *cds, void *buff, int len,
>                                     CcwDataStreamOp op)
>   {
> @@ -804,6 +809,9 @@ static int ccw_dstream_rw_noflags(CcwDataStream *cds, void *buff, int len,
>       if (ret <= 0) {
>           return ret;
>       }
> +    if (!cds_ccw_addrs_ok(cds->cda, len, cds->flags & CDS_F_FMT)) {
> +        return -EINVAL; /* channel program check */
> +    }
>       if (op == CDS_OP_A) {
>           goto incr;
>       }
> @@ -828,7 +836,9 @@ void ccw_dstream_init(CcwDataStream *cds, CCW1 const *ccw, ORB const *orb)
>       g_assert(!(orb->ctrl1 & ORB_CTRL1_MASK_MIDAW));
>       cds->flags = (orb->ctrl0 & ORB_CTRL0_MASK_I2K ? CDS_F_I2K : 0) |
>                    (orb->ctrl0 & ORB_CTRL0_MASK_C64 ? CDS_F_C64 : 0) |
> +                 (orb->ctrl0 & ORB_CTRL0_MASK_FMT ? CDS_F_FMT : 0) |
>                    (ccw->flags & CCW_FLAG_IDA ? CDS_F_IDA : 0);
> +
>       cds->count = ccw->count;
>       cds->cda_orig = ccw->cda;
>       ccw_dstream_rewind(cds);
> diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
> index 078356e94c..69b374730e 100644
> --- a/include/hw/s390x/css.h
> +++ b/include/hw/s390x/css.h
> @@ -87,6 +87,7 @@ typedef struct CcwDataStream {
>   #define CDS_F_MIDA  0x02
>   #define CDS_F_I2K   0x04
>   #define CDS_F_C64   0x08
> +#define CDS_F_FMT   0x10 /* CCW format-1 */
>   #define CDS_F_STREAM_BROKEN  0x80
>       uint8_t flags;
>       uint8_t at_idaw;
> 

LGTM

Reviewed-by: Pierre Morel<pmorel@linux.vnet.ibm.com>

* Halil Pasic <pasic@linux.vnet.ibm.com> [2017-09-21 20:08:40 +0200]:

> The architecture mandates the addresses to be accessed on the first
> indirection level (that is, the data addresses without IDA, and the
> (M)IDAW addresses with (M)IDA) to be checked against an CCW format
> dependent limit maximum address.  If a violation is detected, the storage
> access is not to be performed and a channel program check needs to be
> generated. As of today, we fail to do this check.
> 
> Let us stick even closer to the architecture specification.
> 
> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> ---
>  hw/s390x/css.c         | 10 ++++++++++
>  include/hw/s390x/css.h |  1 +
>  2 files changed, 11 insertions(+)
> 
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index e0d989829f..cd5580ebb8 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -795,6 +795,11 @@ static inline int cds_check_len(CcwDataStream *cds, int len)
>      return cds->flags & CDS_F_STREAM_BROKEN ? -EINVAL : len;
>  }
> 
> +static inline bool cds_ccw_addrs_ok(hwaddr addr, int len, bool ccw_fmt1)
> +{
> +    return (addr + len) < (ccw_fmt1 ? (1UL << 31) : (1UL << 24));
> +}
> +
>  static int ccw_dstream_rw_noflags(CcwDataStream *cds, void *buff, int len,
>                                    CcwDataStreamOp op)
>  {
> @@ -804,6 +809,9 @@ static int ccw_dstream_rw_noflags(CcwDataStream *cds, void *buff, int len,
>      if (ret <= 0) {
>          return ret;
>      }
> +    if (!cds_ccw_addrs_ok(cds->cda, len, cds->flags & CDS_F_FMT)) {
> +        return -EINVAL; /* channel program check */
> +    }
>      if (op == CDS_OP_A) {
>          goto incr;
>      }
> @@ -828,7 +836,9 @@ void ccw_dstream_init(CcwDataStream *cds, CCW1 const *ccw, ORB const *orb)
>      g_assert(!(orb->ctrl1 & ORB_CTRL1_MASK_MIDAW));
>      cds->flags = (orb->ctrl0 & ORB_CTRL0_MASK_I2K ? CDS_F_I2K : 0) |
>                   (orb->ctrl0 & ORB_CTRL0_MASK_C64 ? CDS_F_C64 : 0) |
> +                 (orb->ctrl0 & ORB_CTRL0_MASK_FMT ? CDS_F_FMT : 0) |
>                   (ccw->flags & CCW_FLAG_IDA ? CDS_F_IDA : 0);
> +
>      cds->count = ccw->count;
>      cds->cda_orig = ccw->cda;
>      ccw_dstream_rewind(cds);
> diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
> index 078356e94c..69b374730e 100644
> --- a/include/hw/s390x/css.h
> +++ b/include/hw/s390x/css.h
> @@ -87,6 +87,7 @@ typedef struct CcwDataStream {
>  #define CDS_F_MIDA  0x02
>  #define CDS_F_I2K   0x04
>  #define CDS_F_C64   0x08
> +#define CDS_F_FMT   0x10 /* CCW format-1 */
>  #define CDS_F_STREAM_BROKEN  0x80
>      uint8_t flags;
>      uint8_t at_idaw;
> -- 
> 2.13.5
> 

Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>