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

[SeaBIOS] [PATCH v3 1/2] QEMU DMA: Add DMA write capability

ben@skyportsystems.com posted 2 patches 8 years, 3 months ago
There is a newer version of this series
[SeaBIOS] [PATCH v3 1/2] QEMU DMA: Add DMA write capability
Posted by ben@skyportsystems.com 8 years, 3 months ago 
From: Ben Warren <ben@skyportsystems.com>

This allows BIOS to write data back to QEMU using the DMA interface and
provides a higher-level abstraction to write to a fw_cfg file

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

diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
index 6de70f6..75cb992 100644
--- a/src/fw/paravirt.c
+++ b/src/fw/paravirt.c
@@ -253,6 +253,20 @@ qemu_cfg_read(void *buf, int len)
 }
 
 static void
+qemu_cfg_write(void *buf, int len)
+{
+    if (len == 0) {
+        return;
+    }
+
+    if (qemu_cfg_dma_enabled()) {
+        qemu_cfg_dma_transfer(buf, len, QEMU_CFG_DMA_CTL_WRITE);
+    } else {
+        warn_internalerror();
+    }
+}
+
+static void
 qemu_cfg_skip(int len)
 {
     if (len == 0) {
@@ -280,6 +294,18 @@ qemu_cfg_read_entry(void *buf, int e, int len)
     }
 }
 
+static void
+qemu_cfg_write_entry(void *buf, int e, int len)
+{
+    if (qemu_cfg_dma_enabled()) {
+        u32 control = (e << 16) | QEMU_CFG_DMA_CTL_SELECT
+                        | QEMU_CFG_DMA_CTL_WRITE;
+        qemu_cfg_dma_transfer(buf, len, control);
+    } else {
+        warn_internalerror();
+    }
+}
+
 struct qemu_romfile_s {
     struct romfile_s file;
     int select, skip;
@@ -303,6 +329,29 @@ qemu_cfg_read_file(struct romfile_s *file, void *dst, u32 maxlen)
     return file->size;
 }
 
+int
+qemu_cfg_write_file(void *src, struct romfile_s *file, u32 offset, u32 len)
+{
+    if ((file->size + offset) < len)
+        return -1;
+
+    if (!qemu_cfg_dma_enabled() || (file->copy != qemu_cfg_read_file)) {
+        warn_internalerror();
+        return -1;
+    }
+    struct qemu_romfile_s *qfile;
+    qfile = container_of(file, struct qemu_romfile_s, file);
+    if (offset == 0) {
+        /* Do it in one transfer */
+        qemu_cfg_write_entry(src, qfile->select, len);
+    } else {
+        qemu_cfg_select(qfile->select);
+        qemu_cfg_skip(offset);
+        qemu_cfg_write(src, len);
+    }
+    return len;
+}
+
 static void
 qemu_romfile_add(char *name, int select, int skip, int size)
 {
diff --git a/src/fw/paravirt.h b/src/fw/paravirt.h
index d8eb7c4..fb220d8 100644
--- a/src/fw/paravirt.h
+++ b/src/fw/paravirt.h
@@ -3,6 +3,7 @@
 
 #include "config.h" // CONFIG_*
 #include "biosvar.h" // GET_GLOBAL
+#include "romfile.h" // struct romfile_s
 
 // Types of paravirtualized platforms.
 #define PF_QEMU     (1<<0)
@@ -43,6 +44,7 @@ static inline int runningOnKVM(void) {
 #define QEMU_CFG_DMA_CTL_READ    0x02
 #define QEMU_CFG_DMA_CTL_SKIP    0x04
 #define QEMU_CFG_DMA_CTL_SELECT  0x08
+#define QEMU_CFG_DMA_CTL_WRITE   0x10
 
 // QEMU_CFG_DMA ID bit
 #define QEMU_CFG_VERSION_DMA    2
@@ -53,5 +55,6 @@ void qemu_platform_setup(void);
 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);
 
 #endif
-- 
2.7.4


_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v3 1/2] QEMU DMA: Add DMA write capability
Posted by Laszlo Ersek 8 years, 3 months ago 
On 02/05/17 18:09, ben@skyportsystems.com wrote:
> From: Ben Warren <ben@skyportsystems.com>
> 
> This allows BIOS to write data back to QEMU using the DMA interface and
> provides a higher-level abstraction to write to a fw_cfg file
> 
> Signed-off-by: Ben Warren <ben@skyportsystems.com>
> ---
>  src/fw/paravirt.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  src/fw/paravirt.h |  3 +++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
> index 6de70f6..75cb992 100644
> --- a/src/fw/paravirt.c
> +++ b/src/fw/paravirt.c
> @@ -253,6 +253,20 @@ qemu_cfg_read(void *buf, int len)
>  }
>  
>  static void
> +qemu_cfg_write(void *buf, int len)
> +{
> +    if (len == 0) {
> +        return;
> +    }
> +
> +    if (qemu_cfg_dma_enabled()) {
> +        qemu_cfg_dma_transfer(buf, len, QEMU_CFG_DMA_CTL_WRITE);
> +    } else {
> +        warn_internalerror();
> +    }
> +}
> +
> +static void
>  qemu_cfg_skip(int len)
>  {
>      if (len == 0) {
> @@ -280,6 +294,18 @@ qemu_cfg_read_entry(void *buf, int e, int len)
>      }
>  }
>  
> +static void
> +qemu_cfg_write_entry(void *buf, int e, int len)
> +{
> +    if (qemu_cfg_dma_enabled()) {
> +        u32 control = (e << 16) | QEMU_CFG_DMA_CTL_SELECT
> +                        | QEMU_CFG_DMA_CTL_WRITE;
> +        qemu_cfg_dma_transfer(buf, len, control);
> +    } else {
> +        warn_internalerror();
> +    }
> +}
> +
>  struct qemu_romfile_s {
>      struct romfile_s file;
>      int select, skip;
> @@ -303,6 +329,29 @@ qemu_cfg_read_file(struct romfile_s *file, void *dst, u32 maxlen)
>      return file->size;
>  }
>  
> +int
> +qemu_cfg_write_file(void *src, struct romfile_s *file, u32 offset, u32 len)
> +{
> +    if ((file->size + offset) < len)
> +        return -1;

I'm confused about this; mathematically, we want to require

  offset + len <= size

If we want to avoid any possibility for overflow in the addition, we can
subtract "len":

  offset <= size - len

However, in this case the subtraction needs to be protected, and we end
up with the following condition to proceed:

  size >= len && offset <= size - len

If we want a condition for bailing out early, we got to negate the above:

  size < len || offset > size - len

The rest looks good to me.

Thanks
Laszlo

> +
> +    if (!qemu_cfg_dma_enabled() || (file->copy != qemu_cfg_read_file)) {
> +        warn_internalerror();
> +        return -1;
> +    }
> +    struct qemu_romfile_s *qfile;
> +    qfile = container_of(file, struct qemu_romfile_s, file);
> +    if (offset == 0) {
> +        /* Do it in one transfer */
> +        qemu_cfg_write_entry(src, qfile->select, len);
> +    } else {
> +        qemu_cfg_select(qfile->select);
> +        qemu_cfg_skip(offset);
> +        qemu_cfg_write(src, len);
> +    }
> +    return len;
> +}
> +
>  static void
>  qemu_romfile_add(char *name, int select, int skip, int size)
>  {
> diff --git a/src/fw/paravirt.h b/src/fw/paravirt.h
> index d8eb7c4..fb220d8 100644
> --- a/src/fw/paravirt.h
> +++ b/src/fw/paravirt.h
> @@ -3,6 +3,7 @@
>  
>  #include "config.h" // CONFIG_*
>  #include "biosvar.h" // GET_GLOBAL
> +#include "romfile.h" // struct romfile_s
>  
>  // Types of paravirtualized platforms.
>  #define PF_QEMU     (1<<0)
> @@ -43,6 +44,7 @@ static inline int runningOnKVM(void) {
>  #define QEMU_CFG_DMA_CTL_READ    0x02
>  #define QEMU_CFG_DMA_CTL_SKIP    0x04
>  #define QEMU_CFG_DMA_CTL_SELECT  0x08
> +#define QEMU_CFG_DMA_CTL_WRITE   0x10
>  
>  // QEMU_CFG_DMA ID bit
>  #define QEMU_CFG_VERSION_DMA    2
> @@ -53,5 +55,6 @@ void qemu_platform_setup(void);
>  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);
>  
>  #endif
> 


_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v3 1/2] QEMU DMA: Add DMA write capability
Posted by Ben Warren 8 years, 3 months ago 
> On Feb 8, 2017, at 4:13 AM, Laszlo Ersek <lersek@redhat.com> wrote:
> 
> On 02/05/17 18:09, ben@skyportsystems.com <mailto:ben@skyportsystems.com> wrote:
>> From: Ben Warren <ben@skyportsystems.com>
>> 
>> This allows BIOS to write data back to QEMU using the DMA interface and
>> provides a higher-level abstraction to write to a fw_cfg file
>> 
>> Signed-off-by: Ben Warren <ben@skyportsystems.com>
>> ---
>> src/fw/paravirt.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>> src/fw/paravirt.h |  3 +++
>> 2 files changed, 52 insertions(+)
>> 
>> diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
>> index 6de70f6..75cb992 100644
>> --- a/src/fw/paravirt.c
>> +++ b/src/fw/paravirt.c
>> @@ -253,6 +253,20 @@ qemu_cfg_read(void *buf, int len)
>> }
>> 
>> static void
>> +qemu_cfg_write(void *buf, int len)
>> +{
>> +    if (len == 0) {
>> +        return;
>> +    }
>> +
>> +    if (qemu_cfg_dma_enabled()) {
>> +        qemu_cfg_dma_transfer(buf, len, QEMU_CFG_DMA_CTL_WRITE);
>> +    } else {
>> +        warn_internalerror();
>> +    }
>> +}
>> +
>> +static void
>> qemu_cfg_skip(int len)
>> {
>>     if (len == 0) {
>> @@ -280,6 +294,18 @@ qemu_cfg_read_entry(void *buf, int e, int len)
>>     }
>> }
>> 
>> +static void
>> +qemu_cfg_write_entry(void *buf, int e, int len)
>> +{
>> +    if (qemu_cfg_dma_enabled()) {
>> +        u32 control = (e << 16) | QEMU_CFG_DMA_CTL_SELECT
>> +                        | QEMU_CFG_DMA_CTL_WRITE;
>> +        qemu_cfg_dma_transfer(buf, len, control);
>> +    } else {
>> +        warn_internalerror();
>> +    }
>> +}
>> +
>> struct qemu_romfile_s {
>>     struct romfile_s file;
>>     int select, skip;
>> @@ -303,6 +329,29 @@ qemu_cfg_read_file(struct romfile_s *file, void *dst, u32 maxlen)
>>     return file->size;
>> }
>> 
>> +int
>> +qemu_cfg_write_file(void *src, struct romfile_s *file, u32 offset, u32 len)
>> +{
>> +    if ((file->size + offset) < len)
>> +        return -1;
> 
> I'm confused about this; mathematically, we want to require
> 
>  offset + len <= size
> 
> If we want to avoid any possibility for overflow in the addition, we can
> subtract "len":
> 
>  offset <= size - len
> 
> However, in this case the subtraction needs to be protected, and we end
> up with the following condition to proceed:
> 
>  size >= len && offset <= size - len
> 
> If we want a condition for bailing out early, we got to negate the above:
> 
>  size < len || offset > size - len
> 
> The rest looks good to me.
> 
This was dumb.  I meant to do:

if ((offset + len) > file->size) {
    return -1;
}

but got mixed up.  Will fix.
> Thanks
> Laszlo
> 
>> +
>> +    if (!qemu_cfg_dma_enabled() || (file->copy != qemu_cfg_read_file)) {
>> +        warn_internalerror();
>> +        return -1;
>> +    }
>> +    struct qemu_romfile_s *qfile;
>> +    qfile = container_of(file, struct qemu_romfile_s, file);
>> +    if (offset == 0) {
>> +        /* Do it in one transfer */
>> +        qemu_cfg_write_entry(src, qfile->select, len);
>> +    } else {
>> +        qemu_cfg_select(qfile->select);
>> +        qemu_cfg_skip(offset);
>> +        qemu_cfg_write(src, len);
>> +    }
>> +    return len;
>> +}
>> +
>> static void
>> qemu_romfile_add(char *name, int select, int skip, int size)
>> {
>> diff --git a/src/fw/paravirt.h b/src/fw/paravirt.h
>> index d8eb7c4..fb220d8 100644
>> --- a/src/fw/paravirt.h
>> +++ b/src/fw/paravirt.h
>> @@ -3,6 +3,7 @@
>> 
>> #include "config.h" // CONFIG_*
>> #include "biosvar.h" // GET_GLOBAL
>> +#include "romfile.h" // struct romfile_s
>> 
>> // Types of paravirtualized platforms.
>> #define PF_QEMU     (1<<0)
>> @@ -43,6 +44,7 @@ static inline int runningOnKVM(void) {
>> #define QEMU_CFG_DMA_CTL_READ    0x02
>> #define QEMU_CFG_DMA_CTL_SKIP    0x04
>> #define QEMU_CFG_DMA_CTL_SELECT  0x08
>> +#define QEMU_CFG_DMA_CTL_WRITE   0x10
>> 
>> // QEMU_CFG_DMA ID bit
>> #define QEMU_CFG_VERSION_DMA    2
>> @@ -53,5 +55,6 @@ void qemu_platform_setup(void);
>> 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);
>> 
>> #endif

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.