On 05/05/14 17:10, Cornelia Huck wrote:
> Current css code saves the operation request block (orb) in the
> subchannel structure for later consumption by the start function
> handler. This might make sense for asynchronous execution of the
> start function (which qemu doesn't support), but not in our case;
> it would even be wrong since orb contains a reference to a local
> variable in the base ssch handler.
>
> Let's just pass the orb through the start function call chain for
> ssch; for rsch, we can pass NULL as the backend function does not
> use any information passed via the orb there.
>
> Signed-off-by: Cornelia Huck <cornelia.h...@de.ibm.com>
Good, this makes the whole thing simpler. Furthermore, this also avoids the
need to take care of the orb in the vmstate for migration since it is no longer
a state.
Acked-by: Christian Borntraeger <borntrae...@de.ibm.com>
> ---
> hw/s390x/css.c | 21 ++++++++-------------
> hw/s390x/css.h | 1 -
> hw/s390x/virtio-ccw.c | 1 -
> 3 files changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index 7074d2b..122cc7e 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -140,7 +140,6 @@ static void sch_handle_clear_func(SubchDev *sch)
> s->flags &= ~SCSW_FLAGS_MASK_PNO;
>
> /* We always 'attempt to issue the clear signal', and we always succeed.
> */
> - sch->orb = NULL;
> sch->channel_prog = 0x0;
> sch->last_cmd_valid = false;
> s->ctrl &= ~SCSW_ACTL_CLEAR_PEND;
> @@ -163,7 +162,6 @@ static void sch_handle_halt_func(SubchDev *sch)
> path = 0x80;
>
> /* We always 'attempt to issue the halt signal', and we always succeed.
> */
> - sch->orb = NULL;
> sch->channel_prog = 0x0;
> sch->last_cmd_valid = false;
> s->ctrl &= ~SCSW_ACTL_HALT_PEND;
> @@ -317,12 +315,11 @@ static int css_interpret_ccw(SubchDev *sch, hwaddr
> ccw_addr)
> return ret;
> }
>
> -static void sch_handle_start_func(SubchDev *sch)
> +static void sch_handle_start_func(SubchDev *sch, ORB *orb)
> {
>
> PMCW *p = &sch->curr_status.pmcw;
> SCSW *s = &sch->curr_status.scsw;
> - ORB *orb = sch->orb;
> int path;
> int ret;
>
> @@ -331,6 +328,7 @@ static void sch_handle_start_func(SubchDev *sch)
>
> if (!(s->ctrl & SCSW_ACTL_SUSP)) {
> /* Look at the orb and try to execute the channel program. */
> + assert(orb != NULL); /* resume does not pass an orb */
> p->intparm = orb->intparm;
> if (!(orb->lpm & path)) {
> /* Generate a deferred cc 3 condition. */
> @@ -406,7 +404,7 @@ static void sch_handle_start_func(SubchDev *sch)
> * read/writes) asynchronous later on if we start supporting more than
> * our current very simple devices.
> */
> -static void do_subchannel_work(SubchDev *sch)
> +static void do_subchannel_work(SubchDev *sch, ORB *orb)
> {
>
> SCSW *s = &sch->curr_status.scsw;
> @@ -416,7 +414,7 @@ static void do_subchannel_work(SubchDev *sch)
> } else if (s->ctrl & SCSW_FCTL_HALT_FUNC) {
> sch_handle_halt_func(sch);
> } else if (s->ctrl & SCSW_FCTL_START_FUNC) {
> - sch_handle_start_func(sch);
> + sch_handle_start_func(sch, orb);
> } else {
> /* Cannot happen. */
> return;
> @@ -594,7 +592,6 @@ int css_do_xsch(SubchDev *sch)
> SCSW_ACTL_SUSP);
> sch->channel_prog = 0x0;
> sch->last_cmd_valid = false;
> - sch->orb = NULL;
> s->dstat = 0;
> s->cstat = 0;
> ret = 0;
> @@ -618,7 +615,7 @@ int css_do_csch(SubchDev *sch)
> s->ctrl &= ~(SCSW_CTRL_MASK_FCTL | SCSW_CTRL_MASK_ACTL);
> s->ctrl |= SCSW_FCTL_CLEAR_FUNC | SCSW_FCTL_CLEAR_FUNC;
>
> - do_subchannel_work(sch);
> + do_subchannel_work(sch, NULL);
> ret = 0;
>
> out:
> @@ -659,7 +656,7 @@ int css_do_hsch(SubchDev *sch)
> }
> s->ctrl |= SCSW_ACTL_HALT_PEND;
>
> - do_subchannel_work(sch);
> + do_subchannel_work(sch, NULL);
> ret = 0;
>
> out:
> @@ -721,13 +718,12 @@ int css_do_ssch(SubchDev *sch, ORB *orb)
> if (channel_subsys->chnmon_active) {
> css_update_chnmon(sch);
> }
> - sch->orb = orb;
> sch->channel_prog = orb->cpa;
> /* Trigger the start function. */
> s->ctrl |= (SCSW_FCTL_START_FUNC | SCSW_ACTL_START_PEND);
> s->flags &= ~SCSW_FLAGS_MASK_PNO;
>
> - do_subchannel_work(sch);
> + do_subchannel_work(sch, orb);
> ret = 0;
>
> out:
> @@ -957,7 +953,7 @@ int css_do_rsch(SubchDev *sch)
> }
>
> s->ctrl |= SCSW_ACTL_RESUME_PEND;
> - do_subchannel_work(sch);
> + do_subchannel_work(sch, NULL);
> ret = 0;
>
> out:
> @@ -1267,7 +1263,6 @@ void css_reset_sch(SubchDev *sch)
>
> sch->channel_prog = 0x0;
> sch->last_cmd_valid = false;
> - sch->orb = NULL;
> sch->thinint_active = false;
> }
>
> diff --git a/hw/s390x/css.h b/hw/s390x/css.h
> index e9b4405..220169e 100644
> --- a/hw/s390x/css.h
> +++ b/hw/s390x/css.h
> @@ -76,7 +76,6 @@ struct SubchDev {
> hwaddr channel_prog;
> CCW1 last_cmd;
> bool last_cmd_valid;
> - ORB *orb;
> bool thinint_active;
> /* transport-provided data: */
> int (*ccw_cb) (SubchDev *, CCW1);
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 2bf0af8..1cb4e2c 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -559,7 +559,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev,
> VirtIODevice *vdev)
> /* Initialize subchannel structure. */
> sch->channel_prog = 0x0;
> sch->last_cmd_valid = false;
> - sch->orb = NULL;
> sch->thinint_active = false;
> /*
> * Use a device number if provided. Otherwise, fall back to subchannel
>
