From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56342) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VEdJh-0008HP-C5 for qemu-devel@nongnu.org; Wed, 28 Aug 2013 06:49:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VEdJb-0000iM-QC for qemu-devel@nongnu.org; Wed, 28 Aug 2013 06:49:37 -0400 Message-ID: <521DD5B7.6070004@suse.de> Date: Wed, 28 Aug 2013 12:49:27 +0200 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <1377686237-17683-1-git-send-email-aik@ozlabs.ru> In-Reply-To: <1377686237-17683-1-git-send-email-aik@ozlabs.ru> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v5] 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 28.08.2013 12:37, 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 does the following: > 1. add a PVR mask support for a CPU family; > 2. make CPU family not abstract; > 3. hide family CPU classes from "-cpu ?" list. >=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 > --- >=20 > I would really love to avoid adding masks to other classes as long as p= ossible - > I do not know them well, they do not know me, I am scared of breaking t= hem :) >=20 >=20 > --- > Changes: > 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 | 3 ++- > target-ppc/cpu-models.h | 7 ++++++ > target-ppc/cpu-qom.h | 1 + > target-ppc/kvm.c | 1 + > target-ppc/translate_init.c | 53 +++++++++++++++++++++++++++++++++++++= ++++++-- > 5 files changed, 62 insertions(+), 3 deletions(-) >=20 > diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c > index 8dea560..7c9466f 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; = \ > } = \ > @@ -1139,7 +1140,7 @@ > "POWER7 v2.1") > POWERPC_DEF("POWER7_v2.3", CPU_POWERPC_POWER7_v23, P= OWER7, > "POWER7 v2.3") > - POWERPC_DEF("POWER7+_v2.1", CPU_POWERPC_POWER7P_v21, P= OWER7, > + POWERPC_DEF("POWER7+_v2.1", CPU_POWERPC_POWER7P_v21, P= OWER7P, > "POWER7+ v2.1") > POWERPC_DEF("POWER8_v1.0", CPU_POWERPC_POWER8_v10, P= OWER8, > "POWER8 v1.0") As always, please put independent changes like this POWER7P introduction in its own patch. > diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h > index d9145d1..49ba4a4 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,16 @@ 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_BASE =3D 0x004A0000, > + CPU_POWERPC_POWER7P_MASK =3D 0xFFFF0000, > 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..0ae8b09 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; > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c > index 30a870e..d7d9865 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 */ > diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c > index 761b2e5..39cb69b 100644 > --- a/target-ppc/translate_init.c > +++ b/target-ppc/translate_init.c > @@ -3105,7 +3105,6 @@ static int check_pow_hid0_74xx (CPUPPCState *env) > glue(glue(ppc_, _name), _cpu_family_type_info) =3D { = \ > .name =3D stringify(_name) "-family-" TYPE_POWERPC_CPU, = \ > .parent =3D TYPE_POWERPC_CPU, = \ > - .abstract =3D true, = \ > .class_init =3D glue(glue(ppc_, _name), _cpu_family_class_init= ), \ > }; = \ > = \ Comment not yet incorporated. > @@ -7216,6 +7215,45 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *da= ta) > =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 | > + PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | > + PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | > + PPC_FLOAT_STFIWX | > + PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ | > + PPC_MEM_SYNC | PPC_MEM_EIEIO | > + PPC_MEM_TLBIE | PPC_MEM_TLBSYNC | > + PPC_64B | PPC_ALTIVEC | > + PPC_SEGMENT_64B | PPC_SLBI | > + PPC_POPCNTB | PPC_POPCNTWD; > + pcc->insns_flags2 =3D PPC2_VSX | PPC2_DFP | PPC2_DBRX | PPC2_ISA20= 5; > + pcc->msr_mask =3D 0x800000000204FF37ULL; > + pcc->mmu_model =3D POWERPC_MMU_2_06; > +#if defined(CONFIG_SOFTMMU) > + pcc->handle_mmu_fault =3D ppc_hash64_handle_mmu_fault; > +#endif > + pcc->excp_model =3D POWERPC_EXCP_POWER7; > + pcc->bus_model =3D PPC_FLAGS_INPUT_POWER7; > + pcc->bfd_mach =3D bfd_mach_ppc64; > + pcc->flags =3D POWERPC_FLAG_VRE | POWERPC_FLAG_SE | > + POWERPC_FLAG_BE | POWERPC_FLAG_PMM | > + POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR; > + pcc->l1_dcache_size =3D 0x8000; > + pcc->l1_icache_size =3D 0x8000; > +} > + > +POWERPC_FAMILY(POWER7P)(ObjectClass *oc, void *data) > +{ > + DeviceClass *dc =3D DEVICE_CLASS(oc); > + PowerPCCPUClass *pcc =3D POWERPC_CPU_CLASS(oc); > + > + dc->fw_name =3D "PowerPC,POWER7+"; According to the only reply I received, it's "POWER7", not "POWER7+" - see my patch description. If that information was wrong, we'll need to move POWER7P introduction before my fw_name patch and update it according= ly. > + dc->desc =3D "POWER7+"; > + pcc->pvr =3D CPU_POWERPC_POWER7P_BASE; > + pcc->pvr_mask =3D CPU_POWERPC_POWER7P_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 +7289,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 | > @@ -8165,7 +8205,7 @@ static gint ppc_cpu_compare_class_pvr(gconstpoint= er a, gconstpointer b) > } > #endif > =20 > - return pcc->pvr =3D=3D pvr ? 0 : -1; > + return ((pcc->pvr =3D=3D (pvr & pcc->pvr_mask)) ? 0 : -1); As discussed in lengths this is the wrong place IMO. Also, the comparison should be: (pcc->pvr & pcc->pvr_mask) =3D=3D (pvr & pcc->pvr_mask) to match specific models where available. Note that pvr_mask gets inherited from the family like any other class field. > } > =20 > PowerPCCPUClass *ppc_cpu_class_by_pvr(uint32_t pvr) > @@ -8326,6 +8366,8 @@ static void ppc_cpu_list_entry(gpointer data, gpo= inter user_data) > CPUListState *s =3D user_data; > PowerPCCPUClass *pcc =3D POWERPC_CPU_CLASS(oc); > const char *typename =3D object_class_get_name(oc); > + ObjectClass *oc_parent =3D object_class_get_parent(oc); > + const char *typename_parent =3D object_class_get_name(oc_parent); > char *name; > int i; > =20 > @@ -8338,6 +8380,11 @@ static void ppc_cpu_list_entry(gpointer data, gp= ointer user_data) > return; > } > =20 > + /* Do not show non-abstract family CPU classes */ > + if (unlikely(strcmp(typename_parent, TYPE_POWERPC_CPU) =3D=3D 0)) = { > + return; > + } > + > name =3D g_strndup(typename, > strlen(typename) - strlen("-" TYPE_POWERPC_CPU)); > (*s->cpu_fprintf)(s->file, "PowerPC %-16s PVR %08x\n", Becomes unnecessary when dropping the abstractness change. > @@ -8557,6 +8604,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; > dc->realize =3D ppc_cpu_realizefn; > dc->unrealize =3D ppc_cpu_unrealizefn; > =20 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