On 06/28/2017 11:18 AM, Laurent Vivier wrote: > On 28/06/2017 11:11, Cédric Le Goater wrote: >> On 06/28/2017 10:18 AM, David Gibson wrote: >>> On Wed, Jun 28, 2017 at 09:09:24AM +0200, Thomas Huth wrote: >>>> On 28.06.2017 03:42, jos...@linux.vnet.ibm.com wrote: >>>>> On Fri, Jun 23, 2017 at 04:10:55PM +0200, Laurent Vivier wrote: >>>>>> On 23/06/2017 11:21, David Gibson wrote: >>>>>>> On Thu, Jun 22, 2017 at 01:31:24PM +0200, Thomas Huth wrote: >>>>>>>> On 22.06.2017 13:26, Laurent Vivier wrote: >>>>>>>>> CPU_POWERPC_POWER9_DD1 is 0x004E0100, so this is the POWER9 v1.0. >>>>>>>>> >>>>>>>>> When we run qemu on a POWER9 DD1 host, we must use either >>>>>>>>> "-cpu host" or "-cpu POWER9", but in the latter case it fails with >>>>>>>>> >>>>>>>>> Unable to find sPAPR CPU Core definition >>>>>>>>> >>>>>>>>> because POWER9 DD1 doesn't appear in the list of known CPUs. >>>>>>>>> >>>>>>>>> This patch fixes this by defining POWER9_v1.0 with POWER9 DD1 >>>>>>>>> PVR instead of CPU_POWERPC_POWER9_BASE. >>>>>>>>> >>>>>>>>> Signed-off-by: Laurent Vivier <lviv...@redhat.com> >>>>>>>>> --- >>>>>>>>> target/ppc/cpu-models.c | 2 +- >>>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>>>> >>>>>>>>> diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c >>>>>>>>> index 4d3e635..a22363c 100644 >>>>>>>>> --- a/target/ppc/cpu-models.c >>>>>>>>> +++ b/target/ppc/cpu-models.c >>>>>>>>> @@ -1144,7 +1144,7 @@ >>>>>>>>> POWERPC_DEF("970_v2.2", CPU_POWERPC_970_v22, >>>>>>>>> 970, >>>>>>>>> "PowerPC 970 v2.2") >>>>>>>>> >>>>>>>>> - POWERPC_DEF("POWER9_v1.0", CPU_POWERPC_POWER9_BASE, >>>>>>>>> POWER9, >>>>>>>>> + POWERPC_DEF("POWER9_v1.0", CPU_POWERPC_POWER9_DD1, >>>>>>>>> POWER9, >>>>>>>>> "POWER9 v1.0") >>>>>>>>> >>>>>>>>> POWERPC_DEF("970fx_v1.0", CPU_POWERPC_970FX_v10, >>>>>>>>> 970, >>>>>>>>> >>>>>>>> >>>>>>>> I think this also makes sense for running in TCG mode to get a valid >>>>>>>> real PVR there. >>>>>>> >>>>>>> I'm not so convinced. >>>>>>> >>>>>>> IIUC, this will make TCG default (for now) to a DD1 POWER9. That's a) >>>>>>> probably not what anyone wants - who'd select a buggy prototype and b) >>>>>>> not accurate - TCG does not implement DD1's bugs. >>>>>> >>>>>> According to the POWER8 user manual (I didn't fine the POWER9 one): >>>>>> >>>>>> "3.6.3.1 Processor Version Register (PVR) >>>>>> >>>>>> The processor revision level (PVR[16:31]) starts at x‘0100’, indicating >>>>>> revision ‘1.0’. As revisions are made, bits [29:31] will indicate minor >>>>>> revisions. Similarly, bits [20:23] indicate major changes." >>>>>> >>>>>> POWER9 DD1 PVR is 0x004E0100, so this is really version 1.0 of the >>>>>> POWER9. >>>>>> >>>>>> Perhaps we can define POWER9_v1.0 as CPU_POWERPC_POWER9_DD1, and >>>>>> introduce a POWER9_v0.0 set to CPU_POWERPC_POWER9_BASE and define it as >>>>>> the default one? >>>>> >>>>> I like the suggestion to set a v0.0 to CPU_POWERPC_POWER9_BASE. But, I >>>>> think we could have only that option, removing the >>>>> CPU_POWERPC_POWER9_DD1 entry. >>>> I really dislike the idea of having a CPU called "v0.0" ... we do not >>>> have this for any other CPU generation, and it sounds like it could be >>>> very confusing for the users (you'd need to document somewhere what the >>>> v0.0 exactly means). If we really want to go this way, I think we should >>>> name it "POWER9-generic" or "PowerISA-3.0" or something similar instead. >>>> >>>> Or does somebody already know the exact PVR for DD2? If so, we could >>>> simply add a POWER9_v2.0 CPU already and let the POWER9 alias point to >>>> that version instead. >>> >>> Yes, I think that's a better idea. I don't know the DD2 PVR, but I'm >>> pretty sure we should be able to find out from someone at IBM. >>> >>> I've CCed Sam & Suraj - can you ask Mikey or someone what the PVR >>> value for DD2.0 will be? >> >> I would expect something like : >> >> 0x200D104980000000UL; /* P9 Nimbus DD2.0 */ > > > I would expect something like 0x004Exxxx.
ah yes, I am mistaking the PVR and the CFAM ID. C.