From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45857) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VHZSR-0007HW-3r for qemu-devel@nongnu.org; Thu, 05 Sep 2013 09:18:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VHZSL-0008Jm-H2 for qemu-devel@nongnu.org; Thu, 05 Sep 2013 09:18:47 -0400 Message-ID: <522884AD.7070909@suse.de> Date: Thu, 05 Sep 2013 15:18:37 +0200 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <1378360877-4455-1-git-send-email-aik@ozlabs.ru> In-Reply-To: <1378360877-4455-1-git-send-email-aik@ozlabs.ru> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v6] powerpc: add PVR mask support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: qemu-ppc@nongnu.org, Paul Mackerras , qemu-devel@nongnu.org, Alexander Graf Am 05.09.2013 08:01, schrieb Alexey Kardashevskiy: > IBM POWERPC processors encode PVR as a CPU family in higher 16 bits and > a CPU version in lower 16 bits. Since there is no significant change > in behavior between versions, there is no point to add every single CPU > version in QEMU's CPU list. Also, new CPU versions of already supported > CPU won't break the existing code. >=20 > This adds PVR value/mask support for KVM, i.e. for -cpu host option. >=20 > As CPU family class name for POWER7 is "POWER7-family", there is no nee= d > to touch aliases. >=20 > Cc: Andreas F=C3=A4rber > Signed-off-by: Alexey Kardashevskiy >=20 > --- > Changes: > v6: > * family classes are abstract again > * POWER7+ moved to a separate patch as it also need a separate family > * added ppc_cpu_class_by_pvr_mask() which is a copy of > ppc_cpu_class_by_pvr() but compares PVRs with masks; this function is > called from KVM code only to support the "-cpu host" option; unlike > the original search function, the new one also includes abstract family > classes. >=20 > v5: > * removed pvr_default > * added hiding of family CPU classes (should not appear in -cpu ?) > * separated POWER7+ into a class (it used to be POWER7) and added a mas= k for it > * added mask for POWER8 > * added _BASE suffix to PVR mask constants and moved them before actual= CPUs >=20 > v4: > * removed bogus layer from hierarchy >=20 > v3: > * renamed macros to describe the functionality better > * added default PVR value for the powerpc cpu family (what alias used t= o do) >=20 > v2: > * aliases are replaced with another level in class hierarchy > --- > target-ppc/cpu-models.c | 1 + > target-ppc/cpu-models.h | 5 +++++ > target-ppc/cpu-qom.h | 2 ++ > target-ppc/kvm.c | 4 ++++ > target-ppc/translate_init.c | 45 +++++++++++++++++++++++++++++++++++++= +++++++- > 5 files changed, 56 insertions(+), 1 deletion(-) >=20 > diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c > index 8dea560..04d88c5 100644 > --- a/target-ppc/cpu-models.c > +++ b/target-ppc/cpu-models.c > @@ -44,6 +44,7 @@ > PowerPCCPUClass *pcc =3D POWERPC_CPU_CLASS(oc); = \ > = \ > pcc->pvr =3D _pvr; = \ > + pcc->pvr_mask =3D CPU_POWERPC_DEFAULT_MASK; = \ > pcc->svr =3D _svr; = \ > dc->desc =3D _desc; = \ > } = \ > diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h > index d9145d1..731ec4a 100644 > --- a/target-ppc/cpu-models.h > +++ b/target-ppc/cpu-models.h > @@ -39,6 +39,7 @@ extern PowerPCCPUAlias ppc_cpu_aliases[]; > /*********************************************************************= ********/ > /* PVR definitions for most known PowerPC = */ > enum { > + CPU_POWERPC_DEFAULT_MASK =3D 0xFFFFFFFF, > /* PowerPC 401 family */ > /* Generic PowerPC 401 */ > #define CPU_POWERPC_401 CPU_POWERPC_401G2 > @@ -552,10 +553,14 @@ enum { > CPU_POWERPC_POWER6 =3D 0x003E0000, > CPU_POWERPC_POWER6_5 =3D 0x0F000001, /* POWER6 in POWER5= mode */ > CPU_POWERPC_POWER6A =3D 0x0F000002, > + CPU_POWERPC_POWER7_BASE =3D 0x003F0000, > + CPU_POWERPC_POWER7_MASK =3D 0xFFFF0000, > CPU_POWERPC_POWER7_v20 =3D 0x003F0200, > CPU_POWERPC_POWER7_v21 =3D 0x003F0201, > CPU_POWERPC_POWER7_v23 =3D 0x003F0203, > CPU_POWERPC_POWER7P_v21 =3D 0x004A0201, > + CPU_POWERPC_POWER8_BASE =3D 0x004B0000, > + CPU_POWERPC_POWER8_MASK =3D 0xFFFF0000, > CPU_POWERPC_POWER8_v10 =3D 0x004B0100, > CPU_POWERPC_970 =3D 0x00390202, > CPU_POWERPC_970FX_v10 =3D 0x00391100, > diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h > index f3c710a..3f82629 100644 > --- a/target-ppc/cpu-qom.h > +++ b/target-ppc/cpu-qom.h > @@ -54,6 +54,7 @@ typedef struct PowerPCCPUClass { > void (*parent_reset)(CPUState *cpu); > =20 > uint32_t pvr; > + uint32_t pvr_mask; > uint32_t svr; > uint64_t insns_flags; > uint64_t insns_flags2; > @@ -99,6 +100,7 @@ static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCStat= e *env) > #define ENV_OFFSET offsetof(PowerPCCPU, env) > =20 > PowerPCCPUClass *ppc_cpu_class_by_pvr(uint32_t pvr); > +PowerPCCPUClass *ppc_cpu_class_by_pvr_mask(uint32_t pvr); > =20 > void ppc_cpu_do_interrupt(CPUState *cpu); > void ppc_cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function cpu_f= printf, > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c > index 8a196c6..d10dba2 100644 > --- a/target-ppc/kvm.c > +++ b/target-ppc/kvm.c > @@ -1732,6 +1732,7 @@ static void kvmppc_host_cpu_class_init(ObjectClas= s *oc, void *data) > uint32_t icache_size =3D kvmppc_read_int_cpu_dt("i-cache-size"); > =20 > /* Now fix up the class with information we can query from the hos= t */ > + pcc->pvr =3D mfpvr(); > =20 > if (vmx !=3D -1) { > /* Only override when we know what the host supports */ > @@ -1782,6 +1783,9 @@ static int kvm_ppc_register_host_cpu_type(void) > =20 > pvr_pcc =3D ppc_cpu_class_by_pvr(host_pvr); > if (pvr_pcc =3D=3D NULL) { > + pvr_pcc =3D ppc_cpu_class_by_pvr_mask(host_pvr); > + } > + if (pvr_pcc =3D=3D NULL) { > return -1; > } > type_info.parent =3D object_class_get_name(OBJECT_CLASS(pvr_pcc)); Not quite what I had expected but I'm okay with that as well - allows later reuse of the helper and doing it in two explicit steps, once without masking, once with, avoids having to do list filtering/ordering. > diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c > index 761b2e5..7e37cf2 100644 > --- a/target-ppc/translate_init.c > +++ b/target-ppc/translate_init.c > @@ -7216,6 +7216,8 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *dat= a) > =20 > dc->fw_name =3D "PowerPC,POWER7"; > dc->desc =3D "POWER7"; > + pcc->pvr =3D CPU_POWERPC_POWER7_BASE; > + pcc->pvr_mask =3D CPU_POWERPC_POWER7_MASK; > pcc->init_proc =3D init_proc_POWER7; > pcc->check_pow =3D check_pow_nocheck; > pcc->insns_flags =3D PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_= MFTB | > @@ -7251,6 +7253,8 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *dat= a) > =20 > dc->fw_name =3D "PowerPC,POWER8"; > dc->desc =3D "POWER8"; > + pcc->pvr =3D CPU_POWERPC_POWER8_BASE; > + pcc->pvr_mask =3D CPU_POWERPC_POWER8_MASK; > pcc->init_proc =3D init_proc_POWER7; > pcc->check_pow =3D check_pow_nocheck; > pcc->insns_flags =3D PPC_INSNS_BASE | PPC_STRING | PPC_MFTB | > @@ -8164,7 +8168,6 @@ static gint ppc_cpu_compare_class_pvr(gconstpoint= er a, gconstpointer b) > return -1; > } > #endif > - > return pcc->pvr =3D=3D pvr ? 0 : -1; > } > =20 Stray whitespace change FWIW. > @@ -8183,6 +8186,44 @@ PowerPCCPUClass *ppc_cpu_class_by_pvr(uint32_t p= vr) > return pcc; > } > =20 > +static gint ppc_cpu_compare_class_pvr_mask(gconstpointer a, gconstpoin= ter b) > +{ > + ObjectClass *oc =3D (ObjectClass *)a; > + uint32_t pvr =3D *(uint32_t *)b; > + PowerPCCPUClass *pcc =3D (PowerPCCPUClass *)a; > + gint ret; > + > + /* -cpu host does a PVR lookup during construction */ > + if (unlikely(strcmp(object_class_get_name(oc), > + TYPE_HOST_POWERPC_CPU) =3D=3D 0)) { > + return -1; > + } > + > +#if defined(TARGET_PPCEMB) > + if (pcc->mmu_model !=3D POWERPC_MMU_BOOKE) { > + return -1; > + } > +#endif > + ret =3D (((pcc->pvr & pcc->pvr_mask) =3D=3D (pvr & pcc->pvr_mask))= ? 0 : -1); > + > + return ret; > +} > + > +PowerPCCPUClass *ppc_cpu_class_by_pvr_mask(uint32_t pvr) > +{ > + GSList *list, *item; > + PowerPCCPUClass *pcc =3D NULL; > + > + list =3D object_class_get_list(TYPE_POWERPC_CPU, true); > + item =3D g_slist_find_custom(list, &pvr, ppc_cpu_compare_class_pvr= _mask); > + if (item !=3D NULL) { > + pcc =3D POWERPC_CPU_CLASS(item->data); > + } > + g_slist_free(list); > + > + return pcc; > +} > + > static gint ppc_cpu_compare_class_name(gconstpointer a, gconstpointer = b) > { > ObjectClass *oc =3D (ObjectClass *)a; > @@ -8557,6 +8598,8 @@ static void ppc_cpu_class_init(ObjectClass *oc, v= oid *data) > DeviceClass *dc =3D DEVICE_CLASS(oc); > =20 > pcc->parent_realize =3D dc->realize; > + pcc->pvr =3D CPU_POWERPC_DEFAULT_MASK; > + pcc->pvr_mask =3D 0; I guess you meant pcc->pvr_mask =3D CPU_POWERPC_DEFAULT_MASK instead? No need to zero-initialize fields in class_init. > dc->realize =3D ppc_cpu_realizefn; > dc->unrealize =3D ppc_cpu_unrealizefn; > =20 Otherwise looking very good IMO. Alex? Regards, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg