* [Qemu-devel] [PATCH v5] powerpc: add PVR mask support
@ 2013-08-28 10:37 Alexey Kardashevskiy
2013-08-28 10:49 ` Andreas Färber
0 siblings, 1 reply; 7+ messages in thread
From: Alexey Kardashevskiy @ 2013-08-28 10:37 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, Alexander Graf, qemu-ppc, Paul Mackerras,
Andreas Färber
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 <afaerber@suse.de>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
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")
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), \
}; \
\
@@ -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+";
+ 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);
}
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",
@@ -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;
--
1.8.4.rc4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v5] powerpc: add PVR mask support
2013-08-28 10:37 [Qemu-devel] [PATCH v5] powerpc: add PVR mask support Alexey Kardashevskiy
@ 2013-08-28 10:49 ` Andreas Färber
2013-08-28 11:01 ` Alexey Kardashevskiy
2013-08-29 3:33 ` Paul Mackerras
0 siblings, 2 replies; 7+ messages in thread
From: Andreas Färber @ 2013-08-28 10:49 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: qemu-ppc, Paul Mackerras, qemu-devel, 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.
>
> 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 <afaerber@suse.de>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>
> ---
>
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v5] powerpc: add PVR mask support
2013-08-28 10:49 ` Andreas Färber
@ 2013-08-28 11:01 ` Alexey Kardashevskiy
2013-09-04 10:15 ` Alexey Kardashevskiy
2013-08-29 3:33 ` Paul Mackerras
1 sibling, 1 reply; 7+ messages in thread
From: Alexey Kardashevskiy @ 2013-08-28 11:01 UTC (permalink / raw)
To: Andreas Färber; +Cc: qemu-ppc, Paul Mackerras, qemu-devel, Alexander Graf
On 08/28/2013 08:49 PM, Andreas Färber wrote:
> 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 <afaerber@suse.de>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>
>> ---
>>
>> 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.
This I will double check tomorrow with Paul.
>
>> + 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.
Sorry, I missed that. What is the correct place? Or do you mean here a
am-I-compatible-with-this-host callback?
> 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.
Will it protect us from some real case (I cannot think of any good example
right now) or it is just against theoretical case when the mask is from one
class and the pvr from another?
>> }
>>
>> 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.
Ah. Sorry. Just realized what you meant. Mosaic became a picture now :)
>
>> @@ -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;
--
Alexey
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v5] powerpc: add PVR mask support
2013-08-28 10:49 ` Andreas Färber
2013-08-28 11:01 ` Alexey Kardashevskiy
@ 2013-08-29 3:33 ` Paul Mackerras
2013-08-29 4:52 ` Paul Mackerras
1 sibling, 1 reply; 7+ messages in thread
From: Paul Mackerras @ 2013-08-29 3:33 UTC (permalink / raw)
To: Andreas Färber
Cc: Alexey Kardashevskiy, qemu-ppc, qemu-devel, Alexander Graf
On Wed, Aug 28, 2013 at 12:49:27PM +0200, Andreas Färber wrote:
> 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.
It should be "POWER7+" for consistency with other hypervisors and
firmware.
Regards,
Paul.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v5] powerpc: add PVR mask support
2013-08-29 3:33 ` Paul Mackerras
@ 2013-08-29 4:52 ` Paul Mackerras
2013-08-29 4:59 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 7+ messages in thread
From: Paul Mackerras @ 2013-08-29 4:52 UTC (permalink / raw)
To: Andreas Färber
Cc: Alexey Kardashevskiy, qemu-ppc, qemu-devel, Alexander Graf
On Thu, Aug 29, 2013 at 01:33:07PM +1000, Paul Mackerras wrote:
> On Wed, Aug 28, 2013 at 12:49:27PM +0200, Andreas Färber wrote:
>
> > 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.
>
> It should be "POWER7+" for consistency with other hypervisors and
> firmware.
Actually, on a POWER7+ machine under PowerVM here, I am seeing
PowerPC,POWER7@xxx --- but PowerVM is running the partition in v2.06
architected mode rather than POWER7+ native mode. I'm not sure at the
moment how to get it to run in POWER7+ native mode (or even if there
is such a thing). In any case, I don't think it really matters
whether it's PowerPC,POWER7 or PowerPC,POWER7+.
Which brings me to another question. PAPR specifies that when a
partition is being run in a compatibility mode for an older processor,
the cpu-version property in the cpu nodes has a "logical PVR" value
representing the architecture level of the compatibility mode; for
example, 0x0f000001 for architecture 2.04, 0x0f000002 for 2.05
(POWER6), 0x0f000003 for 2.06 (POWER7), 0x0f000004 for 2.07 (POWER8).
We are going to need a way to do that for pseries guests. So for
example if we are running on a POWER8 machine and using KVM, we would
want to be able to tell qemu that we want a POWER7 machine and have
qemu put 0x0f000003 in the device tree, and also tell KVM to enable
POWER7 compatibility mode when running the guest, presumably by
setting the PVR for the guest to 0x0f000003. That PVR setting doesn't
(and can't) modify the PVR seen by the guest, since the hardware
doesn't provide any way to virtualize the PVR, but we will make it set
the compatibility mode for the guest.
How do you think we should handle this in qemu?
Regards,
Paul.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v5] powerpc: add PVR mask support
2013-08-29 4:52 ` Paul Mackerras
@ 2013-08-29 4:59 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 7+ messages in thread
From: Benjamin Herrenschmidt @ 2013-08-29 4:59 UTC (permalink / raw)
To: Paul Mackerras
Cc: Alexey Kardashevskiy, Alexander Graf, qemu-ppc,
Andreas Färber, qemu-devel
On Thu, 2013-08-29 at 14:52 +1000, Paul Mackerras wrote:
> We are going to need a way to do that for pseries guests. So for
> example if we are running on a POWER8 machine and using KVM, we would
> want to be able to tell qemu that we want a POWER7 machine and have
> qemu put 0x0f000003 in the device tree, and also tell KVM to enable
> POWER7 compatibility mode when running the guest, presumably by
> setting the PVR for the guest to 0x0f000003. That PVR setting doesn't
> (and can't) modify the PVR seen by the guest, since the hardware
> doesn't provide any way to virtualize the PVR, but we will make it set
> the compatibility mode for the guest.
>
> How do you think we should handle this in qemu?
And how does that relate to what we will do during reconfiguration
reboots ?
Cheers,
Ben.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v5] powerpc: add PVR mask support
2013-08-28 11:01 ` Alexey Kardashevskiy
@ 2013-09-04 10:15 ` Alexey Kardashevskiy
0 siblings, 0 replies; 7+ messages in thread
From: Alexey Kardashevskiy @ 2013-09-04 10:15 UTC (permalink / raw)
To: Andreas Färber; +Cc: qemu-ppc, Paul Mackerras, qemu-devel, Alexander Graf
On 08/28/2013 09:01 PM, Alexey Kardashevskiy wrote:
> On 08/28/2013 08:49 PM, Andreas Färber wrote:
>> 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 <afaerber@suse.de>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>
>>> ---
>>>
>>> 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.
>
>
> This I will double check tomorrow with Paul.
>
>>
>>> + 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.
>
> Sorry, I missed that. What is the correct place? Or do you mean here a
> am-I-compatible-with-this-host callback?
Andreas, could you please comment on that? Thanks.
--
Alexey
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-09-04 10:16 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-28 10:37 [Qemu-devel] [PATCH v5] powerpc: add PVR mask support Alexey Kardashevskiy
2013-08-28 10:49 ` Andreas Färber
2013-08-28 11:01 ` Alexey Kardashevskiy
2013-09-04 10:15 ` Alexey Kardashevskiy
2013-08-29 3:33 ` Paul Mackerras
2013-08-29 4:52 ` Paul Mackerras
2013-08-29 4:59 ` Benjamin Herrenschmidt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).