Change in ...coreboot[master]: libpayload: define cbtable tags as enum to identify conflicts

[Patrick Georgi (Code Review)]

Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32957

Change subject: libpayload: define cbtable tags as enum to identify conflicts
......................................................................

libpayload: define cbtable tags as enum to identify conflicts

We had a value that was overloaded thrice.
By moving them in a common structure and ordering them by value such
issues are hopefully avoided in the future.

Change-Id: I0c7762601d7620413989b458fa634d7606accc9d
Signed-off-by: Patrick Georgi 
---
M payloads/libpayload/include/coreboot_tables.h
1 file changed, 49 insertions(+), 55 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/32957/1

diff --git a/payloads/libpayload/include/coreboot_tables.h b/payloads/libpayload/include/coreboot_tables.h
index 8e5cec0..c12db03 100644
--- a/payloads/libpayload/include/coreboot_tables.h
+++ b/payloads/libpayload/include/coreboot_tables.h
@@ -33,6 +33,55 @@
 #include <arch/types.h>
 #include <ipchksum.h>

+enum {
+	CB_TAG_UNUSED			= 0x0000,
+	CB_TAG_MEMORY			= 0x0001,
+	CB_TAG_HWRPB			= 0x0002,
+	CB_TAG_MAINBOARD		= 0x0003,
+	CB_TAG_VERSION			= 0x0004,
+	CB_TAG_EXTRA_VERSION		= 0x0005,
+	CB_TAG_BUILD			= 0x0006,
+	CB_TAG_COMPILE_TIME		= 0x0007,
+	CB_TAG_COMPILE_BY		= 0x0008,
+	CB_TAG_COMPILE_HOST		= 0x0009,
+	CB_TAG_COMPILE_DOMAIN		= 0x000a,
+	CB_TAG_COMPILER			= 0x000b,
+	CB_TAG_LINKER			= 0x000c,
+	CB_TAG_ASSEMBLER		= 0x000d,
+	CB_TAG_SERIAL			= 0x000f,
+	CB_TAG_CONSOLE			= 0x0010,
+	CB_TAG_FORWARD			= 0x0011,
+	CB_TAG_FRAMEBUFFER		= 0x0012,
+	CB_TAG_GPIO			= 0x0013,
+	CB_TAG_TIMESTAMPS		= 0x0016,
+	CB_TAG_CBMEM_CONSOLE		= 0x0017,
+	CB_TAG_MRC_CACHE		= 0x0018,
+	CB_TAG_VBNV			= 0x0019,
+	CB_TAG_VBOOT_HANDOFF		= 0x0020,
+	CB_TAG_X86_ROM_MTRR		= 0x0021,
+	CB_TAG_DMA			= 0x0022,
+	CB_TAG_RAM_OOPS			= 0x0023,
+	CB_TAG_ACPI_GNVS		= 0x0024,
+	CB_TAG_BOARD_ID			= 0x0025,
+	CB_TAG_WIFI_CALIBRATION		= 0x0027,
+	CB_TAG_RAM_CODE			= 0x0028,
+	CB_TAG_SERIALNO			= 0x002a,
+	CB_TAG_SPI_FLASH		= 0x0029,
+	CB_TAG_MTC			= 0x002b,
+	CB_TAG_VPD			= 0x002c,
+	CB_TAG_SKU_ID			= 0x002d,
+	CB_TAG_BOOT_MEDIA_PARAMS	= 0x0030,
+	CB_TAG_TSC_INFO			= 0x0032,
+	CB_TAG_MAC_ADDRS		= 0x0033,
+	CB_TAG_VBOOT_WORKBUF		= 0x0034,
+	CB_TAG_MMC_INFO			= 0x0035,
+	CB_TAG_CMOS_OPTION_TABLE	= 0x00c8,
+	CB_TAG_OPTION			= 0x00c9,
+	CB_TAG_OPTION_ENUM		= 0x00ca,
+	CB_TAG_OPTION_DEFAULTS		= 0x00cb,
+	CB_TAG_OPTION_CHECKSUM		= 0x00cc,
+};
+
 struct cbuint64 {
    u32 lo;
    u32 hi;
@@ -52,9 +101,6 @@
    u32 size;
 };

-#define CB_TAG_UNUSED     0x0000
-#define CB_TAG_MEMORY     0x0001
-
 struct cb_memory_range {
    struct cbuint64 start;
    struct cbuint64 size;
@@ -75,16 +121,12 @@
    struct cb_memory_range map[0];
 };

-#define CB_TAG_HWRPB      0x0002
-
 struct cb_hwrpb {
    u32 tag;
    u32 size;
    u64 hwrpb;
 };

