From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34651) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c3z4Y-0000IN-Ai for qemu-devel@nongnu.org; Tue, 08 Nov 2016 00:35:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c3z4X-0002GC-2P for qemu-devel@nongnu.org; Tue, 08 Nov 2016 00:35:50 -0500 Date: Tue, 8 Nov 2016 16:14:13 +1100 From: David Gibson Message-ID: <20161108051413.GR28688@umbus.fritz.box> References: <1477825928-10803-1-git-send-email-david@gibson.dropbear.id.au> <1477825928-10803-10-git-send-email-david@gibson.dropbear.id.au> <20161031083911.GT18226@umbus.fritz.box> <70ab01a2-2732-6615-f5fc-94b1eb6001b4@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="IUSVF+LtaR4kWxuH" Content-Disposition: inline In-Reply-To: <70ab01a2-2732-6615-f5fc-94b1eb6001b4@ozlabs.ru> Subject: Re: [Qemu-devel] [RFC 09/17] ppc: Validate compatibility modes when setting List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: nikunj@linux.vnet.ibm.com, mdroth@linux.vnet.ibm.com, thuth@redhat.com, lvivier@redhat.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org --IUSVF+LtaR4kWxuH Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Nov 04, 2016 at 02:45:02PM +1100, Alexey Kardashevskiy wrote: > On 31/10/16 19:39, David Gibson wrote: > > On Mon, Oct 31, 2016 at 04:55:42PM +1100, Alexey Kardashevskiy wrote: > >> On 30/10/16 22:12, David Gibson wrote: > >>> Current ppc_set_compat() will attempt to set any compatiblity mode > >>> specified, regardless of whether it's available on the CPU. The call= er is > >>> expected to make sure it is setting a possible mode, which is awkwward > >>> because most of the information to make that decision is at the CPU l= evel. > >>> > >>> This begins to clean this up by introducing a ppc_check_compat() func= tion > >>> which will determine if a given compatiblity mode is supported on a C= PU > >>> (and also whether it lies within specified minimum and maximum compat > >>> levels, which will be useful later). It also contains an assertion t= hat > >>> the CPU has a "virtual hypervisor"[1], that is, that the guest isn't > >>> permitted to execute hypervisor privilege code. Without that, the gu= est > >>> would own the PCR and so could override any mode set here. Only mach= ine > >>> types which use a virtual hypervisor (i.e. 'pseries') should use > >>> ppc_check_compat(). > >>> > >>> ppc_set_compat() is modified to validate the compatibility mode it is= given > >>> and fail if it's not available on this CPU. > >>> > >>> [1] Or user-only mode, which also obviously doesn't allow access to t= he > >>> hypervisor privileged PCR. We don't use that now, but could in futur= e. > >>> > >>> Signed-off-by: David Gibson > >>> --- > >>> target-ppc/compat.c | 41 +++++++++++++++++++++++++++++++++++++++++ > >>> target-ppc/cpu.h | 2 ++ > >>> 2 files changed, 43 insertions(+) > >>> > >>> diff --git a/target-ppc/compat.c b/target-ppc/compat.c > >>> index 66529a6..1059555 100644 > >>> --- a/target-ppc/compat.c > >>> +++ b/target-ppc/compat.c > >>> @@ -28,29 +28,37 @@ > >>> typedef struct { > >>> uint32_t pvr; > >>> uint64_t pcr; > >>> + uint64_t pcr_level; > >>> int max_threads; > >>> } CompatInfo; > >>> =20 > >>> static const CompatInfo compat_table[] =3D { > >>> + /* > >>> + * Ordered from oldest to newest - the code relies on this >=20 > In last 5+ years, I have never seen pointer compared anyhow but using "= =3D=3D" > and "!=3D". A bit unusual. Unusual, yes, but it has its uses from time to time. >=20 >=20 > Reviewed-by: Alexey Kardashevskiy >=20 >=20 >=20 >=20 >=20 > >>> + */ > >>> { /* POWER6, ISA2.05 */ > >>> .pvr =3D CPU_POWERPC_LOGICAL_2_05, > >>> .pcr =3D PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05 > >>> | PCR_TM_DIS | PCR_VSX_DIS, > >>> + .pcr_level =3D PCR_COMPAT_2_05, > >>> .max_threads =3D 2, > >>> }, > >>> { /* POWER7, ISA2.06 */ > >>> .pvr =3D CPU_POWERPC_LOGICAL_2_06, > >>> .pcr =3D PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_TM_DIS, > >>> + .pcr_level =3D PCR_COMPAT_2_06, > >>> .max_threads =3D 4, > >>> }, > >>> { > >>> .pvr =3D CPU_POWERPC_LOGICAL_2_06_PLUS, > >>> .pcr =3D PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_TM_DIS, > >>> + .pcr_level =3D PCR_COMPAT_2_06, > >>> .max_threads =3D 4, > >>> }, > >>> { /* POWER8, ISA2.07 */ > >>> .pvr =3D CPU_POWERPC_LOGICAL_2_07, > >>> .pcr =3D PCR_COMPAT_2_07, > >>> + .pcr_level =3D PCR_COMPAT_2_07, > >>> .max_threads =3D 8, > >>> }, > >>> }; > >>> @@ -67,6 +75,35 @@ static const CompatInfo *compat_by_pvr(uint32_t pv= r) > >>> return NULL; > >>> } > >>> =20 > >>> +bool ppc_check_compat(PowerPCCPU *cpu, uint32_t compat_pvr, > >>> + uint32_t min_compat_pvr, uint32_t max_compat_p= vr) > >>> +{ > >>> + PowerPCCPUClass *pcc =3D POWERPC_CPU_GET_CLASS(cpu); > >>> + const CompatInfo *compat =3D compat_by_pvr(compat_pvr); > >>> + const CompatInfo *min =3D compat_by_pvr(min_compat_pvr); > >>> + const CompatInfo *max =3D compat_by_pvr(max_compat_pvr); > >> > >> > >> You keep giving very generic names (as "min" and "max") to local > >> variables ;) > >=20 > > For local variables, brevity is a virtue. >=20 >=20 >=20 >=20 >=20 --=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 --IUSVF+LtaR4kWxuH Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYIV8kAAoJEGw4ysog2bOSkEoQAMMrrus2nQ0UcG+u9EnPwQEI xDgIkjcaD3lym6C6/1c8dtcXvDV6aiz/cVE3nStyozDSvnCmUQb9jZkALRTAFsSH i8a7klVEK/hC5dRXO5sZW2qJfKwapzXROLxiQHeYsTS44qJGmHTam2U1ne0A1zNi Nb1X0ZzJqFfHwTLRQyVjAiz6h3IFYFXD3g3JpBr7LK8SjrUzvtPW5N9FW6XCG012 KtISN2k3ATi0vcWD4Mky1Ggf2sZQYE2xs+4Cusy7avjl+eCwpIr+2Amp7M6T1RsJ LDwnqGXT2Hq/yTc7VZUN3zw7rgPQ8jYFgkOntb0kY7MlWomGVvroWstUaLefhvUO 5QmHY1FFX9NnGvlAfpHHLkt4oD1wY9ND5gBdwYdCLCOxtVFtkWMepJ5vS7rkKM9Z dNJtTFOeJgxdxsJBYgXzexaIaoW91rP2vm4hIqY1JzIiHSmxOk3yw+PYfcsqRmkC n7tGukWwdaKGmyfN+c6IQxmUXyGEJmktwe+vQ1J1z+nf9bHIP+BdWKbhKgirgCQf iL0iljtTs6CJ31gyd7XnayBw/+NSQGUlNN8u2Sfiwws69bJeZvkAAmmvcQ2mWBm4 28sG7RAkZ3Bv2fr9T8rOTjekA3x/t9JY9qWkLP2pwi4uf9H2UzKntXMerfZdCT5B TMYoDM0sgekrrUOY+bG3 =L9l7 -----END PGP SIGNATURE----- --IUSVF+LtaR4kWxuH--