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

["Richard W.M. Jones" <rjones@redhat.com>]

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