From c8d8ef00a121ee7326c87a76cc9f49716ed68917 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 7 May 2019 12:55:01 +0100 Subject: pc: Rearrange pc_system_firmware_init()'s legacy -drive loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The loop does two things: map legacy -drive to properties, and collect all the backends for use after the loop. The next patch will factor out the former for reuse in hw/arm/virt.c. To make that easier, rearrange the loop so it does the first thing first, and the second thing second. Signed-off-by: Markus Armbruster Reviewed-by: Laszlo Ersek Reviewed-by: Philippe Mathieu-Daudé Message-id: 20190416091348.26075-2-armbru@redhat.com Signed-off-by: Peter Maydell --- hw/i386/pc_sysfw.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) (limited to 'hw') diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c index c628540774..75925f5d3f 100644 --- a/hw/i386/pc_sysfw.c +++ b/hw/i386/pc_sysfw.c @@ -280,21 +280,19 @@ void pc_system_firmware_init(PCMachineState *pcms, /* Map legacy -drive if=pflash to machine properties */ for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) { - pflash_blk[i] = pflash_cfi01_get_blk(pcms->flash[i]); pflash_drv = drive_get(IF_PFLASH, 0, i); - if (!pflash_drv) { - continue; - } - loc_push_none(&loc); - qemu_opts_loc_restore(pflash_drv->opts); - if (pflash_blk[i]) { - error_report("clashes with -machine"); - exit(1); + if (pflash_drv) { + loc_push_none(&loc); + qemu_opts_loc_restore(pflash_drv->opts); + if (pflash_cfi01_get_blk(pcms->flash[i])) { + error_report("clashes with -machine"); + exit(1); + } + qdev_prop_set_drive(DEVICE(pcms->flash[i]), "drive", + blk_by_legacy_dinfo(pflash_drv), &error_fatal); + loc_pop(&loc); } - pflash_blk[i] = blk_by_legacy_dinfo(pflash_drv); - qdev_prop_set_drive(DEVICE(pcms->flash[i]), - "drive", pflash_blk[i], &error_fatal); - loc_pop(&loc); + pflash_blk[i] = pflash_cfi01_get_blk(pcms->flash[i]); } /* Reject gaps */ -- cgit v1.2.3 From 2d731dbd5e7173961cd76dc12c939e672cbca2bd Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 7 May 2019 12:55:02 +0100 Subject: pflash_cfi01: New pflash_cfi01_legacy_drive() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Factored out of pc_system_firmware_init() so the next commit can reuse it in hw/arm/virt.c. Signed-off-by: Markus Armbruster Reviewed-by: Laszlo Ersek Reviewed-by: Philippe Mathieu-Daudé Message-id: 20190416091348.26075-3-armbru@redhat.com Signed-off-by: Peter Maydell --- hw/block/pflash_cfi01.c | 28 ++++++++++++++++++++++++++++ hw/i386/pc_sysfw.c | 16 ++-------------- 2 files changed, 30 insertions(+), 14 deletions(-) (limited to 'hw') diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c index 16dfae14b8..333b736277 100644 --- a/hw/block/pflash_cfi01.c +++ b/hw/block/pflash_cfi01.c @@ -44,9 +44,12 @@ #include "qapi/error.h" #include "qemu/timer.h" #include "qemu/bitops.h" +#include "qemu/error-report.h" #include "qemu/host-utils.h" #include "qemu/log.h" +#include "qemu/option.h" #include "hw/sysbus.h" +#include "sysemu/blockdev.h" #include "sysemu/sysemu.h" #include "trace.h" @@ -968,6 +971,31 @@ MemoryRegion *pflash_cfi01_get_memory(PFlashCFI01 *fl) return &fl->mem; } +/* + * Handle -drive if=pflash for machines that use properties. + * If @dinfo is null, do nothing. + * Else if @fl's property "drive" is already set, fatal error. + * Else set it to the BlockBackend with @dinfo. + */ +void pflash_cfi01_legacy_drive(PFlashCFI01 *fl, DriveInfo *dinfo) +{ + Location loc; + + if (!dinfo) { + return; + } + + loc_push_none(&loc); + qemu_opts_loc_restore(dinfo->opts); + if (fl->blk) { + error_report("clashes with -machine"); + exit(1); + } + qdev_prop_set_drive(DEVICE(fl), "drive", + blk_by_legacy_dinfo(dinfo), &error_fatal); + loc_pop(&loc); +} + static void postload_update_cb(void *opaque, int running, RunState state) { PFlashCFI01 *pfl = opaque; diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c index 75925f5d3f..751fcafa12 100644 --- a/hw/i386/pc_sysfw.c +++ b/hw/i386/pc_sysfw.c @@ -269,9 +269,7 @@ void pc_system_firmware_init(PCMachineState *pcms, { PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms); int i; - DriveInfo *pflash_drv; BlockBackend *pflash_blk[ARRAY_SIZE(pcms->flash)]; - Location loc; if (!pcmc->pci_enabled) { old_pc_system_rom_init(rom_memory, true); @@ -280,18 +278,8 @@ void pc_system_firmware_init(PCMachineState *pcms, /* Map legacy -drive if=pflash to machine properties */ for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) { - pflash_drv = drive_get(IF_PFLASH, 0, i); - if (pflash_drv) { - loc_push_none(&loc); - qemu_opts_loc_restore(pflash_drv->opts); - if (pflash_cfi01_get_blk(pcms->flash[i])) { - error_report("clashes with -machine"); - exit(1); - } - qdev_prop_set_drive(DEVICE(pcms->flash[i]), "drive", - blk_by_legacy_dinfo(pflash_drv), &error_fatal); - loc_pop(&loc); - } + pflash_cfi01_legacy_drive(pcms->flash[i], + drive_get(IF_PFLASH, 0, i)); pflash_blk[i] = pflash_cfi01_get_blk(pcms->flash[i]); } -- cgit v1.2.3 From e0561e60f170b220c5d73d185fa8eaa66fa8e6ef Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 7 May 2019 12:55:02 +0100 Subject: hw/arm/virt: Support firmware configuration with -blockdev MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ARM virt machines put firmware in flash memory. To configure it, you use -drive if=pflash,unit=0,... and optionally -drive if=pflash,unit=1,... Why two -drive? This permits setting up one part of the flash memory read-only, and the other part read/write. It also makes upgrading firmware on the host easier. Below the hood, we get two separate flash devices, because we were too lazy to improve our flash device models to support sector protection. The problem at hand is to do the same with -blockdev somehow, as one more step towards deprecating -drive. We recently solved this problem for x86 PC machines, in commit ebc29e1beab. See the commit message for design rationale. This commit solves it for ARM virt basically the same way: new machine properties pflash0, pflash1 forward to the onboard flash devices' properties. Requires creating the onboard devices in the .instance_init() method virt_instance_init(). The existing code to pick up drives defined with -drive if=pflash is replaced by code to desugar into the machine properties. There are a few behavioral differences, though: * The flash devices are always present (x86: only present if configured) * Flash base addresses and sizes are fixed (x86: sizes depend on images, mapped back to back below a fixed address) * -bios configures contents of first pflash (x86: -bios configures ROM contents) * -bios is rejected when first pflash is also configured with -machine pflash0=... (x86: bios is silently ignored then) * -machine pflash1=... does not require -machine pflash0=... (x86: it does). The actual code is a bit simpler than for x86 mostly due to the first two differences. Before the patch, all the action is in create_flash(), called from the machine's .init() method machvirt_init(): main() machine_run_board_init() machvirt_init() create_flash() create_one_flash() for flash[0] create configure includes obeying -drive if=pflash,unit=0 realize map fall back to -bios create_one_flash() for flash[1] create configure includes obeying -drive if=pflash,unit=1 realize map update FDT To make the machine properties work, we need to move device creation to its .instance_init() method virt_instance_init(). Another complication is machvirt_init()'s computation of @firmware_loaded: it predicts what create_flash() will do. Instead of predicting what create_flash()'s replacement virt_firmware_init() will do, I decided to have virt_firmware_init() return what it did. Requires calling it a bit earlier. Resulting call tree: main() current_machine = object_new() ... virt_instance_init() virt_flash_create() virt_flash_create1() for flash[0] create configure: set defaults become child of machine [NEW] add machine prop pflash0 as alias for drive [NEW] virt_flash_create1() for flash[1] create configure: set defaults become child of machine [NEW] add machine prop pflash1 as alias for drive [NEW] for all machine props from the command line: machine_set_property() ... property_set_alias() for machine props pflash0, pflash1 ... set_drive() for cfi.pflash01 prop drive this is how -machine pflash0=... etc set machine_run_board_init(current_machine); virt_firmware_init() pflash_cfi01_legacy_drive() legacy -drive if=pflash,unit=0 and =1 [NEW] virt_flash_map() virt_flash_map1() for flash[0] configure: num-blocks realize map virt_flash_map1() for flash[1] configure: num-blocks realize map fall back to -bios virt_flash_fdt() update FDT You have László to thank for making me explain this in detail. Signed-off-by: Markus Armbruster Acked-by: Laszlo Ersek Message-id: 20190416091348.26075-4-armbru@redhat.com Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- hw/arm/virt.c | 196 +++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 127 insertions(+), 69 deletions(-) (limited to 'hw') diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 16ba67f7a7..5331ab71e2 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -30,6 +30,7 @@ #include "qemu/osdep.h" #include "qemu/units.h" +#include "qemu/option.h" #include "qapi/error.h" #include "hw/sysbus.h" #include "hw/arm/arm.h" @@ -871,25 +872,19 @@ static void create_virtio_devices(const VirtMachineState *vms, qemu_irq *pic) } } -static void create_one_flash(const char *name, hwaddr flashbase, - hwaddr flashsize, const char *file, - MemoryRegion *sysmem) +#define VIRT_FLASH_SECTOR_SIZE (256 * KiB) + +static PFlashCFI01 *virt_flash_create1(VirtMachineState *vms, + const char *name, + const char *alias_prop_name) { - /* Create and map a single flash device. We use the same - * parameters as the flash devices on the Versatile Express board. + /* + * Create a single flash device. We use the same parameters as + * the flash devices on the Versatile Express board. */ - DriveInfo *dinfo = drive_get_next(IF_PFLASH); DeviceState *dev = qdev_create(NULL, TYPE_PFLASH_CFI01); - SysBusDevice *sbd = SYS_BUS_DEVICE(dev); - const uint64_t sectorlength = 256 * 1024; - - if (dinfo) { - qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo), - &error_abort); - } - qdev_prop_set_uint32(dev, "num-blocks", flashsize / sectorlength); - qdev_prop_set_uint64(dev, "sector-length", sectorlength); + qdev_prop_set_uint64(dev, "sector-length", VIRT_FLASH_SECTOR_SIZE); qdev_prop_set_uint8(dev, "width", 4); qdev_prop_set_uint8(dev, "device-width", 2); qdev_prop_set_bit(dev, "big-endian", false); @@ -898,41 +893,41 @@ static void create_one_flash(const char *name, hwaddr flashbase, qdev_prop_set_uint16(dev, "id2", 0x00); qdev_prop_set_uint16(dev, "id3", 0x00); qdev_prop_set_string(dev, "name", name); - qdev_init_nofail(dev); + object_property_add_child(OBJECT(vms), name, OBJECT(dev), + &error_abort); + object_property_add_alias(OBJECT(vms), alias_prop_name, + OBJECT(dev), "drive", &error_abort); + return PFLASH_CFI01(dev); +} - memory_region_add_subregion(sysmem, flashbase, - sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0)); +static void virt_flash_create(VirtMachineState *vms) +{ + vms->flash[0] = virt_flash_create1(vms, "virt.flash0", "pflash0"); + vms->flash[1] = virt_flash_create1(vms, "virt.flash1", "pflash1"); +} - if (file) { - char *fn; - int image_size; +static void virt_flash_map1(PFlashCFI01 *flash, + hwaddr base, hwaddr size, + MemoryRegion *sysmem) +{ + DeviceState *dev = DEVICE(flash); - if (drive_get(IF_PFLASH, 0, 0)) { - error_report("The contents of the first flash device may be " - "specified with -bios or with -drive if=pflash... " - "but you cannot use both options at once"); - exit(1); - } - fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, file); - if (!fn) { - error_report("Could not find ROM image '%s'", file); - exit(1); - } - image_size = load_image_mr(fn, sysbus_mmio_get_region(sbd, 0)); - g_free(fn); - if (image_size < 0) { - error_report("Could not load ROM image '%s'", file); - exit(1); - } - } + assert(size % VIRT_FLASH_SECTOR_SIZE == 0); + assert(size / VIRT_FLASH_SECTOR_SIZE <= UINT32_MAX); + qdev_prop_set_uint32(dev, "num-blocks", size / VIRT_FLASH_SECTOR_SIZE); + qdev_init_nofail(dev); + + memory_region_add_subregion(sysmem, base, + sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), + 0)); } -static void create_flash(const VirtMachineState *vms, - MemoryRegion *sysmem, - MemoryRegion *secure_sysmem) +static void virt_flash_map(VirtMachineState *vms, + MemoryRegion *sysmem, + MemoryRegion *secure_sysmem) { - /* Create two flash devices to fill the VIRT_FLASH space in the memmap. - * Any file passed via -bios goes in the first of these. + /* + * Map two flash devices to fill the VIRT_FLASH space in the memmap. * sysmem is the system memory space. secure_sysmem is the secure view * of the system, and the first flash device should be made visible only * there. The second flash device is visible to both secure and nonsecure. @@ -941,12 +936,20 @@ static void create_flash(const VirtMachineState *vms, */ hwaddr flashsize = vms->memmap[VIRT_FLASH].size / 2; hwaddr flashbase = vms->memmap[VIRT_FLASH].base; - char *nodename; - create_one_flash("virt.flash0", flashbase, flashsize, - bios_name, secure_sysmem); - create_one_flash("virt.flash1", flashbase + flashsize, flashsize, - NULL, sysmem); + virt_flash_map1(vms->flash[0], flashbase, flashsize, + secure_sysmem); + virt_flash_map1(vms->flash[1], flashbase + flashsize, flashsize, + sysmem); +} + +static void virt_flash_fdt(VirtMachineState *vms, + MemoryRegion *sysmem, + MemoryRegion *secure_sysmem) +{ + hwaddr flashsize = vms->memmap[VIRT_FLASH].size / 2; + hwaddr flashbase = vms->memmap[VIRT_FLASH].base; + char *nodename; if (sysmem == secure_sysmem) { /* Report both flash devices as a single node in the DT */ @@ -959,7 +962,8 @@ static void create_flash(const VirtMachineState *vms, qemu_fdt_setprop_cell(vms->fdt, nodename, "bank-width", 4); g_free(nodename); } else { - /* Report the devices as separate nodes so we can mark one as + /* + * Report the devices as separate nodes so we can mark one as * only visible to the secure world. */ nodename = g_strdup_printf("/secflash@%" PRIx64, flashbase); @@ -982,6 +986,54 @@ static void create_flash(const VirtMachineState *vms, } } +static bool virt_firmware_init(VirtMachineState *vms, + MemoryRegion *sysmem, + MemoryRegion *secure_sysmem) +{ + int i; + BlockBackend *pflash_blk0; + + /* Map legacy -drive if=pflash to machine properties */ + for (i = 0; i < ARRAY_SIZE(vms->flash); i++) { + pflash_cfi01_legacy_drive(vms->flash[i], + drive_get(IF_PFLASH, 0, i)); + } + + virt_flash_map(vms, sysmem, secure_sysmem); + + pflash_blk0 = pflash_cfi01_get_blk(vms->flash[0]); + + if (bios_name) { + char *fname; + MemoryRegion *mr; + int image_size; + + if (pflash_blk0) { + error_report("The contents of the first flash device may be " + "specified with -bios or with -drive if=pflash... " + "but you cannot use both options at once"); + exit(1); + } + + /* Fall back to -bios */ + + fname = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); + if (!fname) { + error_report("Could not find ROM image '%s'", bios_name); + exit(1); + } + mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(vms->flash[0]), 0); + image_size = load_image_mr(fname, mr); + g_free(fname); + if (image_size < 0) { + error_report("Could not load ROM image '%s'", bios_name); + exit(1); + } + } + + return pflash_blk0 || bios_name; +} + static FWCfgState *create_fw_cfg(const VirtMachineState *vms, AddressSpace *as) { hwaddr base = vms->memmap[VIRT_FW_CFG].base; @@ -1421,7 +1473,7 @@ static void machvirt_init(MachineState *machine) MemoryRegion *secure_sysmem = NULL; int n, virt_max_cpus; MemoryRegion *ram = g_new(MemoryRegion, 1); - bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0); + bool firmware_loaded; bool aarch64 = true; /* @@ -1460,6 +1512,27 @@ static void machvirt_init(MachineState *machine) exit(1); } + if (vms->secure) { + if (kvm_enabled()) { + error_report("mach-virt: KVM does not support Security extensions"); + exit(1); + } + + /* + * The Secure view of the world is the same as the NonSecure, + * but with a few extra devices. Create it as a container region + * containing the system memory at low priority; any secure-only + * devices go in at higher priority and take precedence. + */ + secure_sysmem = g_new(MemoryRegion, 1); + memory_region_init(secure_sysmem, OBJECT(machine), "secure-memory", + UINT64_MAX); + memory_region_add_subregion_overlap(secure_sysmem, 0, sysmem, -1); + } + + firmware_loaded = virt_firmware_init(vms, sysmem, + secure_sysmem ?: sysmem); + /* If we have an EL3 boot ROM then the assumption is that it will * implement PSCI itself, so disable QEMU's internal implementation * so it doesn't get in the way. Instead of starting secondary @@ -1505,23 +1578,6 @@ static void machvirt_init(MachineState *machine) exit(1); } - if (vms->secure) { - if (kvm_enabled()) { - error_report("mach-virt: KVM does not support Security extensions"); - exit(1); - } - - /* The Secure view of the world is the same as the NonSecure, - * but with a few extra devices. Create it as a container region - * containing the system memory at low priority; any secure-only - * devices go in at higher priority and take precedence. - */ - secure_sysmem = g_new(MemoryRegion, 1); - memory_region_init(secure_sysmem, OBJECT(machine), "secure-memory", - UINT64_MAX); - memory_region_add_subregion_overlap(secure_sysmem, 0, sysmem, -1); - } - create_fdt(vms); possible_cpus = mc->possible_cpu_arch_ids(machine); @@ -1610,7 +1666,7 @@ static void machvirt_init(MachineState *machine) &machine->device_memory->mr); } - create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem); + virt_flash_fdt(vms, sysmem, secure_sysmem); create_gic(vms, pic); @@ -1956,6 +2012,8 @@ static void virt_instance_init(Object *obj) NULL); vms->irqmap = a15irqmap; + + virt_flash_create(vms); } static const TypeInfo virt_machine_info = { -- cgit v1.2.3 From ff3dcf28c0b7a3ac261399c3754bf2f410c2e91e Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 7 May 2019 12:55:02 +0100 Subject: hw/arm/raspi: Diagnose requests for too much RAM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Raspberry Pi boards have a physical memory map which does not allow for more than 1GB of RAM. Currently if the user tries to ask for more then we fail in a confusing way: $ qemu-system-aarch64 --machine raspi3 -m 8G Unexpected error in visit_type_uintN() at qapi/qapi-visit-core.c:164: qemu-system-aarch64: Parameter 'vcram-base' expects uint32_t Aborted (core dumped) Catch this earlier and diagnose it with a more friendly message: $ qemu-system-aarch64 --machine raspi3 -m 8G qemu-system-aarch64: Requested ram size is too large for this machine: maximum is 1GB Fixes: https://bugs.launchpad.net/qemu/+bug/1794187 Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Reviewed-by: Wainer dos Santos Moschetta --- hw/arm/raspi.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'hw') diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c index 66899c28dc..fe2bb511b9 100644 --- a/hw/arm/raspi.c +++ b/hw/arm/raspi.c @@ -12,6 +12,7 @@ */ #include "qemu/osdep.h" +#include "qemu/units.h" #include "qapi/error.h" #include "qemu-common.h" #include "cpu.h" @@ -175,6 +176,12 @@ static void raspi_init(MachineState *machine, int version) BusState *bus; DeviceState *carddev; + if (machine->ram_size > 1 * GiB) { + error_report("Requested ram size is too large for this machine: " + "maximum is 1GB"); + exit(1); + } + object_initialize(&s->soc, sizeof(s->soc), version == 3 ? TYPE_BCM2837 : TYPE_BCM2836); object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc), -- cgit v1.2.3 From a9df9622bc28ff94bb65e4264ddbf7e600d911dc Mon Sep 17 00:00:00 2001 From: Joel Stanley Date: Tue, 7 May 2019 12:55:02 +0100 Subject: arm: aspeed: Set SDRAM size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We currently use Qemu's default of 128MB. As we know how much ram each machine ships with, make it easier on users by setting a default. It can still be overridden with -m on the command line. Signed-off-by: Joel Stanley Reviewed-by: Andrew Jeffery Reviewed-by: Richard Henderson Message-id: 20190503022958.1394-1-joel@jms.id.au Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Peter Maydell --- hw/arm/aspeed.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'hw') diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index 1c23ebd992..29d225ed14 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -25,6 +25,7 @@ #include "sysemu/block-backend.h" #include "hw/loader.h" #include "qemu/error-report.h" +#include "qemu/units.h" static struct arm_boot_info aspeed_board_binfo = { .board_id = -1, /* device-tree-only board */ @@ -331,6 +332,9 @@ static void aspeed_machine_class_init(ObjectClass *oc, void *data) mc->no_floppy = 1; mc->no_cdrom = 1; mc->no_parallel = 1; + if (board->ram) { + mc->default_ram_size = board->ram; + } amc->board = board; } @@ -352,6 +356,7 @@ static const AspeedBoardConfig aspeed_boards[] = { .spi_model = "mx25l25635e", .num_cs = 1, .i2c_init = palmetto_bmc_i2c_init, + .ram = 256 * MiB, }, { .name = MACHINE_TYPE_NAME("ast2500-evb"), .desc = "Aspeed AST2500 EVB (ARM1176)", @@ -361,6 +366,7 @@ static const AspeedBoardConfig aspeed_boards[] = { .spi_model = "mx25l25635e", .num_cs = 1, .i2c_init = ast2500_evb_i2c_init, + .ram = 512 * MiB, }, { .name = MACHINE_TYPE_NAME("romulus-bmc"), .desc = "OpenPOWER Romulus BMC (ARM1176)", @@ -370,6 +376,7 @@ static const AspeedBoardConfig aspeed_boards[] = { .spi_model = "mx66l1g45g", .num_cs = 2, .i2c_init = romulus_bmc_i2c_init, + .ram = 512 * MiB, }, { .name = MACHINE_TYPE_NAME("witherspoon-bmc"), .desc = "OpenPOWER Witherspoon BMC (ARM1176)", @@ -379,6 +386,7 @@ static const AspeedBoardConfig aspeed_boards[] = { .spi_model = "mx66l1g45g", .num_cs = 2, .i2c_init = witherspoon_bmc_i2c_init, + .ram = 512 * MiB, }, }; -- cgit v1.2.3 From b01e2f0284a2df11aef990219104e3f52c317061 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 7 May 2019 12:55:03 +0100 Subject: hw/arm/armv7m_nvic: Check subpriority in nvic_recompute_state_secure() Rule R_CQRV says that if two pending interrupts have the same group priority then ties are broken by looking at the subpriority. We had a comment describing this but had forgotten to actually implement the subpriority comparison. Correct the omission. (The further tie break rules of "lowest exception number" and "secure before non-secure" are handled implicitly by the order in which we iterate through the exceptions in the loops.) Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20190430131439.25251-2-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'hw') diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index fff6e694e6..131b5938b9 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -213,6 +213,7 @@ static void nvic_recompute_state_secure(NVICState *s) int active_prio = NVIC_NOEXC_PRIO; int pend_irq = 0; bool pending_is_s_banked = false; + int pend_subprio = 0; /* R_CQRV: precedence is by: * - lowest group priority; if both the same then @@ -226,7 +227,7 @@ static void nvic_recompute_state_secure(NVICState *s) for (i = 1; i < s->num_irq; i++) { for (bank = M_REG_S; bank >= M_REG_NS; bank--) { VecInfo *vec; - int prio; + int prio, subprio; bool targets_secure; if (bank == M_REG_S) { @@ -241,8 +242,12 @@ static void nvic_recompute_state_secure(NVICState *s) } prio = exc_group_prio(s, vec->prio, targets_secure); - if (vec->enabled && vec->pending && prio < pend_prio) { + subprio = vec->prio & ~nvic_gprio_mask(s, targets_secure); + if (vec->enabled && vec->pending && + ((prio < pend_prio) || + (prio == pend_prio && prio >= 0 && subprio < pend_subprio))) { pend_prio = prio; + pend_subprio = subprio; pend_irq = i; pending_is_s_banked = (bank == M_REG_S); } -- cgit v1.2.3 From 339327b6d4a2830cba230c6be7a17a4a2fc3d546 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 7 May 2019 12:55:03 +0100 Subject: hw/intc/armv7m_nvic: NS BFAR and BFSR are RAZ/WI if BFHFNMINS == 0 The non-secure versions of the BFAR and BFSR registers are supposed to be RAZ/WI if AICR.BFHFNMINS == 0; we were incorrectly allowing NS code to access the real values. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20190430131439.25251-3-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) (limited to 'hw') diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 131b5938b9..15cba63c96 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -1167,6 +1167,10 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs) if (!arm_feature(&cpu->env, ARM_FEATURE_M_MAIN)) { goto bad_offset; } + if (!attrs.secure && + !(s->cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) { + return 0; + } return cpu->env.v7m.bfar; case 0xd3c: /* Aux Fault Status. */ /* TODO: Implement fault status registers. */ @@ -1646,6 +1650,10 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, if (!arm_feature(&cpu->env, ARM_FEATURE_M_MAIN)) { goto bad_offset; } + if (!attrs.secure && + !(s->cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) { + return; + } cpu->env.v7m.bfar = value; return; case 0xd3c: /* Aux Fault Status. */ @@ -2130,11 +2138,18 @@ static MemTxResult nvic_sysreg_read(void *opaque, hwaddr addr, val = 0; break; }; - /* The BFSR bits [15:8] are shared between security states - * and we store them in the NS copy + /* + * The BFSR bits [15:8] are shared between security states + * and we store them in the NS copy. They are RAZ/WI for + * NS code if AIRCR.BFHFNMINS is 0. */ val = s->cpu->env.v7m.cfsr[attrs.secure]; - val |= s->cpu->env.v7m.cfsr[M_REG_NS] & R_V7M_CFSR_BFSR_MASK; + if (!attrs.secure && + !(s->cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) { + val &= ~R_V7M_CFSR_BFSR_MASK; + } else { + val |= s->cpu->env.v7m.cfsr[M_REG_NS] & R_V7M_CFSR_BFSR_MASK; + } val = extract32(val, (offset - 0xd28) * 8, size * 8); break; case 0xfe0 ... 0xfff: /* ID. */ @@ -2249,6 +2264,12 @@ static MemTxResult nvic_sysreg_write(void *opaque, hwaddr addr, */ value <<= ((offset - 0xd28) * 8); + if (!attrs.secure && + !(s->cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) { + /* BFSR bits are RAZ/WI for NS if BFHFNMINS is set */ + value &= ~R_V7M_CFSR_BFSR_MASK; + } + s->cpu->env.v7m.cfsr[attrs.secure] &= ~value; if (attrs.secure) { /* The BFSR bits [15:8] are shared between security states -- cgit v1.2.3 From a03ffaefce5dbb455ee1bc7c7709faf377dfbd45 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 7 May 2019 12:55:03 +0100 Subject: hw/intc/armv7m_nvic: Don't enable ARMV7M_EXCP_DEBUG from reset The M-profile architecture specifies that the DebugMonitor exception should be initially disabled, not enabled. It should be controlled by the DEMCR register's MON_EN bit, but we don't implement that register yet (like most of the debug architecture for M-profile). Note that BKPT instructions will still work, because they will be escalated to HardFault. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20190430131439.25251-4-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'hw') diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 15cba63c96..3a346a682a 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -2491,10 +2491,12 @@ static void armv7m_nvic_reset(DeviceState *dev) * the System Handler Control register */ s->vectors[ARMV7M_EXCP_SVC].enabled = 1; - s->vectors[ARMV7M_EXCP_DEBUG].enabled = 1; s->vectors[ARMV7M_EXCP_PENDSV].enabled = 1; s->vectors[ARMV7M_EXCP_SYSTICK].enabled = 1; + /* DebugMonitor is enabled via DEMCR.MON_EN */ + s->vectors[ARMV7M_EXCP_DEBUG].enabled = 0; + resetprio = arm_feature(&s->cpu->env, ARM_FEATURE_V8) ? -4 : -3; s->vectors[ARMV7M_EXCP_RESET].prio = resetprio; s->vectors[ARMV7M_EXCP_NMI].prio = -2; -- cgit v1.2.3