-#define CB_TAG_MAINBOARD  0x0003
-
 struct cb_mainboard {
    u32 tag;
    u32 size;
@@ -93,25 +135,12 @@
    u8 strings[0];
 };

-#define CB_TAG_VERSION        0x0004
-#define CB_TAG_EXTRA_VERSION  0x0005
-#define CB_TAG_BUILD          0x0006
-#define CB_TAG_COMPILE_TIME   0x0007
-#define CB_TAG_COMPILE_BY     0x0008
-#define CB_TAG_COMPILE_HOST   0x0009
-#define CB_TAG_COMPILE_DOMAIN 0x000a
-#define CB_TAG_COMPILER       0x000b
-#define CB_TAG_LINKER         0x000c
-#define CB_TAG_ASSEMBLER      0x000d
-
 struct cb_string {
    u32 tag;
    u32 size;
    u8 string[0];
 };

-#define CB_TAG_SERIAL         0x000f
-
 struct cb_serial {
    u32 tag;
    u32 size;
@@ -138,8 +167,6 @@
    u32 uart_pci_addr;
 };

-#define CB_TAG_CONSOLE       0x00010
-
 struct cb_console {
    u32 tag;
    u32 size;
@@ -153,15 +180,12 @@
 #define CB_TAG_CONSOLE_SROM       4 // OBSOLETE
 #define CB_TAG_CONSOLE_EHCI       5

-#define CB_TAG_FORWARD       0x00011
-
 struct cb_forward {
    u32 tag;
    u32 size;
    u64 forward;
 };

-#define CB_TAG_FRAMEBUFFER      0x0012
 struct cb_framebuffer {
    u32 tag;
    u32 size;
@@ -181,7 +205,6 @@
    u8 reserved_mask_size;
 };

-#define CB_TAG_GPIO 0x0013
 #define CB_GPIO_ACTIVE_LOW 0
 #define CB_GPIO_ACTIVE_HIGH 1
 #define CB_GPIO_MAX_NAME_LENGTH 16
@@ -200,13 +223,6 @@
    struct cb_gpio gpios[0];
 };

-#define CB_TAG_VBNV		0x0019
-#define CB_TAG_VBOOT_HANDOFF	0x0020
-#define CB_TAG_VBOOT_WORKBUF	0x0034
-#define CB_TAG_DMA		0x0022
-#define CB_TAG_RAM_OOPS		0x0023
-#define CB_TAG_MTC		0x002b
-#define CB_TAG_VPD		0x002c
 struct lb_range {
    uint32_t tag;
    uint32_t size;
@@ -214,18 +230,12 @@
    uint32_t range_size;
 };

-#define CB_TAG_TIMESTAMPS	0x0016
-#define CB_TAG_CBMEM_CONSOLE	0x0017
-#define CB_TAG_MRC_CACHE	0x0018
-#define CB_TAG_ACPI_GNVS	0x0024
-#define CB_TAG_WIFI_CALIBRATION	0x0027
 struct cb_cbmem_tab {
    uint32_t tag;
    uint32_t size;
    uint64_t cbmem_tab;
 };

-#define CB_TAG_X86_ROM_MTRR	0x0021
 struct cb_x86_rom_mtrr {
    uint32_t tag;
    uint32_t size;
@@ -236,17 +246,12 @@
    uint32_t index;
 };

-
-#define CB_TAG_BOARD_ID		0x0025
-#define CB_TAG_RAM_CODE		0x0028
-#define CB_TAG_SKU_ID		0x002d
 struct cb_strapping_id {
    uint32_t tag;
    uint32_t size;
    uint32_t id_code;
 };

-#define CB_TAG_SPI_FLASH	0x0029
 struct cb_spi_flash {
    uint32_t tag;
    uint32_t size;
@@ -255,7 +260,6 @@
    uint32_t erase_cmd;
 };

-#define CB_TAG_BOOT_MEDIA_PARAMS 0x0030
 struct cb_boot_media_params {
    uint32_t tag;
    uint32_t size;
@@ -266,7 +270,6 @@
    uint64_t boot_media_size;
 };

-#define CB_TAG_TSC_INFO 0x0032
 struct cb_tsc_info {
    uint32_t tag;
    uint32_t size;
@@ -274,7 +277,6 @@
    uint32_t freq_khz;
 };

-#define CB_TAG_MAC_ADDRS       0x0033
 struct mac_address {
    uint8_t mac_addr[6];
    uint8_t pad[2];         /* Pad it to 8 bytes to keep it simple. */
@@ -287,7 +289,6 @@
    struct mac_address mac_addrs[0];
 };

