ACK: [PATCH] bios: mtrr: fix overlaped MTRR (LP: #1694710)

[Colin Ian King]

On 05/06/17 09:42, Ivan Hu wrote:
> Some firmware will set more than one MTRR types, which cause the error checking
> with the wrong type. such as,
> reg00: base=0x000000000 ( 0MB), size=131072MB, count=1: write-back
> reg01: base=0x0c8000000 ( 3200MB), size= 128MB, count=1: uncachable
> reg02: base=0x0d0000000 ( 3328MB), size= 256MB, count=1: uncachable
> reg03: base=0x0e0000000 ( 3584MB), size= 512MB, count=1: uncachable
> 
> and got error as:
> mtrr: Memory range 0xc8000000 to 0xc8ffffff (0000:02:01.0) has incorrect
> attribute Write-Back.
> 
> From Intel Software Developer Manual, 11.11.4.1 MTRR Precedences:
> Below rules are added for FWTS checking,
> — If two or more variable memory ranges match and one of the memory types is UC,
>  the UC memory type used.
> — If two or more variable memory ranges match and the memory types are WT and WB
> , the WT memory type is used.
> — For overlaps not defined by the above rules, processor behavior is undefined.
> 
> Signed-off-by: Ivan Hu <ivan.hu at canonical.com>
> ---
>  src/bios/mtrr/mtrr.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c
> index bef4aff..7d29c2b 100644
> --- a/src/bios/mtrr/mtrr.c
> +++ b/src/bios/mtrr/mtrr.c
> @@ -342,6 +342,42 @@ static int guess_cache_type(
>  	return FWTS_OK;
>  }
>  
> +void multi_types_check(fwts_framework *fw, int *type)
> +{
> +	int n_types = 0;
> +
> +	/* checking number fo types set, UNCACHED, WRITE_BACK, WRITE_COMBINING, WRITE_THROUGH, WRITE_PROTECT */
> +	for (int i = 0; i < 5; i++) {
> +		if (*type & (1 << i))
> +			n_types++;
> +	}
> +
> +	if (n_types >= 2) {
> +		if (*type & UNCACHED) {
> +			fwts_log_info(fw, "Detected more than one type has been set. "
> +					"From Intel Software Developer Manual 11.11.4.1 MTRR Precedences, "
> +					"If two or more variable memory ranges match and one of the memory "
> +					"types is UC, the UC memory type used. FWTS will check with type "
> +					"UNCACHED here.");
> +			*type = UNCACHED;
> +			return;
> +		} else if ((*type & WRITE_THROUGH) && (*type & WRITE_BACK)) {
> +			fwts_log_info(fw, "Detected more than one type has been set. "
> +					"From Intel Software Developer Manual 11.11.4.1 MTRR Precedences, "
> +					"If two or more variable memory ranges match and the memory types "
> +					"are WT and WB, the WT memory type is used. FWTS will check with type "
> +					"WRITE_THROUGH here.");
> +			*type = WRITE_THROUGH;
> +			return;
> +		} else {
> +			fwts_warning(fw, "Detected more than one type has been set. The overlaps not defined, "
> +					"processor behavior is undefined.");
> +			return;
> +		}
> +	}
> +	return;
> +}
> +
>  static int validate_iomem(fwts_framework *fw)
>  {
>  	FILE *file;
> @@ -406,6 +442,8 @@ static int validate_iomem(fwts_framework *fw)
>  
>  		type = cache_types(start, end);
>  
> +		(void)multi_types_check(fw, &type);
> +
>  		if (guess_cache_type(fw, c2, &type_must, &type_mustnot, start) != FWTS_OK) {
>  			/*  This has failed, give up at this point */
>  			fwts_skipped(fw,
> 
Thanks Ivan.

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