* [Qemu-devel] [PATCH v2 1/6] acpi: convert linker from GArray to BIOSLinker structure
2016-05-19 13:19 [Qemu-devel] [PATCH v2 0/6] acpi: simplify bios_linker API Igor Mammedov
@ 2016-05-19 13:19 ` Igor Mammedov
2016-05-19 13:19 ` [Qemu-devel] [PATCH v2 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-05-19 13:19 UTC (permalink / raw)
To: qemu-devel; +Cc: guangrong.xiao, ehabkost, mst, qemu-arm, zhaoshenglong
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 | 24 +++++++++++-----------
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, 70 insertions(+), 58 deletions(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index cedb74e..a2c9270 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)
{
@@ -1473,7 +1472,7 @@ Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target)
}
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)
{
@@ -1541,7 +1540,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 5153ab1..6b15a85 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)
@@ -139,7 +145,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);
}
/*
@@ -149,7 +155,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
@@ -167,7 +173,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)
@@ -189,14 +195,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
@@ -213,7 +219,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,
@@ -236,5 +242,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 9531340..cadb628 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -353,7 +353,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;
@@ -577,7 +577,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, *field;
int mem_addr_offset, nvdimm_ssdt;
@@ -691,7 +691,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 26a7bac..4fe150a 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -353,7 +353,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);
@@ -382,7 +382,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];
@@ -415,7 +415,7 @@ build_spcr(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
}
static void
-build_srat(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
+build_srat(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
{
AcpiSystemResourceAffinityTable *srat;
AcpiSratProcessorGiccAffinity *core;
@@ -455,13 +455,12 @@ build_srat(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
mem_base += numa_info[i].node_mem;
}
- build_header(linker, table_data,
- (void *)(table_data->data + srat_start), "SRAT",
+ build_header(linker, table_data, (void *)srat, "SRAT",
table_data->len - srat_start, 3, NULL, NULL);
}
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;
@@ -481,7 +480,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;
@@ -507,7 +506,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;
@@ -566,7 +565,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));
@@ -591,7 +590,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;
@@ -730,7 +729,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);
@@ -788,7 +787,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 279f0d7..77aefd5 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -262,7 +262,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);
@@ -307,7 +307,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)
{
@@ -334,7 +334,7 @@ build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm,
}
static void
-build_madt(GArray *table_data, GArray *linker, PCMachineState *pcms)
+build_madt(GArray *table_data, BIOSLinker *linker, PCMachineState *pcms)
{
MachineClass *mc = MACHINE_GET_CLASS(pcms);
CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(pcms));
@@ -1979,7 +1979,7 @@ static Aml *build_q35_osc_method(void)
}
static void
-build_dsdt(GArray *table_data, GArray *linker,
+build_dsdt(GArray *table_data, BIOSLinker *linker,
AcpiPmInfo *pm, AcpiMiscInfo *misc,
PcPciInfo *pci, MachineState *machine)
{
@@ -2373,7 +2373,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;
@@ -2388,7 +2388,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);
@@ -2413,7 +2413,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;
@@ -2428,7 +2428,7 @@ build_tpm2(GArray *table_data, GArray *linker)
}
static void
-build_srat(GArray *table_data, GArray *linker, MachineState *machine)
+build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
{
AcpiSystemResourceAffinityTable *srat;
AcpiSratProcessorAffinity *core;
@@ -2525,7 +2525,7 @@ build_srat(GArray *table_data, GArray *linker, MachineState *machine)
}
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;
@@ -2554,7 +2554,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;
@@ -2578,7 +2578,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);
@@ -2790,7 +2790,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
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);
@@ -2830,7 +2830,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);
}
@@ -2894,7 +2894,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 7eb51c7..5681237 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
@@ -210,7 +211,7 @@ struct AcpiBuildTables {
GArray *table_data;
GArray *rsdp;
GArray *tcpalog;
- GArray *linker;
+ BIOSLinker *linker;
} AcpiBuildTables;
/**
@@ -363,7 +364,7 @@ Aml *aml_sizeof(Aml *arg);
Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target);
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);
@@ -372,7 +373,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);
int
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 517de9c..32e1445 100644
--- a/include/hw/mem/nvdimm.h
+++ b/include/hw/mem/nvdimm.h
@@ -24,6 +24,7 @@
#define QEMU_NVDIMM_H
#include "hw/mem/pc-dimm.h"
+#include "hw/acpi/bios-linker-loader.h"
#define NVDIMM_DEBUG 0
#define nvdimm_debug(fmt, ...) \
@@ -58,5 +59,5 @@ typedef struct AcpiNVDIMMState AcpiNVDIMMState;
void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
FWCfgState *fw_cfg, Object *owner);
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 v2 2/6] acpi: simplify bios_linker API by removing redundant 'table' argument
2016-05-19 13:19 [Qemu-devel] [PATCH v2 0/6] acpi: simplify bios_linker API Igor Mammedov
2016-05-19 13:19 ` [Qemu-devel] [PATCH v2 1/6] acpi: convert linker from GArray to BIOSLinker structure Igor Mammedov
@ 2016-05-19 13:19 ` Igor Mammedov
2016-05-19 13:19 ` [Qemu-devel] [PATCH v2 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-05-19 13:19 UTC (permalink / raw)
To: qemu-devel; +Cc: guangrong.xiao, ehabkost, mst, qemu-arm, zhaoshenglong
'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>
---
v2:
- fix conflicts in nvdimm code
---
hw/acpi/aml-build.c | 4 +-
hw/acpi/bios-linker-loader.c | 82 +++++++++++++++++++++++++++---------
hw/acpi/nvdimm.c | 15 ++++---
hw/arm/virt-acpi-build.c | 11 ++---
hw/i386/acpi-build.c | 20 +++++----
include/hw/acpi/bios-linker-loader.h | 7 +--
include/hw/mem/nvdimm.h | 2 +-
7 files changed, 94 insertions(+), 47 deletions(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index a2c9270..8104d06 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1499,7 +1499,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)
@@ -1556,7 +1556,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 6b15a85..a2f20ec 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 = alloc_fseg ? BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG :
@@ -158,38 +199,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);
@@ -205,13 +245,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
@@ -222,14 +261,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/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index cadb628..7ed64d0 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -577,7 +577,8 @@ 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, BIOSLinker *linker)
+ GArray *table_data, BIOSLinker *linker,
+ GArray *dsm_dma_arrea)
{
Aml *ssdt, *sb_scope, *dev, *field;
int mem_addr_offset, nvdimm_ssdt;
@@ -678,10 +679,11 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
mem_addr_offset = build_append_named_dword(table_data,
NVDIMM_ACPI_MEM_ADDR);
- bios_linker_loader_alloc(linker, NVDIMM_DSM_MEM_FILE, TARGET_PAGE_SIZE,
- false /* high memory */);
+ bios_linker_loader_alloc(linker,
+ NVDIMM_DSM_MEM_FILE, dsm_dma_arrea,
+ TARGET_PAGE_SIZE, false /* high memory */);
bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
- NVDIMM_DSM_MEM_FILE, table_data,
+ NVDIMM_DSM_MEM_FILE,
table_data->data + mem_addr_offset,
sizeof(uint32_t));
build_header(linker, table_data,
@@ -691,7 +693,7 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
}
void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
- BIOSLinker *linker)
+ BIOSLinker *linker, GArray *dsm_dma_arrea)
{
GSList *device_list;
@@ -701,6 +703,7 @@ void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
return;
}
nvdimm_build_nfit(device_list, table_offsets, table_data, linker);
- nvdimm_build_ssdt(device_list, table_offsets, table_data, linker);
+ nvdimm_build_ssdt(device_list, table_offsets, table_data, linker,
+ dsm_dma_arrea);
g_slist_free(device_list);
}
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 4fe150a..2d85bc4 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -357,7 +357,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));
@@ -370,12 +370,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;
@@ -581,7 +581,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,
@@ -650,7 +650,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 77aefd5..ce96533 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -317,14 +317,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);
@@ -2397,13 +2397,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,
@@ -2582,7 +2582,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);
@@ -2591,12 +2591,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;
@@ -2670,7 +2670,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
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 */);
@@ -2727,7 +2728,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
build_dmar_q35(tables_blob, tables->linker);
}
if (pcms->acpi_nvdimm_state.is_enabled) {
- nvdimm_build_acpi(table_offsets, tables_blob, tables->linker);
+ nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
+ pcms->acpi_nvdimm_state.dsm_mem);
}
/* Add tables supplied by user (if any) */
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);
diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
index 32e1445..60ee92b 100644
--- a/include/hw/mem/nvdimm.h
+++ b/include/hw/mem/nvdimm.h
@@ -59,5 +59,5 @@ typedef struct AcpiNVDIMMState AcpiNVDIMMState;
void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
FWCfgState *fw_cfg, Object *owner);
void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
- BIOSLinker *linker);
+ BIOSLinker *linker, GArray *dsm_dma_arrea);
#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Qemu-devel] [PATCH v2 3/6] acpi: cleanup bios_linker_loader_cleanup()
2016-05-19 13:19 [Qemu-devel] [PATCH v2 0/6] acpi: simplify bios_linker API Igor Mammedov
2016-05-19 13:19 ` [Qemu-devel] [PATCH v2 1/6] acpi: convert linker from GArray to BIOSLinker structure Igor Mammedov
2016-05-19 13:19 ` [Qemu-devel] [PATCH v2 2/6] acpi: simplify bios_linker API by removing redundant 'table' argument Igor Mammedov
@ 2016-05-19 13:19 ` Igor Mammedov
2016-05-19 13:19 ` [Qemu-devel] [PATCH v2 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-05-19 13:19 UTC (permalink / raw)
To: qemu-devel; +Cc: guangrong.xiao, ehabkost, mst, qemu-arm, zhaoshenglong
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 | 8 +++-----
include/hw/acpi/bios-linker-loader.h | 2 +-
3 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 8104d06..42955d6 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1531,8 +1531,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 a2f20ec..7a2e4d2 100644
--- a/hw/acpi/bios-linker-loader.c
+++ b/hw/acpi/bios-linker-loader.c
@@ -128,14 +128,12 @@ BIOSLinker *bios_linker_loader_init(void)
return linker;
}
-/* Free linker wrapper and return the linker commands array. */
-void *bios_linker_loader_cleanup(BIOSLinker *linker)
+/* Free linker wrapper */
+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 v2 4/6] tpm: apci: cleanup TCPA table initialization
2016-05-19 13:19 [Qemu-devel] [PATCH v2 0/6] acpi: simplify bios_linker API Igor Mammedov
` (2 preceding siblings ...)
2016-05-19 13:19 ` [Qemu-devel] [PATCH v2 3/6] acpi: cleanup bios_linker_loader_cleanup() Igor Mammedov
@ 2016-05-19 13:19 ` Igor Mammedov
2016-05-19 13:19 ` [Qemu-devel] [PATCH v2 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-05-19 13:19 UTC (permalink / raw)
To: qemu-devel; +Cc: guangrong.xiao, ehabkost, mst, qemu-arm, zhaoshenglong
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.
Also as 'TCPA' is allocated 0 filled, there is no point
in calculating always 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 not needed offset
calculations.
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 ce96533..c99dbe7 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2391,11 +2391,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, le32_to_cpu(tcpa->log_area_minimum_length));
bios_linker_loader_alloc(linker, ACPI_BUILD_TPMLOG_FILE, tcpalog, 1,
false /* high memory */);
@@ -2408,8 +2407,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 v2 5/6] acpi: make bios_linker_loader_add_pointer() API offset based
2016-05-19 13:19 [Qemu-devel] [PATCH v2 0/6] acpi: simplify bios_linker API Igor Mammedov
` (3 preceding siblings ...)
2016-05-19 13:19 ` [Qemu-devel] [PATCH v2 4/6] tpm: apci: cleanup TCPA table initialization Igor Mammedov
@ 2016-05-19 13:19 ` Igor Mammedov
2016-05-19 13:19 ` [Qemu-devel] [PATCH v2 6/6] acpi: make bios_linker_loader_add_checksum() " Igor Mammedov
2016-05-31 15:09 ` [Qemu-devel] [PATCH v2 0/6] acpi: simplify bios_linker API Igor Mammedov
6 siblings, 0 replies; 8+ messages in thread
From: Igor Mammedov @ 2016-05-19 13:19 UTC (permalink / raw)
To: qemu-devel; +Cc: guangrong.xiao, ehabkost, mst, qemu-arm, zhaoshenglong
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>
---
v2:
- fix conflicts in nvdimm code
---
hw/acpi/aml-build.c | 20 ++++++++------
hw/acpi/bios-linker-loader.c | 53 ++++++++++++++++++++----------------
hw/acpi/nvdimm.c | 7 ++---
hw/arm/virt-acpi-build.c | 26 +++++++++---------
hw/i386/acpi-build.c | 45 +++++++++++++++---------------
include/hw/acpi/bios-linker-loader.h | 5 ++--
6 files changed, 82 insertions(+), 74 deletions(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 42955d6..46ffce0 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1542,21 +1542,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 7a2e4d2..4551c3a 100644
--- a/hw/acpi/bios-linker-loader.c
+++ b/hw/acpi/bios-linker-loader.c
@@ -237,37 +237,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,
@@ -275,10 +276,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/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 7ed64d0..4b57403 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -682,10 +682,9 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
bios_linker_loader_alloc(linker,
NVDIMM_DSM_MEM_FILE, dsm_dma_arrea,
TARGET_PAGE_SIZE, false /* high memory */);
- bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
- NVDIMM_DSM_MEM_FILE,
- table_data->data + mem_addr_offset,
- sizeof(uint32_t));
+ bios_linker_loader_add_pointer(linker,
+ ACPI_BUILD_TABLE_FILE, mem_addr_offset, sizeof(uint32_t),
+ NVDIMM_DSM_MEM_FILE, 0);
build_header(linker, table_data,
(void *)(table_data->data + nvdimm_ssdt),
"SSDT", table_data->len - nvdimm_ssdt, 1, NULL, "NVDIMM");
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 2d85bc4..1e37681 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -353,9 +353,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 */);
@@ -365,13 +368,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,
@@ -565,9 +566,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);
@@ -577,12 +579,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 c99dbe7..5442768 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -308,26 +308,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);
@@ -2391,6 +2388,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);
@@ -2400,10 +2400,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);
@@ -2575,21 +2574,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 v2 6/6] acpi: make bios_linker_loader_add_checksum() API offset based
2016-05-19 13:19 [Qemu-devel] [PATCH v2 0/6] acpi: simplify bios_linker API Igor Mammedov
` (4 preceding siblings ...)
2016-05-19 13:19 ` [Qemu-devel] [PATCH v2 5/6] acpi: make bios_linker_loader_add_pointer() API offset based Igor Mammedov
@ 2016-05-19 13:19 ` Igor Mammedov
2016-05-31 15:09 ` [Qemu-devel] [PATCH v2 0/6] acpi: simplify bios_linker API Igor Mammedov
6 siblings, 0 replies; 8+ messages in thread
From: Igor Mammedov @ 2016-05-19 13:19 UTC (permalink / raw)
To: qemu-devel; +Cc: guangrong.xiao, ehabkost, mst, qemu-arm, zhaoshenglong
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 46ffce0..66888c7 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1476,6 +1476,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;
@@ -1496,10 +1498,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 4551c3a..7fd462f 100644
--- a/hw/acpi/bios-linker-loader.c
+++ b/hw/acpi/bios-linker-loader.c
@@ -188,8 +188,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).
@@ -197,35 +197,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 1e37681..f2cb117 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -373,11 +373,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 5442768..4515382 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2591,11 +2591,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 v2 0/6] acpi: simplify bios_linker API
2016-05-19 13:19 [Qemu-devel] [PATCH v2 0/6] acpi: simplify bios_linker API Igor Mammedov
` (5 preceding siblings ...)
2016-05-19 13:19 ` [Qemu-devel] [PATCH v2 6/6] acpi: make bios_linker_loader_add_checksum() " Igor Mammedov
@ 2016-05-31 15:09 ` Igor Mammedov
6 siblings, 0 replies; 8+ messages in thread
From: Igor Mammedov @ 2016-05-31 15:09 UTC (permalink / raw)
To: qemu-devel; +Cc: zhaoshenglong, qemu-arm, mst
On Thu, 19 May 2016 15:19:24 +0200
Igor Mammedov <imammedo@redhat.com> wrote:
> Simplify bios_linker API by adding file 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,
> which is what is usualy used in context of file APIs,
> (and as it's what actually current API were doing only internally),
> instead of using host pointers.
> I hope it makes it easier for user to use API without
> need to look at implementation in QEMU/SeaBIOS.
ping
>
>
> Igor Mammedov (6):
> acpi: convert linker from GArray to BIOSLinker 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 | 22 +++--
> hw/arm/virt-acpi-build.c | 56 +++++------
> hw/i386/acpi-build.c | 92 +++++++++---------
> include/hw/acpi/aml-build.h | 7 +-
> include/hw/acpi/bios-linker-loader.h | 28 +++---
> include/hw/mem/nvdimm.h | 3 +-
> 8 files changed, 235 insertions(+), 185 deletions(-)
>
^ permalink raw reply [flat|nested] 8+ messages in thread