[3/3] bios: mtrr: redefine the end point of memory ranges

Currently the end of an entry partition is calculated as

   entry->end = entry->start + entry->size

However, this leads to memory _ranges_ that are off by one byte at the end.
For instance, if we had a range starting at 0x10 with a size of 0x10, the
range should be [0x10, 0x1f]. Thus, entry->start should be 0x10 and
entry->end should be 0x1f.

Furthermore, Linux determines the size of the MTRR range by looking at the
contents of the MTRR_PHYSMASKn registers. These registers are set in
such a manner that a mask, not a size, determines whether a memory location
belongs to the MTRR memory range [1].

The entry->start and entry->end values are used for two purposes:

  1) Display the memory _ranges_ covered by each of the MTRR registers, and
  2) Determine whether a particular memory range is covered by any of the
     MTRR registers.

For 1), redefining the value of entry->end a) represents better what the
MTRR registers actually mean and b) make the printout of such ranges
similar to what the /proc/iomem file presents, which is used as input for
the MTRR tests.

For 2), the cache_type function needs to be updated for this new definition
of entry->end. Simple algebra shows how the inequalities need to be
adjusted. Let A, A' and B integers where A' = A - 1. If we have

                                  A > B
                             A' + 1 > B

then we can drop the second member of the expression at the left by using a
greater-than sign. This can be done as involved quantities are integers. We
can simply do

                                 A' >= B

[1]. http://www.intel.com/content/www/us/en/processors
     /architectures-software-developer-manuals.html

Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
 src/bios/mtrr/mtrr.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

On 27/04/16 21:11, Ricardo Neri wrote:
> Currently the end of an entry partition is calculated as
> 
>    entry->end = entry->start + entry->size
> 
> However, this leads to memory _ranges_ that are off by one byte at the end.
> For instance, if we had a range starting at 0x10 with a size of 0x10, the
> range should be [0x10, 0x1f]. Thus, entry->start should be 0x10 and
> entry->end should be 0x1f.
> 
> Furthermore, Linux determines the size of the MTRR range by looking at the
> contents of the MTRR_PHYSMASKn registers. These registers are set in
> such a manner that a mask, not a size, determines whether a memory location
> belongs to the MTRR memory range [1].
> 
> The entry->start and entry->end values are used for two purposes:
> 
>   1) Display the memory _ranges_ covered by each of the MTRR registers, and
>   2) Determine whether a particular memory range is covered by any of the
>      MTRR registers.
> 
> For 1), redefining the value of entry->end a) represents better what the
> MTRR registers actually mean and b) make the printout of such ranges
> similar to what the /proc/iomem file presents, which is used as input for
> the MTRR tests.
> 
> For 2), the cache_type function needs to be updated for this new definition
> of entry->end. Simple algebra shows how the inequalities need to be
> adjusted. Let A, A' and B integers where A' = A - 1. If we have
> 
>                                   A > B
>                              A' + 1 > B
> 
> then we can drop the second member of the expression at the left by using a
> greater-than sign. This can be done as involved quantities are integers. We
> can simply do
> 
>                                  A' >= B
> 
> [1]. http://www.intel.com/content/www/us/en/processors
>      /architectures-software-developer-manuals.html
> 
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> ---
>  src/bios/mtrr/mtrr.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c
> index 8176803..120b943 100644
> --- a/src/bios/mtrr/mtrr.c
> +++ b/src/bios/mtrr/mtrr.c
> @@ -144,7 +144,7 @@ static int get_mtrrs(void)
>  		if (ptr2 && (*ptr2 == 'k'))
>  			entry->size *= 1024;
>  
> -		entry->end = entry->start + entry->size;
> +		entry->end = entry->start + entry->size - 1;
>  
>  		if (strstr(line, "write-back"))
>  			entry->type = WRITE_BACK;
> @@ -174,7 +174,7 @@ static int cache_types(uint64_t start, uint64_t end)
>  	fwts_list_foreach(item, mtrr_list) {
>  		entry = fwts_list_data(struct mtrr_entry*, item);
>  
> -		if (entry->end > start && entry->start <= end)
> +		if (entry->end >= start && entry->start <= end)
>  			type |= entry->type;
>  	}
>  
> @@ -186,7 +186,7 @@ restart:
>  	fwts_list_foreach(item, mtrr_list) {
>  		entry = fwts_list_data(struct mtrr_entry*, item);
>  
> -		if (entry->end > end && entry->start < end) {
> +		if (entry->end >= end && entry->start < end) {
>  			end = entry->start;
>  			if (end < start)
>  				end = start;
> 
Acked-by: Colin Ian King <colin.king@canonical.com>

On 2016-04-28 04:11 AM, Ricardo Neri wrote:
> Currently the end of an entry partition is calculated as
>
>     entry->end = entry->start + entry->size
>
> However, this leads to memory _ranges_ that are off by one byte at the end.
> For instance, if we had a range starting at 0x10 with a size of 0x10, the
> range should be [0x10, 0x1f]. Thus, entry->start should be 0x10 and
> entry->end should be 0x1f.
>
> Furthermore, Linux determines the size of the MTRR range by looking at the
> contents of the MTRR_PHYSMASKn registers. These registers are set in
> such a manner that a mask, not a size, determines whether a memory location
> belongs to the MTRR memory range [1].
>
> The entry->start and entry->end values are used for two purposes:
>
>    1) Display the memory _ranges_ covered by each of the MTRR registers, and
>    2) Determine whether a particular memory range is covered by any of the
>       MTRR registers.
>
> For 1), redefining the value of entry->end a) represents better what the
> MTRR registers actually mean and b) make the printout of such ranges
> similar to what the /proc/iomem file presents, which is used as input for
> the MTRR tests.
>
> For 2), the cache_type function needs to be updated for this new definition
> of entry->end. Simple algebra shows how the inequalities need to be
> adjusted. Let A, A' and B integers where A' = A - 1. If we have
>
>                                    A > B
>                               A' + 1 > B
>
> then we can drop the second member of the expression at the left by using a
> greater-than sign. This can be done as involved quantities are integers. We
> can simply do
>
>                                   A' >= B
>
> [1]. http://www.intel.com/content/www/us/en/processors
>       /architectures-software-developer-manuals.html
>
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> ---
>   src/bios/mtrr/mtrr.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c
> index 8176803..120b943 100644
> --- a/src/bios/mtrr/mtrr.c
> +++ b/src/bios/mtrr/mtrr.c
> @@ -144,7 +144,7 @@ static int get_mtrrs(void)
>   		if (ptr2 && (*ptr2 == 'k'))
>   			entry->size *= 1024;
>
> -		entry->end = entry->start + entry->size;
> +		entry->end = entry->start + entry->size - 1;
>
>   		if (strstr(line, "write-back"))
>   			entry->type = WRITE_BACK;
> @@ -174,7 +174,7 @@ static int cache_types(uint64_t start, uint64_t end)
>   	fwts_list_foreach(item, mtrr_list) {
>   		entry = fwts_list_data(struct mtrr_entry*, item);
>
> -		if (entry->end > start && entry->start <= end)
> +		if (entry->end >= start && entry->start <= end)
>   			type |= entry->type;
>   	}
>
> @@ -186,7 +186,7 @@ restart:
>   	fwts_list_foreach(item, mtrr_list) {
>   		entry = fwts_list_data(struct mtrr_entry*, item);
>
> -		if (entry->end > end && entry->start < end) {
> +		if (entry->end >= end && entry->start < end) {
>   			end = entry->start;
>   			if (end < start)
>   				end = start;
>

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