[v2,for-2.11,1/2] tests/boot-sector: Do not overwrite the x86 buffer on other architectures

Submitted by Thomas Huth on Aug. 11, 2017, 5:57 a.m.

Details

Message ID 1502431076-22849-2-git-send-email-thuth@redhat.com
State New
Headers show

Commit Message

Thomas Huth Aug. 11, 2017, 5:57 a.m.
Re-using the boot_sector code buffer from x86 for other architectures
is not very nice, especially if we add more architectures later. It's
also ugly that the test uses a huge pre-initialized array at all - the
size of the executable is very huge due to this array. So let's use a
separate buffer for each architecture instead, allocated from the heap,
so that we really just use the memory that we need.

Suggested-by: Michael Tsirkin <mst@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/boot-sector.c | 41 ++++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)

Comments

Cornelia Huck Aug. 15, 2017, 2:54 p.m.
On Fri, 11 Aug 2017 07:57:55 +0200
Thomas Huth <thuth@redhat.com> wrote:

> Re-using the boot_sector code buffer from x86 for other architectures
> is not very nice, especially if we add more architectures later. It's
> also ugly that the test uses a huge pre-initialized array at all - the
> size of the executable is very huge due to this array. So let's use a
> separate buffer for each architecture instead, allocated from the heap,
> so that we really just use the memory that we need.
> 
> Suggested-by: Michael Tsirkin <mst@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/boot-sector.c | 41 ++++++++++++++++++++++++++---------------
>  1 file changed, 26 insertions(+), 15 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Patch hide | download patch | download mbox

diff --git a/tests/boot-sector.c b/tests/boot-sector.c
index e3880f4..8729562 100644
--- a/tests/boot-sector.c
+++ b/tests/boot-sector.c
@@ -21,13 +21,12 @@ 
 #define SIGNATURE 0xdead
 #define SIGNATURE_OFFSET 0x10
 #define BOOT_SECTOR_ADDRESS 0x7c00
+#define SIGNATURE_ADDR (BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET)
 
-/* Boot sector code: write SIGNATURE into memory,
+/* x86 boot sector code: write SIGNATURE into memory,
  * then halt.
- * Q35 machine requires a minimum 0x7e000 bytes disk.
- * (bug or feature?)
  */
-static uint8_t boot_sector[0x7e000] = {
+static uint8_t x86_boot_sector[512] = {
     /* The first sector will be placed at RAM address 00007C00, and
      * the BIOS transfers control to 00007C00
      */
@@ -50,8 +49,8 @@  static uint8_t boot_sector[0x7e000] = {
     [0x07] = HIGH(SIGNATURE),
     /* 7c08:  mov %ax,0x7c10 */
     [0x08] = 0xa3,
-    [0x09] = LOW(BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET),
-    [0x0a] = HIGH(BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET),
+    [0x09] = LOW(SIGNATURE_ADDR),
+    [0x0a] = HIGH(SIGNATURE_ADDR),
 
     /* 7c0b cli */
     [0x0b] = 0xfa,
@@ -72,7 +71,9 @@  static uint8_t boot_sector[0x7e000] = {
 int boot_sector_init(char *fname)
 {
     int fd, ret;
-    size_t len = sizeof boot_sector;
+    size_t len;
+    char *boot_code;
+    const char *arch = qtest_get_arch();
 
     fd = mkstemp(fname);
     if (fd < 0) {
@@ -80,16 +81,26 @@  int boot_sector_init(char *fname)
         return 1;
     }
 
-    /* For Open Firmware based system, we can use a Forth script instead */
-    if (strcmp(qtest_get_arch(), "ppc64") == 0) {
-        len = sprintf((char *)boot_sector, "\\ Bootscript\n%x %x c! %x %x c!\n",
-                LOW(SIGNATURE), BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET,
-                HIGH(SIGNATURE), BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET + 1);
+    if (g_str_equal(arch, "i386") || g_str_equal(arch, "x86_64")) {
+        /* Q35 requires a minimum 0x7e000 bytes disk (bug or feature?) */
+        len = MAX(0x7e000, sizeof(x86_boot_sector));
+        boot_code = g_malloc0(len);
+        memcpy(boot_code, x86_boot_sector, sizeof(x86_boot_sector));
+    } else if (g_str_equal(arch, "ppc64")) {
+        /* For Open Firmware based system, use a Forth script */
+        boot_code = g_strdup_printf("\\ Bootscript\n%x %x c! %x %x c!\n",
+                                    LOW(SIGNATURE), SIGNATURE_ADDR,
+                                    HIGH(SIGNATURE), SIGNATURE_ADDR + 1);
+        len = strlen(boot_code);
+    } else {
+        g_assert_not_reached();
     }
 
-    ret = write(fd, boot_sector, len);
+    ret = write(fd, boot_code, len);
     close(fd);
 
+    g_free(boot_code);
+
     if (ret != len) {
         fprintf(stderr, "Could not write \"%s\"", fname);
         return 1;
@@ -115,8 +126,8 @@  void boot_sector_test(void)
      * instruction.
      */
     for (i = 0; i < TEST_CYCLES; ++i) {
-        signature_low = readb(BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET);
-        signature_high = readb(BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET + 1);
+        signature_low = readb(SIGNATURE_ADDR);
+        signature_high = readb(SIGNATURE_ADDR + 1);
         signature = (signature_high << 8) | signature_low;
         if (signature == SIGNATURE) {
             break;