Re: [qemu-s390x] [PATCH 1/3] s390x/css: unrestrict cssids

[Dong Jia Shi]

* Halil Pasic <address@hidden> [2017-12-01 15:31:34 +0100]:

[...]

No comment for the message part.

The code looks good to me. So after squashing with patch #2:
Reviewed-by: Dong Jia Shi <address@hidden>

> ---
>  hw/s390x/3270-ccw.c        |  2 +-
>  hw/s390x/css.c             | 28 ++++------------------------
>  hw/s390x/s390-ccw.c        |  2 +-
>  hw/s390x/s390-virtio-ccw.c |  1 -
>  hw/s390x/virtio-ccw.c      |  2 +-
>  include/hw/s390x/css.h     | 12 ++++--------
>  6 files changed, 11 insertions(+), 36 deletions(-)
> 
> diff --git a/hw/s390x/3270-ccw.c b/hw/s390x/3270-ccw.c
> index 081e3ef6f4..3af13ea027 100644
> --- a/hw/s390x/3270-ccw.c
> +++ b/hw/s390x/3270-ccw.c
> @@ -104,7 +104,7 @@ static void emulated_ccw_3270_realize(DeviceState *ds, 
> Error **errp)
>      SubchDev *sch;
>      Error *err = NULL;
> 
> -    sch = css_create_sch(cdev->devno, true, cbus->squash_mcss, errp);
> +    sch = css_create_sch(cdev->devno, cbus->squash_mcss, errp);
>      if (!sch) {
>          return;
>      }
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index f6b5c807cd..cd26f32050 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -2370,22 +2370,12 @@ const PropertyInfo css_devid_ro_propinfo = {
>      .get = get_css_devid,
>  };
> 
> -SubchDev *css_create_sch(CssDevId bus_id, bool is_virtual, bool squash_mcss,
> -                         Error **errp)
> +SubchDev *css_create_sch(CssDevId bus_id, bool squash_mcss, Error **errp)
>  {
>      uint16_t schid = 0;
>      SubchDev *sch;
> 
>      if (bus_id.valid) {
> -        if (is_virtual != (bus_id.cssid == VIRTUAL_CSSID)) {
> -            error_setg(errp, "cssid %hhx not valid for %s devices",
> -                       bus_id.cssid,
> -                       (is_virtual ? "virtual" : "non-virtual"));
> -            return NULL;
> -        }
> -    }
> -
> -    if (bus_id.valid) {
>          if (squash_mcss) {
>              bus_id.cssid = channel_subsys.default_cssid;
>          } else if (!channel_subsys.css[bus_id.cssid]) {
> @@ -2396,19 +2386,8 @@ SubchDev *css_create_sch(CssDevId bus_id, bool 
> is_virtual, bool squash_mcss,
>                                             bus_id.devid, &schid, errp)) {
>              return NULL;
>          }
> -    } else if (squash_mcss || is_virtual) {
> -        bus_id.cssid = channel_subsys.default_cssid;
> -
> -        if (!css_find_free_subch_and_devno(bus_id.cssid, &bus_id.ssid,
> -                                           &bus_id.devid, &schid, errp)) {
> -            return NULL;
> -        }
>      } else {
> -        for (bus_id.cssid = 0; bus_id.cssid < MAX_CSSID; ++bus_id.cssid) {
> -            if (bus_id.cssid == VIRTUAL_CSSID) {
> -                continue;
> -            }
> -
> +        for (bus_id.cssid = channel_subsys.default_cssid;;) {
>              if (!channel_subsys.css[bus_id.cssid]) {
>                  css_create_css_image(bus_id.cssid, false);
>              }
> @@ -2418,7 +2397,8 @@ SubchDev *css_create_sch(CssDevId bus_id, bool 
> is_virtual, bool squash_mcss,
>                                                  NULL)) {
>                  break;
>              }
> -            if (bus_id.cssid == MAX_CSSID) {
> +            bus_id.cssid = (bus_id.cssid + 1) % MAX_CSSID;
> +            if (bus_id.cssid == channel_subsys.default_cssid) {
>                  error_setg(errp, "Virtual channel subsystem is full!");
>                  return NULL;
>              }
> diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c
> index 0ef232ec27..4a9d4d2534 100644
> --- a/hw/s390x/s390-ccw.c
> +++ b/hw/s390x/s390-ccw.c
> @@ -77,7 +77,7 @@ static void s390_ccw_realize(S390CCWDevice *cdev, char 
> *sysfsdev, Error **errp)
>          goto out_err_propagate;
>      }
> 
> -    sch = css_create_sch(ccw_dev->devno, false, cbus->squash_mcss, &err);
> +    sch = css_create_sch(ccw_dev->devno, cbus->squash_mcss, &err);
>      if (!sch) {
>          goto out_mdevid_free;
>      }
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 6a57f94197..4d65a50334 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -302,7 +302,6 @@ static void ccw_init(MachineState *machine)
>      /*
>       * Non mcss-e enabled guests only see the devices from the default
>       * css, which is determined by the value of the squash_mcss property.
> -     * Note: we must not squash non virtual devices to css 0xFE.
>       */
>      if (css_bus->squash_mcss) {
>          ret = css_create_css_image(0, true);
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 184515ce94..3dd902a664 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -701,7 +701,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice 
> *dev, Error **errp)
>      SubchDev *sch;
>      Error *err = NULL;
> 
> -    sch = css_create_sch(ccw_dev->devno, true, cbus->squash_mcss, errp);
> +    sch = css_create_sch(ccw_dev->devno, cbus->squash_mcss, errp);
>      if (!sch) {
>          return;
>      }
> diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
> index ab6ebe66b5..53c270a216 100644
> --- a/include/hw/s390x/css.h
> +++ b/include/hw/s390x/css.h
> @@ -272,12 +272,9 @@ extern const PropertyInfo css_devid_ro_propinfo;
>   * default css image for it.
>   * If @p bus_id is valid, and @p squash_mcss is false, verify that it is
>   * not already in use, and find a free devno for it.
> - * If @p bus_id is not valid, and if either @p squash_mcss or @p is_virtual
> - * is true, find a free subchannel id and device number across all
> - * subchannel sets from the default css image.
> - * If @p bus_id is not valid, and if both @p squash_mcss and @p is_virtual
> - * are false, find a non-full css image and find a free subchannel id and
> - * device number across all subchannel sets from it.
> + * If @p bus_id is not valid find a free subchannel id and device number
> + * across all subchannel sets and all css images starting from the default
> + * css image.
>   *
>   * If either of the former actions succeed, allocate a subchannel structure,
>   * initialise it with the bus id, subchannel id and device number, register
> @@ -286,8 +283,7 @@ extern const PropertyInfo css_devid_ro_propinfo;
>   * The caller becomes owner of the returned subchannel structure and
>   * is responsible for unregistering and freeing it.
>   */
> -SubchDev *css_create_sch(CssDevId bus_id, bool is_virtual, bool squash_mcss,
> -                         Error **errp);
> +SubchDev *css_create_sch(CssDevId bus_id, bool squash_mcss, Error **errp);
> 
>  /** Turn on css migration */
>  void css_register_vmstate(void);
> -- 
> 2.13.5
> 

-- 
Dong Jia Shi