[libvirt] [RFC] cpu_map: Remove pconfig from Icelake-Server CPU model
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@redhat.com> --- 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'/> -- 2.21.0
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@redhat.com> --- 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. Jirka
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@redhat.com> --- 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
-----Original Message----- From: Eduardo Habkost <ehabkost@redhat.com> Sent: Monday, September 30, 2019 22:11 To: Jiri Denemark <jdenemar@redhat.com> Cc: libvir-list@redhat.com; qemu-devel@nongnu.org; Paolo Bonzini <pbonzini@redhat.com>; Daniel P. Berrangé <berrange@redhat.com>; Kang, Luwei <luwei.kang@intel.com>; thomas.lendacky@amd.com; Robert Hoo <robert.hu@linux.intel.com>; Richard Henderson <rth@twiddle.net>; Jiri Denemark <jdenemar@redhat.com>; Huang, Kai <kai.huang@intel.com>; Hu, Robert <robert.hu@intel.com> 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@redhat.com> --- 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? [Robert Hoo] Thanks Eduardo for efforts in resolving this issue, introduced from my Icelake CPU model patch. I've no idea about PCONFIG's detail and plan. Let me sync with Huang, Kai and answer you soon.
-- Eduardo
On 30/09/19 16:31, Hu, Robert wrote:
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? [Robert Hoo] Thanks Eduardo for efforts in resolving this issue, introduced from my Icelake CPU model patch. I've no idea about PCONFIG's detail and plan. Let me sync with Huang, Kai and answer you soon.
It's really, really unlikely. It's possible that some future processor overloads PCONFIG in such a way that it will become virtualizable, but not IceLake. Would it make sense for libvirt to treat absent CPU flags as "default off" during migration, so that it can leave out the flag in the command line if it's off? If it's on, libvirt would pass pconfig=on as usual. This is a variant of [2], but more generally applicable:
[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.
Paolo
On Mon, Sep 30, 2019 at 17:16:27 +0200, Paolo Bonzini wrote:
On 30/09/19 16:31, Hu, Robert wrote:
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? [Robert Hoo] Thanks Eduardo for efforts in resolving this issue, introduced from my Icelake CPU model patch. I've no idea about PCONFIG's detail and plan. Let me sync with Huang, Kai and answer you soon.
It's really, really unlikely. It's possible that some future processor overloads PCONFIG in such a way that it will become virtualizable, but not IceLake.
I guess, the likelihood of this happening would be similar to reintroducing other features, such as osxsave or ospke, right?
Would it make sense for libvirt to treat absent CPU flags as "default off" during migration, so that it can leave out the flag in the command line if it's off? If it's on, libvirt would pass pconfig=on as usual. This is a variant of [2], but more generally applicable:
[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.
The domain XML does not contain a complete list of all CPU features. Features which are implicitly included in a CPU model are not listed in the XML. Count in the differences in libvirt's vs QEMU's definitions of a particular CPU model and you can see feat=off cannot be mechanically dropped from the command line as the CPU model itself could turn it on by default and thus feat=off is not redundant. Jirka
On 30/09/19 18:16, Jiri Denemark wrote:
On Mon, Sep 30, 2019 at 17:16:27 +0200, Paolo Bonzini wrote:
On 30/09/19 16:31, Hu, Robert wrote:
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? [Robert Hoo] Thanks Eduardo for efforts in resolving this issue, introduced from my Icelake CPU model patch. I've no idea about PCONFIG's detail and plan. Let me sync with Huang, Kai and answer you soon.
It's really, really unlikely. It's possible that some future processor overloads PCONFIG in such a way that it will become virtualizable, but not IceLake.
I guess, the likelihood of this happening would be similar to reintroducing other features, such as osxsave or ospke, right?
No, haveing osxsave and ospke was a mistake in the first place (they are not CPU features at all; they are more like a special way to let unprivileged programs read some bits of CR4). For pconfig, it's just very unlikely.
Would it make sense for libvirt to treat absent CPU flags as "default off" during migration, so that it can leave out the flag in the command line if it's off? If it's on, libvirt would pass pconfig=on as usual. This is a variant of [2], but more generally applicable:
[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.
The domain XML does not contain a complete list of all CPU features. Features which are implicitly included in a CPU model are not listed in the XML. Count in the differences in libvirt's vs QEMU's definitions of a particular CPU model and you can see feat=off cannot be mechanically dropped from the command line as the CPU model itself could turn it on by default and thus feat=off is not redundant.
I think I wasn't very clear, I meant "unsupported by QEMU" when I said "absent". Libvirt on the destination knows that from query-cpu-model-expansion, so it can leave off pconfig if it is not supported by the destination QEMU. Paolo
On Tue, Oct 01, 2019 at 11:20:42 +0200, Paolo Bonzini wrote:
On 30/09/19 18:16, Jiri Denemark wrote:
On Mon, Sep 30, 2019 at 17:16:27 +0200, Paolo Bonzini wrote:
On 30/09/19 16:31, Hu, Robert wrote:
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? [Robert Hoo] Thanks Eduardo for efforts in resolving this issue, introduced from my Icelake CPU model patch. I've no idea about PCONFIG's detail and plan. Let me sync with Huang, Kai and answer you soon.
It's really, really unlikely. It's possible that some future processor overloads PCONFIG in such a way that it will become virtualizable, but not IceLake.
I guess, the likelihood of this happening would be similar to reintroducing other features, such as osxsave or ospke, right?
No, haveing osxsave and ospke was a mistake in the first place (they are not CPU features at all; they are more like a special way to let unprivileged programs read some bits of CR4). For pconfig, it's just very unlikely.
Would it make sense for libvirt to treat absent CPU flags as "default off" during migration, so that it can leave out the flag in the command line if it's off? If it's on, libvirt would pass pconfig=on as usual. This is a variant of [2], but more generally applicable:
[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.
The domain XML does not contain a complete list of all CPU features. Features which are implicitly included in a CPU model are not listed in the XML. Count in the differences in libvirt's vs QEMU's definitions of a particular CPU model and you can see feat=off cannot be mechanically dropped from the command line as the CPU model itself could turn it on by default and thus feat=off is not redundant.
I think I wasn't very clear, I meant "unsupported by QEMU" when I said "absent". Libvirt on the destination knows that from query-cpu-model-expansion, so it can leave off pconfig if it is not supported by the destination QEMU.
Oh yeah, we should do this (and I plan to do so), but it won't really help us in this case. Although it could potentially save us some work in case we end up in a similar situation. Jirka
On Mon, 2019-09-30 at 17:16 +0200, Paolo Bonzini wrote:
On 30/09/19 16:31, Hu, Robert wrote:
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? [Robert Hoo] Thanks Eduardo for efforts in resolving this issue, introduced from my Icelake CPU model patch. I've no idea about PCONFIG's detail and plan. Let me sync with Huang, Kai and answer you soon.
It's really, really unlikely. It's possible that some future processor overloads PCONFIG in such a way that it will become virtualizable, but not IceLake.
I agree. Not Icelake. Thanks, -Kai
Would it make sense for libvirt to treat absent CPU flags as "default off" during migration, so that it can leave out the flag in the command line if it's off? If it's on, libvirt would pass pconfig=on as usual. This is a variant of [2], but more generally applicable:
[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.
Paolo
participants (5)
-
Eduardo Habkost -
Hu, Robert -
Huang, Kai -
Jiri Denemark -
Paolo Bonzini