Subject: [patch 4/8] 2.6.22-rc3 perfmon2 : IBS implementation for AMD64

This patch rearranges AMD64 MSR definitions.

Signed-off-by: Robert Richter <[email protected]>

Index: linux-2.6.22-rc3/include/asm-i386/msr-index.h
===================================================================
--- linux-2.6.22-rc3.orig/include/asm-i386/msr-index.h
+++ linux-2.6.22-rc3/include/asm-i386/msr-index.h
@@ -75,6 +75,18 @@

/* K7/K8 MSRs. Not complete. See the architecture manual for a more
complete list. */
+
+/* K8 MSRs */
+#define MSR_K8_TOP_MEM1 0xC001001A
+#define MSR_K8_TOP_MEM2 0xC001001D
+#define MSR_K8_SYSCFG 0xC0010010
+#define MSR_K8_HWCR 0xC0010015
+#define MSR_K8_ENABLE_C1E 0xc0010055
+#define K8_MTRRFIXRANGE_DRAM_ENABLE 0x00040000 /* MtrrFixDramEn bit */
+#define K8_MTRRFIXRANGE_DRAM_MODIFY 0x00080000 /* MtrrFixDramModEn bit */
+#define K8_MTRR_RDMEM_WRMEM_MASK 0x18181818 /* Mask: RdMem|WrMem */
+
+/* K7 MSRs */
#define MSR_K7_EVNTSEL0 0xc0010000
#define MSR_K7_PERFCTR0 0xc0010004
#define MSR_K7_EVNTSEL1 0xc0010001
@@ -83,20 +95,10 @@
#define MSR_K7_PERFCTR2 0xc0010006
#define MSR_K7_EVNTSEL3 0xc0010003
#define MSR_K7_PERFCTR3 0xc0010007
-#define MSR_K8_TOP_MEM1 0xc001001a
#define MSR_K7_CLK_CTL 0xc001001b
-#define MSR_K8_TOP_MEM2 0xc001001d
-#define MSR_K8_SYSCFG 0xc0010010
-
-#define K8_MTRRFIXRANGE_DRAM_ENABLE 0x00040000 /* MtrrFixDramEn bit */
-#define K8_MTRRFIXRANGE_DRAM_MODIFY 0x00080000 /* MtrrFixDramModEn bit */
-#define K8_MTRR_RDMEM_WRMEM_MASK 0x18181818 /* Mask: RdMem|WrMem */
-
#define MSR_K7_HWCR 0xc0010015
-#define MSR_K8_HWCR 0xc0010015
#define MSR_K7_FID_VID_CTL 0xc0010041
#define MSR_K7_FID_VID_STATUS 0xc0010042
-#define MSR_K8_ENABLE_C1E 0xc0010055

/* K6 MSRs */
#define MSR_K6_EFER 0xc0000080

--
AMD Saxony, Dresden, Germany
Operating System Research Center
email: [email protected]





2007-06-15 18:55:51

by David Rientjes

[permalink] [raw]
Subject: Re: [patch 4/8] 2.6.22-rc3 perfmon2 : IBS implementation for AMD64

On Fri, 15 Jun 2007, Robert Richter wrote:

> Index: linux-2.6.22-rc3/include/asm-i386/msr-index.h
> ===================================================================
> --- linux-2.6.22-rc3.orig/include/asm-i386/msr-index.h
> +++ linux-2.6.22-rc3/include/asm-i386/msr-index.h
> @@ -75,6 +75,18 @@
>
> /* K7/K8 MSRs. Not complete. See the architecture manual for a more
> complete list. */
> +
> +/* K8 MSRs */
> +#define MSR_K8_TOP_MEM1 0xC001001A
> +#define MSR_K8_TOP_MEM2 0xC001001D
> +#define MSR_K8_SYSCFG 0xC0010010
> +#define MSR_K8_HWCR 0xC0010015
> +#define MSR_K8_ENABLE_C1E 0xc0010055

Please don't include mixed cases of hex digits. This entire file has all
hex digits in lowercase type, so please conform to that.

> +#define K8_MTRRFIXRANGE_DRAM_ENABLE 0x00040000 /* MtrrFixDramEn bit */
> +#define K8_MTRRFIXRANGE_DRAM_MODIFY 0x00080000 /* MtrrFixDramModEn bit */
> +#define K8_MTRR_RDMEM_WRMEM_MASK 0x18181818 /* Mask: RdMem|WrMem */

Masks like K8_MTRR_RDMEM_WRMEM_MASK are prone to bugs when the values they
are testing change and somebody forgets to update the mask. Can you make
K8_MTRR_RDMEM_WRMEM_MASK defined to be the result of another preprocessor
macro expression? Or, even better, get rid of it completely and modify
set_fixed_range()?

