Please CC me on your future submissions, thanks.

On Fri, Nov 04, 2016 at 03:07:19PM +0800, He Chen wrote:
> The spec can be found in Intel Software Developer Manual or in
> Instruction Set Extensions Programming Reference.

This commit message is completely useless. Write commit messages in
the way as if you're explaining to another person *why* this change is
needed and that other person doesn't have an idea what you're doing.

> Signed-off-by: Luwei Kang <luwei.k...@intel.com>
> Signed-off-by: He Chen <he.c...@linux.intel.com>

This SOB chain means what exactly?

> ---
> Changes in v3:
> * add a helper in scattered.c to get scattered leaf.

The modification to scattered et al without the kvm use should be a
separate patch.

>   * Capabilities of Intel PT hardware, such as number of address bits or
>   * supported output schemes, are cached and exported to userspace as "caps"
> diff --git a/arch/x86/include/asm/processor.h 
> b/arch/x86/include/asm/processor.h
> index 984a7bf..47978b7 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -137,6 +137,13 @@ struct cpuinfo_x86 {
>       u32                     microcode;
>  };
> 
> +enum cpuid_regs_idx {

cpuid_regs was just fine.

> +     CR_EAX = 0,
> +     CR_ECX,
> +     CR_EDX,
> +     CR_EBX
> +};
> +
>  #define X86_VENDOR_INTEL     0
>  #define X86_VENDOR_CYRIX     1
>  #define X86_VENDOR_AMD               2
> @@ -178,6 +185,8 @@ extern void identify_secondary_cpu(struct cpuinfo_x86 *);
>  extern void print_cpu_info(struct cpuinfo_x86 *);
>  void print_cpu_msr(struct cpuinfo_x86 *);
>  extern void init_scattered_cpuid_features(struct cpuinfo_x86 *c);
> +extern u32 get_scattered_cpuid_leaf(unsigned int level,
> +             unsigned int sub_leaf, enum cpuid_regs_idx reg);

Align arguments on the opening brace like this:

extern u32
get_scattered_cpuid_leaf(unsigned int level, unsigned int sub_leaf,
                         enum cpuid_regs_idx reg);


>  extern unsigned int init_intel_cacheinfo(struct cpuinfo_x86 *c);
>  extern void init_amd_cacheinfo(struct cpuinfo_x86 *c);
> 
> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
> index 1db8dc4..ca3c605 100644
> --- a/arch/x86/kernel/cpu/scattered.c
> +++ b/arch/x86/kernel/cpu/scattered.c
> @@ -17,11 +17,17 @@ struct cpuid_bit {
>       u32 sub_leaf;
>  };
> 
> -enum cpuid_regs {
> -     CR_EAX = 0,
> -     CR_ECX,
> -     CR_EDX,
> -     CR_EBX
> +/* Please keep the leaf sorted. */

                                ... by cpuid_bit.level for faster search. */

> +static const struct cpuid_bit cpuid_bits[] = {
> +     { X86_FEATURE_APERFMPERF,       CR_ECX,  0, 0x00000006, 0 },
> +     { X86_FEATURE_EPB,              CR_ECX,  3, 0x00000006, 0 },
> +     { X86_FEATURE_INTEL_PT,         CR_EBX, 25, 0x00000007, 0 },
> +     { X86_FEATURE_AVX512_4VNNIW,    CR_EDX,  2, 0x00000007, 0 },
> +     { X86_FEATURE_AVX512_4FMAPS,    CR_EDX,  3, 0x00000007, 0 },
> +     { X86_FEATURE_HW_PSTATE,        CR_EDX,  7, 0x80000007, 0 },
> +     { X86_FEATURE_CPB,              CR_EDX,  9, 0x80000007, 0 },
> +     { X86_FEATURE_PROC_FEEDBACK,    CR_EDX, 11, 0x80000007, 0 },
> +     { 0, 0, 0, 0, 0 }
>  };

...

> @@ -57,3 +51,27 @@ void init_scattered_cpuid_features(struct cpuinfo_x86 *c)
>                       set_cpu_cap(c, cb->feature);
>       }
>  }
> +
> +u32 get_scattered_cpuid_leaf(unsigned int level, unsigned int sub_leaf,
> +                          enum cpuid_regs_idx reg)

Align arguments on the opening brace.

> +{
> +     u32 cpuid_val = 0;
> +     const struct cpuid_bit *cb;


Please sort function local variables declaration in a reverse christmas
tree order:

        <type> longest_variable_name;
        <type> shorter_var_name;
        <type> even_shorter;
        <type> i;

> +
> +     for (cb = cpuid_bits; cb->feature; cb++) {
> +
> +             if (level > cb->level)
> +                     continue;
> +
> +             if (level < cb->level)
> +                     break;
> +
> +             if (reg == cb->reg && sub_leaf == cb->sub_leaf) {
> +                     if (cpu_has(&boot_cpu_data, cb->feature))
> +                             cpuid_val |= BIT(cb->bit);
> +             }
> +     }
> +
> +     return cpuid_val;
> +}

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Reply via email to