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

[Ben Warren]

> On Feb 8, 2017, at 4:13 AM, Laszlo Ersek <lersek at redhat.com> wrote:
> 
> On 02/05/17 18:09, ben at skyportsystems.com <mailto:ben at skyportsystems.com> wrote:
>> From: Ben Warren <ben at 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 at 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.coreboot.org/pipermail/seabios/attachments/20170208/53735d28/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3583 bytes
Desc: not available
URL: <http://www.coreboot.org/pipermail/seabios/attachments/20170208/53735d28/attachment-0001.p7s>