[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH qom-cpu 4/4] target-i386: Replace cpuid_*feature
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH qom-cpu 4/4] target-i386: Replace cpuid_*features fields with a feature word array |
Date: |
Tue, 16 Apr 2013 20:52:29 +0200 |
On Tue, 16 Apr 2013 13:12:04 -0300
Eduardo Habkost <address@hidden> wrote:
> This replaces the feature-bit fields on both X86CPU and x86_def_t
> structs with an array.
>
> With this, we will be able to simplify code that simply does the same
> operation on all feature words (e.g. kvm_check_features_against_host(),
> filter_features_for_kvm(), add_flagname_to_bitmaps(), CPU feature-bit
> property lookup/registration, and the proposed "feature-words" property)
>
> The following field replacements were made on X86CPU and x86_def_t:
>
> (cpuid_)features -> features[FEAT_1_EDX]
> (cpuid_)ext_features -> features[FEAT_1_ECX]
> (cpuid_)ext2_features -> features[FEAT_8000_0001_EDX]
> (cpuid_)ext3_features -> features[FEAT_8000_0001_ECX]
> (cpuid_)ext4_features -> features[FEAT_C000_0001_EDX]
> (cpuid_)kvm_features -> features[FEAT_KVM]
> (cpuid_)svm_features -> features[FEAT_SVM]
> (cpuid_)7_0_ebx_features -> features[FEAT_7_0_EBX]
>
> Signed-off-by: Eduardo Habkost <address@hidden>
> ---
> This should help us avoid bugs like the ones introduced when we added
> cpuid_7_0_ebx_features. Today, adding a new feature word to the code requires
> chaning 5 or 6 different places in the code, and it's very easy to miss a
> problem when we forget to update one of those parts. See, for example:
>
> * The bug solved by commit ffa8c11f0bbf47e1b7a3a62f97bc1da591c6734a;
> (CPUID 7 leaf was not being filtered based on host capabilities)
> * The bug solved by commit 07ca59450c9a0c5df65665ce46aa8487af59a1dd
> (check/enforce flags were not checking all feature flags)
>
> This patch was created solely using a sed script and no manual changes,
> to try to avoid mistakes while converting the code, and make it easier
> to rebase if necessary. The sed script can be seen at:
> https://gist.github.com/4271991
> The script is a lot simpler now, it is basically 8 s/.../.../ commands
> and a few insert/delete commands to change the cpu.h and cpu.c structs.
>
> Changes v9:
> * commit message changes: move some text below "---", add table
> showing how each field was replaced
>
> Changes v8:
> * Move line-breaking and code-formatting changes to separate patches
> * Rebase on top of qom-cpu commit b4d31f73
> (qdev: Set device's parent before calling realize() down inheritance chain)
>
> Changes v7:
> * Rebase on top qom-cpu-next
> (commit 3755f0a9d48da07258f4a0ef5e883272799e47b9)
>
> Changes v7:
> * Rebase on top of Andreas' qom-cpu tree (commit
> 9260944307077b93a66bf861a467107af986fe47)
> * Break lines on kvm_check_features_against_host()
> * Break the lines on builtin_x86_defs just after the "=".
> This way the feature lists stay on separate lines, this patch gets
> easier to review, and future patches that touches the code around
> builtin_x86_defs will be even easier to review (as they won't need
> to touch the lines containing the fature lists again)
>
> Signed-off-by: Eduardo Habkost <address@hidden>
>
> Conflicts:
> target-i386/kvm.c
> ---
> bsd-user/elfload.c | 2 +-
> bsd-user/main.c | 4 +-
> hw/i386/kvm/clock.c | 2 +-
> linux-user/elfload.c | 2 +-
> linux-user/main.c | 4 +-
> target-i386/cpu.c | 329
> +++++++++++++++++++++++-----------------------
> target-i386/cpu.h | 11 +-
> target-i386/helper.c | 4 +-
> target-i386/kvm.c | 4 +-
> target-i386/misc_helper.c | 14 +-
> target-i386/translate.c | 10 +-
> 11 files changed, 186 insertions(+), 200 deletions(-)
>
> diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
> index a6cd3ab..44e1568 100644
> --- a/bsd-user/elfload.c
> +++ b/bsd-user/elfload.c
> @@ -110,7 +110,7 @@ static const char *get_elf_platform(void)
>
> static uint32_t get_elf_hwcap(void)
> {
> - return thread_env->cpuid_features;
> + return thread_env->features[FEAT_1_EDX];
wrong ident, since line is touched it should be fixed.
> }
>
[...]
--
Regards,
Igor
- [Qemu-devel] [PATCH qom-cpu 0/4] replace cpuid_*features fields with a featue word array (v9), Eduardo Habkost, 2013/04/16
- [Qemu-devel] [PATCH qom-cpu 1/4] target-i386: cleanup: Group together level, xlevel, xlevel2 fields, Eduardo Habkost, 2013/04/16
- [Qemu-devel] [PATCH qom-cpu 2/4] target-i386/kvm.c: Code formatting changes, Eduardo Habkost, 2013/04/16
- [Qemu-devel] [PATCH qom-cpu 3/4] target-i386/cpu.c: Break lines so they don't get too long, Eduardo Habkost, 2013/04/16
- [Qemu-devel] [PATCH qom-cpu 4/4] target-i386: Replace cpuid_*features fields with a feature word array, Eduardo Habkost, 2013/04/16
- Re: [Qemu-devel] [PATCH qom-cpu 4/4] target-i386: Replace cpuid_*features fields with a feature word array,
Igor Mammedov <=
- Re: [Qemu-devel] [PATCH qom-cpu 0/4] replace cpuid_*features fields with a featue word array (v9), Igor Mammedov, 2013/04/16