[Qemu-devel] [PATCH v6] Add optionrom compatible with fw_cfg DMA version

[Qemu-devel] [PATCH v6] Add optionrom compatible with fw_cfg DMA version

[Richard W.M. Jones]

v5 -> v6:

 - Changed the xen_load_linux assertion as suggested by Stefan.

 - I renamed the variables in get_e801_addr function, since the
   registers were really (16 bit 8086-style) AX, not EAX etc.  Also I
   changed the GCC asm to make it a little bit more efficient.  I
   verified by disassembling the function that GCC is still generating
   the right code after this change.

 - Re-tested with small (342K) libguestfs initramfs and with large
   (20M) Fedora initramfs, and works fine in both cases.

Rich.

[Qemu-devel] [PATCH v6] Add optionrom compatible with fw_cfg DMA version

[Richard W.M. Jones]

From: Marc Marí <markmb@redhat.com>

This optionrom is based on linuxboot.S.

Signed-off-by: Marc Marí <markmb@redhat.com>
Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
---
 .gitignore                        |   4 +
 hw/i386/pc.c                      |  10 +-
 hw/nvram/fw_cfg.c                 |   2 +-
 include/hw/nvram/fw_cfg.h         |   1 +
 pc-bios/optionrom/Makefile        |  13 +-
 pc-bios/optionrom/linuxboot_dma.c | 302 ++++++++++++++++++++++++++++++++++++++
 6 files changed, 327 insertions(+), 5 deletions(-)
 create mode 100644 pc-bios/optionrom/linuxboot_dma.c

diff --git a/.gitignore b/.gitignore
index 88a80ff..101d1e0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -94,6 +94,10 @@
 /pc-bios/optionrom/linuxboot.bin
 /pc-bios/optionrom/linuxboot.raw
 /pc-bios/optionrom/linuxboot.img
+/pc-bios/optionrom/linuxboot_dma.asm
+/pc-bios/optionrom/linuxboot_dma.bin
+/pc-bios/optionrom/linuxboot_dma.raw
+/pc-bios/optionrom/linuxboot_dma.img
 /pc-bios/optionrom/multiboot.asm
 /pc-bios/optionrom/multiboot.bin
 /pc-bios/optionrom/multiboot.raw
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 99437e0..098d571 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -999,8 +999,13 @@ static void load_linux(PCMachineState *pcms,
     fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_SIZE, setup_size);
     fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA, setup, setup_size);
 
-    option_rom[nb_option_roms].name = "linuxboot.bin";
-    option_rom[nb_option_roms].bootindex = 0;
+    if (fw_cfg_dma_enabled(fw_cfg)) {
+        option_rom[nb_option_roms].name = "linuxboot_dma.bin";
+        option_rom[nb_option_roms].bootindex = 0;
+    } else {
+        option_rom[nb_option_roms].name = "linuxboot.bin";
+        option_rom[nb_option_roms].bootindex = 0;
+    }
     nb_option_roms++;
 }
 
@@ -1263,6 +1268,7 @@ void xen_load_linux(PCMachineState *pcms)
     load_linux(pcms, fw_cfg);
     for (i = 0; i < nb_option_roms; i++) {
         assert(!strcmp(option_rom[i].name, "linuxboot.bin") ||
+               !strcmp(option_rom[i].name, "linuxboot_dma.bin") ||
                !strcmp(option_rom[i].name, "multiboot.bin"));
         rom_add_option(option_rom[i].name, option_rom[i].bootindex);
     }
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 999f480..114aea8 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -551,7 +551,7 @@ static bool is_version_1(void *opaque, int version_id)
     return version_id == 1;
 }
 
-static bool fw_cfg_dma_enabled(void *opaque)
+bool fw_cfg_dma_enabled(void *opaque)
 {
     FWCfgState *s = opaque;
 
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index d008112..5c27a1f 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -182,5 +182,6 @@ FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr,
                                  hwaddr dma_addr, AddressSpace *dma_as);
 
 FWCfgState *fw_cfg_find(void);
+bool fw_cfg_dma_enabled(void *opaque);
 
 #endif
diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
index ce4852a..d44a62a 100644
--- a/pc-bios/optionrom/Makefile
+++ b/pc-bios/optionrom/Makefile
@@ -13,15 +13,24 @@ CFLAGS := -Wall -Wstrict-prototypes -Werror -fomit-frame-pointer -fno-builtin
 CFLAGS += -I$(SRC_PATH)
 CFLAGS += $(call cc-option, $(CFLAGS), -fno-stack-protector)
 CFLAGS += $(CFLAGS_NOPIE)
+CFLAGS += -m32
 QEMU_CFLAGS = $(CFLAGS)
 
-build-all: multiboot.bin linuxboot.bin kvmvapic.bin
+ASFLAGS += -32
+
+build-all: multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin
 
 # suppress auto-removal of intermediate files
 .SECONDARY:
 
+ifdef CONFIG_WIN32
+LD_EMULATION = i386pe
+else
+LD_EMULATION = elf_i386
+endif
+
 %.img: %.o
-	$(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -Ttext 0 -e _start -s -o $@ $<,"  Building $(TARGET_DIR)$@")
+	$(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -m $(LD_EMULATION) -Ttext 0 -e _start -s -o $@ $<,"  Building $(TARGET_DIR)$@")
 
 %.raw: %.img
 	$(call quiet-command,$(OBJCOPY) -O binary -j .text $< $@,"  Building $(TARGET_DIR)$@")
