coreboot

coreboot@coreboot.org

6 participants
39686 discussions

New Defects reported by Coverity Scan for coreboot
by scan-admin＠coverity.com 25 Nov '22

25 Nov '22

Hi, Please find the latest report on new defect(s) introduced to coreboot found with Coverity Scan. 13 new defect(s) introduced to coreboot found with Coverity Scan. 1 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan. New defect(s) Reported-by: Coverity Scan Showing 13 of 13 defect(s) ** CID 1501089: Null pointer dereferences (FORWARD_NULL) /src/ec/clevo/it5570e/ec.c: 37 in pnp_configure_smfi() ________________________________________________________________________________________________________ *** CID 1501089: Null pointer dereferences (FORWARD_NULL) /src/ec/clevo/it5570e/ec.c: 37 in pnp_configure_smfi() 31 32 struct device dev = { 33 .path.type = DEVICE_PATH_PNP, 34 .path.pnp.port = 0x2e, 35 .path.pnp.device = IT5570E_SMFI, 36 }; >>> CID 1501089: Null pointer dereferences (FORWARD_NULL) >>> Dereferencing null pointer "dev.ops". 37 dev.ops->ops_pnp_mode = &pnp_conf_mode_870155_aa; 38 39 /* Configure SMFI for LGMR */ 40 pnp_enter_conf_mode(&dev); 41 pnp_set_logical_device(&dev); 42 pnp_set_enable(&dev, 1); ** CID 1501088: Code maintainability issues (UNUSED_VALUE) /src/cpu/intel/slot_1/l2_cache.c: 152 in calculate_l2_latency() ________________________________________________________________________________________________________ *** CID 1501088: Code maintainability issues (UNUSED_VALUE) /src/cpu/intel/slot_1/l2_cache.c: 152 in calculate_l2_latency() 146 printk(BIOS_DEBUG, "L2 latency type = %x\n", t); 147 148 /* Get CPUID family/model */ 149 signature = cpuid_eax(1) & 0xfff0; 150 151 /* Read EBL_CR_POWERON */ >>> CID 1501088: Code maintainability issues (UNUSED_VALUE) >>> Assigning value from "rdmsr(42U)" to "msr" here, but that stored value is overwritten before it can be used. 152 msr = rdmsr(EBL_CR_POWERON); 153 /* Get clock multiplier and FSB frequency. 154 * Multiplier is in [25:22]. 155 * FSB is in [19:18] in Katmai, [19] in Deschutes ([18] is zero 156 * for them). 157 */ ** CID 1501087: (STRING_OVERFLOW) ________________________________________________________________________________________________________ *** CID 1501087: (STRING_OVERFLOW) /src/arch/x86/smbios.c: 1138 in smbios_write_type39() 1132 1133 t->power_unit_group = unit_group; 1134 t->location = smbios_add_string(t->eos, loc); 1135 t->device_name = smbios_add_string(t->eos, dev_name); 1136 t->manufacturer = smbios_add_string(t->eos, man); 1137 t->serial_number = smbios_add_string(t->eos, serial_num); >>> CID 1501087: (STRING_OVERFLOW) >>> You might overrun the 2-character fixed-size string "t->eos" by copying "tag_num" without checking the length. 1138 t->asset_tag_number = smbios_add_string(t->eos, tag_num); 1139 t->model_part_number = smbios_add_string(t->eos, part_num); 1140 t->revision_level = smbios_add_string(t->eos, rev_lvl); 1141 t->max_power_capacity = max_pow_cap; 1142 1143 ps_type = ps_ch->power_supply_type & 0xF; /src/arch/x86/smbios.c: 1140 in smbios_write_type39() 1134 t->location = smbios_add_string(t->eos, loc); 1135 t->device_name = smbios_add_string(t->eos, dev_name); 1136 t->manufacturer = smbios_add_string(t->eos, man); 1137 t->serial_number = smbios_add_string(t->eos, serial_num); 1138 t->asset_tag_number = smbios_add_string(t->eos, tag_num); 1139 t->model_part_number = smbios_add_string(t->eos, part_num); >>> CID 1501087: (STRING_OVERFLOW) >>> You might overrun the 2-character fixed-size string "t->eos" by copying "rev_lvl" without checking the length. 1140 t->revision_level = smbios_add_string(t->eos, rev_lvl); 1141 t->max_power_capacity = max_pow_cap; 1142 1143 ps_type = ps_ch->power_supply_type & 0xF; 1144 ps_status = ps_ch->power_supply_status & 0x7; 1145 vol_switch = ps_ch->input_voltage_range_switch & 0xF; /src/arch/x86/smbios.c: 1134 in smbios_write_type39() 1128 sizeof(*t), *handle); 1129 1130 uint16_t val = 0; 1131 uint16_t ps_type, ps_status, vol_switch, ps_unplug, ps_present, hot_rep; 1132 1133 t->power_unit_group = unit_group; >>> CID 1501087: (STRING_OVERFLOW) >>> You might overrun the 2-character fixed-size string "t->eos" by copying "loc" without checking the length. 1134 t->location = smbios_add_string(t->eos, loc); 1135 t->device_name = smbios_add_string(t->eos, dev_name); 1136 t->manufacturer = smbios_add_string(t->eos, man); 1137 t->serial_number = smbios_add_string(t->eos, serial_num); 1138 t->asset_tag_number = smbios_add_string(t->eos, tag_num); 1139 t->model_part_number = smbios_add_string(t->eos, part_num); /src/arch/x86/smbios.c: 1139 in smbios_write_type39() 1133 t->power_unit_group = unit_group; 1134 t->location = smbios_add_string(t->eos, loc); 1135 t->device_name = smbios_add_string(t->eos, dev_name); 1136 t->manufacturer = smbios_add_string(t->eos, man); 1137 t->serial_number = smbios_add_string(t->eos, serial_num); 1138 t->asset_tag_number = smbios_add_string(t->eos, tag_num); >>> CID 1501087: (STRING_OVERFLOW) >>> You might overrun the 2-character fixed-size string "t->eos" by copying "part_num" without checking the length. 1139 t->model_part_number = smbios_add_string(t->eos, part_num); 1140 t->revision_level = smbios_add_string(t->eos, rev_lvl); 1141 t->max_power_capacity = max_pow_cap; 1142 1143 ps_type = ps_ch->power_supply_type & 0xF; 1144 ps_status = ps_ch->power_supply_status & 0x7; /src/arch/x86/smbios.c: 1135 in smbios_write_type39() 1129 1130 uint16_t val = 0; 1131 uint16_t ps_type, ps_status, vol_switch, ps_unplug, ps_present, hot_rep; 1132 1133 t->power_unit_group = unit_group; 1134 t->location = smbios_add_string(t->eos, loc); >>> CID 1501087: (STRING_OVERFLOW) >>> You might overrun the 2-character fixed-size string "t->eos" by copying "dev_name" without checking the length. 1135 t->device_name = smbios_add_string(t->eos, dev_name); 1136 t->manufacturer = smbios_add_string(t->eos, man); 1137 t->serial_number = smbios_add_string(t->eos, serial_num); 1138 t->asset_tag_number = smbios_add_string(t->eos, tag_num); 1139 t->model_part_number = smbios_add_string(t->eos, part_num); 1140 t->revision_level = smbios_add_string(t->eos, rev_lvl); /src/arch/x86/smbios.c: 1136 in smbios_write_type39() 1130 uint16_t val = 0; 1131 uint16_t ps_type, ps_status, vol_switch, ps_unplug, ps_present, hot_rep; 1132 1133 t->power_unit_group = unit_group; 1134 t->location = smbios_add_string(t->eos, loc); 1135 t->device_name = smbios_add_string(t->eos, dev_name); >>> CID 1501087: (STRING_OVERFLOW) >>> You might overrun the 2-character fixed-size string "t->eos" by copying "man" without checking the length. 1136 t->manufacturer = smbios_add_string(t->eos, man); 1137 t->serial_number = smbios_add_string(t->eos, serial_num); 1138 t->asset_tag_number = smbios_add_string(t->eos, tag_num); 1139 t->model_part_number = smbios_add_string(t->eos, part_num); 1140 t->revision_level = smbios_add_string(t->eos, rev_lvl); 1141 t->max_power_capacity = max_pow_cap; /src/arch/x86/smbios.c: 1137 in smbios_write_type39() 1131 uint16_t ps_type, ps_status, vol_switch, ps_unplug, ps_present, hot_rep; 1132 1133 t->power_unit_group = unit_group; 1134 t->location = smbios_add_string(t->eos, loc); 1135 t->device_name = smbios_add_string(t->eos, dev_name); 1136 t->manufacturer = smbios_add_string(t->eos, man); >>> CID 1501087: (STRING_OVERFLOW) >>> You might overrun the 2-character fixed-size string "t->eos" by copying "serial_num" without checking the length. 1137 t->serial_number = smbios_add_string(t->eos, serial_num); 1138 t->asset_tag_number = smbios_add_string(t->eos, tag_num); 1139 t->model_part_number = smbios_add_string(t->eos, part_num); 1140 t->revision_level = smbios_add_string(t->eos, rev_lvl); 1141 t->max_power_capacity = max_pow_cap; 1142 ** CID 1501086: API usage errors (BAD_COMPARE) /src/device/oprom/yabel/biosemu.c: 200 in biosemu() ________________________________________________________________________________________________________ *** CID 1501086: API usage errors (BAD_COMPARE) /src/device/oprom/yabel/biosemu.c: 200 in biosemu() 194 clr_ci(); 195 my_wrb((uintptr_t)mem_img + i, c); 196 } 197 #endif 198 copy_count++; 199 set_ci(); >>> CID 1501086: API usage errors (BAD_COMPARE) >>> Truncating the result of "memcmp" to "unsigned char" may cause it to be misinterpreted as 0. Note that "memcmp" may return an integer besides -1, 0, or 1. 200 cmp_result = memcmp(mem_img, rom_image, bios_device.img_size); 201 clr_ci(); 202 } 203 while ((copy_count < 5) && (cmp_result != 0)); 204 if (cmp_result != 0) { 205 printf ** CID 1501085: Control flow issues (DEADCODE) /src/device/oprom/yabel/interrupt.c: 299 in handleInt16() ________________________________________________________________________________________________________ *** CID 1501085: Control flow issues (DEADCODE) /src/device/oprom/yabel/interrupt.c: 299 in handleInt16() 293 /* TODO: we need getchar... */ 294 c = -1; //getchar(); 295 if (c == -1) { 296 // no key available 297 SET_FLAG(F_ZF); 298 } else { >>> CID 1501085: Control flow issues (DEADCODE) >>> Execution cannot reach this statement: "*keycode = c;". 299 *keycode = c; 300 301 // since after an ESC it may take a while to receive the next char, 302 // we send something that is not shown on the screen, and then try to get 303 // the next char 304 // TODO: only after ESC?? what about other multibyte keys ** CID 1501084: (UNUSED_VALUE) /src/device/oprom/x86emu/decode.c: 829 in decode_sib_address() /src/device/oprom/x86emu/decode.c: 834 in decode_sib_address() ________________________________________________________________________________________________________ *** CID 1501084: (UNUSED_VALUE) /src/device/oprom/x86emu/decode.c: 829 in decode_sib_address() 823 DECODE_PRINTF2("[%d]", displacement); 824 offset = displacement; 825 break; 826 case 1: 827 displacement = (s8)fetch_byte_imm(); 828 DECODE_PRINTF2("[%d][EBP]", displacement); >>> CID 1501084: (UNUSED_VALUE) >>> Assigning value from "_X86EMU_env.x86.spc.BP.I32_reg.e_reg + displacement" to "offset" here, but that stored value is overwritten before it can be used. 829 offset = M.x86.R_EBP + displacement; 830 break; 831 case 2: 832 displacement = (s32)fetch_long_imm(); 833 DECODE_PRINTF2("[%d][EBP]", displacement); 834 offset = M.x86.R_EBP + displacement; /src/device/oprom/x86emu/decode.c: 834 in decode_sib_address() 828 DECODE_PRINTF2("[%d][EBP]", displacement); 829 offset = M.x86.R_EBP + displacement; 830 break; 831 case 2: 832 displacement = (s32)fetch_long_imm(); 833 DECODE_PRINTF2("[%d][EBP]", displacement); >>> CID 1501084: (UNUSED_VALUE) >>> Assigning value from "_X86EMU_env.x86.spc.BP.I32_reg.e_reg + displacement" to "offset" here, but that stored value is overwritten before it can be used. 834 offset = M.x86.R_EBP + displacement; 835 break; 836 default: 837 HALT_SYS(); 838 } 839 DECODE_PRINTF("[EAX]"); ** CID 1501083: Null pointer dereferences (FORWARD_NULL) /src/device/oprom/yabel/biosemu.c: 246 in biosemu() ________________________________________________________________________________________________________ *** CID 1501083: Null pointer dereferences (FORWARD_NULL) /src/device/oprom/yabel/biosemu.c: 246 in biosemu() 240 my_wrl(0x1a * 4, 0xf000fe6e); 241 my_wrb(0x000ffe6e, 0xcf); 242 243 // setup BIOS Data Area (0000:04xx, or 0040:00xx) 244 // we currently 0 this area, meaning "we don't have 245 // any hardware" :-) no serial/parallel ports, floppys, ... >>> CID 1501083: Null pointer dereferences (FORWARD_NULL) >>> Passing null pointer "biosmem + 1024" to "memset", which dereferences it. [Note: The source code implementation of the function has been overridden by a builtin model.] 246 memset(biosmem + 0x400, 0x0, 0x100); 247 248 // at offset 13h in BDA is the memory size in kbytes 249 my_wrw(0x413, biosmem_size / 1024); 250 // at offset 0eh in BDA is the segment of the Extended BIOS Data Area 251 // see setup further down ** CID 1299819: (BAD_SHIFT) /src/device/oprom/x86emu/prim_ops.c: 2136 in idiv_long() /src/device/oprom/x86emu/prim_ops.c: 2136 in idiv_long() /src/device/oprom/x86emu/prim_ops.c: 2143 in idiv_long() /src/device/oprom/x86emu/prim_ops.c: 2136 in idiv_long() ________________________________________________________________________________________________________ *** CID 1299819: (BAD_SHIFT) /src/device/oprom/x86emu/prim_ops.c: 2136 in idiv_long() 2130 do { 2131 div <<= 1; 2132 carry = (l_dvd >= l_s) ? 0 : 1; 2133 2134 if (abs_h_dvd < (h_s + carry)) { 2135 h_s >>= 1; >>> CID 1299819: (BAD_SHIFT) >>> In expression "abs_s << --counter", shifting by a negative amount has undefined behavior. The shift amount, "--counter", is as little as -1. 2136 l_s = abs_s << (--counter); 2137 continue; 2138 } else { 2139 abs_h_dvd -= (h_s + carry); 2140 l_dvd = carry ? ((0xFFFFFFFF - l_s) + l_dvd + 1) 2141 : (l_dvd - l_s); /src/device/oprom/x86emu/prim_ops.c: 2136 in idiv_long() 2130 do { 2131 div <<= 1; 2132 carry = (l_dvd >= l_s) ? 0 : 1; 2133 2134 if (abs_h_dvd < (h_s + carry)) { 2135 h_s >>= 1; >>> CID 1299819: (BAD_SHIFT) >>> In expression "abs_s << --counter", shifting by a negative amount has undefined behavior. The shift amount, "--counter", is as little as -1. 2136 l_s = abs_s << (--counter); 2137 continue; 2138 } else { 2139 abs_h_dvd -= (h_s + carry); 2140 l_dvd = carry ? ((0xFFFFFFFF - l_s) + l_dvd + 1) 2141 : (l_dvd - l_s); /src/device/oprom/x86emu/prim_ops.c: 2143 in idiv_long() 2137 continue; 2138 } else { 2139 abs_h_dvd -= (h_s + carry); 2140 l_dvd = carry ? ((0xFFFFFFFF - l_s) + l_dvd + 1) 2141 : (l_dvd - l_s); 2142 h_s >>= 1; >>> CID 1299819: (BAD_SHIFT) >>> In expression "abs_s << --counter", shifting by a negative amount has undefined behavior. The shift amount, "--counter", is as little as -1. 2143 l_s = abs_s << (--counter); 2144 div |= 1; 2145 continue; 2146 } 2147 2148 } while (counter > -1); /src/device/oprom/x86emu/prim_ops.c: 2136 in idiv_long() 2130 do { 2131 div <<= 1; 2132 carry = (l_dvd >= l_s) ? 0 : 1; 2133 2134 if (abs_h_dvd < (h_s + carry)) { 2135 h_s >>= 1; >>> CID 1299819: (BAD_SHIFT) >>> In expression "abs_s << --counter", shifting by a negative amount has undefined behavior. The shift amount, "--counter", is as little as -1. 2136 l_s = abs_s << (--counter); 2137 continue; 2138 } else { 2139 abs_h_dvd -= (h_s + carry); 2140 l_dvd = carry ? ((0xFFFFFFFF - l_s) + l_dvd + 1) 2141 : (l_dvd - l_s); ** CID 1299818: (BAD_SHIFT) /src/device/oprom/x86emu/prim_ops.c: 2260 in div_long() /src/device/oprom/x86emu/prim_ops.c: 2260 in div_long() /src/device/oprom/x86emu/prim_ops.c: 2260 in div_long() /src/device/oprom/x86emu/prim_ops.c: 2267 in div_long() ________________________________________________________________________________________________________ *** CID 1299818: (BAD_SHIFT) /src/device/oprom/x86emu/prim_ops.c: 2260 in div_long() 2254 do { 2255 div <<= 1; 2256 carry = (l_dvd >= l_s) ? 0 : 1; 2257 2258 if (h_dvd < (h_s + carry)) { 2259 h_s >>= 1; >>> CID 1299818: (BAD_SHIFT) >>> In expression "s << --counter", shifting by a negative amount has undefined behavior. The shift amount, "--counter", is as little as -1. 2260 l_s = s << (--counter); 2261 continue; 2262 } else { 2263 h_dvd -= (h_s + carry); 2264 l_dvd = carry ? ((0xFFFFFFFF - l_s) + l_dvd + 1) 2265 : (l_dvd - l_s); /src/device/oprom/x86emu/prim_ops.c: 2260 in div_long() 2254 do { 2255 div <<= 1; 2256 carry = (l_dvd >= l_s) ? 0 : 1; 2257 2258 if (h_dvd < (h_s + carry)) { 2259 h_s >>= 1; >>> CID 1299818: (BAD_SHIFT) >>> In expression "s << --counter", shifting by a negative amount has undefined behavior. The shift amount, "--counter", is as little as -1. 2260 l_s = s << (--counter); 2261 continue; 2262 } else { 2263 h_dvd -= (h_s + carry); 2264 l_dvd = carry ? ((0xFFFFFFFF - l_s) + l_dvd + 1) 2265 : (l_dvd - l_s); /src/device/oprom/x86emu/prim_ops.c: 2260 in div_long() 2254 do { 2255 div <<= 1; 2256 carry = (l_dvd >= l_s) ? 0 : 1; 2257 2258 if (h_dvd < (h_s + carry)) { 2259 h_s >>= 1; >>> CID 1299818: (BAD_SHIFT) >>> In expression "s << --counter", shifting by a negative amount has undefined behavior. The shift amount, "--counter", is as little as -1. 2260 l_s = s << (--counter); 2261 continue; 2262 } else { 2263 h_dvd -= (h_s + carry); 2264 l_dvd = carry ? ((0xFFFFFFFF - l_s) + l_dvd + 1) 2265 : (l_dvd - l_s); /src/device/oprom/x86emu/prim_ops.c: 2267 in div_long() 2261 continue; 2262 } else { 2263 h_dvd -= (h_s + carry); 2264 l_dvd = carry ? ((0xFFFFFFFF - l_s) + l_dvd + 1) 2265 : (l_dvd - l_s); 2266 h_s >>= 1; >>> CID 1299818: (BAD_SHIFT) >>> In expression "s << --counter", shifting by a negative amount has undefined behavior. The shift amount, "--counter", is as little as -1. 2267 l_s = s << (--counter); 2268 div |= 1; 2269 continue; 2270 } 2271 2272 } while (counter > -1); ** CID 1198906: Integer handling issues (BAD_SHIFT) /src/device/oprom/x86emu/prim_ops.c: 835 in rcl_long() ________________________________________________________________________________________________________ *** CID 1198906: Integer handling issues (BAD_SHIFT) /src/device/oprom/x86emu/prim_ops.c: 835 in rcl_long() 829 { 830 u32 res, cnt, mask, cf; 831 832 res = d; 833 if ((cnt = s % 33) != 0) { 834 cf = (d >> (32 - cnt)) & 0x1; >>> CID 1198906: Integer handling issues (BAD_SHIFT) >>> In expression "d << cnt", left shifting by more than 31 bits has undefined behavior. The shift amount, "cnt", is as much as 32. 835 res = (d << cnt) & 0xffffffff; 836 mask = (1 << (cnt - 1)) - 1; 837 res |= (d >> (33 - cnt)) & mask; 838 if (ACCESS_FLAG(F_CF)) { /* carry flag is set */ 839 res |= 1 << (cnt - 1); 840 } ** CID 1198905: Integer handling issues (BAD_SHIFT) /src/device/oprom/x86emu/prim_ops.c: 904 in rcr_byte() ________________________________________________________________________________________________________ *** CID 1198905: Integer handling issues (BAD_SHIFT) /src/device/oprom/x86emu/prim_ops.c: 904 in rcr_byte() 898 This is effectively done by shifting the 899 object to the right. The result must be masked, 900 in case the object came in and was treated 901 as a negative number. Needed??? */ 902 903 mask = (1 << (8 - cnt)) - 1; >>> CID 1198905: Integer handling issues (BAD_SHIFT) >>> In expression "d >> cnt", right shifting "d" by more than 7 bits always yields zero. The shift amount, "cnt", is as much as 8. 904 res = (d >> cnt) & mask; 905 906 /* now the high stuff which rotated around 907 into the positions B_cnt-2 .. B_0 */ 908 /* B_(7) .. B_(8-(n-1)) <- b_(n-2) .. b_(0) */ 909 /* shift it downward, 7-(n-2) = 9-n positions. ** CID 1198904: Integer handling issues (BAD_SHIFT) /src/device/oprom/x86emu/prim_ops.c: 982 in rcr_long() ________________________________________________________________________________________________________ *** CID 1198904: Integer handling issues (BAD_SHIFT) /src/device/oprom/x86emu/prim_ops.c: 982 in rcr_long() 976 if (cnt == 1) { 977 cf = d & 0x1; 978 ocf = ACCESS_FLAG(F_CF) != 0; 979 } else 980 cf = (d >> (cnt - 1)) & 0x1; 981 mask = (1 << (32 - cnt)) - 1; >>> CID 1198904: Integer handling issues (BAD_SHIFT) >>> In expression "d >> cnt", right shifting by more than 31 bits has undefined behavior. The shift amount, "cnt", is as much as 32. 982 res = (d >> cnt) & mask; 983 if (cnt != 1) 984 res |= (d << (33 - cnt)); 985 if (ACCESS_FLAG(F_CF)) { /* carry flag is set */ 986 res |= 1 << (32 - cnt); 987 } ** CID 1198903: Integer handling issues (BAD_SHIFT) /src/device/oprom/x86emu/prim_ops.c: 950 in rcr_word() ________________________________________________________________________________________________________ *** CID 1198903: Integer handling issues (BAD_SHIFT) /src/device/oprom/x86emu/prim_ops.c: 950 in rcr_word() 944 if (cnt == 1) { 945 cf = d & 0x1; 946 ocf = ACCESS_FLAG(F_CF) != 0; 947 } else 948 cf = (d >> (cnt - 1)) & 0x1; 949 mask = (1 << (16 - cnt)) - 1; >>> CID 1198903: Integer handling issues (BAD_SHIFT) >>> In expression "d >> cnt", right shifting "d" by more than 15 bits always yields zero. The shift amount, "cnt", is as much as 16. 950 res = (d >> cnt) & mask; 951 res |= (d << (17 - cnt)); 952 if (ACCESS_FLAG(F_CF)) { 953 res |= 1 << (16 - cnt); 954 } 955 CONDITIONAL_SET_FLAG(cf, F_CF); ________________________________________________________________________________________________________ To view the defects in Coverity Scan visit, https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P…

