[PATCH] read /proc/mtrr rather than use ioctl() interface
Keng-Yü Lin
kengyu at canonical.com
Tue Feb 21 02:52:04 UTC 2012
On Fri, Feb 10, 2012 at 4:10 PM, Alex Hung <alex.hung at canonical.com> wrote:
> On 02/10/2012 06:29 AM, Colin King wrote:
>>
>> From: Colin Ian King<colin.king at canonical.com>
>>
>> The /proc/mtrr ioctl() MTRRIOC_GET_ENTRY returns zero'd entries for
>> memory bases> 4GB which is wrong. However reading /proc/mtrr
>> provides the correct entries for>4GB but we now have to manually
>> parse the output, which is a pain.
>>
>> This patch removes the use of the ioctl() interface and parses the
>> /proc/mtrr output to get the all the MTRR entries.
>>
>> Signed-off-by: Colin Ian King<colin.king at canonical.com>
>> ---
>> src/bios/mtrr/mtrr.c | 110
>> ++++++++++++++++++++++++++++++--------------------
>> 1 files changed, 66 insertions(+), 44 deletions(-)
>>
>> diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c
>> index f94a465..1f2ba78 100644
>> --- a/src/bios/mtrr/mtrr.c
>> +++ b/src/bios/mtrr/mtrr.c
>> @@ -33,6 +33,7 @@
>> #include<sys/ioctl.h>
>> #include<fcntl.h>
>> #include<unistd.h>
>> +#include<ctype.h>
>> #include<asm/mtrr.h>
>>
>> static fwts_list *klog;
>> @@ -80,59 +81,78 @@ static char *cache_to_string(int type)
>>
>> static int get_mtrrs(void)
>> {
>> - struct mtrr_gentry gentry;
>> struct mtrr_entry *entry;
>> - int fd;
>> + FILE *fp;
>> char line[4096];
>>
>> - memset(line, 0, 4096);
>> -
>> if ((mtrr_list = fwts_list_new()) == NULL)
>> return FWTS_ERROR;
>>
>> - if ((fd = open("/proc/mtrr", O_RDONLY, 0))< 0)
>> + if ((fp = fopen("/proc/mtrr", "r")) == NULL)
>> return FWTS_ERROR;
>>
>> - memset(&gentry, 0, sizeof(gentry));
>> -
>> - for (gentry.regnum = 0;
>> - ioctl (fd, MTRRIOC_GET_ENTRY,&gentry) == 0;
>>
>> - ++gentry.regnum) {
>> - if ((entry = calloc(1, sizeof(struct mtrr_entry))) !=
>> NULL) {
>> - entry->reg = gentry.regnum;
>> - if (gentry.size> 0) {
>> - entry->start = gentry.base;
>> - entry->size = gentry.size;
>> - entry->end = gentry.base + gentry.size;
>> - switch (gentry.type) {
>> - case 0: /* uncachable */
>> - entry->type = UNCACHED;
>> - break;
>> - case 1: /* write combining */
>> - entry->type = WRITE_COMBINING;
>> - break;
>> - case 4: /* write through */
>> - entry->type = WRITE_THROUGH;
>> - break;
>> - case 5: /* write protect */
>> - entry->type = WRITE_PROTECT;
>> - break;
>> - case 6: /* write combining */
>> - entry->type = WRITE_BACK;
>> - break;
>> - default: /* unknown */
>> - entry->type = UNKNOWN;
>> - break;
>> - }
>> - }
>> - else {
>> - entry->type = DISABLED;
>> - }
>> + while (!feof(fp)) {
>> + char *ptr1, *ptr2;
>>
>> - fwts_list_append(mtrr_list, entry);
>> + if (fgets(line, sizeof(line), fp) == NULL)
>> + break;
>> +
>> + if ((entry = calloc(1, sizeof(struct mtrr_entry))) ==
>> NULL) {
>> + fwts_list_free(mtrr_list, free);
>> + fclose(fp);
>> + return FWTS_ERROR;
>> }
>> +
>> + /*
>> + * Put all text to lower case since the output
>> + * from /proc/mtrr is variable upper/lower case
>> + * across kernel versions so forcing to lower
>> + * saves comparing for upper/lower case variants.
>> + */
>> + for (ptr1 = line; *ptr1; ptr1++)
>> + *ptr1 = tolower(*ptr1);
>> +
>> + /* Parse the following:
>> + * reg01: base=0x080000000 ( 2048MB), size= 1024MB,
>> count=1: write-back
>> + */
>> +
>> + /* Get register, in decimal */
>> + if (strncmp(line, "reg", 3))
>> + continue;
>> + entry->reg = strtoul(line + 3, NULL, 10);
>> +
>> + /* Get base, in hex */
>> + if ((ptr1 = strstr(line, "base=0x")) == NULL)
>> + continue;
>> + entry->start = strtoull(ptr1 + 5, NULL, 16);
>> +
>> + /* Get size, in decimal */
>> + if ((ptr1 = strstr(line, "size=")) == NULL)
>> + continue;
>> +
>> + entry->size = strtoull(ptr1 + 5,&ptr2, 10);
>> + if (ptr2&& (*ptr2 == 'm'))
>>
>> + entry->size *= 1024 * 1024;
>> + if (ptr2&& (*ptr2 == 'k'))
>>
>> + entry->size *= 1024;
>> +
>> + entry->end = entry->start + entry->size;
>> +
>> + if (strstr(line, "write-back"))
>> + entry->type = WRITE_BACK;
>> + else if (strstr(line, "uncachable"))
>> + entry->type = UNCACHED;
>> + else if (strstr(line, "write-through"))
>> + entry->type = WRITE_THROUGH;
>> + else if (strstr(line, "write-combining"))
>> + entry->type = WRITE_COMBINING;
>> + else if (strstr(line, "write-protect"))
>> + entry->type = WRITE_PROTECT;
>> + else entry->type = UNKNOWN;
>> +
>> + fwts_list_append(mtrr_list, entry);
>> }
>> - close(fd);
>> + fclose(fp);
>>
>> return FWTS_OK;
>> }
>> @@ -481,7 +501,7 @@ static void do_mtrr_resource(fwts_framework *fw)
>> fwts_log_info_verbatum(fw, "Reg %hhu: disabled\n",
>> entry->reg);
>> else
>> fwts_log_info_verbatum(fw,
>> - "Reg %hhu: 0x%08llx - 0x%08llx (%6lld %cB)
>> %s \n",
>> + "Reg %hhu: 0x%16.16llx - 0x%16.16llx
>> (%6lld %cB) %s \n",
>> entry->reg,
>> (unsigned long long int)entry->start,
>> (unsigned long long int)entry->end,
>> @@ -496,8 +516,10 @@ static int mtrr_init(fwts_framework *fw)
>> if (fwts_check_executable(fw, fw->lspci, "lspci"))
>> return FWTS_ERROR;
>>
>> - if (get_mtrrs())
>> + if (get_mtrrs() != FWTS_OK) {
>> fwts_log_error(fw, "Failed to read /proc/mtrr.");
>> + return FWTS_ERROR;
>> + }
>>
>> if (access("/proc/mtrr", R_OK))
>> return FWTS_ERROR;
>
> The patch fixes the zero's > 4GB on my machine
>
> Acked by: Alex Hung <alex.hung at canonical.com>
>
Acked-by: Keng-Yu Lin <kengyu at canonical.com>
More information about the fwts-devel
mailing list