Re: [PATCH] i386: Re-add "pconfig" CPUID flag name

[Paolo Bonzini <pbonzini@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3541 bytes --]

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.

Paolo

Il gio 26 set 2019, 23:23 Eduardo Habkost <ehabkost@redhat.com> 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@redhat.com>
> ---
>  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
>
>

[-- Attachment #2: Type: text/html, Size: 4562 bytes --]