All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/4] pvh: add new PVH option rom
@ 2019-01-15 10:00 Stefano Garzarella
  2019-01-15 10:00 ` [Qemu-devel] [PATCH v2 1/4] linuxboot_dma: remove duplicate definitions of FW_CFG Stefano Garzarella
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Stefano Garzarella @ 2019-01-15 10:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: Boris Ostrovsky, Eduardo Habkost, Michael S. Tsirkin,
	Gerd Hoffmann, George Kennedy, Marcel Apfelbaum, Paolo Bonzini,
	Eric Blake, Marc-Andre Lureau, Richard Henderson,
	Stefan Hajnoczi, Liam Merwick, Maran Wilson

This patch series is based on "[RFC v2 0/4] QEMU changes to do PVH boot" and
provides a PVH option rom that can be used with SeaBIOS to boot uncompressed
kernel using the x86/HVM direct boot ABI.

Patches 1 and 2 are to prepare the PVH option rom, moving common functions in a
new header.  Patch 3 adds the new PVH option rom and patch 4 uses it when we
are booting an uncompressed kernel using the x86/HVM direct boot ABI.

Changes in v2:
- addressed comments by Stefan and Eric:
  - Patch 2: moved inludes on top of linuxboot_dma.c and add <stdint.h> in
    optrom.h
  - Patch 4: added check of pvh.bin in xen_load_linux()
- modified commit message of patch 2 to explain better the patch

Stefano Garzarella (4):
  linuxboot_dma: remove duplicate definitions of FW_CFG
  linuxboot_dma: move common functions in a new header
  optionrom: add new PVH option rom
  hw/i386/pc: use PVH option rom

 hw/i386/pc.c                      |   5 +
 pc-bios/optionrom/Makefile        |   5 +-
 pc-bios/optionrom/linuxboot_dma.c | 112 +++--------------
 pc-bios/optionrom/optrom.h        | 110 ++++++++++++++++
 pc-bios/optionrom/optrom_fw_cfg.h |  92 ++++++++++++++
 pc-bios/optionrom/pvh.S           | 200 ++++++++++++++++++++++++++++++
 pc-bios/optionrom/pvh_main.c      | 117 +++++++++++++++++
 7 files changed, 544 insertions(+), 97 deletions(-)
 create mode 100644 pc-bios/optionrom/optrom.h
 create mode 100644 pc-bios/optionrom/optrom_fw_cfg.h
 create mode 100644 pc-bios/optionrom/pvh.S
 create mode 100644 pc-bios/optionrom/pvh_main.c

-- 
2.20.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2 1/4] linuxboot_dma: remove duplicate definitions of FW_CFG
  2019-01-15 10:00 [Qemu-devel] [PATCH v2 0/4] pvh: add new PVH option rom Stefano Garzarella
@ 2019-01-15 10:00 ` Stefano Garzarella
  2019-01-15 10:00 ` [Qemu-devel] [PATCH v2 2/4] linuxboot_dma: move common functions in a new header Stefano Garzarella
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Stefano Garzarella @ 2019-01-15 10:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: Boris Ostrovsky, Eduardo Habkost, Michael S. Tsirkin,
	Gerd Hoffmann, George Kennedy, Marcel Apfelbaum, Paolo Bonzini,
	Eric Blake, Marc-Andre Lureau, Richard Henderson,
	Stefan Hajnoczi, Liam Merwick, Maran Wilson

FW_CFG_DMA_CTL_* bits and struct fw_cfg_dma_access are
defined in the qemu_fw_cfg.h header file already included
in linuxboot_dma.c, so we can remove the definition of
BIOS_CFG_DMA_CTL_* and struct FWCfgDmaAccess.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 pc-bios/optionrom/linuxboot_dma.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/pc-bios/optionrom/linuxboot_dma.c b/pc-bios/optionrom/linuxboot_dma.c
index d856d41b55..f728dc839f 100644
--- a/pc-bios/optionrom/linuxboot_dma.c
+++ b/pc-bios/optionrom/linuxboot_dma.c
@@ -58,12 +58,6 @@ asm(
 "   jmp load_kernel\n"
 );
 
-/* 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
 
@@ -75,12 +69,6 @@ asm(
 
 #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));
@@ -153,9 +141,9 @@ static inline uint32_t be32_to_cpu(uint32_t x)
 static __attribute__((__noinline__))
 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;
+    struct fw_cfg_dma_access access;
+    uint32_t control = (entry << 16) | FW_CFG_DMA_CTL_SELECT
+                        | FW_CFG_DMA_CTL_READ;
 
     access.address = cpu_to_be64((uint64_t)(uint32_t)buf);
     access.length = cpu_to_be32(len);
@@ -165,7 +153,7 @@ void bios_cfg_read_entry(void *buf, uint16_t entry, uint32_t len)
 
     outl(cpu_to_be32((uint32_t)&access), BIOS_CFG_DMA_ADDR_LOW);
 
-    while (be32_to_cpu(access.control) & ~BIOS_CFG_DMA_CTL_ERROR) {
+    while (be32_to_cpu(access.control) & ~FW_CFG_DMA_CTL_ERROR) {
         barrier();
     }
 }
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2 2/4] linuxboot_dma: move common functions in a new header
  2019-01-15 10:00 [Qemu-devel] [PATCH v2 0/4] pvh: add new PVH option rom Stefano Garzarella
  2019-01-15 10:00 ` [Qemu-devel] [PATCH v2 1/4] linuxboot_dma: remove duplicate definitions of FW_CFG Stefano Garzarella
@ 2019-01-15 10:00 ` Stefano Garzarella
  2019-01-15 10:00 ` [Qemu-devel] [PATCH v2 3/4] optionrom: add new PVH option rom Stefano Garzarella
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Stefano Garzarella @ 2019-01-15 10:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: Boris Ostrovsky, Eduardo Habkost, Michael S. Tsirkin,
	Gerd Hoffmann, George Kennedy, Marcel Apfelbaum, Paolo Bonzini,
	Eric Blake, Marc-Andre Lureau, Richard Henderson,
	Stefan Hajnoczi, Liam Merwick, Maran Wilson

In order to allow other option roms to use these common
useful functions and definitions, this patch put them
in two new C header files called optrom.h and
optrom_fw_cfg.h. We also add useful out*() in*()
functions for different size, and new fw_cfg functions
to use when DMA feature is not available.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 pc-bios/optionrom/linuxboot_dma.c | 100 +++++----------------------
 pc-bios/optionrom/optrom.h        | 110 ++++++++++++++++++++++++++++++
 pc-bios/optionrom/optrom_fw_cfg.h |  92 +++++++++++++++++++++++++
 3 files changed, 218 insertions(+), 84 deletions(-)
 create mode 100644 pc-bios/optionrom/optrom.h
 create mode 100644 pc-bios/optionrom/optrom_fw_cfg.h

diff --git a/pc-bios/optionrom/linuxboot_dma.c b/pc-bios/optionrom/linuxboot_dma.c
index f728dc839f..bf6ee79c38 100644
--- a/pc-bios/optionrom/linuxboot_dma.c
+++ b/pc-bios/optionrom/linuxboot_dma.c
@@ -20,6 +20,10 @@
  *     Richard W.M. Jones <rjones@redhat.com>
  */
 
+#include <stdint.h>
+#include "optrom.h"
+#include "optrom_fw_cfg.h"
+
 asm(
 ".text\n"
 ".global _start\n"
@@ -58,34 +62,12 @@ asm(
 "   jmp load_kernel\n"
 );
 
-#define BIOS_CFG_DMA_ADDR_HIGH 0x514
-#define BIOS_CFG_DMA_ADDR_LOW  0x518
-
-#define uint64_t unsigned long long
-#define uint32_t unsigned int
-#define uint16_t unsigned short
-
-#include "../../include/standard-headers/linux/qemu_fw_cfg.h"
-
-#define barrier() asm("" : : : "memory")
-
-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));
 }
 
-#ifdef __clang__
-#define ADDR32
-#else
-#define ADDR32 "addr32 "
-#endif
-
 static inline uint16_t readw_es(uint16_t offset)
 {
     uint16_t val;
@@ -108,56 +90,6 @@ static inline void writel_es(uint16_t offset, uint32_t val)
     asm(ADDR32 "movl %0, %%es:(%1)" : : "r"(val), "r"((uint32_t)offset));
 }
 
-static inline uint32_t bswap32(uint32_t x)
-{
-    asm("bswapl %0" : "=r" (x) : "0" (x));
-    return x;
-}
-
-static inline uint64_t bswap64(uint64_t x)
-{
-    asm("bswapl %%eax; bswapl %%edx; xchg %%eax, %%edx" : "=A" (x) : "0" (x));
-    return x;
-}
-
-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);
-}
-
-/* clang is happy to inline this function, and bloats the
- * ROM.
- */
-static __attribute__((__noinline__))
-void bios_cfg_read_entry(void *buf, uint16_t entry, uint32_t len)
-{
-    struct fw_cfg_dma_access access;
-    uint32_t control = (entry << 16) | FW_CFG_DMA_CTL_SELECT
-                        | FW_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) & ~FW_CFG_DMA_CTL_ERROR) {
-        barrier();
-    }
-}
-
 /* Return top of memory using BIOS function E801. */
 static uint32_t get_e801_addr(void)
 {
@@ -211,9 +143,9 @@ void load_kernel(void)
     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);
+    bios_cfg_read_entry_dma(&setup_addr, FW_CFG_SETUP_ADDR, 4);
+    bios_cfg_read_entry_dma(&setup_size, FW_CFG_SETUP_SIZE, 4);
+    bios_cfg_read_entry_dma(setup_addr, FW_CFG_SETUP_DATA, setup_size);
 
     set_es(setup_addr);
 
@@ -223,8 +155,8 @@ void load_kernel(void)
         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);
+    bios_cfg_read_entry_dma(&initrd_addr, FW_CFG_INITRD_ADDR, 4);
+    bios_cfg_read_entry_dma(&initrd_size, FW_CFG_INITRD_SIZE, 4);
 
     initrd_end_page = ((uint32_t)(initrd_addr + initrd_size) & -4096);
     max_allowed_page = (readl_es(0x22c) & -4096);
@@ -239,15 +171,15 @@ void load_kernel(void)
 
     }
 
-    bios_cfg_read_entry(initrd_addr, FW_CFG_INITRD_DATA, initrd_size);
+    bios_cfg_read_entry_dma(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_dma(&kernel_addr, FW_CFG_KERNEL_ADDR, 4);
+    bios_cfg_read_entry_dma(&kernel_size, FW_CFG_KERNEL_SIZE, 4);
+    bios_cfg_read_entry_dma(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);
+    bios_cfg_read_entry_dma(&cmdline_addr, FW_CFG_CMDLINE_ADDR, 4);
+    bios_cfg_read_entry_dma(&cmdline_size, FW_CFG_CMDLINE_SIZE, 4);
+    bios_cfg_read_entry_dma(cmdline_addr, FW_CFG_CMDLINE_DATA, cmdline_size);
 
     /* Boot linux */
     segment_addr = ((uint32_t)setup_addr >> 4);
diff --git a/pc-bios/optionrom/optrom.h b/pc-bios/optionrom/optrom.h
new file mode 100644
index 0000000000..c5c8caa22f
--- /dev/null
+++ b/pc-bios/optionrom/optrom.h
@@ -0,0 +1,110 @@
+/*
+ * Common Option ROM Functions for C code
+ *
+ * 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-2019 Red Hat Inc.
+ *   Authors:
+ *     Marc Marí <marc.mari.barcelo@gmail.com>
+ *     Richard W.M. Jones <rjones@redhat.com>
+ *     Stefano Garzarella <sgarzare@redhat.com>
+ */
+
+#ifndef OPTROM_H
+#define OPTROM_H
+
+#include <stdint.h>
+#include "../../include/standard-headers/linux/qemu_fw_cfg.h"
+
+#define barrier() asm("" : : : "memory")
+
+#ifdef __clang__
+#define ADDR32
+#else
+#define ADDR32 "addr32 "
+#endif
+
+static inline void outb(uint8_t value, uint16_t port)
+{
+    asm volatile("outb %0, %w1" : : "a"(value), "Nd"(port));
+}
+
+static inline void outw(uint16_t value, uint16_t port)
+{
+    asm volatile("outw %0, %w1" : : "a"(value), "Nd"(port));
+}
+
+static inline void outl(uint32_t value, uint16_t port)
+{
+    asm volatile("outl %0, %w1" : : "a"(value), "Nd"(port));
+}
+
+static inline uint8_t inb(uint16_t port)
+{
+    uint8_t value;
+
+    asm volatile("outl %w1, %0" : : "a"(value), "Nd"(port));
+    return value;
+}
+
+static inline uint16_t inw(uint16_t port)
+{
+    uint16_t value;
+
+    asm volatile("outl %w1, %0" : : "a"(value), "Nd"(port));
+    return value;
+}
+
+static inline uint32_t inl(uint16_t port)
+{
+    uint32_t value;
+
+    asm volatile("outl %w1, %0" : : "a"(value), "Nd"(port));
+    return value;
+}
+
+static inline void insb(uint16_t port, uint8_t *buf, uint32_t len)
+{
+    asm volatile("rep insb (%%dx), %%es:(%%edi)"
+                 : "+c"(len), "+D"(buf) : "d"(port) : "memory");
+}
+
+static inline uint32_t bswap32(uint32_t x)
+{
+    asm("bswapl %0" : "=r" (x) : "0" (x));
+    return x;
+}
+
+static inline uint64_t bswap64(uint64_t x)
+{
+    asm("bswapl %%eax; bswapl %%edx; xchg %%eax, %%edx" : "=A" (x) : "0" (x));
+    return x;
+}
+
+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);
+}
+
+#endif /* OPTROM_H */
diff --git a/pc-bios/optionrom/optrom_fw_cfg.h b/pc-bios/optionrom/optrom_fw_cfg.h
new file mode 100644
index 0000000000..a3660a5200
--- /dev/null
+++ b/pc-bios/optionrom/optrom_fw_cfg.h
@@ -0,0 +1,92 @@
+/*
+ * Common Option ROM Functions for fw_cfg
+ *
+ * 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-2019 Red Hat Inc.
+ *   Authors:
+ *     Marc Marí <marc.mari.barcelo@gmail.com>
+ *     Richard W.M. Jones <rjones@redhat.com>
+ *     Stefano Garzarella <sgarzare@redhat.com>
+ */
+
+#ifndef OPTROM_FW_CFG_H
+#define OPTROM_FW_CFG_H
+
+#include "../../include/standard-headers/linux/qemu_fw_cfg.h"
+
+#define BIOS_CFG_IOPORT_CFG     0x510
+#define BIOS_CFG_IOPORT_DATA    0x511
+#define BIOS_CFG_DMA_ADDR_HIGH  0x514
+#define BIOS_CFG_DMA_ADDR_LOW   0x518
+
+static __attribute__((unused))
+void bios_cfg_select(uint16_t key)
+{
+    outw(key, BIOS_CFG_IOPORT_CFG);
+}
+
+static __attribute__((unused))
+void bios_cfg_read_entry_io(void *buf, uint16_t entry, uint32_t len)
+{
+    bios_cfg_select(entry);
+    insb(BIOS_CFG_IOPORT_DATA, buf, len);
+}
+
+/*
+ * clang is happy to inline this function, and bloats the
+ * ROM.
+ */
+static __attribute__((__noinline__)) __attribute__((unused))
+void bios_cfg_read_entry_dma(void *buf, uint16_t entry, uint32_t len)
+{
+    struct fw_cfg_dma_access access;
+    uint32_t control = (entry << 16) | FW_CFG_DMA_CTL_SELECT
+                        | FW_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) & ~FW_CFG_DMA_CTL_ERROR) {
+        barrier();
+    }
+}
+
+static __attribute__((unused))
+void bios_cfg_read_entry(void *buf, uint16_t entry, uint32_t len,
+                         uint32_t version)
+{
+    if (version & FW_CFG_VERSION_DMA) {
+        bios_cfg_read_entry_dma(buf, entry, len);
+    } else {
+        bios_cfg_read_entry_io(buf, entry, len);
+    }
+}
+
+static __attribute__((unused))
+uint32_t bios_cfg_version(void)
+{
+    uint32_t version;
+
+    bios_cfg_read_entry_io(&version, FW_CFG_ID, sizeof(version));
+
+    return version;
+}
+
+#endif /* OPTROM_FW_CFG_H */
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2 3/4] optionrom: add new PVH option rom
  2019-01-15 10:00 [Qemu-devel] [PATCH v2 0/4] pvh: add new PVH option rom Stefano Garzarella
  2019-01-15 10:00 ` [Qemu-devel] [PATCH v2 1/4] linuxboot_dma: remove duplicate definitions of FW_CFG Stefano Garzarella
  2019-01-15 10:00 ` [Qemu-devel] [PATCH v2 2/4] linuxboot_dma: move common functions in a new header Stefano Garzarella
@ 2019-01-15 10:00 ` Stefano Garzarella
  2019-01-16 10:16   ` Liam Merwick
  2019-01-15 10:00 ` [Qemu-devel] [PATCH v2 4/4] hw/i386/pc: use " Stefano Garzarella
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Stefano Garzarella @ 2019-01-15 10:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: Boris Ostrovsky, Eduardo Habkost, Michael S. Tsirkin,
	Gerd Hoffmann, George Kennedy, Marcel Apfelbaum, Paolo Bonzini,
	Eric Blake, Marc-Andre Lureau, Richard Henderson,
	Stefan Hajnoczi, Liam Merwick, Maran Wilson

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
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 <sgarzare@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@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 <agraf@suse.de>
+ *
+ * Copyright (c) 2019 Red Hat Inc.
+ *   Authors: Stefano Garzarella <sgarzare@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 <sgarzare@redhat.com>
+ */
+
+asm (".code32"); /* this code will be executed in protect mode */
+
+#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;
+struct hvm_modlist_entry ramdisk_mod;
+uint8_t cmdline_buffer[CMDLINE_BUFSIZE];
+
+
+/* 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));
+}
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2 4/4] hw/i386/pc: use PVH option rom
  2019-01-15 10:00 [Qemu-devel] [PATCH v2 0/4] pvh: add new PVH option rom Stefano Garzarella
                   ` (2 preceding siblings ...)
  2019-01-15 10:00 ` [Qemu-devel] [PATCH v2 3/4] optionrom: add new PVH option rom Stefano Garzarella
@ 2019-01-15 10:00 ` Stefano Garzarella
  2019-01-15 18:57   ` Michael S. Tsirkin
  2019-01-15 14:24 ` [Qemu-devel] [PATCH v2 0/4] pvh: add new " Stefan Hajnoczi
  2019-01-16 10:18 ` Liam Merwick
  5 siblings, 1 reply; 16+ messages in thread
From: Stefano Garzarella @ 2019-01-15 10:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: Boris Ostrovsky, Eduardo Habkost, Michael S. Tsirkin,
	Gerd Hoffmann, George Kennedy, Marcel Apfelbaum, Paolo Bonzini,
	Eric Blake, Marc-Andre Lureau, Richard Henderson,
	Stefan Hajnoczi, Liam Merwick, Maran Wilson

Use pvh.bin option rom when we are booting an uncompressed
kernel using the x86/HVM direct boot ABI.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Based-on: <1545422632-24444-5-git-send-email-liam.merwick@oracle.com>
---
 hw/i386/pc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 06bce6a101..7564ba51d2 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1005,6 +1005,10 @@ static void load_linux(PCMachineState *pcms,
             fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA,
                              header, sizeof(header));
 
+            option_rom[nb_option_roms].bootindex = 0;
+            option_rom[nb_option_roms].name = "pvh.bin";
+            nb_option_roms++;
+
             return;
         }
         /* This looks like a multiboot kernel. If it is, let's stop
@@ -1456,6 +1460,7 @@ void xen_load_linux(PCMachineState *pcms)
     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, "pvh.bin") ||
                !strcmp(option_rom[i].name, "multiboot.bin"));
         rom_add_option(option_rom[i].name, option_rom[i].bootindex);
     }
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/4] pvh: add new PVH option rom
  2019-01-15 10:00 [Qemu-devel] [PATCH v2 0/4] pvh: add new PVH option rom Stefano Garzarella
                   ` (3 preceding siblings ...)
  2019-01-15 10:00 ` [Qemu-devel] [PATCH v2 4/4] hw/i386/pc: use " Stefano Garzarella
@ 2019-01-15 14:24 ` Stefan Hajnoczi
  2019-01-16 10:18 ` Liam Merwick
  5 siblings, 0 replies; 16+ messages in thread
From: Stefan Hajnoczi @ 2019-01-15 14:24 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: qemu-devel, Eduardo Habkost, Michael S. Tsirkin, Stefan Hajnoczi,
	George Kennedy, Gerd Hoffmann, Marc-Andre Lureau, Paolo Bonzini,
	Boris Ostrovsky, Maran Wilson, Richard Henderson

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

On Tue, Jan 15, 2019 at 11:00:54AM +0100, Stefano Garzarella wrote:
> This patch series is based on "[RFC v2 0/4] QEMU changes to do PVH boot" and
> provides a PVH option rom that can be used with SeaBIOS to boot uncompressed
> kernel using the x86/HVM direct boot ABI.
> 
> Patches 1 and 2 are to prepare the PVH option rom, moving common functions in a
> new header.  Patch 3 adds the new PVH option rom and patch 4 uses it when we
> are booting an uncompressed kernel using the x86/HVM direct boot ABI.
> 
> Changes in v2:
> - addressed comments by Stefan and Eric:
>   - Patch 2: moved inludes on top of linuxboot_dma.c and add <stdint.h> in
>     optrom.h
>   - Patch 4: added check of pvh.bin in xen_load_linux()
> - modified commit message of patch 2 to explain better the patch
> 
> Stefano Garzarella (4):
>   linuxboot_dma: remove duplicate definitions of FW_CFG
>   linuxboot_dma: move common functions in a new header
>   optionrom: add new PVH option rom
>   hw/i386/pc: use PVH option rom
> 
>  hw/i386/pc.c                      |   5 +
>  pc-bios/optionrom/Makefile        |   5 +-
>  pc-bios/optionrom/linuxboot_dma.c | 112 +++--------------
>  pc-bios/optionrom/optrom.h        | 110 ++++++++++++++++
>  pc-bios/optionrom/optrom_fw_cfg.h |  92 ++++++++++++++
>  pc-bios/optionrom/pvh.S           | 200 ++++++++++++++++++++++++++++++
>  pc-bios/optionrom/pvh_main.c      | 117 +++++++++++++++++
>  7 files changed, 544 insertions(+), 97 deletions(-)
>  create mode 100644 pc-bios/optionrom/optrom.h
>  create mode 100644 pc-bios/optionrom/optrom_fw_cfg.h
>  create mode 100644 pc-bios/optionrom/pvh.S
>  create mode 100644 pc-bios/optionrom/pvh_main.c
> 
> -- 
> 2.20.1
> 
> 

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

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/4] hw/i386/pc: use PVH option rom
  2019-01-15 10:00 ` [Qemu-devel] [PATCH v2 4/4] hw/i386/pc: use " Stefano Garzarella
@ 2019-01-15 18:57   ` Michael S. Tsirkin
  2019-01-15 19:12     ` Eric Blake
  2019-01-18 17:33     ` Eduardo Habkost
  0 siblings, 2 replies; 16+ messages in thread
From: Michael S. Tsirkin @ 2019-01-15 18:57 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: qemu-devel, Boris Ostrovsky, Eduardo Habkost, Gerd Hoffmann,
	George Kennedy, Marcel Apfelbaum, Paolo Bonzini, Eric Blake,
	Marc-Andre Lureau, Richard Henderson, Stefan Hajnoczi,
	Liam Merwick, Maran Wilson

On Tue, Jan 15, 2019 at 11:00:58AM +0100, Stefano Garzarella wrote:
> Use pvh.bin option rom when we are booting an uncompressed
> kernel using the x86/HVM direct boot ABI.
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> Based-on: <1545422632-24444-5-git-send-email-liam.merwick@oracle.com>

I don't think this is a great way to give attribution.
Can you pls include the author name and the S.O.B from there as well?


> ---
>  hw/i386/pc.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 06bce6a101..7564ba51d2 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1005,6 +1005,10 @@ static void load_linux(PCMachineState *pcms,
>              fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA,
>                               header, sizeof(header));
>  
> +            option_rom[nb_option_roms].bootindex = 0;
> +            option_rom[nb_option_roms].name = "pvh.bin";
> +            nb_option_roms++;
> +
>              return;
>          }
>          /* This looks like a multiboot kernel. If it is, let's stop
> @@ -1456,6 +1460,7 @@ void xen_load_linux(PCMachineState *pcms)
>      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, "pvh.bin") ||
>                 !strcmp(option_rom[i].name, "multiboot.bin"));
>          rom_add_option(option_rom[i].name, option_rom[i].bootindex);
>      }

OK but this is guest visible so needs to be guarded by the
new machine type.


> -- 
> 2.20.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/4] hw/i386/pc: use PVH option rom
  2019-01-15 18:57   ` Michael S. Tsirkin
@ 2019-01-15 19:12     ` Eric Blake
  2019-01-15 20:05       ` Michael S. Tsirkin
  2019-01-16  8:22       ` Stefano Garzarella
  2019-01-18 17:33     ` Eduardo Habkost
  1 sibling, 2 replies; 16+ messages in thread
From: Eric Blake @ 2019-01-15 19:12 UTC (permalink / raw)
  To: Michael S. Tsirkin, Stefano Garzarella
  Cc: qemu-devel, Boris Ostrovsky, Eduardo Habkost, Gerd Hoffmann,
	George Kennedy, Marcel Apfelbaum, Paolo Bonzini,
	Marc-Andre Lureau, Richard Henderson, Stefan Hajnoczi,
	Liam Merwick, Maran Wilson

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

On 1/15/19 12:57 PM, Michael S. Tsirkin wrote:
> On Tue, Jan 15, 2019 at 11:00:58AM +0100, Stefano Garzarella wrote:
>> Use pvh.bin option rom when we are booting an uncompressed
>> kernel using the x86/HVM direct boot ABI.
>>
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> Based-on: <1545422632-24444-5-git-send-email-liam.merwick@oracle.com>
> 
> I don't think this is a great way to give attribution.
> Can you pls include the author name and the S.O.B from there as well?
> 
> 
>> ---

Patchew understands Based-on: tags as meaning a prerequisite patch on
the list that must be applied first before this patch can apply (and NOT
in the sense that this patch is an updated revision to replace an
earlier revision posted by someone else) - but when using the annotation
to keep Patchew happy, it should either be in a 0/N cover letter, or for
a single patch, after the --- separator, as the condition of a
prerequisite patch not being applied is a transient condition relevant
to the current build but not something that needs to be recorded in
long-term git history.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 484 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/4] hw/i386/pc: use PVH option rom
  2019-01-15 19:12     ` Eric Blake
@ 2019-01-15 20:05       ` Michael S. Tsirkin
  2019-01-15 21:50         ` Paolo Bonzini
  2019-01-16  8:19         ` Stefano Garzarella
  2019-01-16  8:22       ` Stefano Garzarella
  1 sibling, 2 replies; 16+ messages in thread
From: Michael S. Tsirkin @ 2019-01-15 20:05 UTC (permalink / raw)
  To: Eric Blake
  Cc: Stefano Garzarella, qemu-devel, Boris Ostrovsky, Eduardo Habkost,
	Gerd Hoffmann, George Kennedy, Marcel Apfelbaum, Paolo Bonzini,
	Marc-Andre Lureau, Richard Henderson, Stefan Hajnoczi,
	Liam Merwick, Maran Wilson

On Tue, Jan 15, 2019 at 01:12:21PM -0600, Eric Blake wrote:
> On 1/15/19 12:57 PM, Michael S. Tsirkin wrote:
> > On Tue, Jan 15, 2019 at 11:00:58AM +0100, Stefano Garzarella wrote:
> >> Use pvh.bin option rom when we are booting an uncompressed
> >> kernel using the x86/HVM direct boot ABI.
> >>
> >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> >> Based-on: <1545422632-24444-5-git-send-email-liam.merwick@oracle.com>
> > 
> > I don't think this is a great way to give attribution.
> > Can you pls include the author name and the S.O.B from there as well?
> > 
> > 
> >> ---
> 
> Patchew understands Based-on: tags as meaning a prerequisite patch on
> the list that must be applied first before this patch can apply (and NOT
> in the sense that this patch is an updated revision to replace an
> earlier revision posted by someone else) - but when using the annotation
> to keep Patchew happy, it should either be in a 0/N cover letter, or for
> a single patch, after the --- separator, as the condition of a
> prerequisite patch not being applied is a transient condition relevant
> to the current build but not something that needs to be recorded in
> long-term git history.
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
> 

Good to know.
But I still think listing the original author (e.g. Based on patches by
Foo Bar) or such is the nice thing to do.

-- 
MST

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/4] hw/i386/pc: use PVH option rom
  2019-01-15 20:05       ` Michael S. Tsirkin
