[PATCH] target/ppc/cpu-models: Update max alias to power10

[PATCH] target/ppc/cpu-models: Update max alias to power10

[Murilo Opsfelder Araujo]

Update max alias to power10 so users can take advantage of a more
recent CPU model when '-cpu max' is provided.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1038
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Cédric Le Goater <clg@kaod.org>
Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
Cc: Fabiano Rosas <farosas@linux.ibm.com>
Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
---
 target/ppc/cpu-models.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
index 976be5e0d1..c15fcb43a1 100644
--- a/target/ppc/cpu-models.c
+++ b/target/ppc/cpu-models.c
@@ -879,7 +879,6 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
     { "755", "755_v2.8" },
     { "goldfinger", "755_v2.8" },
     { "7400", "7400_v2.9" },
-    { "max", "7400_v2.9" },
     { "g4",  "7400_v2.9" },
     { "7410", "7410_v1.4" },
     { "nitro", "7410_v1.4" },
@@ -910,6 +909,8 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
     { "power8nvl", "power8nvl_v1.0" },
     { "power9", "power9_v2.0" },
     { "power10", "power10_v2.0" },
+    /* Update the 'max' alias to the latest CPU model */
+    { "max", "power10_v2.0" },
 #endif
 
     /* Generic PowerPCs */
-- 
2.36.1

Re: [PATCH] target/ppc/cpu-models: Update max alias to power10

[Cédric Le Goater]

On 5/31/22 19:27, Murilo Opsfelder Araujo wrote:
> Update max alias to power10 so users can take advantage of a more
> recent CPU model when '-cpu max' is provided.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1038
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Cédric Le Goater <clg@kaod.org>
> Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
> Cc: Fabiano Rosas <farosas@linux.ibm.com>
> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>   target/ppc/cpu-models.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
> index 976be5e0d1..c15fcb43a1 100644
> --- a/target/ppc/cpu-models.c
> +++ b/target/ppc/cpu-models.c
> @@ -879,7 +879,6 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
>       { "755", "755_v2.8" },
>       { "goldfinger", "755_v2.8" },
>       { "7400", "7400_v2.9" },
> -    { "max", "7400_v2.9" },
>       { "g4",  "7400_v2.9" },
>       { "7410", "7410_v1.4" },
>       { "nitro", "7410_v1.4" },
> @@ -910,6 +909,8 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
>       { "power8nvl", "power8nvl_v1.0" },
>       { "power9", "power9_v2.0" },
>       { "power10", "power10_v2.0" },
> +    /* Update the 'max' alias to the latest CPU model */
> +    { "max", "power10_v2.0" },
>   #endif
>   
>       /* Generic PowerPCs */

Re: [PATCH] target/ppc/cpu-models: Update max alias to power10

[Matheus K. Ferst]

On 31/05/2022 14:27, Murilo Opsfelder Araujo wrote:
> Update max alias to power10 so users can take advantage of a more
> recent CPU model when '-cpu max' is provided.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1038
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Cédric Le Goater <clg@kaod.org>
> Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
> Cc: Fabiano Rosas <farosas@linux.ibm.com>
> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
> ---
>   target/ppc/cpu-models.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
> index 976be5e0d1..c15fcb43a1 100644
> --- a/target/ppc/cpu-models.c
> +++ b/target/ppc/cpu-models.c
> @@ -879,7 +879,6 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
>       { "755", "755_v2.8" },
>       { "goldfinger", "755_v2.8" },
>       { "7400", "7400_v2.9" },
> -    { "max", "7400_v2.9" },
>       { "g4",  "7400_v2.9" },
>       { "7410", "7410_v1.4" },
>       { "nitro", "7410_v1.4" },
> @@ -910,6 +909,8 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
>       { "power8nvl", "power8nvl_v1.0" },
>       { "power9", "power9_v2.0" },
>       { "power10", "power10_v2.0" },
> +    /* Update the 'max' alias to the latest CPU model */
> +    { "max", "power10_v2.0" },
>   #endif
> 
>       /* Generic PowerPCs */
> --
> 2.36.1
> 
> 

Hi Murilo,

I guess we need a "max" for qemu-system-ppc too, so maybe something like

 >     /* Update the 'max' alias to the latest CPU model */
 > #if defined(TARGET_PPC64)
 >     { "max", "power10_v2.0" },
 > #else
 >     { "max", "7457a_v1.2" },
 > #endif

Or some other CPU which is considered the max for 32-bit...

Thanks,
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>

Re: [PATCH] target/ppc/cpu-models: Update max alias to power10

[Murilo Opsfelder Araújo]

Hi, Matheus.

On 5/31/22 15:04, Matheus K. Ferst wrote:
> On 31/05/2022 14:27, Murilo Opsfelder Araujo wrote:
>> Update max alias to power10 so users can take advantage of a more
>> recent CPU model when '-cpu max' is provided.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1038
>> Cc: Daniel P. Berrangé <berrange@redhat.com>
>> Cc: Thomas Huth <thuth@redhat.com>
>> Cc: Cédric Le Goater <clg@kaod.org>
>> Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
>> Cc: Fabiano Rosas <farosas@linux.ibm.com>
>> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
>> ---
>>   target/ppc/cpu-models.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
>> index 976be5e0d1..c15fcb43a1 100644
>> --- a/target/ppc/cpu-models.c
>> +++ b/target/ppc/cpu-models.c
>> @@ -879,7 +879,6 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
>>       { "755", "755_v2.8" },
>>       { "goldfinger", "755_v2.8" },
>>       { "7400", "7400_v2.9" },
>> -    { "max", "7400_v2.9" },
>>       { "g4",  "7400_v2.9" },
>>       { "7410", "7410_v1.4" },
>>       { "nitro", "7410_v1.4" },
>> @@ -910,6 +909,8 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
>>       { "power8nvl", "power8nvl_v1.0" },
>>       { "power9", "power9_v2.0" },
>>       { "power10", "power10_v2.0" },
>> +    /* Update the 'max' alias to the latest CPU model */
>> +    { "max", "power10_v2.0" },
>>   #endif
>>
>>       /* Generic PowerPCs */
>> --
>> 2.36.1
>>
>>
>
> Hi Murilo,
>
> I guess we need a "max" for qemu-system-ppc too, so maybe something like
>
>  >     /* Update the 'max' alias to the latest CPU model */
>  > #if defined(TARGET_PPC64)
>  >     { "max", "power10_v2.0" },
>  > #else
>  >     { "max", "7457a_v1.2" },
>  > #endif

Thanks for reviewing.

I'm more inclined to the idea of selecting the default CPU type of the
machine, as other folks suggested in the replies, instead of
hard-coding an alias.

Cheers!

--
Murilo

Re: [PATCH] target/ppc/cpu-models: Update max alias to power10

[Thomas Huth]

On 31/05/2022 19.27, Murilo Opsfelder Araujo wrote:
> Update max alias to power10 so users can take advantage of a more
> recent CPU model when '-cpu max' is provided.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1038
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Cédric Le Goater <clg@kaod.org>
> Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
> Cc: Fabiano Rosas <farosas@linux.ibm.com>
> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
> ---
>   target/ppc/cpu-models.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
> index 976be5e0d1..c15fcb43a1 100644
> --- a/target/ppc/cpu-models.c
> +++ b/target/ppc/cpu-models.c
> @@ -879,7 +879,6 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
>       { "755", "755_v2.8" },
>       { "goldfinger", "755_v2.8" },
>       { "7400", "7400_v2.9" },
> -    { "max", "7400_v2.9" },
>       { "g4",  "7400_v2.9" },
>       { "7410", "7410_v1.4" },
>       { "nitro", "7410_v1.4" },
> @@ -910,6 +909,8 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
>       { "power8nvl", "power8nvl_v1.0" },
>       { "power9", "power9_v2.0" },
>       { "power10", "power10_v2.0" },
> +    /* Update the 'max' alias to the latest CPU model */
> +    { "max", "power10_v2.0" },
>   #endif

I'm not sure whether "max" should really be fixed alias in this list here? 
What if a user runs with KVM on a POWER8 host - then "max" won't work this 
way, will it?

And in the long run, it would also be good if this would work with other 
machines like the "g3beige", too (which don't support the new 64-bit POWER 
CPUs), so you should at least mention in the commit description that this is 
only a temporary hack for the pseries machine, I think.

  Thomas

Re: [PATCH] target/ppc/cpu-models: Update max alias to power10

[Cédric Le Goater]

On 6/1/22 09:27, Thomas Huth wrote:
> On 31/05/2022 19.27, Murilo Opsfelder Araujo wrote:
>> Update max alias to power10 so users can take advantage of a more
>> recent CPU model when '-cpu max' is provided.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1038
>> Cc: Daniel P. Berrangé <berrange@redhat.com>
>> Cc: Thomas Huth <thuth@redhat.com>
>> Cc: Cédric Le Goater <clg@kaod.org>
>> Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
>> Cc: Fabiano Rosas <farosas@linux.ibm.com>
>> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
>> ---
>>   target/ppc/cpu-models.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
>> index 976be5e0d1..c15fcb43a1 100644
>> --- a/target/ppc/cpu-models.c
>> +++ b/target/ppc/cpu-models.c
>> @@ -879,7 +879,6 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
>>       { "755", "755_v2.8" },
>>       { "goldfinger", "755_v2.8" },
>>       { "7400", "7400_v2.9" },
>> -    { "max", "7400_v2.9" },
>>       { "g4",  "7400_v2.9" },
>>       { "7410", "7410_v1.4" },
>>       { "nitro", "7410_v1.4" },
>> @@ -910,6 +909,8 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
>>       { "power8nvl", "power8nvl_v1.0" },
>>       { "power9", "power9_v2.0" },
>>       { "power10", "power10_v2.0" },
>> +    /* Update the 'max' alias to the latest CPU model */
>> +    { "max", "power10_v2.0" },
>>   #endif
> 
> I'm not sure whether "max" should really be fixed alias in this list here? What if a user runs with KVM on a POWER8 host - then "max" won't work this way, will it?
> 
> And in the long run, it would also be good if this would work with other machines like the "g3beige", too (which don't support the new 64-bit POWER CPUs), so you should at least mention in the commit description that this is only a temporary hack for the pseries machine, I think.

Yes. You are right, Thomas.

s390 and x86 have a nice way to address "max".

Thanks,

C.

> 
>   Thomas
> 

Re: [PATCH] target/ppc/cpu-models: Update max alias to power10

[Murilo Opsfelder Araújo]

Hi, Cédric.

On 6/1/22 04:44, Cédric Le Goater wrote:
> On 6/1/22 09:27, Thomas Huth wrote:
>> On 31/05/2022 19.27, Murilo Opsfelder Araujo wrote:
>>> Update max alias to power10 so users can take advantage of a more
>>> recent CPU model when '-cpu max' is provided.
>>>
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1038
>>> Cc: Daniel P. Berrangé <berrange@redhat.com>
>>> Cc: Thomas Huth <thuth@redhat.com>
>>> Cc: Cédric Le Goater <clg@kaod.org>
>>> Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
>>> Cc: Fabiano Rosas <farosas@linux.ibm.com>
>>> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
>>> ---
>>>   target/ppc/cpu-models.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
>>> index 976be5e0d1..c15fcb43a1 100644
>>> --- a/target/ppc/cpu-models.c
>>> +++ b/target/ppc/cpu-models.c
>>> @@ -879,7 +879,6 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
>>>       { "755", "755_v2.8" },
>>>       { "goldfinger", "755_v2.8" },
>>>       { "7400", "7400_v2.9" },
>>> -    { "max", "7400_v2.9" },
>>>       { "g4",  "7400_v2.9" },
>>>       { "7410", "7410_v1.4" },
>>>       { "nitro", "7410_v1.4" },
>>> @@ -910,6 +909,8 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
>>>       { "power8nvl", "power8nvl_v1.0" },
>>>       { "power9", "power9_v2.0" },
>>>       { "power10", "power10_v2.0" },
>>> +    /* Update the 'max' alias to the latest CPU model */
>>> +    { "max", "power10_v2.0" },
>>>   #endif
>>
>> I'm not sure whether "max" should really be fixed alias in this list here? What if a user runs with KVM on a POWER8 host - then "max" won't work this way, will it?
>>
>> And in the long run, it would also be good if this would work with other machines like the "g3beige", too (which don't support the new 64-bit POWER CPUs), so you should at least mention in the commit description that this is only a temporary hack for the pseries machine, I think.
>
> Yes. You are right, Thomas.
>
> s390 and x86 have a nice way to address "max".

If I understood the code correctly, they implement "-cpu max" based on
a CPU model with additional CPU features enabled.  The resulting
emulated CPU is not necessarily a CPU model that exists as a hardware.
So, the "-cpu max" never gets any CPU feature dropped.  Features are
only added in.

I'm not keen on this idea of having a CPU model that doesn't even
exist as a hardware.

--
Murilo

Re: [PATCH] target/ppc/cpu-models: Update max alias to power10

[Greg Kurz]

On Wed, 1 Jun 2022 09:27:31 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 31/05/2022 19.27, Murilo Opsfelder Araujo wrote:
> > Update max alias to power10 so users can take advantage of a more
> > recent CPU model when '-cpu max' is provided.
> > 
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1038
> > Cc: Daniel P. Berrangé <berrange@redhat.com>
> > Cc: Thomas Huth <thuth@redhat.com>
> > Cc: Cédric Le Goater <clg@kaod.org>
> > Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
> > Cc: Fabiano Rosas <farosas@linux.ibm.com>
> > Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
> > ---
> >   target/ppc/cpu-models.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
> > index 976be5e0d1..c15fcb43a1 100644
> > --- a/target/ppc/cpu-models.c
> > +++ b/target/ppc/cpu-models.c
> > @@ -879,7 +879,6 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
> >       { "755", "755_v2.8" },
> >       { "goldfinger", "755_v2.8" },
> >       { "7400", "7400_v2.9" },
> > -    { "max", "7400_v2.9" },
> >       { "g4",  "7400_v2.9" },
> >       { "7410", "7410_v1.4" },
> >       { "nitro", "7410_v1.4" },
> > @@ -910,6 +909,8 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
> >       { "power8nvl", "power8nvl_v1.0" },
> >       { "power9", "power9_v2.0" },
> >       { "power10", "power10_v2.0" },
> > +    /* Update the 'max' alias to the latest CPU model */
> > +    { "max", "power10_v2.0" },
> >   #endif
> 
> I'm not sure whether "max" should really be fixed alias in this list here? 
> What if a user runs with KVM on a POWER8 host - then "max" won't work this 
> way, will it?
> 
> And in the long run, it would also be good if this would work with other 
> machines like the "g3beige", too (which don't support the new 64-bit POWER 
> CPUs), so you should at least mention in the commit description that this is 
> only a temporary hack for the pseries machine, I think.
> 

I didn't even know there was a "max" alias :-)

This was introduced by commit:

commit 3fc6c082e3aad85addf25d36740030982963c0c8
Author: Fabrice Bellard <fabrice@bellard.org>
Date:   Sat Jul 2 20:59:34 2005 +0000

    preliminary patch to support more PowerPC CPUs (Jocelyn Mayer)

This was already a 7400 model at the time. Obviously we've never
maintained that and I hardly see how it is useful... As Thomas
noted, "max" has implicit semantics that depend on the host.
This isn't migration friendly and I'm pretty sure libvirt
doesn't use it (Daniel ?)

We already have the concept of default CPU for the spapr
machine types, that is usually popped up to the newer
CPU model that is going to be supported in production.
This goes with a bump of the machine type version as
well for the sake of migration. This seems a lot more
reliable than the "max" thingy IMHO.

Unless there's a very important use case I'm missing,
I'd rather kill the thing instead of trying to resurrect
it.

Cheers,

--
Greg

>   Thomas
> 

Re: [PATCH] target/ppc/cpu-models: Update max alias to power10

[Thomas Huth]

On 01/06/2022 10.38, Greg Kurz wrote:
> On Wed, 1 Jun 2022 09:27:31 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
>> On 31/05/2022 19.27, Murilo Opsfelder Araujo wrote:
>>> Update max alias to power10 so users can take advantage of a more
>>> recent CPU model when '-cpu max' is provided.
...
> We already have the concept of default CPU for the spapr
> machine types, that is usually popped up to the newer
> CPU model that is going to be supported in production.
> This goes with a bump of the machine type version as
> well for the sake of migration. This seems a lot more
> reliable than the "max" thingy IMHO.
> 
> Unless there's a very important use case I'm missing,
> I'd rather kill the thing instead of trying to resurrect
> it.

It's about making ppc similar to other architectures, which
have "-cpu max" as well, see:

  https://gitlab.com/qemu-project/qemu/-/issues/1038

It would be nice to get something similar on ppc.

By the way, the warnings that you currently get when running with
TCG are quite ugly, too:

$ ./qemu-system-ppc64
qemu-system-ppc64: warning: TCG doesn't support requested feature, 
cap-cfpc=workaround
qemu-system-ppc64: warning: TCG doesn't support requested feature, 
cap-sbbc=workaround
qemu-system-ppc64: warning: TCG doesn't support requested feature, 
cap-ibs=workaround
qemu-system-ppc64: warning: TCG doesn't support requested feature, 
cap-ccf-assist=on

Maybe these could get fixed with a proper "max" CPU in TCG
mode, too?

  Thomas

Re: [PATCH] target/ppc/cpu-models: Update max alias to power10

[Daniel Henrique Barboza]

On 6/1/22 06:25, Thomas Huth wrote:
> On 01/06/2022 10.38, Greg Kurz wrote:
>> On Wed, 1 Jun 2022 09:27:31 +0200
>> Thomas Huth <thuth@redhat.com> wrote:
>>
>>> On 31/05/2022 19.27, Murilo Opsfelder Araujo wrote:
>>>> Update max alias to power10 so users can take advantage of a more
>>>> recent CPU model when '-cpu max' is provided.
> ...
>> We already have the concept of default CPU for the spapr
>> machine types, that is usually popped up to the newer
>> CPU model that is going to be supported in production.
>> This goes with a bump of the machine type version as
>> well for the sake of migration. This seems a lot more
>> reliable than the "max" thingy IMHO.
>>
>> Unless there's a very important use case I'm missing,
>> I'd rather kill the thing instead of trying to resurrect
>> it.
> 
> It's about making ppc similar to other architectures, which
> have "-cpu max" as well, see:
> 
>   https://gitlab.com/qemu-project/qemu/-/issues/1038
> 
> It would be nice to get something similar on ppc.


I agree that it's preferable to fix it.

This is how I would implement -cpu max today:

pseries (default ppc64 machine):
  - kvm: equal to -cpu host
  - tcg: the latest IBM chip available (POWER10 today)

powernv8: POWER8E
powernv9: POWER9
powernv10: POWER10

pseries requires more work because the -cpu max varies with the host CPU
when running with KVM.

About the implementation, for the bug fix it's fine to just hardcode the alias
for each machine-CPU pair. In the long run I would add more code to make -cpu max
always point to the current default CPU of the chosen machine by default, with
each machine overwriting it if needed. This would prevent this alias to be
deprecated over time because we forgot to change it after adding new CPUs.

For qemu-system-ppc the default machine seems to be g3beige and its default
CPU is PowerPC 750. I would set -cpu max to this CPU in this case. Matter of
fact I would attempt to set -cpu max = default cpu for all 32 bits CPUs for
simplicity. This is also outside of gitlab 1038 as well since the bug isn't
mentioning 32 bit machines, hence can be done later.


Thanks,

Daniel


> 
> By the way, the warnings that you currently get when running with
> TCG are quite ugly, too:
> 
> $ ./qemu-system-ppc64
> qemu-system-ppc64: warning: TCG doesn't support requested feature, cap-cfpc=workaround
> qemu-system-ppc64: warning: TCG doesn't support requested feature, cap-sbbc=workaround
> qemu-system-ppc64: warning: TCG doesn't support requested feature, cap-ibs=workaround
> qemu-system-ppc64: warning: TCG doesn't support requested feature, cap-ccf-assist=on
> 
> Maybe these could get fixed with a proper "max" CPU in TCG
> mode, too?
> 
>   Thomas
> 

Re: [PATCH] target/ppc/cpu-models: Update max alias to power10

[Murilo Opsfelder Araújo]

Hi, Daniel.

On 6/1/22 06:59, Daniel Henrique Barboza wrote:
>
>
> On 6/1/22 06:25, Thomas Huth wrote:
>> On 01/06/2022 10.38, Greg Kurz wrote:
>>> On Wed, 1 Jun 2022 09:27:31 +0200
>>> Thomas Huth <thuth@redhat.com> wrote:
>>>
>>>> On 31/05/2022 19.27, Murilo Opsfelder Araujo wrote:
>>>>> Update max alias to power10 so users can take advantage of a more
>>>>> recent CPU model when '-cpu max' is provided.
>> ...
>>> We already have the concept of default CPU for the spapr
>>> machine types, that is usually popped up to the newer
>>> CPU model that is going to be supported in production.
>>> This goes with a bump of the machine type version as
>>> well for the sake of migration. This seems a lot more
>>> reliable than the "max" thingy IMHO.
>>>
>>> Unless there's a very important use case I'm missing,
>>> I'd rather kill the thing instead of trying to resurrect
>>> it.
>>
>> It's about making ppc similar to other architectures, which
>> have "-cpu max" as well, see:
>>
>>   https://gitlab.com/qemu-project/qemu/-/issues/1038
>>
>> It would be nice to get something similar on ppc.
>
>
> I agree that it's preferable to fix it.
>
> This is how I would implement -cpu max today:
>
> pseries (default ppc64 machine):
>   - kvm: equal to -cpu host
>   - tcg: the latest IBM chip available (POWER10 today)
>
> powernv8: POWER8E
> powernv9: POWER9
> powernv10: POWER10
>
> pseries requires more work because the -cpu max varies with the host CPU
> when running with KVM.
>
> About the implementation, for the bug fix it's fine to just hardcode the alias
> for each machine-CPU pair. In the long run I would add more code to make -cpu max
> always point to the current default CPU of the chosen machine by default, with
> each machine overwriting it if needed. This would prevent this alias to be
> deprecated over time because we forgot to change it after adding new CPUs.

I agree with using the default CPU type of the machine as the selected
CPU for "-cpu max".

Anyone disagree?

> For qemu-system-ppc the default machine seems to be g3beige and its default
> CPU is PowerPC 750. I would set -cpu max to this CPU in this case. Matter of
> fact I would attempt to set -cpu max = default cpu for all 32 bits CPUs for
> simplicity. This is also outside of gitlab 1038 as well since the bug isn't
> mentioning 32 bit machines, hence can be done later.
>
>
> Thanks,
>
> Daniel

Cheeers!

--
Murilo

Re: [PATCH] target/ppc/cpu-models: Update max alias to power10

[Greg Kurz]

On Wed, 1 Jun 2022 11:25:43 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 01/06/2022 10.38, Greg Kurz wrote:
> > On Wed, 1 Jun 2022 09:27:31 +0200
> > Thomas Huth <thuth@redhat.com> wrote:
> > 
> >> On 31/05/2022 19.27, Murilo Opsfelder Araujo wrote:
> >>> Update max alias to power10 so users can take advantage of a more
> >>> recent CPU model when '-cpu max' is provided.
> ...
> > We already have the concept of default CPU for the spapr
> > machine types, that is usually popped up to the newer
> > CPU model that is going to be supported in production.
> > This goes with a bump of the machine type version as
> > well for the sake of migration. This seems a lot more
> > reliable than the "max" thingy IMHO.
> > 
> > Unless there's a very important use case I'm missing,
> > I'd rather kill the thing instead of trying to resurrect
> > it.
> 
> It's about making ppc similar to other architectures, which
> have "-cpu max" as well, see:
> 
>   https://gitlab.com/qemu-project/qemu/-/issues/1038
> 
> It would be nice to get something similar on ppc.
> 

Problem is that on ppc, given the variety of models and boards,
the concept of "max" is quite fuzzy... i.e. a lot of cases to
consider for a benefit that is unclear to me. Hence my questioning.
If the idea is just to match what other targets do without a specific
use case in mind, this looks quite useless to me.

> By the way, the warnings that you currently get when running with
> TCG are quite ugly, too:
> 
> $ ./qemu-system-ppc64
> qemu-system-ppc64: warning: TCG doesn't support requested feature, 
> cap-cfpc=workaround
> qemu-system-ppc64: warning: TCG doesn't support requested feature, 
> cap-sbbc=workaround
> qemu-system-ppc64: warning: TCG doesn't support requested feature, 
> cap-ibs=workaround
> qemu-system-ppc64: warning: TCG doesn't support requested feature, 
> cap-ccf-assist=on
> 
> Maybe these could get fixed with a proper "max" CPU in TCG
> mode, too?
> 

I don't think so. These warnings are the consequence of pseries
being the default machine for ppc64, and the default pseries
machine decides on the default CPU model and default values for
features (in this case, these are mitigations for spectre/meltdown).
TCG doesn't support them but we certainly don't want to add more
divergence between TCG and KVM.

Cheers,

--
Greg

>   Thomas
> 

Re: [PATCH] target/ppc/cpu-models: Update max alias to power10

[Daniel Henrique Barboza]

On 6/1/22 07:03, Greg Kurz wrote:
> On Wed, 1 Jun 2022 11:25:43 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
>> On 01/06/2022 10.38, Greg Kurz wrote:
>>> On Wed, 1 Jun 2022 09:27:31 +0200
>>> Thomas Huth <thuth@redhat.com> wrote:
>>>
>>>> On 31/05/2022 19.27, Murilo Opsfelder Araujo wrote:
>>>>> Update max alias to power10 so users can take advantage of a more
>>>>> recent CPU model when '-cpu max' is provided.
>> ...
>>> We already have the concept of default CPU for the spapr
>>> machine types, that is usually popped up to the newer
>>> CPU model that is going to be supported in production.
>>> This goes with a bump of the machine type version as
>>> well for the sake of migration. This seems a lot more
>>> reliable than the "max" thingy IMHO.
>>>
>>> Unless there's a very important use case I'm missing,
>>> I'd rather kill the thing instead of trying to resurrect
>>> it.
>>
>> It's about making ppc similar to other architectures, which
>> have "-cpu max" as well, see:
>>
>>    https://gitlab.com/qemu-project/qemu/-/issues/1038
>>
>> It would be nice to get something similar on ppc.
>>
> 
> Problem is that on ppc, given the variety of models and boards,
> the concept of "max" is quite fuzzy... i.e. a lot of cases to
> consider for a benefit that is unclear to me. Hence my questioning.
> If the idea is just to match what other targets do without a specific
> use case in mind, this looks quite useless to me.

I mean, yes, the use case is that users/tooling are using -cpu max with x86
and arm. We'd rather not increase the gap between them and ppc64 because we
ended up removing -cpu max.

Even if the concept might not be applicable to every machine we have we can alias
-cpu max to the default machine CPU.

> 
>> By the way, the warnings that you currently get when running with
>> TCG are quite ugly, too:
>>
>> $ ./qemu-system-ppc64
>> qemu-system-ppc64: warning: TCG doesn't support requested feature,
>> cap-cfpc=workaround
>> qemu-system-ppc64: warning: TCG doesn't support requested feature,
>> cap-sbbc=workaround
>> qemu-system-ppc64: warning: TCG doesn't support requested feature,
>> cap-ibs=workaround
>> qemu-system-ppc64: warning: TCG doesn't support requested feature,
>> cap-ccf-assist=on
>>
>> Maybe these could get fixed with a proper "max" CPU in TCG
>> mode, too?
>>
> 
> I don't think so. These warnings are the consequence of pseries
> being the default machine for ppc64, and the default pseries
> machine decides on the default CPU model and default values for
> features (in this case, these are mitigations for spectre/meltdown).
> TCG doesn't support them but we certainly don't want to add more
> divergence between TCG and KVM.

I sent a patch last year trying to suppress the warning:

https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg05029.html

I proposed to suppress these warnings when the user didn't specifically
set those caps to true (which TCG doesn't support). David thought that
this was also a bad idea and we reached an impasse. Back then seemed like
I was the only one severely aggravated by these messages so I gave up :)


Thanks,


Daniel

> 
> Cheers,
> 
> --
> Greg
> 
>>    Thomas
>>
> 

Re: [PATCH] target/ppc/cpu-models: Update max alias to power10

[Daniel P. Berrangé]

On Wed, Jun 01, 2022 at 12:03:24PM +0200, Greg Kurz wrote:
> On Wed, 1 Jun 2022 11:25:43 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
> > On 01/06/2022 10.38, Greg Kurz wrote:
> > > On Wed, 1 Jun 2022 09:27:31 +0200
> > > Thomas Huth <thuth@redhat.com> wrote:
> > > 
> > >> On 31/05/2022 19.27, Murilo Opsfelder Araujo wrote:
> > >>> Update max alias to power10 so users can take advantage of a more
> > >>> recent CPU model when '-cpu max' is provided.
> > ...
> > > We already have the concept of default CPU for the spapr
> > > machine types, that is usually popped up to the newer
> > > CPU model that is going to be supported in production.
> > > This goes with a bump of the machine type version as
> > > well for the sake of migration. This seems a lot more
> > > reliable than the "max" thingy IMHO.
> > > 
> > > Unless there's a very important use case I'm missing,
> > > I'd rather kill the thing instead of trying to resurrect
> > > it.
> > 
> > It's about making ppc similar to other architectures, which
> > have "-cpu max" as well, see:
> > 
> >   https://gitlab.com/qemu-project/qemu/-/issues/1038
> > 
> > It would be nice to get something similar on ppc.
> > 
> 
> Problem is that on ppc, given the variety of models and boards,
> the concept of "max" is quite fuzzy... i.e. a lot of cases to
> consider for a benefit that is unclear to me. Hence my questioning.
> If the idea is just to match what other targets do without a specific
> use case in mind, this looks quite useless to me.

Essentially "max" is a way for a mgmt application to ask for the
most feature rich CPU available for their given configuration.

With KVM this is trivially equiv to '-cpu host'.

With TCG on other archs this has been "all the features that
TCG implements". This implicitly assumes that CPU features are
generally additive and compatible with all historical machine
types. This is fine for x86, but if it doesn't work for PPC
we can come up with an alternative definition.

For example if you have a set of 3 possible CPU models that
work with a given machine board, then expand 'max' to be the
"best" of these 3 possible models.  This still satisfies the
goal of the mgmt app, which is to be able to ask for a good
CPU without having to know its architecture/machine type
specific name.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] target/ppc/cpu-models: Update max alias to power10

[Murilo Opsfelder Araújo]

Hi, Greg.

On 6/1/22 05:38, Greg Kurz wrote:
> On Wed, 1 Jun 2022 09:27:31 +0200
> Thomas Huth <thuth@redhat.com> wrote:
>
>> On 31/05/2022 19.27, Murilo Opsfelder Araujo wrote:
>>> Update max alias to power10 so users can take advantage of a more
>>> recent CPU model when '-cpu max' is provided.
>>>
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1038
>>> Cc: Daniel P. Berrangé <berrange@redhat.com>
>>> Cc: Thomas Huth <thuth@redhat.com>
>>> Cc: Cédric Le Goater <clg@kaod.org>
>>> Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
>>> Cc: Fabiano Rosas <farosas@linux.ibm.com>
>>> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
>>> ---
>>>    target/ppc/cpu-models.c | 3 ++-
>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
>>> index 976be5e0d1..c15fcb43a1 100644
>>> --- a/target/ppc/cpu-models.c
>>> +++ b/target/ppc/cpu-models.c
>>> @@ -879,7 +879,6 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
>>>        { "755", "755_v2.8" },
>>>        { "goldfinger", "755_v2.8" },
>>>        { "7400", "7400_v2.9" },
>>> -    { "max", "7400_v2.9" },
>>>        { "g4",  "7400_v2.9" },
>>>        { "7410", "7410_v1.4" },
>>>        { "nitro", "7410_v1.4" },
>>> @@ -910,6 +909,8 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
>>>        { "power8nvl", "power8nvl_v1.0" },
>>>        { "power9", "power9_v2.0" },
>>>        { "power10", "power10_v2.0" },
>>> +    /* Update the 'max' alias to the latest CPU model */
>>> +    { "max", "power10_v2.0" },
>>>    #endif
>>
>> I'm not sure whether "max" should really be fixed alias in this list here?
>> What if a user runs with KVM on a POWER8 host - then "max" won't work this
>> way, will it?
>>
>> And in the long run, it would also be good if this would work with other
>> machines like the "g3beige", too (which don't support the new 64-bit POWER
>> CPUs), so you should at least mention in the commit description that this is
>> only a temporary hack for the pseries machine, I think.
>>
>
> I didn't even know there was a "max" alias :-)

Me too.  :)

