Re: [Xen-devel] [RFC v2 0/4] QEMU changes to do PVH boot

[Stefano Garzarella <sgarzare@xxxxxxxxxx>]

On Wed, Jan 09, 2019 at 01:18:12PM -0800, Maran Wilson wrote:
> On 1/9/2019 11:53 AM, Boris Ostrovsky wrote:
> > On 1/9/19 6:53 AM, Stefano Garzarella wrote:
> > > Hi Liam,
> > > 
> > > On Tue, Jan 8, 2019 at 3:47 PM Liam Merwick <liam.merwick@xxxxxxxxxx> 
> > > wrote:
> > > > QEMU sets the hvm_modlist_entry in load_linux() after the call to
> > > > load_elfboot() and then qboot loads it in boot_pvh_from_fw_cfg()
> > > > 
> > > > But the current PVH patches don't handle initrd (they have
> > > > start_info.nr_modules == 1).
> > > Looking in the linux kernel (arch/x86/platform/pvh/enlighten.c) I saw:
> > >      /* The first module is always ramdisk. */
> > >      if (pvh_start_info.nr_modules) {
> > >          struct hvm_modlist_entry *modaddr =
> > >              __va(pvh_start_info.modlist_paddr);
> > >          pvh_bootparams.hdr.ramdisk_image = modaddr->paddr;
> > >          pvh_bootparams.hdr.ramdisk_size = modaddr->size;
> > >      }
> > > 
> > > So, putting start_info.nr_modules = 1, means that the first
> > > hvm_modlist_entry should have the ramdisk paddr and size. Is it
> > > correct?
> 
> That's my understanding.
> 
> I think what's missing, is that we just need Qemu or qboot/seabios to
> properly populate the pvh_start_info.modlist_paddr with the address (as
> usable by the guest) of the hvm_modlist_entry which correctly defines the
> details of the initrd that has already been loaded into memory that is
> accessible by the guest.
> 
> -Maran
> 

I tried and it works, I modified QEMU to load the initrd and to expose it
through fw_cfg, then qboot loads it and set correctly the hvm_modlist_entry.

You can find the patch of QEMU at the end of this email and the qboot
patch here: 
https://github.com/stefano-garzarella/qboot/commit/41e1fd765c8419e270fd79d9b3af5d53576e88a8

Do you think can be a good approach? If you want, you can add this patch
to your series.

Thanks,
Stefano


From d5c0d51768f5a8fb214be6c2bb0cb7e86e9917b7 Mon Sep 17 00:00:00 2001
From: Stefano Garzarella <sgarzare@xxxxxxxxxx>
Date: Thu, 10 Jan 2019 15:16:44 +0100
Subject: [PATCH] pvh: load initrd and expose it through fw_cfg

When initrd is specified, load and expose it to the guest firmware
through fw_cfg. The firmware will fill the hvm_start_info for the
kernel.

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

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 06bce6a101..f6721f51be 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -986,25 +986,45 @@ static void load_linux(PCMachineState *pcms,
          */
         if (load_elfboot(kernel_filename, kernel_size,
                          header, pvh_start_addr, fw_cfg)) {
-            struct hvm_modlist_entry ramdisk_mod = { 0 };
-
             fclose(f);
 
             fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE,
                 strlen(kernel_cmdline) + 1);
             fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, kernel_cmdline);
 
-            assert(machine->device_memory != NULL);
-            ramdisk_mod.paddr = machine->device_memory->base;
-            ramdisk_mod.size =
-                memory_region_size(&machine->device_memory->mr);
-
-            fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA, &ramdisk_mod,
-                             sizeof(ramdisk_mod));
             fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_SIZE, sizeof(header));
             fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA,
                              header, sizeof(header));
 
+            /* load initrd */
+            if (initrd_filename) {
+                gsize initrd_size;
+                gchar *initrd_data;
+                GError *gerr = NULL;
+
+                if (!g_file_get_contents(initrd_filename, &initrd_data,
+                            &initrd_size, &gerr)) {
+                    fprintf(stderr, "qemu: error reading initrd %s: %s\n",
+                            initrd_filename, gerr->message);
+                    exit(1);
+                }
+
+                initrd_max = pcms->below_4g_mem_size - pcmc->acpi_data_size - 
1;
+                if (initrd_size >= initrd_max) {
+                    fprintf(stderr, "qemu: initrd is too large, cannot 
support."
+                            "(max: %"PRIu32", need %"PRId64")\n",
+                            initrd_max, (uint64_t)initrd_size);
+                    exit(1);
+                }
+
+                initrd_addr = (initrd_max - initrd_size) & ~4095;
+
+                fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_addr);
+                fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
+                fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, initrd_data,
+                                 initrd_size);
+            }
+
             return;
         }
         /* This looks like a multiboot kernel. If it is, let's stop
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel