Message ID | 1327917811-14753-1-git-send-email-alex.hung@canonical.com |
---|---|
State | Rejected |
Headers | show |
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.
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;
Signed-off-by: Alex Hung <alex.hung@canonical.com> --- src/cpu/msr/msr.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)