qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-arm] [PATCH v3 7/7] raspi: add raspberry pi 2 machine


From: Peter Crosthwaite
Subject: Re: [Qemu-arm] [PATCH v3 7/7] raspi: add raspberry pi 2 machine
Date: Mon, 11 Jan 2016 19:57:53 -0800
User-agent: Mutt/1.5.21 (2010-09-15)

On Thu, Dec 31, 2015 at 04:31:34PM -0800, Andrew Baumann wrote:
> bcm2835/Pi1 requires more peripherals, and will be added in a later
> patch series.
> 
> Signed-off-by: Andrew Baumann <address@hidden>
> ---
> 
> Notes:
>     v3:
>      * fix board setup to remain Pi1 compatible
>      * pass ram property
> 
>  hw/arm/Makefile.objs |   2 +-
>  hw/arm/raspi.c       | 180 
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 181 insertions(+), 1 deletion(-)
>  create mode 100644 hw/arm/raspi.c
> 
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index f55f8d2..a711e4d 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -11,7 +11,7 @@ obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o 
> pxa2xx_pic.o
>  obj-$(CONFIG_DIGIC) += digic.o
>  obj-y += omap1.o omap2.o strongarm.o
>  obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
> -obj-$(CONFIG_RASPI) += bcm2835_peripherals.o bcm2836.o
> +obj-$(CONFIG_RASPI) += bcm2835_peripherals.o bcm2836.o raspi.o
>  obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
>  obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o xlnx-ep108.o
>  obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> new file mode 100644
> index 0000000..b73f544
> --- /dev/null
> +++ b/hw/arm/raspi.c
> @@ -0,0 +1,180 @@
> +/*
> + * Raspberry Pi emulation (c) 2012 Gregory Estrade
> + * Upstreaming code cleanup [including bcm2835_*] (c) 2013 Jan Petrous
> + *
> + * Rasperry Pi 2 emulation Copyright (c) 2015, Microsoft
> + * Written by Andrew Baumann
> + *
> + * This code is licensed under the GNU GPLv2 and later.
> + */
> +
> +/* Based on versatilepb.c, copyright terms below. */
> +
> +/*

No need to break and restart comments.

> + * ARM Versatile Platform/Application Baseboard System emulation.
> + *

Looks unrelated, I think this has reached total-rewrite status.

> + * Copyright (c) 2005-2007 CodeSourcery.
> + * Written by Paul Brook
> + *
> + * This code is licensed under the GPL.
> + */
> +
> +#include "hw/arm/bcm2836.h"
> +#include "qemu/error-report.h"
> +#include "hw/boards.h"
> +#include "hw/loader.h"
> +#include "hw/arm/arm.h"
> +#include "sysemu/sysemu.h"
> +
> +#define SMPBOOT_ADDR    0x300 /* this should leave enough space for ATAGS */
> +#define MVBAR_ADDR      0x400 /* secure vectors */
> +#define BOARDSETUP_ADDR (MVBAR_ADDR + 0x20) /* board setup code */
> +#define FIRMWARE_ADDR   0x8000 /* Pi loads kernel.img here by default */
> +
> +/* Table of Linux board IDs for different Pi versions */
> +static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43};
> +
> +typedef struct RaspiMachineState {
> +    union {
> +        Object obj;
> +        BCM2836State pi2;
> +    } soc;
> +    MemoryRegion ram;
> +} RaspiMachineState;
> +
> +static void write_smpboot(ARMCPU *cpu, const struct arm_boot_info *info)
> +{
> +    static const uint32_t smpboot[] = {
> +        0xE1A0E00F, /*    mov     lr, pc */
> +        0xE3A0FE42, /*    mov     pc, #0x420           ;call BOARDSETUP_ADDR 
> */

Check highbank blobs to see how machine code addresses can be done as
relocatable. We should try and make these blobs relocatable from the top
level address definitions where possible.

> +        0xEE100FB0, /*    mrc     p15, 0, r0, c0, c0, 5;get core ID */
> +        0xE7E10050, /*    ubfx    r0, r0, #0, #2       ;extract LSB */
> +        0xE59F5014, /*    ldr     r5, =0x400000CC      ;load mbox base */
> +        0xE320F001, /* 1: yield */
> +        0xE7953200, /*    ldr     r3, [r5, r0, lsl #4] ;read mbox for our 
> core*/
> +        0xE3530000, /*    cmp     r3, #0               ;spin while zero */
> +        0x0AFFFFFB, /*    beq     1b */
> +        0xE7853200, /*    str     r3, [r5, r0, lsl #4] ;clear mbox */
> +        0xE12FFF13, /*    bx      r3                   ;jump to target */
> +        0x400000CC, /* (constant: mailbox 3 read/clear base) */
> +    };
> +
> +    assert(SMPBOOT_ADDR + sizeof(smpboot) <= MVBAR_ADDR);
> +    rom_add_blob_fixed("raspi_smpboot", smpboot, sizeof(smpboot),
> +                       info->smp_loader_start);
> +}
> +
> +static void write_board_setup(ARMCPU *cpu, const struct arm_boot_info *info)

