Hi Stefano,

Code LGTM, just a few minor comments below

On 15/01/2019 10:00, Stefano Garzarella wrote:
The new pvh.bin option rom can be used with SeaBIOS to boot
uncompressed kernel using the x86/HVM direct boot ABI.

pvh.S contains the entry point of the option rom. It runs
in real mode, loads the e820 table querying the BIOS, and
then it switches to 32bit protect mode and jump to the

"protect" -> "protected"
"jump" -> "jumps"

pvh_load_kernel() written in pvh_main.c.
pvh_load_kernel() loads the cmdline and kernel entry_point
using fw_cfg, then it looks for RSDP, fills the
hvm_start_info required by x86/HVM ABI, and finally jumps
to the kernel entry_point.

Signed-off-by: Stefano Garzarella <sgarz...@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>
---
  pc-bios/optionrom/Makefile   |   5 +-
  pc-bios/optionrom/pvh.S      | 200 +++++++++++++++++++++++++++++++++++
  pc-bios/optionrom/pvh_main.c | 117 ++++++++++++++++++++
  3 files changed, 321 insertions(+), 1 deletion(-)
  create mode 100644 pc-bios/optionrom/pvh.S
  create mode 100644 pc-bios/optionrom/pvh_main.c

diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
index a9a9e5e7eb..92c91d9949 100644
--- a/pc-bios/optionrom/Makefile
+++ b/pc-bios/optionrom/Makefile
@@ -37,7 +37,7 @@ Wa = -Wa,
  ASFLAGS += -32
  QEMU_CFLAGS += $(call cc-c-option, $(QEMU_CFLAGS), $(Wa)-32)
-build-all: multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin
+build-all: multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin pvh.bin
# suppress auto-removal of intermediate files
  .SECONDARY:
@@ -46,6 +46,9 @@ build-all: multiboot.bin linuxboot.bin linuxboot_dma.bin 
kvmvapic.bin
  %.o: %.S
        $(call quiet-command,$(CPP) $(QEMU_INCLUDES) $(QEMU_DGFLAGS) -c -o - $< | $(AS) $(ASFLAGS) 
-o $@,"AS","$(TARGET_DIR)$@")
+%.img: %.o %_main.o
+       $(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -m $(LD_I386_EMULATION) -T 
$(SRC_PATH)/pc-bios/optionrom/flat.lds -s -o $@ $?,"BUILD","$(TARGET_DIR)$@")
+
  %.img: %.o
        $(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -m $(LD_I386_EMULATION) -T 
$(SRC_PATH)/pc-bios/optionrom/flat.lds -s -o $@ $<,"BUILD","$(TARGET_DIR)$@")
diff --git a/pc-bios/optionrom/pvh.S b/pc-bios/optionrom/pvh.S
new file mode 100644
index 0000000000..e1d7f4a7a7
--- /dev/null
+++ b/pc-bios/optionrom/pvh.S
@@ -0,0 +1,200 @@
+/*
+ * PVH Option ROM
+ *
+ * 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 Novell Inc, 2009
+ *   Authors: Alexander Graf <ag...@suse.de>
+ *
+ * Copyright (c) 2019 Red Hat Inc.
+ *   Authors: Stefano Garzarella <sgarz...@redhat.com>
+ */
+
+#include "optionrom.h"
+
+#define BOOT_ROM_PRODUCT "PVH loader"
+
+#define GS_PROT_JUMP           0
+#define GS_GDT_DESC            6
+
+#ifdef OPTION_ROM_START
+#undef OPTION_ROM_START
+#endif
+#ifdef OPTION_ROM_END
+#undef OPTION_ROM_END
+#endif
+
+/*
+ * Redefine OPTION_ROM_START and OPTION_ROM_END, because this rom is produced
+ * linking multiple objects.
+ * signrom.py will add padding.
+ */
+#define OPTION_ROM_START                                \
+    .code16;                                           \
+    .text;                                             \
+       .global         _start;                         \
+    _start:;                                           \
+       .short          0xaa55;                         \
+       .byte           3; /* desired size in 512 units */
+
+#define OPTION_ROM_END                                 \
+    _end:
+
+BOOT_ROM_START
+
+run_pvhboot:
+
+       cli
+       cld
+
+       mov             %cs, %eax
+       shl             $0x4, %eax
+
+       /* set up a long jump descriptor that is PC relative */
+
+       /* move stack memory to %gs */
+       mov             %ss, %ecx
+       shl             $0x4, %ecx
+       mov             %esp, %ebx
+       add             %ebx, %ecx
+       sub             $0x20, %ecx
+       sub             $0x30, %esp
+       shr             $0x4, %ecx
+       mov             %cx, %gs
+
+       /* now push the indirect jump descriptor there */
+       mov             (prot_jump), %ebx
+       add             %eax, %ebx
+       movl            %ebx, %gs:GS_PROT_JUMP
+       mov             $8, %bx
+       movw            %bx, %gs:GS_PROT_JUMP + 4
+
+       /* fix the gdt descriptor to be PC relative */
+       movw            (gdt_desc), %bx
+       movw            %bx, %gs:GS_GDT_DESC
+       movl            (gdt_desc+2), %ebx
+       add             %eax, %ebx
+       movl            %ebx, %gs:GS_GDT_DESC + 2
+
+       /* initialize HVM memmap table using int 0x15(e820) */
+
+       /* ES = pvh_e820 struct */
+       mov             $pvh_e820, %eax
+       shr             $4, %eax
+       mov             %ax, %es
+
+       /* start storing memmap table at %es:8 (pvh_e820.table) */
+       mov             $8,%edi
+       xor             %ebx, %ebx
+       jmp             memmap_loop
+
+memmap_loop_check:
+       /* pvh_e820 can contains up to 128 entries */
+       cmp             $128, %ebx
+       je              memmap_done
+
+memmap_loop:
+       /* entry size (hvm_memmap_table_entry) & max buffer size (int15) */
+       movl            $24, %ecx
+       /* e820 */
+       movl            $0x0000e820, %eax
+       /* 'SMAP' magic */
+       movl            $0x534d4150, %edx
+       /* store counter value at %es:0 (pvh_e820.entries) */
+       movl            %ebx, %es:0
+
+       int             $0x15
+       /* error or last entry already done? */
+       jb              memmap_err
+
+       /* %edi += entry size (hvm_memmap_table_entry) */
+       add             $24, %edi
+
+       /* continuation value 0 means last entry */
+       test            %ebx, %ebx
+       jnz             memmap_loop_check
+
+       /* increase pvh_e820.entries to save the last entry */
+       movl            %es:0, %ebx
+       inc             %ebx
+
+memmap_done:
+       movl            %ebx, %es:0
+
+memmap_err:
+
+       /* load the GDT before going into protected mode */
+lgdt:
+       data32 lgdt     %gs:GS_GDT_DESC
+
+       /* get us to protected mode now */
+       movl            $1, %eax
+       movl            %eax, %cr0
+
+       /* the LJMP sets CS for us and gets us to 32-bit */
+ljmp:
+       data32 ljmp     *%gs:GS_PROT_JUMP
+
+prot_mode:
+.code32
+
+       /* initialize all other segments */
+       movl            $0x10, %eax
+       movl            %eax, %ss
+       movl            %eax, %ds
+       movl            %eax, %es
+       movl            %eax, %fs
+       movl            %eax, %gs
+
+       jmp pvh_load_kernel
+
+/* Variables */
+.align 4, 0
+prot_jump:     .long prot_mode
+               .short 8
+
+.align 4, 0
+gdt:
+       /* 0x00 */
+.byte  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+
+       /*
+        * 0x08: code segment
+        * (base=0, limit=0xfffff, type=32bit code exec/read, DPL=0, 4k)
+        */
+.byte  0xff, 0xff, 0x00, 0x00, 0x00, 0x9a, 0xcf, 0x00
+
+       /*
+        * 0x10: data segment
+        * (base=0, limit=0xfffff, type=32bit data read/write, DPL=0, 4k)
+        */
+.byte  0xff, 0xff, 0x00, 0x00, 0x00, 0x92, 0xcf, 0x00
+
+       /*
+        * 0x18: code segment
+        * (base=0, limit=0x0ffff, type=16bit code exec/read/conf, DPL=0, 1b)
+        */
+.byte  0xff, 0xff, 0x00, 0x00, 0x00, 0x9e, 0x00, 0x00
+
+       /*
+        * 0x20: data segment
+        * (base=0, limit=0x0ffff, type=16bit data read/write, DPL=0, 1b)
+        */
+.byte  0xff, 0xff, 0x00, 0x00, 0x00, 0x92, 0x00, 0x00
+
+gdt_desc:
+.short (5 * 8) - 1
+.long  gdt
+
+BOOT_ROM_END
diff --git a/pc-bios/optionrom/pvh_main.c b/pc-bios/optionrom/pvh_main.c
new file mode 100644
index 0000000000..9100952463
--- /dev/null
+++ b/pc-bios/optionrom/pvh_main.c
@@ -0,0 +1,117 @@
+/*
+ * PVH 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) 2019 Red Hat Inc.
+ *   Authors:
+ *     Stefano Garzarella <sgarz...@redhat.com>
+ */
+
+asm (".code32"); /* this code will be executed in protect mode */


