* [Qemu-devel] [PATCH v5 0/8] KASLR kernel dump support
@ 2017-08-07 18:16 Marc-André Lureau
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 1/8] fw_cfg: rename read callback Marc-André Lureau
` (9 more replies)
0 siblings, 10 replies; 19+ messages in thread
From: Marc-André Lureau @ 2017-08-07 18:16 UTC (permalink / raw)
To: qemu-devel
Cc: imammedo, berrange, ehabkost, mst, anderson, lersek,
Marc-André Lureau
Recent linux kernels enable KASLR to randomize phys/virt memory
addresses. This series aims to provide enough information in qemu
dumps so that crash utility can work with randomized kernel too (it
hasn't been tested on other archs than x86 though, help welcome).
The previous design to provide qemu with debug details (using qemu-ga
and a dedicated vmcoreinfo ACPI device) failed to satisfy the
requirements during previous iterations.
In particular, the previous proposed vmcoreinfo ACPI device had the
following issues:
- hazardous memory handling with no explicit synchronization
- occupy 2 fw-cfg entries (for memory and pointer)
- occupy 4k of guest memory (this could have been tweaked)
- did not provide ACPI methods (this could have been added)
- may be difficult to maintain compatibility (according to Michael)
This is a new proposal, that leverage fw-cfg device instead of adding
a new device. A "etc/vmcoreinfo" entry is added, where the guest,
during boot or later, can write the addr/size location of an ELF note
to be appended in the qemu dump.
Note: only guest kernel is expected to write to a fw-cfg entry. This
method is not meant for general qemu/user-space communication. There
are more appropriate devices for this purpose, and the guest kernel
should not expose this facility.
This is quite easier to implement, and uses less of the limited fw-cfg
slots, and guest memory. It also solves the synchronization issue, and
may be easier to discover or to maintain compatibility.
The Linux ELF note is expected to be the VMCOREINFO note, which will
have a special handling in qemu in this case helping kaslr-kernel
debugging. But it could be any valid ELF note.
Crash 7.1.9 will parse the "phys_base" value from the VMCOREINFO note,
and thus will work with KASLR-dump produced by this series.
The series implements the note addition in qemu ELF/kdump,
as well as the python scripts/dump-guest-memory.py.
To test:
Using kernel from https://github.com/elmarco/linux fw-cfg branch,
Compile and run guest kernel with CONFIG_RANDOMIZE_BASE=y & CONFIG_FW_CFG_SYSFS=y.
Run qemu with -global fw_cfg.vmcoreinfo=on
Produce an ELF dump:
{ "execute": "dump-guest-memory", "arguments": { "protocol": "file:dump", "paging": false } }
Produce a kdump:
{ "execute": "dump-guest-memory", "arguments": { "protocol": "file:dump", "paging": false, "format": "kdump-zlib" } }
Or with (gdb) dump-guest-memory, with scripts/dump-guest-memory.py script.
Analyze with crash >= 7.1.9 (or the git version for 4.13 fixes..):
$ crash vmlinux dump
v5:
- removed x-write-pointer-available patch from this series
- drop vmcoreinfo device
- add write callback to fw_cfg entries
- add a writable fw_cfg "vmcoreinfo" entry
- split phys_base update from VMCOREINFO note in a seperate patch
- most patches had non-trivial changes, dropping reviewed-by tags
v4: from Laszlo review
- switch to warn_report*()
- update test to follow vmgenid and use boot-sector infrastructure
- fix range checks in the python script
- add vmcoreinfo_get() stub
v3: from Laszlo review
- change vmcoreinfo offset to 36
- reset err to null after report
- use PRIu32
- change name_size and desc_size against MAX_VMCOREINFO_SIZE
- python code simplification
- check boundaries of blocks in phys_memory_read()
- fix some vmgi vs vmci names
- add more comments in code
- fix comment indentation
- add r-b tags
v2: from Laszlo review
- vmci: fix guest endianess handling
- vmci: fix wrong sizeof()
- vmci: add back reset logic from vmgenid
- dump: have 1MB size limit for vmcoreinfo
- dump: fix potential off-by-1 buffer manipulation
- dump: use temporary variable for qemu_strtou64
- dump: fixed VMCOREINFO duplication in kdump
- update gdb script to not call into qemu process
- update MAINTAINERS with some new files
Marc-André Lureau (8):
fw_cfg: rename read callback
fw_cfg: add write callback
fw_cfg: add vmcoreinfo file
dump: add guest ELF note
dump: update phys_base header field based on VMCOREINFO content
kdump: set vmcoreinfo location
scripts/dump-guest-memory.py: add vmcoreinfo
MAINTAINERS: add Dump maintainers
scripts/dump-guest-memory.py | 51 ++++++++++++
include/hw/compat.h | 8 ++
include/hw/loader.h | 2 +-
include/hw/nvram/fw_cfg.h | 18 ++++-
include/sysemu/dump.h | 2 +
dump.c | 179 +++++++++++++++++++++++++++++++++++++++++++
hw/acpi/vmgenid.c | 2 +-
hw/core/loader.c | 4 +-
hw/i386/acpi-build.c | 2 +-
hw/isa/lpc_ich9.c | 4 +-
hw/nvram/fw_cfg.c | 64 ++++++++++++----
MAINTAINERS | 9 +++
docs/specs/fw_cfg.txt | 24 ++++++
13 files changed, 343 insertions(+), 26 deletions(-)
--
2.14.0.1.geff633fa0
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v5 1/8] fw_cfg: rename read callback
2017-08-07 18:16 [Qemu-devel] [PATCH v5 0/8] KASLR kernel dump support Marc-André Lureau
@ 2017-08-07 18:16 ` Marc-André Lureau
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 2/8] fw_cfg: add write callback Marc-André Lureau
` (8 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Marc-André Lureau @ 2017-08-07 18:16 UTC (permalink / raw)
To: qemu-devel
Cc: imammedo, berrange, ehabkost, mst, anderson, lersek,
Marc-André Lureau
The callback is called on select.
Furthermore, the next patch introduced a new callback, so rename the
function type with a generic name.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
include/hw/loader.h | 2 +-
include/hw/nvram/fw_cfg.h | 7 ++++---
hw/core/loader.c | 2 +-
hw/nvram/fw_cfg.c | 30 ++++++++++++++++--------------
4 files changed, 22 insertions(+), 19 deletions(-)
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 490c9ff8e6..355fe0f5a2 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -192,7 +192,7 @@ int rom_add_file(const char *file, const char *fw_dir,
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,
+ FWCfgCallback fw_callback,
void *callback_opaque, AddressSpace *as,
bool read_only);
int rom_add_elf_program(const char *name, void *data, size_t datasize,
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index b77ea48abb..f50d068563 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -44,7 +44,7 @@ typedef struct FWCfgDmaAccess {
uint64_t address;
} QEMU_PACKED FWCfgDmaAccess;
-typedef void (*FWCfgReadCallback)(void *opaque);
+typedef void (*FWCfgCallback)(void *opaque);
struct FWCfgState {
/*< private >*/
@@ -182,7 +182,7 @@ void fw_cfg_add_file(FWCfgState *s, const char *filename, void *data,
* fw_cfg_add_file_callback:
* @s: fw_cfg device being modified
* @filename: name of new fw_cfg file item
- * @callback: callback function
+ * @select_cb: callback function when selecting
* @callback_opaque: argument to be passed into callback function
* @data: pointer to start of item data
* @len: size of item data
@@ -201,7 +201,8 @@ void fw_cfg_add_file(FWCfgState *s, const char *filename, void *data,
* with FW_CFG_DMA_CTL_SELECT).
*/
void fw_cfg_add_file_callback(FWCfgState *s, const char *filename,
- FWCfgReadCallback callback, void *callback_opaque,
+ FWCfgCallback select_cb,
+ void *callback_opaque,
void *data, size_t len, bool read_only);
/**
diff --git a/hw/core/loader.c b/hw/core/loader.c
index ebe574c7ea..4593061445 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -989,7 +989,7 @@ 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,
+ FWCfgCallback fw_callback, void *callback_opaque,
AddressSpace *as, bool read_only)
{
MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 5bd904487f..e3bd626b8c 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -55,7 +55,7 @@ struct FWCfgEntry {
bool allow_write;
uint8_t *data;
void *callback_opaque;
- FWCfgReadCallback read_callback;
+ FWCfgCallback select_cb;
};
#define JPG_FILE 0
@@ -236,8 +236,8 @@ static int fw_cfg_select(FWCfgState *s, uint16_t key)
/* entry successfully selected, now run callback if present */
arch = !!(key & FW_CFG_ARCH_LOCAL);
e = &s->entries[arch][key & FW_CFG_ENTRY_MASK];
- if (e->read_callback) {
- e->read_callback(e->callback_opaque);
+ if (e->select_cb) {
+ e->select_cb(e->callback_opaque);
}
}
@@ -568,11 +568,11 @@ static const VMStateDescription vmstate_fw_cfg = {
}
};
-static void fw_cfg_add_bytes_read_callback(FWCfgState *s, uint16_t key,
- FWCfgReadCallback callback,
- void *callback_opaque,
- void *data, size_t len,
- bool read_only)
+static void fw_cfg_add_bytes_callback(FWCfgState *s, uint16_t key,
+ FWCfgCallback select_cb,
+ void *callback_opaque,
+ void *data, size_t len,
+ bool read_only)
{
int arch = !!(key & FW_CFG_ARCH_LOCAL);
@@ -583,7 +583,7 @@ static void fw_cfg_add_bytes_read_callback(FWCfgState *s, uint16_t key,
s->entries[arch][key].data = data;
s->entries[arch][key].len = (uint32_t)len;
- s->entries[arch][key].read_callback = callback;
+ s->entries[arch][key].select_cb = select_cb;
s->entries[arch][key].callback_opaque = callback_opaque;
s->entries[arch][key].allow_write = !read_only;
}
@@ -610,7 +610,7 @@ static void *fw_cfg_modify_bytes_read(FWCfgState *s, uint16_t key,
void fw_cfg_add_bytes(FWCfgState *s, uint16_t key, void *data, size_t len)
{
- fw_cfg_add_bytes_read_callback(s, key, NULL, NULL, data, len, true);
+ fw_cfg_add_bytes_callback(s, key, NULL, NULL, data, len, true);
}
void fw_cfg_add_string(FWCfgState *s, uint16_t key, const char *value)
@@ -736,7 +736,8 @@ static int get_fw_cfg_order(FWCfgState *s, const char *name)
}
void fw_cfg_add_file_callback(FWCfgState *s, const char *filename,
- FWCfgReadCallback callback, void *callback_opaque,
+ FWCfgCallback select_cb,
+ void *callback_opaque,
void *data, size_t len, bool read_only)
{
int i, index, count;
@@ -798,9 +799,10 @@ void fw_cfg_add_file_callback(FWCfgState *s, const char *filename,
}
}
- fw_cfg_add_bytes_read_callback(s, FW_CFG_FILE_FIRST + index,
- callback, callback_opaque, data, len,
- read_only);
+ fw_cfg_add_bytes_callback(s, FW_CFG_FILE_FIRST + index,
+ select_cb,
+ callback_opaque, data, len,
+ read_only);
s->files->f[index].size = cpu_to_be32(len);
s->files->f[index].select = cpu_to_be16(FW_CFG_FILE_FIRST + index);
--
2.14.0.1.geff633fa0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v5 2/8] fw_cfg: add write callback
2017-08-07 18:16 [Qemu-devel] [PATCH v5 0/8] KASLR kernel dump support Marc-André Lureau
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 1/8] fw_cfg: rename read callback Marc-André Lureau
@ 2017-08-07 18:16 ` Marc-André Lureau
2017-09-08 12:40 ` Michael S. Tsirkin
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 3/8] fw_cfg: add vmcoreinfo file Marc-André Lureau
` (7 subsequent siblings)
9 siblings, 1 reply; 19+ messages in thread
From: Marc-André Lureau @ 2017-08-07 18:16 UTC (permalink / raw)
To: qemu-devel
Cc: imammedo, berrange, ehabkost, mst, anderson, lersek,
Marc-André Lureau, Ben Warren, Paolo Bonzini,
Richard Henderson
Reintroduce the write callback that was removed when write support was
removed in commit 023e3148567ac898c7258138f8e86c3c2bb40d07.
The write_cb callback is called when a write reaches the end of file,
that is when the write pointer/offset reaches the size of the file.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
include/hw/nvram/fw_cfg.h | 2 ++
hw/acpi/vmgenid.c | 2 +-
hw/core/loader.c | 2 +-
hw/i386/acpi-build.c | 2 +-
hw/isa/lpc_ich9.c | 4 ++--
hw/nvram/fw_cfg.c | 18 ++++++++++++++----
6 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index f50d068563..3527cd51d8 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -183,6 +183,7 @@ void fw_cfg_add_file(FWCfgState *s, const char *filename, void *data,
* @s: fw_cfg device being modified
* @filename: name of new fw_cfg file item
* @select_cb: callback function when selecting
+ * @write_cb: callback function when write reached EOF
* @callback_opaque: argument to be passed into callback function
* @data: pointer to start of item data
* @len: size of item data
@@ -202,6 +203,7 @@ void fw_cfg_add_file(FWCfgState *s, const char *filename, void *data,
*/
void fw_cfg_add_file_callback(FWCfgState *s, const char *filename,
FWCfgCallback select_cb,
+ FWCfgCallback write_cb,
void *callback_opaque,
void *data, size_t len, bool read_only);
diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
index a32b847fe0..c1e935da9f 100644
--- a/hw/acpi/vmgenid.c
+++ b/hw/acpi/vmgenid.c
@@ -124,7 +124,7 @@ void vmgenid_add_fw_cfg(VmGenIdState *vms, FWCfgState *s, GArray *guid)
fw_cfg_add_file(s, VMGENID_GUID_FW_CFG_FILE, guid->data,
VMGENID_FW_CFG_SIZE);
/* Create a read-write fw_cfg file for Address */
- fw_cfg_add_file_callback(s, VMGENID_ADDR_FW_CFG_FILE, NULL, NULL,
+ fw_cfg_add_file_callback(s, VMGENID_ADDR_FW_CFG_FILE, NULL, NULL, NULL,
vms->vmgenid_addr_le,
ARRAY_SIZE(vms->vmgenid_addr_le), false);
}
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 4593061445..91669d65aa 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1023,7 +1023,7 @@ MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len,
}
fw_cfg_add_file_callback(fw_cfg, fw_file_name,
- fw_callback, callback_opaque,
+ fw_callback, NULL, callback_opaque,
data, rom->datasize, read_only);
}
return mr;
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b9c245c9bb..be2992b708 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2922,7 +2922,7 @@ void acpi_setup(void)
build_state->rsdp = g_memdup(tables.rsdp->data, rsdp_size);
fw_cfg_add_file_callback(pcms->fw_cfg, ACPI_BUILD_RSDP_FILE,
- acpi_build_update, build_state,
+ acpi_build_update, NULL, build_state,
build_state->rsdp, rsdp_size, true);
build_state->rsdp_mr = NULL;
} else {
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index ac8416d42b..de8fbb7260 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -402,12 +402,12 @@ void ich9_lpc_pm_init(PCIDevice *lpc_pci, bool smm_enabled)
* just link them into fw_cfg here.
*/
fw_cfg_add_file_callback(fw_cfg, "etc/smi/requested-features",
- NULL, NULL,
+ NULL, NULL, NULL,
lpc->smi_guest_features_le,
sizeof lpc->smi_guest_features_le,
false);
fw_cfg_add_file_callback(fw_cfg, "etc/smi/features-ok",
- smi_features_ok_callback, lpc,
+ smi_features_ok_callback, NULL, lpc,
&lpc->smi_features_ok,
sizeof lpc->smi_features_ok,
true);
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index e3bd626b8c..28780088b9 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -56,6 +56,7 @@ struct FWCfgEntry {
uint8_t *data;
void *callback_opaque;
FWCfgCallback select_cb;
+ FWCfgCallback write_cb;
};
#define JPG_FILE 0
@@ -384,6 +385,12 @@ static void fw_cfg_dma_transfer(FWCfgState *s)
stl_be_dma(s->dma_as, dma_addr + offsetof(FWCfgDmaAccess, control),
dma.control);
+ if (write &&
+ !(dma.control & FW_CFG_DMA_CTL_ERROR) &&
+ s->cur_offset == e->len) {
+ e->write_cb(e->callback_opaque);
+ }
+
trace_fw_cfg_read(s, 0);
}
@@ -570,6 +577,7 @@ static const VMStateDescription vmstate_fw_cfg = {
static void fw_cfg_add_bytes_callback(FWCfgState *s, uint16_t key,
FWCfgCallback select_cb,
+ FWCfgCallback write_cb,
void *callback_opaque,
void *data, size_t len,
bool read_only)
@@ -584,6 +592,7 @@ static void fw_cfg_add_bytes_callback(FWCfgState *s, uint16_t key,
s->entries[arch][key].data = data;
s->entries[arch][key].len = (uint32_t)len;
s->entries[arch][key].select_cb = select_cb;
+ s->entries[arch][key].write_cb = write_cb;
s->entries[arch][key].callback_opaque = callback_opaque;
s->entries[arch][key].allow_write = !read_only;
}
@@ -610,7 +619,7 @@ static void *fw_cfg_modify_bytes_read(FWCfgState *s, uint16_t key,
void fw_cfg_add_bytes(FWCfgState *s, uint16_t key, void *data, size_t len)
{
- fw_cfg_add_bytes_callback(s, key, NULL, NULL, data, len, true);
+ fw_cfg_add_bytes_callback(s, key, NULL, NULL, NULL, data, len, true);
}
void fw_cfg_add_string(FWCfgState *s, uint16_t key, const char *value)
@@ -737,6 +746,7 @@ static int get_fw_cfg_order(FWCfgState *s, const char *name)
void fw_cfg_add_file_callback(FWCfgState *s, const char *filename,
FWCfgCallback select_cb,
+ FWCfgCallback write_cb,
void *callback_opaque,
void *data, size_t len, bool read_only)
{
@@ -800,7 +810,7 @@ void fw_cfg_add_file_callback(FWCfgState *s, const char *filename,
}
fw_cfg_add_bytes_callback(s, FW_CFG_FILE_FIRST + index,
- select_cb,
+ select_cb, write_cb,
callback_opaque, data, len,
read_only);
@@ -815,7 +825,7 @@ void fw_cfg_add_file_callback(FWCfgState *s, const char *filename,
void fw_cfg_add_file(FWCfgState *s, const char *filename,
void *data, size_t len)
{
- fw_cfg_add_file_callback(s, filename, NULL, NULL, data, len, true);
+ fw_cfg_add_file_callback(s, filename, NULL, NULL, NULL, data, len, true);
}
void *fw_cfg_modify_file(FWCfgState *s, const char *filename,
@@ -838,7 +848,7 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename,
}
}
/* add new one */
- fw_cfg_add_file_callback(s, filename, NULL, NULL, data, len, true);
+ fw_cfg_add_file_callback(s, filename, NULL, NULL, NULL, data, len, true);
return NULL;
}
--
2.14.0.1.geff633fa0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v5 3/8] fw_cfg: add vmcoreinfo file
2017-08-07 18:16 [Qemu-devel] [PATCH v5 0/8] KASLR kernel dump support Marc-André Lureau
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 1/8] fw_cfg: rename read callback Marc-André Lureau
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 2/8] fw_cfg: add write callback Marc-André Lureau
@ 2017-08-07 18:16 ` Marc-André Lureau
2017-09-08 12:32 ` Michael S. Tsirkin
` (3 more replies)
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 4/8] dump: add guest ELF note Marc-André Lureau
` (6 subsequent siblings)
9 siblings, 4 replies; 19+ messages in thread
From: Marc-André Lureau @ 2017-08-07 18:16 UTC (permalink / raw)
To: qemu-devel
Cc: imammedo, berrange, ehabkost, mst, anderson, lersek,
Marc-André Lureau
See docs/specs/fw_cfg.txt for details.
The "etc/vmcoreinfo" is added when using "-global
fw_cfg.vmcoreinfo=on" qemu option.
Disabled by default for machine types v2.9 and older.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
include/hw/compat.h | 8 ++++++++
include/hw/nvram/fw_cfg.h | 9 +++++++++
hw/nvram/fw_cfg.c | 20 ++++++++++++++++++++
docs/specs/fw_cfg.txt | 16 ++++++++++++++++
4 files changed, 53 insertions(+)
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 08f36004da..317fd2e2e3 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -18,6 +18,14 @@
.driver = "pcie-root-port",\
.property = "x-migrate-msix",\
.value = "false",\
+ },{\
+ .driver = "fw_cfg_mem",\
+ .property = "vmcoreinfo",\
+ .value = "off",\
+ },{\
+ .driver = "fw_cfg_io",\
+ .property = "vmcoreinfo",\
+ .value = "off",\
},
#define HW_COMPAT_2_8 \
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index 3527cd51d8..a35f47405d 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -30,6 +30,11 @@ typedef struct FWCfgFile {
void fw_cfg_set_order_override(FWCfgState *fw_cfg, int order);
void fw_cfg_reset_order_override(FWCfgState *fw_cfg);
+typedef struct FWCfgVMCoreInfo {
+ uint64_t paddr;
+ uint32_t size;
+} QEMU_PACKED FWCfgVMCoreInfo;
+
typedef struct FWCfgFiles {
uint32_t count;
FWCfgFile f[];
@@ -65,6 +70,10 @@ struct FWCfgState {
dma_addr_t dma_addr;
AddressSpace *dma_as;
MemoryRegion dma_iomem;
+
+ bool vmcoreinfo_enabled;
+ bool has_vmcoreinfo;
+ FWCfgVMCoreInfo vmcoreinfo;
};
struct FWCfgIoState {
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 28780088b9..342afc4ed2 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -504,6 +504,7 @@ static void fw_cfg_reset(DeviceState *d)
/* we never register a read callback for FW_CFG_SIGNATURE */
fw_cfg_select(s, FW_CFG_SIGNATURE);
+ s->has_vmcoreinfo = false;
}
/* Save restore 32 bit int as uint16_t
@@ -869,7 +870,12 @@ static void fw_cfg_machine_ready(struct Notifier *n, void *data)
qemu_register_reset(fw_cfg_machine_reset, s);
}
+static void fw_cfg_vmci_written(void *dev)
+{
+ FWCfgState *s = FW_CFG(dev);
+ s->has_vmcoreinfo = true;
+}
static void fw_cfg_common_realize(DeviceState *dev, Error **errp)
{
@@ -895,6 +901,16 @@ static void fw_cfg_common_realize(DeviceState *dev, Error **errp)
fw_cfg_add_i32(s, FW_CFG_ID, version);
+ if (s->vmcoreinfo_enabled) {
+ if (!s->dma_enabled) {
+ error_setg(errp, "vmcoreinfo requires dma_enabled");
+ return;
+ }
+ fw_cfg_add_file_callback(s, "etc/vmcoreinfo",
+ NULL, fw_cfg_vmci_written, s,
+ &s->vmcoreinfo, sizeof(s->vmcoreinfo), false);
+ }
+
s->machine_ready.notify = fw_cfg_machine_ready;
qemu_add_machine_init_done_notifier(&s->machine_ready);
}
@@ -1031,6 +1047,8 @@ static void fw_cfg_file_slots_allocate(FWCfgState *s, Error **errp)
static Property fw_cfg_io_properties[] = {
DEFINE_PROP_BOOL("dma_enabled", FWCfgIoState, parent_obj.dma_enabled,
true),
+ DEFINE_PROP_BOOL("vmcoreinfo", FWCfgIoState, parent_obj.vmcoreinfo_enabled,
+ true),
DEFINE_PROP_UINT16("x-file-slots", FWCfgIoState, parent_obj.file_slots,
FW_CFG_FILE_SLOTS_DFLT),
DEFINE_PROP_END_OF_LIST(),
@@ -1082,6 +1100,8 @@ static Property fw_cfg_mem_properties[] = {
DEFINE_PROP_UINT32("data_width", FWCfgMemState, data_width, -1),
DEFINE_PROP_BOOL("dma_enabled", FWCfgMemState, parent_obj.dma_enabled,
true),
+ DEFINE_PROP_BOOL("vmcoreinfo", FWCfgMemState, parent_obj.vmcoreinfo_enabled,
+ true),
DEFINE_PROP_UINT16("x-file-slots", FWCfgMemState, parent_obj.file_slots,
FW_CFG_FILE_SLOTS_DFLT),
DEFINE_PROP_END_OF_LIST(),
diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
index 08c00bdf44..37d0f9f40a 100644
--- a/docs/specs/fw_cfg.txt
+++ b/docs/specs/fw_cfg.txt
@@ -136,6 +136,22 @@ struct FWCfgFile { /* an individual file entry, 64 bytes total */
char name[56]; /* fw_cfg item name, NUL-terminated ascii */
};
+=== etc/vmcoreinfo ===
+
+A guest may use this entry to add information details to qemu
+dumps. The entry gives location and size of an ELF note that is
+appended in qemu dumps.
+
+The entry is of 12 bytes with this format:
+
+struct FWCfgVMCoreInfo {
+ uint64_t paddr; /* physical address of ELF note, LE */
+ uint32_t size; /* size of ELF note region, LE */
+};
+
+The note format/class must be of the target bitness and the size must
+be less than 1Mb.
+
=== All Other Data Items ===
Please consult the QEMU source for the most up-to-date and authoritative list
--
2.14.0.1.geff633fa0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v5 4/8] dump: add guest ELF note
2017-08-07 18:16 [Qemu-devel] [PATCH v5 0/8] KASLR kernel dump support Marc-André Lureau
` (2 preceding siblings ...)
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 3/8] fw_cfg: add vmcoreinfo file Marc-André Lureau
@ 2017-08-07 18:16 ` Marc-André Lureau
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 5/8] dump: update phys_base header field based on VMCOREINFO content Marc-André Lureau
` (5 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Marc-André Lureau @ 2017-08-07 18:16 UTC (permalink / raw)
To: qemu-devel
Cc: imammedo, berrange, ehabkost, mst, anderson, lersek,
Marc-André Lureau
Read the guest ELF PT_NOTE from guest memory when fw_cfg
etc/vmcoreinfo entry provides the location, and write it as an
additional note in the dump.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
include/sysemu/dump.h | 2 +
dump.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 105 insertions(+)
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 2672a15f8b..df43bd0e07 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -192,6 +192,8 @@ typedef struct DumpState {
* this could be used to calculate
* how much work we have
* finished. */
+ uint8_t *guest_note; /* ELF note content */
+ size_t guest_note_size;
} DumpState;
uint16_t cpu_to_dump16(DumpState *s, uint16_t val);
diff --git a/dump.c b/dump.c
index d9090a24cc..7fe1044280 100644
--- a/dump.c
+++ b/dump.c
@@ -26,6 +26,8 @@
#include "qapi/qmp/qerror.h"
#include "qmp-commands.h"
#include "qapi-event.h"
+#include "qemu/error-report.h"
+#include "hw/nvram/fw_cfg.h"
#include <zlib.h>
#ifdef CONFIG_LZO
@@ -38,6 +40,13 @@
#define ELF_MACHINE_UNAME "Unknown"
#endif
+#define MAX_GUEST_NOTE_SIZE (1 << 20) /* 1MB should be enough */
+
+#define ELF_NOTE_SIZE(hdr_size, name_size, desc_size) \
+ ((DIV_ROUND_UP((hdr_size), 4) + \
+ DIV_ROUND_UP((name_size), 4) + \
+ DIV_ROUND_UP((desc_size), 4)) * 4)
+
uint16_t cpu_to_dump16(DumpState *s, uint16_t val)
{
if (s->dump_info.d_endian == ELFDATA2LSB) {
@@ -76,6 +85,8 @@ static int dump_cleanup(DumpState *s)
guest_phys_blocks_free(&s->guest_phys_blocks);
memory_mapping_list_free(&s->list);
close(s->fd);
+ g_free(s->guest_note);
+ s->guest_note = NULL;
if (s->resume) {
if (s->detached) {
qemu_mutex_lock_iothread();
@@ -235,6 +246,19 @@ static inline int cpu_index(CPUState *cpu)
return cpu->cpu_index + 1;
}
+static void write_guest_note(WriteCoreDumpFunction f, DumpState *s,
+ Error **errp)
+{
+ int ret;
+
+ if (s->guest_note) {
+ ret = f(s->guest_note, s->guest_note_size, s);
+ if (ret < 0) {
+ error_setg(errp, "dump: failed to write guest note");
+ }
+ }
+}
+
static void write_elf64_notes(WriteCoreDumpFunction f, DumpState *s,
Error **errp)
{
@@ -258,6 +282,8 @@ static void write_elf64_notes(WriteCoreDumpFunction f, DumpState *s,
return;
}
}
+
+ write_guest_note(f, s, errp);
}
static void write_elf32_note(DumpState *s, Error **errp)
@@ -303,6 +329,8 @@ static void write_elf32_notes(WriteCoreDumpFunction f, DumpState *s,
return;
}
}
+
+ write_guest_note(f, s, errp);
}
static void write_elf_section(DumpState *s, int type, Error **errp)
@@ -714,6 +742,44 @@ static int buf_write_note(const void *buf, size_t size, void *opaque)
return 0;
}
+/*
+ * This function retrieves various sizes from an elf header.
+ *
+ * @note has to be a valid ELF note. The return sizes are unmodified
+ * (not padded or rounded up to be multiple of 4).
+ */
+static void get_note_sizes(DumpState *s, const void *note,
+ uint64_t *note_head_size,
+ uint64_t *name_size,
+ uint64_t *desc_size)
+{
+ uint64_t note_head_sz;
+ uint64_t name_sz;
+ uint64_t desc_sz;
+
+ if (s->dump_info.d_class == ELFCLASS64) {
+ const Elf64_Nhdr *hdr = note;
+ note_head_sz = sizeof(Elf64_Nhdr);
+ name_sz = tswap64(hdr->n_namesz);
+ desc_sz = tswap64(hdr->n_descsz);
+ } else {
+ const Elf32_Nhdr *hdr = note;
+ note_head_sz = sizeof(Elf32_Nhdr);
+ name_sz = tswap32(hdr->n_namesz);
+ desc_sz = tswap32(hdr->n_descsz);
+ }
+
+ if (note_head_size) {
+ *note_head_size = note_head_sz;
+ }
+ if (name_size) {
+ *name_size = name_sz;
+ }
+ if (desc_size) {
+ *desc_size = desc_sz;
+ }
+}
+
/* write common header, sub header and elf note to vmcore */
static void create_header32(DumpState *s, Error **errp)
{
@@ -1492,6 +1558,7 @@ static void dump_init(DumpState *s, int fd, bool has_format,
DumpGuestMemoryFormat format, bool paging, bool has_filter,
int64_t begin, int64_t length, Error **errp)
{
+ FWCfgState *fw_cfg = fw_cfg_find();
CPUState *cpu;
int nr_cpus;
Error *err = NULL;
@@ -1563,6 +1630,42 @@ static void dump_init(DumpState *s, int fd, bool has_format,
goto cleanup;
}
+ /*
+ * The goal of this block is to copy the guest note out of
+ * the guest. Failure to do so is not fatal for dumping.
+ */
+ if (fw_cfg) {
+ uint64_t addr, note_head_size, name_size, desc_size;
+ uint32_t size;
+
+ note_head_size = s->dump_info.d_class == ELFCLASS32 ?
+ sizeof(Elf32_Nhdr) : sizeof(Elf64_Nhdr);
+
+ size = le32_to_cpu(fw_cfg->vmcoreinfo.size);
+ addr = le64_to_cpu(fw_cfg->vmcoreinfo.paddr);
+ if (!fw_cfg->has_vmcoreinfo) {
+ warn_report("guest note is not present");
+ } else if (size < note_head_size || size > MAX_GUEST_NOTE_SIZE) {
+ warn_report("guest note size is invalid: %" PRIu32, size);
+ } else {
+ s->guest_note = g_malloc(size + 1); /* +1 for adding \0 */
+ cpu_physical_memory_read(addr, s->guest_note, size);
+
+ get_note_sizes(s, s->guest_note, NULL, &name_size, &desc_size);
+ s->guest_note_size = ELF_NOTE_SIZE(note_head_size, name_size,
+ desc_size);
+ if (name_size > MAX_GUEST_NOTE_SIZE ||
+ desc_size > MAX_GUEST_NOTE_SIZE ||
+ s->guest_note_size > size) {
+ warn_report("Invalid guest note header");
+ g_free(s->guest_note);
+ s->guest_note = NULL;
+ } else {
+ s->note_size += s->guest_note_size;
+ }
+ }
+ }
+
/* get memory mapping */
if (paging) {
qemu_get_guest_memory_mapping(&s->list, &s->guest_phys_blocks, &err);
--
2.14.0.1.geff633fa0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v5 5/8] dump: update phys_base header field based on VMCOREINFO content
2017-08-07 18:16 [Qemu-devel] [PATCH v5 0/8] KASLR kernel dump support Marc-André Lureau
` (3 preceding siblings ...)
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 4/8] dump: add guest ELF note Marc-André Lureau
@ 2017-08-07 18:16 ` Marc-André Lureau
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 6/8] kdump: set vmcoreinfo location Marc-André Lureau
` (4 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Marc-André Lureau @ 2017-08-07 18:16 UTC (permalink / raw)
To: qemu-devel
Cc: imammedo, berrange, ehabkost, mst, anderson, lersek,
Marc-André Lureau
If the guest note is VMCOREINFO, try to get phys_base from it.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
dump.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++--
docs/specs/fw_cfg.txt | 8 ++++++++
2 files changed, 62 insertions(+), 2 deletions(-)
diff --git a/dump.c b/dump.c
index 7fe1044280..cef2dd5bf9 100644
--- a/dump.c
+++ b/dump.c
@@ -780,6 +780,23 @@ static void get_note_sizes(DumpState *s, const void *note,
}
}
+static bool note_name_equal(DumpState *s,
+ const uint8_t *note, const char *name)
+{
+ int len = strlen(name) + 1;
+ uint64_t head_size, name_size;
+
+ get_note_sizes(s, note, &head_size, &name_size, NULL);
+ head_size = ROUND_UP(head_size, 4);
+
+ if (name_size != len ||
+ memcmp(note + head_size, "VMCOREINFO", len)) {
+ return false;
+ }
+
+ return true;
+}
+
/* write common header, sub header and elf note to vmcore */
static void create_header32(DumpState *s, Error **errp)
{
@@ -1554,6 +1571,39 @@ static int64_t dump_calculate_size(DumpState *s)
return total;
}
+static void vmcoreinfo_update_phys_base(DumpState *s)
+{
+ uint64_t size, note_head_size, name_size, phys_base;
+ char **lines;
+ uint8_t *vmci;
+ size_t i;
+
+ if (!note_name_equal(s, s->guest_note, "VMCOREINFO")) {
+ return;
+ }
+
+ get_note_sizes(s, s->guest_note, ¬e_head_size, &name_size, &size);
+ note_head_size = ROUND_UP(note_head_size, 4);
+
+ vmci = s->guest_note + note_head_size + ROUND_UP(name_size, 4);
+ *(vmci + size) = '\0';
+
+ lines = g_strsplit((char *)vmci, "\n", -1);
+ for (i = 0; lines[i]; i++) {
+ if (g_str_has_prefix(lines[i], "NUMBER(phys_base)=")) {
+ if (qemu_strtou64(lines[i] + 18, NULL, 16,
+ &phys_base) < 0) {
+ warn_report("Failed to read NUMBER(phys_base)=");
+ } else {
+ s->dump_info.phys_base = phys_base;
+ }
+ break;
+ }
+ }
+
+ g_strfreev(lines);
+}
+
static void dump_init(DumpState *s, int fd, bool has_format,
DumpGuestMemoryFormat format, bool paging, bool has_filter,
int64_t begin, int64_t length, Error **errp)
@@ -1631,8 +1681,9 @@ static void dump_init(DumpState *s, int fd, bool has_format,
}
/*
- * The goal of this block is to copy the guest note out of
- * the guest. Failure to do so is not fatal for dumping.
+ * The goal of this block is to (a) update the previously guessed
+ * phys_base, (b) copy the guest note note out of the guest.
+ * Failure to do so is not fatal for dumping.
*/
if (fw_cfg) {
uint64_t addr, note_head_size, name_size, desc_size;
@@ -1661,6 +1712,7 @@ static void dump_init(DumpState *s, int fd, bool has_format,
g_free(s->guest_note);
s->guest_note = NULL;
} else {
+ vmcoreinfo_update_phys_base(s);
s->note_size += s->guest_note_size;
}
}
diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
index 37d0f9f40a..64c6aaed1f 100644
--- a/docs/specs/fw_cfg.txt
+++ b/docs/specs/fw_cfg.txt
@@ -152,6 +152,14 @@ struct FWCfgVMCoreInfo {
The note format/class must be of the target bitness and the size must
be less than 1Mb.
+If the ELF note name is "VMCOREINFO", it is expected to be the Linux
+vmcoreinfo note (see Documentation/ABI/testing/sysfs-kernel-vmcoreinfo
+in Linux source). In this case, qemu dump code will read the content
+as a key=value text file, looking for "NUMBER(phys_base)" key
+value. The value is expected to be more accurate than architecture
+guess of the value. This is useful for KASLR-enabled guest with
+ancient tools not handling the VMCOREINFO note.
+
=== All Other Data Items ===
Please consult the QEMU source for the most up-to-date and authoritative list
--
2.14.0.1.geff633fa0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v5 6/8] kdump: set vmcoreinfo location
2017-08-07 18:16 [Qemu-devel] [PATCH v5 0/8] KASLR kernel dump support Marc-André Lureau
` (4 preceding siblings ...)
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 5/8] dump: update phys_base header field based on VMCOREINFO content Marc-André Lureau
@ 2017-08-07 18:16 ` Marc-André Lureau
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 7/8] scripts/dump-guest-memory.py: add vmcoreinfo Marc-André Lureau
` (3 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Marc-André Lureau @ 2017-08-07 18:16 UTC (permalink / raw)
To: qemu-devel
Cc: imammedo, berrange, ehabkost, mst, anderson, lersek,
Marc-André Lureau
kdump header provides offset and size of the vmcoreinfo content,
append it if available (skip the ELF note header).
crash-7.1.9 was the first version that started looking in the
vmcoreinfo data for phys_base instead of in the kdump_sub_header.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
dump.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/dump.c b/dump.c
index cef2dd5bf9..49805a3cdc 100644
--- a/dump.c
+++ b/dump.c
@@ -858,6 +858,18 @@ static void create_header32(DumpState *s, Error **errp)
kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
+ if (s->guest_note &&
+ note_name_equal(s, s->guest_note, "VMCOREINFO")) {
+ uint64_t hsize, name_size, size_vmcoreinfo_desc, offset_vmcoreinfo;
+
+ get_note_sizes(s, s->guest_note,
+ &hsize, &name_size, &size_vmcoreinfo_desc);
+ offset_vmcoreinfo = offset_note + s->note_size - s->guest_note_size +
+ (DIV_ROUND_UP(hsize, 4) + DIV_ROUND_UP(name_size, 4)) * 4;
+ kh->offset_vmcoreinfo = cpu_to_dump64(s, offset_vmcoreinfo);
+ kh->size_vmcoreinfo = cpu_to_dump32(s, size_vmcoreinfo_desc);
+ }
+
kh->offset_note = cpu_to_dump64(s, offset_note);
kh->note_size = cpu_to_dump32(s, s->note_size);
@@ -958,6 +970,18 @@ static void create_header64(DumpState *s, Error **errp)
kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
+ if (s->guest_note &&
+ note_name_equal(s, s->guest_note, "VMCOREINFO")) {
+ uint64_t hsize, name_size, size_vmcoreinfo_desc, offset_vmcoreinfo;
+
+ get_note_sizes(s, s->guest_note,
+ &hsize, &name_size, &size_vmcoreinfo_desc);
+ offset_vmcoreinfo = offset_note + s->note_size - s->guest_note_size +
+ (DIV_ROUND_UP(hsize, 4) + DIV_ROUND_UP(name_size, 4)) * 4;
+ kh->offset_vmcoreinfo = cpu_to_dump64(s, offset_vmcoreinfo);
+ kh->size_vmcoreinfo = cpu_to_dump64(s, size_vmcoreinfo_desc);
+ }
+
kh->offset_note = cpu_to_dump64(s, offset_note);
kh->note_size = cpu_to_dump64(s, s->note_size);
--
2.14.0.1.geff633fa0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v5 7/8] scripts/dump-guest-memory.py: add vmcoreinfo
2017-08-07 18:16 [Qemu-devel] [PATCH v5 0/8] KASLR kernel dump support Marc-André Lureau
` (5 preceding siblings ...)
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 6/8] kdump: set vmcoreinfo location Marc-André Lureau
@ 2017-08-07 18:16 ` Marc-André Lureau
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 8/8] MAINTAINERS: add Dump maintainers Marc-André Lureau
` (2 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Marc-André Lureau @ 2017-08-07 18:16 UTC (permalink / raw)
To: qemu-devel
Cc: imammedo, berrange, ehabkost, mst, anderson, lersek,
Marc-André Lureau
Add vmcoreinfo ELF note if fw-cfg has the memory location details.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
scripts/dump-guest-memory.py | 51 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
index f7c6635f15..1f97373f0d 100644
--- a/scripts/dump-guest-memory.py
+++ b/scripts/dump-guest-memory.py
@@ -14,6 +14,7 @@ the COPYING file in the top-level directory.
"""
import ctypes
+import struct
UINTPTR_T = gdb.lookup_type("uintptr_t")
@@ -45,6 +46,12 @@ EM_S390 = 22
EM_AARCH = 183
EM_X86_64 = 62
+def le32_to_cpu(val):
+ return struct.unpack("<I", struct.pack("=I", val))[0]
+
+def le64_to_cpu(val):
+ return struct.unpack("<Q", struct.pack("=Q", val))[0]
+
class ELF(object):
"""Representation of a ELF file."""
@@ -120,6 +127,25 @@ class ELF(object):
self.segments[0].p_filesz += ctypes.sizeof(note)
self.segments[0].p_memsz += ctypes.sizeof(note)
+
+ def add_vmcoreinfo_note(self, vmcoreinfo):
+ """Adds a vmcoreinfo note to the ELF dump."""
+ # compute the header size, and copy that many bytes from the note
+ header = get_arch_note(self.endianness, 0, 0)
+ ctypes.memmove(ctypes.pointer(header),
+ vmcoreinfo, ctypes.sizeof(header))
+ if header.n_descsz > 1 << 20:
+ print('warning: invalid vmcoreinfo size')
+ return
+ # now get the full note
+ note = get_arch_note(self.endianness,
+ header.n_namesz - 1, header.n_descsz)
+ ctypes.memmove(ctypes.pointer(note), vmcoreinfo, ctypes.sizeof(note))
+
+ self.notes.append(note)
+ self.segments[0].p_filesz += ctypes.sizeof(note)
+ self.segments[0].p_memsz += ctypes.sizeof(note)
+
def add_segment(self, p_type, p_paddr, p_size):
"""Adds a segment to the elf."""
@@ -505,6 +531,30 @@ shape and this command should mostly work."""
cur += chunk_size
left -= chunk_size
+ def phys_memory_read(self, addr, size):
+ qemu_core = gdb.inferiors()[0]
+ for block in self.guest_phys_blocks:
+ if block["target_start"] <= addr \
+ and addr + size <= block["target_end"]:
+ haddr = block["host_addr"] + (addr - block["target_start"])
+ return qemu_core.read_memory(haddr, size)
+ return None
+
+ def add_vmcoreinfo(self):
+ if not gdb.parse_and_eval("fw_cfg_find()") \
+ or not gdb.parse_and_eval("fw_cfg_find()->have_vmcoreinfo"):
+ return
+
+ addr = gdb.parse_and_eval("fw_cfg_find()->vmcoreinfo.paddr")
+ size = gdb.parse_and_eval("fw_cfg_find()->vmcoreinfo.size")
+
+ addr = le64_to_cpu(addr)
+ size = le32_to_cpu(size)
+
+ vmcoreinfo = self.phys_memory_read(addr, size)
+ if vmcoreinfo:
+ self.elf.add_vmcoreinfo_note(vmcoreinfo.tobytes())
+
def invoke(self, args, from_tty):
"""Handles command invocation from gdb."""
@@ -518,6 +568,7 @@ shape and this command should mostly work."""
self.elf = ELF(argv[1])
self.guest_phys_blocks = get_guest_phys_blocks()
+ self.add_vmcoreinfo()
with open(argv[0], "wb") as vmcore:
self.dump_init(vmcore)
--
2.14.0.1.geff633fa0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v5 8/8] MAINTAINERS: add Dump maintainers
2017-08-07 18:16 [Qemu-devel] [PATCH v5 0/8] KASLR kernel dump support Marc-André Lureau
` (6 preceding siblings ...)
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 7/8] scripts/dump-guest-memory.py: add vmcoreinfo Marc-André Lureau
@ 2017-08-07 18:16 ` Marc-André Lureau
2017-08-16 20:15 ` [Qemu-devel] [PATCH v5 0/8] KASLR kernel dump support Michael S. Tsirkin
2017-09-08 12:46 ` Michael S. Tsirkin
9 siblings, 0 replies; 19+ messages in thread
From: Marc-André Lureau @ 2017-08-07 18:16 UTC (permalink / raw)
To: qemu-devel
Cc: imammedo, berrange, ehabkost, mst, anderson, lersek,
Marc-André Lureau
Proposing myself, since I have some familiarity with the code now.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
---
MAINTAINERS | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index ccee28b12d..ed07334ffb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1286,6 +1286,15 @@ S: Maintained
F: device_tree.c
F: include/sysemu/device_tree.h
+Dump
+S: Supported
+M: Marc-André Lureau <marcandre.lureau@redhat.com>
+F: dump.c
+F: stubs/dump.c
+F: include/sysemu/dump.h
+F: include/sysemu/dump-arch.h
+F: scripts/dump-guest-memory.py
+
Error reporting
M: Markus Armbruster <armbru@redhat.com>
S: Supported
--
2.14.0.1.geff633fa0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5 0/8] KASLR kernel dump support
2017-08-07 18:16 [Qemu-devel] [PATCH v5 0/8] KASLR kernel dump support Marc-André Lureau
` (7 preceding siblings ...)
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 8/8] MAINTAINERS: add Dump maintainers Marc-André Lureau
@ 2017-08-16 20:15 ` Michael S. Tsirkin
2017-09-08 12:46 ` Michael S. Tsirkin
9 siblings, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2017-08-16 20:15 UTC (permalink / raw)
To: Marc-André Lureau
Cc: qemu-devel, imammedo, berrange, ehabkost, anderson, lersek
On Mon, Aug 07, 2017 at 08:16:10PM +0200, Marc-André Lureau wrote:
> Recent linux kernels enable KASLR to randomize phys/virt memory
> addresses. This series aims to provide enough information in qemu
> dumps so that crash utility can work with randomized kernel too (it
> hasn't been tested on other archs than x86 though, help welcome).
>
> The previous design to provide qemu with debug details (using qemu-ga
> and a dedicated vmcoreinfo ACPI device) failed to satisfy the
> requirements during previous iterations.
>
> In particular, the previous proposed vmcoreinfo ACPI device had the
> following issues:
> - hazardous memory handling with no explicit synchronization
> - occupy 2 fw-cfg entries (for memory and pointer)
> - occupy 4k of guest memory (this could have been tweaked)
> - did not provide ACPI methods (this could have been added)
> - may be difficult to maintain compatibility (according to Michael)
>
> This is a new proposal, that leverage fw-cfg device instead of adding
> a new device. A "etc/vmcoreinfo" entry is added, where the guest,
> during boot or later, can write the addr/size location of an ELF note
> to be appended in the qemu dump.
Looks ok to me. Pls remember to ping after release.
> Note: only guest kernel is expected to write to a fw-cfg entry. This
> method is not meant for general qemu/user-space communication. There
> are more appropriate devices for this purpose, and the guest kernel
> should not expose this facility.
>
> This is quite easier to implement, and uses less of the limited fw-cfg
> slots, and guest memory. It also solves the synchronization issue, and
> may be easier to discover or to maintain compatibility.
>
> The Linux ELF note is expected to be the VMCOREINFO note, which will
> have a special handling in qemu in this case helping kaslr-kernel
> debugging. But it could be any valid ELF note.
>
> Crash 7.1.9 will parse the "phys_base" value from the VMCOREINFO note,
> and thus will work with KASLR-dump produced by this series.
>
> The series implements the note addition in qemu ELF/kdump,
> as well as the python scripts/dump-guest-memory.py.
>
> To test:
>
> Using kernel from https://github.com/elmarco/linux fw-cfg branch,
> Compile and run guest kernel with CONFIG_RANDOMIZE_BASE=y & CONFIG_FW_CFG_SYSFS=y.
>
> Run qemu with -global fw_cfg.vmcoreinfo=on
>
> Produce an ELF dump:
> { "execute": "dump-guest-memory", "arguments": { "protocol": "file:dump", "paging": false } }
>
> Produce a kdump:
> { "execute": "dump-guest-memory", "arguments": { "protocol": "file:dump", "paging": false, "format": "kdump-zlib" } }
>
> Or with (gdb) dump-guest-memory, with scripts/dump-guest-memory.py script.
>
> Analyze with crash >= 7.1.9 (or the git version for 4.13 fixes..):
>
> $ crash vmlinux dump
>
> v5:
> - removed x-write-pointer-available patch from this series
> - drop vmcoreinfo device
> - add write callback to fw_cfg entries
> - add a writable fw_cfg "vmcoreinfo" entry
> - split phys_base update from VMCOREINFO note in a seperate patch
> - most patches had non-trivial changes, dropping reviewed-by tags
>
> v4: from Laszlo review
> - switch to warn_report*()
> - update test to follow vmgenid and use boot-sector infrastructure
> - fix range checks in the python script
> - add vmcoreinfo_get() stub
>
> v3: from Laszlo review
> - change vmcoreinfo offset to 36
> - reset err to null after report
> - use PRIu32
> - change name_size and desc_size against MAX_VMCOREINFO_SIZE
> - python code simplification
> - check boundaries of blocks in phys_memory_read()
> - fix some vmgi vs vmci names
> - add more comments in code
> - fix comment indentation
> - add r-b tags
>
> v2: from Laszlo review
> - vmci: fix guest endianess handling
> - vmci: fix wrong sizeof()
> - vmci: add back reset logic from vmgenid
> - dump: have 1MB size limit for vmcoreinfo
> - dump: fix potential off-by-1 buffer manipulation
> - dump: use temporary variable for qemu_strtou64
> - dump: fixed VMCOREINFO duplication in kdump
> - update gdb script to not call into qemu process
> - update MAINTAINERS with some new files
>
> Marc-André Lureau (8):
> fw_cfg: rename read callback
> fw_cfg: add write callback
> fw_cfg: add vmcoreinfo file
> dump: add guest ELF note
> dump: update phys_base header field based on VMCOREINFO content
> kdump: set vmcoreinfo location
> scripts/dump-guest-memory.py: add vmcoreinfo
> MAINTAINERS: add Dump maintainers
>
> scripts/dump-guest-memory.py | 51 ++++++++++++
> include/hw/compat.h | 8 ++
> include/hw/loader.h | 2 +-
> include/hw/nvram/fw_cfg.h | 18 ++++-
> include/sysemu/dump.h | 2 +
> dump.c | 179 +++++++++++++++++++++++++++++++++++++++++++
> hw/acpi/vmgenid.c | 2 +-
> hw/core/loader.c | 4 +-
> hw/i386/acpi-build.c | 2 +-
> hw/isa/lpc_ich9.c | 4 +-
> hw/nvram/fw_cfg.c | 64 ++++++++++++----
> MAINTAINERS | 9 +++
> docs/specs/fw_cfg.txt | 24 ++++++
> 13 files changed, 343 insertions(+), 26 deletions(-)
>
> --
> 2.14.0.1.geff633fa0
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/8] fw_cfg: add vmcoreinfo file
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 3/8] fw_cfg: add vmcoreinfo file Marc-André Lureau
@ 2017-09-08 12:32 ` Michael S. Tsirkin
2017-09-08 12:36 ` Michael S. Tsirkin
` (2 subsequent siblings)
3 siblings, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2017-09-08 12:32 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel, ehabkost, anderson, imammedo, lersek
On Mon, Aug 07, 2017 at 08:16:13PM +0200, Marc-André Lureau wrote:
> See docs/specs/fw_cfg.txt for details.
>
> The "etc/vmcoreinfo" is added when using "-global
> fw_cfg.vmcoreinfo=on" qemu option.
>
> Disabled by default for machine types v2.9 and older.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
I don't like it that it's part of the fw cfg device.
Could we make this off by default and only add this with
some -device?
Thanks,
MST
> ---
> include/hw/compat.h | 8 ++++++++
> include/hw/nvram/fw_cfg.h | 9 +++++++++
> hw/nvram/fw_cfg.c | 20 ++++++++++++++++++++
> docs/specs/fw_cfg.txt | 16 ++++++++++++++++
> 4 files changed, 53 insertions(+)
>
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 08f36004da..317fd2e2e3 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -18,6 +18,14 @@
> .driver = "pcie-root-port",\
> .property = "x-migrate-msix",\
> .value = "false",\
> + },{\
> + .driver = "fw_cfg_mem",\
> + .property = "vmcoreinfo",\
> + .value = "off",\
> + },{\
> + .driver = "fw_cfg_io",\
> + .property = "vmcoreinfo",\
> + .value = "off",\
> },
>
> #define HW_COMPAT_2_8 \
> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
> index 3527cd51d8..a35f47405d 100644
> --- a/include/hw/nvram/fw_cfg.h
> +++ b/include/hw/nvram/fw_cfg.h
> @@ -30,6 +30,11 @@ typedef struct FWCfgFile {
> void fw_cfg_set_order_override(FWCfgState *fw_cfg, int order);
> void fw_cfg_reset_order_override(FWCfgState *fw_cfg);
>
> +typedef struct FWCfgVMCoreInfo {
> + uint64_t paddr;
> + uint32_t size;
> +} QEMU_PACKED FWCfgVMCoreInfo;
> +
> typedef struct FWCfgFiles {
> uint32_t count;
> FWCfgFile f[];
> @@ -65,6 +70,10 @@ struct FWCfgState {
> dma_addr_t dma_addr;
> AddressSpace *dma_as;
> MemoryRegion dma_iomem;
> +
> + bool vmcoreinfo_enabled;
> + bool has_vmcoreinfo;
> + FWCfgVMCoreInfo vmcoreinfo;
> };
>
> struct FWCfgIoState {
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 28780088b9..342afc4ed2 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -504,6 +504,7 @@ static void fw_cfg_reset(DeviceState *d)
>
> /* we never register a read callback for FW_CFG_SIGNATURE */
> fw_cfg_select(s, FW_CFG_SIGNATURE);
> + s->has_vmcoreinfo = false;
> }
>
> /* Save restore 32 bit int as uint16_t
> @@ -869,7 +870,12 @@ static void fw_cfg_machine_ready(struct Notifier *n, void *data)
> qemu_register_reset(fw_cfg_machine_reset, s);
> }
>
> +static void fw_cfg_vmci_written(void *dev)
> +{
> + FWCfgState *s = FW_CFG(dev);
>
> + s->has_vmcoreinfo = true;
> +}
>
> static void fw_cfg_common_realize(DeviceState *dev, Error **errp)
> {
> @@ -895,6 +901,16 @@ static void fw_cfg_common_realize(DeviceState *dev, Error **errp)
>
> fw_cfg_add_i32(s, FW_CFG_ID, version);
>
> + if (s->vmcoreinfo_enabled) {
> + if (!s->dma_enabled) {
> + error_setg(errp, "vmcoreinfo requires dma_enabled");
> + return;
> + }
> + fw_cfg_add_file_callback(s, "etc/vmcoreinfo",
> + NULL, fw_cfg_vmci_written, s,
> + &s->vmcoreinfo, sizeof(s->vmcoreinfo), false);
> + }
> +
> s->machine_ready.notify = fw_cfg_machine_ready;
> qemu_add_machine_init_done_notifier(&s->machine_ready);
> }
> @@ -1031,6 +1047,8 @@ static void fw_cfg_file_slots_allocate(FWCfgState *s, Error **errp)
> static Property fw_cfg_io_properties[] = {
> DEFINE_PROP_BOOL("dma_enabled", FWCfgIoState, parent_obj.dma_enabled,
> true),
> + DEFINE_PROP_BOOL("vmcoreinfo", FWCfgIoState, parent_obj.vmcoreinfo_enabled,
> + true),
> DEFINE_PROP_UINT16("x-file-slots", FWCfgIoState, parent_obj.file_slots,
> FW_CFG_FILE_SLOTS_DFLT),
> DEFINE_PROP_END_OF_LIST(),
> @@ -1082,6 +1100,8 @@ static Property fw_cfg_mem_properties[] = {
> DEFINE_PROP_UINT32("data_width", FWCfgMemState, data_width, -1),
> DEFINE_PROP_BOOL("dma_enabled", FWCfgMemState, parent_obj.dma_enabled,
> true),
> + DEFINE_PROP_BOOL("vmcoreinfo", FWCfgMemState, parent_obj.vmcoreinfo_enabled,
> + true),
> DEFINE_PROP_UINT16("x-file-slots", FWCfgMemState, parent_obj.file_slots,
> FW_CFG_FILE_SLOTS_DFLT),
> DEFINE_PROP_END_OF_LIST(),
> diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
> index 08c00bdf44..37d0f9f40a 100644
> --- a/docs/specs/fw_cfg.txt
> +++ b/docs/specs/fw_cfg.txt
> @@ -136,6 +136,22 @@ struct FWCfgFile { /* an individual file entry, 64 bytes total */
> char name[56]; /* fw_cfg item name, NUL-terminated ascii */
> };
>
> +=== etc/vmcoreinfo ===
> +
> +A guest may use this entry to add information details to qemu
> +dumps. The entry gives location and size of an ELF note that is
> +appended in qemu dumps.
> +
> +The entry is of 12 bytes with this format:
> +
> +struct FWCfgVMCoreInfo {
> + uint64_t paddr; /* physical address of ELF note, LE */
> + uint32_t size; /* size of ELF note region, LE */
> +};
> +
> +The note format/class must be of the target bitness and the size must
> +be less than 1Mb.
> +
> === All Other Data Items ===
>
> Please consult the QEMU source for the most up-to-date and authoritative list
> --
> 2.14.0.1.geff633fa0
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/8] fw_cfg: add vmcoreinfo file
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 3/8] fw_cfg: add vmcoreinfo file Marc-André Lureau
2017-09-08 12:32 ` Michael S. Tsirkin
@ 2017-09-08 12:36 ` Michael S. Tsirkin
2017-09-08 12:42 ` Michael S. Tsirkin
2017-09-08 15:39 ` Michael S. Tsirkin
3 siblings, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2017-09-08 12:36 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel, ehabkost, anderson, imammedo, lersek
On Mon, Aug 07, 2017 at 08:16:13PM +0200, Marc-André Lureau wrote:
> See docs/specs/fw_cfg.txt for details.
>
> The "etc/vmcoreinfo" is added when using "-global
> fw_cfg.vmcoreinfo=on" qemu option.
>
> Disabled by default for machine types v2.9 and older.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> include/hw/compat.h | 8 ++++++++
> include/hw/nvram/fw_cfg.h | 9 +++++++++
> hw/nvram/fw_cfg.c | 20 ++++++++++++++++++++
> docs/specs/fw_cfg.txt | 16 ++++++++++++++++
> 4 files changed, 53 insertions(+)
>
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 08f36004da..317fd2e2e3 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -18,6 +18,14 @@
> .driver = "pcie-root-port",\
> .property = "x-migrate-msix",\
> .value = "false",\
> + },{\
> + .driver = "fw_cfg_mem",\
> + .property = "vmcoreinfo",\
> + .value = "off",\
> + },{\
> + .driver = "fw_cfg_io",\
> + .property = "vmcoreinfo",\
> + .value = "off",\
> },
>
> #define HW_COMPAT_2_8 \
> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
> index 3527cd51d8..a35f47405d 100644
> --- a/include/hw/nvram/fw_cfg.h
> +++ b/include/hw/nvram/fw_cfg.h
> @@ -30,6 +30,11 @@ typedef struct FWCfgFile {
> void fw_cfg_set_order_override(FWCfgState *fw_cfg, int order);
> void fw_cfg_reset_order_override(FWCfgState *fw_cfg);
>
> +typedef struct FWCfgVMCoreInfo {
> + uint64_t paddr;
> + uint32_t size;
> +} QEMU_PACKED FWCfgVMCoreInfo;
> +
> typedef struct FWCfgFiles {
> uint32_t count;
> FWCfgFile f[];
> @@ -65,6 +70,10 @@ struct FWCfgState {
> dma_addr_t dma_addr;
> AddressSpace *dma_as;
> MemoryRegion dma_iomem;
> +
> + bool vmcoreinfo_enabled;
> + bool has_vmcoreinfo;
> + FWCfgVMCoreInfo vmcoreinfo;
> };
>
> struct FWCfgIoState {
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 28780088b9..342afc4ed2 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -504,6 +504,7 @@ static void fw_cfg_reset(DeviceState *d)
>
> /* we never register a read callback for FW_CFG_SIGNATURE */
> fw_cfg_select(s, FW_CFG_SIGNATURE);
> + s->has_vmcoreinfo = false;
> }
>
> /* Save restore 32 bit int as uint16_t
> @@ -869,7 +870,12 @@ static void fw_cfg_machine_ready(struct Notifier *n, void *data)
> qemu_register_reset(fw_cfg_machine_reset, s);
> }
>
> +static void fw_cfg_vmci_written(void *dev)
> +{
> + FWCfgState *s = FW_CFG(dev);
>
> + s->has_vmcoreinfo = true;
> +}
>
> static void fw_cfg_common_realize(DeviceState *dev, Error **errp)
> {
I guess this means there's no value guest can write to clear it
without a reset. That's not good for e.g. kexec.
Let's specify all 0s means disabled too?
> @@ -895,6 +901,16 @@ static void fw_cfg_common_realize(DeviceState *dev, Error **errp)
>
> fw_cfg_add_i32(s, FW_CFG_ID, version);
>
> + if (s->vmcoreinfo_enabled) {
> + if (!s->dma_enabled) {
> + error_setg(errp, "vmcoreinfo requires dma_enabled");
> + return;
> + }
> + fw_cfg_add_file_callback(s, "etc/vmcoreinfo",
> + NULL, fw_cfg_vmci_written, s,
> + &s->vmcoreinfo, sizeof(s->vmcoreinfo), false);
> + }
> +
> s->machine_ready.notify = fw_cfg_machine_ready;
> qemu_add_machine_init_done_notifier(&s->machine_ready);
> }
> @@ -1031,6 +1047,8 @@ static void fw_cfg_file_slots_allocate(FWCfgState *s, Error **errp)
> static Property fw_cfg_io_properties[] = {
> DEFINE_PROP_BOOL("dma_enabled", FWCfgIoState, parent_obj.dma_enabled,
> true),
> + DEFINE_PROP_BOOL("vmcoreinfo", FWCfgIoState, parent_obj.vmcoreinfo_enabled,
> + true),
> DEFINE_PROP_UINT16("x-file-slots", FWCfgIoState, parent_obj.file_slots,
> FW_CFG_FILE_SLOTS_DFLT),
> DEFINE_PROP_END_OF_LIST(),
> @@ -1082,6 +1100,8 @@ static Property fw_cfg_mem_properties[] = {
> DEFINE_PROP_UINT32("data_width", FWCfgMemState, data_width, -1),
> DEFINE_PROP_BOOL("dma_enabled", FWCfgMemState, parent_obj.dma_enabled,
> true),
> + DEFINE_PROP_BOOL("vmcoreinfo", FWCfgMemState, parent_obj.vmcoreinfo_enabled,
> + true),
> DEFINE_PROP_UINT16("x-file-slots", FWCfgMemState, parent_obj.file_slots,
> FW_CFG_FILE_SLOTS_DFLT),
> DEFINE_PROP_END_OF_LIST(),
> diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
> index 08c00bdf44..37d0f9f40a 100644
> --- a/docs/specs/fw_cfg.txt
> +++ b/docs/specs/fw_cfg.txt
> @@ -136,6 +136,22 @@ struct FWCfgFile { /* an individual file entry, 64 bytes total */
> char name[56]; /* fw_cfg item name, NUL-terminated ascii */
> };
>
> +=== etc/vmcoreinfo ===
> +
> +A guest may use this entry to add information details to qemu
> +dumps. The entry gives location and size of an ELF note that is
> +appended in qemu dumps.
> +
> +The entry is of 12 bytes with this format:
> +
> +struct FWCfgVMCoreInfo {
> + uint64_t paddr; /* physical address of ELF note, LE */
> + uint32_t size; /* size of ELF note region, LE */
> +};
> +
> +The note format/class must be of the target bitness
and endian-ness?
> and the size must
> +be less than 1Mb.
> +
> === All Other Data Items ===
>
> Please consult the QEMU source for the most up-to-date and authoritative list
> --
> 2.14.0.1.geff633fa0
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5 2/8] fw_cfg: add write callback
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 2/8] fw_cfg: add write callback Marc-André Lureau
@ 2017-09-08 12:40 ` Michael S. Tsirkin
0 siblings, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2017-09-08 12:40 UTC (permalink / raw)
To: Marc-André Lureau
Cc: qemu-devel, ehabkost, Ben Warren, Paolo Bonzini, anderson,
imammedo, lersek, Richard Henderson
On Mon, Aug 07, 2017 at 08:16:12PM +0200, Marc-André Lureau wrote:
> Reintroduce the write callback that was removed when write support was
> removed in commit 023e3148567ac898c7258138f8e86c3c2bb40d07.
>
> The write_cb callback is called when a write reaches the end of file,
> that is when the write pointer/offset reaches the size of the file.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> include/hw/nvram/fw_cfg.h | 2 ++
> hw/acpi/vmgenid.c | 2 +-
> hw/core/loader.c | 2 +-
> hw/i386/acpi-build.c | 2 +-
> hw/isa/lpc_ich9.c | 4 ++--
> hw/nvram/fw_cfg.c | 18 ++++++++++++++----
> 6 files changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
> index f50d068563..3527cd51d8 100644
> --- a/include/hw/nvram/fw_cfg.h
> +++ b/include/hw/nvram/fw_cfg.h
> @@ -183,6 +183,7 @@ void fw_cfg_add_file(FWCfgState *s, const char *filename, void *data,
> * @s: fw_cfg device being modified
> * @filename: name of new fw_cfg file item
> * @select_cb: callback function when selecting
> + * @write_cb: callback function when write reached EOF
> * @callback_opaque: argument to be passed into callback function
> * @data: pointer to start of item data
> * @len: size of item data
> @@ -202,6 +203,7 @@ void fw_cfg_add_file(FWCfgState *s, const char *filename, void *data,
> */
> void fw_cfg_add_file_callback(FWCfgState *s, const char *filename,
> FWCfgCallback select_cb,
> + FWCfgCallback write_cb,
> void *callback_opaque,
> void *data, size_t len, bool read_only);
>
> diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
> index a32b847fe0..c1e935da9f 100644
> --- a/hw/acpi/vmgenid.c
> +++ b/hw/acpi/vmgenid.c
> @@ -124,7 +124,7 @@ void vmgenid_add_fw_cfg(VmGenIdState *vms, FWCfgState *s, GArray *guid)
> fw_cfg_add_file(s, VMGENID_GUID_FW_CFG_FILE, guid->data,
> VMGENID_FW_CFG_SIZE);
> /* Create a read-write fw_cfg file for Address */
> - fw_cfg_add_file_callback(s, VMGENID_ADDR_FW_CFG_FILE, NULL, NULL,
> + fw_cfg_add_file_callback(s, VMGENID_ADDR_FW_CFG_FILE, NULL, NULL, NULL,
> vms->vmgenid_addr_le,
> ARRAY_SIZE(vms->vmgenid_addr_le), false);
> }
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index 4593061445..91669d65aa 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -1023,7 +1023,7 @@ MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len,
> }
>
> fw_cfg_add_file_callback(fw_cfg, fw_file_name,
> - fw_callback, callback_opaque,
> + fw_callback, NULL, callback_opaque,
> data, rom->datasize, read_only);
> }
> return mr;
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index b9c245c9bb..be2992b708 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2922,7 +2922,7 @@ void acpi_setup(void)
>
> build_state->rsdp = g_memdup(tables.rsdp->data, rsdp_size);
> fw_cfg_add_file_callback(pcms->fw_cfg, ACPI_BUILD_RSDP_FILE,
> - acpi_build_update, build_state,
> + acpi_build_update, NULL, build_state,
> build_state->rsdp, rsdp_size, true);
> build_state->rsdp_mr = NULL;
> } else {
> diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
> index ac8416d42b..de8fbb7260 100644
> --- a/hw/isa/lpc_ich9.c
> +++ b/hw/isa/lpc_ich9.c
> @@ -402,12 +402,12 @@ void ich9_lpc_pm_init(PCIDevice *lpc_pci, bool smm_enabled)
> * just link them into fw_cfg here.
> */
> fw_cfg_add_file_callback(fw_cfg, "etc/smi/requested-features",
> - NULL, NULL,
> + NULL, NULL, NULL,
> lpc->smi_guest_features_le,
> sizeof lpc->smi_guest_features_le,
> false);
> fw_cfg_add_file_callback(fw_cfg, "etc/smi/features-ok",
> - smi_features_ok_callback, lpc,
> + smi_features_ok_callback, NULL, lpc,
> &lpc->smi_features_ok,
> sizeof lpc->smi_features_ok,
> true);
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index e3bd626b8c..28780088b9 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -56,6 +56,7 @@ struct FWCfgEntry {
> uint8_t *data;
> void *callback_opaque;
> FWCfgCallback select_cb;
> + FWCfgCallback write_cb;
> };
>
> #define JPG_FILE 0
> @@ -384,6 +385,12 @@ static void fw_cfg_dma_transfer(FWCfgState *s)
> stl_be_dma(s->dma_as, dma_addr + offsetof(FWCfgDmaAccess, control),
> dma.control);
>
> + if (write &&
> + !(dma.control & FW_CFG_DMA_CTL_ERROR) &&
> + s->cur_offset == e->len) {
> + e->write_cb(e->callback_opaque);
> + }
> +
I think callback should be invoked on every write.
And your device should verify that all of it is written.
We are doing DMA so all of it can be written in one go.
> trace_fw_cfg_read(s, 0);
> }
>
> @@ -570,6 +577,7 @@ static const VMStateDescription vmstate_fw_cfg = {
>
> static void fw_cfg_add_bytes_callback(FWCfgState *s, uint16_t key,
> FWCfgCallback select_cb,
> + FWCfgCallback write_cb,
> void *callback_opaque,
> void *data, size_t len,
> bool read_only)
> @@ -584,6 +592,7 @@ static void fw_cfg_add_bytes_callback(FWCfgState *s, uint16_t key,
> s->entries[arch][key].data = data;
> s->entries[arch][key].len = (uint32_t)len;
> s->entries[arch][key].select_cb = select_cb;
> + s->entries[arch][key].write_cb = write_cb;
> s->entries[arch][key].callback_opaque = callback_opaque;
> s->entries[arch][key].allow_write = !read_only;
> }
> @@ -610,7 +619,7 @@ static void *fw_cfg_modify_bytes_read(FWCfgState *s, uint16_t key,
>
> void fw_cfg_add_bytes(FWCfgState *s, uint16_t key, void *data, size_t len)
> {
> - fw_cfg_add_bytes_callback(s, key, NULL, NULL, data, len, true);
> + fw_cfg_add_bytes_callback(s, key, NULL, NULL, NULL, data, len, true);
> }
>
> void fw_cfg_add_string(FWCfgState *s, uint16_t key, const char *value)
> @@ -737,6 +746,7 @@ static int get_fw_cfg_order(FWCfgState *s, const char *name)
>
> void fw_cfg_add_file_callback(FWCfgState *s, const char *filename,
> FWCfgCallback select_cb,
> + FWCfgCallback write_cb,
> void *callback_opaque,
> void *data, size_t len, bool read_only)
> {
> @@ -800,7 +810,7 @@ void fw_cfg_add_file_callback(FWCfgState *s, const char *filename,
> }
>
> fw_cfg_add_bytes_callback(s, FW_CFG_FILE_FIRST + index,
> - select_cb,
> + select_cb, write_cb,
> callback_opaque, data, len,
> read_only);
>
> @@ -815,7 +825,7 @@ void fw_cfg_add_file_callback(FWCfgState *s, const char *filename,
> void fw_cfg_add_file(FWCfgState *s, const char *filename,
> void *data, size_t len)
> {
> - fw_cfg_add_file_callback(s, filename, NULL, NULL, data, len, true);
> + fw_cfg_add_file_callback(s, filename, NULL, NULL, NULL, data, len, true);
> }
>
> void *fw_cfg_modify_file(FWCfgState *s, const char *filename,
> @@ -838,7 +848,7 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename,
> }
> }
> /* add new one */
> - fw_cfg_add_file_callback(s, filename, NULL, NULL, data, len, true);
> + fw_cfg_add_file_callback(s, filename, NULL, NULL, NULL, data, len, true);
> return NULL;
> }
>
> --
> 2.14.0.1.geff633fa0
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/8] fw_cfg: add vmcoreinfo file
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 3/8] fw_cfg: add vmcoreinfo file Marc-André Lureau
2017-09-08 12:32 ` Michael S. Tsirkin
2017-09-08 12:36 ` Michael S. Tsirkin
@ 2017-09-08 12:42 ` Michael S. Tsirkin
2017-09-08 15:39 ` Michael S. Tsirkin
3 siblings, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2017-09-08 12:42 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel, ehabkost, anderson, imammedo, lersek
On Mon, Aug 07, 2017 at 08:16:13PM +0200, Marc-André Lureau wrote:
> See docs/specs/fw_cfg.txt for details.
>
> The "etc/vmcoreinfo" is added when using "-global
> fw_cfg.vmcoreinfo=on" qemu option.
>
> Disabled by default for machine types v2.9 and older.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> include/hw/compat.h | 8 ++++++++
> include/hw/nvram/fw_cfg.h | 9 +++++++++
> hw/nvram/fw_cfg.c | 20 ++++++++++++++++++++
> docs/specs/fw_cfg.txt | 16 ++++++++++++++++
> 4 files changed, 53 insertions(+)
>
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 08f36004da..317fd2e2e3 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -18,6 +18,14 @@
> .driver = "pcie-root-port",\
> .property = "x-migrate-msix",\
> .value = "false",\
> + },{\
> + .driver = "fw_cfg_mem",\
> + .property = "vmcoreinfo",\
> + .value = "off",\
> + },{\
> + .driver = "fw_cfg_io",\
> + .property = "vmcoreinfo",\
> + .value = "off",\
> },
>
> #define HW_COMPAT_2_8 \
> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
> index 3527cd51d8..a35f47405d 100644
> --- a/include/hw/nvram/fw_cfg.h
> +++ b/include/hw/nvram/fw_cfg.h
> @@ -30,6 +30,11 @@ typedef struct FWCfgFile {
> void fw_cfg_set_order_override(FWCfgState *fw_cfg, int order);
> void fw_cfg_reset_order_override(FWCfgState *fw_cfg);
>
> +typedef struct FWCfgVMCoreInfo {
> + uint64_t paddr;
> + uint32_t size;
Pls add padding to align structure size to multiple of 8 bytes.
> +} QEMU_PACKED FWCfgVMCoreInfo;
> +
> typedef struct FWCfgFiles {
> uint32_t count;
> FWCfgFile f[];
> @@ -65,6 +70,10 @@ struct FWCfgState {
> dma_addr_t dma_addr;
> AddressSpace *dma_as;
> MemoryRegion dma_iomem;
> +
> + bool vmcoreinfo_enabled;
> + bool has_vmcoreinfo;
> + FWCfgVMCoreInfo vmcoreinfo;
> };
>
> struct FWCfgIoState {
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 28780088b9..342afc4ed2 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -504,6 +504,7 @@ static void fw_cfg_reset(DeviceState *d)
>
> /* we never register a read callback for FW_CFG_SIGNATURE */
> fw_cfg_select(s, FW_CFG_SIGNATURE);
> + s->has_vmcoreinfo = false;
I do not think this is enough. If guest only writes the last couple of
bytes you leak some info from before to after reset.
You want to zero the whole structure.
> }
>
> /* Save restore 32 bit int as uint16_t
> @@ -869,7 +870,12 @@ static void fw_cfg_machine_ready(struct Notifier *n, void *data)
> qemu_register_reset(fw_cfg_machine_reset, s);
> }
>
> +static void fw_cfg_vmci_written(void *dev)
> +{
> + FWCfgState *s = FW_CFG(dev);
>
> + s->has_vmcoreinfo = true;
> +}
>
> static void fw_cfg_common_realize(DeviceState *dev, Error **errp)
> {
> @@ -895,6 +901,16 @@ static void fw_cfg_common_realize(DeviceState *dev, Error **errp)
>
> fw_cfg_add_i32(s, FW_CFG_ID, version);
>
> + if (s->vmcoreinfo_enabled) {
> + if (!s->dma_enabled) {
> + error_setg(errp, "vmcoreinfo requires dma_enabled");
> + return;
> + }
> + fw_cfg_add_file_callback(s, "etc/vmcoreinfo",
> + NULL, fw_cfg_vmci_written, s,
> + &s->vmcoreinfo, sizeof(s->vmcoreinfo), false);
> + }
> +
> s->machine_ready.notify = fw_cfg_machine_ready;
> qemu_add_machine_init_done_notifier(&s->machine_ready);
> }
> @@ -1031,6 +1047,8 @@ static void fw_cfg_file_slots_allocate(FWCfgState *s, Error **errp)
> static Property fw_cfg_io_properties[] = {
> DEFINE_PROP_BOOL("dma_enabled", FWCfgIoState, parent_obj.dma_enabled,
> true),
> + DEFINE_PROP_BOOL("vmcoreinfo", FWCfgIoState, parent_obj.vmcoreinfo_enabled,
> + true),
> DEFINE_PROP_UINT16("x-file-slots", FWCfgIoState, parent_obj.file_slots,
> FW_CFG_FILE_SLOTS_DFLT),
> DEFINE_PROP_END_OF_LIST(),
> @@ -1082,6 +1100,8 @@ static Property fw_cfg_mem_properties[] = {
> DEFINE_PROP_UINT32("data_width", FWCfgMemState, data_width, -1),
> DEFINE_PROP_BOOL("dma_enabled", FWCfgMemState, parent_obj.dma_enabled,
> true),
> + DEFINE_PROP_BOOL("vmcoreinfo", FWCfgMemState, parent_obj.vmcoreinfo_enabled,
> + true),
> DEFINE_PROP_UINT16("x-file-slots", FWCfgMemState, parent_obj.file_slots,
> FW_CFG_FILE_SLOTS_DFLT),
> DEFINE_PROP_END_OF_LIST(),
> diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
> index 08c00bdf44..37d0f9f40a 100644
> --- a/docs/specs/fw_cfg.txt
> +++ b/docs/specs/fw_cfg.txt
> @@ -136,6 +136,22 @@ struct FWCfgFile { /* an individual file entry, 64 bytes total */
> char name[56]; /* fw_cfg item name, NUL-terminated ascii */
> };
>
> +=== etc/vmcoreinfo ===
> +
> +A guest may use this entry to add information details to qemu
> +dumps. The entry gives location and size of an ELF note that is
> +appended in qemu dumps.
> +
> +The entry is of 12 bytes with this format:
> +
> +struct FWCfgVMCoreInfo {
> + uint64_t paddr; /* physical address of ELF note, LE */
> + uint32_t size; /* size of ELF note region, LE */
> +};
> +
> +The note format/class must be of the target bitness and the size must
> +be less than 1Mb.
> +
> === All Other Data Items ===
>
> Please consult the QEMU source for the most up-to-date and authoritative list
> --
> 2.14.0.1.geff633fa0
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5 0/8] KASLR kernel dump support
2017-08-07 18:16 [Qemu-devel] [PATCH v5 0/8] KASLR kernel dump support Marc-André Lureau
` (8 preceding siblings ...)
2017-08-16 20:15 ` [Qemu-devel] [PATCH v5 0/8] KASLR kernel dump support Michael S. Tsirkin
@ 2017-09-08 12:46 ` Michael S. Tsirkin
9 siblings, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2017-09-08 12:46 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel, ehabkost, anderson, imammedo, lersek
On Mon, Aug 07, 2017 at 08:16:10PM +0200, Marc-André Lureau wrote:
> Recent linux kernels enable KASLR to randomize phys/virt memory
> addresses. This series aims to provide enough information in qemu
> dumps so that crash utility can work with randomized kernel too (it
> hasn't been tested on other archs than x86 though, help welcome).
>
> The previous design to provide qemu with debug details (using qemu-ga
> and a dedicated vmcoreinfo ACPI device) failed to satisfy the
> requirements during previous iterations.
>
> In particular, the previous proposed vmcoreinfo ACPI device had the
> following issues:
> - hazardous memory handling with no explicit synchronization
> - occupy 2 fw-cfg entries (for memory and pointer)
> - occupy 4k of guest memory (this could have been tweaked)
> - did not provide ACPI methods (this could have been added)
> - may be difficult to maintain compatibility (according to Michael)
>
> This is a new proposal, that leverage fw-cfg device instead of adding
> a new device. A "etc/vmcoreinfo" entry is added, where the guest,
> during boot or later, can write the addr/size location of an ELF note
> to be appended in the qemu dump.
>
> Note: only guest kernel is expected to write to a fw-cfg entry. This
> method is not meant for general qemu/user-space communication. There
> are more appropriate devices for this purpose, and the guest kernel
> should not expose this facility.
>
> This is quite easier to implement, and uses less of the limited fw-cfg
> slots, and guest memory. It also solves the synchronization issue, and
> may be easier to discover or to maintain compatibility.
>
> The Linux ELF note is expected to be the VMCOREINFO note, which will
> have a special handling in qemu in this case helping kaslr-kernel
> debugging. But it could be any valid ELF note.
>
> Crash 7.1.9 will parse the "phys_base" value from the VMCOREINFO note,
> and thus will work with KASLR-dump produced by this series.
>
> The series implements the note addition in qemu ELF/kdump,
> as well as the python scripts/dump-guest-memory.py.
So still some comments and I'll be on vacation -
I think if it's a separate device someone else can
merge it easily.
FW CFG API changes themselves are OK if they satisfy
needs of this device.
> To test:
>
> Using kernel from https://github.com/elmarco/linux fw-cfg branch,
> Compile and run guest kernel with CONFIG_RANDOMIZE_BASE=y & CONFIG_FW_CFG_SYSFS=y.
>
> Run qemu with -global fw_cfg.vmcoreinfo=on
>
> Produce an ELF dump:
> { "execute": "dump-guest-memory", "arguments": { "protocol": "file:dump", "paging": false } }
>
> Produce a kdump:
> { "execute": "dump-guest-memory", "arguments": { "protocol": "file:dump", "paging": false, "format": "kdump-zlib" } }
>
> Or with (gdb) dump-guest-memory, with scripts/dump-guest-memory.py script.
>
> Analyze with crash >= 7.1.9 (or the git version for 4.13 fixes..):
>
> $ crash vmlinux dump
>
> v5:
> - removed x-write-pointer-available patch from this series
> - drop vmcoreinfo device
> - add write callback to fw_cfg entries
> - add a writable fw_cfg "vmcoreinfo" entry
> - split phys_base update from VMCOREINFO note in a seperate patch
> - most patches had non-trivial changes, dropping reviewed-by tags
>
> v4: from Laszlo review
> - switch to warn_report*()
> - update test to follow vmgenid and use boot-sector infrastructure
> - fix range checks in the python script
> - add vmcoreinfo_get() stub
>
> v3: from Laszlo review
> - change vmcoreinfo offset to 36
> - reset err to null after report
> - use PRIu32
> - change name_size and desc_size against MAX_VMCOREINFO_SIZE
> - python code simplification
> - check boundaries of blocks in phys_memory_read()
> - fix some vmgi vs vmci names
> - add more comments in code
> - fix comment indentation
> - add r-b tags
>
> v2: from Laszlo review
> - vmci: fix guest endianess handling
> - vmci: fix wrong sizeof()
> - vmci: add back reset logic from vmgenid
> - dump: have 1MB size limit for vmcoreinfo
> - dump: fix potential off-by-1 buffer manipulation
> - dump: use temporary variable for qemu_strtou64
> - dump: fixed VMCOREINFO duplication in kdump
> - update gdb script to not call into qemu process
> - update MAINTAINERS with some new files
>
> Marc-André Lureau (8):
> fw_cfg: rename read callback
> fw_cfg: add write callback
> fw_cfg: add vmcoreinfo file
> dump: add guest ELF note
> dump: update phys_base header field based on VMCOREINFO content
> kdump: set vmcoreinfo location
> scripts/dump-guest-memory.py: add vmcoreinfo
> MAINTAINERS: add Dump maintainers
>
> scripts/dump-guest-memory.py | 51 ++++++++++++
> include/hw/compat.h | 8 ++
> include/hw/loader.h | 2 +-
> include/hw/nvram/fw_cfg.h | 18 ++++-
> include/sysemu/dump.h | 2 +
> dump.c | 179 +++++++++++++++++++++++++++++++++++++++++++
> hw/acpi/vmgenid.c | 2 +-
> hw/core/loader.c | 4 +-
> hw/i386/acpi-build.c | 2 +-
> hw/isa/lpc_ich9.c | 4 +-
> hw/nvram/fw_cfg.c | 64 ++++++++++++----
> MAINTAINERS | 9 +++
> docs/specs/fw_cfg.txt | 24 ++++++
> 13 files changed, 343 insertions(+), 26 deletions(-)
>
> --
> 2.14.0.1.geff633fa0
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/8] fw_cfg: add vmcoreinfo file
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 3/8] fw_cfg: add vmcoreinfo file Marc-André Lureau
` (2 preceding siblings ...)
2017-09-08 12:42 ` Michael S. Tsirkin
@ 2017-09-08 15:39 ` Michael S. Tsirkin
2017-09-08 15:39 ` Michael S. Tsirkin
3 siblings, 1 reply; 19+ messages in thread
From: Michael S. Tsirkin @ 2017-09-08 15:39 UTC (permalink / raw)
To: Marc-André Lureau
Cc: qemu-devel, imammedo, berrange, ehabkost, anderson, lersek
On Mon, Aug 07, 2017 at 08:16:13PM +0200, Marc-André Lureau wrote:
> diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
> index 08c00bdf44..37d0f9f40a 100644
> --- a/docs/specs/fw_cfg.txt
> +++ b/docs/specs/fw_cfg.txt
> @@ -136,6 +136,22 @@ struct FWCfgFile { /* an individual file entry, 64 bytes total */
> char name[56]; /* fw_cfg item name, NUL-terminated ascii */
> };
>
> +=== etc/vmcoreinfo ===
> +
> +A guest may use this entry to add information details to qemu
> +dumps. The entry gives location and size of an ELF note that is
> +appended in qemu dumps.
> +
> +The entry is of 12 bytes with this format:
> +
> +struct FWCfgVMCoreInfo {
> + uint64_t paddr; /* physical address of ELF note, LE */
> + uint32_t size; /* size of ELF note region, LE */
> +};
> +
> +The note format/class must be of the target bitness and the size must
> +be less than 1Mb.
> +
I would say adding a format bitmap would make sense for future compatibility.
How about:
struct FWCfgVMCoreInfo {
uint16_t host_format; /* Formats host supports. 0x1 LE - ELF note. Other bits - ignored. */
uint16_t guest_format; /* Formats guest supplies. Must be 0x1 LE */
uint32_t size; /* size of ELF note region, LE */
uint64_t paddr; /* physical address of ELF note, LE */
};
> === All Other Data Items ===
>
> Please consult the QEMU source for the most up-to-date and authoritative list
> --
> 2.14.0.1.geff633fa0
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/8] fw_cfg: add vmcoreinfo file
2017-09-08 15:39 ` Michael S. Tsirkin
@ 2017-09-08 15:39 ` Michael S. Tsirkin
2017-09-08 15:49 ` Marc-André Lureau
0 siblings, 1 reply; 19+ messages in thread
From: Michael S. Tsirkin @ 2017-09-08 15:39 UTC (permalink / raw)
To: Marc-André Lureau
Cc: qemu-devel, imammedo, berrange, ehabkost, anderson, lersek
On Fri, Sep 08, 2017 at 06:39:01PM +0300, Michael S. Tsirkin wrote:
> On Mon, Aug 07, 2017 at 08:16:13PM +0200, Marc-André Lureau wrote:
> > diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
> > index 08c00bdf44..37d0f9f40a 100644
> > --- a/docs/specs/fw_cfg.txt
> > +++ b/docs/specs/fw_cfg.txt
> > @@ -136,6 +136,22 @@ struct FWCfgFile { /* an individual file entry, 64 bytes total */
> > char name[56]; /* fw_cfg item name, NUL-terminated ascii */
> > };
> >
> > +=== etc/vmcoreinfo ===
> > +
> > +A guest may use this entry to add information details to qemu
> > +dumps. The entry gives location and size of an ELF note that is
> > +appended in qemu dumps.
> > +
> > +The entry is of 12 bytes with this format:
> > +
> > +struct FWCfgVMCoreInfo {
> > + uint64_t paddr; /* physical address of ELF note, LE */
> > + uint32_t size; /* size of ELF note region, LE */
> > +};
> > +
> > +The note format/class must be of the target bitness and the size must
> > +be less than 1Mb.
> > +
>
> I would say adding a format bitmap would make sense for future compatibility.
> How about:
>
> struct FWCfgVMCoreInfo {
> uint16_t host_format; /* Formats host supports. 0x1 LE - ELF note. Other bits - ignored. */
> uint16_t guest_format; /* Formats guest supplies. Must be 0x1 LE */
.. to set the ELF note info, or 0x0 to reset the device.
> uint32_t size; /* size of ELF note region, LE */
> uint64_t paddr; /* physical address of ELF note, LE */
> };
>
>
> > === All Other Data Items ===
> >
> > Please consult the QEMU source for the most up-to-date and authoritative list
> > --
> > 2.14.0.1.geff633fa0
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/8] fw_cfg: add vmcoreinfo file
2017-09-08 15:39 ` Michael S. Tsirkin
@ 2017-09-08 15:49 ` Marc-André Lureau
2017-09-10 1:52 ` Michael S. Tsirkin
0 siblings, 1 reply; 19+ messages in thread
From: Marc-André Lureau @ 2017-09-08 15:49 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, imammedo, berrange, ehabkost, anderson, lersek
Hi
----- Original Message -----
> On Fri, Sep 08, 2017 at 06:39:01PM +0300, Michael S. Tsirkin wrote:
> > On Mon, Aug 07, 2017 at 08:16:13PM +0200, Marc-André Lureau wrote:
> > > diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
> > > index 08c00bdf44..37d0f9f40a 100644
> > > --- a/docs/specs/fw_cfg.txt
> > > +++ b/docs/specs/fw_cfg.txt
> > > @@ -136,6 +136,22 @@ struct FWCfgFile { /* an individual file entry, 64
> > > bytes total */
> > > char name[56]; /* fw_cfg item name, NUL-terminated ascii */
> > > };
> > >
> > > +=== etc/vmcoreinfo ===
> > > +
> > > +A guest may use this entry to add information details to qemu
> > > +dumps. The entry gives location and size of an ELF note that is
> > > +appended in qemu dumps.
> > > +
> > > +The entry is of 12 bytes with this format:
> > > +
> > > +struct FWCfgVMCoreInfo {
> > > + uint64_t paddr; /* physical address of ELF note, LE */
> > > + uint32_t size; /* size of ELF note region, LE */
> > > +};
> > > +
> > > +The note format/class must be of the target bitness and the size must
> > > +be less than 1Mb.
> > > +
> >
> > I would say adding a format bitmap would make sense for future
> > compatibility.
> > How about:
> >
> > struct FWCfgVMCoreInfo {
> > uint16_t host_format; /* Formats host supports. 0x1 LE - ELF
> > note. Other bits - ignored. */
Could this be added later?
For ex, extend the entry to 16 bytes, with those 2 values appended?
If the size is 12, assume it is ELF note only, and that the host supports it.
> > uint16_t guest_format; /* Formats guest supplies. Must be 0x1 LE
> > */
>
> .. to set the ELF note info, or 0x0 to reset the device.
>
> > uint32_t size; /* size of ELF note region, LE */
> > uint64_t paddr; /* physical address of ELF note, LE */
> > };
> >
> >
> > > === All Other Data Items ===
> > >
> > > Please consult the QEMU source for the most up-to-date and authoritative
> > > list
> > > --
> > > 2.14.0.1.geff633fa0
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/8] fw_cfg: add vmcoreinfo file
2017-09-08 15:49 ` Marc-André Lureau
@ 2017-09-10 1:52 ` Michael S. Tsirkin
0 siblings, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2017-09-10 1:52 UTC (permalink / raw)
To: Marc-André Lureau
Cc: qemu-devel, imammedo, berrange, ehabkost, anderson, lersek
On Fri, Sep 08, 2017 at 11:49:46AM -0400, Marc-André Lureau wrote:
> Hi
>
> ----- Original Message -----
> > On Fri, Sep 08, 2017 at 06:39:01PM +0300, Michael S. Tsirkin wrote:
> > > On Mon, Aug 07, 2017 at 08:16:13PM +0200, Marc-André Lureau wrote:
> > > > diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
> > > > index 08c00bdf44..37d0f9f40a 100644
> > > > --- a/docs/specs/fw_cfg.txt
> > > > +++ b/docs/specs/fw_cfg.txt
> > > > @@ -136,6 +136,22 @@ struct FWCfgFile { /* an individual file entry, 64
> > > > bytes total */
> > > > char name[56]; /* fw_cfg item name, NUL-terminated ascii */
> > > > };
> > > >
> > > > +=== etc/vmcoreinfo ===
> > > > +
> > > > +A guest may use this entry to add information details to qemu
> > > > +dumps. The entry gives location and size of an ELF note that is
> > > > +appended in qemu dumps.
> > > > +
> > > > +The entry is of 12 bytes with this format:
> > > > +
> > > > +struct FWCfgVMCoreInfo {
> > > > + uint64_t paddr; /* physical address of ELF note, LE */
> > > > + uint32_t size; /* size of ELF note region, LE */
> > > > +};
> > > > +
> > > > +The note format/class must be of the target bitness and the size must
> > > > +be less than 1Mb.
> > > > +
> > >
> > > I would say adding a format bitmap would make sense for future
> > > compatibility.
> > > How about:
> > >
> > > struct FWCfgVMCoreInfo {
> > > uint16_t host_format; /* Formats host supports. 0x1 LE - ELF
> > > note. Other bits - ignored. */
>
> Could this be added later?
>
> For ex, extend the entry to 16 bytes, with those 2 values appended?
I think aligned size is better overall so you can write
bindings in any language without issues.
I'm ok with just keeping 0s there for now if you prefer.
> If the size is 12, assume it is ELF note only, and that the host supports it.
Seems more like a hack that will make code ugly down the road.
> > > uint16_t guest_format; /* Formats guest supplies. Must be 0x1 LE
> > > */
> >
> > .. to set the ELF note info, or 0x0 to reset the device.
> >
> > > uint32_t size; /* size of ELF note region, LE */
> > > uint64_t paddr; /* physical address of ELF note, LE */
> > > };
> > >
> > >
> > > > === All Other Data Items ===
> > > >
> > > > Please consult the QEMU source for the most up-to-date and authoritative
> > > > list
> > > > --
> > > > 2.14.0.1.geff633fa0
> >
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2017-09-10 1:52 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-07 18:16 [Qemu-devel] [PATCH v5 0/8] KASLR kernel dump support Marc-André Lureau
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 1/8] fw_cfg: rename read callback Marc-André Lureau
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 2/8] fw_cfg: add write callback Marc-André Lureau
2017-09-08 12:40 ` Michael S. Tsirkin
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 3/8] fw_cfg: add vmcoreinfo file Marc-André Lureau
2017-09-08 12:32 ` Michael S. Tsirkin
2017-09-08 12:36 ` Michael S. Tsirkin
2017-09-08 12:42 ` Michael S. Tsirkin
2017-09-08 15:39 ` Michael S. Tsirkin
2017-09-08 15:39 ` Michael S. Tsirkin
2017-09-08 15:49 ` Marc-André Lureau
2017-09-10 1:52 ` Michael S. Tsirkin
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 4/8] dump: add guest ELF note Marc-André Lureau
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 5/8] dump: update phys_base header field based on VMCOREINFO content Marc-André Lureau
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 6/8] kdump: set vmcoreinfo location Marc-André Lureau
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 7/8] scripts/dump-guest-memory.py: add vmcoreinfo Marc-André Lureau
2017-08-07 18:16 ` [Qemu-devel] [PATCH v5 8/8] MAINTAINERS: add Dump maintainers Marc-André Lureau
2017-08-16 20:15 ` [Qemu-devel] [PATCH v5 0/8] KASLR kernel dump support Michael S. Tsirkin
2017-09-08 12:46 ` Michael S. Tsirkin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.