Re: [pve-devel] [PATCH v3 qemu-server 1/7] cpuconfig: add new x86-64-vX models

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

Le jeudi 01 juin 2023 à 11:17 +0200, Fiona Ebner a écrit :
> Am 31.05.23 um 17:08 schrieb DERUMIER, Alexandre:
> > > >  
> > > > +my $builtin_models = {
> > > > +    'x86-64-v1' => {
> > > > +       'reported-model' => 'Opteron_G1',
> > > 
> > > It's unfortunate that we'll report this model and hence also AMD
> > > as
> > > vendor even on Intel hosts and vice versa for the other models.
> > > We
> > > could
> > > set the vendor to the host's vendor (in get_cpu_options() handle
> > > getting
> > > the vendor for the built-in models differently), 
> > I think it'll break if you migrate between intel/amd host anyway ?
> 
> That's true :)
> 
> > > but that's also
> > > strange, because then it would be Opteron_G1 with vendor
> > > GenuineIntel
> > > :/
> > > So maybe better to just leave it?
> > Well, kvm64 guest have vendor Authentic amd (even on intel host;),
> > with
> > modelname "common kvm processor")
> > cat /proc/cpuinfo
> > vendor_id       : AuthenticAmd
> > model name      : "Common KVM processor"
> 
> Are you sure? Or was this a migrated machine?
> 
> We have this comment
> 
> >     # generic types, use vendor from host node
> >     host => 'default',
> >     kvm32 => 'default',
> >     kvm64 => 'default',
> 
> and for a colleague, it is GenuineIntel with kvm64 on an Intel host.
> 
oh, you are right, it's indeed inherit the vendorid from the host
(tested with kvm64 && qemu64).
Maybe they are some specific trick for theses model in qemu
(because in cpu definition, the vendor is really intel for kvm64 && amd
for qemu64. Maybe they are some other part in code to inherit from the
host vendor)
https://github.com/qemu/qemu/blob/master/target/i386/cpu.c

> > If we don't want to expose the original modelname from where we
> > derivate, afaik, the only way is to patch qemu directly (like in my
> > v1).
> 
> We can actually just use the model-id option for -cpu and I think we
> should for these built-in models. I.e. set the vendor to the one from
> the host and the model-id to something generic too. Maybe "Common
> x86_64-abi1-compatible processor", but that feels involved, or maybe
> just "Common KVM processor" again?
ah ok, i wasn't aware of model-id. don't have preference, can be
"Common KVM processor" or "specific version". 
just tested it, vendor can also be specified
",model-id="Common KVM processor",vendor=GenuineIntel"

(I think it shouldn't break live migration if it's working with kvm64,
I think that vendor is not changing until the guest is restart.)


> 
> > > 
> > > > +       flags => "-vme;-svm;-vmx",
> > > 
> > > Why remove the svm and vmx flags? They are not exposed by us, so
> > > a
> > > user
> > > cannot even enable them back if needed, but needs to switch to a
> > > different CPU type.
> > yes, that's was the idea to forbid user to enable them, as it's
> > breaking livemigration, so it don't make any sense to use this
> > model
> > instead host model.
> > 
> > But I can remove them, no problem.
> 
> Oh, I missed the following in the referenced mail:
> 
> > None of the CPU models declare any VMX/SVM capability features.
> > IOW, even if a "vmx"/"svm" flag is added, it will still be unsafe
> > to attempt to live migrate the L1 guest if there are any L2
> > guests running with hardware virtualization.
> 
> Please keep them off then.
> 
ok, no problem

> > > > @@ -96,6 +115,9 @@ my $cpu_vendor_list = {
> > > >      kvm64 => 'default',
> > > >      qemu32 => 'default',
> > > >      qemu64 => 'default',
> > > > +    'x86-64-v1' => 'default',
> > > > +    'x86-64-v2' => 'default',
> > > > +    'x86-64-v3' => 'default',
> > > 
> > > 
> > > Currently all of the others are actual models we can pass
> > > directly to
> > > QEMU/KVM. I'd rather not add these custom built-in ones here.
> > > You'll
> > > need to adapt validate_vm_cpu_conf() of course, to also accept
> > > the
> > > built-in ones.
> > > 
> > > Because of adding them here, I can also set them as the
> > > 'reported-
> > > model'
> > > for a custom CPU in /etc/pve/virtual-guest/cpu-models.conf and
> > > parsing
> > > the file will work, but then starting a VM with that custom CPU
> > > will
> > > fail with kvm: unable to find CPU model 'x86-64-v1'.
> > > 
> > > If we'd like to enable using the built-in ones as base for custom
> > > CPU
> > > models, we'll need to handle them differently, but I'm not sure
> > > we
> > > should until there is enough user demand.
> > > 
> > Maybe it could be simplier to really add true build-model in qemu ?
> > (The qemu patch is pretty small, and shouldn't be difficult to
> > maintain)
> > 
> > I'm not sure, but maybe user will think that it's strange than x86-
> > 64-
> > v2 will display nahelem in guest && in qemu command line ?
> > 
> 
> Yes, for this it would be easier, but I also don't think we need to
> allow these as a base for custom models (at least not until there is
> enough user demand). And we can still switch later to make them true
> QEMU models if we really need to.
> 
ok,no problem, I'll rework my patch with model/vendor and all your
comments.

Thanks for your review !