target-arm queue: mostly just cleanup/minor stuff, but this does include the raspi3 board model. -- PMM The following changes since commit 9f9c53368b219a9115eddb39f0ff5ad19c977134: Merge remote-tracking branch 'remotes/vivier/tags/m68k-for-2.12-pull-request' into staging (2018-02-15 10:14:11 +0000) are available in the Git repository at: git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20180215 for you to fetch changes up to e545f0f9be1f9e60951017c1e6558216732cc14e: target/arm: Implement v8M MSPLIM and PSPLIM registers (2018-02-15 13:48:11 +0000) ---------------------------------------------------------------- target-arm queue: * aspeed: code cleanup to use unimplemented_device * add 'raspi3' RaspberryPi 3 machine model * more SVE prep work * v8M: add minor missing registers * v7M: fix bug where we weren't migrating v7m.other_sp * v7M: fix bugs in handling of interrupt registers for external interrupts beyond 32 ---------------------------------------------------------------- Pekka Enberg (3): bcm2836: Make CPU type configurable raspi: Raspberry Pi 3 support raspi: Add "raspi3" machine type Peter Maydell (11): hw/intc/armv7m_nvic: Don't hardcode M profile ID registers in NVIC hw/intc/armv7m_nvic: Fix ICSR PENDNMISET/CLR handling hw/intc/armv7m_nvic: Implement M profile cache maintenance ops hw/intc/armv7m_nvic: Implement v8M CPPWR register hw/intc/armv7m_nvic: Implement cache ID registers hw/intc/armv7m_nvic: Implement SCR target/arm: Implement writing to CONTROL_NS for v8M hw/intc/armv7m_nvic: Fix byte-to-interrupt number conversions target/arm: Add AIRCR to vmstate struct target/arm: Migrate v7m.other_sp target/arm: Implement v8M MSPLIM and PSPLIM registers Philippe Mathieu-Daudé (2): hw/arm/aspeed: directly map the serial device to the system address space hw/arm/aspeed: simplify using the 'unimplemented device' for aspeed_soc.io Richard Henderson (5): target/arm: Remove ARM_CP_64BIT from ZCR_EL registers target/arm: Enforce FP access to FPCR/FPSR target/arm: Suppress TB end for FPCR/FPSR target/arm: Enforce access to ZCR_EL at translation target/arm: Handle SVE registers when using clear_vec_high include/hw/arm/aspeed_soc.h | 1 - include/hw/arm/bcm2836.h | 1 + target/arm/cpu.h | 71 ++++++++++++----- target/arm/internals.h | 6 ++ hw/arm/aspeed_soc.c | 35 ++------- hw/arm/bcm2836.c | 17 +++-- hw/arm/raspi.c | 57 +++++++++++--- hw/intc/armv7m_nvic.c | 98 ++++++++++++++++++------ target/arm/cpu.c | 28 +++++++ target/arm/helper.c | 84 +++++++++++++++----- target/arm/machine.c | 84 ++++++++++++++++++++ target/arm/translate-a64.c | 181 ++++++++++++++++++++------------------------ 12 files changed, 452 insertions(+), 211 deletions(-) From: Philippe Mathieu-Daudé <f4bug@amsat.org> (qemu) info mtree address-space: cpu-memory-0 0000000000000000-ffffffffffffffff (prio 0, i/o): system 0000000000000000-0000000007ffffff (prio 0, rom): aspeed.boot_rom 000000001e600000-000000001e7fffff (prio -1, i/o): aspeed_soc.io - 000000001e784000-000000001e78401f (prio 0, i/o): serial 000000001e620000-000000001e6200ff (prio 0, i/o): aspeed.smc.ast2500-fmc 000000001e630000-000000001e6300ff (prio 0, i/o): aspeed.smc.ast2500-spi1 [...] 000000001e720000-000000001e728fff (prio 0, ram): aspeed.sram 000000001e782000-000000001e782fff (prio 0, i/o): aspeed.timer + 000000001e784000-000000001e78401f (prio 0, i/o): serial 000000001e785000-000000001e78501f (prio 0, i/o): aspeed.wdt 000000001e785020-000000001e78503f (prio 0, i/o): aspeed.wdt Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Andrew Jeffery <andrew@aj.id.au> Message-id: 20180209085755.30414-2-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/aspeed_soc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed_soc.c +++ b/hw/arm/aspeed_soc.c @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) /* UART - attach an 8250 to the IO space as our UART5 */ if (serial_hds[0]) { qemu_irq uart5 = qdev_get_gpio_in(DEVICE(&s->vic), uart_irqs[4]); - serial_mm_init(&s->iomem, ASPEED_SOC_UART_5_BASE, 2, + serial_mm_init(get_system_memory(), + ASPEED_SOC_IOMEM_BASE + ASPEED_SOC_UART_5_BASE, 2, uart5, 38400, serial_hds[0], DEVICE_LITTLE_ENDIAN); } -- 2.16.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> (qemu) info mtree address-space: cpu-memory-0 0000000000000000-ffffffffffffffff (prio 0, i/o): system 0000000000000000-0000000007ffffff (prio 0, rom): aspeed.boot_rom - 000000001e600000-000000001e7fffff (prio -1, i/o): aspeed_soc.io + 000000001e600000-000000001e7fffff (prio -1000, i/o): aspeed_soc.io 000000001e620000-000000001e6200ff (prio 0, i/o): aspeed.smc.ast2500-fmc 000000001e630000-000000001e6300ff (prio 0, i/o): aspeed.smc.ast2500-spi1 000000001e631000-000000001e6310ff (prio 0, i/o): aspeed.smc.ast2500-spi2 Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Andrew Jeffery <andrew@aj.id.au> Message-id: 20180209085755.30414-3-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/aspeed_soc.h | 1 - hw/arm/aspeed_soc.c | 32 +++----------------------------- 2 files changed, 3 insertions(+), 30 deletions(-) diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/aspeed_soc.h +++ b/include/hw/arm/aspeed_soc.h @@ -XXX,XX +XXX,XX @@ typedef struct AspeedSoCState { /*< public >*/ ARMCPU cpu; - MemoryRegion iomem; MemoryRegion sram; AspeedVICState vic; AspeedTimerCtrlState timerctrl; diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed_soc.c +++ b/hw/arm/aspeed_soc.c @@ -XXX,XX +XXX,XX @@ #include "qemu-common.h" #include "cpu.h" #include "exec/address-spaces.h" +#include "hw/misc/unimp.h" #include "hw/arm/aspeed_soc.h" #include "hw/char/serial.h" #include "qemu/log.h" @@ -XXX,XX +XXX,XX @@ static const AspeedSoCInfo aspeed_socs[] = { }, }; -/* - * IO handlers: simply catch any reads/writes to IO addresses that aren't - * handled by a device mapping. - */ - -static uint64_t aspeed_soc_io_read(void *p, hwaddr offset, unsigned size) -{ - qemu_log_mask(LOG_UNIMP, "%s: 0x%" HWADDR_PRIx " [%u]\n", - __func__, offset, size); - return 0; -} - -static void aspeed_soc_io_write(void *opaque, hwaddr offset, uint64_t value, - unsigned size) -{ - qemu_log_mask(LOG_UNIMP, "%s: 0x%" HWADDR_PRIx " <- 0x%" PRIx64 " [%u]\n", - __func__, offset, value, size); -} - -static const MemoryRegionOps aspeed_soc_io_ops = { - .read = aspeed_soc_io_read, - .write = aspeed_soc_io_write, - .endianness = DEVICE_LITTLE_ENDIAN, -}; - static void aspeed_soc_init(Object *obj) { AspeedSoCState *s = ASPEED_SOC(obj); @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) Error *err = NULL, *local_err = NULL; /* IO space */ - memory_region_init_io(&s->iomem, NULL, &aspeed_soc_io_ops, NULL, - "aspeed_soc.io", ASPEED_SOC_IOMEM_SIZE); - memory_region_add_subregion_overlap(get_system_memory(), - ASPEED_SOC_IOMEM_BASE, &s->iomem, -1); + create_unimplemented_device("aspeed_soc.io", + ASPEED_SOC_IOMEM_BASE, ASPEED_SOC_IOMEM_SIZE); /* CPU */ object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err); -- 2.16.1 From: Pekka Enberg <penberg@iki.fi> This patch adds a "cpu-type" property to BCM2836 SoC in preparation for reusing the code for the Raspberry Pi 3, which has a different processor model. Signed-off-by: Pekka Enberg <penberg@iki.fi> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/bcm2836.h | 1 + hw/arm/bcm2836.c | 17 +++++++++-------- hw/arm/raspi.c | 3 +++ 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/include/hw/arm/bcm2836.h b/include/hw/arm/bcm2836.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/bcm2836.h +++ b/include/hw/arm/bcm2836.h @@ -XXX,XX +XXX,XX @@ typedef struct BCM2836State { DeviceState parent_obj; /*< public >*/ + char *cpu_type; uint32_t enabled_cpus; ARMCPU cpus[BCM2836_NCPUS]; diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/bcm2836.c +++ b/hw/arm/bcm2836.c @@ -XXX,XX +XXX,XX @@ static void bcm2836_init(Object *obj) { BCM2836State *s = BCM2836(obj); - int n; - - for (n = 0; n < BCM2836_NCPUS; n++) { - object_initialize(&s->cpus[n], sizeof(s->cpus[n]), - "cortex-a15-" TYPE_ARM_CPU); - object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpus[n]), - &error_abort); - } object_initialize(&s->control, sizeof(s->control), TYPE_BCM2836_CONTROL); object_property_add_child(obj, "control", OBJECT(&s->control), NULL); @@ -XXX,XX +XXX,XX @@ static void bcm2836_realize(DeviceState *dev, Error **errp) /* common peripherals from bcm2835 */ + obj = OBJECT(dev); + for (n = 0; n < BCM2836_NCPUS; n++) { + object_initialize(&s->cpus[n], sizeof(s->cpus[n]), + s->cpu_type); + object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpus[n]), + &error_abort); + } + obj = object_property_get_link(OBJECT(dev), "ram", &err); if (obj == NULL) { error_setg(errp, "%s: required ram link not found: %s", @@ -XXX,XX +XXX,XX @@ static void bcm2836_realize(DeviceState *dev, Error **errp) } static Property bcm2836_props[] = { + DEFINE_PROP_STRING("cpu-type", BCM2836State, cpu_type), DEFINE_PROP_UINT32("enabled-cpus", BCM2836State, enabled_cpus, BCM2836_NCPUS), DEFINE_PROP_END_OF_LIST() }; diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/raspi.c +++ b/hw/arm/raspi.c @@ -XXX,XX +XXX,XX @@ static void raspi2_init(MachineState *machine) /* Setup the SOC */ object_property_add_const_link(OBJECT(&s->soc), "ram", OBJECT(&s->ram), &error_abort); + object_property_set_str(OBJECT(&s->soc), machine->cpu_type, "cpu-type", + &error_abort); object_property_set_int(OBJECT(&s->soc), smp_cpus, "enabled-cpus", &error_abort); object_property_set_int(OBJECT(&s->soc), 0xa21041, "board-rev", @@ -XXX,XX +XXX,XX @@ static void raspi2_machine_init(MachineClass *mc) mc->no_parallel = 1; mc->no_floppy = 1; mc->no_cdrom = 1; + mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15"); mc->max_cpus = BCM2836_NCPUS; mc->min_cpus = BCM2836_NCPUS; mc->default_cpus = BCM2836_NCPUS; -- 2.16.1 From: Pekka Enberg <penberg@iki.fi> This patch adds Raspberry Pi 3 support to hw/arm/raspi.c. The differences to Pi 2 are: - Firmware address - Board ID - Board revision The CPU is different too, but that's going to be configured as part of the machine default CPU when we introduce a new machine type. The patch was written from scratch by me but the logic is similar to Zoltán Baldaszti's previous work, which I used as a reference (with permission from the author): https://github.com/bztsrc/qemu-raspi3 Signed-off-by: Pekka Enberg <penberg@iki.fi> [PMM: fixed trailing whitespace on one line] Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/raspi.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/raspi.c +++ b/hw/arm/raspi.c @@ -XXX,XX +XXX,XX @@ * Rasperry Pi 2 emulation Copyright (c) 2015, Microsoft * Written by Andrew Baumann * + * Raspberry Pi 3 emulation Copyright (c) 2018 Zoltán Baldaszti + * Upstream code cleanup (c) 2018 Pekka Enberg + * * This code is licensed under the GNU GPLv2 and later. */ @@ -XXX,XX +XXX,XX @@ #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 */ +#define FIRMWARE_ADDR_2 0x8000 /* Pi 2 loads kernel.img here by default */ +#define FIRMWARE_ADDR_3 0x80000 /* Pi 3 loads kernel.img here by default */ /* Table of Linux board IDs for different Pi versions */ -static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43}; +static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43, [3] = 0xc44}; typedef struct RasPiState { BCM2836State soc; @@ -XXX,XX +XXX,XX @@ static void setup_boot(MachineState *machine, int version, size_t ram_size) binfo.secure_board_setup = true; binfo.secure_boot = true; - /* Pi2 requires SMP setup */ - if (version == 2) { + /* Pi2 and Pi3 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; @@ -XXX,XX +XXX,XX @@ static void setup_boot(MachineState *machine, int version, size_t ram_size) * the normal Linux boot process */ if (machine->firmware) { + hwaddr firmware_addr = version == 3 ? FIRMWARE_ADDR_3 : FIRMWARE_ADDR_2; /* load the firmware image (typically kernel.img) */ - r = load_image_targphys(machine->firmware, FIRMWARE_ADDR, - ram_size - FIRMWARE_ADDR); + 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); } - binfo.entry = FIRMWARE_ADDR; + binfo.entry = firmware_addr; binfo.firmware_loaded = true; } else { binfo.kernel_filename = machine->kernel_filename; @@ -XXX,XX +XXX,XX @@ static void setup_boot(MachineState *machine, int version, size_t ram_size) arm_load_kernel(ARM_CPU(first_cpu), &binfo); } -static void raspi2_init(MachineState *machine) +static void raspi_init(MachineState *machine, int version) { RasPiState *s = g_new0(RasPiState, 1); uint32_t vcram_size; @@ -XXX,XX +XXX,XX @@ static void raspi2_init(MachineState *machine) &error_abort); object_property_set_int(OBJECT(&s->soc), smp_cpus, "enabled-cpus", &error_abort); - object_property_set_int(OBJECT(&s->soc), 0xa21041, "board-rev", + int board_rev = version == 3 ? 0xa02082 : 0xa21041; + object_property_set_int(OBJECT(&s->soc), board_rev, "board-rev", &error_abort); object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_abort); @@ -XXX,XX +XXX,XX @@ static void raspi2_init(MachineState *machine) vcram_size = object_property_get_uint(OBJECT(&s->soc), "vcram-size", &error_abort); - setup_boot(machine, 2, machine->ram_size - vcram_size); + setup_boot(machine, version, machine->ram_size - vcram_size); +} + +static void raspi2_init(MachineState *machine) +{ + raspi_init(machine, 2); } static void raspi2_machine_init(MachineClass *mc) -- 2.16.1 From: Pekka Enberg <penberg@iki.fi> This patch adds a "raspi3" machine type, which can now be selected as the machine to run on by users via the "-M" command line option to QEMU. The machine type does *not* ignore memory transaction failures so we likely need to add some dummy devices later when people run something more complicated than what I'm using for testing. Signed-off-by: Pekka Enberg <penberg@iki.fi> [PMM: added #ifdef TARGET_AARCH64 so we don't provide the 64-bit board in the 32-bit only arm-softmmu build.] Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/raspi.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/raspi.c +++ b/hw/arm/raspi.c @@ -XXX,XX +XXX,XX @@ static void raspi2_machine_init(MachineClass *mc) mc->ignore_memory_transaction_failures = true; }; DEFINE_MACHINE("raspi2", raspi2_machine_init) + +#ifdef TARGET_AARCH64 +static void raspi3_init(MachineState *machine) +{ + raspi_init(machine, 3); +} + +static void raspi3_machine_init(MachineClass *mc) +{ + mc->desc = "Raspberry Pi 3"; + mc->init = raspi3_init; + mc->block_default_type = IF_SD; + mc->no_parallel = 1; + mc->no_floppy = 1; + mc->no_cdrom = 1; + mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53"); + mc->max_cpus = BCM2836_NCPUS; + mc->min_cpus = BCM2836_NCPUS; + mc->default_cpus = BCM2836_NCPUS; + mc->default_ram_size = 1024 * 1024 * 1024; +} +DEFINE_MACHINE("raspi3", raspi3_machine_init) +#endif -- 2.16.1 From: Richard Henderson <richard.henderson@linaro.org> Because they are ARM_CP_STATE_AA64, ARM_CP_64BIT is implied. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180211205848.4568-2-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static void zcr_write(CPUARMState *env, const ARMCPRegInfo *ri, static const ARMCPRegInfo zcr_el1_reginfo = { .name = "ZCR_EL1", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 0, .crn = 1, .crm = 2, .opc2 = 0, - .access = PL1_RW, .accessfn = zcr_access, .type = ARM_CP_64BIT, + .access = PL1_RW, .accessfn = zcr_access, .fieldoffset = offsetof(CPUARMState, vfp.zcr_el[1]), .writefn = zcr_write, .raw_writefn = raw_write }; @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo zcr_el1_reginfo = { static const ARMCPRegInfo zcr_el2_reginfo = { .name = "ZCR_EL2", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 2, .opc2 = 0, - .access = PL2_RW, .accessfn = zcr_access, .type = ARM_CP_64BIT, + .access = PL2_RW, .accessfn = zcr_access, .fieldoffset = offsetof(CPUARMState, vfp.zcr_el[2]), .writefn = zcr_write, .raw_writefn = raw_write }; @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo zcr_el2_reginfo = { static const ARMCPRegInfo zcr_no_el2_reginfo = { .name = "ZCR_EL2", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 2, .opc2 = 0, - .access = PL2_RW, .type = ARM_CP_64BIT, + .access = PL2_RW, .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore }; static const ARMCPRegInfo zcr_el3_reginfo = { .name = "ZCR_EL3", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 2, .opc2 = 0, - .access = PL3_RW, .accessfn = zcr_access, .type = ARM_CP_64BIT, + .access = PL3_RW, .accessfn = zcr_access, .fieldoffset = offsetof(CPUARMState, vfp.zcr_el[3]), .writefn = zcr_write, .raw_writefn = raw_write }; -- 2.16.1 From: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180211205848.4568-3-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 35 ++++++++++++++++++----------------- target/arm/helper.c | 6 ++++-- target/arm/translate-a64.c | 3 +++ 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) } /* ARMCPRegInfo type field bits. If the SPECIAL bit is set this is a - * special-behaviour cp reg and bits [15..8] indicate what behaviour + * special-behaviour cp reg and bits [11..8] indicate what behaviour * it has. Otherwise it is a simple cp reg, where CONST indicates that * TCG can assume the value to be constant (ie load at translate time) * and 64BIT indicates a 64 bit wide coprocessor register. SUPPRESS_TB_END @@ -XXX,XX +XXX,XX @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) * need to be surrounded by gen_io_start()/gen_io_end(). In particular, * registers which implement clocks or timers require this. */ -#define ARM_CP_SPECIAL 1 -#define ARM_CP_CONST 2 -#define ARM_CP_64BIT 4 -#define ARM_CP_SUPPRESS_TB_END 8 -#define ARM_CP_OVERRIDE 16 -#define ARM_CP_ALIAS 32 -#define ARM_CP_IO 64 -#define ARM_CP_NO_RAW 128 -#define ARM_CP_NOP (ARM_CP_SPECIAL | (1 << 8)) -#define ARM_CP_WFI (ARM_CP_SPECIAL | (2 << 8)) -#define ARM_CP_NZCV (ARM_CP_SPECIAL | (3 << 8)) -#define ARM_CP_CURRENTEL (ARM_CP_SPECIAL | (4 << 8)) -#define ARM_CP_DC_ZVA (ARM_CP_SPECIAL | (5 << 8)) -#define ARM_LAST_SPECIAL ARM_CP_DC_ZVA +#define ARM_CP_SPECIAL 0x0001 +#define ARM_CP_CONST 0x0002 +#define ARM_CP_64BIT 0x0004 +#define ARM_CP_SUPPRESS_TB_END 0x0008 +#define ARM_CP_OVERRIDE 0x0010 +#define ARM_CP_ALIAS 0x0020 +#define ARM_CP_IO 0x0040 +#define ARM_CP_NO_RAW 0x0080 +#define ARM_CP_NOP (ARM_CP_SPECIAL | 0x0100) +#define ARM_CP_WFI (ARM_CP_SPECIAL | 0x0200) +#define ARM_CP_NZCV (ARM_CP_SPECIAL | 0x0300) +#define ARM_CP_CURRENTEL (ARM_CP_SPECIAL | 0x0400) +#define ARM_CP_DC_ZVA (ARM_CP_SPECIAL | 0x0500) +#define ARM_LAST_SPECIAL ARM_CP_DC_ZVA +#define ARM_CP_FPU 0x1000 /* Used only as a terminator for ARMCPRegInfo lists */ -#define ARM_CP_SENTINEL 0xffff +#define ARM_CP_SENTINEL 0xffff /* Mask of only the flag bits in a type field */ -#define ARM_CP_FLAG_MASK 0xff +#define ARM_CP_FLAG_MASK 0x10ff /* Valid values for ARMCPRegInfo state field, indicating which of * the AArch32 and AArch64 execution states this register is visible in. diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v8_cp_reginfo[] = { .writefn = aa64_daif_write, .resetfn = arm_cp_reset_ignore }, { .name = "FPCR", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 3, .opc2 = 0, .crn = 4, .crm = 4, - .access = PL0_RW, .readfn = aa64_fpcr_read, .writefn = aa64_fpcr_write }, + .access = PL0_RW, .type = ARM_CP_FPU, + .readfn = aa64_fpcr_read, .writefn = aa64_fpcr_write }, { .name = "FPSR", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 4, .crm = 4, - .access = PL0_RW, .readfn = aa64_fpsr_read, .writefn = aa64_fpsr_write }, + .access = PL0_RW, .type = ARM_CP_FPU, + .readfn = aa64_fpsr_read, .writefn = aa64_fpsr_write }, { .name = "DCZID_EL0", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 3, .opc2 = 7, .crn = 0, .crm = 0, .access = PL0_R, .type = ARM_CP_NO_RAW, diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, default: break; } + if ((ri->type & ARM_CP_FPU) && !fp_access_check(s)) { + return; + } if ((tb_cflags(s->base.tb) & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO)) { gen_io_start(); -- 2.16.1 From: Richard Henderson <richard.henderson@linaro.org> Nothing in either register affects the TB. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180211205848.4568-4-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v8_cp_reginfo[] = { .writefn = aa64_daif_write, .resetfn = arm_cp_reset_ignore }, { .name = "FPCR", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 3, .opc2 = 0, .crn = 4, .crm = 4, - .access = PL0_RW, .type = ARM_CP_FPU, + .access = PL0_RW, .type = ARM_CP_FPU | ARM_CP_SUPPRESS_TB_END, .readfn = aa64_fpcr_read, .writefn = aa64_fpcr_write }, { .name = "FPSR", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 4, .crm = 4, - .access = PL0_RW, .type = ARM_CP_FPU, + .access = PL0_RW, .type = ARM_CP_FPU | ARM_CP_SUPPRESS_TB_END, .readfn = aa64_fpsr_read, .writefn = aa64_fpsr_write }, { .name = "DCZID_EL0", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 3, .opc2 = 7, .crn = 0, .crm = 0, -- 2.16.1 From: Richard Henderson <richard.henderson@linaro.org> This also makes sure that we get the correct ordering of SVE vs FP exceptions. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180211205848.4568-5-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 3 ++- target/arm/internals.h | 6 ++++++ target/arm/helper.c | 22 ++++------------------ target/arm/translate-a64.c | 16 ++++++++++++++++ 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) #define ARM_CP_DC_ZVA (ARM_CP_SPECIAL | 0x0500) #define ARM_LAST_SPECIAL ARM_CP_DC_ZVA #define ARM_CP_FPU 0x1000 +#define ARM_CP_SVE 0x2000 /* Used only as a terminator for ARMCPRegInfo lists */ #define ARM_CP_SENTINEL 0xffff /* Mask of only the flag bits in a type field */ -#define ARM_CP_FLAG_MASK 0x10ff +#define ARM_CP_FLAG_MASK 0x30ff /* Valid values for ARMCPRegInfo state field, indicating which of * the AArch32 and AArch64 execution states this register is visible in. diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ enum arm_exception_class { EC_AA64_HVC = 0x16, EC_AA64_SMC = 0x17, EC_SYSTEMREGISTERTRAP = 0x18, + EC_SVEACCESSTRAP = 0x19, EC_INSNABORT = 0x20, EC_INSNABORT_SAME_EL = 0x21, EC_PCALIGNMENT = 0x22, @@ -XXX,XX +XXX,XX @@ static inline uint32_t syn_fp_access_trap(int cv, int cond, bool is_16bit) | (cv << 24) | (cond << 20); } +static inline uint32_t syn_sve_access_trap(void) +{ + return EC_SVEACCESSTRAP << ARM_EL_EC_SHIFT; +} + static inline uint32_t syn_insn_abort(int same_el, int ea, int s1ptw, int fsc) { return (EC_INSNABORT << ARM_EL_EC_SHIFT) | (same_el << ARM_EL_EC_SHIFT) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static int sve_exception_el(CPUARMState *env) return 0; } -static CPAccessResult zcr_access(CPUARMState *env, const ARMCPRegInfo *ri, - bool isread) -{ - switch (sve_exception_el(env)) { - case 3: - return CP_ACCESS_TRAP_EL3; - case 2: - return CP_ACCESS_TRAP_EL2; - case 1: - return CP_ACCESS_TRAP; - } - return CP_ACCESS_OK; -} - static void zcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) { @@ -XXX,XX +XXX,XX @@ static void zcr_write(CPUARMState *env, const ARMCPRegInfo *ri, static const ARMCPRegInfo zcr_el1_reginfo = { .name = "ZCR_EL1", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 0, .crn = 1, .crm = 2, .opc2 = 0, - .access = PL1_RW, .accessfn = zcr_access, + .access = PL1_RW, .type = ARM_CP_SVE | ARM_CP_FPU, .fieldoffset = offsetof(CPUARMState, vfp.zcr_el[1]), .writefn = zcr_write, .raw_writefn = raw_write }; @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo zcr_el1_reginfo = { static const ARMCPRegInfo zcr_el2_reginfo = { .name = "ZCR_EL2", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 2, .opc2 = 0, - .access = PL2_RW, .accessfn = zcr_access, + .access = PL2_RW, .type = ARM_CP_SVE | ARM_CP_FPU, .fieldoffset = offsetof(CPUARMState, vfp.zcr_el[2]), .writefn = zcr_write, .raw_writefn = raw_write }; @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo zcr_el2_reginfo = { static const ARMCPRegInfo zcr_no_el2_reginfo = { .name = "ZCR_EL2", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 2, .opc2 = 0, - .access = PL2_RW, + .access = PL2_RW, .type = ARM_CP_SVE | ARM_CP_FPU, .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore }; static const ARMCPRegInfo zcr_el3_reginfo = { .name = "ZCR_EL3", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 2, .opc2 = 0, - .access = PL3_RW, .accessfn = zcr_access, + .access = PL3_RW, .type = ARM_CP_SVE | ARM_CP_FPU, .fieldoffset = offsetof(CPUARMState, vfp.zcr_el[3]), .writefn = zcr_write, .raw_writefn = raw_write }; diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static inline bool fp_access_check(DisasContext *s) return false; } +/* Check that SVE access is enabled. If it is, return true. + * If not, emit code to generate an appropriate exception and return false. + */ +static inline bool sve_access_check(DisasContext *s) +{ + if (s->sve_excp_el) { + gen_exception_insn(s, 4, EXCP_UDEF, syn_sve_access_trap(), + s->sve_excp_el); + return false; + } + return true; +} + /* * This utility function is for doing register extension with an * optional shift. You will likely want to pass a temporary for the @@ -XXX,XX +XXX,XX @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, default: break; } + if ((ri->type & ARM_CP_SVE) && !sve_access_check(s)) { + return; + } if ((ri->type & ARM_CP_FPU) && !fp_access_check(s)) { return; } -- 2.16.1 From: Richard Henderson <richard.henderson@linaro.org> When storing to an AdvSIMD FP register, all of the high bits of the SVE register are zeroed. Therefore, call it more often with is_q as a parameter. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180211205848.4568-6-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.c | 162 +++++++++++++++++---------------------------- 1 file changed, 62 insertions(+), 100 deletions(-) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static TCGv_i32 read_fp_sreg(DisasContext *s, int reg) return v; } +/* Clear the bits above an N-bit vector, for N = (is_q ? 128 : 64). + * If SVE is not enabled, then there are only 128 bits in the vector. + */ +static void clear_vec_high(DisasContext *s, bool is_q, int rd) +{ + unsigned ofs = fp_reg_offset(s, rd, MO_64); + unsigned vsz = vec_full_reg_size(s); + + if (!is_q) { + TCGv_i64 tcg_zero = tcg_const_i64(0); + tcg_gen_st_i64(tcg_zero, cpu_env, ofs + 8); + tcg_temp_free_i64(tcg_zero); + } + if (vsz > 16) { + tcg_gen_gvec_dup8i(ofs + 16, vsz - 16, vsz - 16, 0); + } +} + static void write_fp_dreg(DisasContext *s, int reg, TCGv_i64 v) { - TCGv_i64 tcg_zero = tcg_const_i64(0); + unsigned ofs = fp_reg_offset(s, reg, MO_64); - tcg_gen_st_i64(v, cpu_env, fp_reg_offset(s, reg, MO_64)); - tcg_gen_st_i64(tcg_zero, cpu_env, fp_reg_hi_offset(s, reg)); - tcg_temp_free_i64(tcg_zero); + tcg_gen_st_i64(v, cpu_env, ofs); + clear_vec_high(s, false, reg); } static void write_fp_sreg(DisasContext *s, int reg, TCGv_i32 v) @@ -XXX,XX +XXX,XX @@ static void do_fp_ld(DisasContext *s, int destidx, TCGv_i64 tcg_addr, int size) tcg_temp_free_i64(tmplo); tcg_temp_free_i64(tmphi); + + clear_vec_high(s, true, destidx); } /* @@ -XXX,XX +XXX,XX @@ static void write_vec_element_i32(DisasContext *s, TCGv_i32 tcg_src, } } -/* Clear the high 64 bits of a 128 bit vector (in general non-quad - * vector ops all need to do this). - */ -static void clear_vec_high(DisasContext *s, int rd) -{ - TCGv_i64 tcg_zero = tcg_const_i64(0); - - write_vec_element(s, tcg_zero, rd, 1, MO_64); - tcg_temp_free_i64(tcg_zero); -} - /* Store from vector register to memory */ static void do_vec_st(DisasContext *s, int srcidx, int element, TCGv_i64 tcg_addr, int size) @@ -XXX,XX +XXX,XX @@ static void disas_ldst_multiple_struct(DisasContext *s, uint32_t insn) /* For non-quad operations, setting a slice of the low * 64 bits of the register clears the high 64 bits (in * the ARM ARM pseudocode this is implicit in the fact - * that 'rval' is a 64 bit wide variable). We optimize - * by noticing that we only need to do this the first - * time we touch a register. + * that 'rval' is a 64 bit wide variable). + * For quad operations, we might still need to zero the + * high bits of SVE. We optimize by noticing that we only + * need to do this the first time we touch a register. */ - if (!is_q && e == 0 && (r == 0 || xs == selem - 1)) { - clear_vec_high(s, tt); + if (e == 0 && (r == 0 || xs == selem - 1)) { + clear_vec_high(s, is_q, tt); } } tcg_gen_addi_i64(tcg_addr, tcg_addr, ebytes); @@ -XXX,XX +XXX,XX @@ static void disas_ldst_single_struct(DisasContext *s, uint32_t insn) write_vec_element(s, tcg_tmp, rt, 0, MO_64); if (is_q) { write_vec_element(s, tcg_tmp, rt, 1, MO_64); - } else { - clear_vec_high(s, rt); } tcg_temp_free_i64(tcg_tmp); + clear_vec_high(s, is_q, rt); } else { /* Load/store one element per register */ if (is_load) { @@ -XXX,XX +XXX,XX @@ static void handle_vec_simd_sqshrn(DisasContext *s, bool is_scalar, bool is_q, } if (!is_q) { - clear_vec_high(s, rd); write_vec_element(s, tcg_final, rd, 0, MO_64); } else { write_vec_element(s, tcg_final, rd, 1, MO_64); @@ -XXX,XX +XXX,XX @@ static void handle_vec_simd_sqshrn(DisasContext *s, bool is_scalar, bool is_q, tcg_temp_free_i64(tcg_rd); tcg_temp_free_i32(tcg_rd_narrowed); tcg_temp_free_i64(tcg_final); - return; + + clear_vec_high(s, is_q, rd); } /* SQSHLU, UQSHL, SQSHL: saturating left shifts */ @@ -XXX,XX +XXX,XX @@ static void handle_simd_qshl(DisasContext *s, bool scalar, bool is_q, tcg_temp_free_i64(tcg_op); } tcg_temp_free_i64(tcg_shift); - - if (!is_q) { - clear_vec_high(s, rd); - } + clear_vec_high(s, is_q, rd); } else { TCGv_i32 tcg_shift = tcg_const_i32(shift); static NeonGenTwoOpEnvFn * const fns[2][2][3] = { @@ -XXX,XX +XXX,XX @@ static void handle_simd_qshl(DisasContext *s, bool scalar, bool is_q, } tcg_temp_free_i32(tcg_shift); - if (!is_q && !scalar) { - clear_vec_high(s, rd); + if (!scalar) { + clear_vec_high(s, is_q, rd); } } } @@ -XXX,XX +XXX,XX @@ static void handle_simd_intfp_conv(DisasContext *s, int rd, int rn, } } - if (!is_double && elements == 2) { - clear_vec_high(s, rd); - } - tcg_temp_free_i64(tcg_int); tcg_temp_free_ptr(tcg_fpst); tcg_temp_free_i32(tcg_shift); + + clear_vec_high(s, elements << size == 16, rd); } /* UCVTF/SCVTF - Integer to FP conversion */ @@ -XXX,XX +XXX,XX @@ static void handle_simd_shift_fpint_conv(DisasContext *s, bool is_scalar, write_vec_element(s, tcg_op, rd, pass, MO_64); tcg_temp_free_i64(tcg_op); } - if (!is_q) { - clear_vec_high(s, rd); - } + clear_vec_high(s, is_q, rd); } else { int maxpass = is_scalar ? 1 : is_q ? 4 : 2; for (pass = 0; pass < maxpass; pass++) { @@ -XXX,XX +XXX,XX @@ static void handle_simd_shift_fpint_conv(DisasContext *s, bool is_scalar, } tcg_temp_free_i32(tcg_op); } - if (!is_q && !is_scalar) { - clear_vec_high(s, rd); + if (!is_scalar) { + clear_vec_high(s, is_q, rd); } } @@ -XXX,XX +XXX,XX @@ static void handle_3same_float(DisasContext *s, int size, int elements, tcg_temp_free_ptr(fpst); - if ((elements << size) < 4) { - /* scalar, or non-quad vector op */ - clear_vec_high(s, rd); - } + clear_vec_high(s, elements * (size ? 8 : 4) > 8, rd); } /* AdvSIMD scalar three same @@ -XXX,XX +XXX,XX @@ static void handle_2misc_fcmp_zero(DisasContext *s, int opcode, } write_vec_element(s, tcg_res, rd, pass, MO_64); } - if (is_scalar) { - clear_vec_high(s, rd); - } - tcg_temp_free_i64(tcg_res); tcg_temp_free_i64(tcg_zero); tcg_temp_free_i64(tcg_op); + + clear_vec_high(s, !is_scalar, rd); } else { TCGv_i32 tcg_op = tcg_temp_new_i32(); TCGv_i32 tcg_zero = tcg_const_i32(0); @@ -XXX,XX +XXX,XX @@ static void handle_2misc_fcmp_zero(DisasContext *s, int opcode, tcg_temp_free_i32(tcg_res); tcg_temp_free_i32(tcg_zero); tcg_temp_free_i32(tcg_op); - if (!is_q && !is_scalar) { - clear_vec_high(s, rd); + if (!is_scalar) { + clear_vec_high(s, is_q, rd); } } @@ -XXX,XX +XXX,XX @@ static void handle_2misc_reciprocal(DisasContext *s, int opcode, } write_vec_element(s, tcg_res, rd, pass, MO_64); } - if (is_scalar) { - clear_vec_high(s, rd); - } - tcg_temp_free_i64(tcg_res); tcg_temp_free_i64(tcg_op); + clear_vec_high(s, !is_scalar, rd); } else { TCGv_i32 tcg_op = tcg_temp_new_i32(); TCGv_i32 tcg_res = tcg_temp_new_i32(); @@ -XXX,XX +XXX,XX @@ static void handle_2misc_reciprocal(DisasContext *s, int opcode, } tcg_temp_free_i32(tcg_res); tcg_temp_free_i32(tcg_op); - if (!is_q && !is_scalar) { - clear_vec_high(s, rd); + if (!is_scalar) { + clear_vec_high(s, is_q, rd); } } tcg_temp_free_ptr(fpst); @@ -XXX,XX +XXX,XX @@ static void handle_2misc_narrow(DisasContext *s, bool scalar, write_vec_element_i32(s, tcg_res[pass], rd, destelt + pass, MO_32); tcg_temp_free_i32(tcg_res[pass]); } - if (!is_q) { - clear_vec_high(s, rd); - } + clear_vec_high(s, is_q, rd); } /* Remaining saturating accumulating ops */ @@ -XXX,XX +XXX,XX @@ static void handle_2misc_satacc(DisasContext *s, bool is_scalar, bool is_u, } write_vec_element(s, tcg_rd, rd, pass, MO_64); } - if (is_scalar) { - clear_vec_high(s, rd); - } - tcg_temp_free_i64(tcg_rd); tcg_temp_free_i64(tcg_rn); + clear_vec_high(s, !is_scalar, rd); } else { TCGv_i32 tcg_rn = tcg_temp_new_i32(); TCGv_i32 tcg_rd = tcg_temp_new_i32(); @@ -XXX,XX +XXX,XX @@ static void handle_2misc_satacc(DisasContext *s, bool is_scalar, bool is_u, } write_vec_element_i32(s, tcg_rd, rd, pass, MO_32); } - - if (!is_q) { - clear_vec_high(s, rd); - } - tcg_temp_free_i32(tcg_rd); tcg_temp_free_i32(tcg_rn); + clear_vec_high(s, is_q, rd); } } @@ -XXX,XX +XXX,XX @@ static void handle_vec_simd_shri(DisasContext *s, bool is_q, bool is_u, tcg_temp_free_i64(tcg_round); done: - if (!is_q) { - clear_vec_high(s, rd); - } + clear_vec_high(s, is_q, rd); } static void gen_shl8_ins_i64(TCGv_i64 d, TCGv_i64 a, int64_t shift) @@ -XXX,XX +XXX,XX @@ static void handle_vec_simd_shrn(DisasContext *s, bool is_q, } if (!is_q) { - clear_vec_high(s, rd); write_vec_element(s, tcg_final, rd, 0, MO_64); } else { write_vec_element(s, tcg_final, rd, 1, MO_64); } - if (round) { tcg_temp_free_i64(tcg_round); } tcg_temp_free_i64(tcg_rn); tcg_temp_free_i64(tcg_rd); tcg_temp_free_i64(tcg_final); - return; + + clear_vec_high(s, is_q, rd); } @@ -XXX,XX +XXX,XX @@ static void handle_3rd_narrowing(DisasContext *s, int is_q, int is_u, int size, write_vec_element_i32(s, tcg_res[pass], rd, pass + part, MO_32); tcg_temp_free_i32(tcg_res[pass]); } - if (!is_q) { - clear_vec_high(s, rd); - } + clear_vec_high(s, is_q, rd); } static void handle_pmull_64(DisasContext *s, int is_q, int rd, int rn, int rm) @@ -XXX,XX +XXX,XX @@ static void handle_simd_3same_pair(DisasContext *s, int is_q, int u, int opcode, write_vec_element_i32(s, tcg_res[pass], rd, pass, MO_32); tcg_temp_free_i32(tcg_res[pass]); } - if (!is_q) { - clear_vec_high(s, rd); - } + clear_vec_high(s, is_q, rd); } if (fpst) { @@ -XXX,XX +XXX,XX @@ static void disas_simd_3same_int(DisasContext *s, uint32_t insn) tcg_temp_free_i32(tcg_op2); } } - - if (!is_q) { - clear_vec_high(s, rd); - } + clear_vec_high(s, is_q, rd); } /* AdvSIMD three same @@ -XXX,XX +XXX,XX @@ static void handle_rev(DisasContext *s, int opcode, bool u, write_vec_element(s, tcg_tmp, rd, i, grp_size); tcg_temp_free_i64(tcg_tmp); } - if (!is_q) { - clear_vec_high(s, rd); - } + clear_vec_high(s, is_q, rd); } else { int revmask = (1 << grp_size) - 1; int esize = 8 << size; @@ -XXX,XX +XXX,XX @@ static void disas_simd_two_reg_misc(DisasContext *s, uint32_t insn) tcg_temp_free_i32(tcg_op); } } - if (!is_q) { - clear_vec_high(s, rd); - } + clear_vec_high(s, is_q, rd); if (need_rmode) { gen_helper_set_rmode(tcg_rmode, tcg_rmode, cpu_env); @@ -XXX,XX +XXX,XX @@ static void disas_simd_indexed(DisasContext *s, uint32_t insn) tcg_temp_free_i64(tcg_res); } - if (is_scalar) { - clear_vec_high(s, rd); - } - tcg_temp_free_i64(tcg_idx); + clear_vec_high(s, !is_scalar, rd); } else if (!is_long) { /* 32 bit floating point, or 16 or 32 bit integer. * For the 16 bit scalar case we use the usual Neon helpers and @@ -XXX,XX +XXX,XX @@ static void disas_simd_indexed(DisasContext *s, uint32_t insn) } tcg_temp_free_i32(tcg_idx); - - if (!is_q) { - clear_vec_high(s, rd); - } + clear_vec_high(s, is_q, rd); } else { /* long ops: 16x16->32 or 32x32->64 */ TCGv_i64 tcg_res[2]; @@ -XXX,XX +XXX,XX @@ static void disas_simd_indexed(DisasContext *s, uint32_t insn) } tcg_temp_free_i64(tcg_idx); - if (is_scalar) { - clear_vec_high(s, rd); - } + clear_vec_high(s, !is_scalar, rd); } else { TCGv_i32 tcg_idx = tcg_temp_new_i32(); -- 2.16.1 Instead of hardcoding the values of M profile ID registers in the NVIC, use the fields in the CPU struct. This will allow us to give different M profile CPU types different ID register values. This commit includes the addition of the missing ID_ISAR5, which exists as RES0 in both v7M and v8M. (The values of the ID registers might be wrong for the M4 -- this commit leaves the behaviour there unchanged.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180209165810.6668-2-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 30 ++++++++++++++++-------------- target/arm/cpu.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs) "Aux Fault status registers unimplemented\n"); return 0; case 0xd40: /* PFR0. */ - return 0x00000030; - case 0xd44: /* PRF1. */ - return 0x00000200; + return cpu->id_pfr0; + case 0xd44: /* PFR1. */ + return cpu->id_pfr1; case 0xd48: /* DFR0. */ - return 0x00100000; + return cpu->id_dfr0; case 0xd4c: /* AFR0. */ - return 0x00000000; + return cpu->id_afr0; case 0xd50: /* MMFR0. */ - return 0x00000030; + return cpu->id_mmfr0; case 0xd54: /* MMFR1. */ - return 0x00000000; + return cpu->id_mmfr1; case 0xd58: /* MMFR2. */ - return 0x00000000; + return cpu->id_mmfr2; case 0xd5c: /* MMFR3. */ - return 0x00000000; + return cpu->id_mmfr3; case 0xd60: /* ISAR0. */ - return 0x01141110; + return cpu->id_isar0; case 0xd64: /* ISAR1. */ - return 0x02111000; + return cpu->id_isar1; case 0xd68: /* ISAR2. */ - return 0x21112231; + return cpu->id_isar2; case 0xd6c: /* ISAR3. */ - return 0x01111110; + return cpu->id_isar3; case 0xd70: /* ISAR4. */ - return 0x01310102; + return cpu->id_isar4; + case 0xd74: /* ISAR5. */ + return cpu->id_isar5; /* TODO: Implement debug registers. */ case 0xd90: /* MPU_TYPE */ /* Unified MPU; if the MPU is not present this value is zero */ diff --git a/target/arm/cpu.c b/target/arm/cpu.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -XXX,XX +XXX,XX @@ static void cortex_m3_initfn(Object *obj) set_feature(&cpu->env, ARM_FEATURE_M); cpu->midr = 0x410fc231; cpu->pmsav7_dregion = 8; + cpu->id_pfr0 = 0x00000030; + cpu->id_pfr1 = 0x00000200; + cpu->id_dfr0 = 0x00100000; + cpu->id_afr0 = 0x00000000; + cpu->id_mmfr0 = 0x00000030; + cpu->id_mmfr1 = 0x00000000; + cpu->id_mmfr2 = 0x00000000; + cpu->id_mmfr3 = 0x00000000; + cpu->id_isar0 = 0x01141110; + cpu->id_isar1 = 0x02111000; + cpu->id_isar2 = 0x21112231; + cpu->id_isar3 = 0x01111110; + cpu->id_isar4 = 0x01310102; + cpu->id_isar5 = 0x00000000; } static void cortex_m4_initfn(Object *obj) @@ -XXX,XX +XXX,XX @@ static void cortex_m4_initfn(Object *obj) set_feature(&cpu->env, ARM_FEATURE_THUMB_DSP); cpu->midr = 0x410fc240; /* r0p0 */ cpu->pmsav7_dregion = 8; + cpu->id_pfr0 = 0x00000030; + cpu->id_pfr1 = 0x00000200; + cpu->id_dfr0 = 0x00100000; + cpu->id_afr0 = 0x00000000; + cpu->id_mmfr0 = 0x00000030; + cpu->id_mmfr1 = 0x00000000; + cpu->id_mmfr2 = 0x00000000; + cpu->id_mmfr3 = 0x00000000; + cpu->id_isar0 = 0x01141110; + cpu->id_isar1 = 0x02111000; + cpu->id_isar2 = 0x21112231; + cpu->id_isar3 = 0x01111110; + cpu->id_isar4 = 0x01310102; + cpu->id_isar5 = 0x00000000; } static void arm_v7m_class_init(ObjectClass *oc, void *data) -- 2.16.1 The PENDNMISET/CLR bits in the ICSR should be RAZ/WI from NonSecure state if the AIRCR.BFHFNMINS bit is zero. We had misimplemented this as making the bits RAZ/WI from both Secure and NonSecure states. Fix this bug by checking attrs.secure so that Secure code can pend and unpend NMIs. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180209165810.6668-3-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs) } } /* NMIPENDSET */ - if ((cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK) && - s->vectors[ARMV7M_EXCP_NMI].pending) { + if ((attrs.secure || (cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) + && s->vectors[ARMV7M_EXCP_NMI].pending) { val |= (1 << 31); } /* ISRPREEMPT: RES0 when halting debug not implemented */ @@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, break; } case 0xd04: /* Interrupt Control State (ICSR) */ - if (cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK) { + if (attrs.secure || cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK) { if (value & (1 << 31)) { armv7m_nvic_set_pending(s, ARMV7M_EXCP_NMI, false); } else if (value & (1 << 30) && -- 2.16.1 For M profile cores, cache maintenance operations are done by writing to special registers in the system register space. For QEMU, cache operations are always NOPs, since we don't implement the cache. Implementing these explicitly avoids a spurious LOG_GUEST_ERROR when the guest uses them. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180209165810.6668-4-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, } break; } + case 0xf50: /* ICIALLU */ + case 0xf58: /* ICIMVAU */ + case 0xf5c: /* DCIMVAC */ + case 0xf60: /* DCISW */ + case 0xf64: /* DCCMVAU */ + case 0xf68: /* DCCMVAC */ + case 0xf6c: /* DCCSW */ + case 0xf70: /* DCCIMVAC */ + case 0xf74: /* DCCISW */ + case 0xf78: /* BPIALL */ + /* Cache and branch predictor maintenance: for QEMU these always NOP */ + break; default: bad_offset: qemu_log_mask(LOG_GUEST_ERROR, -- 2.16.1 The Coprocessor Power Control Register (CPPWR) is new in v8M. It allows software to control whether coprocessors are allowed to power down and lose their state. QEMU doesn't have any notion of power control, so we choose the IMPDEF option of making the whole register RAZ/WI (indicating that no coprocessors can ever power down and lose state). Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180209165810.6668-5-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs) switch (offset) { case 4: /* Interrupt Control Type. */ return ((s->num_irq - NVIC_FIRST_IRQ) / 32) - 1; + case 0xc: /* CPPWR */ + if (!arm_feature(&cpu->env, ARM_FEATURE_V8)) { + goto bad_offset; + } + /* We make the IMPDEF choice that nothing can ever go into a + * non-retentive power state, which allows us to RAZ/WI this. + */ + return 0; case 0x380 ... 0x3bf: /* NVIC_ITNS<n> */ { int startvec = 8 * (offset - 0x380) + NVIC_FIRST_IRQ; @@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, ARMCPU *cpu = s->cpu; switch (offset) { + case 0xc: /* CPPWR */ + if (!arm_feature(&cpu->env, ARM_FEATURE_V8)) { + goto bad_offset; + } + /* Make the IMPDEF choice to RAZ/WI this. */ + break; case 0x380 ... 0x3bf: /* NVIC_ITNS<n> */ { int startvec = 8 * (offset - 0x380) + NVIC_FIRST_IRQ; -- 2.16.1 M profile cores have a similar setup for cache ID registers to A profile: * Cache Level ID Register (CLIDR) is a fixed value * Cache Type Register (CTR) is a fixed value * Cache Size ID Registers (CCSIDR) are a bank of registers; which one you see is selected by the Cache Size Selection Register (CSSELR) The only difference is that they're in the NVIC memory mapped register space rather than being coprocessor registers. Implement the M profile view of them. Since neither Cortex-M3 nor Cortex-M4 implement caches, we don't need to update their init functions and can leave the ctr/clidr/ccsidr[] fields in their ARMCPU structs at zero. Newer cores (like the Cortex-M33) will want to be able to set these ID registers to non-zero values, though. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180209165810.6668-6-peter.maydell@linaro.org --- target/arm/cpu.h | 26 ++++++++++++++++++++++++++ hw/intc/armv7m_nvic.c | 16 ++++++++++++++++ target/arm/machine.c | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ typedef struct CPUARMState { uint32_t faultmask[M_REG_NUM_BANKS]; uint32_t aircr; /* only holds r/w state if security extn implemented */ uint32_t secure; /* Is CPU in Secure state? (not guest visible) */ + uint32_t csselr[M_REG_NUM_BANKS]; } v7m; /* Information associated with an exception about to be taken: @@ -XXX,XX +XXX,XX @@ FIELD(V7M_MPU_CTRL, ENABLE, 0, 1) FIELD(V7M_MPU_CTRL, HFNMIENA, 1, 1) FIELD(V7M_MPU_CTRL, PRIVDEFENA, 2, 1) +/* v7M CLIDR bits */ +FIELD(V7M_CLIDR, CTYPE_ALL, 0, 21) +FIELD(V7M_CLIDR, LOUIS, 21, 3) +FIELD(V7M_CLIDR, LOC, 24, 3) +FIELD(V7M_CLIDR, LOUU, 27, 3) +FIELD(V7M_CLIDR, ICB, 30, 2) + +FIELD(V7M_CSSELR, IND, 0, 1) +FIELD(V7M_CSSELR, LEVEL, 1, 3) +/* We use the combination of InD and Level to index into cpu->ccsidr[]; + * define a mask for this and check that it doesn't permit running off + * the end of the array. + */ +FIELD(V7M_CSSELR, INDEX, 0, 4) + +QEMU_BUILD_BUG_ON(ARRAY_SIZE(((ARMCPU *)0)->ccsidr) <= R_V7M_CSSELR_INDEX_MASK); + /* If adding a feature bit which corresponds to a Linux ELF * HWCAP bit, remember to update the feature-bit-to-hwcap * mapping in linux-user/elfload.c:get_elf_hwcap(). @@ -XXX,XX +XXX,XX @@ static inline int arm_debug_target_el(CPUARMState *env) } } +static inline bool arm_v7m_csselr_razwi(ARMCPU *cpu) +{ + /* If all the CLIDR.Ctypem bits are 0 there are no caches, and + * CSSELR is RAZ/WI. + */ + return (cpu->clidr & R_V7M_CLIDR_CTYPE_ALL_MASK) != 0; +} + static inline bool aa64_generate_debug_exceptions(CPUARMState *env) { if (arm_is_secure(env)) { diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs) return cpu->id_isar4; case 0xd74: /* ISAR5. */ return cpu->id_isar5; + case 0xd78: /* CLIDR */ + return cpu->clidr; + case 0xd7c: /* CTR */ + return cpu->ctr; + case 0xd80: /* CSSIDR */ + { + int idx = cpu->env.v7m.csselr[attrs.secure] & R_V7M_CSSELR_INDEX_MASK; + return cpu->ccsidr[idx]; + } + case 0xd84: /* CSSELR */ + return cpu->env.v7m.csselr[attrs.secure]; /* TODO: Implement debug registers. */ case 0xd90: /* MPU_TYPE */ /* Unified MPU; if the MPU is not present this value is zero */ @@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, qemu_log_mask(LOG_UNIMP, "NVIC: Aux fault status registers unimplemented\n"); break; + case 0xd84: /* CSSELR */ + if (!arm_v7m_csselr_razwi(cpu)) { + cpu->env.v7m.csselr[attrs.secure] = value & R_V7M_CSSELR_INDEX_MASK; + } + break; case 0xd90: /* MPU_TYPE */ return; /* RO */ case 0xd94: /* MPU_CTRL */ diff --git a/target/arm/machine.c b/target/arm/machine.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/machine.c +++ b/target/arm/machine.c @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_m_faultmask_primask = { } }; +/* CSSELR is in a subsection because we didn't implement it previously. + * Migration from an old implementation will leave it at zero, which + * is OK since the only CPUs in the old implementation make the + * register RAZ/WI. + * Since there was no version of QEMU which implemented the CSSELR for + * just non-secure, we transfer both banks here rather than putting + * the secure banked version in the m-security subsection. + */ +static bool csselr_vmstate_validate(void *opaque, int version_id) +{ + ARMCPU *cpu = opaque; + + return cpu->env.v7m.csselr[M_REG_NS] <= R_V7M_CSSELR_INDEX_MASK + && cpu->env.v7m.csselr[M_REG_S] <= R_V7M_CSSELR_INDEX_MASK; +} + +static bool m_csselr_needed(void *opaque) +{ + ARMCPU *cpu = opaque; + + return !arm_v7m_csselr_razwi(cpu); +} + +static const VMStateDescription vmstate_m_csselr = { + .name = "cpu/m/csselr", + .version_id = 1, + .minimum_version_id = 1, + .needed = m_csselr_needed, + .fields = (VMStateField[]) { + VMSTATE_UINT32_ARRAY(env.v7m.csselr, ARMCPU, M_REG_NUM_BANKS), + VMSTATE_VALIDATE("CSSELR is valid", csselr_vmstate_validate), + VMSTATE_END_OF_LIST() + } +}; + static const VMStateDescription vmstate_m = { .name = "cpu/m", .version_id = 4, @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_m = { }, .subsections = (const VMStateDescription*[]) { &vmstate_m_faultmask_primask, + &vmstate_m_csselr, NULL } }; -- 2.16.1 We were previously making the system control register (SCR) just RAZ/WI. Although we don't implement the functionality this register controls, we should at least provide the state, including the banked state for v8M. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180209165810.6668-7-peter.maydell@linaro.org --- target/arm/cpu.h | 7 +++++++ hw/intc/armv7m_nvic.c | 12 ++++++++---- target/arm/machine.c | 12 ++++++++++++ 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ typedef struct CPUARMState { uint32_t aircr; /* only holds r/w state if security extn implemented */ uint32_t secure; /* Is CPU in Secure state? (not guest visible) */ uint32_t csselr[M_REG_NUM_BANKS]; + uint32_t scr[M_REG_NUM_BANKS]; } v7m; /* Information associated with an exception about to be taken: @@ -XXX,XX +XXX,XX @@ FIELD(V7M_CCR, STKALIGN, 9, 1) FIELD(V7M_CCR, DC, 16, 1) FIELD(V7M_CCR, IC, 17, 1) +/* V7M SCR bits */ +FIELD(V7M_SCR, SLEEPONEXIT, 1, 1) +FIELD(V7M_SCR, SLEEPDEEP, 2, 1) +FIELD(V7M_SCR, SLEEPDEEPS, 3, 1) +FIELD(V7M_SCR, SEVONPEND, 4, 1) + /* V7M AIRCR bits */ FIELD(V7M_AIRCR, VECTRESET, 0, 1) FIELD(V7M_AIRCR, VECTCLRACTIVE, 1, 1) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs) } return val; case 0xd10: /* System Control. */ - /* TODO: Implement SLEEPONEXIT. */ - return 0; + return cpu->env.v7m.scr[attrs.secure]; case 0xd14: /* Configuration Control. */ /* The BFHFNMIGN bit is the only non-banked bit; we * keep it in the non-secure copy of the register. @@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, } break; case 0xd10: /* System Control. */ - /* TODO: Implement control registers. */ - qemu_log_mask(LOG_UNIMP, "NVIC: SCR unimplemented\n"); + /* We don't implement deep-sleep so these bits are RAZ/WI. + * The other bits in the register are banked. + * QEMU's implementation ignores SEVONPEND and SLEEPONEXIT, which + * is architecturally permitted. + */ + value &= ~(R_V7M_SCR_SLEEPDEEP_MASK | R_V7M_SCR_SLEEPDEEPS_MASK); + cpu->env.v7m.scr[attrs.secure] = value; break; case 0xd14: /* Configuration Control. */ /* Enforce RAZ/WI on reserved and must-RAZ/WI bits */ diff --git a/target/arm/machine.c b/target/arm/machine.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/machine.c +++ b/target/arm/machine.c @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_m_csselr = { } }; +static const VMStateDescription vmstate_m_scr = { + .name = "cpu/m/scr", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT32(env.v7m.scr[M_REG_NS], ARMCPU), + VMSTATE_END_OF_LIST() + } +}; + static const VMStateDescription vmstate_m = { .name = "cpu/m", .version_id = 4, @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_m = { .subsections = (const VMStateDescription*[]) { &vmstate_m_faultmask_primask, &vmstate_m_csselr, + &vmstate_m_scr, NULL } }; @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_m_security = { VMSTATE_UINT32(env.sau.rnr, ARMCPU), VMSTATE_VALIDATE("SAU_RNR is valid", sau_rnr_vmstate_validate), VMSTATE_UINT32(env.sau.ctrl, ARMCPU), + VMSTATE_UINT32(env.v7m.scr[M_REG_S], ARMCPU), VMSTATE_END_OF_LIST() } }; -- 2.16.1 In commit 50f11062d4c896 we added support for MSR/MRS access to the NS banked special registers, but we forgot to implement the support for writing to CONTROL_NS. Correct the omission. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180209165810.6668-8-peter.maydell@linaro.org --- target/arm/helper.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val) } env->v7m.faultmask[M_REG_NS] = val & 1; return; + case 0x94: /* CONTROL_NS */ + if (!env->v7m.secure) { + return; + } + write_v7m_control_spsel_for_secstate(env, + val & R_V7M_CONTROL_SPSEL_MASK, + M_REG_NS); + env->v7m.control[M_REG_NS] &= ~R_V7M_CONTROL_NPRIV_MASK; + env->v7m.control[M_REG_NS] |= val & R_V7M_CONTROL_NPRIV_MASK; + return; case 0x98: /* SP_NS */ { /* This gives the non-secure SP selected based on whether we're -- 2.16.1 In many of the NVIC registers relating to interrupts, we have to convert from a byte offset within a register set into the number of the first interrupt which is affected. We were getting this wrong for: * reads of NVIC_ISPR<n>, NVIC_ISER<n>, NVIC_ICPR<n>, NVIC_ICER<n>, NVIC_IABR<n> -- in all these cases we were missing the "* 8" needed to convert from the byte offset to the interrupt number (since all these registers use one bit per interrupt) * writes of NVIC_IPR<n> had the opposite problem of a spurious "* 8" (since these registers use one byte per interrupt) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20180209165810.6668-9-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ static MemTxResult nvic_sysreg_read(void *opaque, hwaddr addr, /* fall through */ case 0x180 ... 0x1bf: /* NVIC Clear enable */ val = 0; - startvec = offset - 0x180 + NVIC_FIRST_IRQ; /* vector # */ + startvec = 8 * (offset - 0x180) + NVIC_FIRST_IRQ; /* vector # */ for (i = 0, end = size * 8; i < end && startvec + i < s->num_irq; i++) { if (s->vectors[startvec + i].enabled && @@ -XXX,XX +XXX,XX @@ static MemTxResult nvic_sysreg_read(void *opaque, hwaddr addr, /* fall through */ case 0x280 ... 0x2bf: /* NVIC Clear pend */ val = 0; - startvec = offset - 0x280 + NVIC_FIRST_IRQ; /* vector # */ + startvec = 8 * (offset - 0x280) + NVIC_FIRST_IRQ; /* vector # */ for (i = 0, end = size * 8; i < end && startvec + i < s->num_irq; i++) { if (s->vectors[startvec + i].pending && (attrs.secure || s->itns[startvec + i])) { @@ -XXX,XX +XXX,XX @@ static MemTxResult nvic_sysreg_read(void *opaque, hwaddr addr, break; case 0x300 ... 0x33f: /* NVIC Active */ val = 0; - startvec = offset - 0x300 + NVIC_FIRST_IRQ; /* vector # */ + startvec = 8 * (offset - 0x300) + NVIC_FIRST_IRQ; /* vector # */ for (i = 0, end = size * 8; i < end && startvec + i < s->num_irq; i++) { if (s->vectors[startvec + i].active && @@ -XXX,XX +XXX,XX @@ static MemTxResult nvic_sysreg_write(void *opaque, hwaddr addr, case 0x300 ... 0x33f: /* NVIC Active */ return MEMTX_OK; /* R/O */ case 0x400 ... 0x5ef: /* NVIC Priority */ - startvec = 8 * (offset - 0x400) + NVIC_FIRST_IRQ; /* vector # */ + startvec = (offset - 0x400) + NVIC_FIRST_IRQ; /* vector # */ for (i = 0; i < size && startvec + i < s->num_irq; i++) { if (attrs.secure || s->itns[startvec + i]) { -- 2.16.1 In commit commit 3b2e934463121 we added support for the AIRCR register holding state, but forgot to add it to the vmstate structs. Since it only holds r/w state if the security extension is implemented, we can just add it to vmstate_m_security. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180209165810.6668-10-peter.maydell@linaro.org --- target/arm/machine.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/target/arm/machine.c b/target/arm/machine.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/machine.c +++ b/target/arm/machine.c @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_m_security = { VMSTATE_VALIDATE("SAU_RNR is valid", sau_rnr_vmstate_validate), VMSTATE_UINT32(env.sau.ctrl, ARMCPU), VMSTATE_UINT32(env.v7m.scr[M_REG_S], ARMCPU), + /* AIRCR is not secure-only, but our implementation is R/O if the + * security extension is unimplemented, so we migrate it here. + */ + VMSTATE_UINT32(env.v7m.aircr, ARMCPU), VMSTATE_END_OF_LIST() } }; -- 2.16.1 In commit abc24d86cc0364f we accidentally broke migration of the stack pointer value for the mode (process, handler) the CPU is not currently running as. (The commit correctly removed the no-longer-used v7m.current_sp flag from the VMState but also deleted the still very much in use v7m.other_sp SP value field.) Add a subsection to migrate it again. (We don't need to care about trying to retain compatibility with pre-abc24d86cc0364f versions of QEMU, because that commit bumped the version_id and we've since bumped it again a couple of times.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180209165810.6668-11-peter.maydell@linaro.org --- target/arm/machine.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/target/arm/machine.c b/target/arm/machine.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/machine.c +++ b/target/arm/machine.c @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_m_scr = { } }; +static const VMStateDescription vmstate_m_other_sp = { + .name = "cpu/m/other-sp", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT32(env.v7m.other_sp, ARMCPU), + VMSTATE_END_OF_LIST() + } +}; + static const VMStateDescription vmstate_m = { .name = "cpu/m", .version_id = 4, @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_m = { &vmstate_m_faultmask_primask, &vmstate_m_csselr, &vmstate_m_scr, + &vmstate_m_other_sp, NULL } }; -- 2.16.1 The v8M architecture includes hardware support for enforcing stack pointer limits. We don't implement this behaviour yet, but provide the MSPLIM and PSPLIM stack pointer limit registers as reads-as-written, so that when we do implement the checks in future this won't break guest migration. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180209165810.6668-12-peter.maydell@linaro.org --- target/arm/cpu.h | 2 ++ target/arm/helper.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ target/arm/machine.c | 21 +++++++++++++++++++++ 3 files changed, 69 insertions(+) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ typedef struct CPUARMState { uint32_t secure; /* Is CPU in Secure state? (not guest visible) */ uint32_t csselr[M_REG_NUM_BANKS]; uint32_t scr[M_REG_NUM_BANKS]; + uint32_t msplim[M_REG_NUM_BANKS]; + uint32_t psplim[M_REG_NUM_BANKS]; } v7m; /* Information associated with an exception about to be taken: diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) return 0; } return env->v7m.other_ss_psp; + case 0x8a: /* MSPLIM_NS */ + if (!env->v7m.secure) { + return 0; + } + return env->v7m.msplim[M_REG_NS]; + case 0x8b: /* PSPLIM_NS */ + if (!env->v7m.secure) { + return 0; + } + return env->v7m.psplim[M_REG_NS]; case 0x90: /* PRIMASK_NS */ if (!env->v7m.secure) { return 0; @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) return v7m_using_psp(env) ? env->v7m.other_sp : env->regs[13]; case 9: /* PSP */ return v7m_using_psp(env) ? env->regs[13] : env->v7m.other_sp; + case 10: /* MSPLIM */ + if (!arm_feature(env, ARM_FEATURE_V8)) { + goto bad_reg; + } + return env->v7m.msplim[env->v7m.secure]; + case 11: /* PSPLIM */ + if (!arm_feature(env, ARM_FEATURE_V8)) { + goto bad_reg; + } + return env->v7m.psplim[env->v7m.secure]; case 16: /* PRIMASK */ return env->v7m.primask[env->v7m.secure]; case 17: /* BASEPRI */ @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) case 19: /* FAULTMASK */ return env->v7m.faultmask[env->v7m.secure]; default: + bad_reg: qemu_log_mask(LOG_GUEST_ERROR, "Attempt to read unknown special" " register %d\n", reg); return 0; @@ -XXX,XX +XXX,XX @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val) } env->v7m.other_ss_psp = val; return; + case 0x8a: /* MSPLIM_NS */ + if (!env->v7m.secure) { + return; + } + env->v7m.msplim[M_REG_NS] = val & ~7; + return; + case 0x8b: /* PSPLIM_NS */ + if (!env->v7m.secure) { + return; + } + env->v7m.psplim[M_REG_NS] = val & ~7; + return; case 0x90: /* PRIMASK_NS */ if (!env->v7m.secure) { return; @@ -XXX,XX +XXX,XX @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val) env->v7m.other_sp = val; } break; + case 10: /* MSPLIM */ + if (!arm_feature(env, ARM_FEATURE_V8)) { + goto bad_reg; + } + env->v7m.msplim[env->v7m.secure] = val & ~7; + break; + case 11: /* PSPLIM */ + if (!arm_feature(env, ARM_FEATURE_V8)) { + goto bad_reg; + } + env->v7m.psplim[env->v7m.secure] = val & ~7; + break; case 16: /* PRIMASK */ env->v7m.primask[env->v7m.secure] = val & 1; break; @@ -XXX,XX +XXX,XX @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val) env->v7m.control[env->v7m.secure] |= val & R_V7M_CONTROL_NPRIV_MASK; break; default: + bad_reg: qemu_log_mask(LOG_GUEST_ERROR, "Attempt to write unknown special" " register %d\n", reg); return; diff --git a/target/arm/machine.c b/target/arm/machine.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/machine.c +++ b/target/arm/machine.c @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_m_other_sp = { } }; +static bool m_v8m_needed(void *opaque) +{ + ARMCPU *cpu = opaque; + CPUARMState *env = &cpu->env; + + return arm_feature(env, ARM_FEATURE_M) && arm_feature(env, ARM_FEATURE_V8); +} + +static const VMStateDescription vmstate_m_v8m = { + .name = "cpu/m/v8m", + .version_id = 1, + .minimum_version_id = 1, + .needed = m_v8m_needed, + .fields = (VMStateField[]) { + VMSTATE_UINT32_ARRAY(env.v7m.msplim, ARMCPU, M_REG_NUM_BANKS), + VMSTATE_UINT32_ARRAY(env.v7m.psplim, ARMCPU, M_REG_NUM_BANKS), + VMSTATE_END_OF_LIST() + } +}; + static const VMStateDescription vmstate_m = { .name = "cpu/m", .version_id = 4, @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_m = { &vmstate_m_csselr, &vmstate_m_scr, &vmstate_m_other_sp, + &vmstate_m_v8m, NULL } }; -- 2.16.1 target-arm queue: mostly smallish stuff. I expect to send out another pullreq at the end of this week, but since this is up to 32 patches already I'd rather send it out now than accumulate a monster sized patchset. thanks -- PMM The following changes since commit 0ab4c574a55448a37b9f616259b82950742c9427: Merge remote-tracking branch 'remotes/kraxel/tags/ui-20180626-pull-request' into staging (2018-06-26 16:44:57 +0100) are available in the Git repository at: git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20180626 for you to fetch changes up to 9b945a9ee36a34eaeca412ef9ef35fbfe33c2c85: aspeed/timer: use the APB frequency from the SCU (2018-06-26 17:50:42 +0100) ---------------------------------------------------------------- target-arm queue: * aspeed: set APB clocks correctly (fixes slowdown on palmetto) * smmuv3: cache config data and TLB entries * v7m/v8m: support read/write from MPU regions smaller than 1K * various: clean up logging/debug messages * xilinx_spips: Make dma transactions as per dma_burst_size ---------------------------------------------------------------- Cédric Le Goater (6): aspeed/smc: fix dummy cycles count when in dual IO mode aspeed/smc: fix HW strapping aspeed/smc: rename aspeed_smc_flash_send_addr() to aspeed_smc_flash_setup() aspeed/scu: introduce clock frequencies aspeed: initialize the SCU controller first aspeed/timer: use the APB frequency from the SCU Eric Auger (3): hw/arm/smmuv3: Cache/invalidate config data hw/arm/smmuv3: IOTLB emulation hw/arm/smmuv3: Add notifications on invalidation Jia He (1): hw/arm/smmuv3: Fix translate error handling Joel Stanley (1): MAINTAINERS: Add ASPEED BMCs Peter Maydell (3): tcg: Support MMU protection regions smaller than TARGET_PAGE_SIZE target/arm: Set page (region) size in get_phys_addr_pmsav7() target/arm: Handle small regions in get_phys_addr_pmsav8() Philippe Mathieu-Daudé (17): MAINTAINERS: Adopt the Gumstix computers-on-module machines hw/input/pckbd: Use qemu_log_mask(GUEST_ERROR) instead of fprintf hw/input/tsc2005: Use qemu_log_mask(GUEST_ERROR) instead of fprintf hw/dma/omap_dma: Use qemu_log_mask(UNIMP) instead of printf hw/dma/omap_dma: Use qemu_log_mask(GUEST_ERROR) instead of fprintf hw/ssi/omap_spi: Use qemu_log_mask(GUEST_ERROR) instead of fprintf hw/sd/omap_mmc: Use qemu_log_mask(UNIMP) instead of printf hw/i2c/omap_i2c: Use qemu_log_mask(UNIMP) instead of fprintf hw/arm/omap1: Use qemu_log_mask(GUEST_ERROR) instead of fprintf hw/arm/omap: Use qemu_log_mask(GUEST_ERROR) instead of fprintf hw/arm/stellaris: Use qemu_log_mask(UNIMP) instead of fprintf hw/net/stellaris_enet: Fix a typo hw/net/stellaris_enet: Use qemu_log_mask(GUEST_ERROR) instead of hw_error hw/net/smc91c111: Use qemu_log_mask(GUEST_ERROR) instead of hw_error hw/net/smc91c111: Use qemu_log_mask(UNIMP) instead of fprintf hw/arm/stellaris: Fix gptm_write() error message hw/arm/stellaris: Use HWADDR_PRIx to display register address Sai Pavan Boddu (1): xilinx_spips: Make dma transactions as per dma_burst_size accel/tcg/softmmu_template.h | 24 ++- hw/arm/smmuv3-internal.h | 12 +- include/exec/cpu-all.h | 5 +- include/hw/arm/omap.h | 30 +-- include/hw/arm/smmu-common.h | 24 +++ include/hw/arm/smmuv3.h | 1 + include/hw/misc/aspeed_scu.h | 70 ++++++- include/hw/ssi/xilinx_spips.h | 5 +- include/hw/timer/aspeed_timer.h | 4 + accel/tcg/cputlb.c | 131 +++++++++++-- hw/arm/aspeed_soc.c | 42 ++-- hw/arm/omap1.c | 18 +- hw/arm/smmu-common.c | 118 ++++++++++- hw/arm/smmuv3.c | 420 ++++++++++++++++++++++++++++++++++++---- hw/arm/stellaris.c | 8 +- hw/dma/omap_dma.c | 70 ++++--- hw/i2c/omap_i2c.c | 20 +- hw/input/pckbd.c | 4 +- hw/input/tsc2005.c | 13 +- hw/misc/aspeed_scu.c | 106 ++++++++++ hw/net/smc91c111.c | 21 +- hw/net/stellaris_enet.c | 11 +- hw/sd/omap_mmc.c | 13 +- hw/ssi/aspeed_smc.c | 48 ++--- hw/ssi/omap_spi.c | 15 +- hw/ssi/xilinx_spips.c | 23 ++- hw/timer/aspeed_timer.c | 19 +- target/arm/helper.c | 115 +++++++---- MAINTAINERS | 14 +- hw/arm/trace-events | 27 ++- 30 files changed, 1176 insertions(+), 255 deletions(-) From: Cédric Le Goater <clg@kaod.org> When configured in dual I/O mode, address and data are sent in dual mode, including the dummy byte cycles in between. Adapt the count to the IO setting. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Andrew Jeffery <andrew@aj.id.au> Message-id: 20180612065716.10587-2-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/ssi/aspeed_smc.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -XXX,XX +XXX,XX @@ /* CEx Control Register */ #define R_CTRL0 (0x10 / 4) +#define CTRL_IO_DUAL_DATA (1 << 29) +#define CTRL_IO_DUAL_ADDR_DATA (1 << 28) /* Includes dummies */ #define CTRL_CMD_SHIFT 16 #define CTRL_CMD_MASK 0xff #define CTRL_DUMMY_HIGH_SHIFT 14 @@ -XXX,XX +XXX,XX @@ static int aspeed_smc_flash_dummies(const AspeedSMCFlash *fl) uint32_t r_ctrl0 = s->regs[s->r_ctrl0 + fl->id]; uint32_t dummy_high = (r_ctrl0 >> CTRL_DUMMY_HIGH_SHIFT) & 0x1; uint32_t dummy_low = (r_ctrl0 >> CTRL_DUMMY_LOW_SHIFT) & 0x3; + uint32_t dummies = ((dummy_high << 2) | dummy_low) * 8; - return ((dummy_high << 2) | dummy_low) * 8; + if (r_ctrl0 & CTRL_IO_DUAL_ADDR_DATA) { + dummies /= 2; + } + + return dummies; } static void aspeed_smc_flash_send_addr(AspeedSMCFlash *fl, uint32_t addr) -- 2.17.1 From: Cédric Le Goater <clg@kaod.org> Only the flash type is strapped by HW. The 4BYTE mode is set by firmware when the flash device is detected. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Andrew Jeffery <andrew@aj.id.au> Message-id: 20180612065716.10587-3-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/ssi/aspeed_smc.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -XXX,XX +XXX,XX @@ static void aspeed_smc_reset(DeviceState *d) aspeed_smc_segment_to_reg(&s->ctrl->segments[i]); } - /* HW strapping for AST2500 FMC controllers */ + /* HW strapping flash type for FMC controllers */ if (s->ctrl->segments == aspeed_segments_ast2500_fmc) { /* flash type is fixed to SPI for CE0 and CE1 */ s->regs[s->r_conf] |= (CONF_FLASH_TYPE_SPI << CONF_FLASH_TYPE0); s->regs[s->r_conf] |= (CONF_FLASH_TYPE_SPI << CONF_FLASH_TYPE1); - - /* 4BYTE mode is autodetected for CE0. Let's force it to 1 for - * now */ - s->regs[s->r_ce_ctrl] |= (1 << (CTRL_EXTENDED0)); } /* HW strapping for AST2400 FMC controllers (SCU70). Let's use the * configuration of the palmetto-bmc machine */ if (s->ctrl->segments == aspeed_segments_fmc) { s->regs[s->r_conf] |= (CONF_FLASH_TYPE_SPI << CONF_FLASH_TYPE0); - - s->regs[s->r_ce_ctrl] |= (1 << (CTRL_EXTENDED0)); } } -- 2.17.1 From: Cédric Le Goater <clg@kaod.org> Also handle the fake transfers for dummy bytes in this setup routine. It will be useful when we activate MMIO execution. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Andrew Jeffery <andrew@aj.id.au> Message-id: 20180612065716.10587-4-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/ssi/aspeed_smc.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -XXX,XX +XXX,XX @@ static int aspeed_smc_flash_dummies(const AspeedSMCFlash *fl) return dummies; } -static void aspeed_smc_flash_send_addr(AspeedSMCFlash *fl, uint32_t addr) +static void aspeed_smc_flash_setup(AspeedSMCFlash *fl, uint32_t addr) { const AspeedSMCState *s = fl->controller; uint8_t cmd = aspeed_smc_flash_cmd(fl); + int i; /* Flash access can not exceed CS segment */ addr = aspeed_smc_check_segment_addr(fl, addr); @@ -XXX,XX +XXX,XX @@ static void aspeed_smc_flash_send_addr(AspeedSMCFlash *fl, uint32_t addr) ssi_transfer(s->spi, (addr >> 16) & 0xff); ssi_transfer(s->spi, (addr >> 8) & 0xff); ssi_transfer(s->spi, (addr & 0xff)); + + /* + * Use fake transfers to model dummy bytes. The value should + * be configured to some non-zero value in fast read mode and + * zero in read mode. But, as the HW allows inconsistent + * settings, let's check for fast read mode. + */ + if (aspeed_smc_flash_mode(fl) == CTRL_FREADMODE) { + for (i = 0; i < aspeed_smc_flash_dummies(fl); i++) { + ssi_transfer(fl->controller->spi, 0xFF); + } + } } static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size) @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size) case CTRL_READMODE: case CTRL_FREADMODE: aspeed_smc_flash_select(fl); - aspeed_smc_flash_send_addr(fl, addr); - - /* - * Use fake transfers to model dummy bytes. The value should - * be configured to some non-zero value in fast read mode and - * zero in read mode. But, as the HW allows inconsistent - * settings, let's check for fast read mode. - */ - if (aspeed_smc_flash_mode(fl) == CTRL_FREADMODE) { - for (i = 0; i < aspeed_smc_flash_dummies(fl); i++) { - ssi_transfer(fl->controller->spi, 0xFF); - } - } + aspeed_smc_flash_setup(fl, addr); for (i = 0; i < size; i++) { ret |= ssi_transfer(s->spi, 0x0) << (8 * i); @@ -XXX,XX +XXX,XX @@ static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data, break; case CTRL_WRITEMODE: aspeed_smc_flash_select(fl); - aspeed_smc_flash_send_addr(fl, addr); + aspeed_smc_flash_setup(fl, addr); for (i = 0; i < size; i++) { ssi_transfer(s->spi, (data >> (8 * i)) & 0xff); -- 2.17.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> These COMs are hard to find, and the companie dropped the support few years ago. Per the "Gumstix Product Changes, Known Issues, and EOL" pdf: - Phasing out: PXA270-based Verdex product line September 2012 - Phasing out: PXA255-based Basix & Connex September 2009 However there are still booting SD card image availables, very convenient to stress test the QEMU SD card implementation. Therefore I volunteer to keep an eye on this file, while it is useful for testing. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-id: 20180606144706.29732-1-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- MAINTAINERS | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index XXXXXXX..XXXXXXX 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -XXX,XX +XXX,XX @@ F: include/hw/arm/digic.h F: hw/*/digic* Gumstix +M: Philippe Mathieu-Daudé <f4bug@amsat.org> L: qemu-devel@nongnu.org L: qemu-arm@nongnu.org -S: Orphan +S: Odd Fixes F: hw/arm/gumstix.c i.MX31 -- 2.17.1 From: Sai Pavan Boddu <saipava@xilinx.com> Qspi dma has a burst length of 64 bytes, So limit the transactions w.r.t dma-burst-size property. Signed-off-by: Sai Pavan Boddu <saipava@xilinx.com> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Message-id: 1529660880-30376-1-git-send-email-sai.pavan.boddu@xilinx.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/ssi/xilinx_spips.h | 5 ++++- hw/ssi/xilinx_spips.c | 23 ++++++++++++++++++++--- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/include/hw/ssi/xilinx_spips.h b/include/hw/ssi/xilinx_spips.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/ssi/xilinx_spips.h +++ b/include/hw/ssi/xilinx_spips.h @@ -XXX,XX +XXX,XX @@ typedef struct XilinxSPIPS XilinxSPIPS; /* Bite off 4k chunks at a time */ #define LQSPI_CACHE_SIZE 1024 +#define QSPI_DMA_MAX_BURST_SIZE 2048 + typedef enum { READ = 0x3, READ_4 = 0x13, FAST_READ = 0xb, FAST_READ_4 = 0x0c, @@ -XXX,XX +XXX,XX @@ typedef struct { XilinxQSPIPS parent_obj; StreamSlave *dma; - uint8_t dma_buf[4]; int gqspi_irqline; uint32_t regs[XLNX_ZYNQMP_SPIPS_R_MAX]; @@ -XXX,XX +XXX,XX @@ typedef struct { uint8_t rx_fifo_g_align; uint8_t tx_fifo_g_align; bool man_start_com_g; + uint32_t dma_burst_size; + uint8_t dma_buf[QSPI_DMA_MAX_BURST_SIZE]; } XlnxZynqMPQSPIPS; typedef struct XilinxSPIPSClass { diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ssi/xilinx_spips.c +++ b/hw/ssi/xilinx_spips.c @@ -XXX,XX +XXX,XX @@ static void xlnx_zynqmp_qspips_notify(void *opaque) { size_t ret; uint32_t num; - const void *rxd = pop_buf(recv_fifo, 4, &num); + const void *rxd; + int len; + + len = recv_fifo->num >= rq->dma_burst_size ? rq->dma_burst_size : + recv_fifo->num; + rxd = pop_buf(recv_fifo, len, &num); memcpy(rq->dma_buf, rxd, num); - ret = stream_push(rq->dma, rq->dma_buf, 4); - assert(ret == 4); + ret = stream_push(rq->dma, rq->dma_buf, num); + assert(ret == num); xlnx_zynqmp_qspips_check_flush(rq); } } @@ -XXX,XX +XXX,XX @@ static void xlnx_zynqmp_qspips_realize(DeviceState *dev, Error **errp) XlnxZynqMPQSPIPS *s = XLNX_ZYNQMP_QSPIPS(dev); XilinxSPIPSClass *xsc = XILINX_SPIPS_GET_CLASS(s); + if (s->dma_burst_size > QSPI_DMA_MAX_BURST_SIZE) { + error_setg(errp, + "qspi dma burst size %u exceeds maximum limit %d", + s->dma_burst_size, QSPI_DMA_MAX_BURST_SIZE); + return; + } xilinx_qspips_realize(dev, errp); fifo8_create(&s->rx_fifo_g, xsc->rx_fifo_size); fifo8_create(&s->tx_fifo_g, xsc->tx_fifo_size); @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_xlnx_zynqmp_qspips = { } }; +static Property xilinx_zynqmp_qspips_properties[] = { + DEFINE_PROP_UINT32("dma-burst-size", XlnxZynqMPQSPIPS, dma_burst_size, 64), + DEFINE_PROP_END_OF_LIST(), +}; + static Property xilinx_qspips_properties[] = { /* We had to turn this off for 2.10 as it is not compatible with migration. * It can be enabled but will prevent the device to be migrated. @@ -XXX,XX +XXX,XX @@ static void xlnx_zynqmp_qspips_class_init(ObjectClass *klass, void * data) dc->realize = xlnx_zynqmp_qspips_realize; dc->reset = xlnx_zynqmp_qspips_reset; dc->vmsd = &vmstate_xlnx_zynqmp_qspips; + dc->props = xilinx_zynqmp_qspips_properties; xsc->reg_ops = &xlnx_zynqmp_qspips_ops; xsc->rx_fifo_size = RXFF_A_Q; xsc->tx_fifo_size = TXFF_A_Q; -- 2.17.1 From: Joel Stanley <joel@jms.id.au> This adds Cedric as the maintainer, with Andrew and I as reviewers, for the ASPEED boards and the peripherals we have developed. Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Andrew Jeffery <andrew@aj.id.au> Acked-by: Cédric Le Goater <clg@kaod.org> Signed-off-by: Joel Stanley <joel@jms.id.au> Message-id: 20180625140055.32223-1-joel@jms.id.au Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- MAINTAINERS | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index XXXXXXX..XXXXXXX 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -XXX,XX +XXX,XX @@ M: Subbaraya Sundeep <sundeep.lkml@gmail.com> S: Maintained F: hw/arm/msf2-som.c +ASPEED BMCs +M: Cédric Le Goater <clg@kaod.org> +R: Andrew Jeffery <andrew@aj.id.au> +R: Joel Stanley <joel@jms.id.au> +L: qemu-arm@nongnu.org +S: Maintained +F: hw/*/*aspeed* +F: include/hw/*/*aspeed* +F: hw/net/ftgmac100.c +F: include/hw/net/ftgmac100.h + CRIS Machines ------------- Axis Dev88 -- 2.17.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-id: 20180624040609.17572-2-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/input/pckbd.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c index XXXXXXX..XXXXXXX 100644 --- a/hw/input/pckbd.c +++ b/hw/input/pckbd.c @@ -XXX,XX +XXX,XX @@ * THE SOFTWARE. */ #include "qemu/osdep.h" +#include "qemu/log.h" #include "hw/hw.h" #include "hw/isa/isa.h" #include "hw/i386/pc.h" @@ -XXX,XX +XXX,XX @@ static void kbd_write_command(void *opaque, hwaddr addr, /* ignore that */ break; default: - fprintf(stderr, "qemu: unsupported keyboard cmd=0x%02x\n", (int)val); + qemu_log_mask(LOG_GUEST_ERROR, + "unsupported keyboard cmd=0x%02" PRIx64 "\n", val); break; } } -- 2.17.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-id: 20180624040609.17572-3-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/input/tsc2005.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/hw/input/tsc2005.c b/hw/input/tsc2005.c index XXXXXXX..XXXXXXX 100644 --- a/hw/input/tsc2005.c +++ b/hw/input/tsc2005.c @@ -XXX,XX +XXX,XX @@ */ #include "qemu/osdep.h" +#include "qemu/log.h" #include "hw/hw.h" #include "qemu/timer.h" #include "ui/console.h" @@ -XXX,XX +XXX,XX @@ static void tsc2005_write(TSC2005State *s, int reg, uint16_t data) } s->nextprecision = (data >> 13) & 1; s->timing[0] = data & 0x1fff; - if ((s->timing[0] >> 11) == 3) - fprintf(stderr, "%s: illegal conversion clock setting\n", - __func__); + if ((s->timing[0] >> 11) == 3) { + qemu_log_mask(LOG_GUEST_ERROR, + "tsc2005_write: illegal conversion clock setting\n"); + } break; case 0xd: /* CFR1 */ s->timing[1] = data & 0xf07; @@ -XXX,XX +XXX,XX @@ static void tsc2005_write(TSC2005State *s, int reg, uint16_t data) break; default: - fprintf(stderr, "%s: write into read-only register %x\n", - __func__, reg); + qemu_log_mask(LOG_GUEST_ERROR, + "%s: write into read-only register 0x%x\n", + __func__, reg); } } -- 2.17.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-id: 20180624040609.17572-4-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/dma/omap_dma.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/dma/omap_dma.c b/hw/dma/omap_dma.c index XXXXXXX..XXXXXXX 100644 --- a/hw/dma/omap_dma.c +++ b/hw/dma/omap_dma.c @@ -XXX,XX +XXX,XX @@ * with this program; if not, see <http://www.gnu.org/licenses/>. */ #include "qemu/osdep.h" +#include "qemu/log.h" #include "qemu-common.h" #include "qemu/timer.h" #include "hw/arm/omap.h" @@ -XXX,XX +XXX,XX @@ static int omap_dma_sys_read(struct omap_dma_s *s, int offset, case 0x480: /* DMA_PCh0_SR */ case 0x482: /* DMA_PCh1_SR */ case 0x4c0: /* DMA_PChD_SR_0 */ - printf("%s: Physical Channel Status Registers not implemented.\n", - __func__); + qemu_log_mask(LOG_UNIMP, + "%s: Physical Channel Status Registers not implemented\n", + __func__); *ret = 0xff; break; -- 2.17.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-id: 20180624040609.17572-5-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/dma/omap_dma.c | 64 +++++++++++++++++++++++++++++------------------ 1 file changed, 40 insertions(+), 24 deletions(-) diff --git a/hw/dma/omap_dma.c b/hw/dma/omap_dma.c index XXXXXXX..XXXXXXX 100644 --- a/hw/dma/omap_dma.c +++ b/hw/dma/omap_dma.c @@ -XXX,XX +XXX,XX @@ static int omap_dma_ch_reg_write(struct omap_dma_s *s, ch->burst[0] = (value & 0x0180) >> 7; ch->pack[0] = (value & 0x0040) >> 6; ch->port[0] = (enum omap_dma_port) ((value & 0x003c) >> 2); - if (ch->port[0] >= __omap_dma_port_last) - printf("%s: invalid DMA port %i\n", __func__, - ch->port[0]); - if (ch->port[1] >= __omap_dma_port_last) - printf("%s: invalid DMA port %i\n", __func__, - ch->port[1]); + if (ch->port[0] >= __omap_dma_port_last) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid DMA port %i\n", + __func__, ch->port[0]); + } + if (ch->port[1] >= __omap_dma_port_last) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid DMA port %i\n", + __func__, ch->port[1]); + } ch->data_type = 1 << (value & 3); if ((value & 3) == 3) { - printf("%s: bad data_type for DMA channel\n", __func__); + qemu_log_mask(LOG_GUEST_ERROR, + "%s: bad data_type for DMA channel\n", __func__); ch->data_type >>= 1; } break; @@ -XXX,XX +XXX,XX @@ static void omap_dma4_write(void *opaque, hwaddr addr, if (value & 2) /* SOFTRESET */ omap_dma_reset(s->dma); s->ocp = value & 0x3321; - if (((s->ocp >> 12) & 3) == 3) /* MIDLEMODE */ - fprintf(stderr, "%s: invalid DMA power mode\n", __func__); + if (((s->ocp >> 12) & 3) == 3) { /* MIDLEMODE */ + qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid DMA power mode\n", + __func__); + } return; case 0x78: /* DMA4_GCR */ s->gcr = value & 0x00ff00ff; - if ((value & 0xff) == 0x00) /* MAX_CHANNEL_FIFO_DEPTH */ - fprintf(stderr, "%s: wrong FIFO depth in GCR\n", __func__); + if ((value & 0xff) == 0x00) { /* MAX_CHANNEL_FIFO_DEPTH */ + qemu_log_mask(LOG_GUEST_ERROR, "%s: wrong FIFO depth in GCR\n", + __func__); + } return; case 0x80 ... 0xfff: @@ -XXX,XX +XXX,XX @@ static void omap_dma4_write(void *opaque, hwaddr addr, case 0x00: /* DMA4_CCR */ ch->buf_disable = (value >> 25) & 1; ch->src_sync = (value >> 24) & 1; /* XXX For CamDMA must be 1 */ - if (ch->buf_disable && !ch->src_sync) - fprintf(stderr, "%s: Buffering disable is not allowed in " - "destination synchronised mode\n", __func__); + if (ch->buf_disable && !ch->src_sync) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: Buffering disable is not allowed in " + "destination synchronised mode\n", __func__); + } ch->prefetch = (value >> 23) & 1; ch->bs = (value >> 18) & 1; ch->transparent_copy = (value >> 17) & 1; @@ -XXX,XX +XXX,XX @@ static void omap_dma4_write(void *opaque, hwaddr addr, ch->suspend = (value & 0x0100) >> 8; ch->priority = (value & 0x0040) >> 6; ch->fs = (value & 0x0020) >> 5; - if (ch->fs && ch->bs && ch->mode[0] && ch->mode[1]) - fprintf(stderr, "%s: For a packet transfer at least one port " - "must be constant-addressed\n", __func__); + if (ch->fs && ch->bs && ch->mode[0] && ch->mode[1]) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: For a packet transfer at least one port " + "must be constant-addressed\n", __func__); + } ch->sync = (value & 0x001f) | ((value >> 14) & 0x0060); /* XXX must be 0x01 for CamDMA */ @@ -XXX,XX +XXX,XX @@ static void omap_dma4_write(void *opaque, hwaddr addr, ch->endian_lock[0] =(value >> 20) & 1; ch->endian[1] =(value >> 19) & 1; ch->endian_lock[1] =(value >> 18) & 1; - if (ch->endian[0] != ch->endian[1]) - fprintf(stderr, "%s: DMA endianness conversion enable attempt\n", - __func__); + if (ch->endian[0] != ch->endian[1]) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: DMA endianness conversion enable attempt\n", + __func__); + } ch->write_mode = (value >> 16) & 3; ch->burst[1] = (value & 0xc000) >> 14; ch->pack[1] = (value & 0x2000) >> 13; @@ -XXX,XX +XXX,XX @@ static void omap_dma4_write(void *opaque, hwaddr addr, ch->burst[0] = (value & 0x0180) >> 7; ch->pack[0] = (value & 0x0040) >> 6; ch->translate[0] = (value & 0x003c) >> 2; - if (ch->translate[0] | ch->translate[1]) - fprintf(stderr, "%s: bad MReqAddressTranslate sideband signal\n", - __func__); + if (ch->translate[0] | ch->translate[1]) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: bad MReqAddressTranslate sideband signal\n", + __func__); + } ch->data_type = 1 << (value & 3); if ((value & 3) == 3) { - printf("%s: bad data_type for DMA channel\n", __func__); + qemu_log_mask(LOG_GUEST_ERROR, + "%s: bad data_type for DMA channel\n", __func__); ch->data_type >>= 1; } break; -- 2.17.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-id: 20180624040609.17572-6-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/ssi/omap_spi.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/hw/ssi/omap_spi.c b/hw/ssi/omap_spi.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ssi/omap_spi.c +++ b/hw/ssi/omap_spi.c @@ -XXX,XX +XXX,XX @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ #include "qemu/osdep.h" +#include "qemu/log.h" #include "hw/hw.h" #include "hw/arm/omap.h" @@ -XXX,XX +XXX,XX @@ static void omap_mcspi_write(void *opaque, hwaddr addr, case 0x2c: /* MCSPI_CHCONF */ if ((value ^ s->ch[ch].config) & (3 << 14)) /* DMAR | DMAW */ omap_mcspi_dmarequest_update(s->ch + ch); - if (((value >> 12) & 3) == 3) /* TRM */ - fprintf(stderr, "%s: invalid TRM value (3)\n", __func__); - if (((value >> 7) & 0x1f) < 3) /* WL */ - fprintf(stderr, "%s: invalid WL value (%" PRIx64 ")\n", - __func__, (value >> 7) & 0x1f); + if (((value >> 12) & 3) == 3) { /* TRM */ + qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid TRM value (3)\n", + __func__); + } + if (((value >> 7) & 0x1f) < 3) { /* WL */ + qemu_log_mask(LOG_GUEST_ERROR, + "%s: invalid WL value (%" PRIx64 ")\n", + __func__, (value >> 7) & 0x1f); + } s->ch[ch].config = value & 0x7fffff; break; -- 2.17.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-id: 20180624040609.17572-7-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/sd/omap_mmc.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/sd/omap_mmc.c +++ b/hw/sd/omap_mmc.c @@ -XXX,XX +XXX,XX @@ * with this program; if not, see <http://www.gnu.org/licenses/>. */ #include "qemu/osdep.h" +#include "qemu/log.h" #include "hw/hw.h" #include "hw/arm/omap.h" #include "hw/sd/sd.h" @@ -XXX,XX +XXX,XX @@ static void omap_mmc_write(void *opaque, hwaddr offset, s->enable = (value >> 11) & 1; s->be = (value >> 10) & 1; s->clkdiv = (value >> 0) & (s->rev >= 2 ? 0x3ff : 0xff); - if (s->mode != 0) - printf("SD mode %i unimplemented!\n", s->mode); - if (s->be != 0) - printf("SD FIFO byte sex unimplemented!\n"); + if (s->mode != 0) { + qemu_log_mask(LOG_UNIMP, + "omap_mmc_wr: mode #%i unimplemented\n", s->mode); + } + if (s->be != 0) { + qemu_log_mask(LOG_UNIMP, + "omap_mmc_wr: Big Endian not implemented\n"); + } if (s->dw != 0 && s->lines < 4) printf("4-bit SD bus enabled\n"); if (!s->enable) -- 2.17.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-id: 20180624040609.17572-8-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/i2c/omap_i2c.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/hw/i2c/omap_i2c.c b/hw/i2c/omap_i2c.c index XXXXXXX..XXXXXXX 100644 --- a/hw/i2c/omap_i2c.c +++ b/hw/i2c/omap_i2c.c @@ -XXX,XX +XXX,XX @@ * with this program; if not, see <http://www.gnu.org/licenses/>. */ #include "qemu/osdep.h" +#include "qemu/log.h" #include "hw/hw.h" #include "hw/i2c/i2c.h" #include "hw/arm/omap.h" @@ -XXX,XX +XXX,XX @@ static void omap_i2c_write(void *opaque, hwaddr addr, } break; } - if ((value & (1 << 15)) && !(value & (1 << 10))) { /* MST */ - fprintf(stderr, "%s: I^2C slave mode not supported\n", - __func__); + if ((value & (1 << 15)) && !(value & (1 << 10))) { /* MST */ + qemu_log_mask(LOG_UNIMP, "%s: I^2C slave mode not supported\n", + __func__); break; } - if ((value & (1 << 15)) && value & (1 << 8)) { /* XA */ - fprintf(stderr, "%s: 10-bit addressing mode not supported\n", - __func__); + if ((value & (1 << 15)) && value & (1 << 8)) { /* XA */ + qemu_log_mask(LOG_UNIMP, + "%s: 10-bit addressing mode not supported\n", + __func__); break; } if ((value & (1 << 15)) && value & (1 << 0)) { /* STT */ @@ -XXX,XX +XXX,XX @@ static void omap_i2c_write(void *opaque, hwaddr addr, s->stat |= 0x3f; omap_i2c_interrupts_update(s); } - if (value & (1 << 15)) /* ST_EN */ - fprintf(stderr, "%s: System Test not supported\n", __func__); + if (value & (1 << 15)) { /* ST_EN */ + qemu_log_mask(LOG_UNIMP, + "%s: System Test not supported\n", __func__); + } break; default: -- 2.17.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> TCMI_VERBOSE is no more used, drop the OMAP_8/16/32B_REG macros. Suggested-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-id: 20180624040609.17572-9-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/omap.h | 18 ------------------ hw/arm/omap1.c | 18 ++++++++++++------ 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/include/hw/arm/omap.h b/include/hw/arm/omap.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/omap.h +++ b/include/hw/arm/omap.h @@ -XXX,XX +XXX,XX @@ enum { #define OMAP_GPIOSW_INVERTED 0x0001 #define OMAP_GPIOSW_OUTPUT 0x0002 -# define TCMI_VERBOSE 1 - -# ifdef TCMI_VERBOSE -# define OMAP_8B_REG(paddr) \ - fprintf(stderr, "%s: 8-bit register " OMAP_FMT_plx "\n", \ - __func__, paddr) -# define OMAP_16B_REG(paddr) \ - fprintf(stderr, "%s: 16-bit register " OMAP_FMT_plx "\n", \ - __func__, paddr) -# define OMAP_32B_REG(paddr) \ - fprintf(stderr, "%s: 32-bit register " OMAP_FMT_plx "\n", \ - __func__, paddr) -# else -# define OMAP_8B_REG(paddr) -# define OMAP_16B_REG(paddr) -# define OMAP_32B_REG(paddr) -# endif - # define OMAP_MPUI_REG_MASK 0x000007ff #endif /* hw_omap_h */ diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/omap1.c +++ b/hw/arm/omap1.c @@ -XXX,XX +XXX,XX @@ #include "qemu/cutils.h" #include "qemu/bcd.h" +static inline void omap_log_badwidth(const char *funcname, hwaddr addr, int sz) +{ + qemu_log_mask(LOG_GUEST_ERROR, "%s: %d-bit register %#08" HWADDR_PRIx "\n", + funcname, 8 * sz, addr); +} + /* Should signal the TCMI/GPMC */ uint32_t omap_badwidth_read8(void *opaque, hwaddr addr) { uint8_t ret; - OMAP_8B_REG(addr); + omap_log_badwidth(__func__, addr, 1); cpu_physical_memory_read(addr, &ret, 1); return ret; } @@ -XXX,XX +XXX,XX @@ void omap_badwidth_write8(void *opaque, hwaddr addr, { uint8_t val8 = value; - OMAP_8B_REG(addr); + omap_log_badwidth(__func__, addr, 1); cpu_physical_memory_write(addr, &val8, 1); } @@ -XXX,XX +XXX,XX @@ uint32_t omap_badwidth_read16(void *opaque, hwaddr addr) { uint16_t ret; - OMAP_16B_REG(addr); + omap_log_badwidth(__func__, addr, 2); cpu_physical_memory_read(addr, &ret, 2); return ret; } @@ -XXX,XX +XXX,XX @@ void omap_badwidth_write16(void *opaque, hwaddr addr, { uint16_t val16 = value; - OMAP_16B_REG(addr); + omap_log_badwidth(__func__, addr, 2); cpu_physical_memory_write(addr, &val16, 2); } @@ -XXX,XX +XXX,XX @@ uint32_t omap_badwidth_read32(void *opaque, hwaddr addr) { uint32_t ret; - OMAP_32B_REG(addr); + omap_log_badwidth(__func__, addr, 4); cpu_physical_memory_read(addr, &ret, 4); return ret; } @@ -XXX,XX +XXX,XX @@ uint32_t omap_badwidth_read32(void *opaque, hwaddr addr) void omap_badwidth_write32(void *opaque, hwaddr addr, uint32_t value) { - OMAP_32B_REG(addr); + omap_log_badwidth(__func__, addr, 4); cpu_physical_memory_write(addr, &value, 4); } -- 2.17.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-id: 20180624040609.17572-10-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/omap.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/include/hw/arm/omap.h b/include/hw/arm/omap.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/omap.h +++ b/include/hw/arm/omap.h @@ -XXX,XX +XXX,XX @@ # define hw_omap_h "omap.h" #include "hw/irq.h" #include "target/arm/cpu-qom.h" +#include "qemu/log.h" # define OMAP_EMIFS_BASE 0x00000000 # define OMAP2_Q0_BASE 0x00000000 @@ -XXX,XX +XXX,XX @@ struct omap_mpu_state_s *omap2420_mpu_init(MemoryRegion *sysmem, unsigned long sdram_size, const char *core); -#define OMAP_FMT_plx "%#08" HWADDR_PRIx - uint32_t omap_badwidth_read8(void *opaque, hwaddr addr); void omap_badwidth_write8(void *opaque, hwaddr addr, uint32_t value); @@ -XXX,XX +XXX,XX @@ void omap_badwidth_write32(void *opaque, hwaddr addr, void omap_mpu_wakeup(void *opaque, int irq, int req); # define OMAP_BAD_REG(paddr) \ - fprintf(stderr, "%s: Bad register " OMAP_FMT_plx "\n", \ - __func__, paddr) + qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad register %#08"HWADDR_PRIx"\n", \ + __func__, paddr) # define OMAP_RO_REG(paddr) \ - fprintf(stderr, "%s: Read-only register " OMAP_FMT_plx "\n", \ - __func__, paddr) + qemu_log_mask(LOG_GUEST_ERROR, "%s: Read-only register %#08" \ + HWADDR_PRIx "\n", \ + __func__, paddr) /* OMAP-specific Linux bootloader tags for the ATAG_BOARD area (Board-specifc tags are not here) */ -- 2.17.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-id: 20180624040609.17572-11-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/stellaris.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/stellaris.c +++ b/hw/arm/stellaris.c @@ -XXX,XX +XXX,XX @@ static void ssys_write(void *opaque, hwaddr offset, case 0x040: /* SRCR0 */ case 0x044: /* SRCR1 */ case 0x048: /* SRCR2 */ - fprintf(stderr, "Peripheral reset not implemented\n"); + qemu_log_mask(LOG_UNIMP, "Peripheral reset not implemented\n"); break; case 0x054: /* IMC */ s->int_mask = value & 0x7f; -- 2.17.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> Suggested-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20180624040609.17572-12-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/net/stellaris_enet.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c index XXXXXXX..XXXXXXX 100644 --- a/hw/net/stellaris_enet.c +++ b/hw/net/stellaris_enet.c @@ -XXX,XX +XXX,XX @@ static uint64_t stellaris_enet_read(void *opaque, hwaddr offset, return s->np; case 0x38: /* TR */ return 0; - case 0x3c: /* Undocuented: Timestamp? */ + case 0x3c: /* Undocumented: Timestamp? */ return 0; default: hw_error("stellaris_enet_read: Bad offset %x\n", (int)offset); -- 2.17.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> hw_error() finally calls abort(), but there is no need to abort here. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20180624040609.17572-13-f4bug@amsat.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/net/stellaris_enet.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c index XXXXXXX..XXXXXXX 100644 --- a/hw/net/stellaris_enet.c +++ b/hw/net/stellaris_enet.c @@ -XXX,XX +XXX,XX @@ #include "qemu/osdep.h" #include "hw/sysbus.h" #include "net/net.h" +#include "qemu/log.h" #include <zlib.h> //#define DEBUG_STELLARIS_ENET 1 @@ -XXX,XX +XXX,XX @@ static uint64_t stellaris_enet_read(void *opaque, hwaddr offset, case 0x3c: /* Undocumented: Timestamp? */ return 0; default: - hw_error("stellaris_enet_read: Bad offset %x\n", (int)offset); + qemu_log_mask(LOG_GUEST_ERROR, "stellaris_enet_rd%d: Illegal register" + " 0x02%" HWADDR_PRIx "\n", + size * 8, offset); return 0; } } @@ -XXX,XX +XXX,XX @@ static void stellaris_enet_write(void *opaque, hwaddr offset, /* Ignored. */ break; default: - hw_error("stellaris_enet_write: Bad offset %x\n", (int)offset); + qemu_log_mask(LOG_GUEST_ERROR, "stellaris_enet_wr%d: Illegal register " + "0x02%" HWADDR_PRIx " = 0x%" PRIx64 "\n", + size * 8, offset, value); } } -- 2.17.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> hw_error() finally calls abort(), but there is no need to abort here. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-id: 20180624040609.17572-14-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/net/smc91c111.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/hw/net/smc91c111.c b/hw/net/smc91c111.c index XXXXXXX..XXXXXXX 100644 --- a/hw/net/smc91c111.c +++ b/hw/net/smc91c111.c @@ -XXX,XX +XXX,XX @@ #include "hw/sysbus.h" #include "net/net.h" #include "hw/devices.h" +#include "qemu/log.h" /* For crc32 */ #include <zlib.h> @@ -XXX,XX +XXX,XX @@ static void smc91c111_writeb(void *opaque, hwaddr offset, } break; } - hw_error("smc91c111_write: Bad reg %d:%x\n", s->bank, (int)offset); + qemu_log_mask(LOG_GUEST_ERROR, "smc91c111_write(bank:%d) Illegal register" + " 0x%" HWADDR_PRIx " = 0x%x\n", + s->bank, offset, value); } static uint32_t smc91c111_readb(void *opaque, hwaddr offset) @@ -XXX,XX +XXX,XX @@ static uint32_t smc91c111_readb(void *opaque, hwaddr offset) } break; } - hw_error("smc91c111_read: Bad reg %d:%x\n", s->bank, (int)offset); + qemu_log_mask(LOG_GUEST_ERROR, "smc91c111_read(bank:%d) Illegal register" + " 0x%" HWADDR_PRIx "\n", + s->bank, offset); return 0; } -- 2.17.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-id: 20180624040609.17572-15-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/net/smc91c111.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/hw/net/smc91c111.c b/hw/net/smc91c111.c index XXXXXXX..XXXXXXX 100644 --- a/hw/net/smc91c111.c +++ b/hw/net/smc91c111.c @@ -XXX,XX +XXX,XX @@ static void smc91c111_writeb(void *opaque, hwaddr offset, SET_HIGH(gpr, value); return; case 12: /* Control */ - if (value & 1) - fprintf(stderr, "smc91c111:EEPROM store not implemented\n"); - if (value & 2) - fprintf(stderr, "smc91c111:EEPROM reload not implemented\n"); + if (value & 1) { + qemu_log_mask(LOG_UNIMP, + "smc91c111: EEPROM store not implemented\n"); + } + if (value & 2) { + qemu_log_mask(LOG_UNIMP, + "smc91c111: EEPROM reload not implemented\n"); + } value &= ~3; SET_LOW(ctr, value); return; -- 2.17.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> Missed in df3692e04b2. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20180624040609.17572-16-f4bug@amsat.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/stellaris.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/stellaris.c +++ b/hw/arm/stellaris.c @@ -XXX,XX +XXX,XX @@ static void gptm_write(void *opaque, hwaddr offset, break; default: qemu_log_mask(LOG_GUEST_ERROR, - "GPTM: read at bad offset 0x%x\n", (int)offset); + "GPTM: write at bad offset 0x%x\n", (int)offset); } gptm_update_irq(s); } -- 2.17.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> Suggested-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20180624040609.17572-17-f4bug@amsat.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/stellaris.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/stellaris.c +++ b/hw/arm/stellaris.c @@ -XXX,XX +XXX,XX @@ static uint64_t gptm_read(void *opaque, hwaddr offset, return 0; default: qemu_log_mask(LOG_GUEST_ERROR, - "GPTM: read at bad offset 0x%x\n", (int)offset); + "GPTM: read at bad offset 0x02%" HWADDR_PRIx "\n", + offset); return 0; } } @@ -XXX,XX +XXX,XX @@ static void gptm_write(void *opaque, hwaddr offset, break; default: qemu_log_mask(LOG_GUEST_ERROR, - "GPTM: write at bad offset 0x%x\n", (int)offset); + "GPTM: write at bad offset 0x02%" HWADDR_PRIx "\n", + offset); } gptm_update_irq(s); } -- 2.17.1 Add support for MMU protection regions that are smaller than TARGET_PAGE_SIZE. We do this by marking the TLB entry for those pages with a flag TLB_RECHECK. This flag causes us to always take the slow-path for accesses. In the slow path we can then special case them to always call tlb_fill() again, so we have the correct information for the exact address being accessed. This change allows us to handle reading and writing from small regions; we cannot deal with execution from the small region. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180620130619.11362-2-peter.maydell@linaro.org --- accel/tcg/softmmu_template.h | 24 ++++--- include/exec/cpu-all.h | 5 +- accel/tcg/cputlb.c | 131 +++++++++++++++++++++++++++++------ 3 files changed, 130 insertions(+), 30 deletions(-) diff --git a/accel/tcg/softmmu_template.h b/accel/tcg/softmmu_template.h index XXXXXXX..XXXXXXX 100644 --- a/accel/tcg/softmmu_template.h +++ b/accel/tcg/softmmu_template.h @@ -XXX,XX +XXX,XX @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState *env, size_t mmu_idx, size_t index, target_ulong addr, - uintptr_t retaddr) + uintptr_t retaddr, + bool recheck) { CPUIOTLBEntry *iotlbentry = &env->iotlb[mmu_idx][index]; - return io_readx(env, iotlbentry, mmu_idx, addr, retaddr, DATA_SIZE); + return io_readx(env, iotlbentry, mmu_idx, addr, retaddr, recheck, + DATA_SIZE); } #endif @@ -XXX,XX +XXX,XX @@ WORD_TYPE helper_le_ld_name(CPUArchState *env, target_ulong addr, /* ??? Note that the io helpers always read data in the target byte ordering. We should push the LE/BE request down into io. */ - res = glue(io_read, SUFFIX)(env, mmu_idx, index, addr, retaddr); + res = glue(io_read, SUFFIX)(env, mmu_idx, index, addr, retaddr, + tlb_addr & TLB_RECHECK); res = TGT_LE(res); return res; } @@ -XXX,XX +XXX,XX @@ WORD_TYPE helper_be_ld_name(CPUArchState *env, target_ulong addr, /* ??? Note that the io helpers always read data in the target byte ordering. We should push the LE/BE request down into io. */ - res = glue(io_read, SUFFIX)(env, mmu_idx, index, addr, retaddr); + res = glue(io_read, SUFFIX)(env, mmu_idx, index, addr, retaddr, + tlb_addr & TLB_RECHECK); res = TGT_BE(res); return res; } @@ -XXX,XX +XXX,XX @@ static inline void glue(io_write, SUFFIX)(CPUArchState *env, size_t mmu_idx, size_t index, DATA_TYPE val, target_ulong addr, - uintptr_t retaddr) + uintptr_t retaddr, + bool recheck) { CPUIOTLBEntry *iotlbentry = &env->iotlb[mmu_idx][index]; - return io_writex(env, iotlbentry, mmu_idx, val, addr, retaddr, DATA_SIZE); + return io_writex(env, iotlbentry, mmu_idx, val, addr, retaddr, + recheck, DATA_SIZE); } void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val, @@ -XXX,XX +XXX,XX @@ void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val, /* ??? Note that the io helpers always read data in the target byte ordering. We should push the LE/BE request down into io. */ val = TGT_LE(val); - glue(io_write, SUFFIX)(env, mmu_idx, index, val, addr, retaddr); + glue(io_write, SUFFIX)(env, mmu_idx, index, val, addr, + retaddr, tlb_addr & TLB_RECHECK); return; } @@ -XXX,XX +XXX,XX @@ void helper_be_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val, /* ??? Note that the io helpers always read data in the target byte ordering. We should push the LE/BE request down into io. */ val = TGT_BE(val); - glue(io_write, SUFFIX)(env, mmu_idx, index, val, addr, retaddr); + glue(io_write, SUFFIX)(env, mmu_idx, index, val, addr, retaddr, + tlb_addr & TLB_RECHECK); return; } diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/cpu-all.h +++ b/include/exec/cpu-all.h @@ -XXX,XX +XXX,XX @@ CPUArchState *cpu_copy(CPUArchState *env); #define TLB_NOTDIRTY (1 << (TARGET_PAGE_BITS - 2)) /* Set if TLB entry is an IO callback. */ #define TLB_MMIO (1 << (TARGET_PAGE_BITS - 3)) +/* Set if TLB entry must have MMU lookup repeated for every access */ +#define TLB_RECHECK (1 << (TARGET_PAGE_BITS - 4)) /* Use this mask to check interception with an alignment mask * in a TCG backend. */ -#define TLB_FLAGS_MASK (TLB_INVALID_MASK | TLB_NOTDIRTY | TLB_MMIO) +#define TLB_FLAGS_MASK (TLB_INVALID_MASK | TLB_NOTDIRTY | TLB_MMIO \ + | TLB_RECHECK) void dump_exec_info(FILE *f, fprintf_function cpu_fprintf); void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf); diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c index XXXXXXX..XXXXXXX 100644 --- a/accel/tcg/cputlb.c +++ b/accel/tcg/cputlb.c @@ -XXX,XX +XXX,XX @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr, target_ulong code_address; uintptr_t addend; CPUTLBEntry *te, *tv, tn; - hwaddr iotlb, xlat, sz; + hwaddr iotlb, xlat, sz, paddr_page; + target_ulong vaddr_page; unsigned vidx = env->vtlb_index++ % CPU_VTLB_SIZE; int asidx = cpu_asidx_from_attrs(cpu, attrs); assert_cpu_is_self(cpu); - assert(size >= TARGET_PAGE_SIZE); - if (size != TARGET_PAGE_SIZE) { - tlb_add_large_page(env, vaddr, size); - } - sz = size; - section = address_space_translate_for_iotlb(cpu, asidx, paddr, &xlat, &sz, - attrs, &prot); + if (size < TARGET_PAGE_SIZE) { + sz = TARGET_PAGE_SIZE; + } else { + if (size > TARGET_PAGE_SIZE) { + tlb_add_large_page(env, vaddr, size); + } + sz = size; + } + vaddr_page = vaddr & TARGET_PAGE_MASK; + paddr_page = paddr & TARGET_PAGE_MASK; + + section = address_space_translate_for_iotlb(cpu, asidx, paddr_page, + &xlat, &sz, attrs, &prot); assert(sz >= TARGET_PAGE_SIZE); tlb_debug("vaddr=" TARGET_FMT_lx " paddr=0x" TARGET_FMT_plx " prot=%x idx=%d\n", vaddr, paddr, prot, mmu_idx); - address = vaddr; - if (!memory_region_is_ram(section->mr) && !memory_region_is_romd(section->mr)) { + address = vaddr_page; + if (size < TARGET_PAGE_SIZE) { + /* + * Slow-path the TLB entries; we will repeat the MMU check and TLB + * fill on every access. + */ + address |= TLB_RECHECK; + } + if (!memory_region_is_ram(section->mr) && + !memory_region_is_romd(section->mr)) { /* IO memory case */ address |= TLB_MMIO; addend = 0; @@ -XXX,XX +XXX,XX @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr, } code_address = address; - iotlb = memory_region_section_get_iotlb(cpu, section, vaddr, paddr, xlat, - prot, &address); + iotlb = memory_region_section_get_iotlb(cpu, section, vaddr_page, + paddr_page, xlat, prot, &address); - index = (vaddr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); + index = (vaddr_page >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); te = &env->tlb_table[mmu_idx][index]; /* do not discard the translation in te, evict it into a victim tlb */ tv = &env->tlb_v_table[mmu_idx][vidx]; @@ -XXX,XX +XXX,XX @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr, * TARGET_PAGE_BITS, and either * + the ram_addr_t of the page base of the target RAM (if NOTDIRTY or ROM) * + the offset within section->mr of the page base (otherwise) - * We subtract the vaddr (which is page aligned and thus won't + * We subtract the vaddr_page (which is page aligned and thus won't * disturb the low bits) to give an offset which can be added to the * (non-page-aligned) vaddr of the eventual memory access to get * the MemoryRegion offset for the access. Note that the vaddr we * subtract here is that of the page base, and not the same as the * vaddr we add back in io_readx()/io_writex()/get_page_addr_code(). */ - env->iotlb[mmu_idx][index].addr = iotlb - vaddr; + env->iotlb[mmu_idx][index].addr = iotlb - vaddr_page; env->iotlb[mmu_idx][index].attrs = attrs; /* Now calculate the new entry */ - tn.addend = addend - vaddr; + tn.addend = addend - vaddr_page; if (prot & PAGE_READ) { tn.addr_read = address; } else { @@ -XXX,XX +XXX,XX @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr, tn.addr_write = address | TLB_MMIO; } else if (memory_region_is_ram(section->mr) && cpu_physical_memory_is_clean( - memory_region_get_ram_addr(section->mr) + xlat)) { + memory_region_get_ram_addr(section->mr) + xlat)) { tn.addr_write = address | TLB_NOTDIRTY; } else { tn.addr_write = address; @@ -XXX,XX +XXX,XX @@ static inline ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr) static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry, int mmu_idx, - target_ulong addr, uintptr_t retaddr, int size) + target_ulong addr, uintptr_t retaddr, + bool recheck, int size) { CPUState *cpu = ENV_GET_CPU(env); hwaddr mr_offset; @@ -XXX,XX +XXX,XX @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry, bool locked = false; MemTxResult r; + if (recheck) { + /* + * This is a TLB_RECHECK access, where the MMU protection + * covers a smaller range than a target page, and we must + * repeat the MMU check here. This tlb_fill() call might + * longjump out if this access should cause a guest exception. + */ + int index; + target_ulong tlb_addr; + + tlb_fill(cpu, addr, size, MMU_DATA_LOAD, mmu_idx, retaddr); + + index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); + tlb_addr = env->tlb_table[mmu_idx][index].addr_read; + if (!(tlb_addr & ~(TARGET_PAGE_MASK | TLB_RECHECK))) { + /* RAM access */ + uintptr_t haddr = addr + env->tlb_table[mmu_idx][index].addend; + + return ldn_p((void *)haddr, size); + } + /* Fall through for handling IO accesses */ + } + section = iotlb_to_section(cpu, iotlbentry->addr, iotlbentry->attrs); mr = section->mr; mr_offset = (iotlbentry->addr & TARGET_PAGE_MASK) + addr; @@ -XXX,XX +XXX,XX @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry, static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry, int mmu_idx, uint64_t val, target_ulong addr, - uintptr_t retaddr, int size) + uintptr_t retaddr, bool recheck, int size) { CPUState *cpu = ENV_GET_CPU(env); hwaddr mr_offset; @@ -XXX,XX +XXX,XX @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry, bool locked = false; MemTxResult r; + if (recheck) { + /* + * This is a TLB_RECHECK access, where the MMU protection + * covers a smaller range than a target page, and we must + * repeat the MMU check here. This tlb_fill() call might + * longjump out if this access should cause a guest exception. + */ + int index; + target_ulong tlb_addr; + + tlb_fill(cpu, addr, size, MMU_DATA_STORE, mmu_idx, retaddr); + + index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); + tlb_addr = env->tlb_table[mmu_idx][index].addr_write; + if (!(tlb_addr & ~(TARGET_PAGE_MASK | TLB_RECHECK))) { + /* RAM access */ + uintptr_t haddr = addr + env->tlb_table[mmu_idx][index].addend; + + stn_p((void *)haddr, size, val); + return; + } + /* Fall through for handling IO accesses */ + } + section = iotlb_to_section(cpu, iotlbentry->addr, iotlbentry->attrs); mr = section->mr; mr_offset = (iotlbentry->addr & TARGET_PAGE_MASK) + addr; @@ -XXX,XX +XXX,XX @@ tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr) tlb_fill(ENV_GET_CPU(env), addr, 0, MMU_INST_FETCH, mmu_idx, 0); } } + + if (unlikely(env->tlb_table[mmu_idx][index].addr_code & TLB_RECHECK)) { + /* + * This is a TLB_RECHECK access, where the MMU protection + * covers a smaller range than a target page, and we must + * repeat the MMU check here. This tlb_fill() call might + * longjump out if this access should cause a guest exception. + */ + int index; + target_ulong tlb_addr; + + tlb_fill(cpu, addr, 0, MMU_INST_FETCH, mmu_idx, 0); + + index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); + tlb_addr = env->tlb_table[mmu_idx][index].addr_code; + if (!(tlb_addr & ~(TARGET_PAGE_MASK | TLB_RECHECK))) { + /* RAM access. We can't handle this, so for now just stop */ + cpu_abort(cpu, "Unable to handle guest executing from RAM within " + "a small MPU region at 0x" TARGET_FMT_lx, addr); + } + /* + * Fall through to handle IO accesses (which will almost certainly + * also result in failure) + */ + } + iotlbentry = &env->iotlb[mmu_idx][index]; section = iotlb_to_section(cpu, iotlbentry->addr, iotlbentry->attrs); mr = section->mr; @@ -XXX,XX +XXX,XX @@ static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr, tlb_addr = tlbe->addr_write & ~TLB_INVALID_MASK; } - /* Notice an IO access */ - if (unlikely(tlb_addr & TLB_MMIO)) { + /* Notice an IO access or a needs-MMU-lookup access */ + if (unlikely(tlb_addr & (TLB_MMIO | TLB_RECHECK))) { /* There's really nothing that can be done to support this apart from stop-the-world. */ goto stop_the_world; -- 2.17.1 We want to handle small MPU region sizes for ARMv7M. To do this, make get_phys_addr_pmsav7() set the page size to the region size if it is less that TARGET_PAGE_SIZE, rather than working only in TARGET_PAGE_SIZE chunks. Since the core TCG code con't handle execution from small MPU regions, we strip the exec permission from them so that any execution attempts will cause an MPU exception, rather than allowing it to end up with a cpu_abort() in get_page_addr_code(). (The previous code's intention was to make any small page be treated as having no permissions, but unfortunately errors in the implementation meant that it didn't behave that way. It's possible that some binaries using small regions were accidentally working with our old behaviour and won't now.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180620130619.11362-3-peter.maydell@linaro.org --- target/arm/helper.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static inline bool m_is_system_region(CPUARMState *env, uint32_t address) static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, hwaddr *phys_ptr, int *prot, + target_ulong *page_size, ARMMMUFaultInfo *fi) { ARMCPU *cpu = arm_env_get_cpu(env); @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, bool is_user = regime_is_user(env, mmu_idx); *phys_ptr = address; + *page_size = TARGET_PAGE_SIZE; *prot = 0; if (regime_translation_disabled(env, mmu_idx) || @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, rsize++; } } - if (rsize < TARGET_PAGE_BITS) { - qemu_log_mask(LOG_UNIMP, - "DRSR[%d]: No support for MPU (sub)region size of" - " %" PRIu32 " bytes. Minimum is %d.\n", - n, (1 << rsize), TARGET_PAGE_SIZE); - continue; - } if (srdis) { continue; } + if (rsize < TARGET_PAGE_BITS) { + *page_size = 1 << rsize; + } break; } @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, fi->type = ARMFault_Permission; fi->level = 1; + /* + * Core QEMU code can't handle execution from small pages yet, so + * don't try it. This way we'll get an MPU exception, rather than + * eventually causing QEMU to exit in get_page_addr_code(). + */ + if (*page_size < TARGET_PAGE_SIZE && (*prot & PAGE_EXEC)) { + qemu_log_mask(LOG_UNIMP, + "MPU: No support for execution from regions " + "smaller than 1K\n"); + *prot &= ~PAGE_EXEC; + } return !(*prot & (1 << access_type)); } @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr(CPUARMState *env, target_ulong address, } else if (arm_feature(env, ARM_FEATURE_V7)) { /* PMSAv7 */ ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx, - phys_ptr, prot, fi); + phys_ptr, prot, page_size, fi); } else { /* Pre-v7 MPU */ ret = get_phys_addr_pmsav5(env, address, access_type, mmu_idx, @@ -XXX,XX +XXX,XX @@ bool arm_tlb_fill(CPUState *cs, vaddr address, core_to_arm_mmu_idx(env, mmu_idx), &phys_addr, &attrs, &prot, &page_size, fi, NULL); if (!ret) { - /* Map a single [sub]page. */ - phys_addr &= TARGET_PAGE_MASK; - address &= TARGET_PAGE_MASK; + /* + * Map a single [sub]page. Regions smaller than our declared + * target page size are handled specially, so for those we + * pass in the exact addresses. + */ + if (page_size >= TARGET_PAGE_SIZE) { + phys_addr &= TARGET_PAGE_MASK; + address &= TARGET_PAGE_MASK; + } tlb_set_page_with_attrs(cs, address, phys_addr, attrs, prot, mmu_idx, page_size); return 0; -- 2.17.1 Allow ARMv8M to handle small MPU and SAU region sizes, by making get_phys_add_pmsav8() set the page size to the 1 if the MPU or SAU region covers less than a TARGET_PAGE_SIZE. We choose to use a size of 1 because it makes no difference to the core code, and avoids having to track both the base and limit for SAU and MPU and then convert into an artificially restricted "page size" that the core code will then ignore. Since the core TCG code can't handle execution from small MPU regions, we strip the exec permission from them so that any execution attempts will cause an MPU exception, rather than allowing it to end up with a cpu_abort() in get_page_addr_code(). (The previous code's intention was to make any small page be treated as having no permissions, but unfortunately errors in the implementation meant that it didn't behave that way. It's possible that some binaries using small regions were accidentally working with our old behaviour and won't now.) We also retain an existing bug, where we ignored the possibility that the SAU region might not cover the entire page, in the case of executable regions. This is necessary because some currently-working guest code images rely on being able to execute from addresses which are covered by a page-sized MPU region but a smaller SAU region. We can remove this workaround if we ever support execution from small regions. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180620130619.11362-4-peter.maydell@linaro.org --- target/arm/helper.c | 78 ++++++++++++++++++++++++++++++++------------- 1 file changed, 55 insertions(+), 23 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, /* Security attributes for an address, as returned by v8m_security_lookup. */ typedef struct V8M_SAttributes { + bool subpage; /* true if these attrs don't cover the whole TARGET_PAGE */ bool ns; bool nsc; uint8_t sregion; @@ -XXX,XX +XXX,XX @@ static void v8m_security_lookup(CPUARMState *env, uint32_t address, int r; bool idau_exempt = false, idau_ns = true, idau_nsc = true; int idau_region = IREGION_NOTVALID; + uint32_t addr_page_base = address & TARGET_PAGE_MASK; + uint32_t addr_page_limit = addr_page_base + (TARGET_PAGE_SIZE - 1); if (cpu->idau) { IDAUInterfaceClass *iic = IDAU_INTERFACE_GET_CLASS(cpu->idau); @@ -XXX,XX +XXX,XX @@ static void v8m_security_lookup(CPUARMState *env, uint32_t address, uint32_t limit = env->sau.rlar[r] | 0x1f; if (base <= address && limit >= address) { + if (base > addr_page_base || limit < addr_page_limit) { + sattrs->subpage = true; + } if (sattrs->srvalid) { /* If we hit in more than one region then we must report * as Secure, not NS-Callable, with no valid region @@ -XXX,XX +XXX,XX @@ static void v8m_security_lookup(CPUARMState *env, uint32_t address, static bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, hwaddr *phys_ptr, MemTxAttrs *txattrs, - int *prot, ARMMMUFaultInfo *fi, uint32_t *mregion) + int *prot, bool *is_subpage, + ARMMMUFaultInfo *fi, uint32_t *mregion) { /* Perform a PMSAv8 MPU lookup (without also doing the SAU check * that a full phys-to-virt translation does). * mregion is (if not NULL) set to the region number which matched, * or -1 if no region number is returned (MPU off, address did not * hit a region, address hit in multiple regions). + * We set is_subpage to true if the region hit doesn't cover the + * entire TARGET_PAGE the address is within. */ ARMCPU *cpu = arm_env_get_cpu(env); bool is_user = regime_is_user(env, mmu_idx); @@ -XXX,XX +XXX,XX @@ static bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, int n; int matchregion = -1; bool hit = false; + uint32_t addr_page_base = address & TARGET_PAGE_MASK; + uint32_t addr_page_limit = addr_page_base + (TARGET_PAGE_SIZE - 1); + *is_subpage = false; *phys_ptr = address; *prot = 0; if (mregion) { @@ -XXX,XX +XXX,XX @@ static bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, continue; } + if (base > addr_page_base || limit < addr_page_limit) { + *is_subpage = true; + } + if (hit) { /* Multiple regions match -- always a failure (unlike * PMSAv7 where highest-numbered-region wins) @@ -XXX,XX +XXX,XX @@ static bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, matchregion = n; hit = true; - - if (base & ~TARGET_PAGE_MASK) { - qemu_log_mask(LOG_UNIMP, - "MPU_RBAR[%d]: No support for MPU region base" - "address of 0x%" PRIx32 ". Minimum alignment is " - "%d\n", - n, base, TARGET_PAGE_BITS); - continue; - } - if ((limit + 1) & ~TARGET_PAGE_MASK) { - qemu_log_mask(LOG_UNIMP, - "MPU_RBAR[%d]: No support for MPU region limit" - "address of 0x%" PRIx32 ". Minimum alignment is " - "%d\n", - n, limit, TARGET_PAGE_BITS); - continue; - } } } @@ -XXX,XX +XXX,XX @@ static bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, fi->type = ARMFault_Permission; fi->level = 1; + /* + * Core QEMU code can't handle execution from small pages yet, so + * don't try it. This means any attempted execution will generate + * an MPU exception, rather than eventually causing QEMU to exit in + * get_page_addr_code(). + */ + if (*is_subpage && (*prot & PAGE_EXEC)) { + qemu_log_mask(LOG_UNIMP, + "MPU: No support for execution from regions " + "smaller than 1K\n"); + *prot &= ~PAGE_EXEC; + } return !(*prot & (1 << access_type)); } @@ -XXX,XX +XXX,XX @@ static bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, hwaddr *phys_ptr, MemTxAttrs *txattrs, - int *prot, ARMMMUFaultInfo *fi) + int *prot, target_ulong *page_size, + ARMMMUFaultInfo *fi) { uint32_t secure = regime_is_secure(env, mmu_idx); V8M_SAttributes sattrs = {}; + bool ret; + bool mpu_is_subpage; if (arm_feature(env, ARM_FEATURE_M_SECURITY)) { v8m_security_lookup(env, address, access_type, mmu_idx, &sattrs); @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address, } else { fi->type = ARMFault_QEMU_SFault; } + *page_size = sattrs.subpage ? 1 : TARGET_PAGE_SIZE; *phys_ptr = address; *prot = 0; return true; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address, * for M_FAKE_FSR_SFAULT in arm_v7m_cpu_do_interrupt(). */ fi->type = ARMFault_QEMU_SFault; + *page_size = sattrs.subpage ? 1 : TARGET_PAGE_SIZE; *phys_ptr = address; *prot = 0; return true; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address, } } - return pmsav8_mpu_lookup(env, address, access_type, mmu_idx, phys_ptr, - txattrs, prot, fi, NULL); + ret = pmsav8_mpu_lookup(env, address, access_type, mmu_idx, phys_ptr, + txattrs, prot, &mpu_is_subpage, fi, NULL); + /* + * TODO: this is a temporary hack to ignore the fact that the SAU region + * is smaller than a page if this is an executable region. We never + * supported small MPU regions, but we did (accidentally) allow small + * SAU regions, and if we now made small SAU regions not be executable + * then this would break previously working guest code. We can't + * remove this until/unless we implement support for execution from + * small regions. + */ + if (*prot & PAGE_EXEC) { + sattrs.subpage = false; + } + *page_size = sattrs.subpage || mpu_is_subpage ? 1 : TARGET_PAGE_SIZE; + return ret; } static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address, @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr(CPUARMState *env, target_ulong address, if (arm_feature(env, ARM_FEATURE_V8)) { /* PMSAv8 */ ret = get_phys_addr_pmsav8(env, address, access_type, mmu_idx, - phys_ptr, attrs, prot, fi); + phys_ptr, attrs, prot, page_size, fi); } else if (arm_feature(env, ARM_FEATURE_V7)) { /* PMSAv7 */ ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx, @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(v7m_tt)(CPUARMState *env, uint32_t addr, uint32_t op) uint32_t mregion; bool targetpriv; bool targetsec = env->v7m.secure; + bool is_subpage; /* Work out what the security state and privilege level we're * interested in is... @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(v7m_tt)(CPUARMState *env, uint32_t addr, uint32_t op) if (arm_current_el(env) != 0 || alt) { /* We can ignore the return value as prot is always set */ pmsav8_mpu_lookup(env, addr, MMU_DATA_LOAD, mmu_idx, - &phys_addr, &attrs, &prot, &fi, &mregion); + &phys_addr, &attrs, &prot, &is_subpage, + &fi, &mregion); if (mregion == -1) { mrvalid = false; mregion = 0; -- 2.17.1 From: Jia He <hejianet@gmail.com> In case the STE's config is "Bypass" we currently don't set the IOMMUTLBEntry perm flags and the access does not succeed. Also if the config is 0b0xx (Aborted/Reserved), decode_ste and smmuv3_decode_config currently returns -EINVAL and we don't enter the expected code path: we record an event whereas we should not. This patch fixes those bugs and simplifies the error handling. decode_ste and smmuv3_decode_config now return 0 if aborted or bypassed config was found. Only bad config info produces negative error values. In smmuv3_translate we more clearly differentiate errors, bypass/smmu disabled, aborted and success cases. Also trace points are differentiated. Fixes: 9bde7f0674fe ("hw/arm/smmuv3: Implement translate callback") Reported-by: jia.he@hxt-semitech.com Signed-off-by: jia.he@hxt-semitech.com Signed-off-by: Eric Auger <eric.auger@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 1529653501-15358-2-git-send-email-eric.auger@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/smmuv3-internal.h | 12 ++++- hw/arm/smmuv3.c | 96 +++++++++++++++++++++++++++------------- hw/arm/trace-events | 7 +-- 3 files changed, 80 insertions(+), 35 deletions(-) diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/smmuv3-internal.h +++ b/hw/arm/smmuv3-internal.h @@ -XXX,XX +XXX,XX @@ #include "hw/arm/smmu-common.h" +typedef enum SMMUTranslationStatus { + SMMU_TRANS_DISABLE, + SMMU_TRANS_ABORT, + SMMU_TRANS_BYPASS, + SMMU_TRANS_ERROR, + SMMU_TRANS_SUCCESS, +} SMMUTranslationStatus; + /* MMIO Registers */ REG32(IDR0, 0x0) @@ -XXX,XX +XXX,XX @@ enum { /* Command completion notification */ /* Events */ typedef enum SMMUEventType { - SMMU_EVT_OK = 0x00, + SMMU_EVT_NONE = 0x00, SMMU_EVT_F_UUT , SMMU_EVT_C_BAD_STREAMID , SMMU_EVT_F_STE_FETCH , @@ -XXX,XX +XXX,XX @@ typedef enum SMMUEventType { } SMMUEventType; static const char *event_stringify[] = { - [SMMU_EVT_OK] = "SMMU_EVT_OK", + [SMMU_EVT_NONE] = "no recorded event", [SMMU_EVT_F_UUT] = "SMMU_EVT_F_UUT", [SMMU_EVT_C_BAD_STREAMID] = "SMMU_EVT_C_BAD_STREAMID", [SMMU_EVT_F_STE_FETCH] = "SMMU_EVT_F_STE_FETCH", diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/smmuv3.c +++ b/hw/arm/smmuv3.c @@ -XXX,XX +XXX,XX @@ #include "hw/qdev-core.h" #include "hw/pci/pci.h" #include "exec/address-spaces.h" +#include "cpu.h" #include "trace.h" #include "qemu/log.h" #include "qemu/error-report.h" @@ -XXX,XX +XXX,XX @@ void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo *info) EVT_SET_SID(&evt, info->sid); switch (info->type) { - case SMMU_EVT_OK: + case SMMU_EVT_NONE: return; case SMMU_EVT_F_UUT: EVT_SET_SSID(&evt, info->u.f_uut.ssid); @@ -XXX,XX +XXX,XX @@ static int smmu_get_cd(SMMUv3State *s, STE *ste, uint32_t ssid, return 0; } -/* Returns <0 if the caller has no need to continue the translation */ +/* Returns < 0 in case of invalid STE, 0 otherwise */ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg, STE *ste, SMMUEventInfo *event) { uint32_t config; - int ret = -EINVAL; if (!STE_VALID(ste)) { goto bad_ste; @@ -XXX,XX +XXX,XX @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg, config = STE_CONFIG(ste); if (STE_CFG_ABORT(config)) { - cfg->aborted = true; /* abort but don't record any event */ - return ret; + cfg->aborted = true; + return 0; } if (STE_CFG_BYPASS(config)) { cfg->bypassed = true; - return ret; + return 0; } if (STE_CFG_S2_ENABLED(config)) { @@ -XXX,XX +XXX,XX @@ bad_cd: * the different configuration decoding steps * @event: must be zero'ed by the caller * - * return < 0 if the translation needs to be aborted (@event is filled + * return < 0 in case of config decoding error (@event is filled * accordingly). Return 0 otherwise. */ static int smmuv3_decode_config(IOMMUMemoryRegion *mr, SMMUTransCfg *cfg, @@ -XXX,XX +XXX,XX @@ static int smmuv3_decode_config(IOMMUMemoryRegion *mr, SMMUTransCfg *cfg, SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu); uint32_t sid = smmu_get_sid(sdev); SMMUv3State *s = sdev->smmu; - int ret = -EINVAL; + int ret; STE ste; CD cd; - if (smmu_find_ste(s, sid, &ste, event)) { + ret = smmu_find_ste(s, sid, &ste, event); + if (ret) { return ret; } - if (decode_ste(s, cfg, &ste, event)) { + ret = decode_ste(s, cfg, &ste, event); + if (ret) { return ret; } - if (smmu_get_cd(s, &ste, 0 /* ssid */, &cd, event)) { + if (cfg->aborted || cfg->bypassed) { + return 0; + } + + ret = smmu_get_cd(s, &ste, 0 /* ssid */, &cd, event); + if (ret) { return ret; } @@ -XXX,XX +XXX,XX @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu); SMMUv3State *s = sdev->smmu; uint32_t sid = smmu_get_sid(sdev); - SMMUEventInfo event = {.type = SMMU_EVT_OK, .sid = sid}; + SMMUEventInfo event = {.type = SMMU_EVT_NONE, .sid = sid}; SMMUPTWEventInfo ptw_info = {}; + SMMUTranslationStatus status; SMMUTransCfg cfg = {}; IOMMUTLBEntry entry = { .target_as = &address_space_memory, @@ -XXX,XX +XXX,XX @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, .addr_mask = ~(hwaddr)0, .perm = IOMMU_NONE, }; - int ret = 0; if (!smmu_enabled(s)) { - goto out; + status = SMMU_TRANS_DISABLE; + goto epilogue; } - ret = smmuv3_decode_config(mr, &cfg, &event); - if (ret) { - goto out; + if (smmuv3_decode_config(mr, &cfg, &event)) { + status = SMMU_TRANS_ERROR; + goto epilogue; } if (cfg.aborted) { - goto out; + status = SMMU_TRANS_ABORT; + goto epilogue; } - ret = smmu_ptw(&cfg, addr, flag, &entry, &ptw_info); - if (ret) { + if (cfg.bypassed) { + status = SMMU_TRANS_BYPASS; + goto epilogue; + } + + if (smmu_ptw(&cfg, addr, flag, &entry, &ptw_info)) { switch (ptw_info.type) { case SMMU_PTW_ERR_WALK_EABT: event.type = SMMU_EVT_F_WALK_EABT; @@ -XXX,XX +XXX,XX @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, default: g_assert_not_reached(); } + status = SMMU_TRANS_ERROR; + } else { + status = SMMU_TRANS_SUCCESS; } -out: - if (ret) { - qemu_log_mask(LOG_GUEST_ERROR, - "%s translation failed for iova=0x%"PRIx64"(%d)\n", - mr->parent_obj.name, addr, ret); - entry.perm = IOMMU_NONE; - smmuv3_record_event(s, &event); - } else if (!cfg.aborted) { + +epilogue: + switch (status) { + case SMMU_TRANS_SUCCESS: entry.perm = flag; - trace_smmuv3_translate(mr->parent_obj.name, sid, addr, - entry.translated_addr, entry.perm); + trace_smmuv3_translate_success(mr->parent_obj.name, sid, addr, + entry.translated_addr, entry.perm); + break; + case SMMU_TRANS_DISABLE: + entry.perm = flag; + entry.addr_mask = ~TARGET_PAGE_MASK; + trace_smmuv3_translate_disable(mr->parent_obj.name, sid, addr, + entry.perm); + break; + case SMMU_TRANS_BYPASS: + entry.perm = flag; + entry.addr_mask = ~TARGET_PAGE_MASK; + trace_smmuv3_translate_bypass(mr->parent_obj.name, sid, addr, + entry.perm); + break; + case SMMU_TRANS_ABORT: + /* no event is recorded on abort */ + trace_smmuv3_translate_abort(mr->parent_obj.name, sid, addr, + entry.perm); + break; + case SMMU_TRANS_ERROR: + qemu_log_mask(LOG_GUEST_ERROR, + "%s translation failed for iova=0x%"PRIx64"(%s)\n", + mr->parent_obj.name, addr, smmu_event_string(event.type)); + smmuv3_record_event(s, &event); + break; } return entry; diff --git a/hw/arm/trace-events b/hw/arm/trace-events index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/trace-events +++ b/hw/arm/trace-events @@ -XXX,XX +XXX,XX @@ smmuv3_record_event(const char *type, uint32_t sid) "%s sid=%d" smmuv3_find_ste(uint16_t sid, uint32_t features, uint16_t sid_split) "SID:0x%x features:0x%x, sid_split:0x%x" smmuv3_find_ste_2lvl(uint64_t strtab_base, uint64_t l1ptr, int l1_ste_offset, uint64_t l2ptr, int l2_ste_offset, int max_l2_ste) "strtab_base:0x%"PRIx64" l1ptr:0x%"PRIx64" l1_off:0x%x, l2ptr:0x%"PRIx64" l2_off:0x%x max_l2_ste:%d" smmuv3_get_ste(uint64_t addr) "STE addr: 0x%"PRIx64 -smmuv3_translate_bypass(const char *n, uint16_t sid, uint64_t addr, bool is_write) "%s sid=%d bypass iova:0x%"PRIx64" is_write=%d" -smmuv3_translate_in(uint16_t sid, int pci_bus_num, uint64_t strtab_base) "SID:0x%x bus:%d strtab_base:0x%"PRIx64 +smmuv3_translate_disable(const char *n, uint16_t sid, uint64_t addr, bool is_write) "%s sid=%d bypass (smmu disabled) iova:0x%"PRIx64" is_write=%d" +smmuv3_translate_bypass(const char *n, uint16_t sid, uint64_t addr, bool is_write) "%s sid=%d STE bypass iova:0x%"PRIx64" is_write=%d" +smmuv3_translate_abort(const char *n, uint16_t sid, uint64_t addr, bool is_write) "%s sid=%d abort on iova:0x%"PRIx64" is_write=%d" +smmuv3_translate_success(const char *n, uint16_t sid, uint64_t iova, uint64_t translated, int perm) "%s sid=%d iova=0x%"PRIx64" translated=0x%"PRIx64" perm=0x%x" smmuv3_get_cd(uint64_t addr) "CD addr: 0x%"PRIx64 -smmuv3_translate(const char *n, uint16_t sid, uint64_t iova, uint64_t translated, int perm) "%s sid=%d iova=0x%"PRIx64" translated=0x%"PRIx64" perm=0x%x" smmuv3_decode_cd(uint32_t oas) "oas=%d" smmuv3_decode_cd_tt(int i, uint32_t tsz, uint64_t ttb, uint32_t granule_sz) "TT[%d]:tsz:%d ttb:0x%"PRIx64" granule_sz:%d" -- 2.17.1 From: Eric Auger <eric.auger@redhat.com> Let's cache config data to avoid fetching and parsing STE/CD structures on each translation. We invalidate them on data structure invalidation commands. We put in place a per-smmu mutex to protect the config cache. This will be useful too to protect the IOTLB cache. The caches can be accessed without BQL, ie. in IO dataplane. The same kind of mutex was put in place in the intel viommu. Signed-off-by: Eric Auger <eric.auger@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 1529653501-15358-3-git-send-email-eric.auger@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/smmu-common.h | 5 ++ include/hw/arm/smmuv3.h | 1 + hw/arm/smmu-common.c | 24 ++++++- hw/arm/smmuv3.c | 135 +++++++++++++++++++++++++++++++++-- hw/arm/trace-events | 6 ++ 5 files changed, 164 insertions(+), 7 deletions(-) diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/smmu-common.h +++ b/include/hw/arm/smmu-common.h @@ -XXX,XX +XXX,XX @@ typedef struct SMMUDevice { int devfn; IOMMUMemoryRegion iommu; AddressSpace as; + uint32_t cfg_cache_hits; + uint32_t cfg_cache_misses; } SMMUDevice; typedef struct SMMUNotifierNode { @@ -XXX,XX +XXX,XX @@ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm, */ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova); +/* Return the iommu mr associated to @sid, or NULL if none */ +IOMMUMemoryRegion *smmu_iommu_mr(SMMUState *s, uint32_t sid); + #endif /* HW_ARM_SMMU_COMMON */ diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/smmuv3.h +++ b/include/hw/arm/smmuv3.h @@ -XXX,XX +XXX,XX @@ typedef struct SMMUv3State { SMMUQueue eventq, cmdq; qemu_irq irq[4]; + QemuMutex mutex; } SMMUv3State; typedef enum { diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/smmu-common.c +++ b/hw/arm/smmu-common.c @@ -XXX,XX +XXX,XX @@ static AddressSpace *smmu_find_add_as(PCIBus *bus, void *opaque, int devfn) return &sdev->as; } +IOMMUMemoryRegion *smmu_iommu_mr(SMMUState *s, uint32_t sid) +{ + uint8_t bus_n, devfn; + SMMUPciBus *smmu_bus; + SMMUDevice *smmu; + + bus_n = PCI_BUS_NUM(sid); + smmu_bus = smmu_find_smmu_pcibus(s, bus_n); + if (smmu_bus) { + devfn = sid & 0x7; + smmu = smmu_bus->pbdev[devfn]; + if (smmu) { + return &smmu->iommu; + } + } + return NULL; +} + static void smmu_base_realize(DeviceState *dev, Error **errp) { SMMUState *s = ARM_SMMU(dev); @@ -XXX,XX +XXX,XX @@ static void smmu_base_realize(DeviceState *dev, Error **errp) error_propagate(errp, local_err); return; } - + s->configs = g_hash_table_new_full(NULL, NULL, NULL, g_free); s->smmu_pcibus_by_busptr = g_hash_table_new(NULL, NULL); if (s->primary_bus) { @@ -XXX,XX +XXX,XX @@ static void smmu_base_realize(DeviceState *dev, Error **errp) static void smmu_base_reset(DeviceState *dev) { - /* will be filled later on */ + SMMUState *s = ARM_SMMU(dev); + + g_hash_table_remove_all(s->configs); } static Property smmu_dev_properties[] = { diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/smmuv3.c +++ b/hw/arm/smmuv3.c @@ -XXX,XX +XXX,XX @@ static int smmuv3_decode_config(IOMMUMemoryRegion *mr, SMMUTransCfg *cfg, return decode_cd(cfg, &cd, event); } +/** + * smmuv3_get_config - Look up for a cached copy of configuration data for + * @sdev and on cache miss performs a configuration structure decoding from + * guest RAM. + * + * @sdev: SMMUDevice handle + * @event: output event info + * + * The configuration cache contains data resulting from both STE and CD + * decoding under the form of an SMMUTransCfg struct. The hash table is indexed + * by the SMMUDevice handle. + */ +static SMMUTransCfg *smmuv3_get_config(SMMUDevice *sdev, SMMUEventInfo *event) +{ + SMMUv3State *s = sdev->smmu; + SMMUState *bc = &s->smmu_state; + SMMUTransCfg *cfg; + + cfg = g_hash_table_lookup(bc->configs, sdev); + if (cfg) { + sdev->cfg_cache_hits++; + trace_smmuv3_config_cache_hit(smmu_get_sid(sdev), + sdev->cfg_cache_hits, sdev->cfg_cache_misses, + 100 * sdev->cfg_cache_hits / + (sdev->cfg_cache_hits + sdev->cfg_cache_misses)); + } else { + sdev->cfg_cache_misses++; + trace_smmuv3_config_cache_miss(smmu_get_sid(sdev), + sdev->cfg_cache_hits, sdev->cfg_cache_misses, + 100 * sdev->cfg_cache_hits / + (sdev->cfg_cache_hits + sdev->cfg_cache_misses)); + cfg = g_new0(SMMUTransCfg, 1); + + if (!smmuv3_decode_config(&sdev->iommu, cfg, event)) { + g_hash_table_insert(bc->configs, sdev, cfg); + } else { + g_free(cfg); + cfg = NULL; + } + } + return cfg; +} + +static void smmuv3_flush_config(SMMUDevice *sdev) +{ + SMMUv3State *s = sdev->smmu; + SMMUState *bc = &s->smmu_state; + + trace_smmuv3_config_cache_inv(smmu_get_sid(sdev)); + g_hash_table_remove(bc->configs, sdev); +} + static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, IOMMUAccessFlags flag, int iommu_idx) { @@ -XXX,XX +XXX,XX @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, SMMUEventInfo event = {.type = SMMU_EVT_NONE, .sid = sid}; SMMUPTWEventInfo ptw_info = {}; SMMUTranslationStatus status; - SMMUTransCfg cfg = {}; + SMMUTransCfg *cfg = NULL; IOMMUTLBEntry entry = { .target_as = &address_space_memory, .iova = addr, @@ -XXX,XX +XXX,XX @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, .perm = IOMMU_NONE, }; + qemu_mutex_lock(&s->mutex); + if (!smmu_enabled(s)) { status = SMMU_TRANS_DISABLE; goto epilogue; } - if (smmuv3_decode_config(mr, &cfg, &event)) { + cfg = smmuv3_get_config(sdev, &event); + if (!cfg) { status = SMMU_TRANS_ERROR; goto epilogue; } - if (cfg.aborted) { + if (cfg->aborted) { status = SMMU_TRANS_ABORT; goto epilogue; } - if (cfg.bypassed) { + if (cfg->bypassed) { status = SMMU_TRANS_BYPASS; goto epilogue; } - if (smmu_ptw(&cfg, addr, flag, &entry, &ptw_info)) { + if (smmu_ptw(cfg, addr, flag, &entry, &ptw_info)) { switch (ptw_info.type) { case SMMU_PTW_ERR_WALK_EABT: event.type = SMMU_EVT_F_WALK_EABT; @@ -XXX,XX +XXX,XX @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, } epilogue: + qemu_mutex_unlock(&s->mutex); switch (status) { case SMMU_TRANS_SUCCESS: entry.perm = flag; @@ -XXX,XX +XXX,XX @@ epilogue: static int smmuv3_cmdq_consume(SMMUv3State *s) { + SMMUState *bs = ARM_SMMU(s); SMMUCmdError cmd_error = SMMU_CERROR_NONE; SMMUQueue *q = &s->cmdq; SMMUCommandType type = 0; @@ -XXX,XX +XXX,XX @@ static int smmuv3_cmdq_consume(SMMUv3State *s) trace_smmuv3_cmdq_opcode(smmu_cmd_string(type)); + qemu_mutex_lock(&s->mutex); switch (type) { case SMMU_CMD_SYNC: if (CMD_SYNC_CS(&cmd) & CMD_SYNC_SIG_IRQ) { @@ -XXX,XX +XXX,XX @@ static int smmuv3_cmdq_consume(SMMUv3State *s) break; case SMMU_CMD_PREFETCH_CONFIG: case SMMU_CMD_PREFETCH_ADDR: + break; case SMMU_CMD_CFGI_STE: + { + uint32_t sid = CMD_SID(&cmd); + IOMMUMemoryRegion *mr = smmu_iommu_mr(bs, sid); + SMMUDevice *sdev; + + if (CMD_SSEC(&cmd)) { + cmd_error = SMMU_CERROR_ILL; + break; + } + + if (!mr) { + break; + } + + trace_smmuv3_cmdq_cfgi_ste(sid); + sdev = container_of(mr, SMMUDevice, iommu); + smmuv3_flush_config(sdev); + + break; + } case SMMU_CMD_CFGI_STE_RANGE: /* same as SMMU_CMD_CFGI_ALL */ + { + uint32_t start = CMD_SID(&cmd), end, i; + uint8_t range = CMD_STE_RANGE(&cmd); + + if (CMD_SSEC(&cmd)) { + cmd_error = SMMU_CERROR_ILL; + break; + } + + end = start + (1 << (range + 1)) - 1; + trace_smmuv3_cmdq_cfgi_ste_range(start, end); + + for (i = start; i <= end; i++) { + IOMMUMemoryRegion *mr = smmu_iommu_mr(bs, i); + SMMUDevice *sdev; + + if (!mr) { + continue; + } + sdev = container_of(mr, SMMUDevice, iommu); + smmuv3_flush_config(sdev); + } + break; + } case SMMU_CMD_CFGI_CD: case SMMU_CMD_CFGI_CD_ALL: + { + uint32_t sid = CMD_SID(&cmd); + IOMMUMemoryRegion *mr = smmu_iommu_mr(bs, sid); + SMMUDevice *sdev; + + if (CMD_SSEC(&cmd)) { + cmd_error = SMMU_CERROR_ILL; + break; + } + + if (!mr) { + break; + } + + trace_smmuv3_cmdq_cfgi_cd(sid); + sdev = container_of(mr, SMMUDevice, iommu); + smmuv3_flush_config(sdev); + break; + } case SMMU_CMD_TLBI_NH_ALL: case SMMU_CMD_TLBI_NH_ASID: case SMMU_CMD_TLBI_NH_VA: @@ -XXX,XX +XXX,XX @@ static int smmuv3_cmdq_consume(SMMUv3State *s) "Illegal command type: %d\n", CMD_TYPE(&cmd)); break; } + qemu_mutex_unlock(&s->mutex); if (cmd_error) { break; } @@ -XXX,XX +XXX,XX @@ static void smmu_realize(DeviceState *d, Error **errp) return; } + qemu_mutex_init(&s->mutex); + memory_region_init_io(&sys->iomem, OBJECT(s), &smmu_mem_ops, sys, TYPE_ARM_SMMUV3, 0x20000); diff --git a/hw/arm/trace-events b/hw/arm/trace-events index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/trace-events +++ b/hw/arm/trace-events @@ -XXX,XX +XXX,XX @@ smmuv3_translate_success(const char *n, uint16_t sid, uint64_t iova, uint64_t tr smmuv3_get_cd(uint64_t addr) "CD addr: 0x%"PRIx64 smmuv3_decode_cd(uint32_t oas) "oas=%d" smmuv3_decode_cd_tt(int i, uint32_t tsz, uint64_t ttb, uint32_t granule_sz) "TT[%d]:tsz:%d ttb:0x%"PRIx64" granule_sz:%d" +smmuv3_cmdq_cfgi_ste(int streamid) "streamid =%d" +smmuv3_cmdq_cfgi_ste_range(int start, int end) "start=0x%d - end=0x%d" +smmuv3_cmdq_cfgi_cd(uint32_t sid) "streamid = %d" +smmuv3_config_cache_hit(uint32_t sid, uint32_t hits, uint32_t misses, uint32_t perc) "Config cache HIT for sid %d (hits=%d, misses=%d, hit rate=%d)" +smmuv3_config_cache_miss(uint32_t sid, uint32_t hits, uint32_t misses, uint32_t perc) "Config cache MISS for sid %d (hits=%d, misses=%d, hit rate=%d)" +smmuv3_config_cache_inv(uint32_t sid) "Config cache INV for sid %d" -- 2.17.1 From: Eric Auger <eric.auger@redhat.com> We emulate a TLB cache of size SMMU_IOTLB_MAX_SIZE=256. It is implemented as a hash table whose key is a combination of the 16b asid and 48b IOVA (Jenkins hash). Entries are invalidated on TLB invalidation commands, either globally, or per asid, or per asid/iova. Signed-off-by: Eric Auger <eric.auger@redhat.com> Message-id: 1529653501-15358-4-git-send-email-eric.auger@redhat.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/smmu-common.h | 13 +++++ hw/arm/smmu-common.c | 60 ++++++++++++++++++++++ hw/arm/smmuv3.c | 98 ++++++++++++++++++++++++++++++++++-- hw/arm/trace-events | 9 ++++ 4 files changed, 176 insertions(+), 4 deletions(-) diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/smmu-common.h +++ b/include/hw/arm/smmu-common.h @@ -XXX,XX +XXX,XX @@ typedef struct SMMUTransCfg { uint8_t tbi; /* Top Byte Ignore */ uint16_t asid; SMMUTransTableInfo tt[2]; + uint32_t iotlb_hits; /* counts IOTLB hits for this asid */ + uint32_t iotlb_misses; /* counts IOTLB misses for this asid */ } SMMUTransCfg; typedef struct SMMUDevice { @@ -XXX,XX +XXX,XX @@ typedef struct SMMUPciBus { SMMUDevice *pbdev[0]; /* Parent array is sparse, so dynamically alloc */ } SMMUPciBus; +typedef struct SMMUIOTLBKey { + uint64_t iova; + uint16_t asid; +} SMMUIOTLBKey; + typedef struct SMMUState { /* <private> */ SysBusDevice dev; @@ -XXX,XX +XXX,XX @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova); /* Return the iommu mr associated to @sid, or NULL if none */ IOMMUMemoryRegion *smmu_iommu_mr(SMMUState *s, uint32_t sid); +#define SMMU_IOTLB_MAX_SIZE 256 + +void smmu_iotlb_inv_all(SMMUState *s); +void smmu_iotlb_inv_asid(SMMUState *s, uint16_t asid); +void smmu_iotlb_inv_iova(SMMUState *s, uint16_t asid, dma_addr_t iova); + #endif /* HW_ARM_SMMU_COMMON */ diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/smmu-common.c +++ b/hw/arm/smmu-common.c @@ -XXX,XX +XXX,XX @@ #include "qom/cpu.h" #include "hw/qdev-properties.h" #include "qapi/error.h" +#include "qemu/jhash.h" #include "qemu/error-report.h" #include "hw/arm/smmu-common.h" #include "smmu-internal.h" +/* IOTLB Management */ + +inline void smmu_iotlb_inv_all(SMMUState *s) +{ + trace_smmu_iotlb_inv_all(); + g_hash_table_remove_all(s->iotlb); +} + +static gboolean smmu_hash_remove_by_asid(gpointer key, gpointer value, + gpointer user_data) +{ + uint16_t asid = *(uint16_t *)user_data; + SMMUIOTLBKey *iotlb_key = (SMMUIOTLBKey *)key; + + return iotlb_key->asid == asid; +} + +inline void smmu_iotlb_inv_iova(SMMUState *s, uint16_t asid, dma_addr_t iova) +{ + SMMUIOTLBKey key = {.asid = asid, .iova = iova}; + + trace_smmu_iotlb_inv_iova(asid, iova); + g_hash_table_remove(s->iotlb, &key); +} + +inline void smmu_iotlb_inv_asid(SMMUState *s, uint16_t asid) +{ + trace_smmu_iotlb_inv_asid(asid); + g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_asid, &asid); +} + /* VMSAv8-64 Translation */ /** @@ -XXX,XX +XXX,XX @@ IOMMUMemoryRegion *smmu_iommu_mr(SMMUState *s, uint32_t sid) return NULL; } +static guint smmu_iotlb_key_hash(gconstpointer v) +{ + SMMUIOTLBKey *key = (SMMUIOTLBKey *)v; + uint32_t a, b, c; + + /* Jenkins hash */ + a = b = c = JHASH_INITVAL + sizeof(*key); + a += key->asid; + b += extract64(key->iova, 0, 32); + c += extract64(key->iova, 32, 32); + + __jhash_mix(a, b, c); + __jhash_final(a, b, c); + + return c; +} + +static gboolean smmu_iotlb_key_equal(gconstpointer v1, gconstpointer v2) +{ + const SMMUIOTLBKey *k1 = v1; + const SMMUIOTLBKey *k2 = v2; + + return (k1->asid == k2->asid) && (k1->iova == k2->iova); +} + static void smmu_base_realize(DeviceState *dev, Error **errp) { SMMUState *s = ARM_SMMU(dev); @@ -XXX,XX +XXX,XX @@ static void smmu_base_realize(DeviceState *dev, Error **errp) return; } s->configs = g_hash_table_new_full(NULL, NULL, NULL, g_free); + s->iotlb = g_hash_table_new_full(smmu_iotlb_key_hash, smmu_iotlb_key_equal, + g_free, g_free); s->smmu_pcibus_by_busptr = g_hash_table_new(NULL, NULL); if (s->primary_bus) { @@ -XXX,XX +XXX,XX @@ static void smmu_base_reset(DeviceState *dev) SMMUState *s = ARM_SMMU(dev); g_hash_table_remove_all(s->configs); + g_hash_table_remove_all(s->iotlb); } static Property smmu_dev_properties[] = { diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/smmuv3.c +++ b/hw/arm/smmuv3.c @@ -XXX,XX +XXX,XX @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, SMMUEventInfo event = {.type = SMMU_EVT_NONE, .sid = sid}; SMMUPTWEventInfo ptw_info = {}; SMMUTranslationStatus status; + SMMUState *bs = ARM_SMMU(s); + uint64_t page_mask, aligned_addr; + IOMMUTLBEntry *cached_entry = NULL; + SMMUTransTableInfo *tt; SMMUTransCfg *cfg = NULL; IOMMUTLBEntry entry = { .target_as = &address_space_memory, @@ -XXX,XX +XXX,XX @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, .addr_mask = ~(hwaddr)0, .perm = IOMMU_NONE, }; + SMMUIOTLBKey key, *new_key; qemu_mutex_lock(&s->mutex); @@ -XXX,XX +XXX,XX @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, goto epilogue; } - if (smmu_ptw(cfg, addr, flag, &entry, &ptw_info)) { + tt = select_tt(cfg, addr); + if (!tt) { + if (event.record_trans_faults) { + event.type = SMMU_EVT_F_TRANSLATION; + event.u.f_translation.addr = addr; + event.u.f_translation.rnw = flag & 0x1; + } + status = SMMU_TRANS_ERROR; + goto epilogue; + } + + page_mask = (1ULL << (tt->granule_sz)) - 1; + aligned_addr = addr & ~page_mask; + + key.asid = cfg->asid; + key.iova = aligned_addr; + + cached_entry = g_hash_table_lookup(bs->iotlb, &key); + if (cached_entry) { + cfg->iotlb_hits++; + trace_smmu_iotlb_cache_hit(cfg->asid, aligned_addr, + cfg->iotlb_hits, cfg->iotlb_misses, + 100 * cfg->iotlb_hits / + (cfg->iotlb_hits + cfg->iotlb_misses)); + if ((flag & IOMMU_WO) && !(cached_entry->perm & IOMMU_WO)) { + status = SMMU_TRANS_ERROR; + if (event.record_trans_faults) { + event.type = SMMU_EVT_F_PERMISSION; + event.u.f_permission.addr = addr; + event.u.f_permission.rnw = flag & 0x1; + } + } else { + status = SMMU_TRANS_SUCCESS; + } + goto epilogue; + } + + cfg->iotlb_misses++; + trace_smmu_iotlb_cache_miss(cfg->asid, addr & ~page_mask, + cfg->iotlb_hits, cfg->iotlb_misses, + 100 * cfg->iotlb_hits / + (cfg->iotlb_hits + cfg->iotlb_misses)); + + if (g_hash_table_size(bs->iotlb) >= SMMU_IOTLB_MAX_SIZE) { + smmu_iotlb_inv_all(bs); + } + + cached_entry = g_new0(IOMMUTLBEntry, 1); + + if (smmu_ptw(cfg, aligned_addr, flag, cached_entry, &ptw_info)) { + g_free(cached_entry); switch (ptw_info.type) { case SMMU_PTW_ERR_WALK_EABT: event.type = SMMU_EVT_F_WALK_EABT; @@ -XXX,XX +XXX,XX @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, } status = SMMU_TRANS_ERROR; } else { + new_key = g_new0(SMMUIOTLBKey, 1); + new_key->asid = cfg->asid; + new_key->iova = aligned_addr; + g_hash_table_insert(bs->iotlb, new_key, cached_entry); status = SMMU_TRANS_SUCCESS; } @@ -XXX,XX +XXX,XX @@ epilogue: switch (status) { case SMMU_TRANS_SUCCESS: entry.perm = flag; + entry.translated_addr = cached_entry->translated_addr + + (addr & page_mask); + entry.addr_mask = cached_entry->addr_mask; trace_smmuv3_translate_success(mr->parent_obj.name, sid, addr, entry.translated_addr, entry.perm); break; @@ -XXX,XX +XXX,XX @@ static int smmuv3_cmdq_consume(SMMUv3State *s) smmuv3_flush_config(sdev); break; } - case SMMU_CMD_TLBI_NH_ALL: case SMMU_CMD_TLBI_NH_ASID: - case SMMU_CMD_TLBI_NH_VA: + { + uint16_t asid = CMD_ASID(&cmd); + + trace_smmuv3_cmdq_tlbi_nh_asid(asid); + smmu_iotlb_inv_asid(bs, asid); + break; + } + case SMMU_CMD_TLBI_NH_ALL: + case SMMU_CMD_TLBI_NSNH_ALL: + trace_smmuv3_cmdq_tlbi_nh(); + smmu_iotlb_inv_all(bs); + break; case SMMU_CMD_TLBI_NH_VAA: + { + dma_addr_t addr = CMD_ADDR(&cmd); + uint16_t vmid = CMD_VMID(&cmd); + + trace_smmuv3_cmdq_tlbi_nh_vaa(vmid, addr); + smmu_iotlb_inv_all(bs); + break; + } + case SMMU_CMD_TLBI_NH_VA: + { + uint16_t asid = CMD_ASID(&cmd); + uint16_t vmid = CMD_VMID(&cmd); + dma_addr_t addr = CMD_ADDR(&cmd); + bool leaf = CMD_LEAF(&cmd); + + trace_smmuv3_cmdq_tlbi_nh_va(vmid, asid, addr, leaf); + smmu_iotlb_inv_iova(bs, asid, addr); + break; + } case SMMU_CMD_TLBI_EL3_ALL: case SMMU_CMD_TLBI_EL3_VA: case SMMU_CMD_TLBI_EL2_ALL: @@ -XXX,XX +XXX,XX @@ static int smmuv3_cmdq_consume(SMMUv3State *s) case SMMU_CMD_TLBI_EL2_VAA: case SMMU_CMD_TLBI_S12_VMALL: case SMMU_CMD_TLBI_S2_IPA: - case SMMU_CMD_TLBI_NSNH_ALL: case SMMU_CMD_ATC_INV: case SMMU_CMD_PRI_RESP: case SMMU_CMD_RESUME: diff --git a/hw/arm/trace-events b/hw/arm/trace-events index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/trace-events +++ b/hw/arm/trace-events @@ -XXX,XX +XXX,XX @@ smmu_ptw_invalid_pte(int stage, int level, uint64_t baseaddr, uint64_t pteaddr, smmu_ptw_page_pte(int stage, int level, uint64_t iova, uint64_t baseaddr, uint64_t pteaddr, uint64_t pte, uint64_t address) "stage=%d level=%d iova=0x%"PRIx64" base@=0x%"PRIx64" pte@=0x%"PRIx64" pte=0x%"PRIx64" page address = 0x%"PRIx64 smmu_ptw_block_pte(int stage, int level, uint64_t baseaddr, uint64_t pteaddr, uint64_t pte, uint64_t iova, uint64_t gpa, int bsize_mb) "stage=%d level=%d base@=0x%"PRIx64" pte@=0x%"PRIx64" pte=0x%"PRIx64" iova=0x%"PRIx64" block address = 0x%"PRIx64" block size = %d MiB" smmu_get_pte(uint64_t baseaddr, int index, uint64_t pteaddr, uint64_t pte) "baseaddr=0x%"PRIx64" index=0x%x, pteaddr=0x%"PRIx64", pte=0x%"PRIx64 +smmu_iotlb_cache_hit(uint16_t asid, uint64_t addr, uint32_t hit, uint32_t miss, uint32_t p) "IOTLB cache HIT asid=%d addr=0x%"PRIx64" hit=%d miss=%d hit rate=%d" +smmu_iotlb_cache_miss(uint16_t asid, uint64_t addr, uint32_t hit, uint32_t miss, uint32_t p) "IOTLB cache MISS asid=%d addr=0x%"PRIx64" hit=%d miss=%d hit rate=%d" +smmu_iotlb_inv_all(void) "IOTLB invalidate all" +smmu_iotlb_inv_asid(uint16_t asid) "IOTLB invalidate asid=%d" +smmu_iotlb_inv_iova(uint16_t asid, uint64_t addr) "IOTLB invalidate asid=%d addr=0x%"PRIx64 #hw/arm/smmuv3.c smmuv3_read_mmio(uint64_t addr, uint64_t val, unsigned size, uint32_t r) "addr: 0x%"PRIx64" val:0x%"PRIx64" size: 0x%x(%d)" @@ -XXX,XX +XXX,XX @@ smmuv3_cmdq_cfgi_ste_range(int start, int end) "start=0x%d - end=0x%d" smmuv3_cmdq_cfgi_cd(uint32_t sid) "streamid = %d" smmuv3_config_cache_hit(uint32_t sid, uint32_t hits, uint32_t misses, uint32_t perc) "Config cache HIT for sid %d (hits=%d, misses=%d, hit rate=%d)" smmuv3_config_cache_miss(uint32_t sid, uint32_t hits, uint32_t misses, uint32_t perc) "Config cache MISS for sid %d (hits=%d, misses=%d, hit rate=%d)" +smmuv3_cmdq_tlbi_nh_va(int vmid, int asid, uint64_t addr, bool leaf) "vmid =%d asid =%d addr=0x%"PRIx64" leaf=%d" +smmuv3_cmdq_tlbi_nh_vaa(int vmid, uint64_t addr) "vmid =%d addr=0x%"PRIx64 +smmuv3_cmdq_tlbi_nh(void) "" +smmuv3_cmdq_tlbi_nh_asid(uint16_t asid) "asid=%d" smmuv3_config_cache_inv(uint32_t sid) "Config cache INV for sid %d" -- 2.17.1 From: Eric Auger <eric.auger@redhat.com> On TLB invalidation commands, let's call registered IOMMU notifiers. Those can only be UNMAP notifiers. SMMUv3 does not support notification on MAP (VFIO). This patch allows vhost use case where IOTLB API is notified on each guest IOTLB invalidation. Signed-off-by: Eric Auger <eric.auger@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 1529653501-15358-5-git-send-email-eric.auger@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/smmu-common.h | 6 +++ hw/arm/smmu-common.c | 34 +++++++++++++ hw/arm/smmuv3.c | 99 +++++++++++++++++++++++++++++++++++- hw/arm/trace-events | 5 ++ 4 files changed, 142 insertions(+), 2 deletions(-) diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/smmu-common.h +++ b/include/hw/arm/smmu-common.h @@ -XXX,XX +XXX,XX @@ void smmu_iotlb_inv_all(SMMUState *s); void smmu_iotlb_inv_asid(SMMUState *s, uint16_t asid); void smmu_iotlb_inv_iova(SMMUState *s, uint16_t asid, dma_addr_t iova); +/* Unmap the range of all the notifiers registered to any IOMMU mr */ +void smmu_inv_notifiers_all(SMMUState *s); + +/* Unmap the range of all the notifiers registered to @mr */ +void smmu_inv_notifiers_mr(IOMMUMemoryRegion *mr); + #endif /* HW_ARM_SMMU_COMMON */ diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/smmu-common.c +++ b/hw/arm/smmu-common.c @@ -XXX,XX +XXX,XX @@ static gboolean smmu_iotlb_key_equal(gconstpointer v1, gconstpointer v2) return (k1->asid == k2->asid) && (k1->iova == k2->iova); } +/* Unmap the whole notifier's range */ +static void smmu_unmap_notifier_range(IOMMUNotifier *n) +{ + IOMMUTLBEntry entry; + + entry.target_as = &address_space_memory; + entry.iova = n->start; + entry.perm = IOMMU_NONE; + entry.addr_mask = n->end - n->start; + + memory_region_notify_one(n, &entry); +} + +/* Unmap all notifiers attached to @mr */ +inline void smmu_inv_notifiers_mr(IOMMUMemoryRegion *mr) +{ + IOMMUNotifier *n; + + trace_smmu_inv_notifiers_mr(mr->parent_obj.name); + IOMMU_NOTIFIER_FOREACH(n, mr) { + smmu_unmap_notifier_range(n); + } +} + +/* Unmap all notifiers of all mr's */ +void smmu_inv_notifiers_all(SMMUState *s) +{ + SMMUNotifierNode *node; + + QLIST_FOREACH(node, &s->notifiers_list, next) { + smmu_inv_notifiers_mr(&node->sdev->iommu); + } +} + static void smmu_base_realize(DeviceState *dev, Error **errp) { SMMUState *s = ARM_SMMU(dev); diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/smmuv3.c +++ b/hw/arm/smmuv3.c @@ -XXX,XX +XXX,XX @@ epilogue: return entry; } +/** + * smmuv3_notify_iova - call the notifier @n for a given + * @asid and @iova tuple. + * + * @mr: IOMMU mr region handle + * @n: notifier to be called + * @asid: address space ID or negative value if we don't care + * @iova: iova + */ +static void smmuv3_notify_iova(IOMMUMemoryRegion *mr, + IOMMUNotifier *n, + int asid, + dma_addr_t iova) +{ + SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu); + SMMUEventInfo event = {}; + SMMUTransTableInfo *tt; + SMMUTransCfg *cfg; + IOMMUTLBEntry entry; + + cfg = smmuv3_get_config(sdev, &event); + if (!cfg) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s error decoding the configuration for iommu mr=%s\n", + __func__, mr->parent_obj.name); + return; + } + + if (asid >= 0 && cfg->asid != asid) { + return; + } + + tt = select_tt(cfg, iova); + if (!tt) { + return; + } + + entry.target_as = &address_space_memory; + entry.iova = iova; + entry.addr_mask = (1 << tt->granule_sz) - 1; + entry.perm = IOMMU_NONE; + + memory_region_notify_one(n, &entry); +} + +/* invalidate an asid/iova tuple in all mr's */ +static void smmuv3_inv_notifiers_iova(SMMUState *s, int asid, dma_addr_t iova) +{ + SMMUNotifierNode *node; + + QLIST_FOREACH(node, &s->notifiers_list, next) { + IOMMUMemoryRegion *mr = &node->sdev->iommu; + IOMMUNotifier *n; + + trace_smmuv3_inv_notifiers_iova(mr->parent_obj.name, asid, iova); + + IOMMU_NOTIFIER_FOREACH(n, mr) { + smmuv3_notify_iova(mr, n, asid, iova); + } + } +} + static int smmuv3_cmdq_consume(SMMUv3State *s) { SMMUState *bs = ARM_SMMU(s); @@ -XXX,XX +XXX,XX @@ static int smmuv3_cmdq_consume(SMMUv3State *s) uint16_t asid = CMD_ASID(&cmd); trace_smmuv3_cmdq_tlbi_nh_asid(asid); + smmu_inv_notifiers_all(&s->smmu_state); smmu_iotlb_inv_asid(bs, asid); break; } case SMMU_CMD_TLBI_NH_ALL: case SMMU_CMD_TLBI_NSNH_ALL: trace_smmuv3_cmdq_tlbi_nh(); + smmu_inv_notifiers_all(&s->smmu_state); smmu_iotlb_inv_all(bs); break; case SMMU_CMD_TLBI_NH_VAA: @@ -XXX,XX +XXX,XX @@ static int smmuv3_cmdq_consume(SMMUv3State *s) uint16_t vmid = CMD_VMID(&cmd); trace_smmuv3_cmdq_tlbi_nh_vaa(vmid, addr); + smmuv3_inv_notifiers_iova(bs, -1, addr); smmu_iotlb_inv_all(bs); break; } @@ -XXX,XX +XXX,XX @@ static int smmuv3_cmdq_consume(SMMUv3State *s) bool leaf = CMD_LEAF(&cmd); trace_smmuv3_cmdq_tlbi_nh_va(vmid, asid, addr, leaf); + smmuv3_inv_notifiers_iova(bs, asid, addr); smmu_iotlb_inv_iova(bs, asid, addr); break; } @@ -XXX,XX +XXX,XX @@ static void smmuv3_notify_flag_changed(IOMMUMemoryRegion *iommu, IOMMUNotifierFlag old, IOMMUNotifierFlag new) { + SMMUDevice *sdev = container_of(iommu, SMMUDevice, iommu); + SMMUv3State *s3 = sdev->smmu; + SMMUState *s = &(s3->smmu_state); + SMMUNotifierNode *node = NULL; + SMMUNotifierNode *next_node = NULL; + + if (new & IOMMU_NOTIFIER_MAP) { + int bus_num = pci_bus_num(sdev->bus); + PCIDevice *pcidev = pci_find_device(sdev->bus, bus_num, sdev->devfn); + + warn_report("SMMUv3 does not support notification on MAP: " + "device %s will not function properly", pcidev->name); + } + if (old == IOMMU_NOTIFIER_NONE) { - warn_report("SMMUV3 does not support vhost/vfio integration yet: " - "devices of those types will not function properly"); + trace_smmuv3_notify_flag_add(iommu->parent_obj.name); + node = g_malloc0(sizeof(*node)); + node->sdev = sdev; + QLIST_INSERT_HEAD(&s->notifiers_list, node, next); + return; + } + + /* update notifier node with new flags */ + QLIST_FOREACH_SAFE(node, &s->notifiers_list, next, next_node) { + if (node->sdev == sdev) { + if (new == IOMMU_NOTIFIER_NONE) { + trace_smmuv3_notify_flag_del(iommu->parent_obj.name); + QLIST_REMOVE(node, next); + g_free(node); + } + return; + } } } diff --git a/hw/arm/trace-events b/hw/arm/trace-events index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/trace-events +++ b/hw/arm/trace-events @@ -XXX,XX +XXX,XX @@ smmu_iotlb_cache_miss(uint16_t asid, uint64_t addr, uint32_t hit, uint32_t miss, smmu_iotlb_inv_all(void) "IOTLB invalidate all" smmu_iotlb_inv_asid(uint16_t asid) "IOTLB invalidate asid=%d" smmu_iotlb_inv_iova(uint16_t asid, uint64_t addr) "IOTLB invalidate asid=%d addr=0x%"PRIx64 +smmu_inv_notifiers_mr(const char *name) "iommu mr=%s" #hw/arm/smmuv3.c smmuv3_read_mmio(uint64_t addr, uint64_t val, unsigned size, uint32_t r) "addr: 0x%"PRIx64" val:0x%"PRIx64" size: 0x%x(%d)" @@ -XXX,XX +XXX,XX @@ smmuv3_cmdq_tlbi_nh_vaa(int vmid, uint64_t addr) "vmid =%d addr=0x%"PRIx64 smmuv3_cmdq_tlbi_nh(void) "" smmuv3_cmdq_tlbi_nh_asid(uint16_t asid) "asid=%d" smmuv3_config_cache_inv(uint32_t sid) "Config cache INV for sid %d" +smmuv3_notify_flag_add(const char *iommu) "ADD SMMUNotifier node for iommu mr=%s" +smmuv3_notify_flag_del(const char *iommu) "DEL SMMUNotifier node for iommu mr=%s" +smmuv3_inv_notifiers_iova(const char *name, uint16_t asid, uint64_t iova) "iommu mr=%s asid=%d iova=0x%"PRIx64 + -- 2.17.1 From: Cédric Le Goater <clg@kaod.org> All Aspeed SoC clocks are driven by an input source clock which can have different frequencies : 24MHz or 25MHz, and also, on the Aspeed AST2400 SoC, 48MHz. The H-PLL (CPU) clock is defined from a calculation using parameters in the H-PLL Parameter register or from a predefined set of frequencies if the setting is strapped by hardware (Aspeed AST2400 SoC). The other clocks of the SoC are then defined from the H-PLL using dividers. We introduce first the APB clock because it should be used to drive the Aspeed timer model. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Andrew Jeffery <andrew@aj.id.au> Message-id: 20180622075700.5923-2-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/misc/aspeed_scu.h | 70 +++++++++++++++++++++-- hw/misc/aspeed_scu.c | 106 +++++++++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 4 deletions(-) diff --git a/include/hw/misc/aspeed_scu.h b/include/hw/misc/aspeed_scu.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/misc/aspeed_scu.h +++ b/include/hw/misc/aspeed_scu.h @@ -XXX,XX +XXX,XX @@ typedef struct AspeedSCUState { uint32_t hw_strap1; uint32_t hw_strap2; uint32_t hw_prot_key; + + uint32_t clkin; + uint32_t hpll; + uint32_t apb_freq; } AspeedSCUState; #define AST2400_A0_SILICON_REV 0x02000303U @@ -XXX,XX +XXX,XX @@ extern bool is_supported_silicon_rev(uint32_t silicon_rev); * 1. 2012/12/29 Ryan Chen Create */ -/* Hardware Strapping Register definition (for Aspeed AST2400 SOC) +/* SCU08 Clock Selection Register + * + * 31 Enable Video Engine clock dynamic slow down + * 30:28 Video Engine clock slow down setting + * 27 2D Engine GCLK clock source selection + * 26 2D Engine GCLK clock throttling enable + * 25:23 APB PCLK divider selection + * 22:20 LPC Host LHCLK divider selection + * 19 LPC Host LHCLK clock generation/output enable control + * 18:16 MAC AHB bus clock divider selection + * 15 SD/SDIO clock running enable + * 14:12 SD/SDIO divider selection + * 11 Reserved + * 10:8 Video port output clock delay control bit + * 7 ARM CPU/AHB clock slow down enable + * 6:4 ARM CPU/AHB clock slow down setting + * 3:2 ECLK clock source selection + * 1 CPU/AHB clock slow down idle timer + * 0 CPU/AHB clock dynamic slow down enable (defined in bit[6:4]) + */ +#define SCU_CLK_GET_PCLK_DIV(x) (((x) >> 23) & 0x7) + +/* SCU24 H-PLL Parameter Register (for Aspeed AST2400 SOC) + * + * 18 H-PLL parameter selection + * 0: Select H-PLL by strapping resistors + * 1: Select H-PLL by the programmed registers (SCU24[17:0]) + * 17 Enable H-PLL bypass mode + * 16 Turn off H-PLL + * 10:5 H-PLL Numerator + * 4 H-PLL Output Divider + * 3:0 H-PLL Denumerator + * + * (Output frequency) = 24MHz * (2-OD) * [(Numerator+2) / (Denumerator+1)] + */ + +#define SCU_AST2400_H_PLL_PROGRAMMED (0x1 << 18) +#define SCU_AST2400_H_PLL_BYPASS_EN (0x1 << 17) +#define SCU_AST2400_H_PLL_OFF (0x1 << 16) + +/* SCU24 H-PLL Parameter Register (for Aspeed AST2500 SOC) + * + * 21 Enable H-PLL reset + * 20 Enable H-PLL bypass mode + * 19 Turn off H-PLL + * 18:13 H-PLL Post Divider + * 12:5 H-PLL Numerator (M) + * 4:0 H-PLL Denumerator (N) + * + * (Output frequency) = CLKIN(24MHz) * [(M+1) / (N+1)] / (P+1) + * + * The default frequency is 792Mhz when CLKIN = 24MHz + */ + +#define SCU_H_PLL_BYPASS_EN (0x1 << 20) +#define SCU_H_PLL_OFF (0x1 << 19) + +/* SCU70 Hardware Strapping Register definition (for Aspeed AST2400 SOC) * * 31:29 Software defined strapping registers * 28:27 DRAM size setting (for VGA driver use) @@ -XXX,XX +XXX,XX @@ extern bool is_supported_silicon_rev(uint32_t silicon_rev); #define SCU_AST2400_HW_STRAP_GET_CLK_SOURCE(x) (((((x) >> 23) & 0x1) << 1) \ | (((x) >> 18) & 0x1)) #define SCU_AST2400_HW_STRAP_CLK_SOURCE_MASK ((0x1 << 23) | (0x1 << 18)) -#define AST2400_CLK_25M_IN (0x1 << 23) +#define SCU_HW_STRAP_CLK_25M_IN (0x1 << 23) #define AST2400_CLK_24M_IN 0 #define AST2400_CLK_48M_IN 1 #define AST2400_CLK_25M_IN_24M_USB_CKI 2 #define AST2400_CLK_25M_IN_48M_USB_CKI 3 +#define SCU_HW_STRAP_CLK_48M_IN (0x1 << 18) #define SCU_HW_STRAP_2ND_BOOT_WDT (0x1 << 17) #define SCU_HW_STRAP_SUPER_IO_CONFIG (0x1 << 16) #define SCU_HW_STRAP_VGA_CLASS_CODE (0x1 << 15) @@ -XXX,XX +XXX,XX @@ extern bool is_supported_silicon_rev(uint32_t silicon_rev); #define AST2400_DIS_BOOT 3 /* - * Hardware strapping register definition (for Aspeed AST2500 SoC and - * higher) + * SCU70 Hardware strapping register definition (for Aspeed AST2500 + * SoC and higher) * * 31 Enable SPI Flash Strap Auto Fetch Mode * 30 Enable GPIO Strap Mode diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c index XXXXXXX..XXXXXXX 100644 --- a/hw/misc/aspeed_scu.c +++ b/hw/misc/aspeed_scu.c @@ -XXX,XX +XXX,XX @@ static uint32_t aspeed_scu_get_random(void) return num; } +static void aspeed_scu_set_apb_freq(AspeedSCUState *s) +{ + uint32_t apb_divider; + + switch (s->silicon_rev) { + case AST2400_A0_SILICON_REV: + case AST2400_A1_SILICON_REV: + apb_divider = 2; + break; + case AST2500_A0_SILICON_REV: + case AST2500_A1_SILICON_REV: + apb_divider = 4; + break; + default: + g_assert_not_reached(); + } + + s->apb_freq = s->hpll / (SCU_CLK_GET_PCLK_DIV(s->regs[CLK_SEL]) + 1) + / apb_divider; +} + static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size) { AspeedSCUState *s = ASPEED_SCU(opaque); @@ -XXX,XX +XXX,XX @@ static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data, case PROT_KEY: s->regs[reg] = (data == ASPEED_SCU_PROT_KEY) ? 1 : 0; return; + case CLK_SEL: + s->regs[reg] = data; + aspeed_scu_set_apb_freq(s); + break; case FREQ_CNTR_EVAL: case VGA_SCRATCH1 ... VGA_SCRATCH8: @@ -XXX,XX +XXX,XX @@ static const MemoryRegionOps aspeed_scu_ops = { .valid.unaligned = false, }; +static uint32_t aspeed_scu_get_clkin(AspeedSCUState *s) +{ + if (s->hw_strap1 & SCU_HW_STRAP_CLK_25M_IN) { + return 25000000; + } else if (s->hw_strap1 & SCU_HW_STRAP_CLK_48M_IN) { + return 48000000; + } else { + return 24000000; + } +} + +/* + * Strapped frequencies for the AST2400 in MHz. They depend on the + * clkin frequency. + */ +static const uint32_t hpll_ast2400_freqs[][4] = { + { 384, 360, 336, 408 }, /* 24MHz or 48MHz */ + { 400, 375, 350, 425 }, /* 25MHz */ +}; + +static uint32_t aspeed_scu_calc_hpll_ast2400(AspeedSCUState *s) +{ + uint32_t hpll_reg = s->regs[HPLL_PARAM]; + uint8_t freq_select; + bool clk_25m_in; + + if (hpll_reg & SCU_AST2400_H_PLL_OFF) { + return 0; + } + + if (hpll_reg & SCU_AST2400_H_PLL_PROGRAMMED) { + uint32_t multiplier = 1; + + if (!(hpll_reg & SCU_AST2400_H_PLL_BYPASS_EN)) { + uint32_t n = (hpll_reg >> 5) & 0x3f; + uint32_t od = (hpll_reg >> 4) & 0x1; + uint32_t d = hpll_reg & 0xf; + + multiplier = (2 - od) * ((n + 2) / (d + 1)); + } + + return s->clkin * multiplier; + } + + /* HW strapping */ + clk_25m_in = !!(s->hw_strap1 & SCU_HW_STRAP_CLK_25M_IN); + freq_select = SCU_AST2400_HW_STRAP_GET_H_PLL_CLK(s->hw_strap1); + + return hpll_ast2400_freqs[clk_25m_in][freq_select] * 1000000; +} + +static uint32_t aspeed_scu_calc_hpll_ast2500(AspeedSCUState *s) +{ + uint32_t hpll_reg = s->regs[HPLL_PARAM]; + uint32_t multiplier = 1; + + if (hpll_reg & SCU_H_PLL_OFF) { + return 0; + } + + if (!(hpll_reg & SCU_H_PLL_BYPASS_EN)) { + uint32_t p = (hpll_reg >> 13) & 0x3f; + uint32_t m = (hpll_reg >> 5) & 0xff; + uint32_t n = hpll_reg & 0x1f; + + multiplier = ((m + 1) / (n + 1)) / (p + 1); + } + + return s->clkin * multiplier; +} + static void aspeed_scu_reset(DeviceState *dev) { AspeedSCUState *s = ASPEED_SCU(dev); const uint32_t *reset; + uint32_t (*calc_hpll)(AspeedSCUState *s); switch (s->silicon_rev) { case AST2400_A0_SILICON_REV: case AST2400_A1_SILICON_REV: reset = ast2400_a0_resets; + calc_hpll = aspeed_scu_calc_hpll_ast2400; break; case AST2500_A0_SILICON_REV: case AST2500_A1_SILICON_REV: reset = ast2500_a1_resets; + calc_hpll = aspeed_scu_calc_hpll_ast2500; break; default: g_assert_not_reached(); @@ -XXX,XX +XXX,XX @@ static void aspeed_scu_reset(DeviceState *dev) s->regs[HW_STRAP1] = s->hw_strap1; s->regs[HW_STRAP2] = s->hw_strap2; s->regs[PROT_KEY] = s->hw_prot_key; + + /* + * All registers are set. Now compute the frequencies of the main clocks + */ + s->clkin = aspeed_scu_get_clkin(s); + s->hpll = calc_hpll(s); + aspeed_scu_set_apb_freq(s); } static uint32_t aspeed_silicon_revs[] = { -- 2.17.1 From: Cédric Le Goater <clg@kaod.org> The System Control Unit should be initialized first as it drives all the configuration of the SoC and other device models. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Acked-by: Andrew Jeffery <andrew@aj.id.au> Message-id: 20180622075700.5923-3-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/aspeed_soc.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed_soc.c +++ b/hw/arm/aspeed_soc.c @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_init(Object *obj) object_initialize(&s->cpu, sizeof(s->cpu), sc->info->cpu_type); object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL); - object_initialize(&s->vic, sizeof(s->vic), TYPE_ASPEED_VIC); - object_property_add_child(obj, "vic", OBJECT(&s->vic), NULL); - qdev_set_parent_bus(DEVICE(&s->vic), sysbus_get_default()); - - object_initialize(&s->timerctrl, sizeof(s->timerctrl), TYPE_ASPEED_TIMER); - object_property_add_child(obj, "timerctrl", OBJECT(&s->timerctrl), NULL); - qdev_set_parent_bus(DEVICE(&s->timerctrl), sysbus_get_default()); - - object_initialize(&s->i2c, sizeof(s->i2c), TYPE_ASPEED_I2C); - object_property_add_child(obj, "i2c", OBJECT(&s->i2c), NULL); - qdev_set_parent_bus(DEVICE(&s->i2c), sysbus_get_default()); - object_initialize(&s->scu, sizeof(s->scu), TYPE_ASPEED_SCU); object_property_add_child(obj, "scu", OBJECT(&s->scu), NULL); qdev_set_parent_bus(DEVICE(&s->scu), sysbus_get_default()); @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_init(Object *obj) object_property_add_alias(obj, "hw-prot-key", OBJECT(&s->scu), "hw-prot-key", &error_abort); + object_initialize(&s->vic, sizeof(s->vic), TYPE_ASPEED_VIC); + object_property_add_child(obj, "vic", OBJECT(&s->vic), NULL); + qdev_set_parent_bus(DEVICE(&s->vic), sysbus_get_default()); + + object_initialize(&s->timerctrl, sizeof(s->timerctrl), TYPE_ASPEED_TIMER); + object_property_add_child(obj, "timerctrl", OBJECT(&s->timerctrl), NULL); + qdev_set_parent_bus(DEVICE(&s->timerctrl), sysbus_get_default()); + + object_initialize(&s->i2c, sizeof(s->i2c), TYPE_ASPEED_I2C); + object_property_add_child(obj, "i2c", OBJECT(&s->i2c), NULL); + qdev_set_parent_bus(DEVICE(&s->i2c), sysbus_get_default()); + object_initialize(&s->fmc, sizeof(s->fmc), sc->info->fmc_typename); object_property_add_child(obj, "fmc", OBJECT(&s->fmc), NULL); qdev_set_parent_bus(DEVICE(&s->fmc), sysbus_get_default()); @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) memory_region_add_subregion(get_system_memory(), ASPEED_SOC_SRAM_BASE, &s->sram); + /* SCU */ + object_property_set_bool(OBJECT(&s->scu), true, "realized", &err); + if (err) { + error_propagate(errp, err); + return; + } + sysbus_mmio_map(SYS_BUS_DEVICE(&s->scu), 0, ASPEED_SOC_SCU_BASE); + /* VIC */ object_property_set_bool(OBJECT(&s->vic), true, "realized", &err); if (err) { @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) sysbus_connect_irq(SYS_BUS_DEVICE(&s->timerctrl), i, irq); } - /* SCU */ - object_property_set_bool(OBJECT(&s->scu), true, "realized", &err); - if (err) { - error_propagate(errp, err); - return; - } - sysbus_mmio_map(SYS_BUS_DEVICE(&s->scu), 0, ASPEED_SOC_SCU_BASE); - /* UART - attach an 8250 to the IO space as our UART5 */ if (serial_hd(0)) { qemu_irq uart5 = qdev_get_gpio_in(DEVICE(&s->vic), uart_irqs[4]); -- 2.17.1 From: Cédric Le Goater <clg@kaod.org> The timer controller can be driven by either an external 1MHz clock or by the APB clock. Today, the model makes the assumption that the APB frequency is always set to 24MHz but this is incorrect. The AST2400 SoC on the palmetto machines uses a 48MHz input clock source and the APB can be set to 48MHz. The consequence is a general system slowdown. The QEMU machines using the AST2500 SoC do not seem impacted today because the APB frequency is still set to 24MHz. We fix the timer frequency for all SoCs by linking the Timer model to the SCU model. The APB frequency driving the timers is now the one configured for the SoC. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Reviewed-by: Andrew Jeffery <andrew@aj.id.au> Message-id: 20180622075700.5923-4-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/timer/aspeed_timer.h | 4 ++++ hw/arm/aspeed_soc.c | 2 ++ hw/timer/aspeed_timer.c | 19 +++++++++++++++---- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/include/hw/timer/aspeed_timer.h b/include/hw/timer/aspeed_timer.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/timer/aspeed_timer.h +++ b/include/hw/timer/aspeed_timer.h @@ -XXX,XX +XXX,XX @@ #include "qemu/timer.h" +typedef struct AspeedSCUState AspeedSCUState; + #define ASPEED_TIMER(obj) \ OBJECT_CHECK(AspeedTimerCtrlState, (obj), TYPE_ASPEED_TIMER); #define TYPE_ASPEED_TIMER "aspeed.timer" @@ -XXX,XX +XXX,XX @@ typedef struct AspeedTimerCtrlState { uint32_t ctrl; uint32_t ctrl2; AspeedTimer timers[ASPEED_TIMER_NR_TIMERS]; + + AspeedSCUState *scu; } AspeedTimerCtrlState; #endif /* ASPEED_TIMER_H */ diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed_soc.c +++ b/hw/arm/aspeed_soc.c @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_init(Object *obj) object_initialize(&s->timerctrl, sizeof(s->timerctrl), TYPE_ASPEED_TIMER); object_property_add_child(obj, "timerctrl", OBJECT(&s->timerctrl), NULL); + object_property_add_const_link(OBJECT(&s->timerctrl), "scu", + OBJECT(&s->scu), &error_abort); qdev_set_parent_bus(DEVICE(&s->timerctrl), sysbus_get_default()); object_initialize(&s->i2c, sizeof(s->i2c), TYPE_ASPEED_I2C); diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c index XXXXXXX..XXXXXXX 100644 --- a/hw/timer/aspeed_timer.c +++ b/hw/timer/aspeed_timer.c @@ -XXX,XX +XXX,XX @@ */ #include "qemu/osdep.h" +#include "qapi/error.h" #include "hw/sysbus.h" #include "hw/timer/aspeed_timer.h" +#include "hw/misc/aspeed_scu.h" #include "qemu-common.h" #include "qemu/bitops.h" #include "qemu/timer.h" @@ -XXX,XX +XXX,XX @@ #define TIMER_CLOCK_USE_EXT true #define TIMER_CLOCK_EXT_HZ 1000000 #define TIMER_CLOCK_USE_APB false -#define TIMER_CLOCK_APB_HZ 24000000 #define TIMER_REG_STATUS 0 #define TIMER_REG_RELOAD 1 @@ -XXX,XX +XXX,XX @@ static inline bool timer_external_clock(AspeedTimer *t) return timer_ctrl_status(t, op_external_clock); } -static uint32_t clock_rates[] = { TIMER_CLOCK_APB_HZ, TIMER_CLOCK_EXT_HZ }; - static inline uint32_t calculate_rate(struct AspeedTimer *t) { - return clock_rates[timer_external_clock(t)]; + AspeedTimerCtrlState *s = timer_to_ctrl(t); + + return timer_external_clock(t) ? TIMER_CLOCK_EXT_HZ : s->scu->apb_freq; } static inline uint32_t calculate_ticks(struct AspeedTimer *t, uint64_t now_ns) @@ -XXX,XX +XXX,XX @@ static void aspeed_timer_realize(DeviceState *dev, Error **errp) int i; SysBusDevice *sbd = SYS_BUS_DEVICE(dev); AspeedTimerCtrlState *s = ASPEED_TIMER(dev); + Object *obj; + Error *err = NULL; + + obj = object_property_get_link(OBJECT(dev), "scu", &err); + if (!obj) { + error_propagate(errp, err); + error_prepend(errp, "required link 'scu' not found: "); + return; + } + s->scu = ASPEED_SCU(obj); for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) { aspeed_init_one_timer(s, i); -- 2.17.1