Re: [PATCH] target/ppc/cpu-models: Update max alias to power10
On Wed, Jun 01, 2022 at 12:03:24PM +0200, Greg Kurz wrote: > On Wed, 1 Jun 2022 11:25:43 +0200 > Thomas Huth wrote: > > > On 01/06/2022 10.38, Greg Kurz wrote: > > > On Wed, 1 Jun 2022 09:27:31 +0200 > > > 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. > > ... > > > 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
On Thu, 2 Jun 2022 09:10:57 -0300
Murilo Opsfelder Araújo wrote:
> Hi, Greg.
>
> On 6/1/22 05:38, Greg Kurz wrote:
> > On Wed, 1 Jun 2022 09:27:31 +0200
> > 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é
> >>> Cc: Thomas Huth
> >>> Cc: Cédric Le Goater
> >>> Cc: Daniel Henrique Barboza
> >>> Cc: Fabiano Rosas
> >>> Signed-off-by: Murilo Opsfelder Araujo
> >>> ---
> >>>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
> > Date: Sat Jul 2 20:59:34 2005 +
> >
> > 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
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é
Cc: Thomas Huth
Cc: Cédric Le Goater
Cc: Daniel Henrique Barboza
Cc: Fabiano Rosas
Signed-off-by: Murilo Opsfelder Araujo
---
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
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 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
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é
Cc: Thomas Huth
Cc: Cédric Le Goater
Cc: Daniel Henrique Barboza
Cc: Fabiano Rosas
Signed-off-by: Murilo Opsfelder Araujo
---
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
Hi, Greg.
On 6/1/22 05:38, Greg Kurz wrote:
On Wed, 1 Jun 2022 09:27:31 +0200
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é
Cc: Thomas Huth
Cc: Cédric Le Goater
Cc: Daniel Henrique Barboza
Cc: Fabiano Rosas
Signed-off-by: Murilo Opsfelder Araujo
---
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
Date: Sat Jul 2 20:59:34 2005 +
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
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é
Cc: Thomas Huth
Cc: Cédric Le Goater
Cc: Daniel Henrique Barboza
Cc: Fabiano Rosas
Signed-off-by: Murilo Opsfelder Araujo
---
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
Re: [PATCH] target/ppc/cpu-models: Update max alias to power10
On 6/1/22 07:03, Greg Kurz wrote: On Wed, 1 Jun 2022 11:25:43 +0200 Thomas Huth wrote: On 01/06/2022 10.38, Greg Kurz wrote: On Wed, 1 Jun 2022 09:27:31 +0200 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. ... 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
On Wed, 1 Jun 2022 11:25:43 +0200 Thomas Huth wrote: > On 01/06/2022 10.38, Greg Kurz wrote: > > On Wed, 1 Jun 2022 09:27:31 +0200 > > 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. > ... > > 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
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 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
On 01/06/2022 10.38, Greg Kurz wrote: On Wed, 1 Jun 2022 09:27:31 +0200 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. ... 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
On Wed, 1 Jun 2022 09:27:31 +0200
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é
> > Cc: Thomas Huth
> > Cc: Cédric Le Goater
> > Cc: Daniel Henrique Barboza
> > Cc: Fabiano Rosas
> > Signed-off-by: Murilo Opsfelder Araujo
> > ---
> > 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
Date: Sat Jul 2 20:59:34 2005 +
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
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é
Cc: Thomas Huth
Cc: Cédric Le Goater
Cc: Daniel Henrique Barboza
Cc: Fabiano Rosas
Signed-off-by: Murilo Opsfelder Araujo
---
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
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é
Cc: Thomas Huth
Cc: Cédric Le Goater
Cc: Daniel Henrique Barboza
Cc: Fabiano Rosas
Signed-off-by: Murilo Opsfelder Araujo
---
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
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é
Cc: Thomas Huth
Cc: Cédric Le Goater
Cc: Daniel Henrique Barboza
Cc: Fabiano Rosas
Signed-off-by: Murilo Opsfelder Araujo
---
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
Re: [PATCH] target/ppc/cpu-models: Update max alias to power10
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é
Cc: Thomas Huth
Cc: Cédric Le Goater
Cc: Daniel Henrique Barboza
Cc: Fabiano Rosas
Signed-off-by: Murilo Opsfelder Araujo
Reviewed-by: Cédric Le Goater
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 */
[PATCH] target/ppc/cpu-models: Update max alias to power10
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é
Cc: Thomas Huth
Cc: Cédric Le Goater
Cc: Daniel Henrique Barboza
Cc: Fabiano Rosas
Signed-off-by: Murilo Opsfelder Araujo
---
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
