[PATCH v4 4/5] cpu_map: Add POWER11 cpu model support

Narayana Murty N posted 5 patches 8 months ago
There is a newer version of this series
[PATCH v4 4/5] cpu_map: Add POWER11 cpu model support
Posted by Narayana Murty N 8 months ago
Add POWER11 as a supported cpu model for ppc64.

Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
---
 src/cpu_map/index.xml                      | 1 +
 src/cpu_map/meson.build                    | 1 +
 src/cpu_map/ppc64_POWER11.xml              | 6 ++++++
 tests/domaincapsdata/qemu_10.0.0.ppc64.xml | 1 +
 4 files changed, 9 insertions(+)
 create mode 100644 src/cpu_map/ppc64_POWER11.xml

diff --git a/src/cpu_map/index.xml b/src/cpu_map/index.xml
index 87db338cee..790c3b2f83 100644
--- a/src/cpu_map/index.xml
+++ b/src/cpu_map/index.xml
@@ -181,6 +181,7 @@
       <include filename='ppc64_POWER8.xml'/>
       <include filename='ppc64_POWER9.xml'/>
       <include filename='ppc64_POWER10.xml'/>
+      <include filename='ppc64_POWER11.xml'/>
     </group>
 
     <group name='Freescale-based CPU models'>
diff --git a/src/cpu_map/meson.build b/src/cpu_map/meson.build
index dee8441a13..abf9c21e4f 100644
--- a/src/cpu_map/meson.build
+++ b/src/cpu_map/meson.build
@@ -17,6 +17,7 @@ cpumap_data = [
   'arm_vendors.xml',
   'index.xml',
   'ppc64_POWER10.xml',
+  'ppc64_POWER11.xml',
   'ppc64_POWER6.xml',
   'ppc64_POWER7.xml',
   'ppc64_POWER8.xml',
diff --git a/src/cpu_map/ppc64_POWER11.xml b/src/cpu_map/ppc64_POWER11.xml
new file mode 100644
index 0000000000..d624043072
--- /dev/null
+++ b/src/cpu_map/ppc64_POWER11.xml
@@ -0,0 +1,6 @@
+<cpus>
+  <model name='power11'>
+    <vendor name='IBM'/>
+    <pvr value='0x00820000' mask='0xffff0000'/>
+  </model>
+</cpus>
diff --git a/tests/domaincapsdata/qemu_10.0.0.ppc64.xml b/tests/domaincapsdata/qemu_10.0.0.ppc64.xml
index 3c864146eb..c449d96f86 100644
--- a/tests/domaincapsdata/qemu_10.0.0.ppc64.xml
+++ b/tests/domaincapsdata/qemu_10.0.0.ppc64.xml
@@ -39,6 +39,7 @@
       <model usable='unknown' vendor='IBM'>POWER7</model>
       <model usable='unknown' vendor='IBM'>POWER8</model>
       <model usable='unknown' vendor='IBM'>POWER9</model>
+      <model usable='unknown' vendor='IBM'>power11</model>
     </mode>
   </cpu>
   <memoryBacking supported='yes'>
-- 
2.48.1
Re: [PATCH v4 4/5] cpu_map: Add POWER11 cpu model support
Posted by Daniel P. Berrangé via Devel 7 months, 3 weeks ago
On Tue, Apr 15, 2025 at 04:13:08AM -0400, Narayana Murty N wrote:
> Add POWER11 as a supported cpu model for ppc64.
> 
> Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
> ---
>  src/cpu_map/index.xml                      | 1 +
>  src/cpu_map/meson.build                    | 1 +
>  src/cpu_map/ppc64_POWER11.xml              | 6 ++++++
>  tests/domaincapsdata/qemu_10.0.0.ppc64.xml | 1 +
>  4 files changed, 9 insertions(+)
>  create mode 100644 src/cpu_map/ppc64_POWER11.xml
> 
> diff --git a/src/cpu_map/index.xml b/src/cpu_map/index.xml
> index 87db338cee..790c3b2f83 100644
> --- a/src/cpu_map/index.xml
> +++ b/src/cpu_map/index.xml
> @@ -181,6 +181,7 @@
>        <include filename='ppc64_POWER8.xml'/>
>        <include filename='ppc64_POWER9.xml'/>
>        <include filename='ppc64_POWER10.xml'/>
> +      <include filename='ppc64_POWER11.xml'/>
>      </group>
>  
>      <group name='Freescale-based CPU models'>
> diff --git a/src/cpu_map/meson.build b/src/cpu_map/meson.build
> index dee8441a13..abf9c21e4f 100644
> --- a/src/cpu_map/meson.build
> +++ b/src/cpu_map/meson.build
> @@ -17,6 +17,7 @@ cpumap_data = [
>    'arm_vendors.xml',
>    'index.xml',
>    'ppc64_POWER10.xml',
> +  'ppc64_POWER11.xml',
>    'ppc64_POWER6.xml',
>    'ppc64_POWER7.xml',
>    'ppc64_POWER8.xml',
> diff --git a/src/cpu_map/ppc64_POWER11.xml b/src/cpu_map/ppc64_POWER11.xml
> new file mode 100644
> index 0000000000..d624043072
> --- /dev/null
> +++ b/src/cpu_map/ppc64_POWER11.xml
> @@ -0,0 +1,6 @@
> +<cpus>
> +  <model name='power11'>
> +    <vendor name='IBM'/>
> +    <pvr value='0x00820000' mask='0xffff0000'/>
> +  </model>
> +</cpus>
> diff --git a/tests/domaincapsdata/qemu_10.0.0.ppc64.xml b/tests/domaincapsdata/qemu_10.0.0.ppc64.xml
> index 3c864146eb..c449d96f86 100644
> --- a/tests/domaincapsdata/qemu_10.0.0.ppc64.xml
> +++ b/tests/domaincapsdata/qemu_10.0.0.ppc64.xml
> @@ -39,6 +39,7 @@
>        <model usable='unknown' vendor='IBM'>POWER7</model>
>        <model usable='unknown' vendor='IBM'>POWER8</model>
>        <model usable='unknown' vendor='IBM'>POWER9</model>
> +      <model usable='unknown' vendor='IBM'>power11</model>

QEMU allows both upper & lowercase for CPU names, and libvirt
has stuck with uppercase historically.

What's the justification for changing this approach for
power11 ?


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 v4 4/5] cpu_map: Add POWER11 cpu model support
Posted by Peter Krempa via Devel 7 months, 3 weeks ago
On Tue, Apr 29, 2025 at 12:36:40 +0100, Daniel P. Berrangé wrote:
> On Tue, Apr 15, 2025 at 04:13:08AM -0400, Narayana Murty N wrote:
> > Add POWER11 as a supported cpu model for ppc64.
> > 
> > Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
> > ---
> >  src/cpu_map/index.xml                      | 1 +
> >  src/cpu_map/meson.build                    | 1 +
> >  src/cpu_map/ppc64_POWER11.xml              | 6 ++++++
> >  tests/domaincapsdata/qemu_10.0.0.ppc64.xml | 1 +
> >  4 files changed, 9 insertions(+)
> >  create mode 100644 src/cpu_map/ppc64_POWER11.xml
> > diff --git a/tests/domaincapsdata/qemu_10.0.0.ppc64.xml b/tests/domaincapsdata/qemu_10.0.0.ppc64.xml

[...]

> > index 3c864146eb..c449d96f86 100644
> > --- a/tests/domaincapsdata/qemu_10.0.0.ppc64.xml
> > +++ b/tests/domaincapsdata/qemu_10.0.0.ppc64.xml
> > @@ -39,6 +39,7 @@
> >        <model usable='unknown' vendor='IBM'>POWER7</model>
> >        <model usable='unknown' vendor='IBM'>POWER8</model>
> >        <model usable='unknown' vendor='IBM'>POWER9</model>
> > +      <model usable='unknown' vendor='IBM'>power11</model>
> 
> QEMU allows both upper & lowercase for CPU names, and libvirt
> has stuck with uppercase historically.
> 
> What's the justification for changing this approach for
> power11 ?

qemu itself reports the model lowercase now. In v2 of this series I've
pointed that out because defining this caused test data to change which
was suspiciuous.

Jirka then said that we should stick with qemu naming.
Re: [PATCH v4 4/5] cpu_map: Add POWER11 cpu model support
Posted by Daniel P. Berrangé via Devel 7 months, 3 weeks ago
On Tue, Apr 29, 2025 at 01:42:22PM +0200, Peter Krempa wrote:
> On Tue, Apr 29, 2025 at 12:36:40 +0100, Daniel P. Berrangé wrote:
> > On Tue, Apr 15, 2025 at 04:13:08AM -0400, Narayana Murty N wrote:
> > > Add POWER11 as a supported cpu model for ppc64.
> > > 
> > > Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
> > > ---
> > >  src/cpu_map/index.xml                      | 1 +
> > >  src/cpu_map/meson.build                    | 1 +
> > >  src/cpu_map/ppc64_POWER11.xml              | 6 ++++++
> > >  tests/domaincapsdata/qemu_10.0.0.ppc64.xml | 1 +
> > >  4 files changed, 9 insertions(+)
> > >  create mode 100644 src/cpu_map/ppc64_POWER11.xml
> > > diff --git a/tests/domaincapsdata/qemu_10.0.0.ppc64.xml b/tests/domaincapsdata/qemu_10.0.0.ppc64.xml
> 
> [...]
> 
> > > index 3c864146eb..c449d96f86 100644
> > > --- a/tests/domaincapsdata/qemu_10.0.0.ppc64.xml
> > > +++ b/tests/domaincapsdata/qemu_10.0.0.ppc64.xml
> > > @@ -39,6 +39,7 @@
> > >        <model usable='unknown' vendor='IBM'>POWER7</model>
> > >        <model usable='unknown' vendor='IBM'>POWER8</model>
> > >        <model usable='unknown' vendor='IBM'>POWER9</model>
> > > +      <model usable='unknown' vendor='IBM'>power11</model>
> > 
> > QEMU allows both upper & lowercase for CPU names, and libvirt
> > has stuck with uppercase historically.
> > 
> > What's the justification for changing this approach for
> > power11 ?
> 
> qemu itself reports the model lowercase now. In v2 of this series I've
> pointed that out because defining this caused test data to change which
> was suspiciuous.

By 'reports the model lowercase' i presume you're referring to '-cpu help'
and/or 'query-cpu-definitions' ?  That doesn't seem to be a new change
either though. I've not gone far back, but qemu 8.2.0 seems to already
be using lowercase.

So I'm still wondering why we're changing our decision about CPU naming
/now/ with power11 ?  The commit message needs to explain and justify
this change.

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 v4 4/5] cpu_map: Add POWER11 cpu model support
Posted by Jiří Denemark via Devel 7 months, 3 weeks ago
On Tue, Apr 29, 2025 at 13:03:12 +0100, Daniel P. Berrangé wrote:
> On Tue, Apr 29, 2025 at 01:42:22PM +0200, Peter Krempa wrote:
> > On Tue, Apr 29, 2025 at 12:36:40 +0100, Daniel P. Berrangé wrote:
> > > On Tue, Apr 15, 2025 at 04:13:08AM -0400, Narayana Murty N wrote:
> > > > Add POWER11 as a supported cpu model for ppc64.
> > > > 
> > > > Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
> > > > ---
> > > >  src/cpu_map/index.xml                      | 1 +
> > > >  src/cpu_map/meson.build                    | 1 +
> > > >  src/cpu_map/ppc64_POWER11.xml              | 6 ++++++
> > > >  tests/domaincapsdata/qemu_10.0.0.ppc64.xml | 1 +
> > > >  4 files changed, 9 insertions(+)
> > > >  create mode 100644 src/cpu_map/ppc64_POWER11.xml
> > > > diff --git a/tests/domaincapsdata/qemu_10.0.0.ppc64.xml b/tests/domaincapsdata/qemu_10.0.0.ppc64.xml
> > 
> > [...]
> > 
> > > > index 3c864146eb..c449d96f86 100644
> > > > --- a/tests/domaincapsdata/qemu_10.0.0.ppc64.xml
> > > > +++ b/tests/domaincapsdata/qemu_10.0.0.ppc64.xml
> > > > @@ -39,6 +39,7 @@
> > > >        <model usable='unknown' vendor='IBM'>POWER7</model>
> > > >        <model usable='unknown' vendor='IBM'>POWER8</model>
> > > >        <model usable='unknown' vendor='IBM'>POWER9</model>
> > > > +      <model usable='unknown' vendor='IBM'>power11</model>
> > > 
> > > QEMU allows both upper & lowercase for CPU names, and libvirt
> > > has stuck with uppercase historically.
> > > 
> > > What's the justification for changing this approach for
> > > power11 ?
> > 
> > qemu itself reports the model lowercase now. In v2 of this series I've
> > pointed that out because defining this caused test data to change which
> > was suspiciuous.
> 
> By 'reports the model lowercase' i presume you're referring to '-cpu help'
> and/or 'query-cpu-definitions' ?  That doesn't seem to be a new change
> either though. I've not gone far back, but qemu 8.2.0 seems to already
> be using lowercase.
> 
> So I'm still wondering why we're changing our decision about CPU naming
> /now/ with power11 ?  The commit message needs to explain and justify
> this change.

Well, when I replied to the original patch, my memory was telling me
QEMU switched to lower case names and had to add compatibility code to
translate our existing upper case names. And adding a new CPU model
looked like a good excuse to stop doing that (for new models of course)
and just define the model in lower case. Now that you're questioning
this I tried to find evidence and I could not find it :-( So there are
two options... either I'm bad at searching or I my memory confused this
with dropping version suffixes (_vX.Y) in virCPUppc64ConvertLegacy. Or
both options are true at the same time :-)

Maybe Andrea can remember more, I think he was involved in all this ages
ago.

Jirka
Re: [PATCH v4 4/5] cpu_map: Add POWER11 cpu model support
Posted by Andrea Bolognani via Devel 7 months, 2 weeks ago
On Tue, Apr 29, 2025 at 05:41:43PM +0200, Jiří Denemark wrote:
> On Tue, Apr 29, 2025 at 13:03:12 +0100, Daniel P. Berrangé wrote:
> > On Tue, Apr 29, 2025 at 01:42:22PM +0200, Peter Krempa wrote:
> > > On Tue, Apr 29, 2025 at 12:36:40 +0100, Daniel P. Berrangé wrote:
> > > > On Tue, Apr 15, 2025 at 04:13:08AM -0400, Narayana Murty N wrote:
> > > > >        <model usable='unknown' vendor='IBM'>POWER7</model>
> > > > >        <model usable='unknown' vendor='IBM'>POWER8</model>
> > > > >        <model usable='unknown' vendor='IBM'>POWER9</model>
> > > > > +      <model usable='unknown' vendor='IBM'>power11</model>
> > > >
> > > > QEMU allows both upper & lowercase for CPU names, and libvirt
> > > > has stuck with uppercase historically.
> > > >
> > > > What's the justification for changing this approach for
> > > > power11 ?
> > >
> > > qemu itself reports the model lowercase now. In v2 of this series I've
> > > pointed that out because defining this caused test data to change which
> > > was suspiciuous.
> >
> > By 'reports the model lowercase' i presume you're referring to '-cpu help'
> > and/or 'query-cpu-definitions' ?  That doesn't seem to be a new change
> > either though. I've not gone far back, but qemu 8.2.0 seems to already
> > be using lowercase.
> >
> > So I'm still wondering why we're changing our decision about CPU naming
> > /now/ with power11 ?  The commit message needs to explain and justify
> > this change.
>
> Well, when I replied to the original patch, my memory was telling me
> QEMU switched to lower case names and had to add compatibility code to
> translate our existing upper case names. And adding a new CPU model
> looked like a good excuse to stop doing that (for new models of course)
> and just define the model in lower case. Now that you're questioning
> this I tried to find evidence and I could not find it :-( So there are
> two options... either I'm bad at searching or I my memory confused this
> with dropping version suffixes (_vX.Y) in virCPUppc64ConvertLegacy. Or
> both options are true at the same time :-)
>
> Maybe Andrea can remember more, I think he was involved in all this ages
> ago.

It seems that CPU models have been reported in lower case at least
going all the way back to QEMU 3.0.0 from 2018:

  https://gitlab.com/libvirt/libvirt/-/blob/c7e09b7b5fe7f64f12318d150b281343c6d64fd5/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.replies

It would probably make sense for libvirt to align with this decision,
and accept/generate lower case CPU model names. Of course upper case
CPU models would still need to be accepted for backwards
compatibility reasons.

Note that I'm suggesting we adopt this approach consistently for all
CPU models, including existing ones. So power8 should be accepted, as
should be POWER11.

CC'ing Daniel and David, both of which have worked on ppc64 in the
past and might be able to offer additional insights / historical
context.

-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH v4 4/5] cpu_map: Add POWER11 cpu model support
Posted by Daniel Henrique Barboza 7 months, 2 weeks ago

On 5/5/25 5:58 AM, Andrea Bolognani wrote:
> On Tue, Apr 29, 2025 at 05:41:43PM +0200, Jiří Denemark wrote:
>> On Tue, Apr 29, 2025 at 13:03:12 +0100, Daniel P. Berrangé wrote:
>>> On Tue, Apr 29, 2025 at 01:42:22PM +0200, Peter Krempa wrote:
>>>> On Tue, Apr 29, 2025 at 12:36:40 +0100, Daniel P. Berrangé wrote:
>>>>> On Tue, Apr 15, 2025 at 04:13:08AM -0400, Narayana Murty N wrote:
>>>>>>         <model usable='unknown' vendor='IBM'>POWER7</model>
>>>>>>         <model usable='unknown' vendor='IBM'>POWER8</model>
>>>>>>         <model usable='unknown' vendor='IBM'>POWER9</model>
>>>>>> +      <model usable='unknown' vendor='IBM'>power11</model>
>>>>>
>>>>> QEMU allows both upper & lowercase for CPU names, and libvirt
>>>>> has stuck with uppercase historically.
>>>>>
>>>>> What's the justification for changing this approach for
>>>>> power11 ?
>>>>
>>>> qemu itself reports the model lowercase now. In v2 of this series I've
>>>> pointed that out because defining this caused test data to change which
>>>> was suspiciuous.
>>>
>>> By 'reports the model lowercase' i presume you're referring to '-cpu help'
>>> and/or 'query-cpu-definitions' ?  That doesn't seem to be a new change
>>> either though. I've not gone far back, but qemu 8.2.0 seems to already
>>> be using lowercase.
>>>
>>> So I'm still wondering why we're changing our decision about CPU naming
>>> /now/ with power11 ?  The commit message needs to explain and justify
>>> this change.
>>
>> Well, when I replied to the original patch, my memory was telling me
>> QEMU switched to lower case names and had to add compatibility code to
>> translate our existing upper case names. And adding a new CPU model
>> looked like a good excuse to stop doing that (for new models of course)
>> and just define the model in lower case. Now that you're questioning
>> this I tried to find evidence and I could not find it :-( So there are
>> two options... either I'm bad at searching or I my memory confused this
>> with dropping version suffixes (_vX.Y) in virCPUppc64ConvertLegacy. Or
>> both options are true at the same time :-)
>>
>> Maybe Andrea can remember more, I think he was involved in all this ages
>> ago.
> 
> It seems that CPU models have been reported in lower case at least
> going all the way back to QEMU 3.0.0 from 2018:
> 
>    https://gitlab.com/libvirt/libvirt/-/blob/c7e09b7b5fe7f64f12318d150b281343c6d64fd5/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.replies
> 
> It would probably make sense for libvirt to align with this decision,
> and accept/generate lower case CPU model names. Of course upper case
> CPU models would still need to be accepted for backwards
> compatibility reasons.
> 
> Note that I'm suggesting we adopt this approach consistently for all
> CPU models, including existing ones. So power8 should be accepted, as
> should be POWER11.
> 
> CC'ing Daniel and David, both of which have worked on ppc64 in the
> past and might be able to offer additional insights / historical
> context.
> 

