All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v5 0/4] allow to load initrd below 4G for recent kernel
@ 2019-01-11  8:57 Li Zhijian
  2019-01-11  8:57 ` [Qemu-devel] [PATCH v5 1/4] unify len and addr type for memory/address APIs Li Zhijian
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Li Zhijian @ 2019-01-11  8:57 UTC (permalink / raw)
  To: qemu-devel, mst, peter.maydell
  Cc: philmd, zhijianx.li, Li Zhijian, Paolo Bonzini,
	Richard Henderson, Eduardo Habkost, Marcel Apfelbaum,
	Stefano Garzarella, Peter Crosthwaite

Long long ago, linux kernel has supported up to 4G initrd, but it's header
still hard code to allow loading initrd below 2G only.
 cutting from arch/boot/x86/header.S:
 # (Header version 0x0203 or later) the highest safe address for the contents
 # of an initrd. The current kernel allows up to 4 GB, but leave it at 2 GB to
 # avoid possible bootloader bugs.

In order to support more than 2G initrd, qemu must allow loading initrd
above 2G address. Luckly, recent kernel introduced a new field to linux header
named xloadflags:XLF_CAN_BE_LOADED_ABOVE_4G which tells bootloader an optional
and safe address to load initrd.

It's believed that if XLF_CAN_BE_LOADED_ABOVE_4G is set, initrd can
be loaded into any address.

Default roms(Seabios + optionrom(linuxboot_dma)) works as expected with this
patchset.

I stole some comments from yours, fell free to let me know if you don't like this.

changes:
V5: add a few reviewed-tag and update 4/4 changelog and comments
V4:
  - add Reviwed-by tag to 1/4 and 2/4
  - use scripts/update-linux-headers.sh to import bootparam.h
  - minor fix at commit log
V3:
 - rebase code basing on http://patchwork.ozlabs.org/cover/1005990 and
   https://patchew.org/QEMU/20181122133507.30950-1-peter.maydell@linaro.org
 - add new patch 3/4 to import header bootparam.h (Michael S. Tsirkin)

V2: add 2 patches(3/5, 4/5) to fix potential loading issue.


CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Richard Henderson <rth@twiddle.net>
CC: Eduardo Habkost <ehabkost@redhat.com>
CC: "Michael S. Tsirkin" <mst@redhat.com>
CC: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
CC: Stefano Garzarella <sgarzare@redhat.com>
CC: Peter Crosthwaite <crosthwaite.peter@gmail.com>
CC: Peter Maydell <peter.maydell@linaro.org>

Li Zhijian (4):
  unify len and addr type for memory/address APIs
  hw/core/loader.c: Read as long as possible in load_image_size()
  i386: import & use bootparam.h
  i386: allow to load initrd below 4G for recent linux

 exec.c                                       | 47 ++++++++++++++--------------
 hw/core/loader.c                             | 11 +++----
 hw/i386/pc.c                                 | 32 ++++++++++++++-----
 include/exec/cpu-all.h                       |  2 +-
 include/exec/cpu-common.h                    |  8 ++---
 include/exec/memory.h                        | 22 ++++++-------
 include/standard-headers/asm-x86/bootparam.h | 34 ++++++++++++++++++++
 scripts/update-linux-headers.sh              |  4 +++
 8 files changed, 106 insertions(+), 54 deletions(-)
 create mode 100644 include/standard-headers/asm-x86/bootparam.h

-- 
2.7.4

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

* [Qemu-devel] [PATCH v5 1/4] unify len and addr type for memory/address APIs
  2019-01-11  8:57 [Qemu-devel] [PATCH v5 0/4] allow to load initrd below 4G for recent kernel Li Zhijian
@ 2019-01-11  8:57 ` Li Zhijian
  2019-01-11  8:57 ` [Qemu-devel] [PATCH v5 2/4] hw/core/loader.c: Read as long as possible in load_image_size() Li Zhijian
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Li Zhijian @ 2019-01-11  8:57 UTC (permalink / raw)
  To: qemu-devel, mst, peter.maydell
  Cc: philmd, zhijianx.li, Li Zhijian, Paolo Bonzini,
	Peter Crosthwaite, Richard Henderson, Stefano Garzarella

Some address/memory APIs have different type between
'hwaddr/target_ulong addr' and 'int len'. It is very unsafe, especially
some APIs will be passed a non-int len by caller which might cause
overflow quietly.
Below is an potential overflow case:
    dma_memory_read(uint32_t len)
      -> dma_memory_rw(uint32_t len)
        -> dma_memory_rw_relaxed(uint32_t len)
          -> address_space_rw(int len) # len overflow

CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Peter Crosthwaite <crosthwaite.peter@gmail.com>
CC: Richard Henderson <rth@twiddle.net>
CC: Peter Maydell <peter.maydell@linaro.org>
CC: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

---
V5: Fix typo and Reviewed-tag (Stefano Garzarella)
V4: minor fix at commit message and add Reviewed-by tag
V3: use the same type between len and addr(Peter Maydell)
    rebase code basing on https://patchew.org/QEMU/20181122133507.30950-1-peter.maydell@linaro.org/
---
 exec.c                    | 47 +++++++++++++++++++++++------------------------
 include/exec/cpu-all.h    |  2 +-
 include/exec/cpu-common.h |  8 ++++----
 include/exec/memory.h     | 22 +++++++++++-----------
 4 files changed, 39 insertions(+), 40 deletions(-)

diff --git a/exec.c b/exec.c
index 6e875f0..f475974 100644
--- a/exec.c
+++ b/exec.c
@@ -2848,10 +2848,10 @@ static const MemoryRegionOps watch_mem_ops = {
 };
 
 static MemTxResult flatview_read(FlatView *fv, hwaddr addr,
-                                      MemTxAttrs attrs, uint8_t *buf, int len);
+                                      MemTxAttrs attrs, uint8_t *buf, hwaddr len);
 static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
-                                  const uint8_t *buf, int len);
-static bool flatview_access_valid(FlatView *fv, hwaddr addr, int len,
+                                  const uint8_t *buf, hwaddr len);
+static bool flatview_access_valid(FlatView *fv, hwaddr addr, hwaddr len,
                                   bool is_write, MemTxAttrs attrs);
 
 static MemTxResult subpage_read(void *opaque, hwaddr addr, uint64_t *data,
@@ -3099,10 +3099,10 @@ MemoryRegion *get_system_io(void)
 /* physical memory access (slow version, mainly for debug) */
 #if defined(CONFIG_USER_ONLY)
 int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
-                        uint8_t *buf, int len, int is_write)
+                        uint8_t *buf, target_ulong len, int is_write)
 {
-    int l, flags;
-    target_ulong page;
+    int flags;
+    target_ulong l, page;
     void * p;
 
     while (len > 0) {
@@ -3215,7 +3215,7 @@ static bool prepare_mmio_access(MemoryRegion *mr)
 static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr,
                                            MemTxAttrs attrs,
                                            const uint8_t *buf,
-                                           int len, hwaddr addr1,
+                                           hwaddr len, hwaddr addr1,
                                            hwaddr l, MemoryRegion *mr)
 {
     uint8_t *ptr;
@@ -3260,7 +3260,7 @@ static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr,
 
 /* Called from RCU critical section.  */
 static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
-                                  const uint8_t *buf, int len)
+                                  const uint8_t *buf, hwaddr len)
 {
     hwaddr l;
     hwaddr addr1;
@@ -3278,7 +3278,7 @@ static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
 /* Called within RCU critical section.  */
 MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr,
                                    MemTxAttrs attrs, uint8_t *buf,
-                                   int len, hwaddr addr1, hwaddr l,
+                                   hwaddr len, hwaddr addr1, hwaddr l,
                                    MemoryRegion *mr)
 {
     uint8_t *ptr;
@@ -3321,7 +3321,7 @@ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr,
 
 /* Called from RCU critical section.  */
 static MemTxResult flatview_read(FlatView *fv, hwaddr addr,
-                                 MemTxAttrs attrs, uint8_t *buf, int len)
+                                 MemTxAttrs attrs, uint8_t *buf, hwaddr len)
 {
     hwaddr l;
     hwaddr addr1;
@@ -3334,7 +3334,7 @@ static MemTxResult flatview_read(FlatView *fv, hwaddr addr,
 }
 
 MemTxResult address_space_read_full(AddressSpace *as, hwaddr addr,
-                                    MemTxAttrs attrs, uint8_t *buf, int len)
+                                    MemTxAttrs attrs, uint8_t *buf, hwaddr len)
 {
     MemTxResult result = MEMTX_OK;
     FlatView *fv;
@@ -3351,7 +3351,7 @@ MemTxResult address_space_read_full(AddressSpace *as, hwaddr addr,
 
 MemTxResult address_space_write(AddressSpace *as, hwaddr addr,
                                 MemTxAttrs attrs,
-                                const uint8_t *buf, int len)
+                                const uint8_t *buf, hwaddr len)
 {
     MemTxResult result = MEMTX_OK;
     FlatView *fv;
@@ -3367,7 +3367,7 @@ MemTxResult address_space_write(AddressSpace *as, hwaddr addr,
 }
 
 MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
-                             uint8_t *buf, int len, bool is_write)
+                             uint8_t *buf, hwaddr len, bool is_write)
 {
     if (is_write) {
         return address_space_write(as, addr, attrs, buf, len);
@@ -3377,7 +3377,7 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
 }
 
 void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf,
-                            int len, int is_write)
+                            hwaddr len, int is_write)
 {
     address_space_rw(&address_space_memory, addr, MEMTXATTRS_UNSPECIFIED,
                      buf, len, is_write);
@@ -3392,7 +3392,7 @@ static inline MemTxResult address_space_write_rom_internal(AddressSpace *as,
                                                            hwaddr addr,
                                                            MemTxAttrs attrs,
                                                            const uint8_t *buf,
-                                                           int len,
+                                                           hwaddr len,
                                                            enum write_rom_type type)
 {
     hwaddr l;
@@ -3432,13 +3432,13 @@ static inline MemTxResult address_space_write_rom_internal(AddressSpace *as,
 /* used for ROM loading : can write in RAM and ROM */
 MemTxResult address_space_write_rom(AddressSpace *as, hwaddr addr,
                                     MemTxAttrs attrs,
-                                    const uint8_t *buf, int len)
+                                    const uint8_t *buf, hwaddr len)
 {
     return address_space_write_rom_internal(as, addr, attrs,
                                             buf, len, WRITE_DATA);
 }
 
-void cpu_flush_icache_range(hwaddr start, int len)
+void cpu_flush_icache_range(hwaddr start, hwaddr len)
 {
     /*
      * This function should do the same thing as an icache flush that was
@@ -3541,7 +3541,7 @@ static void cpu_notify_map_clients(void)
     qemu_mutex_unlock(&map_client_list_lock);
 }
 
-static bool flatview_access_valid(FlatView *fv, hwaddr addr, int len,
+static bool flatview_access_valid(FlatView *fv, hwaddr addr, hwaddr len,
                                   bool is_write, MemTxAttrs attrs)
 {
     MemoryRegion *mr;
@@ -3564,7 +3564,7 @@ static bool flatview_access_valid(FlatView *fv, hwaddr addr, int len,
 }
 
 bool address_space_access_valid(AddressSpace *as, hwaddr addr,
-                                int len, bool is_write,
+                                hwaddr len, bool is_write,
                                 MemTxAttrs attrs)
 {
     FlatView *fv;
@@ -3817,7 +3817,7 @@ static inline MemoryRegion *address_space_translate_cached(
  */
 void
 address_space_read_cached_slow(MemoryRegionCache *cache, hwaddr addr,
-                                   void *buf, int len)
+                                   void *buf, hwaddr len)
 {
     hwaddr addr1, l;
     MemoryRegion *mr;
@@ -3835,7 +3835,7 @@ address_space_read_cached_slow(MemoryRegionCache *cache, hwaddr addr,
  */
 void
 address_space_write_cached_slow(MemoryRegionCache *cache, hwaddr addr,
-                                    const void *buf, int len)
+                                    const void *buf, hwaddr len)
 {
     hwaddr addr1, l;
     MemoryRegion *mr;
@@ -3858,11 +3858,10 @@ address_space_write_cached_slow(MemoryRegionCache *cache, hwaddr addr,
 
 /* virtual memory access for debug (includes writing to ROM) */
 int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
-                        uint8_t *buf, int len, int is_write)
+                        uint8_t *buf, target_ulong len, int is_write)
 {
-    int l;
     hwaddr phys_addr;
-    target_ulong page;
+    target_ulong l, page;
 
     cpu_synchronize_state(cpu);
     while (len > 0) {
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 117d2fb..b16c9ec 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -367,7 +367,7 @@ void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf);
 #endif /* !CONFIG_USER_ONLY */
 
 int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
-                        uint8_t *buf, int len, int is_write);
+                        uint8_t *buf, target_ulong len, int is_write);
 
 int cpu_exec(CPUState *cpu);
 
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 2ad2d6d..63ec1f9 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -83,14 +83,14 @@ size_t qemu_ram_pagesize(RAMBlock *block);
 size_t qemu_ram_pagesize_largest(void);
 
 void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf,
-                            int len, int is_write);
+                            hwaddr len, int is_write);
 static inline void cpu_physical_memory_read(hwaddr addr,
-                                            void *buf, int len)
+                                            void *buf, hwaddr len)
 {
     cpu_physical_memory_rw(addr, buf, len, 0);
 }
 static inline void cpu_physical_memory_write(hwaddr addr,
-                                             const void *buf, int len)
+                                             const void *buf, hwaddr len)
 {
     cpu_physical_memory_rw(addr, (void *)buf, len, 1);
 }
@@ -111,7 +111,7 @@ bool cpu_physical_memory_is_io(hwaddr phys_addr);
  */
 void qemu_flush_coalesced_mmio_buffer(void);
 
-void cpu_flush_icache_range(hwaddr start, int len);
+void cpu_flush_icache_range(hwaddr start, hwaddr len);
 
 extern struct MemoryRegion io_mem_rom;
 extern struct MemoryRegion io_mem_notdirty;
diff --git a/include/exec/memory.h b/include/exec/memory.h
index ffd23ed..6235f77 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1773,7 +1773,7 @@ void address_space_destroy(AddressSpace *as);
  */
 MemTxResult address_space_rw(AddressSpace *as, hwaddr addr,
                              MemTxAttrs attrs, uint8_t *buf,
-                             int len, bool is_write);
+                             hwaddr len, bool is_write);
 
 /**
  * address_space_write: write to address space.
@@ -1790,7 +1790,7 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr addr,
  */
 MemTxResult address_space_write(AddressSpace *as, hwaddr addr,
                                 MemTxAttrs attrs,
-                                const uint8_t *buf, int len);
+                                const uint8_t *buf, hwaddr len);
 
 /**
  * address_space_write_rom: write to address space, including ROM.
@@ -1816,7 +1816,7 @@ MemTxResult address_space_write(AddressSpace *as, hwaddr addr,
  */
 MemTxResult address_space_write_rom(AddressSpace *as, hwaddr addr,
                                     MemTxAttrs attrs,
-                                    const uint8_t *buf, int len);
+                                    const uint8_t *buf, hwaddr len);
 
 /* address_space_ld*: load from an address space
  * address_space_st*: store to an address space
@@ -2017,7 +2017,7 @@ static inline MemoryRegion *address_space_translate(AddressSpace *as,
  * @is_write: indicates the transfer direction
  * @attrs: memory attributes
  */
-bool address_space_access_valid(AddressSpace *as, hwaddr addr, int len,
+bool address_space_access_valid(AddressSpace *as, hwaddr addr, hwaddr len,
                                 bool is_write, MemTxAttrs attrs);
 
 /* address_space_map: map a physical memory region into a host virtual address
@@ -2054,19 +2054,19 @@ void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
 
 /* Internal functions, part of the implementation of address_space_read.  */
 MemTxResult address_space_read_full(AddressSpace *as, hwaddr addr,
-                                    MemTxAttrs attrs, uint8_t *buf, int len);
+                                    MemTxAttrs attrs, uint8_t *buf, hwaddr len);
 MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr,
                                    MemTxAttrs attrs, uint8_t *buf,
-                                   int len, hwaddr addr1, hwaddr l,
+                                   hwaddr len, hwaddr addr1, hwaddr l,
                                    MemoryRegion *mr);
 void *qemu_map_ram_ptr(RAMBlock *ram_block, ram_addr_t addr);
 
 /* Internal functions, part of the implementation of address_space_read_cached
  * and address_space_write_cached.  */
 void address_space_read_cached_slow(MemoryRegionCache *cache,
-                                    hwaddr addr, void *buf, int len);
+                                    hwaddr addr, void *buf, hwaddr len);
 void address_space_write_cached_slow(MemoryRegionCache *cache,
-                                     hwaddr addr, const void *buf, int len);
+                                     hwaddr addr, const void *buf, hwaddr len);
 
 static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
 {
@@ -2094,7 +2094,7 @@ static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
 static inline __attribute__((__always_inline__))
 MemTxResult address_space_read(AddressSpace *as, hwaddr addr,
                                MemTxAttrs attrs, uint8_t *buf,
-                               int len)
+                               hwaddr len)
 {
     MemTxResult result = MEMTX_OK;
     hwaddr l, addr1;
@@ -2133,7 +2133,7 @@ MemTxResult address_space_read(AddressSpace *as, hwaddr addr,
  */
 static inline void
 address_space_read_cached(MemoryRegionCache *cache, hwaddr addr,
-                          void *buf, int len)
+                          void *buf, hwaddr len)
 {
     assert(addr < cache->len && len <= cache->len - addr);
     if (likely(cache->ptr)) {
@@ -2153,7 +2153,7 @@ address_space_read_cached(MemoryRegionCache *cache, hwaddr addr,
  */
 static inline void
 address_space_write_cached(MemoryRegionCache *cache, hwaddr addr,
-                           void *buf, int len)
+                           void *buf, hwaddr len)
 {
     assert(addr < cache->len && len <= cache->len - addr);
     if (likely(cache->ptr)) {
-- 
2.7.4

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

* [Qemu-devel] [PATCH v5 2/4] hw/core/loader.c: Read as long as possible in load_image_size()
  2019-01-11  8:57 [Qemu-devel] [PATCH v5 0/4] allow to load initrd below 4G for recent kernel Li Zhijian
  2019-01-11  8:57 ` [Qemu-devel] [PATCH v5 1/4] unify len and addr type for memory/address APIs Li Zhijian
