Re: [Qemu-devel] [PATCH v2] target-ppc: move POWER7+ to a separate family
On 11/12/2013 06:18 PM, Alexey Kardashevskiy wrote: On 11/09/2013 11:20 AM, Alexey Kardashevskiy wrote: On 11/09/2013 03:59 AM, Andreas Färber wrote: Am 08.11.2013 15:54, schrieb Alexey Kardashevskiy: On 11/09/2013 12:44 AM, Andreas Färber wrote: Am 08.11.2013 03:37, schrieb Alexey Kardashevskiy: So far POWER7+ was a part of POWER7 family. However it has a different PVR base value so in order to support PVR masks, it needs a separate family class. Alexey, Another reason to make a POWER7+ family is that its name in the device tree (/proc/device-tree/cpus/cpu*) should be Power7+ but not Power7 and this cannot be easily fixed without a new family class. This adds a new family class, PVR base and mask values and moves Power7+ v2.1 CPU to a new family. The class init function is copied from the POWER7 family. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- Changes: v2: * added VSX enable bit --- target-ppc/cpu-models.c | 2 +- target-ppc/cpu-models.h | 2 ++ target-ppc/translate_init.c | 38 ++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c index 04d88c5..7c9466f 100644 --- a/target-ppc/cpu-models.c +++ b/target-ppc/cpu-models.c @@ -1140,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 731ec4a..49ba4a4 100644 --- a/target-ppc/cpu-models.h +++ b/target-ppc/cpu-models.h @@ -558,6 +558,8 @@ enum { CPU_POWERPC_POWER7_v20 = 0x003F0200, CPU_POWERPC_POWER7_v21 = 0x003F0201, CPU_POWERPC_POWER7_v23 = 0x003F0203, +CPU_POWERPC_POWER7P_BASE = 0x004A, +CPU_POWERPC_POWER7P_MASK = 0x, CPU_POWERPC_POWER7P_v21= 0x004A0201, CPU_POWERPC_POWER8_BASE= 0x004B, CPU_POWERPC_POWER8_MASK= 0x, diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index 35d1389..c030a20 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -7253,6 +7253,44 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data) 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+; Apart from the commit message differing from the code... In what part? The spelling of POWER7. You write it should be Power7+ but implement it as upper-case POWER7+ (ignoring the PowerPC, prefix, that is). Ah. Sorry. We've had this discussion before: Jacques reported that on his POWER7+ box only POWER7 is shown, not POWER7+, equivalent to my POWER5+ box showing only PowerPC,POWER5. Compare my commit, which documents this: http://git.qemu.org/?p=qemu.git;a=commit;h=793826cd460828975591f289de78672af4a47ef9 So, adding a POWER7P family seems correct to me, just the fw_name seems wrong - or you'll need to investigate further why there are conflicting reports of how it is shown. Possibly based on revision or pHyp vs. SLOF? Yes we have had this discussion. Paul said it should POWER7+. The only P7+ machine I have handy shows +: [aik@vpl4 ~]$ ls -d /proc/device-tree/cpus/PowerPC* /proc/device-tree/cpus/PowerPC,POWER7+@0 /proc/device-tree/cpus/PowerPC,POWER7+@2c /proc/device-tree/cpus/PowerPC,POWER7+@10 /proc/device-tree/cpus/PowerPC,POWER7+@30 /proc/device-tree/cpus/PowerPC,POWER7+@14 /proc/device-tree/cpus/PowerPC,POWER7+@34 /proc/device-tree/cpus/PowerPC,POWER7+@18 /proc/device-tree/cpus/PowerPC,POWER7+@38 /proc/device-tree/cpus/PowerPC,POWER7+@1c /proc/device-tree/cpus/PowerPC,POWER7+@3c /proc/device-tree/cpus/PowerPC,POWER7+@20 /proc/device-tree/cpus/PowerPC,POWER7+@4 /proc/device-tree/cpus/PowerPC,POWER7+@24 /proc/device-tree/cpus/PowerPC,POWER7+@8 /proc/device-tree/cpus/PowerPC,POWER7+@28 /proc/device-tree/cpus/PowerPC,POWER7+@c And this is a host, not a guest. I do not see any good reason to make dt names different. And this does not really matter if there is + or not for anybody as far as we concerned, ppc64_cpu works either way. Right, it may not matter, but I expect you to reference the above commit id and explain why it should be POWER7+ after all. You failed to come up with that answer before that patch got applied, so we need to correct me/it now. I have checked with Dinar that under Linux using the Sapphire firmware PowerPC,POWER7+@0 does indeed show up in
Re: [Qemu-devel] [PATCH v2] target-ppc: move POWER7+ to a separate family
On 18.11.2013, at 03:55, Alexey Kardashevskiy a...@ozlabs.ru wrote: On 11/12/2013 06:18 PM, Alexey Kardashevskiy wrote: On 11/09/2013 11:20 AM, Alexey Kardashevskiy wrote: On 11/09/2013 03:59 AM, Andreas Färber wrote: Am 08.11.2013 15:54, schrieb Alexey Kardashevskiy: On 11/09/2013 12:44 AM, Andreas Färber wrote: Am 08.11.2013 03:37, schrieb Alexey Kardashevskiy: So far POWER7+ was a part of POWER7 family. However it has a different PVR base value so in order to support PVR masks, it needs a separate family class. Alexey, Another reason to make a POWER7+ family is that its name in the device tree (/proc/device-tree/cpus/cpu*) should be Power7+ but not Power7 and this cannot be easily fixed without a new family class. This adds a new family class, PVR base and mask values and moves Power7+ v2.1 CPU to a new family. The class init function is copied from the POWER7 family. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- Changes: v2: * added VSX enable bit --- target-ppc/cpu-models.c | 2 +- target-ppc/cpu-models.h | 2 ++ target-ppc/translate_init.c | 38 ++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c index 04d88c5..7c9466f 100644 --- a/target-ppc/cpu-models.c +++ b/target-ppc/cpu-models.c @@ -1140,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 731ec4a..49ba4a4 100644 --- a/target-ppc/cpu-models.h +++ b/target-ppc/cpu-models.h @@ -558,6 +558,8 @@ enum { CPU_POWERPC_POWER7_v20 = 0x003F0200, CPU_POWERPC_POWER7_v21 = 0x003F0201, CPU_POWERPC_POWER7_v23 = 0x003F0203, +CPU_POWERPC_POWER7P_BASE = 0x004A, +CPU_POWERPC_POWER7P_MASK = 0x, CPU_POWERPC_POWER7P_v21= 0x004A0201, CPU_POWERPC_POWER8_BASE= 0x004B, CPU_POWERPC_POWER8_MASK= 0x, diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index 35d1389..c030a20 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -7253,6 +7253,44 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data) 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+; Apart from the commit message differing from the code... In what part? The spelling of POWER7. You write it should be Power7+ but implement it as upper-case POWER7+ (ignoring the PowerPC, prefix, that is). Ah. Sorry. We've had this discussion before: Jacques reported that on his POWER7+ box only POWER7 is shown, not POWER7+, equivalent to my POWER5+ box showing only PowerPC,POWER5. Compare my commit, which documents this: http://git.qemu.org/?p=qemu.git;a=commit;h=793826cd460828975591f289de78672af4a47ef9 So, adding a POWER7P family seems correct to me, just the fw_name seems wrong - or you'll need to investigate further why there are conflicting reports of how it is shown. Possibly based on revision or pHyp vs. SLOF? Yes we have had this discussion. Paul said it should POWER7+. The only P7+ machine I have handy shows +: [aik@vpl4 ~]$ ls -d /proc/device-tree/cpus/PowerPC* /proc/device-tree/cpus/PowerPC,POWER7+@0 /proc/device-tree/cpus/PowerPC,POWER7+@2c /proc/device-tree/cpus/PowerPC,POWER7+@10 /proc/device-tree/cpus/PowerPC,POWER7+@30 /proc/device-tree/cpus/PowerPC,POWER7+@14 /proc/device-tree/cpus/PowerPC,POWER7+@34 /proc/device-tree/cpus/PowerPC,POWER7+@18 /proc/device-tree/cpus/PowerPC,POWER7+@38 /proc/device-tree/cpus/PowerPC,POWER7+@1c /proc/device-tree/cpus/PowerPC,POWER7+@3c /proc/device-tree/cpus/PowerPC,POWER7+@20 /proc/device-tree/cpus/PowerPC,POWER7+@4 /proc/device-tree/cpus/PowerPC,POWER7+@24 /proc/device-tree/cpus/PowerPC,POWER7+@8 /proc/device-tree/cpus/PowerPC,POWER7+@28 /proc/device-tree/cpus/PowerPC,POWER7+@c And this is a host, not a guest. I do not see any good reason to make dt names different. And this does not really matter if there is + or not for anybody as far as we concerned, ppc64_cpu works either way. Right, it may not matter, but I expect you to reference the above commit id and explain why it should be POWER7+ after all. You failed to come up with that answer before that patch got applied, so we need to correct me/it now. I have checked with Dinar that under
Re: [Qemu-devel] [PATCH v2] target-ppc: move POWER7+ to a separate family
On 11/09/2013 11:20 AM, Alexey Kardashevskiy wrote: On 11/09/2013 03:59 AM, Andreas Färber wrote: Am 08.11.2013 15:54, schrieb Alexey Kardashevskiy: On 11/09/2013 12:44 AM, Andreas Färber wrote: Am 08.11.2013 03:37, schrieb Alexey Kardashevskiy: So far POWER7+ was a part of POWER7 family. However it has a different PVR base value so in order to support PVR masks, it needs a separate family class. Alexey, Another reason to make a POWER7+ family is that its name in the device tree (/proc/device-tree/cpus/cpu*) should be Power7+ but not Power7 and this cannot be easily fixed without a new family class. This adds a new family class, PVR base and mask values and moves Power7+ v2.1 CPU to a new family. The class init function is copied from the POWER7 family. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- Changes: v2: * added VSX enable bit --- target-ppc/cpu-models.c | 2 +- target-ppc/cpu-models.h | 2 ++ target-ppc/translate_init.c | 38 ++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c index 04d88c5..7c9466f 100644 --- a/target-ppc/cpu-models.c +++ b/target-ppc/cpu-models.c @@ -1140,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 731ec4a..49ba4a4 100644 --- a/target-ppc/cpu-models.h +++ b/target-ppc/cpu-models.h @@ -558,6 +558,8 @@ enum { CPU_POWERPC_POWER7_v20 = 0x003F0200, CPU_POWERPC_POWER7_v21 = 0x003F0201, CPU_POWERPC_POWER7_v23 = 0x003F0203, +CPU_POWERPC_POWER7P_BASE = 0x004A, +CPU_POWERPC_POWER7P_MASK = 0x, CPU_POWERPC_POWER7P_v21= 0x004A0201, CPU_POWERPC_POWER8_BASE= 0x004B, CPU_POWERPC_POWER8_MASK= 0x, diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index 35d1389..c030a20 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -7253,6 +7253,44 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data) 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+; Apart from the commit message differing from the code... In what part? The spelling of POWER7. You write it should be Power7+ but implement it as upper-case POWER7+ (ignoring the PowerPC, prefix, that is). Ah. Sorry. We've had this discussion before: Jacques reported that on his POWER7+ box only POWER7 is shown, not POWER7+, equivalent to my POWER5+ box showing only PowerPC,POWER5. Compare my commit, which documents this: http://git.qemu.org/?p=qemu.git;a=commit;h=793826cd460828975591f289de78672af4a47ef9 So, adding a POWER7P family seems correct to me, just the fw_name seems wrong - or you'll need to investigate further why there are conflicting reports of how it is shown. Possibly based on revision or pHyp vs. SLOF? Yes we have had this discussion. Paul said it should POWER7+. The only P7+ machine I have handy shows +: [aik@vpl4 ~]$ ls -d /proc/device-tree/cpus/PowerPC* /proc/device-tree/cpus/PowerPC,POWER7+@0 /proc/device-tree/cpus/PowerPC,POWER7+@2c /proc/device-tree/cpus/PowerPC,POWER7+@10 /proc/device-tree/cpus/PowerPC,POWER7+@30 /proc/device-tree/cpus/PowerPC,POWER7+@14 /proc/device-tree/cpus/PowerPC,POWER7+@34 /proc/device-tree/cpus/PowerPC,POWER7+@18 /proc/device-tree/cpus/PowerPC,POWER7+@38 /proc/device-tree/cpus/PowerPC,POWER7+@1c /proc/device-tree/cpus/PowerPC,POWER7+@3c /proc/device-tree/cpus/PowerPC,POWER7+@20 /proc/device-tree/cpus/PowerPC,POWER7+@4 /proc/device-tree/cpus/PowerPC,POWER7+@24 /proc/device-tree/cpus/PowerPC,POWER7+@8 /proc/device-tree/cpus/PowerPC,POWER7+@28 /proc/device-tree/cpus/PowerPC,POWER7+@c And this is a host, not a guest. I do not see any good reason to make dt names different. And this does not really matter if there is + or not for anybody as far as we concerned, ppc64_cpu works either way. Right, it may not matter, but I expect you to reference the above commit id and explain why it should be POWER7+ after all. You failed to come up with that answer before that patch got applied, so we need to correct me/it now. I have checked with Dinar that under Linux using the Sapphire firmware PowerPC,POWER7+@0 does indeed show up in /proc/device-tree/cpus. So that matches what this
Re: [Qemu-devel] [PATCH v2] target-ppc: move POWER7+ to a separate family
Am 08.11.2013 03:37, schrieb Alexey Kardashevskiy: So far POWER7+ was a part of POWER7 family. However it has a different PVR base value so in order to support PVR masks, it needs a separate family class. Alexey, Another reason to make a POWER7+ family is that its name in the device tree (/proc/device-tree/cpus/cpu*) should be Power7+ but not Power7 and this cannot be easily fixed without a new family class. This adds a new family class, PVR base and mask values and moves Power7+ v2.1 CPU to a new family. The class init function is copied from the POWER7 family. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- Changes: v2: * added VSX enable bit --- target-ppc/cpu-models.c | 2 +- target-ppc/cpu-models.h | 2 ++ target-ppc/translate_init.c | 38 ++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c index 04d88c5..7c9466f 100644 --- a/target-ppc/cpu-models.c +++ b/target-ppc/cpu-models.c @@ -1140,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 731ec4a..49ba4a4 100644 --- a/target-ppc/cpu-models.h +++ b/target-ppc/cpu-models.h @@ -558,6 +558,8 @@ enum { CPU_POWERPC_POWER7_v20 = 0x003F0200, CPU_POWERPC_POWER7_v21 = 0x003F0201, CPU_POWERPC_POWER7_v23 = 0x003F0203, +CPU_POWERPC_POWER7P_BASE = 0x004A, +CPU_POWERPC_POWER7P_MASK = 0x, CPU_POWERPC_POWER7P_v21= 0x004A0201, CPU_POWERPC_POWER8_BASE= 0x004B, CPU_POWERPC_POWER8_MASK= 0x, diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index 35d1389..c030a20 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -7253,6 +7253,44 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data) 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+; Apart from the commit message differing from the code... We've had this discussion before: Jacques reported that on his POWER7+ box only POWER7 is shown, not POWER7+, equivalent to my POWER5+ box showing only PowerPC,POWER5. Compare my commit, which documents this: http://git.qemu.org/?p=qemu.git;a=commit;h=793826cd460828975591f289de78672af4a47ef9 So, adding a POWER7P family seems correct to me, just the fw_name seems wrong - or you'll need to investigate further why there are conflicting reports of how it is shown. Possibly based on revision or pHyp vs. SLOF? Regards, Andreas +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 | + 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 = 0x8204FF37ULL; +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 | + POWERPC_FLAG_VSX; +pcc-l1_dcache_size = 0x8000; +pcc-l1_icache_size = 0x8000; +} + POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data) { DeviceClass *dc = DEVICE_CLASS(oc); -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v2] target-ppc: move POWER7+ to a separate family
On 11/09/2013 12:44 AM, Andreas Färber wrote: Am 08.11.2013 03:37, schrieb Alexey Kardashevskiy: So far POWER7+ was a part of POWER7 family. However it has a different PVR base value so in order to support PVR masks, it needs a separate family class. Alexey, Another reason to make a POWER7+ family is that its name in the device tree (/proc/device-tree/cpus/cpu*) should be Power7+ but not Power7 and this cannot be easily fixed without a new family class. This adds a new family class, PVR base and mask values and moves Power7+ v2.1 CPU to a new family. The class init function is copied from the POWER7 family. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- Changes: v2: * added VSX enable bit --- target-ppc/cpu-models.c | 2 +- target-ppc/cpu-models.h | 2 ++ target-ppc/translate_init.c | 38 ++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c index 04d88c5..7c9466f 100644 --- a/target-ppc/cpu-models.c +++ b/target-ppc/cpu-models.c @@ -1140,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 731ec4a..49ba4a4 100644 --- a/target-ppc/cpu-models.h +++ b/target-ppc/cpu-models.h @@ -558,6 +558,8 @@ enum { CPU_POWERPC_POWER7_v20 = 0x003F0200, CPU_POWERPC_POWER7_v21 = 0x003F0201, CPU_POWERPC_POWER7_v23 = 0x003F0203, +CPU_POWERPC_POWER7P_BASE = 0x004A, +CPU_POWERPC_POWER7P_MASK = 0x, CPU_POWERPC_POWER7P_v21= 0x004A0201, CPU_POWERPC_POWER8_BASE= 0x004B, CPU_POWERPC_POWER8_MASK= 0x, diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index 35d1389..c030a20 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -7253,6 +7253,44 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data) 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+; Apart from the commit message differing from the code... In what part? We've had this discussion before: Jacques reported that on his POWER7+ box only POWER7 is shown, not POWER7+, equivalent to my POWER5+ box showing only PowerPC,POWER5. Compare my commit, which documents this: http://git.qemu.org/?p=qemu.git;a=commit;h=793826cd460828975591f289de78672af4a47ef9 So, adding a POWER7P family seems correct to me, just the fw_name seems wrong - or you'll need to investigate further why there are conflicting reports of how it is shown. Possibly based on revision or pHyp vs. SLOF? Yes we have had this discussion. Paul said it should POWER7+. The only P7+ machine I have handy shows +: [aik@vpl4 ~]$ ls -d /proc/device-tree/cpus/PowerPC* /proc/device-tree/cpus/PowerPC,POWER7+@0 /proc/device-tree/cpus/PowerPC,POWER7+@2c /proc/device-tree/cpus/PowerPC,POWER7+@10 /proc/device-tree/cpus/PowerPC,POWER7+@30 /proc/device-tree/cpus/PowerPC,POWER7+@14 /proc/device-tree/cpus/PowerPC,POWER7+@34 /proc/device-tree/cpus/PowerPC,POWER7+@18 /proc/device-tree/cpus/PowerPC,POWER7+@38 /proc/device-tree/cpus/PowerPC,POWER7+@1c /proc/device-tree/cpus/PowerPC,POWER7+@3c /proc/device-tree/cpus/PowerPC,POWER7+@20 /proc/device-tree/cpus/PowerPC,POWER7+@4 /proc/device-tree/cpus/PowerPC,POWER7+@24 /proc/device-tree/cpus/PowerPC,POWER7+@8 /proc/device-tree/cpus/PowerPC,POWER7+@28 /proc/device-tree/cpus/PowerPC,POWER7+@c And this is a host, not a guest. I do not see any good reason to make dt names different. And this does not really matter if there is + or not for anybody as far as we concerned, ppc64_cpu works either way. Regards, Andreas +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 | + 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 | +
Re: [Qemu-devel] [PATCH v2] target-ppc: move POWER7+ to a separate family
Am 08.11.2013 15:54, schrieb Alexey Kardashevskiy: On 11/09/2013 12:44 AM, Andreas Färber wrote: Am 08.11.2013 03:37, schrieb Alexey Kardashevskiy: So far POWER7+ was a part of POWER7 family. However it has a different PVR base value so in order to support PVR masks, it needs a separate family class. Alexey, Another reason to make a POWER7+ family is that its name in the device tree (/proc/device-tree/cpus/cpu*) should be Power7+ but not Power7 and this cannot be easily fixed without a new family class. This adds a new family class, PVR base and mask values and moves Power7+ v2.1 CPU to a new family. The class init function is copied from the POWER7 family. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- Changes: v2: * added VSX enable bit --- target-ppc/cpu-models.c | 2 +- target-ppc/cpu-models.h | 2 ++ target-ppc/translate_init.c | 38 ++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c index 04d88c5..7c9466f 100644 --- a/target-ppc/cpu-models.c +++ b/target-ppc/cpu-models.c @@ -1140,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 731ec4a..49ba4a4 100644 --- a/target-ppc/cpu-models.h +++ b/target-ppc/cpu-models.h @@ -558,6 +558,8 @@ enum { CPU_POWERPC_POWER7_v20 = 0x003F0200, CPU_POWERPC_POWER7_v21 = 0x003F0201, CPU_POWERPC_POWER7_v23 = 0x003F0203, +CPU_POWERPC_POWER7P_BASE = 0x004A, +CPU_POWERPC_POWER7P_MASK = 0x, CPU_POWERPC_POWER7P_v21= 0x004A0201, CPU_POWERPC_POWER8_BASE= 0x004B, CPU_POWERPC_POWER8_MASK= 0x, diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index 35d1389..c030a20 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -7253,6 +7253,44 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data) 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+; Apart from the commit message differing from the code... In what part? The spelling of POWER7. You write it should be Power7+ but implement it as upper-case POWER7+ (ignoring the PowerPC, prefix, that is). We've had this discussion before: Jacques reported that on his POWER7+ box only POWER7 is shown, not POWER7+, equivalent to my POWER5+ box showing only PowerPC,POWER5. Compare my commit, which documents this: http://git.qemu.org/?p=qemu.git;a=commit;h=793826cd460828975591f289de78672af4a47ef9 So, adding a POWER7P family seems correct to me, just the fw_name seems wrong - or you'll need to investigate further why there are conflicting reports of how it is shown. Possibly based on revision or pHyp vs. SLOF? Yes we have had this discussion. Paul said it should POWER7+. The only P7+ machine I have handy shows +: [aik@vpl4 ~]$ ls -d /proc/device-tree/cpus/PowerPC* /proc/device-tree/cpus/PowerPC,POWER7+@0 /proc/device-tree/cpus/PowerPC,POWER7+@2c /proc/device-tree/cpus/PowerPC,POWER7+@10 /proc/device-tree/cpus/PowerPC,POWER7+@30 /proc/device-tree/cpus/PowerPC,POWER7+@14 /proc/device-tree/cpus/PowerPC,POWER7+@34 /proc/device-tree/cpus/PowerPC,POWER7+@18 /proc/device-tree/cpus/PowerPC,POWER7+@38 /proc/device-tree/cpus/PowerPC,POWER7+@1c /proc/device-tree/cpus/PowerPC,POWER7+@3c /proc/device-tree/cpus/PowerPC,POWER7+@20 /proc/device-tree/cpus/PowerPC,POWER7+@4 /proc/device-tree/cpus/PowerPC,POWER7+@24 /proc/device-tree/cpus/PowerPC,POWER7+@8 /proc/device-tree/cpus/PowerPC,POWER7+@28 /proc/device-tree/cpus/PowerPC,POWER7+@c And this is a host, not a guest. I do not see any good reason to make dt names different. And this does not really matter if there is + or not for anybody as far as we concerned, ppc64_cpu works either way. Right, it may not matter, but I expect you to reference the above commit id and explain why it should be POWER7+ after all. You failed to come up with that answer before that patch got applied, so we need to correct me/it now. I have checked with Dinar that under Linux using the Sapphire firmware PowerPC,POWER7+@0 does indeed show up in /proc/device-tree/cpus. So that matches what this patch changes and what you report above. What could be different in Jacques' setup that he reported it different from us? He
Re: [Qemu-devel] [PATCH v2] target-ppc: move POWER7+ to a separate family
On 11/09/2013 03:59 AM, Andreas Färber wrote: Am 08.11.2013 15:54, schrieb Alexey Kardashevskiy: On 11/09/2013 12:44 AM, Andreas Färber wrote: Am 08.11.2013 03:37, schrieb Alexey Kardashevskiy: So far POWER7+ was a part of POWER7 family. However it has a different PVR base value so in order to support PVR masks, it needs a separate family class. Alexey, Another reason to make a POWER7+ family is that its name in the device tree (/proc/device-tree/cpus/cpu*) should be Power7+ but not Power7 and this cannot be easily fixed without a new family class. This adds a new family class, PVR base and mask values and moves Power7+ v2.1 CPU to a new family. The class init function is copied from the POWER7 family. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- Changes: v2: * added VSX enable bit --- target-ppc/cpu-models.c | 2 +- target-ppc/cpu-models.h | 2 ++ target-ppc/translate_init.c | 38 ++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c index 04d88c5..7c9466f 100644 --- a/target-ppc/cpu-models.c +++ b/target-ppc/cpu-models.c @@ -1140,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 731ec4a..49ba4a4 100644 --- a/target-ppc/cpu-models.h +++ b/target-ppc/cpu-models.h @@ -558,6 +558,8 @@ enum { CPU_POWERPC_POWER7_v20 = 0x003F0200, CPU_POWERPC_POWER7_v21 = 0x003F0201, CPU_POWERPC_POWER7_v23 = 0x003F0203, +CPU_POWERPC_POWER7P_BASE = 0x004A, +CPU_POWERPC_POWER7P_MASK = 0x, CPU_POWERPC_POWER7P_v21= 0x004A0201, CPU_POWERPC_POWER8_BASE= 0x004B, CPU_POWERPC_POWER8_MASK= 0x, diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index 35d1389..c030a20 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -7253,6 +7253,44 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data) 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+; Apart from the commit message differing from the code... In what part? The spelling of POWER7. You write it should be Power7+ but implement it as upper-case POWER7+ (ignoring the PowerPC, prefix, that is). Ah. Sorry. We've had this discussion before: Jacques reported that on his POWER7+ box only POWER7 is shown, not POWER7+, equivalent to my POWER5+ box showing only PowerPC,POWER5. Compare my commit, which documents this: http://git.qemu.org/?p=qemu.git;a=commit;h=793826cd460828975591f289de78672af4a47ef9 So, adding a POWER7P family seems correct to me, just the fw_name seems wrong - or you'll need to investigate further why there are conflicting reports of how it is shown. Possibly based on revision or pHyp vs. SLOF? Yes we have had this discussion. Paul said it should POWER7+. The only P7+ machine I have handy shows +: [aik@vpl4 ~]$ ls -d /proc/device-tree/cpus/PowerPC* /proc/device-tree/cpus/PowerPC,POWER7+@0 /proc/device-tree/cpus/PowerPC,POWER7+@2c /proc/device-tree/cpus/PowerPC,POWER7+@10 /proc/device-tree/cpus/PowerPC,POWER7+@30 /proc/device-tree/cpus/PowerPC,POWER7+@14 /proc/device-tree/cpus/PowerPC,POWER7+@34 /proc/device-tree/cpus/PowerPC,POWER7+@18 /proc/device-tree/cpus/PowerPC,POWER7+@38 /proc/device-tree/cpus/PowerPC,POWER7+@1c /proc/device-tree/cpus/PowerPC,POWER7+@3c /proc/device-tree/cpus/PowerPC,POWER7+@20 /proc/device-tree/cpus/PowerPC,POWER7+@4 /proc/device-tree/cpus/PowerPC,POWER7+@24 /proc/device-tree/cpus/PowerPC,POWER7+@8 /proc/device-tree/cpus/PowerPC,POWER7+@28 /proc/device-tree/cpus/PowerPC,POWER7+@c And this is a host, not a guest. I do not see any good reason to make dt names different. And this does not really matter if there is + or not for anybody as far as we concerned, ppc64_cpu works either way. Right, it may not matter, but I expect you to reference the above commit id and explain why it should be POWER7+ after all. You failed to come up with that answer before that patch got applied, so we need to correct me/it now. I have checked with Dinar that under Linux using the Sapphire firmware PowerPC,POWER7+@0 does indeed show up in /proc/device-tree/cpus. So that matches what this patch changes and what you report above. What could be
[Qemu-devel] [PATCH v2] target-ppc: move POWER7+ to a separate family
So far POWER7+ was a part of POWER7 family. However it has a different PVR base value so in order to support PVR masks, it needs a separate family class. Another reason to make a POWER7+ family is that its name in the device tree (/proc/device-tree/cpus/cpu*) should be Power7+ but not Power7 and this cannot be easily fixed without a new family class. This adds a new family class, PVR base and mask values and moves Power7+ v2.1 CPU to a new family. The class init function is copied from the POWER7 family. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- Changes: v2: * added VSX enable bit --- target-ppc/cpu-models.c | 2 +- target-ppc/cpu-models.h | 2 ++ target-ppc/translate_init.c | 38 ++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c index 04d88c5..7c9466f 100644 --- a/target-ppc/cpu-models.c +++ b/target-ppc/cpu-models.c @@ -1140,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 731ec4a..49ba4a4 100644 --- a/target-ppc/cpu-models.h +++ b/target-ppc/cpu-models.h @@ -558,6 +558,8 @@ enum { CPU_POWERPC_POWER7_v20 = 0x003F0200, CPU_POWERPC_POWER7_v21 = 0x003F0201, CPU_POWERPC_POWER7_v23 = 0x003F0203, +CPU_POWERPC_POWER7P_BASE = 0x004A, +CPU_POWERPC_POWER7P_MASK = 0x, CPU_POWERPC_POWER7P_v21= 0x004A0201, CPU_POWERPC_POWER8_BASE= 0x004B, CPU_POWERPC_POWER8_MASK= 0x, diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index 35d1389..c030a20 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -7253,6 +7253,44 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data) 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 | + 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 = 0x8204FF37ULL; +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 | + POWERPC_FLAG_VSX; +pcc-l1_dcache_size = 0x8000; +pcc-l1_icache_size = 0x8000; +} + POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data) { DeviceClass *dc = DEVICE_CLASS(oc); -- 1.8.4.rc4