cpu:msr: fixed SMRR_PHYSBASE boundry check and incorrect offset for SMRR_PHYSMASK

Signed-off-by: Alex Hung <alex.hung@canonical.com>
---
 src/cpu/msr/msr.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

On 30/01/12 10:03, Alex Hung wrote:
> Signed-off-by: Alex Hung<alex.hung@canonical.com>
> ---
>   src/cpu/msr/msr.c |    4 ++--
>   1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/cpu/msr/msr.c b/src/cpu/msr/msr.c
> index 06a756f..f84c7a6 100644
> --- a/src/cpu/msr/msr.c
> +++ b/src/cpu/msr/msr.c
> @@ -194,14 +194,14 @@ static int msr_smrr(fwts_framework *fw)
>   			if (fwts_cpu_readmsr(0, 0x1f2,&val) == FWTS_OK) {
>   				uint64_t physbase = val&  0xfffff000;
>   				uint64_t type = val&  7;
> -				if ((physbase % 0x7fffff) != 0)
> +				if ((physbase&  0x7fffff) != 0)
>   					fwts_failed(fw, LOG_LEVEL_HIGH, "MSRSMRR_PHYSBASE8MBBoundary",
>   						"SMRR: SMRR_PHYSBASE is NOT on an 8MB boundary: %llx.", (unsigned long long)physbase);
>   				if (type != 6)
>   					fwts_failed(fw, LOG_LEVEL_HIGH, "MSRSMRR_TYPE",
>   						"SMRR: SMRR_TYPE is 0x%x, should be 0x6 (Write-Back).", (int)type);
>   			}
> -			if (fwts_cpu_readmsr(0, 0x1f2,&val) == FWTS_OK) {
> +			if (fwts_cpu_readmsr(0, 0x1f3,&val) == FWTS_OK) {
>   				uint64_t physmask = val&  0xfffff000;
>   				uint64_t valid = (val>>  11)&  1;
>

This fixes the stupid typos I introduced into the code - the base 
address is now being correctly checked for the 8M boundary and the 
PHYSMASK MSR 0x1f3 is the correct one.

However, minor style issue: can this patch be re-worked:

if ((physbase&  0x7fffff) != 0)

to:

if ((physbase & 0x7fffff) != 0)

Thanks Alex for spotting these bugs

On 01/30/2012 06:34 PM, Colin Ian King wrote:
> On 30/01/12 10:03, Alex Hung wrote:
>> Signed-off-by: Alex Hung<alex.hung@canonical.com>
>> ---
>>   src/cpu/msr/msr.c |    4 ++--
>>   1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/cpu/msr/msr.c b/src/cpu/msr/msr.c
>> index 06a756f..f84c7a6 100644
>> --- a/src/cpu/msr/msr.c
>> +++ b/src/cpu/msr/msr.c
>> @@ -194,14 +194,14 @@ static int msr_smrr(fwts_framework *fw)
>>               if (fwts_cpu_readmsr(0, 0x1f2,&val) == FWTS_OK) {
>>                   uint64_t physbase = val&  0xfffff000;
>>                   uint64_t type = val&  7;
>> -                if ((physbase % 0x7fffff) != 0)
>> +                if ((physbase&  0x7fffff) != 0)
>>                       fwts_failed(fw, LOG_LEVEL_HIGH, 
>> "MSRSMRR_PHYSBASE8MBBoundary",
>>                           "SMRR: SMRR_PHYSBASE is NOT on an 8MB 
>> boundary: %llx.", (unsigned long long)physbase);
>>                   if (type != 6)
>>                       fwts_failed(fw, LOG_LEVEL_HIGH, "MSRSMRR_TYPE",
>>                           "SMRR: SMRR_TYPE is 0x%x, should be 0x6 
>> (Write-Back).", (int)type);
>>               }
>> -            if (fwts_cpu_readmsr(0, 0x1f2,&val) == FWTS_OK) {
>> +            if (fwts_cpu_readmsr(0, 0x1f3,&val) == FWTS_OK) {
>>                   uint64_t physmask = val&  0xfffff000;
>>                   uint64_t valid = (val>>  11)&  1;
>>
>
> This fixes the stupid typos I introduced into the code - the base 
> address is now being correctly checked for the 8M boundary and the 
> PHYSMASK MSR 0x1f3 is the correct one.
>
> However, minor style issue: can this patch be re-worked:
>
> if ((physbase&  0x7fffff) != 0)
>
> to:
>
> if ((physbase & 0x7fffff) != 0)
>
> Thanks Alex for spotting these bugs
>
Hi Colin,

I checked the patch file and the spacing of *&* sign looks correct, i.e. 
if ((physbase & 0x7fffff) != 0) here; however, I spotted a spelling 
error at the changelog and I will a new patch again.