-#define CB_TAG_MMC_INFO		0x0035
 struct cb_mmc_info {
    uint32_t tag;
    uint32_t size;
@@ -302,17 +303,14 @@
    int32_t early_cmd1_status;
 };

-#define CB_TAG_SERIALNO		0x002a
 #define CB_MAX_SERIALNO_LENGTH	32

-#define CB_TAG_CMOS_OPTION_TABLE 0x00c8
 struct cb_cmos_option_table {
    u32 tag;
    u32 size;
    u32 header_length;
 };

-#define CB_TAG_OPTION         0x00c9
 #define CB_CMOS_MAX_NAME_LENGTH    32
 struct cb_cmos_entries {
    u32 tag;
@@ -324,8 +322,6 @@
    u8 name[CB_CMOS_MAX_NAME_LENGTH];
 };

-
-#define CB_TAG_OPTION_ENUM    0x00ca
 #define CB_CMOS_MAX_TEXT_LENGTH 32
 struct cb_cmos_enums {
    u32 tag;
@@ -335,7 +331,6 @@
    u8 text[CB_CMOS_MAX_TEXT_LENGTH];
 };

-#define CB_TAG_OPTION_DEFAULTS 0x00cb
 #define CB_CMOS_IMAGE_BUFFER_SIZE 128
 struct cb_cmos_defaults {
    u32 tag;
@@ -345,7 +340,6 @@
    u8 default_set[CB_CMOS_IMAGE_BUFFER_SIZE];
 };

-#define CB_TAG_OPTION_CHECKSUM 0x00cc
 #define CB_CHECKSUM_NONE	0
 #define CB_CHECKSUM_PCBIOS	1
 struct	cb_cmos_checksum {

[Patrick Georgi (Code Review)]

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/32957

to look at the new patch set (#2).

Change subject: libpayload: define cbtable tags as enum to identify conflicts
......................................................................

libpayload: define cbtable tags as enum to identify conflicts

We had a value that was overloaded thrice.
By moving them in a common structure and ordering them by value such
issues are hopefully avoided in the future.

Change-Id: I0c7762601d7620413989b458fa634d7606accc9d
Signed-off-by: Patrick Georgi 
---
M payloads/libpayload/include/coreboot_tables.h
1 file changed, 49 insertions(+), 55 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/32957/2

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32957 )

Change subject: libpayload: define cbtable tags as enum to identify conflicts
......................................................................

Patch Set 2: Code-Review+1

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32957 )

Change subject: libpayload: define cbtable tags as enum to identify conflicts
......................................................................

Patch Set 3: Code-Review+2

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32957 )

Change subject: libpayload: define cbtable tags as enum to identify conflicts
......................................................................

Patch Set 3: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32957 )

Change subject: libpayload: define cbtable tags as enum to identify conflicts
......................................................................

libpayload: define cbtable tags as enum to identify conflicts

We had a value that was overloaded thrice.
By moving them in a common structure and ordering them by value such
issues are hopefully avoided in the future.

Change-Id: I0c7762601d7620413989b458fa634d7606accc9d
Signed-off-by: Patrick Georgi 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32957
Reviewed-by: Arthur Heymans 
Reviewed-by: HAOUAS Elyes 
Tested-by: build bot (Jenkins) 
---
M payloads/libpayload/include/coreboot_tables.h
1 file changed, 49 insertions(+), 55 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  HAOUAS Elyes: Looks good to me, approved
  Arthur Heymans: Looks good to me, approved

diff --git a/payloads/libpayload/include/coreboot_tables.h b/payloads/libpayload/include/coreboot_tables.h
index 8e5cec0..7226c0a 100644
--- a/payloads/libpayload/include/coreboot_tables.h
+++ b/payloads/libpayload/include/coreboot_tables.h
@@ -33,6 +33,55 @@
 #include <arch/types.h>
 #include <ipchksum.h>

