Re: [Qemu-devel] [PATCH 17/17] Implement support for i.MX7 Sabre board

[Peter Maydell]

On 18 September 2017 at 20:51, Andrey Smirnov <address@hidden> wrote:
> Cc: Peter Maydell <address@hidden>
> Cc: address@hidden
> Cc: address@hidden
> Cc: address@hidden
> Signed-off-by: Andrey Smirnov <address@hidden>
> ---
>  hw/arm/Makefile.objs   |   2 +-
>  hw/arm/mcimx7d-sabre.c | 100 
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 101 insertions(+), 1 deletion(-)
>  create mode 100644 hw/arm/mcimx7d-sabre.c
>
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index 33f6051ae3..fc4a963de8 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -19,5 +19,5 @@ obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o kzm.o
>  obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o sabrelite.o
>  obj-$(CONFIG_ASPEED_SOC) += aspeed_soc.o aspeed.o
>  obj-$(CONFIG_MPS2) += mps2.o
> -obj-$(CONFIG_FSL_IMX7) += fsl-imx7.o
> +obj-$(CONFIG_FSL_IMX7) += fsl-imx7.o mcimx7d-sabre.o
>
> diff --git a/hw/arm/mcimx7d-sabre.c b/hw/arm/mcimx7d-sabre.c
> new file mode 100644
> index 0000000000..34e3933db8
> --- /dev/null
> +++ b/hw/arm/mcimx7d-sabre.c
> @@ -0,0 +1,100 @@
> +/*
> + * Copyright (c) 2017, Impinj, Inc.
> + *
> + * MCIMX7D_SABRE Board System emulation.
> + *
> + * Author: Andrey Smirnov <address@hidden>
> + *
> + * This code is licensed under the GPL, version 2 or later.
> + * See the file `COPYING' in the top level directory.
> + *
> + * It (partially) emulates a mcimx7d_sabre board, with a Freescale
> + * i.MX7 SoC
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qemu-common.h"
> +#include "hw/arm/fsl-imx7.h"
> +#include "hw/boards.h"
> +#include "sysemu/sysemu.h"
> +#include "sysemu/device_tree.h"
> +#include "qemu/error-report.h"
> +#include "sysemu/qtest.h"
> +#include "net/net.h"
> +
> +typedef struct {
> +    FslIMX7State soc;
> +    MemoryRegion ram;
> +} MCIMX7Sabre;
> +
> +static void mcimx7d_add_psci_node(const struct arm_boot_info *boot_info, 
> void *fdt)
> +{
> +    const char comp[] = "arm,psci-0.2\0arm,psci";
> +
> +    qemu_fdt_add_subnode(fdt, "/psci");
> +    qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp));
> +    qemu_fdt_setprop_string(fdt, "/psci", "method", "smc");
> +}

You shouldn't need this -- we should just be able to work with whatever
the real hardware's device tree is. ("virt" is a special case because
we create our own device tree there.)

> +
> +static void mcimx7d_sabre_init(MachineState *machine)
> +{
> +    static struct arm_boot_info boot_info;
> +    MCIMX7Sabre *s = g_new0(MCIMX7Sabre, 1);
> +    Object *soc;
> +    int i;
> +
> +    if (machine->ram_size > FSL_IMX7_MMDC_SIZE) {
> +        error_report("RAM size " RAM_ADDR_FMT " above max supported (%08x)",
> +                     machine->ram_size, FSL_IMX7_MMDC_SIZE);
> +        exit(1);
> +    }
> +
> +    boot_info = (struct arm_boot_info) {
> +        .loader_start = FSL_IMX7_MMDC_ADDR,
> +        .board_id = -1,
> +        .ram_size = machine->ram_size,
> +        .kernel_filename = machine->kernel_filename,
> +        .kernel_cmdline = machine->kernel_cmdline,
> +        .initrd_filename = machine->initrd_filename,
> +        .nb_cpus = smp_cpus,
> +        .modify_dtb = mcimx7d_add_psci_node,
> +    };
> +
> +    object_initialize(&s->soc, sizeof(s->soc), TYPE_FSL_IMX7);
> +    soc = OBJECT(&s->soc);
> +    object_property_add_child(OBJECT(machine), "soc", soc, &error_fatal);
> +    object_property_set_bool(soc, true, "realized", &error_fatal);
> +
> +    memory_region_allocate_system_memory(&s->ram, NULL, "mcimx7d-sabre.ram",
> +                                         machine->ram_size);
> +    memory_region_add_subregion(get_system_memory(),
> +                                FSL_IMX7_MMDC_ADDR, &s->ram);
> +
> +    for (i = 0; i < FSL_IMX7_NUM_USDHCS; i++) {
> +        BusState *bus;
> +        DeviceState *carddev;
> +        DriveInfo *di;
> +        BlockBackend *blk;
> +
> +        di = drive_get_next(IF_SD);
> +        blk = di ? blk_by_legacy_dinfo(di) : NULL;
> +        bus = qdev_get_child_bus(DEVICE(&s->soc.usdhc[i]), "sd-bus");
> +        carddev = qdev_create(bus, TYPE_SD_CARD);
> +        qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
> +        object_property_set_bool(OBJECT(carddev), true, "realized", 
> &error_fatal);
> +    }
> +
> +    if (!qtest_enabled()) {
> +        arm_load_kernel(&s->soc.cpu[0], &boot_info);
> +    }
> +}
> +
> +static void mcimx7d_sabre_machine_init(MachineClass *mc)
> +{
> +    mc->desc = "Freescale i.MX7 DUAL SABRE (Cortex A7)";
> +    mc->init = mcimx7d_sabre_init;
> +    mc->max_cpus = FSL_IMX7_NUM_CPUS;
> +    mc->ignore_memory_transaction_failures = true;

Please don't set this flag on new board models -- it is only
for legacy existing board models (where we don't know what
running guest code might be broken by turning bad accesses
into guest aborts. For a new board model, you should leave
the flag unset and instead stub out any devices that you
need to using create_unimplemented_device() so that your
guest code boots.

thanks
-- PMM