coreboot-gerrit July 2021

coreboot-gerrit@coreboot.org

1 participants
5824 discussions

Change in coreboot[master]: SMBIOS: Introduce `smbios_full_table_len` function
by Patrick Georgi (Code Review) 01 Jul '21

01 Jul '21

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/55909 ) Change subject: SMBIOS: Introduce `smbios_full_table_len` function ...................................................................... SMBIOS: Introduce `smbios_full_table_len` function Introduce the `smbios_full_table_len` function to consolidate table length calculation. The case where the length of a table equals the length of the structure happens when a table has no strings. Change-Id: Ibc60075e82eb66b5d0b7132b16da000b153413f9 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/55909 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Reviewed-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> --- M src/arch/x86/smbios.c M src/drivers/elog/elog.c M src/drivers/wifi/generic/smbios.c M src/include/smbios.h M src/mainboard/emulation/qemu-i440fx/northbridge.c M src/mainboard/pcengines/apu1/mainboard.c M src/mainboard/pcengines/apu2/mainboard.c M src/mainboard/samsung/lumpy/mainboard.c 8 files changed, 34 insertions(+), 26 deletions(-) Approvals: build bot (Jenkins): Verified Michał Żygowski: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved diff --git a/src/arch/x86/smbios.c b/src/arch/x86/smbios.c index 96b5789..e686992 100644 --- a/src/arch/x86/smbios.c +++ b/src/arch/x86/smbios.c @@ -118,6 +118,11 @@ return len + 1; } +int smbios_full_table_len(struct smbios_header *header, u8 *str_table_start) +{ + return header->length + smbios_string_table_len(str_table_start); +} + void *smbios_carve_table(unsigned long start, u8 type, u8 length, u16 handle) { struct smbios_header *t = (struct smbios_header *)start; @@ -329,7 +334,7 @@ t->phys_memory_array_handle = type16_handle; *handle += 1; - return t->length + smbios_string_table_len(t->eos); + return smbios_full_table_len(&t->header, t->eos); } #define VERSION_VPD "firmware_version" @@ -429,7 +434,7 @@ t->bios_characteristics_ext1 = BIOS_EXT1_CHARACTERISTICS_ACPI; t->bios_characteristics_ext2 = BIOS_EXT2_CHARACTERISTICS_TARGET; - const int len = t->length + smbios_string_table_len(t->eos); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; return len; } @@ -528,7 +533,7 @@ t->family = smbios_add_string(t->eos, CONFIG_MAINBOARD_FAMILY); #endif smbios_system_set_uuid(t->uuid); - const int len = t->length + smbios_string_table_len(t->eos); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; return len; } @@ -548,7 +553,7 @@ smbios_mainboard_location_in_chassis()); t->board_type = smbios_mainboard_board_type(); t->chassis_handle = chassis_handle; - const int len = t->length + smbios_string_table_len(t->eos); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; return len; } @@ -568,7 +573,7 @@ t->asset_tag_number = smbios_add_string(t->eos, smbios_mainboard_asset_tag()); t->version = smbios_add_string(t->eos, smbios_chassis_version()); t->serial_number = smbios_add_string(t->eos, smbios_chassis_serial_number()); - const int len = t->length + smbios_string_table_len(t->eos); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; return len; } @@ -667,7 +672,7 @@ if (cpu_voltage > 0) t->voltage = 0x80 | cpu_voltage; - const int len = t->length + smbios_string_table_len(t->eos); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; return len; } @@ -740,7 +745,7 @@ t->error_correction_type = smbios_cache_error_correction_type(level); t->system_cache_type = type; - const int len = t->length + smbios_string_table_len(t->eos); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; return len; } @@ -901,8 +906,9 @@ t->external_connector_type = port->external_connector_type; t->port_type = port->port_type; *handle += 1; - *current += t->length + smbios_string_table_len(t->eos); - totallen += t->length + smbios_string_table_len(t->eos); + const int len = smbios_full_table_len(&t->header, t->eos); + *current += len; + totallen += len; } return totallen; } @@ -931,7 +937,7 @@ t->device_function_number = dev_func; t->data_bus_width = SlotDataBusWidthOther; - const int len = t->length + smbios_string_table_len(t->eos); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; *handle += 1; return len; @@ -953,7 +959,7 @@ return 0; } - const int len = t->length + smbios_string_table_len(t->eos); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; (*handle)++; return len; @@ -999,7 +1005,7 @@ } t->number_of_memory_devices = meminfo->number_of_devices; - const int len = t->length + smbios_string_table_len(t->eos); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; (*handle)++; return len; @@ -1074,7 +1080,7 @@ t->extended_ending_address--; } - const int len = t->length + smbios_string_table_len(t->eos); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; *handle += 1; return len; @@ -1085,7 +1091,7 @@ struct smbios_type32 *t = smbios_carve_table(*current, SMBIOS_SYSTEM_BOOT_INFORMATION, sizeof(*t), handle); - const int len = sizeof(*t); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; return len; } @@ -1107,7 +1113,7 @@ t->base_address_modifier = base_modifier; t->irq = irq; - const int len = sizeof(*t); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; *handle += 1; return len; @@ -1130,7 +1136,7 @@ t->device_number = device; t->function_number = function; - const int len = t->length + smbios_string_table_len(t->eos); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; *handle += 1; return len; @@ -1141,7 +1147,7 @@ struct smbios_type127 *t = smbios_carve_table(*current, SMBIOS_END_OF_TABLE, sizeof(*t), handle); - const int len = sizeof(*t); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; return len; } diff --git a/src/drivers/elog/elog.c b/src/drivers/elog/elog.c index 48b9dc3..97555bc 100644 --- a/src/drivers/elog/elog.c +++ b/src/drivers/elog/elog.c @@ -626,7 +626,7 @@ t->log_type_descriptors = 0; t->log_type_descriptor_length = 2; - const int len = sizeof(*t); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; return len; } diff --git a/src/drivers/wifi/generic/smbios.c b/src/drivers/wifi/generic/smbios.c index e8936be..7c697ad 100644 --- a/src/drivers/wifi/generic/smbios.c +++ b/src/drivers/wifi/generic/smbios.c @@ -28,7 +28,7 @@ /* Intel wifi driver expects this string to be in the table 0x85. */ t->str = smbios_add_string(t->eos, "KHOIHGIUCCHHII"); - const int len = t->length + smbios_string_table_len(t->eos); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; *handle += 1; return len; diff --git a/src/include/smbios.h b/src/include/smbios.h index 8390099..edc56f7 100644 --- a/src/include/smbios.h +++ b/src/include/smbios.h @@ -10,6 +10,8 @@ int smbios_add_string(u8 *start, const char *str); int smbios_string_table_len(u8 *start); +struct smbios_header; +int smbios_full_table_len(struct smbios_header *header, u8 *str_table_start); void *smbios_carve_table(unsigned long start, u8 type, u8 length, u16 handle); /* Used by mainboard to add an on-board device */ diff --git a/src/mainboard/emulation/qemu-i440fx/northbridge.c b/src/mainboard/emulation/qemu-i440fx/northbridge.c index 2f70f98..68410b0 100644 --- a/src/mainboard/emulation/qemu-i440fx/northbridge.c +++ b/src/mainboard/emulation/qemu-i440fx/northbridge.c @@ -173,7 +173,7 @@ t->memory_error_correction = MEMORY_ARRAY_ECC_NONE; t->maximum_capacity = qemu_get_memory_size(); - const int len = sizeof(*t); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; return len; } @@ -195,7 +195,7 @@ t->clock_speed = 200; t->manufacturer = smbios_add_string(t->eos, CONFIG_MAINBOARD_VENDOR); - const int len = t->length + smbios_string_table_len(t->eos); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; return len; } diff --git a/src/mainboard/pcengines/apu1/mainboard.c b/src/mainboard/pcengines/apu1/mainboard.c index 87bab71..f7b34f0 100644 --- a/src/mainboard/pcengines/apu1/mainboard.c +++ b/src/mainboard/pcengines/apu1/mainboard.c @@ -180,7 +180,7 @@ t->memory_error_information_handle = 0xfffe; t->number_of_memory_devices = 1; - const int len = sizeof(*t); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; return len; } @@ -212,7 +212,7 @@ t->minimum_voltage = 1500; /* From SPD: 1.5V */ t->maximum_voltage = 1500; - const int len = t->length + smbios_string_table_len(t->eos); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; return len; } diff --git a/src/mainboard/pcengines/apu2/mainboard.c b/src/mainboard/pcengines/apu2/mainboard.c index cab89a2..9371ee9 100644 --- a/src/mainboard/pcengines/apu2/mainboard.c +++ b/src/mainboard/pcengines/apu2/mainboard.c @@ -162,7 +162,7 @@ t->memory_error_information_handle = 0xfffe; t->number_of_memory_devices = 1; - const int len = sizeof(*t); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; return len; } @@ -198,7 +198,7 @@ t->minimum_voltage = 1500; /* From SPD: 1.5V */ t->maximum_voltage = 1500; - const int len = t->length + smbios_string_table_len(t->eos); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; return len; } diff --git a/src/mainboard/samsung/lumpy/mainboard.c b/src/mainboard/samsung/lumpy/mainboard.c index 8b76a50..d804b2b 100644 --- a/src/mainboard/samsung/lumpy/mainboard.c +++ b/src/mainboard/samsung/lumpy/mainboard.c @@ -41,7 +41,7 @@ t->function_number = 0; t->device_number = 0; - const int len = t->length + smbios_string_table_len(t->eos); + const int len = smbios_full_table_len(&t->header, t->eos); *current += len; *handle += 1; return len; -- To view, visit https://review.coreboot.org/c/coreboot/+/55909 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibc60075e82eb66b5d0b7132b16da000b153413f9 Gerrit-Change-Number: 55909 Gerrit-PatchSet: 3 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: arch/x86/smbios.c Move calculation next to usage
by Patrick Georgi (Code Review) 01 Jul '21

