[Qemu-devel] [PATCH for-2.9 v2 0/2] i386: Don't override -cpu options on -cpu host/max

Eduardo Habkost posted 2 patches 7 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170327144815.8043-1-ehabkost@redhat.com
Test checkpatch passed
Test docker passed
Test s390x passed
target/i386/cpu.h |  2 ++
target/i386/cpu.c | 32 ++++++++++++++++++++------------
2 files changed, 22 insertions(+), 12 deletions(-)
[Qemu-devel] [PATCH for-2.9 v2 0/2] i386: Don't override -cpu options on -cpu host/max
Posted by Eduardo Habkost 7 years ago
The existing code for "host" and "max" CPU models overrides every
single feature in the CPU object at realize time, even the ones
that were explicitly enabled or disabled by the user using
"feat=on" or "feat=off", while features set using +feat/-feat are
kept.

This means "-cpu host,+invtsc" works as expected, while
"-cpu host,invtsc=on" doesn't.

This was a known bug, already documented in a comment inside
x86_cpu_expand_features(). What makes this bug worse now is that
libvirt 3.0.0 and newer now use "feat=on|off" instead of
+feat/-feat when it detects a QEMU version that supports it (see
libvirt commit d47db7b16dd5422c7e487c8c8ee5b181a2f9cd66).

This series fixes the bug.

Changes v1 -> v2:
* Split changes in two patches (Igor)
* Don't clear existing bits on env->features (they are already
  supposed to be all zeroes) (Igor)
* Fix typo on comment (Igor)

v1 -> v2 diff:

  diff --git a/target/i386/cpu.c b/target/i386/cpu.c
  index 5f2addbf75..13c0985f11 100644
  --- a/target/i386/cpu.c
  +++ b/target/i386/cpu.c
  @@ -3380,10 +3380,9 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
        */
       if (cpu->max_features) {
           for (w = 0; w < FEATURE_WORDS; w++) {
  -            /* Override only features that weren't not set explicitly
  +            /* Override only features that weren't set explicitly
                * by the user.
                */
  -            env->features[w] &= env->user_features[w];
               env->features[w] |=
                   x86_cpu_get_supported_feature_word(w, cpu->migratable) &
                   ~env->user_features[w];

Eduardo Habkost (2):
  i386: Replace uint32_t* with FeatureWord on feature getter/setter
  i386: Don't override -cpu options on -cpu host/max

 target/i386/cpu.h |  2 ++
 target/i386/cpu.c | 32 ++++++++++++++++++++------------
 2 files changed, 22 insertions(+), 12 deletions(-)

-- 
2.11.0.259.g40922b1


Re: [Qemu-devel] [PATCH for-2.9 v2 0/2] i386: Don't override -cpu options on -cpu host/max
Posted by Jiri Denemark 7 years ago
On Mon, Mar 27, 2017 at 11:48:13 -0300, Eduardo Habkost wrote:
> The existing code for "host" and "max" CPU models overrides every
> single feature in the CPU object at realize time, even the ones
> that were explicitly enabled or disabled by the user using
> "feat=on" or "feat=off", while features set using +feat/-feat are
> kept.
> 
> This means "-cpu host,+invtsc" works as expected, while
> "-cpu host,invtsc=on" doesn't.
> 
> This was a known bug, already documented in a comment inside
> x86_cpu_expand_features(). What makes this bug worse now is that
> libvirt 3.0.0 and newer now use "feat=on|off" instead of
> +feat/-feat when it detects a QEMU version that supports it (see
> libvirt commit d47db7b16dd5422c7e487c8c8ee5b181a2f9cd66).
> 
> This series fixes the bug.

Thanks.

Tested-by: Jiri Denemark <jdenemar@redhat.com>

Jirka