This is almost identical to Highbank, I'm guessing you are stubbing monitor
firmware where you get away with nopping all the SMCs? Maybe we should split
Highbanks version off to common code, and parameterise the few differences.

write_board_setup_dummy_monitior(ARMCPU *cpu ..., uint32_t scr_flags);

or something like it. Makes sense to be in arm/boot.c .

> +{
> +    static const uint32_t board_setup[] = {
> +        /* MVBAR_ADDR: secure monitor vectors */
> +        0xEAFFFFFE, /* (spin) */
> +        0xEAFFFFFE, /* (spin) */
> +        0xE1B0F00E, /* movs pc, lr ;SMC exception return */
> +        0xEAFFFFFE, /* (spin) */
> +        0xEAFFFFFE, /* (spin) */
> +        0xEAFFFFFE, /* (spin) */
> +        0xEAFFFFFE, /* (spin) */
> +        0xEAFFFFFE, /* (spin) */
> +        /* BOARDSETUP_ADDR */
> +        0xE3A00B01, /* mov     r0, #0x400             ;MVBAR_ADDR */
> +        0xEE0C0F30, /* mcr     p15, 0, r0, c12, c0, 1 ;set MVBAR */
> +        0xE3A00031, /* mov     r0, #0x31              ;enable AW, FW, NS */
> +        0xEE010F11, /* mcr     p15, 0, r0, c1, c1, 0  ;write SCR */

If combining with HB, could you do this as read-modify-write?

> +        0xE1A0100E, /* mov     r1, lr                 ;save LR across SMC */
> +        0xE1600070, /* smc     #0                     ;monitor call */
> +        0xE1A0F001, /* mov     pc, r1                 ;return */

I'm looking at the Highbank version which doesn't save lr across the SMC and
wondering why it doesn't. Is this banked by CPU mode and do you get from
already-in-monitor-mode? Or simply, that Highbank code may have a bug?

> +    };
> +
> +    rom_add_blob_fixed("raspi_boardsetup", board_setup, sizeof(board_setup),
> +                       MVBAR_ADDR);
> +}
> +
> +static void reset_secondary(ARMCPU *cpu, const struct arm_boot_info *info)
> +{
> +    CPUState *cs = CPU(cpu);
> +    cpu_set_pc(cs, info->smp_loader_start);
> +}
> +
> +static void setup_boot(MachineState *machine, int version, size_t ram_size)
> +{
> +    static struct arm_boot_info binfo;
> +    int r;
> +
> +    binfo.board_id = raspi_boardid[version];
> +    binfo.ram_size = ram_size;
> +    binfo.nb_cpus = smp_cpus;
> +    binfo.board_setup_addr = BOARDSETUP_ADDR;
> +    binfo.write_board_setup = write_board_setup;
> +    binfo.secure_board_setup = true;
> +    binfo.secure_boot = true;
> +
> +    /* Pi2 requires SMP setup */
> +    if (version == 2) {
> +        binfo.smp_loader_start = SMPBOOT_ADDR;
> +        binfo.write_secondary_boot = write_smpboot;
> +        binfo.secondary_cpu_reset_hook = reset_secondary;
> +    }
> +
> +    /* If the user specified a "firmware" image (e.g. UEFI), we bypass
> +       the normal Linux boot process */

Multi-line comment style should be

/* text
 * text
 */

> +    if (machine->firmware) {
> +        /* load the firmware image (typically kernel.img) */
> +        r = load_image_targphys(machine->firmware, FIRMWARE_ADDR,
> +                                ram_size - FIRMWARE_ADDR);
> +        if (r < 0) {
> +            error_report("Failed to load firmware from %s", 
> machine->firmware);
> +            exit(1);
> +        }
> +
> +        /* set variables so arm_load_kernel does the right thing */
> +        binfo.entry = FIRMWARE_ADDR;
> +        binfo.firmware_loaded = true;
> +    } else {
> +        /* Just let arm_load_kernel do everything for us... */
> +        binfo.kernel_filename = machine->kernel_filename;
> +        binfo.kernel_cmdline = machine->kernel_cmdline;
> +        binfo.initrd_filename = machine->initrd_filename;
> +    }
> +
> +    arm_load_kernel(ARM_CPU(first_cpu), &binfo);
> +}
> +
> +static void raspi2_init(MachineState *machine)
> +{
> +    RaspiMachineState *s = g_new0(RaspiMachineState, 1);

Use of the MachineState name-stem suggest that you are QOM inheriting from
MachineState but you are not. So the struct just needs a rename to something
without "MachineState"

> +
> +    /* Initialise the SOC */
> +    object_initialize(&s->soc.pi2, sizeof(s->soc.pi2), TYPE_BCM2836);
> +    object_property_add_child(OBJECT(machine), "soc", &s->soc.obj,
> +                              &error_abort);
> +
> +    /* Allocate and map RAM */
> +    memory_region_allocate_system_memory(&s->ram, OBJECT(machine), "ram",
> +                                         machine->ram_size);
> +    memory_region_add_subregion_overlap(get_system_memory(), 0, &s->ram, 0);

I thought the SoC handled this now? Why do you need to add to system_memory?

> +
> +    /* Setup the SOC */
> +    object_property_add_const_link(&s->soc.obj, "ram", OBJECT(&s->ram),
> +                                   &error_abort);

Just cast using OBJECT() rather than having the union.

> +    object_property_set_bool(&s->soc.obj, true, "realized", &error_abort);
> +
> +    /* Boot! */

Not really. You just setup the boot for core code to do it later.

> +    setup_boot(machine, 2, machine->ram_size);
> +}
> +
> +static void raspi2_machine_init(MachineClass *mc)
> +{
> +    mc->desc = "Raspberry Pi 2";
> +    mc->init = raspi2_init;
> +    mc->block_default_type = IF_SD;

> +    mc->no_parallel = 1;
> +    mc->no_floppy = 1;
> +    mc->no_cdrom = 1;

Curious, what do these do from a user-visible point of view? Maybe we should
add them to more ARM boards as they certainly make sense.

Regards,
Peter

> +    mc->max_cpus = BCM2836_NCPUS;
> +    /* XXX: Temporary restriction in RAM size from the full 1GB. Since
> +     * we do not yet support the framebuffer / GPU, we need to limit
> +     * RAM usable by the OS to sit below the peripherals. */
> +    mc->default_ram_size = 0x3F000000; /* BCM2836_PERI_BASE */
> +};
> +DEFINE_MACHINE("raspi2", raspi2_machine_init)
> -- 
> 2.5.3
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]