@ 2019-01-15 21:50         ` Paolo Bonzini
  2019-01-16  8:19         ` Stefano Garzarella
  1 sibling, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2019-01-15 21:50 UTC (permalink / raw)
  To: Michael S. Tsirkin, Eric Blake
  Cc: Stefano Garzarella, qemu-devel, Boris Ostrovsky, Eduardo Habkost,
	Gerd Hoffmann, George Kennedy, Marcel Apfelbaum,
	Marc-Andre Lureau, Richard Henderson, Stefan Hajnoczi,
	Liam Merwick, Maran Wilson

On 15/01/19 21:05, Michael S. Tsirkin wrote:
> On Tue, Jan 15, 2019 at 01:12:21PM -0600, Eric Blake wrote:
>> On 1/15/19 12:57 PM, Michael S. Tsirkin wrote:
>>> On Tue, Jan 15, 2019 at 11:00:58AM +0100, Stefano Garzarella wrote:
>>>> Use pvh.bin option rom when we are booting an uncompressed
>>>> kernel using the x86/HVM direct boot ABI.
>>>>
>>>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>>>> Based-on: <1545422632-24444-5-git-send-email-liam.merwick@oracle.com>
>>>
>>> I don't think this is a great way to give attribution.
>>> Can you pls include the author name and the S.O.B from there as well?
>
> Good to know.
> But I still think listing the original author (e.g. Based on patches by
> Foo Bar) or such is the nice thing to do.

It's not based on patches from another author, the work is entirely
Stefano's.  It must be applied on top of those patches.

Paolo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/4] hw/i386/pc: use PVH option rom
  2019-01-15 20:05       ` Michael S. Tsirkin
  2019-01-15 21:50         ` Paolo Bonzini
@ 2019-01-16  8:19         ` Stefano Garzarella
  1 sibling, 0 replies; 16+ messages in thread
From: Stefano Garzarella @ 2019-01-16  8:19 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Eric Blake, qemu-devel, Boris Ostrovsky, Eduardo Habkost,
	Gerd Hoffmann, George Kennedy, Marcel Apfelbaum, Paolo Bonzini,
	Marc-Andre Lureau, Richard Henderson, Stefan Hajnoczi,
	Liam Merwick, Maran Wilson

Hi Michael,

On Tue, Jan 15, 2019 at 03:05:27PM -0500, Michael S. Tsirkin wrote:
> On Tue, Jan 15, 2019 at 01:12:21PM -0600, Eric Blake wrote:
> > On 1/15/19 12:57 PM, Michael S. Tsirkin wrote:
> > > On Tue, Jan 15, 2019 at 11:00:58AM +0100, Stefano Garzarella wrote:
> > >> Use pvh.bin option rom when we are booting an uncompressed
> > >> kernel using the x86/HVM direct boot ABI.
> > >>
> > >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > >> Based-on: <1545422632-24444-5-git-send-email-liam.merwick@oracle.com>
> > > 
> > > I don't think this is a great way to give attribution.
> > > Can you pls include the author name and the S.O.B from there as well?
> > > 
> > > 
> > >> ---
> > 
> > Patchew understands Based-on: tags as meaning a prerequisite patch on
> > the list that must be applied first before this patch can apply (and NOT
> > in the sense that this patch is an updated revision to replace an
> > earlier revision posted by someone else) - but when using the annotation
> > to keep Patchew happy, it should either be in a 0/N cover letter, or for
> > a single patch, after the --- separator, as the condition of a
> > prerequisite patch not being applied is a transient condition relevant
> > to the current build but not something that needs to be recorded in
> > long-term git history.
> > 
> > -- 
> > Eric Blake, Principal Software Engineer
> > Red Hat, Inc.           +1-919-301-3226
> > Virtualization:  qemu.org | libvirt.org
> > 
> 
> Good to know.
> But I still think listing the original author (e.g. Based on patches by
> Foo Bar) or such is the nice thing to do.

very sorry for the misunderstanding, as Eric and Paolo said, I used Based-on
tag only to inform Patchew and maintainers that this patch must be applied
after Liam's patch that is under review.

Thanks,
Stefano

> 
> -- 
> MST

-- 

Stefano Garzarella
Red Hat

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/4] hw/i386/pc: use PVH option rom
  2019-01-15 19:12     ` Eric Blake
  2019-01-15 20:05       ` Michael S. Tsirkin
@ 2019-01-16  8:22       ` Stefano Garzarella
  1 sibling, 0 replies; 16+ messages in thread
