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

Alex Hung alex.hung at canonical.com
Tue Jan 31 01:53:28 UTC 2012


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 at 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.






More information about the fwts-devel mailing list