On 08/06/15 19:14, Wei Huang wrote:
> Current smbios builds type 19 table from e820, which is x86 specific.
> This patch removes smbios' dependency on e820 by passing an array
> of memory area to smbios_get_tables().
>
> Signed-off-by: Wei Huang <w...@redhat.com>
> ---
> hw/i386/pc.c | 18 +++++++++++++++++-
> hw/i386/smbios.c | 14 +++++++-------
> include/hw/i386/smbios.h | 10 +++++++++-
> 3 files changed, 33 insertions(+), 9 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 00e45f3..34e9133 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -722,6 +722,8 @@ static void pc_build_smbios(FWCfgState *fw_cfg)
> {
> uint8_t *smbios_tables, *smbios_anchor;
> size_t smbios_tables_len, smbios_anchor_len;
> + struct smbios_phys_mem_area *mem_array;
> + unsigned i, array_count;
>
> smbios_tables = smbios_get_table_legacy(&smbios_tables_len);
> if (smbios_tables) {
> @@ -729,8 +731,22 @@ static void pc_build_smbios(FWCfgState *fw_cfg)
> smbios_tables, smbios_tables_len);
> }
>
> - smbios_get_tables(&smbios_tables, &smbios_tables_len,
> + /* build the array of physical mem area from e820 table */
> + mem_array = g_malloc0(sizeof(*mem_array) * e820_get_num_entries());
> + for (i = 0, array_count = 0; i < e820_get_num_entries(); i++) {
> + uint64_t addr, len;
> +
> + if (e820_get_entry(i, E820_RAM, &addr, &len)) {
> + mem_array[array_count].address = addr;
> + mem_array[array_count].length = len;
> + array_count++;
> + }
> + }
> + smbios_get_tables(mem_array, array_count,
> + &smbios_tables, &smbios_tables_len,
> &smbios_anchor, &smbios_anchor_len);
> + g_free(mem_array);
> +
> if (smbios_anchor) {
> fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-tables",
> smbios_tables, smbios_tables_len);
> diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
> index 1341e02..6f715c6 100644
> --- a/hw/i386/smbios.c
> +++ b/hw/i386/smbios.c
> @@ -831,10 +831,12 @@ static void smbios_entry_point_setup(void)
> ep.structure_table_address = cpu_to_le32(0);
> }
>
> -void smbios_get_tables(uint8_t **tables, size_t *tables_len,
> +void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
> + const unsigned int mem_array_size,
> + uint8_t **tables, size_t *tables_len,
> uint8_t **anchor, size_t *anchor_len)
> {
> - unsigned i, dimm_cnt, instance;
> + unsigned i, dimm_cnt;
>
> if (smbios_legacy) {
> *tables = *anchor = NULL;
> @@ -867,11 +869,9 @@ void smbios_get_tables(uint8_t **tables, size_t
> *tables_len,
> smbios_build_type_17_table(i, GET_DIMM_SZ);
> }
>
> - for (i = 0, instance = 0; i < e820_get_num_entries(); i++) {
> - uint64_t address, length;
> - if (e820_get_entry(i, E820_RAM, &address, &length)) {
> - smbios_build_type_19_table(instance++, address, length);
> - }
> + for (i = 0; i < mem_array_size; i++) {
> + smbios_build_type_19_table(i, mem_array[i].address,
> + mem_array[i].length);
> }
>
> smbios_build_type_32_table();
> diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
> index d2850be..4269aab 100644
> --- a/include/hw/i386/smbios.h
> +++ b/include/hw/i386/smbios.h
> @@ -17,13 +17,21 @@
>
> #define SMBIOS_MAX_TYPE 127
>
> +/* memory area description, used by type 19 table */
> +struct smbios_phys_mem_area {
> + uint64_t address;
> + uint64_t length;
> +};
In general we'd also introduce a CamelCase typedefs here, but
"include/hw/i386/smbios.h" does not follow that style at all. So this
structure type definition is consistent with the existent style in that
file.
> +
> void smbios_entry_add(QemuOpts *opts);
> void smbios_set_cpuid(uint32_t version, uint32_t features);
> void smbios_set_defaults(const char *manufacturer, const char *product,
> const char *version, bool legacy_mode,
> bool uuid_encoded);
> uint8_t *smbios_get_table_legacy(size_t *length);
> -void smbios_get_tables(uint8_t **tables, size_t *tables_len,
> +void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
> + const unsigned int mem_array_size,
> + uint8_t **tables, size_t *tables_len,
> uint8_t **anchor, size_t *anchor_len);
>
> /*
>
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