It has to do with IBM marketing.

POWER is an acronym (Performance Optimization With Enhanced RISC) and all
marketing material demanded that it was mentioned in full caps, e.g. POWER7,
POWER8, POWER9. The reason why libvirt kept using full caps in libvirt, even
after QEMU changed it to lower, was due to IBM marketing demands. IBM wanted
users to use the full caps name in the domain XML, which is/was the official way
customers would interact with the virtualization stack.

In power10 it seems like IBM marketing changed its mind [1] and now the model
name is camel case, so now we're supposed to call them Power10, Power11, etc.

Narayana can surely provide more details than I can, but I believe this is the
reason why we would want a change for case insensitive CPU models in the IBM
PowerPC family in libvirt. There are domains in the wild that are using full
caps names, and we can't break those, but now IBM would tell customers to use

<model fallback='forbid'>power11</model>"

or similar. Ignoring case seems a sensible change to make since we do not want
to be hostage of IBM marketing whims.

But, as Daniel said, this should be a properly justified and documented change.
Thanks,


The other Daniel


[1] https://en.wikipedia.org/wiki/Power10, section "Branding"
Re: [PATCH v4 4/5] cpu_map: Add POWER11 cpu model support
Posted by Daniel P. Berrangé via Devel 7 months, 2 weeks ago
On Mon, May 05, 2025 at 09:07:52AM -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 5/5/25 5:58 AM, Andrea Bolognani wrote:
> > On Tue, Apr 29, 2025 at 05:41:43PM +0200, Jiří Denemark wrote:
> > > On Tue, Apr 29, 2025 at 13:03:12 +0100, Daniel P. Berrangé wrote:
> > > > On Tue, Apr 29, 2025 at 01:42:22PM +0200, Peter Krempa wrote:
> > > > > On Tue, Apr 29, 2025 at 12:36:40 +0100, Daniel P. Berrangé wrote:
> > > > > > On Tue, Apr 15, 2025 at 04:13:08AM -0400, Narayana Murty N wrote:
> > > > > > >         <model usable='unknown' vendor='IBM'>POWER7</model>
> > > > > > >         <model usable='unknown' vendor='IBM'>POWER8</model>
> > > > > > >         <model usable='unknown' vendor='IBM'>POWER9</model>
> > > > > > > +      <model usable='unknown' vendor='IBM'>power11</model>
> > > > > > 
> > > > > > QEMU allows both upper & lowercase for CPU names, and libvirt
> > > > > > has stuck with uppercase historically.
> > > > > > 
> > > > > > What's the justification for changing this approach for
> > > > > > power11 ?
> > > > > 
> > > > > qemu itself reports the model lowercase now. In v2 of this series I've
> > > > > pointed that out because defining this caused test data to change which
> > > > > was suspiciuous.
> > > > 
> > > > By 'reports the model lowercase' i presume you're referring to '-cpu help'
> > > > and/or 'query-cpu-definitions' ?  That doesn't seem to be a new change
> > > > either though. I've not gone far back, but qemu 8.2.0 seems to already
> > > > be using lowercase.
> > > > 
> > > > So I'm still wondering why we're changing our decision about CPU naming
> > > > /now/ with power11 ?  The commit message needs to explain and justify
> > > > this change.
> > > 
> > > Well, when I replied to the original patch, my memory was telling me
> > > QEMU switched to lower case names and had to add compatibility code to
> > > translate our existing upper case names. And adding a new CPU model
> > > looked like a good excuse to stop doing that (for new models of course)
> > > and just define the model in lower case. Now that you're questioning
> > > this I tried to find evidence and I could not find it :-( So there are
> > > two options... either I'm bad at searching or I my memory confused this
> > > with dropping version suffixes (_vX.Y) in virCPUppc64ConvertLegacy. Or
> > > both options are true at the same time :-)
> > > 
> > > Maybe Andrea can remember more, I think he was involved in all this ages
> > > ago.
> > 
> > It seems that CPU models have been reported in lower case at least
> > going all the way back to QEMU 3.0.0 from 2018:
> > 
> >    https://gitlab.com/libvirt/libvirt/-/blob/c7e09b7b5fe7f64f12318d150b281343c6d64fd5/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.replies
> > 
> > It would probably make sense for libvirt to align with this decision,
> > and accept/generate lower case CPU model names. Of course upper case
> > CPU models would still need to be accepted for backwards
> > compatibility reasons.
> > 
> > Note that I'm suggesting we adopt this approach consistently for all
> > CPU models, including existing ones. So power8 should be accepted, as
> > should be POWER11.
> > 
> > CC'ing Daniel and David, both of which have worked on ppc64 in the
> > past and might be able to offer additional insights / historical
> > context.
> > 
> 
> It has to do with IBM marketing.
> 
> POWER is an acronym (Performance Optimization With Enhanced RISC) and all
> marketing material demanded that it was mentioned in full caps, e.g. POWER7,
> POWER8, POWER9. The reason why libvirt kept using full caps in libvirt, even
> after QEMU changed it to lower, was due to IBM marketing demands. IBM wanted
> users to use the full caps name in the domain XML, which is/was the official way
> customers would interact with the virtualization stack.

