diff mbox

bios: mtrr: fix overlaped MTRR (LP: #1694710)

Message ID 1496652164-6326-1-git-send-email-ivan.hu@canonical.com
State Accepted
Headers show

Commit Message

Ivan Hu June 5, 2017, 8:42 a.m. UTC
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@canonical.com>
---
 src/bios/mtrr/mtrr.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

Comments

Colin Ian King June 5, 2017, 9:11 a.m. UTC | #1
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@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@canonical.com>
Alex Hung June 6, 2017, 6:10 a.m. UTC | #2
On 2017-06-05 01:42 AM, 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@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,
> 

Acked-by: Alex Hung <alex.hung@canonical.com>
diff mbox

Patch

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,