1. Patchew
  2. Seabios
  3. [SeaBIOS] [PATCH v5 0/5] QEMU: Add capability to write back fw_cfg address to QEMU
  4. View patch

[SeaBIOS] [PATCH v6 4/5] QEMU fw_cfg: Add functions for accessing files by key

ben@skyportsystems.com posted 5 patches 8 years, 9 months ago
There is a newer version of this series
[SeaBIOS] [PATCH v6 4/5] QEMU fw_cfg: Add functions for accessing files by key
Posted by ben@skyportsystems.com 8 years, 9 months ago 
From: Ben Warren <ben@skyportsystems.com>

Due to memory contraints, when resuming from S3 the fw_cfg "files" API
isn't available.  This adds a simple API to get a file 'key', and to
write to the file using the key as a reference.

Signed-off-by: Ben Warren <ben@skyportsystems.com>
---
 src/fw/paravirt.c | 32 ++++++++++++++++++++++++++------
 src/fw/paravirt.h |  2 ++
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
index 4618647..225b08b 100644
--- a/src/fw/paravirt.c
+++ b/src/fw/paravirt.c
@@ -329,6 +329,17 @@ qemu_cfg_read_file(struct romfile_s *file, void *dst, u32 maxlen)
     return file->size;
 }
 
+// Bare-bones function for writing a file knowing only its unique
+// identifying key (select)
+int
+qemu_cfg_write_file_simple(void *src, u16 key, u32 offset, u32 len)
+{
+    qemu_cfg_select(key);
+    qemu_cfg_skip(offset);
+    qemu_cfg_write(src, len);
+    return len;
+}
+
 int
 qemu_cfg_write_file(void *src, struct romfile_s *file, u32 offset, u32 len)
 {
@@ -339,15 +350,12 @@ qemu_cfg_write_file(void *src, struct romfile_s *file, u32 offset, u32 len)
         warn_internalerror();
         return -1;
     }
-    struct qemu_romfile_s *qfile;
-    qfile = container_of(file, struct qemu_romfile_s, file);
+    u16 key = qemu_get_romfile_key(file);
     if (offset == 0) {
         /* Do it in one transfer */
-        qemu_cfg_write_entry(src, qfile->select, len);
+        qemu_cfg_write_entry(src, key, len);
     } else {
-        qemu_cfg_select(qfile->select);
-        qemu_cfg_skip(offset);
-        qemu_cfg_write(src, len);
+        qemu_cfg_write_file_simple(src, key, offset, len);
     }
     return len;
 }
@@ -370,6 +378,18 @@ qemu_romfile_add(char *name, int select, int skip, int size)
 }
 
 u16
+qemu_get_romfile_key(struct romfile_s *file)
+{
+    struct qemu_romfile_s *qfile;
+    if (file->copy != qemu_cfg_read_file) {
+        warn_internalerror();
+        return 0;
+    }
+    qfile = container_of(file, struct qemu_romfile_s, file);
+    return qfile->select;
+}
+
+u16
 qemu_get_present_cpus_count(void)
 {
     u16 smp_count = 0;
diff --git a/src/fw/paravirt.h b/src/fw/paravirt.h
index fb220d8..16f3d9a 100644
--- a/src/fw/paravirt.h
+++ b/src/fw/paravirt.h
@@ -56,5 +56,7 @@ void qemu_cfg_init(void);
 
 u16 qemu_get_present_cpus_count(void);
 int qemu_cfg_write_file(void *src, struct romfile_s *file, u32 offset, u32 len);
+int qemu_cfg_write_file_simple(void *src, u16 key, u32 offset, u32 len);
+u16 qemu_get_romfile_key(struct romfile_s *file);
 
 #endif
-- 
2.7.4


_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v6 4/5] QEMU fw_cfg: Add functions for accessing files by key
Posted by Laszlo Ersek 8 years, 9 months ago 
On 02/20/17 21:14, ben@skyportsystems.com wrote:
> From: Ben Warren <ben@skyportsystems.com>
> 
> Due to memory contraints, when resuming from S3 the fw_cfg "files" API
> isn't available.  This adds a simple API to get a file 'key', and to
> write to the file using the key as a reference.
> 
> Signed-off-by: Ben Warren <ben@skyportsystems.com>
> ---
>  src/fw/paravirt.c | 32 ++++++++++++++++++++++++++------
>  src/fw/paravirt.h |  2 ++
>  2 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
> index 4618647..225b08b 100644
> --- a/src/fw/paravirt.c
> +++ b/src/fw/paravirt.c
> @@ -329,6 +329,17 @@ qemu_cfg_read_file(struct romfile_s *file, void *dst, u32 maxlen)
>      return file->size;
>  }
>  
> +// Bare-bones function for writing a file knowing only its unique
> +// identifying key (select)
> +int
> +qemu_cfg_write_file_simple(void *src, u16 key, u32 offset, u32 len)
> +{
> +    qemu_cfg_select(key);
> +    qemu_cfg_skip(offset);
> +    qemu_cfg_write(src, len);
> +    return len;
> +}
> +
>  int
>  qemu_cfg_write_file(void *src, struct romfile_s *file, u32 offset, u32 len)
>  {
> @@ -339,15 +350,12 @@ qemu_cfg_write_file(void *src, struct romfile_s *file, u32 offset, u32 len)
>          warn_internalerror();
>          return -1;
>      }
> -    struct qemu_romfile_s *qfile;
> -    qfile = container_of(file, struct qemu_romfile_s, file);
> +    u16 key = qemu_get_romfile_key(file);
>      if (offset == 0) {
>          /* Do it in one transfer */
> -        qemu_cfg_write_entry(src, qfile->select, len);
> +        qemu_cfg_write_entry(src, key, len);
>      } else {
> -        qemu_cfg_select(qfile->select);
> -        qemu_cfg_skip(offset);
> -        qemu_cfg_write(src, len);
> +        qemu_cfg_write_file_simple(src, key, offset, len);
>      }
>      return len;
>  }

One of the ideas that I mentioned here was to move not just the second
branch of the "if" to qemu_cfg_write_file_simple(), but the entire "if"
-- both branches. Because, qemu_cfg_write_entry() looks suitable for S3
too, and if that kind of optimization makes sense for normal boot, then
it makes sense for S3 resume as well.

Anyway, this is not a functional problem, I won't obsess about it.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo


> @@ -370,6 +378,18 @@ qemu_romfile_add(char *name, int select, int skip, int size)
>  }
>  
>  u16
> +qemu_get_romfile_key(struct romfile_s *file)
> +{
> +    struct qemu_romfile_s *qfile;
> +    if (file->copy != qemu_cfg_read_file) {
> +        warn_internalerror();
> +        return 0;
> +    }
> +    qfile = container_of(file, struct qemu_romfile_s, file);
> +    return qfile->select;
> +}
> +
> +u16
>  qemu_get_present_cpus_count(void)
>  {
>      u16 smp_count = 0;
> diff --git a/src/fw/paravirt.h b/src/fw/paravirt.h
> index fb220d8..16f3d9a 100644
> --- a/src/fw/paravirt.h
> +++ b/src/fw/paravirt.h
> @@ -56,5 +56,7 @@ void qemu_cfg_init(void);
>  
>  u16 qemu_get_present_cpus_count(void);
>  int qemu_cfg_write_file(void *src, struct romfile_s *file, u32 offset, u32 len);
> +int qemu_cfg_write_file_simple(void *src, u16 key, u32 offset, u32 len);
> +u16 qemu_get_romfile_key(struct romfile_s *file);
>  
>  #endif
> 


_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v6 4/5] QEMU fw_cfg: Add functions for accessing files by key
Posted by Ben Warren 8 years, 9 months ago 
> On Feb 20, 2017, at 1:06 PM, Laszlo Ersek <lersek@redhat.com> wrote:
> 
> On 02/20/17 21:14, ben@skyportsystems.com <mailto:ben@skyportsystems.com> wrote:
>> From: Ben Warren <ben@skyportsystems.com>
>> 
>> Due to memory contraints, when resuming from S3 the fw_cfg "files" API
>> isn't available.  This adds a simple API to get a file 'key', and to
>> write to the file using the key as a reference.
>> 
>> Signed-off-by: Ben Warren <ben@skyportsystems.com>
>> ---
>> src/fw/paravirt.c | 32 ++++++++++++++++++++++++++------
>> src/fw/paravirt.h |  2 ++
>> 2 files changed, 28 insertions(+), 6 deletions(-)
>> 
>> diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
>> index 4618647..225b08b 100644
>> --- a/src/fw/paravirt.c
>> +++ b/src/fw/paravirt.c
>> @@ -329,6 +329,17 @@ qemu_cfg_read_file(struct romfile_s *file, void *dst, u32 maxlen)
>>     return file->size;
>> }
>> 
>> +// Bare-bones function for writing a file knowing only its unique
>> +// identifying key (select)
>> +int
>> +qemu_cfg_write_file_simple(void *src, u16 key, u32 offset, u32 len)
>> +{
>> +    qemu_cfg_select(key);
>> +    qemu_cfg_skip(offset);
>> +    qemu_cfg_write(src, len);
>> +    return len;
>> +}
>> +
>> int
>> qemu_cfg_write_file(void *src, struct romfile_s *file, u32 offset, u32 len)
>> {
>> @@ -339,15 +350,12 @@ qemu_cfg_write_file(void *src, struct romfile_s *file, u32 offset, u32 len)
>>         warn_internalerror();
>>         return -1;
>>     }
>> -    struct qemu_romfile_s *qfile;
>> -    qfile = container_of(file, struct qemu_romfile_s, file);
>> +    u16 key = qemu_get_romfile_key(file);
>>     if (offset == 0) {
>>         /* Do it in one transfer */
>> -        qemu_cfg_write_entry(src, qfile->select, len);
>> +        qemu_cfg_write_entry(src, key, len);
>>     } else {
>> -        qemu_cfg_select(qfile->select);
>> -        qemu_cfg_skip(offset);
>> -        qemu_cfg_write(src, len);
>> +        qemu_cfg_write_file_simple(src, key, offset, len);
>>     }
>>     return len;
>> }
> 
> One of the ideas that I mentioned here was to move not just the second
> branch of the "if" to qemu_cfg_write_file_simple(), but the entire "if"
> -- both branches. Because, qemu_cfg_write_entry() looks suitable for S3
> too, and if that kind of optimization makes sense for normal boot, then
> it makes sense for S3 resume as well.
> 
> Anyway, this is not a functional problem, I won't obsess about it.
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com <mailto:lersek@redhat.com>>
> 
Sorry, I misinterpreted your idea.  It makes a lot of sense, so we may as well do it now.  New patch coming.
> Thanks
> Laszlo
> 
> 
—Ben
<snip>_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.