| Message ID | 1345047221-26898-8-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Aug 15, 2012 at 06:13:27PM +0200, Igor Mammedov wrote: > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > -- > v2: > * replaced mask/ffs tricks by plain 'for (bit = 0; bit < 32; bit++)' > as suggested by Eduardo Habkost > --- > target-i386/cpu.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 101 insertions(+) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 37ba5ef..440e724 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -606,6 +606,101 @@ static int check_features_against_host(x86_def_t *guest_def) > return rv; > } > > +static bool is_feature_set(const char *name, const uint32_t featbitmap, > + const char **featureset) > +{ > + uint32_t bit; > + > + for (bit = 0; bit < 32; ++bit) { > + if (featureset[bit] && !altcmp(name, NULL, featureset[bit])) { > + if (featbitmap & (1 << bit)) { > + return true; > + } > + } > + } > + return false; > +} > + > +static void x86_cpuid_get_feature(Object *obj, Visitor *v, void *opaque, > + const char *name, Error **errp) > +{ > + X86CPU *cpu = X86_CPU(obj); > + CPUX86State *env = &cpu->env; > + bool value = true; > + > + if (!is_feature_set(name, env->cpuid_features, feature_name) && > + !is_feature_set(name, env->cpuid_ext_features, ext_feature_name) && > + !is_feature_set(name, env->cpuid_ext2_features, ext2_feature_name) && > + !is_feature_set(name, env->cpuid_ext3_features, ext3_feature_name) && > + !is_feature_set(name, env->cpuid_kvm_features, kvm_feature_name) && > + !is_feature_set(name, env->cpuid_svm_features, svm_feature_name)) { > + value = false; > + } > + > + visit_type_bool(v, &value, name, errp); > +} > + > +static void x86_cpuid_set_feature(Object *obj, Visitor *v, void *opaque, > + const char *name, Error **errp) > +{ > + X86CPU *cpu = X86_CPU(obj); > + CPUX86State *env = &cpu->env; > + uint32_t mask = 0; > + uint32_t *dst_features; > + bool value; > + > + visit_type_bool(v, &value, name, errp); > + if (error_is_set(errp)) { > + return; > + } > + > + if (lookup_feature(&mask, name, NULL, feature_name)) { > + dst_features = &env->cpuid_features; > + } else if (lookup_feature(&mask, name, NULL, ext_feature_name)) { > + dst_features = &env->cpuid_ext_features; > + } else if (lookup_feature(&mask, name, NULL, ext2_feature_name)) { > + dst_features = &env->cpuid_ext2_features; > + } else if (lookup_feature(&mask, name, NULL, ext3_feature_name)) { > + dst_features = &env->cpuid_ext3_features; > + } else if (lookup_feature(&mask, name, NULL, kvm_feature_name)) { > + dst_features = &env->cpuid_kvm_features; > + } else if (lookup_feature(&mask, name, NULL, svm_feature_name)) { > + dst_features = &env->cpuid_svm_features; > + } else { > + error_set(errp, QERR_PROPERTY_NOT_FOUND, "", name); > + return; > + } Some feature names are duplicated on feature_names and ext_feature_names. On AMD CPU models, we have to set both, on Intel models we need to set the bits only on cpuid_features. Maybe it's better to: 1) eliminate the duplication and set the names only on feature_name array; 2) At the end of CPU initialization, set the features on cpuid_ext2_features as copies of the corresponding cpuid_features bits if CPU vendor == AMD (or, maybe, if some boolean "ext2_features_aliases" flag is set, to make it not vendor-dependent). > + > + if (value) { > + *dst_features |= mask; > + } else { > + *dst_features &= ~mask; > + } > +} > + > +static void x86_register_cpuid_properties(Object *obj, const char **featureset) > +{ > + uint32_t bit; > + > + for (bit = 0; bit < 32; ++bit) { > + if (featureset[bit]) { > + char *feature_name, *save_ptr; > + char buf[32]; > + if (strlen(featureset[bit]) > sizeof(buf) - 1) { > + abort(); > + } > + pstrcpy(buf, sizeof(buf), featureset[bit]); > + feature_name = strtok_r(buf, "|", &save_ptr); > + while (feature_name) { > + object_property_add(obj, feature_name, "bool", > + x86_cpuid_get_feature, > + x86_cpuid_set_feature, NULL, NULL, NULL); > + feature_name = strtok_r(NULL, "|", &save_ptr); What happens when object_property_add() is called twice with the same feature name? > + } > + } > + } > +} > + > static void x86_cpuid_version_get_family(Object *obj, Visitor *v, void *opaque, > const char *name, Error **errp) > { > @@ -1824,6 +1919,12 @@ static void x86_cpu_initfn(Object *obj) > object_property_add(obj, "tsc-frequency", "int", > x86_cpuid_get_tsc_freq, > x86_cpuid_set_tsc_freq, NULL, NULL, NULL); > + x86_register_cpuid_properties(obj, feature_name); > + x86_register_cpuid_properties(obj, ext_feature_name); > + x86_register_cpuid_properties(obj, ext2_feature_name); > + x86_register_cpuid_properties(obj, ext3_feature_name); > + x86_register_cpuid_properties(obj, kvm_feature_name); > + x86_register_cpuid_properties(obj, svm_feature_name); > > env->cpuid_apic_id = env->cpu_index; > > -- > 1.7.11.2 > >
On Thu, Aug 16, 2012 at 03:10:50PM -0300, Eduardo Habkost wrote: > On Wed, Aug 15, 2012 at 06:13:27PM +0200, Igor Mammedov wrote: > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > -- > > v2: > > * replaced mask/ffs tricks by plain 'for (bit = 0; bit < 32; bit++)' > > as suggested by Eduardo Habkost > > --- > > target-i386/cpu.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 101 insertions(+) > > > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index 37ba5ef..440e724 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -606,6 +606,101 @@ static int check_features_against_host(x86_def_t *guest_def) > > return rv; > > } > > > > +static bool is_feature_set(const char *name, const uint32_t featbitmap, > > + const char **featureset) > > +{ > > + uint32_t bit; > > + > > + for (bit = 0; bit < 32; ++bit) { > > + if (featureset[bit] && !altcmp(name, NULL, featureset[bit])) { > > + if (featbitmap & (1 << bit)) { > > + return true; > > + } > > + } > > + } > > + return false; > > +} > > + > > +static void x86_cpuid_get_feature(Object *obj, Visitor *v, void *opaque, > > + const char *name, Error **errp) > > +{ > > + X86CPU *cpu = X86_CPU(obj); > > + CPUX86State *env = &cpu->env; > > + bool value = true; > > + > > + if (!is_feature_set(name, env->cpuid_features, feature_name) && > > + !is_feature_set(name, env->cpuid_ext_features, ext_feature_name) && > > + !is_feature_set(name, env->cpuid_ext2_features, ext2_feature_name) && > > + !is_feature_set(name, env->cpuid_ext3_features, ext3_feature_name) && > > + !is_feature_set(name, env->cpuid_kvm_features, kvm_feature_name) && > > + !is_feature_set(name, env->cpuid_svm_features, svm_feature_name)) { > > + value = false; > > + } > > + > > + visit_type_bool(v, &value, name, errp); > > +} > > + > > +static void x86_cpuid_set_feature(Object *obj, Visitor *v, void *opaque, > > + const char *name, Error **errp) > > +{ > > + X86CPU *cpu = X86_CPU(obj); > > + CPUX86State *env = &cpu->env; > > + uint32_t mask = 0; > > + uint32_t *dst_features; > > + bool value; > > + > > + visit_type_bool(v, &value, name, errp); > > + if (error_is_set(errp)) { > > + return; > > + } > > + > > + if (lookup_feature(&mask, name, NULL, feature_name)) { > > + dst_features = &env->cpuid_features; > > + } else if (lookup_feature(&mask, name, NULL, ext_feature_name)) { > > + dst_features = &env->cpuid_ext_features; > > + } else if (lookup_feature(&mask, name, NULL, ext2_feature_name)) { > > + dst_features = &env->cpuid_ext2_features; > > + } else if (lookup_feature(&mask, name, NULL, ext3_feature_name)) { > > + dst_features = &env->cpuid_ext3_features; > > + } else if (lookup_feature(&mask, name, NULL, kvm_feature_name)) { > > + dst_features = &env->cpuid_kvm_features; > > + } else if (lookup_feature(&mask, name, NULL, svm_feature_name)) { > > + dst_features = &env->cpuid_svm_features; > > + } else { > > + error_set(errp, QERR_PROPERTY_NOT_FOUND, "", name); > > + return; > > + } > > Some feature names are duplicated on feature_names and > ext_feature_names. On AMD CPU models, we have to set both, on Intel > models we need to set the bits only on cpuid_features. > > Maybe it's better to: > > 1) eliminate the duplication and set the names only on feature_name > array; > 2) At the end of CPU initialization, set the features on > cpuid_ext2_features as copies of the corresponding cpuid_features > bits if CPU vendor == AMD (or, maybe, if some boolean > "ext2_features_aliases" flag is set, to make it not > vendor-dependent). I implemented the above (after killing "cpudef" support), and pushed to: https://github.com/ehabkost/qemu-hacks/tree/work/unduplicate-feature-names-v0.4-2012-08-17 I will submit it as an RFC soon. I also made an experimental rebase of your series on top of that branch, at: https://github.com/ehabkost/qemu-hacks/tree/work/cpu-properties-igor-rebase-v3.4-2012-08-17 and also rebased my CPUID code work-in-progress, at: https://github.com/ehabkost/qemu-hacks/tree/work/cpuid-refactor-v0.9-2012-08-15 > > > > + > > + if (value) { > > + *dst_features |= mask; > > + } else { > > + *dst_features &= ~mask; > > + } > > +} > > + > > +static void x86_register_cpuid_properties(Object *obj, const char **featureset) > > +{ > > + uint32_t bit; > > + > > + for (bit = 0; bit < 32; ++bit) { > > + if (featureset[bit]) { > > + char *feature_name, *save_ptr; > > + char buf[32]; > > + if (strlen(featureset[bit]) > sizeof(buf) - 1) { > > + abort(); > > + } > > + pstrcpy(buf, sizeof(buf), featureset[bit]); > > + feature_name = strtok_r(buf, "|", &save_ptr); > > + while (feature_name) { > > + object_property_add(obj, feature_name, "bool", > > + x86_cpuid_get_feature, > > + x86_cpuid_set_feature, NULL, NULL, NULL); > > + feature_name = strtok_r(NULL, "|", &save_ptr); > > What happens when object_property_add() is called twice with the same > feature name? > > > + } > > + } > > + } > > +} > > + > > static void x86_cpuid_version_get_family(Object *obj, Visitor *v, void *opaque, > > const char *name, Error **errp) > > { > > @@ -1824,6 +1919,12 @@ static void x86_cpu_initfn(Object *obj) > > object_property_add(obj, "tsc-frequency", "int", > > x86_cpuid_get_tsc_freq, > > x86_cpuid_set_tsc_freq, NULL, NULL, NULL); > > + x86_register_cpuid_properties(obj, feature_name); > > + x86_register_cpuid_properties(obj, ext_feature_name); > > + x86_register_cpuid_properties(obj, ext2_feature_name); > > + x86_register_cpuid_properties(obj, ext3_feature_name); > > + x86_register_cpuid_properties(obj, kvm_feature_name); > > + x86_register_cpuid_properties(obj, svm_feature_name); > > > > env->cpuid_apic_id = env->cpu_index; > > > > -- > > 1.7.11.2 > > > > > > -- > Eduardo >
On Thu, 16 Aug 2012 15:10:50 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Wed, Aug 15, 2012 at 06:13:27PM +0200, Igor Mammedov wrote: > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > -- > > v2: > > * replaced mask/ffs tricks by plain 'for (bit = 0; bit < 32; bit++)' > > as suggested by Eduardo Habkost > > --- > > target-i386/cpu.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 101 insertions(+) > > > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index 37ba5ef..440e724 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -606,6 +606,101 @@ static int check_features_against_host(x86_def_t *guest_def) > > return rv; > > } > > > > +static bool is_feature_set(const char *name, const uint32_t featbitmap, > > + const char **featureset) > > +{ > > + uint32_t bit; > > + > > + for (bit = 0; bit < 32; ++bit) { > > + if (featureset[bit] && !altcmp(name, NULL, featureset[bit])) { > > + if (featbitmap & (1 << bit)) { > > + return true; > > + } > > + } > > + } > > + return false; > > +} > > + > > +static void x86_cpuid_get_feature(Object *obj, Visitor *v, void *opaque, > > + const char *name, Error **errp) > > +{ > > + X86CPU *cpu = X86_CPU(obj); > > + CPUX86State *env = &cpu->env; > > + bool value = true; > > + > > + if (!is_feature_set(name, env->cpuid_features, feature_name) && > > + !is_feature_set(name, env->cpuid_ext_features, ext_feature_name) && > > + !is_feature_set(name, env->cpuid_ext2_features, ext2_feature_name) && > > + !is_feature_set(name, env->cpuid_ext3_features, ext3_feature_name) && > > + !is_feature_set(name, env->cpuid_kvm_features, kvm_feature_name) && > > + !is_feature_set(name, env->cpuid_svm_features, svm_feature_name)) { > > + value = false; > > + } > > + > > + visit_type_bool(v, &value, name, errp); > > +} > > + > > +static void x86_cpuid_set_feature(Object *obj, Visitor *v, void *opaque, > > + const char *name, Error **errp) > > +{ > > + X86CPU *cpu = X86_CPU(obj); > > + CPUX86State *env = &cpu->env; > > + uint32_t mask = 0; > > + uint32_t *dst_features; > > + bool value; > > + > > + visit_type_bool(v, &value, name, errp); > > + if (error_is_set(errp)) { > > + return; > > + } > > + > > + if (lookup_feature(&mask, name, NULL, feature_name)) { > > + dst_features = &env->cpuid_features; > > + } else if (lookup_feature(&mask, name, NULL, ext_feature_name)) { > > + dst_features = &env->cpuid_ext_features; > > + } else if (lookup_feature(&mask, name, NULL, ext2_feature_name)) { > > + dst_features = &env->cpuid_ext2_features; > > + } else if (lookup_feature(&mask, name, NULL, ext3_feature_name)) { > > + dst_features = &env->cpuid_ext3_features; > > + } else if (lookup_feature(&mask, name, NULL, kvm_feature_name)) { > > + dst_features = &env->cpuid_kvm_features; > > + } else if (lookup_feature(&mask, name, NULL, svm_feature_name)) { > > + dst_features = &env->cpuid_svm_features; > > + } else { > > + error_set(errp, QERR_PROPERTY_NOT_FOUND, "", name); > > + return; > > + } > > Some feature names are duplicated on feature_names and > ext_feature_names. On AMD CPU models, we have to set both, on Intel > models we need to set the bits only on cpuid_features. > > Maybe it's better to: > > 1) eliminate the duplication and set the names only on feature_name > array; > 2) At the end of CPU initialization, set the features on > cpuid_ext2_features as copies of the corresponding cpuid_features > bits if CPU vendor == AMD (or, maybe, if some boolean > "ext2_features_aliases" flag is set, to make it not > vendor-dependent). So far I was trying to keep original behavior and keep scope of this series only on moving CPU features into properties. I'll rebase this series on top of what you proposed in a week when I'm back from vacation. > > > + > > + if (value) { > > + *dst_features |= mask; > > + } else { > > + *dst_features &= ~mask; > > + } > > +} > > + > > +static void x86_register_cpuid_properties(Object *obj, const char **featureset) > > +{ > > + uint32_t bit; > > + > > + for (bit = 0; bit < 32; ++bit) { > > + if (featureset[bit]) { > > + char *feature_name, *save_ptr; > > + char buf[32]; > > + if (strlen(featureset[bit]) > sizeof(buf) - 1) { > > + abort(); > > + } > > + pstrcpy(buf, sizeof(buf), featureset[bit]); > > + feature_name = strtok_r(buf, "|", &save_ptr); > > + while (feature_name) { > > + object_property_add(obj, feature_name, "bool", > > + x86_cpuid_get_feature, > > + x86_cpuid_set_feature, NULL, NULL, NULL); > > + feature_name = strtok_r(NULL, "|", &save_ptr); > > What happens when object_property_add() is called twice with the same > feature name? Just extra item in list (pure waste). Thanks for pointing out. I've fixed it (not yet pushed) by checking if property already exists before adding it. > > > + } > > + } > > + } > > +} > > + > > static void x86_cpuid_version_get_family(Object *obj, Visitor *v, void *opaque, > > const char *name, Error **errp) > > { > > @@ -1824,6 +1919,12 @@ static void x86_cpu_initfn(Object *obj) > > object_property_add(obj, "tsc-frequency", "int", > > x86_cpuid_get_tsc_freq, > > x86_cpuid_set_tsc_freq, NULL, NULL, NULL); > > + x86_register_cpuid_properties(obj, feature_name); > > + x86_register_cpuid_properties(obj, ext_feature_name); > > + x86_register_cpuid_properties(obj, ext2_feature_name); > > + x86_register_cpuid_properties(obj, ext3_feature_name); > > + x86_register_cpuid_properties(obj, kvm_feature_name); > > + x86_register_cpuid_properties(obj, svm_feature_name); > > > > env->cpuid_apic_id = env->cpu_index; > > > > -- > > 1.7.11.2 > > > > > > -- > Eduardo >
On Mon, Aug 20, 2012 at 02:16:26PM +0200, Igor Mammedov wrote: > On Thu, 16 Aug 2012 15:10:50 -0300 > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > On Wed, Aug 15, 2012 at 06:13:27PM +0200, Igor Mammedov wrote: > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > -- > > > v2: > > > * replaced mask/ffs tricks by plain 'for (bit = 0; bit < 32; bit++)' > > > as suggested by Eduardo Habkost > > > --- > > > target-i386/cpu.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 101 insertions(+) > > > > > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > > index 37ba5ef..440e724 100644 > > > --- a/target-i386/cpu.c > > > +++ b/target-i386/cpu.c > > > @@ -606,6 +606,101 @@ static int check_features_against_host(x86_def_t *guest_def) > > > return rv; > > > } > > > > > > +static bool is_feature_set(const char *name, const uint32_t featbitmap, > > > + const char **featureset) > > > +{ > > > + uint32_t bit; > > > + > > > + for (bit = 0; bit < 32; ++bit) { > > > + if (featureset[bit] && !altcmp(name, NULL, featureset[bit])) { > > > + if (featbitmap & (1 << bit)) { > > > + return true; > > > + } > > > + } > > > + } > > > + return false; > > > +} > > > + > > > +static void x86_cpuid_get_feature(Object *obj, Visitor *v, void *opaque, > > > + const char *name, Error **errp) > > > +{ > > > + X86CPU *cpu = X86_CPU(obj); > > > + CPUX86State *env = &cpu->env; > > > + bool value = true; > > > + > > > + if (!is_feature_set(name, env->cpuid_features, feature_name) && > > > + !is_feature_set(name, env->cpuid_ext_features, ext_feature_name) && > > > + !is_feature_set(name, env->cpuid_ext2_features, ext2_feature_name) && > > > + !is_feature_set(name, env->cpuid_ext3_features, ext3_feature_name) && > > > + !is_feature_set(name, env->cpuid_kvm_features, kvm_feature_name) && > > > + !is_feature_set(name, env->cpuid_svm_features, svm_feature_name)) { > > > + value = false; > > > + } > > > + > > > + visit_type_bool(v, &value, name, errp); > > > +} > > > + > > > +static void x86_cpuid_set_feature(Object *obj, Visitor *v, void *opaque, > > > + const char *name, Error **errp) > > > +{ > > > + X86CPU *cpu = X86_CPU(obj); > > > + CPUX86State *env = &cpu->env; > > > + uint32_t mask = 0; > > > + uint32_t *dst_features; > > > + bool value; > > > + > > > + visit_type_bool(v, &value, name, errp); > > > + if (error_is_set(errp)) { > > > + return; > > > + } > > > + > > > + if (lookup_feature(&mask, name, NULL, feature_name)) { > > > + dst_features = &env->cpuid_features; > > > + } else if (lookup_feature(&mask, name, NULL, ext_feature_name)) { > > > + dst_features = &env->cpuid_ext_features; > > > + } else if (lookup_feature(&mask, name, NULL, ext2_feature_name)) { > > > + dst_features = &env->cpuid_ext2_features; > > > + } else if (lookup_feature(&mask, name, NULL, ext3_feature_name)) { > > > + dst_features = &env->cpuid_ext3_features; > > > + } else if (lookup_feature(&mask, name, NULL, kvm_feature_name)) { > > > + dst_features = &env->cpuid_kvm_features; > > > + } else if (lookup_feature(&mask, name, NULL, svm_feature_name)) { > > > + dst_features = &env->cpuid_svm_features; > > > + } else { > > > + error_set(errp, QERR_PROPERTY_NOT_FOUND, "", name); > > > + return; > > > + } > > > > Some feature names are duplicated on feature_names and > > ext_feature_names. On AMD CPU models, we have to set both, on Intel > > models we need to set the bits only on cpuid_features. > > > > Maybe it's better to: > > > > 1) eliminate the duplication and set the names only on feature_name > > array; > > 2) At the end of CPU initialization, set the features on > > cpuid_ext2_features as copies of the corresponding cpuid_features > > bits if CPU vendor == AMD (or, maybe, if some boolean > > "ext2_features_aliases" flag is set, to make it not > > vendor-dependent). > > So far I was trying to keep original behavior and keep scope of this series > only on moving CPU features into properties. I'll rebase this series on top > of what you proposed in a week when I'm back from vacation. Sure, keeping the behavior while changing the code to use properties makes perfect sense. The only problem is that the current behavior is incorrect and makes the property handling more complicated, so it makes sense to fix it first. > > > > > > + > > > + if (value) { > > > + *dst_features |= mask; > > > + } else { > > > + *dst_features &= ~mask; > > > + } > > > +} > > > + > > > +static void x86_register_cpuid_properties(Object *obj, const char **featureset) > > > +{ > > > + uint32_t bit; > > > + > > > + for (bit = 0; bit < 32; ++bit) { > > > + if (featureset[bit]) { > > > + char *feature_name, *save_ptr; > > > + char buf[32]; > > > + if (strlen(featureset[bit]) > sizeof(buf) - 1) { > > > + abort(); > > > + } > > > + pstrcpy(buf, sizeof(buf), featureset[bit]); > > > + feature_name = strtok_r(buf, "|", &save_ptr); > > > + while (feature_name) { > > > + object_property_add(obj, feature_name, "bool", > > > + x86_cpuid_get_feature, > > > + x86_cpuid_set_feature, NULL, NULL, NULL); > > > + feature_name = strtok_r(NULL, "|", &save_ptr); > > > > What happens when object_property_add() is called twice with the same > > feature name? > Just extra item in list (pure waste). Thanks for pointing out. > I've fixed it (not yet pushed) by checking if property already exists before > adding it. > > > > > > + } > > > + } > > > + } > > > +} > > > + > > > static void x86_cpuid_version_get_family(Object *obj, Visitor *v, void *opaque, > > > const char *name, Error **errp) > > > { > > > @@ -1824,6 +1919,12 @@ static void x86_cpu_initfn(Object *obj) > > > object_property_add(obj, "tsc-frequency", "int", > > > x86_cpuid_get_tsc_freq, > > > x86_cpuid_set_tsc_freq, NULL, NULL, NULL); > > > + x86_register_cpuid_properties(obj, feature_name); > > > + x86_register_cpuid_properties(obj, ext_feature_name); > > > + x86_register_cpuid_properties(obj, ext2_feature_name); > > > + x86_register_cpuid_properties(obj, ext3_feature_name); > > > + x86_register_cpuid_properties(obj, kvm_feature_name); > > > + x86_register_cpuid_properties(obj, svm_feature_name); > > > > > > env->cpuid_apic_id = env->cpu_index; > > > > > > -- > > > 1.7.11.2 > > > > > > > > > > -- > > Eduardo > > > > > -- > Regards, > Igor
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 37ba5ef..440e724 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -606,6 +606,101 @@ static int check_features_against_host(x86_def_t *guest_def) return rv; } +static bool is_feature_set(const char *name, const uint32_t featbitmap, + const char **featureset) +{ + uint32_t bit; + + for (bit = 0; bit < 32; ++bit) { + if (featureset[bit] && !altcmp(name, NULL, featureset[bit])) { + if (featbitmap & (1 << bit)) { + return true; + } + } + } + return false; +} + +static void x86_cpuid_get_feature(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + X86CPU *cpu = X86_CPU(obj); + CPUX86State *env = &cpu->env; + bool value = true; + + if (!is_feature_set(name, env->cpuid_features, feature_name) && + !is_feature_set(name, env->cpuid_ext_features, ext_feature_name) && + !is_feature_set(name, env->cpuid_ext2_features, ext2_feature_name) && + !is_feature_set(name, env->cpuid_ext3_features, ext3_feature_name) && + !is_feature_set(name, env->cpuid_kvm_features, kvm_feature_name) && + !is_feature_set(name, env->cpuid_svm_features, svm_feature_name)) { + value = false; + } + + visit_type_bool(v, &value, name, errp); +} + +static void x86_cpuid_set_feature(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + X86CPU *cpu = X86_CPU(obj); + CPUX86State *env = &cpu->env; + uint32_t mask = 0; + uint32_t *dst_features; + bool value; + + visit_type_bool(v, &value, name, errp); + if (error_is_set(errp)) { + return; + } + + if (lookup_feature(&mask, name, NULL, feature_name)) { + dst_features = &env->cpuid_features; + } else if (lookup_feature(&mask, name, NULL, ext_feature_name)) { + dst_features = &env->cpuid_ext_features; + } else if (lookup_feature(&mask, name, NULL, ext2_feature_name)) { + dst_features = &env->cpuid_ext2_features; + } else if (lookup_feature(&mask, name, NULL, ext3_feature_name)) { + dst_features = &env->cpuid_ext3_features; + } else if (lookup_feature(&mask, name, NULL, kvm_feature_name)) { + dst_features = &env->cpuid_kvm_features; + } else if (lookup_feature(&mask, name, NULL, svm_feature_name)) { + dst_features = &env->cpuid_svm_features; + } else { + error_set(errp, QERR_PROPERTY_NOT_FOUND, "", name); + return; + } + + if (value) { + *dst_features |= mask; + } else { + *dst_features &= ~mask; + } +} + +static void x86_register_cpuid_properties(Object *obj, const char **featureset) +{ + uint32_t bit; + + for (bit = 0; bit < 32; ++bit) { + if (featureset[bit]) { + char *feature_name, *save_ptr; + char buf[32]; + if (strlen(featureset[bit]) > sizeof(buf) - 1) { + abort(); + } + pstrcpy(buf, sizeof(buf), featureset[bit]); + feature_name = strtok_r(buf, "|", &save_ptr); + while (feature_name) { + object_property_add(obj, feature_name, "bool", + x86_cpuid_get_feature, + x86_cpuid_set_feature, NULL, NULL, NULL); + feature_name = strtok_r(NULL, "|", &save_ptr); + } + } + } +} + static void x86_cpuid_version_get_family(Object *obj, Visitor *v, void *opaque, const char *name, Error **errp) { @@ -1824,6 +1919,12 @@ static void x86_cpu_initfn(Object *obj) object_property_add(obj, "tsc-frequency", "int", x86_cpuid_get_tsc_freq, x86_cpuid_set_tsc_freq, NULL, NULL, NULL); + x86_register_cpuid_properties(obj, feature_name); + x86_register_cpuid_properties(obj, ext_feature_name); + x86_register_cpuid_properties(obj, ext2_feature_name); + x86_register_cpuid_properties(obj, ext3_feature_name); + x86_register_cpuid_properties(obj, kvm_feature_name); + x86_register_cpuid_properties(obj, svm_feature_name); env->cpuid_apic_id = env->cpu_index;
Signed-off-by: Igor Mammedov <imammedo@redhat.com> -- v2: * replaced mask/ffs tricks by plain 'for (bit = 0; bit < 32; bit++)' as suggested by Eduardo Habkost --- target-i386/cpu.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+)