From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57450) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aIq5q-0007pL-BJ for qemu-devel@nongnu.org; Mon, 11 Jan 2016 22:58:03 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aIq5m-0001Sb-6k for qemu-devel@nongnu.org; Mon, 11 Jan 2016 22:58:02 -0500 From: Peter Crosthwaite Date: Mon, 11 Jan 2016 19:57:53 -0800 Message-ID: <20160112035753.GD3308@pcrost-box> References: <1451608294-16432-1-git-send-email-Andrew.Baumann@microsoft.com> <1451608294-16432-8-git-send-email-Andrew.Baumann@microsoft.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1451608294-16432-8-git-send-email-Andrew.Baumann@microsoft.com> Subject: Re: [Qemu-devel] [PATCH v3 7/7] raspi: add raspberry pi 2 machine List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andrew Baumann Cc: Peter Maydell , =?iso-8859-1?Q?Gr=E9gory?= ESTRADE , Stefan Weil , qemu-devel@nongnu.org, qemu-arm@nongnu.org, Paolo Bonzini 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 > --- > > 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 >