[coreboot] Endian issue for libpayload options

Sean McNeil seanmcneil3 at gmail.com
Thu May 29 11:37:35 CEST 2014


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
>




More information about the coreboot mailing list