[SeaBIOS] [PATCH v5 5/5] QEMU fw_cfg: Write fw_cfg back on S3 resume
Laszlo Ersek
lersek at redhat.com
Mon Feb 20 11:28:52 CET 2017
On 02/18/17 07:21, ben at skyportsystems.com wrote:
> From: Ben Warren <ben at skyportsystems.com>
>
> Any pointers to BIOS-allocated memory that were written back to QEMU
> fw_cfg files are replayed when resuming from S3 sleep.
>
> Signed-off-by: Ben Warren <ben at skyportsystems.com>
> ---
> src/fw/romfile_loader.c | 35 +++++++++++++++++++++++++++++++++++
> src/fw/romfile_loader.h | 2 ++
> src/resume.c | 4 ++++
> 3 files changed, 41 insertions(+)
>
> diff --git a/src/fw/romfile_loader.c b/src/fw/romfile_loader.c
> index 30e7b58..33aaec4 100644
> --- a/src/fw/romfile_loader.c
> +++ b/src/fw/romfile_loader.c
> @@ -4,6 +4,7 @@
> #include "string.h" // strcmp
> #include "romfile.h" // struct romfile_s
> #include "malloc.h" // Zone*, _malloc
> +#include "list.h" // struct hlist_node
> #include "output.h" // warn_*
> #include "paravirt.h" // qemu_cfg_write_file
>
> @@ -16,6 +17,16 @@ struct romfile_loader_files {
> struct romfile_loader_file files[];
> };
>
> +// Data structures for storing "write pointer" entries for possible replay
> +struct romfile_wr_pointer_entry {
> + u64 pointer;
> + u32 offset;
> + u16 key;
> + u8 ptr_size;
> + struct hlist_node node;
> +};
> +static struct hlist_head romfile_pointer_list;
> +
> static struct romfile_loader_file *
> romfile_loader_find(const char *name,
> struct romfile_loader_files *files)
> @@ -29,6 +40,16 @@ romfile_loader_find(const char *name,
> return NULL;
> }
>
> +// Replay "write pointer" entries back to QEMU
> +void romfile_fw_cfg_resume(void)
> +{
> + struct romfile_wr_pointer_entry *entry;
> + hlist_for_each_entry(entry, &romfile_pointer_list, node) {
> + qemu_cfg_write_file_simple(&entry->pointer, entry->key,
> + entry->offset, entry->ptr_size);
> + }
> +}
> +
> static void romfile_loader_allocate(struct romfile_loader_entry_s *entry,
> struct romfile_loader_files *files)
> {
> @@ -163,6 +184,20 @@ static void romfile_loader_write_pointer(struct romfile_loader_entry_s *entry,
> entry->wr_pointer.size) != entry->wr_pointer.size) {
> goto err;
> }
> +
> + /* Store the info so it can replayed later if necessary */
> + struct romfile_wr_pointer_entry *store = _malloc(&ZoneHigh,
> + sizeof(*store), 4);
I don't know enough of the SeaBIOS memory allocation system to know if
this is safe. I assume this will place the allocation in reserved memory.
> + struct hlist_node **pprev = &romfile_pointer_list.first;
> + store->pointer = pointer;
> + store->key = qemu_get_romfile_key(dest_file);
> + if (store->key == 0) {
> + goto err;
> + }
Based on my comment on container_of() in patch #4,
qemu_get_romfile_key() shouldn't be able to fail, so this check should
be unnecessary.
> + store->offset = dst_offset;
> + store->ptr_size = entry->wr_pointer.size;
> + hlist_add(&store->node, pprev);
I think the code can be simplified a bit, by calling hlist_add_head()
instead (you can drop the local "pprev" variable then):
hlist_add_head(&store->node, &romfile_pointer_list);
> +
> return;
> err:
> warn_internalerror();
> diff --git a/src/fw/romfile_loader.h b/src/fw/romfile_loader.h
> index 4dc50ab..fcd4ab2 100644
> --- a/src/fw/romfile_loader.h
> +++ b/src/fw/romfile_loader.h
> @@ -86,4 +86,6 @@ enum {
>
> int romfile_loader_execute(const char *name);
>
> +void romfile_fw_cfg_resume(void);
> +
> #endif
> diff --git a/src/resume.c b/src/resume.c
> index e67cfce..99fa34f 100644
> --- a/src/resume.c
> +++ b/src/resume.c
> @@ -17,6 +17,7 @@
> #include "string.h" // memset
> #include "util.h" // dma_setup
> #include "tcgbios.h" // tpm_s3_resume
> +#include "fw/romfile_loader.h" // romfile_fw_cfg_resume
>
> // Handler for post calls that look like a resume.
> void VISIBLE16
> @@ -105,6 +106,9 @@ s3_resume(void)
> tpm_s3_resume();
> s3_resume_vga();
>
> + /* Replay any fw_cfg entries that go back to the host */
> + romfile_fw_cfg_resume();
> +
The functionality in "romfile_loader.c" is conditional on CONFIG_QEMU:
qemu_platform_setup() [src/fw/paravirt.c]
// returns immediately if !CONFIG_QEMU
romfile_loader_execute() [src/fw/romfile_loader.c]
So we shouldn't make the replay unconditional either.
Although "romfile_pointer_list" will be empty, if
romfile_loader_execute() never runs, we should also save on code size
(-> build time optimization) if CONFIG_QEMU is false. Please add the check
if (!CONFIG_QEMU)
return;
to the top of romfile_fw_cfg_resume(), similarly to the example in
smp_resume() [src/fw/smp.c], which is also called from s3_resume().
(Comments from others are most welcome, of course; this is just how I
see things.)
Thanks!
Laszlo
> make_bios_readonly();
>
> // Invoke the resume vector.
>
More information about the SeaBIOS
mailing list