(CCing libvir-list)
On Thu, Sep 26, 2019 at 11:58:30PM +0200, Paolo Bonzini wrote:
> Is this really needed? QEMU's value of pconfig=on vs. off should be
> provided by QMP CPU model queries, if a property is not available then
> Libvirt should not try to set it to off.
>
Libvirt can easily work around it for new VMs, and it should.
The issue are VMs that were created with QEMU 3.1.0. QEMU 3.1.0
was telling libvirt "Icelake-Server can't be used unless
pconfig=off is used", and libvirt was adding pconfig=off to the
domain XML as expected.
It would be wrong for libvirt to remove a device option when
migrating an existing VM to another QEMU version. We can change
the rules (and document that), but do we want to?
> Paolo
>
> Il gio 26 set 2019, 23:23 Eduardo Habkost <ehabkost@xxxxxxxxxx> ha scritto:
>
> > QEMU 3.1.0 was shipped with the "pconfig" CPU property available,
> > added by commit 5131dc433df5 ("i386: Add CPUID bit for PCONFIG").
> >
> > Then the feature was removed in QEMU 4.0.0 (and 3.1.1), by commit
> > 712f807e1965 ("Revert 'i386: Add CPUID bit for PCONFIG'").
> >
> > In theory this would be OK, but we do have a problem: existing
> > software (like libvirt) was already using "pconfig=off" since
> > QEMU 3.1.0 on some cases. This means software that worked with
> > QEMU 3.1.0 doesn't work with QEMU 3.1.1 and newer.
> >
> > One symptom is the following error being generated by
> > virt-install while trying to use the 'host-model' CPU model, on a
> > host that's identified as Icelake-Server:
> >
> > ERROR internal error: qemu unexpectedly closed the monitor: \
> > 2019-09-24T22:57:42.550032Z qemu-kvm: \
> > can't apply global Icelake-Server-x86_64-cpu.pconfig=off: Property
> > '.pconfig' not found
> >
> > Re-add "pconfig" to feature_word_info[FEAT_7_0_EDX].feat_names so
> > "pconfig=off" will work again.
> >
> > This change still won't let users set "monitor=on" because all
> > accelerators currently report the feature as unsupported. But to
> > make sure PCONFIG won't be enabled by accident in the future
> > before we implement the necessary migration code, also add the
> > feature to .unmigratable_flags.
> >
> > Fixes: 712f807e1965 ("Revert 'i386: Add CPUID bit for PCONFIG'")
> > Signed-off-by: Eduardo Habkost <ehabkost@xxxxxxxxxx>
> > ---
> > target/i386/cpu.h | 2 ++
> > target/i386/cpu.c | 8 +++++++-
> > 2 files changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > index 8e090acd74..b728bd22f1 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -731,6 +731,8 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> > #define CPUID_7_0_EDX_AVX512_4VNNIW (1U << 2)
> > /* AVX512 Multiply Accumulation Single Precision */
> > #define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3)
> > +/* PCONFIG Instruction */
> > +#define CPUID_7_0_EDX_PCONFIG (1U << 18)
> > /* Speculation Control */
> > #define CPUID_7_0_EDX_SPEC_CTRL (1U << 26)
> > /* Arch Capabilities */
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 71034aeb5a..3e25505bd3 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -1084,7 +1084,7 @@ static FeatureWordInfo
> > feature_word_info[FEATURE_WORDS] = {
> > NULL, NULL, NULL, NULL,
> > NULL, NULL, "md-clear", NULL,
> > NULL, NULL, NULL, NULL,
> > - NULL, NULL, NULL /* pconfig */, NULL,
> > + NULL, NULL, "pconfig", NULL,
> > NULL, NULL, NULL, NULL,
> > NULL, NULL, "spec-ctrl", "stibp",
> > NULL, "arch-capabilities", "core-capability", "ssbd",
> > @@ -1095,6 +1095,12 @@ static FeatureWordInfo
> > feature_word_info[FEATURE_WORDS] = {
> > .reg = R_EDX,
> > },
> > .tcg_features = TCG_7_0_EDX_FEATURES,
> > + /*
> > + * CPU state altered by the PCONFIG instruction (e.g. MKTME key
> > table)
> > + * is not migrated by QEMU yet, so PCONFIG is unmigratable until
> > + * this is implemented.
> > + */
> > + .unmigratable_flags = CPUID_7_0_EDX_PCONFIG,
> > },
> > [FEAT_7_1_EAX] = {
> > .type = CPUID_FEATURE_WORD,
> > --
> > 2.21.0
> >
> >
--
Eduardo
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
