| Message ID | 20170807181618.22562-4-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 >
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 >
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 >
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
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
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 >
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 > >
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
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(+)