* [Qemu-devel] [PATCH 0/6] acpi: simplify bios_linker API
@ 2016-03-07 15:19 Igor Mammedov
2016-03-07 15:19 ` [Qemu-devel] [PATCH 1/6] acpi: convert linker from GArray to BOISLinker structure Igor Mammedov
` (6 more replies)
0 siblings, 7 replies; 8+ messages in thread
From: Igor Mammedov @ 2016-03-07 15:19 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, guangrong.xiao, ehabkost, mst, qemu-arm,
zhaoshenglong, pbonzini
Simplify bios_linker API by adding filei name + blob tracking
so user would have to suppily less arguments which
reduces chances of mistake by using wrong blob pointer
with not related file.
And also refactor API to take file offsets, as it's what
actually API were doing only internally, instead of
using host pointers. I hope ti makes it easier for API user
to use API without need to look at implementation on
QEMU/SeaBIOS.
Igor Mammedov (6):
acpi: convert linker from GArray to BOISLinker structure
acpi: simplify bios_linker API by removing redundant 'table' argument
acpi: cleanup bios_linker_loader_cleanup()
tpm: apci: cleanup TCPA table initialization
acpi: make bios_linker_loader_add_pointer() API offset based
acpi: make bios_linker_loader_add_checksum() API offset based
hw/acpi/aml-build.c | 33 +++----
hw/acpi/bios-linker-loader.c | 179 +++++++++++++++++++++--------------
hw/acpi/nvdimm.c | 6 +-
hw/arm/virt-acpi-build.c | 51 +++++-----
hw/i386/acpi-build.c | 89 +++++++++--------
include/hw/acpi/aml-build.h | 7 +-
include/hw/acpi/bios-linker-loader.h | 28 +++---
include/hw/mem/nvdimm.h | 3 +-
8 files changed, 222 insertions(+), 174 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 1/6] acpi: convert linker from GArray to BOISLinker structure
2016-03-07 15:19 [Qemu-devel] [PATCH 0/6] acpi: simplify bios_linker API Igor Mammedov
@ 2016-03-07 15:19 ` Igor Mammedov
2016-03-07 15:19 ` [Qemu-devel] [PATCH 2/6] acpi: simplify bios_linker API by removing redundant 'table' argument Igor Mammedov
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Igor Mammedov @ 2016-03-07 15:19 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, guangrong.xiao, ehabkost, mst, qemu-arm,
zhaoshenglong, pbonzini
Patch just changes type of of linker variables to
a structure, there aren't any functional changes.
Converting linker to a structure will allow to extend
it functionality in follow up patch adding sanity blob
checks.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/acpi/aml-build.c | 5 ++---
hw/acpi/bios-linker-loader.c | 40 +++++++++++++++++++++---------------
hw/acpi/nvdimm.c | 6 +++---
hw/arm/virt-acpi-build.c | 19 +++++++++--------
hw/i386/acpi-build.c | 29 +++++++++++++-------------
include/hw/acpi/aml-build.h | 7 ++++---
include/hw/acpi/bios-linker-loader.h | 14 ++++++++-----
include/hw/mem/nvdimm.h | 3 ++-
8 files changed, 68 insertions(+), 55 deletions(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 6675535..07af4fa 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -24,7 +24,6 @@
#include "hw/acpi/aml-build.h"
#include "qemu/bswap.h"
#include "qemu/bitops.h"
-#include "hw/acpi/bios-linker-loader.h"
static GArray *build_alloc_array(void)
{
@@ -1424,7 +1423,7 @@ Aml *aml_alias(const char *source_object, const char *alias_object)
}
void
-build_header(GArray *linker, GArray *table_data,
+build_header(BIOSLinker *linker, GArray *table_data,
AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
const char *oem_id, const char *oem_table_id)
{
@@ -1492,7 +1491,7 @@ void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre)
/* Build rsdt table */
void
-build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets,
+build_rsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
const char *oem_id, const char *oem_table_id)
{
AcpiRsdtDescriptorRev1 *rsdt;
diff --git a/hw/acpi/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c
index ace9abb..9c89a48 100644
--- a/hw/acpi/bios-linker-loader.c
+++ b/hw/acpi/bios-linker-loader.c
@@ -96,33 +96,39 @@ enum {
};
/*
- * bios_linker_loader_init: allocate a new linker file blob array.
+ * bios_linker_loader_init: allocate a new linker object instance.
*
* After initialization, linker commands can be added, and will
- * be stored in the array.
+ * be stored in the linker.cmd_blob array.
*/
-GArray *bios_linker_loader_init(void)
+BIOSLinker *bios_linker_loader_init(void)
{
- return g_array_new(false, true /* clear */, 1);
+ BIOSLinker *linker = g_new(BIOSLinker, 1);
+
+ linker->cmd_blob = g_array_new(false, true /* clear */, 1);
+ return linker;
}
-/* Free linker wrapper and return the linker array. */
-void *bios_linker_loader_cleanup(GArray *linker)
+/* Free linker wrapper and return the linker commands array. */
+void *bios_linker_loader_cleanup(BIOSLinker *linker)
{
- return g_array_free(linker, false);
+ void *cmd_blob = g_array_free(linker->cmd_blob, false);
+
+ g_free(linker);
+ return cmd_blob;
}
/*
* bios_linker_loader_alloc: ask guest to load file into guest memory.
*
- * @linker: linker file blob array
- * @file: file to be loaded
+ * @linker: linker object instance
+ * @file: name of the file blob to be loaded
* @alloc_align: required minimal alignment in bytes. Must be a power of 2.
* @alloc_fseg: request allocation in FSEG zone (useful for the RSDP ACPI table)
*
* Note: this command must precede any other linker command using this file.
*/
-void bios_linker_loader_alloc(GArray *linker,
+void bios_linker_loader_alloc(BIOSLinker *linker,
const char *file,
uint32_t alloc_align,
bool alloc_fseg)
@@ -140,7 +146,7 @@ void bios_linker_loader_alloc(GArray *linker,
BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH);
/* Alloc entries must come first, so prepend them */
- g_array_prepend_vals(linker, &entry, sizeof entry);
+ g_array_prepend_vals(linker->cmd_blob, &entry, sizeof entry);
}
/*
@@ -150,7 +156,7 @@ void bios_linker_loader_alloc(GArray *linker,
* Checksum calculation simply sums -X for each byte X in the range
* using 8-bit math (i.e. ACPI checksum).
*
- * @linker: linker file blob array
+ * @linker: linker object instance
* @file: file that includes the checksum to be calculated
* and the data to be checksummed
* @table: @file blob contents
@@ -168,7 +174,7 @@ void bios_linker_loader_alloc(GArray *linker,
* - To avoid confusion, caller must always put 0x0 at @checksum.
* - @file must be loaded into Guest memory using bios_linker_loader_alloc
*/
-void bios_linker_loader_add_checksum(GArray *linker, const char *file,
+void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file,
GArray *table,
void *start, unsigned size,
uint8_t *checksum)
@@ -190,14 +196,14 @@ void bios_linker_loader_add_checksum(GArray *linker, const char *file,
entry.cksum.start = cpu_to_le32(start_offset);
entry.cksum.length = cpu_to_le32(size);
- g_array_append_vals(linker, &entry, sizeof entry);
+ g_array_append_vals(linker->cmd_blob, &entry, sizeof entry);
}
/*
* bios_linker_loader_add_pointer: ask guest to add address of source file
* into destination file at the specified pointer.
*
- * @linker: linker file blob array
+ * @linker: linker object instance
* @dest_file: destination file that must be changed
* @src_file: source file who's address must be taken
* @table: @dest_file blob contents array
@@ -214,7 +220,7 @@ void bios_linker_loader_add_checksum(GArray *linker, const char *file,
* - Both @dest_file and @src_file must be
* loaded into Guest memory using bios_linker_loader_alloc
*/
-void bios_linker_loader_add_pointer(GArray *linker,
+void bios_linker_loader_add_pointer(BIOSLinker *linker,
const char *dest_file,
const char *src_file,
GArray *table, void *pointer,
@@ -237,5 +243,5 @@ void bios_linker_loader_add_pointer(GArray *linker,
assert(pointer_size == 1 || pointer_size == 2 ||
pointer_size == 4 || pointer_size == 8);
- g_array_append_vals(linker, &entry, sizeof entry);
+ g_array_append_vals(linker->cmd_blob, &entry, sizeof entry);
}
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 49ee68e..fe5f7d1 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -351,7 +351,7 @@ static GArray *nvdimm_build_device_structure(GSList *device_list)
}
static void nvdimm_build_nfit(GSList *device_list, GArray *table_offsets,
- GArray *table_data, GArray *linker)
+ GArray *table_data, BIOSLinker *linker)
{
GArray *structures = nvdimm_build_device_structure(device_list);
unsigned int header;
@@ -433,7 +433,7 @@ static void nvdimm_build_nvdimm_devices(GSList *device_list, Aml *root_dev)
}
static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
- GArray *table_data, GArray *linker)
+ GArray *table_data, BIOSLinker *linker)
{
Aml *ssdt, *sb_scope, *dev;
@@ -476,7 +476,7 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
}
void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
- GArray *linker)
+ BIOSLinker *linker)
{
GSList *device_list;
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index b8b3ece..e6ea6db 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -337,7 +337,7 @@ static void acpi_dsdt_add_power_button(Aml *scope)
/* RSDP */
static GArray *
-build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
+build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt)
{
AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
@@ -366,7 +366,7 @@ build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
}
static void
-build_spcr(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
+build_spcr(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
{
AcpiSerialPortConsoleRedirection *spcr;
const MemMapEntry *uart_memmap = &guest_info->memmap[VIRT_UART];
@@ -399,7 +399,7 @@ build_spcr(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
}
static void
-build_mcfg(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
+build_mcfg(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
{
AcpiTableMcfg *mcfg;
const MemMapEntry *memmap = guest_info->memmap;
@@ -419,7 +419,7 @@ build_mcfg(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
/* GTDT */
static void
-build_gtdt(GArray *table_data, GArray *linker)
+build_gtdt(GArray *table_data, BIOSLinker *linker)
{
int gtdt_start = table_data->len;
AcpiGenericTimerTable *gtdt;
@@ -445,7 +445,7 @@ build_gtdt(GArray *table_data, GArray *linker)
/* MADT */
static void
-build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
+build_madt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
{
int madt_start = table_data->len;
const MemMapEntry *memmap = guest_info->memmap;
@@ -504,7 +504,7 @@ build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
/* FADT */
static void
-build_fadt(GArray *table_data, GArray *linker, unsigned dsdt)
+build_fadt(GArray *table_data, BIOSLinker *linker, unsigned dsdt)
{
AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
@@ -529,7 +529,7 @@ build_fadt(GArray *table_data, GArray *linker, unsigned dsdt)
/* DSDT */
static void
-build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
+build_dsdt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
{
Aml *scope, *dsdt;
const MemMapEntry *memmap = guest_info->memmap;
@@ -662,7 +662,7 @@ static void virt_acpi_build_update(void *build_opaque)
acpi_ram_update(build_state->table_mr, tables.table_data);
acpi_ram_update(build_state->rsdp_mr, tables.rsdp);
- acpi_ram_update(build_state->linker_mr, tables.linker);
+ acpi_ram_update(build_state->linker_mr, tables.linker->cmd_blob);
acpi_build_tables_cleanup(&tables, true);
@@ -720,7 +720,8 @@ void virt_acpi_setup(VirtGuestInfo *guest_info)
assert(build_state->table_mr != NULL);
build_state->linker_mr =
- acpi_add_rom_blob(build_state, tables.linker, "etc/table-loader", 0);
+ acpi_add_rom_blob(build_state, tables.linker->cmd_blob,
+ "etc/table-loader", 0);
fw_cfg_add_file(guest_info->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
tables.tcpalog->data, acpi_data_len(tables.tcpalog));
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 52c9470..5db65a2 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -290,7 +290,7 @@ static void acpi_align_size(GArray *blob, unsigned align)
/* FACS */
static void
-build_facs(GArray *table_data, GArray *linker)
+build_facs(GArray *table_data, BIOSLinker *linker)
{
AcpiFacsDescriptorRev1 *facs = acpi_data_push(table_data, sizeof *facs);
memcpy(&facs->signature, "FACS", 4);
@@ -335,7 +335,7 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)
/* FADT */
static void
-build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm,
+build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
unsigned facs, unsigned dsdt,
const char *oem_id, const char *oem_table_id)
{
@@ -362,7 +362,7 @@ build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm,
}
static void
-build_madt(GArray *table_data, GArray *linker, AcpiCpuInfo *cpu)
+build_madt(GArray *table_data, BIOSLinker *linker, AcpiCpuInfo *cpu)
{
PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
int madt_start = table_data->len;
@@ -1936,7 +1936,7 @@ static Aml *build_q35_osc_method(void)
}
static void
-build_dsdt(GArray *table_data, GArray *linker,
+build_dsdt(GArray *table_data, BIOSLinker *linker,
AcpiCpuInfo *cpu, AcpiPmInfo *pm, AcpiMiscInfo *misc,
PcPciInfo *pci)
{
@@ -2292,7 +2292,7 @@ build_dsdt(GArray *table_data, GArray *linker,
}
static void
-build_hpet(GArray *table_data, GArray *linker)
+build_hpet(GArray *table_data, BIOSLinker *linker)
{
Acpi20Hpet *hpet;
@@ -2307,7 +2307,7 @@ build_hpet(GArray *table_data, GArray *linker)
}
static void
-build_tpm_tcpa(GArray *table_data, GArray *linker, GArray *tcpalog)
+build_tpm_tcpa(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)
{
Acpi20Tcpa *tcpa = acpi_data_push(table_data, sizeof *tcpa);
uint64_t log_area_start_address = acpi_data_len(tcpalog);
@@ -2332,7 +2332,7 @@ build_tpm_tcpa(GArray *table_data, GArray *linker, GArray *tcpalog)
}
static void
-build_tpm2(GArray *table_data, GArray *linker)
+build_tpm2(GArray *table_data, BIOSLinker *linker)
{
Acpi20TPM2 *tpm2_ptr;
@@ -2367,7 +2367,7 @@ acpi_build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
}
static void
-build_srat(GArray *table_data, GArray *linker)
+build_srat(GArray *table_data, BIOSLinker *linker)
{
AcpiSystemResourceAffinityTable *srat;
AcpiSratProcessorAffinity *core;
@@ -2461,7 +2461,7 @@ build_srat(GArray *table_data, GArray *linker)
}
static void
-build_mcfg_q35(GArray *table_data, GArray *linker, AcpiMcfgInfo *info)
+build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
{
AcpiTableMcfg *mcfg;
const char *sig;
@@ -2490,7 +2490,7 @@ build_mcfg_q35(GArray *table_data, GArray *linker, AcpiMcfgInfo *info)
}
static void
-build_dmar_q35(GArray *table_data, GArray *linker)
+build_dmar_q35(GArray *table_data, BIOSLinker *linker)
{
int dmar_start = table_data->len;
@@ -2514,7 +2514,7 @@ build_dmar_q35(GArray *table_data, GArray *linker)
}
static GArray *
-build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
+build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt)
{
AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
@@ -2736,7 +2736,7 @@ void acpi_build(AcpiBuildTables *tables)
acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
}
- acpi_align_size(tables->linker, ACPI_BUILD_ALIGN_SIZE);
+ acpi_align_size(tables->linker->cmd_blob, ACPI_BUILD_ALIGN_SIZE);
/* Cleanup memory that's no longer used. */
g_array_free(table_offsets, true);
@@ -2776,7 +2776,7 @@ static void acpi_build_update(void *build_opaque)
acpi_ram_update(build_state->rsdp_mr, tables.rsdp);
}
- acpi_ram_update(build_state->linker_mr, tables.linker);
+ acpi_ram_update(build_state->linker_mr, tables.linker->cmd_blob);
acpi_build_tables_cleanup(&tables, true);
}
@@ -2840,7 +2840,8 @@ void acpi_setup(void)
assert(build_state->table_mr != NULL);
build_state->linker_mr =
- acpi_add_rom_blob(build_state, tables.linker, "etc/table-loader", 0);
+ acpi_add_rom_blob(build_state, tables.linker->cmd_blob,
+ "etc/table-loader", 0);
fw_cfg_add_file(pcms->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
tables.tcpalog->data, acpi_data_len(tables.tcpalog));
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index d3e0c8f..2f3eeaf 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -3,6 +3,7 @@
#include <glib.h>
#include "hw/acpi/acpi-defs.h"
+#include "hw/acpi/bios-linker-loader.h"
/* Reserve RAM space for tables: add another order of magnitude. */
#define ACPI_BUILD_TABLE_MAX_SIZE 0x200000
@@ -203,7 +204,7 @@ struct AcpiBuildTables {
GArray *table_data;
GArray *rsdp;
GArray *tcpalog;
- GArray *linker;
+ BIOSLinker *linker;
} AcpiBuildTables;
/**
@@ -353,7 +354,7 @@ Aml *aml_derefof(Aml *arg);
Aml *aml_sizeof(Aml *arg);
void
-build_header(GArray *linker, GArray *table_data,
+build_header(BIOSLinker *linker, GArray *table_data,
AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
const char *oem_id, const char *oem_table_id);
void *acpi_data_push(GArray *table_data, unsigned size);
@@ -362,7 +363,7 @@ void acpi_add_table(GArray *table_offsets, GArray *table_data);
void acpi_build_tables_init(AcpiBuildTables *tables);
void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre);
void
-build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets,
+build_rsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
const char *oem_id, const char *oem_table_id);
#endif
diff --git a/include/hw/acpi/bios-linker-loader.h b/include/hw/acpi/bios-linker-loader.h
index 82f1af6..4145c56 100644
--- a/include/hw/acpi/bios-linker-loader.h
+++ b/include/hw/acpi/bios-linker-loader.h
@@ -3,23 +3,27 @@
#include <glib.h>
-GArray *bios_linker_loader_init(void);
+typedef struct BIOSLinker {
+ GArray *cmd_blob;
+} BIOSLinker;
-void bios_linker_loader_alloc(GArray *linker,
+BIOSLinker *bios_linker_loader_init(void);
+
+void bios_linker_loader_alloc(BIOSLinker *linker,
const char *file,
uint32_t alloc_align,
bool alloc_fseg);
-void bios_linker_loader_add_checksum(GArray *linker, const char *file,
+void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file,
GArray *table,
void *start, unsigned size,
uint8_t *checksum);
-void bios_linker_loader_add_pointer(GArray *linker,
+void bios_linker_loader_add_pointer(BIOSLinker *linker,
const char *dest_file,
const char *src_file,
GArray *table, void *pointer,
uint8_t pointer_size);
-void *bios_linker_loader_cleanup(GArray *linker);
+void *bios_linker_loader_cleanup(BIOSLinker *linker);
#endif
diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
index 49183c1..bb9359d 100644
--- a/include/hw/mem/nvdimm.h
+++ b/include/hw/mem/nvdimm.h
@@ -24,9 +24,10 @@
#define QEMU_NVDIMM_H
#include "hw/mem/pc-dimm.h"
+#include "hw/acpi/bios-linker-loader.h"
#define TYPE_NVDIMM "nvdimm"
void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
- GArray *linker);
+ BIOSLinker *linker);
#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 2/6] acpi: simplify bios_linker API by removing redundant 'table' argument
2016-03-07 15:19 [Qemu-devel] [PATCH 0/6] acpi: simplify bios_linker API Igor Mammedov
2016-03-07 15:19 ` [Qemu-devel] [PATCH 1/6] acpi: convert linker from GArray to BOISLinker structure Igor Mammedov
@ 2016-03-07 15:19 ` Igor Mammedov
2016-03-07 15:19 ` [Qemu-devel] [PATCH 3/6] acpi: cleanup bios_linker_loader_cleanup() Igor Mammedov
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Igor Mammedov @ 2016-03-07 15:19 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, guangrong.xiao, ehabkost, mst, qemu-arm,
zhaoshenglong, pbonzini
'table' argument in bios_linker_add_foo() commands is
a data blob of one of files also passed to the same API.
So instead of passing blob in every API call, add and keep
file name association with related blob at bios_linker_loader_alloc()
time.
And find blob by name looking up allocated file entries
inside of bios_linker_add_foo() commands.
It will:
- make API less confusing,
- enforce calling bios_linker_loader_alloc() before
calling any bios_linker_add_foo()
- make sure that blob is the correct one, i.e.
associated with the right file name
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/acpi/aml-build.c | 4 +-
hw/acpi/bios-linker-loader.c | 82 +++++++++++++++++++++++++++---------
hw/arm/virt-acpi-build.c | 11 ++---
hw/i386/acpi-build.c | 17 ++++----
include/hw/acpi/bios-linker-loader.h | 7 +--
5 files changed, 82 insertions(+), 39 deletions(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 07af4fa..aad4b8a 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1450,7 +1450,7 @@ build_header(BIOSLinker *linker, GArray *table_data,
h->checksum = 0;
/* Checksum to be filled in by Guest linker */
bios_linker_loader_add_checksum(linker, ACPI_BUILD_TABLE_FILE,
- table_data, h, len, &h->checksum);
+ h, len, &h->checksum);
}
void *acpi_data_push(GArray *table_data, unsigned size)
@@ -1507,7 +1507,7 @@ build_rsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
bios_linker_loader_add_pointer(linker,
ACPI_BUILD_TABLE_FILE,
ACPI_BUILD_TABLE_FILE,
- table_data, &rsdt->table_offset_entry[i],
+ &rsdt->table_offset_entry[i],
sizeof(uint32_t));
}
build_header(linker, table_data,
diff --git a/hw/acpi/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c
index 9c89a48..873e61c 100644
--- a/hw/acpi/bios-linker-loader.c
+++ b/hw/acpi/bios-linker-loader.c
@@ -96,6 +96,22 @@ enum {
};
/*
+ * BiosLinkerFileEntry:
+ *
+ * An internal type used for book-keeping file entries
+ */
+typedef struct BiosLinkerFileEntry {
+ char *name; /* file name */
+ GArray *blob; /* data accosiated with @name */
+} BiosLinkerFileEntry;
+
+static void bios_linker_free_file_entry(gpointer data)
+{
+ BiosLinkerFileEntry *entry = data;
+ g_free(entry->name);
+}
+
+/*
* bios_linker_loader_init: allocate a new linker object instance.
*
* After initialization, linker commands can be added, and will
@@ -106,6 +122,9 @@ BIOSLinker *bios_linker_loader_init(void)
BIOSLinker *linker = g_new(BIOSLinker, 1);
linker->cmd_blob = g_array_new(false, true /* clear */, 1);
+ linker->file_list = g_array_new(false, true /* clear */,
+ sizeof(BiosLinkerFileEntry));
+ g_array_set_clear_func(linker->file_list, bios_linker_free_file_entry);
return linker;
}
@@ -114,31 +133,53 @@ void *bios_linker_loader_cleanup(BIOSLinker *linker)
{
void *cmd_blob = g_array_free(linker->cmd_blob, false);
+ g_array_free(linker->file_list, true);
g_free(linker);
return cmd_blob;
}
+static const BiosLinkerFileEntry *
+bios_linker_find_file(const BIOSLinker *linker, const char *name)
+{
+ int i;
+ BiosLinkerFileEntry *entry;
+
+ for (i = 0; i < linker->file_list->len; i++) {
+ entry = &g_array_index(linker->file_list, BiosLinkerFileEntry, i);
+ if (!strcmp(entry->name, name)) {
+ return entry;
+ }
+ }
+ return NULL;
+}
+
/*
* bios_linker_loader_alloc: ask guest to load file into guest memory.
*
* @linker: linker object instance
- * @file: name of the file blob to be loaded
+ * @file_name: name of the file blob to be loaded
+ * @file_blob: pointer to blob corresponding to @file_name
* @alloc_align: required minimal alignment in bytes. Must be a power of 2.
* @alloc_fseg: request allocation in FSEG zone (useful for the RSDP ACPI table)
*
* Note: this command must precede any other linker command using this file.
*/
void bios_linker_loader_alloc(BIOSLinker *linker,
- const char *file,
+ const char *file_name,
+ GArray *file_blob,
uint32_t alloc_align,
bool alloc_fseg)
{
BiosLinkerLoaderEntry entry;
+ BiosLinkerFileEntry file = { g_strdup(file_name), file_blob};
assert(!(alloc_align & (alloc_align - 1)));
+ assert(!bios_linker_find_file(linker, file_name));
+ g_array_append_val(linker->file_list, file);
+
memset(&entry, 0, sizeof entry);
- strncpy(entry.alloc.file, file, sizeof entry.alloc.file - 1);
+ strncpy(entry.alloc.file, file_name, sizeof entry.alloc.file - 1);
entry.command = cpu_to_le32(BIOS_LINKER_LOADER_COMMAND_ALLOCATE);
entry.alloc.align = cpu_to_le32(alloc_align);
entry.alloc.zone = cpu_to_le32(alloc_fseg ?
@@ -159,38 +200,37 @@ void bios_linker_loader_alloc(BIOSLinker *linker,
* @linker: linker object instance
* @file: file that includes the checksum to be calculated
* and the data to be checksummed
- * @table: @file blob contents
* @start, @size: range of data to checksum
* @checksum: location of the checksum to be patched within file blob
*
* Notes:
- * - checksum byte initial value must have been pushed into @table
- * and reside at address @checksum.
- * - @size bytes must have been pushed into @table and reside at address
- * @start.
+ * - checksum byte initial value must have been pushed into blob
+ * associated with @file and reside at address @checksum.
+ * - @size bytes must have been pushed into blob associated wtih @file
+ * and reside at address @start.
* - Guest calculates checksum of specified range of data, result is added to
* initial value at @checksum into copy of @file in Guest memory.
* - Range might include the checksum itself.
* - To avoid confusion, caller must always put 0x0 at @checksum.
* - @file must be loaded into Guest memory using bios_linker_loader_alloc
*/
-void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file,
- GArray *table,
+void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file_name,
void *start, unsigned size,
uint8_t *checksum)
{
BiosLinkerLoaderEntry entry;
- ptrdiff_t checksum_offset = (gchar *)checksum - table->data;
- ptrdiff_t start_offset = (gchar *)start - table->data;
+ const BiosLinkerFileEntry *file = bios_linker_find_file(linker, file_name);
+ ptrdiff_t checksum_offset = (gchar *)checksum - file->blob->data;
+ ptrdiff_t start_offset = (gchar *)start - file->blob->data;
assert(checksum_offset >= 0);
assert(start_offset >= 0);
- assert(checksum_offset + 1 <= table->len);
- assert(start_offset + size <= table->len);
+ assert(checksum_offset + 1 <= file->blob->len);
+ assert(start_offset + size <= file->blob->len);
assert(*checksum == 0x0);
memset(&entry, 0, sizeof entry);
- strncpy(entry.cksum.file, file, sizeof entry.cksum.file - 1);
+ strncpy(entry.cksum.file, file_name, sizeof entry.cksum.file - 1);
entry.command = cpu_to_le32(BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM);
entry.cksum.offset = cpu_to_le32(checksum_offset);
entry.cksum.start = cpu_to_le32(start_offset);
@@ -206,13 +246,12 @@ void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file,
* @linker: linker object instance
* @dest_file: destination file that must be changed
* @src_file: source file who's address must be taken
- * @table: @dest_file blob contents array
* @pointer: location of the pointer to be patched within destination file blob
* @pointer_size: size of pointer to be patched, in bytes
*
* Notes:
- * - @pointer_size bytes must have been pushed into @table
- * and reside at address @pointer.
+ * - @pointer_size bytes must have been pushed into blob associated with
+ * @dest_file and reside at address @pointer.
* - Guest address is added to initial value at @pointer
* into copy of @dest_file in Guest memory.
* e.g. to get start of src_file in guest memory, put 0x0 there
@@ -223,14 +262,15 @@ void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file,
void bios_linker_loader_add_pointer(BIOSLinker *linker,
const char *dest_file,
const char *src_file,
- GArray *table, void *pointer,
+ void *pointer,
uint8_t pointer_size)
{
BiosLinkerLoaderEntry entry;
- ptrdiff_t offset = (gchar *)pointer - table->data;
+ const BiosLinkerFileEntry *file = bios_linker_find_file(linker, dest_file);
+ ptrdiff_t offset = (gchar *)pointer - file->blob->data;
assert(offset >= 0);
- assert(offset + pointer_size <= table->len);
+ assert(offset + pointer_size <= file->blob->len);
memset(&entry, 0, sizeof entry);
strncpy(entry.pointer.dest_file, dest_file,
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index e6ea6db..b0ac70d 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -341,7 +341,7 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt)
{
AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
- bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, 16,
+ bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, rsdp_table, 16,
true /* fseg memory */);
memcpy(&rsdp->signature, "RSD PTR ", sizeof(rsdp->signature));
@@ -354,12 +354,12 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt)
/* Address to be filled by Guest linker */
bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE,
ACPI_BUILD_TABLE_FILE,
- rsdp_table, &rsdp->rsdt_physical_address,
+ &rsdp->rsdt_physical_address,
sizeof rsdp->rsdt_physical_address);
rsdp->checksum = 0;
/* Checksum to be filled by Guest linker */
bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
- rsdp_table, rsdp, sizeof *rsdp,
+ rsdp, sizeof *rsdp,
&rsdp->checksum);
return rsdp_table;
@@ -520,7 +520,7 @@ build_fadt(GArray *table_data, BIOSLinker *linker, unsigned dsdt)
/* DSDT address to be filled by Guest linker */
bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
ACPI_BUILD_TABLE_FILE,
- table_data, &fadt->dsdt,
+ &fadt->dsdt,
sizeof fadt->dsdt);
build_header(linker, table_data,
@@ -588,7 +588,8 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
table_offsets = g_array_new(false, true /* clear */,
sizeof(uint32_t));
- bios_linker_loader_alloc(tables->linker, ACPI_BUILD_TABLE_FILE,
+ bios_linker_loader_alloc(tables->linker,
+ ACPI_BUILD_TABLE_FILE, tables_blob,
64, false /* high memory */);
/*
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 5db65a2..e13f96d 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -345,14 +345,14 @@ build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
/* FACS address to be filled by Guest linker */
bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
ACPI_BUILD_TABLE_FILE,
- table_data, &fadt->firmware_ctrl,
+ &fadt->firmware_ctrl,
sizeof fadt->firmware_ctrl);
fadt->dsdt = cpu_to_le32(dsdt);
/* DSDT address to be filled by Guest linker */
bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
ACPI_BUILD_TABLE_FILE,
- table_data, &fadt->dsdt,
+ &fadt->dsdt,
sizeof fadt->dsdt);
fadt_setup(fadt, pm);
@@ -2316,13 +2316,13 @@ build_tpm_tcpa(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)
tcpa->log_area_minimum_length = cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE);
tcpa->log_area_start_address = cpu_to_le64(log_area_start_address);
- bios_linker_loader_alloc(linker, ACPI_BUILD_TPMLOG_FILE, 1,
+ bios_linker_loader_alloc(linker, ACPI_BUILD_TPMLOG_FILE, tcpalog, 1,
false /* high memory */);
/* log area start address to be filled by Guest linker */
bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
ACPI_BUILD_TPMLOG_FILE,
- table_data, &tcpa->log_area_start_address,
+ &tcpa->log_area_start_address,
sizeof(tcpa->log_area_start_address));
build_header(linker, table_data,
@@ -2518,7 +2518,7 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt)
{
AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
- bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, 16,
+ bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, rsdp_table, 16,
true /* fseg memory */);
memcpy(&rsdp->signature, "RSD PTR ", 8);
@@ -2527,12 +2527,12 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt)
/* Address to be filled by Guest linker */
bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE,
ACPI_BUILD_TABLE_FILE,
- rsdp_table, &rsdp->rsdt_physical_address,
+ &rsdp->rsdt_physical_address,
sizeof rsdp->rsdt_physical_address);
rsdp->checksum = 0;
/* Checksum to be filled by Guest linker */
bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
- rsdp_table, rsdp, sizeof *rsdp,
+ rsdp, sizeof *rsdp,
&rsdp->checksum);
return rsdp_table;
@@ -2615,7 +2615,8 @@ void acpi_build(AcpiBuildTables *tables)
sizeof(uint32_t));
ACPI_BUILD_DPRINTF("init ACPI tables\n");
- bios_linker_loader_alloc(tables->linker, ACPI_BUILD_TABLE_FILE,
+ bios_linker_loader_alloc(tables->linker,
+ ACPI_BUILD_TABLE_FILE, tables_blob,
64 /* Ensure FACS is aligned */,
false /* high memory */);
diff --git a/include/hw/acpi/bios-linker-loader.h b/include/hw/acpi/bios-linker-loader.h
index 4145c56..bee6dee 100644
--- a/include/hw/acpi/bios-linker-loader.h
+++ b/include/hw/acpi/bios-linker-loader.h
@@ -5,24 +5,25 @@
typedef struct BIOSLinker {
GArray *cmd_blob;
+ GArray *file_list;
} BIOSLinker;
BIOSLinker *bios_linker_loader_init(void);
void bios_linker_loader_alloc(BIOSLinker *linker,
- const char *file,
+ const char *file_name,
+ GArray *file_blob,
uint32_t alloc_align,
bool alloc_fseg);
void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file,
- GArray *table,
void *start, unsigned size,
uint8_t *checksum);
void bios_linker_loader_add_pointer(BIOSLinker *linker,
const char *dest_file,
const char *src_file,
- GArray *table, void *pointer,
+ void *pointer,
uint8_t pointer_size);
void *bios_linker_loader_cleanup(BIOSLinker *linker);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 3/6] acpi: cleanup bios_linker_loader_cleanup()
2016-03-07 15:19 [Qemu-devel] [PATCH 0/6] acpi: simplify bios_linker API Igor Mammedov
2016-03-07 15:19 ` [Qemu-devel] [PATCH 1/6] acpi: convert linker from GArray to BOISLinker structure Igor Mammedov
2016-03-07 15:19 ` [Qemu-devel] [PATCH 2/6] acpi: simplify bios_linker API by removing redundant 'table' argument Igor Mammedov
@ 2016-03-07 15:19 ` Igor Mammedov
2016-03-07 15:19 ` [Qemu-devel] [PATCH 4/6] tpm: apci: cleanup TCPA table initialization Igor Mammedov
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Igor Mammedov @ 2016-03-07 15:19 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, guangrong.xiao, ehabkost, mst, qemu-arm,
zhaoshenglong, pbonzini
bios_linker_loader_cleanup() is called only from one place
and returned value is immediately freed wich makes returning
pointer from bios_linker_loader_cleanup() useless.
Cleanup bios_linker_loader_cleanup() by freeing
data there so that caller won't have to free it.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/acpi/aml-build.c | 3 +--
hw/acpi/bios-linker-loader.c | 6 ++----
include/hw/acpi/bios-linker-loader.h | 2 +-
3 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index aad4b8a..f72b5ef 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1482,8 +1482,7 @@ void acpi_build_tables_init(AcpiBuildTables *tables)
void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre)
{
- void *linker_data = bios_linker_loader_cleanup(tables->linker);
- g_free(linker_data);
+ bios_linker_loader_cleanup(tables->linker);
g_array_free(tables->rsdp, true);
g_array_free(tables->table_data, true);
g_array_free(tables->tcpalog, mfre);
diff --git a/hw/acpi/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c
index 873e61c..a2aeb66 100644
--- a/hw/acpi/bios-linker-loader.c
+++ b/hw/acpi/bios-linker-loader.c
@@ -129,13 +129,11 @@ BIOSLinker *bios_linker_loader_init(void)
}
/* Free linker wrapper and return the linker commands array. */
-void *bios_linker_loader_cleanup(BIOSLinker *linker)
+void bios_linker_loader_cleanup(BIOSLinker *linker)
{
- void *cmd_blob = g_array_free(linker->cmd_blob, false);
-
+ g_array_free(linker->cmd_blob, true);
g_array_free(linker->file_list, true);
g_free(linker);
- return cmd_blob;
}
static const BiosLinkerFileEntry *
diff --git a/include/hw/acpi/bios-linker-loader.h b/include/hw/acpi/bios-linker-loader.h
index bee6dee..564e192 100644
--- a/include/hw/acpi/bios-linker-loader.h
+++ b/include/hw/acpi/bios-linker-loader.h
@@ -26,5 +26,5 @@ void bios_linker_loader_add_pointer(BIOSLinker *linker,
void *pointer,
uint8_t pointer_size);
-void *bios_linker_loader_cleanup(BIOSLinker *linker);
+void bios_linker_loader_cleanup(BIOSLinker *linker);
#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 4/6] tpm: apci: cleanup TCPA table initialization
2016-03-07 15:19 [Qemu-devel] [PATCH 0/6] acpi: simplify bios_linker API Igor Mammedov
` (2 preceding siblings ...)
2016-03-07 15:19 ` [Qemu-devel] [PATCH 3/6] acpi: cleanup bios_linker_loader_cleanup() Igor Mammedov
@ 2016-03-07 15:19 ` Igor Mammedov
2016-03-07 15:19 ` [Qemu-devel] [PATCH 5/6] acpi: make bios_linker_loader_add_pointer() API offset based Igor Mammedov
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Igor Mammedov @ 2016-03-07 15:19 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, guangrong.xiao, ehabkost, mst, qemu-arm,
zhaoshenglong, pbonzini
At the time build_tpm_tcpa() is called the tcpalog size is
always 0, so log_area_start_address which is actually offset
from the start of ACPI_BUILD_TPMLOG_FILE is always 0.
More over if tcpalog would be not 0 sized at build_tpm_tcpa()
calling time it would make tcpa->log_area_start_address
point to ACPI_BUILD_TPMLOG_FILE+log_area_start_address causing
guest to write beyound ACPI_BUILD_TPMLOG_FILE which
would resulted to memory corruption.
As 'TCPA' is allocated 0 filled, there is not point
to calculate constant 0 log_area_start_address and set
tcpa->log_area_start_address to it since the field should
always point to start of ACPI_BUILD_TPMLOG_FILE.
Make code easier to read dropping misleading at best offset
calculations and making it impossible to silently backfire
if tcpalog size becomes non 0 at the time build_tpm_tcpa()
is called in future.
While at that move tcpalog allocation closer to the code
that defines its size.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/i386/acpi-build.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index e13f96d..3540815 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2310,11 +2310,10 @@ static void
build_tpm_tcpa(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)
{
Acpi20Tcpa *tcpa = acpi_data_push(table_data, sizeof *tcpa);
- uint64_t log_area_start_address = acpi_data_len(tcpalog);
tcpa->platform_class = cpu_to_le16(TPM_TCPA_ACPI_CLASS_CLIENT);
tcpa->log_area_minimum_length = cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE);
- tcpa->log_area_start_address = cpu_to_le64(log_area_start_address);
+ acpi_data_push(tcpalog, tcpa->log_area_minimum_length);
bios_linker_loader_alloc(linker, ACPI_BUILD_TPMLOG_FILE, tcpalog, 1,
false /* high memory */);
@@ -2327,8 +2326,6 @@ build_tpm_tcpa(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)
build_header(linker, table_data,
(void *)tcpa, "TCPA", sizeof(*tcpa), 2, NULL, NULL);
-
- acpi_data_push(tcpalog, TPM_LOG_AREA_MINIMUM_SIZE);
}
static void
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 5/6] acpi: make bios_linker_loader_add_pointer() API offset based
2016-03-07 15:19 [Qemu-devel] [PATCH 0/6] acpi: simplify bios_linker API Igor Mammedov
` (3 preceding siblings ...)
2016-03-07 15:19 ` [Qemu-devel] [PATCH 4/6] tpm: apci: cleanup TCPA table initialization Igor Mammedov
@ 2016-03-07 15:19 ` Igor Mammedov
2016-03-07 15:19 ` [Qemu-devel] [PATCH 6/6] acpi: make bios_linker_loader_add_checksum() " Igor Mammedov
2016-03-07 15:28 ` [Qemu-devel] [PATCH 0/6] acpi: simplify bios_linker API Michael S. Tsirkin
6 siblings, 0 replies; 8+ messages in thread
From: Igor Mammedov @ 2016-03-07 15:19 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, guangrong.xiao, ehabkost, mst, qemu-arm,
zhaoshenglong, pbonzini
cleanup bios_linker_loader_add_pointer() API by switching
arguments to taking offsets relative to corresponding files
instead of doing pointer arithmetic on behalf of user which
were confusing.
Also make offset inside of source file explicit in API
so that user won't have to manually set it in
destination file blob and while at it add additional
boundary checks.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/acpi/aml-build.c | 20 ++++++++------
hw/acpi/bios-linker-loader.c | 53 ++++++++++++++++++++----------------
hw/arm/virt-acpi-build.c | 26 +++++++++---------
hw/i386/acpi-build.c | 45 +++++++++++++++---------------
include/hw/acpi/bios-linker-loader.h | 5 ++--
5 files changed, 79 insertions(+), 70 deletions(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index f72b5ef..8fc1ed1 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1493,21 +1493,23 @@ void
build_rsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
const char *oem_id, const char *oem_table_id)
{
- AcpiRsdtDescriptorRev1 *rsdt;
- size_t rsdt_len;
int i;
- const int table_data_len = (sizeof(uint32_t) * table_offsets->len);
+ unsigned rsdt_entries_offset;
+ AcpiRsdtDescriptorRev1 *rsdt;
+ const unsigned table_data_len = (sizeof(uint32_t) * table_offsets->len);
+ const unsigned rsdt_entry_size = sizeof(rsdt->table_offset_entry[0]);
+ const size_t rsdt_len = sizeof(*rsdt) + table_data_len;
- rsdt_len = sizeof(*rsdt) + table_data_len;
rsdt = acpi_data_push(table_data, rsdt_len);
- memcpy(rsdt->table_offset_entry, table_offsets->data, table_data_len);
+ rsdt_entries_offset = (char *)rsdt->table_offset_entry - table_data->data;
for (i = 0; i < table_offsets->len; ++i) {
+ uint32_t ref_tbl_offset = g_array_index(table_offsets, uint32_t, i);
+ uint32_t rsdt_entry_offset = rsdt_entries_offset + rsdt_entry_size * i;
+
/* rsdt->table_offset_entry to be filled by Guest linker */
bios_linker_loader_add_pointer(linker,
- ACPI_BUILD_TABLE_FILE,
- ACPI_BUILD_TABLE_FILE,
- &rsdt->table_offset_entry[i],
- sizeof(uint32_t));
+ ACPI_BUILD_TABLE_FILE, rsdt_entry_offset, rsdt_entry_size,
+ ACPI_BUILD_TABLE_FILE, ref_tbl_offset);
}
build_header(linker, table_data,
(void *)rsdt, "RSDT", rsdt_len, 1, oem_id, oem_table_id);
diff --git a/hw/acpi/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c
index a2aeb66..d567362 100644
--- a/hw/acpi/bios-linker-loader.c
+++ b/hw/acpi/bios-linker-loader.c
@@ -238,37 +238,38 @@ void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file_name,
}
/*
- * bios_linker_loader_add_pointer: ask guest to add address of source file
- * into destination file at the specified pointer.
+ * bios_linker_loader_add_pointer: ask guest to patch address in
+ * destination file with a pointer to source file
*
* @linker: linker object instance
* @dest_file: destination file that must be changed
+ * @dst_patched_offset: location within destination file blob to be patched
+ * with the pointer to @src_file+@src_offset (i.e. source
+ * blob allocated in guest memory + @src_offset), in bytes
+ * @dst_patched_offset_size: size of the pointer to be patched
+ * at @dst_patched_offset in @dest_file blob, in bytes
* @src_file: source file who's address must be taken
- * @pointer: location of the pointer to be patched within destination file blob
- * @pointer_size: size of pointer to be patched, in bytes
- *
- * Notes:
- * - @pointer_size bytes must have been pushed into blob associated with
- * @dest_file and reside at address @pointer.
- * - Guest address is added to initial value at @pointer
- * into copy of @dest_file in Guest memory.
- * e.g. to get start of src_file in guest memory, put 0x0 there
- * to get address of a field at offset 0x10 in src_file, put 0x10 there
- * - Both @dest_file and @src_file must be
- * loaded into Guest memory using bios_linker_loader_alloc
+ * @src_offset: location within source file blob to which
+ * @dest_file+@dst_patched_offset will point to after
+ * firmware's executed ADD_POINTER command
*/
void bios_linker_loader_add_pointer(BIOSLinker *linker,
const char *dest_file,
+ uint32_t dst_patched_offset,
+ uint8_t dst_patched_size,
const char *src_file,
- void *pointer,
- uint8_t pointer_size)
+ uint32_t src_offset)
{
+ uint64_t le_src_offset;
BiosLinkerLoaderEntry entry;
- const BiosLinkerFileEntry *file = bios_linker_find_file(linker, dest_file);
- ptrdiff_t offset = (gchar *)pointer - file->blob->data;
+ const BiosLinkerFileEntry *dst_file =
+ bios_linker_find_file(linker, dest_file);
+ const BiosLinkerFileEntry *source_file =
+ bios_linker_find_file(linker, src_file);
- assert(offset >= 0);
- assert(offset + pointer_size <= file->blob->len);
+ assert(dst_patched_offset < dst_file->blob->len);
+ assert(dst_patched_offset + dst_patched_size <= dst_file->blob->len);
+ assert(src_offset < source_file->blob->len);
memset(&entry, 0, sizeof entry);
strncpy(entry.pointer.dest_file, dest_file,
@@ -276,10 +277,14 @@ void bios_linker_loader_add_pointer(BIOSLinker *linker,
strncpy(entry.pointer.src_file, src_file,
sizeof entry.pointer.src_file - 1);
entry.command = cpu_to_le32(BIOS_LINKER_LOADER_COMMAND_ADD_POINTER);
- entry.pointer.offset = cpu_to_le32(offset);
- entry.pointer.size = pointer_size;
- assert(pointer_size == 1 || pointer_size == 2 ||
- pointer_size == 4 || pointer_size == 8);
+ entry.pointer.offset = cpu_to_le32(dst_patched_offset);
+ entry.pointer.size = dst_patched_size;
+ assert(dst_patched_size == 1 || dst_patched_size == 2 ||
+ dst_patched_size == 4 || dst_patched_size == 8);
+
+ le_src_offset = cpu_to_le64(src_offset);
+ memcpy(dst_file->blob->data + dst_patched_offset,
+ &le_src_offset, dst_patched_size);
g_array_append_vals(linker->cmd_blob, &entry, sizeof entry);
}
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index b0ac70d..e51a06a 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -337,9 +337,12 @@ static void acpi_dsdt_add_power_button(Aml *scope)
/* RSDP */
static GArray *
-build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt)
+build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt_tbl_offset)
{
AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
+ unsigned rsdt_pa_size = sizeof(rsdp->rsdt_physical_address);
+ unsigned rsdt_pa_offset =
+ (char *)&rsdp->rsdt_physical_address - rsdp_table->data;
bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, rsdp_table, 16,
true /* fseg memory */);
@@ -349,13 +352,11 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt)
rsdp->length = cpu_to_le32(sizeof(*rsdp));
rsdp->revision = 0x02;
- /* Point to RSDT */
- rsdp->rsdt_physical_address = cpu_to_le32(rsdt);
/* Address to be filled by Guest linker */
- bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE,
- ACPI_BUILD_TABLE_FILE,
- &rsdp->rsdt_physical_address,
- sizeof rsdp->rsdt_physical_address);
+ bios_linker_loader_add_pointer(linker,
+ ACPI_BUILD_RSDP_FILE, rsdt_pa_offset, rsdt_pa_size,
+ ACPI_BUILD_TABLE_FILE, rsdt_tbl_offset);
+
rsdp->checksum = 0;
/* Checksum to be filled by Guest linker */
bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
@@ -504,9 +505,10 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
/* FADT */
static void
-build_fadt(GArray *table_data, BIOSLinker *linker, unsigned dsdt)
+build_fadt(GArray *table_data, BIOSLinker *linker, unsigned dsdt_tbl_offset)
{
AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
+ unsigned dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data;
/* Hardware Reduced = 1 and use PSCI 0.2+ and with HVC */
fadt->flags = cpu_to_le32(1 << ACPI_FADT_F_HW_REDUCED_ACPI);
@@ -516,12 +518,10 @@ build_fadt(GArray *table_data, BIOSLinker *linker, unsigned dsdt)
/* ACPI v5.1 (fadt->revision.fadt->minor_revision) */
fadt->minor_revision = 0x1;
- fadt->dsdt = cpu_to_le32(dsdt);
/* DSDT address to be filled by Guest linker */
- bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
- ACPI_BUILD_TABLE_FILE,
- &fadt->dsdt,
- sizeof fadt->dsdt);
+ bios_linker_loader_add_pointer(linker,
+ ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt),
+ ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
build_header(linker, table_data,
(void *)fadt, "FACP", sizeof(*fadt), 5, NULL, NULL);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 3540815..ec9f9ae 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -336,26 +336,23 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)
/* FADT */
static void
build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
- unsigned facs, unsigned dsdt,
+ unsigned facs_tbl_offset, unsigned dsdt_tbl_offset,
const char *oem_id, const char *oem_table_id)
{
AcpiFadtDescriptorRev1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
+ unsigned fw_ctrl_offset = (char *)&fadt->firmware_ctrl - table_data->data;
+ unsigned dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data;
- fadt->firmware_ctrl = cpu_to_le32(facs);
/* FACS address to be filled by Guest linker */
- bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
- ACPI_BUILD_TABLE_FILE,
- &fadt->firmware_ctrl,
- sizeof fadt->firmware_ctrl);
+ bios_linker_loader_add_pointer(linker,
+ ACPI_BUILD_TABLE_FILE, fw_ctrl_offset, sizeof(fadt->firmware_ctrl),
+ ACPI_BUILD_TABLE_FILE, facs_tbl_offset);
- fadt->dsdt = cpu_to_le32(dsdt);
/* DSDT address to be filled by Guest linker */
- bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
- ACPI_BUILD_TABLE_FILE,
- &fadt->dsdt,
- sizeof fadt->dsdt);
-
fadt_setup(fadt, pm);
+ bios_linker_loader_add_pointer(linker,
+ ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt),
+ ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
build_header(linker, table_data,
(void *)fadt, "FACP", sizeof(*fadt), 1, oem_id, oem_table_id);
@@ -2310,6 +2307,9 @@ static void
build_tpm_tcpa(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)
{
Acpi20Tcpa *tcpa = acpi_data_push(table_data, sizeof *tcpa);
+ unsigned log_addr_size = sizeof(tcpa->log_area_start_address);
+ unsigned log_addr_offset =
+ (char *)&tcpa->log_area_start_address - table_data->data;
tcpa->platform_class = cpu_to_le16(TPM_TCPA_ACPI_CLASS_CLIENT);
tcpa->log_area_minimum_length = cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE);
@@ -2319,10 +2319,9 @@ build_tpm_tcpa(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)
false /* high memory */);
/* log area start address to be filled by Guest linker */
- bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
- ACPI_BUILD_TPMLOG_FILE,
- &tcpa->log_area_start_address,
- sizeof(tcpa->log_area_start_address));
+ bios_linker_loader_add_pointer(linker,
+ ACPI_BUILD_TABLE_FILE, log_addr_offset, log_addr_size,
+ ACPI_BUILD_TPMLOG_FILE, 0);
build_header(linker, table_data,
(void *)tcpa, "TCPA", sizeof(*tcpa), 2, NULL, NULL);
@@ -2511,21 +2510,23 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker)
}
static GArray *
-build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt)
+build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt_tbl_offset)
{
AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
+ unsigned rsdt_pa_size = sizeof(rsdp->rsdt_physical_address);
+ unsigned rsdt_pa_offset =
+ (char *)&rsdp->rsdt_physical_address - rsdp_table->data;
bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, rsdp_table, 16,
true /* fseg memory */);
memcpy(&rsdp->signature, "RSD PTR ", 8);
memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, 6);
- rsdp->rsdt_physical_address = cpu_to_le32(rsdt);
/* Address to be filled by Guest linker */
- bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE,
- ACPI_BUILD_TABLE_FILE,
- &rsdp->rsdt_physical_address,
- sizeof rsdp->rsdt_physical_address);
+ bios_linker_loader_add_pointer(linker,
+ ACPI_BUILD_RSDP_FILE, rsdt_pa_offset, rsdt_pa_size,
+ ACPI_BUILD_TABLE_FILE, rsdt_tbl_offset);
+
rsdp->checksum = 0;
/* Checksum to be filled by Guest linker */
bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
diff --git a/include/hw/acpi/bios-linker-loader.h b/include/hw/acpi/bios-linker-loader.h
index 564e192..f666b7c 100644
--- a/include/hw/acpi/bios-linker-loader.h
+++ b/include/hw/acpi/bios-linker-loader.h
@@ -22,9 +22,10 @@ void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file,
void bios_linker_loader_add_pointer(BIOSLinker *linker,
const char *dest_file,
+ uint32_t dst_patched_offset,
+ uint8_t dst_patched_size,
const char *src_file,
- void *pointer,
- uint8_t pointer_size);
+ uint32_t src_offset);
void bios_linker_loader_cleanup(BIOSLinker *linker);
#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 6/6] acpi: make bios_linker_loader_add_checksum() API offset based
2016-03-07 15:19 [Qemu-devel] [PATCH 0/6] acpi: simplify bios_linker API Igor Mammedov
` (4 preceding siblings ...)
2016-03-07 15:19 ` [Qemu-devel] [PATCH 5/6] acpi: make bios_linker_loader_add_pointer() API offset based Igor Mammedov
@ 2016-03-07 15:19 ` Igor Mammedov
2016-03-07 15:28 ` [Qemu-devel] [PATCH 0/6] acpi: simplify bios_linker API Michael S. Tsirkin
6 siblings, 0 replies; 8+ messages in thread
From: Igor Mammedov @ 2016-03-07 15:19 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, guangrong.xiao, ehabkost, mst, qemu-arm,
zhaoshenglong, pbonzini
It should help to make clear that bios_linker works in terms
of offsets within a file. Also it should prevent mistakes
where user passes as arguments pointers to unrelated to file blobs.
While at it, considering that it's a ACPI checksum and
it's initial value must be 0, move checksum field zeroing
into bios_linker_loader_add_checksum() instead of doing it
at every call site manually before bios_linker_loader_add_checksum()
is called.
In addition add extra boundary checks.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/acpi/aml-build.c | 5 +++--
hw/acpi/bios-linker-loader.c | 36 +++++++++++++-----------------------
hw/arm/virt-acpi-build.c | 5 ++---
hw/i386/acpi-build.c | 5 ++---
include/hw/acpi/bios-linker-loader.h | 4 ++--
5 files changed, 22 insertions(+), 33 deletions(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 8fc1ed1..65b8f17 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1427,6 +1427,8 @@ build_header(BIOSLinker *linker, GArray *table_data,
AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
const char *oem_id, const char *oem_table_id)
{
+ unsigned tbl_offset = (char *)h - table_data->data;
+ unsigned checksum_offset = (char *)&h->checksum - table_data->data;
memcpy(&h->signature, sig, 4);
h->length = cpu_to_le32(len);
h->revision = rev;
@@ -1447,10 +1449,9 @@ build_header(BIOSLinker *linker, GArray *table_data,
h->oem_revision = cpu_to_le32(1);
memcpy(h->asl_compiler_id, ACPI_BUILD_APPNAME4, 4);
h->asl_compiler_revision = cpu_to_le32(1);
- h->checksum = 0;
/* Checksum to be filled in by Guest linker */
bios_linker_loader_add_checksum(linker, ACPI_BUILD_TABLE_FILE,
- h, len, &h->checksum);
+ tbl_offset, len, checksum_offset);
}
void *acpi_data_push(GArray *table_data, unsigned size)
diff --git a/hw/acpi/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c
index d567362..e7fd61a 100644
--- a/hw/acpi/bios-linker-loader.c
+++ b/hw/acpi/bios-linker-loader.c
@@ -189,8 +189,8 @@ void bios_linker_loader_alloc(BIOSLinker *linker,
}
/*
- * bios_linker_loader_add_checksum: ask guest to add checksum of file data
- * into (same) file at the specified pointer.
+ * bios_linker_loader_add_checksum: ask guest to add checksum of ACPI
+ * table in the specified file at the specified offset.
*
* Checksum calculation simply sums -X for each byte X in the range
* using 8-bit math (i.e. ACPI checksum).
@@ -198,35 +198,25 @@ void bios_linker_loader_alloc(BIOSLinker *linker,
* @linker: linker object instance
* @file: file that includes the checksum to be calculated
* and the data to be checksummed
- * @start, @size: range of data to checksum
- * @checksum: location of the checksum to be patched within file blob
- *
- * Notes:
- * - checksum byte initial value must have been pushed into blob
- * associated with @file and reside at address @checksum.
- * - @size bytes must have been pushed into blob associated wtih @file
- * and reside at address @start.
- * - Guest calculates checksum of specified range of data, result is added to
- * initial value at @checksum into copy of @file in Guest memory.
- * - Range might include the checksum itself.
- * - To avoid confusion, caller must always put 0x0 at @checksum.
- * - @file must be loaded into Guest memory using bios_linker_loader_alloc
+ * @start_offset, @size: range of data in the file to checksum,
+ * relative to the start of file blob
+ * @checksum_offset: location of the checksum to be patched within file blob,
+ * relative to the start of file blob
*/
void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file_name,
- void *start, unsigned size,
- uint8_t *checksum)
+ unsigned start_offset, unsigned size,
+ unsigned checksum_offset)
{
BiosLinkerLoaderEntry entry;
const BiosLinkerFileEntry *file = bios_linker_find_file(linker, file_name);
- ptrdiff_t checksum_offset = (gchar *)checksum - file->blob->data;
- ptrdiff_t start_offset = (gchar *)start - file->blob->data;
- assert(checksum_offset >= 0);
- assert(start_offset >= 0);
- assert(checksum_offset + 1 <= file->blob->len);
+ assert(file);
+ assert(start_offset < file->blob->len);
assert(start_offset + size <= file->blob->len);
- assert(*checksum == 0x0);
+ assert(checksum_offset >= start_offset);
+ assert(checksum_offset + 1 <= start_offset + size);
+ *(file->blob->data + checksum_offset) = 0;
memset(&entry, 0, sizeof entry);
strncpy(entry.cksum.file, file_name, sizeof entry.cksum.file - 1);
entry.command = cpu_to_le32(BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM);
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index e51a06a..f958231 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -357,11 +357,10 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt_tbl_offset)
ACPI_BUILD_RSDP_FILE, rsdt_pa_offset, rsdt_pa_size,
ACPI_BUILD_TABLE_FILE, rsdt_tbl_offset);
- rsdp->checksum = 0;
/* Checksum to be filled by Guest linker */
bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
- rsdp, sizeof *rsdp,
- &rsdp->checksum);
+ (char *)rsdp - rsdp_table->data, sizeof *rsdp,
+ (char *)&rsdp->checksum - rsdp_table->data);
return rsdp_table;
}
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index ec9f9ae..7f31cc2 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2527,11 +2527,10 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt_tbl_offset)
ACPI_BUILD_RSDP_FILE, rsdt_pa_offset, rsdt_pa_size,
ACPI_BUILD_TABLE_FILE, rsdt_tbl_offset);
- rsdp->checksum = 0;
/* Checksum to be filled by Guest linker */
bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
- rsdp, sizeof *rsdp,
- &rsdp->checksum);
+ (char *)rsdp - rsdp_table->data, sizeof *rsdp,
+ (char *)&rsdp->checksum - rsdp_table->data);
return rsdp_table;
}
diff --git a/include/hw/acpi/bios-linker-loader.h b/include/hw/acpi/bios-linker-loader.h
index f666b7c..a05227e 100644
--- a/include/hw/acpi/bios-linker-loader.h
+++ b/include/hw/acpi/bios-linker-loader.h
@@ -17,8 +17,8 @@ void bios_linker_loader_alloc(BIOSLinker *linker,
bool alloc_fseg);
void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file,
- void *start, unsigned size,
- uint8_t *checksum);
+ unsigned start_offset, unsigned size,
+ unsigned checksum_offset);
void bios_linker_loader_add_pointer(BIOSLinker *linker,
const char *dest_file,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] acpi: simplify bios_linker API
2016-03-07 15:19 [Qemu-devel] [PATCH 0/6] acpi: simplify bios_linker API Igor Mammedov
` (5 preceding siblings ...)
2016-03-07 15:19 ` [Qemu-devel] [PATCH 6/6] acpi: make bios_linker_loader_add_checksum() " Igor Mammedov
@ 2016-03-07 15:28 ` Michael S. Tsirkin
6 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2016-03-07 15:28 UTC (permalink / raw)
To: Igor Mammedov
Cc: peter.maydell, guangrong.xiao, ehabkost, qemu-devel, qemu-arm,
zhaoshenglong, pbonzini
On Mon, Mar 07, 2016 at 04:19:02PM +0100, Igor Mammedov wrote:
> Simplify bios_linker API by adding filei name + blob tracking
> so user would have to suppily less arguments which
> reduces chances of mistake by using wrong blob pointer
> with not related file.
> And also refactor API to take file offsets, as it's what
> actually API were doing only internally, instead of
> using host pointers. I hope ti makes it easier for API user
> to use API without need to look at implementation on
> QEMU/SeaBIOS.
Thanks!
As we are in freeze, I'll probably defer this until after the release though.
> Igor Mammedov (6):
> acpi: convert linker from GArray to BOISLinker structure
> acpi: simplify bios_linker API by removing redundant 'table' argument
> acpi: cleanup bios_linker_loader_cleanup()
> tpm: apci: cleanup TCPA table initialization
> acpi: make bios_linker_loader_add_pointer() API offset based
> acpi: make bios_linker_loader_add_checksum() API offset based
>
> hw/acpi/aml-build.c | 33 +++----
> hw/acpi/bios-linker-loader.c | 179 +++++++++++++++++++++--------------
> hw/acpi/nvdimm.c | 6 +-
> hw/arm/virt-acpi-build.c | 51 +++++-----
> hw/i386/acpi-build.c | 89 +++++++++--------
> include/hw/acpi/aml-build.h | 7 +-
> include/hw/acpi/bios-linker-loader.h | 28 +++---
> include/hw/mem/nvdimm.h | 3 +-
> 8 files changed, 222 insertions(+), 174 deletions(-)
>
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-03-07 15:28 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-07 15:19 [Qemu-devel] [PATCH 0/6] acpi: simplify bios_linker API Igor Mammedov
2016-03-07 15:19 ` [Qemu-devel] [PATCH 1/6] acpi: convert linker from GArray to BOISLinker structure Igor Mammedov
2016-03-07 15:19 ` [Qemu-devel] [PATCH 2/6] acpi: simplify bios_linker API by removing redundant 'table' argument Igor Mammedov
2016-03-07 15:19 ` [Qemu-devel] [PATCH 3/6] acpi: cleanup bios_linker_loader_cleanup() Igor Mammedov
2016-03-07 15:19 ` [Qemu-devel] [PATCH 4/6] tpm: apci: cleanup TCPA table initialization Igor Mammedov
2016-03-07 15:19 ` [Qemu-devel] [PATCH 5/6] acpi: make bios_linker_loader_add_pointer() API offset based Igor Mammedov
2016-03-07 15:19 ` [Qemu-devel] [PATCH 6/6] acpi: make bios_linker_loader_add_checksum() " Igor Mammedov
2016-03-07 15:28 ` [Qemu-devel] [PATCH 0/6] acpi: simplify bios_linker API Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).