First arm pullreq of 5.0! The following changes since commit 084a398bf8aa7634738e6c6c0103236ee1b3b72f: Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-12-13 18:14:07 +0000) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20191216-1 for you to fetch changes up to f80741d107673f162e3b097fc76a1590036cc9d1: target/arm: ensure we use current exception state after SCR update (2019-12-16 10:52:58 +0000) ---------------------------------------------------------------- target-arm queue: * Add support for Cortex-M7 CPU * exynos4210_gic: Suppress gcc9 format-truncation warnings * aspeed: Various minor bug fixes and improvements * aspeed: Add support for the tacoma-bmc board * Honour HCR_EL32.TID1 and .TID2 trapping requirements * Handle trapping to EL2 of AArch32 VMRS instructions * Handle AArch32 CP15 trapping via HSTR_EL2 * Add support for missing Jazelle system registers * arm/arm-powerctl: set NSACR.{CP11, CP10} bits in arm_set_cpu_on * Add support for DC CVAP & DC CVADP instructions * Fix assertion when SCR.NS is changed in Secure-SVC &c * enable SHPC native hot plug in arm ACPI ---------------------------------------------------------------- Alex Bennée (1): target/arm: ensure we use current exception state after SCR update Beata Michalska (4): tcg: cputlb: Add probe_read Memory: Enable writeback for given memory region migration: ram: Switch to ram block writeback target/arm: Add support for DC CVAP & DC CVADP ins Christophe Lyon (1): target/arm: Add support for cortex-m7 CPU Cédric Le Goater (12): aspeed/i2c: Add support for pool buffer transfers aspeed/i2c: Check SRAM enablement on AST2500 aspeed: Add a DRAM memory region at the SoC level aspeed/i2c: Add support for DMA transfers aspeed/i2c: Add trace events aspeed/smc: Restore default AHB window mapping at reset aspeed/smc: Do not map disabled segment on the AST2600 aspeed/smc: Add AST2600 timings registers aspeed: Remove AspeedBoardConfig array and use AspeedMachineClass aspeed: Add support for the tacoma-bmc board aspeed: Change the "scu" property definition aspeed: Change the "nic" property definition David Gibson (1): exynos4210_gic: Suppress gcc9 format-truncation warnings Heyi Guo (2): hw/arm/acpi: simplify AML bit and/or statement hw/arm/acpi: enable SHPC native hot plug Joel Stanley (4): aspeed/sdmc: Make ast2600 default 1G aspeed/scu: Fix W1C behavior watchdog/aspeed: Improve watchdog timeout message watchdog/aspeed: Fix AST2600 frequency behaviour Marc Zyngier (5): target/arm: Honor HCR_EL2.TID2 trapping requirements target/arm: Honor HCR_EL2.TID1 trapping requirements target/arm: Handle trapping to EL2 of AArch32 VMRS instructions target/arm: Handle AArch32 CP15 trapping via HSTR_EL2 target/arm: Add support for missing Jazelle system registers Niek Linnenbank (1): arm/arm-powerctl: set NSACR.{CP11, CP10} bits in arm_set_cpu_on() PanNengyuan (1): gpio: fix memory leak in aspeed_gpio_init() Philippe Mathieu-Daudé (2): hw/arm/sbsa-ref: Simplify by moving the gic in the machine state hw/arm/virt: Simplify by moving the gic in the machine state include/exec/exec-all.h | 6 + include/exec/memory.h | 6 + include/exec/ram_addr.h | 8 + include/hw/arm/aspeed.h | 24 +-- include/hw/arm/aspeed_soc.h | 1 + include/hw/arm/virt.h | 1 + include/hw/i2c/aspeed_i2c.h | 16 ++ include/hw/ssi/aspeed_smc.h | 1 + include/hw/watchdog/wdt_aspeed.h | 1 + include/qemu/cutils.h | 1 + target/arm/cpu.h | 20 +- target/arm/helper.h | 3 + target/arm/translate.h | 2 + exec.c | 36 ++++ hw/arm/aspeed.c | 271 +++++++++++++---------- hw/arm/aspeed_ast2600.c | 25 ++- hw/arm/aspeed_soc.c | 22 +- hw/arm/sbsa-ref.c | 86 ++++---- hw/arm/virt-acpi-build.c | 21 +- hw/arm/virt.c | 109 +++++----- hw/gpio/aspeed_gpio.c | 1 + hw/i2c/aspeed_i2c.c | 439 +++++++++++++++++++++++++++++++++++--- hw/intc/exynos4210_gic.c | 9 +- hw/misc/aspeed_scu.c | 19 +- hw/misc/aspeed_sdmc.c | 6 +- hw/net/ftgmac100.c | 19 +- hw/ssi/aspeed_smc.c | 63 ++++-- hw/timer/aspeed_timer.c | 17 +- hw/watchdog/wdt_aspeed.c | 41 ++-- linux-user/elfload.c | 2 + memory.c | 12 ++ migration/ram.c | 5 +- target/arm/arm-powerctl.c | 3 + target/arm/cpu.c | 33 +++ target/arm/cpu64.c | 1 + target/arm/helper.c | 170 ++++++++++++++- target/arm/op_helper.c | 22 ++ target/arm/translate-vfp.inc.c | 20 +- target/arm/translate.c | 9 +- target/arm/vfp_helper.c | 29 +++ util/cutils.c | 38 ++++ hw/i2c/trace-events | 9 + tests/data/acpi/virt/DSDT | Bin 18470 -> 18462 bytes tests/data/acpi/virt/DSDT.memhp | Bin 19807 -> 19799 bytes tests/data/acpi/virt/DSDT.numamem | Bin 18470 -> 18462 bytes 45 files changed, 1273 insertions(+), 354 deletions(-) From: Christophe Lyon <christophe.lyon@linaro.org> This is derived from cortex-m4 description, adding DP support and FPv5 instructions with the corresponding flags in isar and mvfr2. Checked that it could successfully execute vrinta.f32 s15, s15 while cortex-m4 emulation rejects it with "illegal instruction". Signed-off-by: Christophe Lyon <christophe.lyon@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20191025090841.10299-1-christophe.lyon@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) 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_m4_initfn(Object *obj) cpu->isar.id_isar6 = 0x00000000; } +static void cortex_m7_initfn(Object *obj) +{ + ARMCPU *cpu = ARM_CPU(obj); + + set_feature(&cpu->env, ARM_FEATURE_V7); + set_feature(&cpu->env, ARM_FEATURE_M); + set_feature(&cpu->env, ARM_FEATURE_M_MAIN); + set_feature(&cpu->env, ARM_FEATURE_THUMB_DSP); + set_feature(&cpu->env, ARM_FEATURE_VFP4); + cpu->midr = 0x411fc272; /* r1p2 */ + cpu->pmsav7_dregion = 8; + cpu->isar.mvfr0 = 0x10110221; + cpu->isar.mvfr1 = 0x12000011; + cpu->isar.mvfr2 = 0x00000040; + cpu->id_pfr0 = 0x00000030; + cpu->id_pfr1 = 0x00000200; + cpu->id_dfr0 = 0x00100000; + cpu->id_afr0 = 0x00000000; + cpu->id_mmfr0 = 0x00100030; + cpu->id_mmfr1 = 0x00000000; + cpu->id_mmfr2 = 0x01000000; + cpu->id_mmfr3 = 0x00000000; + cpu->isar.id_isar0 = 0x01101110; + cpu->isar.id_isar1 = 0x02112000; + cpu->isar.id_isar2 = 0x20232231; + cpu->isar.id_isar3 = 0x01111131; + cpu->isar.id_isar4 = 0x01310132; + cpu->isar.id_isar5 = 0x00000000; + cpu->isar.id_isar6 = 0x00000000; +} + static void cortex_m33_initfn(Object *obj) { ARMCPU *cpu = ARM_CPU(obj); @@ -XXX,XX +XXX,XX @@ static const ARMCPUInfo arm_cpus[] = { .class_init = arm_v7m_class_init }, { .name = "cortex-m4", .initfn = cortex_m4_initfn, .class_init = arm_v7m_class_init }, + { .name = "cortex-m7", .initfn = cortex_m7_initfn, + .class_init = arm_v7m_class_init }, { .name = "cortex-m33", .initfn = cortex_m33_initfn, .class_init = arm_v7m_class_init }, { .name = "cortex-r5", .initfn = cortex_r5_initfn }, -- 2.20.1 From: David Gibson <david@gibson.dropbear.id.au> exynos4210_gic_realize() prints the number of cpus into some temporary buffers, but it only allows 3 bytes space for it. That's plenty: existing machines will only ever set this value to EXYNOS4210_NCPUS (2). But the compiler can't always figure that out, so some[*] gcc9 versions emit -Wformat-truncation warnings. We can fix that by hinting the constraint to the compiler with a suitably placed assert(). [*] The bizarre thing here, is that I've long gotten these warnings compiling in a 32-bit x86 container as host - Fedora 30 with gcc-9.2.1-1.fc30.i686 - but it compiles just fine on my normal x86_64 host - Fedora 30 with and gcc-9.2.1-1.fc30.x86_64. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> [PMM: deleted stray blank line] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/exynos4210_gic.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/exynos4210_gic.c +++ b/hw/intc/exynos4210_gic.c @@ -XXX,XX +XXX,XX @@ static void exynos4210_gic_realize(DeviceState *dev, Error **errp) char cpu_alias_name[sizeof(cpu_prefix) + 3]; char dist_alias_name[sizeof(cpu_prefix) + 3]; SysBusDevice *gicbusdev; + uint32_t n = s->num_cpu; uint32_t i; s->gic = qdev_create(NULL, "arm_gic"); @@ -XXX,XX +XXX,XX @@ static void exynos4210_gic_realize(DeviceState *dev, Error **errp) memory_region_init(&s->dist_container, obj, "exynos4210-dist-container", EXYNOS4210_EXT_GIC_DIST_REGION_SIZE); - for (i = 0; i < s->num_cpu; i++) { + /* + * This clues in gcc that our on-stack buffers do, in fact have + * enough room for the cpu numbers. gcc 9.2.1 on 32-bit x86 + * doesn't figure this out, otherwise and gives spurious warnings. + */ + assert(n <= EXYNOS4210_NCPUS); + for (i = 0; i < n; i++) { /* Map CPU interface per SMP Core */ sprintf(cpu_alias_name, "%s%x", cpu_prefix, i); memory_region_init_alias(&s->cpu_alias[i], obj, -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> The Aspeed I2C controller can operate in different transfer modes : - Byte Buffer mode, using a dedicated register to transfer a byte. This is what the model supports today. - Pool Buffer mode, using an internal SRAM to transfer multiple bytes in the same command sequence. Each SoC has different SRAM characteristics. On the AST2400, 2048 bytes of SRAM are available at offset 0x800 of the controller AHB window. The pool buffer can be configured from 1 to 256 bytes per bus. On the AST2500, the SRAM is at offset 0x200 and the pool buffer is of 16 bytes per bus. On the AST2600, the SRAM is at offset 0xC00 and the pool buffer is of 32 bytes per bus. It can be splitted in two for TX and RX but the current model does not add support for it as it it unused by known drivers. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Tested-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-2-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/i2c/aspeed_i2c.h | 8 ++ hw/i2c/aspeed_i2c.c | 197 ++++++++++++++++++++++++++++++++---- 2 files changed, 186 insertions(+), 19 deletions(-) diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/i2c/aspeed_i2c.h +++ b/include/hw/i2c/aspeed_i2c.h @@ -XXX,XX +XXX,XX @@ OBJECT_CHECK(AspeedI2CState, (obj), TYPE_ASPEED_I2C) #define ASPEED_I2C_NR_BUSSES 16 +#define ASPEED_I2C_MAX_POOL_SIZE 0x800 struct AspeedI2CState; @@ -XXX,XX +XXX,XX @@ typedef struct AspeedI2CBus { uint32_t intr_status; uint32_t cmd; uint32_t buf; + uint32_t pool_ctrl; } AspeedI2CBus; typedef struct AspeedI2CState { @@ -XXX,XX +XXX,XX @@ typedef struct AspeedI2CState { qemu_irq irq; uint32_t intr_status; + MemoryRegion pool_iomem; + uint8_t pool[ASPEED_I2C_MAX_POOL_SIZE]; AspeedI2CBus busses[ASPEED_I2C_NR_BUSSES]; } AspeedI2CState; @@ -XXX,XX +XXX,XX @@ typedef struct AspeedI2CClass { uint8_t reg_size; uint8_t gap; qemu_irq (*bus_get_irq)(AspeedI2CBus *); + + uint64_t pool_size; + hwaddr pool_base; + uint8_t *(*bus_pool_base)(AspeedI2CBus *); } AspeedI2CClass; I2CBus *aspeed_i2c_get_bus(DeviceState *dev, int busnr); diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c index XXXXXXX..XXXXXXX 100644 --- a/hw/i2c/aspeed_i2c.c +++ b/hw/i2c/aspeed_i2c.c @@ -XXX,XX +XXX,XX @@ /* I2C Device (Bus) Register */ #define I2CD_FUN_CTRL_REG 0x00 /* I2CD Function Control */ -#define I2CD_BUFF_SEL_MASK (0x7 << 20) -#define I2CD_BUFF_SEL(x) (x << 20) +#define I2CD_POOL_PAGE_SEL(x) (((x) >> 20) & 0x7) /* AST2400 */ #define I2CD_M_SDA_LOCK_EN (0x1 << 16) #define I2CD_MULTI_MASTER_DIS (0x1 << 15) #define I2CD_M_SCL_DRIVE_EN (0x1 << 14) @@ -XXX,XX +XXX,XX @@ #define I2CD_SCL_O_OUT_DIR (0x1 << 12) #define I2CD_BUS_RECOVER_CMD_EN (0x1 << 11) #define I2CD_S_ALT_EN (0x1 << 10) -#define I2CD_RX_DMA_ENABLE (0x1 << 9) -#define I2CD_TX_DMA_ENABLE (0x1 << 8) /* Command Bit */ +#define I2CD_RX_DMA_ENABLE (0x1 << 9) +#define I2CD_TX_DMA_ENABLE (0x1 << 8) +#define I2CD_RX_BUFF_ENABLE (0x1 << 7) +#define I2CD_TX_BUFF_ENABLE (0x1 << 6) #define I2CD_M_STOP_CMD (0x1 << 5) #define I2CD_M_S_RX_CMD_LAST (0x1 << 4) #define I2CD_M_RX_CMD (0x1 << 3) @@ -XXX,XX +XXX,XX @@ #define I2CD_M_START_CMD (0x1) #define I2CD_DEV_ADDR_REG 0x18 /* Slave Device Address */ -#define I2CD_BUF_CTRL_REG 0x1c /* Pool Buffer Control */ +#define I2CD_POOL_CTRL_REG 0x1c /* Pool Buffer Control */ +#define I2CD_POOL_RX_COUNT(x) (((x) >> 24) & 0xff) +#define I2CD_POOL_RX_SIZE(x) ((((x) >> 16) & 0xff) + 1) +#define I2CD_POOL_TX_COUNT(x) ((((x) >> 8) & 0xff) + 1) +#define I2CD_POOL_OFFSET(x) (((x) & 0x3f) << 2) /* AST2400 */ #define I2CD_BYTE_BUF_REG 0x20 /* Transmit/Receive Byte Buffer */ #define I2CD_BYTE_BUF_TX_SHIFT 0 #define I2CD_BYTE_BUF_TX_MASK 0xff @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_i2c_bus_read(void *opaque, hwaddr offset, return bus->intr_ctrl; case I2CD_INTR_STS_REG: return bus->intr_status; + case I2CD_POOL_CTRL_REG: + return bus->pool_ctrl; case I2CD_BYTE_BUF_REG: return bus->buf; case I2CD_CMD_REG: @@ -XXX,XX +XXX,XX @@ static uint8_t aspeed_i2c_get_state(AspeedI2CBus *bus) return (bus->cmd >> I2CD_TX_STATE_SHIFT) & I2CD_TX_STATE_MASK; } +static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start) +{ + AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller); + int ret = -1; + int i; + + if (bus->cmd & I2CD_TX_BUFF_ENABLE) { + for (i = pool_start; i < I2CD_POOL_TX_COUNT(bus->pool_ctrl); i++) { + uint8_t *pool_base = aic->bus_pool_base(bus); + + ret = i2c_send(bus->bus, pool_base[i]); + if (ret) { + break; + } + } + bus->cmd &= ~I2CD_TX_BUFF_ENABLE; + } else { + ret = i2c_send(bus->bus, bus->buf); + } + + return ret; +} + +static void aspeed_i2c_bus_recv(AspeedI2CBus *bus) +{ + AspeedI2CState *s = bus->controller; + AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(s); + uint8_t data; + int i; + + if (bus->cmd & I2CD_RX_BUFF_ENABLE) { + uint8_t *pool_base = aic->bus_pool_base(bus); + + for (i = 0; i < I2CD_POOL_RX_SIZE(bus->pool_ctrl); i++) { + pool_base[i] = i2c_recv(bus->bus); + } + + /* Update RX count */ + bus->pool_ctrl &= ~(0xff << 24); + bus->pool_ctrl |= (i & 0xff) << 24; + bus->cmd &= ~I2CD_RX_BUFF_ENABLE; + } else { + data = i2c_recv(bus->bus); + bus->buf = (data & I2CD_BYTE_BUF_RX_MASK) << I2CD_BYTE_BUF_RX_SHIFT; + } +} + static void aspeed_i2c_handle_rx_cmd(AspeedI2CBus *bus) { - uint8_t ret; - aspeed_i2c_set_state(bus, I2CD_MRXD); - ret = i2c_recv(bus->bus); + aspeed_i2c_bus_recv(bus); bus->intr_status |= I2CD_INTR_RX_DONE; - bus->buf = (ret & I2CD_BYTE_BUF_RX_MASK) << I2CD_BYTE_BUF_RX_SHIFT; if (bus->cmd & I2CD_M_S_RX_CMD_LAST) { i2c_nack(bus->bus); } @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_handle_rx_cmd(AspeedI2CBus *bus) aspeed_i2c_set_state(bus, I2CD_MACTIVE); } +static uint8_t aspeed_i2c_get_addr(AspeedI2CBus *bus) +{ + AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller); + + if (bus->cmd & I2CD_TX_BUFF_ENABLE) { + uint8_t *pool_base = aic->bus_pool_base(bus); + + return pool_base[0]; + } else { + return bus->buf; + } +} + /* * The state machine needs some refinement. It is only used to track * invalid STOP commands for the moment. */ static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value) { + uint8_t pool_start = 0; + bus->cmd &= ~0xFFFF; bus->cmd |= value & 0xFFFF; if (bus->cmd & I2CD_M_START_CMD) { uint8_t state = aspeed_i2c_get_state(bus) & I2CD_MACTIVE ? I2CD_MSTARTR : I2CD_MSTART; + uint8_t addr; aspeed_i2c_set_state(bus, state); - if (i2c_start_transfer(bus->bus, extract32(bus->buf, 1, 7), - extract32(bus->buf, 0, 1))) { + addr = aspeed_i2c_get_addr(bus); + + if (i2c_start_transfer(bus->bus, extract32(addr, 1, 7), + extract32(addr, 0, 1))) { bus->intr_status |= I2CD_INTR_TX_NAK; } else { bus->intr_status |= I2CD_INTR_TX_ACK; } - /* START command is also a TX command, as the slave address is - * sent on the bus */ - bus->cmd &= ~(I2CD_M_START_CMD | I2CD_M_TX_CMD); + bus->cmd &= ~I2CD_M_START_CMD; + + /* + * The START command is also a TX command, as the slave + * address is sent on the bus. Drop the TX flag if nothing + * else needs to be sent in this sequence. + */ + if (bus->cmd & I2CD_TX_BUFF_ENABLE) { + if (I2CD_POOL_TX_COUNT(bus->pool_ctrl) == 1) { + bus->cmd &= ~I2CD_M_TX_CMD; + } else { + /* + * Increase the start index in the TX pool buffer to + * skip the address byte. + */ + pool_start++; + } + } else { + bus->cmd &= ~I2CD_M_TX_CMD; + } /* No slave found */ if (!i2c_bus_busy(bus->bus)) { @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value) if (bus->cmd & I2CD_M_TX_CMD) { aspeed_i2c_set_state(bus, I2CD_MTXD); - if (i2c_send(bus->bus, bus->buf)) { + if (aspeed_i2c_bus_send(bus, pool_start)) { bus->intr_status |= (I2CD_INTR_TX_NAK); i2c_end_transfer(bus->bus); } else { @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_write(void *opaque, hwaddr offset, qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n", __func__); break; + case I2CD_POOL_CTRL_REG: + bus->pool_ctrl &= ~0xffffff; + bus->pool_ctrl |= (value & 0xffffff); + break; + case I2CD_BYTE_BUF_REG: bus->buf = (value & I2CD_BYTE_BUF_TX_MASK) << I2CD_BYTE_BUF_TX_SHIFT; break; @@ -XXX,XX +XXX,XX @@ static const MemoryRegionOps aspeed_i2c_ctrl_ops = { .endianness = DEVICE_LITTLE_ENDIAN, }; +static uint64_t aspeed_i2c_pool_read(void *opaque, hwaddr offset, + unsigned size) +{ + AspeedI2CState *s = opaque; + uint64_t ret = 0; + int i; + + for (i = 0; i < size; i++) { + ret |= (uint64_t) s->pool[offset + i] << (8 * i); + } + + return ret; +} + +static void aspeed_i2c_pool_write(void *opaque, hwaddr offset, + uint64_t value, unsigned size) +{ + AspeedI2CState *s = opaque; + int i; + + for (i = 0; i < size; i++) { + s->pool[offset + i] = (value >> (8 * i)) & 0xFF; + } +} + +static const MemoryRegionOps aspeed_i2c_pool_ops = { + .read = aspeed_i2c_pool_read, + .write = aspeed_i2c_pool_write, + .endianness = DEVICE_LITTLE_ENDIAN, + .valid = { + .min_access_size = 1, + .max_access_size = 4, + }, +}; + static const VMStateDescription aspeed_i2c_bus_vmstate = { .name = TYPE_ASPEED_I2C, - .version_id = 1, - .minimum_version_id = 1, + .version_id = 2, + .minimum_version_id = 2, .fields = (VMStateField[]) { VMSTATE_UINT8(id, AspeedI2CBus), VMSTATE_UINT32(ctrl, AspeedI2CBus), @@ -XXX,XX +XXX,XX @@ static const VMStateDescription aspeed_i2c_bus_vmstate = { VMSTATE_UINT32(intr_status, AspeedI2CBus), VMSTATE_UINT32(cmd, AspeedI2CBus), VMSTATE_UINT32(buf, AspeedI2CBus), + VMSTATE_UINT32(pool_ctrl, AspeedI2CBus), VMSTATE_END_OF_LIST() } }; static const VMStateDescription aspeed_i2c_vmstate = { .name = TYPE_ASPEED_I2C, - .version_id = 1, - .minimum_version_id = 1, + .version_id = 2, + .minimum_version_id = 2, .fields = (VMStateField[]) { VMSTATE_UINT32(intr_status, AspeedI2CState), VMSTATE_STRUCT_ARRAY(busses, AspeedI2CState, ASPEED_I2C_NR_BUSSES, 1, aspeed_i2c_bus_vmstate, AspeedI2CBus), + VMSTATE_UINT8_ARRAY(pool, AspeedI2CState, ASPEED_I2C_MAX_POOL_SIZE), VMSTATE_END_OF_LIST() } }; @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_realize(DeviceState *dev, Error **errp) memory_region_add_subregion(&s->iomem, aic->reg_size * (i + offset), &s->busses[i].mr); } + + memory_region_init_io(&s->pool_iomem, OBJECT(s), &aspeed_i2c_pool_ops, s, + "aspeed.i2c-pool", aic->pool_size); + memory_region_add_subregion(&s->iomem, aic->pool_base, &s->pool_iomem); } static void aspeed_i2c_class_init(ObjectClass *klass, void *data) @@ -XXX,XX +XXX,XX @@ static qemu_irq aspeed_2400_i2c_bus_get_irq(AspeedI2CBus *bus) return bus->controller->irq; } +static uint8_t *aspeed_2400_i2c_bus_pool_base(AspeedI2CBus *bus) +{ + uint8_t *pool_page = + &bus->controller->pool[I2CD_POOL_PAGE_SEL(bus->ctrl) * 0x100]; + + return &pool_page[I2CD_POOL_OFFSET(bus->pool_ctrl)]; +} + static void aspeed_2400_i2c_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static void aspeed_2400_i2c_class_init(ObjectClass *klass, void *data) aic->reg_size = 0x40; aic->gap = 7; aic->bus_get_irq = aspeed_2400_i2c_bus_get_irq; + aic->pool_size = 0x800; + aic->pool_base = 0x800; + aic->bus_pool_base = aspeed_2400_i2c_bus_pool_base; } static const TypeInfo aspeed_2400_i2c_info = { @@ -XXX,XX +XXX,XX @@ static qemu_irq aspeed_2500_i2c_bus_get_irq(AspeedI2CBus *bus) return bus->controller->irq; } +static uint8_t *aspeed_2500_i2c_bus_pool_base(AspeedI2CBus *bus) +{ + return &bus->controller->pool[bus->id * 0x10]; +} + static void aspeed_2500_i2c_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static void aspeed_2500_i2c_class_init(ObjectClass *klass, void *data) aic->reg_size = 0x40; aic->gap = 7; aic->bus_get_irq = aspeed_2500_i2c_bus_get_irq; + aic->pool_size = 0x100; + aic->pool_base = 0x200; + aic->bus_pool_base = aspeed_2500_i2c_bus_pool_base; } static const TypeInfo aspeed_2500_i2c_info = { @@ -XXX,XX +XXX,XX @@ static qemu_irq aspeed_2600_i2c_bus_get_irq(AspeedI2CBus *bus) return bus->irq; } +static uint8_t *aspeed_2600_i2c_bus_pool_base(AspeedI2CBus *bus) +{ + return &bus->controller->pool[bus->id * 0x20]; +} + static void aspeed_2600_i2c_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static void aspeed_2600_i2c_class_init(ObjectClass *klass, void *data) aic->reg_size = 0x80; aic->gap = -1; /* no gap */ aic->bus_get_irq = aspeed_2600_i2c_bus_get_irq; + aic->pool_size = 0x200; + aic->pool_base = 0xC00; + aic->bus_pool_base = aspeed_2600_i2c_bus_pool_base; } static const TypeInfo aspeed_2600_i2c_info = { -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> The SRAM must be enabled before using the Buffer Pool mode or the DMA mode. This is not required on other SoCs. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Tested-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-3-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/i2c/aspeed_i2c.h | 3 +++ hw/i2c/aspeed_i2c.c | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/i2c/aspeed_i2c.h +++ b/include/hw/i2c/aspeed_i2c.h @@ -XXX,XX +XXX,XX @@ typedef struct AspeedI2CState { qemu_irq irq; uint32_t intr_status; + uint32_t ctrl_global; MemoryRegion pool_iomem; uint8_t pool[ASPEED_I2C_MAX_POOL_SIZE]; @@ -XXX,XX +XXX,XX @@ typedef struct AspeedI2CClass { uint64_t pool_size; hwaddr pool_base; uint8_t *(*bus_pool_base)(AspeedI2CBus *); + bool check_sram; + } AspeedI2CClass; I2CBus *aspeed_i2c_get_bus(DeviceState *dev, int busnr); diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c index XXXXXXX..XXXXXXX 100644 --- a/hw/i2c/aspeed_i2c.c +++ b/hw/i2c/aspeed_i2c.c @@ -XXX,XX +XXX,XX @@ #define I2C_CTRL_STATUS 0x00 /* Device Interrupt Status */ #define I2C_CTRL_ASSIGN 0x08 /* Device Interrupt Target Assignment */ +#define I2C_CTRL_GLOBAL 0x0C /* Global Control Register */ +#define I2C_CTRL_SRAM_EN BIT(0) /* I2C Device (Bus) Register */ @@ -XXX,XX +XXX,XX @@ static uint8_t aspeed_i2c_get_addr(AspeedI2CBus *bus) } } +static bool aspeed_i2c_check_sram(AspeedI2CBus *bus) +{ + AspeedI2CState *s = bus->controller; + AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(s); + + if (!aic->check_sram) { + return true; + } + + /* + * AST2500: SRAM must be enabled before using the Buffer Pool or + * DMA mode. + */ + if (!(s->ctrl_global & I2C_CTRL_SRAM_EN) && + (bus->cmd & (I2CD_RX_DMA_ENABLE | I2CD_TX_DMA_ENABLE | + I2CD_RX_BUFF_ENABLE | I2CD_TX_BUFF_ENABLE))) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: SRAM is not enabled\n", __func__); + return false; + } + + return true; +} + /* * The state machine needs some refinement. It is only used to track * invalid STOP commands for the moment. @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value) bus->cmd &= ~0xFFFF; bus->cmd |= value & 0xFFFF; + if (!aspeed_i2c_check_sram(bus)) { + return; + } + if (bus->cmd & I2CD_M_START_CMD) { uint8_t state = aspeed_i2c_get_state(bus) & I2CD_MACTIVE ? I2CD_MSTARTR : I2CD_MSTART; @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_i2c_ctrl_read(void *opaque, hwaddr offset, switch (offset) { case I2C_CTRL_STATUS: return s->intr_status; + case I2C_CTRL_GLOBAL: + return s->ctrl_global; default: qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__, offset); @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_i2c_ctrl_read(void *opaque, hwaddr offset, static void aspeed_i2c_ctrl_write(void *opaque, hwaddr offset, uint64_t value, unsigned size) { + AspeedI2CState *s = opaque; + switch (offset) { + case I2C_CTRL_GLOBAL: + s->ctrl_global = value; + break; case I2C_CTRL_STATUS: default: qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n", @@ -XXX,XX +XXX,XX @@ static void aspeed_2500_i2c_class_init(ObjectClass *klass, void *data) aic->pool_size = 0x100; aic->pool_base = 0x200; aic->bus_pool_base = aspeed_2500_i2c_bus_pool_base; + aic->check_sram = true; } static const TypeInfo aspeed_2500_i2c_info = { -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> Currently, we link the DRAM memory region to the FMC model (for DMAs) through a property alias at the SoC level. The I2C model will need a similar region for DMA support, add a DRAM region property at the SoC level for both model to use. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Tested-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-4-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/aspeed_soc.h | 1 + hw/arm/aspeed_ast2600.c | 7 +++++-- hw/arm/aspeed_soc.c | 9 +++++++-- 3 files changed, 13 insertions(+), 4 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 { ARMCPU cpu[ASPEED_CPUS_NUM]; uint32_t num_cpus; A15MPPrivState a7mpcore; + MemoryRegion *dram_mr; MemoryRegion sram; AspeedVICState vic; AspeedRtcState rtc; diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed_ast2600.c +++ b/hw/arm/aspeed_ast2600.c @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_init(Object *obj) typename); object_property_add_alias(obj, "num-cs", OBJECT(&s->fmc), "num-cs", &error_abort); - object_property_add_alias(obj, "dram", OBJECT(&s->fmc), "dram", - &error_abort); for (i = 0; i < sc->spis_num; i++) { snprintf(typename, sizeof(typename), "aspeed.spi%d-%s", i + 1, socname); @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp) } /* FMC, The number of CS is set at the board level */ + object_property_set_link(OBJECT(&s->fmc), OBJECT(s->dram_mr), "dram", &err); + if (err) { + error_propagate(errp, err); + return; + } object_property_set_int(OBJECT(&s->fmc), sc->memmap[ASPEED_SDRAM], "sdram-base", &err); if (err) { 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) typename); object_property_add_alias(obj, "num-cs", OBJECT(&s->fmc), "num-cs", &error_abort); - object_property_add_alias(obj, "dram", OBJECT(&s->fmc), "dram", - &error_abort); for (i = 0; i < sc->spis_num; i++) { snprintf(typename, sizeof(typename), "aspeed.spi%d-%s", i + 1, socname); @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) aspeed_soc_get_irq(s, ASPEED_I2C)); /* FMC, The number of CS is set at the board level */ + object_property_set_link(OBJECT(&s->fmc), OBJECT(s->dram_mr), "dram", &err); + if (err) { + error_propagate(errp, err); + return; + } object_property_set_int(OBJECT(&s->fmc), sc->memmap[ASPEED_SDRAM], "sdram-base", &err); if (err) { @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) } static Property aspeed_soc_properties[] = { DEFINE_PROP_UINT32("num-cpus", AspeedSoCState, num_cpus, 0), + DEFINE_PROP_LINK("dram", AspeedSoCState, dram_mr, TYPE_MEMORY_REGION, + MemoryRegion *), DEFINE_PROP_END_OF_LIST(), }; -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> The I2C controller of the Aspeed AST2500 and AST2600 SoCs supports DMA transfers to and from DRAM. A pair of registers defines the buffer address and the length of the DMA transfer. The address should be aligned on 4 bytes and the maximum length should not exceed 4K. The receive or transmit DMA transfer can then be initiated with specific bits in the Command/Status register of the controller. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Tested-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-5-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/i2c/aspeed_i2c.h | 5 ++ hw/arm/aspeed_ast2600.c | 5 ++ hw/arm/aspeed_soc.c | 5 ++ hw/i2c/aspeed_i2c.c | 126 +++++++++++++++++++++++++++++++++++- 4 files changed, 138 insertions(+), 3 deletions(-) diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/i2c/aspeed_i2c.h +++ b/include/hw/i2c/aspeed_i2c.h @@ -XXX,XX +XXX,XX @@ typedef struct AspeedI2CBus { uint32_t cmd; uint32_t buf; uint32_t pool_ctrl; + uint32_t dma_addr; + uint32_t dma_len; } AspeedI2CBus; typedef struct AspeedI2CState { @@ -XXX,XX +XXX,XX @@ typedef struct AspeedI2CState { uint8_t pool[ASPEED_I2C_MAX_POOL_SIZE]; AspeedI2CBus busses[ASPEED_I2C_NR_BUSSES]; + MemoryRegion *dram_mr; + AddressSpace dram_as; } AspeedI2CState; #define ASPEED_I2C_CLASS(klass) \ @@ -XXX,XX +XXX,XX @@ typedef struct AspeedI2CClass { hwaddr pool_base; uint8_t *(*bus_pool_base)(AspeedI2CBus *); bool check_sram; + bool has_dma; } AspeedI2CClass; diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed_ast2600.c +++ b/hw/arm/aspeed_ast2600.c @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp) } /* I2C */ + object_property_set_link(OBJECT(&s->i2c), OBJECT(s->dram_mr), "dram", &err); + if (err) { + error_propagate(errp, err); + return; + } object_property_set_bool(OBJECT(&s->i2c), true, "realized", &err); if (err) { error_propagate(errp, err); 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) } /* I2C */ + object_property_set_link(OBJECT(&s->i2c), OBJECT(s->dram_mr), "dram", &err); + if (err) { + error_propagate(errp, err); + return; + } object_property_set_bool(OBJECT(&s->i2c), true, "realized", &err); if (err) { error_propagate(errp, err); diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c index XXXXXXX..XXXXXXX 100644 --- a/hw/i2c/aspeed_i2c.c +++ b/hw/i2c/aspeed_i2c.c @@ -XXX,XX +XXX,XX @@ #include "migration/vmstate.h" #include "qemu/log.h" #include "qemu/module.h" +#include "qemu/error-report.h" +#include "qapi/error.h" #include "hw/i2c/aspeed_i2c.h" #include "hw/irq.h" +#include "hw/qdev-properties.h" /* I2C Global Register */ @@ -XXX,XX +XXX,XX @@ #define I2CD_BYTE_BUF_TX_MASK 0xff #define I2CD_BYTE_BUF_RX_SHIFT 8 #define I2CD_BYTE_BUF_RX_MASK 0xff - +#define I2CD_DMA_ADDR 0x24 /* DMA Buffer Address */ +#define I2CD_DMA_LEN 0x28 /* DMA Transfer Length < 4KB */ static inline bool aspeed_i2c_bus_is_master(AspeedI2CBus *bus) { @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_i2c_bus_read(void *opaque, hwaddr offset, unsigned size) { AspeedI2CBus *bus = opaque; + AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller); switch (offset) { case I2CD_FUN_CTRL_REG: @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_i2c_bus_read(void *opaque, hwaddr offset, return bus->buf; case I2CD_CMD_REG: return bus->cmd | (i2c_bus_busy(bus->bus) << 16); + case I2CD_DMA_ADDR: + if (!aic->has_dma) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n", __func__); + return -1; + } + return bus->dma_addr; + case I2CD_DMA_LEN: + if (!aic->has_dma) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n", __func__); + return -1; + } + return bus->dma_len; default: qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__, offset); @@ -XXX,XX +XXX,XX @@ static uint8_t aspeed_i2c_get_state(AspeedI2CBus *bus) return (bus->cmd >> I2CD_TX_STATE_SHIFT) & I2CD_TX_STATE_MASK; } +static int aspeed_i2c_dma_read(AspeedI2CBus *bus, uint8_t *data) +{ + MemTxResult result; + AspeedI2CState *s = bus->controller; + + result = address_space_read(&s->dram_as, bus->dma_addr, + MEMTXATTRS_UNSPECIFIED, data, 1); + if (result != MEMTX_OK) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: DRAM read failed @%08x\n", + __func__, bus->dma_addr); + return -1; + } + + bus->dma_addr++; + bus->dma_len--; + return 0; +} + static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start) { AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller); @@ -XXX,XX +XXX,XX @@ static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start) } } bus->cmd &= ~I2CD_TX_BUFF_ENABLE; + } else if (bus->cmd & I2CD_TX_DMA_ENABLE) { + while (bus->dma_len) { + uint8_t data; + aspeed_i2c_dma_read(bus, &data); + ret = i2c_send(bus->bus, data); + if (ret) { + break; + } + } + bus->cmd &= ~I2CD_TX_DMA_ENABLE; } else { ret = i2c_send(bus->bus, bus->buf); } @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus) bus->pool_ctrl &= ~(0xff << 24); bus->pool_ctrl |= (i & 0xff) << 24; bus->cmd &= ~I2CD_RX_BUFF_ENABLE; + } else if (bus->cmd & I2CD_RX_DMA_ENABLE) { + uint8_t data; + + while (bus->dma_len) { + MemTxResult result; + + data = i2c_recv(bus->bus); + result = address_space_write(&s->dram_as, bus->dma_addr, + MEMTXATTRS_UNSPECIFIED, &data, 1); + if (result != MEMTX_OK) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: DRAM write failed @%08x\n", + __func__, bus->dma_addr); + return; + } + bus->dma_addr++; + bus->dma_len--; + } + bus->cmd &= ~I2CD_RX_DMA_ENABLE; } else { data = i2c_recv(bus->bus); bus->buf = (data & I2CD_BYTE_BUF_RX_MASK) << I2CD_BYTE_BUF_RX_SHIFT; @@ -XXX,XX +XXX,XX @@ static uint8_t aspeed_i2c_get_addr(AspeedI2CBus *bus) uint8_t *pool_base = aic->bus_pool_base(bus); return pool_base[0]; + } else if (bus->cmd & I2CD_TX_DMA_ENABLE) { + uint8_t data; + + aspeed_i2c_dma_read(bus, &data); + return data; } else { return bus->buf; } @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value) */ pool_start++; } + } else if (bus->cmd & I2CD_TX_DMA_ENABLE) { + if (bus->dma_len == 0) { + bus->cmd &= ~I2CD_M_TX_CMD; + } } else { bus->cmd &= ~I2CD_M_TX_CMD; } @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_write(void *opaque, hwaddr offset, break; } + if (!aic->has_dma && + value & (I2CD_RX_DMA_ENABLE | I2CD_TX_DMA_ENABLE)) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n", __func__); + break; + } + aspeed_i2c_bus_handle_cmd(bus, value); aspeed_i2c_bus_raise_interrupt(bus); break; + case I2CD_DMA_ADDR: + if (!aic->has_dma) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n", __func__); + break; + } + + bus->dma_addr = value & 0xfffffffc; + break; + + case I2CD_DMA_LEN: + if (!aic->has_dma) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n", __func__); + break; + } + + bus->dma_len = value & 0xfff; + if (!bus->dma_len) { + qemu_log_mask(LOG_UNIMP, "%s: invalid DMA length\n", __func__); + } + break; default: qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n", @@ -XXX,XX +XXX,XX @@ static const MemoryRegionOps aspeed_i2c_pool_ops = { static const VMStateDescription aspeed_i2c_bus_vmstate = { .name = TYPE_ASPEED_I2C, - .version_id = 2, - .minimum_version_id = 2, + .version_id = 3, + .minimum_version_id = 3, .fields = (VMStateField[]) { VMSTATE_UINT8(id, AspeedI2CBus), VMSTATE_UINT32(ctrl, AspeedI2CBus), @@ -XXX,XX +XXX,XX @@ static const VMStateDescription aspeed_i2c_bus_vmstate = { VMSTATE_UINT32(cmd, AspeedI2CBus), VMSTATE_UINT32(buf, AspeedI2CBus), VMSTATE_UINT32(pool_ctrl, AspeedI2CBus), + VMSTATE_UINT32(dma_addr, AspeedI2CBus), + VMSTATE_UINT32(dma_len, AspeedI2CBus), VMSTATE_END_OF_LIST() } }; @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_reset(DeviceState *dev) s->busses[i].intr_status = 0; s->busses[i].cmd = 0; s->busses[i].buf = 0; + s->busses[i].dma_addr = 0; + s->busses[i].dma_len = 0; i2c_end_transfer(s->busses[i].bus); } } @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_realize(DeviceState *dev, Error **errp) memory_region_init_io(&s->pool_iomem, OBJECT(s), &aspeed_i2c_pool_ops, s, "aspeed.i2c-pool", aic->pool_size); memory_region_add_subregion(&s->iomem, aic->pool_base, &s->pool_iomem); + + if (aic->has_dma) { + if (!s->dram_mr) { + error_setg(errp, TYPE_ASPEED_I2C ": 'dram' link not set"); + return; + } + + address_space_init(&s->dram_as, s->dram_mr, "dma-dram"); + } } +static Property aspeed_i2c_properties[] = { + DEFINE_PROP_LINK("dram", AspeedI2CState, dram_mr, + TYPE_MEMORY_REGION, MemoryRegion *), + DEFINE_PROP_END_OF_LIST(), +}; + static void aspeed_i2c_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); dc->vmsd = &aspeed_i2c_vmstate; dc->reset = aspeed_i2c_reset; + dc->props = aspeed_i2c_properties; dc->realize = aspeed_i2c_realize; dc->desc = "Aspeed I2C Controller"; } @@ -XXX,XX +XXX,XX @@ static void aspeed_2500_i2c_class_init(ObjectClass *klass, void *data) aic->pool_base = 0x200; aic->bus_pool_base = aspeed_2500_i2c_bus_pool_base; aic->check_sram = true; + aic->has_dma = true; } static const TypeInfo aspeed_2500_i2c_info = { @@ -XXX,XX +XXX,XX @@ static void aspeed_2600_i2c_class_init(ObjectClass *klass, void *data) aic->pool_size = 0x200; aic->pool_base = 0xC00; aic->bus_pool_base = aspeed_2600_i2c_bus_pool_base; + aic->has_dma = true; } static const TypeInfo aspeed_2600_i2c_info = { -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Tested-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-6-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/i2c/aspeed_i2c.c | 93 ++++++++++++++++++++++++++++++++++++++------- hw/i2c/trace-events | 9 +++++ 2 files changed, 89 insertions(+), 13 deletions(-) diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c index XXXXXXX..XXXXXXX 100644 --- a/hw/i2c/aspeed_i2c.c +++ b/hw/i2c/aspeed_i2c.c @@ -XXX,XX +XXX,XX @@ #include "hw/i2c/aspeed_i2c.h" #include "hw/irq.h" #include "hw/qdev-properties.h" +#include "trace.h" /* I2C Global Register */ @@ -XXX,XX +XXX,XX @@ static inline void aspeed_i2c_bus_raise_interrupt(AspeedI2CBus *bus) { AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller); + trace_aspeed_i2c_bus_raise_interrupt(bus->intr_status, + bus->intr_status & I2CD_INTR_TX_NAK ? "nak|" : "", + bus->intr_status & I2CD_INTR_TX_ACK ? "ack|" : "", + bus->intr_status & I2CD_INTR_RX_DONE ? "done|" : "", + bus->intr_status & I2CD_INTR_NORMAL_STOP ? "normal|" : "", + bus->intr_status & I2CD_INTR_ABNORMAL ? "abnormal" : ""); + bus->intr_status &= bus->intr_ctrl; if (bus->intr_status) { bus->controller->intr_status |= 1 << bus->id; @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_i2c_bus_read(void *opaque, hwaddr offset, { AspeedI2CBus *bus = opaque; AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller); + uint64_t value = -1; switch (offset) { case I2CD_FUN_CTRL_REG: - return bus->ctrl; + value = bus->ctrl; + break; case I2CD_AC_TIMING_REG1: - return bus->timing[0]; + value = bus->timing[0]; + break; case I2CD_AC_TIMING_REG2: - return bus->timing[1]; + value = bus->timing[1]; + break; case I2CD_INTR_CTRL_REG: - return bus->intr_ctrl; + value = bus->intr_ctrl; + break; case I2CD_INTR_STS_REG: - return bus->intr_status; + value = bus->intr_status; + break; case I2CD_POOL_CTRL_REG: - return bus->pool_ctrl; + value = bus->pool_ctrl; + break; case I2CD_BYTE_BUF_REG: - return bus->buf; + value = bus->buf; + break; case I2CD_CMD_REG: - return bus->cmd | (i2c_bus_busy(bus->bus) << 16); + value = bus->cmd | (i2c_bus_busy(bus->bus) << 16); + break; case I2CD_DMA_ADDR: if (!aic->has_dma) { qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n", __func__); - return -1; + break; } - return bus->dma_addr; + value = bus->dma_addr; + break; case I2CD_DMA_LEN: if (!aic->has_dma) { qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n", __func__); - return -1; + break; } - return bus->dma_len; + value = bus->dma_len; + break; + default: qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__, offset); - return -1; + value = -1; + break; } + + trace_aspeed_i2c_bus_read(bus->id, offset, size, value); + return value; } static void aspeed_i2c_set_state(AspeedI2CBus *bus, uint8_t state) @@ -XXX,XX +XXX,XX @@ static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start) for (i = pool_start; i < I2CD_POOL_TX_COUNT(bus->pool_ctrl); i++) { uint8_t *pool_base = aic->bus_pool_base(bus); + trace_aspeed_i2c_bus_send("BUF", i + 1, + I2CD_POOL_TX_COUNT(bus->pool_ctrl), + pool_base[i]); ret = i2c_send(bus->bus, pool_base[i]); if (ret) { break; @@ -XXX,XX +XXX,XX @@ static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start) while (bus->dma_len) { uint8_t data; aspeed_i2c_dma_read(bus, &data); + trace_aspeed_i2c_bus_send("DMA", bus->dma_len, bus->dma_len, data); ret = i2c_send(bus->bus, data); if (ret) { break; @@ -XXX,XX +XXX,XX @@ static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start) } bus->cmd &= ~I2CD_TX_DMA_ENABLE; } else { + trace_aspeed_i2c_bus_send("BYTE", pool_start, 1, bus->buf); ret = i2c_send(bus->bus, bus->buf); } @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus) for (i = 0; i < I2CD_POOL_RX_SIZE(bus->pool_ctrl); i++) { pool_base[i] = i2c_recv(bus->bus); + trace_aspeed_i2c_bus_recv("BUF", i + 1, + I2CD_POOL_RX_SIZE(bus->pool_ctrl), + pool_base[i]); } /* Update RX count */ @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus) MemTxResult result; data = i2c_recv(bus->bus); + trace_aspeed_i2c_bus_recv("DMA", bus->dma_len, bus->dma_len, data); result = address_space_write(&s->dram_as, bus->dma_addr, MEMTXATTRS_UNSPECIFIED, &data, 1); if (result != MEMTX_OK) { @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus) bus->cmd &= ~I2CD_RX_DMA_ENABLE; } else { data = i2c_recv(bus->bus); + trace_aspeed_i2c_bus_recv("BYTE", 1, 1, bus->buf); bus->buf = (data & I2CD_BYTE_BUF_RX_MASK) << I2CD_BYTE_BUF_RX_SHIFT; } } @@ -XXX,XX +XXX,XX @@ static bool aspeed_i2c_check_sram(AspeedI2CBus *bus) return true; } +static void aspeed_i2c_bus_cmd_dump(AspeedI2CBus *bus) +{ + g_autofree char *cmd_flags; + uint32_t count; + + if (bus->cmd & (I2CD_RX_BUFF_ENABLE | I2CD_RX_BUFF_ENABLE)) { + count = I2CD_POOL_TX_COUNT(bus->pool_ctrl); + } else if (bus->cmd & (I2CD_RX_DMA_ENABLE | I2CD_RX_DMA_ENABLE)) { + count = bus->dma_len; + } else { /* BYTE mode */ + count = 1; + } + + cmd_flags = g_strdup_printf("%s%s%s%s%s%s%s%s%s", + bus->cmd & I2CD_M_START_CMD ? "start|" : "", + bus->cmd & I2CD_RX_DMA_ENABLE ? "rxdma|" : "", + bus->cmd & I2CD_TX_DMA_ENABLE ? "txdma|" : "", + bus->cmd & I2CD_RX_BUFF_ENABLE ? "rxbuf|" : "", + bus->cmd & I2CD_TX_BUFF_ENABLE ? "txbuf|" : "", + bus->cmd & I2CD_M_TX_CMD ? "tx|" : "", + bus->cmd & I2CD_M_RX_CMD ? "rx|" : "", + bus->cmd & I2CD_M_S_RX_CMD_LAST ? "last|" : "", + bus->cmd & I2CD_M_STOP_CMD ? "stop" : ""); + + trace_aspeed_i2c_bus_cmd(bus->cmd, cmd_flags, count, bus->intr_status); +} + /* * The state machine needs some refinement. It is only used to track * invalid STOP commands for the moment. @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value) return; } + if (trace_event_get_state_backends(TRACE_ASPEED_I2C_BUS_CMD)) { + aspeed_i2c_bus_cmd_dump(bus); + } + if (bus->cmd & I2CD_M_START_CMD) { uint8_t state = aspeed_i2c_get_state(bus) & I2CD_MACTIVE ? I2CD_MSTARTR : I2CD_MSTART; @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_write(void *opaque, hwaddr offset, AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller); bool handle_rx; + trace_aspeed_i2c_bus_write(bus->id, offset, size, value); + switch (offset) { case I2CD_FUN_CTRL_REG: if (value & I2CD_SLAVE_EN) { diff --git a/hw/i2c/trace-events b/hw/i2c/trace-events index XXXXXXX..XXXXXXX 100644 --- a/hw/i2c/trace-events +++ b/hw/i2c/trace-events @@ -XXX,XX +XXX,XX @@ i2c_event(const char *event, uint8_t address) "%s(addr:0x%02x)" i2c_send(uint8_t address, uint8_t data) "send(addr:0x%02x) data:0x%02x" i2c_recv(uint8_t address, uint8_t data) "recv(addr:0x%02x) data:0x%02x" + +# aspeed_i2c.c + +aspeed_i2c_bus_cmd(uint32_t cmd, const char *cmd_flags, uint32_t count, uint32_t intr_status) "handling cmd=0x%x %s count=%d intr=0x%x" +aspeed_i2c_bus_raise_interrupt(uint32_t intr_status, const char *str1, const char *str2, const char *str3, const char *str4, const char *str5) "handled intr=0x%x %s%s%s%s%s" +aspeed_i2c_bus_read(uint32_t busid, uint64_t offset, unsigned size, uint64_t value) "bus[%d]: To 0x%" PRIx64 " of size %u: 0x%" PRIx64 +aspeed_i2c_bus_write(uint32_t busid, uint64_t offset, unsigned size, uint64_t value) "bus[%d]: To 0x%" PRIx64 " of size %u: 0x%" PRIx64 +aspeed_i2c_bus_send(const char *mode, int i, int count, uint8_t byte) "%s send %d/%d 0x%02x" +aspeed_i2c_bus_recv(const char *mode, int i, int count, uint8_t byte) "%s recv %d/%d 0x%02x" -- 2.20.1 From: Joel Stanley <joel@jms.id.au> Most boards have this much. Reviewed-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-7-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/misc/aspeed_sdmc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/misc/aspeed_sdmc.c +++ b/hw/misc/aspeed_sdmc.c @@ -XXX,XX +XXX,XX @@ static int ast2600_rambits(AspeedSDMCState *s) } /* use a common default */ - warn_report("Invalid RAM size 0x%" PRIx64 ". Using default 512M", + warn_report("Invalid RAM size 0x%" PRIx64 ". Using default 1024M", s->ram_size); - s->ram_size = 512 << 20; - return ASPEED_SDMC_AST2600_512MB; + s->ram_size = 1024 << 20; + return ASPEED_SDMC_AST2600_1024MB; } static void aspeed_sdmc_reset(DeviceState *dev) -- 2.20.1 From: Joel Stanley <joel@jms.id.au> This models the clock write one to clear registers, and fixes up some incorrect behavior in all of the write to clear registers. There was also a typo in one of the register definitions. Reviewed-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-8-clg@kaod.org [clg: checkpatch.pl fixes ] Signed-off-by: Cédric Le Goater <clg@kaod.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/misc/aspeed_scu.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) 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 @@ #define AST2600_CLK_STOP_CTRL TO_REG(0x80) #define AST2600_CLK_STOP_CTRL_CLR TO_REG(0x84) #define AST2600_CLK_STOP_CTRL2 TO_REG(0x90) -#define AST2600_CLK_STOP_CTR2L_CLR TO_REG(0x94) +#define AST2600_CLK_STOP_CTRL2_CLR TO_REG(0x94) #define AST2600_SDRAM_HANDSHAKE TO_REG(0x100) #define AST2600_HPLL_PARAM TO_REG(0x200) #define AST2600_HPLL_EXT TO_REG(0x204) @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_ast2600_scu_read(void *opaque, hwaddr offset, return s->regs[reg]; } -static void aspeed_ast2600_scu_write(void *opaque, hwaddr offset, uint64_t data, - unsigned size) +static void aspeed_ast2600_scu_write(void *opaque, hwaddr offset, + uint64_t data64, unsigned size) { AspeedSCUState *s = ASPEED_SCU(opaque); int reg = TO_REG(offset); + /* Truncate here so bitwise operations below behave as expected */ + uint32_t data = data64; if (reg >= ASPEED_AST2600_SCU_NR_REGS) { qemu_log_mask(LOG_GUEST_ERROR, @@ -XXX,XX +XXX,XX @@ static void aspeed_ast2600_scu_write(void *opaque, hwaddr offset, uint64_t data, /* fall through */ case AST2600_SYS_RST_CTRL: case AST2600_SYS_RST_CTRL2: + case AST2600_CLK_STOP_CTRL: + case AST2600_CLK_STOP_CTRL2: /* W1S (Write 1 to set) registers */ s->regs[reg] |= data; return; case AST2600_SYS_RST_CTRL_CLR: case AST2600_SYS_RST_CTRL2_CLR: + case AST2600_CLK_STOP_CTRL_CLR: + case AST2600_CLK_STOP_CTRL2_CLR: case AST2600_HW_STRAP1_CLR: case AST2600_HW_STRAP2_CLR: - /* W1C (Write 1 to clear) registers */ - s->regs[reg] &= ~data; + /* + * W1C (Write 1 to clear) registers are offset by one address from + * the data register + */ + s->regs[reg - 1] &= ~data; return; case AST2600_RNG_DATA: -- 2.20.1 From: Joel Stanley <joel@jms.id.au> Users benefit from knowing which watchdog timer has expired. The address of the watchdog's registers unambiguously indicates which has expired, so log that. Reviewed-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-9-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/watchdog/wdt_aspeed.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c index XXXXXXX..XXXXXXX 100644 --- a/hw/watchdog/wdt_aspeed.c +++ b/hw/watchdog/wdt_aspeed.c @@ -XXX,XX +XXX,XX @@ static void aspeed_wdt_timer_expired(void *dev) return; } - qemu_log_mask(CPU_LOG_RESET, "Watchdog timer expired.\n"); + qemu_log_mask(CPU_LOG_RESET, "Watchdog timer %" HWADDR_PRIx " expired.\n", + s->iomem.addr); watchdog_perform_action(); timer_del(s->timer); } -- 2.20.1 From: Joel Stanley <joel@jms.id.au> The AST2600 control register sneakily changed the meaning of bit 4 without anyone noticing. It no longer controls the 1MHz vs APB clock select, and instead always runs at 1MHz. The AST2500 was always 1MHz too, but it retained bit 4, making it read only. We can model both using the same fixed 1MHz calculation. Fixes: 6b2b2a703cad ("hw: wdt_aspeed: Add AST2600 support") Reviewed-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-10-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/watchdog/wdt_aspeed.h | 1 + hw/watchdog/wdt_aspeed.c | 21 +++++++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/include/hw/watchdog/wdt_aspeed.h b/include/hw/watchdog/wdt_aspeed.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/watchdog/wdt_aspeed.h +++ b/include/hw/watchdog/wdt_aspeed.h @@ -XXX,XX +XXX,XX @@ typedef struct AspeedWDTClass { uint32_t ext_pulse_width_mask; uint32_t reset_ctrl_reg; void (*reset_pulse)(AspeedWDTState *s, uint32_t property); + void (*wdt_reload)(AspeedWDTState *s); } AspeedWDTClass; #endif /* WDT_ASPEED_H */ diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c index XXXXXXX..XXXXXXX 100644 --- a/hw/watchdog/wdt_aspeed.c +++ b/hw/watchdog/wdt_aspeed.c @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_wdt_read(void *opaque, hwaddr offset, unsigned size) } -static void aspeed_wdt_reload(AspeedWDTState *s, bool pclk) +static void aspeed_wdt_reload(AspeedWDTState *s) { uint64_t reload; - if (pclk) { + if (!(s->regs[WDT_CTRL] & WDT_CTRL_1MHZ_CLK)) { reload = muldiv64(s->regs[WDT_RELOAD_VALUE], NANOSECONDS_PER_SECOND, s->pclk_freq); } else { @@ -XXX,XX +XXX,XX @@ static void aspeed_wdt_reload(AspeedWDTState *s, bool pclk) } } +static void aspeed_wdt_reload_1mhz(AspeedWDTState *s) +{ + uint64_t reload = s->regs[WDT_RELOAD_VALUE] * 1000ULL; + + if (aspeed_wdt_is_enabled(s)) { + timer_mod(s->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + reload); + } +} + + static void aspeed_wdt_write(void *opaque, hwaddr offset, uint64_t data, unsigned size) { @@ -XXX,XX +XXX,XX @@ static void aspeed_wdt_write(void *opaque, hwaddr offset, uint64_t data, case WDT_RESTART: if ((data & 0xFFFF) == WDT_RESTART_MAGIC) { s->regs[WDT_STATUS] = s->regs[WDT_RELOAD_VALUE]; - aspeed_wdt_reload(s, !(s->regs[WDT_CTRL] & WDT_CTRL_1MHZ_CLK)); + awc->wdt_reload(s); } break; case WDT_CTRL: if (enable && !aspeed_wdt_is_enabled(s)) { s->regs[WDT_CTRL] = data; - aspeed_wdt_reload(s, !(data & WDT_CTRL_1MHZ_CLK)); + awc->wdt_reload(s); } else if (!enable && aspeed_wdt_is_enabled(s)) { s->regs[WDT_CTRL] = data; timer_del(s->timer); @@ -XXX,XX +XXX,XX @@ static void aspeed_2400_wdt_class_init(ObjectClass *klass, void *data) awc->offset = 0x20; awc->ext_pulse_width_mask = 0xff; awc->reset_ctrl_reg = SCU_RESET_CONTROL1; + awc->wdt_reload = aspeed_wdt_reload; } static const TypeInfo aspeed_2400_wdt_info = { @@ -XXX,XX +XXX,XX @@ static void aspeed_2500_wdt_class_init(ObjectClass *klass, void *data) awc->ext_pulse_width_mask = 0xfffff; awc->reset_ctrl_reg = SCU_RESET_CONTROL1; awc->reset_pulse = aspeed_2500_wdt_reset_pulse; + awc->wdt_reload = aspeed_wdt_reload_1mhz; } static const TypeInfo aspeed_2500_wdt_info = { @@ -XXX,XX +XXX,XX @@ static void aspeed_2600_wdt_class_init(ObjectClass *klass, void *data) awc->ext_pulse_width_mask = 0xfffff; /* TODO */ awc->reset_ctrl_reg = AST2600_SCU_RESET_CONTROL1; awc->reset_pulse = aspeed_2500_wdt_reset_pulse; + awc->wdt_reload = aspeed_wdt_reload_1mhz; } static const TypeInfo aspeed_2600_wdt_info = { -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> The current model only restores the Segment Register values but leaves the previous CS mapping behind. Introduce a helper setting the register value and mapping the region at the requested address. Use this helper when a Segment register is set and at reset. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-11-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/ssi/aspeed_smc.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 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 bool aspeed_smc_flash_overlap(const AspeedSMCState *s, return false; } +static void aspeed_smc_flash_set_segment_region(AspeedSMCState *s, int cs, + uint64_t regval) +{ + AspeedSMCFlash *fl = &s->flashes[cs]; + AspeedSegments seg; + + s->ctrl->reg_to_segment(s, regval, &seg); + + memory_region_transaction_begin(); + memory_region_set_size(&fl->mmio, seg.size); + memory_region_set_address(&fl->mmio, seg.addr - s->ctrl->flash_window_base); + memory_region_set_enabled(&fl->mmio, true); + memory_region_transaction_commit(); + + s->regs[R_SEG_ADDR0 + cs] = regval; +} + static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs, uint64_t new) { - AspeedSMCFlash *fl = &s->flashes[cs]; AspeedSegments seg; s->ctrl->reg_to_segment(s, new, &seg); @@ -XXX,XX +XXX,XX @@ static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs, aspeed_smc_flash_overlap(s, &seg, cs); /* All should be fine now to move the region */ - memory_region_transaction_begin(); - memory_region_set_size(&fl->mmio, seg.size); - memory_region_set_address(&fl->mmio, seg.addr - s->ctrl->flash_window_base); - memory_region_set_enabled(&fl->mmio, true); - memory_region_transaction_commit(); - - s->regs[R_SEG_ADDR0 + cs] = new; + aspeed_smc_flash_set_segment_region(s, cs, new); } static uint64_t aspeed_smc_flash_default_read(void *opaque, hwaddr addr, @@ -XXX,XX +XXX,XX @@ static void aspeed_smc_reset(DeviceState *d) qemu_set_irq(s->cs_lines[i], true); } - /* setup default segment register values for all */ + /* setup the default segment register values and regions for all */ for (i = 0; i < s->ctrl->max_slaves; ++i) { - s->regs[R_SEG_ADDR0 + i] = - s->ctrl->segment_to_reg(s, &s->ctrl->segments[i]); + aspeed_smc_flash_set_segment_region(s, i, + s->ctrl->segment_to_reg(s, &s->ctrl->segments[i])); } /* HW strapping flash type for the AST2600 controllers */ -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> The segments can be disabled on the AST2600 (zero register value). CS0 is open by default but not the other CS. This is closing the access to the flash device in user mode and forbids scanning. In the model, check the segment size and disable the associated region when the value is zero. Fixes: bcaa8ddd081c ("aspeed/smc: Add AST2600 support") Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-12-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/ssi/aspeed_smc.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 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_2600_smc_reg_to_segment(const AspeedSMCState *s, uint32_t start_offset = (reg << 16) & AST2600_SEG_ADDR_MASK; uint32_t end_offset = reg & AST2600_SEG_ADDR_MASK; - seg->addr = s->ctrl->flash_window_base + start_offset; - seg->size = end_offset + MiB - start_offset; + if (reg) { + seg->addr = s->ctrl->flash_window_base + start_offset; + seg->size = end_offset + MiB - start_offset; + } else { + seg->addr = s->ctrl->flash_window_base; + seg->size = 0; + } } static bool aspeed_smc_flash_overlap(const AspeedSMCState *s, @@ -XXX,XX +XXX,XX @@ static void aspeed_smc_flash_set_segment_region(AspeedSMCState *s, int cs, memory_region_transaction_begin(); memory_region_set_size(&fl->mmio, seg.size); memory_region_set_address(&fl->mmio, seg.addr - s->ctrl->flash_window_base); - memory_region_set_enabled(&fl->mmio, true); + memory_region_set_enabled(&fl->mmio, !!seg.size); memory_region_transaction_commit(); s->regs[R_SEG_ADDR0 + cs] = regval; @@ -XXX,XX +XXX,XX @@ static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs, } /* Keep the segment in the overall flash window */ - if (seg.addr + seg.size <= s->ctrl->flash_window_base || - seg.addr > s->ctrl->flash_window_base + s->ctrl->flash_window_size) { + if (seg.size && + (seg.addr + seg.size <= s->ctrl->flash_window_base || + seg.addr > s->ctrl->flash_window_base + s->ctrl->flash_window_size)) { qemu_log_mask(LOG_GUEST_ERROR, "%s: new segment for CS%d is invalid : " "[ 0x%"HWADDR_PRIx" - 0x%"HWADDR_PRIx" ]\n", s->ctrl->name, cs, seg.addr, seg.addr + seg.size); -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> Each CS has its own Read Timing Compensation Register on newer SoCs. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-13-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/ssi/aspeed_smc.h | 1 + hw/ssi/aspeed_smc.c | 17 ++++++++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/ssi/aspeed_smc.h +++ b/include/hw/ssi/aspeed_smc.h @@ -XXX,XX +XXX,XX @@ typedef struct AspeedSMCController { uint8_t r_ce_ctrl; uint8_t r_ctrl0; uint8_t r_timings; + uint8_t nregs_timings; uint8_t conf_enable_w0; uint8_t max_slaves; const AspeedSegments *segments; 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 @@ /* Checksum Calculation Result */ #define R_DMA_CHECKSUM (0x90 / 4) -/* Misc Control Register #2 */ +/* Read Timing Compensation Register */ #define R_TIMINGS (0x94 / 4) /* SPI controller registers and bits (AST2400) */ @@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = { .r_ce_ctrl = R_CE_CTRL, .r_ctrl0 = R_CTRL0, .r_timings = R_TIMINGS, + .nregs_timings = 1, .conf_enable_w0 = CONF_ENABLE_W0, .max_slaves = 5, .segments = aspeed_segments_legacy, @@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = { .r_ce_ctrl = R_CE_CTRL, .r_ctrl0 = R_CTRL0, .r_timings = R_TIMINGS, + .nregs_timings = 1, .conf_enable_w0 = CONF_ENABLE_W0, .max_slaves = 5, .segments = aspeed_segments_fmc, @@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = { .r_ce_ctrl = 0xff, .r_ctrl0 = R_SPI_CTRL0, .r_timings = R_SPI_TIMINGS, + .nregs_timings = 1, .conf_enable_w0 = SPI_CONF_ENABLE_W0, .max_slaves = 1, .segments = aspeed_segments_spi, @@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = { .r_ce_ctrl = R_CE_CTRL, .r_ctrl0 = R_CTRL0, .r_timings = R_TIMINGS, + .nregs_timings = 1, .conf_enable_w0 = CONF_ENABLE_W0, .max_slaves = 3, .segments = aspeed_segments_ast2500_fmc, @@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = { .r_ce_ctrl = R_CE_CTRL, .r_ctrl0 = R_CTRL0, .r_timings = R_TIMINGS, + .nregs_timings = 1, .conf_enable_w0 = CONF_ENABLE_W0, .max_slaves = 2, .segments = aspeed_segments_ast2500_spi1, @@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = { .r_ce_ctrl = R_CE_CTRL, .r_ctrl0 = R_CTRL0, .r_timings = R_TIMINGS, + .nregs_timings = 1, .conf_enable_w0 = CONF_ENABLE_W0, .max_slaves = 2, .segments = aspeed_segments_ast2500_spi2, @@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = { .r_ce_ctrl = R_CE_CTRL, .r_ctrl0 = R_CTRL0, .r_timings = R_TIMINGS, + .nregs_timings = 1, .conf_enable_w0 = CONF_ENABLE_W0, .max_slaves = 3, .segments = aspeed_segments_ast2600_fmc, @@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = { .r_ce_ctrl = R_CE_CTRL, .r_ctrl0 = R_CTRL0, .r_timings = R_TIMINGS, + .nregs_timings = 2, .conf_enable_w0 = CONF_ENABLE_W0, .max_slaves = 2, .segments = aspeed_segments_ast2600_spi1, @@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = { .r_ce_ctrl = R_CE_CTRL, .r_ctrl0 = R_CTRL0, .r_timings = R_TIMINGS, + .nregs_timings = 3, .conf_enable_w0 = CONF_ENABLE_W0, .max_slaves = 3, .segments = aspeed_segments_ast2600_spi2, @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size) addr >>= 2; if (addr == s->r_conf || - addr == s->r_timings || + (addr >= s->r_timings && + addr < s->r_timings + s->ctrl->nregs_timings) || addr == s->r_ce_ctrl || addr == R_INTR_CTRL || addr == R_DUMMY_DATA || @@ -XXX,XX +XXX,XX @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data, addr >>= 2; if (addr == s->r_conf || - addr == s->r_timings || + (addr >= s->r_timings && + addr < s->r_timings + s->ctrl->nregs_timings) || addr == s->r_ce_ctrl) { s->regs[addr] = value; } else if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs) { -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> AspeedBoardConfig is a redundant way to define class attributes and it complexifies the machine definition and initialization. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-14-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/aspeed.h | 24 ++-- hw/arm/aspeed.c | 243 ++++++++++++++++++++++------------------ 2 files changed, 143 insertions(+), 124 deletions(-) diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/aspeed.h +++ b/include/hw/arm/aspeed.h @@ -XXX,XX +XXX,XX @@ typedef struct AspeedBoardState AspeedBoardState; -typedef struct AspeedBoardConfig { - const char *name; - const char *desc; - const char *soc_name; - uint32_t hw_strap1; - uint32_t hw_strap2; - const char *fmc_model; - const char *spi_model; - uint32_t num_cs; - void (*i2c_init)(AspeedBoardState *bmc); - uint32_t ram; -} AspeedBoardConfig; - #define TYPE_ASPEED_MACHINE MACHINE_TYPE_NAME("aspeed") #define ASPEED_MACHINE(obj) \ OBJECT_CHECK(AspeedMachine, (obj), TYPE_ASPEED_MACHINE) @@ -XXX,XX +XXX,XX @@ typedef struct AspeedMachine { typedef struct AspeedMachineClass { MachineClass parent_obj; - const AspeedBoardConfig *board; + + const char *name; + const char *desc; + const char *soc_name; + uint32_t hw_strap1; + uint32_t hw_strap2; + const char *fmc_model; + const char *spi_model; + uint32_t num_cs; + void (*i2c_init)(AspeedBoardState *bmc); } AspeedMachineClass; diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -XXX,XX +XXX,XX @@ static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype, } } -static void aspeed_board_init(MachineState *machine, - const AspeedBoardConfig *cfg) +static void aspeed_machine_init(MachineState *machine) { AspeedBoardState *bmc; + AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(machine); AspeedSoCClass *sc; DriveInfo *drive0 = drive_get(IF_MTD, 0, 0); ram_addr_t max_ram_size; @@ -XXX,XX +XXX,XX @@ static void aspeed_board_init(MachineState *machine, UINT32_MAX); object_initialize_child(OBJECT(machine), "soc", &bmc->soc, - (sizeof(bmc->soc)), cfg->soc_name, &error_abort, + (sizeof(bmc->soc)), amc->soc_name, &error_abort, NULL); sc = ASPEED_SOC_GET_CLASS(&bmc->soc); object_property_set_uint(OBJECT(&bmc->soc), ram_size, "ram-size", &error_abort); - object_property_set_int(OBJECT(&bmc->soc), cfg->hw_strap1, "hw-strap1", + object_property_set_int(OBJECT(&bmc->soc), amc->hw_strap1, "hw-strap1", &error_abort); - object_property_set_int(OBJECT(&bmc->soc), cfg->hw_strap2, "hw-strap2", + object_property_set_int(OBJECT(&bmc->soc), amc->hw_strap2, "hw-strap2", &error_abort); - object_property_set_int(OBJECT(&bmc->soc), cfg->num_cs, "num-cs", + object_property_set_int(OBJECT(&bmc->soc), amc->num_cs, "num-cs", &error_abort); object_property_set_int(OBJECT(&bmc->soc), machine->smp.cpus, "num-cpus", &error_abort); @@ -XXX,XX +XXX,XX @@ static void aspeed_board_init(MachineState *machine, "max_ram", max_ram_size - ram_size); memory_region_add_subregion(&bmc->ram_container, ram_size, &bmc->max_ram); - aspeed_board_init_flashes(&bmc->soc.fmc, cfg->fmc_model, &error_abort); - aspeed_board_init_flashes(&bmc->soc.spi[0], cfg->spi_model, &error_abort); + aspeed_board_init_flashes(&bmc->soc.fmc, amc->fmc_model, &error_abort); + aspeed_board_init_flashes(&bmc->soc.spi[0], amc->spi_model, &error_abort); /* Install first FMC flash content as a boot rom. */ if (drive0) { @@ -XXX,XX +XXX,XX @@ static void aspeed_board_init(MachineState *machine, aspeed_board_binfo.loader_start = sc->memmap[ASPEED_SDRAM]; aspeed_board_binfo.nb_cpus = bmc->soc.num_cpus; - if (cfg->i2c_init) { - cfg->i2c_init(bmc); + if (amc->i2c_init) { + amc->i2c_init(bmc); } for (i = 0; i < ARRAY_SIZE(bmc->soc.sdhci.slots); i++) { @@ -XXX,XX +XXX,XX @@ static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc) 0x60); } -static void aspeed_machine_init(MachineState *machine) -{ - AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(machine); - - aspeed_board_init(machine, amc->board); -} - static void aspeed_machine_class_init(ObjectClass *oc, void *data) { MachineClass *mc = MACHINE_CLASS(oc); - AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); - const AspeedBoardConfig *board = data; - mc->desc = board->desc; mc->init = aspeed_machine_init; mc->max_cpus = ASPEED_CPUS_NUM; mc->no_floppy = 1; mc->no_cdrom = 1; mc->no_parallel = 1; - if (board->ram) { - mc->default_ram_size = board->ram; - } - amc->board = board; } -static const TypeInfo aspeed_machine_type = { - .name = TYPE_ASPEED_MACHINE, - .parent = TYPE_MACHINE, - .instance_size = sizeof(AspeedMachine), - .class_size = sizeof(AspeedMachineClass), - .abstract = true, -}; - -static const AspeedBoardConfig aspeed_boards[] = { - { - .name = MACHINE_TYPE_NAME("palmetto-bmc"), - .desc = "OpenPOWER Palmetto BMC (ARM926EJ-S)", - .soc_name = "ast2400-a1", - .hw_strap1 = PALMETTO_BMC_HW_STRAP1, - .fmc_model = "n25q256a", - .spi_model = "mx25l25635e", - .num_cs = 1, - .i2c_init = palmetto_bmc_i2c_init, - .ram = 256 * MiB, - }, { - .name = MACHINE_TYPE_NAME("ast2500-evb"), - .desc = "Aspeed AST2500 EVB (ARM1176)", - .soc_name = "ast2500-a1", - .hw_strap1 = AST2500_EVB_HW_STRAP1, - .fmc_model = "w25q256", - .spi_model = "mx25l25635e", - .num_cs = 1, - .i2c_init = ast2500_evb_i2c_init, - .ram = 512 * MiB, - }, { - .name = MACHINE_TYPE_NAME("romulus-bmc"), - .desc = "OpenPOWER Romulus BMC (ARM1176)", - .soc_name = "ast2500-a1", - .hw_strap1 = ROMULUS_BMC_HW_STRAP1, - .fmc_model = "n25q256a", - .spi_model = "mx66l1g45g", - .num_cs = 2, - .i2c_init = romulus_bmc_i2c_init, - .ram = 512 * MiB, - }, { - .name = MACHINE_TYPE_NAME("swift-bmc"), - .desc = "OpenPOWER Swift BMC (ARM1176)", - .soc_name = "ast2500-a1", - .hw_strap1 = SWIFT_BMC_HW_STRAP1, - .fmc_model = "mx66l1g45g", - .spi_model = "mx66l1g45g", - .num_cs = 2, - .i2c_init = swift_bmc_i2c_init, - .ram = 512 * MiB, - }, { - .name = MACHINE_TYPE_NAME("witherspoon-bmc"), - .desc = "OpenPOWER Witherspoon BMC (ARM1176)", - .soc_name = "ast2500-a1", - .hw_strap1 = WITHERSPOON_BMC_HW_STRAP1, - .fmc_model = "mx25l25635e", - .spi_model = "mx66l1g45g", - .num_cs = 2, - .i2c_init = witherspoon_bmc_i2c_init, - .ram = 512 * MiB, - }, { - .name = MACHINE_TYPE_NAME("ast2600-evb"), - .desc = "Aspeed AST2600 EVB (Cortex A7)", - .soc_name = "ast2600-a0", - .hw_strap1 = AST2600_EVB_HW_STRAP1, - .hw_strap2 = AST2600_EVB_HW_STRAP2, - .fmc_model = "w25q512jv", - .spi_model = "mx66u51235f", - .num_cs = 1, - .i2c_init = ast2600_evb_i2c_init, - .ram = 1 * GiB, - }, -}; - -static void aspeed_machine_types(void) +static void aspeed_machine_palmetto_class_init(ObjectClass *oc, void *data) { - int i; + MachineClass *mc = MACHINE_CLASS(oc); + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); - type_register_static(&aspeed_machine_type); - for (i = 0; i < ARRAY_SIZE(aspeed_boards); ++i) { - TypeInfo ti = { - .name = aspeed_boards[i].name, - .parent = TYPE_ASPEED_MACHINE, - .class_init = aspeed_machine_class_init, - .class_data = (void *)&aspeed_boards[i], - }; - type_register(&ti); + mc->desc = "OpenPOWER Palmetto BMC (ARM926EJ-S)"; + amc->soc_name = "ast2400-a1"; + amc->hw_strap1 = PALMETTO_BMC_HW_STRAP1; + amc->fmc_model = "n25q256a"; + amc->spi_model = "mx25l25635e"; + amc->num_cs = 1; + amc->i2c_init = palmetto_bmc_i2c_init; + mc->default_ram_size = 256 * MiB; +}; + +static void aspeed_machine_ast2500_evb_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); + + mc->desc = "Aspeed AST2500 EVB (ARM1176)"; + amc->soc_name = "ast2500-a1"; + amc->hw_strap1 = AST2500_EVB_HW_STRAP1; + amc->fmc_model = "w25q256"; + amc->spi_model = "mx25l25635e"; + amc->num_cs = 1; + amc->i2c_init = ast2500_evb_i2c_init; + mc->default_ram_size = 512 * MiB; +}; + +static void aspeed_machine_romulus_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); + + mc->desc = "OpenPOWER Romulus BMC (ARM1176)"; + amc->soc_name = "ast2500-a1"; + amc->hw_strap1 = ROMULUS_BMC_HW_STRAP1; + amc->fmc_model = "n25q256a"; + amc->spi_model = "mx66l1g45g"; + amc->num_cs = 2; + amc->i2c_init = romulus_bmc_i2c_init; + mc->default_ram_size = 512 * MiB; +}; + +static void aspeed_machine_swift_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); + + mc->desc = "OpenPOWER Swift BMC (ARM1176)"; + amc->soc_name = "ast2500-a1"; + amc->hw_strap1 = SWIFT_BMC_HW_STRAP1; + amc->fmc_model = "mx66l1g45g"; + amc->spi_model = "mx66l1g45g"; + amc->num_cs = 2; + amc->i2c_init = swift_bmc_i2c_init; + mc->default_ram_size = 512 * MiB; +}; + +static void aspeed_machine_witherspoon_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); + + mc->desc = "OpenPOWER Witherspoon BMC (ARM1176)"; + amc->soc_name = "ast2500-a1"; + amc->hw_strap1 = WITHERSPOON_BMC_HW_STRAP1; + amc->fmc_model = "mx25l25635e"; + amc->spi_model = "mx66l1g45g"; + amc->num_cs = 2; + amc->i2c_init = witherspoon_bmc_i2c_init; + mc->default_ram_size = 512 * MiB; +}; + +static void aspeed_machine_ast2600_evb_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); + + mc->desc = "Aspeed AST2600 EVB (Cortex A7)"; + amc->soc_name = "ast2600-a0"; + amc->hw_strap1 = AST2600_EVB_HW_STRAP1; + amc->hw_strap2 = AST2600_EVB_HW_STRAP2; + amc->fmc_model = "w25q512jv"; + amc->spi_model = "mx66u51235f"; + amc->num_cs = 1; + amc->i2c_init = ast2600_evb_i2c_init; + mc->default_ram_size = 1 * GiB; +}; + +static const TypeInfo aspeed_machine_types[] = { + { + .name = MACHINE_TYPE_NAME("palmetto-bmc"), + .parent = TYPE_ASPEED_MACHINE, + .class_init = aspeed_machine_palmetto_class_init, + }, { + .name = MACHINE_TYPE_NAME("ast2500-evb"), + .parent = TYPE_ASPEED_MACHINE, + .class_init = aspeed_machine_ast2500_evb_class_init, + }, { + .name = MACHINE_TYPE_NAME("romulus-bmc"), + .parent = TYPE_ASPEED_MACHINE, + .class_init = aspeed_machine_romulus_class_init, + }, { + .name = MACHINE_TYPE_NAME("swift-bmc"), + .parent = TYPE_ASPEED_MACHINE, + .class_init = aspeed_machine_swift_class_init, + }, { + .name = MACHINE_TYPE_NAME("witherspoon-bmc"), + .parent = TYPE_ASPEED_MACHINE, + .class_init = aspeed_machine_witherspoon_class_init, + }, { + .name = MACHINE_TYPE_NAME("ast2600-evb"), + .parent = TYPE_ASPEED_MACHINE, + .class_init = aspeed_machine_ast2600_evb_class_init, + }, { + .name = TYPE_ASPEED_MACHINE, + .parent = TYPE_MACHINE, + .instance_size = sizeof(AspeedMachine), + .class_size = sizeof(AspeedMachineClass), + .class_init = aspeed_machine_class_init, + .abstract = true, } -} +}; -type_init(aspeed_machine_types) +DEFINE_TYPES(aspeed_machine_types) -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> The Tacoma BMC board is replacement board for the BMC of the OpenPOWER Witherspoon system. It uses a AST2600 SoC instead of a AST2500 and the I2C layout is the same as it controls the same main board. Used for HW bringup. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-15-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/aspeed.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -XXX,XX +XXX,XX @@ struct AspeedBoardState { #define AST2600_EVB_HW_STRAP1 0x000000C0 #define AST2600_EVB_HW_STRAP2 0x00000003 +/* Tacoma hardware value */ +#define TACOMA_BMC_HW_STRAP1 0x00000000 +#define TACOMA_BMC_HW_STRAP2 0x00000000 + /* * The max ram region is for firmwares that scan the address space * with load/store to guess how much RAM the SoC has. @@ -XXX,XX +XXX,XX @@ static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc) AspeedSoCState *soc = &bmc->soc; uint8_t *eeprom_buf = g_malloc0(8 * 1024); + /* Bus 3: TODO bmp280@77 */ + /* Bus 3: TODO max31785@52 */ + /* Bus 3: TODO dps310@76 */ i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 3), TYPE_PCA9552, 0x60); @@ -XXX,XX +XXX,XX @@ static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc) eeprom_buf); i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11), TYPE_PCA9552, 0x60); + /* Bus 11: TODO ucd90160@64 */ } static void aspeed_machine_class_init(ObjectClass *oc, void *data) @@ -XXX,XX +XXX,XX @@ static void aspeed_machine_ast2600_evb_class_init(ObjectClass *oc, void *data) mc->default_ram_size = 1 * GiB; }; +static void aspeed_machine_tacoma_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); + + mc->desc = "Aspeed AST2600 EVB (Cortex A7)"; + amc->soc_name = "ast2600-a0"; + amc->hw_strap1 = TACOMA_BMC_HW_STRAP1; + amc->hw_strap2 = TACOMA_BMC_HW_STRAP2; + amc->fmc_model = "mx66l1g45g"; + amc->spi_model = "mx66l1g45g"; + amc->num_cs = 2; + amc->i2c_init = witherspoon_bmc_i2c_init; /* Same board layout */ + mc->default_ram_size = 1 * GiB; +}; + static const TypeInfo aspeed_machine_types[] = { { .name = MACHINE_TYPE_NAME("palmetto-bmc"), @@ -XXX,XX +XXX,XX @@ static const TypeInfo aspeed_machine_types[] = { .name = MACHINE_TYPE_NAME("ast2600-evb"), .parent = TYPE_ASPEED_MACHINE, .class_init = aspeed_machine_ast2600_evb_class_init, + }, { + .name = MACHINE_TYPE_NAME("tacoma-bmc"), + .parent = TYPE_ASPEED_MACHINE, + .class_init = aspeed_machine_tacoma_class_init, }, { .name = TYPE_ASPEED_MACHINE, .parent = TYPE_MACHINE, -- 2.20.1 From: PanNengyuan <pannengyuan@huawei.com> Address Sanitizer shows memory leak in hw/gpio/aspeed_gpio.c:875 Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: PanNengyuan <pannengyuan@huawei.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-16-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/gpio/aspeed_gpio.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c index XXXXXXX..XXXXXXX 100644 --- a/hw/gpio/aspeed_gpio.c +++ b/hw/gpio/aspeed_gpio.c @@ -XXX,XX +XXX,XX @@ static void aspeed_gpio_init(Object *obj) pin_idx % GPIOS_PER_GROUP); object_property_add(obj, name, "bool", aspeed_gpio_get_pin, aspeed_gpio_set_pin, NULL, NULL, NULL); + g_free(name); } } -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> The Aspeed Watchdog and Timer models have a link pointing to the SCU controller model of the machine. Change the "scu" property definition so that it explicitly sets the pointer. The property isn't optional : not being able to set the link is a bug and QEMU should rather abort than exit in this case. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Greg Kurz <groug@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-17-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/aspeed_ast2600.c | 8 ++++---- hw/arm/aspeed_soc.c | 8 ++++---- hw/timer/aspeed_timer.c | 17 +++++++++-------- hw/watchdog/wdt_aspeed.c | 17 ++++++++--------- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed_ast2600.c +++ b/hw/arm/aspeed_ast2600.c @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_init(Object *obj) snprintf(typename, sizeof(typename), "aspeed.timer-%s", socname); sysbus_init_child_obj(obj, "timerctrl", OBJECT(&s->timerctrl), sizeof(s->timerctrl), typename); - object_property_add_const_link(OBJECT(&s->timerctrl), "scu", - OBJECT(&s->scu), &error_abort); snprintf(typename, sizeof(typename), "aspeed.i2c-%s", socname); sysbus_init_child_obj(obj, "i2c", OBJECT(&s->i2c), sizeof(s->i2c), @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_init(Object *obj) snprintf(typename, sizeof(typename), "aspeed.wdt-%s", socname); sysbus_init_child_obj(obj, "wdt[*]", OBJECT(&s->wdt[i]), sizeof(s->wdt[i]), typename); - object_property_add_const_link(OBJECT(&s->wdt[i]), "scu", - OBJECT(&s->scu), &error_abort); } for (i = 0; i < sc->macs_num; i++) { @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp) aspeed_soc_get_irq(s, ASPEED_RTC)); /* Timer */ + object_property_set_link(OBJECT(&s->timerctrl), + OBJECT(&s->scu), "scu", &error_abort); object_property_set_bool(OBJECT(&s->timerctrl), true, "realized", &err); if (err) { error_propagate(errp, err); @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp) for (i = 0; i < sc->wdts_num; i++) { AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(&s->wdt[i]); + object_property_set_link(OBJECT(&s->wdt[i]), + OBJECT(&s->scu), "scu", &error_abort); object_property_set_bool(OBJECT(&s->wdt[i]), true, "realized", &err); if (err) { error_propagate(errp, err); 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) snprintf(typename, sizeof(typename), "aspeed.timer-%s", socname); sysbus_init_child_obj(obj, "timerctrl", OBJECT(&s->timerctrl), sizeof(s->timerctrl), typename); - object_property_add_const_link(OBJECT(&s->timerctrl), "scu", - OBJECT(&s->scu), &error_abort); snprintf(typename, sizeof(typename), "aspeed.i2c-%s", socname); sysbus_init_child_obj(obj, "i2c", OBJECT(&s->i2c), sizeof(s->i2c), @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_init(Object *obj) snprintf(typename, sizeof(typename), "aspeed.wdt-%s", socname); sysbus_init_child_obj(obj, "wdt[*]", OBJECT(&s->wdt[i]), sizeof(s->wdt[i]), typename); - object_property_add_const_link(OBJECT(&s->wdt[i]), "scu", - OBJECT(&s->scu), &error_abort); } for (i = 0; i < sc->macs_num; i++) { @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) aspeed_soc_get_irq(s, ASPEED_RTC)); /* Timer */ + object_property_set_link(OBJECT(&s->timerctrl), + OBJECT(&s->scu), "scu", &error_abort); object_property_set_bool(OBJECT(&s->timerctrl), true, "realized", &err); if (err) { error_propagate(errp, err); @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) for (i = 0; i < sc->wdts_num; i++) { AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(&s->wdt[i]); + object_property_set_link(OBJECT(&s->wdt[i]), + OBJECT(&s->scu), "scu", &error_abort); object_property_set_bool(OBJECT(&s->wdt[i]), true, "realized", &err); if (err) { error_propagate(errp, err); 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/timer.h" #include "qemu/log.h" #include "qemu/module.h" +#include "hw/qdev-properties.h" #include "trace.h" #define TIMER_NR_REGS 4 @@ -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_prepend(errp, err, "required link 'scu' not found: "); - return; - } - s->scu = ASPEED_SCU(obj); + assert(s->scu); for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) { aspeed_init_one_timer(s, i); @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_aspeed_timer_state = { } }; +static Property aspeed_timer_properties[] = { + DEFINE_PROP_LINK("scu", AspeedTimerCtrlState, scu, TYPE_ASPEED_SCU, + AspeedSCUState *), + DEFINE_PROP_END_OF_LIST(), +}; + static void timer_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static void timer_class_init(ObjectClass *klass, void *data) dc->reset = aspeed_timer_reset; dc->desc = "ASPEED Timer"; dc->vmsd = &vmstate_aspeed_timer_state; + dc->props = aspeed_timer_properties; } static const TypeInfo aspeed_timer_info = { diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c index XXXXXXX..XXXXXXX 100644 --- a/hw/watchdog/wdt_aspeed.c +++ b/hw/watchdog/wdt_aspeed.c @@ -XXX,XX +XXX,XX @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp) { SysBusDevice *sbd = SYS_BUS_DEVICE(dev); AspeedWDTState *s = ASPEED_WDT(dev); - Error *err = NULL; - Object *obj; - 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); + assert(s->scu); s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, aspeed_wdt_timer_expired, dev); @@ -XXX,XX +XXX,XX @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp) sysbus_init_mmio(sbd, &s->iomem); } +static Property aspeed_wdt_properties[] = { + DEFINE_PROP_LINK("scu", AspeedWDTState, scu, TYPE_ASPEED_SCU, + AspeedSCUState *), + DEFINE_PROP_END_OF_LIST(), +}; + static void aspeed_wdt_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static void aspeed_wdt_class_init(ObjectClass *klass, void *data) dc->reset = aspeed_wdt_reset; set_bit(DEVICE_CATEGORY_MISC, dc->categories); dc->vmsd = &vmstate_aspeed_wdt; + dc->props = aspeed_wdt_properties; } static const TypeInfo aspeed_wdt_info = { -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> The Aspeed MII model has a link pointing to its associated FTGMAC100 NIC in the machine. Change the "nic" property definition so that it explicitly sets the pointer. The property isn't optional : not being able to set the link is a bug and QEMU should rather abort than exit in this case. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Greg Kurz <groug@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-18-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/aspeed_ast2600.c | 5 ++--- hw/net/ftgmac100.c | 19 +++++++++---------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed_ast2600.c +++ b/hw/arm/aspeed_ast2600.c @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_init(Object *obj) sysbus_init_child_obj(obj, "mii[*]", &s->mii[i], sizeof(s->mii[i]), TYPE_ASPEED_MII); - object_property_add_const_link(OBJECT(&s->mii[i]), "nic", - OBJECT(&s->ftgmac100[i]), - &error_abort); } sysbus_init_child_obj(obj, "xdma", OBJECT(&s->xdma), sizeof(s->xdma), @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp) sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[i]), 0, aspeed_soc_get_irq(s, ASPEED_ETH1 + i)); + object_property_set_link(OBJECT(&s->mii[i]), OBJECT(&s->ftgmac100[i]), + "nic", &error_abort); object_property_set_bool(OBJECT(&s->mii[i]), true, "realized", &err); if (err) { diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c index XXXXXXX..XXXXXXX 100644 --- a/hw/net/ftgmac100.c +++ b/hw/net/ftgmac100.c @@ -XXX,XX +XXX,XX @@ static void aspeed_mii_realize(DeviceState *dev, Error **errp) { AspeedMiiState *s = ASPEED_MII(dev); SysBusDevice *sbd = SYS_BUS_DEVICE(dev); - Object *obj; - Error *local_err = NULL; - obj = object_property_get_link(OBJECT(dev), "nic", &local_err); - if (!obj) { - error_propagate(errp, local_err); - error_prepend(errp, "required link 'nic' not found: "); - return; - } - - s->nic = FTGMAC100(obj); + assert(s->nic); memory_region_init_io(&s->iomem, OBJECT(dev), &aspeed_mii_ops, s, TYPE_ASPEED_MII, 0x8); @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_aspeed_mii = { VMSTATE_END_OF_LIST() } }; + +static Property aspeed_mii_properties[] = { + DEFINE_PROP_LINK("nic", AspeedMiiState, nic, TYPE_FTGMAC100, + FTGMAC100State *), + DEFINE_PROP_END_OF_LIST(), +}; + static void aspeed_mii_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static void aspeed_mii_class_init(ObjectClass *klass, void *data) dc->reset = aspeed_mii_reset; dc->realize = aspeed_mii_realize; dc->desc = "Aspeed MII controller"; + dc->props = aspeed_mii_properties; } static const TypeInfo aspeed_mii_info = { -- 2.20.1 From: Marc Zyngier <maz@kernel.org> HCR_EL2.TID2 mandates that access from EL1 to CTR_EL0, CCSIDR_EL1, CCSIDR2_EL1, CLIDR_EL1, CSSELR_EL1 are trapped to EL2, and QEMU completely ignores it, making it impossible for hypervisors to virtualize the cache hierarchy. Do the right thing by trapping to EL2 if HCR_EL2.TID2 is set. Signed-off-by: Marc Zyngier <maz@kernel.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20191201122018.25808-2-maz@kernel.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 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 scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) raw_write(env, ri, value); } +static CPAccessResult access_aa64_tid2(CPUARMState *env, + const ARMCPRegInfo *ri, + bool isread) +{ + if (arm_current_el(env) == 1 && (arm_hcr_el2_eff(env) & HCR_TID2)) { + return CP_ACCESS_TRAP_EL2; + } + + return CP_ACCESS_OK; +} + static uint64_t ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri) { ARMCPU *cpu = env_archcpu(env); @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v7_cp_reginfo[] = { .writefn = pmintenclr_write }, { .name = "CCSIDR", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 0, - .access = PL1_R, .readfn = ccsidr_read, .type = ARM_CP_NO_RAW }, + .access = PL1_R, + .accessfn = access_aa64_tid2, + .readfn = ccsidr_read, .type = ARM_CP_NO_RAW }, { .name = "CSSELR", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 2, .opc2 = 0, - .access = PL1_RW, .writefn = csselr_write, .resetvalue = 0, + .access = PL1_RW, + .accessfn = access_aa64_tid2, + .writefn = csselr_write, .resetvalue = 0, .bank_fieldoffsets = { offsetof(CPUARMState, cp15.csselr_s), offsetof(CPUARMState, cp15.csselr_ns) } }, /* Auxiliary ID register: this actually has an IMPDEF value but for now @@ -XXX,XX +XXX,XX @@ static CPAccessResult ctr_el0_access(CPUARMState *env, const ARMCPRegInfo *ri, if (arm_current_el(env) == 0 && !(env->cp15.sctlr_el[1] & SCTLR_UCT)) { return CP_ACCESS_TRAP; } + + if (arm_current_el(env) < 2 && arm_hcr_el2_eff(env) & HCR_TID2) { + return CP_ACCESS_TRAP_EL2; + } + return CP_ACCESS_OK; } @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) ARMCPRegInfo clidr = { .name = "CLIDR", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 1, - .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->clidr + .access = PL1_R, .type = ARM_CP_CONST, + .accessfn = access_aa64_tid2, + .resetvalue = cpu->clidr }; define_one_arm_cp_reg(cpu, &clidr); define_arm_cp_regs(cpu, v7_cp_reginfo); @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) /* These are common to v8 and pre-v8 */ { .name = "CTR", .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 1, - .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->ctr }, + .access = PL1_R, .accessfn = ctr_el0_access, + .type = ARM_CP_CONST, .resetvalue = cpu->ctr }, { .name = "CTR_EL0", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 0, .crm = 0, .access = PL0_R, .accessfn = ctr_el0_access, -- 2.20.1 From: Marc Zyngier <maz@kernel.org> HCR_EL2.TID1 mandates that access from EL1 to REVIDR_EL1, AIDR_EL1 (and their 32bit equivalents) as well as TCMTR, TLBTR are trapped to EL2. QEMU ignores it, making it harder for a hypervisor to virtualize the HW (though to be fair, no known hypervisor actually cares). Do the right thing by trapping to EL2 if HCR_EL2.TID1 is set. Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Signed-off-by: Marc Zyngier <maz@kernel.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20191201122018.25808-3-maz@kernel.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 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 uint64_t isr_read(CPUARMState *env, const ARMCPRegInfo *ri) return ret; } +static CPAccessResult access_aa64_tid1(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) +{ + if (arm_current_el(env) == 1 && (arm_hcr_el2_eff(env) & HCR_TID1)) { + return CP_ACCESS_TRAP_EL2; + } + + return CP_ACCESS_OK; +} + +static CPAccessResult access_aa32_tid1(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) +{ + if (arm_feature(env, ARM_FEATURE_V8)) { + return access_aa64_tid1(env, ri, isread); + } + + return CP_ACCESS_OK; +} + static const ARMCPRegInfo v7_cp_reginfo[] = { /* the old v6 WFI, UNPREDICTABLE in v7 but we choose to NOP */ { .name = "NOP", .cp = 15, .crn = 7, .crm = 0, .opc1 = 0, .opc2 = 4, @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v7_cp_reginfo[] = { */ { .name = "AIDR", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .opc1 = 1, .crn = 0, .crm = 0, .opc2 = 7, - .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 }, + .access = PL1_R, .type = ARM_CP_CONST, + .accessfn = access_aa64_tid1, + .resetvalue = 0 }, /* Auxiliary fault status registers: these also are IMPDEF, and we * choose to RAZ/WI for all cores. */ @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) .access = PL1_R, .resetvalue = cpu->midr }, { .name = "REVIDR_EL1", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 0, .opc2 = 6, - .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->revidr }, + .access = PL1_R, + .accessfn = access_aa64_tid1, + .type = ARM_CP_CONST, .resetvalue = cpu->revidr }, REGINFO_SENTINEL }; ARMCPRegInfo id_cp_reginfo[] = { @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) /* TCMTR and TLBTR exist in v8 but have no 64-bit versions */ { .name = "TCMTR", .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 2, - .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 }, + .access = PL1_R, + .accessfn = access_aa32_tid1, + .type = ARM_CP_CONST, .resetvalue = 0 }, REGINFO_SENTINEL }; /* TLBTR is specific to VMSA */ ARMCPRegInfo id_tlbtr_reginfo = { .name = "TLBTR", .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 3, - .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0, + .access = PL1_R, + .accessfn = access_aa32_tid1, + .type = ARM_CP_CONST, .resetvalue = 0, }; /* MPUIR is specific to PMSA V6+ */ ARMCPRegInfo id_mpuir_reginfo = { -- 2.20.1 From: Marc Zyngier <maz@kernel.org> HCR_EL2.TID3 requires that AArch32 reads of MVFR[012] are trapped to EL2, and HCR_EL2.TID0 does the same for reads of FPSID. In order to handle this, introduce a new TCG helper function that checks for these control bits before executing the VMRC instruction. Tested with a hacked-up version of KVM/arm64 that sets the control bits for 32bit guests. Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Signed-off-by: Marc Zyngier <maz@kernel.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20191201122018.25808-4-maz@kernel.org [PMM: move helper declaration to helper.h; make it TCG_CALL_NO_WG] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.h | 2 ++ target/arm/translate-vfp.inc.c | 20 ++++++++++++++++---- target/arm/vfp_helper.c | 29 +++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/target/arm/helper.h b/target/arm/helper.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -XXX,XX +XXX,XX @@ DEF_HELPER_FLAGS_2(rintd, TCG_CALL_NO_RWG, f64, f64, ptr) DEF_HELPER_FLAGS_2(vjcvt, TCG_CALL_NO_RWG, i32, f64, env) DEF_HELPER_FLAGS_2(fjcvtzs, TCG_CALL_NO_RWG, i64, f64, ptr) +DEF_HELPER_FLAGS_3(check_hcr_el2_trap, TCG_CALL_NO_WG, void, env, i32, i32) + /* neon_helper.c */ DEF_HELPER_FLAGS_3(neon_qadd_u8, TCG_CALL_NO_RWG, i32, env, i32, i32) DEF_HELPER_FLAGS_3(neon_qadd_s8, TCG_CALL_NO_RWG, i32, env, i32, i32) diff --git a/target/arm/translate-vfp.inc.c b/target/arm/translate-vfp.inc.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-vfp.inc.c +++ b/target/arm/translate-vfp.inc.c @@ -XXX,XX +XXX,XX @@ static bool trans_VMSR_VMRS(DisasContext *s, arg_VMSR_VMRS *a) if (a->l) { /* VMRS, move VFP special register to gp register */ switch (a->reg) { - case ARM_VFP_FPSID: - case ARM_VFP_FPEXC: - case ARM_VFP_FPINST: - case ARM_VFP_FPINST2: case ARM_VFP_MVFR0: case ARM_VFP_MVFR1: case ARM_VFP_MVFR2: + case ARM_VFP_FPSID: + if (s->current_el == 1) { + TCGv_i32 tcg_reg, tcg_rt; + + gen_set_condexec(s); + gen_set_pc_im(s, s->pc_curr); + tcg_reg = tcg_const_i32(a->reg); + tcg_rt = tcg_const_i32(a->rt); + gen_helper_check_hcr_el2_trap(cpu_env, tcg_rt, tcg_reg); + tcg_temp_free_i32(tcg_reg); + tcg_temp_free_i32(tcg_rt); + } + /* fall through */ + case ARM_VFP_FPEXC: + case ARM_VFP_FPINST: + case ARM_VFP_FPINST2: tmp = load_cpu_field(vfp.xregs[a->reg]); break; case ARM_VFP_FPSCR: diff --git a/target/arm/vfp_helper.c b/target/arm/vfp_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/vfp_helper.c +++ b/target/arm/vfp_helper.c @@ -XXX,XX +XXX,XX @@ float64 HELPER(frint64_d)(float64 f, void *fpst) return frint_d(f, fpst, 64); } +void HELPER(check_hcr_el2_trap)(CPUARMState *env, uint32_t rt, uint32_t reg) +{ + uint32_t syndrome; + + switch (reg) { + case ARM_VFP_MVFR0: + case ARM_VFP_MVFR1: + case ARM_VFP_MVFR2: + if (!(arm_hcr_el2_eff(env) & HCR_TID3)) { + return; + } + break; + case ARM_VFP_FPSID: + if (!(arm_hcr_el2_eff(env) & HCR_TID0)) { + return; + } + break; + default: + g_assert_not_reached(); + } + + syndrome = ((EC_FPIDTRAP << ARM_EL_EC_SHIFT) + | ARM_EL_IL + | (1 << 24) | (0xe << 20) | (7 << 14) + | (reg << 10) | (rt << 5) | 1); + + raise_exception(env, EXCP_HYP_TRAP, syndrome, 2); +} + #endif -- 2.20.1 From: Marc Zyngier <maz@kernel.org> HSTR_EL2 offers a way to trap ranges of CP15 system register accesses to EL2, and it looks like this register is completely ignored by QEMU. To avoid adding extra .accessfn filters all over the place (which would have a direct performance impact), let's add a new TB flag that gets set whenever HSTR_EL2 is non-zero and that QEMU translates a context where this trap has a chance to apply, and only generate the extra access check if the hypervisor is actively using this feature. Tested with a hand-crafted KVM guest accessing CBAR. Signed-off-by: Marc Zyngier <maz@kernel.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20191201122018.25808-5-maz@kernel.org [PMM: use is_a64(); fix comment syntax] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 2 ++ target/arm/translate.h | 2 ++ target/arm/helper.c | 6 ++++++ target/arm/op_helper.c | 22 ++++++++++++++++++++++ target/arm/translate.c | 3 ++- 5 files changed, 34 insertions(+), 1 deletion(-) 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 @@ FIELD(TBFLAG_A32, NS, 6, 1) FIELD(TBFLAG_A32, VFPEN, 7, 1) /* Partially cached, minus FPEXC. */ FIELD(TBFLAG_A32, CONDEXEC, 8, 8) /* Not cached. */ FIELD(TBFLAG_A32, SCTLR_B, 16, 1) +FIELD(TBFLAG_A32, HSTR_ACTIVE, 17, 1) + /* For M profile only, set if FPCCR.LSPACT is set */ FIELD(TBFLAG_A32, LSPACT, 18, 1) /* Not cached. */ /* For M profile only, set if we must create a new FP context */ diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ typedef struct DisasContext { bool pauth_active; /* True with v8.5-BTI and SCTLR_ELx.BT* set. */ bool bt; + /* True if any CP15 access is trapped by HSTR_EL2 */ + bool hstr_active; /* * >= 0, a copy of PSTATE.BTYPE, which will be 0 without v8.5-BTI. * < 0, set by the current instruction. 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 uint32_t rebuild_hflags_a32(CPUARMState *env, int fp_el, if (arm_el_is_aa64(env, 1)) { flags = FIELD_DP32(flags, TBFLAG_A32, VFPEN, 1); } + + if (arm_current_el(env) < 2 && env->cp15.hstr_el2 && + (arm_hcr_el2_eff(env) & (HCR_E2H | HCR_TGE)) != (HCR_E2H | HCR_TGE)) { + flags = FIELD_DP32(flags, TBFLAG_A32, HSTR_ACTIVE, 1); + } + return rebuild_hflags_common_32(env, fp_el, mmu_idx, flags); } diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -XXX,XX +XXX,XX @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome, raise_exception(env, EXCP_UDEF, syndrome, exception_target_el(env)); } + /* + * Check for an EL2 trap due to HSTR_EL2. We expect EL0 accesses + * to sysregs non accessible at EL0 to have UNDEF-ed already. + */ + if (!is_a64(env) && arm_current_el(env) < 2 && ri->cp == 15 && + (arm_hcr_el2_eff(env) & (HCR_E2H | HCR_TGE)) != (HCR_E2H | HCR_TGE)) { + uint32_t mask = 1 << ri->crn; + + if (ri->type & ARM_CP_64BIT) { + mask = 1 << ri->crm; + } + + /* T4 and T14 are RES0 */ + mask &= ~((1 << 4) | (1 << 14)); + + if (env->cp15.hstr_el2 & mask) { + target_el = 2; + goto exept; + } + } + if (!ri->accessfn) { return; } @@ -XXX,XX +XXX,XX @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome, g_assert_not_reached(); } +exept: raise_exception(env, EXCP_UDEF, syndrome, target_el); } diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static int disas_coproc_insn(DisasContext *s, uint32_t insn) return 1; } - if (ri->accessfn || + if (s->hstr_active || ri->accessfn || (arm_dc_feature(s, ARM_FEATURE_XSCALE) && cpnum < 14)) { /* Emit code to perform further access permissions checks at * runtime; this may result in an exception. @@ -XXX,XX +XXX,XX @@ static void arm_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) !arm_el_is_aa64(env, 3); dc->thumb = FIELD_EX32(tb_flags, TBFLAG_A32, THUMB); dc->sctlr_b = FIELD_EX32(tb_flags, TBFLAG_A32, SCTLR_B); + dc->hstr_active = FIELD_EX32(tb_flags, TBFLAG_A32, HSTR_ACTIVE); dc->be_data = FIELD_EX32(tb_flags, TBFLAG_ANY, BE_DATA) ? MO_BE : MO_LE; condexec = FIELD_EX32(tb_flags, TBFLAG_A32, CONDEXEC); dc->condexec_mask = (condexec & 0xf) << 1; -- 2.20.1 From: Marc Zyngier <maz@kernel.org> QEMU lacks the minimum Jazelle implementation that is required by the architecture (everything is RAZ or RAZ/WI). Add it together with the HCR_EL2.TID0 trapping that goes with it. Signed-off-by: Marc Zyngier <maz@kernel.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20191201122018.25808-6-maz@kernel.org [PMM: moved ARMCPRegInfo array to file scope, marked it 'static global', moved new condition down in register_cp_regs_for_features() to go with other feature things rather than up with the v6/v7/v8 stuff] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 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 @@ static CPAccessResult access_aa32_tid3(CPUARMState *env, const ARMCPRegInfo *ri, return CP_ACCESS_OK; } +static CPAccessResult access_jazelle(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) +{ + if (arm_current_el(env) == 1 && (arm_hcr_el2_eff(env) & HCR_TID0)) { + return CP_ACCESS_TRAP_EL2; + } + + return CP_ACCESS_OK; +} + +static const ARMCPRegInfo jazelle_regs[] = { + { .name = "JIDR", + .cp = 14, .crn = 0, .crm = 0, .opc1 = 7, .opc2 = 0, + .access = PL1_R, .accessfn = access_jazelle, + .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "JOSCR", + .cp = 14, .crn = 1, .crm = 0, .opc1 = 7, .opc2 = 0, + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "JMCR", + .cp = 14, .crn = 2, .crm = 0, .opc1 = 7, .opc2 = 0, + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + REGINFO_SENTINEL +}; + void register_cp_regs_for_features(ARMCPU *cpu) { /* Register all the coprocessor registers based on feature bits */ @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) if (arm_feature(env, ARM_FEATURE_LPAE)) { define_arm_cp_regs(cpu, lpae_cp_reginfo); } + if (cpu_isar_feature(jazelle, cpu)) { + define_arm_cp_regs(cpu, jazelle_regs); + } /* Slightly awkwardly, the OMAP and StrongARM cores need all of * cp15 crn=0 to be writes-ignored, whereas for other cores they should * be read-only (ie write causes UNDEF exception). -- 2.20.1 From: Niek Linnenbank <nieklinnenbank@gmail.com> This change ensures that the FPU can be accessed in Non-Secure mode when the CPU core is reset using the arm_set_cpu_on() function call. The NSACR.{CP11,CP10} bits define the exception level required to access the FPU in Non-Secure mode. Without these bits set, the CPU will give an undefined exception trap on the first FPU access for the secondary cores under Linux. This is necessary because in this power-control codepath QEMU is effectively emulating a bit of EL3 firmware, and has to set the CPU up as the EL3 firmware would. Fixes: fc1120a7f5 Cc: qemu-stable@nongnu.org Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com> [PMM: added clarifying para to commit message] Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/arm-powerctl.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/arm-powerctl.c +++ b/target/arm/arm-powerctl.c @@ -XXX,XX +XXX,XX @@ static void arm_set_cpu_on_async_work(CPUState *target_cpu_state, /* Processor is not in secure mode */ target_cpu->env.cp15.scr_el3 |= SCR_NS; + /* Set NSACR.{CP11,CP10} so NS can access the FPU */ + target_cpu->env.cp15.nsacr |= 3 << 10; + /* * If QEMU is providing the equivalent of EL3 firmware, then we need * to make sure a CPU targeting EL2 comes out of reset with a -- 2.20.1 From: Beata Michalska <beata.michalska@linaro.org> Add probe_read alongside the write probing equivalent. Signed-off-by: Beata Michalska <beata.michalska@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20191121000843.24844-2-beata.michalska@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/exec/exec-all.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -XXX,XX +XXX,XX @@ static inline void *probe_write(CPUArchState *env, target_ulong addr, int size, return probe_access(env, addr, size, MMU_DATA_STORE, mmu_idx, retaddr); } +static inline void *probe_read(CPUArchState *env, target_ulong addr, int size, + int mmu_idx, uintptr_t retaddr) +{ + return probe_access(env, addr, size, MMU_DATA_LOAD, mmu_idx, retaddr); +} + #define CODE_GEN_ALIGN 16 /* must be >= of the size of a icache line */ /* Estimated block size for TB allocation. */ -- 2.20.1 From: Beata Michalska <beata.michalska@linaro.org> Add an option to trigger memory writeback to sync given memory region with the corresponding backing store, case one is available. This extends the support for persistent memory, allowing syncing on-demand. Signed-off-by: Beata Michalska <beata.michalska@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20191121000843.24844-3-beata.michalska@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/exec/memory.h | 6 ++++++ include/exec/ram_addr.h | 8 ++++++++ include/qemu/cutils.h | 1 + exec.c | 36 ++++++++++++++++++++++++++++++++++++ memory.c | 12 ++++++++++++ util/cutils.c | 38 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 101 insertions(+) diff --git a/include/exec/memory.h b/include/exec/memory.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -XXX,XX +XXX,XX @@ void *memory_region_get_ram_ptr(MemoryRegion *mr); */ void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize, Error **errp); +/** + * memory_region_do_writeback: Trigger writeback for selected address range + * [addr, addr + size] + * + */ +void memory_region_do_writeback(MemoryRegion *mr, hwaddr addr, hwaddr size); /** * memory_region_set_log: Turn dirty logging on or off for a region. diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/ram_addr.h +++ b/include/exec/ram_addr.h @@ -XXX,XX +XXX,XX @@ void qemu_ram_free(RAMBlock *block); int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp); +void qemu_ram_writeback(RAMBlock *block, ram_addr_t start, ram_addr_t length); + +/* Clear whole block of mem */ +static inline void qemu_ram_block_writeback(RAMBlock *block) +{ + qemu_ram_writeback(block, 0, block->used_length); +} + #define DIRTY_CLIENTS_ALL ((1 << DIRTY_MEMORY_NUM) - 1) #define DIRTY_CLIENTS_NOCODE (DIRTY_CLIENTS_ALL & ~(1 << DIRTY_MEMORY_CODE)) diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h index XXXXXXX..XXXXXXX 100644 --- a/include/qemu/cutils.h +++ b/include/qemu/cutils.h @@ -XXX,XX +XXX,XX @@ const char *qemu_strchrnul(const char *s, int c); #endif time_t mktimegm(struct tm *tm); int qemu_fdatasync(int fd); +int qemu_msync(void *addr, size_t length, int fd); int fcntl_setfl(int fd, int flag); int qemu_parse_fd(const char *param); int qemu_strtoi(const char *nptr, const char **endptr, int base, diff --git a/exec.c b/exec.c index XXXXXXX..XXXXXXX 100644 --- a/exec.c +++ b/exec.c @@ -XXX,XX +XXX,XX @@ #include "exec/ram_addr.h" #include "exec/log.h" +#include "qemu/pmem.h" + #include "migration/vmstate.h" #include "qemu/range.h" @@ -XXX,XX +XXX,XX @@ int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp) return 0; } +/* + * Trigger sync on the given ram block for range [start, start + length] + * with the backing store if one is available. + * Otherwise no-op. + * @Note: this is supposed to be a synchronous op. + */ +void qemu_ram_writeback(RAMBlock *block, ram_addr_t start, ram_addr_t length) +{ + void *addr = ramblock_ptr(block, start); + + /* The requested range should fit in within the block range */ + g_assert((start + length) <= block->used_length); + +#ifdef CONFIG_LIBPMEM + /* The lack of support for pmem should not block the sync */ + if (ramblock_is_pmem(block)) { + pmem_persist(addr, length); + return; + } +#endif + if (block->fd >= 0) { + /** + * Case there is no support for PMEM or the memory has not been + * specified as persistent (or is not one) - use the msync. + * Less optimal but still achieves the same goal + */ + if (qemu_msync(addr, length, block->fd)) { + warn_report("%s: failed to sync memory range: start: " + RAM_ADDR_FMT " length: " RAM_ADDR_FMT, + __func__, start, length); + } + } +} + /* Called with ram_list.mutex held */ static void dirty_memory_extend(ram_addr_t old_ram_size, ram_addr_t new_ram_size) diff --git a/memory.c b/memory.c index XXXXXXX..XXXXXXX 100644 --- a/memory.c +++ b/memory.c @@ -XXX,XX +XXX,XX @@ void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize, Error **errp qemu_ram_resize(mr->ram_block, newsize, errp); } + +void memory_region_do_writeback(MemoryRegion *mr, hwaddr addr, hwaddr size) +{ + /* + * Might be extended case needed to cover + * different types of memory regions + */ + if (mr->ram_block && mr->dirty_log_mask) { + qemu_ram_writeback(mr->ram_block, addr, size); + } +} + /* * Call proper memory listeners about the change on the newly * added/removed CoalescedMemoryRange. diff --git a/util/cutils.c b/util/cutils.c index XXXXXXX..XXXXXXX 100644 --- a/util/cutils.c +++ b/util/cutils.c @@ -XXX,XX +XXX,XX @@ int qemu_fdatasync(int fd) #endif } +/** + * Sync changes made to the memory mapped file back to the backing + * storage. For POSIX compliant systems this will fallback + * to regular msync call. Otherwise it will trigger whole file sync + * (including the metadata case there is no support to skip that otherwise) + * + * @addr - start of the memory area to be synced + * @length - length of the are to be synced + * @fd - file descriptor for the file to be synced + * (mandatory only for POSIX non-compliant systems) + */ +int qemu_msync(void *addr, size_t length, int fd) +{ +#ifdef CONFIG_POSIX + size_t align_mask = ~(qemu_real_host_page_size - 1); + + /** + * There are no strict reqs as per the length of mapping + * to be synced. Still the length needs to follow the address + * alignment changes. Additionally - round the size to the multiple + * of PAGE_SIZE + */ + length += ((uintptr_t)addr & (qemu_real_host_page_size - 1)); + length = (length + ~align_mask) & align_mask; + + addr = (void *)((uintptr_t)addr & align_mask); + + return msync(addr, length, MS_SYNC); +#else /* CONFIG_POSIX */ + /** + * Perform the sync based on the file descriptor + * The sync range will most probably be wider than the one + * requested - but it will still get the job done + */ + return qemu_fdatasync(fd); +#endif /* CONFIG_POSIX */ +} + #ifndef _WIN32 /* Sets a specific flag */ int fcntl_setfl(int fd, int flag) -- 2.20.1 From: Beata Michalska <beata.michalska@linaro.org> Switch to ram block writeback for pmem migration. Signed-off-by: Beata Michalska <beata.michalska@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Message-id: 20191121000843.24844-4-beata.michalska@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- migration/ram.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/migration/ram.c b/migration/ram.c index XXXXXXX..XXXXXXX 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -XXX,XX +XXX,XX @@ #include "qemu/bitops.h" #include "qemu/bitmap.h" #include "qemu/main-loop.h" -#include "qemu/pmem.h" #include "xbzrle.h" #include "ram.h" #include "migration.h" @@ -XXX,XX +XXX,XX @@ static int ram_load_cleanup(void *opaque) RAMBlock *rb; RAMBLOCK_FOREACH_NOT_IGNORED(rb) { - if (ramblock_is_pmem(rb)) { - pmem_persist(rb->host, rb->used_length); - } + qemu_ram_block_writeback(rb); } xbzrle_load_cleanup(); -- 2.20.1 From: Beata Michalska <beata.michalska@linaro.org> ARMv8.2 introduced support for Data Cache Clean instructions to PoP (point-of-persistence) - DC CVAP and PoDP (point-of-deep-persistence) - DV CVADP. Both specify conceptual points in a memory system where all writes that are to reach them are considered persistent. The support provided considers both to be actually the same so there is no distinction between the two. If none is available (there is no backing store for given memory) both will result in Data Cache Clean up to the point of coherency. Otherwise sync for the specified range shall be performed. Signed-off-by: Beata Michalska <beata.michalska@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20191121000843.24844-5-beata.michalska@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 10 ++++++++ linux-user/elfload.c | 2 ++ target/arm/cpu64.c | 1 + target/arm/helper.c | 56 ++++++++++++++++++++++++++++++++++++++++++++ 4 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 @@ static inline bool isar_feature_aa64_frint(const ARMISARegisters *id) return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, FRINTTS) != 0; } +static inline bool isar_feature_aa64_dcpop(const ARMISARegisters *id) +{ + return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, DPB) != 0; +} + +static inline bool isar_feature_aa64_dcpodp(const ARMISARegisters *id) +{ + return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, DPB) >= 2; +} + static inline bool isar_feature_aa64_fp16(const ARMISARegisters *id) { /* We always set the AdvSIMD and FP fields identically wrt FP16. */ diff --git a/linux-user/elfload.c b/linux-user/elfload.c index XXXXXXX..XXXXXXX 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -XXX,XX +XXX,XX @@ static uint32_t get_elf_hwcap(void) GET_FEATURE_ID(aa64_jscvt, ARM_HWCAP_A64_JSCVT); GET_FEATURE_ID(aa64_sb, ARM_HWCAP_A64_SB); GET_FEATURE_ID(aa64_condm_4, ARM_HWCAP_A64_FLAGM); + GET_FEATURE_ID(aa64_dcpop, ARM_HWCAP_A64_DCPOP); return hwcaps; } @@ -XXX,XX +XXX,XX @@ static uint32_t get_elf_hwcap2(void) ARMCPU *cpu = ARM_CPU(thread_cpu); uint32_t hwcaps = 0; + GET_FEATURE_ID(aa64_dcpodp, ARM_HWCAP2_A64_DCPODP); GET_FEATURE_ID(aa64_condm_5, ARM_HWCAP2_A64_FLAGM2); GET_FEATURE_ID(aa64_frint, ARM_HWCAP2_A64_FRINT); diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -XXX,XX +XXX,XX @@ static void aarch64_max_initfn(Object *obj) cpu->isar.id_aa64isar0 = t; t = cpu->isar.id_aa64isar1; + t = FIELD_DP64(t, ID_AA64ISAR1, DPB, 2); t = FIELD_DP64(t, ID_AA64ISAR1, JSCVT, 1); t = FIELD_DP64(t, ID_AA64ISAR1, FCMA, 1); t = FIELD_DP64(t, ID_AA64ISAR1, APA, 1); /* PAuth, architected only */ 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 rndr_reginfo[] = { .access = PL0_R, .readfn = rndr_readfn }, REGINFO_SENTINEL }; + +#ifndef CONFIG_USER_ONLY +static void dccvap_writefn(CPUARMState *env, const ARMCPRegInfo *opaque, + uint64_t value) +{ + ARMCPU *cpu = env_archcpu(env); + /* CTR_EL0 System register -> DminLine, bits [19:16] */ + uint64_t dline_size = 4 << ((cpu->ctr >> 16) & 0xF); + uint64_t vaddr_in = (uint64_t) value; + uint64_t vaddr = vaddr_in & ~(dline_size - 1); + void *haddr; + int mem_idx = cpu_mmu_index(env, false); + + /* This won't be crossing page boundaries */ + haddr = probe_read(env, vaddr, dline_size, mem_idx, GETPC()); + if (haddr) { + + ram_addr_t offset; + MemoryRegion *mr; + + /* RCU lock is already being held */ + mr = memory_region_from_host(haddr, &offset); + + if (mr) { + memory_region_do_writeback(mr, offset, dline_size); + } + } +} + +static const ARMCPRegInfo dcpop_reg[] = { + { .name = "DC_CVAP", .state = ARM_CP_STATE_AA64, + .opc0 = 1, .opc1 = 3, .crn = 7, .crm = 12, .opc2 = 1, + .access = PL0_W, .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END, + .accessfn = aa64_cacheop_access, .writefn = dccvap_writefn }, + REGINFO_SENTINEL +}; + +static const ARMCPRegInfo dcpodp_reg[] = { + { .name = "DC_CVADP", .state = ARM_CP_STATE_AA64, + .opc0 = 1, .opc1 = 3, .crn = 7, .crm = 13, .opc2 = 1, + .access = PL0_W, .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END, + .accessfn = aa64_cacheop_access, .writefn = dccvap_writefn }, + REGINFO_SENTINEL +}; +#endif /*CONFIG_USER_ONLY*/ + #endif static CPAccessResult access_predinv(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) if (cpu_isar_feature(aa64_rndr, cpu)) { define_arm_cp_regs(cpu, rndr_reginfo); } +#ifndef CONFIG_USER_ONLY + /* Data Cache clean instructions up to PoP */ + if (cpu_isar_feature(aa64_dcpop, cpu)) { + define_one_arm_cp_reg(cpu, dcpop_reg); + + if (cpu_isar_feature(aa64_dcpodp, cpu)) { + define_one_arm_cp_reg(cpu, dcpodp_reg); + } + } +#endif /*CONFIG_USER_ONLY*/ #endif /* -- 2.20.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> Make the gic a field in the machine state, and instead of filling an array of qemu_irq and passing it around, directly call qdev_get_gpio_in() on the gic field. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-id: 20191206162303.30338-1-philmd@redhat.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/sbsa-ref.c | 86 +++++++++++++++++++++++------------------------ 1 file changed, 42 insertions(+), 44 deletions(-) diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/sbsa-ref.c +++ b/hw/arm/sbsa-ref.c @@ -XXX,XX +XXX,XX @@ typedef struct { void *fdt; int fdt_size; int psci_conduit; + DeviceState *gic; PFlashCFI01 *flash[2]; } SBSAMachineState; @@ -XXX,XX +XXX,XX @@ static void create_secure_ram(SBSAMachineState *sms, memory_region_add_subregion(secure_sysmem, base, secram); } -static void create_gic(SBSAMachineState *sms, qemu_irq *pic) +static void create_gic(SBSAMachineState *sms) { unsigned int smp_cpus = MACHINE(sms)->smp.cpus; - DeviceState *gicdev; SysBusDevice *gicbusdev; const char *gictype; uint32_t redist0_capacity, redist0_count; @@ -XXX,XX +XXX,XX @@ static void create_gic(SBSAMachineState *sms, qemu_irq *pic) gictype = gicv3_class_name(); - gicdev = qdev_create(NULL, gictype); - qdev_prop_set_uint32(gicdev, "revision", 3); - qdev_prop_set_uint32(gicdev, "num-cpu", smp_cpus); + sms->gic = qdev_create(NULL, gictype); + qdev_prop_set_uint32(sms->gic, "revision", 3); + qdev_prop_set_uint32(sms->gic, "num-cpu", smp_cpus); /* * Note that the num-irq property counts both internal and external * interrupts; there are always 32 of the former (mandated by GIC spec). */ - qdev_prop_set_uint32(gicdev, "num-irq", NUM_IRQS + 32); - qdev_prop_set_bit(gicdev, "has-security-extensions", true); + qdev_prop_set_uint32(sms->gic, "num-irq", NUM_IRQS + 32); + qdev_prop_set_bit(sms->gic, "has-security-extensions", true); redist0_capacity = sbsa_ref_memmap[SBSA_GIC_REDIST].size / GICV3_REDIST_SIZE; redist0_count = MIN(smp_cpus, redist0_capacity); - qdev_prop_set_uint32(gicdev, "len-redist-region-count", 1); - qdev_prop_set_uint32(gicdev, "redist-region-count[0]", redist0_count); + qdev_prop_set_uint32(sms->gic, "len-redist-region-count", 1); + qdev_prop_set_uint32(sms->gic, "redist-region-count[0]", redist0_count); - qdev_init_nofail(gicdev); - gicbusdev = SYS_BUS_DEVICE(gicdev); + qdev_init_nofail(sms->gic); + gicbusdev = SYS_BUS_DEVICE(sms->gic); sysbus_mmio_map(gicbusdev, 0, sbsa_ref_memmap[SBSA_GIC_DIST].base); sysbus_mmio_map(gicbusdev, 1, sbsa_ref_memmap[SBSA_GIC_REDIST].base); @@ -XXX,XX +XXX,XX @@ static void create_gic(SBSAMachineState *sms, qemu_irq *pic) for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) { qdev_connect_gpio_out(cpudev, irq, - qdev_get_gpio_in(gicdev, + qdev_get_gpio_in(sms->gic, ppibase + timer_irq[irq])); } qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt", 0, - qdev_get_gpio_in(gicdev, ppibase + qdev_get_gpio_in(sms->gic, ppibase + ARCH_GIC_MAINT_IRQ)); qdev_connect_gpio_out_named(cpudev, "pmu-interrupt", 0, - qdev_get_gpio_in(gicdev, ppibase + qdev_get_gpio_in(sms->gic, ppibase + VIRTUAL_PMU_IRQ)); sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ)); @@ -XXX,XX +XXX,XX @@ static void create_gic(SBSAMachineState *sms, qemu_irq *pic) sysbus_connect_irq(gicbusdev, i + 3 * smp_cpus, qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ)); } - - for (i = 0; i < NUM_IRQS; i++) { - pic[i] = qdev_get_gpio_in(gicdev, i); - } } -static void create_uart(const SBSAMachineState *sms, qemu_irq *pic, int uart, +static void create_uart(const SBSAMachineState *sms, int uart, MemoryRegion *mem, Chardev *chr) { hwaddr base = sbsa_ref_memmap[uart].base; @@ -XXX,XX +XXX,XX @@ static void create_uart(const SBSAMachineState *sms, qemu_irq *pic, int uart, qdev_init_nofail(dev); memory_region_add_subregion(mem, base, sysbus_mmio_get_region(s, 0)); - sysbus_connect_irq(s, 0, pic[irq]); + sysbus_connect_irq(s, 0, qdev_get_gpio_in(sms->gic, irq)); } -static void create_rtc(const SBSAMachineState *sms, qemu_irq *pic) +static void create_rtc(const SBSAMachineState *sms) { hwaddr base = sbsa_ref_memmap[SBSA_RTC].base; int irq = sbsa_ref_irqmap[SBSA_RTC]; - sysbus_create_simple("pl031", base, pic[irq]); + sysbus_create_simple("pl031", base, qdev_get_gpio_in(sms->gic, irq)); } static DeviceState *gpio_key_dev; @@ -XXX,XX +XXX,XX @@ static Notifier sbsa_ref_powerdown_notifier = { .notify = sbsa_ref_powerdown_req }; -static void create_gpio(const SBSAMachineState *sms, qemu_irq *pic) +static void create_gpio(const SBSAMachineState *sms) { DeviceState *pl061_dev; hwaddr base = sbsa_ref_memmap[SBSA_GPIO].base; int irq = sbsa_ref_irqmap[SBSA_GPIO]; - pl061_dev = sysbus_create_simple("pl061", base, pic[irq]); + pl061_dev = sysbus_create_simple("pl061", base, + qdev_get_gpio_in(sms->gic, irq)); gpio_key_dev = sysbus_create_simple("gpio-key", -1, qdev_get_gpio_in(pl061_dev, 3)); @@ -XXX,XX +XXX,XX @@ static void create_gpio(const SBSAMachineState *sms, qemu_irq *pic) qemu_register_powerdown_notifier(&sbsa_ref_powerdown_notifier); } -static void create_ahci(const SBSAMachineState *sms, qemu_irq *pic) +static void create_ahci(const SBSAMachineState *sms) { hwaddr base = sbsa_ref_memmap[SBSA_AHCI].base; int irq = sbsa_ref_irqmap[SBSA_AHCI]; @@ -XXX,XX +XXX,XX @@ static void create_ahci(const SBSAMachineState *sms, qemu_irq *pic) qdev_prop_set_uint32(dev, "num-ports", NUM_SATA_PORTS); qdev_init_nofail(dev); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); - sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]); + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, qdev_get_gpio_in(sms->gic, irq)); sysahci = SYSBUS_AHCI(dev); ahci = &sysahci->ahci; @@ -XXX,XX +XXX,XX @@ static void create_ahci(const SBSAMachineState *sms, qemu_irq *pic) } } -static void create_ehci(const SBSAMachineState *sms, qemu_irq *pic) +static void create_ehci(const SBSAMachineState *sms) { hwaddr base = sbsa_ref_memmap[SBSA_EHCI].base; int irq = sbsa_ref_irqmap[SBSA_EHCI]; - sysbus_create_simple("platform-ehci-usb", base, pic[irq]); + sysbus_create_simple("platform-ehci-usb", base, + qdev_get_gpio_in(sms->gic, irq)); } -static void create_smmu(const SBSAMachineState *sms, qemu_irq *pic, - PCIBus *bus) +static void create_smmu(const SBSAMachineState *sms, PCIBus *bus) { hwaddr base = sbsa_ref_memmap[SBSA_SMMU].base; int irq = sbsa_ref_irqmap[SBSA_SMMU]; @@ -XXX,XX +XXX,XX @@ static void create_smmu(const SBSAMachineState *sms, qemu_irq *pic, qdev_init_nofail(dev); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); for (i = 0; i < NUM_SMMU_IRQS; i++) { - sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]); + sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, + qdev_get_gpio_in(sms->gic, irq + 1)); } } -static void create_pcie(SBSAMachineState *sms, qemu_irq *pic) +static void create_pcie(SBSAMachineState *sms) { hwaddr base_ecam = sbsa_ref_memmap[SBSA_PCIE_ECAM].base; hwaddr size_ecam = sbsa_ref_memmap[SBSA_PCIE_ECAM].size; @@ -XXX,XX +XXX,XX @@ static void create_pcie(SBSAMachineState *sms, qemu_irq *pic) sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, base_pio); for (i = 0; i < GPEX_NUM_IRQS; i++) { - sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]); + sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, + qdev_get_gpio_in(sms->gic, irq + 1)); gpex_set_irq_num(GPEX_HOST(dev), i, irq + i); } @@ -XXX,XX +XXX,XX @@ static void create_pcie(SBSAMachineState *sms, qemu_irq *pic) pci_create_simple(pci->bus, -1, "VGA"); - create_smmu(sms, pic, pci->bus); + create_smmu(sms, pci->bus); } static void *sbsa_ref_dtb(const struct arm_boot_info *binfo, int *fdt_size) @@ -XXX,XX +XXX,XX @@ static void sbsa_ref_init(MachineState *machine) bool firmware_loaded; const CPUArchIdList *possible_cpus; int n, sbsa_max_cpus; - qemu_irq pic[NUM_IRQS]; if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) { error_report("sbsa-ref: CPU type other than the built-in " @@ -XXX,XX +XXX,XX @@ static void sbsa_ref_init(MachineState *machine) create_secure_ram(sms, secure_sysmem); - create_gic(sms, pic); + create_gic(sms); - create_uart(sms, pic, SBSA_UART, sysmem, serial_hd(0)); - create_uart(sms, pic, SBSA_SECURE_UART, secure_sysmem, serial_hd(1)); + create_uart(sms, SBSA_UART, sysmem, serial_hd(0)); + create_uart(sms, SBSA_SECURE_UART, secure_sysmem, serial_hd(1)); /* Second secure UART for RAS and MM from EL0 */ - create_uart(sms, pic, SBSA_SECURE_UART_MM, secure_sysmem, serial_hd(2)); + create_uart(sms, SBSA_SECURE_UART_MM, secure_sysmem, serial_hd(2)); - create_rtc(sms, pic); + create_rtc(sms); - create_gpio(sms, pic); + create_gpio(sms); - create_ahci(sms, pic); + create_ahci(sms); - create_ehci(sms, pic); + create_ehci(sms); - create_pcie(sms, pic); + create_pcie(sms); sms->bootinfo.ram_size = machine->ram_size; sms->bootinfo.nb_cpus = smp_cpus; -- 2.20.1 From: Heyi Guo <guoheyi@huawei.com> The last argument of AML bit and/or statement is the target variable, so we don't need to use a NULL target and then an additional store operation; using just aml_and() or aml_or() statement is enough. Also update tests/data/acpi/virt/DSDT* to pass "make check". Cc: Shannon Zhao <shannon.zhaosl@gmail.com> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Igor Mammedov <imammedo@redhat.com> Suggested-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Heyi Guo <guoheyi@huawei.com> Message-id: 20191209063719.23086-2-guoheyi@huawei.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt-acpi-build.c | 16 ++++++++-------- tests/data/acpi/virt/DSDT | Bin 18470 -> 18462 bytes tests/data/acpi/virt/DSDT.memhp | Bin 19807 -> 19799 bytes tests/data/acpi/virt/DSDT.numamem | Bin 18470 -> 18462 bytes 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -XXX,XX +XXX,XX @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3")); aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP"))); aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL"))); - aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D), NULL), - aml_name("CTRL"))); + aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D), + aml_name("CTRL"))); ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1)))); - aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08), NULL), - aml_name("CDW1"))); + aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x08), + aml_name("CDW1"))); aml_append(ifctx, ifctx1); ifctx1 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), aml_name("CTRL")))); - aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10), NULL), - aml_name("CDW1"))); + aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x10), + aml_name("CDW1"))); aml_append(ifctx, ifctx1); aml_append(ifctx, aml_store(aml_name("CTRL"), aml_name("CDW3"))); @@ -XXX,XX +XXX,XX @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_append(method, ifctx); elsectx = aml_else(); - aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4), NULL), - aml_name("CDW1"))); + aml_append(elsectx, aml_or(aml_name("CDW1"), aml_int(4), + aml_name("CDW1"))); aml_append(elsectx, aml_return(aml_arg(3))); aml_append(method, elsectx); aml_append(dev, method); diff --git a/tests/data/acpi/virt/DSDT b/tests/data/acpi/virt/DSDT index XXXXXXX..XXXXXXX 100644 GIT binary patch delta 133 zcmZ2BfpOjhMlP3Nmk>D*1_q|2iCof5o%I{lJ2{y;?{412x!p#<jWgaq*qNm(o59&7 z+;D-%<VrV7_iE>mARjJS5V=5L(&S9WT970c2Uv;Nq{%?q7$gZ1761tsfcPNsCD{x4 MAmS{W8QoPG0j8@bzW@LL delta 141 zcmbO?fpOUcMlP3Nmk>1%1_q`n6S<_B8XGpMcXBc{-rKy1bGwazA7{LOuro_nHiNTE zxZwi7$(3%F{sq;}AwfP|vJ4<<fzYJMnT!RsAbBnhh%$*ulYv}gkTg_604z}e5&_99 R$zCV`m0@An{L@X95dZ+BD!u>! diff --git a/tests/data/acpi/virt/DSDT.memhp b/tests/data/acpi/virt/DSDT.memhp index XXXXXXX..XXXXXXX 100644 GIT binary patch delta 132 zcmcaVi}Cs_MlP3NmymE@1_ma@iCof*O&is^IGH-{Zr;SX-A2HTGu}VgnWZb6!PzC; zaDm6<N;gaQYUhw3A1+xCxj<mj<V?m|kR%reSc%xA$w1l|Bnc4~00|d>_#p8m*$ep~ L;w+mP-Q(B*s{AMU delta 140 zcmcaUi}C&}MlP3Nmymd01_maViCof*T^rT9IGGynZQjJW-A2HVGu}VgnWZb6!PzC; zaDm_CN;gaYf@<fGARjJS1`xGCXwu|N#)4XqJQoK<nZ%^YK&~-J8Y&?GmM8#;fMk|r QFBE{vurO@?=@!QZ00dYn_y7O^ diff --git a/tests/data/acpi/virt/DSDT.numamem b/tests/data/acpi/virt/DSDT.numamem index XXXXXXX..XXXXXXX 100644 GIT binary patch delta 133 zcmZ2BfpOjhMlP3Nmk>D*1_q|2iCof5o%I{lJ2{y;?{412x!p#<jWgaq*qNm(o59&7 z+;D-%<VrV7_iE>mARjJS5V=5L(&S9WT970c2Uv;Nq{%?q7$gZ1761tsfcPNsCD{x4 MAmS{W8QoPG0j8@bzW@LL delta 141 zcmbO?fpOUcMlP3Nmk>1%1_q`n6S<_B8XGpMcXBc{-rKy1bGwazA7{LOuro_nHiNTE zxZwi7$(3%F{sq;}AwfP|vJ4<<fzYJMnT!RsAbBnhh%$*ulYv}gkTg_604z}e5&_99 R$zCV`m0@An{L@X95dZ+BD!u>! -- 2.20.1 From: Heyi Guo <guoheyi@huawei.com> After the introduction of generic PCIe root port and PCIe-PCI bridge, we will also have SHPC controller on ARM, so just enable SHPC native hot plug. Also update tests/data/acpi/virt/DSDT* to pass "make check". Cc: Shannon Zhao <shannon.zhaosl@gmail.com> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Heyi Guo <guoheyi@huawei.com> Message-id: 20191209063719.23086-3-guoheyi@huawei.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt-acpi-build.c | 7 ++++++- tests/data/acpi/virt/DSDT | Bin 18462 -> 18462 bytes tests/data/acpi/virt/DSDT.memhp | Bin 19799 -> 19799 bytes tests/data/acpi/virt/DSDT.numamem | Bin 18462 -> 18462 bytes 4 files changed, 6 insertions(+), 1 deletion(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -XXX,XX +XXX,XX @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3")); aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP"))); aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL"))); - aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D), + + /* + * Allow OS control for all 5 features: + * PCIeHotplug SHPCHotplug PME AER PCIeCapability. + */ + aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1F), aml_name("CTRL"))); ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1)))); diff --git a/tests/data/acpi/virt/DSDT b/tests/data/acpi/virt/DSDT index XXXXXXX..XXXXXXX 100644 GIT binary patch delta 28 kcmbO?fpOjhMlP3Nmk>D*1_q{tja=*8809zbbW3Ff0C~9xM*si- delta 28 kcmbO?fpOjhMlP3Nmk>D*1_q|2ja=*87-cu_bW3Ff0C~j-M*si- diff --git a/tests/data/acpi/virt/DSDT.memhp b/tests/data/acpi/virt/DSDT.memhp index XXXXXXX..XXXXXXX 100644 GIT binary patch delta 28 kcmcaUi}Cs_MlP3NmymE@1_mbija=*8809zbbeqQp0Eq|*2mk;8 delta 28 kcmcaUi}Cs_MlP3NmymE@1_ma@ja=*87-cu_beqQp0ErX{2mk;8 diff --git a/tests/data/acpi/virt/DSDT.numamem b/tests/data/acpi/virt/DSDT.numamem index XXXXXXX..XXXXXXX 100644 GIT binary patch delta 28 kcmbO?fpOjhMlP3Nmk>D*1_q{tja=*8809zbbW3Ff0C~9xM*si- delta 28 kcmbO?fpOjhMlP3Nmk>D*1_q|2ja=*87-cu_bW3Ff0C~j-M*si- -- 2.20.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> Make the gic a field in the machine state, and instead of filling an array of qemu_irq and passing it around, directly call qdev_get_gpio_in() on the gic field. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Luc Michel <luc.michel@greensocs.com> Message-id: 20191209090306.20433-1-philmd@redhat.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/virt.h | 1 + hw/arm/virt.c | 109 +++++++++++++++++++++--------------------- 2 files changed, 55 insertions(+), 55 deletions(-) diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -XXX,XX +XXX,XX @@ typedef struct { uint32_t iommu_phandle; int psci_conduit; hwaddr highest_gpa; + DeviceState *gic; DeviceState *acpi_dev; Notifier powerdown_notifier; } VirtMachineState; diff --git a/hw/arm/virt.c b/hw/arm/virt.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -XXX,XX +XXX,XX @@ static void fdt_add_pmu_nodes(const VirtMachineState *vms) } } -static inline DeviceState *create_acpi_ged(VirtMachineState *vms, qemu_irq *pic) +static inline DeviceState *create_acpi_ged(VirtMachineState *vms) { DeviceState *dev; MachineState *ms = MACHINE(vms); @@ -XXX,XX +XXX,XX @@ static inline DeviceState *create_acpi_ged(VirtMachineState *vms, qemu_irq *pic) sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, vms->memmap[VIRT_ACPI_GED].base); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, vms->memmap[VIRT_PCDIMM_ACPI].base); - sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]); + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, qdev_get_gpio_in(vms->gic, irq)); qdev_init_nofail(dev); return dev; } -static void create_its(VirtMachineState *vms, DeviceState *gicdev) +static void create_its(VirtMachineState *vms) { const char *itsclass = its_class_name(); DeviceState *dev; @@ -XXX,XX +XXX,XX @@ static void create_its(VirtMachineState *vms, DeviceState *gicdev) dev = qdev_create(NULL, itsclass); - object_property_set_link(OBJECT(dev), OBJECT(gicdev), "parent-gicv3", + object_property_set_link(OBJECT(dev), OBJECT(vms->gic), "parent-gicv3", &error_abort); qdev_init_nofail(dev); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, vms->memmap[VIRT_GIC_ITS].base); @@ -XXX,XX +XXX,XX @@ static void create_its(VirtMachineState *vms, DeviceState *gicdev) fdt_add_its_gic_node(vms); } -static void create_v2m(VirtMachineState *vms, qemu_irq *pic) +static void create_v2m(VirtMachineState *vms) { int i; int irq = vms->irqmap[VIRT_GIC_V2M]; @@ -XXX,XX +XXX,XX @@ static void create_v2m(VirtMachineState *vms, qemu_irq *pic) qdev_init_nofail(dev); for (i = 0; i < NUM_GICV2M_SPIS; i++) { - sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]); + sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, + qdev_get_gpio_in(vms->gic, irq + i)); } fdt_add_v2m_gic_node(vms); } -static void create_gic(VirtMachineState *vms, qemu_irq *pic) +static void create_gic(VirtMachineState *vms) { MachineState *ms = MACHINE(vms); /* We create a standalone GIC */ - DeviceState *gicdev; SysBusDevice *gicbusdev; const char *gictype; int type = vms->gic_version, i; @@ -XXX,XX +XXX,XX @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic) gictype = (type == 3) ? gicv3_class_name() : gic_class_name(); - gicdev = qdev_create(NULL, gictype); - qdev_prop_set_uint32(gicdev, "revision", type); - qdev_prop_set_uint32(gicdev, "num-cpu", smp_cpus); + vms->gic = qdev_create(NULL, gictype); + qdev_prop_set_uint32(vms->gic, "revision", type); + qdev_prop_set_uint32(vms->gic, "num-cpu", smp_cpus); /* Note that the num-irq property counts both internal and external * interrupts; there are always 32 of the former (mandated by GIC spec). */ - qdev_prop_set_uint32(gicdev, "num-irq", NUM_IRQS + 32); + qdev_prop_set_uint32(vms->gic, "num-irq", NUM_IRQS + 32); if (!kvm_irqchip_in_kernel()) { - qdev_prop_set_bit(gicdev, "has-security-extensions", vms->secure); + qdev_prop_set_bit(vms->gic, "has-security-extensions", vms->secure); } if (type == 3) { @@ -XXX,XX +XXX,XX @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic) nb_redist_regions = virt_gicv3_redist_region_count(vms); - qdev_prop_set_uint32(gicdev, "len-redist-region-count", + qdev_prop_set_uint32(vms->gic, "len-redist-region-count", nb_redist_regions); - qdev_prop_set_uint32(gicdev, "redist-region-count[0]", redist0_count); + qdev_prop_set_uint32(vms->gic, "redist-region-count[0]", redist0_count); if (nb_redist_regions == 2) { uint32_t redist1_capacity = vms->memmap[VIRT_HIGH_GIC_REDIST2].size / GICV3_REDIST_SIZE; - qdev_prop_set_uint32(gicdev, "redist-region-count[1]", + qdev_prop_set_uint32(vms->gic, "redist-region-count[1]", MIN(smp_cpus - redist0_count, redist1_capacity)); } } else { if (!kvm_irqchip_in_kernel()) { - qdev_prop_set_bit(gicdev, "has-virtualization-extensions", + qdev_prop_set_bit(vms->gic, "has-virtualization-extensions", vms->virt); } } - qdev_init_nofail(gicdev); - gicbusdev = SYS_BUS_DEVICE(gicdev); + qdev_init_nofail(vms->gic); + gicbusdev = SYS_BUS_DEVICE(vms->gic); sysbus_mmio_map(gicbusdev, 0, vms->memmap[VIRT_GIC_DIST].base); if (type == 3) { sysbus_mmio_map(gicbusdev, 1, vms->memmap[VIRT_GIC_REDIST].base); @@ -XXX,XX +XXX,XX @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic) for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) { qdev_connect_gpio_out(cpudev, irq, - qdev_get_gpio_in(gicdev, + qdev_get_gpio_in(vms->gic, ppibase + timer_irq[irq])); } if (type == 3) { - qemu_irq irq = qdev_get_gpio_in(gicdev, + qemu_irq irq = qdev_get_gpio_in(vms->gic, ppibase + ARCH_GIC_MAINT_IRQ); qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt", 0, irq); } else if (vms->virt) { - qemu_irq irq = qdev_get_gpio_in(gicdev, + qemu_irq irq = qdev_get_gpio_in(vms->gic, ppibase + ARCH_GIC_MAINT_IRQ); sysbus_connect_irq(gicbusdev, i + 4 * smp_cpus, irq); } qdev_connect_gpio_out_named(cpudev, "pmu-interrupt", 0, - qdev_get_gpio_in(gicdev, ppibase + qdev_get_gpio_in(vms->gic, ppibase + VIRTUAL_PMU_IRQ)); sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ)); @@ -XXX,XX +XXX,XX @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic) qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ)); } - for (i = 0; i < NUM_IRQS; i++) { - pic[i] = qdev_get_gpio_in(gicdev, i); - } - fdt_add_gic_node(vms); if (type == 3 && vms->its) { - create_its(vms, gicdev); + create_its(vms); } else if (type == 2) { - create_v2m(vms, pic); + create_v2m(vms); } } -static void create_uart(const VirtMachineState *vms, qemu_irq *pic, int uart, +static void create_uart(const VirtMachineState *vms, int uart, MemoryRegion *mem, Chardev *chr) { char *nodename; @@ -XXX,XX +XXX,XX @@ static void create_uart(const VirtMachineState *vms, qemu_irq *pic, int uart, qdev_init_nofail(dev); memory_region_add_subregion(mem, base, sysbus_mmio_get_region(s, 0)); - sysbus_connect_irq(s, 0, pic[irq]); + sysbus_connect_irq(s, 0, qdev_get_gpio_in(vms->gic, irq)); nodename = g_strdup_printf("/pl011@%" PRIx64, base); qemu_fdt_add_subnode(vms->fdt, nodename); @@ -XXX,XX +XXX,XX @@ static void create_uart(const VirtMachineState *vms, qemu_irq *pic, int uart, g_free(nodename); } -static void create_rtc(const VirtMachineState *vms, qemu_irq *pic) +static void create_rtc(const VirtMachineState *vms) { char *nodename; hwaddr base = vms->memmap[VIRT_RTC].base; @@ -XXX,XX +XXX,XX @@ static void create_rtc(const VirtMachineState *vms, qemu_irq *pic) int irq = vms->irqmap[VIRT_RTC]; const char compat[] = "arm,pl031\0arm,primecell"; - sysbus_create_simple("pl031", base, pic[irq]); + sysbus_create_simple("pl031", base, qdev_get_gpio_in(vms->gic, irq)); nodename = g_strdup_printf("/pl031@%" PRIx64, base); qemu_fdt_add_subnode(vms->fdt, nodename); @@ -XXX,XX +XXX,XX @@ static void virt_powerdown_req(Notifier *n, void *opaque) } } -static void create_gpio(const VirtMachineState *vms, qemu_irq *pic) +static void create_gpio(const VirtMachineState *vms) { char *nodename; DeviceState *pl061_dev; @@ -XXX,XX +XXX,XX @@ static void create_gpio(const VirtMachineState *vms, qemu_irq *pic) int irq = vms->irqmap[VIRT_GPIO]; const char compat[] = "arm,pl061\0arm,primecell"; - pl061_dev = sysbus_create_simple("pl061", base, pic[irq]); + pl061_dev = sysbus_create_simple("pl061", base, + qdev_get_gpio_in(vms->gic, irq)); uint32_t phandle = qemu_fdt_alloc_phandle(vms->fdt); nodename = g_strdup_printf("/pl061@%" PRIx64, base); @@ -XXX,XX +XXX,XX @@ static void create_gpio(const VirtMachineState *vms, qemu_irq *pic) g_free(nodename); } -static void create_virtio_devices(const VirtMachineState *vms, qemu_irq *pic) +static void create_virtio_devices(const VirtMachineState *vms) { int i; hwaddr size = vms->memmap[VIRT_MMIO].size; @@ -XXX,XX +XXX,XX @@ static void create_virtio_devices(const VirtMachineState *vms, qemu_irq *pic) int irq = vms->irqmap[VIRT_MMIO] + i; hwaddr base = vms->memmap[VIRT_MMIO].base + i * size; - sysbus_create_simple("virtio-mmio", base, pic[irq]); + sysbus_create_simple("virtio-mmio", base, + qdev_get_gpio_in(vms->gic, irq)); } /* We add dtb nodes in reverse order so that they appear in the finished @@ -XXX,XX +XXX,XX @@ static void create_pcie_irq_map(const VirtMachineState *vms, 0x7 /* PCI irq */); } -static void create_smmu(const VirtMachineState *vms, qemu_irq *pic, +static void create_smmu(const VirtMachineState *vms, PCIBus *bus) { char *node; @@ -XXX,XX +XXX,XX @@ static void create_smmu(const VirtMachineState *vms, qemu_irq *pic, qdev_init_nofail(dev); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); for (i = 0; i < NUM_SMMU_IRQS; i++) { - sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]); + sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, + qdev_get_gpio_in(vms->gic, irq + i)); } node = g_strdup_printf("/smmuv3@%" PRIx64, base); @@ -XXX,XX +XXX,XX @@ static void create_smmu(const VirtMachineState *vms, qemu_irq *pic, g_free(node); } -static void create_pcie(VirtMachineState *vms, qemu_irq *pic) +static void create_pcie(VirtMachineState *vms) { hwaddr base_mmio = vms->memmap[VIRT_PCIE_MMIO].base; hwaddr size_mmio = vms->memmap[VIRT_PCIE_MMIO].size; @@ -XXX,XX +XXX,XX @@ static void create_pcie(VirtMachineState *vms, qemu_irq *pic) sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, base_pio); for (i = 0; i < GPEX_NUM_IRQS; i++) { - sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]); + sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, + qdev_get_gpio_in(vms->gic, irq + i)); gpex_set_irq_num(GPEX_HOST(dev), i, irq + i); } @@ -XXX,XX +XXX,XX @@ static void create_pcie(VirtMachineState *vms, qemu_irq *pic) if (vms->iommu) { vms->iommu_phandle = qemu_fdt_alloc_phandle(vms->fdt); - create_smmu(vms, pic, pci->bus); + create_smmu(vms, pci->bus); qemu_fdt_setprop_cells(vms->fdt, nodename, "iommu-map", 0x0, vms->iommu_phandle, 0x0, 0x10000); @@ -XXX,XX +XXX,XX @@ static void create_pcie(VirtMachineState *vms, qemu_irq *pic) g_free(nodename); } -static void create_platform_bus(VirtMachineState *vms, qemu_irq *pic) +static void create_platform_bus(VirtMachineState *vms) { DeviceState *dev; SysBusDevice *s; @@ -XXX,XX +XXX,XX @@ static void create_platform_bus(VirtMachineState *vms, qemu_irq *pic) s = SYS_BUS_DEVICE(dev); for (i = 0; i < PLATFORM_BUS_NUM_IRQS; i++) { - int irqn = vms->irqmap[VIRT_PLATFORM_BUS] + i; - sysbus_connect_irq(s, i, pic[irqn]); + int irq = vms->irqmap[VIRT_PLATFORM_BUS] + i; + sysbus_connect_irq(s, i, qdev_get_gpio_in(vms->gic, irq)); } memory_region_add_subregion(sysmem, @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(machine); MachineClass *mc = MACHINE_GET_CLASS(machine); const CPUArchIdList *possible_cpus; - qemu_irq pic[NUM_IRQS]; MemoryRegion *sysmem = get_system_memory(); MemoryRegion *secure_sysmem = NULL; int n, virt_max_cpus; @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) virt_flash_fdt(vms, sysmem, secure_sysmem ?: sysmem); - create_gic(vms, pic); + create_gic(vms); fdt_add_pmu_nodes(vms); - create_uart(vms, pic, VIRT_UART, sysmem, serial_hd(0)); + create_uart(vms, VIRT_UART, sysmem, serial_hd(0)); if (vms->secure) { create_secure_ram(vms, secure_sysmem); - create_uart(vms, pic, VIRT_SECURE_UART, secure_sysmem, serial_hd(1)); + create_uart(vms, VIRT_SECURE_UART, secure_sysmem, serial_hd(1)); } vms->highmem_ecam &= vms->highmem && (!firmware_loaded || aarch64); - create_rtc(vms, pic); + create_rtc(vms); - create_pcie(vms, pic); + create_pcie(vms); if (has_ged && aarch64 && firmware_loaded && acpi_enabled) { - vms->acpi_dev = create_acpi_ged(vms, pic); + vms->acpi_dev = create_acpi_ged(vms); } else { - create_gpio(vms, pic); + create_gpio(vms); } /* connect powerdown request */ @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) * (which will be automatically plugged in to the transports). If * no backend is created the transport will just sit harmlessly idle. */ - create_virtio_devices(vms, pic); + create_virtio_devices(vms); vms->fw_cfg = create_fw_cfg(vms, &address_space_memory); rom_set_fw(vms->fw_cfg); - create_platform_bus(vms, pic); + create_platform_bus(vms); vms->bootinfo.ram_size = machine->ram_size; vms->bootinfo.nb_cpus = smp_cpus; -- 2.20.1 From: Alex Bennée <alex.bennee@linaro.org> A write to the SCR can change the effective EL by droppping the system from secure to non-secure mode. However if we use a cached current_el from before the change we'll rebuild the flags incorrectly. To fix this we introduce the ARM_CP_NEWEL CP flag to indicate the new EL should be used when recomputing the flags. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20191212114734.6962-1-alex.bennee@linaro.org Cc: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20191209143723.6368-1-alex.bennee@linaro.org> Cc: qemu-stable@nongnu.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 8 ++++++-- target/arm/helper.h | 1 + target/arm/helper.c | 14 +++++++++++++- target/arm/translate.c | 6 +++++- 4 files changed, 25 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 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) * RAISES_EXC is for when the read or write hook might raise an exception; * the generated code will synchronize the CPU state before calling the hook * so that it is safe for the hook to call raise_exception(). + * NEWEL is for writes to registers that might change the exception + * level - typically on older ARM chips. For those cases we need to + * re-read the new el when recomputing the translation flags. */ #define ARM_CP_SPECIAL 0x0001 #define ARM_CP_CONST 0x0002 @@ -XXX,XX +XXX,XX @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) #define ARM_CP_SVE 0x2000 #define ARM_CP_NO_GDB 0x4000 #define ARM_CP_RAISES_EXC 0x8000 +#define ARM_CP_NEWEL 0x10000 /* Used only as a terminator for ARMCPRegInfo lists */ -#define ARM_CP_SENTINEL 0xffff +#define ARM_CP_SENTINEL 0xfffff /* Mask of only the flag bits in a type field */ -#define ARM_CP_FLAG_MASK 0xf0ff +#define ARM_CP_FLAG_MASK 0x1f0ff /* 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.h b/target/arm/helper.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -XXX,XX +XXX,XX @@ DEF_HELPER_2(get_user_reg, i32, env, i32) DEF_HELPER_3(set_user_reg, void, env, i32, i32) DEF_HELPER_FLAGS_2(rebuild_hflags_m32, TCG_CALL_NO_RWG, void, env, int) +DEF_HELPER_FLAGS_1(rebuild_hflags_a32_newel, TCG_CALL_NO_RWG, void, env) DEF_HELPER_FLAGS_2(rebuild_hflags_a32, TCG_CALL_NO_RWG, void, env, int) DEF_HELPER_FLAGS_2(rebuild_hflags_a64, TCG_CALL_NO_RWG, void, env, int) 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 el3_cp_reginfo[] = { .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 0, .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.scr_el3), .resetvalue = 0, .writefn = scr_write }, - { .name = "SCR", .type = ARM_CP_ALIAS, + { .name = "SCR", .type = ARM_CP_ALIAS | ARM_CP_NEWEL, .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 0, .access = PL1_RW, .accessfn = access_trap_aa32s_el1, .fieldoffset = offsetoflow32(CPUARMState, cp15.scr_el3), @@ -XXX,XX +XXX,XX @@ void HELPER(rebuild_hflags_m32)(CPUARMState *env, int el) env->hflags = rebuild_hflags_m32(env, fp_el, mmu_idx); } +/* + * If we have triggered a EL state change we can't rely on the + * translator having passed it too us, we need to recompute. + */ +void HELPER(rebuild_hflags_a32_newel)(CPUARMState *env) +{ + int el = arm_current_el(env); + int fp_el = fp_exception_el(env, el); + ARMMMUIdx mmu_idx = arm_mmu_idx_el(env, el); + env->hflags = rebuild_hflags_a32(env, fp_el, mmu_idx); +} + void HELPER(rebuild_hflags_a32)(CPUARMState *env, int el) { int fp_el = fp_exception_el(env, el); diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static int disas_coproc_insn(DisasContext *s, uint32_t insn) if (arm_dc_feature(s, ARM_FEATURE_M)) { gen_helper_rebuild_hflags_m32(cpu_env, tcg_el); } else { - gen_helper_rebuild_hflags_a32(cpu_env, tcg_el); + if (ri->type & ARM_CP_NEWEL) { + gen_helper_rebuild_hflags_a32_newel(cpu_env); + } else { + gen_helper_rebuild_hflags_a32(cpu_env, tcg_el); + } } tcg_temp_free_i32(tcg_el); /* -- 2.20.1 Patches for rc1: nothing major, just some minor bugfixes and code cleanups. -- PMM The following changes since commit f7e1914adad8885a5d4c70239ab90d901ed97e9f: Merge remote-tracking branch 'remotes/alistair/tags/pull-riscv-to-apply-20201109' into staging (2020-11-10 09:24:56 +0000) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20201110 for you to fetch changes up to b6c56c8a9a4064ea783f352f43c5df6231a110fa: target/arm/translate-neon.c: Handle VTBL UNDEF case before VFP access check (2020-11-10 11:03:48 +0000) ---------------------------------------------------------------- target-arm queue: * hw/arm/Kconfig: ARM_V7M depends on PTIMER * Minor coding style fixes * docs: add some notes on the sbsa-ref machine * hw/arm/virt: Remove dependency on Cortex-A15 MPCore peripherals * target/arm: Fix neon VTBL/VTBX for len > 1 * hw/arm/armsse: Correct expansion MPC interrupt lines * hw/misc/stm32f2xx_syscfg: Remove extraneous IRQ * hw/arm/nseries: Remove invalid/unnecessary n8x0_uart_setup() * hw/arm/musicpal: Don't connect two qemu_irqs directly to the same input * hw/arm/musicpal: Only use qdev_get_gpio_in() when necessary * hw/arm/nseries: Check return value from load_image_targphys() * tests/qtest/npcm7xx_rng-test: count runs properly * target/arm/translate-neon.c: Handle VTBL UNDEF case before VFP access check ---------------------------------------------------------------- Alex Bennée (1): docs: add some notes on the sbsa-ref machine AlexChen (1): ssi: Fix bad printf format specifiers Andrew Jones (1): hw/arm/Kconfig: ARM_V7M depends on PTIMER Havard Skinnemoen (1): tests/qtest/npcm7xx_rng-test: count runs properly Peter Maydell (2): hw/arm/nseries: Check return value from load_image_targphys() target/arm/translate-neon.c: Handle VTBL UNDEF case before VFP access check Philippe Mathieu-Daudé (6): hw/arm/virt: Remove dependency on Cortex-A15 MPCore peripherals hw/arm/armsse: Correct expansion MPC interrupt lines hw/misc/stm32f2xx_syscfg: Remove extraneous IRQ hw/arm/nseries: Remove invalid/unnecessary n8x0_uart_setup() hw/arm/musicpal: Don't connect two qemu_irqs directly to the same input hw/arm/musicpal: Only use qdev_get_gpio_in() when necessary Richard Henderson (1): target/arm: Fix neon VTBL/VTBX for len > 1 Xinhao Zhang (3): target/arm: add spaces around operator target/arm: Don't use '#' flag of printf format target/arm: add space before the open parenthesis '(' docs/system/arm/sbsa.rst | 32 ++++++++++++++++++++++ docs/system/target-arm.rst | 1 + include/hw/misc/stm32f2xx_syscfg.h | 2 -- target/arm/helper.h | 2 +- hw/arm/armsse.c | 3 +- hw/arm/musicpal.c | 40 +++++++++++++++++---------- hw/arm/nseries.c | 26 ++++++++---------- hw/arm/stm32f205_soc.c | 1 - hw/misc/stm32f2xx_syscfg.c | 2 -- hw/ssi/imx_spi.c | 2 +- hw/ssi/xilinx_spi.c | 2 +- target/arm/arch_dump.c | 8 +++--- target/arm/arm-semi.c | 8 +++--- target/arm/helper.c | 2 +- target/arm/op_helper.c | 23 +++++++++------- target/arm/translate-a64.c | 4 +-- target/arm/translate.c | 2 +- tests/qtest/npcm7xx_rng-test.c | 2 +- hw/arm/Kconfig | 3 +- target/arm/translate-neon.c.inc | 56 ++++++++++++++------------------------ 20 files changed, 123 insertions(+), 98 deletions(-) create mode 100644 docs/system/arm/sbsa.rst From: Andrew Jones <drjones@redhat.com> commit 32bd322a0134 ("hw/timer/armv7m_systick: Rewrite to use ptimers") changed armv7m_systick to build on ptimers. Make sure we have ptimers in the build when building armv7m_systick. Signed-off-by: Andrew Jones <drjones@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-id: 20201104103343.30392-1-drjones@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -XXX,XX +XXX,XX @@ config ZYNQ config ARM_V7M bool + select PTIMER config ALLWINNER_A10 bool -- 2.20.1 From: AlexChen <alex.chen@huawei.com> We should use printf format specifier "%u" instead of "%d" for argument of type "unsigned int". Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Alex Chen <alex.chen@huawei.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-id: 5FA280F5.8060902@huawei.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/ssi/imx_spi.c | 2 +- hw/ssi/xilinx_spi.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ssi/imx_spi.c +++ b/hw/ssi/imx_spi.c @@ -XXX,XX +XXX,XX @@ static const char *imx_spi_reg_name(uint32_t reg) case ECSPI_MSGDATA: return "ECSPI_MSGDATA"; default: - sprintf(unknown, "%d ?", reg); + sprintf(unknown, "%u ?", reg); return unknown; } } diff --git a/hw/ssi/xilinx_spi.c b/hw/ssi/xilinx_spi.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ssi/xilinx_spi.c +++ b/hw/ssi/xilinx_spi.c @@ -XXX,XX +XXX,XX @@ static void xlx_spi_update_irq(XilinxSPI *s) irq chain unless things really changed. */ if (pending != s->irqline) { s->irqline = pending; - DB_PRINT("irq_change of state %d ISR:%x IER:%X\n", + DB_PRINT("irq_change of state %u ISR:%x IER:%X\n", pending, s->regs[R_IPISR], s->regs[R_IPIER]); qemu_set_irq(s->irq, pending); } -- 2.20.1 From: Xinhao Zhang <zhangxinhao1@huawei.com> Fix code style. Operator needs spaces both sides. Signed-off-by: Xinhao Zhang <zhangxinhao1@huawei.com> Signed-off-by: Kai Deng <dengkai1@huawei.com> Message-id: 20201103114529.638233-1-zhangxinhao1@huawei.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/arch_dump.c | 8 ++++---- target/arm/arm-semi.c | 8 ++++---- target/arm/helper.c | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/target/arm/arch_dump.c b/target/arm/arch_dump.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/arch_dump.c +++ b/target/arm/arch_dump.c @@ -XXX,XX +XXX,XX @@ static int aarch64_write_elf64_prfpreg(WriteCoreDumpFunction f, for (i = 0; i < 32; ++i) { uint64_t *q = aa64_vfp_qreg(env, i); - note.vfp.vregs[2*i + 0] = cpu_to_dump64(s, q[0]); - note.vfp.vregs[2*i + 1] = cpu_to_dump64(s, q[1]); + note.vfp.vregs[2 * i + 0] = cpu_to_dump64(s, q[0]); + note.vfp.vregs[2 * i + 1] = cpu_to_dump64(s, q[1]); } if (s->dump_info.d_endian == ELFDATA2MSB) { @@ -XXX,XX +XXX,XX @@ static int aarch64_write_elf64_prfpreg(WriteCoreDumpFunction f, */ for (i = 0; i < 32; ++i) { uint64_t tmp = note.vfp.vregs[2*i]; - note.vfp.vregs[2*i] = note.vfp.vregs[2*i+1]; - note.vfp.vregs[2*i+1] = tmp; + note.vfp.vregs[2 * i] = note.vfp.vregs[2 * i + 1]; + note.vfp.vregs[2 * i + 1] = tmp; } } diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/arm-semi.c +++ b/target/arm/arm-semi.c @@ -XXX,XX +XXX,XX @@ target_ulong do_arm_semihosting(CPUARMState *env) if (use_gdb_syscalls()) { arm_semi_open_guestfd = guestfd; ret = arm_gdb_syscall(cpu, arm_semi_open_cb, "open,%s,%x,1a4", arg0, - (int)arg2+1, gdb_open_modeflags[arg1]); + (int)arg2 + 1, gdb_open_modeflags[arg1]); } else { ret = set_swi_errno(env, open(s, open_modeflags[arg1], 0644)); if (ret == (uint32_t)-1) { @@ -XXX,XX +XXX,XX @@ target_ulong do_arm_semihosting(CPUARMState *env) GET_ARG(1); if (use_gdb_syscalls()) { ret = arm_gdb_syscall(cpu, arm_semi_cb, "unlink,%s", - arg0, (int)arg1+1); + arg0, (int)arg1 + 1); } else { s = lock_user_string(arg0); if (!s) { @@ -XXX,XX +XXX,XX @@ target_ulong do_arm_semihosting(CPUARMState *env) GET_ARG(3); if (use_gdb_syscalls()) { return arm_gdb_syscall(cpu, arm_semi_cb, "rename,%s,%s", - arg0, (int)arg1+1, arg2, (int)arg3+1); + arg0, (int)arg1 + 1, arg2, (int)arg3 + 1); } else { char *s2; s = lock_user_string(arg0); @@ -XXX,XX +XXX,XX @@ target_ulong do_arm_semihosting(CPUARMState *env) GET_ARG(1); if (use_gdb_syscalls()) { return arm_gdb_syscall(cpu, arm_semi_cb, "system,%s", - arg0, (int)arg1+1); + arg0, (int)arg1 + 1); } else { s = lock_user_string(arg0); if (!s) { 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(usad8)(uint32_t a, uint32_t b) uint32_t sum; sum = do_usad(a, b); sum += do_usad(a >> 8, b >> 8); - sum += do_usad(a >> 16, b >>16); + sum += do_usad(a >> 16, b >> 16); sum += do_usad(a >> 24, b >> 24); return sum; } -- 2.20.1 From: Xinhao Zhang <zhangxinhao1@huawei.com> Fix code style. Don't use '#' flag of printf format ('%#') in format strings, use '0x' prefix instead Signed-off-by: Xinhao Zhang <zhangxinhao1@huawei.com> Signed-off-by: Kai Deng <dengkai1@huawei.com> Message-id: 20201103114529.638233-2-zhangxinhao1@huawei.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.c | 4 ++-- 1 file changed, 2 insertions(+), 2 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 void disas_simd_three_reg_same_fp16(DisasContext *s, uint32_t insn) gen_helper_advsimd_acgt_f16(tcg_res, tcg_op1, tcg_op2, fpst); break; default: - fprintf(stderr, "%s: insn %#04x, fpop %#2x @ %#" PRIx64 "\n", + fprintf(stderr, "%s: insn 0x%04x, fpop 0x%2x @ 0x%" PRIx64 "\n", __func__, insn, fpopcode, s->pc_curr); g_assert_not_reached(); } @@ -XXX,XX +XXX,XX @@ static void disas_simd_two_reg_misc_fp16(DisasContext *s, uint32_t insn) case 0x7f: /* FSQRT (vector) */ break; default: - fprintf(stderr, "%s: insn %#04x fpop %#2x\n", __func__, insn, fpop); + fprintf(stderr, "%s: insn 0x%04x fpop 0x%2x\n", __func__, insn, fpop); g_assert_not_reached(); } -- 2.20.1 From: Xinhao Zhang <zhangxinhao1@huawei.com> Fix code style. Space required before the open parenthesis '('. Signed-off-by: Xinhao Zhang <zhangxinhao1@huawei.com> Signed-off-by: Kai Deng <dengkai1@huawei.com> Message-id: 20201103114529.638233-3-zhangxinhao1@huawei.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) - Hardware watchpoints. Hardware breakpoints have already been handled and skip this code. */ - switch(dc->base.is_jmp) { + switch (dc->base.is_jmp) { case DISAS_NEXT: case DISAS_TOO_MANY: gen_goto_tb(dc, 1, dc->base.pc_next); -- 2.20.1 From: Alex Bennée <alex.bennee@linaro.org> We should at least document what this machine is about. Reviewed-by: Graeme Gregory <graeme@nuviainc.com> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20201104165254.24822-1-alex.bennee@linaro.org Cc: Leif Lindholm <leif@nuviainc.com> Cc: Shashi Mallela <shashi.mallela@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> [PMM: fixed filename mismatch] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- docs/system/arm/sbsa.rst | 32 ++++++++++++++++++++++++++++++++ docs/system/target-arm.rst | 1 + 2 files changed, 33 insertions(+) create mode 100644 docs/system/arm/sbsa.rst diff --git a/docs/system/arm/sbsa.rst b/docs/system/arm/sbsa.rst new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/docs/system/arm/sbsa.rst @@ -XXX,XX +XXX,XX @@ +Arm Server Base System Architecture Reference board (``sbsa-ref``) +================================================================== + +While the `virt` board is a generic board platform that doesn't match +any real hardware the `sbsa-ref` board intends to look like real +hardware. The `Server Base System Architecture +<https://developer.arm.com/documentation/den0029/latest>` defines a +minimum base line of hardware support and importantly how the firmware +reports that to any operating system. It is a static system that +reports a very minimal DT to the firmware for non-discoverable +information about components affected by the qemu command line (i.e. +cpus and memory). As a result it must have a firmware specifically +built to expect a certain hardware layout (as you would in a real +machine). + +It is intended to be a machine for developing firmware and testing +standards compliance with operating systems. + +Supported devices +""""""""""""""""" + +The sbsa-ref board supports: + + - A configurable number of AArch64 CPUs + - GIC version 3 + - System bus AHCI controller + - System bus EHCI controller + - CDROM and hard disc on AHCI bus + - E1000E ethernet card on PCIe bus + - VGA display adaptor on PCIe bus + - A generic SBSA watchdog device + diff --git a/docs/system/target-arm.rst b/docs/system/target-arm.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/system/target-arm.rst +++ b/docs/system/target-arm.rst @@ -XXX,XX +XXX,XX @@ undocumented; you can get a complete list by running arm/mps2 arm/musca arm/realview + arm/sbsa arm/versatile arm/vexpress arm/aspeed -- 2.20.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> When using a Cortex-A15, the Virt machine does not use any MPCore peripherals. Remove the dependency. Fixes: 7951c7b7c05 ("hw/arm: Express dependencies of the virt machine with Kconfig") Reported-by: Miroslav Rezanina <mrezanin@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-id: 20201107114852.271922-1-philmd@redhat.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -XXX,XX +XXX,XX @@ config ARM_VIRT imply VFIO_PLATFORM imply VFIO_XGMAC imply TPM_TIS_SYSBUS - select A15MPCORE select ACPI select ARM_SMMUV3 select GPIO_KEY -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> The helper function did not get updated when we reorganized the vector register file for SVE. Since then, the neon dregs are non-sequential and cannot be simply indexed. At the same time, make the helper function operate on 64-bit quantities so that we do not have to call it twice. Fixes: c39c2b9043e Reported-by: Ard Biesheuvel <ardb@kernel.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> [PMM: use aa32_vfp_dreg() rather than opencoding] Message-id: 20201105171126.88014-1-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.h | 2 +- target/arm/op_helper.c | 23 +++++++++-------- target/arm/translate-neon.c.inc | 44 +++++++++++---------------------- 3 files changed, 29 insertions(+), 40 deletions(-) diff --git a/target/arm/helper.h b/target/arm/helper.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -XXX,XX +XXX,XX @@ DEF_HELPER_FLAGS_2(rsqrte_f32, TCG_CALL_NO_RWG, f32, f32, ptr) DEF_HELPER_FLAGS_2(rsqrte_f64, TCG_CALL_NO_RWG, f64, f64, ptr) DEF_HELPER_FLAGS_1(recpe_u32, TCG_CALL_NO_RWG, i32, i32) DEF_HELPER_FLAGS_1(rsqrte_u32, TCG_CALL_NO_RWG, i32, i32) -DEF_HELPER_FLAGS_4(neon_tbl, TCG_CALL_NO_RWG, i32, i32, i32, ptr, i32) +DEF_HELPER_FLAGS_4(neon_tbl, TCG_CALL_NO_RWG, i64, env, i32, i64, i64) DEF_HELPER_3(shl_cc, i32, env, i32, i32) DEF_HELPER_3(shr_cc, i32, env, i32, i32) diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -XXX,XX +XXX,XX @@ void raise_exception_ra(CPUARMState *env, uint32_t excp, uint32_t syndrome, cpu_loop_exit_restore(cs, ra); } -uint32_t HELPER(neon_tbl)(uint32_t ireg, uint32_t def, void *vn, - uint32_t maxindex) +uint64_t HELPER(neon_tbl)(CPUARMState *env, uint32_t desc, + uint64_t ireg, uint64_t def) { - uint32_t val, shift; - uint64_t *table = vn; + uint64_t tmp, val = 0; + uint32_t maxindex = ((desc & 3) + 1) * 8; + uint32_t base_reg = desc >> 2; + uint32_t shift, index, reg; - val = 0; - for (shift = 0; shift < 32; shift += 8) { - uint32_t index = (ireg >> shift) & 0xff; + for (shift = 0; shift < 64; shift += 8) { + index = (ireg >> shift) & 0xff; if (index < maxindex) { - uint32_t tmp = (table[index >> 3] >> ((index & 7) << 3)) & 0xff; - val |= tmp << shift; + reg = base_reg + (index >> 3); + tmp = *aa32_vfp_dreg(env, reg); + tmp = ((tmp >> ((index & 7) << 3)) & 0xff) << shift; } else { - val |= def & (0xff << shift); + tmp = def & (0xffull << shift); } + val |= tmp; } return val; } diff --git a/target/arm/translate-neon.c.inc b/target/arm/translate-neon.c.inc index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.c.inc +++ b/target/arm/translate-neon.c.inc @@ -XXX,XX +XXX,XX @@ static bool trans_VEXT(DisasContext *s, arg_VEXT *a) static bool trans_VTBL(DisasContext *s, arg_VTBL *a) { - int n; - TCGv_i32 tmp, tmp2, tmp3, tmp4; - TCGv_ptr ptr1; + TCGv_i64 val, def; + TCGv_i32 desc; if (!arm_dc_feature(s, ARM_FEATURE_NEON)) { return false; @@ -XXX,XX +XXX,XX @@ static bool trans_VTBL(DisasContext *s, arg_VTBL *a) return true; } - n = a->len + 1; - if ((a->vn + n) > 32) { + if ((a->vn + a->len + 1) > 32) { /* * This is UNPREDICTABLE; we choose to UNDEF to avoid the * helper function running off the end of the register file. */ return false; } - n <<= 3; - tmp = tcg_temp_new_i32(); - if (a->op) { - read_neon_element32(tmp, a->vd, 0, MO_32); - } else { - tcg_gen_movi_i32(tmp, 0); - } - tmp2 = tcg_temp_new_i32(); - read_neon_element32(tmp2, a->vm, 0, MO_32); - ptr1 = vfp_reg_ptr(true, a->vn); - tmp4 = tcg_const_i32(n); - gen_helper_neon_tbl(tmp2, tmp2, tmp, ptr1, tmp4); + desc = tcg_const_i32((a->vn << 2) | a->len); + def = tcg_temp_new_i64(); if (a->op) { - read_neon_element32(tmp, a->vd, 1, MO_32); + read_neon_element64(def, a->vd, 0, MO_64); } else { - tcg_gen_movi_i32(tmp, 0); + tcg_gen_movi_i64(def, 0); } - tmp3 = tcg_temp_new_i32(); - read_neon_element32(tmp3, a->vm, 1, MO_32); - gen_helper_neon_tbl(tmp3, tmp3, tmp, ptr1, tmp4); - tcg_temp_free_i32(tmp); - tcg_temp_free_i32(tmp4); - tcg_temp_free_ptr(ptr1); + val = tcg_temp_new_i64(); + read_neon_element64(val, a->vm, 0, MO_64); - write_neon_element32(tmp2, a->vd, 0, MO_32); - write_neon_element32(tmp3, a->vd, 1, MO_32); - tcg_temp_free_i32(tmp2); - tcg_temp_free_i32(tmp3); + gen_helper_neon_tbl(val, cpu_env, desc, val, def); + write_neon_element64(val, a->vd, 0, MO_64); + + tcg_temp_free_i64(def); + tcg_temp_free_i64(val); + tcg_temp_free_i32(desc); return true; } -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> We can use one MPC per SRAM bank, but we currently only wire the IRQ from the first expansion MPC to the IRQ splitter. Fix that. Fixes: bb75e16d5e6 ("hw/arm/iotkit: Wire up MPC interrupt lines") Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201107193403.436146-2-f4bug@amsat.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/armsse.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/armsse.c +++ b/hw/arm/armsse.c @@ -XXX,XX +XXX,XX @@ static void armsse_realize(DeviceState *dev, Error **errp) qdev_get_gpio_in(dev_splitter, 0)); qdev_connect_gpio_out(dev_splitter, 0, qdev_get_gpio_in_named(dev_secctl, - "mpc_status", 0)); + "mpc_status", + i - IOTS_NUM_EXP_MPC)); } qdev_connect_gpio_out(dev_splitter, 1, -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> The system configuration controller (SYSCFG) doesn't have any output IRQ (and the INTC input #71 belongs to the UART6). Remove the invalid code. Fixes: db635521a02 ("stm32f205: Add the stm32f205 SoC") Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201107193403.436146-3-f4bug@amsat.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/misc/stm32f2xx_syscfg.h | 2 -- hw/arm/stm32f205_soc.c | 1 - hw/misc/stm32f2xx_syscfg.c | 2 -- 3 files changed, 5 deletions(-) diff --git a/include/hw/misc/stm32f2xx_syscfg.h b/include/hw/misc/stm32f2xx_syscfg.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/misc/stm32f2xx_syscfg.h +++ b/include/hw/misc/stm32f2xx_syscfg.h @@ -XXX,XX +XXX,XX @@ struct STM32F2XXSyscfgState { uint32_t syscfg_exticr3; uint32_t syscfg_exticr4; uint32_t syscfg_cmpcr; - - qemu_irq irq; }; #endif /* HW_STM32F2XX_SYSCFG_H */ diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/stm32f205_soc.c +++ b/hw/arm/stm32f205_soc.c @@ -XXX,XX +XXX,XX @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp) } busdev = SYS_BUS_DEVICE(dev); sysbus_mmio_map(busdev, 0, 0x40013800); - sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, 71)); /* Attach UART (uses USART registers) and USART controllers */ for (i = 0; i < STM_NUM_USARTS; i++) { diff --git a/hw/misc/stm32f2xx_syscfg.c b/hw/misc/stm32f2xx_syscfg.c index XXXXXXX..XXXXXXX 100644 --- a/hw/misc/stm32f2xx_syscfg.c +++ b/hw/misc/stm32f2xx_syscfg.c @@ -XXX,XX +XXX,XX @@ static void stm32f2xx_syscfg_init(Object *obj) { STM32F2XXSyscfgState *s = STM32F2XX_SYSCFG(obj); - sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq); - memory_region_init_io(&s->mmio, obj, &stm32f2xx_syscfg_ops, s, TYPE_STM32F2XX_SYSCFG, 0x400); sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio); -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> omap2420_mpu_init() introduced in commit 827df9f3c5f ("Add basic OMAP2 chip support") takes care of creating the 3 UARTs. Then commit 58a26b477e9 ("Emulate a serial bluetooth HCI with H4+ extensions and attach to n8x0's UART") added n8x0_uart_setup() which create the UART and connects it to an IRQ output, overwritting the existing peripheral and its IRQ connection. This is incorrect. Fortunately we don't need to fix this, because commit 6da68df7f9b ("hw/arm/nseries: Replace the bluetooth chardev with a "null" chardev") removed the use of this peripheral. We can simply remove the code. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201107193403.436146-4-f4bug@amsat.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/nseries.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/nseries.c +++ b/hw/arm/nseries.c @@ -XXX,XX +XXX,XX @@ static void n8x0_cbus_setup(struct n800_s *s) cbus_attach(cbus, s->tahvo = tahvo_init(tahvo_irq, 1)); } -static void n8x0_uart_setup(struct n800_s *s) -{ - Chardev *radio = qemu_chr_new("bt-dummy-uart", "null", NULL); - /* - * Note: We used to connect N8X0_BT_RESET_GPIO and N8X0_BT_WKUP_GPIO - * here, but this code has been removed with the bluetooth backend. - */ - omap_uart_attach(s->mpu->uart[BT_UART], radio); -} - static void n8x0_usb_setup(struct n800_s *s) { SysBusDevice *dev; @@ -XXX,XX +XXX,XX @@ static void n8x0_init(MachineState *machine, n8x0_spi_setup(s); n8x0_dss_setup(s); n8x0_cbus_setup(s); - n8x0_uart_setup(s); if (machine_usb(machine)) { n8x0_usb_setup(s); } -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> The MusicPal board code connects both of the IRQ outputs of the UART to the same INTC qemu_irq. Connecting two qemu_irqs outputs directly to the same input is not valid as it produces subtly wrong behaviour (for instance if both the IRQ lines are high, and then one goes low, the INTC input will see this as a high-to-low transition even though the second IRQ line should still be holding it high). This kind of wiring needs an explicitly created OR gate; add one. Inspired-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201107193403.436146-5-f4bug@amsat.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/musicpal.c | 17 +++++++++++++---- hw/arm/Kconfig | 1 + 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/musicpal.c +++ b/hw/arm/musicpal.c @@ -XXX,XX +XXX,XX @@ #include "ui/console.h" #include "hw/i2c/i2c.h" #include "hw/irq.h" +#include "hw/or-irq.h" #include "hw/audio/wm8750.h" #include "sysemu/block-backend.h" #include "sysemu/runstate.h" @@ -XXX,XX +XXX,XX @@ #define MP_TIMER4_IRQ 7 #define MP_EHCI_IRQ 8 #define MP_ETH_IRQ 9 -#define MP_UART1_IRQ 11 -#define MP_UART2_IRQ 11 +#define MP_UART_SHARED_IRQ 11 #define MP_GPIO_IRQ 12 #define MP_RTC_IRQ 28 #define MP_AUDIO_IRQ 30 @@ -XXX,XX +XXX,XX @@ static void musicpal_init(MachineState *machine) ARMCPU *cpu; qemu_irq pic[32]; DeviceState *dev; + DeviceState *uart_orgate; DeviceState *i2c_dev; DeviceState *lcd_dev; DeviceState *key_dev; @@ -XXX,XX +XXX,XX @@ static void musicpal_init(MachineState *machine) pic[MP_TIMER2_IRQ], pic[MP_TIMER3_IRQ], pic[MP_TIMER4_IRQ], NULL); - serial_mm_init(address_space_mem, MP_UART1_BASE, 2, pic[MP_UART1_IRQ], + /* Logically OR both UART IRQs together */ + uart_orgate = DEVICE(object_new(TYPE_OR_IRQ)); + object_property_set_int(OBJECT(uart_orgate), "num-lines", 2, &error_fatal); + qdev_realize_and_unref(uart_orgate, NULL, &error_fatal); + qdev_connect_gpio_out(DEVICE(uart_orgate), 0, pic[MP_UART_SHARED_IRQ]); + + serial_mm_init(address_space_mem, MP_UART1_BASE, 2, + qdev_get_gpio_in(uart_orgate, 0), 1825000, serial_hd(0), DEVICE_NATIVE_ENDIAN); - serial_mm_init(address_space_mem, MP_UART2_BASE, 2, pic[MP_UART2_IRQ], + serial_mm_init(address_space_mem, MP_UART2_BASE, 2, + qdev_get_gpio_in(uart_orgate, 1), 1825000, serial_hd(1), DEVICE_NATIVE_ENDIAN); /* Register flash */ diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -XXX,XX +XXX,XX @@ config MUSCA config MUSICPAL bool + select OR_IRQ select BITBANG_I2C select MARVELL_88W8618 select PTIMER -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> We don't need to fill the full pic[] array if we only use few of the interrupt lines. Directly call qdev_get_gpio_in() when necessary. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201107193403.436146-6-f4bug@amsat.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/musicpal.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/musicpal.c +++ b/hw/arm/musicpal.c @@ -XXX,XX +XXX,XX @@ static struct arm_boot_info musicpal_binfo = { static void musicpal_init(MachineState *machine) { ARMCPU *cpu; - qemu_irq pic[32]; DeviceState *dev; + DeviceState *pic; DeviceState *uart_orgate; DeviceState *i2c_dev; DeviceState *lcd_dev; @@ -XXX,XX +XXX,XX @@ static void musicpal_init(MachineState *machine) &error_fatal); memory_region_add_subregion(address_space_mem, MP_SRAM_BASE, sram); - dev = sysbus_create_simple(TYPE_MV88W8618_PIC, MP_PIC_BASE, + pic = sysbus_create_simple(TYPE_MV88W8618_PIC, MP_PIC_BASE, qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ)); - for (i = 0; i < 32; i++) { - pic[i] = qdev_get_gpio_in(dev, i); - } - sysbus_create_varargs(TYPE_MV88W8618_PIT, MP_PIT_BASE, pic[MP_TIMER1_IRQ], - pic[MP_TIMER2_IRQ], pic[MP_TIMER3_IRQ], - pic[MP_TIMER4_IRQ], NULL); + sysbus_create_varargs(TYPE_MV88W8618_PIT, MP_PIT_BASE, + qdev_get_gpio_in(pic, MP_TIMER1_IRQ), + qdev_get_gpio_in(pic, MP_TIMER2_IRQ), + qdev_get_gpio_in(pic, MP_TIMER3_IRQ), + qdev_get_gpio_in(pic, MP_TIMER4_IRQ), NULL); /* Logically OR both UART IRQs together */ uart_orgate = DEVICE(object_new(TYPE_OR_IRQ)); object_property_set_int(OBJECT(uart_orgate), "num-lines", 2, &error_fatal); qdev_realize_and_unref(uart_orgate, NULL, &error_fatal); - qdev_connect_gpio_out(DEVICE(uart_orgate), 0, pic[MP_UART_SHARED_IRQ]); + qdev_connect_gpio_out(DEVICE(uart_orgate), 0, + qdev_get_gpio_in(pic, MP_UART_SHARED_IRQ)); serial_mm_init(address_space_mem, MP_UART1_BASE, 2, qdev_get_gpio_in(uart_orgate, 0), @@ -XXX,XX +XXX,XX @@ static void musicpal_init(MachineState *machine) OBJECT(get_system_memory()), &error_fatal); sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, MP_ETH_BASE); - sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[MP_ETH_IRQ]); + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, + qdev_get_gpio_in(pic, MP_ETH_IRQ)); sysbus_create_simple("mv88w8618_wlan", MP_WLAN_BASE, NULL); sysbus_create_simple(TYPE_MUSICPAL_MISC, MP_MISC_BASE, NULL); dev = sysbus_create_simple(TYPE_MUSICPAL_GPIO, MP_GPIO_BASE, - pic[MP_GPIO_IRQ]); + qdev_get_gpio_in(pic, MP_GPIO_IRQ)); i2c_dev = sysbus_create_simple("gpio_i2c", -1, NULL); i2c = (I2CBus *)qdev_get_child_bus(i2c_dev, "i2c"); @@ -XXX,XX +XXX,XX @@ static void musicpal_init(MachineState *machine) NULL); sysbus_realize_and_unref(s, &error_fatal); sysbus_mmio_map(s, 0, MP_AUDIO_BASE); - sysbus_connect_irq(s, 0, pic[MP_AUDIO_IRQ]); + sysbus_connect_irq(s, 0, qdev_get_gpio_in(pic, MP_AUDIO_IRQ)); musicpal_binfo.ram_size = MP_RAM_DEFAULT_SIZE; arm_load_kernel(cpu, machine, &musicpal_binfo); -- 2.20.1 The nseries machines have a codepath that allows them to load a secondary bootloader. This code wasn't checking that the load_image_targphys() succeeded. Check the return value and report the error to the user. While we're in the vicinity, fix the comment style of the comment documenting what this image load is doing. Fixes: Coverity CID 1192904 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201103114918.11807-1-peter.maydell@linaro.org --- hw/arm/nseries.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/nseries.c +++ b/hw/arm/nseries.c @@ -XXX,XX +XXX,XX @@ static void n8x0_init(MachineState *machine, /* No, wait, better start at the ROM. */ s->mpu->cpu->env.regs[15] = OMAP2_Q2_BASE + 0x400000; - /* This is intended for loading the `secondary.bin' program from + /* + * This is intended for loading the `secondary.bin' program from * Nokia images (the NOLO bootloader). The entry point seems * to be at OMAP2_Q2_BASE + 0x400000. * @@ -XXX,XX +XXX,XX @@ static void n8x0_init(MachineState *machine, * for them the entry point needs to be set to OMAP2_SRAM_BASE. * * The code above is for loading the `zImage' file from Nokia - * images. */ - load_image_targphys(option_rom[0].name, OMAP2_Q2_BASE + 0x400000, - machine->ram_size - 0x400000); + * images. + */ + if (load_image_targphys(option_rom[0].name, + OMAP2_Q2_BASE + 0x400000, + machine->ram_size - 0x400000) < 0) { + error_report("Failed to load secondary bootloader %s", + option_rom[0].name); + exit(EXIT_FAILURE); + } n800_setup_nolo_tags(nolo_tags); cpu_physical_memory_write(OMAP2_SRAM_BASE, nolo_tags, 0x10000); -- 2.20.1 From: Havard Skinnemoen <hskinnemoen@google.com> The number of runs is equal to the number of 0-1 and 1-0 transitions, plus one. Currently, it's counting the number of times these transitions do _not_ happen, plus one. Source: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-22r1a.pdf section 2.3.4 point (3). Signed-off-by: Havard Skinnemoen <hskinnemoen@google.com> Message-id: 20201103011457.2959989-2-hskinnemoen@google.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tests/qtest/npcm7xx_rng-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qtest/npcm7xx_rng-test.c b/tests/qtest/npcm7xx_rng-test.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/npcm7xx_rng-test.c +++ b/tests/qtest/npcm7xx_rng-test.c @@ -XXX,XX +XXX,XX @@ static double calc_runs_p(const unsigned long *buf, unsigned int nr_bits) pi = (double)nr_ones / nr_bits; for (k = 0; k < nr_bits - 1; k++) { - vn_obs += !(test_bit(k, buf) ^ test_bit(k + 1, buf)); + vn_obs += (test_bit(k, buf) ^ test_bit(k + 1, buf)); } vn_obs += 1; -- 2.20.1 Checks for UNDEF cases should go before the "is VFP enabled?" access check, except in special cases. Move a stray UNDEF check in the VTBL trans function up above the access check. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201109145324.2859-1-peter.maydell@linaro.org --- target/arm/translate-neon.c.inc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/target/arm/translate-neon.c.inc b/target/arm/translate-neon.c.inc index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.c.inc +++ b/target/arm/translate-neon.c.inc @@ -XXX,XX +XXX,XX @@ static bool trans_VTBL(DisasContext *s, arg_VTBL *a) return false; } - if (!vfp_access_check(s)) { - return true; - } - if ((a->vn + a->len + 1) > 32) { /* * This is UNPREDICTABLE; we choose to UNDEF to avoid the @@ -XXX,XX +XXX,XX @@ static bool trans_VTBL(DisasContext *s, arg_VTBL *a) return false; } + if (!vfp_access_check(s)) { + return true; + } + desc = tcg_const_i32((a->vn << 2) | a->len); def = tcg_temp_new_i64(); if (a->op) { -- 2.20.1