Re: [Qemu-devel] [PATCH] target/ppc/cpu-models: set POWER9_v1.0 as POWER9 DD1

[Laurent Vivier <lvivier@redhat.com>]

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 <lvivier@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?

Laurent