diff --git a/pc-bios/optionrom/linuxboot_dma.c b/pc-bios/optionrom/linuxboot_dma.c
new file mode 100644
index 0000000..509ddbe
--- /dev/null
+++ b/pc-bios/optionrom/linuxboot_dma.c
@@ -0,0 +1,302 @@
+/*
+ * Linux Boot Option ROM for fw_cfg DMA
+ *
+ * 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/>.
+ *
+ * Copyright (c) 2015-2016 Red Hat Inc.
+ *   Authors:
+ *     Marc Marí <markmb@redhat.com>
+ *     Richard W.M. Jones <rjones@redhat.com>
+ */
+
+asm(
+".text\n"
+".global _start\n"
+"_start:\n"
+"   .short 0xaa55\n"
+"   .byte (_end - _start) / 512\n"
+"   lret\n"
+"   .org 0x18\n"
+"   .short 0\n"
+"   .short _pnph\n"
+"_pnph:\n"
+"   .ascii \"$PnP\"\n"
+"   .byte 0x01\n"
+"   .byte ( _pnph_len / 16 )\n"
+"   .short 0x0000\n"
+"   .byte 0x00\n"
+"   .byte 0x00\n"
+"   .long 0x00000000\n"
+"   .short _manufacturer\n"
+"   .short _product\n"
+"   .long 0x00000000\n"
+"   .short 0x0000\n"
+"   .short 0x0000\n"
+"   .short _bev\n"
+"   .short 0x0000\n"
+"   .short 0x0000\n"
+"   .equ _pnph_len, . - _pnph\n"
+"   .align 4, 0\n"
+"_bev:\n"
+".code16gcc\n"
+/* DS = CS */
+"   movw %cs, %ax\n"
+"   movw %ax, %ds\n"
+"   movl %esp, %ebp\n"
+"   cli\n"
+"   cld\n"
+"   jmp load_kernel\n"
+);
+
+#include "../../include/hw/nvram/fw_cfg_keys.h"
+
+#define BOOT_ROM_PRODUCT "Linux loader DMA"
+
+/* QEMU_CFG_DMA_CONTROL bits */
+#define BIOS_CFG_DMA_CTL_ERROR   0x01
+#define BIOS_CFG_DMA_CTL_READ    0x02
+#define BIOS_CFG_DMA_CTL_SKIP    0x04
+#define BIOS_CFG_DMA_CTL_SELECT  0x08
+
+#define BIOS_CFG_DMA_ADDR_HIGH 0x514
+#define BIOS_CFG_DMA_ADDR_LOW  0x518
+
+#define _stringify(S)   #S
+#define stringify(S) _stringify(S)
+
+#define uint64_t unsigned long long
+#define uint32_t unsigned int
+#define uint16_t unsigned short
+
+#define barrier() asm("" : : : "memory")
+
+typedef struct FWCfgDmaAccess {
+    uint32_t control;
+    uint32_t length;
+    uint64_t address;
+} __attribute__((packed)) FWCfgDmaAccess;
+
+static inline void outl(uint32_t value, uint16_t port)
+{
+    asm("outl %0, %w1" : : "a"(value), "Nd"(port));
+}
+
+static inline void set_es(void *addr)
+{
+    uint32_t seg = (uint32_t)addr >> 4;
+    asm("movl %0, %%es" : : "r"(seg));
+}
+
+static inline uint16_t readw_es(uint16_t offset)
+{
+    uint16_t val;
+    asm("addr32 movw %%es:(%1), %0" : "=r"(val) : "r"((uint32_t)offset));
+    barrier();
+    return val;
+}
+
+static inline uint32_t readl_es(uint16_t offset)
+{
+    uint32_t val;
+    asm("addr32 movl %%es:(%1), %0" : "=r"(val) : "r"((uint32_t)offset));
+    barrier();
+    return val;
+}
+
+static inline void writel_es(uint16_t offset, uint32_t val)
+{
+    barrier();
+    asm("addr32 movl %0, %%es:(%1)" : : "r"(val), "r"((uint32_t)offset));
+}
+
+static inline uint32_t bswap32(uint32_t x)
+{
+    return
+        ((x & 0x000000ffU) << 24) |
+        ((x & 0x0000ff00U) <<  8) |
+        ((x & 0x00ff0000U) >>  8) |
+        ((x & 0xff000000U) >> 24);
+}
+
+static inline uint64_t bswap64(uint64_t x)
+{
+    return
+        ((x & 0x00000000000000ffULL) << 56) |
+        ((x & 0x000000000000ff00ULL) << 40) |
+        ((x & 0x0000000000ff0000ULL) << 24) |
+        ((x & 0x00000000ff000000ULL) <<  8) |
+        ((x & 0x000000ff00000000ULL) >>  8) |
+        ((x & 0x0000ff0000000000ULL) >> 24) |
+        ((x & 0x00ff000000000000ULL) >> 40) |
+        ((x & 0xff00000000000000ULL) >> 56);
+}
+
+static inline uint64_t cpu_to_be64(uint64_t x)
+{
+    return bswap64(x);
+}
+
+static inline uint32_t cpu_to_be32(uint32_t x)
+{
+    return bswap32(x);
+}
+
+static inline uint32_t be32_to_cpu(uint32_t x)
+{
+    return bswap32(x);
+}
+
+static void bios_cfg_read_entry(void *buf, uint16_t entry, uint32_t len)
+{
+    FWCfgDmaAccess access;
+    uint32_t control = (entry << 16) | BIOS_CFG_DMA_CTL_SELECT
+                        | BIOS_CFG_DMA_CTL_READ;
+
+    access.address = cpu_to_be64((uint64_t)(uint32_t)buf);
+    access.length = cpu_to_be32(len);
+    access.control = cpu_to_be32(control);
+
+    barrier();
+
+    outl(cpu_to_be32((uint32_t)&access), BIOS_CFG_DMA_ADDR_LOW);
+
+    while (be32_to_cpu(access.control) & ~BIOS_CFG_DMA_CTL_ERROR) {
+        barrier();
+    }
+}
+
+/* Return top of memory using BIOS function E801. */
+static uint32_t get_e801_addr(void)
+{
+    uint16_t ax, bx, cx, dx;
+    uint32_t ret;
+
+    asm("int $0x15\n"
+        : "=a"(ax), "=b"(bx), "=c"(cx), "=d"(dx)
+        : "a"(0xe801), "b"(0), "c"(0), "d"(0));
+
+    /* Not SeaBIOS, but in theory a BIOS could return CX=DX=0 in which
+     * case we need to use the result from AX & BX instead.
+     */
+    if (cx == 0 && dx == 0) {
+        cx = ax;
+        dx = bx;
+    }
+
+    if (dx) {
+        /* DX = extended memory above 16M, in 64K units.
+         * Convert it to bytes and return.
+         */
+        ret = ((uint32_t)dx + 256 /* 16M in 64K units */) << 16;
+    } else {
+        /* This is a fallback path for machines with <= 16MB of RAM,
+         * which probably would never be the case, but deal with it
+         * anyway.
+         *
+         * CX = extended memory between 1M and 16M, in kilobytes
+         * Convert it to bytes and return.
+         */
+        ret = ((uint32_t)cx + 1024 /* 1M in K */) << 10;
+    }
+
+    return ret;
+}
+
+void load_kernel(void)
+{
+    void *setup_addr;
+    void *initrd_addr;
+    void *kernel_addr;
+    void *cmdline_addr;
+    uint32_t setup_size;
+    uint32_t initrd_size;
+    uint32_t kernel_size;
+    uint32_t cmdline_size;
+    uint32_t initrd_end_page, max_allowed_page;
+    uint32_t segment_addr, stack_addr;
+
+    bios_cfg_read_entry(&setup_addr, FW_CFG_SETUP_ADDR, 4);
+    bios_cfg_read_entry(&setup_size, FW_CFG_SETUP_SIZE, 4);
+    bios_cfg_read_entry(setup_addr, FW_CFG_SETUP_DATA, setup_size);
+
+    set_es(setup_addr);
+
+    /* For protocol < 0x203 we don't have initrd_max ... */
+    if (readw_es(0x206) < 0x203) {
+        /* ... so we assume initrd_max = 0x37ffffff. */
+        writel_es(0x22c, 0x37ffffff);
+    }
+
+    bios_cfg_read_entry(&initrd_addr, FW_CFG_INITRD_ADDR, 4);
+    bios_cfg_read_entry(&initrd_size, FW_CFG_INITRD_SIZE, 4);
+
+    initrd_end_page = ((uint32_t)(initrd_addr + initrd_size) & -4096);
+    max_allowed_page = (readl_es(0x22c) & -4096);
+
+    if (initrd_end_page != 0 && max_allowed_page != 0 &&
+        initrd_end_page != max_allowed_page) {
+        /* Initrd at the end of memory. Compute better initrd address
+         * based on e801 data
+         */
+        initrd_addr = (void *)((get_e801_addr() - initrd_size) & -4096);
+        writel_es(0x218, (uint32_t)initrd_addr);
+
+    }
+
+    bios_cfg_read_entry(initrd_addr, FW_CFG_INITRD_DATA, initrd_size);
+
+    bios_cfg_read_entry(&kernel_addr, FW_CFG_KERNEL_ADDR, 4);
+    bios_cfg_read_entry(&kernel_size, FW_CFG_KERNEL_SIZE, 4);
+    bios_cfg_read_entry(kernel_addr, FW_CFG_KERNEL_DATA, kernel_size);
+
+    bios_cfg_read_entry(&cmdline_addr, FW_CFG_CMDLINE_ADDR, 4);
+    bios_cfg_read_entry(&cmdline_size, FW_CFG_CMDLINE_SIZE, 4);
+    bios_cfg_read_entry(cmdline_addr, FW_CFG_CMDLINE_DATA, cmdline_size);
+
+    /* Boot linux */
+    segment_addr = ((uint32_t)setup_addr >> 4);
+    stack_addr = (uint32_t)(cmdline_addr - setup_addr - 16);
+
+    /* As we are changing critical registers, we cannot leave freedom to the
+     * compiler.
+     */
+    asm("movw %%ax, %%ds\n"
+        "movw %%ax, %%es\n"
+        "movw %%ax, %%fs\n"
+        "movw %%ax, %%gs\n"
+        "movw %%ax, %%ss\n"
+        "movl %%ebx, %%esp\n"
+        "addw $0x20, %%ax\n"
+        "pushw %%ax\n" /* CS */
+        "pushw $0\n" /* IP */
+        /* Clear registers and jump to Linux */
+        "xor %%ebx, %%ebx\n"
+        "xor %%ecx, %%ecx\n"
+        "xor %%edx, %%edx\n"
+        "xor %%edi, %%edi\n"
+        "xor %%ebp, %%ebp\n"
+        "lretw\n"
+        : : "a"(segment_addr), "b"(stack_addr));
+}
+
+asm(
+"_manufacturer:\n"
+".asciz \"QEMU\"\n"
+"_product:\n"
+".asciz "stringify(BOOT_ROM_PRODUCT)"\n"
+".byte 0\n"
+".align 512, 0\n"
+"_end:\n"
+);
+
-- 
2.7.4

