[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/5] target-i386: fix kvm_default_features overw
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH 5/5] target-i386: fix kvm_default_features overwriting kvm_arch_get_supported_cpuid() |
Date: |
Tue, 5 Feb 2013 19:46:09 +0100 |
On Tue, 5 Feb 2013 15:53:04 -0200
Eduardo Habkost <address@hidden> wrote:
> On Tue, Feb 05, 2013 at 05:39:24PM +0100, Igor Mammedov wrote:
> > ORing kvm_default_features to def->kvm_features might set unsupported
> > bits if kvm_arch_get_supported_cpuid() returns less bits set than in
> > kvm_default_features. Fix it by removing kvm_default_features and using
> > only what kvm_arch_get_supported_cpuid() returns.
>
> This is exactly what we _must not_ do! We can't change CPUID bits in a
> machine-type or we will change the guest ABI. We must keep them stable
> for a machine-type and:
>
> * Not change if QEMU is upgraded;
> * Not change if the host kernel is upgraded;
> * Not change if running on different host hardware.
Is all of this applicable to CPU 'host' or only to built-in cpu_models?
>
> The only way we are allowed to change the guest ABI is on a new
> machine-type. This was the whole point of the bug fix at commit
> ef8621b1a3, and this patch reintroduces the bug.
>
> If you are starting QEMU in a host that doesn't support the set of KVM
> features QEMU is configured to expose (through the command-line or by
> choosing a machine-type), it _should_ fail to start if using -cpu
> enforce. This is a feature.
>
> We could enable this behavior on the "pc-next" machine, if we create it
> one day, but not on the stable machine-types.
>
>
> >
> > Mark disable_kvm_pv_eoi() to be removed in favor of compat_props
> > when available.
> >
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> > target-i386/cpu.c | 20 ++++++++------------
> > target-i386/cpu.h | 2 ++
> > 2 files changed, 10 insertions(+), 12 deletions(-)
> >
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 62fdc84..e66362e 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -220,17 +220,12 @@ typedef struct model_features_t {
> > int check_cpuid = 0;
> > int enforce_cpuid = 0;
> >
> > -static uint32_t kvm_default_features = (1 << KVM_FEATURE_CLOCKSOURCE) |
> > - (1 << KVM_FEATURE_NOP_IO_DELAY) |
> > - (1 << KVM_FEATURE_CLOCKSOURCE2) |
> > - (1 << KVM_FEATURE_ASYNC_PF) |
> > - (1 << KVM_FEATURE_STEAL_TIME) |
> > - (1 << KVM_FEATURE_PV_EOI) |
> > - (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT);
> > -
> > +/* TODO: replace it with compat properties when feature bits
> > + * are converted into static properties */
> > +bool x86_cpu_no_pv_eoi;
> > void disable_kvm_pv_eoi(void)
> > {
> > - kvm_default_features &= ~(1UL << KVM_FEATURE_PV_EOI);
> > + x86_cpu_no_pv_eoi = true;
> > }
> >
> > void host_cpuid(uint32_t function, uint32_t count,
> > @@ -2061,9 +2056,6 @@ static void x86_cpu_initfn(Object *obj)
> > env->cpuid_ext3_features = def->ext3_features;
> > object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", NULL);
> > env->cpuid_kvm_features = def->kvm_features;
> > - if (kvm_enabled()) {
> > - env->cpuid_kvm_features |= kvm_default_features;
> > - }
> > env->cpuid_svm_features = def->svm_features;
> > env->cpuid_ext4_features = def->ext4_features;
> > env->cpuid_7_0_ebx_features = def->cpuid_7_0_ebx_features;
> > @@ -2107,6 +2099,10 @@ static void x86_cpu_def_class_init(ObjectClass *oc,
> > void *data)
> > */
> > memcpy(xcc->info.vendor, hostcc->info.vendor,
> > sizeof(xcc->info.vendor));
> > + xcc->info.kvm_features = xcc->info.kvm_features;
> > + if (x86_cpu_no_pv_eoi) {
> > + xcc->info.kvm_features &= ~(1UL << KVM_FEATURE_PV_EOI);
> > + }
> > }
> >
> > /* Look for specific models that have the QEMU version in .model_id */
> > diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> > index 11ef942..51f20b6 100644
> > --- a/target-i386/cpu.h
> > +++ b/target-i386/cpu.h
> > @@ -1246,6 +1246,8 @@ void do_smm_enter(CPUX86State *env1);
> >
> > void cpu_report_tpr_access(CPUX86State *env, TPRAccess access);
> >
> > +/* TODO: convert to compat_props */
> > +extern bool x86_cpu_no_pv_eoi;
> > void disable_kvm_pv_eoi(void);
> >
> > /* Return name of 32-bit register, from a R_* constant */
> > --
> > 1.7.1
> >
>
> --
> Eduardo
--
Regards,
Igor
- [Qemu-devel] [PATCH 1/5] target-i386: Move cpu_x86_init(), (continued)
[Qemu-devel] [PATCH 3/5] target-i386: Slim conversion to X86CPU subclasses, Igor Mammedov, 2013/02/05