+enum {
+	CB_TAG_UNUSED			= 0x0000,
+	CB_TAG_MEMORY			= 0x0001,
+	CB_TAG_HWRPB			= 0x0002,
+	CB_TAG_MAINBOARD		= 0x0003,
+	CB_TAG_VERSION			= 0x0004,
+	CB_TAG_EXTRA_VERSION		= 0x0005,
+	CB_TAG_BUILD			= 0x0006,
+	CB_TAG_COMPILE_TIME		= 0x0007,
+	CB_TAG_COMPILE_BY		= 0x0008,
+	CB_TAG_COMPILE_HOST		= 0x0009,
+	CB_TAG_COMPILE_DOMAIN		= 0x000a,
+	CB_TAG_COMPILER			= 0x000b,
+	CB_TAG_LINKER			= 0x000c,
+	CB_TAG_ASSEMBLER		= 0x000d,
+	CB_TAG_SERIAL			= 0x000f,
+	CB_TAG_CONSOLE			= 0x0010,
+	CB_TAG_FORWARD			= 0x0011,
+	CB_TAG_FRAMEBUFFER		= 0x0012,
+	CB_TAG_GPIO			= 0x0013,
+	CB_TAG_TIMESTAMPS		= 0x0016,
+	CB_TAG_CBMEM_CONSOLE		= 0x0017,
+	CB_TAG_MRC_CACHE		= 0x0018,
+	CB_TAG_VBNV			= 0x0019,
+	CB_TAG_VBOOT_HANDOFF		= 0x0020,
+	CB_TAG_X86_ROM_MTRR		= 0x0021,
+	CB_TAG_DMA			= 0x0022,
+	CB_TAG_RAM_OOPS			= 0x0023,
+	CB_TAG_ACPI_GNVS		= 0x0024,
+	CB_TAG_BOARD_ID			= 0x0025,
+	CB_TAG_WIFI_CALIBRATION		= 0x0027,
+	CB_TAG_RAM_CODE			= 0x0028,
+	CB_TAG_SPI_FLASH		= 0x0029,
+	CB_TAG_SERIALNO			= 0x002a,
+	CB_TAG_MTC			= 0x002b,
+	CB_TAG_VPD			= 0x002c,
+	CB_TAG_SKU_ID			= 0x002d,
+	CB_TAG_BOOT_MEDIA_PARAMS	= 0x0030,
+	CB_TAG_TSC_INFO			= 0x0032,
+	CB_TAG_MAC_ADDRS		= 0x0033,
+	CB_TAG_VBOOT_WORKBUF		= 0x0034,
+	CB_TAG_MMC_INFO			= 0x0035,
+	CB_TAG_CMOS_OPTION_TABLE	= 0x00c8,
+	CB_TAG_OPTION			= 0x00c9,
+	CB_TAG_OPTION_ENUM		= 0x00ca,
+	CB_TAG_OPTION_DEFAULTS		= 0x00cb,
+	CB_TAG_OPTION_CHECKSUM		= 0x00cc,
+};
+
 struct cbuint64 {
    u32 lo;
    u32 hi;
@@ -52,9 +101,6 @@
    u32 size;
 };

-#define CB_TAG_UNUSED     0x0000
-#define CB_TAG_MEMORY     0x0001
-
 struct cb_memory_range {
    struct cbuint64 start;
    struct cbuint64 size;
@@ -75,16 +121,12 @@
    struct cb_memory_range map[0];
 };

-#define CB_TAG_HWRPB      0x0002
-
 struct cb_hwrpb {
    u32 tag;
    u32 size;
    u64 hwrpb;
 };

-#define CB_TAG_MAINBOARD  0x0003
-
 struct cb_mainboard {
    u32 tag;
    u32 size;
@@ -93,25 +135,12 @@
    u8 strings[0];
 };

-#define CB_TAG_VERSION        0x0004
-#define CB_TAG_EXTRA_VERSION  0x0005
-#define CB_TAG_BUILD          0x0006
-#define CB_TAG_COMPILE_TIME   0x0007
-#define CB_TAG_COMPILE_BY     0x0008
-#define CB_TAG_COMPILE_HOST   0x0009
-#define CB_TAG_COMPILE_DOMAIN 0x000a
-#define CB_TAG_COMPILER       0x000b
-#define CB_TAG_LINKER         0x000c
-#define CB_TAG_ASSEMBLER      0x000d
-
 struct cb_string {
    u32 tag;
    u32 size;
    u8 string[0];
 };

-#define CB_TAG_SERIAL         0x000f
-
 struct cb_serial {
    u32 tag;
    u32 size;
@@ -138,8 +167,6 @@
    u32 uart_pci_addr;
 };

-#define CB_TAG_CONSOLE       0x00010
-
 struct cb_console {
    u32 tag;
    u32 size;
@@ -153,15 +180,12 @@
 #define CB_TAG_CONSOLE_SROM       4 // OBSOLETE
 #define CB_TAG_CONSOLE_EHCI       5

-#define CB_TAG_FORWARD       0x00011
-
 struct cb_forward {
    u32 tag;
    u32 size;
    u64 forward;
 };

-#define CB_TAG_FRAMEBUFFER      0x0012
 struct cb_framebuffer {
    u32 tag;
    u32 size;
@@ -181,7 +205,6 @@
    u8 reserved_mask_size;
 };

-#define CB_TAG_GPIO 0x0013
 #define CB_GPIO_ACTIVE_LOW 0
 #define CB_GPIO_ACTIVE_HIGH 1
 #define CB_GPIO_MAX_NAME_LENGTH 16
@@ -200,13 +223,6 @@
    struct cb_gpio gpios[0];
 };

