summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorZheng Bao <fishbaozi@gmail.com>2013-12-04 10:22:25 +0800
committerZheng Bao <zheng.bao@amd.com>2013-12-05 03:52:24 +0100
commitabd119d28fae43a98b75824c974519bec9100299 (patch)
tree321a209c305f67d98fe8b5122de8ebeb89d74edf
parentf589909b91e30cb826b18cf7a46299649edc53d6 (diff)
downloadcoreboot-abd119d28fae43a98b75824c974519bec9100299.tar.xz
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)
-rw-r--r--src/vendorcode/amd/agesa/f12/Proc/Fch/Imc/ImcLib.c6
-rw-r--r--src/vendorcode/amd/agesa/f15tn/Proc/Fch/Imc/ImcLib.c6
-rw-r--r--src/vendorcode/amd/agesa/f16kb/Proc/Fch/Imc/ImcLib.c6
3 files changed, 12 insertions, 6 deletions
diff --git a/src/vendorcode/amd/agesa/f12/Proc/Fch/Imc/ImcLib.c b/src/vendorcode/amd/agesa/f12/Proc/Fch/Imc/ImcLib.c
index 1a3f7dd2f5..8795f0b37c 100644
--- a/src/vendorcode/amd/agesa/f12/Proc/Fch/Imc/ImcLib.c
+++ b/src/vendorcode/amd/agesa/f12/Proc/Fch/Imc/ImcLib.c
@@ -55,7 +55,8 @@ WriteECmsg (
{
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;
for (Index = 0; Index <= OpFlag; Index++) {
@@ -77,7 +78,8 @@ ReadECmsg (
{
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;
for (Index = 0; Index <= OpFlag; Index++) {
diff --git a/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Imc/ImcLib.c b/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Imc/ImcLib.c
index a451c41fcd..3e3bf5c04e 100644
--- a/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Imc/ImcLib.c
+++ b/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Imc/ImcLib.c
@@ -55,7 +55,8 @@ WriteECmsg (
{
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;
}
@@ -79,7 +80,8 @@ ReadECmsg (
{
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;
}
diff --git a/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Imc/ImcLib.c b/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Imc/ImcLib.c
index 05f57278da..f911632232 100644
--- a/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Imc/ImcLib.c
+++ b/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Imc/ImcLib.c
@@ -66,7 +66,8 @@ WriteECmsg (
{
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;
}
@@ -101,7 +102,8 @@ ReadECmsg (
{
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;
}