01 Jul '21

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/55908 ) Change subject: arch/x86/smbios.c Move calculation next to usage ...................................................................... arch/x86/smbios.c Move calculation next to usage Change-Id: I949da86605e76b186ef2fdbfbc112b71544b694a Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/55908 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/arch/x86/smbios.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved diff --git a/src/arch/x86/smbios.c b/src/arch/x86/smbios.c index 8e780fd..96b5789 100644 --- a/src/arch/x86/smbios.c +++ b/src/arch/x86/smbios.c @@ -635,7 +635,6 @@ t->serial_number = smbios_add_string(t->eos, smbios_processor_serial_number()); t->status = SMBIOS_PROCESSOR_STATUS_CPU_ENABLED | SMBIOS_PROCESSOR_STATUS_POPULATED; t->processor_upgrade = get_socket_type(); - const int len = t->length + smbios_string_table_len(t->eos); if (cpu_have_cpuid() && cpuid_get_max_func() >= 0x16) { t->current_speed = cpuid_eax(0x16); /* base frequency */ t->external_clock = cpuid_ecx(0x16); @@ -668,6 +667,7 @@ if (cpu_voltage > 0) t->voltage = 0x80 | cpu_voltage; + const int len = t->length + smbios_string_table_len(t->eos); *current += len; return len; } -- To view, visit https://review.coreboot.org/c/coreboot/+/55908 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I949da86605e76b186ef2fdbfbc112b71544b694a Gerrit-Change-Number: 55908 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: SMBIOS: Introduce `smbios_carve_table` function
by Patrick Georgi (Code Review) 01 Jul '21

01 Jul '21

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/55907 ) Change subject: SMBIOS: Introduce `smbios_carve_table` function ...................................................................... SMBIOS: Introduce `smbios_carve_table` function Factor out some boilerplate code into a helper `smbios_carve_table` function, which zeroes out the table memory and fills in the header fields common to all tables. Change-Id: Iece2f64f9151d3c79813f6264dfb3a92d98c2035 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/55907 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Reviewed-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> --- M src/arch/x86/smbios.c M src/drivers/elog/elog.c M src/drivers/wifi/generic/smbios.c M src/include/smbios.h M src/mainboard/emulation/qemu-i440fx/northbridge.c M src/mainboard/pcengines/apu1/mainboard.c M src/mainboard/pcengines/apu2/mainboard.c M src/mainboard/samsung/lumpy/mainboard.c 8 files changed, 94 insertions(+), 178 deletions(-) Approvals: build bot (Jenkins): Verified Michał Żygowski: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved diff --git a/src/arch/x86/smbios.c b/src/arch/x86/smbios.c index 4637dc0..8e780fd 100644 --- a/src/arch/x86/smbios.c +++ b/src/arch/x86/smbios.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <assert.h> #include <string.h> #include <smbios.h> #include <console/console.h> @@ -117,6 +118,18 @@ return len + 1; } +void *smbios_carve_table(unsigned long start, u8 type, u8 length, u16 handle) +{ + struct smbios_header *t = (struct smbios_header *)start; + + assert(length >= sizeof(*t)); + memset(t, 0, length); + t->type = type; + t->length = length - 2; + t->handle = handle; + return t; +} + static int smbios_cpu_vendor(u8 *start) { if (cpu_have_cpuid()) { @@ -241,9 +254,9 @@ unsigned long *current, int *handle, int type16_handle) { - struct smbios_type17 *t = (struct smbios_type17 *)*current; + struct smbios_type17 *t = smbios_carve_table(*current, SMBIOS_MEMORY_DEVICE, + sizeof(*t), *handle); - memset(t, 0, sizeof(*t)); t->memory_type = dimm->ddr_type; if (dimm->configured_speed_mts != 0) t->clock_speed = dimm->configured_speed_mts; @@ -253,7 +266,6 @@ t->speed = dimm->max_speed_mts; else t->speed = dimm->ddr_frequency; - t->type = SMBIOS_MEMORY_DEVICE; if (dimm->dimm_size < 0x7fff) { t->size = dimm->dimm_size; } else { @@ -314,11 +326,9 @@ /* no handle for error information */ t->memory_error_information_handle = 0xFFFE; t->attributes = dimm->rank_per_dimm; - t->handle = *handle; t->phys_memory_array_handle = type16_handle; *handle += 1; - t->length = sizeof(*t) - 2; return t->length + smbios_string_table_len(t->eos); } @@ -379,13 +389,8 @@ static int smbios_write_type0(unsigned long *current, int handle) { - struct smbios_type0 *t = (struct smbios_type0 *)*current; - int len = sizeof(*t); - - memset(t, 0, sizeof(*t)); - t->type = SMBIOS_BIOS_INFORMATION; - t->handle = handle; - t->length = len - 2; + struct smbios_type0 *t = smbios_carve_table(*current, SMBIOS_BIOS_INFORMATION, + sizeof(*t), handle); t->vendor = smbios_add_string(t->eos, "coreboot"); t->bios_release_date = smbios_add_string(t->eos, coreboot_dmi_date); @@ -424,7 +429,7 @@ t->bios_characteristics_ext1 = BIOS_EXT1_CHARACTERISTICS_ACPI; t->bios_characteristics_ext2 = BIOS_EXT2_CHARACTERISTICS_TARGET; - len = t->length + smbios_string_table_len(t->eos); + const int len = t->length + smbios_string_table_len(t->eos); *current += len; return len; } @@ -511,13 +516,9 @@ static int smbios_write_type1(unsigned long *current, int handle) { - struct smbios_type1 *t = (struct smbios_type1 *)*current; - int len = sizeof(*t); + struct smbios_type1 *t = smbios_carve_table(*current, SMBIOS_SYSTEM_INFORMATION, + sizeof(*t), handle); - memset(t, 0, sizeof(*t)); - t->type = SMBIOS_SYSTEM_INFORMATION; - t->handle = handle; - t->length = len - 2; t->manufacturer = smbios_add_string(t->eos, smbios_system_manufacturer()); t->product_name = smbios_add_string(t->eos, smbios_system_product_name()); t->serial_number = smbios_add_string(t->eos, smbios_system_serial_number()); @@ -527,20 +528,16 @@ t->family = smbios_add_string(t->eos, CONFIG_MAINBOARD_FAMILY); #endif smbios_system_set_uuid(t->uuid); - len = t->length + smbios_string_table_len(t->eos); + const int len = t->length + smbios_string_table_len(t->eos); *current += len; return len; } static int smbios_write_type2(unsigned long *current, int handle, const int chassis_handle) { - struct smbios_type2 *t = (struct smbios_type2 *)*current; - int len = sizeof(*t); + struct smbios_type2 *t = smbios_carve_table(*current, SMBIOS_BOARD_INFORMATION, + sizeof(*t), handle); - memset(t, 0, sizeof(*t)); - t->type = SMBIOS_BOARD_INFORMATION; - t->handle = handle; - t->length = len - 2; t->manufacturer = smbios_add_string(t->eos, smbios_mainboard_manufacturer()); t->product_name = smbios_add_string(t->eos, smbios_mainboard_product_name()); t->serial_number = smbios_add_string(t->eos, smbios_mainboard_serial_number()); @@ -551,20 +548,16 @@ smbios_mainboard_location_in_chassis()); t->board_type = smbios_mainboard_board_type(); t->chassis_handle = chassis_handle; - len = t->length + smbios_string_table_len(t->eos); + const int len = t->length + smbios_string_table_len(t->eos); *current += len; return len; } static int smbios_write_type3(unsigned long *current, int handle) { - struct smbios_type3 *t = (struct smbios_type3 *)*current; - int len = sizeof(*t); + struct smbios_type3 *t = smbios_carve_table(*current, SMBIOS_SYSTEM_ENCLOSURE, + sizeof(*t), handle); - memset(t, 0, sizeof(*t)); - t->type = SMBIOS_SYSTEM_ENCLOSURE; - t->handle = handle; - t->length = len - 2; t->manufacturer = smbios_add_string(t->eos, smbios_system_manufacturer()); t->bootup_state = SMBIOS_STATE_SAFE; t->power_supply_state = SMBIOS_STATE_SAFE; @@ -575,7 +568,7 @@ t->asset_tag_number = smbios_add_string(t->eos, smbios_mainboard_asset_tag()); t->version = smbios_add_string(t->eos, smbios_chassis_version()); t->serial_number = smbios_add_string(t->eos, smbios_chassis_serial_number()); - len = t->length + smbios_string_table_len(t->eos); + const int len = t->length + smbios_string_table_len(t->eos); *current += len; return len; } @@ -584,8 +577,6 @@ { unsigned int cpu_voltage; struct cpuid_result res; - struct smbios_type4 *t = (struct smbios_type4 *)*current; - int len = sizeof(*t); uint16_t characteristics = 0; static unsigned int cnt = 0; char buf[8]; @@ -597,10 +588,8 @@ if (cpu_have_cpuid()) res = cpuid(1); - memset(t, 0, sizeof(*t)); - t->type = SMBIOS_PROCESSOR_INFORMATION; - t->handle = handle; - t->length = len - 2; + struct smbios_type4 *t = smbios_carve_table(*current, SMBIOS_PROCESSOR_INFORMATION, + sizeof(*t), handle); snprintf(buf, sizeof(buf), "CPU%d", cnt++); t->socket_designation = smbios_add_string(t->eos, buf); @@ -646,7 +635,7 @@ t->serial_number = smbios_add_string(t->eos, smbios_processor_serial_number()); t->status = SMBIOS_PROCESSOR_STATUS_CPU_ENABLED | SMBIOS_PROCESSOR_STATUS_POPULATED; t->processor_upgrade = get_socket_type(); - len = t->length + smbios_string_table_len(t->eos); + const int len = t->length + smbios_string_table_len(t->eos); if (cpu_have_cpuid() && cpuid_get_max_func() >= 0x16) { t->current_speed = cpuid_eax(0x16); /* base frequency */ t->external_clock = cpuid_ecx(0x16); @@ -697,14 +686,10 @@ const size_t max_cache_size, const size_t cache_size) { - struct smbios_type7 *t = (struct smbios_type7 *)*current; - int len = sizeof(*t); char buf[8]; - memset(t, 0, sizeof(*t)); - t->type = SMBIOS_CACHE_INFORMATION; - t->handle = handle; - t->length = len - 2; + struct smbios_type7 *t = smbios_carve_table(*current, SMBIOS_CACHE_INFORMATION, + sizeof(*t), handle); snprintf(buf, sizeof(buf), "CACHE%x", level); t->socket_designation = smbios_add_string(t->eos, buf); @@ -755,7 +740,7 @@ t->error_correction_type = smbios_cache_error_correction_type(level); t->system_cache_type = type; - len = t->length + smbios_string_table_len(t->eos); + const int len = t->length + smbios_string_table_len(t->eos); *current += len; return len; } @@ -905,11 +890,9 @@ unsigned int totallen = 0, i; for (i = 0; i < num_ports; i++, port++) { - struct smbios_type8 *t = (struct smbios_type8 *)*current; - memset(t, 0, sizeof(*t)); - t->type = SMBIOS_PORT_CONNECTOR_INFORMATION; - t->handle = *handle; - t->length = sizeof(*t) - 2; + struct smbios_type8 *t = smbios_carve_table(*current, + SMBIOS_PORT_CONNECTOR_INFORMATION, + sizeof(*t), *handle); t->internal_reference_designator = smbios_add_string(t->eos, port->internal_reference_designator); t->internal_connector_type = port->internal_connector_type; @@ -931,13 +914,9 @@ const enum misc_slot_length length, const u16 id, u8 slot_char1, u8 slot_char2, u8 bus, u8 dev_func) { - struct smbios_type9 *t = (struct smbios_type9 *)*current; - int len = sizeof(*t); + struct smbios_type9 *t = smbios_carve_table(*current, SMBIOS_SYSTEM_SLOTS, + sizeof(*t), *handle); - memset(t, 0, sizeof(*t)); - t->type = SMBIOS_SYSTEM_SLOTS; - t->handle = *handle; - t->length = len - 2; t->slot_designation = smbios_add_string(t->eos, name ? name : "SLOT"); t->slot_type = type; /* TODO add slot_id supoort, will be "_SUN" for ACPI devices */ @@ -952,7 +931,7 @@ t->device_function_number = dev_func; t->data_bus_width = SlotDataBusWidthOther; - len = t->length + smbios_string_table_len(t->eos); + const int len = t->length + smbios_string_table_len(t->eos); *current += len; *handle += 1; return len; @@ -960,14 +939,9 @@ static int smbios_write_type11(unsigned long *current, int *handle) { - struct smbios_type11 *t = (struct smbios_type11 *)*current; - int len; struct device *dev; - - memset(t, 0, sizeof(*t)); - t->type = SMBIOS_OEM_STRINGS; - t->handle = *handle; - t->length = len = sizeof(*t) - 2; + struct smbios_type11 *t = smbios_carve_table(*current, SMBIOS_OEM_STRINGS, + sizeof(*t), *handle); for (dev = all_devices; dev; dev = dev->next) { if (dev->ops && dev->ops->get_smbios_strings) @@ -979,8 +953,7 @@ return 0; } - len += smbios_string_table_len(t->eos); - + const int len = t->length + smbios_string_table_len(t->eos); *current += len; (*handle)++; return len; @@ -988,9 +961,6 @@ static int smbios_write_type16(unsigned long *current, int *handle) { - struct smbios_type16 *t = (struct smbios_type16 *)*current; - - int len; int i; uint64_t max_capacity; @@ -1011,10 +981,8 @@ } } - memset(t, 0, sizeof(*t)); - t->type = SMBIOS_PHYS_MEMORY_ARRAY; - t->handle = *handle; - t->length = len = sizeof(*t) - 2; + struct smbios_type16 *t = smbios_carve_table(*current, SMBIOS_PHYS_MEMORY_ARRAY, + sizeof(*t), *handle); t->location = MEMORY_ARRAY_LOCATION_SYSTEM_BOARD; t->use = MEMORY_ARRAY_USE_SYSTEM; @@ -1031,8 +999,7 @@ } t->number_of_memory_devices = meminfo->number_of_devices; - len += smbios_string_table_len(t->eos); - + const int len = t->length + smbios_string_table_len(t->eos); *current += len; (*handle)++; return len; @@ -1067,7 +1034,6 @@ static int smbios_write_type19(unsigned long *current, int *handle, int type16) { - struct smbios_type19 *t = (struct smbios_type19 *)*current; int i; struct memory_info *meminfo; @@ -1075,11 +1041,10 @@ if (meminfo == NULL) return 0; /* can't find mem info in cbmem */ - memset(t, 0, sizeof(*t)); + struct smbios_type19 *t = smbios_carve_table(*current, + SMBIOS_MEMORY_ARRAY_MAPPED_ADDRESS, + sizeof(*t), *handle); - t->type = SMBIOS_MEMORY_ARRAY_MAPPED_ADDRESS; - t->length = sizeof(*t) - 2; - t->handle = *handle; t->memory_array_handle = type16; for (i = 0; i < meminfo->dimm_cnt && i < ARRAY_SIZE(meminfo->dimm); i++) { @@ -1117,13 +1082,10 @@ static int smbios_write_type32(unsigned long *current, int handle) { - struct smbios_type32 *t = (struct smbios_type32 *)*current; - int len = sizeof(*t); + struct smbios_type32 *t = smbios_carve_table(*current, SMBIOS_SYSTEM_BOOT_INFORMATION, + sizeof(*t), handle); - memset(t, 0, sizeof(*t)); - t->type = SMBIOS_SYSTEM_BOOT_INFORMATION; - t->handle = handle; - t->length = len - 2; + const int len = sizeof(*t); *current += len; return len; } @@ -1134,13 +1096,9 @@ const u64 base_addr, const u8 base_modifier, const u8 irq) { - struct smbios_type38 *t = (struct smbios_type38 *)*current; - int len = sizeof(*t); + struct smbios_type38 *t = smbios_carve_table(*current, SMBIOS_IPMI_DEVICE_INFORMATION, + sizeof(*t), *handle); - memset(t, 0, sizeof(*t)); - t->type = SMBIOS_IPMI_DEVICE_INFORMATION; - t->handle = *handle; - t->length = len - 2; t->interface_type = interface_type; t->ipmi_rev = ipmi_rev; t->i2c_slave_addr = i2c_addr; @@ -1149,9 +1107,9 @@ t->base_address_modifier = base_modifier; t->irq = irq; + const int len = sizeof(*t); *current += len; *handle += 1; - return len; } @@ -1159,13 +1117,10 @@ const char *name, u8 instance, u16 segment, u8 bus, u8 device, u8 function, u8 device_type) { - struct smbios_type41 *t = (struct smbios_type41 *)*current; - int len = sizeof(*t); + struct smbios_type41 *t = smbios_carve_table(*current, + SMBIOS_ONBOARD_DEVICES_EXTENDED_INFORMATION, + sizeof(*t), *handle); - memset(t, 0, sizeof(*t)); - t->type = SMBIOS_ONBOARD_DEVICES_EXTENDED_INFORMATION; - t->handle = *handle; - t->length = len - 2; t->reference_designation = smbios_add_string(t->eos, name); t->device_type = device_type; t->device_status = 1; @@ -1175,7 +1130,7 @@ t->device_number = device; t->function_number = function; - len = t->length + smbios_string_table_len(t->eos); + const int len = t->length + smbios_string_table_len(t->eos); *current += len; *handle += 1; return len; @@ -1183,13 +1138,10 @@ static int smbios_write_type127(unsigned long *current, int handle) { - struct smbios_type127 *t = (struct smbios_type127 *)*current; - int len = sizeof(*t); + struct smbios_type127 *t = smbios_carve_table(*current, SMBIOS_END_OF_TABLE, + sizeof(*t), handle); - memset(t, 0, sizeof(*t)); - t->type = SMBIOS_END_OF_TABLE; - t->handle = handle; - t->length = len - 2; + const int len = sizeof(*t); *current += len; return len; } diff --git a/src/drivers/elog/elog.c b/src/drivers/elog/elog.c index 6e31686..48b9dc3 100644 --- a/src/drivers/elog/elog.c +++ b/src/drivers/elog/elog.c @@ -593,8 +593,6 @@ */ int elog_smbios_write_type15(unsigned long *current, int handle) { - struct smbios_type15 *t = (struct smbios_type15 *)*current; - int len = sizeof(*t); uintptr_t log_address; size_t elog_size = region_device_sz(&elog_state.nv_dev); @@ -614,10 +612,9 @@ return 0; } - memset(t, 0, len); - t->type = SMBIOS_EVENT_LOG; - t->length = len - 2; - t->handle = handle; + struct smbios_type15 *t = smbios_carve_table(*current, SMBIOS_EVENT_LOG, + sizeof(*t), handle); + t->area_length = elog_size - 1; t->header_offset = 0; t->data_offset = sizeof(struct elog_header); @@ -629,6 +626,7 @@ t->log_type_descriptors = 0; t->log_type_descriptor_length = 2; + const int len = sizeof(*t); *current += len; return len; } diff --git a/src/drivers/wifi/generic/smbios.c b/src/drivers/wifi/generic/smbios.c index 96b11d3..e8936be 100644 --- a/src/drivers/wifi/generic/smbios.c +++ b/src/drivers/wifi/generic/smbios.c @@ -22,17 +22,13 @@ u8 eos[2]; } __packed; - struct smbios_type_intel_wifi *t = (struct smbios_type_intel_wifi *)*current; - int len = sizeof(*t); + struct smbios_type_intel_wifi *t = smbios_carve_table(*current, 0x85, + sizeof(*t), *handle); - memset(t, 0, sizeof(*t)); - t->type = 0x85; - t->length = len - 2; - t->handle = *handle; /* Intel wifi driver expects this string to be in the table 0x85. */ t->str = smbios_add_string(t->eos, "KHOIHGIUCCHHII"); - len = t->length + smbios_string_table_len(t->eos); + const int len = t->length + smbios_string_table_len(t->eos); *current += len; *handle += 1; return len; diff --git a/src/include/smbios.h b/src/include/smbios.h index 1e307c8..8390099 100644 --- a/src/include/smbios.h +++ b/src/include/smbios.h @@ -10,6 +10,8 @@ int smbios_add_string(u8 *start, const char *str); int smbios_string_table_len(u8 *start); +void *smbios_carve_table(unsigned long start, u8 type, u8 length, u16 handle); + /* Used by mainboard to add an on-board device */ enum misc_slot_type; enum misc_slot_length; diff --git a/src/mainboard/emulation/qemu-i440fx/northbridge.c b/src/mainboard/emulation/qemu-i440fx/northbridge.c index 01d036a..2f70f98 100644 --- a/src/mainboard/emulation/qemu-i440fx/northbridge.c +++ b/src/mainboard/emulation/qemu-i440fx/northbridge.c @@ -165,31 +165,25 @@ #if CONFIG(GENERATE_SMBIOS_TABLES) static int qemu_get_smbios_data16(int handle, unsigned long *current) { - struct smbios_type16 *t = (struct smbios_type16 *)*current; - int len = sizeof(*t); + struct smbios_type16 *t = smbios_carve_table(*current, SMBIOS_PHYS_MEMORY_ARRAY, + sizeof(*t), handle); - memset(t, 0, sizeof(*t)); - t->type = SMBIOS_PHYS_MEMORY_ARRAY; - t->handle = handle; - t->length = len - 2; t->location = MEMORY_ARRAY_LOCATION_SYSTEM_BOARD; t->use = MEMORY_ARRAY_USE_SYSTEM; t->memory_error_correction = MEMORY_ARRAY_ECC_NONE; t->maximum_capacity = qemu_get_memory_size(); + + const int len = sizeof(*t); *current += len; return len; } static int qemu_get_smbios_data17(int handle, int parent_handle, unsigned long *current) { - struct smbios_type17 *t = (struct smbios_type17 *)*current; - int len; + struct smbios_type17 *t = smbios_carve_table(*current, SMBIOS_MEMORY_DEVICE, + sizeof(*t), handle); - memset(t, 0, sizeof(*t)); - t->type = SMBIOS_MEMORY_DEVICE; - t->handle = handle; t->phys_memory_array_handle = parent_handle; - t->length = sizeof(*t) - 2; t->size = qemu_get_memory_size() / 1024; t->data_width = 64; t->total_width = 64; @@ -200,7 +194,8 @@ t->speed = 200; t->clock_speed = 200; t->manufacturer = smbios_add_string(t->eos, CONFIG_MAINBOARD_VENDOR); - len = t->length + smbios_string_table_len(t->eos); + + const int len = t->length + smbios_string_table_len(t->eos); *current += len; return len; } diff --git a/src/mainboard/pcengines/apu1/mainboard.c b/src/mainboard/pcengines/apu1/mainboard.c index 187f8cf..87bab71 100644 --- a/src/mainboard/pcengines/apu1/mainboard.c +++ b/src/mainboard/pcengines/apu1/mainboard.c @@ -170,14 +170,9 @@ { const u32 max_capacity = get_spd_offset() ? 4 : 2; /* 4GB or 2GB variant */ - struct smbios_type16 *t = (struct smbios_type16 *)*current; - int len = sizeof(*t); - memset(t, 0, len); + struct smbios_type16 *t = smbios_carve_table(*current, SMBIOS_PHYS_MEMORY_ARRAY, + sizeof(*t), *handle); - t->type = SMBIOS_PHYS_MEMORY_ARRAY; - t->handle = *handle; - t->length = len - 2; - t->type = SMBIOS_PHYS_MEMORY_ARRAY; t->use = MEMORY_ARRAY_USE_SYSTEM; t->location = MEMORY_ARRAY_LOCATION_SYSTEM_BOARD; t->memory_error_correction = agesa_dmi->T16.MemoryErrorCorrection; @@ -185,22 +180,16 @@ t->memory_error_information_handle = 0xfffe; t->number_of_memory_devices = 1; + const int len = sizeof(*t); *current += len; - return len; } static int mainboard_smbios_type17(DMI_INFO *agesa_dmi, int *handle, unsigned long *current) { - struct smbios_type17 *t; - int len; + struct smbios_type17 *t = smbios_carve_table(*current, SMBIOS_MEMORY_DEVICE, + sizeof(*t), *handle + 1); - t = (struct smbios_type17 *)*current; - memset(t, 0, sizeof(*t)); - - t->type = SMBIOS_MEMORY_DEVICE; - t->length = sizeof(*t) - 2; - t->handle = *handle + 1; t->phys_memory_array_handle = *handle; t->memory_error_information_handle = 0xfffe; t->total_width = agesa_dmi->T17[0][0][0].TotalWidth; @@ -223,9 +212,8 @@ t->minimum_voltage = 1500; /* From SPD: 1.5V */ t->maximum_voltage = 1500; - len = t->length + smbios_string_table_len(t->eos); + const int len = t->length + smbios_string_table_len(t->eos); *current += len; - return len; } diff --git a/src/mainboard/pcengines/apu2/mainboard.c b/src/mainboard/pcengines/apu2/mainboard.c index 2c3b861..cab89a2 100644 --- a/src/mainboard/pcengines/apu2/mainboard.c +++ b/src/mainboard/pcengines/apu2/mainboard.c @@ -152,14 +152,9 @@ { const u32 max_capacity = get_spd_offset() ? 4 : 2; /* 4GB or 2GB variant */ - struct smbios_type16 *t = (struct smbios_type16 *)*current; - int len = sizeof(*t); - memset(t, 0, len); + struct smbios_type16 *t = smbios_carve_table(*current, SMBIOS_PHYS_MEMORY_ARRAY, + sizeof(*t), *handle); - t->type = SMBIOS_PHYS_MEMORY_ARRAY; - t->handle = *handle; - t->length = len - 2; - t->type = SMBIOS_PHYS_MEMORY_ARRAY; t->use = MEMORY_ARRAY_USE_SYSTEM; t->location = MEMORY_ARRAY_LOCATION_SYSTEM_BOARD; t->memory_error_correction = agesa_dmi->T16.MemoryErrorCorrection; @@ -167,23 +162,17 @@ t->memory_error_information_handle = 0xfffe; t->number_of_memory_devices = 1; + const int len = sizeof(*t); *current += len; - return len; } static int mainboard_smbios_type17(DMI_INFO *agesa_dmi, int *handle, unsigned long *current) { - struct smbios_type17 *t; - int len; + struct smbios_type17 *t = smbios_carve_table(*current, SMBIOS_MEMORY_DEVICE, + sizeof(*t), *handle + 1); - t = (struct smbios_type17 *)*current; - memset(t, 0, sizeof(*t)); - - t->type = SMBIOS_MEMORY_DEVICE; - t->length = sizeof(*t) - 2; - t->handle = *handle + 1; t->phys_memory_array_handle = *handle; t->memory_error_information_handle = 0xfffe; t->total_width = agesa_dmi->T17[0][0][0].TotalWidth; @@ -209,9 +198,8 @@ t->minimum_voltage = 1500; /* From SPD: 1.5V */ t->maximum_voltage = 1500; - len = t->length + smbios_string_table_len(t->eos); + const int len = t->length + smbios_string_table_len(t->eos); *current += len; - return len; } diff --git a/src/mainboard/samsung/lumpy/mainboard.c b/src/mainboard/samsung/lumpy/mainboard.c index c2899b8..8b76a50 100644 --- a/src/mainboard/samsung/lumpy/mainboard.c +++ b/src/mainboard/samsung/lumpy/mainboard.c @@ -28,13 +28,10 @@ static int lumpy_smbios_type41_irq(int *handle, unsigned long *current, const char *name, u8 irq, u8 addr) { - struct smbios_type41 *t = (struct smbios_type41 *)*current; - int len = sizeof(*t); + struct smbios_type41 *t = smbios_carve_table(*current, + SMBIOS_ONBOARD_DEVICES_EXTENDED_INFORMATION, + sizeof(*t), *handle); - memset(t, 0, sizeof(*t)); - t->type = SMBIOS_ONBOARD_DEVICES_EXTENDED_INFORMATION; - t->handle = *handle; - t->length = len - 2; t->reference_designation = smbios_add_string(t->eos, name); t->device_type = SMBIOS_DEVICE_TYPE_OTHER; t->device_status = 1; @@ -44,7 +41,7 @@ t->function_number = 0; t->device_number = 0; - len = t->length + smbios_string_table_len(t->eos); + const int len = t->length + smbios_string_table_len(t->eos); *current += len; *handle += 1; return len; -- To view, visit https://review.coreboot.org/c/coreboot/+/55907 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iece2f64f9151d3c79813f6264dfb3a92d98c2035 Gerrit-Change-Number: 55907 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: SMBIOS: Introduce struct for SMBIOS table header
by Patrick Georgi (Code Review) 01 Jul '21