@ 2019-01-11  8:57 ` Li Zhijian
  2019-01-11  8:57 ` [Qemu-devel] [PATCH v5 3/4] i386: import & use bootparam.h Li Zhijian
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Li Zhijian @ 2019-01-11  8:57 UTC (permalink / raw)
  To: qemu-devel, mst, peter.maydell
  Cc: philmd, zhijianx.li, Li Zhijian, Richard Henderson, Stefano Garzarella

Don't expect read(2) can always read as many as it's told.

CC: Richard Henderson <richard.henderson@linaro.org>
CC: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

---
V5: update subject and add reviewed-by tag (Stefano Garzarella)
V4: add reviewed-by tag
---
 hw/core/loader.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index fa41842..9cbceab 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -77,21 +77,20 @@ int64_t get_image_size(const char *filename)
 ssize_t load_image_size(const char *filename, void *addr, size_t size)
 {
     int fd;
-    ssize_t actsize;
+    ssize_t actsize, l = 0;
 
     fd = open(filename, O_RDONLY | O_BINARY);
     if (fd < 0) {
         return -1;
     }
 
-    actsize = read(fd, addr, size);
-    if (actsize < 0) {
-        close(fd);
-        return -1;
+    while ((actsize = read(fd, addr + l, size - l)) > 0) {
+        l += actsize;
     }
+
     close(fd);
 
-    return actsize;
+    return actsize < 0 ? -1 : l;
 }
 
 /* read()-like version */
-- 
2.7.4

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

* [Qemu-devel] [PATCH v5 3/4] i386: import & use bootparam.h
  2019-01-11  8:57 [Qemu-devel] [PATCH v5 0/4] allow to load initrd below 4G for recent kernel Li Zhijian
  2019-01-11  8:57 ` [Qemu-devel] [PATCH v5 1/4] unify len and addr type for memory/address APIs Li Zhijian
  2019-01-11  8:57 ` [Qemu-devel] [PATCH v5 2/4] hw/core/loader.c: Read as long as possible in load_image_size() Li Zhijian
@ 2019-01-11  8:57 ` Li Zhijian
  2019-01-11  9:48   ` Stefano Garzarella
  2019-01-11  8:57 ` [Qemu-devel] [PATCH v5 4/4] i386: allow to load initrd below 4G for recent linux Li Zhijian
  2019-01-13 14:32 ` [Qemu-devel] [PATCH v5 0/4] allow to load initrd below 4G for recent kernel no-reply
  4 siblings, 1 reply; 12+ messages in thread
From: Li Zhijian @ 2019-01-11  8:57 UTC (permalink / raw)
  To: qemu-devel, mst, peter.maydell; +Cc: philmd, zhijianx.li, Li Zhijian

it's from v4.20-rc5.

CC: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
---
V5: add reviewed-by tag
V4: use scirpt to import bootparam.h (Michael S. Tsirkin)
V3: new patch
---
 hw/i386/pc.c                                 |  8 +------
 include/standard-headers/asm-x86/bootparam.h | 34 ++++++++++++++++++++++++++++
 scripts/update-linux-headers.sh              |  4 ++++
 3 files changed, 39 insertions(+), 7 deletions(-)
 create mode 100644 include/standard-headers/asm-x86/bootparam.h

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f248662..89c25b2 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -74,6 +74,7 @@
 #include "hw/nmi.h"
 #include "hw/i386/intel_iommu.h"
 #include "hw/net/ne2000-isa.h"
+#include "standard-headers/asm-x86/bootparam.h"
 
 /* debug PC/ISA interrupts */
 //#define DEBUG_IRQ
@@ -820,13 +821,6 @@ static long get_file_size(FILE *f)
     return size;
 }
 
-/* setup_data types */
-#define SETUP_NONE     0
-#define SETUP_E820_EXT 1
-#define SETUP_DTB      2
-#define SETUP_PCI      3
-#define SETUP_EFI      4
-
 struct setup_data {
     uint64_t next;
     uint32_t type;
diff --git a/include/standard-headers/asm-x86/bootparam.h b/include/standard-headers/asm-x86/bootparam.h
new file mode 100644
index 0000000..67d4f01
--- /dev/null
+++ b/include/standard-headers/asm-x86/bootparam.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _ASM_X86_BOOTPARAM_H
+#define _ASM_X86_BOOTPARAM_H
+
+/* setup_data types */
+#define SETUP_NONE			0
+#define SETUP_E820_EXT			1
+#define SETUP_DTB			2
+#define SETUP_PCI			3
+#define SETUP_EFI			4
+#define SETUP_APPLE_PROPERTIES		5
+#define SETUP_JAILHOUSE			6
+
+/* ram_size flags */
+#define RAMDISK_IMAGE_START_MASK	0x07FF
+#define RAMDISK_PROMPT_FLAG		0x8000
+#define RAMDISK_LOAD_FLAG		0x4000
+
+/* loadflags */
+#define LOADED_HIGH	(1<<0)
+#define KASLR_FLAG	(1<<1)
+#define QUIET_FLAG	(1<<5)
+#define KEEP_SEGMENTS	(1<<6)
+#define CAN_USE_HEAP	(1<<7)
+
+/* xloadflags */
+#define XLF_KERNEL_64			(1<<0)
+#define XLF_CAN_BE_LOADED_ABOVE_4G	(1<<1)
+#define XLF_EFI_HANDOVER_32		(1<<2)
+#define XLF_EFI_HANDOVER_64		(1<<3)
+#define XLF_EFI_KEXEC			(1<<4)
+
+
+#endif /* _ASM_X86_BOOTPARAM_H */
diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
index 0a964fe..77ec108 100755
--- a/scripts/update-linux-headers.sh
+++ b/scripts/update-linux-headers.sh
@@ -120,6 +120,10 @@ for arch in $ARCHLIST; do
         cp "$tmpdir/include/asm/unistd_x32.h" "$output/linux-headers/asm-x86/"
         cp "$tmpdir/include/asm/unistd_64.h" "$output/linux-headers/asm-x86/"
         cp_portable "$tmpdir/include/asm/kvm_para.h" "$output/include/standard-headers/asm-$arch"
+        # Remove everything except the macros from bootparam.h avoiding the
+        # unnecessary import of several video/ist/etc headers
+        sed -e '/__ASSEMBLY__/,/__ASSEMBLY__/d' $tmpdir/include/asm/bootparam.h > $tmpdir/bootparam.h
+        cp_portable $tmpdir/bootparam.h "$output/include/standard-headers/asm-$arch"
     fi
 done
 
-- 
2.7.4

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

* [Qemu-devel] [PATCH v5 4/4] i386: allow to load initrd below 4G for recent linux
  2019-01-11  8:57 [Qemu-devel] [PATCH v5 0/4] allow to load initrd below 4G for recent kernel Li Zhijian
                   ` (2 preceding siblings ...)
  2019-01-11  8:57 ` [Qemu-devel] [PATCH v5 3/4] i386: import & use bootparam.h Li Zhijian
@ 2019-01-11  8:57 ` Li Zhijian
  2019-01-14 17:53   ` Eduardo Habkost
  2019-01-13 14:32 ` [Qemu-devel] [PATCH v5 0/4] allow to load initrd below 4G for recent kernel no-reply
  4 siblings, 1 reply; 12+ messages in thread
From: Li Zhijian @ 2019-01-11  8:57 UTC (permalink / raw)
  To: qemu-devel, mst, peter.maydell
  Cc: philmd, zhijianx.li, Li Zhijian, Paolo Bonzini,
	Richard Henderson, Eduardo Habkost, Marcel Apfelbaum

Since linux commit: cf8fa920cb42 ("i386: handle an initrd in highmem (version 2)")
linux has supported initrd up to 4 GB, but the header field
ramdisk_max is still set to 2 GB to avoid "possible bootloader bugs".

When use '-kernel vmlinux -initrd initrd.cgz' to launch a VM,
the firmware(it could be linuxboot_dma.bin) helps to read initrd
contents into guest memory(below ramdisk_max) and jump to kernel.
that's similar with what bootloader does, like grub.

In addition, initrd_max is uint32_t simply because QEMU doesn't support
the 64-bit boot protocol (specifically the ext_ramdisk_image field).

Therefore here just limit initrd_max to UINT32_MAX simply as well to
allow initrd to be loaded below 4 GB.

CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Richard Henderson <rth@twiddle.net>
CC: Eduardo Habkost <ehabkost@redhat.com>
CC: "Michael S. Tsirkin" <mst@redhat.com>
CC: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>

---
V5: udpate comments and changelog
V3: correct grammar and check XLF_CAN_BE_LOADED_ABOVE_4G first (Michael S. Tsirkin)

Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
---
 hw/i386/pc.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 89c25b2..ea7a3c7 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -904,7 +904,29 @@ static void load_linux(PCMachineState *pcms,
 #endif
 
     /* highest address for loading the initrd */
-    if (protocol >= 0x203) {
+    if (protocol >= 0x20c &&
+        lduw_p(header+0x236) & XLF_CAN_BE_LOADED_ABOVE_4G) {
+        /*
+         * Linux has supported initrd up to 4 GB for a very long time (2007,
+         * long before XLF_CAN_BE_LOADED_ABOVE_4G which was added in 2013),
+         * though it only sets initrd_max to 2 GB to "work around bootloader
+         * bugs". Luckily, QEMU firmware(which does something like bootloader)
+         * has supported this.
+         *
+         * It's believed that if XLF_CAN_BE_LOADED_ABOVE_4G is set, initrd can
+         * be loaded into any address.
+         *
+         * In addition, initrd_max is uint32_t simply because QEMU doesn't
+         * support the 64-bit boot protocol (specifically the ext_ramdisk_image
+         * field).
+         *
+         * Therefore here just limit initrd_max to UINT32_MAX simply as well.
+         *
+         * FIXME: it's possible that linux protocol within [0x208, 0x20c]
+         * supports up to 4G initrd as well.
+         */
+        initrd_max = UINT32_MAX;
+    } else if (protocol >= 0x203) {
         initrd_max = ldl_p(header+0x22c);
     } else {
         initrd_max = 0x37ffffff;
-- 
2.7.4

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

* Re: [Qemu-devel] [PATCH v5 3/4] i386: import & use bootparam.h
  2019-01-11  8:57 ` [Qemu-devel] [PATCH v5 3/4] i386: import & use bootparam.h Li Zhijian
@ 2019-01-11  9:48   ` Stefano Garzarella
  2019-01-11 10:03     ` Li Zhijian
  0 siblings, 1 reply; 12+ messages in thread
From: Stefano Garzarella @ 2019-01-11  9:48 UTC (permalink / raw)
  To: Li Zhijian
  Cc: qemu-devel, Michael Tsirkin, Peter Maydell, zhijianx.li,
	Philippe Mathieu Daude

Hi Li,

On Fri, Jan 11, 2019 at 10:06 AM Li Zhijian <lizhijian@cn.fujitsu.com> wrote:
>
> it's from v4.20-rc5.
>
> CC: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> V5: add reviewed-by tag
> V4: use scirpt to import bootparam.h (Michael S. Tsirkin)
> V3: new patch
> ---
>  hw/i386/pc.c                                 |  8 +------
>  include/standard-headers/asm-x86/bootparam.h | 34 ++++++++++++++++++++++++++++
>  scripts/update-linux-headers.sh              |  4 ++++
>  3 files changed, 39 insertions(+), 7 deletions(-)
>  create mode 100644 include/standard-headers/asm-x86/bootparam.h
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index f248662..89c25b2 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -74,6 +74,7 @@
>  #include "hw/nmi.h"
>  #include "hw/i386/intel_iommu.h"
>  #include "hw/net/ne2000-isa.h"
> +#include "standard-headers/asm-x86/bootparam.h"
>
>  /* debug PC/ISA interrupts */
>  //#define DEBUG_IRQ
> @@ -820,13 +821,6 @@ static long get_file_size(FILE *f)
>      return size;
>  }
>
> -/* setup_data types */
> -#define SETUP_NONE     0
> -#define SETUP_E820_EXT 1
> -#define SETUP_DTB      2
> -#define SETUP_PCI      3
> -#define SETUP_EFI      4
> -
>  struct setup_data {
>      uint64_t next;
>      uint32_t type;
> diff --git a/include/standard-headers/asm-x86/bootparam.h b/include/standard-headers/asm-x86/bootparam.h
> new file mode 100644
> index 0000000..67d4f01
> --- /dev/null
> +++ b/include/standard-headers/asm-x86/bootparam.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +#ifndef _ASM_X86_BOOTPARAM_H
> +#define _ASM_X86_BOOTPARAM_H
> +
> +/* setup_data types */
> +#define SETUP_NONE                     0
> +#define SETUP_E820_EXT                 1
> +#define SETUP_DTB                      2
> +#define SETUP_PCI                      3
> +#define SETUP_EFI                      4
> +#define SETUP_APPLE_PROPERTIES         5
> +#define SETUP_JAILHOUSE                        6
> +
> +/* ram_size flags */
> +#define RAMDISK_IMAGE_START_MASK       0x07FF
> +#define RAMDISK_PROMPT_FLAG            0x8000
> +#define RAMDISK_LOAD_FLAG              0x4000
> +
> +/* loadflags */
> +#define LOADED_HIGH    (1<<0)
> +#define KASLR_FLAG     (1<<1)
> +#define QUIET_FLAG     (1<<5)
> +#define KEEP_SEGMENTS  (1<<6)
> +#define CAN_USE_HEAP   (1<<7)
> +
> +/* xloadflags */
> +#define XLF_KERNEL_64                  (1<<0)
> +#define XLF_CAN_BE_LOADED_ABOVE_4G     (1<<1)
> +#define XLF_EFI_HANDOVER_32            (1<<2)
> +#define XLF_EFI_HANDOVER_64            (1<<3)
> +#define XLF_EFI_KEXEC                  (1<<4)
> +
> +
> +#endif /* _ASM_X86_BOOTPARAM_H */
> diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
> index 0a964fe..77ec108 100755
> --- a/scripts/update-linux-headers.sh
> +++ b/scripts/update-linux-headers.sh
> @@ -120,6 +120,10 @@ for arch in $ARCHLIST; do
>          cp "$tmpdir/include/asm/unistd_x32.h" "$output/linux-headers/asm-x86/"
>          cp "$tmpdir/include/asm/unistd_64.h" "$output/linux-headers/asm-x86/"
>          cp_portable "$tmpdir/include/asm/kvm_para.h" "$output/include/standard-headers/asm-$arch"
> +        # Remove everything except the macros from bootparam.h avoiding the
> +        # unnecessary import of several video/ist/etc headers
> +        sed -e '/__ASSEMBLY__/,/__ASSEMBLY__/d' $tmpdir/include/asm/bootparam.h > $tmpdir/bootparam.h
> +        cp_portable $tmpdir/bootparam.h "$output/include/standard-headers/asm-$arch"

Maybe you miss my comment. Anyway, IMHO is better to use the double
quotes for all paths.

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

>      fi
>  done

>
> --
> 2.7.4
>
>


--
Stefano Garzarella
Red Hat

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

* Re: [Qemu-devel] [PATCH v5 3/4] i386: import & use bootparam.h
  2019-01-11  9:48   ` Stefano Garzarella
@ 2019-01-11 10:03     ` Li Zhijian
  0 siblings, 0 replies; 12+ messages in thread
From: Li Zhijian @ 2019-01-11 10:03 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: qemu-devel, Michael Tsirkin, Peter Maydell, zhijianx.li,
	Philippe Mathieu Daude

Hi Stefano


On 1/11/19 17:48, Stefano Garzarella wrote:
> Hi Li,
>
> On Fri, Jan 11, 2019 at 10:06 AM Li Zhijian <lizhijian@cn.fujitsu.com> wrote:
>> +        # unnecessary import of several video/ist/etc headers
>> +        sed -e '/__ASSEMBLY__/,/__ASSEMBLY__/d' $tmpdir/include/asm/bootparam.h > $tmpdir/bootparam.h
>> +        cp_portable $tmpdir/bootparam.h "$output/include/standard-headers/asm-$arch"
> Maybe you miss my comment. Anyway, IMHO is better to use the double
> quotes for all paths.
>
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

So sorry about it, i folded it on a wrong branch at the beginning.

will update it soon.


Thanks

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

* Re: [Qemu-devel] [PATCH v5 0/4] allow to load initrd below 4G for recent kernel
  2019-01-11  8:57 [Qemu-devel] [PATCH v5 0/4] allow to load initrd below 4G for recent kernel Li Zhijian
                   ` (3 preceding siblings ...)
  2019-01-11  8:57 ` [Qemu-devel] [PATCH v5 4/4] i386: allow to load initrd below 4G for recent linux Li Zhijian
@ 2019-01-13 14:32 ` no-reply
  4 siblings, 0 replies; 12+ messages in thread
From: no-reply @ 2019-01-13 14:32 UTC (permalink / raw)
  To: lizhijian
  Cc: fam, qemu-devel, mst, peter.maydell, ehabkost, crosthwaite.peter,
	zhijianx.li, pbonzini, sgarzare, philmd, rth

Patchew URL: https://patchew.org/QEMU/1547197071-14504-1-git-send-email-lizhijian@cn.fujitsu.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 1547197071-14504-1-git-send-email-lizhijian@cn.fujitsu.com
Subject: [Qemu-devel] [PATCH v5 0/4] allow to load initrd below 4G for recent kernel

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback --color=always base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 * [new tag]         patchew/1547197071-14504-1-git-send-email-lizhijian@cn.fujitsu.com -> patchew/1547197071-14504-1-git-send-email-lizhijian@cn.fujitsu.com
warning: remote HEAD refers to nonexistent ref, unable to checkout.

Switched to a new branch 'test'
a527fea i386: allow to load initrd below 4G for recent linux
7236341 i386: import & use bootparam.h
ae2981c hw/core/loader.c: Read as long as possible in load_image_size()
49e603a unify len and addr type for memory/address APIs

=== OUTPUT BEGIN ===
1/4 Checking commit 49e603a945f2 (unify len and addr type for memory/address APIs)
WARNING: line over 80 characters
#38: FILE: exec.c:2852:
+                                      MemTxAttrs attrs, uint8_t *buf, hwaddr len);

total: 0 errors, 1 warnings, 270 lines checked

Patch 1/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/4 Checking commit ae2981cbf961 (hw/core/loader.c: Read as long as possible in load_image_size())
3/4 Checking commit 7236341ea82b (i386: import & use bootparam.h)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#42: 
new file mode 100644

ERROR: line over 90 characters
#91: FILE: scripts/update-linux-headers.sh:125:
+        sed -e '/__ASSEMBLY__/,/__ASSEMBLY__/d' $tmpdir/include/asm/bootparam.h > $tmpdir/bootparam.h

WARNING: line over 80 characters
#92: FILE: scripts/update-linux-headers.sh:126:
+        cp_portable $tmpdir/bootparam.h "$output/include/standard-headers/asm-$arch"

total: 1 errors, 2 warnings, 64 lines checked

Patch 3/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

4/4 Checking commit a527fea2b274 (i386: allow to load initrd below 4G for recent linux)
ERROR: spaces required around that '+' (ctx:VxV)
#41: FILE: hw/i386/pc.c:1124:
+        lduw_p(header+0x236) & XLF_CAN_BE_LOADED_ABOVE_4G) {
                      ^

total: 1 errors, 0 warnings, 30 lines checked

Patch 4/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/1547197071-14504-1-git-send-email-lizhijian@cn.fujitsu.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

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

* Re: [Qemu-devel] [PATCH v5 4/4] i386: allow to load initrd below 4G for recent linux
  2019-01-11  8:57 ` [Qemu-devel] [PATCH v5 4/4] i386: allow to load initrd below 4G for recent linux Li Zhijian
@ 2019-01-14 17:53   ` Eduardo Habkost
  2019-01-15  1:35     ` Li Zhijian
  0 siblings, 1 reply; 12+ messages in thread
From: Eduardo Habkost @ 2019-01-14 17:53 UTC (permalink / raw)
  To: Li Zhijian
  Cc: qemu-devel, mst, peter.maydell, philmd, zhijianx.li,
	Paolo Bonzini, Richard Henderson, Marcel Apfelbaum

On Fri, Jan 11, 2019 at 04:57:51PM +0800, Li Zhijian wrote:
> Since linux commit: cf8fa920cb42 ("i386: handle an initrd in highmem (version 2)")
> linux has supported initrd up to 4 GB, but the header field
> ramdisk_max is still set to 2 GB to avoid "possible bootloader bugs".
> 
> When use '-kernel vmlinux -initrd initrd.cgz' to launch a VM,
> the firmware(it could be linuxboot_dma.bin) helps to read initrd
> contents into guest memory(below ramdisk_max) and jump to kernel.
> that's similar with what bootloader does, like grub.
> 
> In addition, initrd_max is uint32_t simply because QEMU doesn't support
> the 64-bit boot protocol (specifically the ext_ramdisk_image field).
> 
> Therefore here just limit initrd_max to UINT32_MAX simply as well to
> allow initrd to be loaded below 4 GB.
> 
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Richard Henderson <rth@twiddle.net>
> CC: Eduardo Habkost <ehabkost@redhat.com>
> CC: "Michael S. Tsirkin" <mst@redhat.com>
> CC: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> 
> ---
> V5: udpate comments and changelog
> V3: correct grammar and check XLF_CAN_BE_LOADED_ABOVE_4G first (Michael S. Tsirkin)
> 
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> ---
>  hw/i386/pc.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 89c25b2..ea7a3c7 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -904,7 +904,29 @@ static void load_linux(PCMachineState *pcms,
>  #endif
>  
>      /* highest address for loading the initrd */
> -    if (protocol >= 0x203) {
> +    if (protocol >= 0x20c &&
> +        lduw_p(header+0x236) & XLF_CAN_BE_LOADED_ABOVE_4G) {
> +        /*
> +         * Linux has supported initrd up to 4 GB for a very long time (2007,
> +         * long before XLF_CAN_BE_LOADED_ABOVE_4G which was added in 2013),
> +         * though it only sets initrd_max to 2 GB to "work around bootloader
> +         * bugs". Luckily, QEMU firmware(which does something like bootloader)
> +         * has supported this.
> +         *
> +         * It's believed that if XLF_CAN_BE_LOADED_ABOVE_4G is set, initrd can
> +         * be loaded into any address.
> +         *
> +         * In addition, initrd_max is uint32_t simply because QEMU doesn't
> +         * support the 64-bit boot protocol (specifically the ext_ramdisk_image
> +         * field).
> +         *
> +         * Therefore here just limit initrd_max to UINT32_MAX simply as well.
> +         *
> +         * FIXME: it's possible that linux protocol within [0x208, 0x20c]
> +         * supports up to 4G initrd as well.

I don't understand what exactly this FIXME comment is
documenting.  What exactly needs to be fixed?


> +         */
> +        initrd_max = UINT32_MAX;
> +    } else if (protocol >= 0x203) {
>          initrd_max = ldl_p(header+0x22c);
>      } else {
>          initrd_max = 0x37ffffff;
> -- 
> 2.7.4
> 

-- 
Eduardo

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

* Re: [Qemu-devel] [PATCH v5 4/4] i386: allow to load initrd below 4G for recent linux
  2019-01-14 17:53   ` Eduardo Habkost
@ 2019-01-15  1:35     ` Li Zhijian
  2019-01-15  1:46       ` Michael S. Tsirkin
  0 siblings, 1 reply; 12+ messages in thread
From: Li Zhijian @ 2019-01-15  1:35 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: qemu-devel, mst, peter.maydell, philmd, zhijianx.li,
	Paolo Bonzini, Richard Henderson, Marcel Apfelbaum

Hi Eduardo


On 1/15/19 01:53, Eduardo Habkost wrote:
>> +    if (protocol >= 0x20c &&
>> +        lduw_p(header+0x236) & XLF_CAN_BE_LOADED_ABOVE_4G) {
>> +        /*
>> +         * Linux has supported initrd up to 4 GB for a very long time (2007,
>> +         * long before XLF_CAN_BE_LOADED_ABOVE_4G which was added in 2013),
>> +         * though it only sets initrd_max to 2 GB to "work around bootloader
>> +         * bugs". Luckily, QEMU firmware(which does something like bootloader)
>> +         * has supported this.
>> +         *
>> +         * It's believed that if XLF_CAN_BE_LOADED_ABOVE_4G is set, initrd can
>> +         * be loaded into any address.
>> +         *
>> +         * In addition, initrd_max is uint32_t simply because QEMU doesn't
>> +         * support the 64-bit boot protocol (specifically the ext_ramdisk_image
>> +         * field).
>> +         *
>> +         * Therefore here just limit initrd_max to UINT32_MAX simply as well.
>> +         *
>> +         * FIXME: it's possible that linux protocol within [0x208, 0x20c]
>> +         * supports up to 4G initrd as well.
> I don't understand what exactly this FIXME comment is
> documenting.  What exactly needs to be fixed?
>
XLF_CAN_BE_LOADED_ABOVE_4G is one of the indicators, actually as comments said,
linux has supported up to 4 GB initrd since linux-2.26(protocol version 0x208).


I just want to comment that linux with protocol within [0x208, 0x20c] supports up to 4 GB initrd as well.

Is documenting with FIXME appropriate?


Thanks

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

* Re: [Qemu-devel] [PATCH v5 4/4] i386: allow to load initrd below 4G for recent linux
  2019-01-15  1:35     ` Li Zhijian
@ 2019-01-15  1:46       ` Michael S. Tsirkin
  2019-01-16 10:19         ` Li Zhijian
  0 siblings, 1 reply; 12+ messages in thread
From: Michael S. Tsirkin @ 2019-01-15  1:46 UTC (permalink / raw)
  To: Li Zhijian
  Cc: Eduardo Habkost, qemu-devel, peter.maydell, philmd, zhijianx.li,
	Paolo Bonzini, Richard Henderson, Marcel Apfelbaum

On Tue, Jan 15, 2019 at 09:35:09AM +0800, Li Zhijian wrote:
> Hi Eduardo
> 
> 
> On 1/15/19 01:53, Eduardo Habkost wrote:
> 
>         +    if (protocol >= 0x20c &&
>         +        lduw_p(header+0x236) & XLF_CAN_BE_LOADED_ABOVE_4G) {
>         +        /*
>         +         * Linux has supported initrd up to 4 GB for a very long time (2007,
>         +         * long before XLF_CAN_BE_LOADED_ABOVE_4G which was added in 2013),
>         +         * though it only sets initrd_max to 2 GB to "work around bootloader
>         +         * bugs". Luckily, QEMU firmware(which does something like bootloader)
>         +         * has supported this.
>         +         *
>         +         * It's believed that if XLF_CAN_BE_LOADED_ABOVE_4G is set, initrd can
>         +         * be loaded into any address.
>         +         *
>         +         * In addition, initrd_max is uint32_t simply because QEMU doesn't
>         +         * support the 64-bit boot protocol (specifically the ext_ramdisk_image
>         +         * field).
>         +         *
>         +         * Therefore here just limit initrd_max to UINT32_MAX simply as well.
>         +         *
>         +         * FIXME: it's possible that linux protocol within [0x208, 0x20c]
>         +         * supports up to 4G initrd as well.
> 
>     I don't understand what exactly this FIXME comment is
>     documenting.  What exactly needs to be fixed?
> 
> 
> XLF_CAN_BE_LOADED_ABOVE_4G is one of the indicators, actually as comments said,
> linux has supported up to 4 GB initrd since linux-2.26(protocol version 0x208).
> 
> 
> I just want to comment that linux with protocol within [0x208, 0x20c] supports up to 4 GB initrd as well.
> 
> Is documenting with FIXME appropriate?
> 
> 
> Thanks
> 
> 


Fixme should say what is missing in the qemu implementation.
E.g.

/*
 * Bar 2010 and up can actually be supported using foo.
 * FIXME: make use of foo to support bar.
 */


-- 
MST

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

* Re: [Qemu-devel] [PATCH v5 4/4] i386: allow to load initrd below 4G for recent linux
  2019-01-15  1:46       ` Michael S. Tsirkin
@ 2019-01-16 10:19         ` Li Zhijian
  0 siblings, 0 replies; 12+ messages in thread
From: Li Zhijian @ 2019-01-16 10:19 UTC (permalink / raw)
  To: Michael S. Tsirkin, Eduardo Habkost
  Cc: qemu-devel, peter.maydell, philmd, zhijianx.li, Paolo Bonzini,
	Richard Henderson, Marcel Apfelbaum

Hi Michael, Eduardo

On 1/15/19 09:46, Michael S. Tsirkin wrote:
> On Tue, Jan 15, 2019 at 09:35:09AM +0800, Li Zhijian wrote:
>> Hi Eduardo
>>
>>
>> On 1/15/19 01:53, Eduardo Habkost wrote:
>>
>>          +    if (protocol >= 0x20c &&
>>          +        lduw_p(header+0x236) & XLF_CAN_BE_LOADED_ABOVE_4G) {
>>          +        /*
>>          +         * Linux has supported initrd up to 4 GB for a very long time (2007,
>>          +         * long before XLF_CAN_BE_LOADED_ABOVE_4G which was added in 2013),
>>          +         * though it only sets initrd_max to 2 GB to "work around bootloader
>>          +         * bugs". Luckily, QEMU firmware(which does something like bootloader)
>>          +         * has supported this.
>>          +         *
>>          +         * It's believed that if XLF_CAN_BE_LOADED_ABOVE_4G is set, initrd can
>>          +         * be loaded into any address.
>>          +         *
>>          +         * In addition, initrd_max is uint32_t simply because QEMU doesn't
>>          +         * support the 64-bit boot protocol (specifically the ext_ramdisk_image
>>          +         * field).
>>          +         *
>>          +         * Therefore here just limit initrd_max to UINT32_MAX simply as well.
>>          +         *
>>          +         * FIXME: it's possible that linux protocol within [0x208, 0x20c]
>>          +         * supports up to 4G initrd as well.
>>
>>      I don't understand what exactly this FIXME comment is
>>      documenting.  What exactly needs to be fixed?
>>
>>
>> XLF_CAN_BE_LOADED_ABOVE_4G is one of the indicators, actually as comments said,
>> linux has supported up to 4 GB initrd since linux-2.26(protocol version 0x208).
>>
>>
>> I just want to comment that linux with protocol within [0x208, 0x20c] supports up to 4 GB initrd as well.
>>
>> Is documenting with FIXME appropriate?
>>
>>
>> Thanks
>>
>>
>
> Fixme should say what is missing in the qemu implementation.



thanks for your explanation @Michael
I'd like to update "FIXME" to "NOTE" and move it into git-commit-log if no objection
and it's okay to delete it simply if it confuses others :)

BTW: any other comments for the others

Thanks
Zhijian




> E.g.
>
> /*
>   * Bar 2010 and up can actually be supported using foo.
>   * FIXME: make use of foo to support bar.
>   */
>
>

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

end of thread, other threads:[~2019-01-16 10:31 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-11  8:57 [Qemu-devel] [PATCH v5 0/4] allow to load initrd below 4G for recent kernel Li Zhijian
2019-01-11  8:57 ` [Qemu-devel] [PATCH v5 1/4] unify len and addr type for memory/address APIs Li Zhijian
2019-01-11  8:57 ` [Qemu-devel] [PATCH v5 2/4] hw/core/loader.c: Read as long as possible in load_image_size() Li Zhijian
2019-01-11  8:57 ` [Qemu-devel] [PATCH v5 3/4] i386: import & use bootparam.h Li Zhijian
2019-01-11  9:48   ` Stefano Garzarella
2019-01-11 10:03     ` Li Zhijian
2019-01-11  8:57 ` [Qemu-devel] [PATCH v5 4/4] i386: allow to load initrd below 4G for recent linux Li Zhijian
2019-01-14 17:53   ` Eduardo Habkost
2019-01-15  1:35     ` Li Zhijian
2019-01-15  1:46       ` Michael S. Tsirkin
2019-01-16 10:19         ` Li Zhijian
2019-01-13 14:32 ` [Qemu-devel] [PATCH v5 0/4] allow to load initrd below 4G for recent kernel no-reply

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.