2007-06-19 13:35:14

by Stephane Eranian

[permalink] [raw]
Subject: Re: [patch 4/8] 2.6.22-rc3 perfmon2 : IBS implementation for AMD64

Robert,

This patch is not specific to perfmon. It modifies the general
infrastructure. As such it could be submitted indepdently.

I will include it in my "infrastructure" patch file for now.

On Fri, Jun 15, 2007 at 06:59:28PM +0200, Robert Richter wrote:
> This patch rearranges AMD64 MSR definitions.
>
> Signed-off-by: Robert Richter <[email protected]>
>
> Index: linux-2.6.22-rc3/include/asm-i386/msr-index.h
> ===================================================================
> --- linux-2.6.22-rc3.orig/include/asm-i386/msr-index.h
> +++ linux-2.6.22-rc3/include/asm-i386/msr-index.h
> @@ -75,6 +75,18 @@
>
> /* K7/K8 MSRs. Not complete. See the architecture manual for a more
> complete list. */
> +
> +/* K8 MSRs */
> +#define MSR_K8_TOP_MEM1 0xC001001A
> +#define MSR_K8_TOP_MEM2 0xC001001D
> +#define MSR_K8_SYSCFG 0xC0010010
> +#define MSR_K8_HWCR 0xC0010015
> +#define MSR_K8_ENABLE_C1E 0xc0010055
> +#define K8_MTRRFIXRANGE_DRAM_ENABLE 0x00040000 /* MtrrFixDramEn bit */
> +#define K8_MTRRFIXRANGE_DRAM_MODIFY 0x00080000 /* MtrrFixDramModEn bit */
> +#define K8_MTRR_RDMEM_WRMEM_MASK 0x18181818 /* Mask: RdMem|WrMem */
> +
> +/* K7 MSRs */
> #define MSR_K7_EVNTSEL0 0xc0010000
> #define MSR_K7_PERFCTR0 0xc0010004
> #define MSR_K7_EVNTSEL1 0xc0010001
> @@ -83,20 +95,10 @@
> #define MSR_K7_PERFCTR2 0xc0010006
> #define MSR_K7_EVNTSEL3 0xc0010003
> #define MSR_K7_PERFCTR3 0xc0010007
> -#define MSR_K8_TOP_MEM1 0xc001001a
> #define MSR_K7_CLK_CTL 0xc001001b
> -#define MSR_K8_TOP_MEM2 0xc001001d
> -#define MSR_K8_SYSCFG 0xc0010010
> -
> -#define K8_MTRRFIXRANGE_DRAM_ENABLE 0x00040000 /* MtrrFixDramEn bit */
> -#define K8_MTRRFIXRANGE_DRAM_MODIFY 0x00080000 /* MtrrFixDramModEn bit */
> -#define K8_MTRR_RDMEM_WRMEM_MASK 0x18181818 /* Mask: RdMem|WrMem */
> -
> #define MSR_K7_HWCR 0xc0010015
> -#define MSR_K8_HWCR 0xc0010015
> #define MSR_K7_FID_VID_CTL 0xc0010041
> #define MSR_K7_FID_VID_STATUS 0xc0010042
> -#define MSR_K8_ENABLE_C1E 0xc0010055
>
> /* K6 MSRs */
> #define MSR_K6_EFER 0xc0000080
>
> --
> AMD Saxony, Dresden, Germany
> Operating System Research Center
> email: [email protected]
>
>
>

--

-Stephane

Subject: Re: [patch 4/8] 2.6.22-rc3 perfmon2 : IBS implementation for AMD64

David,

> Please don't include mixed cases of hex digits. This entire file has all
> hex digits in lowercase type, so please conform to that.

I fixed this in the 2nd version of the patch.

> > +#define K8_MTRRFIXRANGE_DRAM_ENABLE 0x00040000 /* MtrrFixDramEn bit */
> > +#define K8_MTRRFIXRANGE_DRAM_MODIFY 0x00080000 /* MtrrFixDramModEn bit */
> > +#define K8_MTRR_RDMEM_WRMEM_MASK 0x18181818 /* Mask: RdMem|WrMem */
>
> Masks like K8_MTRR_RDMEM_WRMEM_MASK are prone to bugs when the values they
> are testing change and somebody forgets to update the mask. Can you make
> K8_MTRR_RDMEM_WRMEM_MASK defined to be the result of another preprocessor
> macro expression? Or, even better, get rid of it completely and modify
> set_fixed_range()?

This is existing code, won't change that.

Thanks,
-Robert

--
AMD Saxony, Dresden, Germany
Operating System Research Center
email: [email protected]