From: Pierre Morel <pmorel@linux.vnet.ibm.com>
To: Halil Pasic <pasic@linux.vnet.ibm.com>,
Cornelia Huck <cohuck@redhat.com>
Cc: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v2 3/4] virtio-ccw: use ccw data stream
Date: Tue, 19 Sep 2017 16:07:25 +0200 [thread overview]
Message-ID: <883c16a8-6289-18e7-bbdb-cf684b20a9eb@linux.vnet.ibm.com> (raw)
In-Reply-To: <20170913115029.47626-4-pasic@linux.vnet.ibm.com>
On 13/09/2017 13:50, Halil Pasic 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 <pasic@linux.vnet.ibm.com>
>
> ---
>
> v1 --> v2:
> Removed todo comments on possible errno change as with
> https://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg02441.html
> applied it does not matter any more.
>
> Error handling: At the moment we ignore errors reported
> by stream ops to keep the change minimal. An add-on
> patch improving on this is to be expected later.
> ---
> hw/s390x/virtio-ccw.c | 156 +++++++++++++++-----------------------------------
> 1 file changed, 46 insertions(+), 110 deletions(-)
:)
>
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index b1976fdd19..a9baf6f7ab 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);
> 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;
> @@ -342,15 +312,13 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> VirtioRevInfo revinfo;
> uint8_t status;
> VirtioFeatDesc features;
> - void *config;
> hwaddr indicators;
> VqConfigBlock vq_config;
> VirtioCcwDevice *dev = sch->driver_data;
> VirtIODevice *vdev = virtio_ccw_get_vdev(sch);
> bool check_len;
> int len;
> - hwaddr hw_len;
> - VirtioThinintInfo *thinint;
> + VirtioThinintInfo thinint;
>
> if (!dev) {
> return -EINVAL;
> @@ -394,11 +362,8 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> } else {
> VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
>
> - features.index = address_space_ldub(&address_space_memory,
> - ccw.cda
> - + sizeof(features.features),
> - MEMTXATTRS_UNSPECIFIED,
> - NULL);
> + ccw_dstream_advance(&sch->cds, sizeof(features.features));
> + ccw_dstream_read(&sch->cds, features.index);
> if (features.index == 0) {
> if (dev->revision >= 1) {
> /* Don't offer legacy features for modern devices. */
> @@ -417,9 +382,9 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> /* Return zeroes if the guest supports more feature bits. */
> features.features = 0;
> }
> - address_space_stl_le(&address_space_memory, ccw.cda,
> - features.features, MEMTXATTRS_UNSPECIFIED,
> - NULL);
> + ccw_dstream_rewind(&sch->cds);
> + cpu_to_le32s(&features.features);
> + ccw_dstream_write(&sch->cds, features.features);
> sch->curr_status.scsw.count = ccw.count - sizeof(features);
> ret = 0;
> }
> @@ -438,15 +403,8 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> if (!ccw.cda) {
> ret = -EFAULT;
> } else {
> - features.index = address_space_ldub(&address_space_memory,
> - ccw.cda
> - + sizeof(features.features),
> - MEMTXATTRS_UNSPECIFIED,
> - NULL);
> - features.features = address_space_ldl_le(&address_space_memory,
> - ccw.cda,
> - MEMTXATTRS_UNSPECIFIED,
> - NULL);
> + ccw_dstream_read(&sch->cds, features);
> + le32_to_cpus(&features.features);
> if (features.index == 0) {
> virtio_set_features(vdev,
> (vdev->guest_features & 0xffffffff00000000ULL) |
> @@ -488,7 +446,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> } else {
> virtio_bus_get_vdev_config(&dev->bus, vdev->config);
> /* XXX config space endianness */
> - cpu_physical_memory_write(ccw.cda, vdev->config, len);
> + ccw_dstream_write_buf(&sch->cds, vdev->config, len);
> sch->curr_status.scsw.count = ccw.count - len;
> ret = 0;
> }
> @@ -501,21 +459,13 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> }
> }
> len = MIN(ccw.count, vdev->config_len);
> - hw_len = len;
> if (!ccw.cda) {
> ret = -EFAULT;
> } else {
> - config = cpu_physical_memory_map(ccw.cda, &hw_len, 0);
> - if (!config) {
> - ret = -EFAULT;
> - } else {
> - len = hw_len;
> - /* XXX config space endianness */
> - memcpy(vdev->config, config, len);
> - cpu_physical_memory_unmap(config, hw_len, 0, hw_len);
> + ret = ccw_dstream_read_buf(&sch->cds, vdev->config, len);
> + if (!ret) {
> virtio_bus_set_vdev_config(&dev->bus, vdev->config);
> sch->curr_status.scsw.count = ccw.count - len;
> - ret = 0;
> }
> }
> break;
> @@ -553,8 +503,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> if (!ccw.cda) {
> ret = -EFAULT;
> } else {
> - status = address_space_ldub(&address_space_memory, ccw.cda,
> - MEMTXATTRS_UNSPECIFIED, NULL);
> + ccw_dstream_read(&sch->cds, status);
> if (!(status & VIRTIO_CONFIG_S_DRIVER_OK)) {
> virtio_ccw_stop_ioeventfd(dev);
> }
> @@ -597,8 +546,8 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> if (!ccw.cda) {
> ret = -EFAULT;
> } else {
> - indicators = address_space_ldq_be(&address_space_memory, ccw.cda,
> - MEMTXATTRS_UNSPECIFIED, NULL);
> + ccw_dstream_read(&sch->cds, indicators);
> + be64_to_cpus(&indicators);
> dev->indicators = get_indicator(indicators, sizeof(uint64_t));
> sch->curr_status.scsw.count = ccw.count - sizeof(indicators);
> ret = 0;
> @@ -618,8 +567,8 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> if (!ccw.cda) {
> ret = -EFAULT;
> } else {
> - indicators = address_space_ldq_be(&address_space_memory, ccw.cda,
> - MEMTXATTRS_UNSPECIFIED, NULL);
> + ccw_dstream_read(&sch->cds, indicators);
> + be64_to_cpus(&indicators);
> dev->indicators2 = get_indicator(indicators, sizeof(uint64_t));
> sch->curr_status.scsw.count = ccw.count - sizeof(indicators);
> ret = 0;
> @@ -639,67 +588,58 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> if (!ccw.cda) {
> ret = -EFAULT;
> } else {
> - vq_config.index = address_space_lduw_be(&address_space_memory,
> - ccw.cda,
> - MEMTXATTRS_UNSPECIFIED,
> - NULL);
> + ccw_dstream_read(&sch->cds, vq_config.index);
> + be16_to_cpus(&vq_config.index);
> if (vq_config.index >= VIRTIO_QUEUE_MAX) {
> ret = -EINVAL;
> break;
> }
> vq_config.num_max = virtio_queue_get_num(vdev,
> vq_config.index);
> - address_space_stw_be(&address_space_memory,
> - ccw.cda + sizeof(vq_config.index),
> - vq_config.num_max,
> - MEMTXATTRS_UNSPECIFIED,
> - NULL);
> + cpu_to_be16s(&vq_config.num_max);
> + ccw_dstream_write(&sch->cds, vq_config.num_max);
> sch->curr_status.scsw.count = ccw.count - sizeof(vq_config);
> ret = 0;
> }
> break;
> case CCW_CMD_SET_IND_ADAPTER:
> if (check_len) {
> - if (ccw.count != sizeof(*thinint)) {
> + if (ccw.count != sizeof(thinint)) {
> ret = -EINVAL;
> break;
> }
> - } else if (ccw.count < sizeof(*thinint)) {
> + } else if (ccw.count < sizeof(thinint)) {
> /* Can't execute command. */
> ret = -EINVAL;
> break;
> }
> - len = sizeof(*thinint);
> - hw_len = len;
> if (!ccw.cda) {
> ret = -EFAULT;
> } else if (dev->indicators && !sch->thinint_active) {
> /* Trigger a command reject. */
> ret = -ENOSYS;
> } else {
> - thinint = cpu_physical_memory_map(ccw.cda, &hw_len, 0);
> - if (!thinint) {
> + if (ccw_dstream_read(&sch->cds, thinint)) {
> ret = -EFAULT;
> } else {
> - uint64_t ind_bit = ldq_be_p(&thinint->ind_bit);
> + be64_to_cpus(&thinint.ind_bit);
> + be64_to_cpus(&thinint.summary_indicator);
> + be64_to_cpus(&thinint.device_indicator);
>
> - len = hw_len;
> dev->summary_indicator =
> - get_indicator(ldq_be_p(&thinint->summary_indicator),
> - sizeof(uint8_t));
> + get_indicator(thinint.summary_indicator, sizeof(uint8_t));
> dev->indicators =
> - get_indicator(ldq_be_p(&thinint->device_indicator),
> - ind_bit / 8 + 1);
> - dev->thinint_isc = thinint->isc;
> - dev->routes.adapter.ind_offset = ind_bit;
> + get_indicator(thinint.device_indicator,
> + thinint.ind_bit / 8 + 1);
> + dev->thinint_isc = thinint.isc;
> + dev->routes.adapter.ind_offset = thinint.ind_bit;
> dev->routes.adapter.summary_offset = 7;
> - cpu_physical_memory_unmap(thinint, hw_len, 0, hw_len);
> dev->routes.adapter.adapter_id = css_get_adapter_id(
> CSS_IO_ADAPTER_VIRTIO,
> dev->thinint_isc);
> sch->thinint_active = ((dev->indicators != NULL) &&
> (dev->summary_indicator != NULL));
> - sch->curr_status.scsw.count = ccw.count - len;
> + sch->curr_status.scsw.count = ccw.count - sizeof(thinint);
> ret = 0;
> }
> }
> @@ -714,13 +654,9 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> ret = -EFAULT;
> break;
> }
> - revinfo.revision =
> - address_space_lduw_be(&address_space_memory, ccw.cda,
> - MEMTXATTRS_UNSPECIFIED, NULL);
> - revinfo.length =
> - address_space_lduw_be(&address_space_memory,
> - ccw.cda + sizeof(revinfo.revision),
> - MEMTXATTRS_UNSPECIFIED, NULL);
> + ccw_dstream_read_buf(&sch->cds, &revinfo, 4);
> + be16_to_cpus(&revinfo.revision);
> + be16_to_cpus(&revinfo.length);
> if (ccw.count < len + revinfo.length ||
> (check_len && ccw.count > len + revinfo.length)) {
> ret = -EINVAL;
>
Reviewed-by: Pierre Morel<pmorel@linux.vnet.ibm.com>
--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany
next prev parent reply other threads:[~2017-09-19 14:07 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-13 11:50 [Qemu-devel] [PATCH v2 0/4] add CCW indirect data access support Halil Pasic
2017-09-13 11:50 ` [Qemu-devel] [PATCH v2 1/4] s390x/css: introduce css data stream Halil Pasic
2017-09-14 9:08 ` Cornelia Huck
2017-09-19 2:21 ` Dong Jia Shi
2017-09-19 8:38 ` Cornelia Huck
2017-09-19 9:11 ` Pierre Morel
2017-09-19 9:53 ` Cornelia Huck
2017-09-19 11:41 ` Pierre Morel
2017-09-19 12:16 ` Halil Pasic
2017-09-19 13:55 ` Pierre Morel
2017-09-13 11:50 ` [Qemu-devel] [PATCH v2 2/4] s390x/css: use ccw " Halil Pasic
2017-09-19 2:45 ` Dong Jia Shi
2017-09-19 13:57 ` Pierre Morel
2017-09-13 11:50 ` [Qemu-devel] [PATCH v2 3/4] virtio-ccw: " Halil Pasic
2017-09-14 8:47 ` Cornelia Huck
2017-09-19 3:37 ` Dong Jia Shi
2017-09-19 9:06 ` Cornelia Huck
2017-09-19 13:30 ` Halil Pasic
2017-09-20 1:16 ` Dong Jia Shi
2017-09-19 14:07 ` Pierre Morel [this message]
2017-09-13 11:50 ` [Qemu-devel] [PATCH v2 4/4] s390x/css: support ccw IDA Halil Pasic
2017-09-14 9:14 ` Cornelia Huck
2017-09-19 5:50 ` Dong Jia Shi
2017-09-19 9:48 ` Cornelia Huck
2017-09-19 10:36 ` Halil Pasic
2017-09-19 10:57 ` Cornelia Huck
2017-09-19 12:04 ` Halil Pasic
2017-09-19 12:23 ` Cornelia Huck
2017-09-19 12:32 ` Halil Pasic
2017-09-19 14:34 ` Cornelia Huck
2017-09-19 18:05 ` Halil Pasic
2017-09-20 1:37 ` Dong Jia Shi
2017-09-20 6:40 ` Dong Jia Shi
2017-09-19 13:46 ` Pierre Morel
2017-09-19 16:49 ` Halil Pasic
2017-09-14 9:15 ` [Qemu-devel] [PATCH v2 0/4] add CCW indirect data access support Cornelia Huck
2017-09-14 11:02 ` Halil Pasic
2017-09-14 11:19 ` Cornelia Huck
2017-09-14 14:16 ` Cornelia Huck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=883c16a8-6289-18e7-bbdb-cf684b20a9eb@linux.vnet.ibm.com \
--to=pmorel@linux.vnet.ibm.com \
--cc=bjsdjshi@linux.vnet.ibm.com \
--cc=cohuck@redhat.com \
--cc=pasic@linux.vnet.ibm.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).