| Message ID | 1475080395-12068-1-git-send-email-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
(CCing Richard, sorry I forgot to CC you) Ping? Any objection to this fix? On Wed, Sep 28, 2016 at 01:33:15PM -0300, Eduardo Habkost wrote: > A regression was introduced by commit 96193c22a "target-i386: > Move xsave component mask to features array": all > CPUID[EAX=0xD,ECX=0]:EAX bits were being reported as unmigratable > because they don't have feature names defined. This broke > "-cpu host" because it enables only migratable features by > default. > > This adds a new field to FeatureWordInfo: migratable_flags, which > will make those features be reported as migratable even if they > don't have a property name defined. > > Reported-by: Wanpeng Li <wanpeng.li@hotmail.com> > Cc: Paolo Bonzini <bonzini@gnu.org> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > target-i386/cpu.c | 19 +++++++++++-------- > 1 file changed, 11 insertions(+), 8 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 333309b..0807e92 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -258,6 +258,7 @@ typedef struct FeatureWordInfo { > int cpuid_reg; /* output register (R_* constant) */ > uint32_t tcg_features; /* Feature flags supported by TCG */ > uint32_t unmigratable_flags; /* Feature flags known to be unmigratable */ > + uint32_t migratable_flags; /* Feature flags known to be migratable */ > } FeatureWordInfo; > > static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { > @@ -494,6 +495,10 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { > .cpuid_needs_ecx = true, .cpuid_ecx = 0, > .cpuid_reg = R_EAX, > .tcg_features = ~0U, > + .migratable_flags = XSTATE_FP_MASK | XSTATE_SSE_MASK | > + XSTATE_YMM_MASK | XSTATE_BNDREGS_MASK | XSTATE_BNDCSR_MASK | > + XSTATE_OPMASK_MASK | XSTATE_ZMM_Hi256_MASK | XSTATE_Hi16_ZMM_MASK | > + XSTATE_PKRU_MASK, > }, > [FEAT_XSAVE_COMP_HI] = { > .cpuid_eax = 0xD, > @@ -600,15 +605,13 @@ static uint32_t x86_cpu_get_migratable_flags(FeatureWord w) > > for (i = 0; i < 32; i++) { > uint32_t f = 1U << i; > - /* If the feature name is unknown, it is not supported by QEMU yet */ > - if (!wi->feat_names[i]) { > - continue; > - } > - /* Skip features known to QEMU, but explicitly marked as unmigratable */ > - if (wi->unmigratable_flags & f) { > - continue; > + > + /* If the feature name is known, it is implicitly considered migratable, > + * unless it is explicitly set in unmigratable_flags */ > + if ((wi->migratable_flags & f) || > + (wi->feat_names[i] && !(wi->unmigratable_flags & f))) { > + r |= f; > } > - r |= f; > } > return r; > } > -- > 2.7.4 > >
2016-09-30 2:40 GMT+08:00 Eduardo Habkost <ehabkost@redhat.com>: > (CCing Richard, sorry I forgot to CC you) > > Ping? Any objection to this fix? > > On Wed, Sep 28, 2016 at 01:33:15PM -0300, Eduardo Habkost wrote: >> A regression was introduced by commit 96193c22a "target-i386: >> Move xsave component mask to features array": all >> CPUID[EAX=0xD,ECX=0]:EAX bits were being reported as unmigratable >> because they don't have feature names defined. This broke >> "-cpu host" because it enables only migratable features by >> default. >> >> This adds a new field to FeatureWordInfo: migratable_flags, which >> will make those features be reported as migratable even if they >> don't have a property name defined. >> >> Reported-by: Wanpeng Li <wanpeng.li@hotmail.com> >> Cc: Paolo Bonzini <bonzini@gnu.org> >> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Wanpeng Li <wanpeng.li@hotmail.com> >> --- >> target-i386/cpu.c | 19 +++++++++++-------- >> 1 file changed, 11 insertions(+), 8 deletions(-) >> >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c >> index 333309b..0807e92 100644 >> --- a/target-i386/cpu.c >> +++ b/target-i386/cpu.c >> @@ -258,6 +258,7 @@ typedef struct FeatureWordInfo { >> int cpuid_reg; /* output register (R_* constant) */ >> uint32_t tcg_features; /* Feature flags supported by TCG */ >> uint32_t unmigratable_flags; /* Feature flags known to be unmigratable */ >> + uint32_t migratable_flags; /* Feature flags known to be migratable */ >> } FeatureWordInfo; >> >> static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { >> @@ -494,6 +495,10 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { >> .cpuid_needs_ecx = true, .cpuid_ecx = 0, >> .cpuid_reg = R_EAX, >> .tcg_features = ~0U, >> + .migratable_flags = XSTATE_FP_MASK | XSTATE_SSE_MASK | >> + XSTATE_YMM_MASK | XSTATE_BNDREGS_MASK | XSTATE_BNDCSR_MASK | >> + XSTATE_OPMASK_MASK | XSTATE_ZMM_Hi256_MASK | XSTATE_Hi16_ZMM_MASK | >> + XSTATE_PKRU_MASK, >> }, >> [FEAT_XSAVE_COMP_HI] = { >> .cpuid_eax = 0xD, >> @@ -600,15 +605,13 @@ static uint32_t x86_cpu_get_migratable_flags(FeatureWord w) >> >> for (i = 0; i < 32; i++) { >> uint32_t f = 1U << i; >> - /* If the feature name is unknown, it is not supported by QEMU yet */ >> - if (!wi->feat_names[i]) { >> - continue; >> - } >> - /* Skip features known to QEMU, but explicitly marked as unmigratable */ >> - if (wi->unmigratable_flags & f) { >> - continue; >> + >> + /* If the feature name is known, it is implicitly considered migratable, >> + * unless it is explicitly set in unmigratable_flags */ >> + if ((wi->migratable_flags & f) || >> + (wi->feat_names[i] && !(wi->unmigratable_flags & f))) { >> + r |= f; >> } >> - r |= f; >> } >> return r; >> } >> -- >> 2.7.4 >> >> > > -- > Eduardo
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 333309b..0807e92 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -258,6 +258,7 @@ typedef struct FeatureWordInfo { int cpuid_reg; /* output register (R_* constant) */ uint32_t tcg_features; /* Feature flags supported by TCG */ uint32_t unmigratable_flags; /* Feature flags known to be unmigratable */ + uint32_t migratable_flags; /* Feature flags known to be migratable */ } FeatureWordInfo; static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { @@ -494,6 +495,10 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { .cpuid_needs_ecx = true, .cpuid_ecx = 0, .cpuid_reg = R_EAX, .tcg_features = ~0U, + .migratable_flags = XSTATE_FP_MASK | XSTATE_SSE_MASK | + XSTATE_YMM_MASK | XSTATE_BNDREGS_MASK | XSTATE_BNDCSR_MASK | + XSTATE_OPMASK_MASK | XSTATE_ZMM_Hi256_MASK | XSTATE_Hi16_ZMM_MASK | + XSTATE_PKRU_MASK, }, [FEAT_XSAVE_COMP_HI] = { .cpuid_eax = 0xD, @@ -600,15 +605,13 @@ static uint32_t x86_cpu_get_migratable_flags(FeatureWord w) for (i = 0; i < 32; i++) { uint32_t f = 1U << i; - /* If the feature name is unknown, it is not supported by QEMU yet */ - if (!wi->feat_names[i]) { - continue; - } - /* Skip features known to QEMU, but explicitly marked as unmigratable */ - if (wi->unmigratable_flags & f) { - continue; + + /* If the feature name is known, it is implicitly considered migratable, + * unless it is explicitly set in unmigratable_flags */ + if ((wi->migratable_flags & f) || + (wi->feat_names[i] && !(wi->unmigratable_flags & f))) { + r |= f; } - r |= f; } return r; }
A regression was introduced by commit 96193c22a "target-i386: Move xsave component mask to features array": all CPUID[EAX=0xD,ECX=0]:EAX bits were being reported as unmigratable because they don't have feature names defined. This broke "-cpu host" because it enables only migratable features by default. This adds a new field to FeatureWordInfo: migratable_flags, which will make those features be reported as migratable even if they don't have a property name defined. Reported-by: Wanpeng Li <wanpeng.li@hotmail.com> Cc: Paolo Bonzini <bonzini@gnu.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- target-i386/cpu.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)