01 Jul '21

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/55906 ) Change subject: SMBIOS: Introduce struct for SMBIOS table header ...................................................................... SMBIOS: Introduce struct for SMBIOS table header All SMBIOS `type X` tables start with the same 4-byte header. Add a struct definition for it, and use it where applicable. The union is temporary and allows doing the necessary changes in smaller commits. Change-Id: Ibd9a80010f83fd7ebefc014b981d430f5723808c Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/55906 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/drivers/wifi/generic/smbios.c M src/include/smbios.h 2 files changed, 148 insertions(+), 52 deletions(-) Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved diff --git a/src/drivers/wifi/generic/smbios.c b/src/drivers/wifi/generic/smbios.c index b819116..96b11d3 100644 --- a/src/drivers/wifi/generic/smbios.c +++ b/src/drivers/wifi/generic/smbios.c @@ -10,9 +10,14 @@ static int smbios_write_intel_wifi(struct device *dev, int *handle, unsigned long *current) { struct smbios_type_intel_wifi { - u8 type; - u8 length; - u16 handle; + union { + struct { + u8 type; + u8 length; + u16 handle; + }; + struct smbios_header header; + }; u8 str; u8 eos[2]; } __packed; diff --git a/src/include/smbios.h b/src/include/smbios.h index e5e5df5..1e307c8 100644 --- a/src/include/smbios.h +++ b/src/include/smbios.h @@ -276,10 +276,21 @@ u64 struct_table_address; } __packed; -struct smbios_type0 { +struct smbios_header { u8 type; u8 length; u16 handle; +} __packed; + +struct smbios_type0 { + union { + struct { + u8 type; + u8 length; + u16 handle; + }; + struct smbios_header header; + }; u8 vendor; u8 bios_version; u16 bios_start_segment; @@ -297,9 +308,14 @@ } __packed; struct smbios_type1 { - u8 type; - u8 length; - u16 handle; + union { + struct { + u8 type; + u8 length; + u16 handle; + }; + struct smbios_header header; + }; u8 manufacturer; u8 product_name; u8 version; @@ -334,9 +350,14 @@ } smbios_board_type; struct smbios_type2 { - u8 type; - u8 length; - u16 handle; + union { + struct { + u8 type; + u8 length; + u16 handle; + }; + struct smbios_header header; + }; u8 manufacturer; u8 product_name; u8 version; @@ -389,9 +410,14 @@ } smbios_enclosure_type; struct smbios_type3 { - u8 type; - u8 length; - u16 handle; + union { + struct { + u8 type; + u8 length; + u16 handle; + }; + struct smbios_header header; + }; u8 manufacturer; u8 _type; u8 version; @@ -411,9 +437,14 @@ } __packed; struct smbios_type4 { - u8 type; - u8 length; - u16 handle; + union { + struct { + u8 type; + u8 length; + u16 handle; + }; + struct smbios_header header; + }; u8 socket_designation; u8 processor_type; u8 processor_family; @@ -524,9 +555,14 @@ #define SMBIOS_CACHE_OP_MODE_UNKNOWN 3 struct smbios_type7 { - u8 type; - u8 length; - u16 handle; + union { + struct { + u8 type; + u8 length; + u16 handle; + }; + struct smbios_header header; + }; u8 socket_designation; u16 cache_configuration; u16 max_cache_size; @@ -640,9 +676,14 @@ }; struct smbios_type8 { - u8 type; - u8 length; - u16 handle; + union { + struct { + u8 type; + u8 length; + u16 handle; + }; + struct smbios_header header; + }; u8 internal_reference_designator; u8 internal_connector_type; u8 external_reference_designator; @@ -777,9 +818,14 @@ } __packed; struct smbios_type9 { - u8 type; - u8 length; - u16 handle; + union { + struct { + u8 type; + u8 length; + u16 handle; + }; + struct smbios_header header; + }; u8 slot_designation; u8 slot_type; u8 slot_data_bus_width; @@ -798,17 +844,27 @@ } __packed; struct smbios_type11 { - u8 type; - u8 length; - u16 handle; + union { + struct { + u8 type; + u8 length; + u16 handle; + }; + struct smbios_header header; + }; u8 count; u8 eos[2]; } __packed; struct smbios_type15 { - u8 type; - u8 length; - u16 handle; + union { + struct { + u8 type; + u8 length; + u16 handle; + }; + struct smbios_header header; + }; u16 area_length; u16 header_offset; u16 data_offset; @@ -838,9 +894,14 @@ #define SMBIOS_USE_EXTENDED_MAX_CAPACITY (1 << 31) struct smbios_type16 { - u8 type; - u8 length; - u16 handle; + union { + struct { + u8 type; + u8 length; + u16 handle; + }; + struct smbios_header header; + }; u8 location; u8 use; u8 memory_error_correction; @@ -852,9 +913,14 @@ } __packed; struct smbios_type17 { - u8 type; - u8 length; - u16 handle; + union { + struct { + u8 type; + u8 length; + u16 handle; + }; + struct smbios_header header; + }; u16 phys_memory_array_handle; u16 memory_error_information_handle; u16 total_width; @@ -881,9 +947,14 @@ } __packed; struct smbios_type19 { - u8 type; - u8 length; - u16 handle; + union { + struct { + u8 type; + u8 length; + u16 handle; + }; + struct smbios_header header; + }; u32 starting_address; u32 ending_address; u16 memory_array_handle; @@ -894,18 +965,28 @@ } __packed; struct smbios_type32 { - u8 type; - u8 length; - u16 handle; + union { + struct { + u8 type; + u8 length; + u16 handle; + }; + struct smbios_header header; + }; u8 reserved[6]; u8 boot_status; u8 eos[2]; } __packed; struct smbios_type38 { - u8 type; - u8 length; - u16 handle; + union { + struct { + u8 type; + u8 length; + u16 handle; + }; + struct smbios_header header; + }; u8 interface_type; u8 ipmi_rev; u8 i2c_slave_addr; @@ -940,9 +1021,14 @@ #define SMBIOS_DEVICE_TYPE_COUNT 10 struct smbios_type41 { - u8 type; - u8 length; - u16 handle; + union { + struct { + u8 type; + u8 length; + u16 handle; + }; + struct smbios_header header; + }; u8 reference_designation; u8 device_type: 7; u8 device_status: 1; @@ -955,9 +1041,14 @@ } __packed; struct smbios_type127 { - u8 type; - u8 length; - u16 handle; + union { + struct { + u8 type; + u8 length; + u16 handle; + }; + struct smbios_header header; + }; u8 eos[2]; } __packed; -- To view, visit https://review.coreboot.org/c/coreboot/+/55906 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibd9a80010f83fd7ebefc014b981d430f5723808c Gerrit-Change-Number: 55906 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: mb/pcengines/apuX: Refactor to avoid dead assignment
by Patrick Georgi (Code Review) 01 Jul '21

01 Jul '21

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/55905 ) Change subject: mb/pcengines/apuX: Refactor to avoid dead assignment ...................................................................... mb/pcengines/apuX: Refactor to avoid dead assignment The initial value of `len` is never used. Declare and initialise all local variables in a single statement to avoid this problem. Change-Id: Ieb96758f4cd19d9d3f8fdc543e8ca17db06a2123 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/55905 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/mainboard/pcengines/apu1/mainboard.c M src/mainboard/pcengines/apu2/mainboard.c 2 files changed, 6 insertions(+), 12 deletions(-) Approvals: build bot (Jenkins): Verified Michał Żygowski: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved diff --git a/src/mainboard/pcengines/apu1/mainboard.c b/src/mainboard/pcengines/apu1/mainboard.c index 05c496d..187f8cf 100644 --- a/src/mainboard/pcengines/apu1/mainboard.c +++ b/src/mainboard/pcengines/apu1/mainboard.c @@ -168,14 +168,11 @@ #if CONFIG(GENERATE_SMBIOS_TABLES) static int mainboard_smbios_type16(DMI_INFO *agesa_dmi, int *handle, unsigned long *current) { - struct smbios_type16 *t; - u32 max_capacity; - int len; + const u32 max_capacity = get_spd_offset() ? 4 : 2; /* 4GB or 2GB variant */ - t = (struct smbios_type16 *)*current; - len = sizeof(*t); + struct smbios_type16 *t = (struct smbios_type16 *)*current; + int len = sizeof(*t); memset(t, 0, len); - max_capacity = get_spd_offset() ? 4 : 2; /* 4GB or 2GB variant */ t->type = SMBIOS_PHYS_MEMORY_ARRAY; t->handle = *handle; diff --git a/src/mainboard/pcengines/apu2/mainboard.c b/src/mainboard/pcengines/apu2/mainboard.c index bcf2536..2c3b861 100644 --- a/src/mainboard/pcengines/apu2/mainboard.c +++ b/src/mainboard/pcengines/apu2/mainboard.c @@ -150,14 +150,11 @@ static int mainboard_smbios_type16(DMI_INFO *agesa_dmi, int *handle, unsigned long *current) { - struct smbios_type16 *t; - u32 max_capacity; - int len = 0; + const u32 max_capacity = get_spd_offset() ? 4 : 2; /* 4GB or 2GB variant */ - t = (struct smbios_type16 *)*current; - len = sizeof(*t); + struct smbios_type16 *t = (struct smbios_type16 *)*current; + int len = sizeof(*t); memset(t, 0, len); - max_capacity = get_spd_offset() ? 4 : 2; /* 4GB or 2GB variant */ t->type = SMBIOS_PHYS_MEMORY_ARRAY; t->handle = *handle; -- To view, visit https://review.coreboot.org/c/coreboot/+/55905 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ieb96758f4cd19d9d3f8fdc543e8ca17db06a2123 Gerrit-Change-Number: 55905 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: SMBIOS: Avoid `sizeof` on struct type
by Patrick Georgi (Code Review) 01 Jul '21

01 Jul '21

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/55904 ) Change subject: SMBIOS: Avoid `sizeof` on struct type ...................................................................... SMBIOS: Avoid `sizeof` on struct type Where applicable, use the size of the associated variable. Change-Id: Ibbac2a82893232a6f87182a6a965b84a599d633e Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/55904 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Reviewed-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> --- M src/arch/x86/smbios.c M src/drivers/elog/elog.c M src/drivers/wifi/generic/smbios.c M src/mainboard/emulation/qemu-i440fx/northbridge.c M src/mainboard/pcengines/apu1/mainboard.c M src/mainboard/pcengines/apu2/mainboard.c M src/mainboard/samsung/lumpy/mainboard.c 7 files changed, 52 insertions(+), 53 deletions(-) Approvals: build bot (Jenkins): Verified Michał Żygowski: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved diff --git a/src/arch/x86/smbios.c b/src/arch/x86/smbios.c index b21cf67..4637dc0 100644 --- a/src/arch/x86/smbios.c +++ b/src/arch/x86/smbios.c @@ -243,7 +243,7 @@ { struct smbios_type17 *t = (struct smbios_type17 *)*current; - memset(t, 0, sizeof(struct smbios_type17)); + memset(t, 0, sizeof(*t)); t->memory_type = dimm->ddr_type; if (dimm->configured_speed_mts != 0) t->clock_speed = dimm->configured_speed_mts; @@ -318,7 +318,7 @@ t->phys_memory_array_handle = type16_handle; *handle += 1; - t->length = sizeof(struct smbios_type17) - 2; + t->length = sizeof(*t) - 2; return t->length + smbios_string_table_len(t->eos); } @@ -380,9 +380,9 @@ static int smbios_write_type0(unsigned long *current, int handle) { struct smbios_type0 *t = (struct smbios_type0 *)*current; - int len = sizeof(struct smbios_type0); + int len = sizeof(*t); - memset(t, 0, sizeof(struct smbios_type0)); + memset(t, 0, sizeof(*t)); t->type = SMBIOS_BIOS_INFORMATION; t->handle = handle; t->length = len - 2; @@ -512,9 +512,9 @@ static int smbios_write_type1(unsigned long *current, int handle) { struct smbios_type1 *t = (struct smbios_type1 *)*current; - int len = sizeof(struct smbios_type1); + int len = sizeof(*t); - memset(t, 0, sizeof(struct smbios_type1)); + memset(t, 0, sizeof(*t)); t->type = SMBIOS_SYSTEM_INFORMATION; t->handle = handle; t->length = len - 2; @@ -535,9 +535,9 @@ static int smbios_write_type2(unsigned long *current, int handle, const int chassis_handle) { struct smbios_type2 *t = (struct smbios_type2 *)*current; - int len = sizeof(struct smbios_type2); + int len = sizeof(*t); - memset(t, 0, sizeof(struct smbios_type2)); + memset(t, 0, sizeof(*t)); t->type = SMBIOS_BOARD_INFORMATION; t->handle = handle; t->length = len - 2; @@ -559,9 +559,9 @@ static int smbios_write_type3(unsigned long *current, int handle) { struct smbios_type3 *t = (struct smbios_type3 *)*current; - int len = sizeof(struct smbios_type3); + int len = sizeof(*t); - memset(t, 0, sizeof(struct smbios_type3)); + memset(t, 0, sizeof(*t)); t->type = SMBIOS_SYSTEM_ENCLOSURE; t->handle = handle; t->length = len - 2; @@ -585,7 +585,7 @@ unsigned int cpu_voltage; struct cpuid_result res; struct smbios_type4 *t = (struct smbios_type4 *)*current; - int len = sizeof(struct smbios_type4); + int len = sizeof(*t); uint16_t characteristics = 0; static unsigned int cnt = 0; char buf[8]; @@ -597,7 +597,7 @@ if (cpu_have_cpuid()) res = cpuid(1); - memset(t, 0, sizeof(struct smbios_type4)); + memset(t, 0, sizeof(*t)); t->type = SMBIOS_PROCESSOR_INFORMATION; t->handle = handle; t->length = len - 2; @@ -698,10 +698,10 @@ const size_t cache_size) { struct smbios_type7 *t = (struct smbios_type7 *)*current; - int len = sizeof(struct smbios_type7); + int len = sizeof(*t); char buf[8]; - memset(t, 0, sizeof(struct smbios_type7)); + memset(t, 0, sizeof(*t)); t->type = SMBIOS_CACHE_INFORMATION; t->handle = handle; t->length = len - 2; @@ -906,10 +906,10 @@ for (i = 0; i < num_ports; i++, port++) { struct smbios_type8 *t = (struct smbios_type8 *)*current; - memset(t, 0, sizeof(struct smbios_type8)); + memset(t, 0, sizeof(*t)); t->type = SMBIOS_PORT_CONNECTOR_INFORMATION; t->handle = *handle; - t->length = sizeof(struct smbios_type8) - 2; + t->length = sizeof(*t) - 2; t->internal_reference_designator = smbios_add_string(t->eos, port->internal_reference_designator); t->internal_connector_type = port->internal_connector_type; @@ -932,9 +932,9 @@ const u16 id, u8 slot_char1, u8 slot_char2, u8 bus, u8 dev_func) { struct smbios_type9 *t = (struct smbios_type9 *)*current; - int len = sizeof(struct smbios_type9); + int len = sizeof(*t); - memset(t, 0, sizeof(struct smbios_type9)); + memset(t, 0, sizeof(*t)); t->type = SMBIOS_SYSTEM_SLOTS; t->handle = *handle; t->length = len - 2; @@ -1075,10 +1075,10 @@ if (meminfo == NULL) return 0; /* can't find mem info in cbmem */ - memset(t, 0, sizeof(struct smbios_type19)); + memset(t, 0, sizeof(*t)); t->type = SMBIOS_MEMORY_ARRAY_MAPPED_ADDRESS; - t->length = sizeof(struct smbios_type19) - 2; + t->length = sizeof(*t) - 2; t->handle = *handle; t->memory_array_handle = type16; @@ -1118,9 +1118,9 @@ static int smbios_write_type32(unsigned long *current, int handle) { struct smbios_type32 *t = (struct smbios_type32 *)*current; - int len = sizeof(struct smbios_type32); + int len = sizeof(*t); - memset(t, 0, sizeof(struct smbios_type32)); + memset(t, 0, sizeof(*t)); t->type = SMBIOS_SYSTEM_BOOT_INFORMATION; t->handle = handle; t->length = len - 2; @@ -1135,9 +1135,9 @@ const u8 irq) { struct smbios_type38 *t = (struct smbios_type38 *)*current; - int len = sizeof(struct smbios_type38); + int len = sizeof(*t); - memset(t, 0, sizeof(struct smbios_type38)); + memset(t, 0, sizeof(*t)); t->type = SMBIOS_IPMI_DEVICE_INFORMATION; t->handle = *handle; t->length = len - 2; @@ -1160,9 +1160,9 @@ u8 bus, u8 device, u8 function, u8 device_type) { struct smbios_type41 *t = (struct smbios_type41 *)*current; - int len = sizeof(struct smbios_type41); + int len = sizeof(*t); - memset(t, 0, sizeof(struct smbios_type41)); + memset(t, 0, sizeof(*t)); t->type = SMBIOS_ONBOARD_DEVICES_EXTENDED_INFORMATION; t->handle = *handle; t->length = len - 2; @@ -1184,9 +1184,9 @@ static int smbios_write_type127(unsigned long *current, int handle) { struct smbios_type127 *t = (struct smbios_type127 *)*current; - int len = sizeof(struct smbios_type127); + int len = sizeof(*t); - memset(t, 0, sizeof(struct smbios_type127)); + memset(t, 0, sizeof(*t)); t->type = SMBIOS_END_OF_TABLE; t->handle = handle; t->length = len - 2; @@ -1306,11 +1306,11 @@ printk(BIOS_DEBUG, "%s: %08lx\n", __func__, current); se = (struct smbios_entry *)current; - current += sizeof(struct smbios_entry); + current += sizeof(*se); current = ALIGN_UP(current, 16); se3 = (struct smbios_entry30 *)current; - current += sizeof(struct smbios_entry30); + current += sizeof(*se3); current = ALIGN_UP(current, 16); tables = current; @@ -1342,9 +1342,9 @@ update_max(len, max_struct_size, smbios_write_type127(&current, handle++)); /* Install SMBIOS 2.1 entry point */ - memset(se, 0, sizeof(struct smbios_entry)); + memset(se, 0, sizeof(*se)); memcpy(se->anchor, "_SM_", 4); - se->length = sizeof(struct smbios_entry); + se->length = sizeof(*se); se->major_version = 3; se->minor_version = 0; se->max_struct_size = max_struct_size; @@ -1354,21 +1354,20 @@ se->struct_table_address = (u32)tables; se->struct_table_length = len; - se->intermediate_checksum = smbios_checksum((u8 *)se + 0x10, - sizeof(struct smbios_entry) - 0x10); - se->checksum = smbios_checksum((u8 *)se, sizeof(struct smbios_entry)); + se->intermediate_checksum = smbios_checksum((u8 *)se + 0x10, sizeof(*se) - 0x10); + se->checksum = smbios_checksum((u8 *)se, sizeof(*se)); /* Install SMBIOS 3.0 entry point */ - memset(se3, 0, sizeof(struct smbios_entry30)); + memset(se3, 0, sizeof(*se3)); memcpy(se3->anchor, "_SM3_", 5); - se3->length = sizeof(struct smbios_entry30); + se3->length = sizeof(*se3); se3->major_version = 3; se3->minor_version = 0; se3->struct_table_address = (u64)tables; se3->struct_table_length = len; - se3->checksum = smbios_checksum((u8 *)se3, sizeof(struct smbios_entry30)); + se3->checksum = smbios_checksum((u8 *)se3, sizeof(*se3)); return current; } diff --git a/src/drivers/elog/elog.c b/src/drivers/elog/elog.c index 4769559..6e31686 100644 --- a/src/drivers/elog/elog.c +++ b/src/drivers/elog/elog.c @@ -594,7 +594,7 @@ int elog_smbios_write_type15(unsigned long *current, int handle) { struct smbios_type15 *t = (struct smbios_type15 *)*current; - int len = sizeof(struct smbios_type15); + int len = sizeof(*t); uintptr_t log_address; size_t elog_size = region_device_sz(&elog_state.nv_dev); diff --git a/src/drivers/wifi/generic/smbios.c b/src/drivers/wifi/generic/smbios.c index a1a8e4f..b819116 100644 --- a/src/drivers/wifi/generic/smbios.c +++ b/src/drivers/wifi/generic/smbios.c @@ -18,9 +18,9 @@ } __packed; struct smbios_type_intel_wifi *t = (struct smbios_type_intel_wifi *)*current; - int len = sizeof(struct smbios_type_intel_wifi); + int len = sizeof(*t); - memset(t, 0, sizeof(struct smbios_type_intel_wifi)); + memset(t, 0, sizeof(*t)); t->type = 0x85; t->length = len - 2; t->handle = *handle; diff --git a/src/mainboard/emulation/qemu-i440fx/northbridge.c b/src/mainboard/emulation/qemu-i440fx/northbridge.c index 8579b8f..01d036a 100644 --- a/src/mainboard/emulation/qemu-i440fx/northbridge.c +++ b/src/mainboard/emulation/qemu-i440fx/northbridge.c @@ -166,9 +166,9 @@ static int qemu_get_smbios_data16(int handle, unsigned long *current) { struct smbios_type16 *t = (struct smbios_type16 *)*current; - int len = sizeof(struct smbios_type16); + int len = sizeof(*t); - memset(t, 0, sizeof(struct smbios_type16)); + memset(t, 0, sizeof(*t)); t->type = SMBIOS_PHYS_MEMORY_ARRAY; t->handle = handle; t->length = len - 2; @@ -185,11 +185,11 @@ struct smbios_type17 *t = (struct smbios_type17 *)*current; int len; - memset(t, 0, sizeof(struct smbios_type17)); + memset(t, 0, sizeof(*t)); t->type = SMBIOS_MEMORY_DEVICE; t->handle = handle; t->phys_memory_array_handle = parent_handle; - t->length = sizeof(struct smbios_type17) - 2; + t->length = sizeof(*t) - 2; t->size = qemu_get_memory_size() / 1024; t->data_width = 64; t->total_width = 64; diff --git a/src/mainboard/pcengines/apu1/mainboard.c b/src/mainboard/pcengines/apu1/mainboard.c index cb5d4fb..05c496d 100644 --- a/src/mainboard/pcengines/apu1/mainboard.c +++ b/src/mainboard/pcengines/apu1/mainboard.c @@ -173,7 +173,7 @@ int len; t = (struct smbios_type16 *)*current; - len = sizeof(struct smbios_type16); + len = sizeof(*t); memset(t, 0, len); max_capacity = get_spd_offset() ? 4 : 2; /* 4GB or 2GB variant */ @@ -199,10 +199,10 @@ int len; t = (struct smbios_type17 *)*current; - memset(t, 0, sizeof(struct smbios_type17)); + memset(t, 0, sizeof(*t)); t->type = SMBIOS_MEMORY_DEVICE; - t->length = sizeof(struct smbios_type17) - 2; + t->length = sizeof(*t) - 2; t->handle = *handle + 1; t->phys_memory_array_handle = *handle; t->memory_error_information_handle = 0xfffe; diff --git a/src/mainboard/pcengines/apu2/mainboard.c b/src/mainboard/pcengines/apu2/mainboard.c index 56976e5..bcf2536 100644 --- a/src/mainboard/pcengines/apu2/mainboard.c +++ b/src/mainboard/pcengines/apu2/mainboard.c @@ -155,7 +155,7 @@ int len = 0; t = (struct smbios_type16 *)*current; - len = sizeof(struct smbios_type16); + len = sizeof(*t); memset(t, 0, len); max_capacity = get_spd_offset() ? 4 : 2; /* 4GB or 2GB variant */ @@ -182,10 +182,10 @@ int len; t = (struct smbios_type17 *)*current; - memset(t, 0, sizeof(struct smbios_type17)); + memset(t, 0, sizeof(*t)); t->type = SMBIOS_MEMORY_DEVICE; - t->length = sizeof(struct smbios_type17) - 2; + t->length = sizeof(*t) - 2; t->handle = *handle + 1; t->phys_memory_array_handle = *handle; t->memory_error_information_handle = 0xfffe; diff --git a/src/mainboard/samsung/lumpy/mainboard.c b/src/mainboard/samsung/lumpy/mainboard.c index cdeaf72..c2899b8 100644 --- a/src/mainboard/samsung/lumpy/mainboard.c +++ b/src/mainboard/samsung/lumpy/mainboard.c @@ -29,9 +29,9 @@ const char *name, u8 irq, u8 addr) { struct smbios_type41 *t = (struct smbios_type41 *)*current; - int len = sizeof(struct smbios_type41); + int len = sizeof(*t); - memset(t, 0, sizeof(struct smbios_type41)); + memset(t, 0, sizeof(*t)); t->type = SMBIOS_ONBOARD_DEVICES_EXTENDED_INFORMATION; t->handle = *handle; t->length = len - 2; -- To view, visit https://review.coreboot.org/c/coreboot/+/55904 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibbac2a82893232a6f87182a6a965b84a599d633e Gerrit-Change-Number: 55904 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: arch/x86/smbios.c: Trim some `len` variables
by Werner Zeh (Code Review) 01 Jul '21

01 Jul '21

Werner Zeh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/55903 ) Change subject: arch/x86/smbios.c: Trim some `len` variables ...................................................................... arch/x86/smbios.c: Trim some `len` variables Reduce the scope or remove some `len` variables. This is done to ease replacing `sizeof` on struct types in a follow-up commit, by ensuring that all to-be-replaced appearances follow the variable declarations. Change-Id: Ied38fcaf87ef5b1e4f93076b4ba2898ad1f98a72 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/55903 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Patrick Rudolph <siro(a)das-labor.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/arch/x86/smbios.c 1 file changed, 4 insertions(+), 7 deletions(-) Approvals: build bot (Jenkins): Verified Patrick Rudolph: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved diff --git a/src/arch/x86/smbios.c b/src/arch/x86/smbios.c index 9b47ff6..b21cf67 100644 --- a/src/arch/x86/smbios.c +++ b/src/arch/x86/smbios.c @@ -902,7 +902,6 @@ const struct port_information *port, size_t num_ports) { - int len = sizeof(struct smbios_type8); unsigned int totallen = 0, i; for (i = 0; i < num_ports; i++, port++) { @@ -910,7 +909,7 @@ memset(t, 0, sizeof(struct smbios_type8)); t->type = SMBIOS_PORT_CONNECTOR_INFORMATION; t->handle = *handle; - t->length = len - 2; + t->length = sizeof(struct smbios_type8) - 2; t->internal_reference_designator = smbios_add_string(t->eos, port->internal_reference_designator); t->internal_connector_type = port->internal_connector_type; @@ -1041,7 +1040,6 @@ static int smbios_write_type17(unsigned long *current, int *handle, int type16) { - int len = sizeof(struct smbios_type17); int totallen = 0; int i; @@ -1060,7 +1058,7 @@ * Otherwise, the physical installed memory size is guessed from the system * memory map, which results in a slightly smaller value than the actual size. */ - len = create_smbios_type17_for_dimm(dimm, current, handle, type16); + const int len = create_smbios_type17_for_dimm(dimm, current, handle, type16); *current += len; totallen += len; } @@ -1070,7 +1068,6 @@ static int smbios_write_type19(unsigned long *current, int *handle, int type16) { struct smbios_type19 *t = (struct smbios_type19 *)*current; - int len = sizeof(struct smbios_type19); int i; struct memory_info *meminfo; @@ -1081,7 +1078,7 @@ memset(t, 0, sizeof(struct smbios_type19)); t->type = SMBIOS_MEMORY_ARRAY_MAPPED_ADDRESS; - t->length = len - 2; + t->length = sizeof(struct smbios_type19) - 2; t->handle = *handle; t->memory_array_handle = type16; @@ -1112,7 +1109,7 @@ t->extended_ending_address--; } - len = t->length + smbios_string_table_len(t->eos); + const int len = t->length + smbios_string_table_len(t->eos); *current += len; *handle += 1; return len; -- To view, visit https://review.coreboot.org/c/coreboot/+/55903 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ied38fcaf87ef5b1e4f93076b4ba2898ad1f98a72 Gerrit-Change-Number: 55903 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: drivers/mrc_cache: Avoid `sizeof` on struct type
by Werner Zeh (Code Review) 01 Jul '21

01 Jul '21

Werner Zeh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/55902 ) Change subject: drivers/mrc_cache: Avoid `sizeof` on struct type ...................................................................... drivers/mrc_cache: Avoid `sizeof` on struct type Where applicable, use the size of the associated variable. Change-Id: Icf4f1c8fe9f54c44b041a65eb46d6ec9f9fd6367 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/55902 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Patrick Rudolph <siro(a)das-labor.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/drivers/mrc_cache/mrc_cache.c 1 file changed, 2 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Patrick Rudolph: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved diff --git a/src/drivers/mrc_cache/mrc_cache.c b/src/drivers/mrc_cache/mrc_cache.c index f97031d..841c97e 100644 --- a/src/drivers/mrc_cache/mrc_cache.c +++ b/src/drivers/mrc_cache/mrc_cache.c @@ -486,7 +486,7 @@ struct update_region_file_entry entries[] = { [0] = { - .size = sizeof(struct mrc_metadata), + .size = sizeof(*new_md), .data = new_md, }, [1] = { @@ -696,7 +696,7 @@ .data_checksum = compute_ip_checksum(data, size), }; md.header_checksum = - compute_ip_checksum(&md, sizeof(struct mrc_metadata)); + compute_ip_checksum(&md, sizeof(md)); if (CONFIG(MRC_STASH_TO_CBMEM)) { /* Store data in cbmem for use in ramstage */ -- To view, visit https://review.coreboot.org/c/coreboot/+/55902 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icf4f1c8fe9f54c44b041a65eb46d6ec9f9fd6367 Gerrit-Change-Number: 55902 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: Documentation: Remove KASAN from the project ideas list
by Werner Zeh (Code Review) 01 Jul '21

01 Jul '21

Werner Zeh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/55991 ) Change subject: Documentation: Remove KASAN from the project ideas list ...................................................................... Documentation: Remove KASAN from the project ideas list This project is already implemented and therefore should not be mentioned anymore as a new project idea in the documentation. Change-Id: I38c6e274e416b98485943d36536a57a14743945b Signed-off-by: Werner Zeh <werner.zeh(a)siemens.com> --- M Documentation/contributing/project_ideas.md 1 file changed, 0 insertions(+), 19 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/55991/1 diff --git a/Documentation/contributing/project_ideas.md b/Documentation/contributing/project_ideas.md index 141023f..2f6f738 100644 --- a/Documentation/contributing/project_ideas.md +++ b/Documentation/contributing/project_ideas.md @@ -66,25 +66,6 @@ ### Mentors * Timothy Pearson <tpearson(a)raptorengineering.com> -## Add Kernel Address Sanitizer functionality to coreboot -The Kernel Address Sanitizer (KASAN) is a runtime dynamic memory error detector. -The idea is to check every memory access (variables) for its validity -during runtime and find bugs like stack overflow or out-of-bounds accesses. -Implementing this stub into coreboot like "Undefined behavior sanitizer support" -would help to ensure code quality and make the runtime code more robust. - -### Requirements -* knowledge in the coreboot build system and the concept of stages -* the KASAN feature can be improved in a way so that the memory space needed - during runtime is not on a fixed address provided during compile time but - determined during runtime. For this to achieve a small patch to the GCC will - be helpful. Therefore minor GCC knowledge would be beneficial. -* Implementation can be initially done in QEMU and improved on different - mainboards and platforms - -### Mentors -* Werner Zeh <werner.zeh(a)gmx.net> - ## Port payloads to ARM, AArch64 or RISC-V While we have a rather big set of payloads for x86 based platforms, all other architectures are rather limited. Improve the situation by porting a payload -- To view, visit https://review.coreboot.org/c/coreboot/+/55991 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I38c6e274e416b98485943d36536a57a14743945b Gerrit-Change-Number: 55991 Gerrit-PatchSet: 1 Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: mb/google/brya/var/redrix: Generate SPD ID for supported parts
by Wisley Chen (Code Review) 01 Jul '21

01 Jul '21

Attention is currently required from: Chen Wisley. Wisley Chen has uploaded a new patch set (#6) to the change originally created by Chen Wisley. ( https://review.coreboot.org/c/coreboot/+/55885 ) Change subject: mb/google/brya/var/redrix: Generate SPD ID for supported parts ...................................................................... mb/google/brya/var/redrix: Generate SPD ID for supported parts Add supported memory parts in mem_parts_used.txt, and generate SPD id for these parts. MT53E1G32D2NP-046 WT:A H9HCNNNBKMMLXR-NEE K4U6E3S4AA-MGCR MT53E512M32D2NP-046 WT:E H9HCNNNCPMMLXR-NEE K4UBE3D4AA-MGCR H9HCNNNFAMMLXR-NEE MT53E2G32D4NQ-046 WT:A BUG=b:190818098, b:190874372, b:192052098 TEST=build Change-Id: I62ee401e43bef22b4b09f41ea59bbdbc479f293c Signed-off-by: Wisley Chen <wisley.chen(a)quanta.corp-partner.google.com> --- M src/mainboard/google/brya/variants/redrix/memory/Makefile.inc M src/mainboard/google/brya/variants/redrix/memory/dram_id.generated.txt M src/mainboard/google/brya/variants/redrix/memory/mem_parts_used.txt 3 files changed, 22 insertions(+), 13 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/55885/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/55885 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ee401e43bef22b4b09f41ea59bbdbc479f293c Gerrit-Change-Number: 55885 Gerrit-PatchSet: 6 Gerrit-Owner: Chen Wisley <wisley.chen(a)quantatw.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Wisley Chen <wisley.chen(a)quanta.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Chen Wisley <wisley.chen(a)quantatw.com> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit July 2021