[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6] powerpc: add PVR mask support
|
From: |
Andreas Färber |
|
Subject: |
Re: [Qemu-devel] [PATCH v6] powerpc: add PVR mask support |
|
Date: |
Thu, 05 Sep 2013 15:18:37 +0200 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130801 Thunderbird/17.0.8 |
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.
>
> This adds PVR value/mask support for KVM, i.e. for -cpu host option.
>
> 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>
>
> ---
> 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.
>
> 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 | 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(-)
>
> 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 = POWERPC_CPU_CLASS(oc); \
> \
> pcc->pvr = _pvr; \
> + pcc->pvr_mask = CPU_POWERPC_DEFAULT_MASK; \
> pcc->svr = _svr; \
> dc->desc = _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 = 0xFFFFFFFF,
> /* PowerPC 401 family */
> /* Generic PowerPC 401 */
> #define CPU_POWERPC_401 CPU_POWERPC_401G2
> @@ -552,10 +553,14 @@ 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_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..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);
>
> 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(CPUPPCState *env)
> #define ENV_OFFSET offsetof(PowerPCCPU, env)
>
> PowerPCCPUClass *ppc_cpu_class_by_pvr(uint32_t pvr);
> +PowerPCCPUClass *ppc_cpu_class_by_pvr_mask(uint32_t pvr);
>
> void ppc_cpu_do_interrupt(CPUState *cpu);
> void ppc_cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function cpu_fprintf,
> 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(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 */
> @@ -1782,6 +1783,9 @@ static int kvm_ppc_register_host_cpu_type(void)
>
> pvr_pcc = ppc_cpu_class_by_pvr(host_pvr);
> if (pvr_pcc == NULL) {
> + pvr_pcc = ppc_cpu_class_by_pvr_mask(host_pvr);
> + }
> + if (pvr_pcc == NULL) {
> return -1;
> }
> type_info.parent = 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 *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 |
> @@ -7251,6 +7253,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 |
> @@ -8164,7 +8168,6 @@ static gint ppc_cpu_compare_class_pvr(gconstpointer a,
> gconstpointer b)
> return -1;
> }
> #endif
> -
> return pcc->pvr == pvr ? 0 : -1;
> }
>
Stray whitespace change FWIW.
> @@ -8183,6 +8186,44 @@ PowerPCCPUClass *ppc_cpu_class_by_pvr(uint32_t pvr)
> return pcc;
> }
>
> +static gint ppc_cpu_compare_class_pvr_mask(gconstpointer a, gconstpointer b)
> +{
> + ObjectClass *oc = (ObjectClass *)a;
> + uint32_t pvr = *(uint32_t *)b;
> + PowerPCCPUClass *pcc = (PowerPCCPUClass *)a;
> + gint ret;
> +
> + /* -cpu host does a PVR lookup during construction */
> + if (unlikely(strcmp(object_class_get_name(oc),
> + TYPE_HOST_POWERPC_CPU) == 0)) {
> + return -1;
> + }
> +
> +#if defined(TARGET_PPCEMB)
> + if (pcc->mmu_model != POWERPC_MMU_BOOKE) {
> + return -1;
> + }
> +#endif
> + ret = (((pcc->pvr & pcc->pvr_mask) == (pvr & pcc->pvr_mask)) ? 0 : -1);
> +
> + return ret;
> +}
> +
> +PowerPCCPUClass *ppc_cpu_class_by_pvr_mask(uint32_t pvr)
> +{
> + GSList *list, *item;
> + PowerPCCPUClass *pcc = NULL;
> +
> + list = object_class_get_list(TYPE_POWERPC_CPU, true);
> + item = g_slist_find_custom(list, &pvr, ppc_cpu_compare_class_pvr_mask);
> + if (item != NULL) {
> + pcc = POWERPC_CPU_CLASS(item->data);
> + }
> + g_slist_free(list);
> +
> + return pcc;
> +}
> +
> static gint ppc_cpu_compare_class_name(gconstpointer a, gconstpointer b)
> {
> ObjectClass *oc = (ObjectClass *)a;
> @@ -8557,6 +8598,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;
I guess you meant pcc->pvr_mask = CPU_POWERPC_DEFAULT_MASK instead?
No need to zero-initialize fields in class_init.
> dc->realize = ppc_cpu_realizefn;
> dc->unrealize = ppc_cpu_unrealizefn;
>
Otherwise looking very good IMO. Alex?
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