Re: [Qemu-devel] [PATCH v6] Add optionrom compatible with fw_cfg DMA version

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 689 bytes --]

On Mon, Apr 25, 2016 at 05:04:40PM +0100, Richard W.M. Jones wrote:
> v5 -> v6:
> 
>  - Changed the xen_load_linux assertion as suggested by Stefan.
> 
>  - I renamed the variables in get_e801_addr function, since the
>    registers were really (16 bit 8086-style) AX, not EAX etc.  Also I
>    changed the GCC asm to make it a little bit more efficient.  I
>    verified by disassembling the function that GCC is still generating
>    the right code after this change.
> 
>  - Re-tested with small (342K) libguestfs initramfs and with large
>    (20M) Fedora initramfs, and works fine in both cases.
> 
> Rich.
> 
> 

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v6] Add optionrom compatible with fw_cfg DMA version

[Marc Marí]

On Mon, 25 Apr 2016 17:04:41 +0100
"Richard W.M. Jones" <rjones@redhat.com> wrote:

> From: Marc Marí <markmb@redhat.com>
> 
> This optionrom is based on linuxboot.S.
> 
> Signed-off-by: Marc Marí <markmb@redhat.com>
> Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
> ---
>  .gitignore                        |   4 +
>  hw/i386/pc.c                      |  10 +-
>  hw/nvram/fw_cfg.c                 |   2 +-
>  include/hw/nvram/fw_cfg.h         |   1 +
>  pc-bios/optionrom/Makefile        |  13 +-
>  pc-bios/optionrom/linuxboot_dma.c | 302
> ++++++++++++++++++++++++++++++++++++++ 6 files changed, 327
> insertions(+), 5 deletions(-) create mode 100644
> pc-bios/optionrom/linuxboot_dma.c

Not sure if I should send my reviewed-by if it's already signed-off by
me, but here it goes:

Reviewed-by: Marc Marí <marc.mari.barcelo@gmail.com>

Re: [Qemu-devel] [PATCH v6] Add optionrom compatible with fw_cfg DMA version

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 765 bytes --]

On Mon, Apr 25, 2016 at 05:04:40PM +0100, Richard W.M. Jones wrote:
> v5 -> v6:
> 
>  - Changed the xen_load_linux assertion as suggested by Stefan.
> 
>  - I renamed the variables in get_e801_addr function, since the
>    registers were really (16 bit 8086-style) AX, not EAX etc.  Also I
>    changed the GCC asm to make it a little bit more efficient.  I
>    verified by disassembling the function that GCC is still generating
>    the right code after this change.
> 
>  - Re-tested with small (342K) libguestfs initramfs and with large
>    (20M) Fedora initramfs, and works fine in both cases.

No one has picked this up, so I have (for QEMU 2.7).

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v6] Add optionrom compatible with fw_cfg DMA version

[Stefan Hajnoczi]

On Mon, May 9, 2016 at 2:24 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Mon, Apr 25, 2016 at 05:04:40PM +0100, Richard W.M. Jones wrote:
>> v5 -> v6:
>>
>>  - Changed the xen_load_linux assertion as suggested by Stefan.
>>
>>  - I renamed the variables in get_e801_addr function, since the
>>    registers were really (16 bit 8086-style) AX, not EAX etc.  Also I
>>    changed the GCC asm to make it a little bit more efficient.  I
>>    verified by disassembling the function that GCC is still generating
>>    the right code after this change.
>>
>>  - Re-tested with small (342K) libguestfs initramfs and with large
>>    (20M) Fedora initramfs, and works fine in both cases.
>
> No one has picked this up, so I have (for QEMU 2.7).
>
> Thanks, applied to my block tree:
> https://github.com/stefanha/qemu/commits/block

The bad news is this patch breaks the build under clang:

CC optionrom/linuxboot_dma.o
<inline asm>:29:1: error: unexpected directive .code16gcc
.code16gcc
^
<inline asm>:29:11: error: .code16gcc not supported yet
.code16gcc
^
linuxboot_dma.c:104:9: error: unexpected token in argument list
asm("addr32 movw %%es:(%1), %0" : "=r"(val) : "r"((uint32_t)offset));
^
<inline asm>:1:17: note: instantiated into assembly here
addr32 movw %es:(%eax), %cx
^
linuxboot_dma.c:120:9: error: invalid instruction mnemonic 'addr32'
asm("addr32 movl %0, %%es:(%1)" : : "r"(val), "r"((uint32_t)offset));
^
<inline asm>:1:2: note: instantiated into assembly here
addr32 movl %eax, %es:(%edx)
^~~~~~
linuxboot_dma.c:112:9: error: unexpected token in argument list
asm("addr32 movl %%es:(%1), %0" : "=r"(val) : "r"((uint32_t)offset));
^
<inline asm>:1:17: note: instantiated into assembly here
addr32 movl %es:(%eax), %eax
^

