[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5] powerpc: add PVR mask support
|
From: |
Andreas Färber |
|
Subject: |
Re: [Qemu-devel] [PATCH v5] powerpc: add PVR mask support |
|
Date: |
Wed, 28 Aug 2013 12:49:27 +0200 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130801 Thunderbird/17.0.8 |
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.
>
> 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.
>
> As CPU family class name for POWER7 is "POWER7-family", there is no need
> to touch aliases.
>
> Cc: Andreas Färber <address@hidden>
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
>
> ---
>
> I would really love to avoid adding masks to other classes as long as
> possible -
> I do not know them well, they do not know me, I am scared of breaking them :)
>
>
> ---
> 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 mask for
> it
> * added mask for POWER8
> * added _BASE suffix to PVR mask constants and moved them before actual CPUs
>
> v4:
> * removed bogus layer from hierarchy
>
> v3:
> * renamed macros to describe the functionality better
> * added default PVR value for the powerpc cpu family (what alias used to do)
>
> 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(-)
>
> 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 = POWERPC_CPU_CLASS(oc); \
> \
> pcc->pvr = _pvr; \
> + pcc->pvr_mask = CPU_POWERPC_DEFAULT_MASK; \
> pcc->svr = _svr; \
> dc->desc = _desc; \
> } \
> @@ -1139,7 +1140,7 @@
> "POWER7 v2.1")
> POWERPC_DEF("POWER7_v2.3", CPU_POWERPC_POWER7_v23, POWER7,
> "POWER7 v2.3")
> - POWERPC_DEF("POWER7+_v2.1", CPU_POWERPC_POWER7P_v21, POWER7,
> + POWERPC_DEF("POWER7+_v2.1", CPU_POWERPC_POWER7P_v21, POWER7P,
> "POWER7+ v2.1")
> POWERPC_DEF("POWER8_v1.0", CPU_POWERPC_POWER8_v10, POWER8,
> "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 = 0xFFFFFFFF,
> /* PowerPC 401 family */
> /* Generic PowerPC 401 */
> #define CPU_POWERPC_401 CPU_POWERPC_401G2
> @@ -552,10 +553,16 @@ enum {
> CPU_POWERPC_POWER6 = 0x003E0000,
> CPU_POWERPC_POWER6_5 = 0x0F000001, /* POWER6 in POWER5 mode */
> CPU_POWERPC_POWER6A = 0x0F000002,
> + CPU_POWERPC_POWER7_BASE = 0x003F0000,
> + CPU_POWERPC_POWER7_MASK = 0xFFFF0000,
> CPU_POWERPC_POWER7_v20 = 0x003F0200,
> CPU_POWERPC_POWER7_v21 = 0x003F0201,
> CPU_POWERPC_POWER7_v23 = 0x003F0203,
> + CPU_POWERPC_POWER7P_BASE = 0x004A0000,
> + CPU_POWERPC_POWER7P_MASK = 0xFFFF0000,
> CPU_POWERPC_POWER7P_v21 = 0x004A0201,
> + CPU_POWERPC_POWER8_BASE = 0x004B0000,
> + CPU_POWERPC_POWER8_MASK = 0xFFFF0000,
> CPU_POWERPC_POWER8_v10 = 0x004B0100,
> CPU_POWERPC_970 = 0x00390202,
> CPU_POWERPC_970FX_v10 = 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);
>
> 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(ObjectClass *oc,
> void *data)
> uint32_t icache_size = kvmppc_read_int_cpu_dt("i-cache-size");
>
> /* Now fix up the class with information we can query from the host */
> + pcc->pvr = mfpvr();
>
> if (vmx != -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) = { \
> .name = stringify(_name) "-family-" TYPE_POWERPC_CPU, \
> .parent = TYPE_POWERPC_CPU, \
> - .abstract = true, \
> .class_init = glue(glue(ppc_, _name), _cpu_family_class_init), \
> }; \
> \
Comment not yet incorporated.
> @@ -7216,6 +7215,45 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
>
> dc->fw_name = "PowerPC,POWER7";
> dc->desc = "POWER7";
> + pcc->pvr = CPU_POWERPC_POWER7_BASE;
> + pcc->pvr_mask = CPU_POWERPC_POWER7_MASK;
> + pcc->init_proc = init_proc_POWER7;
> + pcc->check_pow = check_pow_nocheck;
> + pcc->insns_flags = 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 = PPC2_VSX | PPC2_DFP | PPC2_DBRX | PPC2_ISA205;
> + pcc->msr_mask = 0x800000000204FF37ULL;
> + pcc->mmu_model = POWERPC_MMU_2_06;
> +#if defined(CONFIG_SOFTMMU)
> + pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
> +#endif
> + pcc->excp_model = POWERPC_EXCP_POWER7;
> + pcc->bus_model = PPC_FLAGS_INPUT_POWER7;
> + pcc->bfd_mach = bfd_mach_ppc64;
> + pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
> + POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
> + POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR;
> + pcc->l1_dcache_size = 0x8000;
> + pcc->l1_icache_size = 0x8000;
> +}
> +
> +POWERPC_FAMILY(POWER7P)(ObjectClass *oc, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(oc);
> + PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
> +
> + dc->fw_name = "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 accordingly.
> + dc->desc = "POWER7+";
> + pcc->pvr = CPU_POWERPC_POWER7P_BASE;
> + pcc->pvr_mask = CPU_POWERPC_POWER7P_MASK;
> pcc->init_proc = init_proc_POWER7;
> pcc->check_pow = check_pow_nocheck;
> pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
> @@ -7251,6 +7289,8 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
>
> dc->fw_name = "PowerPC,POWER8";
> dc->desc = "POWER8";
> + pcc->pvr = CPU_POWERPC_POWER8_BASE;
> + pcc->pvr_mask = CPU_POWERPC_POWER8_MASK;
> pcc->init_proc = init_proc_POWER7;
> pcc->check_pow = check_pow_nocheck;
> pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
> @@ -8165,7 +8205,7 @@ static gint ppc_cpu_compare_class_pvr(gconstpointer a,
> gconstpointer b)
> }
> #endif
>
> - return pcc->pvr == pvr ? 0 : -1;
> + return ((pcc->pvr == (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) == (pvr & pcc->pvr_mask)
to match specific models where available. Note that pvr_mask gets
inherited from the family like any other class field.
> }
>
> PowerPCCPUClass *ppc_cpu_class_by_pvr(uint32_t pvr)
> @@ -8326,6 +8366,8 @@ static void ppc_cpu_list_entry(gpointer data, gpointer
> user_data)
> CPUListState *s = user_data;
> PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
> const char *typename = object_class_get_name(oc);
> + ObjectClass *oc_parent = object_class_get_parent(oc);
> + const char *typename_parent = object_class_get_name(oc_parent);
> char *name;
> int i;
>
> @@ -8338,6 +8380,11 @@ static void ppc_cpu_list_entry(gpointer data, gpointer
> user_data)
> return;
> }
>
> + /* Do not show non-abstract family CPU classes */
> + if (unlikely(strcmp(typename_parent, TYPE_POWERPC_CPU) == 0)) {
> + return;
> + }
> +
> name = 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, void
> *data)
> DeviceClass *dc = DEVICE_CLASS(oc);
>
> pcc->parent_realize = dc->realize;
> + pcc->pvr = CPU_POWERPC_DEFAULT_MASK;
> + pcc->pvr_mask = 0;
> dc->realize = ppc_cpu_realizefn;
> dc->unrealize = ppc_cpu_unrealizefn;
>
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg