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

2017-06-30 Thread Michael Ellerman 
Cédric Le Goater  writes:

 According to https://patchwork.ozlabs.org/patch/776052/

 POWER9 DD2's PVR is expected to be 0x004e1200
>> 
>> Uh.. I spoke to Michael Ellerman, and he said he expected 0x004e0200.
>> Though he did mention there might be several variants.  Can we please
>> get a definitive answer on this from IBM.
>
> Adding Michael Ellerman in cc: but I think Greg is correct. 

I don't claim to be correct :)

AFAIK Mikey's patch (linked above) is the most definitive public
documentation we have on this.

Which says:
  The P9 PVR bits 12-15 don't indicate a revision but instead different
  chip configurations.  From BookIV we have:
 Bits  Configuration
  0 :Scale out 12 cores
  1 :Scale out 24 cores
  2 :Scale up  12 cores
  3 :Scale up  24 cores

His heading of "Bits" is somewhat confusing, I'm pretty sure he just
means "the decimal value of bits 12-15", not a bit number.

Which means there may be "POWER9 DD2" chips with PVRs of:
  - 0x004e02xx
  - 0x004e12xx
  - 0x004e22xx
  - 0x004e32xx
 
So the question is just which of the scale out values to use. Neither
is technically correct, because Qemu won't necessarily be emulating a 12
or 24 core system.

Mikey did say "Linux will mostly use the "Scale out 24 core"
configuration", but AFAIK that's really just about the system designs
that are currently in plan.

Hope that makes it clear as mud :)

cheers

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

2017-06-30 Thread Laurent Vivier 
On 30/06/2017 09:12, David Gibson wrote:
> On Thu, Jun 29, 2017 at 03:42:03PM +1000, Suraj Jitindar Singh wrote:
>> On Thu, 2017-06-29 at 15:37 +1000, Suraj Jitindar Singh wrote:
>>> On Wed, 2017-06-28 at 18:41 +0200, Greg Kurz wrote:
...
>>> That makes the assumption that DD2 doesn't require any work arounds
>>> which TCG can't handle.
>>
>> Actually TCG is really a non-issue since we'll just go into the POWER9
>> architected mode.
>>
>> Can't we just have -cpu POWER9 alias to DD1 for now and add DD2 when we
>> know the pvr?
> 
> No, because calling what qemu does DD1 is simply not accurate, in
> important and guest-visible ways.
> 
> What we should do is add in DD2.0 - we know the PVR, even if the
> chip's not out yet.  Then alias POWER9 to that.
> 

OK, I have patch that define v1.0 to DD1, v2 to DD2, and set the POWER9
alias to v2, but:
- on a DD1 host and KVM HV, "-cpu host" works well and "cpu POWER9"
(that select then the v1.0) boots the kernel and hangs at early boot of
the kernel (first display)
- in TCG mode, boot by default with v2, but some services does not start
correctly, and I have never the console login (perhaps because of some
time out: I test TCG on the POWER9 host, I should test on a x86 one).

I'm trying for the moment to understand why "-cpu POWER9" hangs while
"-cpu host" works.

Thanks,
Laurent

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

2017-06-30 Thread Cédric Le Goater 
>>> According to https://patchwork.ozlabs.org/patch/776052/
>>>
>>> POWER9 DD2's PVR is expected to be 0x004e1200
> 
> Uh.. I spoke to Michael Ellerman, and he said he expected 0x004e0200.
> Though he did mention there might be several variants.  Can we please
> get a definitive answer on this from IBM.

Adding Michael Ellerman in cc: but I think Greg is correct. 

C.

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

2017-06-30 Thread David Gibson 
On Thu, Jun 29, 2017 at 03:42:03PM +1000, Suraj Jitindar Singh wrote:
> On Thu, 2017-06-29 at 15:37 +1000, Suraj Jitindar Singh wrote:
> > On Wed, 2017-06-28 at 18:41 +0200, Greg Kurz wrote:
> > > On Wed, 28 Jun 2017 18:18:06 +0200
> > > Laurent Vivier  wrote:
> > > 
> > > > On 28/06/2017 13:59, Greg Kurz wrote:
> > > > > On Wed, 28 Jun 2017 12:23:06 +0200
> > > > > Cédric Le Goater  wrote:
> > > > >   
> > > > > > 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  > > > > > > > > > > > > > > at
> > > > > > > > > > > > > > > .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_POWER
> > > > > > > > > > > > > > > PC
> > > > > > > > > > > > > > > _970_v22,970,
> > > > > > > > > > > > > > >  "PowerPC 970 v2.2")
> > > > > > > > > > > > > > >  
> > > > > > > > > > > > > > > -POWERPC_DEF("POWER9_v1.0",   CPU_POWER
> > > > > > > > > > > > > > > PC
> > > > > > > > > > > > > > > _POWER9_BASE,POWER9,
> > > > > > > > > > > > > > > +POWERPC_DEF("POWER9_v1.0",   CPU_POWER
> > > > > > > > > > > > > > > PC
> > > > > > > > > > > > > > > _POWER9_DD1, POWER9,
> > > > > > > > > > > > > > >  "POWER9 v1.0")
> > > > > > > > > > > > > > >  
> > > > > > > > > > > > > > >  POWERPC_DEF("970fx_v1.0",CPU_POWER
> > > > > > > > > > > > > > > PC
> > > > > > > > > > > > > > > _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
> > > >

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

2017-06-30 Thread David Gibson 
On Wed, Jun 28, 2017 at 06:18:06PM +0200, Laurent Vivier wrote:
> On 28/06/2017 13:59, Greg Kurz wrote:
> > On Wed, 28 Jun 2017 12:23:06 +0200
> > Cédric Le Goater  wrote:
> > 
> >> 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 
> >>> ---
> >>>  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 :
> 
>   0x200D10498000UL; /* P9 Nimbus DD2.0 */  
> >>>
> >>>
> >>> I would expect something like 0x004E.  
> >>
> >> ah yes, I am mistaking the PVR and the CFAM ID. 
> >>
> >> C. 
> >>  
> > 
> > According to https://patchwork.ozlabs.org/patch/776052/
> > 
> > POWER9 DD2's PVR is expected to be 0x004e1200

Uh..

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

2017-06-29 Thread Eric Blake 
[meta-comment]

On 06/29/2017 01:44 AM, Thomas Huth wrote:
...
>> On 22.06.2017 13:26, Laurent Vivier wrote:
>>> CPU_POWERPC_POWER9_DD1 is 0x004E0100, so this

As a reminder, 14 levels of > quoting is a bit much. It's not only
acceptable, but encouraged, to trim replies to high-volume lists, such
that you only keep context relative to your reply, rather than the full
thread.

>>> FWIW Thomas suggested to do just that and David agreed it was "a
>>> better idea".
>>
>> I assume we would have just -cpu POWER9 alias to DD2?
> 
> Yes.

That way, existing thread readers can quickly see what you're adding to
the thread without scrolling past screens of quotations they've already
seen, and new readers can refer to the archives for the rest of the thread.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

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

2017-06-29 Thread Thomas Huth 
On 29.06.2017 07:37, Suraj Jitindar Singh wrote:
> On Wed, 2017-06-28 at 18:41 +0200, Greg Kurz wrote:
>> On Wed, 28 Jun 2017 18:18:06 +0200
>> Laurent Vivier  wrote:
>>
>>> On 28/06/2017 13:59, Greg Kurz wrote:
 On Wed, 28 Jun 2017 12:23:06 +0200
 Cédric Le Goater  wrote:
   
> 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 > .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

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

2017-06-28 Thread Suraj Jitindar Singh 
On Thu, 2017-06-29 at 15:37 +1000, Suraj Jitindar Singh wrote:
> On Wed, 2017-06-28 at 18:41 +0200, Greg Kurz wrote:
> > On Wed, 28 Jun 2017 18:18:06 +0200
> > Laurent Vivier  wrote:
> > 
> > > On 28/06/2017 13:59, Greg Kurz wrote:
> > > > On Wed, 28 Jun 2017 12:23:06 +0200
> > > > Cédric Le Goater  wrote:
> > > >   
> > > > > 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  > > > > > > > > > > > > > at
> > > > > > > > > > > > > > .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_POWER
> > > > > > > > > > > > > > PC
> > > > > > > > > > > > > > _970_v22,970,
> > > > > > > > > > > > > >  "PowerPC 970 v2.2")
> > > > > > > > > > > > > >  
> > > > > > > > > > > > > > -POWERPC_DEF("POWER9_v1.0",   CPU_POWER
> > > > > > > > > > > > > > PC
> > > > > > > > > > > > > > _POWER9_BASE,POWER9,
> > > > > > > > > > > > > > +POWERPC_DEF("POWER9_v1.0",   CPU_POWER
> > > > > > > > > > > > > > PC
> > > > > > > > > > > > > > _POWER9_DD1, POWER9,
> > > > > > > > > > > > > >  "POWER9 v1.0")
> > > > > > > > > > > > > >  
> > > > > > > > > > > > > >  POWERPC_DEF("970fx_v1.0",CPU_POWER
> > > > > > > > > > > > > > PC
> > > > > > > > > > > > > > _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
> > > > > > > > > > >

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

2017-06-28 Thread Suraj Jitindar Singh 
On Wed, 2017-06-28 at 18:41 +0200, Greg Kurz wrote:
> On Wed, 28 Jun 2017 18:18:06 +0200
> Laurent Vivier  wrote:
> 
> > On 28/06/2017 13:59, Greg Kurz wrote:
> > > On Wed, 28 Jun 2017 12:23:06 +0200
> > > Cédric Le Goater  wrote:
> > >   
> > > > 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  > > > > > > > > > > > > .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
> > > > > > > > >

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

2017-06-28 Thread Greg Kurz 
On Wed, 28 Jun 2017 18:18:06 +0200
Laurent Vivier  wrote:

> On 28/06/2017 13:59, Greg Kurz wrote:
> > On Wed, 28 Jun 2017 12:23:06 +0200
> > Cédric Le Goater  wrote:
> >   
> >> 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 
> >>> ---
> >>>  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 :
> 
>   0x200D10498000UL; /* P9 Nimbus DD2.0 */
> >>>
> >>>
> >>> I would expect something like 0x004E.
> >>
> >> ah yes, I am mistaking the PVR and the CFAM ID. 
> >>
> >> C. 
> >>
> > 
> > According to https://patchwork.ozlabs.org/patch/776052/
> >

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

2017-06-28 Thread Laurent Vivier 
On 28/06/2017 13:59, Greg Kurz wrote:
> On Wed, 28 Jun 2017 12:23:06 +0200
> Cédric Le Goater  wrote:
> 
>> 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 
>>> ---
>>>  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 :

  0x200D10498000UL; /* P9 Nimbus DD2.0 */  
>>>
>>>
>>> I would expect something like 0x004E.  
>>
>> ah yes, I am mistaking the PVR and the CFAM ID. 
>>
>> C. 
>>  
> 
> According to https://patchwork.ozlabs.org/patch/776052/
> 
> POWER9 DD2's PVR is expected to be 0x004e1200
>

So, perhaps I can send a v2 of the patch with POWER9_v1.0 set to DD1
PVR, and POWER9_v2.0 set to DD2 PVR?

Thanks,
Laurent

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

2017-06-28 Thread Greg Kurz 
On Wed, 28 Jun 2017 12:23:06 +0200
Cédric Le Goater  wrote:

> 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 
> > ---
> >  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 :
> >>
> >>  0x200D10498000UL; /* P9 Nimbus DD2.0 */  
> > 
> > 
> > I would expect something like 0x004E.  
> 
> ah yes, I am mistaking the PVR and the CFAM ID. 
> 
> C. 
>  

According to https://patchwork.ozlabs.org/patch/776052/

POWER9 DD2's PVR is expected to be 0x004e1200

Cheers,

--
Greg


pgpce6dff6OHz.pgp
Description: OpenPGP digital signature

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

2017-06-28 Thread Laurent Vivier 
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 
> ---
>  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?

We need the POWER9 DD1 in the list to be able to start QEMU on DD1 host
with "-cpu POWER9" (it works with "-cpu host"). The real question is to
what we should set the default CPU when it is not provided on the CLI.

I agree that using POWER9 DD1 to boot a kernel with TCG will confuse the
kernel.

we could set POWER9 DD1 PVR to define the "POWER9_v1.0", and add a
"POWER9_tcg" (?) set to CPU_POWERPC_POWER9_BASE to use with tcg until we
have the PVR of a public release of POWER9?

Thanks,
Laurent

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

2017-06-28 Thread Cédric Le Goater 
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 
> ---
>  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 :
>>
>>  0x200D10498000UL; /* P9 Nimbus DD2.0 */
> 
> 
> I would expect something like 0x004E.

ah yes, I am mistaking the PVR and the CFAM ID. 

C.

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

2017-06-28 Thread Laurent Vivier 
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 
 ---
  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 :
> 
>  0x200D10498000UL; /* P9 Nimbus DD2.0 */


I would expect something like 0x004E.

Laurent

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

2017-06-28 Thread Cédric Le Goater 
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 
>>> ---
>>>  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 :

 0x200D10498000UL; /* P9 Nimbus DD2.0 */

C.

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

2017-06-28 Thread David Gibson 
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 
> > ---
> >  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?

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

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

2017-06-28 Thread Thomas Huth 
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 
> ---
>  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.

 Thomas

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

2017-06-27 Thread 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 
> >>> ---
> >>>  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
> 
>

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

2017-06-27 Thread Suraj Jitindar Singh 
On Fri, 2017-06-23 at 18:05 +0200, Thomas Huth 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 
> > > > ---
> > > >  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.
> 
> But DD1 = v1.0. There is no real chip which is using
> CPU_POWERPC_POWER9_BASE as PVR ... so using that is also not
> accurate,
> and also likely not what users expect when the select "POWER9_v1.0",
> and
> it just does not work as soon as KVM is enabled. So I think Laurent's
> patch is the right way to go. Or do you have a better suggestion?
> 

I guess we have do decide what we want to be the primary behaviour when
someone puts -cpu POWER9 on the command line. Does that mean that they
want an ISAv3.0 compliant cpu or do they want a POWER9 cpu...

FWIW: Currently POWER8 is aliased to POWER8_v2.0 which afaik is the
last POWER8 revision. Should POWER9 just alias to the most recent
POWER9 cpu (currently DD1)? Then how would you specify an ISAv3.0
compliant processor?

TCG only implements a ISAv3 compliant POWER9 cpu so it doesn't make
sense, and should even refuse to boot with POWER9_DD1 since I think it
will fail to boot if we let it.

My personal preference would be to have POWER9 alias to the base and
the user have to explicitly select if they want a DD* version. But that
might frustrate/confuse people. Maybe we should have some table in
KVM_HV of allowable processor combinations so that if the user is on
ISA compliant hardware then they can specify any such cpu on the
command line and KVM_HV still start successfully.

>  Thomas
>