ACK: [PATCH] bios: mtrr: print out actual default type of MTRR

[Colin Ian King]

On 06/06/17 08:37, Ivan Hu wrote:
> Print out default type of MTRR instead of using "Most probably Uncached".
> Also modify to get default type from MSR once, avoiding to get default type each
> time we check cahch type.
> 
> Signed-off-by: Ivan Hu <ivan.hu at canonical.com>
> ---
>  src/bios/mtrr/mtrr.c | 79 +++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 50 insertions(+), 29 deletions(-)
> 
> diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c
> index 7d29c2b..6386b6e 100644
> --- a/src/bios/mtrr/mtrr.c
> +++ b/src/bios/mtrr/mtrr.c
> @@ -51,6 +51,8 @@ static fwts_cpuinfo_x86 *fwts_cpuinfo;
>  
>  #define MTRR_DEF_TYPE_MSR	0x2FF
>  
> +static	uint64_t mtrr_default;
> +
>  struct mtrr_entry {
>  	uint8_t  reg;
>  	uint64_t start;
> @@ -74,8 +76,21 @@ static char *cache_to_string(int type)
>  		strcat(str," Write-Through");
>  	if (type & WRITE_PROTECT)
>  		strcat(str," Write-Protect");
> -	if (type & DEFAULT)
> -		strcat(str," Default (Most probably Uncached)");
> +	if (type & DEFAULT) {
> +		strcat(str," Default");
> +		if (mtrr_default & UNCACHED)
> +			strcat(str," (Uncached)");
> +		if (mtrr_default & WRITE_BACK)
> +			strcat(str," (Write-Back)");
> +		if (mtrr_default & WRITE_COMBINING)
> +			strcat(str," (Write-Combining)");
> +		if (mtrr_default & WRITE_THROUGH)
> +			strcat(str," (Write-Through)");
> +		if (mtrr_default & WRITE_PROTECT)
> +			strcat(str," (Write-Protect)");
> +		if (mtrr_default & UNKNOWN)
> +			strcat(str," (Unknown)");
> +	}
>  	if (type & UNKNOWN)
>  		strcat(str," Unknown");
>  	return str;
> @@ -167,12 +182,38 @@ static int get_mtrrs(void)
>  	return FWTS_OK;
>  }
>  
> +static int get_default_mtrr(void) {
> +	if (fwts_cpu_readmsr(0, MTRR_DEF_TYPE_MSR, &mtrr_default) == FWTS_OK) {
> +		switch (mtrr_default & 0xFF) {
> +			case 0:
> +				mtrr_default = UNCACHED;
> +				break;
> +			case 1:
> +				mtrr_default = WRITE_COMBINING;
> +				break;
> +			case 4:
> +				mtrr_default = WRITE_THROUGH;
> +				break;
> +			case 5:
> +				mtrr_default = WRITE_PROTECT;
> +				break;
> +			case 6:
> +				mtrr_default = WRITE_BACK;
> +				break;
> +			default:
> +				mtrr_default = UNKNOWN;
> +				break;
> +		}
> +		return FWTS_OK;
> +	}
> +	return FWTS_ERROR;
> +}
> +
>  static int cache_types(uint64_t start, uint64_t end)
>  {
>  	fwts_list_link *item;
>  	struct mtrr_entry *entry;
>  	int type = 0;
> -	uint64_t mtrr_default = UNCACHED;
>  
>  	fwts_list_foreach(item, mtrr_list) {
>  		entry = fwts_list_data(struct mtrr_entry*, item);
> @@ -202,32 +243,9 @@ restart:
>  	if (start != end)
>  		type |= DEFAULT;
>  
> -	if (fwts_cpu_readmsr(0, MTRR_DEF_TYPE_MSR, &mtrr_default) == FWTS_OK) {
> -		switch (mtrr_default & 0xFF) {
> -			case 0:
> -				mtrr_default = UNCACHED;
> -				break;
> -			case 1:
> -				mtrr_default = WRITE_COMBINING;
> -				break;
> -			case 4:
> -				mtrr_default = WRITE_THROUGH;
> -				break;
> -			case 5:
> -				mtrr_default = WRITE_PROTECT;
> -				break;
> -			case 6:
> -				mtrr_default = WRITE_BACK;
> -				break;
> -			default:
> -				mtrr_default = UNKNOWN;
> -				break;
> -		}
> -
> -		if ((type & DEFAULT) && mtrr_default != UNCACHED) {
> -			type &= ~DEFAULT;
> -			type |= mtrr_default;
> -		}
> +	if ((type & DEFAULT) && mtrr_default != UNCACHED) {
> +		type &= ~DEFAULT;
> +		type |= mtrr_default;
>  	}
>  
>  	return type;
> @@ -390,6 +408,9 @@ static int validate_iomem(fwts_framework *fw)
>  	if ((file = fopen("/proc/iomem", "r")) == NULL)
>  		return FWTS_ERROR;
>  
> +	if (get_default_mtrr() != FWTS_OK)
> +		mtrr_default = UNKNOWN;
> +
>  	while (!feof(file)) {
>  		uint64_t start;
>  		uint64_t end;
> 

Yep, good idea. Thanks Ivan.

Acked-by: Colin Ian King <colin.king at canonical.com>