Re: [pve-devel] [PATCH pve-qemu 1/1] patch: add 0001-add-cpu-models-x86-64-abi.patch

["DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>]

Le mercredi 17 mai 2023 à 09:46 +0200, Fiona Ebner a écrit :
> Am 17.05.23 um 09:02 schrieb Alexandre Derumier:
> > Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> > ---
> >  .../pve/0001-add-cpu-models-x86-64-abi.patch  | 272
> > ++++++++++++++++++
> >  debian/patches/series                         |   1 +
> >  2 files changed, 273 insertions(+)
> >  create mode 100644 debian/patches/pve/0001-add-cpu-models-x86-64-
> > abi.patch
> > 
> 
> Do we really need this? Can't we just define these as custom CPU
> models
> in qemu-server? I'd really prefer to not pick up such patches if not
> necessary. Ideally, we reduce divergence from upstream QEMU rather
> than
> increase it. 
yes sure we can do it in qemu-server, with simply setting flags in
command line.

It was more by security to not forgot a cpu flag.


> Since the patch was never applied, what other solution did
> upstream or libvirt go for?
The end of the discussion was that it could be done userland
like qemu-server ;)


I'll rework the patch series.

(BTW, what do you think about bumping the default model ? is it ok for
you ?)




> 
> If we really do need this, we should rather ask upstream again if
> they
> can apply it or why they didn't first. Because it's two years old, so
> if
> there were good reasons, so without asking, it's unlikely that they
> will
> apply it soon and then we'd have to carry it around forever
> ourselves...
> 
> > diff --git a/debian/patches/pve/0001-add-cpu-models-x86-64-
> > abi.patch b/debian/patches/pve/0001-add-cpu-models-x86-64-abi.patch
> > new file mode 100644
> > index 0000000..e98f862
> > --- /dev/null
> > +++ b/debian/patches/pve/0001-add-cpu-models-x86-64-abi.patch
> > @@ -0,0 +1,272 @@
> > +From 7d9ff48a96a7613a15e5427bc8987358e5529e45 Mon Sep 17 00:00:00
> > 2001
> > +From: Alexandre Derumier <aderumier@odiso.com>
> 
> How did you apply the patch from the mail? Here should be the name of
> the one how sent the mail, not yours.
> 
> > +
> > +  CPUID.01H:ECX.pcid [bit 17]
> > +  CPUID.01H:ECX.x2apic [bit 21]
> > +  CPUID.01H:ECX.tsc-deadline [bit 24]
> > +  CPUID.07H:EBX.invpcid [bit 10]
> > +  CPUID.07H:EBX.avx512f [bit 16]
> > +  CPUID.07H:EBX.avx512dq [bit 17]
> > +  CPUID.07H:EBX.rdseed [bit 18]
> > +  CPUID.07H:EBX.avx512cd [bit 28]
> > +  CPUID.07H:EBX.avx512bw [bit 30]
> > +  CPUID.07H:EBX.avx512vl [bit 31]
> > +  CPUID.80000001H:ECX.3dnowprefetch [bit 8]
> > +  CPUID.0DH:EAX.xsavec [bit 1]
> > +
> 
> And here you lost the original
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> That is not okay.
> 
> If you modified the patch you should do:
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> [AD: describe your changes here]
> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> 
> With AD being your initials, but you can also use A or leave them out
> completely. The important part is that the changes are mentioned ;)
> 
> > +---
> > + target/i386/cpu.c | 162
> > ++++++++++++++++++++++++++++++++++++++++++++++
> > + 1 file changed, 162 insertions(+)
> > +
>