[coreboot] [patch][msrtool] Add K8 MSRs

[Carl-Daniel Hailfinger]

On 05.03.2009 21:57, Marc Jones wrote:
> Add K8 support for some of the more important MSRs.
>
> Signed-off-by: Marc Jones <marcj303 at gmail.com>
>   

If you fix the one bug mentioned below and can provide
explanations/answers for my other questions, this is
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>


> +	{ 0xC001001A, MSRTYPE_RDWR, MSR2(0, 0), "TOP_MEM Register", "This register indicates the first byte of I/O above DRAM", {
> +		{ 63, 24, RESERVED },
> +		{ 39, 8, "TOM 16-9", "", PRESENT_HEX, {
>   

I find the bit numbering in the description string here to be rather
unintuitive. IIRC these bits here are not shifted, so "TOM 29-32" would
be the correct name.

> +			{ BITVAL_EOT }
> +		}},
> +		{ 31, 9, "TOM 8-0", "", PRESENT_HEX, {
>   

Same here.

> +			{ BITVAL_EOT }
> +		}},
> +		{ 22, 23, RESERVED },
> +		{ BITS_EOT }
> +	}},
>   

Can TOP_MEM really be larger than 4 GB?

> +
> +	{ 0xC001001D, MSRTYPE_RDWR, MSR2(0, 0), "TOP_MEM2 Register", "This register indicates the Top of Memory above 4GB", {
> +		{ 63, 24, RESERVED },
> +		{ 39, 8, "TOM2 16-9", "", PRESENT_HEX, {
>   

Bit numbering.

> +			{ BITVAL_EOT }
> +		}},
> +		{ 31, 9, "TOM2 8-0", "", PRESENT_HEX, {
>   

Same here.

> +			{ BITVAL_EOT }
> +		}},
> +		{ 22, 23, RESERVED },
> +		{ BITS_EOT }
> +	}},
>   

I thought newer AMD processors have more than 40 bit addressable memory.


> +	{ 0xC0010019, MSRTYPE_RDWR, MSR2(0, 0), "IORRMask0", "This register holds the mask of the variable I/O range", {
>   

Should be IORRMask1.

> +		{ 63, 24, RESERVED },
> +		{ 39, 8, "MASK 27-20", "", PRESENT_HEX, {
> +			{ BITVAL_EOT }
> +		}},
> +		{ 31, 20, "MASK 20-0", "", PRESENT_HEX, {
> +			{ BITVAL_EOT }
> +		}},
> +		{ 11, 1, "V:", "Enables variable I/O range registers", PRESENT_DEC, {
> +			{ MSR1(0), "V I/O range disabled" },
> +			{ MSR1(1), "V I/O range enabled" },
> +			{ BITVAL_EOT }
> +		}},
> +		{ 10, 11, RESERVED },
> +		{ BITS_EOT }
> +	}},
> +
>   

> Index: msrtool/msrutils.c
> ===================================================================
> --- msrtool.orig/msrutils.c	2009-03-05 11:58:19.000000000 -0700
> +++ msrtool/msrutils.c	2009-03-02 15:40:13.000000000 -0700
> @@ -137,7 +137,7 @@
>  	return NULL;
>  }
>  
> -const uint32_t msraddrbyname(const char *name) {
> +uint32_t msraddrbyname(const char *name) {
>   

Any reason for the const removal? The changelog doesn't say anything
about it.

>  	uint8_t t;
>  	const uint32_t addr = strtoul(name, NULL, 16);
>  	const struct msrdef *m;
>   


Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/