Patchwork bios: mtrr: remove unused VGA region check

login
register
mail settings
Submitter Colin King
Date Dec. 7, 2012, 12:09 a.m.
Message ID <1354838953-6481-1-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/204351/
State Accepted
Headers show

Comments

Colin King - Dec. 7, 2012, 12:09 a.m.
From: Colin Ian King <colin.king@canonical.com>

We've had the VGA region check disabled for a few releases now
and I really don't think it will ever be enabled, so I'm removing
this old unused code.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/bios/mtrr/mtrr.c | 148 ---------------------------------------------------
 1 file changed, 148 deletions(-)
Alex Hung - Dec. 13, 2012, 2:27 a.m.
On 12/07/2012 08:09 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> We've had the VGA region check disabled for a few releases now
> and I really don't think it will ever be enabled, so I'm removing
> this old unused code.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/bios/mtrr/mtrr.c | 148 ---------------------------------------------------
>   1 file changed, 148 deletions(-)
>
> diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c
> index 5f3e8d5..294e80b 100644
> --- a/src/bios/mtrr/mtrr.c
> +++ b/src/bios/mtrr/mtrr.c
> @@ -23,8 +23,6 @@
>
>   #ifdef FWTS_ARCH_INTEL
>
> -#define FWTS_TEST_VGA_REGION	0
> -
>   #include <stdlib.h>
>   #include <stdio.h>
>   #include <string.h>
> @@ -194,152 +192,6 @@ restart:
>   	return type;
>   }
>
> -#if FWTS_TEST_VGA_REGION
> -static fwts_list *get_klog_bios_mtrr(void)
> -{
> -	fwts_list *mtrr_bios_list;
> -	fwts_list_link *item;
> -	int scan = 0;
> -
> -	if ((mtrr_bios_list = fwts_list_new()) == NULL)
> -		return NULL;
> -
> -	fwts_list_foreach(item, klog) {
> -		char *str = fwts_text_list_text(item);
> -
> -		if (strstr(str, "MTRR variable ranges enabled")) {
> -			scan = 1;
> -			continue;
> -		}
> -
> -		if (scan) {
> -			char *base = strstr(str, "base");
> -			char *disabled = strstr(str, "disabled");
> -
> -			if ((base == NULL) && (disabled == NULL))
> -				scan = 0;
> -
> -			if (base) {
> -				uint64_t start = strtoull(base+6, NULL, 16);
> -				str = strstr(base, "mask");
> -				if (str) {
> -					struct mtrr_entry *mtrr;
> -
> -					mtrr = calloc(1, sizeof(struct mtrr_entry));
> -					if (mtrr == NULL) {
> -						fwts_list_free(mtrr_bios_list,
> -							free);
> -						return NULL;
> -					}
> -					mtrr->type = 0;
> -
> -					uint64_t mask =
> -						strtoull(str+5, NULL, 16);
> -					uint64_t pat = 0x8000000000000000ULL;
> -					while ((mask & pat) == 0) {
> -						mask |= pat;
> -						pat >>= 1;
> -					}
> -
> -					mtrr->start = start;
> -					mtrr->end = start + ~mask;
> -
> -					fwts_list_append(mtrr_bios_list, mtrr);
> -				}
> -			}
> -		}
> -	}
> -
> -	return mtrr_bios_list;
> -}
> -#endif
> -
> -#if FWTS_TEST_VGA_REGION
> -static int check_vga_controller_address(fwts_framework *fw)
> -{
> -	char line[4096];
> -	fwts_list *lspci_output;
> -	fwts_list_link *item;
> -	fwts_list *mtrr_bios_list;
> -	int vga = 0;
> -	int found = 0;
> -
> -	memset(line,0,4096);
> -
> -	if ((mtrr_bios_list = get_klog_bios_mtrr()) == NULL) {
> -		fwts_log_error("Out of memory fetching MTRR list.");
> -		return FWTS_ERROR;
> -	}
> -
> -	snprintf(line, sizeof(line), "%s -v", fw->lspci);
> -	fwts_pipe_exec(line, &lspci_output);
> -	if (lspci_output == NULL)
> -		return FWTS_ERROR;
> -
> -	fwts_list_foreach(item, lspci_output) {
> -		char *str = fwts_text_list_text(item);
> -		if (strstr(str, "VGA compatible controller"))
> -			vga = 1;
> -		if (*str == '\0')
> -			vga = 0;
> -		if (vga) {
> -			if ((str = strstr(str, "Memory at ")) != NULL) {
> -				struct mtrr_entry *mtrr;
> -				uint64_t start = strtoull(str+10, NULL, 16);
> -				uint64_t size = 0;
> -#if 0
> -				int pref = 0;
> -				if (strstr(str, "non-prefetchable"))
> -					pref = 0;
> -				else if (strstr(str, "(prefetchable"))
> -					pref = 1;
> -				else if (strstr(str, ", prefetchable"))
> -					pref = 1;
> -#endif
> -				if ((str = strstr(str + 10, "size=")) != NULL) {
> -					size = strtoull(str+5, NULL, 10);
> -					if (strstr(str + 5, "K]"))
> -						size *= 1024;
> -					if (strstr(str + 5, "M]"))
> -						size *= (1024*1024);
> -					size--;
> -				}
> -
> -				if (size > 1024*1024) {
> -					fwts_list_link *mtrr_bios_item;
> -
> -					fwts_list_foreach(mtrr_bios_item, mtrr_bios_list) {
> -						mtrr = fwts_list_data(struct mtrr_entry *, mtrr_bios_item);
> -						if (start >= mtrr->start && (start+size)<= mtrr->end) {
> -							found = 1;
> -							fwts_passed(fw, "Found VGA memory region in BIOS initialised MTRR space: %" PRIx64 " - %" PRIx64 "\n",
> -								mtrr->start,
> -								mtrr->end);
> -							break;
> -						}
> -					}
> -				}
> -			}
> -		}
> -	}
> -
> -	if (!found) {
> -		fwts_failed(fw, LOG_LEVEL_LOW, "MTRRVGA", "Did not find a BIOS configured MTRR for VGA memory region. ");
> -		fwts_advice(fw,
> -			"The VGA memory region does not have a MTRR configured "
> -			"by the BIOS. This means that bootloaders rendering to "
> -			"a framebuffer will be rendering slowly and this will "
> -			"slow the boot speed. It is probably worth asking the "
> -			"BIOS vendor to map in the VGA write-combining "
> -			"region.");
> -	}
> -	fwts_list_free(mtrr_bios_list, free);
> -	fwts_list_free(lspci_output, free);
> -
> -	return FWTS_OK;
> -}
> -#endif
> -
>   static int is_prefetchable(fwts_framework *fw, char *device, uint64_t address)
>   {
>   	int pref = 0;
>

Acked-by: Alex Hung <alex.hung@canonical.com>
Ivan Hu - Dec. 13, 2012, 9:42 a.m.
On 12/07/2012 08:09 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> We've had the VGA region check disabled for a few releases now
> and I really don't think it will ever be enabled, so I'm removing
> this old unused code.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/bios/mtrr/mtrr.c | 148 ---------------------------------------------------
>   1 file changed, 148 deletions(-)
>
> diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c
> index 5f3e8d5..294e80b 100644
> --- a/src/bios/mtrr/mtrr.c
> +++ b/src/bios/mtrr/mtrr.c
> @@ -23,8 +23,6 @@
>
>   #ifdef FWTS_ARCH_INTEL
>
> -#define FWTS_TEST_VGA_REGION	0
> -
>   #include <stdlib.h>
>   #include <stdio.h>
>   #include <string.h>
> @@ -194,152 +192,6 @@ restart:
>   	return type;
>   }
>
> -#if FWTS_TEST_VGA_REGION
> -static fwts_list *get_klog_bios_mtrr(void)
> -{
> -	fwts_list *mtrr_bios_list;
> -	fwts_list_link *item;
> -	int scan = 0;
> -
> -	if ((mtrr_bios_list = fwts_list_new()) == NULL)
> -		return NULL;
> -
> -	fwts_list_foreach(item, klog) {
> -		char *str = fwts_text_list_text(item);
> -
> -		if (strstr(str, "MTRR variable ranges enabled")) {
> -			scan = 1;
> -			continue;
> -		}
> -
> -		if (scan) {
> -			char *base = strstr(str, "base");
> -			char *disabled = strstr(str, "disabled");
> -
> -			if ((base == NULL) && (disabled == NULL))
> -				scan = 0;
> -
> -			if (base) {
> -				uint64_t start = strtoull(base+6, NULL, 16);
> -				str = strstr(base, "mask");
> -				if (str) {
> -					struct mtrr_entry *mtrr;
> -
> -					mtrr = calloc(1, sizeof(struct mtrr_entry));
> -					if (mtrr == NULL) {
> -						fwts_list_free(mtrr_bios_list,
> -							free);
> -						return NULL;
> -					}
> -					mtrr->type = 0;
> -
> -					uint64_t mask =
> -						strtoull(str+5, NULL, 16);
> -					uint64_t pat = 0x8000000000000000ULL;
> -					while ((mask & pat) == 0) {
> -						mask |= pat;
> -						pat >>= 1;
> -					}
> -
> -					mtrr->start = start;
> -					mtrr->end = start + ~mask;
> -
> -					fwts_list_append(mtrr_bios_list, mtrr);
> -				}
> -			}
> -		}
> -	}
> -
> -	return mtrr_bios_list;
> -}
> -#endif
> -
> -#if FWTS_TEST_VGA_REGION
> -static int check_vga_controller_address(fwts_framework *fw)
> -{
> -	char line[4096];
> -	fwts_list *lspci_output;
> -	fwts_list_link *item;
> -	fwts_list *mtrr_bios_list;
> -	int vga = 0;
> -	int found = 0;
> -
> -	memset(line,0,4096);
> -
> -	if ((mtrr_bios_list = get_klog_bios_mtrr()) == NULL) {
> -		fwts_log_error("Out of memory fetching MTRR list.");
> -		return FWTS_ERROR;
> -	}
> -
> -	snprintf(line, sizeof(line), "%s -v", fw->lspci);
> -	fwts_pipe_exec(line, &lspci_output);
> -	if (lspci_output == NULL)
> -		return FWTS_ERROR;
> -
> -	fwts_list_foreach(item, lspci_output) {
> -		char *str = fwts_text_list_text(item);
> -		if (strstr(str, "VGA compatible controller"))
> -			vga = 1;
> -		if (*str == '\0')
> -			vga = 0;
> -		if (vga) {
> -			if ((str = strstr(str, "Memory at ")) != NULL) {
> -				struct mtrr_entry *mtrr;
> -				uint64_t start = strtoull(str+10, NULL, 16);
> -				uint64_t size = 0;
> -#if 0
> -				int pref = 0;
> -				if (strstr(str, "non-prefetchable"))
> -					pref = 0;
> -				else if (strstr(str, "(prefetchable"))
> -					pref = 1;
> -				else if (strstr(str, ", prefetchable"))
> -					pref = 1;
> -#endif
> -				if ((str = strstr(str + 10, "size=")) != NULL) {
> -					size = strtoull(str+5, NULL, 10);
> -					if (strstr(str + 5, "K]"))
> -						size *= 1024;
> -					if (strstr(str + 5, "M]"))
> -						size *= (1024*1024);
> -					size--;
> -				}
> -
> -				if (size > 1024*1024) {
> -					fwts_list_link *mtrr_bios_item;
> -
> -					fwts_list_foreach(mtrr_bios_item, mtrr_bios_list) {
> -						mtrr = fwts_list_data(struct mtrr_entry *, mtrr_bios_item);
> -						if (start >= mtrr->start && (start+size)<= mtrr->end) {
> -							found = 1;
> -							fwts_passed(fw, "Found VGA memory region in BIOS initialised MTRR space: %" PRIx64 " - %" PRIx64 "\n",
> -								mtrr->start,
> -								mtrr->end);
> -							break;
> -						}
> -					}
> -				}
> -			}
> -		}
> -	}
> -
> -	if (!found) {
> -		fwts_failed(fw, LOG_LEVEL_LOW, "MTRRVGA", "Did not find a BIOS configured MTRR for VGA memory region. ");
> -		fwts_advice(fw,
> -			"The VGA memory region does not have a MTRR configured "
> -			"by the BIOS. This means that bootloaders rendering to "
> -			"a framebuffer will be rendering slowly and this will "
> -			"slow the boot speed. It is probably worth asking the "
> -			"BIOS vendor to map in the VGA write-combining "
> -			"region.");
> -	}
> -	fwts_list_free(mtrr_bios_list, free);
> -	fwts_list_free(lspci_output, free);
> -
> -	return FWTS_OK;
> -}
> -#endif
> -
>   static int is_prefetchable(fwts_framework *fw, char *device, uint64_t address)
>   {
>   	int pref = 0;
>
Acked-by: Ivan Hu <ivan.hu@canonical.com>

Patch

diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c
index 5f3e8d5..294e80b 100644
--- a/src/bios/mtrr/mtrr.c
+++ b/src/bios/mtrr/mtrr.c
@@ -23,8 +23,6 @@ 
 
 #ifdef FWTS_ARCH_INTEL
 
-#define FWTS_TEST_VGA_REGION	0
-
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
@@ -194,152 +192,6 @@  restart:
 	return type;
 }
 
