| Message ID | 1425432686-29851-1-git-send-email-aik@ozlabs.ru |
|---|---|
| State | New |
| Headers | show |
On 04.03.15 02:31, Alexey Kardashevskiy wrote: > 5b79b1c "target-ppc: Create versionless CPU class per family if KVM" added > a dynamic CPU class registration with the name of the CPU family which > QEMU is running on. For example, this allowed specifying "-cpu POWER7" > on every version of POWER7 machine, not just the one which POWER7 was > an alias of. I.e. before 5b79b1c, "-cpu POWER7" would not work on real > POWER7 2.1 and would work on POWER7 2.3 only. The same story for POWER8. > > However that patch broke POWER5+ support as POWER5+ CPU uses the same > name as the CPU class so dynamic registering of the POWER5+ class failed. > > This redefines POWER5+ server CPUs by adding a version to them and adding > an alias for TCG case. KVM will use dynamically registered CPUs. > > While we are here, do the same for 970 CPU. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> Thanks, applied to ppc-next. Alex
Alex, Am 04.03.2015 um 13:28 schrieb Alexander Graf: > On 04.03.15 02:31, Alexey Kardashevskiy wrote: >> 5b79b1c "target-ppc: Create versionless CPU class per family if KVM" added >> a dynamic CPU class registration with the name of the CPU family which >> QEMU is running on. For example, this allowed specifying "-cpu POWER7" >> on every version of POWER7 machine, not just the one which POWER7 was >> an alias of. I.e. before 5b79b1c, "-cpu POWER7" would not work on real >> POWER7 2.1 and would work on POWER7 2.3 only. The same story for POWER8. >> >> However that patch broke POWER5+ support as POWER5+ CPU uses the same >> name as the CPU class so dynamic registering of the POWER5+ class failed. >> >> This redefines POWER5+ server CPUs by adding a version to them and adding >> an alias for TCG case. KVM will use dynamically registered CPUs. >> >> While we are here, do the same for 970 CPU. >> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > > Thanks, applied to ppc-next. As Alexey predicted, I object. The 970 part looks good and could be applied immediately if it were a separate patch. But the POWER5+ part I have my doubts about: Was there really a v0.0??? Others start with v1.0 and I have: revision : 2.1 (pvr 003b 0201) Also, this is still just bandaid and not a fix to the code that I pointed out. But let's keep that discussion over at the other patch. Regards, Andreas
On 04.03.15 15:14, Andreas Färber wrote: > Alex, > > Am 04.03.2015 um 13:28 schrieb Alexander Graf: >> On 04.03.15 02:31, Alexey Kardashevskiy wrote: >>> 5b79b1c "target-ppc: Create versionless CPU class per family if KVM" added >>> a dynamic CPU class registration with the name of the CPU family which >>> QEMU is running on. For example, this allowed specifying "-cpu POWER7" >>> on every version of POWER7 machine, not just the one which POWER7 was >>> an alias of. I.e. before 5b79b1c, "-cpu POWER7" would not work on real >>> POWER7 2.1 and would work on POWER7 2.3 only. The same story for POWER8. >>> >>> However that patch broke POWER5+ support as POWER5+ CPU uses the same >>> name as the CPU class so dynamic registering of the POWER5+ class failed. >>> >>> This redefines POWER5+ server CPUs by adding a version to them and adding >>> an alias for TCG case. KVM will use dynamically registered CPUs. >>> >>> While we are here, do the same for 970 CPU. >>> >>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> >> Thanks, applied to ppc-next. > > As Alexey predicted, I object. > > The 970 part looks good and could be applied immediately if it were a > separate patch. > > But the POWER5+ part I have my doubts about: Was there really a v0.0??? IIUC most IBM POWER PVRs are regular in that their lower 16 bits are major/minor. The PVR we have as "POWER5+" has those bits as 0, thus v0.0 is the correct translation of that. Whether that CPU ever existed is a different question and arguably out of scope for the patch. > Others start with v1.0 and I have: > > revision : 2.1 (pvr 003b 0201) I wouldn't object to removing the v0.0 version altogether and just make POWER5+ be an alias to v2.1 instead. But that's something for a follow-up patch ;). Alex
On 03/05/2015 01:14 AM, Andreas Färber wrote: > Alex, > > Am 04.03.2015 um 13:28 schrieb Alexander Graf: >> On 04.03.15 02:31, Alexey Kardashevskiy wrote: >>> 5b79b1c "target-ppc: Create versionless CPU class per family if KVM" added >>> a dynamic CPU class registration with the name of the CPU family which >>> QEMU is running on. For example, this allowed specifying "-cpu POWER7" >>> on every version of POWER7 machine, not just the one which POWER7 was >>> an alias of. I.e. before 5b79b1c, "-cpu POWER7" would not work on real >>> POWER7 2.1 and would work on POWER7 2.3 only. The same story for POWER8. >>> >>> However that patch broke POWER5+ support as POWER5+ CPU uses the same >>> name as the CPU class so dynamic registering of the POWER5+ class failed. >>> >>> This redefines POWER5+ server CPUs by adding a version to them and adding >>> an alias for TCG case. KVM will use dynamically registered CPUs. >>> >>> While we are here, do the same for 970 CPU. >>> >>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> >> Thanks, applied to ppc-next. > > As Alexey predicted, I object. > > The 970 part looks good and could be applied immediately if it were a > separate patch. > > But the POWER5+ part I have my doubts about: Was there really a v0.0??? > Others start with v1.0 and I have: > > revision : 2.1 (pvr 003b 0201) > > Also, this is still just bandaid and not a fix to the code that I > pointed out. Does QEMU work for you now, with this patch? How and where (real power5? power7?) do you run QEMU exactly to hit the bug? > But let's keep that discussion over at the other patch. oookay :)
diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c index 3f18996..2b560a4 100644 --- a/target-ppc/cpu-models.c +++ b/target-ppc/cpu-models.c @@ -1124,8 +1124,8 @@ POWERPC_DEF("POWER5", CPU_POWERPC_POWER5, POWER5, "POWER5") #endif - POWERPC_DEF("POWER5+", CPU_POWERPC_POWER5P, POWER5P, - "POWER5+") + POWERPC_DEF("POWER5+_v0.0", CPU_POWERPC_POWER5P_v00, POWER5P, + "POWER5+ v0.0") POWERPC_DEF("POWER5+_v2.1", CPU_POWERPC_POWER5P_v21, POWER5P, "POWER5+ v2.1") #if defined(TODO) @@ -1144,8 +1144,8 @@ "POWER8E v1.0") POWERPC_DEF("POWER8_v1.0", CPU_POWERPC_POWER8_v10, POWER8, "POWER8 v1.0") - POWERPC_DEF("970", CPU_POWERPC_970, 970, - "PowerPC 970") + POWERPC_DEF("970_v2.2", CPU_POWERPC_970_v22, 970, + "PowerPC 970 v2.2") POWERPC_DEF("970fx_v1.0", CPU_POWERPC_970FX_v10, 970, "PowerPC 970FX v1.0 (G5)") POWERPC_DEF("970fx_v2.0", CPU_POWERPC_970FX_v20, 970, @@ -1387,11 +1387,13 @@ PowerPCCPUAlias ppc_cpu_aliases[] = { { "Dino", "POWER3" }, { "POWER3+", "631" }, { "POWER5gr", "POWER5" }, - { "POWER5gs", "POWER5+" }, + { "POWER5+", "POWER5+_v0.0" }, + { "POWER5gs", "POWER5+_v0.0" }, { "POWER7", "POWER7_v2.3" }, { "POWER7+", "POWER7+_v2.1" }, { "POWER8E", "POWER8E_v1.0" }, { "POWER8", "POWER8_v1.0" }, + { "970", "970_v2.2" }, { "970fx", "970fx_v3.1" }, { "970mp", "970mp_v1.1" }, { "Apache", "RS64" }, diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h index 290a759..ee693af 100644 --- a/target-ppc/cpu-models.h +++ b/target-ppc/cpu-models.h @@ -547,7 +547,7 @@ enum { CPU_POWERPC_POWER4P = 0x00380000, /* XXX: missing 0x003A0201 */ CPU_POWERPC_POWER5 = 0x003A0203, - CPU_POWERPC_POWER5P = 0x003B0000, + CPU_POWERPC_POWER5P_v00 = 0x003B0000, CPU_POWERPC_POWER5P_v21 = 0x003B0201, CPU_POWERPC_POWER6 = 0x003E0000, CPU_POWERPC_POWER6_5 = 0x0F000001, /* POWER6 in POWER5 mode */ @@ -561,7 +561,7 @@ enum { CPU_POWERPC_POWER8E_v10 = 0x004B0100, CPU_POWERPC_POWER8_BASE = 0x004D0000, CPU_POWERPC_POWER8_v10 = 0x004D0100, - CPU_POWERPC_970 = 0x00390202, + CPU_POWERPC_970_v22 = 0x00390202, CPU_POWERPC_970FX_v10 = 0x00391100, CPU_POWERPC_970FX_v20 = 0x003C0200, CPU_POWERPC_970FX_v21 = 0x003C0201,
5b79b1c "target-ppc: Create versionless CPU class per family if KVM" added a dynamic CPU class registration with the name of the CPU family which QEMU is running on. For example, this allowed specifying "-cpu POWER7" on every version of POWER7 machine, not just the one which POWER7 was an alias of. I.e. before 5b79b1c, "-cpu POWER7" would not work on real POWER7 2.1 and would work on POWER7 2.3 only. The same story for POWER8. However that patch broke POWER5+ support as POWER5+ CPU uses the same name as the CPU class so dynamic registering of the POWER5+ class failed. This redefines POWER5+ server CPUs by adding a version to them and adding an alias for TCG case. KVM will use dynamically registered CPUs. While we are here, do the same for 970 CPU. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- So before this patch: PowerPC 970 PVR 00390202 PowerPC 970fx_v1.0 PVR 00391100 PowerPC POWER5+ PVR 003b0000 PowerPC POWER5gs (alias for POWER5+) PowerPC POWER5+_v2.1 PVR 003b0201 PowerPC 970fx_v2.0 PVR 003c0200 PowerPC 970fx_v2.1 PVR 003c0201 PowerPC 970fx_v3.0 PVR 003c0300 PowerPC 970fx_v3.1 PVR 003c0301 PowerPC 970fx (alias for 970fx_v3.1) PowerPC ppc64 (alias for 970fx_v3.1) PowerPC POWER7_v2.3 PVR 003f0203 PowerPC POWER7 (alias for POWER7_v2.3) PowerPC 970mp_v1.0 PVR 00440100 PowerPC 970mp_v1.1 PVR 00440101 PowerPC 970mp (alias for 970mp_v1.1) PowerPC POWER7+_v2.1 PVR 004a0201 PowerPC POWER7+ (alias for POWER7+_v2.1) PowerPC POWER8E_v1.0 PVR 004b0100 PowerPC POWER8E (alias for POWER8E_v1.0) PowerPC POWER8_v1.0 PVR 004d0100 PowerPC POWER8 (alias for POWER8_v1.0) After this patch: PowerPC 970_v2.2 PVR 00390202 PowerPC 970 (alias for 970_v2.2) PowerPC 970fx_v1.0 PVR 00391100 PowerPC POWER5+_v0.0 PVR 003b0000 PowerPC POWER5+ (alias for POWER5+_v0.0) PowerPC POWER5gs (alias for POWER5+_v0.0) PowerPC POWER5+_v2.1 PVR 003b0201 PowerPC 970fx_v2.0 PVR 003c0200 PowerPC 970fx_v2.1 PVR 003c0201 PowerPC 970fx_v3.0 PVR 003c0300 PowerPC 970fx_v3.1 PVR 003c0301 PowerPC 970fx (alias for 970fx_v3.1) PowerPC ppc64 (alias for 970fx_v3.1) PowerPC POWER7_v2.3 PVR 003f0203 PowerPC POWER7 (alias for POWER7_v2.3) PowerPC 970mp_v1.0 PVR 00440100 PowerPC 970mp_v1.1 PVR 00440101 PowerPC 970mp (alias for 970mp_v1.1) PowerPC POWER7+_v2.1 PVR 004a0201 PowerPC POWER7+ (alias for POWER7+_v2.1) PowerPC POWER8E_v1.0 PVR 004b0100 PowerPC POWER8E (alias for POWER8E_v1.0) PowerPC POWER8_v1.0 PVR 004d0100 PowerPC POWER8 (alias for POWER8_v1.0) --- target-ppc/cpu-models.c | 12 +++++++----- target-ppc/cpu-models.h | 4 ++-- 2 files changed, 9 insertions(+), 7 deletions(-)