From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44138) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zwqm1-0000H6-HR for qemu-devel@nongnu.org; Thu, 12 Nov 2015 07:14:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zwqm0-0007P8-Fn for qemu-devel@nongnu.org; Thu, 12 Nov 2015 07:14:41 -0500 Date: Thu, 12 Nov 2015 22:53:16 +1100 From: David Gibson Message-ID: <20151112115316.GH4886@voom.redhat.com> References: <20151111171135.4328.41819.stgit@aravindap> <20151111171515.4328.22622.stgit@aravindap> <56444D32.5070105@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="LiQwW4YX+w4axhAx" Content-Disposition: inline In-Reply-To: <56444D32.5070105@redhat.com> Subject: Re: [Qemu-devel] [PATCH 1/4] spapr: Extend rtas-blob List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth Cc: benh@au1.ibm.com, aik@ozlabs.ru, agraf@suse.de, qemu-devel@nongnu.org, qemu-ppc@nongnu.org, sam.bobroff@au1.ibm.com, Aravinda Prasad , paulus@samba.org --LiQwW4YX+w4axhAx Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Nov 12, 2015 at 09:26:26AM +0100, Thomas Huth wrote: > On 11/11/15 18:15, Aravinda Prasad wrote: > > Extend rtas-blob to accommodate error log. Error log > > structure is saved in rtas space upon a machine check > > exception. > >=20 > > Signed-off-by: Aravinda Prasad > > --- > > hw/ppc/spapr.c | 4 ++++ > > 1 file changed, 4 insertions(+) > >=20 > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index 05926a3..b7b9e09 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -1556,6 +1556,10 @@ static void ppc_spapr_init(MachineState *machine) > > exit(1); > > } > > spapr->rtas_size =3D get_image_size(filename); > > + > > + /* Resize blob to accommodate error log. */ > > + spapr->rtas_size =3D TARGET_PAGE_ALIGN(spapr->rtas_size); > > + > > spapr->rtas_blob =3D g_malloc(spapr->rtas_size); > > if (load_image_size(filename, spapr->rtas_blob, spapr->rtas_size) = < 0) { > > error_report("Could not load LPAR rtas '%s'", filename); >=20 > Sorry to say that, but this patch is horrible! >=20 > 1) If the rtas blob ever gets bigger than 512 bytes, we will get > "random" corruption of the RTAS code later when an NMI occurs since the > mc log is blindly copied into the RTAS area later! > =3D=3D> Please add an "assert(spapr->rtas_size < RTAS_ERRLOG_OFFSET)" at = the > beginning of your patch. The assert is a good idea, although I think the chances of the rtas ever growing beyond its current 20 bytes is just about nil. > 2) Why resizing with TARGET_PAGE_ALIGN() ? In the very worst case, this > would not change the size at all (if the rtas_size is already a multiple > of PAGE_SIZE) > =3D=3D> Please set the size to a proper value like > RTAS_ERRLOG_OFFSET + sizeof(struct rtas_mc_log) > instead! That's a good point. --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --LiQwW4YX+w4axhAx Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWRH2sAAoJEGw4ysog2bOSabUP/jhG0umTcw1j6WbhJiaeXYw0 H91Enay1iXQc8rC4gODBMh3E3T9mDWPZxO1fWwCtNDMnjfOB4CSXE38vEx99ksmL 9un+KQO99YfrViGyxjaQtIvaeIJthAr/MmvOjM122H8BFOSjF4A3LEc1Ir2KrmrO JPEZoNApfLYOEmjJ8hIbJ+/+V1k2Q70wbojlsPGPMU7NVpSMsvOmPlengJS4TCV3 YknWh8i5+zS6RO9mqsJgNrJU5a94NP+icG5BRCbvyyjt6O7QWvIe3IRPpnW8JJpi Q+1sfLzshvpeBHV5BLeQfJliJL8wslJo2moRIjYxIYQZ/YHaqfmhrq7Is8zkzZvI nlQ4WiFmlm0AstyUBWQYdQEdJL/b3halZD2cT+5Mwdc4AkqgBqW721QmYWm3qmjq Lsik62vGiWnI680lu6Q5Fy63zzFynj2pOFJKzDTZ0wHu94KZ38Dc5Z4ufQoKiKx1 s2P6YYQ3rrmySCjEowjXJ96Q1Ur2o+h+FdX9NnibdKtk676pKQz64PvMIzkzhvU4 AarDEARDGT9MDC/8JKre1/8UthuWysZQz+UGt6TgkVHNrZk85QFH6ki9LexdfdN4 4d9BlBEU9lewnEJTgpybtT3r5TPJijQ54tlbC0tR/V5+eEGJpzU1RsG+7gzODYM3 HZmpiuhjA9p1L4ICqzp1 =xjeq -----END PGP SIGNATURE----- --LiQwW4YX+w4axhAx--