From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60263) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WhO00-0003Qr-9O for qemu-devel@nongnu.org; Mon, 05 May 2014 14:52:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WhNzr-0001MQ-70 for qemu-devel@nongnu.org; Mon, 05 May 2014 14:52:24 -0400 Received: from e06smtp17.uk.ibm.com ([195.75.94.113]:35314) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WhNzq-0001MI-UG for qemu-devel@nongnu.org; Mon, 05 May 2014 14:52:15 -0400 Received: from /spool/local by e06smtp17.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 5 May 2014 19:52:13 +0100 Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id 2097317D804E for ; Mon, 5 May 2014 19:53:16 +0100 (BST) Received: from d06av10.portsmouth.uk.ibm.com (d06av10.portsmouth.uk.ibm.com [9.149.37.251]) by b06cxnps3075.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s45IqBF2328056 for ; Mon, 5 May 2014 18:52:11 GMT Received: from d06av10.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av10.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s45IqACr017902 for ; Mon, 5 May 2014 12:52:11 -0600 Message-ID: <5367DDDA.3050909@de.ibm.com> Date: Mon, 05 May 2014 20:52:10 +0200 From: Christian Borntraeger MIME-Version: 1.0 References: <1399302619-21944-1-git-send-email-cornelia.huck@de.ibm.com> <1399302619-21944-4-git-send-email-cornelia.huck@de.ibm.com> In-Reply-To: <1399302619-21944-4-git-send-email-cornelia.huck@de.ibm.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/3] s390x/css: Don't save orb in subchannel. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck , qemu-devel@nongnu.org Cc: agraf@suse.de 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 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 > --- > 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 >