From: Stefano Garzarella @ 2019-01-16  8:22 UTC (permalink / raw)
  To: Eric Blake
  Cc: Michael S. Tsirkin, qemu-devel, Boris Ostrovsky, Eduardo Habkost,
	Gerd Hoffmann, George Kennedy, Marcel Apfelbaum, Paolo Bonzini,
	Marc-Andre Lureau, Richard Henderson, Stefan Hajnoczi,
	Liam Merwick, Maran Wilson

Hi Eric,

On Tue, Jan 15, 2019 at 01:12:21PM -0600, Eric Blake wrote:
> On 1/15/19 12:57 PM, Michael S. Tsirkin wrote:
> > On Tue, Jan 15, 2019 at 11:00:58AM +0100, Stefano Garzarella wrote:
> >> Use pvh.bin option rom when we are booting an uncompressed
> >> kernel using the x86/HVM direct boot ABI.
> >>
> >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> >> Based-on: <1545422632-24444-5-git-send-email-liam.merwick@oracle.com>
> > 
> > I don't think this is a great way to give attribution.
> > Can you pls include the author name and the S.O.B from there as well?
> > 
> > 
> >> ---
> 
> Patchew understands Based-on: tags as meaning a prerequisite patch on
> the list that must be applied first before this patch can apply (and NOT
> in the sense that this patch is an updated revision to replace an
> earlier revision posted by someone else) - but when using the annotation
> to keep Patchew happy, it should either be in a 0/N cover letter, or for
> a single patch, after the --- separator, as the condition of a
> prerequisite patch not being applied is a transient condition relevant
> to the current build but not something that needs to be recorded in
> long-term git history.
> 

Thank you very much for the explanation! I'll move the Based-on tag in
the cover letter.

Cheers,
Stefano

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 3/4] optionrom: add new PVH option rom
  2019-01-15 10:00 ` [Qemu-devel] [PATCH v2 3/4] optionrom: add new PVH option rom Stefano Garzarella
@ 2019-01-16 10:16   ` Liam Merwick
  2019-01-16 11:02     ` Stefano Garzarella
  0 siblings, 1 reply; 16+ messages in thread
From: Liam Merwick @ 2019-01-16 10:16 UTC (permalink / raw)
  To: Stefano Garzarella, qemu-devel
  Cc: Boris Ostrovsky, Eduardo Habkost, Michael S. Tsirkin,
	Gerd Hoffmann, George Kennedy, Marcel Apfelbaum, Paolo Bonzini,
	Eric Blake, Marc-Andre Lureau, Richard Henderson,
	Stefan Hajnoczi, Maran Wilson, liam.merwick

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 <sgarzare@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@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 <agraf@suse.de>
> + *
> + * Copyright (c) 2019 Red Hat Inc.
> + *   Authors: Stefano Garzarella <sgarzare@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 <sgarzare@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));
> +}
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/4] pvh: add new PVH option rom
  2019-01-15 10:00 [Qemu-devel] [PATCH v2 0/4] pvh: add new PVH option rom Stefano Garzarella
                   ` (4 preceding siblings ...)
  2019-01-15 14:24 ` [Qemu-devel] [PATCH v2 0/4] pvh: add new " Stefan Hajnoczi
@ 2019-01-16 10:18 ` Liam Merwick
  5 siblings, 0 replies; 16+ messages in thread
From: Liam Merwick @ 2019-01-16 10:18 UTC (permalink / raw)
  To: Stefano Garzarella, qemu-devel
  Cc: Boris Ostrovsky, Eduardo Habkost, Michael S. Tsirkin,
	Gerd Hoffmann, George Kennedy, Marcel Apfelbaum, Paolo Bonzini,
	Eric Blake, Marc-Andre Lureau, Richard Henderson,
	Stefan Hajnoczi, Maran Wilson


On 15/01/2019 10:00, Stefano Garzarella wrote:
> This patch series is based on "[RFC v2 0/4] QEMU changes to do PVH boot" and
> provides a PVH option rom that can be used with SeaBIOS to boot uncompressed
> kernel using the x86/HVM direct boot ABI.
> 
> Patches 1 and 2 are to prepare the PVH option rom, moving common functions in a
> new header.  Patch 3 adds the new PVH option rom and patch 4 uses it when we
> are booting an uncompressed kernel using the x86/HVM direct boot ABI.
> 
> Changes in v2:
> - addressed comments by Stefan and Eric:
>    - Patch 2: moved inludes on top of linuxboot_dma.c and add <stdint.h> in
>      optrom.h
>    - Patch 4: added check of pvh.bin in xen_load_linux()
> - modified commit message of patch 2 to explain better the patch
> 
> Stefano Garzarella (4):
>    linuxboot_dma: remove duplicate definitions of FW_CFG
>    linuxboot_dma: move common functions in a new header
>    optionrom: add new PVH option rom
>    hw/i386/pc: use PVH option rom
> 
>   hw/i386/pc.c                      |   5 +
>   pc-bios/optionrom/Makefile        |   5 +-
>   pc-bios/optionrom/linuxboot_dma.c | 112 +++--------------
>   pc-bios/optionrom/optrom.h        | 110 ++++++++++++++++
>   pc-bios/optionrom/optrom_fw_cfg.h |  92 ++++++++++++++
>   pc-bios/optionrom/pvh.S           | 200 ++++++++++++++++++++++++++++++
>   pc-bios/optionrom/pvh_main.c      | 117 +++++++++++++++++
>   7 files changed, 544 insertions(+), 97 deletions(-)
>   create mode 100644 pc-bios/optionrom/optrom.h
>   create mode 100644 pc-bios/optionrom/optrom_fw_cfg.h
>   create mode 100644 pc-bios/optionrom/pvh.S
>   create mode 100644 pc-bios/optionrom/pvh_main.c
> 

