|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [qemu-xen master] loader: fix handling of custom address spaces when adding ROM blobs
commit aa6c6ae843cbdc251224bc6170d2663ac929b04f
Author: Laszlo Ersek <lersek@xxxxxxxxxx>
AuthorDate: Tue Nov 29 20:55:32 2016 +0100
Commit: Michael S. Tsirkin <mst@xxxxxxxxxx>
CommitDate: Wed Nov 30 04:20:57 2016 +0200
loader: fix handling of custom address spaces when adding ROM blobs
* Commit 3e76099aacb4 ("loader: Allow a custom AddressSpace when loading
ROMs") introduced the "Rom.as" field:
(1) It modified the utility callers of rom_insert() to take "as" as a
new parameter from *their* callers, and set "rom->as" from that
parameter. The functions covered were rom_add_file() and
rom_add_elf_program().
(2) It also modified rom_insert() itself, to auto-assign
"&address_space_memory", in case the external caller passed -- and
the utility caller forwarded -- as=NULL.
Except, commit 3e76099aacb4 forgot to update the third utility caller of
rom_insert(), under point (1), namely rom_add_blob().
* Later, commit 5e774eb3bd264 ("loader: Add AddressSpace loading support
to uImages") added the load_uimage_as() function, and the
rom_add_blob_fixed_as() function-like macro, with the necessary changes
elsewhere to propagate the new "as" parameter to rom_add_blob():
load_uimage_as()
load_uboot_image()
rom_add_blob_fixed_as()
rom_add_blob()
At this point, the signature (and workings) of rom_add_blob() had been
broken already, and the rom_add_blob_fixed_as() macro passed its "_as"
parameter to rom_add_blob() as "callback_opaque". Given that the
"fw_callback" parameter itself was set to NULL (correctly), this did no
additional damage (the opaque arg would never be used), but ultimately
it broke the new functionality of load_uimage_as().
* The load_uimage_as() function would be put to use in one of the later
patches, commit e481a1f63c93 ("generic-loader: Add a generic loader").
* We can fix this only in a unified patch now. Append "AddressSpace *as"
to the signature of rom_add_blob(), and handle the new parameter. Pass
NULL from all current callers, except from rom_add_blob_fixed_as(),
where "_as" has to be bumped to the proper position.
* Note that rom_add_file() rejects the case when both "mr" and "as" are
passed in as non-NULL. The action that this is apparently supposed to
prevent is the
rom->mr = mr;
assignment (that's the only place where the "mr" parameter is used in
rom_add_file()). In rom_add_blob() though, we have no "mr" parameter,
and the actions done on the fw_cfg branch:
if (fw_file_name && fw_cfg) {
if (mc->rom_file_has_mr) {
data = rom_set_mr(rom, OBJECT(fw_cfg), devpath);
mr = rom->mr;
} else {
data = rom->data;
}
reflect those that are performed by rom_add_file() too (with mr==NULL):
if (rom->fw_file && fw_cfg) {
if ((!option_rom || mc->option_rom_has_mr) &&
mc->rom_file_has_mr) {
data = rom_set_mr(rom, OBJECT(fw_cfg), devpath);
} else {
data = rom->data;
}
Hence we need no additional restrictions in rom_add_blob().
* Stable is not affected as both problematic commits appeared first in
v2.8.0-rc0.
Cc: "Michael S. Tsirkin" <mst@xxxxxxxxxx>
Cc: Alistair Francis <alistair.francis@xxxxxxxxxx>
Cc: Igor Mammedov <imammedo@xxxxxxxxxx>
Cc: Michael Walle <michael@xxxxxxxx>
Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
Cc: Peter Maydell <peter.maydell@xxxxxxxxxx>
Cc: Shannon Zhao <zhaoshenglong@xxxxxxxxxx>
Cc: qemu-arm@xxxxxxxxxx
Cc: qemu-devel@xxxxxxxxxx
Fixes: 3e76099aacb4dae0d37ebf95305369e03d1491e6
Fixes: 5e774eb3bd264c76484906f4bd0fb38e00b8090e
Signed-off-by: Laszlo Ersek <lersek@xxxxxxxxxx>
Reviewed-by: Alistair Francis <alistair.francis@xxxxxxxxxx>
Reviewed-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
Reviewed-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
---
hw/arm/virt-acpi-build.c | 2 +-
hw/core/loader.c | 4 +++-
hw/i386/acpi-build.c | 2 +-
hw/lm32/lm32_hwsetup.h | 2 +-
include/hw/loader.h | 6 +++---
5 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index f953610..d4160df 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -809,7 +809,7 @@ static MemoryRegion *acpi_add_rom_blob(AcpiBuildState
*build_state,
uint64_t max_size)
{
return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1,
- name, virt_acpi_build_update, build_state);
+ name, virt_acpi_build_update, build_state, NULL);
}
static const VMStateDescription vmstate_virt_acpi_build = {
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 6e022b5..c0d645a 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -978,7 +978,8 @@ err:
MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len,
size_t max_len, hwaddr addr, const char *fw_file_name,
- FWCfgReadCallback fw_callback, void *callback_opaque)
+ FWCfgReadCallback fw_callback, void *callback_opaque,
+ AddressSpace *as)
{
MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
Rom *rom;
@@ -986,6 +987,7 @@ MemoryRegion *rom_add_blob(const char *name, const void
*blob, size_t len,
rom = g_malloc0(sizeof(*rom));
rom->name = g_strdup(name);
+ rom->as = as;
rom->addr = addr;
rom->romsize = max_len ? max_len : len;
rom->datasize = len;
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 45a2ccf..9708cdc 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2936,7 +2936,7 @@ static MemoryRegion *acpi_add_rom_blob(AcpiBuildState
*build_state,
uint64_t max_size)
{
return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1,
- name, acpi_build_update, build_state);
+ name, acpi_build_update, build_state, NULL);
}
static const VMStateDescription vmstate_acpi_build = {
diff --git a/hw/lm32/lm32_hwsetup.h b/hw/lm32/lm32_hwsetup.h
index b71e6ea..23e1878 100644
--- a/hw/lm32/lm32_hwsetup.h
+++ b/hw/lm32/lm32_hwsetup.h
@@ -75,7 +75,7 @@ static inline void hwsetup_create_rom(HWSetup *hw,
hwaddr base)
{
rom_add_blob("hwsetup", hw->data, TARGET_PAGE_SIZE,
- TARGET_PAGE_SIZE, base, NULL, NULL, NULL);
+ TARGET_PAGE_SIZE, base, NULL, NULL, NULL, NULL);
}
static inline void hwsetup_add_u8(HWSetup *hw, uint8_t u)
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 0381706..0c864cf 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -180,7 +180,7 @@ MemoryRegion *rom_add_blob(const char *name, const void
*blob, size_t len,
size_t max_len, hwaddr addr,
const char *fw_file_name,
FWCfgReadCallback fw_callback,
- void *callback_opaque);
+ void *callback_opaque, AddressSpace *as);
int rom_add_elf_program(const char *name, void *data, size_t datasize,
size_t romsize, hwaddr addr, AddressSpace *as);
int rom_check_and_register_reset(void);
@@ -194,7 +194,7 @@ void hmp_info_roms(Monitor *mon, const QDict *qdict);
#define rom_add_file_fixed(_f, _a, _i) \
rom_add_file(_f, NULL, _a, _i, false, NULL, NULL)
#define rom_add_blob_fixed(_f, _b, _l, _a) \
- rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL)
+ rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL, NULL)
#define rom_add_file_mr(_f, _mr, _i) \
rom_add_file(_f, NULL, 0, _i, false, _mr, NULL)
#define rom_add_file_as(_f, _as, _i) \
@@ -202,7 +202,7 @@ void hmp_info_roms(Monitor *mon, const QDict *qdict);
#define rom_add_file_fixed_as(_f, _a, _i, _as) \
rom_add_file(_f, NULL, _a, _i, false, NULL, _as)
#define rom_add_blob_fixed_as(_f, _b, _l, _a, _as) \
- rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, _as)
+ rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL, _as)
#define PC_ROM_MIN_VGA 0xc0000
#define PC_ROM_MIN_OPTION 0xc8000
--
generated by git-patchbot for /home/xen/git/qemu-xen.git#master
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |