Re: [PATCH v3 4/7] hw/core: implement a guest-loader to support static hypervisor guests

[Alistair Francis <alistair23@xxxxxxxxx>]

On Wed, Mar 3, 2021 at 12:37 PM Alex Bennée <alex.bennee@xxxxxxxxxx> wrote:
>
> Hypervisors, especially type-1 ones, need the firmware/bootcode to put
> their initial guest somewhere in memory and pass the information to it
> via platform data. The guest-loader is modelled after the generic
> loader for exactly this sort of purpose:
>
>   $QEMU $ARGS  -kernel ~/xen.git/xen/xen \
>     -append "dom0_mem=1G,max:1G loglvl=all guest_loglvl=all" \
>     -device 
> guest-loader,addr=0x42000000,kernel=Image,bootargs="root=/dev/sda2 ro 
> console=hvc0 earlyprintk=xen" \
>     -device guest-loader,addr=0x47000000,initrd=rootfs.cpio
>
> Signed-off-by: Alex Bennée <alex.bennee@xxxxxxxxxx>
> Message-Id: <20201105175153.30489-5-alex.bennee@xxxxxxxxxx>
> Message-Id: <20210211171945.18313-5-alex.bennee@xxxxxxxxxx>

Reviewed-by: Alistair Francis <alistair.francis@xxxxxxx>

Alistair

