Re: [PATCH v4 4/5] cpu_map: Add POWER11 cpu model support
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(a)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/5...