> This was introduced by commit:
>
> commit 3fc6c082e3aad85addf25d36740030982963c0c8
> Author: Fabrice Bellard <fabrice@bellard.org>
> Date:   Sat Jul 2 20:59:34 2005 +0000
>
>      preliminary patch to support more PowerPC CPUs (Jocelyn Mayer)
>
> This was already a 7400 model at the time. Obviously we've never
> maintained that and I hardly see how it is useful... As Thomas
> noted, "max" has implicit semantics that depend on the host.
> This isn't migration friendly and I'm pretty sure libvirt
> doesn't use it (Daniel ?)
>
> We already have the concept of default CPU for the spapr
> machine types, that is usually popped up to the newer
> CPU model that is going to be supported in production.
> This goes with a bump of the machine type version as
> well for the sake of migration. This seems a lot more
> reliable than the "max" thingy IMHO.

We can use the default CPU type of the sPAPR machine as the "-cpu
max".  That would be a safer choice, I think.

Cheers!

--
Murilo

Re: [PATCH] target/ppc/cpu-models: Update max alias to power10

[Greg Kurz]

On Thu, 2 Jun 2022 09:10:57 -0300
Murilo Opsfelder Araújo <muriloo@linux.ibm.com> wrote:

> Hi, Greg.
> 
> On 6/1/22 05:38, Greg Kurz wrote:
> > On Wed, 1 Jun 2022 09:27:31 +0200
> > Thomas Huth <thuth@redhat.com> wrote:
> >
> >> On 31/05/2022 19.27, Murilo Opsfelder Araujo wrote:
> >>> Update max alias to power10 so users can take advantage of a more
> >>> recent CPU model when '-cpu max' is provided.
> >>>
> >>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1038
> >>> Cc: Daniel P. Berrangé <berrange@redhat.com>
> >>> Cc: Thomas Huth <thuth@redhat.com>
> >>> Cc: Cédric Le Goater <clg@kaod.org>
> >>> Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
> >>> Cc: Fabiano Rosas <farosas@linux.ibm.com>
> >>> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
> >>> ---
> >>>    target/ppc/cpu-models.c | 3 ++-
> >>>    1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
> >>> index 976be5e0d1..c15fcb43a1 100644
> >>> --- a/target/ppc/cpu-models.c
> >>> +++ b/target/ppc/cpu-models.c
> >>> @@ -879,7 +879,6 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
> >>>        { "755", "755_v2.8" },
> >>>        { "goldfinger", "755_v2.8" },
> >>>        { "7400", "7400_v2.9" },
> >>> -    { "max", "7400_v2.9" },
> >>>        { "g4",  "7400_v2.9" },
> >>>        { "7410", "7410_v1.4" },
> >>>        { "nitro", "7410_v1.4" },
> >>> @@ -910,6 +909,8 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
> >>>        { "power8nvl", "power8nvl_v1.0" },
> >>>        { "power9", "power9_v2.0" },
> >>>        { "power10", "power10_v2.0" },
> >>> +    /* Update the 'max' alias to the latest CPU model */
> >>> +    { "max", "power10_v2.0" },
> >>>    #endif
> >>
> >> I'm not sure whether "max" should really be fixed alias in this list here?
> >> What if a user runs with KVM on a POWER8 host - then "max" won't work this
> >> way, will it?
> >>
> >> And in the long run, it would also be good if this would work with other
> >> machines like the "g3beige", too (which don't support the new 64-bit POWER
> >> CPUs), so you should at least mention in the commit description that this is
> >> only a temporary hack for the pseries machine, I think.
> >>
> >
> > I didn't even know there was a "max" alias :-)
> 
> Me too.  :)
> 
> > This was introduced by commit:
> >
> > commit 3fc6c082e3aad85addf25d36740030982963c0c8
> > Author: Fabrice Bellard <fabrice@bellard.org>
> > Date:   Sat Jul 2 20:59:34 2005 +0000
> >
> >      preliminary patch to support more PowerPC CPUs (Jocelyn Mayer)
> >
> > This was already a 7400 model at the time. Obviously we've never
> > maintained that and I hardly see how it is useful... As Thomas
> > noted, "max" has implicit semantics that depend on the host.
> > This isn't migration friendly and I'm pretty sure libvirt
> > doesn't use it (Daniel ?)
> >
> > We already have the concept of default CPU for the spapr
> > machine types, that is usually popped up to the newer
> > CPU model that is going to be supported in production.
> > This goes with a bump of the machine type version as
> > well for the sake of migration. This seems a lot more
> > reliable than the "max" thingy IMHO.
> 
> We can use the default CPU type of the sPAPR machine as the "-cpu
> max".  That would be a safer choice, I think.
> 