-#if FWTS_TEST_VGA_REGION
-static fwts_list *get_klog_bios_mtrr(void)
-{
-	fwts_list *mtrr_bios_list;
-	fwts_list_link *item;
-	int scan = 0;
-
-	if ((mtrr_bios_list = fwts_list_new()) == NULL)
-		return NULL;
-
-	fwts_list_foreach(item, klog) {
-		char *str = fwts_text_list_text(item);
-
-		if (strstr(str, "MTRR variable ranges enabled")) {
-			scan = 1;
-			continue;
-		}
-
-		if (scan) {
-			char *base = strstr(str, "base");
-			char *disabled = strstr(str, "disabled");
-
-			if ((base == NULL) && (disabled == NULL))
-				scan = 0;
-
-			if (base) {
-				uint64_t start = strtoull(base+6, NULL, 16);
-				str = strstr(base, "mask");
-				if (str) {
-					struct mtrr_entry *mtrr;
-
-					mtrr = calloc(1, sizeof(struct mtrr_entry));
-					if (mtrr == NULL) {
-						fwts_list_free(mtrr_bios_list,
-							free);
-						return NULL;
-					}
-					mtrr->type = 0;
-
-					uint64_t mask =
-						strtoull(str+5, NULL, 16);
-					uint64_t pat = 0x8000000000000000ULL;
-					while ((mask & pat) == 0) {
-						mask |= pat;
-						pat >>= 1;
-					}
-
-					mtrr->start = start;
-					mtrr->end = start + ~mask;
-
-					fwts_list_append(mtrr_bios_list, mtrr);
-				}
-			}
-		}
-	}
-
-	return mtrr_bios_list;
-}
-#endif
-
-#if FWTS_TEST_VGA_REGION
-static int check_vga_controller_address(fwts_framework *fw)
-{
-	char line[4096];
-	fwts_list *lspci_output;
-	fwts_list_link *item;
-	fwts_list *mtrr_bios_list;
-	int vga = 0;
-	int found = 0;
-
-	memset(line,0,4096);
-
-	if ((mtrr_bios_list = get_klog_bios_mtrr()) == NULL) {
-		fwts_log_error("Out of memory fetching MTRR list.");
-		return FWTS_ERROR;
-	}
-
-	snprintf(line, sizeof(line), "%s -v", fw->lspci);
-	fwts_pipe_exec(line, &lspci_output);
-	if (lspci_output == NULL)
-		return FWTS_ERROR;
-
-	fwts_list_foreach(item, lspci_output) {
-		char *str = fwts_text_list_text(item);
-		if (strstr(str, "VGA compatible controller"))
-			vga = 1;
-		if (*str == '\0')
-			vga = 0;
-		if (vga) {
-			if ((str = strstr(str, "Memory at ")) != NULL) {
-				struct mtrr_entry *mtrr;
-				uint64_t start = strtoull(str+10, NULL, 16);
-				uint64_t size = 0;
-#if 0
-				int pref = 0;
-				if (strstr(str, "non-prefetchable"))
-					pref = 0;
-				else if (strstr(str, "(prefetchable"))
-					pref = 1;
-				else if (strstr(str, ", prefetchable"))
-					pref = 1;
-#endif
-				if ((str = strstr(str + 10, "size=")) != NULL) {
-					size = strtoull(str+5, NULL, 10);
-					if (strstr(str + 5, "K]"))
-						size *= 1024;
-					if (strstr(str + 5, "M]"))
-						size *= (1024*1024);
-					size--;
-				}
-
-				if (size > 1024*1024) {
-					fwts_list_link *mtrr_bios_item;
-
-					fwts_list_foreach(mtrr_bios_item, mtrr_bios_list) {
-						mtrr = fwts_list_data(struct mtrr_entry *, mtrr_bios_item);
-						if (start >= mtrr->start && (start+size)<= mtrr->end) {
-							found = 1;
-							fwts_passed(fw, "Found VGA memory region in BIOS initialised MTRR space: %" PRIx64 " - %" PRIx64 "\n",
-								mtrr->start,
-								mtrr->end);
-							break;
-						}
-					}
-				}
-			}
-		}
-	}
-
-	if (!found) {
-		fwts_failed(fw, LOG_LEVEL_LOW, "MTRRVGA", "Did not find a BIOS configured MTRR for VGA memory region. ");
-		fwts_advice(fw,
-			"The VGA memory region does not have a MTRR configured "
-			"by the BIOS. This means that bootloaders rendering to "
-			"a framebuffer will be rendering slowly and this will "
-			"slow the boot speed. It is probably worth asking the "
-			"BIOS vendor to map in the VGA write-combining "
-			"region.");
-	}
-	fwts_list_free(mtrr_bios_list, free);
-	fwts_list_free(lspci_output, free);
-
-	return FWTS_OK;
-}
-#endif
-
 static int is_prefetchable(fwts_framework *fw, char *device, uint64_t address)
 {
 	int pref = 0;