Patchwork [v3,06/14] i386: add bios linker/loader

login
register
mail settings
Submitter Michael S. Tsirkin
Date July 24, 2013, 4:01 p.m.
Message ID <1374681580-17439-7-git-send-email-mst@redhat.com>
Download mbox | patch
Permalink /patch/261442/
State New
Headers show

Comments

Michael S. Tsirkin - July 24, 2013, 4:01 p.m.
This adds a dynamic bios linker/loader.
This will be used by acpi table generation
code to:
    - load each table in the appropriate memory segment
    - link tables to each other
    - fix up checksums after said linking

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/Makefile.objs                |   1 +
 hw/i386/bios-linker-loader.c         | 156 +++++++++++++++++++++++++++++++++++
 include/hw/i386/bios-linker-loader.h |  26 ++++++
 3 files changed, 183 insertions(+)
 create mode 100644 hw/i386/bios-linker-loader.c
 create mode 100644 include/hw/i386/bios-linker-loader.h
Gerd Hoffmann - July 25, 2013, 12:11 p.m.
On 07/24/13 18:01, Michael S. Tsirkin wrote:
> This adds a dynamic bios linker/loader.
> This will be used by acpi table generation
> code to:
>     - load each table in the appropriate memory segment
>     - link tables to each other
>     - fix up checksums after said linking

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

cheers,
  Gerd
Gerd Hoffmann - July 26, 2013, 9:42 a.m.
Hi,

Hacked up coreboot support to verify the interface.

> +        /*
> +         * COMMAND_ALLOCATE - allocate a table from @alloc_file
> +         * subject to @alloc_align alignment (must be power of 2)
> +         * and @alloc_zone (can be HIGH or FSEG) requirements.
> +         *
> +         * Must appear exactly once for each file, and before
> +         * this file is referenced by any other command.
> +         */

Do we need alloc_zone?

Rules are simple: rsdp goes to fseg, everything else to high, correct?

Both seabios and coreboot handle that by first placing *all* acpi tables
in high memory, then go place a rsdp copy in fseg.

> +        /*
> +         * COMMAND_ADD_POINTER - patch the table (originating from
> +         * @dest_file) at @pointer_offset, by adding a pointer to the table
> +         * originating from @src_file. 1,2,4 or 8 byte unsigned
> +         * addition is used depending on @pointer_size.
> +         */

Do we need pointer_size?

Current tables use 4 bytes only.

1+2 bytes is pointless, the src_file address will be big enougth that it
doesn't fit into one or two bytes, so I doubt they will ever be used.

8 bytes would only be used when placing acpi tables above 4g.  Is that
something expected to happen?  If not 4 bytes is enough, even for 8 byte
pointers as the 4 high bytes would be zero no matter what.

cheers,
  Gerd
Michael S. Tsirkin - July 28, 2013, 8:08 a.m.
On Fri, Jul 26, 2013 at 11:42:45AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> Hacked up coreboot support to verify the interface.
> 
> > +        /*
> > +         * COMMAND_ALLOCATE - allocate a table from @alloc_file
> > +         * subject to @alloc_align alignment (must be power of 2)
> > +         * and @alloc_zone (can be HIGH or FSEG) requirements.
> > +         *
> > +         * Must appear exactly once for each file, and before
> > +         * this file is referenced by any other command.
> > +         */
> 
> Do we need alloc_zone?
> 
> Rules are simple: rsdp goes to fseg, everything else to high, correct?

Yes but I prefer marking it explicitly at link time and avoid hard-coding
things in the bios, so that whoever allocates memory can go just by
linker rules and not poke at tables themselves.

> Both seabios and coreboot handle that by first placing *all* acpi tables
> in high memory, then go place a rsdp copy in fseg.

No, what happens is we put everything in tmp memory, then
copy where appropriately.

