On Wed, 20 Sep 2017 13:13:01 +0200
Halil Pasic <pa...@linux.vnet.ibm.com> wrote:
> On 09/20/2017 10:33 AM, Cornelia Huck wrote:
> > On Wed, 20 Sep 2017 15:42:38 +0800
> > Dong Jia Shi <bjsdj...@linux.vnet.ibm.com> wrote:
> >
> >> * Halil Pasic <pa...@linux.vnet.ibm.com> [2017-09-19 20:27:45 +0200]:
> >>> + MEMTXATTRS_UNSPECIFIED, (void *)
> >>> &idaw.fmt2,
> >>> + sizeof(idaw.fmt2), false);
> >>> + cds->cda = be64_to_cpu(idaw.fmt2);
> >>> + } else {
> >>> + idaw_addr = cds->cda_orig + sizeof(idaw.fmt1) * cds->at_idaw;
> >>> + if (idaw_addr & 0x03 && cds_ccw_addrs_ok(idaw_addr, 0,
> >>> ccw_fmt1)) {
> >>> + return -EINVAL; /* channel program check */
> >>> + }
> >>> + ret = address_space_rw(&address_space_memory, idaw_addr,
> >>> + MEMTXATTRS_UNSPECIFIED, (void *)
> >>> &idaw.fmt1,
> >>> + sizeof(idaw.fmt1), false);
> >>> + cds->cda = be64_to_cpu(idaw.fmt1);
> >> Still need to check bit 0x80000000 here I think.
> >
> > Yes, I think this is 'must be zero' for format-1 idaws, and not covered
> > by the ccw-format specific checks above. (Although the PoP can be a bit
> > confusing with many similar terms...)
> >
>
> It's taken care of in ccw_dstream_rw_ida before the actual
> access happens. Code looks like this:
> + if (!idaw_fmt2 && (cds->cda + iter_len) >= (1ULL << 31)) {
> + ret = -EINVAL; /* channel program check */
> + goto err;
> + }
>
> The idea was to have it similar to the non-indirect case.
<looks at patch again>
Ah, I was simply looking for the wrong pattern. Looks correct.
> >>> +static int ccw_dstream_rw_ida(CcwDataStream *cds, void *buff, int len,
> >>> + CcwDataStreamOp op)
> >>> +{
> >>> + uint64_t bsz = ccw_ida_block_size(cds->flags);
> >>> + int ret = 0;
> >>> + uint16_t cont_left, iter_len;
> >>> + const bool idaw_fmt2 = cds->flags & CDS_F_C64;
> >>> + bool ccw_fmt1 = cds->flags & CDS_F_FMT;
> >> Use 'const bool' either? Although I doubt the value of using const here.
> >> ;)
> >
> > Both being the same is still a good idea.
> >
>
> Yeah. For which one should I go (with const or without)?
For the one you prefer :) (I'm not sure if the const adds value here.)
