Re: [SeaBIOS] [PATCH v5 3/5] QEMU fw_cfg: Add command to write back address of file

[Laszlo Ersek]

On 02/18/17 07:21, [e-mail] wrote:
> From: Ben Warren
>
> This command is similar to ADD_POINTER, but instead of patching
> memory, it writes the pointer back to QEMU over the DMA interface.
> Signed-off-by: Ben Warren
>
> src/fw/romfile_loader.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  src/fw/romfile_loader.h | 23 ++++++++++++++++++++---
>  2 files changed, 65 insertions(+), 3 deletions(-)

Reviewed-by: Laszlo Ersek

Thanks
Laszlo
> diff --git a/src/fw/romfile_loader.c b/src/fw/romfile_loader.c
> index 7737453..30e7b58 100644
> --- a/src/fw/romfile_loader.c
> +++ b/src/fw/romfile_loader.c
> @@ -5,6 +5,7 @@
>  #include "romfile.h" // struct romfile_s
>  #include "malloc.h" // Zone*, _malloc
>  #include "output.h" // warn_*
> +#include "paravirt.h" // qemu_cfg_write_file
>
> struct romfile_loader_file {
>      struct romfile_s *file;
> @@ -127,6 +128,46 @@ err:
>      warn_internalerror();
>  }
>
> +static void romfile_loader_write_pointer(struct romfile_loader_entry_s *entry,
>                                     struct romfile_loader_files *files)
>
> +{
> struct romfile_s *dest_file;
> struct romfile_loader_file *src_file;
> unsigned dst_offset = le32_to_cpu(entry->wr_pointer.dst_offset);
> unsigned src_offset = le32_to_cpu(entry->wr_pointer.src_offset);
> u64 pointer = 0;
> /* Writing back to a file that may not be loaded in RAM */
> dest_file = romfile_find(entry->wr_pointer.dest_file);
> src_file = romfile_loader_find(entry->wr_pointer.src_file, files);
> if (!dest_file || !src_file || !src_file->data ||
>    dst_offset + entry->wr_pointer.size < dst_offset ||
>    dst_offset + entry->wr_pointer.size > dest_file->size ||
>    src_offset >= src_file->file->size ||
>    entry->wr_pointer.size < 1 || entry->wr_pointer.size > 8 ||
>    entry->wr_pointer.size & (entry->wr_pointer.size - 1)) {
>    goto err;
> }
> pointer = (unsigned long)src_file->data + src_offset;
> /* Make sure the pointer fits within wr_pointer.size */
> if ((entry->wr_pointer.size != sizeof(u64)) &&
>    ((pointer >> (entry->wr_pointer.size * 8)) > 0)) {
>    goto err;
> }
> pointer = cpu_to_le64(pointer);
> /* Only supported on QEMU */
> if (qemu_cfg_write_file(&pointer, dest_file, dst_offset,
>                        entry->wr_pointer.size) != entry->wr_pointer.size) {
>    goto err;
> }
> return;
> err:
> warn_internalerror();
>
> +}
>
> int romfile_loader_execute(const char *name)
>  {
>      struct romfile_loader_entry_s *entry;
> @@ -161,6 +202,10 @@ int romfile_loader_execute(const char *name)
>                          break;
>                  case ROMFILE_LOADER_COMMAND_ADD_CHECKSUM:
>                          romfile_loader_add_checksum(entry, files);
>                    break;
>            case ROMFILE_LOADER_COMMAND_WRITE_POINTER:
>                    romfile_loader_write_pointer(entry, files);
>                    break;
>            default:
>                    /* Skip commands that we don't recognize. */
>                    break;
>
> diff --git a/src/fw/romfile_loader.h b/src/fw/romfile_loader.h
> index bce3719..4dc50ab 100644
> --- a/src/fw/romfile_loader.h
> +++ b/src/fw/romfile_loader.h
> @@ -51,15 +51,32 @@ struct romfile_loader_entry_s {
>              u32 length;
>          } cksum;
>    /*
>     * COMMAND_WRITE_POINTER - Write back to a host file via DMA,
>     * @wr_pointer.dest_file at offset @wr_pointer.dst_offset, a pointer
>     * to the table originating from @wr_pointer.src_file at offset
>     * @wr_pointer.src_offset.
>     * 1,2,4 or 8 byte unsigned addition is used depending on
>     * @wr_pointer.size.
>     */
>    struct {
>        char dest_file[ROMFILE_LOADER_FILESZ];
>        char src_file[ROMFILE_LOADER_FILESZ];
>        u32 dst_offset;
>        u32 src_offset;
>        u8 size;
>    } wr_pointer;
>    /* padding */
>    char pad[124];
> };
>
> };
>
> enum {
> ROMFILE_LOADER_COMMAND_ALLOCATE     = 0x1,
> ROMFILE_LOADER_COMMAND_ADD_POINTER  = 0x2,
> ROMFILE_LOADER_COMMAND_ADD_CHECKSUM = 0x3,
> ROMFILE_LOADER_COMMAND_ALLOCATE      = 0x1,
> ROMFILE_LOADER_COMMAND_ADD_POINTER   = 0x2,
> ROMFILE_LOADER_COMMAND_ADD_CHECKSUM  = 0x3,
> ROMFILE_LOADER_COMMAND_WRITE_POINTER = 0x4,
>
> };
>
> enum {

[PATCH v5 3/5] QEMU fw_cfg: Add command to write back address of file

[Igor Mammedov]

On Mon, 20 Feb 2017 10:43:46 +0100
Laszlo Ersek  wrote:
> On 02/18/17 07:21, [e-mail] wrote:
> > From: Ben Warren
> >
> > This command is similar to ADD_POINTER, but instead of patching
> > memory, it writes the pointer back to QEMU over the DMA interface.
> > Signed-off-by: Ben Warren
> >
> > src/fw/romfile_loader.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> >  src/fw/romfile_loader.h | 23 ++++++++++++++++++++---
> >  2 files changed, 65 insertions(+), 3 deletions(-)
>
> Reviewed-by: Laszlo Ersek

Reviewed-by: Igor Mammedov
> Thanks
> Laszlo
> > diff --git a/src/fw/romfile_loader.c b/src/fw/romfile_loader.c
> > index 7737453..30e7b58 100644
> > --- a/src/fw/romfile_loader.c
> > +++ b/src/fw/romfile_loader.c
> > @@ -5,6 +5,7 @@
> >  #include "romfile.h" // struct romfile_s
> >  #include "malloc.h" // Zone*, _malloc
> >  #include "output.h" // warn_*
> > +#include "paravirt.h" // qemu_cfg_write_file
> >
> > struct romfile_loader_file {
> >      struct romfile_s *file;
> > @@ -127,6 +128,46 @@ err:
> >      warn_internalerror();
> >  }
> >
> > +static void romfile_loader_write_pointer(struct romfile_loader_entry_s *entry,
> >                                     struct romfile_loader_files *files)
> >
> > +{
> > struct romfile_s *dest_file;
> > struct romfile_loader_file *src_file;
> > unsigned dst_offset = le32_to_cpu(entry->wr_pointer.dst_offset);
> > unsigned src_offset = le32_to_cpu(entry->wr_pointer.src_offset);
> > u64 pointer = 0;
> > /* Writing back to a file that may not be loaded in RAM */
> > dest_file = romfile_find(entry->wr_pointer.dest_file);
> > src_file = romfile_loader_find(entry->wr_pointer.src_file, files);
> > if (!dest_file || !src_file || !src_file->data ||
> >    dst_offset + entry->wr_pointer.size < dst_offset ||
> >    dst_offset + entry->wr_pointer.size > dest_file->size ||
> >    src_offset >= src_file->file->size ||
> >    entry->wr_pointer.size < 1 || entry->wr_pointer.size > 8 ||
> >    entry->wr_pointer.size & (entry->wr_pointer.size - 1)) {
> >    goto err;
> > }
> > pointer = (unsigned long)src_file->data + src_offset;
> > /* Make sure the pointer fits within wr_pointer.size */
> > if ((entry->wr_pointer.size != sizeof(u64)) &&
> >    ((pointer >> (entry->wr_pointer.size * 8)) > 0)) {
> >    goto err;
> > }
> > pointer = cpu_to_le64(pointer);
> > /* Only supported on QEMU */
> > if (qemu_cfg_write_file(&pointer, dest_file, dst_offset,
> >                        entry->wr_pointer.size) != entry->wr_pointer.size) {
> >    goto err;
> > }
> > return;
> > err:
> > warn_internalerror();
> >
> > +}
> >
> > int romfile_loader_execute(const char *name)
> >  {
> >      struct romfile_loader_entry_s *entry;
> > @@ -161,6 +202,10 @@ int romfile_loader_execute(const char *name)
> >                          break;
> >                  case ROMFILE_LOADER_COMMAND_ADD_CHECKSUM:
> >                          romfile_loader_add_checksum(entry, files);
> >                    break;
> >            case ROMFILE_LOADER_COMMAND_WRITE_POINTER:
> >                    romfile_loader_write_pointer(entry, files);
> >                    break;
> >            default:
> >                    /* Skip commands that we don't recognize. */
> >                    break;
> >
> > diff --git a/src/fw/romfile_loader.h b/src/fw/romfile_loader.h
> > index bce3719..4dc50ab 100644
> > --- a/src/fw/romfile_loader.h
> > +++ b/src/fw/romfile_loader.h
> > @@ -51,15 +51,32 @@ struct romfile_loader_entry_s {
> >              u32 length;
> >          } cksum;
> >    /*
> >     * COMMAND_WRITE_POINTER - Write back to a host file via DMA,
> >     * @wr_pointer.dest_file at offset @wr_pointer.dst_offset, a pointer
> >     * to the table originating from @wr_pointer.src_file at offset
> >     * @wr_pointer.src_offset.
> >     * 1,2,4 or 8 byte unsigned addition is used depending on
> >     * @wr_pointer.size.
> >     */
> >    struct {
> >        char dest_file[ROMFILE_LOADER_FILESZ];
> >        char src_file[ROMFILE_LOADER_FILESZ];
> >        u32 dst_offset;
> >        u32 src_offset;
> >        u8 size;
> >    } wr_pointer;
> >    /* padding */
> >    char pad[124];
> > };
> >
> > };
> >
> > enum {
> > ROMFILE_LOADER_COMMAND_ALLOCATE     = 0x1,
> > ROMFILE_LOADER_COMMAND_ADD_POINTER  = 0x2,
> > ROMFILE_LOADER_COMMAND_ADD_CHECKSUM = 0x3,
> > ROMFILE_LOADER_COMMAND_ALLOCATE      = 0x1,
> > ROMFILE_LOADER_COMMAND_ADD_POINTER   = 0x2,
> > ROMFILE_LOADER_COMMAND_ADD_CHECKSUM  = 0x3,
> > ROMFILE_LOADER_COMMAND_WRITE_POINTER = 0x4,
> >
> > };
> >
> > enum {
>
> SeaBIOS mailing list
> [e-mail]
> https://www.coreboot.org/mailman/listinfo/seabios