AFAIK there has never been any IBM marketing request to the libvirt project
in respect of CPU names. Perhaps that was communicated to IBM contributors
writing patches in private, but from POV of the libvirt community upstream
that is irrelevant. We review patches based on their technical characteristics
and AFAIR at the time QEMU became case insensitive, libvirt decided to stick
with uppercase names, driven by the historical precedent that we had already
established with earlier versions of libvirt.


> In power10 it seems like IBM marketing changed its mind [1] and now the model
> name is camel case, so now we're supposed to call them Power10, Power11, etc.

This is the prime example of why marketing should be ignored when making
technical decisions. They will always change their minds every few years.

> Ignoring case seems a sensible change to make since we do not want
> to be hostage of IBM marketing whims.

I'm not sure we have any significant precedent of ignoring the case of user
specified input. The general goal of libvirt XML input is to specify the
data in a single canonical format and avoid redundant ways to specifying
the same thing.

IMHO we have two options - continue to use uppercase in all ppc CPU models,
or switch to lowercase only for new CPU models we add.

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 v4 4/5] cpu_map: Add POWER11 cpu model support
Posted by Narayana Murty N 7 months, 1 week ago
On 06/05/25 6:02 PM, Daniel P. Berrangé wrote:
> AFAIK there has never been any IBM marketing request to the libvirt project
> in respect of CPU names. Perhaps that was communicated to IBM contributors
> writing patches in private, but from POV of the libvirt community upstream
> that is irrelevant. We review patches based on their technical characteristics
> and AFAIR at the time QEMU became case insensitive, libvirt decided to stick
> with uppercase names, driven by the historical precedent that we had already
> established with earlier versions of libvirt.
>
>
>> In power10 it seems like IBM marketing changed its mind [1] and now the model
>> name is camel case, so now we're supposed to call them Power10, Power11, etc.
> This is the prime example of why marketing should be ignored when making
> technical decisions. They will always change their minds every few years.
>
>> Ignoring case seems a sensible change to make since we do not want
>> to be hostage of IBM marketing whims.
> I'm not sure we have any significant precedent of ignoring the case of user
> specified input. The general goal of libvirt XML input is to specify the
> data in a single canonical format and avoid redundant ways to specifying
> the same thing.
>
> IMHO we have two options - continue to use uppercase in all ppc CPU models,
> or switch to lowercase only for new CPU models we add.
Yes, Daniel I agree. We would like to continue with the uppercase for 
POWER cpu model names. I will spin off the new version of patch with 
cpu-model name as POWER11.
> With regards,
> Daniel
Regards, Narayana
Re: [PATCH v4 4/5] cpu_map: Add POWER11 cpu model support
Posted by Narayana Murty N 7 months, 2 weeks ago
On 05/05/25 5:37 PM, Daniel Henrique Barboza wrote:
>
>
> On 5/5/25 5:58 AM, Andrea Bolognani wrote:
>> On Tue, Apr 29, 2025 at 05:41:43PM +0200, Jiří Denemark wrote:
>>> On Tue, Apr 29, 2025 at 13:03:12 +0100, Daniel P. Berrangé wrote:
>>>> On Tue, Apr 29, 2025 at 01:42:22PM +0200, Peter Krempa wrote:
>>>>> On Tue, Apr 29, 2025 at 12:36:40 +0100, Daniel P. Berrangé wrote:
>>>>>> On Tue, Apr 15, 2025 at 04:13:08AM -0400, Narayana Murty N wrote:
>>>>>>>         <model usable='unknown' vendor='IBM'>POWER7</model>
>>>>>>>         <model usable='unknown' vendor='IBM'>POWER8</model>
>>>>>>>         <model usable='unknown' vendor='IBM'>POWER9</model>
>>>>>>> +      <model usable='unknown' vendor='IBM'>power11</model>
>>>>>>
>>>>>> QEMU allows both upper & lowercase for CPU names, and libvirt
>>>>>> has stuck with uppercase historically.
>>>>>>
>>>>>> What's the justification for changing this approach for
>>>>>> power11 ?
>>>>>
>>>>> qemu itself reports the model lowercase now. In v2 of this series 
>>>>> I've
>>>>> pointed that out because defining this caused test data to change 
>>>>> which
>>>>> was suspiciuous.
>>>>
>>>> By 'reports the model lowercase' i presume you're referring to 
>>>> '-cpu help'
>>>> and/or 'query-cpu-definitions' ?  That doesn't seem to be a new change
>>>> either though. I've not gone far back, but qemu 8.2.0 seems to already
>>>> be using lowercase.
>>>>
>>>> So I'm still wondering why we're changing our decision about CPU 
>>>> naming
>>>> /now/ with power11 ?  The commit message needs to explain and justify
>>>> this change.
>>>
>>> Well, when I replied to the original patch, my memory was telling me
>>> QEMU switched to lower case names and had to add compatibility code to
>>> translate our existing upper case names. And adding a new CPU model
>>> looked like a good excuse to stop doing that (for new models of course)
>>> and just define the model in lower case. Now that you're questioning
>>> this I tried to find evidence and I could not find it :-( So there are
>>> two options... either I'm bad at searching or I my memory confused this
>>> with dropping version suffixes (_vX.Y) in virCPUppc64ConvertLegacy. Or
>>> both options are true at the same time :-)
>>>
>>> Maybe Andrea can remember more, I think he was involved in all this 
>>> ages
>>> ago.
>>
>> It seems that CPU models have been reported in lower case at least
>> going all the way back to QEMU 3.0.0 from 2018:
>>
>> https://gitlab.com/libvirt/libvirt/-/blob/c7e09b7b5fe7f64f12318d150b281343c6d64fd5/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.replies
>>
>> It would probably make sense for libvirt to align with this decision,
>> and accept/generate lower case CPU model names. Of course upper case
>> CPU models would still need to be accepted for backwards
>> compatibility reasons.
>>
>> Note that I'm suggesting we adopt this approach consistently for all
>> CPU models, including existing ones. So power8 should be accepted, as
>> should be POWER11.
>>
>> CC'ing Daniel and David, both of which have worked on ppc64 in the
>> past and might be able to offer additional insights / historical
>> context.
>>
>
> It has to do with IBM marketing.
>
> POWER is an acronym (Performance Optimization With Enhanced RISC) and all
> marketing material demanded that it was mentioned in full caps, e.g. 
> POWER7,
> POWER8, POWER9. The reason why libvirt kept using full caps in 
> libvirt, even
> after QEMU changed it to lower, was due to IBM marketing demands. IBM 
> wanted
> users to use the full caps name in the domain XML, which is/was the 
> official way
> customers would interact with the virtualization stack.
>
> In power10 it seems like IBM marketing changed its mind [1] and now 
> the model
> name is camel case, so now we're supposed to call them Power10, 
> Power11, etc.
>
> Narayana can surely provide more details than I can, but I believe 
> this is the
> reason why we would want a change for case insensitive CPU models in 
> the IBM
> PowerPC family in libvirt. There are domains in the wild that are 
> using full
> caps names, and we can't break those, but now IBM would tell customers 
> to use
>
> <model fallback='forbid'>power11</model>"

