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

[Eduardo Habkost <ehabkost@redhat.com>]

On Thu, Sep 26, 2019 at 06:23:26PM -0300, Eduardo Habkost wrote:
> 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

Sorry, I meant "pconfig=on" above.

> 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
> 

-- 
Eduardo