"protect" -> "protected"


+
+#include <stddef.h>
+#include <stdint.h>
+#include "optrom.h"
+#include "optrom_fw_cfg.h"
+#include "../../include/hw/xen/start_info.h"
+
+#define RSDP_SIGNATURE          0x2052545020445352LL /* "RSD PTR " */
+#define RSDP_AREA_ADDR          0x000E0000
+#define RSDP_AREA_SIZE          2048
+#define EBDA_BASE_ADDR          0x0000040E
+#define EBDA_SIZE               1024
+
+#define E820_MAXENTRIES         128
+#define CMDLINE_BUFSIZE         4096
+
+/* e820 table filled in pvh.S using int 0x15 */
+struct pvh_e820_table {
+    uint32_t entries;
+    uint32_t reserved;
+    struct hvm_memmap_table_entry table[E820_MAXENTRIES];
+};
+
+struct pvh_e820_table pvh_e820 __attribute__ ((aligned));
+
+struct hvm_start_info start_info;

Can this be static?

+struct hvm_modlist_entry ramdisk_mod;

Is this used?

+uint8_t cmdline_buffer[CMDLINE_BUFSIZE];

Can this be static?

+
+
+/* Search RSDP signature. */
+static uintptr_t search_rsdp(uint32_t start_addr, uint32_t end_addr)
+{
+    uint64_t *rsdp_p;
+
+    /* RSDP signature is always on a 16 byte boundary */
+    for (rsdp_p = (uint64_t *)start_addr; rsdp_p < (uint64_t *)end_addr;
+         rsdp_p += 2) {
+        if (*rsdp_p == RSDP_SIGNATURE) {
+            return (uintptr_t)rsdp_p;
+        }
+    }
+
+    return 0;
+}
+
+/* Force the asm name without leading underscore, even on Win32. */
+extern void pvh_load_kernel(void) asm("pvh_load_kernel");
+
+void pvh_load_kernel(void)
+{
+    void *cmdline_addr = &cmdline_buffer;
+    void *kernel_entry;
+    uint32_t cmdline_size, fw_cfg_version = bios_cfg_version();
+
+    start_info.magic = XEN_HVM_START_MAGIC_VALUE;
+    start_info.version = 1;
+
+    /*
+     * pvh_e820 is filled in the pvh.S before to switch in protected mode,
+     * because we can use int 0x15 only in real mode.
+     */
+    start_info.memmap_entries = pvh_e820.entries;
+    start_info.memmap_paddr = (uintptr_t)pvh_e820.table;
+
+    /*
+     * Search RSDP in the main BIOS area below 1 MB.
+     * SeaBIOS store the RSDP in this area, so we try it first.
+     */
+    start_info.rsdp_paddr = search_rsdp(RSDP_AREA_ADDR,
+                                        RSDP_AREA_ADDR + RSDP_AREA_SIZE);
+
+    /* Search RSDP in the EBDA if it is not found */
+    if (!start_info.rsdp_paddr) {
+        /*
+         * Th EBDA address is stored at EBDA_BASE_ADDR. It contains 2 bytes
+         * segment pointer to EBDA, so we must convert it to a linear address.
+         */
+        uint32_t ebda_paddr = ((uint32_t)*((uint16_t *)EBDA_BASE_ADDR)) << 4;
+        if (ebda_paddr > 0x400) {
+            uint32_t *ebda = (uint32_t *)ebda_paddr;
+
+            start_info.rsdp_paddr = search_rsdp(*ebda, *ebda + EBDA_SIZE);
+        }
+    }
+
+    bios_cfg_read_entry(&cmdline_size, FW_CFG_CMDLINE_SIZE, 4, fw_cfg_version);
+    bios_cfg_read_entry(cmdline_addr, FW_CFG_CMDLINE_DATA, cmdline_size,
+                        fw_cfg_version);
+    start_info.cmdline_paddr = (uintptr_t)cmdline_addr;
+
+    bios_cfg_read_entry(&kernel_entry, FW_CFG_KERNEL_ENTRY, 4, fw_cfg_version);
+
+    asm volatile("jmp *%1" : : "b"(&start_info), "c"(kernel_entry));
+}


Reply via email to