1 0

New list member..and a project
by Wayne Stuart 21 Nov '22

21 Nov '22

Hi, I just joined this list. I have a project coming up where I need to coreboot a Lenovo W540. I have found a mention of using the t440p setup. I am new to coreboot, though I have used coreboot laptops for a few years. I am looking for direction. Wayne

3 2

Re: coreboot postcode cleanup proposal
by Martin Roth 21 Nov '22

21 Nov '22

The intel cache_as_ram postcodes actually wouldn't go into the bootblock area. The coreboot areas are meant to be common to all platforms. The CAR codes would mostly go into the "SoC specific area", which I've doubled from 16 to 32 codes based on your comment here. ``` * 0x00 : unregulated - 0x00 is not a good postcode. * * 0x01 - 0x07 : general coreboot startup code & common * 0x08 - 0x0b : general coreboot console * 0x0c - 0x0f : <RESERVED> * * 0x10 - 0x1f : general coreboot bootblock * 0x20 - 0x2f : general coreboot romstage * 0x30 - 0x33 : general coreboot postcar * 0x34 - 0x3f : general coreboot ramstage * 0x40 - 0x4f : coreboot ramstage hardwaremain boot stages * * 0x50 - 0x6f : SOC specific - Each SoC/CPU/Chipset can use as it chooses * * 0x70 - 0x7f : <Reserved for expansion> * * 0x80 - 0x87 : ACPI postcodes * 0x88 - 0x8f : Unregulated - may be used anywhere for any purpose. * 0x90 - 0x9f : Vendorcode calls: FSP, AGESA, etc. entry & exit. * * 0xa0 - 0xaf : Mainboard specific - Each mainboard can use as it chooses * * 0xb0 : Driver/SuperIO ID, followed by ID byte (serves as driver entry) * 0xb1 - 0xbf : Driver/SuperIO specific - Can use as the device chooses * * 0xc0 - 0xca : <Reserved for expansion> * * 0xcb : coreboot identifier - used when returning from FSP, AGESA, etc * * 0xcc - 0xcf : <Reserved for expansion> * * 0xd0 : Debug ID, followed by ID byte * 0xd1 - 0xdf : Debug postcodes - never enabled by default. * * 0xe0 - 0xef : Errors * 0xf0 - 0xf7 : general coreboot SMM * 0xf8 - 0xfe : end of coreboot code - resume and the like. * 0xff : coreboot finished - jump to payload ``` I've also moved some reserved area to just after the SoC postcode area in case we find we still need more than 32, we can easily expand it up to 48 postcodes for the SoC or CPU/NB/SB areas. If we find we need to reorganize more in the future, we should be able to do that without too much difficulty in the future. It was suggested in the leadership meeting last week that we could version the postcodes based on releases, but hopefully it'll settle down very shortly, and we won't need to do that. Martin Nov 16, 2022, 15:20 by maximilian.brune(a)9elements.com: > src/soc/intel/common/block/cpu/car/cache_as_ram.S is a candidate which hard codes all post codes (instead of using post_codes.h) and that file alone already uses more than your 16 propsed postcodes for bootblock (I think). Since bootblock is kind of relying on post codes for proper debug (since serial output may not be available at that time), it would make sense to use more post codes for bootblock I guess. > > On Wed, Nov 16, 2022, 17:23 Martin Roth via coreboot <> coreboot(a)coreboot.org> > wrote: > >> The current state of the coreboot postcodes is a mess. There’s no good way to give anyone a postcode table. The post_code() function is called with direct hardcoded values all over the codebase. The post_codes.h file is disorganized with values being added without regard for being contiguous or ordered. As an example, despite using hexadecimal values, console post codes go from 0x39 to 0x40. >> >> >> We can do better than this. >> >> >> Proposal >> -------- >> The postcodes should be better organized and standardized. Each stage should have its own block of postcodes, and they should be easily recognizable as a flow from early coreboot to late coreboot. >> common between blocks: >> - bootblock >> - verstage >> - romstage >> - postcar >> - ramstage >> - smm >> >> We should have blocks of postcodes for different sections as well. >> - SoC/CPU/Chipset >> - Mainboard >> - Drivers >> >> - A set of debug-specific postcodes that may be used in any area, but may not be enabled by default, but only by user request. >> - The debug and driver post codes should have identifiers to show what section of the codebase they're coming from. >> - Finally, there should be small section of uncontrolled values that can be used anywhere that doesn’t have an already allocated specific value. >> >> >> Additional Improvements >> ----------------------- >> - Postcodes should be able to be included in the timestamp table so we can grab timestamps for individual postcodes. >> - Specific Postcodes should have associated text names, similar to the timestamps, and should have a decode tool. >> - Multi-byte postcodes should be implemented for platforms that support them. >> >> >> Proposed postcode ranges >> ------------------------ >> >> ``` >> * 0x00 : unregulated - 0x00 is not a good postcode. >> * >> * 0x01 - 0x07 : coreboot startup code & common >> * 0x08 - 0x0b : coreboot console >> * 0x0c - 0x0f : <RESERVED> >> * >> * 0x10 - 0x1f : coreboot bootblock >> * 0x20 - 0x2f : coreboot romstage >> * 0x30 - 0x33 : postcar >> * 0x34 - 0x3f : coreboot ramstage >> * 0x40 - 0x4f : coreboot ramstage hardwaremain boot stages >> * >> * 0x50 - 0x5f : Mainboard specific - Each mainboard can use as it chooses >> * 0x60 - 0x6f : SOC specific - Each SoC/CPU/Chipset can use as it chooses >> * >> * 0x70 : Driver/SuperIO ID, followed by ID byte >> * 0x71 - 0x7f : Driver/SuperIO specific - Can use as the device chooses >> * >> * 0x80 - 0x87 : ACPI postcodes >> * 0x88 - 0x8f : Unregulated - may be used anywhere for any purpose. >> * 0x90 - 0x9f : Vendorcode calls: FSP, AGESA, etc. >> * >> * 0xa0 - 0xaf : <RESERVED> >> * 0xb0 - 0xbf : <RESERVED> >> * >> * 0xc0 - 0xca : <RESERVED> >> * >> * 0xcb : coreboot identifier - used when returning from FSP, AGESA, etc >> * >> * 0xcc - 0xcf : <RESERVED> >> * >> * 0xd0 - 0xdf : Debug postcodes - never enabled by default. >> * >> * 0xe0 - 0xef : Errors >> * 0xf0 - 0xf7 : SMM >> * 0xf8 - 0xfe : end of coreboot code - resume and the like. >> * 0xff : coreboot finished - jump to payload >> ``` >> >> Driver/SuperIO/Debug IDs >> ------------------------ >> >> Because there are many drivers, and a single board can use multiple of the drivers, we want a way to know which device is putting out postcodes. To handle this, the driver, superIO, or debug section should emit a DRIVER_ID_FOLLOWS or DEBUG_ID_FOLLOWS postcode, followed by the actual ID code. >> >> The ID code would currently be a single byte, with the bytes 0xf0 - 0xff being reserved as page and continue bytes. The continue bytes would tell the code that the ID is on a secondary or later page - See the method used by the SPD to identify Manufacturers as a similar concept. 0xF0 - 0xFE would each point to a page of 240 possible drivers, and 0xff would tell the code to look for the next level of pages, which would follow the same pattern. >> >> The driver IDs and debug IDs would all be contained in separate header files. >> >> This would allow for the area to be uniquely identified by a tool. >> >> Postcode rules >> -------------- >> >> - Don’t use postcodes other than the debug postcodes in loops. Spewing out postcodes isn’t helpful. >> - Don’t output bare numbers, a macro in post_codes.h must be used. A linter will be set up to look the macros and will give an error if it’s not one of the postcode files. >> - All postcode output should be able to be identified by an automatic parser tool (with the exception of unregulated codes) >> >> >> >> _______________________________________________ >> coreboot mailing list -- >> coreboot(a)coreboot.org >> To unsubscribe send an email to >> coreboot-leave(a)coreboot.org >>

