lkml.org 
[lkml]   [2009]   [Mar]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [git-pull -tip] x86: msr architecture debug code

    * Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:

    > The following changes since commit 1d10914bf2c8a1164aef6c341e6c3518a91b8374:
    > Ingo Molnar (1):
    > Merge branch 'core/percpu'
    >
    > are available in the git repository at:
    >
    > git://git.kernel.org/pub/scm/linux/kernel/git/jaswinder/linux-2.6-tip-cpu.git master
    >
    > Jaswinder Singh Rajput (1):
    > x86: msr architecture debug code
    >
    > arch/x86/include/asm/msr_debug.h | 56 ++++++
    > arch/x86/kernel/cpu/Makefile | 2 +-
    > arch/x86/kernel/cpu/msr_debug.c | 362 ++++++++++++++++++++++++++++++++++++++
    > 3 files changed, 419 insertions(+), 1 deletions(-)
    > create mode 100755 arch/x86/include/asm/msr_debug.h
    > create mode 100755 arch/x86/kernel/cpu/msr_debug.c
    >
    > Complete diff:
    > diff --git a/arch/x86/include/asm/msr_debug.h b/arch/x86/include/asm/msr_debug.h
    > new file mode 100755
    > index 0000000..ddc4fe5
    > --- /dev/null
    > +++ b/arch/x86/include/asm/msr_debug.h
    > @@ -0,0 +1,56 @@
    > +#ifndef _ASM_X86_MSR_DEBUG_H
    > +#define _ASM_X86_MSR_DEBUG_H
    > +
    > +/*
    > + * Model Specific Registers (MSR) x86 architecture debug
    > + *
    > + * Copyright(C) 2009 Jaswinder Singh Rajput
    > + */
    > +
    > +#define MSR_ALL 0
    > +#define MSR_PMC 1 /* Performance Monitor counters */
    > +
    > +struct msr_debug_range {
    > + unsigned min;
    > + unsigned max;
    > + unsigned model;
    > +};

    Please vertical-align structure field definitions, like you see
    we do it elsewhere in the x86 code.

    > +
    > +/* Intel MSRs Range */
    > +
    > +/* DisplayFamily_DisplayModel Processor Families/Processor Number Series */
    > +/* -------------------------- ------------------------------------------ */
    > +/* 05_01, 05_02, 05_04 Pentium, Pentium with MMX */
    > +#define MSR_INTEL_PENTIUM (1 << 0)
    > +
    > +/* 06_01 Pentium Pro */
    > +/* 06_03, 06_05 Pentium II Xeon, Pentium II */
    > +/* 06_07, 06_08, 06_0A, 06_0B Pentium III Xeon, Pentum III */
    > +#define MSR_INTEL_P6 (1 << 1)
    > +
    > +/* 06_09, 060D Pentium M */
    > +#define MSR_INTEL_PENTIUM_M (1 << 4)
    > +
    > +/* 06_0E Core Duo, Core Solo */
    > +#define MSR_INTEL_CORE (1 << 5)
    > +
    > +/* 06_0F Xeon 3000, 3200, 5100, 5300, 7300 series,
    > + Core 2 Quad, Core 2 Extreme, Core 2 Duo,
    > + Pentium dual-core */
    > +/* 06_17 Xeon 5200, 5400 series, Core 2 Quad Q9650 */
    > +#define MSR_INTEL_CORE_2 (1 << 6)
    > +
    > +/* 06_1C Atom */
    > +#define MSR_INTEL_ATOM (1 << 7)
    > +
    > +/* 0F_00, 0F_01, 0F_02 Xeon, Xeon MP, Pentium 4 */
    > +/* 0F_03, 0F_04 Xeon, Xeon MP, Pentium 4, Pentium D */
    > +#define MSR_INTEL_XEON (1 << 17)
    > +
    > +/* 0F_06 Xeon 7100, 5000 Series, Xeon MP,
    > + Pentium 4, Pentium D */
    > +#define MSR_INTEL_XEON_MP (1 << 18)
    > +
    > +#define MSR_CPU_ALL (~0)
    > +
    > +#endif /* _ASM_X86_MSR_DEBUG_H */
    > diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
    > index c381330..b2452f3 100644
    > --- a/arch/x86/kernel/cpu/Makefile
    > +++ b/arch/x86/kernel/cpu/Makefile
    > @@ -8,7 +8,7 @@ CFLAGS_REMOVE_common.o = -pg
    > endif
    >
    > obj-y := intel_cacheinfo.o addon_cpuid_features.o
    > -obj-y += proc.o capflags.o powerflags.o common.o
    > +obj-y += proc.o capflags.o powerflags.o common.o msr_debug.o
    > obj-y += vmware.o hypervisor.o
    >
    > obj-$(CONFIG_X86_32) += bugs.o cmpxchg.o
    > diff --git a/arch/x86/kernel/cpu/msr_debug.c b/arch/x86/kernel/cpu/msr_debug.c
    > new file mode 100755
    > index 0000000..505f53f
    > --- /dev/null
    > +++ b/arch/x86/kernel/cpu/msr_debug.c
    > @@ -0,0 +1,362 @@
    > +/*
    > + * Model Specific Registers (MSR) x86 architecture debug code
    > + *
    > + * Copyright(C) 2009 Jaswinder Singh Rajput
    > + *
    > + * For licencing details see kernel-base/COPYING
    > + */
    > +
    > +#include <linux/module.h>
    > +#include <linux/kernel.h>
    > +#include <linux/debugfs.h>
    > +#include <linux/seq_file.h>
    > +#include <asm/msr_debug.h>

    Please have a good look at what the standard include files
    section look like - for example in
    arch/x86/kernel/cpu/perf_counter.c or in arch/x86/mm/fault.c.
    Please use that same ordering of the lines here too.

    > +static struct msr_debug_range msr_intel_range[] = {
    > + { 0x00000000, 0x000006d0, MSR_CPU_ALL },
    > + { 0x000107CC, 0x000107D4, MSR_INTEL_XEON_MP },
    > +};
    > +
    > +static struct msr_debug_range pmc_intel_range[] = {
    > + { 0x00000010, 0x00000011, MSR_CPU_ALL }, /* TSC */
    > + { 0x00000011, 0x00000014, MSR_INTEL_PENTIUM }, /* CESR, CTR */
    > + { 0x0000001B, 0x0000001C, MSR_CPU_ALL }, /* APIC */
    > + { 0x000000C1, 0x000000C3, MSR_INTEL_P6 }, /* PerfCtr */
    > + { 0x00000186, 0x00000188, MSR_INTEL_P6 | MSR_INTEL_ATOM |
    > + MSR_INTEL_CORE_2 }, /* EvtSel */
    > + { 0x000001A0, 0x000001A1, MSR_CPU_ALL }, /* Misc Feature */
    > + { 0x00000300, 0x00000312, MSR_INTEL_XEON }, /* Counter */
    > + { 0x00000309, 0x0000030C, MSR_INTEL_P6 | MSR_INTEL_ATOM |
    > + MSR_INTEL_CORE_2 }, /* Fixed */
    > + { 0x00000360, 0x00000372, MSR_INTEL_XEON }, /* CCCR */
    > + { 0x0000038D, 0x00000391, MSR_INTEL_P6 | MSR_INTEL_ATOM |
    > + MSR_INTEL_CORE_2 }, /* Fixed & Global */
    > + { 0x00000390, 0x00000391, MSR_INTEL_P6 | MSR_INTEL_ATOM |
    > + MSR_INTEL_CORE_2 }, /* OVF */
    > + { 0x000003A0, 0x000003F3, MSR_INTEL_XEON }, /* ESCR */
    > + { 0x000003F1, 0x000003F2, MSR_INTEL_P6 | MSR_INTEL_ATOM |
    > + MSR_INTEL_CORE_2 }, /* PEBS */
    > + { 0x000107CC, 0x000107D4, MSR_INTEL_XEON_MP },
    > +};
    > +
    > +/* AMD MSRs Range */
    > +static struct msr_debug_range msr_amd_range[] = {
    > + { 0x00000000, 0x00000418, MSR_CPU_ALL},
    > + { 0xc0000000, 0xc000040b, MSR_CPU_ALL},
    > + { 0xc0010000, 0xc0010142, MSR_CPU_ALL},
    > + { 0xc0011000, 0xc001103b, MSR_CPU_ALL},
    > +};
    > +
    > +static struct msr_debug_range pmc_amd_range[] = {
    > + { 0x00000010, 0x00000011, MSR_CPU_ALL }, /* TSC */
    > + { 0x0000001B, 0x0000001C, MSR_CPU_ALL }, /* APIC */
    > + { 0xc0010000, 0xc0010008, MSR_CPU_ALL},
    > +};
    > +
    > +
    > +static int get_msr_intel_bit(unsigned model)
    > +{
    > + int ret = 0;
    > +
    > + switch (model) {
    > + case 0x0501:
    > + case 0x0502:
    > + case 0x0504:
    > + ret = MSR_INTEL_PENTIUM;
    > + break;
    > + case 0x0601:
    > + case 0x0603:
    > + case 0x0605:
    > + case 0x0607:
    > + case 0x0608:
    > + case 0x060A:
    > + case 0x060B:
    > + ret = MSR_INTEL_P6;
    > + break;
    > + case 0x0609:
    > + case 0x060D:
    > + ret = MSR_INTEL_PENTIUM_M;
    > + break;
    > + case 0x060E:
    > + ret = MSR_INTEL_CORE;
    > + break;
    > + case 0x060F:
    > + case 0x0617:
    > + ret = MSR_INTEL_CORE_2;
    > + break;
    > + case 0x061C:
    > + ret = MSR_INTEL_ATOM;
    > + break;
    > + case 0x0F00:
    > + case 0x0F01:
    > + case 0x0F02:
    > + case 0x0F03:
    > + case 0x0F04:
    > + ret = MSR_INTEL_XEON;
    > + break;
    > + case 0x0F06:
    > + ret = MSR_INTEL_XEON_MP;
    > + break;
    > + }

    all these magic constants open-coded look a bit ugly. Can it be
    done cleaner?

    > +
    > + return ret;
    > +}
    > +
    > +static int get_msr_cpu_bit(unsigned model)
    > +{
    > + unsigned vendor;
    > +
    > + vendor = model >> 16;
    > + if (vendor == X86_VENDOR_INTEL)
    > + return get_msr_intel_bit(model & 0xffff);
    > + else
    > + return 0;
    > +}
    > +
    > +static int get_msr_range(unsigned *min, unsigned *max, int index,
    > + unsigned flag, unsigned model)
    > +{
    > + unsigned vendor, cpu_bit;
    > + int err = 0;
    > +
    > + vendor = model >> 16;
    > + cpu_bit = get_msr_cpu_bit(model);
    > + switch (flag) {
    > + case MSR_ALL:
    > + if (vendor == 0) {
    > + if (msr_intel_range[index].model & cpu_bit) {
    > + *min = msr_intel_range[index].min;
    > + *max = msr_intel_range[index].max;
    > + } else
    > + err = -EINVAL;
    > + } else if (vendor == X86_VENDOR_AMD) {
    > + *min = msr_amd_range[index].min;
    > + *max = msr_amd_range[index].max;
    > + }
    > + break;
    > + case MSR_PMC:
    > + if (vendor == 0) {
    > + if (pmc_intel_range[index].model & cpu_bit) {
    > + *min = pmc_intel_range[index].min;
    > + *max = pmc_intel_range[index].max;
    > + } else
    > + err = -EINVAL;
    > + } else if (vendor == X86_VENDOR_AMD) {
    > + *min = pmc_amd_range[index].min;
    > + *max = pmc_amd_range[index].max;
    > + } else
    > + err = -EINVAL;
    > + break;
    > + default:
    > + err = -ENOSYS;
    > + }
    > +
    > + return err;
    > +}
    > +
    > +static int get_msr_range_count(unsigned flag, unsigned model)
    > +{
    > + int index = 0;
    > +
    > + model >>= 16;
    > + switch (flag) {
    > + case MSR_ALL:
    > + if (model == X86_VENDOR_INTEL)
    > + index = ARRAY_SIZE(msr_intel_range);
    > + else if (model == X86_VENDOR_AMD)
    > + index = ARRAY_SIZE(msr_amd_range);
    > + break;
    > + case MSR_PMC:
    > + if (model == X86_VENDOR_INTEL)
    > + index = ARRAY_SIZE(pmc_intel_range);
    > + else if (model == X86_VENDOR_AMD)
    > + index = ARRAY_SIZE(pmc_amd_range);
    > + break;
    > + }
    > +
    > + return index;
    > +}
    > +
    > +static void print_intel_msr(struct seq_file *seq, unsigned int cpu,
    > + unsigned flag, unsigned model)
    > +{
    > + int i, range;
    > + u32 low, high;
    > + unsigned msr, msr_min, msr_max;
    > +
    > + range = get_msr_range_count(flag, model);
    > + seq_printf(seq, "processor: %u model 0x%x\n", cpu, model);
    > +
    > + for (i = 0; i < range; i++) {
    > + if (get_msr_range(&msr_min, &msr_max, i, flag, model))
    > + continue;
    > + for (msr = msr_min; msr < msr_max; msr++) {
    > + if (rdmsr_safe_on_cpu(cpu, msr, &low, &high))
    > + continue;
    > + if (seq)
    > + seq_printf(seq, " MSR_%08x: %08x_%08x\n",
    > + msr, high, low);
    > + else
    > + printk(KERN_INFO " MSR_%08x: %08x_%08x\n",
    > + msr, high, low);
    > + }
    > + }
    > +}
    > +
    > +static void print_amd_msr(struct seq_file *seq, unsigned flag, unsigned model)
    > +{
    > + int i, range;
    > + u64 val;
    > + unsigned msr, msr_min, msr_max;
    > +
    > + range = get_msr_range_count(flag, model);
    > + seq_printf(seq, "processor: 0 model 0x%x\n", model);
    > +
    > + for (i = 0; i < range; i++) {
    > + if (get_msr_range(&msr_min, &msr_max, i, flag, model))
    > + continue;
    > + for (msr = msr_min; msr < msr_max; msr++) {
    > + if (rdmsrl_amd_safe(msr, &val))
    > + continue;
    > + if (seq)
    > + seq_printf(seq, " MSR_%08x: %016llx\n",
    > + msr, val);
    > + else
    > + printk(KERN_INFO " MSR_%08x: %016llx\n",
    > + msr, val);
    > + }
    > + }
    > +}
    > +
    > +static int msr_basic_show(struct seq_file *seq, void *v, unsigned flag)
    > +{
    > + unsigned model;
    > + struct cpuinfo_x86 *c = v;
    > +
    > +#ifdef CONFIG_SMP
    > + /* We are only interested for core_id 0 */
    > + if (c->cpu_core_id)
    > + return 0;
    > +#endif
    > + model = ((c->x86_vendor << 16) | (c->x86 << 8) | (c->x86_model));
    > + if ((c->x86_max_cores * smp_num_siblings > 1) &&
    > + (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL))
    > + print_intel_msr(seq, c->phys_proc_id, flag, model);
    > + else
    > + print_amd_msr(seq, flag, model);
    > +
    > +
    > + seq_printf(seq, "\n");
    > +
    > + return 0;
    > +}
    > +
    > +static int msr_seq_show(struct seq_file *seq, void *v)
    > +{
    > + return msr_basic_show(seq, v, MSR_ALL);
    > +}
    > +
    > +static int pmc_seq_show(struct seq_file *seq, void *v)
    > +{
    > + return msr_basic_show(seq, v, MSR_PMC);
    > +}
    > +
    > +static void *msr_seq_start(struct seq_file *seq, loff_t *pos)
    > +{
    > + struct cpuinfo_x86 *c = NULL;
    > +
    > + if (*pos == 0) /* just in case, cpu 0 is not the first */
    > + *pos = first_cpu(cpu_online_map);
    > + else
    > + *pos = next_cpu_nr(*pos - 1, cpu_online_map);
    > +
    > + if ((*pos) < nr_cpu_ids)
    > + c = &cpu_data(*pos);
    > +
    > + return c;
    > +}
    > +
    > +static void *msr_seq_next(struct seq_file *seq, void *v, loff_t *pos)
    > +{
    > + (*pos)++;
    > +
    > + return msr_seq_start(seq, pos);
    > +}
    > +
    > +static void msr_seq_stop(struct seq_file *seq, void *v)
    > +{
    > +}
    > +
    > +static const struct seq_operations msr_seq_ops = {
    > + .start = msr_seq_start,
    > + .next = msr_seq_next,
    > + .stop = msr_seq_stop,
    > + .show = msr_seq_show,
    > +};
    > +
    > +static int msr_seq_open(struct inode *inode, struct file *file)
    > +{
    > + return seq_open(file, &msr_seq_ops);
    > +}
    > +
    > +static const struct file_operations msr_fops = {
    > + .open = msr_seq_open,
    > + .read = seq_read,
    > + .llseek = seq_lseek,
    > + .release = seq_release,
    > +};
    > +
    > +/* Performance monitioring MSRs */
    > +static const struct seq_operations pmc_seq_ops = {
    > + .start = msr_seq_start,
    > + .next = msr_seq_next,
    > + .stop = msr_seq_stop,
    > + .show = pmc_seq_show,
    > +};
    > +
    > +static int pmc_seq_open(struct inode *inode, struct file *file)
    > +{
    > + return seq_open(file, &pmc_seq_ops);
    > +}
    > +
    > +static const struct file_operations pmc_fops = {
    > + .open = pmc_seq_open,
    > + .read = seq_read,
    > + .llseek = seq_lseek,
    > + .release = seq_release,
    > +};

    Check how structure initializations are done in other places of
    the x86 tree - for example perf_counters.c. Apply that style
    here too please.

    > +
    > +static struct dentry *msr_file, *pmc_file, *msr_dir;
    > +static int __init msr_debug_init(void)

    missing newline after static variables.

    > +{
    > + struct cpuinfo_x86 *cpu = &cpu_data(0);
    > +
    > + if (!cpu_has(cpu, X86_FEATURE_MSR))
    > + return -ENODEV;
    > +
    > + msr_dir = debugfs_create_dir("msr", arch_debugfs_dir);
    > +
    > + msr_file = debugfs_create_file("msr", S_IRUGO, msr_dir,
    > + NULL, &msr_fops);
    > + pmc_file = debugfs_create_file("pmc", S_IRUGO, msr_dir,
    > + NULL, &pmc_fops);

    I think it would be possible to have a much more intuitive file
    layout under /debug/x86/msr/ than these two /debug/x86/msr/msr
    and /debug/x86/msr/pmc files.

    Firstly, it should move one level deeper, to /debug/x86/cpu/msr/
    - because the MSR is really a property of the CPU, and there are
    other properties of the CPU we might want to expose in the
    future.

    Secondly, the picking of debugfs (as opposed to sysfs) is a good
    choice, because we probably want to tweak the layout a number of
    times and want to keep flexibility, without being limited by the
    sysfs ABI.

    So i like the idea - but we really want to do even more and add
    more structure to this. If we just want dumb msr enumeration we
    already have /dev/msr.

    Regarding the msr directory: one good approach would be to have
    have several "topic" directories under /debug/x86/cpu/msr/.

    One such topic would be the 'pmu', with a structure like:

    /debug/x86/cpu/msr/pmu/
    /debug/x86/cpu/msr/pmu/pmc_0/
    /debug/x86/cpu/msr/pmu/pmc_0/counter
    /debug/x86/cpu/msr/pmu/pmc_0/eventsel

    There would also be a /debug/x86/cpu/msr/raw/ directory with all
    MSR numbers we know about explicitly, for example:

    /debug/x86/cpu/msr/raw/0x372/value
    /debug/x86/cpu/msr/raw/0x372/width

    Maybe a symlink pointing it back to the topic directory would be
    useful as well. For example:

    /debug/x86/cpu/msr/raw/0x372/topic_dir -> /debug/x86/cpu/msr/pmu/pmc_0/

    Other "topic directories" are possible too: a
    /debug/x86/cpu/msr/apic/ layout would be very useful and
    informative as well, and so are some of the other MSRs we tweak
    during bootup.

    Ingo


    \
     
     \ /
      Last update: 2009-03-02 21:57    [W:0.055 / U:0.932 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site