On Tue, 5 Sep 2017 13:16:43 +0200
Halil Pasic <pa...@linux.vnet.ibm.com> wrote:
> Replace direct access which implicitly assumes no IDA
> or MIDA with the new ccw data stream interface which should
> cope with these transparently in the future.
>
> Signed-off-by: Halil Pasic <pa...@linux.vnet.ibm.com>
>
> ---
>
> Error handling: At the moment we ignore errors reported
> by stream ops to keep the change minimal. It might make sense
> to change this.
Do that as an add-on patch?
> ---
> hw/s390x/virtio-ccw.c | 158
> +++++++++++++++-----------------------------------
> 1 file changed, 48 insertions(+), 110 deletions(-)
>
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index b1976fdd19..72dd129a15 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -287,49 +287,19 @@ static int virtio_ccw_handle_set_vq(SubchDev *sch, CCW1
> ccw, bool check_len,
> return -EFAULT;
> }
> if (is_legacy) {
> - linfo.queue = address_space_ldq_be(&address_space_memory, ccw.cda,
> - MEMTXATTRS_UNSPECIFIED, NULL);
> - linfo.align = address_space_ldl_be(&address_space_memory,
> - ccw.cda + sizeof(linfo.queue),
> - MEMTXATTRS_UNSPECIFIED,
> - NULL);
> - linfo.index = address_space_lduw_be(&address_space_memory,
> - ccw.cda + sizeof(linfo.queue)
> - + sizeof(linfo.align),
> - MEMTXATTRS_UNSPECIFIED,
> - NULL);
> - linfo.num = address_space_lduw_be(&address_space_memory,
> - ccw.cda + sizeof(linfo.queue)
> - + sizeof(linfo.align)
> - + sizeof(linfo.index),
> - MEMTXATTRS_UNSPECIFIED,
> - NULL);
> + ccw_dstream_read(&sch->cds, linfo);
> + be64_to_cpus(&linfo.queue);
> + be32_to_cpus(&linfo.align);
> + be16_to_cpus(&linfo.index);
> + be16_to_cpus(&linfo.num);
That's a very nice contraction :)
> ret = virtio_ccw_set_vqs(sch, NULL, &linfo);
> } else {
> - info.desc = address_space_ldq_be(&address_space_memory, ccw.cda,
> - MEMTXATTRS_UNSPECIFIED, NULL);
> - info.index = address_space_lduw_be(&address_space_memory,
> - ccw.cda + sizeof(info.desc)
> - + sizeof(info.res0),
> - MEMTXATTRS_UNSPECIFIED, NULL);
> - info.num = address_space_lduw_be(&address_space_memory,
> - ccw.cda + sizeof(info.desc)
> - + sizeof(info.res0)
> - + sizeof(info.index),
> - MEMTXATTRS_UNSPECIFIED, NULL);
> - info.avail = address_space_ldq_be(&address_space_memory,
> - ccw.cda + sizeof(info.desc)
> - + sizeof(info.res0)
> - + sizeof(info.index)
> - + sizeof(info.num),
> - MEMTXATTRS_UNSPECIFIED, NULL);
> - info.used = address_space_ldq_be(&address_space_memory,
> - ccw.cda + sizeof(info.desc)
> - + sizeof(info.res0)
> - + sizeof(info.index)
> - + sizeof(info.num)
> - + sizeof(info.avail),
> - MEMTXATTRS_UNSPECIFIED, NULL);
> + ccw_dstream_read(&sch->cds, info);
> + be64_to_cpus(&info.desc);
> + be16_to_cpus(&info.index);
> + be16_to_cpus(&info.num);
> + be64_to_cpus(&info.avail);
> + be64_to_cpus(&info.used);
> ret = virtio_ccw_set_vqs(sch, &info, NULL);
> }
> sch->curr_status.scsw.count = 0;
(...)
> @@ -488,7 +446,8 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> } else {
> virtio_bus_get_vdev_config(&dev->bus, vdev->config);
> /* XXX config space endianness */
Unrelated: That should be fine, I guess?
> - cpu_physical_memory_write(ccw.cda, vdev->config, len);
> + /* TODO we may have made -EINVAL out of -EFAULT */
Eek.
> + ccw_dstream_write_buf(&sch->cds, vdev->config, len);
> sch->curr_status.scsw.count = ccw.count - len;
> ret = 0;
> }
Looks fine in general.