2 1

[coreboot - Bug #439] (New) Lenovo X201 Turbo Boost not working
by Lukas Karg 19 Nov '22

19 Nov '22

Issue #439 has been reported by Lukas Karg. ---------------------------------------- Bug #439: Lenovo X201 Turbo Boost not working https://ticket.coreboot.org/issues/439 * Author: Lukas Karg * Status: New * Priority: High * Target version: none * Start date: 2022-11-18 * Affected versions: master * Affected hardware: Lenovo X201 * Affected OS: Linux, Windows, ... ---------------------------------------- -- You have received this notification because you have either subscribed to it, or are involved in it. To change your notification preferences, please click here: https://ticket.coreboot.org/my/account

1 6

FYI: Changing contents and compression for `config` file in CBFS
by Julius Werner 17 Nov '22

17 Nov '22

I don't know if anybody cares about this, so I thought I'd send an email just in case: I'd like to change the format of the `config` file that automatically gets included in CBFS: right now it just contains the output of Kconfig's `savedefconfig` command, i.e. the minimal `.config` file that is needed to reproduce the full config (with all the default options and options that get `select`ed by something else excluded). This is a problem if you want to use the file to just check if a certain option was enabled in the image or not, since many options are enabled by default in certain cases, and the rules for when exactly what is the default setting can be complicated and change frequently. I would like to have a `config` file that I can just extract and then grep for a certain CONFIG_XXX=y to check if that config was enabled or not. So I'm changing the generation of the `config` file to instead include all options that are not set to false in Kconfig (i.e. basically the full coreboot `.config` file, just with comments removed). Since this makes the file quite a bit bigger I think we should also start to compress it with LZMA, as it's only meant to be re-extracted with cbfstool anyway so there seems to be no reason not to. (Or does anyone care strongly about being able to read the config file when opening the raw image with a hex editor?) I've uploaded this CL as https://review.coreboot.org/69710 . Please comment there if anyone has a concern about this change.

2 1

coreboot postcode cleanup proposal
by Martin Roth 17 Nov '22

17 Nov '22

The current state of the coreboot postcodes is a mess. There’s no good way to give anyone a postcode table. The post_code() function is called with direct hardcoded values all over the codebase. The post_codes.h file is disorganized with values being added without regard for being contiguous or ordered. As an example, despite using hexadecimal values, console post codes go from 0x39 to 0x40. We can do better than this. Proposal -------- The postcodes should be better organized and standardized. Each stage should have its own block of postcodes, and they should be easily recognizable as a flow from early coreboot to late coreboot. common between blocks: - bootblock - verstage - romstage - postcar - ramstage - smm We should have blocks of postcodes for different sections as well. - SoC/CPU/Chipset - Mainboard - Drivers - A set of debug-specific postcodes that may be used in any area, but may not be enabled by default, but only by user request. - The debug and driver post codes should have identifiers to show what section of the codebase they're coming from. - Finally, there should be small section of uncontrolled values that can be used anywhere that doesn’t have an already allocated specific value. Additional Improvements ----------------------- - Postcodes should be able to be included in the timestamp table so we can grab timestamps for individual postcodes. - Specific Postcodes should have associated text names, similar to the timestamps, and should have a decode tool. - Multi-byte postcodes should be implemented for platforms that support them. Proposed postcode ranges ------------------------ ``` * 0x00 : unregulated - 0x00 is not a good postcode. * * 0x01 - 0x07 : coreboot startup code & common * 0x08 - 0x0b : coreboot console * 0x0c - 0x0f : <RESERVED> * * 0x10 - 0x1f : coreboot bootblock * 0x20 - 0x2f : coreboot romstage * 0x30 - 0x33 : postcar * 0x34 - 0x3f : coreboot ramstage * 0x40 - 0x4f : coreboot ramstage hardwaremain boot stages * * 0x50 - 0x5f : Mainboard specific - Each mainboard can use as it chooses * 0x60 - 0x6f : SOC specific - Each SoC/CPU/Chipset can use as it chooses * * 0x70 : Driver/SuperIO ID, followed by ID byte * 0x71 - 0x7f : Driver/SuperIO specific - Can use as the device chooses * * 0x80 - 0x87 : ACPI postcodes * 0x88 - 0x8f : Unregulated - may be used anywhere for any purpose. * 0x90 - 0x9f : Vendorcode calls: FSP, AGESA, etc. * * 0xa0 - 0xaf : <RESERVED> * 0xb0 - 0xbf : <RESERVED> * * 0xc0 - 0xca : <RESERVED> * * 0xcb : coreboot identifier - used when returning from FSP, AGESA, etc * * 0xcc - 0xcf : <RESERVED> * * 0xd0 - 0xdf : Debug postcodes - never enabled by default. * * 0xe0 - 0xef : Errors * 0xf0 - 0xf7 : SMM * 0xf8 - 0xfe : end of coreboot code - resume and the like. * 0xff : coreboot finished - jump to payload ``` Driver/SuperIO/Debug IDs ------------------------ Because there are many drivers, and a single board can use multiple of the drivers, we want a way to know which device is putting out postcodes. To handle this, the driver, superIO, or debug section should emit a DRIVER_ID_FOLLOWS or DEBUG_ID_FOLLOWS postcode, followed by the actual ID code. The ID code would currently be a single byte, with the bytes 0xf0 - 0xff being reserved as page and continue bytes. The continue bytes would tell the code that the ID is on a secondary or later page - See the method used by the SPD to identify Manufacturers as a similar concept. 0xF0 - 0xFE would each point to a page of 240 possible drivers, and 0xff would tell the code to look for the next level of pages, which would follow the same pattern. The driver IDs and debug IDs would all be contained in separate header files. This would allow for the area to be uniquely identified by a tool. Postcode rules -------------- - Don’t use postcodes other than the debug postcodes in loops. Spewing out postcodes isn’t helpful. - Don’t output bare numbers, a macro in post_codes.h must be used. A linter will be set up to look the macros and will give an error if it’s not one of the postcode files. - All postcode output should be able to be identified by an automatic parser tool (with the exception of unregulated codes)

3 2

[RFC] POC: Use FDT as payload handoff instead lb_tables
by Paul Menzel 16 Nov '22

16 Nov '22

Dear coreboot folks, Arthur pushed the proof of concept *Use FDT as payload handoff instead lb_tables* [1] to pass information to the payload via FDT instead of coreboot tables. Julius asked for more background, and Arthur replied. I am moving the discussion to the list for broader visibility, and better way of discussing these things than Gerrit comments. >> Were there any discussions about this that I missed (if so would >> you mind linking to them)? Can you explain who needs this and why, >> what's the overall goal? (I know there have been vague discussions >> about various new firmware handoff formats from Arm and Intel and >> others, but if this is related to those I missed the parts where >> there was suddenly consensus on how it should look, and why that is >> FDT.) > > Most of the discussion happened verbally at osfc, so I can't point to > any written communication. There is > https://www.osfc.io/2022/talks/collaborative-firmware-payload-handoff-desig… > if you have the time to listen to a talk. > So if I have to summarize the idea's/goals: > > * Intel Universal scalable firmware suggested using UEFI HOBs as a handoff structure > * coreboot won't do that as it's no improvement over coreboot tables > * USF won't make coreboot tables a standard either > * Maybe something new can be created that would fit both the needs of payload handoff and other binary information passing between programs (e.g. FSP)? > * USF people suggested using CBOR to encode data to so that serialization/deserialization of data is explicit instead of using structs. > * CBOR is an improvement for the data format, but not what coreboot/(other project) should tell the payload: i.e. what entries are needed > * FDT was suggested as an alternative as it covers both explicit serialization/deserialization + what payload tell each other since the spec already exists, with a lot of existing compatible node strings. (e.g. I reuse simple-framebuffer from Linux here to hand of the framebuffer) Also most payloads and firmware have code for working with FDT. > * (unrelated to payload handof but part of the discussions) Another idea is to also use FDT as an interface between coreboot and FSP. > * a lot of payloads already have code to work with FDT so having FDT as a handoff structure would not be a lot of work > * FDT should make compatibility between coreboot and payload easier when handoff entries are added or changed (plain structs are painful if they are updated as both payload and coreboot definitions need to match) > * debugging the handoff structure is easier with FDT as you can use DTC in the OS userspace or coreboot code can also print out the full structure in human readable format. > > What are your thoughts? > > * Is it a good idea to use FDT and eventually replace coreboot tables. > * If so, do we keep both for a while to make transitioning easier? I think maintaining two structures is not sustainable long term. > * How should the 'entries' be structured? I made compatible nodes that map somewhat to existing coreboot table structs. Kind regards, Paul [1]: https://review.coreboot.org/c/coreboot/+/68719

12 24

[RFC] Use (extended?) Multiboot as payload handoff
by Nico Huber 16 Nov '22

16 Nov '22

Hi all, you can stop staring at your calendar, it's not April 1st. But actually I'm not sure myself if I'm serious with this :D Recent discussions made me look at the current USF spec, in par- ticular the Universal Payload part [1]. Admittedly I only scrolled through it. There are some things in it (beside the HOBs that some would rather put in FDT) that seem incompatible to our current payload mechanism. For instance, payload modules. The whole thing seemed so similar to Multiboot (with HOBs) that I had another look at Multiboot2[2]. The spec looks odd, but it seems like another thing similar to coreboot tables. There are two types of `tags` which is confusing, but the interesting one is the one to pass information to the loaded program: +-------------------+ u32 | type | u32 | size | +-------------------+ It's also very flat. But most of all it's an existing open standard! Thoughts are tumbling in my head right now: The whole Multiboot spec thing seems too complex to put into ramstage. But so does the Universal Payload thing! If we'd end up with a shim after ramstage anyway, we could as well use the existing standard. This might also pave the way for more kernels as payloads... WDYT? Nico [1] https://universalscalablefirmware.github.io/documentation/2_universal_paylo… [2] https://www.gnu.org/software/grub/manual/multiboot2/multiboot.html

6 8

Coreboot porting possibilities on TOPTON N5105
by Mateusz Kuchta 13 Nov '22

13 Nov '22

Hi I have a mini PC with Intel N5105 processor (Jasper Lake) and 6-port NIC (Intel i225-V B3). Device is chinese brand TOPTON, inside Changwang CW-11 motherboard delivered with AMI 5.19 BIOS. There isn't much details about this device in Internet. I'm planning to use it as a firewall box utilizing OPNsense which I've already running virtualized on NAS but wanted to move it to separate box. Before I start with it I thought about porting Coreboot BIOS. Coreboot is actually new to me but I found it can be benefitial in terms of security especially in my case where I'm planning to run firewall on it. It also my tech curiousity whether I could manage that to port BIOS with benefit to other people who owns this device. I read some posts on Reddit and found that first obstacle is Intel Boot Guard technology which Intel N5105 CPU supports. In dmesg I can find mei_me module entry related log: > [ 5.936057] mei_me 0000:00:16.0: enabling device (0000 -> 0002) Using intelmetool I wanted to find out if I can get more details but I got following output. I'm running those commands on Fedora with iomem=relaxed GRUB option set. ./intelmetool -b > Can't find ME PCI device > Can't find ME PCI device ./intelmetool -m > Can't find ME PCI device Next I tried to use fwupdtool to gather security details and result is following. In HSI-2 section main 'Intel BootGuard' entry shows 'Enabled' but other 3 detailed entries shows 'Invalid'. I'm not sure whether it's valid result or can be kind of false-positive and I shouldn't consider it as sure result. FuPluginUefiCapsule skipping device that failed coldplug: ESRT GUID > '00000000-0000-0000-0000-000000000000' was not valid > Host Security ID: HSI:0! (v1.8.6) > HSI-1 > ✔ CSME override: Locked > ✔ Platform Debugging: Disabled > ✔ SPI write: Disabled > ✔ Supported CPU: Valid > ✔ UEFI platform key: Valid > ✘ CSME manufacturing mode: Unlocked > ✘ CSME v0:13.50.15.1436: Invalid > ✘ SPI BIOS region: Unlocked > ✘ SPI lock: Disabled > ✘ TPM v2.0: Not found > HSI-2 > ✔ Intel BootGuard: Enabled > ✔ Platform Debugging: Locked > ✘ IOMMU: Not found > ✘ Intel BootGuard ACM protected: Invalid > ✘ Intel BootGuard OTP fuse: Invalid > ✘ Intel BootGuard verified boot: Invalid > > HSI-3 > ✔ Pre-boot DMA protection: Enabled > ✘ Intel BootGuard error policy: Invalid > ✘ Intel CET Enabled: Not supported > ✘ Suspend-to-idle: Disabled > ✘ Suspend-to-ram: Enabled > > HSI-4 > ✔ Intel SMAP: Enabled > ✘ Encrypted RAM: Not supported > Runtime Suffix -! > ✔ Linux kernel: Untainted > ✔ Linux swap: Encrypted > ✔ fwupd plugins: Untainted > ✘ Linux kernel lockdown: Disabled > ✘ UEFI secure boot: Disabled My question is based on above details can you tell me if there is an ability to port Coreboot into this device? If that details aren't enough, can you tell me what tools I should use to be 100% sure? Currently I don't have BIOS programmer, I was planning to buy it after I confirm possibilities for this device, otherwise it's probably useless to me and I won't need it. Anyway I can get it if this may be required to confirm possibilities. As it's chinese device from minor manufacturer I doubt it may be as secured as devices from big companies on market. Kind regards Mateusz

1 0

New Defects reported by Coverity Scan for coreboot
by scan-admin＠coverity.com 11 Nov '22

11 Nov '22

Hi, Please find the latest report on new defect(s) introduced to coreboot found with Coverity Scan. 1 new defect(s) introduced to coreboot found with Coverity Scan. 3 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan. New defect(s) Reported-by: Coverity Scan Showing 1 of 1 defect(s) ** CID 1500576: Null pointer dereferences (FORWARD_NULL) ________________________________________________________________________________________________________ *** CID 1500576: Null pointer dereferences (FORWARD_NULL) /src/ec/google/chromeec/ec.c: 90 in google_chromeec_kbbacklight() 84 .cmd_data_out = NULL, 85 .cmd_size_in = sizeof(params), 86 .cmd_size_out = 0, 87 .cmd_dev_index = 0, 88 }; 89 >>> CID 1500576: Null pointer dereferences (FORWARD_NULL) >>> Passing "&cmd" to "google_chromeec_command", which dereferences null "cmd.cmd_data_out". 90 if (google_chromeec_command(&cmd) != 0) 91 return -1; 92 93 return 0; 94 } 95 ________________________________________________________________________________________________________ To view the defects in Coverity Scan visit, https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P…

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

2003

2002

coreboot