[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/6] target-i386: sanitize AMD's ext2_features a
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH 2/6] target-i386: sanitize AMD's ext2_features at realize time |
Date: |
Tue, 11 Dec 2012 11:31:45 -0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Tue, Dec 11, 2012 at 11:11:02AM +0100, Igor Mammedov wrote:
> when CPU properties are implemented, ext2_features may change
> between object_new(CPU) and cpu_realize_fn(). Sanitizing
> ext2_features for AMD based CPU at realize() time will keep
> current behavior after CPU features are converted to properties.
>
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
> v2:
> - style fix, make line shorter than 80 characters
>
> amd san
Incomplete sentence?
> ---
> target-i386/cpu.c | 21 +++++++++++----------
> 1 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 63aae86..64b7637 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1539,16 +1539,6 @@ int cpu_x86_register(X86CPU *cpu, const char
> *cpu_model)
> object_property_set_int(OBJECT(cpu), (int64_t)def->tsc_khz * 1000,
> "tsc-frequency", &error);
>
> - /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
> - * CPUID[1].EDX.
> - */
> - if (env->cpuid_vendor1 == CPUID_VENDOR_AMD_1 &&
> - env->cpuid_vendor2 == CPUID_VENDOR_AMD_2 &&
> - env->cpuid_vendor3 == CPUID_VENDOR_AMD_3) {
> - env->cpuid_ext2_features &= ~CPUID_EXT2_AMD_ALIASES;
> - env->cpuid_ext2_features |= (def->features & CPUID_EXT2_AMD_ALIASES);
> - }
> -
> object_property_set_str(OBJECT(cpu), def->model_id, "model-id", &error);
> if (error) {
> fprintf(stderr, "%s\n", error_get_pretty(error));
> @@ -2062,6 +2052,17 @@ void x86_cpu_realize(Object *obj, Error **errp)
> env->cpuid_level = 7;
> }
>
> + /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
> + * CPUID[1].EDX.
> + */
> + if (env->cpuid_vendor1 == CPUID_VENDOR_AMD_1 &&
> + env->cpuid_vendor2 == CPUID_VENDOR_AMD_2 &&
> + env->cpuid_vendor3 == CPUID_VENDOR_AMD_3) {
I would add extra indentation space here, to make it not align with the
statements below, making the condition visually distinct from the body,
like in the original code you are moving.
> + env->cpuid_ext2_features &= ~CPUID_EXT2_AMD_ALIASES;
> + env->cpuid_ext2_features |= (env->cpuid_features
> + & CPUID_EXT2_AMD_ALIASES);
Weird spacing around the "&" above (3 spaces indent, 2 spaces after the
"&").
I would align this as:
env->cpuid_ext2_features |= (env->cpuid_features &
CPUID_EXT2_AMD_ALIASES);
As the above issues are only cosmetic:
Reviewed-by: Eduardo Habkost <address@hidden>
> + }
> +
> if (!kvm_enabled()) {
> env->cpuid_features &= TCG_FEATURES;
> env->cpuid_ext_features &= TCG_EXT_FEATURES;
> --
> 1.7.1
>
--
Eduardo
- Re: [Qemu-devel] [PATCH 3/6] target-i386: explicitly set vendor for each built-in cpudef, (continued)
- [Qemu-devel] [PATCH 1/6] target-i386: filter out not TCG features if running without kvm at realize time, Igor Mammedov, 2012/12/11
- [Qemu-devel] [PATCH 4/6] target-i386: setting default 'vendor' is obsolete, remove it, Igor Mammedov, 2012/12/11
- [Qemu-devel] [PATCH 6/6] target-i386: move out CPU features initialization in separate func, Igor Mammedov, 2012/12/11
- [Qemu-devel] [PATCH 5/6] target-i386: move setting defaults out of cpu_x86_parse_featurestr(), Igor Mammedov, 2012/12/11
- [Qemu-devel] [PATCH 2/6] target-i386: sanitize AMD's ext2_features at realize time, Igor Mammedov, 2012/12/11