AMD IMC AGESA: Access the data in stack by correct length

The bug is hard to find. We were adding the feature of fan control. We
met some strange things which could not be explained. Like, sometimes
adding printk let the error disappear. Then we traced the code by hardware
debug tool (HDT). It turned out the data in stack was overwritten.

The values of AccessWidthxx are
{ AccessWidth8 = 1,
  AccessWidth16,
  AccessWidth32,}
For the case of AccessWidth8, we only need to access the index/data
once. But ReadECmsg and WriteECmsg did the loop twice, 1 more time
than they are supposed to do. The data in stack next to "Value" would
be overwritten.

For all the cases, the code should be
 OpFlag = OpFlag & 0x7f;
 switch (OpFlag) {
    case 1:              /* AccessWidth8 */
         OpFlag = 0;break;
    case 2:              /* AccessWidth16 */
         OpFlag = 1;break;
    case 3:              /* AccessWidth32 */
         OpFlag = 3;break;
    case 4:              /* AccessWidth64 */
         OpFlag = 7;break;
    default:
         error;
 }

Actually, the caller only takes AccessWidth8 as the parameter. We can ignore other
cases for now.

That is an AGESA bug. AMD's AGESA team own this code. They have given the
response that they are going to update this in next release. I presume let them
decide the proper way to fix that. Before that, I change the code as little
as possible to make it run without crash.

Change-Id: I566f74c242ce93f4569eedf69ca07d2fb7fb368d
Signed-off-by: Zheng Bao <zheng.bao@amd.com>
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: http://review.coreboot.org/4297
Reviewed-by: Bruce Griffith <Bruce.Griffith@se-eng.com>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Tested-by: build bot (Jenkins)
This commit is contained in:
Zheng Bao 2013-12-04 10:22:25 +08:00 committed by Zheng Bao
parent f589909b91
commit abd119d28f
3 changed files with 12 additions and 6 deletions

View File

@ -55,7 +55,8 @@ WriteECmsg (
{ {
UINT8 Index; UINT8 Index;
OpFlag = OpFlag & 0x7f; ASSERT (OpFlag < AccessWidth64); /* TODO: Add the assertion to make it not crash for now. */
OpFlag = (OpFlag & 0x7f) - 1;
if (OpFlag == 0x02) OpFlag = 0x03; if (OpFlag == 0x02) OpFlag = 0x03;
for (Index = 0; Index <= OpFlag; Index++) { for (Index = 0; Index <= OpFlag; Index++) {
@ -77,7 +78,8 @@ ReadECmsg (
{ {
UINT8 Index; UINT8 Index;
OpFlag = OpFlag & 0x7f; ASSERT (OpFlag < AccessWidth64); /* TODO: Add the assertion to make it not crash for now. */
OpFlag = (OpFlag & 0x7f) - 1;
if (OpFlag == 0x02) OpFlag = 0x03; if (OpFlag == 0x02) OpFlag = 0x03;
for (Index = 0; Index <= OpFlag; Index++) { for (Index = 0; Index <= OpFlag; Index++) {

View File

@ -55,7 +55,8 @@ WriteECmsg (
{ {
UINT8 Index; UINT8 Index;
OpFlag = OpFlag & 0x7f; ASSERT (OpFlag < AccessWidth64); /* TODO: Add the assertion to make it not crash for now. */
OpFlag = (OpFlag & 0x7f) - 1;
if (OpFlag == 0x02) { if (OpFlag == 0x02) {
OpFlag = 0x03; OpFlag = 0x03;
} }
@ -79,7 +80,8 @@ ReadECmsg (
{ {
UINT8 Index; UINT8 Index;
OpFlag = OpFlag & 0x7f; ASSERT (OpFlag < AccessWidth64); /* TODO: Add the assertion to make it not crash for now. */
OpFlag = (OpFlag & 0x7f) - 1;
if (OpFlag == 0x02) { if (OpFlag == 0x02) {
OpFlag = 0x03; OpFlag = 0x03;
} }

View File

@ -66,7 +66,8 @@ WriteECmsg (
{ {
UINT8 Index; UINT8 Index;
OpFlag = OpFlag & 0x7f; ASSERT (OpFlag < AccessWidth64); /* TODO: Add the assertion to make it not crash for now. */
OpFlag = (OpFlag & 0x7f) - 1;
if (OpFlag == 0x02) { if (OpFlag == 0x02) {
OpFlag = 0x03; OpFlag = 0x03;
} }
@ -101,7 +102,8 @@ ReadECmsg (
{ {
UINT8 Index; UINT8 Index;
OpFlag = OpFlag & 0x7f; ASSERT (OpFlag < AccessWidth64); /* TODO: Add the assertion to make it not crash for now. */
OpFlag = (OpFlag & 0x7f) - 1;
if (OpFlag == 0x02) { if (OpFlag == 0x02) {
OpFlag = 0x03; OpFlag = 0x03;
} }