> > +        /*
> > +         * COMMAND_ADD_POINTER - patch the table (originating from
> > +         * @dest_file) at @pointer_offset, by adding a pointer to the table
> > +         * originating from @src_file. 1,2,4 or 8 byte unsigned
> > +         * addition is used depending on @pointer_size.
> > +         */
> 
> Do we need pointer_size?
> 
> Current tables use 4 bytes only.
> 
> 1+2 bytes is pointless, the src_file address will be big enougth that it
> doesn't fit into one or two bytes, so I doubt they will ever be used.
> 
> 8 bytes would only be used when placing acpi tables above 4g.  Is that
> something expected to happen?

Yes, the spec allows 64 bit pointers.

> If not 4 bytes is enough, even for 8 byte
> pointers as the 4 high bytes would be zero no matter what.
> 
> cheers,
>   Gerd

Patch

diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 013d250..71be2da 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -4,6 +4,7 @@  obj-y += pc.o pc_piix.o pc_q35.o
 obj-$(CONFIG_XEN) += xen_domainbuild.o xen_machine_pv.o
 
 obj-y += kvmvapic.o
+obj-y += bios-linker-loader.o
 
 iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
     ; then echo "$(2)"; else echo "$(3)"; fi ;)
diff --git a/hw/i386/bios-linker-loader.c b/hw/i386/bios-linker-loader.c
new file mode 100644
index 0000000..644016b
--- /dev/null
+++ b/hw/i386/bios-linker-loader.c
@@ -0,0 +1,156 @@ 
+/* Dynamic linker/loader of ACPI tables
+ *
+ * Copyright (C) 2013 Red Hat Inc
+ *
+ * Author: Michael S. Tsirkin <mst@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "hw/i386/bios-linker-loader.h"
+#include "hw/nvram/fw_cfg.h"
+
+#include <string.h>
+#include <assert.h>
+#include "qemu/bswap.h"
+
+#define BIOS_LINKER_LOADER_FILESZ FW_CFG_MAX_FILE_PATH
+
+struct BiosLinkerLoaderEntry {
+    uint32_t command;
+    union {
+        /*
+         * COMMAND_ALLOCATE - allocate a table from @alloc_file
+         * subject to @alloc_align alignment (must be power of 2)
+         * and @alloc_zone (can be HIGH or FSEG) requirements.
+         *
+         * Must appear exactly once for each file, and before
+         * this file is referenced by any other command.
+         */
+        struct {
+            char alloc_file[BIOS_LINKER_LOADER_FILESZ];
+            uint32_t alloc_align;
+            uint8_t alloc_zone;
+        };
+
+        /*
+         * COMMAND_ADD_POINTER - patch the table (originating from
+         * @dest_file) at @pointer_offset, by adding a pointer to the table
+         * originating from @src_file. 1,2,4 or 8 byte unsigned
+         * addition is used depending on @pointer_size.
+         */
+        struct {
+            char pointer_dest_file[BIOS_LINKER_LOADER_FILESZ];
+            char pointer_src_file[BIOS_LINKER_LOADER_FILESZ];
+            uint32_t pointer_offset;
+            uint8_t pointer_size;
+        };
+
+        /*
+         * COMMAND_ADD_CHECKSUM - calculate checksum of the range specified by
+         * @cksum_start and @cksum_length fields,
+         * and then add the value at @cksum_offset.
+         * Checksum simply sums -X for each byte X in the range
+         * using 8-bit math.
+         */
+        struct {
+            char cksum_file[BIOS_LINKER_LOADER_FILESZ];
+            uint32_t cksum_offset;
+            uint32_t cksum_start;
+            uint32_t cksum_length;
+        };
+
+        /* padding */
+        char pad[124];
+    };
+} QEMU_PACKED;
+typedef struct BiosLinkerLoaderEntry BiosLinkerLoaderEntry;
+
+enum {
+    BIOS_LINKER_LOADER_COMMAND_ALLOCATE     = 0x1,
+    BIOS_LINKER_LOADER_COMMAND_ADD_POINTER  = 0x2,
+    BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM = 0x3,
+};
+
+enum {
+    BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH = 0x1,
+    BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG = 0x2,
+};
+
+GArray *bios_linker_init(void)
+{
+    return g_array_new(false, true /* clear */, sizeof(BiosLinkerLoaderEntry));
+}
+
+/* Free linker wrapper and return the linker array. */
+void *bios_linker_cleanup(GArray *linker)
+{
+    return g_array_free(linker, false);
+}
+
+void bios_linker_alloc(GArray *linker,
+                       const char *file,
+                       uint32_t alloc_align,
+                       bool alloc_fseg)
+{
+    BiosLinkerLoaderEntry entry;
+
+    memset(&entry, 0, sizeof entry);
+    strncpy(entry.alloc_file, file, sizeof entry.alloc_file - 1);
+    entry.command = cpu_to_le32(BIOS_LINKER_LOADER_COMMAND_ALLOCATE);
+    entry.alloc_align = cpu_to_le32(alloc_align);
+    entry.alloc_zone = cpu_to_le32(alloc_fseg ?
+                                    BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG :
+                                    BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH);
+
+    /* Alloc entries must come first, so prepend them */
+    g_array_prepend_val(linker, entry);
+}
+
+void bios_linker_add_checksum(GArray *linker, const char *file, void *table,
+                              void *start, unsigned size, uint8_t *checksum)
+{
+    BiosLinkerLoaderEntry entry;
+
+    memset(&entry, 0, sizeof entry);
+    strncpy(entry.cksum_file, file, sizeof entry.cksum_file - 1);
+    entry.command = cpu_to_le32(BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM);
+    entry.cksum_offset = cpu_to_le32(checksum - (uint8_t *)table);
+    entry.cksum_start = cpu_to_le32((uint8_t *)start - (uint8_t *)table);
+    entry.cksum_length = cpu_to_le32(size);
+
+    g_array_append_val(linker, entry);
+}
+
+void bios_linker_add_pointer(GArray *linker,
+                             const char *dest_file,
+                             const char *src_file,
+                             GArray *table, void *pointer,
+                             uint8_t pointer_size)
+{
+    BiosLinkerLoaderEntry entry;
+
+    memset(&entry, 0, sizeof entry);
+    strncpy(entry.pointer_dest_file, dest_file,
+            sizeof entry.pointer_dest_file - 1);
+    strncpy(entry.pointer_src_file, src_file,
+            sizeof entry.pointer_src_file - 1);
+    entry.command = cpu_to_le32(BIOS_LINKER_LOADER_COMMAND_ADD_POINTER);
+    entry.pointer_offset = cpu_to_le32((gchar *)pointer - table->data);
+    entry.pointer_size = pointer_size;
+    assert(pointer_size == 1 || pointer_size == 2 ||
+           pointer_size == 4 || pointer_size == 8);
+
+    g_array_append_val(linker, entry);
+}
diff --git a/include/hw/i386/bios-linker-loader.h b/include/hw/i386/bios-linker-loader.h
new file mode 100644
index 0000000..18c3868
--- /dev/null
+++ b/include/hw/i386/bios-linker-loader.h
@@ -0,0 +1,26 @@ 
+#ifndef BIOS_LINKER_LOADER_H
+#define BIOS_LINKER_LOADER_H
+
+#include <glib.h>
+#include <stdbool.h>
+#include <inttypes.h>
+
+GArray *bios_linker_init(void);
+
+void bios_linker_alloc(GArray *linker,
+                       const char *file,
+                       uint32_t alloc_align,
+                       bool alloc_fseg);
+
+void bios_linker_add_checksum(GArray *linker, const char *file, void *table,
+                              void *start, unsigned size, uint8_t *checksum);
+
+
+void bios_linker_add_pointer(GArray *linker,
+                             const char *dest_file,
+                             const char *src_file,
+                             GArray *table, void *pointer,
+                             uint8_t pointer_size);
+
+void *bios_linker_cleanup(GArray *linker);
+#endif