From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:43339) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UWsEo-0007Ru-Lf for qemu-devel@nongnu.org; Mon, 29 Apr 2013 13:51:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UWsEl-0004gT-Mr for qemu-devel@nongnu.org; Mon, 29 Apr 2013 13:51:42 -0400 Date: Mon, 29 Apr 2013 12:50:35 -0500 From: Scott Wood References: <1367246456-22382-1-git-send-email-Bharat.Bhushan@freescale.com> In-Reply-To: <1367246456-22382-1-git-send-email-Bharat.Bhushan@freescale.com> (from r65777@freescale.com on Mon Apr 29 09:40:56 2013) Message-ID: <1367257835.32182.2@snotra> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2] ppc: initialize GPRs as per epapr List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharat Bhushan Cc: agraf@suse.de, stuart.yoder@freescale.com, qemu-devel@nongnu.org, Bharat Bhushan , qemu-ppc@nongnu.org, david@gibson.dropbear.id.au On 04/29/2013 09:40:56 AM, Bharat Bhushan wrote: > ePAPR defines the initial values of cpu registers. > This patch initialize the GPRs as per ePAPR specification. >=20 > This resolves the issue of guest reboot/reset (guest hang on reboot). >=20 > Signed-off-by: Bharat Bhushan > --- > v1-->v2 > - Dynemic seting of initial map size in gpr[7] > - For this the tlb size calculation is moved into a function. >=20 > hw/ppc/e500.c | 29 ++++++++++++++++++++++++++--- > 1 files changed, 26 insertions(+), 3 deletions(-) >=20 > diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c > index c1bdb6b..decd86c 100644 > --- a/hw/ppc/e500.c > +++ b/hw/ppc/e500.c > @@ -37,6 +37,7 @@ > #include "qemu/host-utils.h" > #include "hw/pci-host/ppce500.h" >=20 > +#define EPAPR_MAGIC (0x45504150) > #define BINARY_DEVICE_TREE_FILE "mpc8544ds.dtb" > #define UIMAGE_LOAD_BASE 0 > #define DTC_LOAD_PAD 0x1800000 > @@ -393,11 +394,10 @@ static inline hwaddr =20 > booke206_page_size_to_tlb(uint64_t size) > return 63 - clz64(size >> 10); > } >=20 > -static void mmubooke_create_initial_mapping(CPUPPCState *env) > +static int booke206_initial_map_tsize(CPUPPCState *env) > { > struct boot_info *bi =3D env->load_info; > - ppcmas_tlb_t *tlb =3D booke206_get_tlbm(env, 1, 0, 0); > - hwaddr size, dt_end; > + hwaddr dt_end; > int ps; >=20 > /* Our initial TLB entry needs to cover everything from 0 to > @@ -408,6 +408,23 @@ static void =20 > mmubooke_create_initial_mapping(CPUPPCState *env) > /* e500v2 can only do even TLB size bits */ > ps++; > } > + return ps; > +} > +static uint64_t mmubooke_initial_mapsize(CPUPPCState *env) Please leave a blank line after each of those } lines. You change the function name to look like it's simply returning =20 information, but it still creates the TLB entry as far as I can see. =20 Then you go calling it multiple times (why?). This may not be harmful, =20 but it is ugly. > +{ > + int tsize; > + > + tsize =3D booke206_initial_map_tsize(env); > + return (1ULL << 10 << tsize); > +} What value does this wrapper have? The caller needs both the "tsize" =20 and the size in bytes, and you only call this wrapper once, so let the =20 caller do the conversion instead. -Scott=