-#define CB_TAG_VBNV		0x0019
-#define CB_TAG_VBOOT_HANDOFF	0x0020
-#define CB_TAG_VBOOT_WORKBUF	0x0034
-#define CB_TAG_DMA		0x0022
-#define CB_TAG_RAM_OOPS		0x0023
-#define CB_TAG_MTC		0x002b
-#define CB_TAG_VPD		0x002c
 struct lb_range {
    uint32_t tag;
    uint32_t size;
@@ -214,18 +230,12 @@
    uint32_t range_size;
 };

-#define CB_TAG_TIMESTAMPS	0x0016
-#define CB_TAG_CBMEM_CONSOLE	0x0017
-#define CB_TAG_MRC_CACHE	0x0018
-#define CB_TAG_ACPI_GNVS	0x0024
-#define CB_TAG_WIFI_CALIBRATION	0x0027
 struct cb_cbmem_tab {
    uint32_t tag;
    uint32_t size;
    uint64_t cbmem_tab;
 };

-#define CB_TAG_X86_ROM_MTRR	0x0021
 struct cb_x86_rom_mtrr {
    uint32_t tag;
    uint32_t size;
@@ -236,17 +246,12 @@
    uint32_t index;
 };

-
-#define CB_TAG_BOARD_ID		0x0025
-#define CB_TAG_RAM_CODE		0x0028
-#define CB_TAG_SKU_ID		0x002d
 struct cb_strapping_id {
    uint32_t tag;
    uint32_t size;
    uint32_t id_code;
 };

-#define CB_TAG_SPI_FLASH	0x0029
 struct cb_spi_flash {
    uint32_t tag;
    uint32_t size;
@@ -255,7 +260,6 @@
    uint32_t erase_cmd;
 };

-#define CB_TAG_BOOT_MEDIA_PARAMS 0x0030
 struct cb_boot_media_params {
    uint32_t tag;
    uint32_t size;
@@ -266,7 +270,6 @@
    uint64_t boot_media_size;
 };

-#define CB_TAG_TSC_INFO 0x0032
 struct cb_tsc_info {
    uint32_t tag;
    uint32_t size;
@@ -274,7 +277,6 @@
    uint32_t freq_khz;
 };

-#define CB_TAG_MAC_ADDRS       0x0033
 struct mac_address {
    uint8_t mac_addr[6];
    uint8_t pad[2];         /* Pad it to 8 bytes to keep it simple. */
@@ -287,7 +289,6 @@
    struct mac_address mac_addrs[0];
 };

-#define CB_TAG_MMC_INFO		0x0035
 struct cb_mmc_info {
    uint32_t tag;
    uint32_t size;
@@ -302,17 +303,14 @@
    int32_t early_cmd1_status;
 };

-#define CB_TAG_SERIALNO		0x002a
 #define CB_MAX_SERIALNO_LENGTH	32

-#define CB_TAG_CMOS_OPTION_TABLE 0x00c8
 struct cb_cmos_option_table {
    u32 tag;
    u32 size;
    u32 header_length;
 };

-#define CB_TAG_OPTION         0x00c9
 #define CB_CMOS_MAX_NAME_LENGTH    32
 struct cb_cmos_entries {
    u32 tag;
@@ -324,8 +322,6 @@
    u8 name[CB_CMOS_MAX_NAME_LENGTH];
 };

-
-#define CB_TAG_OPTION_ENUM    0x00ca
 #define CB_CMOS_MAX_TEXT_LENGTH 32
 struct cb_cmos_enums {
    u32 tag;
@@ -335,7 +331,6 @@
    u8 text[CB_CMOS_MAX_TEXT_LENGTH];
 };

-#define CB_TAG_OPTION_DEFAULTS 0x00cb
 #define CB_CMOS_IMAGE_BUFFER_SIZE 128
 struct cb_cmos_defaults {
    u32 tag;
@@ -345,7 +340,6 @@
    u8 default_set[CB_CMOS_IMAGE_BUFFER_SIZE];
 };

-#define CB_TAG_OPTION_CHECKSUM 0x00cc
 #define CB_CHECKSUM_NONE	0
 #define CB_CHECKSUM_PCBIOS	1
 struct	cb_cmos_checksum {