Re: [PATCH v2 11/20] smbios: build legacy mode code only for 'pc' machine
On Wed, 6 Mar 2024 12:57:47 +0530 Ani Sinha wrote: > > On 05-Mar-2024, at 21:27, Igor Mammedov wrote: > > > > basically moving code around without functional change. > > And exposing some symbols so that they could be shared > > between smbbios.c and new smbios_legacy.c > > > > plus some meson magic to build smbios_legacy.c only > > for 'pc' machine and otherwise replace it with stub > > if not selected. > > When I apply this, I get: > > Applying: smbios: build legacy mode code only for 'pc' machine > .git/rebase-apply/patch:483: new blank line at EOF. > + > warning: 1 line adds whitespace errors. it looks like checkpatch missed that, v3 is on the way as a reply to this > > Can you please look into this? Modulo that, > > > > > Signed-off-by: Igor Mammedov > > Reviewed-by: Ani Sinha > > > --- > > v2: > > moved type0/type1/have_binfile_bitmap/have_fields_bitmap rename into > > a separate patch > > --- > > include/hw/firmware/smbios.h | 5 + > > hw/i386/Kconfig| 1 + > > hw/smbios/Kconfig | 2 + > > hw/smbios/meson.build | 4 + > > hw/smbios/smbios.c | 164 +- > > hw/smbios/smbios_legacy.c | 179 + > > hw/smbios/smbios_legacy_stub.c | 16 +++ > > 7 files changed, 208 insertions(+), 163 deletions(-) > > create mode 100644 hw/smbios/smbios_legacy.c > > create mode 100644 hw/smbios/smbios_legacy_stub.c > > > > diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h > > index bea5c3f1b1..7f47475aa4 100644 > > --- a/include/hw/firmware/smbios.h > > +++ b/include/hw/firmware/smbios.h > > @@ -17,6 +17,9 @@ > > * > > */ > > > > +extern uint8_t *usr_blobs; > > +extern GArray *usr_blobs_sizes; > > + > > typedef struct { > > const char *vendor, *version, *date; > > bool have_major_minor, uefi; > > @@ -306,6 +309,8 @@ struct smbios_type_127 { > > struct smbios_structure_header header; > > } QEMU_PACKED; > > > > +void smbios_validate_table(void); > > +void smbios_add_usr_blob_size(size_t size); > > void smbios_entry_add(QemuOpts *opts, Error **errp); > > void smbios_set_cpuid(uint32_t version, uint32_t features); > > void smbios_set_defaults(const char *manufacturer, const char *product, > > diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig > > index a1846be6f7..a6ee052f9a 100644 > > --- a/hw/i386/Kconfig > > +++ b/hw/i386/Kconfig > > @@ -76,6 +76,7 @@ config I440FX > > select PIIX > > select DIMM > > select SMBIOS > > +select SMBIOS_LEGACY > > select FW_CFG_DMA > > > > config ISAPC > > diff --git a/hw/smbios/Kconfig b/hw/smbios/Kconfig > > index 553adf4bfc..8d989a2f1b 100644 > > --- a/hw/smbios/Kconfig > > +++ b/hw/smbios/Kconfig > > @@ -1,2 +1,4 @@ > > config SMBIOS > > bool > > +config SMBIOS_LEGACY > > +bool > > diff --git a/hw/smbios/meson.build b/hw/smbios/meson.build > > index 7046967462..a59039f669 100644 > > --- a/hw/smbios/meson.build > > +++ b/hw/smbios/meson.build > > @@ -4,5 +4,9 @@ smbios_ss.add(when: 'CONFIG_IPMI', > > if_true: files('smbios_type_38.c'), > > if_false: files('smbios_type_38-stub.c')) > > > > +smbios_ss.add(when: 'CONFIG_SMBIOS_LEGACY', > > + if_true: files('smbios_legacy.c'), > > + if_false: files('smbios_legacy_stub.c')) > > + > > system_ss.add_all(when: 'CONFIG_SMBIOS', if_true: smbios_ss) > > system_ss.add(when: 'CONFIG_SMBIOS', if_false: files('smbios-stub.c')) > > diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c > > index 86cf66b5ce..fb1f05fcde 100644 > > --- a/hw/smbios/smbios.c > > +++ b/hw/smbios/smbios.c > > @@ -31,38 +31,12 @@ > > #include "hw/pci/pci_device.h" > > #include "smbios_build.h" > > > > -/* legacy structures and constants for <= 2.0 machines */ > > -struct smbios_header { > > -uint16_t length; > > -uint8_t type; > > -} QEMU_PACKED; > > - > > -struct smbios_field { > > -struct smbios_header header; > > -uint8_t type; > > -uint16_t offset; > > -uint8_t data[]; > > -} QEMU_PACKED; > > - > > -struct smbios_table { > > -struct smbios_header header; > > -uint8_t data[]; > > -} QEMU_PACKED; > > - > > -#define SMBIOS_FIELD_ENTRY 0 > > -#define SMBIOS_TABLE_ENTRY 1 > > - > > -static uint8_t *smbios_entries; > > -static size_t smbios_entries_len; > > static bool smbios_uuid_encoded = true; > > -/* end: legacy structures & constants for <= 2.0 machines */ > > - > > /* > > * SMBIOS tables provided by user with '-smbios file=' option > > */ > > uint8_t *usr_blobs; > > size_t usr_blobs_len; > > -static GArray *usr_blobs_sizes; > > static unsigned usr_table_max; > > static unsigned usr_table_cnt; > > > > @@ -483,7 +457,7 @@ static void smbios_check_type4_count(uint32_t > > expected_t4_count) > > } > > } > > > > -static void smbios_validate_table(void) > > +void smbios_validate_table(void) > > { > > if (smbios_ep_type == SMBIOS_ENTRY_POINT_TYPE_32 && > > smbios_tables_len >
Re: [PATCH v2 11/20] smbios: build legacy mode code only for 'pc' machine
> On 05-Mar-2024, at 21:27, Igor Mammedov wrote: > > basically moving code around without functional change. > And exposing some symbols so that they could be shared > between smbbios.c and new smbios_legacy.c > > plus some meson magic to build smbios_legacy.c only > for 'pc' machine and otherwise replace it with stub > if not selected. When I apply this, I get: Applying: smbios: build legacy mode code only for 'pc' machine .git/rebase-apply/patch:483: new blank line at EOF. + warning: 1 line adds whitespace errors. Can you please look into this? Modulo that, > > Signed-off-by: Igor Mammedov Reviewed-by: Ani Sinha > --- > v2: > moved type0/type1/have_binfile_bitmap/have_fields_bitmap rename into > a separate patch > --- > include/hw/firmware/smbios.h | 5 + > hw/i386/Kconfig| 1 + > hw/smbios/Kconfig | 2 + > hw/smbios/meson.build | 4 + > hw/smbios/smbios.c | 164 +- > hw/smbios/smbios_legacy.c | 179 + > hw/smbios/smbios_legacy_stub.c | 16 +++ > 7 files changed, 208 insertions(+), 163 deletions(-) > create mode 100644 hw/smbios/smbios_legacy.c > create mode 100644 hw/smbios/smbios_legacy_stub.c > > diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h > index bea5c3f1b1..7f47475aa4 100644 > --- a/include/hw/firmware/smbios.h > +++ b/include/hw/firmware/smbios.h > @@ -17,6 +17,9 @@ > * > */ > > +extern uint8_t *usr_blobs; > +extern GArray *usr_blobs_sizes; > + > typedef struct { > const char *vendor, *version, *date; > bool have_major_minor, uefi; > @@ -306,6 +309,8 @@ struct smbios_type_127 { > struct smbios_structure_header header; > } QEMU_PACKED; > > +void smbios_validate_table(void); > +void smbios_add_usr_blob_size(size_t size); > void smbios_entry_add(QemuOpts *opts, Error **errp); > void smbios_set_cpuid(uint32_t version, uint32_t features); > void smbios_set_defaults(const char *manufacturer, const char *product, > diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig > index a1846be6f7..a6ee052f9a 100644 > --- a/hw/i386/Kconfig > +++ b/hw/i386/Kconfig > @@ -76,6 +76,7 @@ config I440FX > select PIIX > select DIMM > select SMBIOS > +select SMBIOS_LEGACY > select FW_CFG_DMA > > config ISAPC > diff --git a/hw/smbios/Kconfig b/hw/smbios/Kconfig > index 553adf4bfc..8d989a2f1b 100644 > --- a/hw/smbios/Kconfig > +++ b/hw/smbios/Kconfig > @@ -1,2 +1,4 @@ > config SMBIOS > bool > +config SMBIOS_LEGACY > +bool > diff --git a/hw/smbios/meson.build b/hw/smbios/meson.build > index 7046967462..a59039f669 100644 > --- a/hw/smbios/meson.build > +++ b/hw/smbios/meson.build > @@ -4,5 +4,9 @@ smbios_ss.add(when: 'CONFIG_IPMI', > if_true: files('smbios_type_38.c'), > if_false: files('smbios_type_38-stub.c')) > > +smbios_ss.add(when: 'CONFIG_SMBIOS_LEGACY', > + if_true: files('smbios_legacy.c'), > + if_false: files('smbios_legacy_stub.c')) > + > system_ss.add_all(when: 'CONFIG_SMBIOS', if_true: smbios_ss) > system_ss.add(when: 'CONFIG_SMBIOS', if_false: files('smbios-stub.c')) > diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c > index 86cf66b5ce..fb1f05fcde 100644 > --- a/hw/smbios/smbios.c > +++ b/hw/smbios/smbios.c > @@ -31,38 +31,12 @@ > #include "hw/pci/pci_device.h" > #include "smbios_build.h" > > -/* legacy structures and constants for <= 2.0 machines */ > -struct smbios_header { > -uint16_t length; > -uint8_t type; > -} QEMU_PACKED; > - > -struct smbios_field { > -struct smbios_header header; > -uint8_t type; > -uint16_t offset; > -uint8_t data[]; > -} QEMU_PACKED; > - > -struct smbios_table { > -struct smbios_header header; > -uint8_t data[]; > -} QEMU_PACKED; > - > -#define SMBIOS_FIELD_ENTRY 0 > -#define SMBIOS_TABLE_ENTRY 1 > - > -static uint8_t *smbios_entries; > -static size_t smbios_entries_len; > static bool smbios_uuid_encoded = true; > -/* end: legacy structures & constants for <= 2.0 machines */ > - > /* > * SMBIOS tables provided by user with '-smbios file=' option > */ > uint8_t *usr_blobs; > size_t usr_blobs_len; > -static GArray *usr_blobs_sizes; > static unsigned usr_table_max; > static unsigned usr_table_cnt; > > @@ -483,7 +457,7 @@ static void smbios_check_type4_count(uint32_t > expected_t4_count) > } > } > > -static void smbios_validate_table(void) > +void smbios_validate_table(void) > { > if (smbios_ep_type == SMBIOS_ENTRY_POINT_TYPE_32 && > smbios_tables_len > SMBIOS_21_MAX_TABLES_LEN) { > @@ -493,134 +467,6 @@ static void smbios_validate_table(void) > } > } > > - > -/* legacy setup functions for <= 2.0 machines */ > -static void smbios_add_field(int type, int offset, const void *data, size_t > len) > -{ > -struct smbios_field *field; > - > -if (!smbios_entries) { > -smbios_entries_len = sizeof(uint16_t); > -smbios_entries = g_malloc0(smbios_entries_len); >
[PATCH v2 11/20] smbios: build legacy mode code only for 'pc' machine
basically moving code around without functional change. And exposing some symbols so that they could be shared between smbbios.c and new smbios_legacy.c plus some meson magic to build smbios_legacy.c only for 'pc' machine and otherwise replace it with stub if not selected. Signed-off-by: Igor Mammedov --- v2: moved type0/type1/have_binfile_bitmap/have_fields_bitmap rename into a separate patch --- include/hw/firmware/smbios.h | 5 + hw/i386/Kconfig| 1 + hw/smbios/Kconfig | 2 + hw/smbios/meson.build | 4 + hw/smbios/smbios.c | 164 +- hw/smbios/smbios_legacy.c | 179 + hw/smbios/smbios_legacy_stub.c | 16 +++ 7 files changed, 208 insertions(+), 163 deletions(-) create mode 100644 hw/smbios/smbios_legacy.c create mode 100644 hw/smbios/smbios_legacy_stub.c diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h index bea5c3f1b1..7f47475aa4 100644 --- a/include/hw/firmware/smbios.h +++ b/include/hw/firmware/smbios.h @@ -17,6 +17,9 @@ * */ +extern uint8_t *usr_blobs; +extern GArray *usr_blobs_sizes; + typedef struct { const char *vendor, *version, *date; bool have_major_minor, uefi; @@ -306,6 +309,8 @@ struct smbios_type_127 { struct smbios_structure_header header; } QEMU_PACKED; +void smbios_validate_table(void); +void smbios_add_usr_blob_size(size_t size); void smbios_entry_add(QemuOpts *opts, Error **errp); void smbios_set_cpuid(uint32_t version, uint32_t features); void smbios_set_defaults(const char *manufacturer, const char *product, diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig index a1846be6f7..a6ee052f9a 100644 --- a/hw/i386/Kconfig +++ b/hw/i386/Kconfig @@ -76,6 +76,7 @@ config I440FX select PIIX select DIMM select SMBIOS +select SMBIOS_LEGACY select FW_CFG_DMA config ISAPC diff --git a/hw/smbios/Kconfig b/hw/smbios/Kconfig index 553adf4bfc..8d989a2f1b 100644 --- a/hw/smbios/Kconfig +++ b/hw/smbios/Kconfig @@ -1,2 +1,4 @@ config SMBIOS bool +config SMBIOS_LEGACY +bool diff --git a/hw/smbios/meson.build b/hw/smbios/meson.build index 7046967462..a59039f669 100644 --- a/hw/smbios/meson.build +++ b/hw/smbios/meson.build @@ -4,5 +4,9 @@ smbios_ss.add(when: 'CONFIG_IPMI', if_true: files('smbios_type_38.c'), if_false: files('smbios_type_38-stub.c')) +smbios_ss.add(when: 'CONFIG_SMBIOS_LEGACY', + if_true: files('smbios_legacy.c'), + if_false: files('smbios_legacy_stub.c')) + system_ss.add_all(when: 'CONFIG_SMBIOS', if_true: smbios_ss) system_ss.add(when: 'CONFIG_SMBIOS', if_false: files('smbios-stub.c')) diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c index 86cf66b5ce..fb1f05fcde 100644 --- a/hw/smbios/smbios.c +++ b/hw/smbios/smbios.c @@ -31,38 +31,12 @@ #include "hw/pci/pci_device.h" #include "smbios_build.h" -/* legacy structures and constants for <= 2.0 machines */ -struct smbios_header { -uint16_t length; -uint8_t type; -} QEMU_PACKED; - -struct smbios_field { -struct smbios_header header; -uint8_t type; -uint16_t offset; -uint8_t data[]; -} QEMU_PACKED; - -struct smbios_table { -struct smbios_header header; -uint8_t data[]; -} QEMU_PACKED; - -#define SMBIOS_FIELD_ENTRY 0 -#define SMBIOS_TABLE_ENTRY 1 - -static uint8_t *smbios_entries; -static size_t smbios_entries_len; static bool smbios_uuid_encoded = true; -/* end: legacy structures & constants for <= 2.0 machines */ - /* * SMBIOS tables provided by user with '-smbios file=' option */ uint8_t *usr_blobs; size_t usr_blobs_len; -static GArray *usr_blobs_sizes; static unsigned usr_table_max; static unsigned usr_table_cnt; @@ -483,7 +457,7 @@ static void smbios_check_type4_count(uint32_t expected_t4_count) } } -static void smbios_validate_table(void) +void smbios_validate_table(void) { if (smbios_ep_type == SMBIOS_ENTRY_POINT_TYPE_32 && smbios_tables_len > SMBIOS_21_MAX_TABLES_LEN) { @@ -493,134 +467,6 @@ static void smbios_validate_table(void) } } - -/* legacy setup functions for <= 2.0 machines */ -static void smbios_add_field(int type, int offset, const void *data, size_t len) -{ -struct smbios_field *field; - -if (!smbios_entries) { -smbios_entries_len = sizeof(uint16_t); -smbios_entries = g_malloc0(smbios_entries_len); -} -smbios_entries = g_realloc(smbios_entries, smbios_entries_len + - sizeof(*field) + len); -field = (struct smbios_field *)(smbios_entries + smbios_entries_len); -field->header.type = SMBIOS_FIELD_ENTRY; -field->header.length = cpu_to_le16(sizeof(*field) + len); - -field->type = type; -field->offset = cpu_to_le16(offset); -memcpy(field->data, data, len); - -smbios_entries_len += sizeof(*field) + len; -(*(uint16_t *)smbios_entries) = -