Both Power and POWER can be used interchangeably, as IBM has registered 
trademark for both.

https://www.ibm.com/legal/copyright-trademark#p

However, "Power" camel case is recommended.

>
> or similar. Ignoring case seems a sensible change to make since we do 
> not want
> to be hostage of IBM marketing whims.
>

Since the libvirt follows all capital letters convention, so we can 
follow the existing cpu-model convention as POWER.

Patch 5/5 makes the host model comparison case-insensitive in libvirt, 
which aligns with current qemu behavior.


> But, as Daniel said, this should be a properly justified and 
> documented change.
> Thanks,
>
>
> The other Daniel
>
>
> [1] https://en.wikipedia.org/wiki/Power10, section "Branding"
>
>
Regards,

Narayana.

>
>
Re: [PATCH v4 4/5] cpu_map: Add POWER11 cpu model support
Posted by Peter Krempa via Devel 7 months, 3 weeks ago
On Tue, Apr 29, 2025 at 13:03:12 +0100, Daniel P. Berrangé wrote:
> On Tue, Apr 29, 2025 at 01:42:22PM +0200, Peter Krempa wrote:
> > On Tue, Apr 29, 2025 at 12:36:40 +0100, Daniel P. Berrangé wrote:
> > > On Tue, Apr 15, 2025 at 04:13:08AM -0400, Narayana Murty N wrote:
> > > > Add POWER11 as a supported cpu model for ppc64.
> > > > 
> > > > Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
> > > > ---
> > > >  src/cpu_map/index.xml                      | 1 +
> > > >  src/cpu_map/meson.build                    | 1 +
> > > >  src/cpu_map/ppc64_POWER11.xml              | 6 ++++++
> > > >  tests/domaincapsdata/qemu_10.0.0.ppc64.xml | 1 +
> > > >  4 files changed, 9 insertions(+)
> > > >  create mode 100644 src/cpu_map/ppc64_POWER11.xml
> > > > diff --git a/tests/domaincapsdata/qemu_10.0.0.ppc64.xml b/tests/domaincapsdata/qemu_10.0.0.ppc64.xml
> > 
> > [...]
> > 
> > > > index 3c864146eb..c449d96f86 100644
> > > > --- a/tests/domaincapsdata/qemu_10.0.0.ppc64.xml
> > > > +++ b/tests/domaincapsdata/qemu_10.0.0.ppc64.xml
> > > > @@ -39,6 +39,7 @@
> > > >        <model usable='unknown' vendor='IBM'>POWER7</model>
> > > >        <model usable='unknown' vendor='IBM'>POWER8</model>
> > > >        <model usable='unknown' vendor='IBM'>POWER9</model>
> > > > +      <model usable='unknown' vendor='IBM'>power11</model>
> > > 
> > > QEMU allows both upper & lowercase for CPU names, and libvirt
> > > has stuck with uppercase historically.
> > > 
> > > What's the justification for changing this approach for
> > > power11 ?
> > 
> > qemu itself reports the model lowercase now. In v2 of this series I've
> > pointed that out because defining this caused test data to change which
> > was suspiciuous.
> 
> By 'reports the model lowercase' i presume you're referring to '-cpu help'
> and/or 'query-cpu-definitions' ?  That doesn't seem to be a new change
> either though. I've not gone far back, but qemu 8.2.0 seems to already
> be using lowercase.

