From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41604) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c187m-0002M5-Gw for qemu-devel@nongnu.org; Mon, 31 Oct 2016 04:39:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c187l-0004Do-Ei for qemu-devel@nongnu.org; Mon, 31 Oct 2016 04:39:22 -0400 Date: Mon, 31 Oct 2016 19:39:11 +1100 From: David Gibson Message-ID: <20161031083911.GT18226@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> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="wYXww9TlNKyqAMAe" Content-Disposition: inline In-Reply-To: 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 --wYXww9TlNKyqAMAe Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 caller= 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 lev= el. > >=20 > > This begins to clean this up by introducing a ppc_check_compat() functi= on > > which will determine if a given compatiblity mode is supported on a CPU > > (and also whether it lies within specified minimum and maximum compat > > levels, which will be useful later). It also contains an assertion that > > the CPU has a "virtual hypervisor"[1], that is, that the guest isn't > > permitted to execute hypervisor privilege code. Without that, the guest > > would own the PCR and so could override any mode set here. Only machine > > types which use a virtual hypervisor (i.e. 'pseries') should use > > ppc_check_compat(). > >=20 > > ppc_set_compat() is modified to validate the compatibility mode it is g= iven > > and fail if it's not available on this CPU. > >=20 > > [1] Or user-only mode, which also obviously doesn't allow access to the > > hypervisor privileged PCR. We don't use that now, but could in future. > >=20 > > Signed-off-by: David Gibson > > --- > > target-ppc/compat.c | 41 +++++++++++++++++++++++++++++++++++++++++ > > target-ppc/cpu.h | 2 ++ > > 2 files changed, 43 insertions(+) > >=20 > > 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 > > + */ > > { /* 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 pvr) > > return NULL; > > } > > =20 > > +bool ppc_check_compat(PowerPCCPU *cpu, uint32_t compat_pvr, > > + uint32_t min_compat_pvr, uint32_t max_compat_pvr) > > +{ > > + 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); >=20 >=20 > You keep giving very generic names (as "min" and "max") to local > variables ;) For local variables, brevity is a virtue. --=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 --wYXww9TlNKyqAMAe Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYFwMuAAoJEGw4ysog2bOSTUIQAK0hFzWMsuLD7V/pSf1sHPqm xHjnQc+w3vyRYEgKni2zZf4cYuGgduhbS1mjddoANfNE4JxBP4x5RDMgS8vr0SCw LCUIR7OF7it1BdvY3E3s/OIcVhnOVyvNHl2FZKTPDQeJqpKlBzDBWJDNd3rJ8mqo 6MvKSSI7yRomvA02VcEPvUfCtIWgWutdOkKf81CTDZcoOCEanAFiK4s+/Cwu7fHq wvk4bkqnxdTaykmFsgvaX4cNQYKSZ0ICe9bIMzK4Teqy6xID7J1bWNwNAuGzc6eA PdU6ObstQcVcuG+9YPsWA6rbeUlDrsJysfnhM7tgEnVe9KB9Zxr1sScdmEtIrh1U HTlPfZNEBXBQCjO+F8ZZHMu+acCErvpQIMd1C/x8rFIAMADfd+KlR56wc+fvT9MX BJCNDLxH0d9x0CtLKtiX9t8MBod+7J/CIgmZ8LMomKFOWU4H+EM06Vju7M6WVmxf JxZCwQZQE7DuGVynqH+hONsVut+Sk29a1MkHLGIRjId+mWTPqg/17wMI4QQXweqs wcxfhEgJ05ZafmtTur6n1syBQ80TwlVhe33Pkz+hpaOfE5ZAarU0+wX4m1CV6mm1 J23u+B6vynVFQGpyVvQSloWfmS55PTPHLCW4ks32xoVEAijZGWMojDcS2jOA7zjZ BBZQv7Pm6dfGW2ZzjY0B =1Jrx -----END PGP SIGNATURE----- --wYXww9TlNKyqAMAe--