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

[David Rientjes]

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()?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/