Re: [Qemu-devel] [PATCH v3 4/5] 390x/css: introduce maximum data address checking

[Cornelia Huck]

On Wed, 20 Sep 2017 13:34:21 +0200
Halil Pasic <address@hidden> wrote:

> On 09/20/2017 10:06 AM, Cornelia Huck wrote:
> > On Tue, 19 Sep 2017 20:27:44 +0200
> > Halil Pasic <address@hidden> 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 <address@hidden>
> >> ---
> >>  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 6b0cd8861b..2d37a9ddde 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)  
> > 
> > cds_cda_limit_ok?
> >   
> 
> I use cda to point to the 2 level in case of IDA. This is about
> level 1 (addressed by the ccw directly). That's why I used ccw_addrs
> but if you think cds_cda_limit_ok is better I can live with that.

I don't care that much, tbh.

> 
> We could also think about renaming cds->cda. Btw what does cda stand
> for (channel data address is my guess)?

Yes, cda should stand for 'channel data address'. Its usage in cds->cda
is probably the source of this minor confusion.

But, as said, I don't really care that much; so unless one of the other
folks has a strong opinion, feel free to leave as-is.