Re: [libvirt] [RFC] cpu_map: Remove pconfig from Icelake-Server CPU model

-----Original Message----- From: Eduardo Habkost <ehabkost(a)redhat.com&gt; Sent: Monday, September 30, 2019 22:11 To: Jiri Denemark <jdenemar(a)redhat.com&gt; Cc: libvir-list(a)redhat.com; qemu-devel(a)nongnu.org; Paolo Bonzini <pbonzini(a)redhat.com>; Daniel P. Berrangé <berrange(a)redhat.com>; Kang, Luwei <luwei.kang(a)intel.com>; thomas.lendacky(a)amd.com; Robert Hoo <robert.hu(a)linux.intel.com>; Richard Henderson <rth(a)twiddle.net>; Jiri Denemark <jdenemar(a)redhat.com>; Huang, Kai <kai.huang(a)intel.com>; Hu, Robert <robert.hu(a)intel.com&gt; Subject: Re: [RFC] cpu_map: Remove pconfig from Icelake-Server CPU model CCing qemu-devel and QEMU developers. On Mon, Sep 30, 2019 at 12:24:53PM +0200, Jiri Denemark wrote: > On Thu, Sep 26, 2019 at 18:43:05 -0300, Eduardo Habkost wrote: > > The pconfig feature never worked, and adding "pconfig=off" to the > > QEMU command-line triggers a regression in QEMU 3.1.1 and 4.0.0. > > > > Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com&gt; > > --- > > I'm sending this as an RFC because I couldn't test it properly, and > > because I don't know what are the consequences of changing cpu_map > > between libvirt versions. > > --- > > src/cpu_map/x86_Icelake-Server.xml | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/src/cpu_map/x86_Icelake-Server.xml > > b/src/cpu_map/x86_Icelake-Server.xml > > index fb15977a59..188a781282 100644 > > --- a/src/cpu_map/x86_Icelake-Server.xml > > +++ b/src/cpu_map/x86_Icelake-Server.xml > > @@ -56,7 +56,9 @@ > > <feature name='pat'/> > > <feature name='pcid'/> > > <feature name='pclmuldq'/> > > - <feature name='pconfig'/> > > + <!-- 'pconfig' was added by accident in QEMU 3.1.0 but never worked. > > + It was removed in QEMU 3.1.1 and 4.0.0. See QEMU commits > > + 76e5a4d58357 and 712f807e1965 for details --> > > <feature name='pdpe1gb'/> > > <feature name='pge'/> > > <feature name='pku'/> > > IIUC this never worked and a domain started with Icelake-Server CPU > model would actually end up running with pconfig=off, right? In that > case removing pconfig from Icelake-Server would not cause any issues > when a domain is started. However, I'm afraid migration would be broken. > > If a domain is started by new libvirt (with this patch in) using > Icelake-Server CPU model possibly with additional features added or > removed, but without pconfig being explicitly mentioned, libvirt would > not disable pconfig when updating active definition according to the > actual CPU created by QEMU. This is because libvirt did not ask for > pconfig (either explicitly or implicitly via the CPU model). When such > domain gets migrated to an older libvirt (which thinks pconfig is part > of Icelake-Server), it will complain that QEMU disabled pconfig while > the source domain was running with pconfig enabled (which is an > incorrect result caused by inconsistent view of the CPU model). > > Thus we would need to add a hack which would explicitly disable > pconfig in the domain definition used for migration to make sure the > destination libvirtd knows pconfig was disabled. New libvirt would > just drop the disabled pconfig feature from the CPU definition before > starting a domain. > > [1] From this point of view we could just keep the CPU model in > libvirt untouched. This way pconfig would always be explicitly > disabled when a domain is running and the host-model CPU definition > would also show it as explicitly disabled. > > [2] However starting a domain with Icelake-Server so that it can be > migrated or saved/restored on QEMU in 3.1.1 and 4.0.0 would be > impossible. This can be solved by a different hack, which would drop > pconfig=off from QEMU command line. > > [3] But if pconfig was removed from QEMU and never returned back, we > could remove it from any domain XML we see > (virQEMUCapsCPUFilterFeatures mentions several other features which we handle this way). > > That said, I think [3] would be the best option. But if QEMU cannot or > doesn't want to remove pconfig completely, I'd go with [1] as the hack > in [2] would be too ugly and confusing. From the QEMU side, [3] is better, as pconfig was added by accident in 3.1.0 and it would be simpler to not re-add it. This might be a problem if there are plans to eventually make KVM support pconfig, though. Paolo, Robert, are there plans to support pconfig in KVM in the future?

-- Eduardo