Ah I see. Yes I used [1]

 $ qemu-system-ppc64 -cpu ? | grep -i power11

> 
> So I'm still wondering why we're changing our decision about CPU naming
> /now/ with power11 ?  The commit message needs to explain and justify
> this change.

I questioned hunks in the previous version of the commit which did the
following:

> --cpu power11 \
> +-cpu POWER11 \

and

> -    <model fallback='forbid'>power11</model>
> +    <model fallback='forbid'>POWER11</model>

After defining the new model which looked suspicious and wasn't
sufficiently justified either.

The "power11" string appeared after adding the caps dump, which I
presume that was generated on a power11 machine (not stated in commit
message).

To be clear I don't care about the naming ... or ppc64 tbh. I responded
to the original series because it was adding caps dumps from a dirty
qemu tree.

[1] https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/5SS2QGNJB7XE2NK63TQKSFUOE4DSOH77/
Re: [PATCH v4 4/5] cpu_map: Add POWER11 cpu model support
Posted by Peter Krempa via Devel 7 months, 3 weeks ago
On Tue, Apr 15, 2025 at 04:13:08 -0400, Narayana Murty N wrote:
> Add POWER11 as a supported cpu model for ppc64.

uppercase

> 
> Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
> ---
>  src/cpu_map/index.xml                      | 1 +
>  src/cpu_map/meson.build                    | 1 +
>  src/cpu_map/ppc64_POWER11.xml              | 6 ++++++
>  tests/domaincapsdata/qemu_10.0.0.ppc64.xml | 1 +
>  4 files changed, 9 insertions(+)
>  create mode 100644 src/cpu_map/ppc64_POWER11.xml

