From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46021) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZwL7n-0004Jc-A3 for qemu-devel@nongnu.org; Tue, 10 Nov 2015 21:27:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZwL7j-0001Jh-4x for qemu-devel@nongnu.org; Tue, 10 Nov 2015 21:27:03 -0500 Date: Wed, 11 Nov 2015 13:27:28 +1100 From: David Gibson Message-ID: <20151111022728.GF5852@voom.redhat.com> References: <1443766573-47401-1-git-send-email-aik@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="GLp9dJVi+aaipsRk" Content-Disposition: inline In-Reply-To: <1443766573-47401-1-git-send-email-aik@ozlabs.ru> Subject: Re: [Qemu-devel] [PATCH qemu v4] monitor/target-ppc: Define target_get_monitor_def List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: Paolo Bonzini , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, peter.maydell@linaro.org --GLp9dJVi+aaipsRk Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Oct 02, 2015 at 04:16:13PM +1000, Alexey Kardashevskiy wrote: > At the moment get_monitor_def() returns only registers from statically > defined monitor_defs array. However there is a lot of BOOK3S SPRs > which are not in the list and cannot be printed from the monitor. >=20 > This adds a new target platform hook - target_get_monitor_def(). > The hook is called if a register was not found in the static > array returned by the target_monitor_defs() hook. >=20 > The hook is only defined for POWERPC, it returns registered > SPRs and fails on unregistered ones providing the user with information > on what is actually supported on the running CPU. The register value is > saved as uint64_t as it is the biggest supported register size; > target_ulong cannot be used because of the stub - it is in a "common" > code and cannot include "cpu.h", etc; this is also why the hook prototype > is redefined in the stub instead of being included from some header. >=20 > This replaces static descriptors for GPRs, FPRs, SRs with a helper which > looks for a value in a corresponding array in the CPUPPCState. > The immediate effect is that all 32 SRs can be printed now (instead of 16= ); > later this can be reused for VSX or TM registers. >=20 > While we are here, this adds "cr" as a synonym of "ccr". Sorry I've taken so long to look at this. This is a big improvement over the current approach. > Signed-off-by: Alexey Kardashevskiy Reviewed-by: David Gibson (although I note one small nit below) > --- >=20 > Does it make sense to split it into two patches? Meaning one which adds the hook to the monitor, and another which implements it for ppc? Maybe. I'd like to take this, but I'm not sure whether it's reasonable for the small generic monitor change to go through the ppc tree. Peter, Paolo, opinion? [snip] > diff --git a/target-ppc/monitor.c b/target-ppc/monitor.c > index 9cb1fe9..98417f0 100644 > --- a/target-ppc/monitor.c > +++ b/target-ppc/monitor.c > @@ -76,176 +76,21 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict) > dump_mmu((FILE*)mon, (fprintf_function)monitor_printf, env1); > } > =20 > - > const MonitorDef monitor_defs[] =3D { > - /* General purpose registers */ > - { "r0", offsetof(CPUPPCState, gpr[0]) }, > - { "r1", offsetof(CPUPPCState, gpr[1]) }, > - { "r2", offsetof(CPUPPCState, gpr[2]) }, > - { "r3", offsetof(CPUPPCState, gpr[3]) }, > - { "r4", offsetof(CPUPPCState, gpr[4]) }, > - { "r5", offsetof(CPUPPCState, gpr[5]) }, > - { "r6", offsetof(CPUPPCState, gpr[6]) }, > - { "r7", offsetof(CPUPPCState, gpr[7]) }, > - { "r8", offsetof(CPUPPCState, gpr[8]) }, > - { "r9", offsetof(CPUPPCState, gpr[9]) }, > - { "r10", offsetof(CPUPPCState, gpr[10]) }, > - { "r11", offsetof(CPUPPCState, gpr[11]) }, > - { "r12", offsetof(CPUPPCState, gpr[12]) }, > - { "r13", offsetof(CPUPPCState, gpr[13]) }, > - { "r14", offsetof(CPUPPCState, gpr[14]) }, > - { "r15", offsetof(CPUPPCState, gpr[15]) }, > - { "r16", offsetof(CPUPPCState, gpr[16]) }, > - { "r17", offsetof(CPUPPCState, gpr[17]) }, > - { "r18", offsetof(CPUPPCState, gpr[18]) }, > - { "r19", offsetof(CPUPPCState, gpr[19]) }, > - { "r20", offsetof(CPUPPCState, gpr[20]) }, > - { "r21", offsetof(CPUPPCState, gpr[21]) }, > - { "r22", offsetof(CPUPPCState, gpr[22]) }, > - { "r23", offsetof(CPUPPCState, gpr[23]) }, > - { "r24", offsetof(CPUPPCState, gpr[24]) }, > - { "r25", offsetof(CPUPPCState, gpr[25]) }, > - { "r26", offsetof(CPUPPCState, gpr[26]) }, > - { "r27", offsetof(CPUPPCState, gpr[27]) }, > - { "r28", offsetof(CPUPPCState, gpr[28]) }, > - { "r29", offsetof(CPUPPCState, gpr[29]) }, > - { "r30", offsetof(CPUPPCState, gpr[30]) }, > - { "r31", offsetof(CPUPPCState, gpr[31]) }, > - /* Floating point registers */ > - { "f0", offsetof(CPUPPCState, fpr[0]) }, > - { "f1", offsetof(CPUPPCState, fpr[1]) }, > - { "f2", offsetof(CPUPPCState, fpr[2]) }, > - { "f3", offsetof(CPUPPCState, fpr[3]) }, > - { "f4", offsetof(CPUPPCState, fpr[4]) }, > - { "f5", offsetof(CPUPPCState, fpr[5]) }, > - { "f6", offsetof(CPUPPCState, fpr[6]) }, > - { "f7", offsetof(CPUPPCState, fpr[7]) }, > - { "f8", offsetof(CPUPPCState, fpr[8]) }, > - { "f9", offsetof(CPUPPCState, fpr[9]) }, > - { "f10", offsetof(CPUPPCState, fpr[10]) }, > - { "f11", offsetof(CPUPPCState, fpr[11]) }, > - { "f12", offsetof(CPUPPCState, fpr[12]) }, > - { "f13", offsetof(CPUPPCState, fpr[13]) }, > - { "f14", offsetof(CPUPPCState, fpr[14]) }, > - { "f15", offsetof(CPUPPCState, fpr[15]) }, > - { "f16", offsetof(CPUPPCState, fpr[16]) }, > - { "f17", offsetof(CPUPPCState, fpr[17]) }, > - { "f18", offsetof(CPUPPCState, fpr[18]) }, > - { "f19", offsetof(CPUPPCState, fpr[19]) }, > - { "f20", offsetof(CPUPPCState, fpr[20]) }, > - { "f21", offsetof(CPUPPCState, fpr[21]) }, > - { "f22", offsetof(CPUPPCState, fpr[22]) }, > - { "f23", offsetof(CPUPPCState, fpr[23]) }, > - { "f24", offsetof(CPUPPCState, fpr[24]) }, > - { "f25", offsetof(CPUPPCState, fpr[25]) }, > - { "f26", offsetof(CPUPPCState, fpr[26]) }, > - { "f27", offsetof(CPUPPCState, fpr[27]) }, > - { "f28", offsetof(CPUPPCState, fpr[28]) }, > - { "f29", offsetof(CPUPPCState, fpr[29]) }, > - { "f30", offsetof(CPUPPCState, fpr[30]) }, > - { "f31", offsetof(CPUPPCState, fpr[31]) }, > { "fpscr", offsetof(CPUPPCState, fpscr) }, > /* Next instruction pointer */ > { "nip|pc", offsetof(CPUPPCState, nip) }, > { "lr", offsetof(CPUPPCState, lr) }, > { "ctr", offsetof(CPUPPCState, ctr) }, > + { "xer", offsetof(CPUPPCState, xer) }, > + { "msr", offsetof(CPUPPCState, msr) }, Nit: "msr" and "xer" are already listed below, so I think they're listed twice after this patch. > { "decr", 0, &monitor_get_decr, }, > - { "ccr", 0, &monitor_get_ccr, }, > + { "ccr|cr", 0, &monitor_get_ccr, }, > /* Machine state register */ > { "msr", 0, &monitor_get_msr, }, > { "xer", 0, &monitor_get_xer, }, =2E.here. --=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 --GLp9dJVi+aaipsRk Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWQqePAAoJEGw4ysog2bOS+mIP/3/K/pYsf6eIJcGvnP8mET4F QFaIfIQ/Qfe3JZB00pvC6uLyHUe0NehAgkyqB8Hj3im/yBx+xyQB4lcQL2/gE1H9 pcYv+BB55eOmq4c2u6NIOTLWdcSsFKj3CBw0+izRdctiNV87Gi10HospaSy4L71q yd+OJFVyuSwrI2Of/SjCQ1/7PYo4UC+sqlS9yNRpQO4MXP5qv4E5RVWLsfJdj/yr 6EKh+RQP15pR4L/KdYUzYxTJVm3a6iSK7JmLnexubUieCIgZXkTp4/qmAhjimOde b6U4TdbjjgH1Ju3S82QahZg7vpwjxOBZs0hJA8ifcT4kK0Ke+mZykA2qWVBvjzoH E/mg0erQ6xqmNHIFVOHVPOMu2xHwqfQgahu8E/o2aZv9205lJsWMMKD06Gwc/FvH XMDWLgaJ9Uj+cyEzY5FaaDC9gkS04PVa6TcK2u4CUm3khckfeRcPqjQYTLzQabBk RXRDSlGs3YV+aaxhbFABalKC/qLvcedwoef//6rCLgqkfSdMzavSoXHmCmGDFVN4 U7uXe+coEPfwJ2ACx2aZ3H3/AgpJ/QzjgbS2xdK6LPlmYInu/0VNV3xi6lCCrPeJ KliueAG/+EL55OXr/uZDZGawr3EhCDHwWkSp2855CLBNzjD4VQIzPLN8LJV/U9DD KsUGY0JahpKqJGlBcjfq =0/vW -----END PGP SIGNATURE----- --GLp9dJVi+aaipsRk--