From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54319) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SFSqC-0000bB-ET for qemu-devel@nongnu.org; Wed, 04 Apr 2012 12:13:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SFSq2-00083K-Qr for qemu-devel@nongnu.org; Wed, 04 Apr 2012 12:13:48 -0400 Message-ID: <4F7C7330.1080400@suse.de> Date: Wed, 04 Apr 2012 18:13:36 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1332970787-14598-1-git-send-email-david@gibson.dropbear.id.au> <1332970787-14598-2-git-send-email-david@gibson.dropbear.id.au> In-Reply-To: <1332970787-14598-2-git-send-email-david@gibson.dropbear.id.au> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/3] pseries: Fix bug with reset of VIO CRQs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: scottwood@freescale.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org Am 28.03.2012 23:39, schrieb David Gibson: > PAPR specifies a Command Response Queue (CRQ) mechanism used for virtua= l > IO, which we implement. However, we don't correctly clean up registere= d > CRQs when we reset the system. >=20 > This patch adds a reset handler to fix this bug. While we're at it, ad= d > in some of the extra debug messages that were used to track the problem > down. >=20 > Signed-off-by: David Gibson > --- As discussed on IRC, I've applied the following diff on my local branch to drop the h_reg_crq that my __func__ comment was about: diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c index 0bf2c31..97d029a 100644 --- a/hw/spapr_vio.c +++ b/hw/spapr_vio.c @@ -431,13 +431,13 @@ static target_ulong h_reg_crq(CPUPPCState *env, sPAPREnvironment *spapr, /* Check if device supports CRQs */ if (!dev->crq.SendFunc) { - hcall_dprintf("Device does not support CRQ\n"); + hcall_dprintf("h_reg_crq, device does not support CRQ\n"); return H_NOT_FOUND; } /* Already a queue ? */ if (dev->crq.qsize) { - hcall_dprintf("CRQ already registered\n"); + hcall_dprintf("h_reg_crq, CRQ already registered\n"); return H_RESOURCE; } dev->crq.qladdr =3D queue_addr; However, I'm having trouble testing reset. Whether on vanilla master or using this patch on top of ppc-next or this whole series on top of ppc-next, using `ppc64-softmmu/qemu-system-ppc64 -M pseries -m 1G`: a) 0 > reset-all results in: "reboot not available Aborted" Do you need to update SLOF to actually use the newly added RTAS call? b) (qemu) system_reset results in: exception 700 SRR0 =3D 0000000000000000 SRR1 =3D 800000008000000000080000 SPRG2 =3D 0000000000000000 SPRG3 =3D 000000003DCD1AD4 Could you please look into the two above issues? How did you test? Thanks, Andreas > hw/spapr_vio.c | 33 +++++++++++++++++++++++++-------- > 1 files changed, 25 insertions(+), 8 deletions(-) >=20 > diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c > index 1f67e64..97d029a 100644 > --- a/hw/spapr_vio.c > +++ b/hw/spapr_vio.c > @@ -431,12 +431,13 @@ static target_ulong h_reg_crq(CPUPPCState *env, s= PAPREnvironment *spapr, > =20 > /* Check if device supports CRQs */ > if (!dev->crq.SendFunc) { > + hcall_dprintf("h_reg_crq, device does not support CRQ\n"); > return H_NOT_FOUND; > } > =20 > - > /* Already a queue ? */ > if (dev->crq.qsize) { > + hcall_dprintf("h_reg_crq, CRQ already registered\n"); > return H_RESOURCE; > } > dev->crq.qladdr =3D queue_addr; > @@ -449,6 +450,17 @@ static target_ulong h_reg_crq(CPUPPCState *env, sP= APREnvironment *spapr, > return H_SUCCESS; > } > =20 > +static target_ulong free_crq(VIOsPAPRDevice *dev) > +{ > + dev->crq.qladdr =3D 0; > + dev->crq.qsize =3D 0; > + dev->crq.qnext =3D 0; > + > + dprintf("CRQ for dev 0x%" PRIx32 " freed\n", dev->reg); > + > + return H_SUCCESS; > +} > + > static target_ulong h_free_crq(CPUPPCState *env, sPAPREnvironment *spa= pr, > target_ulong opcode, target_ulong *args= ) > { > @@ -460,13 +472,7 @@ static target_ulong h_free_crq(CPUPPCState *env, s= PAPREnvironment *spapr, > return H_PARAMETER; > } > =20 > - dev->crq.qladdr =3D 0; > - dev->crq.qsize =3D 0; > - dev->crq.qnext =3D 0; > - > - dprintf("CRQ for dev 0x" TARGET_FMT_lx " freed\n", reg); > - > - return H_SUCCESS; > + return free_crq(dev); > } > =20 > static target_ulong h_send_crq(CPUPPCState *env, sPAPREnvironment *spa= pr, > @@ -642,6 +648,15 @@ static int spapr_vio_check_reg(VIOsPAPRDevice *sde= v) > return 0; > } > =20 > +static void spapr_vio_busdev_reset(void *opaque) > +{ > + VIOsPAPRDevice *dev =3D (VIOsPAPRDevice *)opaque; > + > + if (dev->crq.qsize) { > + free_crq(dev); > + } > +} > + > static int spapr_vio_busdev_init(DeviceState *qdev) > { > VIOsPAPRDevice *dev =3D (VIOsPAPRDevice *)qdev; > @@ -670,6 +685,8 @@ static int spapr_vio_busdev_init(DeviceState *qdev) > =20 > rtce_init(dev); > =20 > + qemu_register_reset(spapr_vio_busdev_reset, dev); > + > return pc->init(dev); > } > =20 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg