[Qemu-devel] [PATCH v4.1] Add optionrom compatible with fw_cfg DMA version

[Qemu-devel] [PATCH v4.1] Add optionrom compatible with fw_cfg DMA version

[Richard W.M. Jones]

This is an updated version of Marc Marí's Linux DMA patch, based on
version 4 from:

https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg05891.html

All I have done for my "4.1" version is:

 - Modify it so it compiles against the latest qemu.  There was
   a change to how fw_cfg.h is split (6f061ea10f28c) which broke
   the patch.

 - Run checkpatch and fix several problems.

 - Test it.

This patch greatly and transparently improves boot time of the
libguestfs appliance on x86, and so I'd like to see it integrated
upstream.

Rich.

[Qemu-devel] [PATCH v4.1] 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                      |   9 +-
 hw/nvram/fw_cfg.c                 |   2 +-
 include/hw/nvram/fw_cfg.h         |   1 +
 pc-bios/optionrom/Makefile        |   7 +-
 pc-bios/optionrom/linuxboot_dma.c | 297 ++++++++++++++++++++++++++++++++++++++
 6 files changed, 315 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 2ac97c4..7658bfe 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++;
 }
 
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index d96932f..4507ba1 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -547,7 +547,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 d516989..8911ea9 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -174,5 +174,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..bdd0cc1 100644
--- a/pc-bios/optionrom/Makefile
+++ b/pc-bios/optionrom/Makefile
@@ -13,15 +13,18 @@ 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:
 
 %.img: %.o
-	$(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -Ttext 0 -e _start -s -o $@ $<,"  Building $(TARGET_DIR)$@")
+	$(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -m elf_i386 -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..b0026aa
--- /dev/null
+++ b/pc-bios/optionrom/linuxboot_dma.c
@@ -0,0 +1,297 @@
+/*
+ * 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 Red Hat Inc.
+ *   Authors: Marc Marí <markmb@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();
+    }
+}
+
+static uint32_t get_e801_addr(void)
+{
+    uint32_t eax, ebx, ecx, edx;
+    uint32_t ret;
+
+    eax = 0xe801;
+    ebx = 0;
+    ecx = 0;
+    edx = 0;
+    asm("int $0x15\n"
+        : "+a"(eax)
+        : "b"(ebx), "c"(ecx), "d"(edx));
+
+    /* Output could be in AX/BX or CX/DX */
+    if ((uint16_t)ecx || (uint16_t)edx) {
+        if (!(uint16_t)edx) {
+            /* Add 1 MB and convert to bytes */
+            ret = (ecx + 1024) << 10;
+        } else {
+            /* Add 16 MB and convert to bytes */
+            ret = (edx + 256) << 16;
+        }
+    } else {
+        if (!(uint16_t)ebx) {
+            /* Add 1 MB and convert to bytes */
+            ret = (eax + 1024) << 10;
+        } else {
+            /* Add 16 MB and convert to bytes */
+            ret = (ebx + 256) << 16;
+        }
+    }
+
+    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);
+
+    if (readw_es(0x206) < 0x203) {
+        /* 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 v4.1] Add optionrom compatible with fw_cfg DMA version

[Stefan Hajnoczi]

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

On Fri, Apr 01, 2016 at 01:43:47PM +0100, Richard W.M. Jones 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                      |   9 +-
>  hw/nvram/fw_cfg.c                 |   2 +-
>  include/hw/nvram/fw_cfg.h         |   1 +
>  pc-bios/optionrom/Makefile        |   7 +-
>  pc-bios/optionrom/linuxboot_dma.c | 297 ++++++++++++++++++++++++++++++++++++++
>  6 files changed, 315 insertions(+), 5 deletions(-)
>  create mode 100644 pc-bios/optionrom/linuxboot_dma.c

It would be great to merge this but there's more work necessary.

I CCed Marc's current email address.  He was interning at Red Hat but is
now back at university so the Red Hat address is not being read.

Marc: What was still missing from this patch?

> diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
> index ce4852a..bdd0cc1 100644
> --- a/pc-bios/optionrom/Makefile
> +++ b/pc-bios/optionrom/Makefile
> @@ -13,15 +13,18 @@ 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:
>  
>  %.img: %.o
> -	$(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -Ttext 0 -e _start -s -o $@ $<,"  Building $(TARGET_DIR)$@")
> +	$(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -m elf_i386 -Ttext 0 -e _start -s -o $@ $<,"  Building $(TARGET_DIR)$@")

Gerd noted in <1454411187.9300.54.camel@redhat.com>:

  Hmm, that breaks the windows cross build:

  make: Entering directory `/home/kraxel/projects/qemu/build-win32'
  Building optionrom/linuxboot_dma.img
  i686-w64-mingw32-ld: unrecognised emulation mode: elf_i386
  Supported emulations: i386pe
  make[1]: *** [linuxboot_dma.img] Error 1

and then:

  Testing shows two more problems:

  (1) initrd loading is broken, kernel complains it finds only gibberish:

  [    0.934582] Unpacking initramfs...
  [    1.166983] Initramfs unpacking failed: junk in compressed archive
  [    1.168458] Freeing initrd memory: 32812k freed

  (2) going back to non-dma boot via -M pc-$old doesn't work, appearently
      fw_cfg dma is enabled even for old machine types.

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

Re: [Qemu-devel] [PATCH v4.1] Add optionrom compatible with fw_cfg DMA version

[Michael S. Tsirkin]

On Fri, Apr 01, 2016 at 01:43:46PM +0100, Richard W.M. Jones wrote:
> This is an updated version of Marc Marí's Linux DMA patch, based on
> version 4 from:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg05891.html

Pls feel free to iterate, but pls also remember to post after 2.6 is
out as we are in freeze.


> All I have done for my "4.1" version is:
> 
>  - Modify it so it compiles against the latest qemu.  There was
>    a change to how fw_cfg.h is split (6f061ea10f28c) which broke
>    the patch.
> 
>  - Run checkpatch and fix several problems.
> 
>  - Test it.
> 
> This patch greatly and transparently improves boot time of the
> libguestfs appliance on x86, and so I'd like to see it integrated
> upstream.
> 
> Rich.

Re: [Qemu-devel] [PATCH v4.1] Add optionrom compatible with fw_cfg DMA version

[Richard W.M. Jones]

On Mon, Apr 04, 2016 at 04:02:04PM +0100, Stefan Hajnoczi wrote:
>   (1) initrd loading is broken, kernel complains it finds only gibberish:
> 
>   [    0.934582] Unpacking initramfs...
>   [    1.166983] Initramfs unpacking failed: junk in compressed archive
>   [    1.168458] Freeing initrd memory: 32812k freed

That's strange.  I certainly never saw anything like this.  I wonder
if it's because your initrd is particularly large?

>   (2) going back to non-dma boot via -M pc-$old doesn't work, appearently
>       fw_cfg dma is enabled even for old machine types.

IIRC there was a thread about how we accidentally added DMA to old
machine types.  Does this matter?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Re: [Qemu-devel] [PATCH v4.1] Add optionrom compatible with fw_cfg DMA version

[Gerd Hoffmann]

On Mo, 2016-04-04 at 16:21 +0100, Richard W.M. Jones wrote:
> On Mon, Apr 04, 2016 at 04:02:04PM +0100, Stefan Hajnoczi wrote:
> >   (1) initrd loading is broken, kernel complains it finds only gibberish:
> > 
> >   [    0.934582] Unpacking initramfs...
> >   [    1.166983] Initramfs unpacking failed: junk in compressed archive
> >   [    1.168458] Freeing initrd memory: 32812k freed
> 
> That's strange.  I certainly never saw anything like this.  I wonder
> if it's because your initrd is particularly large?

I've simply used /boot/initramfs-$version from the host.  It's 33M.  Not
exactly small, but given this is a standard RHEL-7 install I also
wouldn't rate this as unusual big.

> >   (2) going back to non-dma boot via -M pc-$old doesn't work, appearently
> >       fw_cfg dma is enabled even for old machine types.
> 
> IIRC there was a thread about how we accidentally added DMA to old
> machine types.  Does this matter?

That one should be fixed meanwhile.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v4.1] Add optionrom compatible with fw_cfg DMA version

[Laszlo Ersek]

On 04/04/16 17:21, Richard W.M. Jones wrote:
> On Mon, Apr 04, 2016 at 04:02:04PM +0100, Stefan Hajnoczi wrote:
>>   (1) initrd loading is broken, kernel complains it finds only gibberish:
>>
>>   [    0.934582] Unpacking initramfs...
>>   [    1.166983] Initramfs unpacking failed: junk in compressed archive
>>   [    1.168458] Freeing initrd memory: 32812k freed
> 
> That's strange.  I certainly never saw anything like this.  I wonder
> if it's because your initrd is particularly large?

If you search your mailbox for Message-Id <54C2D2C4.3010806@redhat.com>
(subject: "Test libguestfs on RHELSA"), you'll find an earlier occurence
of the same.

(You had also replied to that email, so you simply didn't remember it
this time.)

In that case, the problem was an oversized initrd, relative to the
amount of RAM that the guest had. You eliminated (worked around?) the
issue in libguestfs commit c24f242521e88.

Of course, without reviewing the patch under discussion (which I won't
volunteer for now, apologies), I cannot exclude that the patch makes the
kernel *think* that the initrd is too large.

>>   (2) going back to non-dma boot via -M pc-$old doesn't work, appearently
>>       fw_cfg dma is enabled even for old machine types.
> 
> IIRC there was a thread about how we accidentally added DMA to old
> machine types.  Does this matter?

What Gerd said -- see e6915b5f3a87.

(I have so few commits in QEMU that if I look for something I wrote, I
can find it simply with "--author=lersek". I don't know if I should
smile or cry...)

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v4.1] Add optionrom compatible with fw_cfg DMA version

[Paolo Bonzini]

On 05/04/2016 14:51, Laszlo Ersek wrote:
> (I have so few commits in QEMU that if I look for something I wrote, I
> can find it simply with "--author=lersek". I don't know if I should
> smile or cry...)

Actually you are in the top 50.  You can't claim to be the 1%, but
that's not shabby at all!

Paolo

Re: [Qemu-devel] [PATCH v4.1] Add optionrom compatible with fw_cfg DMA version

[Marc Marí]

On Mon, 4 Apr 2016 16:02:04 +0100
Stefan Hajnoczi <stefanha@gmail.com> wrote:

> On Fri, Apr 01, 2016 at 01:43:47PM +0100, Richard W.M. Jones 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                      |   9 +-
> >  hw/nvram/fw_cfg.c                 |   2 +-
> >  include/hw/nvram/fw_cfg.h         |   1 +
> >  pc-bios/optionrom/Makefile        |   7 +-
> >  pc-bios/optionrom/linuxboot_dma.c | 297
> > ++++++++++++++++++++++++++++++++++++++ 6 files changed, 315
> > insertions(+), 5 deletions(-) create mode 100644
> > pc-bios/optionrom/linuxboot_dma.c  
> 
> It would be great to merge this but there's more work necessary.
> 
> I CCed Marc's current email address.  He was interning at Red Hat but
> is now back at university so the Red Hat address is not being read.
> 
> Marc: What was still missing from this patch?

I'm sorry I have not answered before, it's been a very busy week.

What's missing (as I remember it):
 - Debug loading an initrd from the new linuxboot
 - Other minor comments in the thread (mainly, compile using mingw).
 - Make it possible to disable DMA fw_cfg.

I've been putting this patch at the back of my todo list for too long.
Let me address some deadlines for this week, and then I'll finish this
patch.

Marc
 
> > diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
> > index ce4852a..bdd0cc1 100644
> > --- a/pc-bios/optionrom/Makefile
> > +++ b/pc-bios/optionrom/Makefile
> > @@ -13,15 +13,18 @@ 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:
> >  
> >  %.img: %.o
> > -	$(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -Ttext 0 -e
> > _start -s -o $@ $<,"  Building $(TARGET_DIR)$@")
> > +	$(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -m elf_i386
> > -Ttext 0 -e _start -s -o $@ $<,"  Building $(TARGET_DIR)$@")  
> 
> Gerd noted in <1454411187.9300.54.camel@redhat.com>:
> 
>   Hmm, that breaks the windows cross build:
> 
>   make: Entering directory `/home/kraxel/projects/qemu/build-win32'
>   Building optionrom/linuxboot_dma.img
>   i686-w64-mingw32-ld: unrecognised emulation mode: elf_i386
>   Supported emulations: i386pe
>   make[1]: *** [linuxboot_dma.img] Error 1
> 
> and then:
> 
>   Testing shows two more problems:
> 
>   (1) initrd loading is broken, kernel complains it finds only
> gibberish:
> 
>   [    0.934582] Unpacking initramfs...
>   [    1.166983] Initramfs unpacking failed: junk in compressed
> archive [    1.168458] Freeing initrd memory: 32812k freed
> 
>   (2) going back to non-dma boot via -M pc-$old doesn't work,
> appearently fw_cfg dma is enabled even for old machine types.

Re: [Qemu-devel] [PATCH v4.1] Add optionrom compatible with fw_cfg DMA version

[Richard W.M. Jones]

On Tue, Apr 05, 2016 at 09:29:28AM +0200, Gerd Hoffmann wrote:
> On Mo, 2016-04-04 at 16:21 +0100, Richard W.M. Jones wrote:
> > On Mon, Apr 04, 2016 at 04:02:04PM +0100, Stefan Hajnoczi wrote:
> > >   (1) initrd loading is broken, kernel complains it finds only gibberish:
> > > 
> > >   [    0.934582] Unpacking initramfs...
> > >   [    1.166983] Initramfs unpacking failed: junk in compressed archive
> > >   [    1.168458] Freeing initrd memory: 32812k freed
> > 
> > That's strange.  I certainly never saw anything like this.  I wonder
> > if it's because your initrd is particularly large?
> 
> I've simply used /boot/initramfs-$version from the host.  It's 33M.  Not
> exactly small, but given this is a standard RHEL-7 install I also
> wouldn't rate this as unusual big.

The problem here was the GCC asm statement that calls the 0xE801 BIOS
function.  It wasn't actually reading %bx, %cx, %dx, and so the whole
calculation of where to put the initrd was wrong.

The attached patch fixes things for me.  I also rewrote the
get_e801_addr function to make it a little bit cleaner and clearer.

However don't consider this patch for now.  I'm going to post a new
version of the whole patch with these changes integrated and the whole
lot retested properly, hopefully later today.

Rich.

diff --git a/pc-bios/optionrom/linuxboot_dma.c b/pc-bios/optionrom/linuxboot_dma.c
index b0026aa..604ff3f 100644
--- a/pc-bios/optionrom/linuxboot_dma.c
+++ b/pc-bios/optionrom/linuxboot_dma.c
@@ -174,36 +174,39 @@ static void bios_cfg_read_entry(void *buf, uint16_t entry, uint32_t len)
     }
 }
 
+/* Return top of memory using BIOS function E801. */
 static uint32_t get_e801_addr(void)
 {
-    uint32_t eax, ebx, ecx, edx;
+    uint16_t eax, ebx, ecx, edx;
     uint32_t ret;
 
-    eax = 0xe801;
     ebx = 0;
     ecx = 0;
     edx = 0;
     asm("int $0x15\n"
-        : "+a"(eax)
-        : "b"(ebx), "c"(ecx), "d"(edx));
+        : "=a"(eax), "+b"(ebx), "+c"(ecx), "+d"(edx)
+        : "a"(0xe801));
 
-    /* Output could be in AX/BX or CX/DX */
-    if ((uint16_t)ecx || (uint16_t)edx) {
-        if (!(uint16_t)edx) {
-            /* Add 1 MB and convert to bytes */
-            ret = (ecx + 1024) << 10;
-        } else {
-            /* Add 16 MB and convert to bytes */
-            ret = (edx + 256) << 16;
-        }
+    /* 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 (ecx == 0 && edx == 0) {
+        ecx = eax;
+        edx = ebx;
+    }
+
+    if (edx == 0) {
+        /* This is for machines with <= 16MB of RAM, which probably
+         * would never be the case, but deal with it anyway.
+         * ECX = extended memory between 1M and 16M, in kilobytes
+         * Convert it to bytes and return.
+         */
+        ret = ((uint32_t)ecx + 1024 /* 1M in K */) << 10;
     } else {
-        if (!(uint16_t)ebx) {
-            /* Add 1 MB and convert to bytes */
-            ret = (eax + 1024) << 10;
-        } else {
-            /* Add 16 MB and convert to bytes */
-            ret = (ebx + 256) << 16;
-        }
+        /* EDX = extended memory above 16M, in 64K units.
+         * Convert it to bytes and return.
+         */
+        ret = ((uint32_t)edx + 256 /* 16M in 64K units */) << 16;
     }
 
     return ret;
 


-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html