Hi Murilo !

After reading the various comments, I agree that the default CPU
type of the machine type [*] with TCG, and the host CPU type with
KVM is the most sensible choice.

[*] taking into account the machine type passed by the user, e.g.
    we want power8_v2.0 when running a pseries-3.1, not power9_v2.0.

Cheers,

--
Greg

> Cheers!
> 
> --
> Murilo
> 

Re: [PATCH] target/ppc/cpu-models: Update max alias to power10

[Murilo Opsfelder Araújo]

Hi, Thomas.

On 6/1/22 04:27, Thomas Huth wrote:
> On 31/05/2022 19.27, Murilo Opsfelder Araujo wrote:
>> Update max alias to power10 so users can take advantage of a more
>> recent CPU model when '-cpu max' is provided.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1038
>> Cc: Daniel P. Berrangé <berrange@redhat.com>
>> Cc: Thomas Huth <thuth@redhat.com>
>> Cc: Cédric Le Goater <clg@kaod.org>
>> Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
>> Cc: Fabiano Rosas <farosas@linux.ibm.com>
>> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
>> ---
>>   target/ppc/cpu-models.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
>> index 976be5e0d1..c15fcb43a1 100644
>> --- a/target/ppc/cpu-models.c
>> +++ b/target/ppc/cpu-models.c
>> @@ -879,7 +879,6 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
>>       { "755", "755_v2.8" },
>>       { "goldfinger", "755_v2.8" },
>>       { "7400", "7400_v2.9" },
>> -    { "max", "7400_v2.9" },
>>       { "g4",  "7400_v2.9" },
>>       { "7410", "7410_v1.4" },
>>       { "nitro", "7410_v1.4" },
>> @@ -910,6 +909,8 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
>>       { "power8nvl", "power8nvl_v1.0" },
>>       { "power9", "power9_v2.0" },
>>       { "power10", "power10_v2.0" },
>> +    /* Update the 'max' alias to the latest CPU model */
>> +    { "max", "power10_v2.0" },
>>   #endif
>
> I'm not sure whether "max" should really be fixed alias in this list here? What if a user runs with KVM on a POWER8 host - then "max" won't work this way, will it?

"-cpu max" as an alias to power10 running with KVM on a P8 host won't
work.  It's already broken with the current 7400_v2.9, anyway.

> And in the long run, it would also be good if this would work with other machines like the "g3beige", too (which don't support the new 64-bit POWER CPUs), so you should at least mention in the commit description that this is only a temporary hack for the pseries machine, I think.

I agree.  I'll mention that if I end up respining the patch.

Thank you!

--
Murilo