> ---
>  hw/core/guest-loader.h |  34 ++++++++++
>  hw/core/guest-loader.c | 145 +++++++++++++++++++++++++++++++++++++++++
>  MAINTAINERS            |   5 ++
>  hw/core/meson.build    |   2 +
>  4 files changed, 186 insertions(+)
>  create mode 100644 hw/core/guest-loader.h
>  create mode 100644 hw/core/guest-loader.c
>
> diff --git a/hw/core/guest-loader.h b/hw/core/guest-loader.h
> new file mode 100644
> index 0000000000..07f4b4884b
> --- /dev/null
> +++ b/hw/core/guest-loader.h
> @@ -0,0 +1,34 @@
> +/*
> + * Guest Loader
> + *
> + * Copyright (C) 2020 Linaro
> + * Written by Alex Bennée <alex.bennee@xxxxxxxxxx>
> + * (based on the generic-loader by Li Guang <lig.fnst@xxxxxxxxxxxxxx>)
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef GUEST_LOADER_H
> +#define GUEST_LOADER_H
> +
> +#include "hw/qdev-core.h"
> +#include "qom/object.h"
> +
> +struct GuestLoaderState {
> +    /* <private> */
> +    DeviceState parent_obj;
> +
> +    /* <public> */
> +    uint64_t addr;
> +    char *kernel;
> +    char *args;
> +    char *initrd;
> +};
> +
> +#define TYPE_GUEST_LOADER "guest-loader"
> +OBJECT_DECLARE_SIMPLE_TYPE(GuestLoaderState, GUEST_LOADER)
> +
> +#endif
> diff --git a/hw/core/guest-loader.c b/hw/core/guest-loader.c
> new file mode 100644
> index 0000000000..bde44e27b4
> --- /dev/null
> +++ b/hw/core/guest-loader.c
> @@ -0,0 +1,145 @@
> +/*
> + * Guest Loader
> + *
> + * Copyright (C) 2020 Linaro
> + * Written by Alex Bennée <alex.bennee@xxxxxxxxxx>
> + * (based on the generic-loader by Li Guang <lig.fnst@xxxxxxxxxxxxxx>)
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +/*
> + * Much like the generic-loader this is treated as a special device
> + * inside QEMU. However unlike the generic-loader this device is used
> + * to load guest images for hypervisors. As part of that process the
> + * hypervisor needs to have platform information passed to it by the
> + * lower levels of the stack (e.g. firmware/bootloader). If you boot
> + * the hypervisor directly you use the guest-loader to load the Dom0
> + * or equivalent guest images in the right place in the same way a
> + * boot loader would.
> + *
> + * This is only relevant for full system emulation.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/core/cpu.h"
> +#include "hw/sysbus.h"
> +#include "sysemu/dma.h"
> +#include "hw/loader.h"
> +#include "hw/qdev-properties.h"
> +#include "qapi/error.h"
> +#include "qemu/module.h"
> +#include "guest-loader.h"
> +#include "sysemu/device_tree.h"
> +#include "hw/boards.h"
> +
> +/*
> + * Insert some FDT nodes for the loaded blob.
> + */
> +static void loader_insert_platform_data(GuestLoaderState *s, int size,
> +                                        Error **errp)
> +{
> +    MachineState *machine = MACHINE(qdev_get_machine());
> +    void *fdt = machine->fdt;
> +    g_autofree char *node = g_strdup_printf("/chosen/module@0x%08" PRIx64,
> +                                            s->addr);
> +    uint64_t reg_attr[2] = {cpu_to_be64(s->addr), cpu_to_be64(size)};
> +
> +    if (!fdt) {
> +        error_setg(errp, "Cannot modify FDT fields if the machine has none");
> +        return;
> +    }
> +
> +    qemu_fdt_add_subnode(fdt, node);
> +    qemu_fdt_setprop(fdt, node, "reg", &reg_attr, sizeof(reg_attr));
> +
> +    if (s->kernel) {
> +        const char *compat[2] = { "multiboot,module", "multiboot,kernel" };
> +        if (qemu_fdt_setprop_string_array(fdt, node, "compatible",
> +                                          (char **) &compat,
> +                                          ARRAY_SIZE(compat)) < 0) {
> +            error_setg(errp, "couldn't set %s/compatible", node);
> +            return;
> +        }
> +        if (s->args) {
> +            if (qemu_fdt_setprop_string(fdt, node, "bootargs", s->args) < 0) 
> {
> +                error_setg(errp, "couldn't set %s/bootargs", node);
> +            }
> +        }
> +    } else if (s->initrd) {
> +        const char *compat[2] = { "multiboot,module", "multiboot,ramdisk" };
> +        if (qemu_fdt_setprop_string_array(fdt, node, "compatible",
> +                                          (char **) &compat,
> +                                          ARRAY_SIZE(compat)) < 0) {
> +            error_setg(errp, "couldn't set %s/compatible", node);
> +            return;
> +        }
> +    }
> +}
> +
> +static void guest_loader_realize(DeviceState *dev, Error **errp)
> +{
> +    GuestLoaderState *s = GUEST_LOADER(dev);
> +    char *file = s->kernel ? s->kernel : s->initrd;
> +    int size = 0;
> +
> +    /* Perform some error checking on the user's options */
> +    if (s->kernel && s->initrd) {
> +        error_setg(errp, "Cannot specify a kernel and initrd in same 
> stanza");
> +        return;
> +    } else if (!s->kernel && !s->initrd)  {
> +        error_setg(errp, "Need to specify a kernel or initrd image");
> +        return;
> +    } else if (!s->addr) {
> +        error_setg(errp, "Need to specify the address of guest blob");
> +        return;
> +    } else if (s->args && !s->kernel) {
> +        error_setg(errp, "Boot args only relevant to kernel blobs");
> +    }
> +
> +    /* Default to the maximum size being the machine's ram size */
> +    size = load_image_targphys_as(file, s->addr, current_machine->ram_size,
> +                                  NULL);
> +    if (size < 0) {
> +        error_setg(errp, "Cannot load specified image %s", file);
> +        return;
> +    }
> +
> +    /* Now the image is loaded we need to update the platform data */
> +    loader_insert_platform_data(s, size, errp);
> +}
> +
> +static Property guest_loader_props[] = {
> +    DEFINE_PROP_UINT64("addr", GuestLoaderState, addr, 0),
> +    DEFINE_PROP_STRING("kernel", GuestLoaderState, kernel),
> +    DEFINE_PROP_STRING("bootargs", GuestLoaderState, args),
> +    DEFINE_PROP_STRING("initrd", GuestLoaderState, initrd),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void guest_loader_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = guest_loader_realize;
> +    device_class_set_props(dc, guest_loader_props);
> +    dc->desc = "Guest Loader";
> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> +}
> +
> +static TypeInfo guest_loader_info = {
> +    .name = TYPE_GUEST_LOADER,
> +    .parent = TYPE_DEVICE,
> +    .instance_size = sizeof(GuestLoaderState),
> +    .class_init = guest_loader_class_init,
> +};
> +
> +static void guest_loader_register_type(void)
> +{
> +    type_register_static(&guest_loader_info);
> +}
> +
> +type_init(guest_loader_register_type)
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9b2aa18e1f..a5b87d5e30 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2007,6 +2007,11 @@ F: hw/core/generic-loader.c
>  F: include/hw/core/generic-loader.h
>  F: docs/generic-loader.txt
>
> +Guest Loader
> +M: Alex Bennée <alex.bennee@xxxxxxxxxx>
> +S: Maintained
> +F: hw/core/guest-loader.c
> +
>  Intel Hexadecimal Object File Loader
>  M: Su Hang <suhang16@xxxxxxxxxxxxxxxx>
>  S: Maintained
> diff --git a/hw/core/meson.build b/hw/core/meson.build
> index 032576f571..9cd72edf51 100644
> --- a/hw/core/meson.build
> +++ b/hw/core/meson.build
> @@ -37,6 +37,8 @@ softmmu_ss.add(files(
>    'clock-vmstate.c',
>  ))
>
> +softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('guest-loader.c'))
> +
>  specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: files(
>    'machine-qmp-cmds.c',
>    'numa.c',
> --
> 2.20.1
>
>