Re: [coreboot] Endian issue for libpayload options

[Sean McNeil]

I think the following patch should be OK for either endian:
diff --git a/payloads/libpayload/drivers/options.c 
b/payloads/libpayload/drivers
index 70c2b17..ae5e421 100644
--- a/payloads/libpayload/drivers/options.c
+++ b/payloads/libpayload/drivers/options.c
@@ -63,15 +63,20 @@ int options_checksum_valid(const struct 
nvram_accessor *nvra
        int range_start = lib_sysinfo.cmos_range_start / 8;
        int range_end = lib_sysinfo.cmos_range_end / 8;
        int checksum_location = lib_sysinfo.cmos_checksum_location / 8;
-       u16 checksum = 0, checksum_old;
+       u16 checksum = 0;
+       union {
+               u16 w;
+               u8 b[2];
+       } checksum_old;

        for(i = range_start; i <= range_end; i++) {
                checksum += nvram->read(i);
        }

-       checksum_old = ((nvram->read(checksum_location)<<8) | 
nvram->read(checks
+       checksum_old.b[0] = nvram->read(checksum_location);
+       checksum_old.b[1] = nvram->read(checksum_location+1);

-       return (checksum_old == checksum);
+       return (checksum_old.w == checksum);
 }

 void fix_options_checksum_with(const struct nvram_accessor *nvram)


On 05/29/2014 03:57 PM, Sean McNeil wrote:
options are broken for x86 because it is assuming big endian. A quick 
patch to make it work:

diff --git a/payloads/libpayload/drivers/options.c 
b/payloads/libpayload/drivers
index 70c2b17..a7a5d82 100644
--- a/payloads/libpayload/drivers/options.c
+++ b/payloads/libpayload/drivers/options.c
@@ -69,7 +69,7 @@ int options_checksum_valid(const struct 
nvram_accessor *nvram)
                checksum += nvram->read(i);
        }

-       checksum_old = ((nvram->read(checksum_location)<<8) | 
nvram->read(checks
+       checksum_old = ((nvram->read(checksum_location+1)<<8) | 
nvram->read(chec

        return (checksum_old == checksum);
 }

Maybe a union and assigning each byte is the correct approach?

Cheers,
Sean



--
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot