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

[joserz]

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 <address@hidden>
> >>> ---
> >>>  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. Then, we add the v2.0 (when ready) as the CPU
emulated by TCG.

To TCG, DD1 wouldn't be listed because it cannot be emulated today.

What do you think?

Ziviani

> 
> Laurent
> 
>