I had a few very minor comments on patch3, but with that, for the series:

Reviewed-by: Liam Merwick <liam.merwick@oracle.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 3/4] optionrom: add new PVH option rom
  2019-01-16 10:16   ` Liam Merwick
@ 2019-01-16 11:02     ` Stefano Garzarella
  0 siblings, 0 replies; 16+ messages in thread
From: Stefano Garzarella @ 2019-01-16 11:02 UTC (permalink / raw)
  To: Liam Merwick
  Cc: qemu-devel, Boris Ostrovsky, Eduardo Habkost, Michael S. Tsirkin,
	Gerd Hoffmann, George Kennedy, Marcel Apfelbaum, Paolo Bonzini,
	Eric Blake, Marc-Andre Lureau, Richard Henderson,
	Stefan Hajnoczi, Maran Wilson

Hi Liam,

On Wed, Jan 16, 2019 at 10:16:04AM +0000, Liam Merwick wrote:
> 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"
> 

Thanks!

> > diff --git a/pc-bios/optionrom/pvh_main.c b/pc-bios/optionrom/pvh_main.c
> > +
> > +asm (".code32"); /* this code will be executed in protect mode */
> 
> 
> "protect" -> "protected"
> 
> 

Thanks!

> > +
> > +struct hvm_start_info start_info;
> 
> Can this be static?
> 

Yes.

> > +struct hvm_modlist_entry ramdisk_mod;
> 
> Is this used?
> 

Thanks, I forgot to remove it. I'm using it for initrd, but of course is
not used in this patch.

> > +uint8_t cmdline_buffer[CMDLINE_BUFSIZE];
> 
> Can this be static?
> 

Yes.

I'll include these changes in v3.

Thanks,
Stefano

--

Stefano Garzarella
Red Hat

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/4] hw/i386/pc: use PVH option rom
  2019-01-15 18:57   ` Michael S. Tsirkin
  2019-01-15 19:12     ` Eric Blake
@ 2019-01-18 17:33     ` Eduardo Habkost
  1 sibling, 0 replies; 16+ messages in thread
From: Eduardo Habkost @ 2019-01-18 17:33 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Stefano Garzarella, qemu-devel, Boris Ostrovsky, Gerd Hoffmann,
	George Kennedy, Marcel Apfelbaum, Paolo Bonzini, Eric Blake,
	Marc-Andre Lureau, Richard Henderson, Stefan Hajnoczi,
	Liam Merwick, Maran Wilson

On Tue, Jan 15, 2019 at 01:57:22PM -0500, Michael S. Tsirkin wrote:
> On Tue, Jan 15, 2019 at 11:00:58AM +0100, Stefano Garzarella wrote:
> > Use pvh.bin option rom when we are booting an uncompressed
> > kernel using the x86/HVM direct boot ABI.
> > 
> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > Based-on: <1545422632-24444-5-git-send-email-liam.merwick@oracle.com>
> 
> I don't think this is a great way to give attribution.
> Can you pls include the author name and the S.O.B from there as well?
> 
> 
> > ---
> >  hw/i386/pc.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 06bce6a101..7564ba51d2 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -1005,6 +1005,10 @@ static void load_linux(PCMachineState *pcms,
> >              fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA,
> >                               header, sizeof(header));
> >  
> > +            option_rom[nb_option_roms].bootindex = 0;
> > +            option_rom[nb_option_roms].name = "pvh.bin";
> > +            nb_option_roms++;
> > +
> >              return;
> >          }
> >          /* This looks like a multiboot kernel. If it is, let's stop
> > @@ -1456,6 +1460,7 @@ void xen_load_linux(PCMachineState *pcms)
> >      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, "pvh.bin") ||
> >                 !strcmp(option_rom[i].name, "multiboot.bin"));
> >          rom_add_option(option_rom[i].name, option_rom[i].bootindex);
> >      }
> 
> OK but this is guest visible so needs to be guarded by the
> new machine type.

Aren't option ROMs treated like other firmware?  i.e.: guest
visible, but copied during live migration and not considered part
of guest ABI.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-01-18 17:33 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-15 10:00 [Qemu-devel] [PATCH v2 0/4] pvh: add new PVH option rom Stefano Garzarella
2019-01-15 10:00 ` [Qemu-devel] [PATCH v2 1/4] linuxboot_dma: remove duplicate definitions of FW_CFG Stefano Garzarella
2019-01-15 10:00 ` [Qemu-devel] [PATCH v2 2/4] linuxboot_dma: move common functions in a new header Stefano Garzarella
2019-01-15 10:00 ` [Qemu-devel] [PATCH v2 3/4] optionrom: add new PVH option rom Stefano Garzarella
2019-01-16 10:16   ` Liam Merwick
2019-01-16 11:02     ` Stefano Garzarella
2019-01-15 10:00 ` [Qemu-devel] [PATCH v2 4/4] hw/i386/pc: use " Stefano Garzarella
2019-01-15 18:57   ` Michael S. Tsirkin
2019-01-15 19:12     ` Eric Blake
2019-01-15 20:05       ` Michael S. Tsirkin
2019-01-15 21:50         ` Paolo Bonzini
2019-01-16  8:19         ` Stefano Garzarella
2019-01-16  8:22       ` Stefano Garzarella
2019-01-18 17:33     ` Eduardo Habkost
2019-01-15 14:24 ` [Qemu-devel] [PATCH v2 0/4] pvh: add new " Stefan Hajnoczi
2019-01-16 10:18 ` Liam Merwick

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.