The .code16gcc issue can probably be resolved with -m16 (which gcc
4.9+ also supports!).  I haven't looked into the other issues though.

Dropped from my block tree.

Stefan

Re: [Qemu-devel] [PATCH v6] Add optionrom compatible with fw_cfg DMA version

[Richard W.M. Jones]

On Mon, May 09, 2016 at 04:31:26PM +0100, Stefan Hajnoczi wrote:
> On Mon, May 9, 2016 at 2:24 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> > On Mon, Apr 25, 2016 at 05:04:40PM +0100, Richard W.M. Jones wrote:
> >> v5 -> v6:
> >>
> >>  - Changed the xen_load_linux assertion as suggested by Stefan.
> >>
> >>  - I renamed the variables in get_e801_addr function, since the
> >>    registers were really (16 bit 8086-style) AX, not EAX etc.  Also I
> >>    changed the GCC asm to make it a little bit more efficient.  I
> >>    verified by disassembling the function that GCC is still generating
> >>    the right code after this change.
> >>
> >>  - Re-tested with small (342K) libguestfs initramfs and with large
> >>    (20M) Fedora initramfs, and works fine in both cases.
> >
> > No one has picked this up, so I have (for QEMU 2.7).
> >
> > Thanks, applied to my block tree:
> > https://github.com/stefanha/qemu/commits/block
> 
> The bad news is this patch breaks the build under clang:
[...]
> The .code16gcc issue can probably be resolved with -m16 (which gcc
> 4.9+ also supports!).  I haven't looked into the other issues though.

It looks as if the easiest way to get around this will be to use
gas instead of clang's integrated assembler, ie:

  CFLAGS += -m16 -fno-integrated-as

However I have no idea if there are platforms we support which use
clang and require the integrated assembler instead of binutils.
Opinions on this?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

Re: [Qemu-devel] [PATCH v6] Add optionrom compatible with fw_cfg DMA version

[Richard W.M. Jones]

Actually there's a rather more fundamental problem.  In the current
linuxboot_dma.c we use asm statements at the top and bottom of the
file (outside any function).  The asm statements define the header and
what I assume is the footer of the file.  At any rate, they encode the
size of the file (the calculation `.byte (_end - _start) / 512').

Clang just rearranges everything in the file, so the _start and _end
asm snippets appear together at the beginning of the input to the
assembler, and nothing works after that.

So that pretty much screws up the whole project of trying to write an
option ROM in C.

Of course we're well outside any standards here.  Can we tell clang
users to use the GCC/pre-compiled option ROMs :-?  Any other ideas?  I
don't think I've missed a flag (GCC has -fno-toplevel-reorder, but
clang 3.8 doesn't ...)

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

Re: [Qemu-devel] [PATCH v6] Add optionrom compatible with fw_cfg DMA version

[Laszlo Ersek]

On 05/09/16 18:48, Richard W.M. Jones wrote:
> 
> Actually there's a rather more fundamental problem.  In the current
> linuxboot_dma.c we use asm statements at the top and bottom of the
> file (outside any function).  The asm statements define the header and
> what I assume is the footer of the file.  At any rate, they encode the
> size of the file (the calculation `.byte (_end - _start) / 512').
> 
> Clang just rearranges everything in the file, so the _start and _end
> asm snippets appear together at the beginning of the input to the
> assembler, and nothing works after that.
> 
> So that pretty much screws up the whole project of trying to write an
> option ROM in C.
> 
> Of course we're well outside any standards here.  Can we tell clang
> users to use the GCC/pre-compiled option ROMs :-?  Any other ideas?  I
> don't think I've missed a flag (GCC has -fno-toplevel-reorder, but
> clang 3.8 doesn't ...)

IIRC for a while we used to ship pre-compiled ACPI byte-code, for people
who didn't have "iasl" installed.

We could make this C source file gcc-only, and provide a pre-compiled
assembly source file for those platforms that only have clang.

Laszlo

Re: [Qemu-devel] [PATCH v6] Add optionrom compatible with fw_cfg DMA version

[Richard W.M. Jones]

FWIW the response from the LLVM developers:

  https://llvm.org/bugs/show_bug.cgi?id=27688

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

Re: [Qemu-devel] [PATCH v6] Add optionrom compatible with fw_cfg DMA version

[Paolo Bonzini]

On 09/05/2016 23:37, Richard W.M. Jones wrote:
> FWIW the response from the LLVM developers:
> 
>   https://llvm.org/bugs/show_bug.cgi?id=27688

Why the heck are they saying that -fno-toplevel-reorder is "gone" (or
deprecated in the duplicate PR)?

Paolo

Re: [Qemu-devel] [PATCH v6] Add optionrom compatible with fw_cfg DMA version

[Paolo Bonzini]

On 09/05/2016 18:48, Richard W.M. Jones wrote:
> 
> Of course we're well outside any standards here.  Can we tell clang
> users to use the GCC/pre-compiled option ROMs :-?  Any other ideas?  I
> don't think I've missed a flag (GCC has -fno-toplevel-reorder, but
> clang 3.8 doesn't ...)

I guess the checksumming script (scripts/signrom.py) could take care of
padding the file to a multiple of 512 bytes, and fill in the size in the
third byte.  Then "_end" would not be necessary anymore and -m16 could
replace the .code16 directive.

Paolo

Re: [Qemu-devel] [PATCH v6] Add optionrom compatible with fw_cfg DMA version

[Richard W.M. Jones]

On Tue, May 10, 2016 at 07:24:28PM +0200, Paolo Bonzini wrote:
> 
> 
> On 09/05/2016 18:48, Richard W.M. Jones wrote:
> > 
> > Of course we're well outside any standards here.  Can we tell clang
> > users to use the GCC/pre-compiled option ROMs :-?  Any other ideas?  I
> > don't think I've missed a flag (GCC has -fno-toplevel-reorder, but
> > clang 3.8 doesn't ...)
> 
> I guess the checksumming script (scripts/signrom.py) could take care of
> padding the file to a multiple of 512 bytes, and fill in the size in the
> third byte.  Then "_end" would not be necessary anymore and -m16 could
> replace the .code16 directive.

In my rather limited testing on gcc, gcc -m16 broke booting.  However
I've not investigated this further.  I'll do so shortly.

However I have a question: is there a formal standard or documentation
for the option ROM format?  Are we sticking to the (ancient) "BIOS Boot
Specification" or is there something newer?  (My copy is from 1996).

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

Re: [Qemu-devel] [PATCH v6] Add optionrom compatible with fw_cfg DMA version

[Laszlo Ersek]

On 05/10/16 19:36, Richard W.M. Jones wrote:
> On Tue, May 10, 2016 at 07:24:28PM +0200, Paolo Bonzini wrote:
>>
>>
>> On 09/05/2016 18:48, Richard W.M. Jones wrote:
>>>
>>> Of course we're well outside any standards here.  Can we tell clang
>>> users to use the GCC/pre-compiled option ROMs :-?  Any other ideas?  I
>>> don't think I've missed a flag (GCC has -fno-toplevel-reorder, but
>>> clang 3.8 doesn't ...)
>>
>> I guess the checksumming script (scripts/signrom.py) could take care of
>> padding the file to a multiple of 512 bytes, and fill in the size in the
>> third byte.  Then "_end" would not be necessary anymore and -m16 could
>> replace the .code16 directive.
> 
> In my rather limited testing on gcc, gcc -m16 broke booting.  However
> I've not investigated this further.  I'll do so shortly.
> 
> However I have a question: is there a formal standard or documentation
> for the option ROM format?  Are we sticking to the (ancient) "BIOS Boot
> Specification" or is there something newer?  (My copy is from 1996).

To my knowledge, the most recent specification that describes PCI
expansion ROMs is:

  PCI Firmware Specification
  Revision 3.2
  January 26, 2015

See Chapter 5, "PCI Expansion ROMs".

(Maybe a newer release has been made, not sure.)

Of course, this spec is not public (you have to be a PCI SIG member to
get it, or some such, which costs $$$), but since Red Hat is a member,
I'll send you a link off-list (and you can't share the PDF outside of
RH, of course).

Regarding general (device independent) oproms: I'm not sure.

Thanks
Laszlo