The naming of the file isn't consistent with the CPU model name and
commit message.

Otherwise looks good.

> 
> diff --git a/src/cpu_map/index.xml b/src/cpu_map/index.xml
> index 87db338cee..790c3b2f83 100644
> --- a/src/cpu_map/index.xml
> +++ b/src/cpu_map/index.xml
> @@ -181,6 +181,7 @@
>        <include filename='ppc64_POWER8.xml'/>
>        <include filename='ppc64_POWER9.xml'/>
>        <include filename='ppc64_POWER10.xml'/>
> +      <include filename='ppc64_POWER11.xml'/>

uppercase

>      </group>
>  
>      <group name='Freescale-based CPU models'>
> diff --git a/src/cpu_map/meson.build b/src/cpu_map/meson.build
> index dee8441a13..abf9c21e4f 100644
> --- a/src/cpu_map/meson.build
> +++ b/src/cpu_map/meson.build
> @@ -17,6 +17,7 @@ cpumap_data = [
>    'arm_vendors.xml',
>    'index.xml',
>    'ppc64_POWER10.xml',
> +  'ppc64_POWER11.xml',

uppercase

>    'ppc64_POWER6.xml',
>    'ppc64_POWER7.xml',
>    'ppc64_POWER8.xml',
> diff --git a/src/cpu_map/ppc64_POWER11.xml b/src/cpu_map/ppc64_POWER11.xml
> new file mode 100644
> index 0000000000..d624043072
> --- /dev/null
> +++ b/src/cpu_map/ppc64_POWER11.xml
> @@ -0,0 +1,6 @@
> +<cpus>
> +  <model name='power11'>

lowercase

> +    <vendor name='IBM'/>
> +    <pvr value='0x00820000' mask='0xffff0000'/>
> +  </model>
> +</cpus>
> diff --git a/tests/domaincapsdata/qemu_10.0.0.ppc64.xml b/tests/domaincapsdata/qemu_10.0.0.ppc64.xml
> index 3c864146eb..c449d96f86 100644
> --- a/tests/domaincapsdata/qemu_10.0.0.ppc64.xml
> +++ b/tests/domaincapsdata/qemu_10.0.0.ppc64.xml
> @@ -39,6 +39,7 @@
>        <model usable='unknown' vendor='IBM'>POWER7</model>
>        <model usable='unknown' vendor='IBM'>POWER8</model>
>        <model usable='unknown' vendor='IBM'>POWER9</model>
> +      <model usable='unknown' vendor='IBM'>power11</model>

lowercase

>      </mode>
>    </cpu>
>    <memoryBacking supported='yes'>
> -- 
> 2.48.1
> 


Since lowercase is the cpu name make sure to name the file the same way
and fix the commit message