Changes v1->v2: it turns out that the raspi3 support exposes a preexisting bug in our register definitions for VMPIDR/VMIDR: https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg04181.html So I've dropped the final "enable raspi3 board" patch for the moment. When that VMIDR/VMPIDR patch gets reviewed we can put the raspi3 patch in with it. thanks -- PMM The following changes since commit f003d07337a6d4d02c43429b26a4270459afb51a: Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2018-02-15 15:45:33 +0000) are available in the Git repository at: git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20180215-1 for you to fetch changes up to bade58166f4466546600d824a2695a00269d10eb: raspi: Raspberry Pi 3 support (2018-02-15 18:33:46 +0000) ---------------------------------------------------------------- target-arm queue: * aspeed: code cleanup to use unimplemented_device * preparatory work for 'raspi3' RaspberryPi 3 machine model * more SVE prep work * v8M: add minor missing registers * v7M: fix bug where we weren't migrating v7m.other_sp * v7M: fix bugs in handling of interrupt registers for external interrupts beyond 32 ---------------------------------------------------------------- Pekka Enberg (2): bcm2836: Make CPU type configurable raspi: Raspberry Pi 3 support Peter Maydell (11): hw/intc/armv7m_nvic: Don't hardcode M profile ID registers in NVIC hw/intc/armv7m_nvic: Fix ICSR PENDNMISET/CLR handling hw/intc/armv7m_nvic: Implement M profile cache maintenance ops hw/intc/armv7m_nvic: Implement v8M CPPWR register hw/intc/armv7m_nvic: Implement cache ID registers hw/intc/armv7m_nvic: Implement SCR target/arm: Implement writing to CONTROL_NS for v8M hw/intc/armv7m_nvic: Fix byte-to-interrupt number conversions target/arm: Add AIRCR to vmstate struct target/arm: Migrate v7m.other_sp target/arm: Implement v8M MSPLIM and PSPLIM registers Philippe Mathieu-Daudé (2): hw/arm/aspeed: directly map the serial device to the system address space hw/arm/aspeed: simplify using the 'unimplemented device' for aspeed_soc.io Richard Henderson (5): target/arm: Remove ARM_CP_64BIT from ZCR_EL registers target/arm: Enforce FP access to FPCR/FPSR target/arm: Suppress TB end for FPCR/FPSR target/arm: Enforce access to ZCR_EL at translation target/arm: Handle SVE registers when using clear_vec_high include/hw/arm/aspeed_soc.h | 1 - include/hw/arm/bcm2836.h | 1 + target/arm/cpu.h | 71 ++++++++++++----- target/arm/internals.h | 6 ++ hw/arm/aspeed_soc.c | 35 ++------- hw/arm/bcm2836.c | 17 +++-- hw/arm/raspi.c | 34 ++++++--- hw/intc/armv7m_nvic.c | 98 ++++++++++++++++++------ target/arm/cpu.c | 28 +++++++ target/arm/helper.c | 84 +++++++++++++++----- target/arm/machine.c | 84 ++++++++++++++++++++ target/arm/translate-a64.c | 181 ++++++++++++++++++++------------------------ 12 files changed, 429 insertions(+), 211 deletions(-) From: Philippe Mathieu-Daudé <f4bug@amsat.org> (qemu) info mtree address-space: cpu-memory-0 0000000000000000-ffffffffffffffff (prio 0, i/o): system 0000000000000000-0000000007ffffff (prio 0, rom): aspeed.boot_rom 000000001e600000-000000001e7fffff (prio -1, i/o): aspeed_soc.io - 000000001e784000-000000001e78401f (prio 0, i/o): serial 000000001e620000-000000001e6200ff (prio 0, i/o): aspeed.smc.ast2500-fmc 000000001e630000-000000001e6300ff (prio 0, i/o): aspeed.smc.ast2500-spi1 [...] 000000001e720000-000000001e728fff (prio 0, ram): aspeed.sram 000000001e782000-000000001e782fff (prio 0, i/o): aspeed.timer + 000000001e784000-000000001e78401f (prio 0, i/o): serial 000000001e785000-000000001e78501f (prio 0, i/o): aspeed.wdt 000000001e785020-000000001e78503f (prio 0, i/o): aspeed.wdt Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Andrew Jeffery <andrew@aj.id.au> Message-id: 20180209085755.30414-2-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/aspeed_soc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed_soc.c +++ b/hw/arm/aspeed_soc.c @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) /* UART - attach an 8250 to the IO space as our UART5 */ if (serial_hds[0]) { qemu_irq uart5 = qdev_get_gpio_in(DEVICE(&s->vic), uart_irqs[4]); - serial_mm_init(&s->iomem, ASPEED_SOC_UART_5_BASE, 2, + serial_mm_init(get_system_memory(), + ASPEED_SOC_IOMEM_BASE + ASPEED_SOC_UART_5_BASE, 2, uart5, 38400, serial_hds[0], DEVICE_LITTLE_ENDIAN); } -- 2.16.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> (qemu) info mtree address-space: cpu-memory-0 0000000000000000-ffffffffffffffff (prio 0, i/o): system 0000000000000000-0000000007ffffff (prio 0, rom): aspeed.boot_rom - 000000001e600000-000000001e7fffff (prio -1, i/o): aspeed_soc.io + 000000001e600000-000000001e7fffff (prio -1000, i/o): aspeed_soc.io 000000001e620000-000000001e6200ff (prio 0, i/o): aspeed.smc.ast2500-fmc 000000001e630000-000000001e6300ff (prio 0, i/o): aspeed.smc.ast2500-spi1 000000001e631000-000000001e6310ff (prio 0, i/o): aspeed.smc.ast2500-spi2 Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Andrew Jeffery <andrew@aj.id.au> Message-id: 20180209085755.30414-3-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/aspeed_soc.h | 1 - hw/arm/aspeed_soc.c | 32 +++----------------------------- 2 files changed, 3 insertions(+), 30 deletions(-) diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/aspeed_soc.h +++ b/include/hw/arm/aspeed_soc.h @@ -XXX,XX +XXX,XX @@ typedef struct AspeedSoCState { /*< public >*/ ARMCPU cpu; - MemoryRegion iomem; MemoryRegion sram; AspeedVICState vic; AspeedTimerCtrlState timerctrl; diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed_soc.c +++ b/hw/arm/aspeed_soc.c @@ -XXX,XX +XXX,XX @@ #include "qemu-common.h" #include "cpu.h" #include "exec/address-spaces.h" +#include "hw/misc/unimp.h" #include "hw/arm/aspeed_soc.h" #include "hw/char/serial.h" #include "qemu/log.h" @@ -XXX,XX +XXX,XX @@ static const AspeedSoCInfo aspeed_socs[] = { }, }; -/* - * IO handlers: simply catch any reads/writes to IO addresses that aren't - * handled by a device mapping. - */ - -static uint64_t aspeed_soc_io_read(void *p, hwaddr offset, unsigned size) -{ - qemu_log_mask(LOG_UNIMP, "%s: 0x%" HWADDR_PRIx " [%u]\n", - __func__, offset, size); - return 0; -} - -static void aspeed_soc_io_write(void *opaque, hwaddr offset, uint64_t value, - unsigned size) -{ - qemu_log_mask(LOG_UNIMP, "%s: 0x%" HWADDR_PRIx " <- 0x%" PRIx64 " [%u]\n", - __func__, offset, value, size); -} - -static const MemoryRegionOps aspeed_soc_io_ops = { - .read = aspeed_soc_io_read, - .write = aspeed_soc_io_write, - .endianness = DEVICE_LITTLE_ENDIAN, -}; - static void aspeed_soc_init(Object *obj) { AspeedSoCState *s = ASPEED_SOC(obj); @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) Error *err = NULL, *local_err = NULL; /* IO space */ - memory_region_init_io(&s->iomem, NULL, &aspeed_soc_io_ops, NULL, - "aspeed_soc.io", ASPEED_SOC_IOMEM_SIZE); - memory_region_add_subregion_overlap(get_system_memory(), - ASPEED_SOC_IOMEM_BASE, &s->iomem, -1); + create_unimplemented_device("aspeed_soc.io", + ASPEED_SOC_IOMEM_BASE, ASPEED_SOC_IOMEM_SIZE); /* CPU */ object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err); -- 2.16.1 From: Richard Henderson <richard.henderson@linaro.org> Because they are ARM_CP_STATE_AA64, ARM_CP_64BIT is implied. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180211205848.4568-2-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static void zcr_write(CPUARMState *env, const ARMCPRegInfo *ri, static const ARMCPRegInfo zcr_el1_reginfo = { .name = "ZCR_EL1", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 0, .crn = 1, .crm = 2, .opc2 = 0, - .access = PL1_RW, .accessfn = zcr_access, .type = ARM_CP_64BIT, + .access = PL1_RW, .accessfn = zcr_access, .fieldoffset = offsetof(CPUARMState, vfp.zcr_el[1]), .writefn = zcr_write, .raw_writefn = raw_write }; @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo zcr_el1_reginfo = { static const ARMCPRegInfo zcr_el2_reginfo = { .name = "ZCR_EL2", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 2, .opc2 = 0, - .access = PL2_RW, .accessfn = zcr_access, .type = ARM_CP_64BIT, + .access = PL2_RW, .accessfn = zcr_access, .fieldoffset = offsetof(CPUARMState, vfp.zcr_el[2]), .writefn = zcr_write, .raw_writefn = raw_write }; @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo zcr_el2_reginfo = { static const ARMCPRegInfo zcr_no_el2_reginfo = { .name = "ZCR_EL2", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 2, .opc2 = 0, - .access = PL2_RW, .type = ARM_CP_64BIT, + .access = PL2_RW, .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore }; static const ARMCPRegInfo zcr_el3_reginfo = { .name = "ZCR_EL3", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 2, .opc2 = 0, - .access = PL3_RW, .accessfn = zcr_access, .type = ARM_CP_64BIT, + .access = PL3_RW, .accessfn = zcr_access, .fieldoffset = offsetof(CPUARMState, vfp.zcr_el[3]), .writefn = zcr_write, .raw_writefn = raw_write }; -- 2.16.1 From: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180211205848.4568-3-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 35 ++++++++++++++++++----------------- target/arm/helper.c | 6 ++++-- target/arm/translate-a64.c | 3 +++ 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) } /* ARMCPRegInfo type field bits. If the SPECIAL bit is set this is a - * special-behaviour cp reg and bits [15..8] indicate what behaviour + * special-behaviour cp reg and bits [11..8] indicate what behaviour * it has. Otherwise it is a simple cp reg, where CONST indicates that * TCG can assume the value to be constant (ie load at translate time) * and 64BIT indicates a 64 bit wide coprocessor register. SUPPRESS_TB_END @@ -XXX,XX +XXX,XX @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) * need to be surrounded by gen_io_start()/gen_io_end(). In particular, * registers which implement clocks or timers require this. */ -#define ARM_CP_SPECIAL 1 -#define ARM_CP_CONST 2 -#define ARM_CP_64BIT 4 -#define ARM_CP_SUPPRESS_TB_END 8 -#define ARM_CP_OVERRIDE 16 -#define ARM_CP_ALIAS 32 -#define ARM_CP_IO 64 -#define ARM_CP_NO_RAW 128 -#define ARM_CP_NOP (ARM_CP_SPECIAL | (1 << 8)) -#define ARM_CP_WFI (ARM_CP_SPECIAL | (2 << 8)) -#define ARM_CP_NZCV (ARM_CP_SPECIAL | (3 << 8)) -#define ARM_CP_CURRENTEL (ARM_CP_SPECIAL | (4 << 8)) -#define ARM_CP_DC_ZVA (ARM_CP_SPECIAL | (5 << 8)) -#define ARM_LAST_SPECIAL ARM_CP_DC_ZVA +#define ARM_CP_SPECIAL 0x0001 +#define ARM_CP_CONST 0x0002 +#define ARM_CP_64BIT 0x0004 +#define ARM_CP_SUPPRESS_TB_END 0x0008 +#define ARM_CP_OVERRIDE 0x0010 +#define ARM_CP_ALIAS 0x0020 +#define ARM_CP_IO 0x0040 +#define ARM_CP_NO_RAW 0x0080 +#define ARM_CP_NOP (ARM_CP_SPECIAL | 0x0100) +#define ARM_CP_WFI (ARM_CP_SPECIAL | 0x0200) +#define ARM_CP_NZCV (ARM_CP_SPECIAL | 0x0300) +#define ARM_CP_CURRENTEL (ARM_CP_SPECIAL | 0x0400) +#define ARM_CP_DC_ZVA (ARM_CP_SPECIAL | 0x0500) +#define ARM_LAST_SPECIAL ARM_CP_DC_ZVA +#define ARM_CP_FPU 0x1000 /* Used only as a terminator for ARMCPRegInfo lists */ -#define ARM_CP_SENTINEL 0xffff +#define ARM_CP_SENTINEL 0xffff /* Mask of only the flag bits in a type field */ -#define ARM_CP_FLAG_MASK 0xff +#define ARM_CP_FLAG_MASK 0x10ff /* Valid values for ARMCPRegInfo state field, indicating which of * the AArch32 and AArch64 execution states this register is visible in. diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v8_cp_reginfo[] = { .writefn = aa64_daif_write, .resetfn = arm_cp_reset_ignore }, { .name = "FPCR", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 3, .opc2 = 0, .crn = 4, .crm = 4, - .access = PL0_RW, .readfn = aa64_fpcr_read, .writefn = aa64_fpcr_write }, + .access = PL0_RW, .type = ARM_CP_FPU, + .readfn = aa64_fpcr_read, .writefn = aa64_fpcr_write }, { .name = "FPSR", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 4, .crm = 4, - .access = PL0_RW, .readfn = aa64_fpsr_read, .writefn = aa64_fpsr_write }, + .access = PL0_RW, .type = ARM_CP_FPU, + .readfn = aa64_fpsr_read, .writefn = aa64_fpsr_write }, { .name = "DCZID_EL0", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 3, .opc2 = 7, .crn = 0, .crm = 0, .access = PL0_R, .type = ARM_CP_NO_RAW, diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, default: break; } + if ((ri->type & ARM_CP_FPU) && !fp_access_check(s)) { + return; + } if ((tb_cflags(s->base.tb) & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO)) { gen_io_start(); -- 2.16.1 From: Richard Henderson <richard.henderson@linaro.org> Nothing in either register affects the TB. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180211205848.4568-4-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v8_cp_reginfo[] = { .writefn = aa64_daif_write, .resetfn = arm_cp_reset_ignore }, { .name = "FPCR", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 3, .opc2 = 0, .crn = 4, .crm = 4, - .access = PL0_RW, .type = ARM_CP_FPU, + .access = PL0_RW, .type = ARM_CP_FPU | ARM_CP_SUPPRESS_TB_END, .readfn = aa64_fpcr_read, .writefn = aa64_fpcr_write }, { .name = "FPSR", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 4, .crm = 4, - .access = PL0_RW, .type = ARM_CP_FPU, + .access = PL0_RW, .type = ARM_CP_FPU | ARM_CP_SUPPRESS_TB_END, .readfn = aa64_fpsr_read, .writefn = aa64_fpsr_write }, { .name = "DCZID_EL0", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 3, .opc2 = 7, .crn = 0, .crm = 0, -- 2.16.1 From: Richard Henderson <richard.henderson@linaro.org> This also makes sure that we get the correct ordering of SVE vs FP exceptions. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180211205848.4568-5-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 3 ++- target/arm/internals.h | 6 ++++++ target/arm/helper.c | 22 ++++------------------ target/arm/translate-a64.c | 16 ++++++++++++++++ 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) #define ARM_CP_DC_ZVA (ARM_CP_SPECIAL | 0x0500) #define ARM_LAST_SPECIAL ARM_CP_DC_ZVA #define ARM_CP_FPU 0x1000 +#define ARM_CP_SVE 0x2000 /* Used only as a terminator for ARMCPRegInfo lists */ #define ARM_CP_SENTINEL 0xffff /* Mask of only the flag bits in a type field */ -#define ARM_CP_FLAG_MASK 0x10ff +#define ARM_CP_FLAG_MASK 0x30ff /* Valid values for ARMCPRegInfo state field, indicating which of * the AArch32 and AArch64 execution states this register is visible in. diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ enum arm_exception_class { EC_AA64_HVC = 0x16, EC_AA64_SMC = 0x17, EC_SYSTEMREGISTERTRAP = 0x18, + EC_SVEACCESSTRAP = 0x19, EC_INSNABORT = 0x20, EC_INSNABORT_SAME_EL = 0x21, EC_PCALIGNMENT = 0x22, @@ -XXX,XX +XXX,XX @@ static inline uint32_t syn_fp_access_trap(int cv, int cond, bool is_16bit) | (cv << 24) | (cond << 20); } +static inline uint32_t syn_sve_access_trap(void) +{ + return EC_SVEACCESSTRAP << ARM_EL_EC_SHIFT; +} + static inline uint32_t syn_insn_abort(int same_el, int ea, int s1ptw, int fsc) { return (EC_INSNABORT << ARM_EL_EC_SHIFT) | (same_el << ARM_EL_EC_SHIFT) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static int sve_exception_el(CPUARMState *env) return 0; } -static CPAccessResult zcr_access(CPUARMState *env, const ARMCPRegInfo *ri, - bool isread) -{ - switch (sve_exception_el(env)) { - case 3: - return CP_ACCESS_TRAP_EL3; - case 2: - return CP_ACCESS_TRAP_EL2; - case 1: - return CP_ACCESS_TRAP; - } - return CP_ACCESS_OK; -} - static void zcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) { @@ -XXX,XX +XXX,XX @@ static void zcr_write(CPUARMState *env, const ARMCPRegInfo *ri, static const ARMCPRegInfo zcr_el1_reginfo = { .name = "ZCR_EL1", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 0, .crn = 1, .crm = 2, .opc2 = 0, - .access = PL1_RW, .accessfn = zcr_access, + .access = PL1_RW, .type = ARM_CP_SVE | ARM_CP_FPU, .fieldoffset = offsetof(CPUARMState, vfp.zcr_el[1]), .writefn = zcr_write, .raw_writefn = raw_write }; @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo zcr_el1_reginfo = { static const ARMCPRegInfo zcr_el2_reginfo = { .name = "ZCR_EL2", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 2, .opc2 = 0, - .access = PL2_RW, .accessfn = zcr_access, + .access = PL2_RW, .type = ARM_CP_SVE | ARM_CP_FPU, .fieldoffset = offsetof(CPUARMState, vfp.zcr_el[2]), .writefn = zcr_write, .raw_writefn = raw_write }; @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo zcr_el2_reginfo = { static const ARMCPRegInfo zcr_no_el2_reginfo = { .name = "ZCR_EL2", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 2, .opc2 = 0, - .access = PL2_RW, + .access = PL2_RW, .type = ARM_CP_SVE | ARM_CP_FPU, .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore }; static const ARMCPRegInfo zcr_el3_reginfo = { .name = "ZCR_EL3", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 2, .opc2 = 0, - .access = PL3_RW, .accessfn = zcr_access, + .access = PL3_RW, .type = ARM_CP_SVE | ARM_CP_FPU, .fieldoffset = offsetof(CPUARMState, vfp.zcr_el[3]), .writefn = zcr_write, .raw_writefn = raw_write }; diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static inline bool fp_access_check(DisasContext *s) return false; } +/* Check that SVE access is enabled. If it is, return true. + * If not, emit code to generate an appropriate exception and return false. + */ +static inline bool sve_access_check(DisasContext *s) +{ + if (s->sve_excp_el) { + gen_exception_insn(s, 4, EXCP_UDEF, syn_sve_access_trap(), + s->sve_excp_el); + return false; + } + return true; +} + /* * This utility function is for doing register extension with an * optional shift. You will likely want to pass a temporary for the @@ -XXX,XX +XXX,XX @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, default: break; } + if ((ri->type & ARM_CP_SVE) && !sve_access_check(s)) { + return; + } if ((ri->type & ARM_CP_FPU) && !fp_access_check(s)) { return; } -- 2.16.1 From: Richard Henderson <richard.henderson@linaro.org> When storing to an AdvSIMD FP register, all of the high bits of the SVE register are zeroed. Therefore, call it more often with is_q as a parameter. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180211205848.4568-6-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.c | 162 +++++++++++++++++---------------------------- 1 file changed, 62 insertions(+), 100 deletions(-) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static TCGv_i32 read_fp_sreg(DisasContext *s, int reg) return v; } +/* Clear the bits above an N-bit vector, for N = (is_q ? 128 : 64). + * If SVE is not enabled, then there are only 128 bits in the vector. + */ +static void clear_vec_high(DisasContext *s, bool is_q, int rd) +{ + unsigned ofs = fp_reg_offset(s, rd, MO_64); + unsigned vsz = vec_full_reg_size(s); + + if (!is_q) { + TCGv_i64 tcg_zero = tcg_const_i64(0); + tcg_gen_st_i64(tcg_zero, cpu_env, ofs + 8); + tcg_temp_free_i64(tcg_zero); + } + if (vsz > 16) { + tcg_gen_gvec_dup8i(ofs + 16, vsz - 16, vsz - 16, 0); + } +} + static void write_fp_dreg(DisasContext *s, int reg, TCGv_i64 v) { - TCGv_i64 tcg_zero = tcg_const_i64(0); + unsigned ofs = fp_reg_offset(s, reg, MO_64); - tcg_gen_st_i64(v, cpu_env, fp_reg_offset(s, reg, MO_64)); - tcg_gen_st_i64(tcg_zero, cpu_env, fp_reg_hi_offset(s, reg)); - tcg_temp_free_i64(tcg_zero); + tcg_gen_st_i64(v, cpu_env, ofs); + clear_vec_high(s, false, reg); } static void write_fp_sreg(DisasContext *s, int reg, TCGv_i32 v) @@ -XXX,XX +XXX,XX @@ static void do_fp_ld(DisasContext *s, int destidx, TCGv_i64 tcg_addr, int size) tcg_temp_free_i64(tmplo); tcg_temp_free_i64(tmphi); + + clear_vec_high(s, true, destidx); } /* @@ -XXX,XX +XXX,XX @@ static void write_vec_element_i32(DisasContext *s, TCGv_i32 tcg_src, } } -/* Clear the high 64 bits of a 128 bit vector (in general non-quad - * vector ops all need to do this). - */ -static void clear_vec_high(DisasContext *s, int rd) -{ - TCGv_i64 tcg_zero = tcg_const_i64(0); - - write_vec_element(s, tcg_zero, rd, 1, MO_64); - tcg_temp_free_i64(tcg_zero); -} - /* Store from vector register to memory */ static void do_vec_st(DisasContext *s, int srcidx, int element, TCGv_i64 tcg_addr, int size) @@ -XXX,XX +XXX,XX @@ static void disas_ldst_multiple_struct(DisasContext *s, uint32_t insn) /* For non-quad operations, setting a slice of the low * 64 bits of the register clears the high 64 bits (in * the ARM ARM pseudocode this is implicit in the fact - * that 'rval' is a 64 bit wide variable). We optimize - * by noticing that we only need to do this the first - * time we touch a register. + * that 'rval' is a 64 bit wide variable). + * For quad operations, we might still need to zero the + * high bits of SVE. We optimize by noticing that we only + * need to do this the first time we touch a register. */ - if (!is_q && e == 0 && (r == 0 || xs == selem - 1)) { - clear_vec_high(s, tt); + if (e == 0 && (r == 0 || xs == selem - 1)) { + clear_vec_high(s, is_q, tt); } } tcg_gen_addi_i64(tcg_addr, tcg_addr, ebytes); @@ -XXX,XX +XXX,XX @@ static void disas_ldst_single_struct(DisasContext *s, uint32_t insn) write_vec_element(s, tcg_tmp, rt, 0, MO_64); if (is_q) { write_vec_element(s, tcg_tmp, rt, 1, MO_64); - } else { - clear_vec_high(s, rt); } tcg_temp_free_i64(tcg_tmp); + clear_vec_high(s, is_q, rt); } else { /* Load/store one element per register */ if (is_load) { @@ -XXX,XX +XXX,XX @@ static void handle_vec_simd_sqshrn(DisasContext *s, bool is_scalar, bool is_q, } if (!is_q) { - clear_vec_high(s, rd); write_vec_element(s, tcg_final, rd, 0, MO_64); } else { write_vec_element(s, tcg_final, rd, 1, MO_64); @@ -XXX,XX +XXX,XX @@ static void handle_vec_simd_sqshrn(DisasContext *s, bool is_scalar, bool is_q, tcg_temp_free_i64(tcg_rd); tcg_temp_free_i32(tcg_rd_narrowed); tcg_temp_free_i64(tcg_final); - return; + + clear_vec_high(s, is_q, rd); } /* SQSHLU, UQSHL, SQSHL: saturating left shifts */ @@ -XXX,XX +XXX,XX @@ static void handle_simd_qshl(DisasContext *s, bool scalar, bool is_q, tcg_temp_free_i64(tcg_op); } tcg_temp_free_i64(tcg_shift); - - if (!is_q) { - clear_vec_high(s, rd); - } + clear_vec_high(s, is_q, rd); } else { TCGv_i32 tcg_shift = tcg_const_i32(shift); static NeonGenTwoOpEnvFn * const fns[2][2][3] = { @@ -XXX,XX +XXX,XX @@ static void handle_simd_qshl(DisasContext *s, bool scalar, bool is_q, } tcg_temp_free_i32(tcg_shift); - if (!is_q && !scalar) { - clear_vec_high(s, rd); + if (!scalar) { + clear_vec_high(s, is_q, rd); } } } @@ -XXX,XX +XXX,XX @@ static void handle_simd_intfp_conv(DisasContext *s, int rd, int rn, } } - if (!is_double && elements == 2) { - clear_vec_high(s, rd); - } - tcg_temp_free_i64(tcg_int); tcg_temp_free_ptr(tcg_fpst); tcg_temp_free_i32(tcg_shift); + + clear_vec_high(s, elements << size == 16, rd); } /* UCVTF/SCVTF - Integer to FP conversion */ @@ -XXX,XX +XXX,XX @@ static void handle_simd_shift_fpint_conv(DisasContext *s, bool is_scalar, write_vec_element(s, tcg_op, rd, pass, MO_64); tcg_temp_free_i64(tcg_op); } - if (!is_q) { - clear_vec_high(s, rd); - } + clear_vec_high(s, is_q, rd); } else { int maxpass = is_scalar ? 1 : is_q ? 4 : 2; for (pass = 0; pass < maxpass; pass++) { @@ -XXX,XX +XXX,XX @@ static void handle_simd_shift_fpint_conv(DisasContext *s, bool is_scalar, } tcg_temp_free_i32(tcg_op); } - if (!is_q && !is_scalar) { - clear_vec_high(s, rd); + if (!is_scalar) { + clear_vec_high(s, is_q, rd); } } @@ -XXX,XX +XXX,XX @@ static void handle_3same_float(DisasContext *s, int size, int elements, tcg_temp_free_ptr(fpst); - if ((elements << size) < 4) { - /* scalar, or non-quad vector op */ - clear_vec_high(s, rd); - } + clear_vec_high(s, elements * (size ? 8 : 4) > 8, rd); } /* AdvSIMD scalar three same @@ -XXX,XX +XXX,XX @@ static void handle_2misc_fcmp_zero(DisasContext *s, int opcode, } write_vec_element(s, tcg_res, rd, pass, MO_64); } - if (is_scalar) { - clear_vec_high(s, rd); - } - tcg_temp_free_i64(tcg_res); tcg_temp_free_i64(tcg_zero); tcg_temp_free_i64(tcg_op); + + clear_vec_high(s, !is_scalar, rd); } else { TCGv_i32 tcg_op = tcg_temp_new_i32(); TCGv_i32 tcg_zero = tcg_const_i32(0); @@ -XXX,XX +XXX,XX @@ static void handle_2misc_fcmp_zero(DisasContext *s, int opcode, tcg_temp_free_i32(tcg_res); tcg_temp_free_i32(tcg_zero); tcg_temp_free_i32(tcg_op); - if (!is_q && !is_scalar) { - clear_vec_high(s, rd); + if (!is_scalar) { + clear_vec_high(s, is_q, rd); } } @@ -XXX,XX +XXX,XX @@ static void handle_2misc_reciprocal(DisasContext *s, int opcode, } write_vec_element(s, tcg_res, rd, pass, MO_64); } - if (is_scalar) { - clear_vec_high(s, rd); - } - tcg_temp_free_i64(tcg_res); tcg_temp_free_i64(tcg_op); + clear_vec_high(s, !is_scalar, rd); } else { TCGv_i32 tcg_op = tcg_temp_new_i32(); TCGv_i32 tcg_res = tcg_temp_new_i32(); @@ -XXX,XX +XXX,XX @@ static void handle_2misc_reciprocal(DisasContext *s, int opcode, } tcg_temp_free_i32(tcg_res); tcg_temp_free_i32(tcg_op); - if (!is_q && !is_scalar) { - clear_vec_high(s, rd); + if (!is_scalar) { + clear_vec_high(s, is_q, rd); } } tcg_temp_free_ptr(fpst); @@ -XXX,XX +XXX,XX @@ static void handle_2misc_narrow(DisasContext *s, bool scalar, write_vec_element_i32(s, tcg_res[pass], rd, destelt + pass, MO_32); tcg_temp_free_i32(tcg_res[pass]); } - if (!is_q) { - clear_vec_high(s, rd); - } + clear_vec_high(s, is_q, rd); } /* Remaining saturating accumulating ops */ @@ -XXX,XX +XXX,XX @@ static void handle_2misc_satacc(DisasContext *s, bool is_scalar, bool is_u, } write_vec_element(s, tcg_rd, rd, pass, MO_64); } - if (is_scalar) { - clear_vec_high(s, rd); - } - tcg_temp_free_i64(tcg_rd); tcg_temp_free_i64(tcg_rn); + clear_vec_high(s, !is_scalar, rd); } else { TCGv_i32 tcg_rn = tcg_temp_new_i32(); TCGv_i32 tcg_rd = tcg_temp_new_i32(); @@ -XXX,XX +XXX,XX @@ static void handle_2misc_satacc(DisasContext *s, bool is_scalar, bool is_u, } write_vec_element_i32(s, tcg_rd, rd, pass, MO_32); } - - if (!is_q) { - clear_vec_high(s, rd); - } - tcg_temp_free_i32(tcg_rd); tcg_temp_free_i32(tcg_rn); + clear_vec_high(s, is_q, rd); } } @@ -XXX,XX +XXX,XX @@ static void handle_vec_simd_shri(DisasContext *s, bool is_q, bool is_u, tcg_temp_free_i64(tcg_round); done: - if (!is_q) { - clear_vec_high(s, rd); - } + clear_vec_high(s, is_q, rd); } static void gen_shl8_ins_i64(TCGv_i64 d, TCGv_i64 a, int64_t shift) @@ -XXX,XX +XXX,XX @@ static void handle_vec_simd_shrn(DisasContext *s, bool is_q, } if (!is_q) { - clear_vec_high(s, rd); write_vec_element(s, tcg_final, rd, 0, MO_64); } else { write_vec_element(s, tcg_final, rd, 1, MO_64); } - if (round) { tcg_temp_free_i64(tcg_round); } tcg_temp_free_i64(tcg_rn); tcg_temp_free_i64(tcg_rd); tcg_temp_free_i64(tcg_final); - return; + + clear_vec_high(s, is_q, rd); } @@ -XXX,XX +XXX,XX @@ static void handle_3rd_narrowing(DisasContext *s, int is_q, int is_u, int size, write_vec_element_i32(s, tcg_res[pass], rd, pass + part, MO_32); tcg_temp_free_i32(tcg_res[pass]); } - if (!is_q) { - clear_vec_high(s, rd); - } + clear_vec_high(s, is_q, rd); } static void handle_pmull_64(DisasContext *s, int is_q, int rd, int rn, int rm) @@ -XXX,XX +XXX,XX @@ static void handle_simd_3same_pair(DisasContext *s, int is_q, int u, int opcode, write_vec_element_i32(s, tcg_res[pass], rd, pass, MO_32); tcg_temp_free_i32(tcg_res[pass]); } - if (!is_q) { - clear_vec_high(s, rd); - } + clear_vec_high(s, is_q, rd); } if (fpst) { @@ -XXX,XX +XXX,XX @@ static void disas_simd_3same_int(DisasContext *s, uint32_t insn) tcg_temp_free_i32(tcg_op2); } } - - if (!is_q) { - clear_vec_high(s, rd); - } + clear_vec_high(s, is_q, rd); } /* AdvSIMD three same @@ -XXX,XX +XXX,XX @@ static void handle_rev(DisasContext *s, int opcode, bool u, write_vec_element(s, tcg_tmp, rd, i, grp_size); tcg_temp_free_i64(tcg_tmp); } - if (!is_q) { - clear_vec_high(s, rd); - } + clear_vec_high(s, is_q, rd); } else { int revmask = (1 << grp_size) - 1; int esize = 8 << size; @@ -XXX,XX +XXX,XX @@ static void disas_simd_two_reg_misc(DisasContext *s, uint32_t insn) tcg_temp_free_i32(tcg_op); } } - if (!is_q) { - clear_vec_high(s, rd); - } + clear_vec_high(s, is_q, rd); if (need_rmode) { gen_helper_set_rmode(tcg_rmode, tcg_rmode, cpu_env); @@ -XXX,XX +XXX,XX @@ static void disas_simd_indexed(DisasContext *s, uint32_t insn) tcg_temp_free_i64(tcg_res); } - if (is_scalar) { - clear_vec_high(s, rd); - } - tcg_temp_free_i64(tcg_idx); + clear_vec_high(s, !is_scalar, rd); } else if (!is_long) { /* 32 bit floating point, or 16 or 32 bit integer. * For the 16 bit scalar case we use the usual Neon helpers and @@ -XXX,XX +XXX,XX @@ static void disas_simd_indexed(DisasContext *s, uint32_t insn) } tcg_temp_free_i32(tcg_idx); - - if (!is_q) { - clear_vec_high(s, rd); - } + clear_vec_high(s, is_q, rd); } else { /* long ops: 16x16->32 or 32x32->64 */ TCGv_i64 tcg_res[2]; @@ -XXX,XX +XXX,XX @@ static void disas_simd_indexed(DisasContext *s, uint32_t insn) } tcg_temp_free_i64(tcg_idx); - if (is_scalar) { - clear_vec_high(s, rd); - } + clear_vec_high(s, !is_scalar, rd); } else { TCGv_i32 tcg_idx = tcg_temp_new_i32(); -- 2.16.1 Instead of hardcoding the values of M profile ID registers in the NVIC, use the fields in the CPU struct. This will allow us to give different M profile CPU types different ID register values. This commit includes the addition of the missing ID_ISAR5, which exists as RES0 in both v7M and v8M. (The values of the ID registers might be wrong for the M4 -- this commit leaves the behaviour there unchanged.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180209165810.6668-2-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 30 ++++++++++++++++-------------- target/arm/cpu.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs) "Aux Fault status registers unimplemented\n"); return 0; case 0xd40: /* PFR0. */ - return 0x00000030; - case 0xd44: /* PRF1. */ - return 0x00000200; + return cpu->id_pfr0; + case 0xd44: /* PFR1. */ + return cpu->id_pfr1; case 0xd48: /* DFR0. */ - return 0x00100000; + return cpu->id_dfr0; case 0xd4c: /* AFR0. */ - return 0x00000000; + return cpu->id_afr0; case 0xd50: /* MMFR0. */ - return 0x00000030; + return cpu->id_mmfr0; case 0xd54: /* MMFR1. */ - return 0x00000000; + return cpu->id_mmfr1; case 0xd58: /* MMFR2. */ - return 0x00000000; + return cpu->id_mmfr2; case 0xd5c: /* MMFR3. */ - return 0x00000000; + return cpu->id_mmfr3; case 0xd60: /* ISAR0. */ - return 0x01141110; + return cpu->id_isar0; case 0xd64: /* ISAR1. */ - return 0x02111000; + return cpu->id_isar1; case 0xd68: /* ISAR2. */ - return 0x21112231; + return cpu->id_isar2; case 0xd6c: /* ISAR3. */ - return 0x01111110; + return cpu->id_isar3; case 0xd70: /* ISAR4. */ - return 0x01310102; + return cpu->id_isar4; + case 0xd74: /* ISAR5. */ + return cpu->id_isar5; /* TODO: Implement debug registers. */ case 0xd90: /* MPU_TYPE */ /* Unified MPU; if the MPU is not present this value is zero */ diff --git a/target/arm/cpu.c b/target/arm/cpu.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -XXX,XX +XXX,XX @@ static void cortex_m3_initfn(Object *obj) set_feature(&cpu->env, ARM_FEATURE_M); cpu->midr = 0x410fc231; cpu->pmsav7_dregion = 8; + cpu->id_pfr0 = 0x00000030; + cpu->id_pfr1 = 0x00000200; + cpu->id_dfr0 = 0x00100000; + cpu->id_afr0 = 0x00000000; + cpu->id_mmfr0 = 0x00000030; + cpu->id_mmfr1 = 0x00000000; + cpu->id_mmfr2 = 0x00000000; + cpu->id_mmfr3 = 0x00000000; + cpu->id_isar0 = 0x01141110; + cpu->id_isar1 = 0x02111000; + cpu->id_isar2 = 0x21112231; + cpu->id_isar3 = 0x01111110; + cpu->id_isar4 = 0x01310102; + cpu->id_isar5 = 0x00000000; } static void cortex_m4_initfn(Object *obj) @@ -XXX,XX +XXX,XX @@ static void cortex_m4_initfn(Object *obj) set_feature(&cpu->env, ARM_FEATURE_THUMB_DSP); cpu->midr = 0x410fc240; /* r0p0 */ cpu->pmsav7_dregion = 8; + cpu->id_pfr0 = 0x00000030; + cpu->id_pfr1 = 0x00000200; + cpu->id_dfr0 = 0x00100000; + cpu->id_afr0 = 0x00000000; + cpu->id_mmfr0 = 0x00000030; + cpu->id_mmfr1 = 0x00000000; + cpu->id_mmfr2 = 0x00000000; + cpu->id_mmfr3 = 0x00000000; + cpu->id_isar0 = 0x01141110; + cpu->id_isar1 = 0x02111000; + cpu->id_isar2 = 0x21112231; + cpu->id_isar3 = 0x01111110; + cpu->id_isar4 = 0x01310102; + cpu->id_isar5 = 0x00000000; } static void arm_v7m_class_init(ObjectClass *oc, void *data) -- 2.16.1 The PENDNMISET/CLR bits in the ICSR should be RAZ/WI from NonSecure state if the AIRCR.BFHFNMINS bit is zero. We had misimplemented this as making the bits RAZ/WI from both Secure and NonSecure states. Fix this bug by checking attrs.secure so that Secure code can pend and unpend NMIs. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180209165810.6668-3-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs) } } /* NMIPENDSET */ - if ((cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK) && - s->vectors[ARMV7M_EXCP_NMI].pending) { + if ((attrs.secure || (cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) + && s->vectors[ARMV7M_EXCP_NMI].pending) { val |= (1 << 31); } /* ISRPREEMPT: RES0 when halting debug not implemented */ @@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, break; } case 0xd04: /* Interrupt Control State (ICSR) */ - if (cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK) { + if (attrs.secure || cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK) { if (value & (1 << 31)) { armv7m_nvic_set_pending(s, ARMV7M_EXCP_NMI, false); } else if (value & (1 << 30) && -- 2.16.1 For M profile cores, cache maintenance operations are done by writing to special registers in the system register space. For QEMU, cache operations are always NOPs, since we don't implement the cache. Implementing these explicitly avoids a spurious LOG_GUEST_ERROR when the guest uses them. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180209165810.6668-4-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, } break; } + case 0xf50: /* ICIALLU */ + case 0xf58: /* ICIMVAU */ + case 0xf5c: /* DCIMVAC */ + case 0xf60: /* DCISW */ + case 0xf64: /* DCCMVAU */ + case 0xf68: /* DCCMVAC */ + case 0xf6c: /* DCCSW */ + case 0xf70: /* DCCIMVAC */ + case 0xf74: /* DCCISW */ + case 0xf78: /* BPIALL */ + /* Cache and branch predictor maintenance: for QEMU these always NOP */ + break; default: bad_offset: qemu_log_mask(LOG_GUEST_ERROR, -- 2.16.1 The Coprocessor Power Control Register (CPPWR) is new in v8M. It allows software to control whether coprocessors are allowed to power down and lose their state. QEMU doesn't have any notion of power control, so we choose the IMPDEF option of making the whole register RAZ/WI (indicating that no coprocessors can ever power down and lose state). Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180209165810.6668-5-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs) switch (offset) { case 4: /* Interrupt Control Type. */ return ((s->num_irq - NVIC_FIRST_IRQ) / 32) - 1; + case 0xc: /* CPPWR */ + if (!arm_feature(&cpu->env, ARM_FEATURE_V8)) { + goto bad_offset; + } + /* We make the IMPDEF choice that nothing can ever go into a + * non-retentive power state, which allows us to RAZ/WI this. + */ + return 0; case 0x380 ... 0x3bf: /* NVIC_ITNS<n> */ { int startvec = 8 * (offset - 0x380) + NVIC_FIRST_IRQ; @@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, ARMCPU *cpu = s->cpu; switch (offset) { + case 0xc: /* CPPWR */ + if (!arm_feature(&cpu->env, ARM_FEATURE_V8)) { + goto bad_offset; + } + /* Make the IMPDEF choice to RAZ/WI this. */ + break; case 0x380 ... 0x3bf: /* NVIC_ITNS<n> */ { int startvec = 8 * (offset - 0x380) + NVIC_FIRST_IRQ; -- 2.16.1 M profile cores have a similar setup for cache ID registers to A profile: * Cache Level ID Register (CLIDR) is a fixed value * Cache Type Register (CTR) is a fixed value * Cache Size ID Registers (CCSIDR) are a bank of registers; which one you see is selected by the Cache Size Selection Register (CSSELR) The only difference is that they're in the NVIC memory mapped register space rather than being coprocessor registers. Implement the M profile view of them. Since neither Cortex-M3 nor Cortex-M4 implement caches, we don't need to update their init functions and can leave the ctr/clidr/ccsidr[] fields in their ARMCPU structs at zero. Newer cores (like the Cortex-M33) will want to be able to set these ID registers to non-zero values, though. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180209165810.6668-6-peter.maydell@linaro.org --- target/arm/cpu.h | 26 ++++++++++++++++++++++++++ hw/intc/armv7m_nvic.c | 16 ++++++++++++++++ target/arm/machine.c | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ typedef struct CPUARMState { uint32_t faultmask[M_REG_NUM_BANKS]; uint32_t aircr; /* only holds r/w state if security extn implemented */ uint32_t secure; /* Is CPU in Secure state? (not guest visible) */ + uint32_t csselr[M_REG_NUM_BANKS]; } v7m; /* Information associated with an exception about to be taken: @@ -XXX,XX +XXX,XX @@ FIELD(V7M_MPU_CTRL, ENABLE, 0, 1) FIELD(V7M_MPU_CTRL, HFNMIENA, 1, 1) FIELD(V7M_MPU_CTRL, PRIVDEFENA, 2, 1) +/* v7M CLIDR bits */ +FIELD(V7M_CLIDR, CTYPE_ALL, 0, 21) +FIELD(V7M_CLIDR, LOUIS, 21, 3) +FIELD(V7M_CLIDR, LOC, 24, 3) +FIELD(V7M_CLIDR, LOUU, 27, 3) +FIELD(V7M_CLIDR, ICB, 30, 2) + +FIELD(V7M_CSSELR, IND, 0, 1) +FIELD(V7M_CSSELR, LEVEL, 1, 3) +/* We use the combination of InD and Level to index into cpu->ccsidr[]; + * define a mask for this and check that it doesn't permit running off + * the end of the array. + */ +FIELD(V7M_CSSELR, INDEX, 0, 4) + +QEMU_BUILD_BUG_ON(ARRAY_SIZE(((ARMCPU *)0)->ccsidr) <= R_V7M_CSSELR_INDEX_MASK); + /* If adding a feature bit which corresponds to a Linux ELF * HWCAP bit, remember to update the feature-bit-to-hwcap * mapping in linux-user/elfload.c:get_elf_hwcap(). @@ -XXX,XX +XXX,XX @@ static inline int arm_debug_target_el(CPUARMState *env) } } +static inline bool arm_v7m_csselr_razwi(ARMCPU *cpu) +{ + /* If all the CLIDR.Ctypem bits are 0 there are no caches, and + * CSSELR is RAZ/WI. + */ + return (cpu->clidr & R_V7M_CLIDR_CTYPE_ALL_MASK) != 0; +} + static inline bool aa64_generate_debug_exceptions(CPUARMState *env) { if (arm_is_secure(env)) { diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs) return cpu->id_isar4; case 0xd74: /* ISAR5. */ return cpu->id_isar5; + case 0xd78: /* CLIDR */ + return cpu->clidr; + case 0xd7c: /* CTR */ + return cpu->ctr; + case 0xd80: /* CSSIDR */ + { + int idx = cpu->env.v7m.csselr[attrs.secure] & R_V7M_CSSELR_INDEX_MASK; + return cpu->ccsidr[idx]; + } + case 0xd84: /* CSSELR */ + return cpu->env.v7m.csselr[attrs.secure]; /* TODO: Implement debug registers. */ case 0xd90: /* MPU_TYPE */ /* Unified MPU; if the MPU is not present this value is zero */ @@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, qemu_log_mask(LOG_UNIMP, "NVIC: Aux fault status registers unimplemented\n"); break; + case 0xd84: /* CSSELR */ + if (!arm_v7m_csselr_razwi(cpu)) { + cpu->env.v7m.csselr[attrs.secure] = value & R_V7M_CSSELR_INDEX_MASK; + } + break; case 0xd90: /* MPU_TYPE */ return; /* RO */ case 0xd94: /* MPU_CTRL */ diff --git a/target/arm/machine.c b/target/arm/machine.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/machine.c +++ b/target/arm/machine.c @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_m_faultmask_primask = { } }; +/* CSSELR is in a subsection because we didn't implement it previously. + * Migration from an old implementation will leave it at zero, which + * is OK since the only CPUs in the old implementation make the + * register RAZ/WI. + * Since there was no version of QEMU which implemented the CSSELR for + * just non-secure, we transfer both banks here rather than putting + * the secure banked version in the m-security subsection. + */ +static bool csselr_vmstate_validate(void *opaque, int version_id) +{ + ARMCPU *cpu = opaque; + + return cpu->env.v7m.csselr[M_REG_NS] <= R_V7M_CSSELR_INDEX_MASK + && cpu->env.v7m.csselr[M_REG_S] <= R_V7M_CSSELR_INDEX_MASK; +} + +static bool m_csselr_needed(void *opaque) +{ + ARMCPU *cpu = opaque; + + return !arm_v7m_csselr_razwi(cpu); +} + +static const VMStateDescription vmstate_m_csselr = { + .name = "cpu/m/csselr", + .version_id = 1, + .minimum_version_id = 1, + .needed = m_csselr_needed, + .fields = (VMStateField[]) { + VMSTATE_UINT32_ARRAY(env.v7m.csselr, ARMCPU, M_REG_NUM_BANKS), + VMSTATE_VALIDATE("CSSELR is valid", csselr_vmstate_validate), + VMSTATE_END_OF_LIST() + } +}; + static const VMStateDescription vmstate_m = { .name = "cpu/m", .version_id = 4, @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_m = { }, .subsections = (const VMStateDescription*[]) { &vmstate_m_faultmask_primask, + &vmstate_m_csselr, NULL } }; -- 2.16.1 We were previously making the system control register (SCR) just RAZ/WI. Although we don't implement the functionality this register controls, we should at least provide the state, including the banked state for v8M. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180209165810.6668-7-peter.maydell@linaro.org --- target/arm/cpu.h | 7 +++++++ hw/intc/armv7m_nvic.c | 12 ++++++++---- target/arm/machine.c | 12 ++++++++++++ 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ typedef struct CPUARMState { uint32_t aircr; /* only holds r/w state if security extn implemented */ uint32_t secure; /* Is CPU in Secure state? (not guest visible) */ uint32_t csselr[M_REG_NUM_BANKS]; + uint32_t scr[M_REG_NUM_BANKS]; } v7m; /* Information associated with an exception about to be taken: @@ -XXX,XX +XXX,XX @@ FIELD(V7M_CCR, STKALIGN, 9, 1) FIELD(V7M_CCR, DC, 16, 1) FIELD(V7M_CCR, IC, 17, 1) +/* V7M SCR bits */ +FIELD(V7M_SCR, SLEEPONEXIT, 1, 1) +FIELD(V7M_SCR, SLEEPDEEP, 2, 1) +FIELD(V7M_SCR, SLEEPDEEPS, 3, 1) +FIELD(V7M_SCR, SEVONPEND, 4, 1) + /* V7M AIRCR bits */ FIELD(V7M_AIRCR, VECTRESET, 0, 1) FIELD(V7M_AIRCR, VECTCLRACTIVE, 1, 1) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs) } return val; case 0xd10: /* System Control. */ - /* TODO: Implement SLEEPONEXIT. */ - return 0; + return cpu->env.v7m.scr[attrs.secure]; case 0xd14: /* Configuration Control. */ /* The BFHFNMIGN bit is the only non-banked bit; we * keep it in the non-secure copy of the register. @@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, } break; case 0xd10: /* System Control. */ - /* TODO: Implement control registers. */ - qemu_log_mask(LOG_UNIMP, "NVIC: SCR unimplemented\n"); + /* We don't implement deep-sleep so these bits are RAZ/WI. + * The other bits in the register are banked. + * QEMU's implementation ignores SEVONPEND and SLEEPONEXIT, which + * is architecturally permitted. + */ + value &= ~(R_V7M_SCR_SLEEPDEEP_MASK | R_V7M_SCR_SLEEPDEEPS_MASK); + cpu->env.v7m.scr[attrs.secure] = value; break; case 0xd14: /* Configuration Control. */ /* Enforce RAZ/WI on reserved and must-RAZ/WI bits */ diff --git a/target/arm/machine.c b/target/arm/machine.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/machine.c +++ b/target/arm/machine.c @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_m_csselr = { } }; +static const VMStateDescription vmstate_m_scr = { + .name = "cpu/m/scr", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT32(env.v7m.scr[M_REG_NS], ARMCPU), + VMSTATE_END_OF_LIST() + } +}; + static const VMStateDescription vmstate_m = { .name = "cpu/m", .version_id = 4, @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_m = { .subsections = (const VMStateDescription*[]) { &vmstate_m_faultmask_primask, &vmstate_m_csselr, + &vmstate_m_scr, NULL } }; @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_m_security = { VMSTATE_UINT32(env.sau.rnr, ARMCPU), VMSTATE_VALIDATE("SAU_RNR is valid", sau_rnr_vmstate_validate), VMSTATE_UINT32(env.sau.ctrl, ARMCPU), + VMSTATE_UINT32(env.v7m.scr[M_REG_S], ARMCPU), VMSTATE_END_OF_LIST() } }; -- 2.16.1 In commit 50f11062d4c896 we added support for MSR/MRS access to the NS banked special registers, but we forgot to implement the support for writing to CONTROL_NS. Correct the omission. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180209165810.6668-8-peter.maydell@linaro.org --- target/arm/helper.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val) } env->v7m.faultmask[M_REG_NS] = val & 1; return; + case 0x94: /* CONTROL_NS */ + if (!env->v7m.secure) { + return; + } + write_v7m_control_spsel_for_secstate(env, + val & R_V7M_CONTROL_SPSEL_MASK, + M_REG_NS); + env->v7m.control[M_REG_NS] &= ~R_V7M_CONTROL_NPRIV_MASK; + env->v7m.control[M_REG_NS] |= val & R_V7M_CONTROL_NPRIV_MASK; + return; case 0x98: /* SP_NS */ { /* This gives the non-secure SP selected based on whether we're -- 2.16.1 In many of the NVIC registers relating to interrupts, we have to convert from a byte offset within a register set into the number of the first interrupt which is affected. We were getting this wrong for: * reads of NVIC_ISPR<n>, NVIC_ISER<n>, NVIC_ICPR<n>, NVIC_ICER<n>, NVIC_IABR<n> -- in all these cases we were missing the "* 8" needed to convert from the byte offset to the interrupt number (since all these registers use one bit per interrupt) * writes of NVIC_IPR<n> had the opposite problem of a spurious "* 8" (since these registers use one byte per interrupt) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20180209165810.6668-9-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ static MemTxResult nvic_sysreg_read(void *opaque, hwaddr addr, /* fall through */ case 0x180 ... 0x1bf: /* NVIC Clear enable */ val = 0; - startvec = offset - 0x180 + NVIC_FIRST_IRQ; /* vector # */ + startvec = 8 * (offset - 0x180) + NVIC_FIRST_IRQ; /* vector # */ for (i = 0, end = size * 8; i < end && startvec + i < s->num_irq; i++) { if (s->vectors[startvec + i].enabled && @@ -XXX,XX +XXX,XX @@ static MemTxResult nvic_sysreg_read(void *opaque, hwaddr addr, /* fall through */ case 0x280 ... 0x2bf: /* NVIC Clear pend */ val = 0; - startvec = offset - 0x280 + NVIC_FIRST_IRQ; /* vector # */ + startvec = 8 * (offset - 0x280) + NVIC_FIRST_IRQ; /* vector # */ for (i = 0, end = size * 8; i < end && startvec + i < s->num_irq; i++) { if (s->vectors[startvec + i].pending && (attrs.secure || s->itns[startvec + i])) { @@ -XXX,XX +XXX,XX @@ static MemTxResult nvic_sysreg_read(void *opaque, hwaddr addr, break; case 0x300 ... 0x33f: /* NVIC Active */ val = 0; - startvec = offset - 0x300 + NVIC_FIRST_IRQ; /* vector # */ + startvec = 8 * (offset - 0x300) + NVIC_FIRST_IRQ; /* vector # */ for (i = 0, end = size * 8; i < end && startvec + i < s->num_irq; i++) { if (s->vectors[startvec + i].active && @@ -XXX,XX +XXX,XX @@ static MemTxResult nvic_sysreg_write(void *opaque, hwaddr addr, case 0x300 ... 0x33f: /* NVIC Active */ return MEMTX_OK; /* R/O */ case 0x400 ... 0x5ef: /* NVIC Priority */ - startvec = 8 * (offset - 0x400) + NVIC_FIRST_IRQ; /* vector # */ + startvec = (offset - 0x400) + NVIC_FIRST_IRQ; /* vector # */ for (i = 0; i < size && startvec + i < s->num_irq; i++) { if (attrs.secure || s->itns[startvec + i]) { -- 2.16.1 In commit commit 3b2e934463121 we added support for the AIRCR register holding state, but forgot to add it to the vmstate structs. Since it only holds r/w state if the security extension is implemented, we can just add it to vmstate_m_security. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180209165810.6668-10-peter.maydell@linaro.org --- target/arm/machine.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/target/arm/machine.c b/target/arm/machine.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/machine.c +++ b/target/arm/machine.c @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_m_security = { VMSTATE_VALIDATE("SAU_RNR is valid", sau_rnr_vmstate_validate), VMSTATE_UINT32(env.sau.ctrl, ARMCPU), VMSTATE_UINT32(env.v7m.scr[M_REG_S], ARMCPU), + /* AIRCR is not secure-only, but our implementation is R/O if the + * security extension is unimplemented, so we migrate it here. + */ + VMSTATE_UINT32(env.v7m.aircr, ARMCPU), VMSTATE_END_OF_LIST() } }; -- 2.16.1 In commit abc24d86cc0364f we accidentally broke migration of the stack pointer value for the mode (process, handler) the CPU is not currently running as. (The commit correctly removed the no-longer-used v7m.current_sp flag from the VMState but also deleted the still very much in use v7m.other_sp SP value field.) Add a subsection to migrate it again. (We don't need to care about trying to retain compatibility with pre-abc24d86cc0364f versions of QEMU, because that commit bumped the version_id and we've since bumped it again a couple of times.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180209165810.6668-11-peter.maydell@linaro.org --- target/arm/machine.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/target/arm/machine.c b/target/arm/machine.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/machine.c +++ b/target/arm/machine.c @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_m_scr = { } }; +static const VMStateDescription vmstate_m_other_sp = { + .name = "cpu/m/other-sp", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT32(env.v7m.other_sp, ARMCPU), + VMSTATE_END_OF_LIST() + } +}; + static const VMStateDescription vmstate_m = { .name = "cpu/m", .version_id = 4, @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_m = { &vmstate_m_faultmask_primask, &vmstate_m_csselr, &vmstate_m_scr, + &vmstate_m_other_sp, NULL } }; -- 2.16.1 The v8M architecture includes hardware support for enforcing stack pointer limits. We don't implement this behaviour yet, but provide the MSPLIM and PSPLIM stack pointer limit registers as reads-as-written, so that when we do implement the checks in future this won't break guest migration. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180209165810.6668-12-peter.maydell@linaro.org --- target/arm/cpu.h | 2 ++ target/arm/helper.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ target/arm/machine.c | 21 +++++++++++++++++++++ 3 files changed, 69 insertions(+) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ typedef struct CPUARMState { uint32_t secure; /* Is CPU in Secure state? (not guest visible) */ uint32_t csselr[M_REG_NUM_BANKS]; uint32_t scr[M_REG_NUM_BANKS]; + uint32_t msplim[M_REG_NUM_BANKS]; + uint32_t psplim[M_REG_NUM_BANKS]; } v7m; /* Information associated with an exception about to be taken: diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) return 0; } return env->v7m.other_ss_psp; + case 0x8a: /* MSPLIM_NS */ + if (!env->v7m.secure) { + return 0; + } + return env->v7m.msplim[M_REG_NS]; + case 0x8b: /* PSPLIM_NS */ + if (!env->v7m.secure) { + return 0; + } + return env->v7m.psplim[M_REG_NS]; case 0x90: /* PRIMASK_NS */ if (!env->v7m.secure) { return 0; @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) return v7m_using_psp(env) ? env->v7m.other_sp : env->regs[13]; case 9: /* PSP */ return v7m_using_psp(env) ? env->regs[13] : env->v7m.other_sp; + case 10: /* MSPLIM */ + if (!arm_feature(env, ARM_FEATURE_V8)) { + goto bad_reg; + } + return env->v7m.msplim[env->v7m.secure]; + case 11: /* PSPLIM */ + if (!arm_feature(env, ARM_FEATURE_V8)) { + goto bad_reg; + } + return env->v7m.psplim[env->v7m.secure]; case 16: /* PRIMASK */ return env->v7m.primask[env->v7m.secure]; case 17: /* BASEPRI */ @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) case 19: /* FAULTMASK */ return env->v7m.faultmask[env->v7m.secure]; default: + bad_reg: qemu_log_mask(LOG_GUEST_ERROR, "Attempt to read unknown special" " register %d\n", reg); return 0; @@ -XXX,XX +XXX,XX @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val) } env->v7m.other_ss_psp = val; return; + case 0x8a: /* MSPLIM_NS */ + if (!env->v7m.secure) { + return; + } + env->v7m.msplim[M_REG_NS] = val & ~7; + return; + case 0x8b: /* PSPLIM_NS */ + if (!env->v7m.secure) { + return; + } + env->v7m.psplim[M_REG_NS] = val & ~7; + return; case 0x90: /* PRIMASK_NS */ if (!env->v7m.secure) { return; @@ -XXX,XX +XXX,XX @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val) env->v7m.other_sp = val; } break; + case 10: /* MSPLIM */ + if (!arm_feature(env, ARM_FEATURE_V8)) { + goto bad_reg; + } + env->v7m.msplim[env->v7m.secure] = val & ~7; + break; + case 11: /* PSPLIM */ + if (!arm_feature(env, ARM_FEATURE_V8)) { + goto bad_reg; + } + env->v7m.psplim[env->v7m.secure] = val & ~7; + break; case 16: /* PRIMASK */ env->v7m.primask[env->v7m.secure] = val & 1; break; @@ -XXX,XX +XXX,XX @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val) env->v7m.control[env->v7m.secure] |= val & R_V7M_CONTROL_NPRIV_MASK; break; default: + bad_reg: qemu_log_mask(LOG_GUEST_ERROR, "Attempt to write unknown special" " register %d\n", reg); return; diff --git a/target/arm/machine.c b/target/arm/machine.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/machine.c +++ b/target/arm/machine.c @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_m_other_sp = { } }; +static bool m_v8m_needed(void *opaque) +{ + ARMCPU *cpu = opaque; + CPUARMState *env = &cpu->env; + + return arm_feature(env, ARM_FEATURE_M) && arm_feature(env, ARM_FEATURE_V8); +} + +static const VMStateDescription vmstate_m_v8m = { + .name = "cpu/m/v8m", + .version_id = 1, + .minimum_version_id = 1, + .needed = m_v8m_needed, + .fields = (VMStateField[]) { + VMSTATE_UINT32_ARRAY(env.v7m.msplim, ARMCPU, M_REG_NUM_BANKS), + VMSTATE_UINT32_ARRAY(env.v7m.psplim, ARMCPU, M_REG_NUM_BANKS), + VMSTATE_END_OF_LIST() + } +}; + static const VMStateDescription vmstate_m = { .name = "cpu/m", .version_id = 4, @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_m = { &vmstate_m_csselr, &vmstate_m_scr, &vmstate_m_other_sp, + &vmstate_m_v8m, NULL } }; -- 2.16.1 From: Pekka Enberg <penberg@iki.fi> This patch adds a "cpu-type" property to BCM2836 SoC in preparation for reusing the code for the Raspberry Pi 3, which has a different processor model. Signed-off-by: Pekka Enberg <penberg@iki.fi> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/bcm2836.h | 1 + hw/arm/bcm2836.c | 17 +++++++++-------- hw/arm/raspi.c | 3 +++ 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/include/hw/arm/bcm2836.h b/include/hw/arm/bcm2836.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/bcm2836.h +++ b/include/hw/arm/bcm2836.h @@ -XXX,XX +XXX,XX @@ typedef struct BCM2836State { DeviceState parent_obj; /*< public >*/ + char *cpu_type; uint32_t enabled_cpus; ARMCPU cpus[BCM2836_NCPUS]; diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/bcm2836.c +++ b/hw/arm/bcm2836.c @@ -XXX,XX +XXX,XX @@ static void bcm2836_init(Object *obj) { BCM2836State *s = BCM2836(obj); - int n; - - for (n = 0; n < BCM2836_NCPUS; n++) { - object_initialize(&s->cpus[n], sizeof(s->cpus[n]), - "cortex-a15-" TYPE_ARM_CPU); - object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpus[n]), - &error_abort); - } object_initialize(&s->control, sizeof(s->control), TYPE_BCM2836_CONTROL); object_property_add_child(obj, "control", OBJECT(&s->control), NULL); @@ -XXX,XX +XXX,XX @@ static void bcm2836_realize(DeviceState *dev, Error **errp) /* common peripherals from bcm2835 */ + obj = OBJECT(dev); + for (n = 0; n < BCM2836_NCPUS; n++) { + object_initialize(&s->cpus[n], sizeof(s->cpus[n]), + s->cpu_type); + object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpus[n]), + &error_abort); + } + obj = object_property_get_link(OBJECT(dev), "ram", &err); if (obj == NULL) { error_setg(errp, "%s: required ram link not found: %s", @@ -XXX,XX +XXX,XX @@ static void bcm2836_realize(DeviceState *dev, Error **errp) } static Property bcm2836_props[] = { + DEFINE_PROP_STRING("cpu-type", BCM2836State, cpu_type), DEFINE_PROP_UINT32("enabled-cpus", BCM2836State, enabled_cpus, BCM2836_NCPUS), DEFINE_PROP_END_OF_LIST() }; diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/raspi.c +++ b/hw/arm/raspi.c @@ -XXX,XX +XXX,XX @@ static void raspi2_init(MachineState *machine) /* Setup the SOC */ object_property_add_const_link(OBJECT(&s->soc), "ram", OBJECT(&s->ram), &error_abort); + object_property_set_str(OBJECT(&s->soc), machine->cpu_type, "cpu-type", + &error_abort); object_property_set_int(OBJECT(&s->soc), smp_cpus, "enabled-cpus", &error_abort); object_property_set_int(OBJECT(&s->soc), 0xa21041, "board-rev", @@ -XXX,XX +XXX,XX @@ static void raspi2_machine_init(MachineClass *mc) mc->no_parallel = 1; mc->no_floppy = 1; mc->no_cdrom = 1; + mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15"); mc->max_cpus = BCM2836_NCPUS; mc->min_cpus = BCM2836_NCPUS; mc->default_cpus = BCM2836_NCPUS; -- 2.16.1 From: Pekka Enberg <penberg@iki.fi> This patch adds Raspberry Pi 3 support to hw/arm/raspi.c. The differences to Pi 2 are: - Firmware address - Board ID - Board revision The CPU is different too, but that's going to be configured as part of the machine default CPU when we introduce a new machine type. The patch was written from scratch by me but the logic is similar to Zoltán Baldaszti's previous work, which I used as a reference (with permission from the author): https://github.com/bztsrc/qemu-raspi3 Signed-off-by: Pekka Enberg <penberg@iki.fi> [PMM: fixed trailing whitespace on one line] Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/raspi.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/raspi.c +++ b/hw/arm/raspi.c @@ -XXX,XX +XXX,XX @@ * Rasperry Pi 2 emulation Copyright (c) 2015, Microsoft * Written by Andrew Baumann * + * Raspberry Pi 3 emulation Copyright (c) 2018 Zoltán Baldaszti + * Upstream code cleanup (c) 2018 Pekka Enberg + * * This code is licensed under the GNU GPLv2 and later. */ @@ -XXX,XX +XXX,XX @@ #define SMPBOOT_ADDR 0x300 /* this should leave enough space for ATAGS */ #define MVBAR_ADDR 0x400 /* secure vectors */ #define BOARDSETUP_ADDR (MVBAR_ADDR + 0x20) /* board setup code */ -#define FIRMWARE_ADDR 0x8000 /* Pi loads kernel.img here by default */ +#define FIRMWARE_ADDR_2 0x8000 /* Pi 2 loads kernel.img here by default */ +#define FIRMWARE_ADDR_3 0x80000 /* Pi 3 loads kernel.img here by default */ /* Table of Linux board IDs for different Pi versions */ -static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43}; +static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43, [3] = 0xc44}; typedef struct RasPiState { BCM2836State soc; @@ -XXX,XX +XXX,XX @@ static void setup_boot(MachineState *machine, int version, size_t ram_size) binfo.secure_board_setup = true; binfo.secure_boot = true; - /* Pi2 requires SMP setup */ - if (version == 2) { + /* Pi2 and Pi3 requires SMP setup */ + if (version >= 2) { binfo.smp_loader_start = SMPBOOT_ADDR; binfo.write_secondary_boot = write_smpboot; binfo.secondary_cpu_reset_hook = reset_secondary; @@ -XXX,XX +XXX,XX @@ static void setup_boot(MachineState *machine, int version, size_t ram_size) * the normal Linux boot process */ if (machine->firmware) { + hwaddr firmware_addr = version == 3 ? FIRMWARE_ADDR_3 : FIRMWARE_ADDR_2; /* load the firmware image (typically kernel.img) */ - r = load_image_targphys(machine->firmware, FIRMWARE_ADDR, - ram_size - FIRMWARE_ADDR); + r = load_image_targphys(machine->firmware, firmware_addr, + ram_size - firmware_addr); if (r < 0) { error_report("Failed to load firmware from %s", machine->firmware); exit(1); } - binfo.entry = FIRMWARE_ADDR; + binfo.entry = firmware_addr; binfo.firmware_loaded = true; } else { binfo.kernel_filename = machine->kernel_filename; @@ -XXX,XX +XXX,XX @@ static void setup_boot(MachineState *machine, int version, size_t ram_size) arm_load_kernel(ARM_CPU(first_cpu), &binfo); } -static void raspi2_init(MachineState *machine) +static void raspi_init(MachineState *machine, int version) { RasPiState *s = g_new0(RasPiState, 1); uint32_t vcram_size; @@ -XXX,XX +XXX,XX @@ static void raspi2_init(MachineState *machine) &error_abort); object_property_set_int(OBJECT(&s->soc), smp_cpus, "enabled-cpus", &error_abort); - object_property_set_int(OBJECT(&s->soc), 0xa21041, "board-rev", + int board_rev = version == 3 ? 0xa02082 : 0xa21041; + object_property_set_int(OBJECT(&s->soc), board_rev, "board-rev", &error_abort); object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_abort); @@ -XXX,XX +XXX,XX @@ static void raspi2_init(MachineState *machine) vcram_size = object_property_get_uint(OBJECT(&s->soc), "vcram-size", &error_abort); - setup_boot(machine, 2, machine->ram_size - vcram_size); + setup_boot(machine, version, machine->ram_size - vcram_size); +} + +static void raspi2_init(MachineState *machine) +{ + raspi_init(machine, 2); } static void raspi2_machine_init(MachineClass *mc) -- 2.16.1 The following changes since commit b10d00d8811fa4eed4862963273d7353ce310c82: Merge remote-tracking branch 'remotes/kraxel/tags/seabios-20220118-pull-request' into staging (2022-01-19 18:46:28 +0000) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20220120 for you to fetch changes up to 9705e3c1dcff96b0b3c7e594b6cd68d27d6c4ced: hw/intc/arm_gicv3: Check for !MEMTX_OK instead of MEMTX_ERROR (2022-01-20 11:47:54 +0000) ---------------------------------------------------------------- target-arm: * hw/intc/arm_gicv3_its: Fix various minor bugs * hw/arm/aspeed: Add the i3c device to the AST2600 SoC * hw/arm: kudo: add lm75s behind bus 1 switch at 75 * hw/arm/virt: Fix support for running guests on hosts with restricted IPA ranges * hw/intc/arm_gic: Allow reset of the running priority * hw/intc/arm_gic: Implement read of GICC_IIDR * hw/arm/virt: Support for virtio-mem-pci * hw/arm/virt: Support CPU cluster on ARM virt machine * docs/can: convert to restructuredText * hw/net: Move MV88W8618 network device out of hw/arm/ directory * hw/arm/virt: KVM: Enable PAuth when supported by the host ---------------------------------------------------------------- Gavin Shan (2): virtio-mem: Correct default THP size for ARM64 hw/arm/virt: Support for virtio-mem-pci Lucas Ramage (1): docs/can: convert to restructuredText Marc Zyngier (7): hw/arm/virt: KVM: Enable PAuth when supported by the host hw/arm/virt: Add a control for the the highmem PCIe MMIO hw/arm/virt: Add a control for the the highmem redistributors hw/arm/virt: Honor highmem setting when computing the memory map hw/arm/virt: Use the PA range to compute the memory map hw/arm/virt: Disable highmem devices that don't fit in the PA range hw/arm/virt: Drop superfluous checks against highmem Patrick Venture (1): hw/arm: kudo add lm75s behind bus 1 switch at 75 Peter Maydell (13): hw/intc/arm_gicv3_its: Fix event ID bounds checks hw/intc/arm_gicv3_its: Convert int ID check to num_intids convention hw/intc/arm_gicv3_its: Fix handling of process_its_cmd() return value hw/intc/arm_gicv3_its: Don't use data if reading command failed hw/intc/arm_gicv3_its: Use enum for return value of process_* functions hw/intc/arm_gicv3_its: Fix return codes in process_its_cmd() hw/intc/arm_gicv3_its: Refactor process_its_cmd() to reduce nesting hw/intc/arm_gicv3_its: Fix return codes in process_mapti() hw/intc/arm_gicv3_its: Fix return codes in process_mapc() hw/intc/arm_gicv3_its: Fix return codes in process_mapd() hw/intc/arm_gicv3_its: Factor out "find address of table entry" code hw/intc/arm_gicv3_its: Check indexes before use, not after hw/intc/arm_gicv3_its: Range-check ICID before indexing into collection table Petr Pavlu (2): hw/intc/arm_gic: Implement read of GICC_IIDR hw/intc/arm_gic: Allow reset of the running priority Philippe Mathieu-Daudé (4): hw: Move MARVELL_88W8618 Kconfig from audio/ to arm/ hw/arm/musicpal: Fix coding style of code related to MV88W8618 device hw/net: Move MV88W8618 network device out of hw/arm/ directory hw/intc/arm_gicv3: Check for !MEMTX_OK instead of MEMTX_ERROR Troy Lee (2): hw/misc/aspeed_i3c.c: Introduce a dummy AST2600 I3C model. hw/arm/aspeed: Add the i3c device to the AST2600 SoC Yanan Wang (6): hw/arm/virt: Support CPU cluster on ARM virt machine hw/arm/virt: Support cluster level in DT cpu-map hw/acpi/aml-build: Improve scalability of PPTT generation tests/acpi/bios-tables-test: Allow changes to virt/PPTT file hw/acpi/aml-build: Support cluster level in PPTT generation tests/acpi/bios-table-test: Update expected virt/PPTT file docs/system/arm/cpu-features.rst | 4 - docs/system/device-emulation.rst | 1 + docs/{can.txt => system/devices/can.rst} | 90 +++--- include/hw/arm/aspeed_soc.h | 3 + include/hw/arm/virt.h | 5 +- include/hw/misc/aspeed_i3c.h | 48 +++ include/hw/net/mv88w8618_eth.h | 12 + target/arm/cpu.h | 1 + hw/acpi/aml-build.c | 68 +++-- hw/arm/aspeed_ast2600.c | 16 + hw/arm/musicpal.c | 381 +----------------------- hw/arm/npcm7xx_boards.c | 10 +- hw/arm/virt-acpi-build.c | 10 +- hw/arm/virt.c | 184 ++++++++++-- hw/intc/arm_gic.c | 11 + hw/intc/arm_gicv3_its.c | 492 ++++++++++++++----------------- hw/intc/arm_gicv3_redist.c | 4 +- hw/misc/aspeed_i3c.c | 381 ++++++++++++++++++++++++ hw/net/mv88w8618_eth.c | 403 +++++++++++++++++++++++++ hw/virtio/virtio-mem.c | 36 ++- target/arm/cpu.c | 16 +- target/arm/cpu64.c | 31 +- target/arm/kvm64.c | 21 ++ MAINTAINERS | 2 + hw/arm/Kconfig | 4 + hw/audio/Kconfig | 3 - hw/misc/meson.build | 1 + hw/misc/trace-events | 6 + hw/net/meson.build | 1 + qemu-options.hx | 10 + tests/data/acpi/virt/PPTT | Bin 76 -> 96 bytes 31 files changed, 1473 insertions(+), 782 deletions(-) rename docs/{can.txt => system/devices/can.rst} (68%) create mode 100644 include/hw/misc/aspeed_i3c.h create mode 100644 include/hw/net/mv88w8618_eth.h create mode 100644 hw/misc/aspeed_i3c.c create mode 100644 hw/net/mv88w8618_eth.c From: Marc Zyngier <maz@kernel.org> Add basic support for Pointer Authentication when running a KVM guest and that the host supports it, loosely based on the SVE support. Although the feature is enabled by default when the host advertises it, it is possible to disable it by setting the 'pauth=off' CPU property. The 'pauth' comment is removed from cpu-features.rst, as it is now common to both TCG and KVM. Tested on an Apple M1 running 5.16-rc6. Cc: Eric Auger <eric.auger@redhat.com> Cc: Richard Henderson <richard.henderson@linaro.org> Cc: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Andrew Jones <drjones@redhat.com> Signed-off-by: Marc Zyngier <maz@kernel.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220107150154.2490308-1-maz@kernel.org [PMM: fixed indentation] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- docs/system/arm/cpu-features.rst | 4 ---- target/arm/cpu.h | 1 + target/arm/cpu.c | 16 +++++----------- target/arm/cpu64.c | 31 +++++++++++++++++++++++++++---- target/arm/kvm64.c | 21 +++++++++++++++++++++ 5 files changed, 54 insertions(+), 19 deletions(-) diff --git a/docs/system/arm/cpu-features.rst b/docs/system/arm/cpu-features.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/system/arm/cpu-features.rst +++ b/docs/system/arm/cpu-features.rst @@ -XXX,XX +XXX,XX @@ TCG VCPU Features TCG VCPU features are CPU features that are specific to TCG. Below is the list of TCG VCPU features and their descriptions. - pauth Enable or disable ``FEAT_Pauth``, pointer - authentication. By default, the feature is - enabled with ``-cpu max``. - pauth-impdef When ``FEAT_Pauth`` is enabled, either the *impdef* (Implementation Defined) algorithm is enabled or the *architected* QARMA algorithm 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 @@ void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq); void aarch64_sve_change_el(CPUARMState *env, int old_el, int new_el, bool el0_a64); void aarch64_add_sve_properties(Object *obj); +void aarch64_add_pauth_properties(Object *obj); /* * SVE registers are encoded in KVM's memory in an endianness-invariant format. 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 @@ void arm_cpu_finalize_features(ARMCPU *cpu, Error **errp) return; } - /* - * KVM does not support modifications to this feature. - * We have not registered the cpu properties when KVM - * is in use, so the user will not be able to set them. - */ - if (!kvm_enabled()) { - arm_cpu_pauth_finalize(cpu, &local_err); - if (local_err != NULL) { - error_propagate(errp, local_err); - return; - } + arm_cpu_pauth_finalize(cpu, &local_err); + if (local_err != NULL) { + error_propagate(errp, local_err); + return; } } @@ -XXX,XX +XXX,XX @@ static void arm_host_initfn(Object *obj) kvm_arm_set_cpu_features_from_host(cpu); if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { aarch64_add_sve_properties(obj); + aarch64_add_pauth_properties(obj); } #else hvf_arm_set_cpu_features_from_host(cpu); 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 @@ void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp) int arch_val = 0, impdef_val = 0; uint64_t t; + /* Exit early if PAuth is enabled, and fall through to disable it */ + if (kvm_enabled() && cpu->prop_pauth) { + if (!cpu_isar_feature(aa64_pauth, cpu)) { + error_setg(errp, "'pauth' feature not supported by KVM on this host"); + } + + return; + } + /* TODO: Handle HaveEnhancedPAC, HaveEnhancedPAC2, HaveFPAC. */ if (cpu->prop_pauth) { if (cpu->prop_pauth_impdef) { @@ -XXX,XX +XXX,XX @@ static Property arm_cpu_pauth_property = static Property arm_cpu_pauth_impdef_property = DEFINE_PROP_BOOL("pauth-impdef", ARMCPU, prop_pauth_impdef, false); +void aarch64_add_pauth_properties(Object *obj) +{ + ARMCPU *cpu = ARM_CPU(obj); + + /* Default to PAUTH on, with the architected algorithm on TCG. */ + qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_property); + if (kvm_enabled()) { + /* + * Mirror PAuth support from the probed sysregs back into the + * property for KVM. Is it just a bit backward? Yes it is! + */ + cpu->prop_pauth = cpu_isar_feature(aa64_pauth, cpu); + } else { + qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_impdef_property); + } +} + /* -cpu max: if KVM is enabled, like -cpu host (best possible with this host); * otherwise, a CPU with as many features enabled as our emulation supports. * The version of '-cpu max' for qemu-system-arm is defined in cpu.c; @@ -XXX,XX +XXX,XX @@ static void aarch64_max_initfn(Object *obj) cpu->dcz_blocksize = 7; /* 512 bytes */ #endif - /* Default to PAUTH on, with the architected algorithm. */ - qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_property); - qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_impdef_property); - bitmap_fill(cpu->sve_vq_supported, ARM_MAX_VQ); } + aarch64_add_pauth_properties(obj); aarch64_add_sve_properties(obj); object_property_add(obj, "sve-max-vq", "uint32", cpu_max_get_sve_max_vq, cpu_max_set_sve_max_vq, NULL, NULL); diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -XXX,XX +XXX,XX @@ static int read_sys_reg64(int fd, uint64_t *pret, uint64_t id) return ioctl(fd, KVM_GET_ONE_REG, &idreg); } +static bool kvm_arm_pauth_supported(void) +{ + return (kvm_check_extension(kvm_state, KVM_CAP_ARM_PTRAUTH_ADDRESS) && + kvm_check_extension(kvm_state, KVM_CAP_ARM_PTRAUTH_GENERIC)); +} + bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) { /* Identify the feature bits corresponding to the host CPU, and @@ -XXX,XX +XXX,XX @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) */ struct kvm_vcpu_init init = { .target = -1, }; + /* + * Ask for Pointer Authentication if supported. We can't play the + * SVE trick of synthesising the ID reg as KVM won't tell us + * whether we have the architected or IMPDEF version of PAuth, so + * we have to use the actual ID regs. + */ + if (kvm_arm_pauth_supported()) { + init.features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS | + 1 << KVM_ARM_VCPU_PTRAUTH_GENERIC); + } + if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) { return false; } @@ -XXX,XX +XXX,XX @@ int kvm_arch_init_vcpu(CPUState *cs) assert(kvm_arm_sve_supported()); cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE; } + if (cpu_isar_feature(aa64_pauth, cpu)) { + cpu->kvm_init_features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS | + 1 << KVM_ARM_VCPU_PTRAUTH_GENERIC); + } /* Do KVM_ARM_VCPU_INIT ioctl */ ret = kvm_arm_vcpu_init(cs); -- 2.25.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> The Marvell 88W8618 is a system-on-chip with an ARM core. We implement its audio codecs and network interface. Homogeneous SoC Kconfig are usually defined in the hw/$ARCH directory. Move it there. Suggested-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20220107184429.423572-2-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/Kconfig | 3 +++ hw/audio/Kconfig | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) 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 select SPLIT_IRQ select UNIMP +config MARVELL_88W8618 + bool + config MUSICPAL bool select OR_IRQ diff --git a/hw/audio/Kconfig b/hw/audio/Kconfig index XXXXXXX..XXXXXXX 100644 --- a/hw/audio/Kconfig +++ b/hw/audio/Kconfig @@ -XXX,XX +XXX,XX @@ config PL041 config CS4231 bool - -config MARVELL_88W8618 - bool -- 2.25.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> We are going to move this code, so fix its style first to avoid: ERROR: spaces required around that '/' (ctx:VxV) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20220107184429.423572-3-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/musicpal.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 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 uint64_t mv88w8618_eth_read(void *opaque, hwaddr offset, return s->imr; case MP_ETH_FRDP0 ... MP_ETH_FRDP3: - return s->frx_queue[(offset - MP_ETH_FRDP0)/4]; + return s->frx_queue[(offset - MP_ETH_FRDP0) / 4]; case MP_ETH_CRDP0 ... MP_ETH_CRDP3: - return s->rx_queue[(offset - MP_ETH_CRDP0)/4]; + return s->rx_queue[(offset - MP_ETH_CRDP0) / 4]; case MP_ETH_CTDP0 ... MP_ETH_CTDP1: - return s->tx_queue[(offset - MP_ETH_CTDP0)/4]; + return s->tx_queue[(offset - MP_ETH_CTDP0) / 4]; default: return 0; @@ -XXX,XX +XXX,XX @@ static void mv88w8618_eth_write(void *opaque, hwaddr offset, break; case MP_ETH_FRDP0 ... MP_ETH_FRDP3: - s->frx_queue[(offset - MP_ETH_FRDP0)/4] = value; + s->frx_queue[(offset - MP_ETH_FRDP0) / 4] = value; break; case MP_ETH_CRDP0 ... MP_ETH_CRDP3: - s->rx_queue[(offset - MP_ETH_CRDP0)/4] = - s->cur_rx[(offset - MP_ETH_CRDP0)/4] = value; + s->rx_queue[(offset - MP_ETH_CRDP0) / 4] = + s->cur_rx[(offset - MP_ETH_CRDP0) / 4] = value; break; case MP_ETH_CTDP0 ... MP_ETH_CTDP1: - s->tx_queue[(offset - MP_ETH_CTDP0)/4] = value; + s->tx_queue[(offset - MP_ETH_CTDP0) / 4] = value; break; } } -- 2.25.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> The Marvell 88W8618 network device is hidden in the Musicpal machine. Move it into a new unit file under the hw/net/ directory. Acked-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20220107184429.423572-4-f4bug@amsat.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/net/mv88w8618_eth.h | 12 + hw/arm/musicpal.c | 381 +------------------------------ hw/net/mv88w8618_eth.c | 403 +++++++++++++++++++++++++++++++++ MAINTAINERS | 2 + hw/net/meson.build | 1 + 5 files changed, 419 insertions(+), 380 deletions(-) create mode 100644 include/hw/net/mv88w8618_eth.h create mode 100644 hw/net/mv88w8618_eth.c diff --git a/include/hw/net/mv88w8618_eth.h b/include/hw/net/mv88w8618_eth.h new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/include/hw/net/mv88w8618_eth.h @@ -XXX,XX +XXX,XX @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Marvell MV88W8618 / Freecom MusicPal emulation. + * + * Copyright (c) 2008-2021 QEMU contributors + */ +#ifndef HW_NET_MV88W8618_H +#define HW_NET_MV88W8618_H + +#define TYPE_MV88W8618_ETH "mv88w8618_eth" + +#endif 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/pixel_ops.h" #include "qemu/cutils.h" #include "qom/object.h" +#include "hw/net/mv88w8618_eth.h" #define MP_MISC_BASE 0x80002000 #define MP_MISC_SIZE 0x00001000 #define MP_ETH_BASE 0x80008000 -#define MP_ETH_SIZE 0x00001000 #define MP_WLAN_BASE 0x8000C000 #define MP_WLAN_SIZE 0x00000800 @@ -XXX,XX +XXX,XX @@ /* Wolfson 8750 I2C address */ #define MP_WM_ADDR 0x1A -/* Ethernet register offsets */ -#define MP_ETH_SMIR 0x010 -#define MP_ETH_PCXR 0x408 -#define MP_ETH_SDCMR 0x448 -#define MP_ETH_ICR 0x450 -#define MP_ETH_IMR 0x458 -#define MP_ETH_FRDP0 0x480 -#define MP_ETH_FRDP1 0x484 -#define MP_ETH_FRDP2 0x488 -#define MP_ETH_FRDP3 0x48C -#define MP_ETH_CRDP0 0x4A0 -#define MP_ETH_CRDP1 0x4A4 -#define MP_ETH_CRDP2 0x4A8 -#define MP_ETH_CRDP3 0x4AC -#define MP_ETH_CTDP0 0x4E0 -#define MP_ETH_CTDP1 0x4E4 - -/* MII PHY access */ -#define MP_ETH_SMIR_DATA 0x0000FFFF -#define MP_ETH_SMIR_ADDR 0x03FF0000 -#define MP_ETH_SMIR_OPCODE (1 << 26) /* Read value */ -#define MP_ETH_SMIR_RDVALID (1 << 27) - -/* PHY registers */ -#define MP_ETH_PHY1_BMSR 0x00210000 -#define MP_ETH_PHY1_PHYSID1 0x00410000 -#define MP_ETH_PHY1_PHYSID2 0x00610000 - -#define MP_PHY_BMSR_LINK 0x0004 -#define MP_PHY_BMSR_AUTONEG 0x0008 - -#define MP_PHY_88E3015 0x01410E20 - -/* TX descriptor status */ -#define MP_ETH_TX_OWN (1U << 31) - -/* RX descriptor status */ -#define MP_ETH_RX_OWN (1U << 31) - -/* Interrupt cause/mask bits */ -#define MP_ETH_IRQ_RX_BIT 0 -#define MP_ETH_IRQ_RX (1 << MP_ETH_IRQ_RX_BIT) -#define MP_ETH_IRQ_TXHI_BIT 2 -#define MP_ETH_IRQ_TXLO_BIT 3 - -/* Port config bits */ -#define MP_ETH_PCXR_2BSM_BIT 28 /* 2-byte incoming suffix */ - -/* SDMA command bits */ -#define MP_ETH_CMD_TXHI (1 << 23) -#define MP_ETH_CMD_TXLO (1 << 22) - -typedef struct mv88w8618_tx_desc { - uint32_t cmdstat; - uint16_t res; - uint16_t bytes; - uint32_t buffer; - uint32_t next; -} mv88w8618_tx_desc; - -typedef struct mv88w8618_rx_desc { - uint32_t cmdstat; - uint16_t bytes; - uint16_t buffer_size; - uint32_t buffer; - uint32_t next; -} mv88w8618_rx_desc; - -#define TYPE_MV88W8618_ETH "mv88w8618_eth" -OBJECT_DECLARE_SIMPLE_TYPE(mv88w8618_eth_state, MV88W8618_ETH) - -struct mv88w8618_eth_state { - /*< private >*/ - SysBusDevice parent_obj; - /*< public >*/ - - MemoryRegion iomem; - qemu_irq irq; - MemoryRegion *dma_mr; - AddressSpace dma_as; - uint32_t smir; - uint32_t icr; - uint32_t imr; - int mmio_index; - uint32_t vlan_header; - uint32_t tx_queue[2]; - uint32_t rx_queue[4]; - uint32_t frx_queue[4]; - uint32_t cur_rx[4]; - NICState *nic; - NICConf conf; -}; - -static void eth_rx_desc_put(AddressSpace *dma_as, uint32_t addr, - mv88w8618_rx_desc *desc) -{ - cpu_to_le32s(&desc->cmdstat); - cpu_to_le16s(&desc->bytes); - cpu_to_le16s(&desc->buffer_size); - cpu_to_le32s(&desc->buffer); - cpu_to_le32s(&desc->next); - dma_memory_write(dma_as, addr, desc, sizeof(*desc), MEMTXATTRS_UNSPECIFIED); -} - -static void eth_rx_desc_get(AddressSpace *dma_as, uint32_t addr, - mv88w8618_rx_desc *desc) -{ - dma_memory_read(dma_as, addr, desc, sizeof(*desc), MEMTXATTRS_UNSPECIFIED); - le32_to_cpus(&desc->cmdstat); - le16_to_cpus(&desc->bytes); - le16_to_cpus(&desc->buffer_size); - le32_to_cpus(&desc->buffer); - le32_to_cpus(&desc->next); -} - -static ssize_t eth_receive(NetClientState *nc, const uint8_t *buf, size_t size) -{ - mv88w8618_eth_state *s = qemu_get_nic_opaque(nc); - uint32_t desc_addr; - mv88w8618_rx_desc desc; - int i; - - for (i = 0; i < 4; i++) { - desc_addr = s->cur_rx[i]; - if (!desc_addr) { - continue; - } - do { - eth_rx_desc_get(&s->dma_as, desc_addr, &desc); - if ((desc.cmdstat & MP_ETH_RX_OWN) && desc.buffer_size >= size) { - dma_memory_write(&s->dma_as, desc.buffer + s->vlan_header, - buf, size, MEMTXATTRS_UNSPECIFIED); - desc.bytes = size + s->vlan_header; - desc.cmdstat &= ~MP_ETH_RX_OWN; - s->cur_rx[i] = desc.next; - - s->icr |= MP_ETH_IRQ_RX; - if (s->icr & s->imr) { - qemu_irq_raise(s->irq); - } - eth_rx_desc_put(&s->dma_as, desc_addr, &desc); - return size; - } - desc_addr = desc.next; - } while (desc_addr != s->rx_queue[i]); - } - return size; -} - -static void eth_tx_desc_put(AddressSpace *dma_as, uint32_t addr, - mv88w8618_tx_desc *desc) -{ - cpu_to_le32s(&desc->cmdstat); - cpu_to_le16s(&desc->res); - cpu_to_le16s(&desc->bytes); - cpu_to_le32s(&desc->buffer); - cpu_to_le32s(&desc->next); - dma_memory_write(dma_as, addr, desc, sizeof(*desc), MEMTXATTRS_UNSPECIFIED); -} - -static void eth_tx_desc_get(AddressSpace *dma_as, uint32_t addr, - mv88w8618_tx_desc *desc) -{ - dma_memory_read(dma_as, addr, desc, sizeof(*desc), MEMTXATTRS_UNSPECIFIED); - le32_to_cpus(&desc->cmdstat); - le16_to_cpus(&desc->res); - le16_to_cpus(&desc->bytes); - le32_to_cpus(&desc->buffer); - le32_to_cpus(&desc->next); -} - -static void eth_send(mv88w8618_eth_state *s, int queue_index) -{ - uint32_t desc_addr = s->tx_queue[queue_index]; - mv88w8618_tx_desc desc; - uint32_t next_desc; - uint8_t buf[2048]; - int len; - - do { - eth_tx_desc_get(&s->dma_as, desc_addr, &desc); - next_desc = desc.next; - if (desc.cmdstat & MP_ETH_TX_OWN) { - len = desc.bytes; - if (len < 2048) { - dma_memory_read(&s->dma_as, desc.buffer, buf, len, - MEMTXATTRS_UNSPECIFIED); - qemu_send_packet(qemu_get_queue(s->nic), buf, len); - } - desc.cmdstat &= ~MP_ETH_TX_OWN; - s->icr |= 1 << (MP_ETH_IRQ_TXLO_BIT - queue_index); - eth_tx_desc_put(&s->dma_as, desc_addr, &desc); - } - desc_addr = next_desc; - } while (desc_addr != s->tx_queue[queue_index]); -} - -static uint64_t mv88w8618_eth_read(void *opaque, hwaddr offset, - unsigned size) -{ - mv88w8618_eth_state *s = opaque; - - switch (offset) { - case MP_ETH_SMIR: - if (s->smir & MP_ETH_SMIR_OPCODE) { - switch (s->smir & MP_ETH_SMIR_ADDR) { - case MP_ETH_PHY1_BMSR: - return MP_PHY_BMSR_LINK | MP_PHY_BMSR_AUTONEG | - MP_ETH_SMIR_RDVALID; - case MP_ETH_PHY1_PHYSID1: - return (MP_PHY_88E3015 >> 16) | MP_ETH_SMIR_RDVALID; - case MP_ETH_PHY1_PHYSID2: - return (MP_PHY_88E3015 & 0xFFFF) | MP_ETH_SMIR_RDVALID; - default: - return MP_ETH_SMIR_RDVALID; - } - } - return 0; - - case MP_ETH_ICR: - return s->icr; - - case MP_ETH_IMR: - return s->imr; - - case MP_ETH_FRDP0 ... MP_ETH_FRDP3: - return s->frx_queue[(offset - MP_ETH_FRDP0) / 4]; - - case MP_ETH_CRDP0 ... MP_ETH_CRDP3: - return s->rx_queue[(offset - MP_ETH_CRDP0) / 4]; - - case MP_ETH_CTDP0 ... MP_ETH_CTDP1: - return s->tx_queue[(offset - MP_ETH_CTDP0) / 4]; - - default: - return 0; - } -} - -static void mv88w8618_eth_write(void *opaque, hwaddr offset, - uint64_t value, unsigned size) -{ - mv88w8618_eth_state *s = opaque; - - switch (offset) { - case MP_ETH_SMIR: - s->smir = value; - break; - - case MP_ETH_PCXR: - s->vlan_header = ((value >> MP_ETH_PCXR_2BSM_BIT) & 1) * 2; - break; - - case MP_ETH_SDCMR: - if (value & MP_ETH_CMD_TXHI) { - eth_send(s, 1); - } - if (value & MP_ETH_CMD_TXLO) { - eth_send(s, 0); - } - if (value & (MP_ETH_CMD_TXHI | MP_ETH_CMD_TXLO) && s->icr & s->imr) { - qemu_irq_raise(s->irq); - } - break; - - case MP_ETH_ICR: - s->icr &= value; - break; - - case MP_ETH_IMR: - s->imr = value; - if (s->icr & s->imr) { - qemu_irq_raise(s->irq); - } - break; - - case MP_ETH_FRDP0 ... MP_ETH_FRDP3: - s->frx_queue[(offset - MP_ETH_FRDP0) / 4] = value; - break; - - case MP_ETH_CRDP0 ... MP_ETH_CRDP3: - s->rx_queue[(offset - MP_ETH_CRDP0) / 4] = - s->cur_rx[(offset - MP_ETH_CRDP0) / 4] = value; - break; - - case MP_ETH_CTDP0 ... MP_ETH_CTDP1: - s->tx_queue[(offset - MP_ETH_CTDP0) / 4] = value; - break; - } -} - -static const MemoryRegionOps mv88w8618_eth_ops = { - .read = mv88w8618_eth_read, - .write = mv88w8618_eth_write, - .endianness = DEVICE_NATIVE_ENDIAN, -}; - -static void eth_cleanup(NetClientState *nc) -{ - mv88w8618_eth_state *s = qemu_get_nic_opaque(nc); - - s->nic = NULL; -} - -static NetClientInfo net_mv88w8618_info = { - .type = NET_CLIENT_DRIVER_NIC, - .size = sizeof(NICState), - .receive = eth_receive, - .cleanup = eth_cleanup, -}; - -static void mv88w8618_eth_init(Object *obj) -{ - SysBusDevice *sbd = SYS_BUS_DEVICE(obj); - DeviceState *dev = DEVICE(sbd); - mv88w8618_eth_state *s = MV88W8618_ETH(dev); - - sysbus_init_irq(sbd, &s->irq); - memory_region_init_io(&s->iomem, obj, &mv88w8618_eth_ops, s, - "mv88w8618-eth", MP_ETH_SIZE); - sysbus_init_mmio(sbd, &s->iomem); -} - -static void mv88w8618_eth_realize(DeviceState *dev, Error **errp) -{ - mv88w8618_eth_state *s = MV88W8618_ETH(dev); - - if (!s->dma_mr) { - error_setg(errp, TYPE_MV88W8618_ETH " 'dma-memory' link not set"); - return; - } - - address_space_init(&s->dma_as, s->dma_mr, "emac-dma"); - s->nic = qemu_new_nic(&net_mv88w8618_info, &s->conf, - object_get_typename(OBJECT(dev)), dev->id, s); -} - -static const VMStateDescription mv88w8618_eth_vmsd = { - .name = "mv88w8618_eth", - .version_id = 1, - .minimum_version_id = 1, - .fields = (VMStateField[]) { - VMSTATE_UINT32(smir, mv88w8618_eth_state), - VMSTATE_UINT32(icr, mv88w8618_eth_state), - VMSTATE_UINT32(imr, mv88w8618_eth_state), - VMSTATE_UINT32(vlan_header, mv88w8618_eth_state), - VMSTATE_UINT32_ARRAY(tx_queue, mv88w8618_eth_state, 2), - VMSTATE_UINT32_ARRAY(rx_queue, mv88w8618_eth_state, 4), - VMSTATE_UINT32_ARRAY(frx_queue, mv88w8618_eth_state, 4), - VMSTATE_UINT32_ARRAY(cur_rx, mv88w8618_eth_state, 4), - VMSTATE_END_OF_LIST() - } -}; - -static Property mv88w8618_eth_properties[] = { - DEFINE_NIC_PROPERTIES(mv88w8618_eth_state, conf), - DEFINE_PROP_LINK("dma-memory", mv88w8618_eth_state, dma_mr, - TYPE_MEMORY_REGION, MemoryRegion *), - DEFINE_PROP_END_OF_LIST(), -}; - -static void mv88w8618_eth_class_init(ObjectClass *klass, void *data) -{ - DeviceClass *dc = DEVICE_CLASS(klass); - - dc->vmsd = &mv88w8618_eth_vmsd; - device_class_set_props(dc, mv88w8618_eth_properties); - dc->realize = mv88w8618_eth_realize; -} - -static const TypeInfo mv88w8618_eth_info = { - .name = TYPE_MV88W8618_ETH, - .parent = TYPE_SYS_BUS_DEVICE, - .instance_size = sizeof(mv88w8618_eth_state), - .instance_init = mv88w8618_eth_init, - .class_init = mv88w8618_eth_class_init, -}; - /* LCD register offsets */ #define MP_LCD_IRQCTRL 0x180 #define MP_LCD_IRQSTAT 0x184 @@ -XXX,XX +XXX,XX @@ static void musicpal_register_types(void) type_register_static(&mv88w8618_pic_info); type_register_static(&mv88w8618_pit_info); type_register_static(&mv88w8618_flashcfg_info); - type_register_static(&mv88w8618_eth_info); type_register_static(&mv88w8618_wlan_info); type_register_static(&musicpal_lcd_info); type_register_static(&musicpal_gpio_info); diff --git a/hw/net/mv88w8618_eth.c b/hw/net/mv88w8618_eth.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/hw/net/mv88w8618_eth.c @@ -XXX,XX +XXX,XX @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Marvell MV88W8618 / Freecom MusicPal emulation. + * + * Copyright (c) 2008 Jan Kiszka + */ + +#include "qemu/osdep.h" +#include "qapi/error.h" +#include "hw/qdev-properties.h" +#include "hw/sysbus.h" +#include "hw/irq.h" +#include "hw/net/mv88w8618_eth.h" +#include "migration/vmstate.h" +#include "sysemu/dma.h" +#include "net/net.h" + +#define MP_ETH_SIZE 0x00001000 + +/* Ethernet register offsets */ +#define MP_ETH_SMIR 0x010 +#define MP_ETH_PCXR 0x408 +#define MP_ETH_SDCMR 0x448 +#define MP_ETH_ICR 0x450 +#define MP_ETH_IMR 0x458 +#define MP_ETH_FRDP0 0x480 +#define MP_ETH_FRDP1 0x484 +#define MP_ETH_FRDP2 0x488 +#define MP_ETH_FRDP3 0x48C +#define MP_ETH_CRDP0 0x4A0 +#define MP_ETH_CRDP1 0x4A4 +#define MP_ETH_CRDP2 0x4A8 +#define MP_ETH_CRDP3 0x4AC +#define MP_ETH_CTDP0 0x4E0 +#define MP_ETH_CTDP1 0x4E4 + +/* MII PHY access */ +#define MP_ETH_SMIR_DATA 0x0000FFFF +#define MP_ETH_SMIR_ADDR 0x03FF0000 +#define MP_ETH_SMIR_OPCODE (1 << 26) /* Read value */ +#define MP_ETH_SMIR_RDVALID (1 << 27) + +/* PHY registers */ +#define MP_ETH_PHY1_BMSR 0x00210000 +#define MP_ETH_PHY1_PHYSID1 0x00410000 +#define MP_ETH_PHY1_PHYSID2 0x00610000 + +#define MP_PHY_BMSR_LINK 0x0004 +#define MP_PHY_BMSR_AUTONEG 0x0008 + +#define MP_PHY_88E3015 0x01410E20 + +/* TX descriptor status */ +#define MP_ETH_TX_OWN (1U << 31) + +/* RX descriptor status */ +#define MP_ETH_RX_OWN (1U << 31) + +/* Interrupt cause/mask bits */ +#define MP_ETH_IRQ_RX_BIT 0 +#define MP_ETH_IRQ_RX (1 << MP_ETH_IRQ_RX_BIT) +#define MP_ETH_IRQ_TXHI_BIT 2 +#define MP_ETH_IRQ_TXLO_BIT 3 + +/* Port config bits */ +#define MP_ETH_PCXR_2BSM_BIT 28 /* 2-byte incoming suffix */ + +/* SDMA command bits */ +#define MP_ETH_CMD_TXHI (1 << 23) +#define MP_ETH_CMD_TXLO (1 << 22) + +typedef struct mv88w8618_tx_desc { + uint32_t cmdstat; + uint16_t res; + uint16_t bytes; + uint32_t buffer; + uint32_t next; +} mv88w8618_tx_desc; + +typedef struct mv88w8618_rx_desc { + uint32_t cmdstat; + uint16_t bytes; + uint16_t buffer_size; + uint32_t buffer; + uint32_t next; +} mv88w8618_rx_desc; + +OBJECT_DECLARE_SIMPLE_TYPE(mv88w8618_eth_state, MV88W8618_ETH) + +struct mv88w8618_eth_state { + /*< private >*/ + SysBusDevice parent_obj; + /*< public >*/ + + MemoryRegion iomem; + qemu_irq irq; + MemoryRegion *dma_mr; + AddressSpace dma_as; + uint32_t smir; + uint32_t icr; + uint32_t imr; + int mmio_index; + uint32_t vlan_header; + uint32_t tx_queue[2]; + uint32_t rx_queue[4]; + uint32_t frx_queue[4]; + uint32_t cur_rx[4]; + NICState *nic; + NICConf conf; +}; + +static void eth_rx_desc_put(AddressSpace *dma_as, uint32_t addr, + mv88w8618_rx_desc *desc) +{ + cpu_to_le32s(&desc->cmdstat); + cpu_to_le16s(&desc->bytes); + cpu_to_le16s(&desc->buffer_size); + cpu_to_le32s(&desc->buffer); + cpu_to_le32s(&desc->next); + dma_memory_write(dma_as, addr, desc, sizeof(*desc), MEMTXATTRS_UNSPECIFIED); +} + +static void eth_rx_desc_get(AddressSpace *dma_as, uint32_t addr, + mv88w8618_rx_desc *desc) +{ + dma_memory_read(dma_as, addr, desc, sizeof(*desc), MEMTXATTRS_UNSPECIFIED); + le32_to_cpus(&desc->cmdstat); + le16_to_cpus(&desc->bytes); + le16_to_cpus(&desc->buffer_size); + le32_to_cpus(&desc->buffer); + le32_to_cpus(&desc->next); +} + +static ssize_t eth_receive(NetClientState *nc, const uint8_t *buf, size_t size) +{ + mv88w8618_eth_state *s = qemu_get_nic_opaque(nc); + uint32_t desc_addr; + mv88w8618_rx_desc desc; + int i; + + for (i = 0; i < 4; i++) { + desc_addr = s->cur_rx[i]; + if (!desc_addr) { + continue; + } + do { + eth_rx_desc_get(&s->dma_as, desc_addr, &desc); + if ((desc.cmdstat & MP_ETH_RX_OWN) && desc.buffer_size >= size) { + dma_memory_write(&s->dma_as, desc.buffer + s->vlan_header, + buf, size, MEMTXATTRS_UNSPECIFIED); + desc.bytes = size + s->vlan_header; + desc.cmdstat &= ~MP_ETH_RX_OWN; + s->cur_rx[i] = desc.next; + + s->icr |= MP_ETH_IRQ_RX; + if (s->icr & s->imr) { + qemu_irq_raise(s->irq); + } + eth_rx_desc_put(&s->dma_as, desc_addr, &desc); + return size; + } + desc_addr = desc.next; + } while (desc_addr != s->rx_queue[i]); + } + return size; +} + +static void eth_tx_desc_put(AddressSpace *dma_as, uint32_t addr, + mv88w8618_tx_desc *desc) +{ + cpu_to_le32s(&desc->cmdstat); + cpu_to_le16s(&desc->res); + cpu_to_le16s(&desc->bytes); + cpu_to_le32s(&desc->buffer); + cpu_to_le32s(&desc->next); + dma_memory_write(dma_as, addr, desc, sizeof(*desc), MEMTXATTRS_UNSPECIFIED); +} + +static void eth_tx_desc_get(AddressSpace *dma_as, uint32_t addr, + mv88w8618_tx_desc *desc) +{ + dma_memory_read(dma_as, addr, desc, sizeof(*desc), MEMTXATTRS_UNSPECIFIED); + le32_to_cpus(&desc->cmdstat); + le16_to_cpus(&desc->res); + le16_to_cpus(&desc->bytes); + le32_to_cpus(&desc->buffer); + le32_to_cpus(&desc->next); +} + +static void eth_send(mv88w8618_eth_state *s, int queue_index) +{ + uint32_t desc_addr = s->tx_queue[queue_index]; + mv88w8618_tx_desc desc; + uint32_t next_desc; + uint8_t buf[2048]; + int len; + + do { + eth_tx_desc_get(&s->dma_as, desc_addr, &desc); + next_desc = desc.next; + if (desc.cmdstat & MP_ETH_TX_OWN) { + len = desc.bytes; + if (len < 2048) { + dma_memory_read(&s->dma_as, desc.buffer, buf, len, + MEMTXATTRS_UNSPECIFIED); + qemu_send_packet(qemu_get_queue(s->nic), buf, len); + } + desc.cmdstat &= ~MP_ETH_TX_OWN; + s->icr |= 1 << (MP_ETH_IRQ_TXLO_BIT - queue_index); + eth_tx_desc_put(&s->dma_as, desc_addr, &desc); + } + desc_addr = next_desc; + } while (desc_addr != s->tx_queue[queue_index]); +} + +static uint64_t mv88w8618_eth_read(void *opaque, hwaddr offset, + unsigned size) +{ + mv88w8618_eth_state *s = opaque; + + switch (offset) { + case MP_ETH_SMIR: + if (s->smir & MP_ETH_SMIR_OPCODE) { + switch (s->smir & MP_ETH_SMIR_ADDR) { + case MP_ETH_PHY1_BMSR: + return MP_PHY_BMSR_LINK | MP_PHY_BMSR_AUTONEG | + MP_ETH_SMIR_RDVALID; + case MP_ETH_PHY1_PHYSID1: + return (MP_PHY_88E3015 >> 16) | MP_ETH_SMIR_RDVALID; + case MP_ETH_PHY1_PHYSID2: + return (MP_PHY_88E3015 & 0xFFFF) | MP_ETH_SMIR_RDVALID; + default: + return MP_ETH_SMIR_RDVALID; + } + } + return 0; + + case MP_ETH_ICR: + return s->icr; + + case MP_ETH_IMR: + return s->imr; + + case MP_ETH_FRDP0 ... MP_ETH_FRDP3: + return s->frx_queue[(offset - MP_ETH_FRDP0) / 4]; + + case MP_ETH_CRDP0 ... MP_ETH_CRDP3: + return s->rx_queue[(offset - MP_ETH_CRDP0) / 4]; + + case MP_ETH_CTDP0 ... MP_ETH_CTDP1: + return s->tx_queue[(offset - MP_ETH_CTDP0) / 4]; + + default: + return 0; + } +} + +static void mv88w8618_eth_write(void *opaque, hwaddr offset, + uint64_t value, unsigned size) +{ + mv88w8618_eth_state *s = opaque; + + switch (offset) { + case MP_ETH_SMIR: + s->smir = value; + break; + + case MP_ETH_PCXR: + s->vlan_header = ((value >> MP_ETH_PCXR_2BSM_BIT) & 1) * 2; + break; + + case MP_ETH_SDCMR: + if (value & MP_ETH_CMD_TXHI) { + eth_send(s, 1); + } + if (value & MP_ETH_CMD_TXLO) { + eth_send(s, 0); + } + if (value & (MP_ETH_CMD_TXHI | MP_ETH_CMD_TXLO) && s->icr & s->imr) { + qemu_irq_raise(s->irq); + } + break; + + case MP_ETH_ICR: + s->icr &= value; + break; + + case MP_ETH_IMR: + s->imr = value; + if (s->icr & s->imr) { + qemu_irq_raise(s->irq); + } + break; + + case MP_ETH_FRDP0 ... MP_ETH_FRDP3: + s->frx_queue[(offset - MP_ETH_FRDP0) / 4] = value; + break; + + case MP_ETH_CRDP0 ... MP_ETH_CRDP3: + s->rx_queue[(offset - MP_ETH_CRDP0) / 4] = + s->cur_rx[(offset - MP_ETH_CRDP0) / 4] = value; + break; + + case MP_ETH_CTDP0 ... MP_ETH_CTDP1: + s->tx_queue[(offset - MP_ETH_CTDP0) / 4] = value; + break; + } +} + +static const MemoryRegionOps mv88w8618_eth_ops = { + .read = mv88w8618_eth_read, + .write = mv88w8618_eth_write, + .endianness = DEVICE_NATIVE_ENDIAN, +}; + +static void eth_cleanup(NetClientState *nc) +{ + mv88w8618_eth_state *s = qemu_get_nic_opaque(nc); + + s->nic = NULL; +} + +static NetClientInfo net_mv88w8618_info = { + .type = NET_CLIENT_DRIVER_NIC, + .size = sizeof(NICState), + .receive = eth_receive, + .cleanup = eth_cleanup, +}; + +static void mv88w8618_eth_init(Object *obj) +{ + SysBusDevice *sbd = SYS_BUS_DEVICE(obj); + DeviceState *dev = DEVICE(sbd); + mv88w8618_eth_state *s = MV88W8618_ETH(dev); + + sysbus_init_irq(sbd, &s->irq); + memory_region_init_io(&s->iomem, obj, &mv88w8618_eth_ops, s, + "mv88w8618-eth", MP_ETH_SIZE); + sysbus_init_mmio(sbd, &s->iomem); +} + +static void mv88w8618_eth_realize(DeviceState *dev, Error **errp) +{ + mv88w8618_eth_state *s = MV88W8618_ETH(dev); + + if (!s->dma_mr) { + error_setg(errp, TYPE_MV88W8618_ETH " 'dma-memory' link not set"); + return; + } + + address_space_init(&s->dma_as, s->dma_mr, "emac-dma"); + s->nic = qemu_new_nic(&net_mv88w8618_info, &s->conf, + object_get_typename(OBJECT(dev)), dev->id, s); +} + +static const VMStateDescription mv88w8618_eth_vmsd = { + .name = "mv88w8618_eth", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT32(smir, mv88w8618_eth_state), + VMSTATE_UINT32(icr, mv88w8618_eth_state), + VMSTATE_UINT32(imr, mv88w8618_eth_state), + VMSTATE_UINT32(vlan_header, mv88w8618_eth_state), + VMSTATE_UINT32_ARRAY(tx_queue, mv88w8618_eth_state, 2), + VMSTATE_UINT32_ARRAY(rx_queue, mv88w8618_eth_state, 4), + VMSTATE_UINT32_ARRAY(frx_queue, mv88w8618_eth_state, 4), + VMSTATE_UINT32_ARRAY(cur_rx, mv88w8618_eth_state, 4), + VMSTATE_END_OF_LIST() + } +}; + +static Property mv88w8618_eth_properties[] = { + DEFINE_NIC_PROPERTIES(mv88w8618_eth_state, conf), + DEFINE_PROP_LINK("dma-memory", mv88w8618_eth_state, dma_mr, + TYPE_MEMORY_REGION, MemoryRegion *), + DEFINE_PROP_END_OF_LIST(), +}; + +static void mv88w8618_eth_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + + dc->vmsd = &mv88w8618_eth_vmsd; + device_class_set_props(dc, mv88w8618_eth_properties); + dc->realize = mv88w8618_eth_realize; +} + +static const TypeInfo mv88w8618_eth_info = { + .name = TYPE_MV88W8618_ETH, + .parent = TYPE_SYS_BUS_DEVICE, + .instance_size = sizeof(mv88w8618_eth_state), + .instance_init = mv88w8618_eth_init, + .class_init = mv88w8618_eth_class_init, +}; + +static void musicpal_register_types(void) +{ + type_register_static(&mv88w8618_eth_info); +} + +type_init(musicpal_register_types) + diff --git a/MAINTAINERS b/MAINTAINERS index XXXXXXX..XXXXXXX 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -XXX,XX +XXX,XX @@ M: Peter Maydell <peter.maydell@linaro.org> L: qemu-arm@nongnu.org S: Odd Fixes F: hw/arm/musicpal.c +F: hw/net/mv88w8618_eth.c +F: include/hw/net/mv88w8618_eth.h F: docs/system/arm/musicpal.rst Nuvoton NPCM7xx diff --git a/hw/net/meson.build b/hw/net/meson.build index XXXXXXX..XXXXXXX 100644 --- a/hw/net/meson.build +++ b/hw/net/meson.build @@ -XXX,XX +XXX,XX @@ softmmu_ss.add(when: 'CONFIG_ALLWINNER_EMAC', if_true: files('allwinner_emac.c') softmmu_ss.add(when: 'CONFIG_ALLWINNER_SUN8I_EMAC', if_true: files('allwinner-sun8i-emac.c')) softmmu_ss.add(when: 'CONFIG_IMX_FEC', if_true: files('imx_fec.c')) softmmu_ss.add(when: 'CONFIG_MSF2', if_true: files('msf2-emac.c')) +softmmu_ss.add(when: 'CONFIG_MARVELL_88W8618', if_true: files('mv88w8618_eth.c')) softmmu_ss.add(when: 'CONFIG_CADENCE', if_true: files('cadence_gem.c')) softmmu_ss.add(when: 'CONFIG_STELLARIS_ENET', if_true: files('stellaris_enet.c')) -- 2.25.1 From: Yanan Wang <wangyanan55@huawei.com> ARM64 machines like Kunpeng Family Server Chips have a level of hardware topology in which a group of CPU cores share L3 cache tag or L2 cache. For example, Kunpeng 920 typically has 6 or 8 clusters in each NUMA node (also represent range of CPU die), and each cluster has 4 CPU cores. All clusters share L3 cache data, but CPU cores in each cluster share a local L3 tag. Running a guest kernel with Cluster-Aware Scheduling on the Hosts which have physical clusters, if we can design a vCPU topology with cluster level for guest kernel and then have a dedicated vCPU pinning, the guest will gain scheduling performance improvement from cache affinity of CPU cluster. So let's enable the support for this new parameter on ARM virt machines. After this patch, we can define a 4-level CPU hierarchy like: cpus=*,maxcpus=*,sockets=*,clusters=*, cores=*,threads=*. Signed-off-by: Yanan Wang <wangyanan55@huawei.com> Reviewed-by: Andrew Jones <drjones@redhat.com> Message-id: 20220107083232.16256-2-wangyanan55@huawei.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt.c | 1 + qemu-options.hx | 10 ++++++++++ 2 files changed, 11 insertions(+) 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 virt_machine_class_init(ObjectClass *oc, void *data) hc->unplug_request = virt_machine_device_unplug_request_cb; hc->unplug = virt_machine_device_unplug_cb; mc->nvdimm_supported = true; + mc->smp_props.clusters_supported = true; mc->auto_enable_numa_with_memhp = true; mc->auto_enable_numa_with_memdev = true; mc->default_ram_id = "mach-virt.ram"; diff --git a/qemu-options.hx b/qemu-options.hx index XXXXXXX..XXXXXXX 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -XXX,XX +XXX,XX @@ SRST -smp 16,sockets=2,dies=2,cores=2,threads=2,maxcpus=16 + The following sub-option defines a CPU topology hierarchy (2 sockets + totally on the machine, 2 clusters per socket, 2 cores per cluster, + 2 threads per core) for ARM virt machines which support sockets/clusters + /cores/threads. Some members of the option can be omitted but their values + will be automatically computed: + + :: + + -smp 16,sockets=2,clusters=2,cores=2,threads=2,maxcpus=16 + Historically preference was given to the coarsest topology parameters when computing missing values (ie sockets preferred over cores, which were preferred over threads), however, this behaviour is considered -- 2.25.1 From: Yanan Wang <wangyanan55@huawei.com> Support one cluster level between core and physical package in the cpu-map of Arm/virt devicetree. This is also consistent with Linux Doc "Documentation/devicetree/bindings/cpu/cpu-topology.txt". Signed-off-by: Yanan Wang <wangyanan55@huawei.com> Reviewed-by: Andrew Jones <drjones@redhat.com> Message-id: 20220107083232.16256-3-wangyanan55@huawei.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) 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_cpu_nodes(const VirtMachineState *vms) * can contain several layers of clustering within a single physical * package and cluster nodes can be contained in parent cluster nodes. * - * Given that cluster is not yet supported in the vCPU topology, - * we currently generate one cluster node within each socket node - * by default. + * Note: currently we only support one layer of clustering within + * each physical package. */ qemu_fdt_add_subnode(ms->fdt, "/cpus/cpu-map"); @@ -XXX,XX +XXX,XX @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms) if (ms->smp.threads > 1) { map_path = g_strdup_printf( - "/cpus/cpu-map/socket%d/cluster0/core%d/thread%d", - cpu / (ms->smp.cores * ms->smp.threads), + "/cpus/cpu-map/socket%d/cluster%d/core%d/thread%d", + cpu / (ms->smp.clusters * ms->smp.cores * ms->smp.threads), + (cpu / (ms->smp.cores * ms->smp.threads)) % ms->smp.clusters, (cpu / ms->smp.threads) % ms->smp.cores, cpu % ms->smp.threads); } else { map_path = g_strdup_printf( - "/cpus/cpu-map/socket%d/cluster0/core%d", - cpu / ms->smp.cores, + "/cpus/cpu-map/socket%d/cluster%d/core%d", + cpu / (ms->smp.clusters * ms->smp.cores), + (cpu / ms->smp.cores) % ms->smp.clusters, cpu % ms->smp.cores); } qemu_fdt_add_path(ms->fdt, map_path); -- 2.25.1 From: Yanan Wang <wangyanan55@huawei.com> Use g_queue APIs to reduce the nested loops and code indentation with the processor hierarchy levels increasing. Consenquently, it's more scalable to add new topology level to build_pptt. No functional change intended. Signed-off-by: Yanan Wang <wangyanan55@huawei.com> Reviewed-by: Andrew Jones <drjones@redhat.com> Message-id: 20220107083232.16256-4-wangyanan55@huawei.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/acpi/aml-build.c | 50 +++++++++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c index XXXXXXX..XXXXXXX 100644 --- a/hw/acpi/aml-build.c +++ b/hw/acpi/aml-build.c @@ -XXX,XX +XXX,XX @@ static void build_processor_hierarchy_node(GArray *tbl, uint32_t flags, void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms, const char *oem_id, const char *oem_table_id) { - int pptt_start = table_data->len; + GQueue *list = g_queue_new(); + guint pptt_start = table_data->len; + guint parent_offset; + guint length, i; int uid = 0; int socket; AcpiTable table = { .sig = "PPTT", .rev = 2, @@ -XXX,XX +XXX,XX @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms, acpi_table_begin(&table, table_data); for (socket = 0; socket < ms->smp.sockets; socket++) { - uint32_t socket_offset = table_data->len - pptt_start; - int core; - + g_queue_push_tail(list, + GUINT_TO_POINTER(table_data->len - pptt_start)); build_processor_hierarchy_node( table_data, /* @@ -XXX,XX +XXX,XX @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms, */ (1 << 0), 0, socket, NULL, 0); + } + length = g_queue_get_length(list); + for (i = 0; i < length; i++) { + int core; + + parent_offset = GPOINTER_TO_UINT(g_queue_pop_head(list)); for (core = 0; core < ms->smp.cores; core++) { - uint32_t core_offset = table_data->len - pptt_start; - int thread; - if (ms->smp.threads > 1) { + g_queue_push_tail(list, + GUINT_TO_POINTER(table_data->len - pptt_start)); build_processor_hierarchy_node( table_data, (0 << 0), /* not a physical package */ - socket_offset, core, NULL, 0); - - for (thread = 0; thread < ms->smp.threads; thread++) { - build_processor_hierarchy_node( - table_data, - (1 << 1) | /* ACPI Processor ID valid */ - (1 << 2) | /* Processor is a Thread */ - (1 << 3), /* Node is a Leaf */ - core_offset, uid++, NULL, 0); - } + parent_offset, core, NULL, 0); } else { build_processor_hierarchy_node( table_data, (1 << 1) | /* ACPI Processor ID valid */ (1 << 3), /* Node is a Leaf */ - socket_offset, uid++, NULL, 0); + parent_offset, uid++, NULL, 0); } } } + length = g_queue_get_length(list); + for (i = 0; i < length; i++) { + int thread; + + parent_offset = GPOINTER_TO_UINT(g_queue_pop_head(list)); + for (thread = 0; thread < ms->smp.threads; thread++) { + build_processor_hierarchy_node( + table_data, + (1 << 1) | /* ACPI Processor ID valid */ + (1 << 2) | /* Processor is a Thread */ + (1 << 3), /* Node is a Leaf */ + parent_offset, uid++, NULL, 0); + } + } + + g_queue_free(list); acpi_table_end(linker, &table); } -- 2.25.1 From: Yanan Wang <wangyanan55@huawei.com> List test/data/acpi/virt/PPTT as the expected files allowed to be changed in tests/qtest/bios-tables-test-allowed-diff.h Signed-off-by: Yanan Wang <wangyanan55@huawei.com> Acked-by: Ani Sinha <ani@anisinha.ca> Message-id: 20220107083232.16256-5-wangyanan55@huawei.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tests/qtest/bios-tables-test-allowed-diff.h | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,2 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/virt/PPTT", -- 2.25.1 From: Yanan Wang <wangyanan55@huawei.com> Support CPU cluster topology level in generation of ACPI Processor Properties Topology Table (PPTT). Signed-off-by: Yanan Wang <wangyanan55@huawei.com> Reviewed-by: Andrew Jones <drjones@redhat.com> Message-id: 20220107083232.16256-6-wangyanan55@huawei.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/acpi/aml-build.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c index XXXXXXX..XXXXXXX 100644 --- a/hw/acpi/aml-build.c +++ b/hw/acpi/aml-build.c @@ -XXX,XX +XXX,XX @@ static void build_processor_hierarchy_node(GArray *tbl, uint32_t flags, void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms, const char *oem_id, const char *oem_table_id) { + MachineClass *mc = MACHINE_GET_CLASS(ms); GQueue *list = g_queue_new(); guint pptt_start = table_data->len; guint parent_offset; @@ -XXX,XX +XXX,XX @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms, 0, socket, NULL, 0); } + if (mc->smp_props.clusters_supported) { + length = g_queue_get_length(list); + for (i = 0; i < length; i++) { + int cluster; + + parent_offset = GPOINTER_TO_UINT(g_queue_pop_head(list)); + for (cluster = 0; cluster < ms->smp.clusters; cluster++) { + g_queue_push_tail(list, + GUINT_TO_POINTER(table_data->len - pptt_start)); + build_processor_hierarchy_node( + table_data, + (0 << 0), /* not a physical package */ + parent_offset, cluster, NULL, 0); + } + } + } + length = g_queue_get_length(list); for (i = 0; i < length; i++) { int core; -- 2.25.1 From: Yanan Wang <wangyanan55@huawei.com> Run ./tests/data/acpi/rebuild-expected-aml.sh from build directory to update PPTT binary. Also empty bios-tables-test-allowed-diff.h. The disassembled differences between actual and expected PPTT: /* * Intel ACPI Component Architecture * AML/ASL+ Disassembler version 20200528 (64-bit version) * Copyright (c) 2000 - 2020 Intel Corporation * - * Disassembly of tests/data/acpi/virt/PPTT, Tue Jan 4 12:51:11 2022 + * Disassembly of /tmp/aml-2ZGOF1, Tue Jan 4 12:51:11 2022 * * ACPI Data Table [PPTT] * * Format: [HexOffset DecimalOffset ByteLength] FieldName : FieldValue */ [000h 0000 4] Signature : "PPTT" [Processor Properties Topology Table] -[004h 0004 4] Table Length : 0000004C +[004h 0004 4] Table Length : 00000060 [008h 0008 1] Revision : 02 -[009h 0009 1] Checksum : A8 +[009h 0009 1] Checksum : 48 [00Ah 0010 6] Oem ID : "BOCHS " [010h 0016 8] Oem Table ID : "BXPC " [018h 0024 4] Oem Revision : 00000001 [01Ch 0028 4] Asl Compiler ID : "BXPC" [020h 0032 4] Asl Compiler Revision : 00000001 [024h 0036 1] Subtable Type : 00 [Processor Hierarchy Node] [025h 0037 1] Length : 14 [026h 0038 2] Reserved : 0000 [028h 0040 4] Flags (decoded below) : 00000001 Physical package : 1 ACPI Processor ID valid : 0 Processor is a thread : 0 Node is a leaf : 0 Identical Implementation : 0 [02Ch 0044 4] Parent : 00000000 [030h 0048 4] ACPI Processor ID : 00000000 [034h 0052 4] Private Resource Number : 00000000 [038h 0056 1] Subtable Type : 00 [Processor Hierarchy Node] [039h 0057 1] Length : 14 [03Ah 0058 2] Reserved : 0000 -[03Ch 0060 4] Flags (decoded below) : 0000000A +[03Ch 0060 4] Flags (decoded below) : 00000000 Physical package : 0 - ACPI Processor ID valid : 1 + ACPI Processor ID valid : 0 Processor is a thread : 0 - Node is a leaf : 1 + Node is a leaf : 0 Identical Implementation : 0 [040h 0064 4] Parent : 00000024 [044h 0068 4] ACPI Processor ID : 00000000 [048h 0072 4] Private Resource Number : 00000000 -Raw Table Data: Length 76 (0x4C) +[04Ch 0076 1] Subtable Type : 00 [Processor Hierarchy Node] +[04Dh 0077 1] Length : 14 +[04Eh 0078 2] Reserved : 0000 +[050h 0080 4] Flags (decoded below) : 0000000A + Physical package : 0 + ACPI Processor ID valid : 1 + Processor is a thread : 0 + Node is a leaf : 1 + Identical Implementation : 0 +[054h 0084 4] Parent : 00000038 +[058h 0088 4] ACPI Processor ID : 00000000 +[05Ch 0092 4] Private Resource Number : 00000000 + +Raw Table Data: Length 96 (0x60) - 0000: 50 50 54 54 4C 00 00 00 02 A8 42 4F 43 48 53 20 // PPTTL.....BOCHS + 0000: 50 50 54 54 60 00 00 00 02 48 42 4F 43 48 53 20 // PPTT`....HBOCHS 0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43 // BXPC ....BXPC 0020: 01 00 00 00 00 14 00 00 01 00 00 00 00 00 00 00 // ................ - 0030: 00 00 00 00 00 00 00 00 00 14 00 00 0A 00 00 00 // ................ - 0040: 24 00 00 00 00 00 00 00 00 00 00 00 // $........... + 0030: 00 00 00 00 00 00 00 00 00 14 00 00 00 00 00 00 // ................ + 0040: 24 00 00 00 00 00 00 00 00 00 00 00 00 14 00 00 // $............... + 0050: 0A 00 00 00 38 00 00 00 00 00 00 00 00 00 00 00 // ....8........... Signed-off-by: Yanan Wang <wangyanan55@huawei.com> Reviewed-by: Ani Sinha <ani@anisinha.ca> Message-id: 20220107083232.16256-7-wangyanan55@huawei.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tests/qtest/bios-tables-test-allowed-diff.h | 1 - tests/data/acpi/virt/PPTT | Bin 76 -> 96 bytes 2 files changed, 1 deletion(-) diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1,2 +1 @@ /* List of comma-separated changed AML files to ignore */ -"tests/data/acpi/virt/PPTT", diff --git a/tests/data/acpi/virt/PPTT b/tests/data/acpi/virt/PPTT index XXXXXXX..XXXXXXX 100644 GIT binary patch delta 53 pcmeZC;0g!`2}xjJU|{l?$YrDgWH5jU5Ca567#O&Klm(arApowi1QY-O delta 32 fcmYfB;R*-{3GrcIU|?D?k;`ae01J-_kOKn%ZFdCM -- 2.25.1 From: Lucas Ramage <lucas.ramage@infinite-omicron.com> Buglink: https://gitlab.com/qemu-project/qemu/-/issues/527 Signed-off-by: Lucas Ramage <lucas.ramage@infinite-omicron.com> Message-id: 20220105205628.5491-1-oxr463@gmx.us Reviewed-by: Peter Maydell <peter.maydell@linaro.org> [PMM: Move to docs/system/devices/ rather than top-level; fix a pre-existing typo in passing] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- docs/system/device-emulation.rst | 1 + docs/{can.txt => system/devices/can.rst} | 90 +++++++++++------------- 2 files changed, 41 insertions(+), 50 deletions(-) rename docs/{can.txt => system/devices/can.rst} (68%) diff --git a/docs/system/device-emulation.rst b/docs/system/device-emulation.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/system/device-emulation.rst +++ b/docs/system/device-emulation.rst @@ -XXX,XX +XXX,XX @@ Emulated Devices .. toctree:: :maxdepth: 1 + devices/can.rst devices/ivshmem.rst devices/net.rst devices/nvme.rst diff --git a/docs/can.txt b/docs/system/devices/can.rst similarity index 68% rename from docs/can.txt rename to docs/system/devices/can.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/can.txt +++ b/docs/system/devices/can.rst @@ -XXX,XX +XXX,XX @@ -QEMU CAN bus emulation support -============================== - +CAN Bus Emulation Support +========================= The CAN bus emulation provides mechanism to connect multiple emulated CAN controller chips together by one or multiple CAN busses (the controller device "canbus" parameter). The individual busses @@ -XXX,XX +XXX,XX @@ emulated environment for testing and RTEMS GSoC slot has been donated to work on CAN hardware emulation on QEMU. Examples how to use CAN emulation for SJA1000 based boards -========================================================== - +---------------------------------------------------------- When QEMU with CAN PCI support is compiled then one of the next CAN boards can be selected - (1) CAN bus Kvaser PCI CAN-S (single SJA1000 channel) boad. QEMU startup options +(1) CAN bus Kvaser PCI CAN-S (single SJA1000 channel) board. QEMU startup options:: + -object can-bus,id=canbus0 -device kvaser_pci,canbus=canbus0 - Add "can-host-socketcan" object to connect device to host system CAN bus + +Add "can-host-socketcan" object to connect device to host system CAN bus:: + -object can-host-socketcan,id=canhost0,if=can0,canbus=canbus0 - (2) CAN bus PCM-3680I PCI (dual SJA1000 channel) emulation +(2) CAN bus PCM-3680I PCI (dual SJA1000 channel) emulation:: + -object can-bus,id=canbus0 -device pcm3680_pci,canbus0=canbus0,canbus1=canbus0 - another example: +Another example:: + -object can-bus,id=canbus0 -object can-bus,id=canbus1 -device pcm3680_pci,canbus0=canbus0,canbus1=canbus1 - (3) CAN bus MIOe-3680 PCI (dual SJA1000 channel) emulation +(3) CAN bus MIOe-3680 PCI (dual SJA1000 channel) emulation:: + -device mioe3680_pci,canbus0=canbus0 - The ''kvaser_pci'' board/device model is compatible with and has been tested with -''kvaser_pci'' driver included in mainline Linux kernel. +the ''kvaser_pci'' driver included in mainline Linux kernel. The tested setup was Linux 4.9 kernel on the host and guest side. -Example for qemu-system-x86_64: + +Example for qemu-system-x86_64:: qemu-system-x86_64 -accel kvm -kernel /boot/vmlinuz-4.9.0-4-amd64 \ -initrd ramdisk.cpio \ @@ -XXX,XX +XXX,XX @@ Example for qemu-system-x86_64: -device kvaser_pci,canbus=canbus0 \ -nographic -append "console=ttyS0" -Example for qemu-system-arm: +Example for qemu-system-arm:: qemu-system-arm -cpu arm1176 -m 256 -M versatilepb \ -kernel kernel-qemu-arm1176-versatilepb \ @@ -XXX,XX +XXX,XX @@ Example for qemu-system-arm: The CAN interface of the host system has to be configured for proper bitrate and set up. Configuration is not propagated from emulated devices through bus to the physical host device. Example configuration -for 1 Mbit/s +for 1 Mbit/s:: ip link set can0 type can bitrate 1000000 ip link set can0 up Virtual (host local only) can interface can be used on the host -side instead of physical interface +side instead of physical interface:: ip link add dev can0 type vcan The CAN interface on the host side can be used to analyze CAN -traffic with "candump" command which is included in "can-utils". +traffic with "candump" command which is included in "can-utils":: candump can0 CTU CAN FD support examples -=========================== - +--------------------------- This open-source core provides CAN FD support. CAN FD drames are delivered even to the host systems when SocketCAN interface is found CAN FD capable. @@ -XXX,XX +XXX,XX @@ on the board. Example how to connect the canbus0-bus (virtual wire) to the host Linux system (SocketCAN used) and to both CTU CAN FD cores emulated on the corresponding PCI card expects that host system CAN bus -is setup according to the previous SJA1000 section. +is setup according to the previous SJA1000 section:: qemu-system-x86_64 -enable-kvm -kernel /boot/vmlinuz-4.19.52+ \ -initrd ramdisk.cpio \ @@ -XXX,XX +XXX,XX @@ is setup according to the previous SJA1000 section. -device ctucan_pci,canbus0=canbus0-bus,canbus1=canbus0-bus \ -nographic -Setup of CTU CAN FD controller in a guest Linux system +Setup of CTU CAN FD controller in a guest Linux system:: insmod ctucanfd.ko || modprobe ctucanfd insmod ctucanfd_pci.ko || modprobe ctucanfd_pci @@ -XXX,XX +XXX,XX @@ Setup of CTU CAN FD controller in a guest Linux system /bin/ip link set $ifc up done -The test can run for example +The test can run for example:: candump can1 -in the guest system and next commands in the host system for basic CAN +in the guest system and next commands in the host system for basic CAN:: cangen can0 -for CAN FD without bitrate switch +for CAN FD without bitrate switch:: cangen can0 -f -and with bitrate switch +and with bitrate switch:: cangen can0 -b @@ -XXX,XX +XXX,XX @@ The test can be run viceversa, generate messages in the guest system and capture in the host one and much more combinations. Links to other resources -======================== +------------------------ - (1) CAN related projects at Czech Technical University, Faculty of Electrical Engineering - http://canbus.pages.fel.cvut.cz/ - (2) Repository with development can-pci branch at Czech Technical University - https://gitlab.fel.cvut.cz/canbus/qemu-canbus - (3) RTEMS page describing project - https://devel.rtems.org/wiki/Developer/Simulators/QEMU/CANEmulation - (4) RTLWS 2015 article about the project and its use with CANopen emulation - http://cmp.felk.cvut.cz/~pisa/can/doc/rtlws-17-pisa-qemu-can.pdf - (5) GNU/Linux, CAN and CANopen in Real-time Control Applications - Slides from LinuxDays 2017 (include updated RTLWS 2015 content) - https://www.linuxdays.cz/2017/video/Pavel_Pisa-CAN_canopen.pdf - (6) Linux SocketCAN utilities - https://github.com/linux-can/can-utils/ - (7) CTU CAN FD project including core VHDL design, Linux driver, - test utilities etc. - https://gitlab.fel.cvut.cz/canbus/ctucanfd_ip_core - (8) CTU CAN FD Core Datasheet Documentation - http://canbus.pages.fel.cvut.cz/ctucanfd_ip_core/Progdokum.pdf - (9) CTU CAN FD Core System Architecture Documentation - http://canbus.pages.fel.cvut.cz/ctucanfd_ip_core/ctu_can_fd_architecture.pdf - (10) CTU CAN FD Driver Documentation - http://canbus.pages.fel.cvut.cz/ctucanfd_ip_core/driver_doc/ctucanfd-driver.html - (11) Integration with PCIe interfacing for Intel/Altera Cyclone IV based board - https://gitlab.fel.cvut.cz/canbus/pcie-ctu_can_fd + (1) `CAN related projects at Czech Technical University, Faculty of Electrical Engineering <http://canbus.pages.fel.cvut.cz>`_ + (2) `Repository with development can-pci branch at Czech Technical University <https://gitlab.fel.cvut.cz/canbus/qemu-canbus>`_ + (3) `RTEMS page describing project <https://devel.rtems.org/wiki/Developer/Simulators/QEMU/CANEmulation>`_ + (4) `RTLWS 2015 article about the project and its use with CANopen emulation <http://cmp.felk.cvut.cz/~pisa/can/doc/rtlws-17-pisa-qemu-can.pdf>`_ + (5) `GNU/Linux, CAN and CANopen in Real-time Control Applications Slides from LinuxDays 2017 (include updated RTLWS 2015 content) <https://www.linuxdays.cz/2017/video/Pavel_Pisa-CAN_canopen.pdf>`_ + (6) `Linux SocketCAN utilities <https://github.com/linux-can/can-utils>`_ + (7) `CTU CAN FD project including core VHDL design, Linux driver, test utilities etc. <https://gitlab.fel.cvut.cz/canbus/ctucanfd_ip_core>`_ + (8) `CTU CAN FD Core Datasheet Documentation <http://canbus.pages.fel.cvut.cz/ctucanfd_ip_core/Progdokum.pdf>`_ + (9) `CTU CAN FD Core System Architecture Documentation <http://canbus.pages.fel.cvut.cz/ctucanfd_ip_core/ctu_can_fd_architecture.pdf>`_ + (10) `CTU CAN FD Driver Documentation <http://canbus.pages.fel.cvut.cz/ctucanfd_ip_core/driver_doc/ctucanfd-driver.html>`_ + (11) `Integration with PCIe interfacing for Intel/Altera Cyclone IV based board <https://gitlab.fel.cvut.cz/canbus/pcie-ctu_can_fd>`_ -- 2.25.1 From: Gavin Shan <gshan@redhat.com> The default block size is same as to the THP size, which is either retrieved from "/sys/kernel/mm/transparent_hugepage/hpage_pmd_size" or hardcoded to 2MB. There are flaws in both mechanisms and this intends to fix them up. * When "/sys/kernel/mm/transparent_hugepage/hpage_pmd_size" is used to getting the THP size, 32MB and 512MB are valid values when we have 16KB and 64KB page size on ARM64. * When the hardcoded THP size is used, 2MB, 32MB and 512MB are valid values when we have 4KB, 16KB and 64KB page sizes on ARM64. Co-developed-by: David Hildenbrand <david@redhat.com> Signed-off-by: Gavin Shan <gshan@redhat.com> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@Huawei.com> Reviewed-by: David Hildenbrand <david@redhat.com> Message-id: 20220111063329.74447-2-gshan@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/virtio/virtio-mem.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c index XXXXXXX..XXXXXXX 100644 --- a/hw/virtio/virtio-mem.c +++ b/hw/virtio/virtio-mem.c @@ -XXX,XX +XXX,XX @@ */ #define VIRTIO_MEM_MIN_BLOCK_SIZE ((uint32_t)(1 * MiB)) -#if defined(__x86_64__) || defined(__arm__) || defined(__aarch64__) || \ - defined(__powerpc64__) -#define VIRTIO_MEM_DEFAULT_THP_SIZE ((uint32_t)(2 * MiB)) -#else - /* fallback to 1 MiB (e.g., the THP size on s390x) */ -#define VIRTIO_MEM_DEFAULT_THP_SIZE VIRTIO_MEM_MIN_BLOCK_SIZE +static uint32_t virtio_mem_default_thp_size(void) +{ + uint32_t default_thp_size = VIRTIO_MEM_MIN_BLOCK_SIZE; + +#if defined(__x86_64__) || defined(__arm__) || defined(__powerpc64__) + default_thp_size = 2 * MiB; +#elif defined(__aarch64__) + if (qemu_real_host_page_size == 4 * KiB) { + default_thp_size = 2 * MiB; + } else if (qemu_real_host_page_size == 16 * KiB) { + default_thp_size = 32 * MiB; + } else if (qemu_real_host_page_size == 64 * KiB) { + default_thp_size = 512 * MiB; + } #endif + return default_thp_size; +} + /* * We want to have a reasonable default block size such that * 1. We avoid splitting THPs when unplugging memory, which degrades @@ -XXX,XX +XXX,XX @@ static uint32_t virtio_mem_thp_size(void) if (g_file_get_contents(HPAGE_PMD_SIZE_PATH, &content, NULL, NULL) && !qemu_strtou64(content, &endptr, 0, &tmp) && (!endptr || *endptr == '\n')) { - /* - * Sanity-check the value, if it's too big (e.g., aarch64 with 64k base - * pages) or weird, fallback to something smaller. - */ - if (!tmp || !is_power_of_2(tmp) || tmp > 16 * MiB) { + /* Sanity-check the value and fallback to something reasonable. */ + if (!tmp || !is_power_of_2(tmp)) { warn_report("Read unsupported THP size: %" PRIx64, tmp); } else { thp_size = tmp; @@ -XXX,XX +XXX,XX @@ static uint32_t virtio_mem_thp_size(void) } if (!thp_size) { - thp_size = VIRTIO_MEM_DEFAULT_THP_SIZE; + thp_size = virtio_mem_default_thp_size(); warn_report("Could not detect THP size, falling back to %" PRIx64 " MiB.", thp_size / MiB); } -- 2.25.1 From: Gavin Shan <gshan@redhat.com> This supports virtio-mem-pci device on "virt" platform, by simply following the implementation on x86. * This implements the hotplug handlers to support virtio-mem-pci device hot-add, while the hot-remove isn't supported as we have on x86. * The block size is 512MB on ARM64 instead of 128MB on x86. * It has been passing the tests with various combinations like 64KB and 4KB page sizes on host and guest, different memory device backends like normal, transparent huge page and HugeTLB, plus migration. Co-developed-by: David Hildenbrand <david@redhat.com> Co-developed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Signed-off-by: Gavin Shan <gshan@redhat.com> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@Huawei.com> Reviewed-by: David Hildenbrand <david@redhat.com> Message-id: 20220111063329.74447-3-gshan@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt.c | 70 ++++++++++++++++++++++++++++++++++++++++++ hw/virtio/virtio-mem.c | 4 ++- hw/arm/Kconfig | 1 + 3 files changed, 74 insertions(+), 1 deletion(-) 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 @@ #include "hw/arm/smmuv3.h" #include "hw/acpi/acpi.h" #include "target/arm/internals.h" +#include "hw/mem/memory-device.h" #include "hw/mem/pc-dimm.h" #include "hw/mem/nvdimm.h" #include "hw/acpi/generic_event_device.h" +#include "hw/virtio/virtio-mem-pci.h" #include "hw/virtio/virtio-iommu.h" #include "hw/char/pl011.h" #include "qemu/guest-random.h" @@ -XXX,XX +XXX,XX @@ static void virt_memory_plug(HotplugHandler *hotplug_dev, dev, &error_abort); } +static void virt_virtio_md_pci_pre_plug(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) +{ + HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev); + Error *local_err = NULL; + + if (!hotplug_dev2 && dev->hotplugged) { + /* + * Without a bus hotplug handler, we cannot control the plug/unplug + * order. We should never reach this point when hotplugging on ARM. + * However, it's nice to add a safety net, similar to what we have + * on x86. + */ + error_setg(errp, "hotplug of virtio based memory devices not supported" + " on this bus."); + return; + } + /* + * First, see if we can plug this memory device at all. If that + * succeeds, branch of to the actual hotplug handler. + */ + memory_device_pre_plug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev), NULL, + &local_err); + if (!local_err && hotplug_dev2) { + hotplug_handler_pre_plug(hotplug_dev2, dev, &local_err); + } + error_propagate(errp, local_err); +} + +static void virt_virtio_md_pci_plug(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) +{ + HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev); + Error *local_err = NULL; + + /* + * Plug the memory device first and then branch off to the actual + * hotplug handler. If that one fails, we can easily undo the memory + * device bits. + */ + memory_device_plug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev)); + if (hotplug_dev2) { + hotplug_handler_plug(hotplug_dev2, dev, &local_err); + if (local_err) { + memory_device_unplug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev)); + } + } + error_propagate(errp, local_err); +} + +static void virt_virtio_md_pci_unplug_request(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) +{ + /* We don't support hot unplug of virtio based memory devices */ + error_setg(errp, "virtio based memory devices cannot be unplugged."); +} + + static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { @@ -XXX,XX +XXX,XX @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev, if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { virt_memory_pre_plug(hotplug_dev, dev, errp); + } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) { + virt_virtio_md_pci_pre_plug(hotplug_dev, dev, errp); } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) { hwaddr db_start = 0, db_end = 0; char *resv_prop_str; @@ -XXX,XX +XXX,XX @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev, if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { virt_memory_plug(hotplug_dev, dev, errp); } + + if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) { + virt_virtio_md_pci_plug(hotplug_dev, dev, errp); + } + if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) { PCIDevice *pdev = PCI_DEVICE(dev); @@ -XXX,XX +XXX,XX @@ static void virt_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev, { if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { virt_dimm_unplug_request(hotplug_dev, dev, errp); + } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) { + virt_virtio_md_pci_unplug_request(hotplug_dev, dev, errp); } else { error_setg(errp, "device unplug request for unsupported device" " type: %s", object_get_typename(OBJECT(dev))); @@ -XXX,XX +XXX,XX @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine, if (device_is_dynamic_sysbus(mc, dev) || object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) || + object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI) || object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) { return HOTPLUG_HANDLER(machine); } diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c index XXXXXXX..XXXXXXX 100644 --- a/hw/virtio/virtio-mem.c +++ b/hw/virtio/virtio-mem.c @@ -XXX,XX +XXX,XX @@ static bool virtio_mem_has_shared_zeropage(RAMBlock *rb) * The memory block size corresponds mostly to the section size. * * This allows e.g., to add 20MB with a section size of 128MB on x86_64, and - * a section size of 1GB on arm64 (as long as the start address is properly + * a section size of 512MB on arm64 (as long as the start address is properly * aligned, similar to ordinary DIMMs). * * We can change this at any time and maybe even make it configurable if @@ -XXX,XX +XXX,XX @@ static bool virtio_mem_has_shared_zeropage(RAMBlock *rb) */ #if defined(TARGET_X86_64) || defined(TARGET_I386) #define VIRTIO_MEM_USABLE_EXTENT (2 * (128 * MiB)) +#elif defined(TARGET_ARM) +#define VIRTIO_MEM_USABLE_EXTENT (2 * (512 * MiB)) #else #error VIRTIO_MEM_USABLE_EXTENT not defined #endif 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 select ACPI_HW_REDUCED select ACPI_APEI select ACPI_VIOT + select VIRTIO_MEM_SUPPORTED config CHEETAH bool -- 2.25.1 From: Petr Pavlu <petr.pavlu@suse.com> Implement support for reading GICC_IIDR. This register is used by the Linux kernel to recognize that GICv2 with GICC_APRn is present. Signed-off-by: Petr Pavlu <petr.pavlu@suse.com> Message-id: 20220113151916.17978-2-ppavlu@suse.cz Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/arm_gic.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -XXX,XX +XXX,XX @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, int offset, } break; } + case 0xfc: + if (s->revision == REV_11MPCORE) { + /* Reserved on 11MPCore */ + *data = 0; + } else { + /* GICv1 or v2; Arm implementation */ + *data = (s->revision << 16) | 0x43b; + } + break; default: qemu_log_mask(LOG_GUEST_ERROR, "gic_cpu_read: Bad offset %x\n", (int)offset); -- 2.25.1 From: Petr Pavlu <petr.pavlu@suse.com> When running Linux on a machine with GICv2, the kernel can crash while processing an interrupt and can subsequently start a kdump kernel from the active interrupt handler. In such a case, the crashed kernel might not gracefully signal the end of interrupt to the GICv2 hardware. The kdump kernel will however try to reset the GIC state on startup to get the controller into a sane state, in particular the kernel writes ones to GICD_ICACTIVERn and wipes out GICC_APRn to make sure that no interrupt is active. The patch adds a logic to recalculate the running priority when GICC_APRn/GICC_NSAPRn is written which makes sure that the mentioned reset works with the GICv2 emulation in QEMU too and the kdump kernel starts receiving interrupts. The described scenario can be reproduced on an AArch64 QEMU virt machine with a kdump-enabled Linux system by using the softdog module. The kdump kernel will hang at some point because QEMU still thinks the running priority is that of the timer interrupt and asserts no new interrupts to the system: $ modprobe softdog soft_margin=10 soft_panic=1 $ cat > /dev/watchdog [Press Enter to start the watchdog, wait for its timeout and observe that the kdump kernel hangs on startup.] Signed-off-by: Petr Pavlu <petr.pavlu@suse.com> Message-id: 20220113151916.17978-3-ppavlu@suse.cz Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/arm_gic.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -XXX,XX +XXX,XX @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, int offset, } else { s->apr[regno][cpu] = value; } + s->running_priority[cpu] = gic_get_prio_from_apr_bits(s, cpu); break; } case 0xe0: case 0xe4: case 0xe8: case 0xec: @@ -XXX,XX +XXX,XX @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, int offset, return MEMTX_OK; } s->nsapr[regno][cpu] = value; + s->running_priority[cpu] = gic_get_prio_from_apr_bits(s, cpu); break; } case 0x1000: -- 2.25.1 From: Marc Zyngier <maz@kernel.org> Just like we can control the enablement of the highmem PCIe ECAM region using highmem_ecam, let's add a control for the highmem PCIe MMIO region. Similarily to highmem_ecam, this region is disabled when highmem is off. Signed-off-by: Marc Zyngier <maz@kernel.org> Reviewed-by: Eric Auger <eric.auger@redhat.com> Message-id: 20220114140741.1358263-2-maz@kernel.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/virt.h | 1 + hw/arm/virt-acpi-build.c | 10 ++++------ hw/arm/virt.c | 7 +++++-- 3 files changed, 10 insertions(+), 8 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 @@ struct VirtMachineState { bool secure; bool highmem; bool highmem_ecam; + bool highmem_mmio; bool its; bool tcg_its; bool virt; 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_virtio(Aml *scope, } static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, - uint32_t irq, bool use_highmem, bool highmem_ecam, - VirtMachineState *vms) + uint32_t irq, VirtMachineState *vms) { - int ecam_id = VIRT_ECAM_ID(highmem_ecam); + int ecam_id = VIRT_ECAM_ID(vms->highmem_ecam); struct GPEXConfig cfg = { .mmio32 = memmap[VIRT_PCIE_MMIO], .pio = memmap[VIRT_PCIE_PIO], @@ -XXX,XX +XXX,XX @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, .bus = vms->bus, }; - if (use_highmem) { + if (vms->highmem_mmio) { cfg.mmio64 = memmap[VIRT_HIGH_PCIE_MMIO]; } @@ -XXX,XX +XXX,XX @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) acpi_dsdt_add_fw_cfg(scope, &memmap[VIRT_FW_CFG]); acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO], (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS); - acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE), - vms->highmem, vms->highmem_ecam, vms); + acpi_dsdt_add_pci(scope, memmap, irqmap[VIRT_PCIE] + ARM_SPI_BASE, vms); if (vms->acpi_dev) { build_ged_aml(scope, "\\_SB."GED_DEVICE, HOTPLUG_HANDLER(vms->acpi_dev), 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 create_pcie(VirtMachineState *vms) mmio_reg, base_mmio, size_mmio); memory_region_add_subregion(get_system_memory(), base_mmio, mmio_alias); - if (vms->highmem) { + if (vms->highmem_mmio) { /* Map high MMIO space */ MemoryRegion *high_mmio_alias = g_new0(MemoryRegion, 1); @@ -XXX,XX +XXX,XX @@ static void create_pcie(VirtMachineState *vms) qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg", 2, base_ecam, 2, size_ecam); - if (vms->highmem) { + if (vms->highmem_mmio) { qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "ranges", 1, FDT_PCI_RANGE_IOPORT, 2, 0, 2, base_pio, 2, size_pio, @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) virt_flash_fdt(vms, sysmem, secure_sysmem ?: sysmem); + vms->highmem_mmio &= vms->highmem; + create_gic(vms, sysmem); virt_cpu_post_init(vms, sysmem); @@ -XXX,XX +XXX,XX @@ static void virt_instance_init(Object *obj) vms->gic_version = VIRT_GIC_VERSION_NOSEL; vms->highmem_ecam = !vmc->no_highmem_ecam; + vms->highmem_mmio = true; if (vmc->no_its) { vms->its = false; -- 2.25.1 From: Marc Zyngier <maz@kernel.org> Just like we can control the enablement of the highmem PCIe region using highmem_ecam, let's add a control for the highmem GICv3 redistributor region. Similarily to highmem_ecam, these redistributors are disabled when highmem is off. Reviewed-by: Andrew Jones <drjones@redhat.com> Signed-off-by: Marc Zyngier <maz@kernel.org> Reviewed-by: Eric Auger <eric.auger@redhat.com> Message-id: 20220114140741.1358263-3-maz@kernel.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/virt.h | 4 +++- hw/arm/virt-acpi-build.c | 2 ++ hw/arm/virt.c | 2 ++ 3 files changed, 7 insertions(+), 1 deletion(-) 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 @@ struct VirtMachineState { bool highmem; bool highmem_ecam; bool highmem_mmio; + bool highmem_redists; bool its; bool tcg_its; bool virt; @@ -XXX,XX +XXX,XX @@ static inline int virt_gicv3_redist_region_count(VirtMachineState *vms) assert(vms->gic_version == VIRT_GIC_VERSION_3); - return MACHINE(vms)->smp.cpus > redist0_capacity ? 2 : 1; + return (MACHINE(vms)->smp.cpus > redist0_capacity && + vms->highmem_redists) ? 2 : 1; } #endif /* QEMU_ARM_VIRT_H */ 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 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) acpi_add_table(table_offsets, tables_blob); build_fadt_rev5(tables_blob, tables->linker, vms, dsdt); + vms->highmem_redists &= vms->highmem; + acpi_add_table(table_offsets, tables_blob); build_madt(tables_blob, tables->linker, vms); 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 machvirt_init(MachineState *machine) virt_flash_fdt(vms, sysmem, secure_sysmem ?: sysmem); vms->highmem_mmio &= vms->highmem; + vms->highmem_redists &= vms->highmem; create_gic(vms, sysmem); @@ -XXX,XX +XXX,XX @@ static void virt_instance_init(Object *obj) vms->highmem_ecam = !vmc->no_highmem_ecam; vms->highmem_mmio = true; + vms->highmem_redists = true; if (vmc->no_its) { vms->its = false; -- 2.25.1 From: Marc Zyngier <maz@kernel.org> Even when the VM is configured with highmem=off, the highest_gpa field includes devices that are above the 4GiB limit. Similarily, nothing seem to check that the memory is within the limit set by the highmem=off option. This leads to failures in virt_kvm_type() on systems that have a crippled IPA range, as the reported IPA space is larger than what it should be. Instead, honor the user-specified limit to only use the devices at the lowest end of the spectrum, and fail if we have memory crossing the 4GiB limit. Reviewed-by: Andrew Jones <drjones@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Signed-off-by: Marc Zyngier <maz@kernel.org> Message-id: 20220114140741.1358263-4-maz@kernel.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) 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 uint64_t virt_cpu_mp_affinity(VirtMachineState *vms, int idx) static void virt_set_memmap(VirtMachineState *vms) { MachineState *ms = MACHINE(vms); - hwaddr base, device_memory_base, device_memory_size; + hwaddr base, device_memory_base, device_memory_size, memtop; int i; vms->memmap = extended_memmap; @@ -XXX,XX +XXX,XX @@ static void virt_set_memmap(VirtMachineState *vms) device_memory_size = ms->maxram_size - ms->ram_size + ms->ram_slots * GiB; /* Base address of the high IO region */ - base = device_memory_base + ROUND_UP(device_memory_size, GiB); + memtop = base = device_memory_base + ROUND_UP(device_memory_size, GiB); + if (!vms->highmem && memtop > 4 * GiB) { + error_report("highmem=off, but memory crosses the 4GiB limit\n"); + exit(EXIT_FAILURE); + } if (base < device_memory_base) { error_report("maxmem/slots too huge"); exit(EXIT_FAILURE); @@ -XXX,XX +XXX,XX @@ static void virt_set_memmap(VirtMachineState *vms) vms->memmap[i].size = size; base += size; } - vms->highest_gpa = base - 1; + vms->highest_gpa = (vms->highmem ? base : memtop) - 1; if (device_memory_size > 0) { ms->device_memory = g_malloc0(sizeof(*ms->device_memory)); ms->device_memory->base = device_memory_base; -- 2.25.1 From: Marc Zyngier <maz@kernel.org> The highmem attribute is nothing but another way to express the PA range of a VM. To support HW that has a smaller PA range then what QEMU assumes, pass this PA range to the virt_set_memmap() function, allowing it to correctly exclude highmem devices if they are outside of the PA range. Signed-off-by: Marc Zyngier <maz@kernel.org> Reviewed-by: Eric Auger <eric.auger@redhat.com> Message-id: 20220114140741.1358263-5-maz@kernel.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt.c | 64 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 52 insertions(+), 12 deletions(-) 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 uint64_t virt_cpu_mp_affinity(VirtMachineState *vms, int idx) return arm_cpu_mp_affinity(idx, clustersz); } -static void virt_set_memmap(VirtMachineState *vms) +static void virt_set_memmap(VirtMachineState *vms, int pa_bits) { MachineState *ms = MACHINE(vms); hwaddr base, device_memory_base, device_memory_size, memtop; @@ -XXX,XX +XXX,XX @@ static void virt_set_memmap(VirtMachineState *vms) exit(EXIT_FAILURE); } + /* + * !highmem is exactly the same as limiting the PA space to 32bit, + * irrespective of the underlying capabilities of the HW. + */ + if (!vms->highmem) { + pa_bits = 32; + } + /* * We compute the base of the high IO region depending on the * amount of initial and device memory. The device memory start/size @@ -XXX,XX +XXX,XX @@ static void virt_set_memmap(VirtMachineState *vms) /* Base address of the high IO region */ memtop = base = device_memory_base + ROUND_UP(device_memory_size, GiB); - if (!vms->highmem && memtop > 4 * GiB) { - error_report("highmem=off, but memory crosses the 4GiB limit\n"); + if (memtop > BIT_ULL(pa_bits)) { + error_report("Addressing limited to %d bits, but memory exceeds it by %llu bytes\n", + pa_bits, memtop - BIT_ULL(pa_bits)); exit(EXIT_FAILURE); } if (base < device_memory_base) { @@ -XXX,XX +XXX,XX @@ static void virt_set_memmap(VirtMachineState *vms) vms->memmap[i].size = size; base += size; } - vms->highest_gpa = (vms->highmem ? base : memtop) - 1; + + /* + * If base fits within pa_bits, all good. If it doesn't, limit it + * to the end of RAM, which is guaranteed to fit within pa_bits. + */ + vms->highest_gpa = (base <= BIT_ULL(pa_bits) ? base : memtop) - 1; + if (device_memory_size > 0) { ms->device_memory = g_malloc0(sizeof(*ms->device_memory)); ms->device_memory->base = device_memory_base; @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) unsigned int smp_cpus = machine->smp.cpus; unsigned int max_cpus = machine->smp.max_cpus; + if (!cpu_type_valid(machine->cpu_type)) { + error_report("mach-virt: CPU type %s not supported", machine->cpu_type); + exit(1); + } + + possible_cpus = mc->possible_cpu_arch_ids(machine); + /* * In accelerated mode, the memory map is computed earlier in kvm_type() * to create a VM with the right number of IPA bits. */ if (!vms->memmap) { - virt_set_memmap(vms); + Object *cpuobj; + ARMCPU *armcpu; + int pa_bits; + + /* + * Instanciate a temporary CPU object to find out about what + * we are about to deal with. Once this is done, get rid of + * the object. + */ + cpuobj = object_new(possible_cpus->cpus[0].type); + armcpu = ARM_CPU(cpuobj); + + if (object_property_get_bool(cpuobj, "aarch64", NULL)) { + pa_bits = arm_pamax(armcpu); + } else if (arm_feature(&armcpu->env, ARM_FEATURE_LPAE)) { + /* v7 with LPAE */ + pa_bits = 40; + } else { + /* Anything else */ + pa_bits = 32; + } + + object_unref(cpuobj); + + virt_set_memmap(vms, pa_bits); } /* We can probe only here because during property set @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) */ finalize_gic_version(vms); - if (!cpu_type_valid(machine->cpu_type)) { - error_report("mach-virt: CPU type %s not supported", machine->cpu_type); - exit(1); - } - if (vms->secure) { /* * The Secure view of the world is the same as the NonSecure, @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) create_fdt(vms); - possible_cpus = mc->possible_cpu_arch_ids(machine); assert(possible_cpus->len == max_cpus); for (n = 0; n < possible_cpus->len; n++) { Object *cpuobj; @@ -XXX,XX +XXX,XX @@ static int virt_kvm_type(MachineState *ms, const char *type_str) max_vm_pa_size = kvm_arm_get_max_vm_ipa_size(ms, &fixed_ipa); /* we freeze the memory map to compute the highest gpa */ - virt_set_memmap(vms); + virt_set_memmap(vms, max_vm_pa_size); requested_pa_size = 64 - clz64(vms->highest_gpa); -- 2.25.1 From: Marc Zyngier <maz@kernel.org> In order to only keep the highmem devices that actually fit in the PA range, check their location against the range and update highest_gpa if they fit. If they don't, mark them as disabled. Signed-off-by: Marc Zyngier <maz@kernel.org> Reviewed-by: Eric Auger <eric.auger@redhat.com> Message-id: 20220114140741.1358263-6-maz@kernel.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt.c | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) 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 virt_set_memmap(VirtMachineState *vms, int pa_bits) base = vms->memmap[VIRT_MEM].base + LEGACY_RAMLIMIT_BYTES; } + /* We know for sure that at least the memory fits in the PA space */ + vms->highest_gpa = memtop - 1; + for (i = VIRT_LOWMEMMAP_LAST; i < ARRAY_SIZE(extended_memmap); i++) { hwaddr size = extended_memmap[i].size; + bool fits; base = ROUND_UP(base, size); vms->memmap[i].base = base; vms->memmap[i].size = size; + + /* + * Check each device to see if they fit in the PA space, + * moving highest_gpa as we go. + * + * For each device that doesn't fit, disable it. + */ + fits = (base + size) <= BIT_ULL(pa_bits); + if (fits) { + vms->highest_gpa = base + size - 1; + } + + switch (i) { + case VIRT_HIGH_GIC_REDIST2: + vms->highmem_redists &= fits; + break; + case VIRT_HIGH_PCIE_ECAM: + vms->highmem_ecam &= fits; + break; + case VIRT_HIGH_PCIE_MMIO: + vms->highmem_mmio &= fits; + break; + } + base += size; } - /* - * If base fits within pa_bits, all good. If it doesn't, limit it - * to the end of RAM, which is guaranteed to fit within pa_bits. - */ - vms->highest_gpa = (base <= BIT_ULL(pa_bits) ? base : memtop) - 1; - if (device_memory_size > 0) { ms->device_memory = g_malloc0(sizeof(*ms->device_memory)); ms->device_memory->base = device_memory_base; -- 2.25.1 From: Marc Zyngier <maz@kernel.org> Now that the devices present in the extended memory map are checked against the available PA space and disabled when they don't fit, there is no need to keep the same checks against highmem, as highmem really is a shortcut for the PA space being 32bit. Reviewed-by: Eric Auger <eric.auger@redhat.com> Signed-off-by: Marc Zyngier <maz@kernel.org> Message-id: 20220114140741.1358263-7-maz@kernel.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt-acpi-build.c | 2 -- hw/arm/virt.c | 5 +---- 2 files changed, 1 insertion(+), 6 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 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) acpi_add_table(table_offsets, tables_blob); build_fadt_rev5(tables_blob, tables->linker, vms, dsdt); - vms->highmem_redists &= vms->highmem; - acpi_add_table(table_offsets, tables_blob); build_madt(tables_blob, tables->linker, vms); 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 machvirt_init(MachineState *machine) virt_flash_fdt(vms, sysmem, secure_sysmem ?: sysmem); - vms->highmem_mmio &= vms->highmem; - vms->highmem_redists &= vms->highmem; - create_gic(vms, sysmem); virt_cpu_post_init(vms, sysmem); @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) machine->ram_size, "mach-virt.tag"); } - vms->highmem_ecam &= vms->highmem && (!firmware_loaded || aarch64); + vms->highmem_ecam &= (!firmware_loaded || aarch64); create_rtc(vms); -- 2.25.1 From: Patrick Venture <venture@google.com> Reviewed-by: Hao Wu <wuhaotsh@google.com> Signed-off-by: Patrick Venture <venture@google.com> Message-id: 20220111172338.1525587-1-venture@google.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/npcm7xx_boards.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/npcm7xx_boards.c +++ b/hw/arm/npcm7xx_boards.c @@ -XXX,XX +XXX,XX @@ static void kudo_bmc_i2c_init(NPCM7xxState *soc) { I2CSlave *i2c_mux; - i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 1), TYPE_PCA9548, 0x75); + i2c_mux = i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 1), + TYPE_PCA9548, 0x75); + + /* tmp105 is compatible with the lm75 */ + i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 4), "tmp105", 0x5c); + i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 5), "tmp105", 0x5c); + i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 6), "tmp105", 0x5c); + i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 7), "tmp105", 0x5c); + i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 1), TYPE_PCA9548, 0x77); i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 4), TYPE_PCA9548, 0x77); -- 2.25.1 From: Troy Lee <troy_lee@aspeedtech.com> Aspeed 2600 SDK enables I3C support by default. The I3C driver will try to reset the device controller and set it up through device address table register. This dummy model responds to these registers with default values as listed in the ast2600v10 datasheet chapter 54.2. This avoids a guest machine kernel panic due to referencing an invalid kernel address if the device address table register isn't set correctly. Signed-off-by: Troy Lee <troy_lee@aspeedtech.com> Reviewed-by: Graeme Gregory <quic_ggregory@quicinc.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Tested-by: Graeme Gregory <quic_ggregory@quicinc.com> Message-id: 20220111084546.4145785-2-troy_lee@aspeedtech.com [PMM: tidied commit message] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/misc/aspeed_i3c.h | 48 +++++ hw/misc/aspeed_i3c.c | 381 +++++++++++++++++++++++++++++++++++ hw/misc/meson.build | 1 + hw/misc/trace-events | 6 + 4 files changed, 436 insertions(+) create mode 100644 include/hw/misc/aspeed_i3c.h create mode 100644 hw/misc/aspeed_i3c.c diff --git a/include/hw/misc/aspeed_i3c.h b/include/hw/misc/aspeed_i3c.h new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/include/hw/misc/aspeed_i3c.h @@ -XXX,XX +XXX,XX @@ +/* + * ASPEED I3C Controller + * + * Copyright (C) 2021 ASPEED Technology Inc. + * + * This code is licensed under the GPL version 2 or later. See + * the COPYING file in the top-level directory. + */ + +#ifndef ASPEED_I3C_H +#define ASPEED_I3C_H + +#include "hw/sysbus.h" + +#define TYPE_ASPEED_I3C "aspeed.i3c" +#define TYPE_ASPEED_I3C_DEVICE "aspeed.i3c.device" +OBJECT_DECLARE_TYPE(AspeedI3CState, AspeedI3CClass, ASPEED_I3C) + +#define ASPEED_I3C_NR_REGS (0x70 >> 2) +#define ASPEED_I3C_DEVICE_NR_REGS (0x300 >> 2) +#define ASPEED_I3C_NR_DEVICES 6 + +OBJECT_DECLARE_SIMPLE_TYPE(AspeedI3CDevice, ASPEED_I3C_DEVICE) +typedef struct AspeedI3CDevice { + /* <private> */ + SysBusDevice parent; + + /* <public> */ + MemoryRegion mr; + qemu_irq irq; + + uint8_t id; + uint32_t regs[ASPEED_I3C_DEVICE_NR_REGS]; +} AspeedI3CDevice; + +typedef struct AspeedI3CState { + /* <private> */ + SysBusDevice parent; + + /* <public> */ + MemoryRegion iomem; + MemoryRegion iomem_container; + qemu_irq irq; + + uint32_t regs[ASPEED_I3C_NR_REGS]; + AspeedI3CDevice devices[ASPEED_I3C_NR_DEVICES]; +} AspeedI3CState; +#endif /* ASPEED_I3C_H */ diff --git a/hw/misc/aspeed_i3c.c b/hw/misc/aspeed_i3c.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/hw/misc/aspeed_i3c.c @@ -XXX,XX +XXX,XX @@ +/* + * ASPEED I3C Controller + * + * Copyright (C) 2021 ASPEED Technology Inc. + * + * This code is licensed under the GPL version 2 or later. See + * the COPYING file in the top-level directory. + */ + +#include "qemu/osdep.h" +#include "qemu/log.h" +#include "qemu/error-report.h" +#include "hw/misc/aspeed_i3c.h" +#include "hw/registerfields.h" +#include "hw/qdev-properties.h" +#include "qapi/error.h" +#include "migration/vmstate.h" +#include "trace.h" + +/* I3C Controller Registers */ +REG32(I3C1_REG0, 0x10) +REG32(I3C1_REG1, 0x14) + FIELD(I3C1_REG1, I2C_MODE, 0, 1) + FIELD(I3C1_REG1, SA_EN, 15, 1) +REG32(I3C2_REG0, 0x20) +REG32(I3C2_REG1, 0x24) + FIELD(I3C2_REG1, I2C_MODE, 0, 1) + FIELD(I3C2_REG1, SA_EN, 15, 1) +REG32(I3C3_REG0, 0x30) +REG32(I3C3_REG1, 0x34) + FIELD(I3C3_REG1, I2C_MODE, 0, 1) + FIELD(I3C3_REG1, SA_EN, 15, 1) +REG32(I3C4_REG0, 0x40) +REG32(I3C4_REG1, 0x44) + FIELD(I3C4_REG1, I2C_MODE, 0, 1) + FIELD(I3C4_REG1, SA_EN, 15, 1) +REG32(I3C5_REG0, 0x50) +REG32(I3C5_REG1, 0x54) + FIELD(I3C5_REG1, I2C_MODE, 0, 1) + FIELD(I3C5_REG1, SA_EN, 15, 1) +REG32(I3C6_REG0, 0x60) +REG32(I3C6_REG1, 0x64) + FIELD(I3C6_REG1, I2C_MODE, 0, 1) + FIELD(I3C6_REG1, SA_EN, 15, 1) + +/* I3C Device Registers */ +REG32(DEVICE_CTRL, 0x00) +REG32(DEVICE_ADDR, 0x04) +REG32(HW_CAPABILITY, 0x08) +REG32(COMMAND_QUEUE_PORT, 0x0c) +REG32(RESPONSE_QUEUE_PORT, 0x10) +REG32(RX_TX_DATA_PORT, 0x14) +REG32(IBI_QUEUE_STATUS, 0x18) +REG32(IBI_QUEUE_DATA, 0x18) +REG32(QUEUE_THLD_CTRL, 0x1c) +REG32(DATA_BUFFER_THLD_CTRL, 0x20) +REG32(IBI_QUEUE_CTRL, 0x24) +REG32(IBI_MR_REQ_REJECT, 0x2c) +REG32(IBI_SIR_REQ_REJECT, 0x30) +REG32(RESET_CTRL, 0x34) +REG32(SLV_EVENT_CTRL, 0x38) +REG32(INTR_STATUS, 0x3c) +REG32(INTR_STATUS_EN, 0x40) +REG32(INTR_SIGNAL_EN, 0x44) +REG32(INTR_FORCE, 0x48) +REG32(QUEUE_STATUS_LEVEL, 0x4c) +REG32(DATA_BUFFER_STATUS_LEVEL, 0x50) +REG32(PRESENT_STATE, 0x54) +REG32(CCC_DEVICE_STATUS, 0x58) +REG32(DEVICE_ADDR_TABLE_POINTER, 0x5c) + FIELD(DEVICE_ADDR_TABLE_POINTER, DEPTH, 16, 16) + FIELD(DEVICE_ADDR_TABLE_POINTER, ADDR, 0, 16) +REG32(DEV_CHAR_TABLE_POINTER, 0x60) +REG32(VENDOR_SPECIFIC_REG_POINTER, 0x6c) +REG32(SLV_MIPI_PID_VALUE, 0x70) +REG32(SLV_PID_VALUE, 0x74) +REG32(SLV_CHAR_CTRL, 0x78) +REG32(SLV_MAX_LEN, 0x7c) +REG32(MAX_READ_TURNAROUND, 0x80) +REG32(MAX_DATA_SPEED, 0x84) +REG32(SLV_DEBUG_STATUS, 0x88) +REG32(SLV_INTR_REQ, 0x8c) +REG32(DEVICE_CTRL_EXTENDED, 0xb0) +REG32(SCL_I3C_OD_TIMING, 0xb4) +REG32(SCL_I3C_PP_TIMING, 0xb8) +REG32(SCL_I2C_FM_TIMING, 0xbc) +REG32(SCL_I2C_FMP_TIMING, 0xc0) +REG32(SCL_EXT_LCNT_TIMING, 0xc8) +REG32(SCL_EXT_TERMN_LCNT_TIMING, 0xcc) +REG32(BUS_FREE_TIMING, 0xd4) +REG32(BUS_IDLE_TIMING, 0xd8) +REG32(I3C_VER_ID, 0xe0) +REG32(I3C_VER_TYPE, 0xe4) +REG32(EXTENDED_CAPABILITY, 0xe8) +REG32(SLAVE_CONFIG, 0xec) + +static const uint32_t ast2600_i3c_device_resets[ASPEED_I3C_DEVICE_NR_REGS] = { + [R_HW_CAPABILITY] = 0x000e00bf, + [R_QUEUE_THLD_CTRL] = 0x01000101, + [R_I3C_VER_ID] = 0x3130302a, + [R_I3C_VER_TYPE] = 0x6c633033, + [R_DEVICE_ADDR_TABLE_POINTER] = 0x00080280, + [R_DEV_CHAR_TABLE_POINTER] = 0x00020200, + [A_VENDOR_SPECIFIC_REG_POINTER] = 0x000000b0, + [R_SLV_MAX_LEN] = 0x00ff00ff, +}; + +static uint64_t aspeed_i3c_device_read(void *opaque, hwaddr offset, + unsigned size) +{ + AspeedI3CDevice *s = ASPEED_I3C_DEVICE(opaque); + uint32_t addr = offset >> 2; + uint64_t value; + + switch (addr) { + case R_COMMAND_QUEUE_PORT: + value = 0; + break; + default: + value = s->regs[addr]; + break; + } + + trace_aspeed_i3c_device_read(s->id, offset, value); + + return value; +} + +static void aspeed_i3c_device_write(void *opaque, hwaddr offset, + uint64_t value, unsigned size) +{ + AspeedI3CDevice *s = ASPEED_I3C_DEVICE(opaque); + uint32_t addr = offset >> 2; + + trace_aspeed_i3c_device_write(s->id, offset, value); + + switch (addr) { + case R_HW_CAPABILITY: + case R_RESPONSE_QUEUE_PORT: + case R_IBI_QUEUE_DATA: + case R_QUEUE_STATUS_LEVEL: + case R_PRESENT_STATE: + case R_CCC_DEVICE_STATUS: + case R_DEVICE_ADDR_TABLE_POINTER: + case R_VENDOR_SPECIFIC_REG_POINTER: + case R_SLV_CHAR_CTRL: + case R_SLV_MAX_LEN: + case R_MAX_READ_TURNAROUND: + case R_I3C_VER_ID: + case R_I3C_VER_TYPE: + case R_EXTENDED_CAPABILITY: + qemu_log_mask(LOG_GUEST_ERROR, + "%s: write to readonly register[%02lx] = %08lx\n", + __func__, offset, value); + break; + case R_RX_TX_DATA_PORT: + break; + case R_RESET_CTRL: + break; + default: + s->regs[addr] = value; + break; + } +} + +static const VMStateDescription aspeed_i3c_device_vmstate = { + .name = TYPE_ASPEED_I3C, + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]){ + VMSTATE_UINT32_ARRAY(regs, AspeedI3CDevice, ASPEED_I3C_DEVICE_NR_REGS), + VMSTATE_END_OF_LIST(), + } +}; + +static const MemoryRegionOps aspeed_i3c_device_ops = { + .read = aspeed_i3c_device_read, + .write = aspeed_i3c_device_write, + .endianness = DEVICE_LITTLE_ENDIAN, +}; + +static void aspeed_i3c_device_reset(DeviceState *dev) +{ + AspeedI3CDevice *s = ASPEED_I3C_DEVICE(dev); + + memcpy(s->regs, ast2600_i3c_device_resets, sizeof(s->regs)); +} + +static void aspeed_i3c_device_realize(DeviceState *dev, Error **errp) +{ + AspeedI3CDevice *s = ASPEED_I3C_DEVICE(dev); + g_autofree char *name = g_strdup_printf(TYPE_ASPEED_I3C_DEVICE ".%d", + s->id); + + sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq); + + memory_region_init_io(&s->mr, OBJECT(s), &aspeed_i3c_device_ops, + s, name, ASPEED_I3C_DEVICE_NR_REGS << 2); +} + +static uint64_t aspeed_i3c_read(void *opaque, hwaddr addr, unsigned int size) +{ + AspeedI3CState *s = ASPEED_I3C(opaque); + uint64_t val = 0; + + val = s->regs[addr >> 2]; + + trace_aspeed_i3c_read(addr, val); + + return val; +} + +static void aspeed_i3c_write(void *opaque, + hwaddr addr, + uint64_t data, + unsigned int size) +{ + AspeedI3CState *s = ASPEED_I3C(opaque); + + trace_aspeed_i3c_write(addr, data); + + addr >>= 2; + + /* I3C controller register */ + switch (addr) { + case R_I3C1_REG1: + case R_I3C2_REG1: + case R_I3C3_REG1: + case R_I3C4_REG1: + case R_I3C5_REG1: + case R_I3C6_REG1: + if (data & R_I3C1_REG1_I2C_MODE_MASK) { + qemu_log_mask(LOG_UNIMP, + "%s: Not support I2C mode [%08lx]=%08lx", + __func__, addr << 2, data); + break; + } + if (data & R_I3C1_REG1_SA_EN_MASK) { + qemu_log_mask(LOG_UNIMP, + "%s: Not support slave mode [%08lx]=%08lx", + __func__, addr << 2, data); + break; + } + s->regs[addr] = data; + break; + default: + s->regs[addr] = data; + break; + } +} + +static const MemoryRegionOps aspeed_i3c_ops = { + .read = aspeed_i3c_read, + .write = aspeed_i3c_write, + .endianness = DEVICE_LITTLE_ENDIAN, + .valid = { + .min_access_size = 1, + .max_access_size = 4, + } +}; + +static void aspeed_i3c_reset(DeviceState *dev) +{ + AspeedI3CState *s = ASPEED_I3C(dev); + memset(s->regs, 0, sizeof(s->regs)); +} + +static void aspeed_i3c_instance_init(Object *obj) +{ + AspeedI3CState *s = ASPEED_I3C(obj); + int i; + + for (i = 0; i < ASPEED_I3C_NR_DEVICES; ++i) { + object_initialize_child(obj, "device[*]", &s->devices[i], + TYPE_ASPEED_I3C_DEVICE); + } +} + +static void aspeed_i3c_realize(DeviceState *dev, Error **errp) +{ + int i; + AspeedI3CState *s = ASPEED_I3C(dev); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); + + memory_region_init(&s->iomem_container, OBJECT(s), + TYPE_ASPEED_I3C ".container", 0x8000); + + sysbus_init_mmio(sbd, &s->iomem_container); + + memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_i3c_ops, s, + TYPE_ASPEED_I3C ".regs", ASPEED_I3C_NR_REGS << 2); + + memory_region_add_subregion(&s->iomem_container, 0x0, &s->iomem); + + for (i = 0; i < ASPEED_I3C_NR_DEVICES; ++i) { + Object *dev = OBJECT(&s->devices[i]); + + if (!object_property_set_uint(dev, "device-id", i, errp)) { + return; + } + + if (!sysbus_realize(SYS_BUS_DEVICE(dev), errp)) { + return; + } + + /* + * Register Address of I3CX Device = + * (Base Address of Global Register) + (Offset of I3CX) + Offset + * X = 0, 1, 2, 3, 4, 5 + * Offset of I3C0 = 0x2000 + * Offset of I3C1 = 0x3000 + * Offset of I3C2 = 0x4000 + * Offset of I3C3 = 0x5000 + * Offset of I3C4 = 0x6000 + * Offset of I3C5 = 0x7000 + */ + memory_region_add_subregion(&s->iomem_container, + 0x2000 + i * 0x1000, &s->devices[i].mr); + } + +} + +static Property aspeed_i3c_device_properties[] = { + DEFINE_PROP_UINT8("device-id", AspeedI3CDevice, id, 0), + DEFINE_PROP_END_OF_LIST(), +}; + +static void aspeed_i3c_device_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + + dc->desc = "Aspeed I3C Device"; + dc->realize = aspeed_i3c_device_realize; + dc->reset = aspeed_i3c_device_reset; + device_class_set_props(dc, aspeed_i3c_device_properties); +} + +static const TypeInfo aspeed_i3c_device_info = { + .name = TYPE_ASPEED_I3C_DEVICE, + .parent = TYPE_SYS_BUS_DEVICE, + .instance_size = sizeof(AspeedI3CDevice), + .class_init = aspeed_i3c_device_class_init, +}; + +static const VMStateDescription vmstate_aspeed_i3c = { + .name = TYPE_ASPEED_I3C, + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT32_ARRAY(regs, AspeedI3CState, ASPEED_I3C_NR_REGS), + VMSTATE_STRUCT_ARRAY(devices, AspeedI3CState, ASPEED_I3C_NR_DEVICES, 1, + aspeed_i3c_device_vmstate, AspeedI3CDevice), + VMSTATE_END_OF_LIST(), + } +}; + +static void aspeed_i3c_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + + dc->realize = aspeed_i3c_realize; + dc->reset = aspeed_i3c_reset; + dc->desc = "Aspeed I3C Controller"; + dc->vmsd = &vmstate_aspeed_i3c; +} + +static const TypeInfo aspeed_i3c_info = { + .name = TYPE_ASPEED_I3C, + .parent = TYPE_SYS_BUS_DEVICE, + .instance_init = aspeed_i3c_instance_init, + .instance_size = sizeof(AspeedI3CState), + .class_init = aspeed_i3c_class_init, +}; + +static void aspeed_i3c_register_types(void) +{ + type_register_static(&aspeed_i3c_device_info); + type_register_static(&aspeed_i3c_info); +} + +type_init(aspeed_i3c_register_types); diff --git a/hw/misc/meson.build b/hw/misc/meson.build index XXXXXXX..XXXXXXX 100644 --- a/hw/misc/meson.build +++ b/hw/misc/meson.build @@ -XXX,XX +XXX,XX @@ softmmu_ss.add(when: 'CONFIG_PVPANIC_PCI', if_true: files('pvpanic-pci.c')) softmmu_ss.add(when: 'CONFIG_AUX', if_true: files('auxbus.c')) softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files( 'aspeed_hace.c', + 'aspeed_i3c.c', 'aspeed_lpc.c', 'aspeed_scu.c', 'aspeed_sdmc.c', diff --git a/hw/misc/trace-events b/hw/misc/trace-events index XXXXXXX..XXXXXXX 100644 --- a/hw/misc/trace-events +++ b/hw/misc/trace-events @@ -XXX,XX +XXX,XX @@ armsse_mhu_write(uint64_t offset, uint64_t data, unsigned size) "SSE-200 MHU wri # aspeed_xdma.c aspeed_xdma_write(uint64_t offset, uint64_t data) "XDMA write: offset 0x%" PRIx64 " data 0x%" PRIx64 +# aspeed_i3c.c +aspeed_i3c_read(uint64_t offset, uint64_t data) "I3C read: offset 0x%" PRIx64 " data 0x%" PRIx64 +aspeed_i3c_write(uint64_t offset, uint64_t data) "I3C write: offset 0x%" PRIx64 " data 0x%" PRIx64 +aspeed_i3c_device_read(uint32_t deviceid, uint64_t offset, uint64_t data) "I3C Dev[%u] read: offset 0x%" PRIx64 " data 0x%" PRIx64 +aspeed_i3c_device_write(uint32_t deviceid, uint64_t offset, uint64_t data) "I3C Dev[%u] write: offset 0x%" PRIx64 " data 0x%" PRIx64 + # bcm2835_property.c bcm2835_mbox_property(uint32_t tag, uint32_t bufsize, size_t resplen) "mbox property tag:0x%08x in_sz:%u out_sz:%zu" -- 2.25.1 From: Troy Lee <troy_lee@aspeedtech.com> Add the new i3c device to the AST2600 SoC. Signed-off-by: Troy Lee <troy_lee@aspeedtech.com> Reviewed-by: Graeme Gregory <quic_ggregory@quicinc.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Tested-by: Graeme Gregory <quic_ggregory@quicinc.com> Message-id: 20220111084546.4145785-3-troy_lee@aspeedtech.com [PMM: tidied commit message] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/aspeed_soc.h | 3 +++ hw/arm/aspeed_ast2600.c | 16 ++++++++++++++++ 2 files changed, 19 insertions(+) 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 @@ #include "hw/timer/aspeed_timer.h" #include "hw/rtc/aspeed_rtc.h" #include "hw/i2c/aspeed_i2c.h" +#include "hw/misc/aspeed_i3c.h" #include "hw/ssi/aspeed_smc.h" #include "hw/misc/aspeed_hace.h" #include "hw/watchdog/wdt_aspeed.h" @@ -XXX,XX +XXX,XX @@ struct AspeedSoCState { AspeedRtcState rtc; AspeedTimerCtrlState timerctrl; AspeedI2CState i2c; + AspeedI3CState i3c; AspeedSCUState scu; AspeedHACEState hace; AspeedXDMAState xdma; @@ -XXX,XX +XXX,XX @@ enum { ASPEED_DEV_HACE, ASPEED_DEV_DPMCU, ASPEED_DEV_DP, + ASPEED_DEV_I3C, }; #endif /* ASPEED_SOC_H */ 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 const hwaddr aspeed_soc_ast2600_memmap[] = { [ASPEED_DEV_UART1] = 0x1E783000, [ASPEED_DEV_UART5] = 0x1E784000, [ASPEED_DEV_VUART] = 0x1E787000, + [ASPEED_DEV_I3C] = 0x1E7A0000, [ASPEED_DEV_SDRAM] = 0x80000000, }; @@ -XXX,XX +XXX,XX @@ static const int aspeed_soc_ast2600_irqmap[] = { [ASPEED_DEV_ETH4] = 33, [ASPEED_DEV_KCS] = 138, /* 138 -> 142 */ [ASPEED_DEV_DP] = 62, + [ASPEED_DEV_I3C] = 102, /* 102 -> 107 */ }; static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl) @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_init(Object *obj) snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname); object_initialize_child(obj, "hace", &s->hace, typename); + + object_initialize_child(obj, "i3c", &s->i3c, TYPE_ASPEED_I3C); } /* @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp) sysbus_mmio_map(SYS_BUS_DEVICE(&s->hace), 0, sc->memmap[ASPEED_DEV_HACE]); sysbus_connect_irq(SYS_BUS_DEVICE(&s->hace), 0, aspeed_soc_get_irq(s, ASPEED_DEV_HACE)); + + /* I3C */ + if (!sysbus_realize(SYS_BUS_DEVICE(&s->i3c), errp)) { + return; + } + sysbus_mmio_map(SYS_BUS_DEVICE(&s->i3c), 0, sc->memmap[ASPEED_DEV_I3C]); + for (i = 0; i < ASPEED_I3C_NR_DEVICES; i++) { + qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore), + sc->irqmap[ASPEED_DEV_I3C] + i); + /* The AST2600 I3C controller has one IRQ per bus. */ + sysbus_connect_irq(SYS_BUS_DEVICE(&s->i3c.devices[i]), 0, irq); + } } static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data) -- 2.25.1 In process_its_cmd() and process_mapti() we must check the event ID against a limit defined by the size field in the DTE, which specifies the number of ID bits minus one. Convert this code to our num_foo convention: * change the variable names * use uint64_t and 1ULL when calculating the number of valid event IDs, because DTE.SIZE is 5 bits and so num_eventids may be up to 2^32 * fix the off-by-one error in the comparison Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20220111171048.3545974-2-peter.maydell@linaro.org --- hw/intc/arm_gicv3_its.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -XXX,XX +XXX,XX @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset, MemTxResult res = MEMTX_OK; bool dte_valid; uint64_t dte = 0; - uint32_t max_eventid; + uint64_t num_eventids; uint16_t icid = 0; uint32_t pIntid = 0; bool ite_valid = false; @@ -XXX,XX +XXX,XX @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset, dte_valid = FIELD_EX64(dte, DTE, VALID); if (dte_valid) { - max_eventid = 1UL << (FIELD_EX64(dte, DTE, SIZE) + 1); + num_eventids = 1ULL << (FIELD_EX64(dte, DTE, SIZE) + 1); ite_valid = get_ite(s, eventid, dte, &icid, &pIntid, &res); @@ -XXX,XX +XXX,XX @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset, dte_valid ? "valid" : "invalid", ite_valid ? "valid" : "invalid", cte_valid ? "valid" : "invalid"); - } else if (eventid > max_eventid) { + } else if (eventid >= num_eventids) { qemu_log_mask(LOG_GUEST_ERROR, - "%s: invalid command attributes: eventid %d > %d\n", - __func__, eventid, max_eventid); + "%s: invalid command attributes: eventid %d >= %" + PRId64 "\n", + __func__, eventid, num_eventids); } else { /* * Current implementation only supports rdbase == procnum @@ -XXX,XX +XXX,XX @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset, AddressSpace *as = &s->gicv3->dma_as; uint32_t devid, eventid; uint32_t pIntid = 0; - uint32_t max_eventid, max_Intid; + uint64_t num_eventids; + uint32_t max_Intid; bool dte_valid; MemTxResult res = MEMTX_OK; uint16_t icid = 0; @@ -XXX,XX +XXX,XX @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset, return result; } dte_valid = FIELD_EX64(dte, DTE, VALID); - max_eventid = 1UL << (FIELD_EX64(dte, DTE, SIZE) + 1); + num_eventids = 1ULL << (FIELD_EX64(dte, DTE, SIZE) + 1); max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1; if ((devid >= s->dt.num_ids) || (icid >= s->ct.num_ids) - || !dte_valid || (eventid > max_eventid) || + || !dte_valid || (eventid >= num_eventids) || (((pIntid < GICV3_LPI_INTID_START) || (pIntid > max_Intid)) && (pIntid != INTID_SPURIOUS))) { qemu_log_mask(LOG_GUEST_ERROR, -- 2.25.1 The bounds check on the number of interrupt IDs is correct, but doesn't match our convention; change the variable name, initialize it to the 2^n value rather than (2^n)-1, and use >= instead of > in the comparison. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20220111171048.3545974-3-peter.maydell@linaro.org --- hw/intc/arm_gicv3_its.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -XXX,XX +XXX,XX @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset, uint32_t devid, eventid; uint32_t pIntid = 0; uint64_t num_eventids; - uint32_t max_Intid; + uint32_t num_intids; bool dte_valid; MemTxResult res = MEMTX_OK; uint16_t icid = 0; @@ -XXX,XX +XXX,XX @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset, } dte_valid = FIELD_EX64(dte, DTE, VALID); num_eventids = 1ULL << (FIELD_EX64(dte, DTE, SIZE) + 1); - max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1; + num_intids = 1ULL << (GICD_TYPER_IDBITS + 1); if ((devid >= s->dt.num_ids) || (icid >= s->ct.num_ids) || !dte_valid || (eventid >= num_eventids) || - (((pIntid < GICV3_LPI_INTID_START) || (pIntid > max_Intid)) && + (((pIntid < GICV3_LPI_INTID_START) || (pIntid >= num_intids)) && (pIntid != INTID_SPURIOUS))) { qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid command attributes " -- 2.25.1 process_its_cmd() returns a bool, like all the other process_ functions. However we were putting its return value into 'res', not 'result', which meant we would ignore it when deciding whether to continue or stall the command queue. Fix the typo. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20220111171048.3545974-4-peter.maydell@linaro.org --- hw/intc/arm_gicv3_its.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -XXX,XX +XXX,XX @@ static void process_cmdq(GICv3ITSState *s) switch (cmd) { case GITS_CMD_INT: - res = process_its_cmd(s, data, cq_offset, INTERRUPT); + result = process_its_cmd(s, data, cq_offset, INTERRUPT); break; case GITS_CMD_CLEAR: - res = process_its_cmd(s, data, cq_offset, CLEAR); + result = process_its_cmd(s, data, cq_offset, CLEAR); break; case GITS_CMD_SYNC: /* -- 2.25.1 In process_cmdq(), we read 64 bits of the command packet, which contain the command identifier, which we then switch() on to dispatch to an appropriate sub-function. However, if address_space_ldq_le() reports a memory transaction failure, we still read the command identifier out of the data and switch() on it. Restructure the code so that we stop immediately (stalling the command queue) in this case. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220111171048.3545974-5-peter.maydell@linaro.org --- hw/intc/arm_gicv3_its.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -XXX,XX +XXX,XX @@ static void process_cmdq(GICv3ITSState *s) data = address_space_ldq_le(as, s->cq.base_addr + cq_offset, MEMTXATTRS_UNSPECIFIED, &res); if (res != MEMTX_OK) { - result = false; + s->creadr = FIELD_DP64(s->creadr, GITS_CREADR, STALLED, 1); + qemu_log_mask(LOG_GUEST_ERROR, + "%s: could not read command at 0x%" PRIx64 "\n", + __func__, s->cq.base_addr + cq_offset); + break; } + cmd = (data & CMD_MASK); switch (cmd) { -- 2.25.1 When an ITS detects an error in a command, it has an implementation-defined (CONSTRAINED UNPREDICTABLE) choice of whether to ignore the command, proceeding to the next one in the queue, or to stall the ITS command queue, processing nothing further. The behaviour required when the read of the command packet from memory fails is less clearly documented, but the same set of choices as for command errors seem reasonable. The intention of the QEMU implementation, as documented in the comments, is that if we encounter a memory error reading the command packet or one of the various data tables then we should stall, but for command parameter errors we should ignore the queue and continue. However, we don't actually do this. To get the desired behaviour, the various process_* functions need to return true to cause process_cmdq() to advance to the next command and keep processing, and false to stall command processing. What they mostly do is return false for any kind of error. To make the code clearer, replace the 'bool' return from the process_ functions with an enum which may be either CMD_STALL or CMD_CONTINUE. In this commit no behaviour changes; in subsequent commits we will adjust the error-return paths for the process_ functions one by one. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20220111171048.3545974-6-peter.maydell@linaro.org --- hw/intc/arm_gicv3_its.c | 59 ++++++++++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 21 deletions(-) diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -XXX,XX +XXX,XX @@ typedef struct { uint64_t itel; } IteEntry; +/* + * The ITS spec permits a range of CONSTRAINED UNPREDICTABLE options + * if a command parameter is not correct. These include both "stall + * processing of the command queue" and "ignore this command, and + * keep processing the queue". In our implementation we choose that + * memory transaction errors reading the command packet provoke a + * stall, but errors in parameters cause us to ignore the command + * and continue processing. + * The process_* functions which handle individual ITS commands all + * return an ItsCmdResult which tells process_cmdq() whether it should + * stall or keep going. + */ +typedef enum ItsCmdResult { + CMD_STALL = 0, + CMD_CONTINUE = 1, +} ItsCmdResult; + static uint64_t baser_base_addr(uint64_t value, uint32_t page_sz) { uint64_t result = 0; @@ -XXX,XX +XXX,XX @@ static uint64_t get_dte(GICv3ITSState *s, uint32_t devid, MemTxResult *res) * 3. handling of ITS CLEAR command * 4. handling of ITS DISCARD command */ -static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset, - ItsCmdType cmd) +static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value, + uint32_t offset, ItsCmdType cmd) { AddressSpace *as = &s->gicv3->dma_as; uint32_t devid, eventid; @@ -XXX,XX +XXX,XX @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset, bool ite_valid = false; uint64_t cte = 0; bool cte_valid = false; - bool result = false; + ItsCmdResult result = CMD_STALL; uint64_t rdbase; if (cmd == NONE) { @@ -XXX,XX +XXX,XX @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset, if (cmd == DISCARD) { IteEntry ite = {}; /* remove mapping from interrupt translation table */ - result = update_ite(s, eventid, dte, ite); + result = update_ite(s, eventid, dte, ite) ? CMD_CONTINUE : CMD_STALL; } } return result; } -static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset, - bool ignore_pInt) +static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value, + uint32_t offset, bool ignore_pInt) { AddressSpace *as = &s->gicv3->dma_as; uint32_t devid, eventid; @@ -XXX,XX +XXX,XX @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset, MemTxResult res = MEMTX_OK; uint16_t icid = 0; uint64_t dte = 0; - bool result = false; + ItsCmdResult result = CMD_STALL; devid = ((value & DEVID_MASK) >> DEVID_SHIFT); offset += NUM_BYTES_IN_DW; @@ -XXX,XX +XXX,XX @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset, ite.itel = FIELD_DP64(ite.itel, ITE_L, DOORBELL, INTID_SPURIOUS); ite.iteh = FIELD_DP32(ite.iteh, ITE_H, ICID, icid); - result = update_ite(s, eventid, dte, ite); + result = update_ite(s, eventid, dte, ite) ? CMD_CONTINUE : CMD_STALL; } return result; @@ -XXX,XX +XXX,XX @@ static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid, } } -static bool process_mapc(GICv3ITSState *s, uint32_t offset) +static ItsCmdResult process_mapc(GICv3ITSState *s, uint32_t offset) { AddressSpace *as = &s->gicv3->dma_as; uint16_t icid; uint64_t rdbase; bool valid; MemTxResult res = MEMTX_OK; - bool result = false; + ItsCmdResult result = CMD_STALL; uint64_t value; offset += NUM_BYTES_IN_DW; @@ -XXX,XX +XXX,XX @@ static bool process_mapc(GICv3ITSState *s, uint32_t offset) * command in the queue */ } else { - result = update_cte(s, icid, valid, rdbase); + result = update_cte(s, icid, valid, rdbase) ? CMD_CONTINUE : CMD_STALL; } return result; @@ -XXX,XX +XXX,XX @@ static bool update_dte(GICv3ITSState *s, uint32_t devid, bool valid, } } -static bool process_mapd(GICv3ITSState *s, uint64_t value, uint32_t offset) +static ItsCmdResult process_mapd(GICv3ITSState *s, uint64_t value, + uint32_t offset) { AddressSpace *as = &s->gicv3->dma_as; uint32_t devid; @@ -XXX,XX +XXX,XX @@ static bool process_mapd(GICv3ITSState *s, uint64_t value, uint32_t offset) uint64_t itt_addr; bool valid; MemTxResult res = MEMTX_OK; - bool result = false; + ItsCmdResult result = CMD_STALL; devid = ((value & DEVID_MASK) >> DEVID_SHIFT); @@ -XXX,XX +XXX,XX @@ static bool process_mapd(GICv3ITSState *s, uint64_t value, uint32_t offset) * command in the queue */ } else { - result = update_dte(s, devid, valid, size, itt_addr); + result = update_dte(s, devid, valid, size, itt_addr) ? CMD_CONTINUE : CMD_STALL; } return result; @@ -XXX,XX +XXX,XX @@ static void process_cmdq(GICv3ITSState *s) uint64_t data; AddressSpace *as = &s->gicv3->dma_as; MemTxResult res = MEMTX_OK; - bool result = true; uint8_t cmd; int i; @@ -XXX,XX +XXX,XX @@ static void process_cmdq(GICv3ITSState *s) } while (wr_offset != rd_offset) { + ItsCmdResult result = CMD_CONTINUE; + cq_offset = (rd_offset * GITS_CMDQ_ENTRY_SIZE); data = address_space_ldq_le(as, s->cq.base_addr + cq_offset, MEMTXATTRS_UNSPECIFIED, &res); @@ -XXX,XX +XXX,XX @@ static void process_cmdq(GICv3ITSState *s) default: break; } - if (result) { + if (result == CMD_CONTINUE) { rd_offset++; rd_offset %= s->cq.num_entries; s->creadr = FIELD_DP64(s->creadr, GITS_CREADR, OFFSET, rd_offset); } else { - /* - * in this implementation, in case of dma read/write error - * we stall the command processing - */ + /* CMD_STALL */ s->creadr = FIELD_DP64(s->creadr, GITS_CREADR, STALLED, 1); qemu_log_mask(LOG_GUEST_ERROR, - "%s: %x cmd processing failed\n", __func__, cmd); + "%s: 0x%x cmd processing failed, stalling\n", + __func__, cmd); break; } } -- 2.25.1 Fix process_its_cmd() to consistently return CMD_STALL for memory errors and CMD_CONTINUE for parameter errors, as we claim in the comments that we do. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220111171048.3545974-7-peter.maydell@linaro.org --- hw/intc/arm_gicv3_its.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value, bool ite_valid = false; uint64_t cte = 0; bool cte_valid = false; - ItsCmdResult result = CMD_STALL; uint64_t rdbase; if (cmd == NONE) { @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value, } if (res != MEMTX_OK) { - return result; + return CMD_STALL; } eventid = (value & EVENTID_MASK); @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value, dte = get_dte(s, devid, &res); if (res != MEMTX_OK) { - return result; + return CMD_STALL; } dte_valid = FIELD_EX64(dte, DTE, VALID); @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value, ite_valid = get_ite(s, eventid, dte, &icid, &pIntid, &res); if (res != MEMTX_OK) { - return result; + return CMD_STALL; } if (ite_valid) { @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value, } if (res != MEMTX_OK) { - return result; + return CMD_STALL; } } else { qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid command attributes: " "invalid dte: %"PRIx64" for %d (MEM_TX: %d)\n", __func__, dte, devid, res); - return result; + return CMD_CONTINUE; } @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value, qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid command attributes: devid %d>=%d", __func__, devid, s->dt.num_ids); - + return CMD_CONTINUE; } else if (!dte_valid || !ite_valid || !cte_valid) { qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid command attributes: " @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value, dte_valid ? "valid" : "invalid", ite_valid ? "valid" : "invalid", cte_valid ? "valid" : "invalid"); + return CMD_CONTINUE; } else if (eventid >= num_eventids) { qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid command attributes: eventid %d >= %" PRId64 "\n", __func__, eventid, num_eventids); + return CMD_CONTINUE; } else { /* * Current implementation only supports rdbase == procnum @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value, rdbase = FIELD_EX64(cte, CTE, RDBASE); if (rdbase >= s->gicv3->num_cpu) { - return result; + return CMD_CONTINUE; } if ((cmd == CLEAR) || (cmd == DISCARD)) { @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value, if (cmd == DISCARD) { IteEntry ite = {}; /* remove mapping from interrupt translation table */ - result = update_ite(s, eventid, dte, ite) ? CMD_CONTINUE : CMD_STALL; + return update_ite(s, eventid, dte, ite) ? CMD_CONTINUE : CMD_STALL; } + return CMD_CONTINUE; } - - return result; } static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value, -- 2.25.1 Refactor process_its_cmd() so that it consistently uses the structure do thing; if (error condition) { return early; } do next thing; rather than doing some of the work nested inside if (not error) code blocks. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220111171048.3545974-8-peter.maydell@linaro.org --- hw/intc/arm_gicv3_its.c | 103 +++++++++++++++++++--------------------- 1 file changed, 50 insertions(+), 53 deletions(-) diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value, } dte_valid = FIELD_EX64(dte, DTE, VALID); - if (dte_valid) { - num_eventids = 1ULL << (FIELD_EX64(dte, DTE, SIZE) + 1); - - ite_valid = get_ite(s, eventid, dte, &icid, &pIntid, &res); - - if (res != MEMTX_OK) { - return CMD_STALL; - } - - if (ite_valid) { - cte_valid = get_cte(s, icid, &cte, &res); - } - - if (res != MEMTX_OK) { - return CMD_STALL; - } - } else { + if (!dte_valid) { qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid command attributes: " - "invalid dte: %"PRIx64" for %d (MEM_TX: %d)\n", - __func__, dte, devid, res); + "invalid dte: %"PRIx64" for %d\n", + __func__, dte, devid); return CMD_CONTINUE; } + num_eventids = 1ULL << (FIELD_EX64(dte, DTE, SIZE) + 1); + + ite_valid = get_ite(s, eventid, dte, &icid, &pIntid, &res); + if (res != MEMTX_OK) { + return CMD_STALL; + } + + if (!ite_valid) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: invalid command attributes: invalid ITE\n", + __func__); + return CMD_CONTINUE; + } + + cte_valid = get_cte(s, icid, &cte, &res); + if (res != MEMTX_OK) { + return CMD_STALL; + } + if (!cte_valid) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: invalid command attributes: " + "invalid cte: %"PRIx64"\n", + __func__, cte); + return CMD_CONTINUE; + } - /* - * In this implementation, in case of guest errors we ignore the - * command and move onto the next command in the queue. - */ if (devid >= s->dt.num_ids) { qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid command attributes: devid %d>=%d", __func__, devid, s->dt.num_ids); return CMD_CONTINUE; - } else if (!dte_valid || !ite_valid || !cte_valid) { - qemu_log_mask(LOG_GUEST_ERROR, - "%s: invalid command attributes: " - "dte: %s, ite: %s, cte: %s\n", - __func__, - dte_valid ? "valid" : "invalid", - ite_valid ? "valid" : "invalid", - cte_valid ? "valid" : "invalid"); - return CMD_CONTINUE; - } else if (eventid >= num_eventids) { + } + if (eventid >= num_eventids) { qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid command attributes: eventid %d >= %" PRId64 "\n", __func__, eventid, num_eventids); return CMD_CONTINUE; - } else { - /* - * Current implementation only supports rdbase == procnum - * Hence rdbase physical address is ignored - */ - rdbase = FIELD_EX64(cte, CTE, RDBASE); + } - if (rdbase >= s->gicv3->num_cpu) { - return CMD_CONTINUE; - } + /* + * Current implementation only supports rdbase == procnum + * Hence rdbase physical address is ignored + */ + rdbase = FIELD_EX64(cte, CTE, RDBASE); - if ((cmd == CLEAR) || (cmd == DISCARD)) { - gicv3_redist_process_lpi(&s->gicv3->cpu[rdbase], pIntid, 0); - } else { - gicv3_redist_process_lpi(&s->gicv3->cpu[rdbase], pIntid, 1); - } - - if (cmd == DISCARD) { - IteEntry ite = {}; - /* remove mapping from interrupt translation table */ - return update_ite(s, eventid, dte, ite) ? CMD_CONTINUE : CMD_STALL; - } + if (rdbase >= s->gicv3->num_cpu) { return CMD_CONTINUE; } + + if ((cmd == CLEAR) || (cmd == DISCARD)) { + gicv3_redist_process_lpi(&s->gicv3->cpu[rdbase], pIntid, 0); + } else { + gicv3_redist_process_lpi(&s->gicv3->cpu[rdbase], pIntid, 1); + } + + if (cmd == DISCARD) { + IteEntry ite = {}; + /* remove mapping from interrupt translation table */ + return update_ite(s, eventid, dte, ite) ? CMD_CONTINUE : CMD_STALL; + } + return CMD_CONTINUE; } static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value, -- 2.25.1 Fix process_mapti() to consistently return CMD_STALL for memory errors and CMD_CONTINUE for parameter errors, as we claim in the comments that we do. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220111171048.3545974-9-peter.maydell@linaro.org --- hw/intc/arm_gicv3_its.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value, MemTxResult res = MEMTX_OK; uint16_t icid = 0; uint64_t dte = 0; - ItsCmdResult result = CMD_STALL; + IteEntry ite = {}; devid = ((value & DEVID_MASK) >> DEVID_SHIFT); offset += NUM_BYTES_IN_DW; @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value, MEMTXATTRS_UNSPECIFIED, &res); if (res != MEMTX_OK) { - return result; + return CMD_STALL; } eventid = (value & EVENTID_MASK); @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value, MEMTXATTRS_UNSPECIFIED, &res); if (res != MEMTX_OK) { - return result; + return CMD_STALL; } icid = value & ICID_MASK; @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value, dte = get_dte(s, devid, &res); if (res != MEMTX_OK) { - return result; + return CMD_STALL; } dte_valid = FIELD_EX64(dte, DTE, VALID); num_eventids = 1ULL << (FIELD_EX64(dte, DTE, SIZE) + 1); @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value, * we ignore this command and move onto the next * command in the queue */ - } else { - /* add ite entry to interrupt translation table */ - IteEntry ite = {}; - ite.itel = FIELD_DP64(ite.itel, ITE_L, VALID, dte_valid); - ite.itel = FIELD_DP64(ite.itel, ITE_L, INTTYPE, ITE_INTTYPE_PHYSICAL); - ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, pIntid); - ite.itel = FIELD_DP64(ite.itel, ITE_L, DOORBELL, INTID_SPURIOUS); - ite.iteh = FIELD_DP32(ite.iteh, ITE_H, ICID, icid); - - result = update_ite(s, eventid, dte, ite) ? CMD_CONTINUE : CMD_STALL; + return CMD_CONTINUE; } - return result; + /* add ite entry to interrupt translation table */ + ite.itel = FIELD_DP64(ite.itel, ITE_L, VALID, dte_valid); + ite.itel = FIELD_DP64(ite.itel, ITE_L, INTTYPE, ITE_INTTYPE_PHYSICAL); + ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, pIntid); + ite.itel = FIELD_DP64(ite.itel, ITE_L, DOORBELL, INTID_SPURIOUS); + ite.iteh = FIELD_DP32(ite.iteh, ITE_H, ICID, icid); + + return update_ite(s, eventid, dte, ite) ? CMD_CONTINUE : CMD_STALL; } static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid, -- 2.25.1 Fix process_mapc() to consistently return CMD_STALL for memory errors and CMD_CONTINUE for parameter errors, as we claim in the comments that we do. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220111171048.3545974-10-peter.maydell@linaro.org --- hw/intc/arm_gicv3_its.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapc(GICv3ITSState *s, uint32_t offset) uint64_t rdbase; bool valid; MemTxResult res = MEMTX_OK; - ItsCmdResult result = CMD_STALL; uint64_t value; offset += NUM_BYTES_IN_DW; @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapc(GICv3ITSState *s, uint32_t offset) MEMTXATTRS_UNSPECIFIED, &res); if (res != MEMTX_OK) { - return result; + return CMD_STALL; } icid = value & ICID_MASK; @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapc(GICv3ITSState *s, uint32_t offset) * we ignore this command and move onto the next * command in the queue */ - } else { - result = update_cte(s, icid, valid, rdbase) ? CMD_CONTINUE : CMD_STALL; + return CMD_CONTINUE; } - return result; + return update_cte(s, icid, valid, rdbase) ? CMD_CONTINUE : CMD_STALL; } static bool update_dte(GICv3ITSState *s, uint32_t devid, bool valid, -- 2.25.1 Fix process_mapd() to consistently return CMD_STALL for memory errors and CMD_CONTINUE for parameter errors, as we claim in the comments that we do. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220111171048.3545974-11-peter.maydell@linaro.org --- hw/intc/arm_gicv3_its.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapd(GICv3ITSState *s, uint64_t value, uint64_t itt_addr; bool valid; MemTxResult res = MEMTX_OK; - ItsCmdResult result = CMD_STALL; devid = ((value & DEVID_MASK) >> DEVID_SHIFT); @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapd(GICv3ITSState *s, uint64_t value, MEMTXATTRS_UNSPECIFIED, &res); if (res != MEMTX_OK) { - return result; + return CMD_STALL; } size = (value & SIZE_MASK); @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapd(GICv3ITSState *s, uint64_t value, MEMTXATTRS_UNSPECIFIED, &res); if (res != MEMTX_OK) { - return result; + return CMD_STALL; } itt_addr = (value & ITTADDR_MASK) >> ITTADDR_SHIFT; @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapd(GICv3ITSState *s, uint64_t value, * we ignore this command and move onto the next * command in the queue */ - } else { - result = update_dte(s, devid, valid, size, itt_addr) ? CMD_CONTINUE : CMD_STALL; + return CMD_CONTINUE; } - return result; + return update_dte(s, devid, valid, size, itt_addr) ? CMD_CONTINUE : CMD_STALL; } /* -- 2.25.1 The ITS has several tables which all share a similar format, described by the TableDesc struct: the guest may configure them to be a single-level table or a two-level table. Currently we open-code the process of finding the table entry in all the functions which read or write the device table or the collection table. Factor out the "get the address of the table entry" logic into a new function, so that the code which needs to read or write a table entry only needs to call table_entry_addr() and then perform a suitable load or store to that address. Note that the error handling is slightly complicated because we want to handle two cases differently: * failure to read the L1 table entry should end up causing a command stall, like other kinds of DMA error * an L1 table entry that says there is no L2 table for this index (ie whose valid bit is 0) must result in us treating the table entry as not-valid on read, and discarding writes (this is mandated by the spec) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20220111171048.3545974-12-peter.maydell@linaro.org --- hw/intc/arm_gicv3_its.c | 212 +++++++++++++--------------------------- 1 file changed, 70 insertions(+), 142 deletions(-) diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -XXX,XX +XXX,XX @@ static uint64_t baser_base_addr(uint64_t value, uint32_t page_sz) return result; } +static uint64_t table_entry_addr(GICv3ITSState *s, TableDesc *td, + uint32_t idx, MemTxResult *res) +{ + /* + * Given a TableDesc describing one of the ITS in-guest-memory + * tables and an index into it, return the guest address + * corresponding to that table entry. + * If there was a memory error reading the L1 table of an + * indirect table, *res is set accordingly, and we return -1. + * If the L1 table entry is marked not valid, we return -1 with + * *res set to MEMTX_OK. + * + * The specification defines the format of level 1 entries of a + * 2-level table, but the format of level 2 entries and the format + * of flat-mapped tables is IMPDEF. + */ + AddressSpace *as = &s->gicv3->dma_as; + uint32_t l2idx; + uint64_t l2; + uint32_t num_l2_entries; + + *res = MEMTX_OK; + + if (!td->indirect) { + /* Single level table */ + return td->base_addr + idx * td->entry_sz; + } + + /* Two level table */ + l2idx = idx / (td->page_sz / L1TABLE_ENTRY_SIZE); + + l2 = address_space_ldq_le(as, + td->base_addr + (l2idx * L1TABLE_ENTRY_SIZE), + MEMTXATTRS_UNSPECIFIED, res); + if (*res != MEMTX_OK) { + return -1; + } + if (!(l2 & L2_TABLE_VALID_MASK)) { + return -1; + } + + num_l2_entries = td->page_sz / td->entry_sz; + return (l2 & ((1ULL << 51) - 1)) + (idx % num_l2_entries) * td->entry_sz; +} + static bool get_cte(GICv3ITSState *s, uint16_t icid, uint64_t *cte, MemTxResult *res) { AddressSpace *as = &s->gicv3->dma_as; - uint64_t l2t_addr; - uint64_t value; - bool valid_l2t; - uint32_t l2t_id; - uint32_t num_l2_entries; + uint64_t entry_addr = table_entry_addr(s, &s->ct, icid, res); - if (s->ct.indirect) { - l2t_id = icid / (s->ct.page_sz / L1TABLE_ENTRY_SIZE); - - value = address_space_ldq_le(as, - s->ct.base_addr + - (l2t_id * L1TABLE_ENTRY_SIZE), - MEMTXATTRS_UNSPECIFIED, res); - - if (*res == MEMTX_OK) { - valid_l2t = (value & L2_TABLE_VALID_MASK) != 0; - - if (valid_l2t) { - num_l2_entries = s->ct.page_sz / s->ct.entry_sz; - - l2t_addr = value & ((1ULL << 51) - 1); - - *cte = address_space_ldq_le(as, l2t_addr + - ((icid % num_l2_entries) * GITS_CTE_SIZE), - MEMTXATTRS_UNSPECIFIED, res); - } - } - } else { - /* Flat level table */ - *cte = address_space_ldq_le(as, s->ct.base_addr + - (icid * GITS_CTE_SIZE), - MEMTXATTRS_UNSPECIFIED, res); + if (entry_addr == -1) { + return false; /* not valid */ } + *cte = address_space_ldq_le(as, entry_addr, MEMTXATTRS_UNSPECIFIED, res); return FIELD_EX64(*cte, CTE, VALID); } @@ -XXX,XX +XXX,XX @@ static bool get_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte, static uint64_t get_dte(GICv3ITSState *s, uint32_t devid, MemTxResult *res) { AddressSpace *as = &s->gicv3->dma_as; - uint64_t l2t_addr; - uint64_t value; - bool valid_l2t; - uint32_t l2t_id; - uint32_t num_l2_entries; + uint64_t entry_addr = table_entry_addr(s, &s->dt, devid, res); - if (s->dt.indirect) { - l2t_id = devid / (s->dt.page_sz / L1TABLE_ENTRY_SIZE); - - value = address_space_ldq_le(as, - s->dt.base_addr + - (l2t_id * L1TABLE_ENTRY_SIZE), - MEMTXATTRS_UNSPECIFIED, res); - - if (*res == MEMTX_OK) { - valid_l2t = (value & L2_TABLE_VALID_MASK) != 0; - - if (valid_l2t) { - num_l2_entries = s->dt.page_sz / s->dt.entry_sz; - - l2t_addr = value & ((1ULL << 51) - 1); - - value = address_space_ldq_le(as, l2t_addr + - ((devid % num_l2_entries) * GITS_DTE_SIZE), - MEMTXATTRS_UNSPECIFIED, res); - } - } - } else { - /* Flat level table */ - value = address_space_ldq_le(as, s->dt.base_addr + - (devid * GITS_DTE_SIZE), - MEMTXATTRS_UNSPECIFIED, res); + if (entry_addr == -1) { + return 0; /* a DTE entry with the Valid bit clear */ } - - return value; + return address_space_ldq_le(as, entry_addr, MEMTXATTRS_UNSPECIFIED, res); } /* @@ -XXX,XX +XXX,XX @@ static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid, uint64_t rdbase) { AddressSpace *as = &s->gicv3->dma_as; - uint64_t value; - uint64_t l2t_addr; - bool valid_l2t; - uint32_t l2t_id; - uint32_t num_l2_entries; + uint64_t entry_addr; uint64_t cte = 0; MemTxResult res = MEMTX_OK; @@ -XXX,XX +XXX,XX @@ static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid, cte = FIELD_DP64(cte, CTE, RDBASE, rdbase); } - /* - * The specification defines the format of level 1 entries of a - * 2-level table, but the format of level 2 entries and the format - * of flat-mapped tables is IMPDEF. - */ - if (s->ct.indirect) { - l2t_id = icid / (s->ct.page_sz / L1TABLE_ENTRY_SIZE); - - value = address_space_ldq_le(as, - s->ct.base_addr + - (l2t_id * L1TABLE_ENTRY_SIZE), - MEMTXATTRS_UNSPECIFIED, &res); - - if (res != MEMTX_OK) { - return false; - } - - valid_l2t = (value & L2_TABLE_VALID_MASK) != 0; - - if (valid_l2t) { - num_l2_entries = s->ct.page_sz / s->ct.entry_sz; - - l2t_addr = value & ((1ULL << 51) - 1); - - address_space_stq_le(as, l2t_addr + - ((icid % num_l2_entries) * GITS_CTE_SIZE), - cte, MEMTXATTRS_UNSPECIFIED, &res); - } - } else { - /* Flat level table */ - address_space_stq_le(as, s->ct.base_addr + (icid * GITS_CTE_SIZE), - cte, MEMTXATTRS_UNSPECIFIED, &res); - } + entry_addr = table_entry_addr(s, &s->ct, icid, &res); if (res != MEMTX_OK) { + /* memory access error: stall */ return false; - } else { + } + if (entry_addr == -1) { + /* No L2 table for this index: discard write and continue */ return true; } + + address_space_stq_le(as, entry_addr, cte, MEMTXATTRS_UNSPECIFIED, &res); + return res == MEMTX_OK; } static ItsCmdResult process_mapc(GICv3ITSState *s, uint32_t offset) @@ -XXX,XX +XXX,XX @@ static bool update_dte(GICv3ITSState *s, uint32_t devid, bool valid, uint8_t size, uint64_t itt_addr) { AddressSpace *as = &s->gicv3->dma_as; - uint64_t value; - uint64_t l2t_addr; - bool valid_l2t; - uint32_t l2t_id; - uint32_t num_l2_entries; + uint64_t entry_addr; uint64_t dte = 0; MemTxResult res = MEMTX_OK; @@ -XXX,XX +XXX,XX @@ static bool update_dte(GICv3ITSState *s, uint32_t devid, bool valid, return true; } - /* - * The specification defines the format of level 1 entries of a - * 2-level table, but the format of level 2 entries and the format - * of flat-mapped tables is IMPDEF. - */ - if (s->dt.indirect) { - l2t_id = devid / (s->dt.page_sz / L1TABLE_ENTRY_SIZE); - - value = address_space_ldq_le(as, - s->dt.base_addr + - (l2t_id * L1TABLE_ENTRY_SIZE), - MEMTXATTRS_UNSPECIFIED, &res); - - if (res != MEMTX_OK) { - return false; - } - - valid_l2t = (value & L2_TABLE_VALID_MASK) != 0; - - if (valid_l2t) { - num_l2_entries = s->dt.page_sz / s->dt.entry_sz; - - l2t_addr = value & ((1ULL << 51) - 1); - - address_space_stq_le(as, l2t_addr + - ((devid % num_l2_entries) * GITS_DTE_SIZE), - dte, MEMTXATTRS_UNSPECIFIED, &res); - } - } else { - /* Flat level table */ - address_space_stq_le(as, s->dt.base_addr + (devid * GITS_DTE_SIZE), - dte, MEMTXATTRS_UNSPECIFIED, &res); - } + entry_addr = table_entry_addr(s, &s->dt, devid, &res); if (res != MEMTX_OK) { + /* memory access error: stall */ return false; - } else { + } + if (entry_addr == -1) { + /* No L2 table for this index: discard write and continue */ return true; } + address_space_stq_le(as, entry_addr, dte, MEMTXATTRS_UNSPECIFIED, &res); + return res == MEMTX_OK; } static ItsCmdResult process_mapd(GICv3ITSState *s, uint64_t value, -- 2.25.1 In a few places in the ITS command handling functions, we were doing the range-check of an event ID or device ID only after using it as a table index; move the checks to before the uses. This misordering wouldn't have very bad effects because the tables are in guest memory anyway. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20220111171048.3545974-13-peter.maydell@linaro.org --- hw/intc/arm_gicv3_its.c | 42 ++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value, eventid = (value & EVENTID_MASK); + if (devid >= s->dt.num_ids) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: invalid command attributes: devid %d>=%d", + __func__, devid, s->dt.num_ids); + return CMD_CONTINUE; + } + dte = get_dte(s, devid, &res); if (res != MEMTX_OK) { @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value, num_eventids = 1ULL << (FIELD_EX64(dte, DTE, SIZE) + 1); + if (eventid >= num_eventids) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: invalid command attributes: eventid %d >= %" + PRId64 "\n", + __func__, eventid, num_eventids); + return CMD_CONTINUE; + } + ite_valid = get_ite(s, eventid, dte, &icid, &pIntid, &res); if (res != MEMTX_OK) { return CMD_STALL; @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value, return CMD_CONTINUE; } - if (devid >= s->dt.num_ids) { - qemu_log_mask(LOG_GUEST_ERROR, - "%s: invalid command attributes: devid %d>=%d", - __func__, devid, s->dt.num_ids); - return CMD_CONTINUE; - } - if (eventid >= num_eventids) { - qemu_log_mask(LOG_GUEST_ERROR, - "%s: invalid command attributes: eventid %d >= %" - PRId64 "\n", - __func__, eventid, num_eventids); - return CMD_CONTINUE; - } - /* * Current implementation only supports rdbase == procnum * Hence rdbase physical address is ignored @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value, icid = value & ICID_MASK; + if (devid >= s->dt.num_ids) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: invalid command attributes: devid %d>=%d", + __func__, devid, s->dt.num_ids); + return CMD_CONTINUE; + } + dte = get_dte(s, devid, &res); if (res != MEMTX_OK) { @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value, num_eventids = 1ULL << (FIELD_EX64(dte, DTE, SIZE) + 1); num_intids = 1ULL << (GICD_TYPER_IDBITS + 1); - if ((devid >= s->dt.num_ids) || (icid >= s->ct.num_ids) + if ((icid >= s->ct.num_ids) || !dte_valid || (eventid >= num_eventids) || (((pIntid < GICV3_LPI_INTID_START) || (pIntid >= num_intids)) && (pIntid != INTID_SPURIOUS))) { qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid command attributes " - "devid %d or icid %d or eventid %d or pIntid %d or" - "unmapped dte %d\n", __func__, devid, icid, eventid, + "icid %d or eventid %d or pIntid %d or" + "unmapped dte %d\n", __func__, icid, eventid, pIntid, dte_valid); /* * in this implementation, in case of error -- 2.25.1 In process_its_cmd(), we read an ICID out of the interrupt table entry, and then use it as an index into the collection table. Add a check that it is within range for the collection table first. This check is not strictly necessary, because: * we range check the ICID from the guest before writing it into the interrupt table entry, so the the only way to get an out of range ICID in process_its_cmd() is if a badly-behaved guest is writing directly to the interrupt table memory * the collection table is in guest memory, so QEMU won't fall over if we read off the end of it However, it seems clearer to include the check. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20220111171048.3545974-14-peter.maydell@linaro.org --- hw/intc/arm_gicv3_its.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value, return CMD_CONTINUE; } + if (icid >= s->ct.num_ids) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: invalid ICID 0x%x in ITE (table corrupted?)\n", + __func__, icid); + return CMD_CONTINUE; + } + cte_valid = get_cte(s, icid, &cte, &res); if (res != MEMTX_OK) { return CMD_STALL; -- 2.25.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> Quoting Peter Maydell: "These MEMTX_* aren't from the memory transaction API functions; they're just being used by gicd_readl() and friends as a way to indicate a success/failure so that the actual MemoryRegionOps read/write fns like gicv3_dist_read() can log a guest error." We are going to introduce more MemTxResult bits, so it is safer to check for !MEMTX_OK rather than MEMTX_ERROR. Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: David Hildenbrand <david@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/arm_gicv3_redist.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_redist.c +++ b/hw/intc/arm_gicv3_redist.c @@ -XXX,XX +XXX,XX @@ MemTxResult gicv3_redist_read(void *opaque, hwaddr offset, uint64_t *data, break; } - if (r == MEMTX_ERROR) { + if (r != MEMTX_OK) { qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid guest read at offset " TARGET_FMT_plx " size %u\n", __func__, offset, size); @@ -XXX,XX +XXX,XX @@ MemTxResult gicv3_redist_write(void *opaque, hwaddr offset, uint64_t data, break; } - if (r == MEMTX_ERROR) { + if (r != MEMTX_OK) { qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid guest write at offset " TARGET_FMT_plx " size %u\n", __func__, offset, size); -- 2.25.1