From 2bd5f41c00686a1f847a60824d0375f3df2c26bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 15 Aug 2019 09:46:41 +0100 Subject: [PATCH 01/29] target/arm: generate a custom MIDR for -cpu max MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While most features are now detected by probing the ID_* registers kernels can (and do) use MIDR_EL1 for working out of they have to apply errata. This can trip up warnings in the kernel as it tries to work out if it should apply workarounds to features that don't actually exist in the reported CPU type. Avoid this problem by synthesising our own MIDR value. Signed-off-by: Alex Bennée Reviewed-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20190726113950.7499-1-alex.bennee@linaro.org Signed-off-by: Peter Maydell --- target/arm/cpu.h | 6 ++++++ target/arm/cpu64.c | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 94c990cddb..67f2af0e16 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -1611,6 +1611,12 @@ FIELD(V7M_FPCCR, ASPEN, 31, 1) /* * System register ID fields. */ +FIELD(MIDR_EL1, REVISION, 0, 4) +FIELD(MIDR_EL1, PARTNUM, 4, 12) +FIELD(MIDR_EL1, ARCHITECTURE, 16, 4) +FIELD(MIDR_EL1, VARIANT, 20, 4) +FIELD(MIDR_EL1, IMPLEMENTER, 24, 8) + FIELD(ID_ISAR0, SWAP, 0, 4) FIELD(ID_ISAR0, BITCOUNT, 4, 4) FIELD(ID_ISAR0, BITFIELD, 8, 4) diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index 1901997a06..ee55237a9b 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -296,6 +296,25 @@ static void aarch64_max_initfn(Object *obj) uint32_t u; aarch64_a57_initfn(obj); + /* + * Reset MIDR so the guest doesn't mistake our 'max' CPU type for a real + * one and try to apply errata workarounds or use impdef features we + * don't provide. + * An IMPLEMENTER field of 0 means "reserved for software use"; + * ARCHITECTURE must be 0xf indicating "v7 or later, check ID registers + * to see which features are present"; + * the VARIANT, PARTNUM and REVISION fields are all implementation + * defined and we choose to define PARTNUM just in case guest + * code needs to distinguish this QEMU CPU from other software + * implementations, though this shouldn't be needed. + */ + t = FIELD_DP64(0, MIDR_EL1, IMPLEMENTER, 0); + t = FIELD_DP64(t, MIDR_EL1, ARCHITECTURE, 0xf); + t = FIELD_DP64(t, MIDR_EL1, PARTNUM, 'Q'); + t = FIELD_DP64(t, MIDR_EL1, VARIANT, 0); + t = FIELD_DP64(t, MIDR_EL1, REVISION, 0); + cpu->midr = t; + t = cpu->isar.id_aa64isar0; t = FIELD_DP64(t, ID_AA64ISAR0, AES, 2); /* AES + PMULL */ t = FIELD_DP64(t, ID_AA64ISAR0, SHA1, 1); From a6b3ed2348dc06623622fefec6753296d431f155 Mon Sep 17 00:00:00 2001 From: Damien Hedde Date: Thu, 15 Aug 2019 09:46:41 +0100 Subject: [PATCH 02/29] hw/misc/zynq_slcr: use standard register definition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the zynq_slcr registers enum and macros using the hw/registerfields.h macros. Signed-off-by: Damien Hedde Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Message-id: 20190729145654.14644-30-damien.hedde@greensocs.com Signed-off-by: Peter Maydell --- hw/misc/zynq_slcr.c | 450 ++++++++++++++++++++++---------------------- 1 file changed, 225 insertions(+), 225 deletions(-) diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c index 6b51ae5ff1..dd766a6779 100644 --- a/hw/misc/zynq_slcr.c +++ b/hw/misc/zynq_slcr.c @@ -21,6 +21,7 @@ #include "sysemu/sysemu.h" #include "qemu/log.h" #include "qemu/module.h" +#include "hw/registerfields.h" #ifndef ZYNQ_SLCR_ERR_DEBUG #define ZYNQ_SLCR_ERR_DEBUG 0 @@ -36,138 +37,135 @@ #define XILINX_LOCK_KEY 0x767b #define XILINX_UNLOCK_KEY 0xdf0d -#define R_PSS_RST_CTRL_SOFT_RST 0x1 +REG32(SCL, 0x000) +REG32(LOCK, 0x004) +REG32(UNLOCK, 0x008) +REG32(LOCKSTA, 0x00c) -enum { - SCL = 0x000 / 4, - LOCK, - UNLOCK, - LOCKSTA, +REG32(ARM_PLL_CTRL, 0x100) +REG32(DDR_PLL_CTRL, 0x104) +REG32(IO_PLL_CTRL, 0x108) +REG32(PLL_STATUS, 0x10c) +REG32(ARM_PLL_CFG, 0x110) +REG32(DDR_PLL_CFG, 0x114) +REG32(IO_PLL_CFG, 0x118) - ARM_PLL_CTRL = 0x100 / 4, - DDR_PLL_CTRL, - IO_PLL_CTRL, - PLL_STATUS, - ARM_PLL_CFG, - DDR_PLL_CFG, - IO_PLL_CFG, - - ARM_CLK_CTRL = 0x120 / 4, - DDR_CLK_CTRL, - DCI_CLK_CTRL, - APER_CLK_CTRL, - USB0_CLK_CTRL, - USB1_CLK_CTRL, - GEM0_RCLK_CTRL, - GEM1_RCLK_CTRL, - GEM0_CLK_CTRL, - GEM1_CLK_CTRL, - SMC_CLK_CTRL, - LQSPI_CLK_CTRL, - SDIO_CLK_CTRL, - UART_CLK_CTRL, - SPI_CLK_CTRL, - CAN_CLK_CTRL, - CAN_MIOCLK_CTRL, - DBG_CLK_CTRL, - PCAP_CLK_CTRL, - TOPSW_CLK_CTRL, +REG32(ARM_CLK_CTRL, 0x120) +REG32(DDR_CLK_CTRL, 0x124) +REG32(DCI_CLK_CTRL, 0x128) +REG32(APER_CLK_CTRL, 0x12c) +REG32(USB0_CLK_CTRL, 0x130) +REG32(USB1_CLK_CTRL, 0x134) +REG32(GEM0_RCLK_CTRL, 0x138) +REG32(GEM1_RCLK_CTRL, 0x13c) +REG32(GEM0_CLK_CTRL, 0x140) +REG32(GEM1_CLK_CTRL, 0x144) +REG32(SMC_CLK_CTRL, 0x148) +REG32(LQSPI_CLK_CTRL, 0x14c) +REG32(SDIO_CLK_CTRL, 0x150) +REG32(UART_CLK_CTRL, 0x154) +REG32(SPI_CLK_CTRL, 0x158) +REG32(CAN_CLK_CTRL, 0x15c) +REG32(CAN_MIOCLK_CTRL, 0x160) +REG32(DBG_CLK_CTRL, 0x164) +REG32(PCAP_CLK_CTRL, 0x168) +REG32(TOPSW_CLK_CTRL, 0x16c) #define FPGA_CTRL_REGS(n, start) \ - FPGA ## n ## _CLK_CTRL = (start) / 4, \ - FPGA ## n ## _THR_CTRL, \ - FPGA ## n ## _THR_CNT, \ - FPGA ## n ## _THR_STA, - FPGA_CTRL_REGS(0, 0x170) - FPGA_CTRL_REGS(1, 0x180) - FPGA_CTRL_REGS(2, 0x190) - FPGA_CTRL_REGS(3, 0x1a0) + REG32(FPGA ## n ## _CLK_CTRL, (start)) \ + REG32(FPGA ## n ## _THR_CTRL, (start) + 0x4)\ + REG32(FPGA ## n ## _THR_CNT, (start) + 0x8)\ + REG32(FPGA ## n ## _THR_STA, (start) + 0xc) +FPGA_CTRL_REGS(0, 0x170) +FPGA_CTRL_REGS(1, 0x180) +FPGA_CTRL_REGS(2, 0x190) +FPGA_CTRL_REGS(3, 0x1a0) - BANDGAP_TRIP = 0x1b8 / 4, - PLL_PREDIVISOR = 0x1c0 / 4, - CLK_621_TRUE, +REG32(BANDGAP_TRIP, 0x1b8) +REG32(PLL_PREDIVISOR, 0x1c0) +REG32(CLK_621_TRUE, 0x1c4) - PSS_RST_CTRL = 0x200 / 4, - DDR_RST_CTRL, - TOPSW_RESET_CTRL, - DMAC_RST_CTRL, - USB_RST_CTRL, - GEM_RST_CTRL, - SDIO_RST_CTRL, - SPI_RST_CTRL, - CAN_RST_CTRL, - I2C_RST_CTRL, - UART_RST_CTRL, - GPIO_RST_CTRL, - LQSPI_RST_CTRL, - SMC_RST_CTRL, - OCM_RST_CTRL, - FPGA_RST_CTRL = 0x240 / 4, - A9_CPU_RST_CTRL, +REG32(PSS_RST_CTRL, 0x200) + FIELD(PSS_RST_CTRL, SOFT_RST, 0, 1) +REG32(DDR_RST_CTRL, 0x204) +REG32(TOPSW_RESET_CTRL, 0x208) +REG32(DMAC_RST_CTRL, 0x20c) +REG32(USB_RST_CTRL, 0x210) +REG32(GEM_RST_CTRL, 0x214) +REG32(SDIO_RST_CTRL, 0x218) +REG32(SPI_RST_CTRL, 0x21c) +REG32(CAN_RST_CTRL, 0x220) +REG32(I2C_RST_CTRL, 0x224) +REG32(UART_RST_CTRL, 0x228) +REG32(GPIO_RST_CTRL, 0x22c) +REG32(LQSPI_RST_CTRL, 0x230) +REG32(SMC_RST_CTRL, 0x234) +REG32(OCM_RST_CTRL, 0x238) +REG32(FPGA_RST_CTRL, 0x240) +REG32(A9_CPU_RST_CTRL, 0x244) - RS_AWDT_CTRL = 0x24c / 4, - RST_REASON, +REG32(RS_AWDT_CTRL, 0x24c) +REG32(RST_REASON, 0x250) - REBOOT_STATUS = 0x258 / 4, - BOOT_MODE, +REG32(REBOOT_STATUS, 0x258) +REG32(BOOT_MODE, 0x25c) - APU_CTRL = 0x300 / 4, - WDT_CLK_SEL, +REG32(APU_CTRL, 0x300) +REG32(WDT_CLK_SEL, 0x304) - TZ_DMA_NS = 0x440 / 4, - TZ_DMA_IRQ_NS, - TZ_DMA_PERIPH_NS, +REG32(TZ_DMA_NS, 0x440) +REG32(TZ_DMA_IRQ_NS, 0x444) +REG32(TZ_DMA_PERIPH_NS, 0x448) - PSS_IDCODE = 0x530 / 4, +REG32(PSS_IDCODE, 0x530) - DDR_URGENT = 0x600 / 4, - DDR_CAL_START = 0x60c / 4, - DDR_REF_START = 0x614 / 4, - DDR_CMD_STA, - DDR_URGENT_SEL, - DDR_DFI_STATUS, +REG32(DDR_URGENT, 0x600) +REG32(DDR_CAL_START, 0x60c) +REG32(DDR_REF_START, 0x614) +REG32(DDR_CMD_STA, 0x618) +REG32(DDR_URGENT_SEL, 0x61c) +REG32(DDR_DFI_STATUS, 0x620) - MIO = 0x700 / 4, +REG32(MIO, 0x700) #define MIO_LENGTH 54 - MIO_LOOPBACK = 0x804 / 4, - MIO_MST_TRI0, - MIO_MST_TRI1, +REG32(MIO_LOOPBACK, 0x804) +REG32(MIO_MST_TRI0, 0x808) +REG32(MIO_MST_TRI1, 0x80c) - SD0_WP_CD_SEL = 0x830 / 4, - SD1_WP_CD_SEL, +REG32(SD0_WP_CD_SEL, 0x830) +REG32(SD1_WP_CD_SEL, 0x834) - LVL_SHFTR_EN = 0x900 / 4, - OCM_CFG = 0x910 / 4, +REG32(LVL_SHFTR_EN, 0x900) +REG32(OCM_CFG, 0x910) - CPU_RAM = 0xa00 / 4, +REG32(CPU_RAM, 0xa00) - IOU = 0xa30 / 4, +REG32(IOU, 0xa30) - DMAC_RAM = 0xa50 / 4, +REG32(DMAC_RAM, 0xa50) - AFI0 = 0xa60 / 4, - AFI1 = AFI0 + 3, - AFI2 = AFI1 + 3, - AFI3 = AFI2 + 3, +REG32(AFI0, 0xa60) +REG32(AFI1, 0xa6c) +REG32(AFI2, 0xa78) +REG32(AFI3, 0xa84) #define AFI_LENGTH 3 - OCM = 0xa90 / 4, +REG32(OCM, 0xa90) - DEVCI_RAM = 0xaa0 / 4, +REG32(DEVCI_RAM, 0xaa0) - CSG_RAM = 0xab0 / 4, +REG32(CSG_RAM, 0xab0) - GPIOB_CTRL = 0xb00 / 4, - GPIOB_CFG_CMOS18, - GPIOB_CFG_CMOS25, - GPIOB_CFG_CMOS33, - GPIOB_CFG_HSTL = 0xb14 / 4, - GPIOB_DRVR_BIAS_CTRL, +REG32(GPIOB_CTRL, 0xb00) +REG32(GPIOB_CFG_CMOS18, 0xb04) +REG32(GPIOB_CFG_CMOS25, 0xb08) +REG32(GPIOB_CFG_CMOS33, 0xb0c) +REG32(GPIOB_CFG_HSTL, 0xb14) +REG32(GPIOB_DRVR_BIAS_CTRL, 0xb18) - DDRIOB = 0xb40 / 4, +REG32(DDRIOB, 0xb40) #define DDRIOB_LENGTH 14 -}; #define ZYNQ_SLCR_MMIO_SIZE 0x1000 #define ZYNQ_SLCR_NUM_REGS (ZYNQ_SLCR_MMIO_SIZE / 4) @@ -190,150 +188,152 @@ static void zynq_slcr_reset(DeviceState *d) DB_PRINT("RESET\n"); - s->regs[LOCKSTA] = 1; + s->regs[R_LOCKSTA] = 1; /* 0x100 - 0x11C */ - s->regs[ARM_PLL_CTRL] = 0x0001A008; - s->regs[DDR_PLL_CTRL] = 0x0001A008; - s->regs[IO_PLL_CTRL] = 0x0001A008; - s->regs[PLL_STATUS] = 0x0000003F; - s->regs[ARM_PLL_CFG] = 0x00014000; - s->regs[DDR_PLL_CFG] = 0x00014000; - s->regs[IO_PLL_CFG] = 0x00014000; + s->regs[R_ARM_PLL_CTRL] = 0x0001A008; + s->regs[R_DDR_PLL_CTRL] = 0x0001A008; + s->regs[R_IO_PLL_CTRL] = 0x0001A008; + s->regs[R_PLL_STATUS] = 0x0000003F; + s->regs[R_ARM_PLL_CFG] = 0x00014000; + s->regs[R_DDR_PLL_CFG] = 0x00014000; + s->regs[R_IO_PLL_CFG] = 0x00014000; /* 0x120 - 0x16C */ - s->regs[ARM_CLK_CTRL] = 0x1F000400; - s->regs[DDR_CLK_CTRL] = 0x18400003; - s->regs[DCI_CLK_CTRL] = 0x01E03201; - s->regs[APER_CLK_CTRL] = 0x01FFCCCD; - s->regs[USB0_CLK_CTRL] = s->regs[USB1_CLK_CTRL] = 0x00101941; - s->regs[GEM0_RCLK_CTRL] = s->regs[GEM1_RCLK_CTRL] = 0x00000001; - s->regs[GEM0_CLK_CTRL] = s->regs[GEM1_CLK_CTRL] = 0x00003C01; - s->regs[SMC_CLK_CTRL] = 0x00003C01; - s->regs[LQSPI_CLK_CTRL] = 0x00002821; - s->regs[SDIO_CLK_CTRL] = 0x00001E03; - s->regs[UART_CLK_CTRL] = 0x00003F03; - s->regs[SPI_CLK_CTRL] = 0x00003F03; - s->regs[CAN_CLK_CTRL] = 0x00501903; - s->regs[DBG_CLK_CTRL] = 0x00000F03; - s->regs[PCAP_CLK_CTRL] = 0x00000F01; + s->regs[R_ARM_CLK_CTRL] = 0x1F000400; + s->regs[R_DDR_CLK_CTRL] = 0x18400003; + s->regs[R_DCI_CLK_CTRL] = 0x01E03201; + s->regs[R_APER_CLK_CTRL] = 0x01FFCCCD; + s->regs[R_USB0_CLK_CTRL] = s->regs[R_USB1_CLK_CTRL] = 0x00101941; + s->regs[R_GEM0_RCLK_CTRL] = s->regs[R_GEM1_RCLK_CTRL] = 0x00000001; + s->regs[R_GEM0_CLK_CTRL] = s->regs[R_GEM1_CLK_CTRL] = 0x00003C01; + s->regs[R_SMC_CLK_CTRL] = 0x00003C01; + s->regs[R_LQSPI_CLK_CTRL] = 0x00002821; + s->regs[R_SDIO_CLK_CTRL] = 0x00001E03; + s->regs[R_UART_CLK_CTRL] = 0x00003F03; + s->regs[R_SPI_CLK_CTRL] = 0x00003F03; + s->regs[R_CAN_CLK_CTRL] = 0x00501903; + s->regs[R_DBG_CLK_CTRL] = 0x00000F03; + s->regs[R_PCAP_CLK_CTRL] = 0x00000F01; /* 0x170 - 0x1AC */ - s->regs[FPGA0_CLK_CTRL] = s->regs[FPGA1_CLK_CTRL] = s->regs[FPGA2_CLK_CTRL] - = s->regs[FPGA3_CLK_CTRL] = 0x00101800; - s->regs[FPGA0_THR_STA] = s->regs[FPGA1_THR_STA] = s->regs[FPGA2_THR_STA] - = s->regs[FPGA3_THR_STA] = 0x00010000; + s->regs[R_FPGA0_CLK_CTRL] = s->regs[R_FPGA1_CLK_CTRL] + = s->regs[R_FPGA2_CLK_CTRL] + = s->regs[R_FPGA3_CLK_CTRL] = 0x00101800; + s->regs[R_FPGA0_THR_STA] = s->regs[R_FPGA1_THR_STA] + = s->regs[R_FPGA2_THR_STA] + = s->regs[R_FPGA3_THR_STA] = 0x00010000; /* 0x1B0 - 0x1D8 */ - s->regs[BANDGAP_TRIP] = 0x0000001F; - s->regs[PLL_PREDIVISOR] = 0x00000001; - s->regs[CLK_621_TRUE] = 0x00000001; + s->regs[R_BANDGAP_TRIP] = 0x0000001F; + s->regs[R_PLL_PREDIVISOR] = 0x00000001; + s->regs[R_CLK_621_TRUE] = 0x00000001; /* 0x200 - 0x25C */ - s->regs[FPGA_RST_CTRL] = 0x01F33F0F; - s->regs[RST_REASON] = 0x00000040; + s->regs[R_FPGA_RST_CTRL] = 0x01F33F0F; + s->regs[R_RST_REASON] = 0x00000040; - s->regs[BOOT_MODE] = 0x00000001; + s->regs[R_BOOT_MODE] = 0x00000001; /* 0x700 - 0x7D4 */ for (i = 0; i < 54; i++) { - s->regs[MIO + i] = 0x00001601; + s->regs[R_MIO + i] = 0x00001601; } for (i = 2; i <= 8; i++) { - s->regs[MIO + i] = 0x00000601; + s->regs[R_MIO + i] = 0x00000601; } - s->regs[MIO_MST_TRI0] = s->regs[MIO_MST_TRI1] = 0xFFFFFFFF; + s->regs[R_MIO_MST_TRI0] = s->regs[R_MIO_MST_TRI1] = 0xFFFFFFFF; - s->regs[CPU_RAM + 0] = s->regs[CPU_RAM + 1] = s->regs[CPU_RAM + 3] - = s->regs[CPU_RAM + 4] = s->regs[CPU_RAM + 7] - = 0x00010101; - s->regs[CPU_RAM + 2] = s->regs[CPU_RAM + 5] = 0x01010101; - s->regs[CPU_RAM + 6] = 0x00000001; + s->regs[R_CPU_RAM + 0] = s->regs[R_CPU_RAM + 1] = s->regs[R_CPU_RAM + 3] + = s->regs[R_CPU_RAM + 4] = s->regs[R_CPU_RAM + 7] + = 0x00010101; + s->regs[R_CPU_RAM + 2] = s->regs[R_CPU_RAM + 5] = 0x01010101; + s->regs[R_CPU_RAM + 6] = 0x00000001; - s->regs[IOU + 0] = s->regs[IOU + 1] = s->regs[IOU + 2] = s->regs[IOU + 3] - = 0x09090909; - s->regs[IOU + 4] = s->regs[IOU + 5] = 0x00090909; - s->regs[IOU + 6] = 0x00000909; + s->regs[R_IOU + 0] = s->regs[R_IOU + 1] = s->regs[R_IOU + 2] + = s->regs[R_IOU + 3] = 0x09090909; + s->regs[R_IOU + 4] = s->regs[R_IOU + 5] = 0x00090909; + s->regs[R_IOU + 6] = 0x00000909; - s->regs[DMAC_RAM] = 0x00000009; + s->regs[R_DMAC_RAM] = 0x00000009; - s->regs[AFI0 + 0] = s->regs[AFI0 + 1] = 0x09090909; - s->regs[AFI1 + 0] = s->regs[AFI1 + 1] = 0x09090909; - s->regs[AFI2 + 0] = s->regs[AFI2 + 1] = 0x09090909; - s->regs[AFI3 + 0] = s->regs[AFI3 + 1] = 0x09090909; - s->regs[AFI0 + 2] = s->regs[AFI1 + 2] = s->regs[AFI2 + 2] - = s->regs[AFI3 + 2] = 0x00000909; + s->regs[R_AFI0 + 0] = s->regs[R_AFI0 + 1] = 0x09090909; + s->regs[R_AFI1 + 0] = s->regs[R_AFI1 + 1] = 0x09090909; + s->regs[R_AFI2 + 0] = s->regs[R_AFI2 + 1] = 0x09090909; + s->regs[R_AFI3 + 0] = s->regs[R_AFI3 + 1] = 0x09090909; + s->regs[R_AFI0 + 2] = s->regs[R_AFI1 + 2] = s->regs[R_AFI2 + 2] + = s->regs[R_AFI3 + 2] = 0x00000909; - s->regs[OCM + 0] = 0x01010101; - s->regs[OCM + 1] = s->regs[OCM + 2] = 0x09090909; + s->regs[R_OCM + 0] = 0x01010101; + s->regs[R_OCM + 1] = s->regs[R_OCM + 2] = 0x09090909; - s->regs[DEVCI_RAM] = 0x00000909; - s->regs[CSG_RAM] = 0x00000001; + s->regs[R_DEVCI_RAM] = 0x00000909; + s->regs[R_CSG_RAM] = 0x00000001; - s->regs[DDRIOB + 0] = s->regs[DDRIOB + 1] = s->regs[DDRIOB + 2] - = s->regs[DDRIOB + 3] = 0x00000e00; - s->regs[DDRIOB + 4] = s->regs[DDRIOB + 5] = s->regs[DDRIOB + 6] - = 0x00000e00; - s->regs[DDRIOB + 12] = 0x00000021; + s->regs[R_DDRIOB + 0] = s->regs[R_DDRIOB + 1] = s->regs[R_DDRIOB + 2] + = s->regs[R_DDRIOB + 3] = 0x00000e00; + s->regs[R_DDRIOB + 4] = s->regs[R_DDRIOB + 5] = s->regs[R_DDRIOB + 6] + = 0x00000e00; + s->regs[R_DDRIOB + 12] = 0x00000021; } static bool zynq_slcr_check_offset(hwaddr offset, bool rnw) { switch (offset) { - case LOCK: - case UNLOCK: - case DDR_CAL_START: - case DDR_REF_START: + case R_LOCK: + case R_UNLOCK: + case R_DDR_CAL_START: + case R_DDR_REF_START: return !rnw; /* Write only */ - case LOCKSTA: - case FPGA0_THR_STA: - case FPGA1_THR_STA: - case FPGA2_THR_STA: - case FPGA3_THR_STA: - case BOOT_MODE: - case PSS_IDCODE: - case DDR_CMD_STA: - case DDR_DFI_STATUS: - case PLL_STATUS: + case R_LOCKSTA: + case R_FPGA0_THR_STA: + case R_FPGA1_THR_STA: + case R_FPGA2_THR_STA: + case R_FPGA3_THR_STA: + case R_BOOT_MODE: + case R_PSS_IDCODE: + case R_DDR_CMD_STA: + case R_DDR_DFI_STATUS: + case R_PLL_STATUS: return rnw;/* read only */ - case SCL: - case ARM_PLL_CTRL ... IO_PLL_CTRL: - case ARM_PLL_CFG ... IO_PLL_CFG: - case ARM_CLK_CTRL ... TOPSW_CLK_CTRL: - case FPGA0_CLK_CTRL ... FPGA0_THR_CNT: - case FPGA1_CLK_CTRL ... FPGA1_THR_CNT: - case FPGA2_CLK_CTRL ... FPGA2_THR_CNT: - case FPGA3_CLK_CTRL ... FPGA3_THR_CNT: - case BANDGAP_TRIP: - case PLL_PREDIVISOR: - case CLK_621_TRUE: - case PSS_RST_CTRL ... A9_CPU_RST_CTRL: - case RS_AWDT_CTRL: - case RST_REASON: - case REBOOT_STATUS: - case APU_CTRL: - case WDT_CLK_SEL: - case TZ_DMA_NS ... TZ_DMA_PERIPH_NS: - case DDR_URGENT: - case DDR_URGENT_SEL: - case MIO ... MIO + MIO_LENGTH - 1: - case MIO_LOOPBACK ... MIO_MST_TRI1: - case SD0_WP_CD_SEL: - case SD1_WP_CD_SEL: - case LVL_SHFTR_EN: - case OCM_CFG: - case CPU_RAM: - case IOU: - case DMAC_RAM: - case AFI0 ... AFI3 + AFI_LENGTH - 1: - case OCM: - case DEVCI_RAM: - case CSG_RAM: - case GPIOB_CTRL ... GPIOB_CFG_CMOS33: - case GPIOB_CFG_HSTL: - case GPIOB_DRVR_BIAS_CTRL: - case DDRIOB ... DDRIOB + DDRIOB_LENGTH - 1: + case R_SCL: + case R_ARM_PLL_CTRL ... R_IO_PLL_CTRL: + case R_ARM_PLL_CFG ... R_IO_PLL_CFG: + case R_ARM_CLK_CTRL ... R_TOPSW_CLK_CTRL: + case R_FPGA0_CLK_CTRL ... R_FPGA0_THR_CNT: + case R_FPGA1_CLK_CTRL ... R_FPGA1_THR_CNT: + case R_FPGA2_CLK_CTRL ... R_FPGA2_THR_CNT: + case R_FPGA3_CLK_CTRL ... R_FPGA3_THR_CNT: + case R_BANDGAP_TRIP: + case R_PLL_PREDIVISOR: + case R_CLK_621_TRUE: + case R_PSS_RST_CTRL ... R_A9_CPU_RST_CTRL: + case R_RS_AWDT_CTRL: + case R_RST_REASON: + case R_REBOOT_STATUS: + case R_APU_CTRL: + case R_WDT_CLK_SEL: + case R_TZ_DMA_NS ... R_TZ_DMA_PERIPH_NS: + case R_DDR_URGENT: + case R_DDR_URGENT_SEL: + case R_MIO ... R_MIO + MIO_LENGTH - 1: + case R_MIO_LOOPBACK ... R_MIO_MST_TRI1: + case R_SD0_WP_CD_SEL: + case R_SD1_WP_CD_SEL: + case R_LVL_SHFTR_EN: + case R_OCM_CFG: + case R_CPU_RAM: + case R_IOU: + case R_DMAC_RAM: + case R_AFI0 ... R_AFI3 + AFI_LENGTH - 1: + case R_OCM: + case R_DEVCI_RAM: + case R_CSG_RAM: + case R_GPIOB_CTRL ... R_GPIOB_CFG_CMOS33: + case R_GPIOB_CFG_HSTL: + case R_GPIOB_DRVR_BIAS_CTRL: + case R_DDRIOB ... R_DDRIOB + DDRIOB_LENGTH - 1: return true; default: return false; @@ -371,24 +371,24 @@ static void zynq_slcr_write(void *opaque, hwaddr offset, } switch (offset) { - case SCL: - s->regs[SCL] = val & 0x1; + case R_SCL: + s->regs[R_SCL] = val & 0x1; return; - case LOCK: + case R_LOCK: if ((val & 0xFFFF) == XILINX_LOCK_KEY) { DB_PRINT("XILINX LOCK 0xF8000000 + 0x%x <= 0x%x\n", (int)offset, (unsigned)val & 0xFFFF); - s->regs[LOCKSTA] = 1; + s->regs[R_LOCKSTA] = 1; } else { DB_PRINT("WRONG XILINX LOCK KEY 0xF8000000 + 0x%x <= 0x%x\n", (int)offset, (unsigned)val & 0xFFFF); } return; - case UNLOCK: + case R_UNLOCK: if ((val & 0xFFFF) == XILINX_UNLOCK_KEY) { DB_PRINT("XILINX UNLOCK 0xF8000000 + 0x%x <= 0x%x\n", (int)offset, (unsigned)val & 0xFFFF); - s->regs[LOCKSTA] = 0; + s->regs[R_LOCKSTA] = 0; } else { DB_PRINT("WRONG XILINX UNLOCK KEY 0xF8000000 + 0x%x <= 0x%x\n", (int)offset, (unsigned)val & 0xFFFF); @@ -396,7 +396,7 @@ static void zynq_slcr_write(void *opaque, hwaddr offset, return; } - if (s->regs[LOCKSTA]) { + if (s->regs[R_LOCKSTA]) { qemu_log_mask(LOG_GUEST_ERROR, "SCLR registers are locked. Unlock them first\n"); return; @@ -404,8 +404,8 @@ static void zynq_slcr_write(void *opaque, hwaddr offset, s->regs[offset] = val; switch (offset) { - case PSS_RST_CTRL: - if (val & R_PSS_RST_CTRL_SOFT_RST) { + case R_PSS_RST_CTRL: + if (FIELD_EX32(val, PSS_RST_CTRL, SOFT_RST)) { qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET); } break; From 88e1b59ee30950a66ea28b740914bf603fa9a1ec Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 15 Aug 2019 09:46:42 +0100 Subject: [PATCH 03/29] Set ENET_BD_BDU in I.MX FEC controller This commit properly sets the ENET_BD_BDU flag once the emulated FEC controller has finished processing the last descriptor. This is done for both transmit and receive descriptors. This allows the QNX 7.0.0 BSP for the Sabrelite board (which can be found at http://blackberry.qnx.com/en/developers/bsp) to properly control the FEC. Without this patch, the BSP ethernet driver will never re-use FEC descriptors, as the unset ENET_BD_BDU flag will cause it to believe that the descriptors are still in use by the NIC. Note that Linux does not appear to use this field at all, and is unaffected by this patch. Without this patch, QNX will think that the NIC is still processing its transaction descriptors, and won't send any more data over the network. For reference: On page 1192 of the I.MX 6DQ reference manual revision (Rev. 5, 06/2018), which can be found at https://www.nxp.com/products/processors-and-microcontrollers/arm-based-processors-and-mcus/i.mx-applications-processors/i.mx-6-processors/i.mx-6quad-processors-high-performance-3d-graphics-hd-video-arm-cortex-a9-core:i.MX6Q?&tab=Documentation_Tab&linkline=Application-Note the 'BDU' field is described as follows for the 'Enhanced transmit buffer descriptor': 'Last buffer descriptor update done. Indicates that the last BD data has been updated by uDMA. This field is written by the user (=0) and uDMA (=1).' The same description is used for the receive buffer descriptor. Signed-off-by: Aaron Hill Message-id: 20190805142417.10433-1-aaron.hill@alertinnovation.com Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- hw/net/imx_fec.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c index 404154ebbf..4693bfb246 100644 --- a/hw/net/imx_fec.c +++ b/hw/net/imx_fec.c @@ -590,6 +590,8 @@ static void imx_enet_do_tx(IMXFECState *s, uint32_t index) if (bd.option & ENET_BD_TX_INT) { s->regs[ENET_EIR] |= int_txf; } + /* Indicate that we've updated the last buffer descriptor. */ + bd.last_buffer = ENET_BD_BDU; } if (bd.option & ENET_BD_TX_INT) { s->regs[ENET_EIR] |= int_txb; @@ -1239,6 +1241,8 @@ static ssize_t imx_enet_receive(NetClientState *nc, const uint8_t *buf, /* Last buffer in frame. */ bd.flags |= flags | ENET_BD_L; FEC_PRINTF("rx frame flags %04x\n", bd.flags); + /* Indicate that we've updated the last buffer descriptor. */ + bd.last_buffer = ENET_BD_BDU; if (bd.option & ENET_BD_RX_INT) { s->regs[ENET_EIR] |= ENET_INT_RXF; } From c1d5f50f094ab204accfacc2ee6aafc9601dd5c4 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 15 Aug 2019 09:46:42 +0100 Subject: [PATCH 04/29] target/arm: Factor out 'generate singlestep exception' function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Factor out code to 'generate a singlestep exception', which is currently repeated in four places. To do this we need to also pull the identical copies of the gen-exception() function out of translate-a64.c and translate.c into translate.h. (There is a bug in the code: we're taking the exception to the wrong target EL. This will be simpler to fix if there's only one place to do it.) Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Alex Bennée Message-id: 20190805130952.4415-2-peter.maydell@linaro.org --- target/arm/translate-a64.c | 19 ++----------------- target/arm/translate.c | 20 ++------------------ target/arm/translate.h | 23 +++++++++++++++++++++++ 3 files changed, 27 insertions(+), 35 deletions(-) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index d3231477a2..f6729b96fd 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -253,19 +253,6 @@ static void gen_exception_internal(int excp) tcg_temp_free_i32(tcg_excp); } -static void gen_exception(int excp, uint32_t syndrome, uint32_t target_el) -{ - TCGv_i32 tcg_excp = tcg_const_i32(excp); - TCGv_i32 tcg_syn = tcg_const_i32(syndrome); - TCGv_i32 tcg_el = tcg_const_i32(target_el); - - gen_helper_exception_with_syndrome(cpu_env, tcg_excp, - tcg_syn, tcg_el); - tcg_temp_free_i32(tcg_el); - tcg_temp_free_i32(tcg_syn); - tcg_temp_free_i32(tcg_excp); -} - static void gen_exception_internal_insn(DisasContext *s, int offset, int excp) { gen_a64_set_pc_im(s->pc - offset); @@ -305,8 +292,7 @@ static void gen_step_complete_exception(DisasContext *s) * of the exception, and our syndrome information is always correct. */ gen_ss_advance(s); - gen_exception(EXCP_UDEF, syn_swstep(s->ss_same_el, 1, s->is_ldex), - default_exception_el(s)); + gen_swstep_exception(s, 1, s->is_ldex); s->base.is_jmp = DISAS_NORETURN; } @@ -14261,8 +14247,7 @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) * bits should be zero. */ assert(dc->base.num_insns == 1); - gen_exception(EXCP_UDEF, syn_swstep(dc->ss_same_el, 0, 0), - default_exception_el(dc)); + gen_swstep_exception(dc, 0, 0); dc->base.is_jmp = DISAS_NORETURN; } else { disas_a64_insn(env, dc); diff --git a/target/arm/translate.c b/target/arm/translate.c index 7853462b21..19b9d8f272 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -282,20 +282,6 @@ static void gen_exception_internal(int excp) tcg_temp_free_i32(tcg_excp); } -static void gen_exception(int excp, uint32_t syndrome, uint32_t target_el) -{ - TCGv_i32 tcg_excp = tcg_const_i32(excp); - TCGv_i32 tcg_syn = tcg_const_i32(syndrome); - TCGv_i32 tcg_el = tcg_const_i32(target_el); - - gen_helper_exception_with_syndrome(cpu_env, tcg_excp, - tcg_syn, tcg_el); - - tcg_temp_free_i32(tcg_el); - tcg_temp_free_i32(tcg_syn); - tcg_temp_free_i32(tcg_excp); -} - static void gen_step_complete_exception(DisasContext *s) { /* We just completed step of an insn. Move from Active-not-pending @@ -308,8 +294,7 @@ static void gen_step_complete_exception(DisasContext *s) * of the exception, and our syndrome information is always correct. */ gen_ss_advance(s); - gen_exception(EXCP_UDEF, syn_swstep(s->ss_same_el, 1, s->is_ldex), - default_exception_el(s)); + gen_swstep_exception(s, 1, s->is_ldex); s->base.is_jmp = DISAS_NORETURN; } @@ -12024,8 +12009,7 @@ static bool arm_pre_translate_insn(DisasContext *dc) * bits should be zero. */ assert(dc->base.num_insns == 1); - gen_exception(EXCP_UDEF, syn_swstep(dc->ss_same_el, 0, 0), - default_exception_el(dc)); + gen_swstep_exception(dc, 0, 0); dc->base.is_jmp = DISAS_NORETURN; return true; } diff --git a/target/arm/translate.h b/target/arm/translate.h index a20f6e2056..45053190ba 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -2,6 +2,7 @@ #define TARGET_ARM_TRANSLATE_H #include "exec/translator.h" +#include "internals.h" /* internal defines */ @@ -232,6 +233,28 @@ static inline void gen_ss_advance(DisasContext *s) } } +static inline void gen_exception(int excp, uint32_t syndrome, + uint32_t target_el) +{ + TCGv_i32 tcg_excp = tcg_const_i32(excp); + TCGv_i32 tcg_syn = tcg_const_i32(syndrome); + TCGv_i32 tcg_el = tcg_const_i32(target_el); + + gen_helper_exception_with_syndrome(cpu_env, tcg_excp, + tcg_syn, tcg_el); + + tcg_temp_free_i32(tcg_el); + tcg_temp_free_i32(tcg_syn); + tcg_temp_free_i32(tcg_excp); +} + +/* Generate an architectural singlestep exception */ +static inline void gen_swstep_exception(DisasContext *s, int isv, int ex) +{ + gen_exception(EXCP_UDEF, syn_swstep(s->ss_same_el, isv, ex), + default_exception_el(s)); +} + /* * Given a VFP floating point constant encoded into an 8 bit immediate in an * instruction, expand it to the actual constant value of the specified From 8bd587c1066f4456ddfe611b571d9439a947d74c Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 15 Aug 2019 09:46:42 +0100 Subject: [PATCH 05/29] target/arm: Fix routing of singlestep exceptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When generating an architectural single-step exception we were routing it to the "default exception level", which is to say the same exception level we execute at except that EL0 exceptions go to EL1. This is incorrect because the debug exception level can be configured by the guest for situations such as single stepping of EL0 and EL1 code by EL2. We have to track the target debug exception level in the TB flags, because it is dependent on CPU state like HCR_EL2.TGE and MDCR_EL2.TDE. (That we were previously calling the arm_debug_target_el() function to determine dc->ss_same_el is itself a bug, though one that would only have manifested as incorrect syndrome information.) Since we are out of TB flag bits unless we want to expand into the cs_base field, we share some bits with the M-profile only HANDLER and STACKCHECK bits, since only A-profile has this singlestep. Fixes: https://bugs.launchpad.net/qemu/+bug/1838913 Signed-off-by: Peter Maydell Reviewed-by: Alex Bennée Tested-by: Alex Bennée Message-id: 20190805130952.4415-3-peter.maydell@linaro.org --- target/arm/cpu.h | 5 +++++ target/arm/helper.c | 6 ++++++ target/arm/translate-a64.c | 2 +- target/arm/translate.c | 4 +++- target/arm/translate.h | 15 +++++++++++---- 5 files changed, 26 insertions(+), 6 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 67f2af0e16..d12c746085 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -3148,6 +3148,11 @@ FIELD(TBFLAG_ANY, PSTATE_SS, 26, 1) /* Target EL if we take a floating-point-disabled exception */ FIELD(TBFLAG_ANY, FPEXC_EL, 24, 2) FIELD(TBFLAG_ANY, BE_DATA, 23, 1) +/* + * For A-profile only, target EL for debug exceptions. + * Note that this overlaps with the M-profile-only HANDLER and STACKCHECK bits. + */ +FIELD(TBFLAG_ANY, DEBUG_TARGET_EL, 21, 2) /* Bit usage when in AArch32 state: */ FIELD(TBFLAG_A32, THUMB, 0, 1) diff --git a/target/arm/helper.c b/target/arm/helper.c index b74c23a9bc..24806c16ca 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -11170,6 +11170,12 @@ void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc, } } + if (!arm_feature(env, ARM_FEATURE_M)) { + int target_el = arm_debug_target_el(env); + + flags = FIELD_DP32(flags, TBFLAG_ANY, DEBUG_TARGET_EL, target_el); + } + *pflags = flags; *cs_base = 0; } diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index f6729b96fd..90850eadc1 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -14180,7 +14180,7 @@ static void aarch64_tr_init_disas_context(DisasContextBase *dcbase, dc->ss_active = FIELD_EX32(tb_flags, TBFLAG_ANY, SS_ACTIVE); dc->pstate_ss = FIELD_EX32(tb_flags, TBFLAG_ANY, PSTATE_SS); dc->is_ldex = false; - dc->ss_same_el = (arm_debug_target_el(env) == dc->current_el); + dc->debug_target_el = FIELD_EX32(tb_flags, TBFLAG_ANY, DEBUG_TARGET_EL); /* Bound the number of insns to execute to those left on the page. */ bound = -(dc->base.pc_first | TARGET_PAGE_MASK) / 4; diff --git a/target/arm/translate.c b/target/arm/translate.c index 19b9d8f272..b32508cd2f 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -11882,7 +11882,9 @@ static void arm_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) dc->ss_active = FIELD_EX32(tb_flags, TBFLAG_ANY, SS_ACTIVE); dc->pstate_ss = FIELD_EX32(tb_flags, TBFLAG_ANY, PSTATE_SS); dc->is_ldex = false; - dc->ss_same_el = false; /* Can't be true since EL_d must be AArch64 */ + if (!arm_feature(env, ARM_FEATURE_M)) { + dc->debug_target_el = FIELD_EX32(tb_flags, TBFLAG_ANY, DEBUG_TARGET_EL); + } dc->page_start = dc->base.pc_first & TARGET_PAGE_MASK; diff --git a/target/arm/translate.h b/target/arm/translate.h index 45053190ba..b65954c669 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -50,6 +50,8 @@ typedef struct DisasContext { uint32_t svc_imm; int aarch64; int current_el; + /* Debug target exception level for single-step exceptions */ + int debug_target_el; GHashTable *cp_regs; uint64_t features; /* CPU features bits */ /* Because unallocated encodings generate different exception syndrome @@ -70,8 +72,6 @@ typedef struct DisasContext { * ie A64 LDX*, LDAX*, A32/T32 LDREX*, LDAEX*. */ bool is_ldex; - /* True if a single-step exception will be taken to the current EL */ - bool ss_same_el; /* True if v8.3-PAuth is active. */ bool pauth_active; /* True with v8.5-BTI and SCTLR_ELx.BT* set. */ @@ -251,8 +251,15 @@ static inline void gen_exception(int excp, uint32_t syndrome, /* Generate an architectural singlestep exception */ static inline void gen_swstep_exception(DisasContext *s, int isv, int ex) { - gen_exception(EXCP_UDEF, syn_swstep(s->ss_same_el, isv, ex), - default_exception_el(s)); + bool same_el = (s->debug_target_el == s->current_el); + + /* + * If singlestep is targeting a lower EL than the current one, + * then s->ss_active must be false and we can never get here. + */ + assert(s->debug_target_el >= s->current_el); + + gen_exception(EXCP_UDEF, syn_swstep(same_el, isv, ex), s->debug_target_el); } /* From 331b1ca616cb708db30dab68e3262d286e687f24 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 15 Aug 2019 09:46:42 +0100 Subject: [PATCH 06/29] target/arm: Pass in pc to thumb_insn_is_16bit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This function is used in two different contexts, and it will be clearer if the function is given the address to which it applies. Signed-off-by: Richard Henderson Reviewed-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20190807045335.1361-2-richard.henderson@linaro.org Signed-off-by: Peter Maydell --- target/arm/translate.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index b32508cd2f..de941e6b3d 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -9246,11 +9246,11 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) } } -static bool thumb_insn_is_16bit(DisasContext *s, uint32_t insn) +static bool thumb_insn_is_16bit(DisasContext *s, uint32_t pc, uint32_t insn) { - /* Return true if this is a 16 bit instruction. We must be precise - * about this (matching the decode). We assume that s->pc still - * points to the first 16 bits of the insn. + /* + * Return true if this is a 16 bit instruction. We must be precise + * about this (matching the decode). */ if ((insn >> 11) < 0x1d) { /* Definitely a 16-bit instruction */ @@ -9270,7 +9270,7 @@ static bool thumb_insn_is_16bit(DisasContext *s, uint32_t insn) return false; } - if ((insn >> 11) == 0x1e && s->pc - s->page_start < TARGET_PAGE_SIZE - 3) { + if ((insn >> 11) == 0x1e && pc - s->page_start < TARGET_PAGE_SIZE - 3) { /* 0b1111_0xxx_xxxx_xxxx : BL/BLX prefix, and the suffix * is not on the next page; we merge this into a 32-bit * insn. @@ -11809,7 +11809,7 @@ static bool insn_crosses_page(CPUARMState *env, DisasContext *s) */ uint16_t insn = arm_lduw_code(env, s->pc, s->sctlr_b); - return !thumb_insn_is_16bit(s, insn); + return !thumb_insn_is_16bit(s, s->pc, insn); } static void arm_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) @@ -12108,7 +12108,7 @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) } insn = arm_lduw_code(env, dc->pc, dc->sctlr_b); - is_16bit = thumb_insn_is_16bit(dc, insn); + is_16bit = thumb_insn_is_16bit(dc, dc->pc, insn); dc->pc += 2; if (!is_16bit) { uint32_t insn2 = arm_lduw_code(env, dc->pc, dc->sctlr_b); From 43722a6d4f0c92f7e7e1e291580039b0f9789df1 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 15 Aug 2019 09:46:43 +0100 Subject: [PATCH 07/29] target/arm: Introduce pc_curr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new field to retain the address of the instruction currently being translated. The 32-bit uses are all within subroutines used by a32 and t32. This will become less obvious when t16 support is merged with a32+t32, and having a clear definition will help. Convert aarch64 as well for consistency. Note that there is one instance of a pre-assert fprintf that used the wrong value for the address of the current instruction. Signed-off-by: Richard Henderson Reviewed-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20190807045335.1361-3-richard.henderson@linaro.org Signed-off-by: Peter Maydell --- target/arm/translate-a64.c | 21 +++++++++++---------- target/arm/translate-a64.h | 2 +- target/arm/translate.c | 14 ++++++++------ target/arm/translate.h | 2 ++ 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index 90850eadc1..a0b557ddce 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -1234,7 +1234,7 @@ static inline AArch64DecodeFn *lookup_disas_fn(const AArch64DecodeTable *table, */ static void disas_uncond_b_imm(DisasContext *s, uint32_t insn) { - uint64_t addr = s->pc + sextract32(insn, 0, 26) * 4 - 4; + uint64_t addr = s->pc_curr + sextract32(insn, 0, 26) * 4; if (insn & (1U << 31)) { /* BL Branch with link */ @@ -1262,7 +1262,7 @@ static void disas_comp_b_imm(DisasContext *s, uint32_t insn) sf = extract32(insn, 31, 1); op = extract32(insn, 24, 1); /* 0: CBZ; 1: CBNZ */ rt = extract32(insn, 0, 5); - addr = s->pc + sextract32(insn, 5, 19) * 4 - 4; + addr = s->pc_curr + sextract32(insn, 5, 19) * 4; tcg_cmp = read_cpu_reg(s, rt, sf); label_match = gen_new_label(); @@ -1291,7 +1291,7 @@ static void disas_test_b_imm(DisasContext *s, uint32_t insn) bit_pos = (extract32(insn, 31, 1) << 5) | extract32(insn, 19, 5); op = extract32(insn, 24, 1); /* 0: TBZ; 1: TBNZ */ - addr = s->pc + sextract32(insn, 5, 14) * 4 - 4; + addr = s->pc_curr + sextract32(insn, 5, 14) * 4; rt = extract32(insn, 0, 5); tcg_cmp = tcg_temp_new_i64(); @@ -1322,7 +1322,7 @@ static void disas_cond_b_imm(DisasContext *s, uint32_t insn) unallocated_encoding(s); return; } - addr = s->pc + sextract32(insn, 5, 19) * 4 - 4; + addr = s->pc_curr + sextract32(insn, 5, 19) * 4; cond = extract32(insn, 0, 4); reset_btype(s); @@ -1706,7 +1706,7 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, TCGv_i32 tcg_syn, tcg_isread; uint32_t syndrome; - gen_a64_set_pc_im(s->pc - 4); + gen_a64_set_pc_im(s->pc_curr); tmpptr = tcg_const_ptr(ri); syndrome = syn_aa64_sysregtrap(op0, op1, op2, crn, crm, rt, isread); tcg_syn = tcg_const_i32(syndrome); @@ -1870,7 +1870,7 @@ static void disas_exc(DisasContext *s, uint32_t insn) /* The pre HVC helper handles cases when HVC gets trapped * as an undefined insn by runtime configuration. */ - gen_a64_set_pc_im(s->pc - 4); + gen_a64_set_pc_im(s->pc_curr); gen_helper_pre_hvc(cpu_env); gen_ss_advance(s); gen_exception_insn(s, 0, EXCP_HVC, syn_aa64_hvc(imm16), 2); @@ -1880,7 +1880,7 @@ static void disas_exc(DisasContext *s, uint32_t insn) unallocated_encoding(s); break; } - gen_a64_set_pc_im(s->pc - 4); + gen_a64_set_pc_im(s->pc_curr); tmp = tcg_const_i32(syn_aa64_smc(imm16)); gen_helper_pre_smc(cpu_env, tmp); tcg_temp_free_i32(tmp); @@ -2601,7 +2601,7 @@ static void disas_ld_lit(DisasContext *s, uint32_t insn) tcg_rt = cpu_reg(s, rt); - clean_addr = tcg_const_i64((s->pc - 4) + imm); + clean_addr = tcg_const_i64(s->pc_curr + imm); if (is_vector) { do_fp_ld(s, rt, clean_addr, size); } else { @@ -3580,7 +3580,7 @@ static void disas_pc_rel_adr(DisasContext *s, uint32_t insn) offset = sextract64(insn, 5, 19); offset = offset << 2 | extract32(insn, 29, 2); rd = extract32(insn, 0, 5); - base = s->pc - 4; + base = s->pc_curr; if (page) { /* ADRP (page based) */ @@ -11519,7 +11519,7 @@ static void disas_simd_three_reg_same_fp16(DisasContext *s, uint32_t insn) break; default: fprintf(stderr, "%s: insn %#04x, fpop %#2x @ %#" PRIx64 "\n", - __func__, insn, fpopcode, s->pc); + __func__, insn, fpopcode, s->pc_curr); g_assert_not_reached(); } @@ -14030,6 +14030,7 @@ static void disas_a64_insn(CPUARMState *env, DisasContext *s) { uint32_t insn; + s->pc_curr = s->pc; insn = arm_ldl_code(env, s->pc, s->sctlr_b); s->insn = insn; s->pc += 4; diff --git a/target/arm/translate-a64.h b/target/arm/translate-a64.h index 9ab40872d8..9cd2b3d238 100644 --- a/target/arm/translate-a64.h +++ b/target/arm/translate-a64.h @@ -25,7 +25,7 @@ void unallocated_encoding(DisasContext *s); qemu_log_mask(LOG_UNIMP, \ "%s:%d: unsupported instruction encoding 0x%08x " \ "at pc=%016" PRIx64 "\n", \ - __FILE__, __LINE__, insn, s->pc - 4); \ + __FILE__, __LINE__, insn, s->pc_curr); \ unallocated_encoding(s); \ } while (0) diff --git a/target/arm/translate.c b/target/arm/translate.c index de941e6b3d..fed08c63f8 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -1197,7 +1197,7 @@ static inline void gen_hvc(DisasContext *s, int imm16) * as an undefined insn by runtime configuration (ie before * the insn really executes). */ - gen_set_pc_im(s, s->pc - 4); + gen_set_pc_im(s, s->pc_curr); gen_helper_pre_hvc(cpu_env); /* Otherwise we will treat this as a real exception which * happens after execution of the insn. (The distinction matters @@ -1216,7 +1216,7 @@ static inline void gen_smc(DisasContext *s) */ TCGv_i32 tmp; - gen_set_pc_im(s, s->pc - 4); + gen_set_pc_im(s, s->pc_curr); tmp = tcg_const_i32(syn_aa32_smc()); gen_helper_pre_smc(cpu_env, tmp); tcg_temp_free_i32(tmp); @@ -3175,7 +3175,7 @@ static void gen_msr_banked(DisasContext *s, int r, int sysm, int rn) /* Sync state because msr_banked() can raise exceptions */ gen_set_condexec(s); - gen_set_pc_im(s, s->pc - 4); + gen_set_pc_im(s, s->pc_curr); tcg_reg = load_reg(s, rn); tcg_tgtmode = tcg_const_i32(tgtmode); tcg_regno = tcg_const_i32(regno); @@ -3197,7 +3197,7 @@ static void gen_mrs_banked(DisasContext *s, int r, int sysm, int rn) /* Sync state because mrs_banked() can raise exceptions */ gen_set_condexec(s); - gen_set_pc_im(s, s->pc - 4); + gen_set_pc_im(s, s->pc_curr); tcg_reg = tcg_temp_new_i32(); tcg_tgtmode = tcg_const_i32(tgtmode); tcg_regno = tcg_const_i32(regno); @@ -7204,7 +7204,7 @@ static int disas_coproc_insn(DisasContext *s, uint32_t insn) } gen_set_condexec(s); - gen_set_pc_im(s, s->pc - 4); + gen_set_pc_im(s, s->pc_curr); tmpptr = tcg_const_ptr(ri); tcg_syn = tcg_const_i32(syndrome); tcg_isread = tcg_const_i32(isread); @@ -7614,7 +7614,7 @@ static void gen_srs(DisasContext *s, tmp = tcg_const_i32(mode); /* get_r13_banked() will raise an exception if called from System mode */ gen_set_condexec(s); - gen_set_pc_im(s, s->pc - 4); + gen_set_pc_im(s, s->pc_curr); gen_helper_get_r13_banked(addr, cpu_env, tmp); tcg_temp_free_i32(tmp); switch (amode) { @@ -12039,6 +12039,7 @@ static void arm_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) return; } + dc->pc_curr = dc->pc; insn = arm_ldl_code(env, dc->pc, dc->sctlr_b); dc->insn = insn; dc->pc += 4; @@ -12107,6 +12108,7 @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) return; } + dc->pc_curr = dc->pc; insn = arm_lduw_code(env, dc->pc, dc->sctlr_b); is_16bit = thumb_insn_is_16bit(dc, dc->pc, insn); dc->pc += 2; diff --git a/target/arm/translate.h b/target/arm/translate.h index b65954c669..53ac50bc02 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -11,6 +11,8 @@ typedef struct DisasContext { const ARMISARegisters *isar; target_ulong pc; + /* The address of the current instruction being translated. */ + target_ulong pc_curr; target_ulong page_start; uint32_t insn; /* Nonzero if this instruction has been conditionally skipped. */ From fdbcf6329d0c2984c55d7019419a72bf8e583c36 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 15 Aug 2019 09:46:43 +0100 Subject: [PATCH 08/29] target/arm: Introduce read_pc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We currently have 3 different ways of computing the architectural value of "PC" as seen in the ARM ARM. The value of s->pc has been incremented past the current insn, but that is all. Thus for a32, PC = s->pc + 4; for t32, PC = s->pc; for t16, PC = s->pc + 2. These differing computations make it impossible at present to unify the various code paths. With the newly introduced s->pc_curr, we can compute the correct value for all cases, using the formula given in the ARM ARM. This changes the behaviour for load_reg() and load_reg_var() when called with reg==15 from a 32-bit Thumb instruction: previously they would have returned the incorrect value of pc_curr + 6, and now they will return the architecturally correct value of PC, which is pc_curr + 4. This will not affect well-behaved guest software, because all of the places we call these functions from T32 code are instructions where using r15 is UNPREDICTABLE. Using the architectural PC value here is more consistent with the T16 and A32 behaviour. Signed-off-by: Richard Henderson Reviewed-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20190807045335.1361-4-richard.henderson@linaro.org [PMM: added commit message note about UNPREDICTABLE T32 cases] Signed-off-by: Peter Maydell --- target/arm/translate.c | 59 ++++++++++++++++-------------------------- 1 file changed, 23 insertions(+), 36 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index fed08c63f8..41523c0241 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -196,17 +196,17 @@ static inline void store_cpu_offset(TCGv_i32 var, int offset) #define store_cpu_field(var, name) \ store_cpu_offset(var, offsetof(CPUARMState, name)) +/* The architectural value of PC. */ +static uint32_t read_pc(DisasContext *s) +{ + return s->pc_curr + (s->thumb ? 4 : 8); +} + /* Set a variable to the value of a CPU register. */ static void load_reg_var(DisasContext *s, TCGv_i32 var, int reg) { if (reg == 15) { - uint32_t addr; - /* normally, since we updated PC, we need only to add one insn */ - if (s->thumb) - addr = (long)s->pc + 2; - else - addr = (long)s->pc + 4; - tcg_gen_movi_i32(var, addr); + tcg_gen_movi_i32(var, read_pc(s)); } else { tcg_gen_mov_i32(var, cpu_R[reg]); } @@ -7853,16 +7853,14 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) /* branch link and change to thumb (blx ) */ int32_t offset; - val = (uint32_t)s->pc; tmp = tcg_temp_new_i32(); - tcg_gen_movi_i32(tmp, val); + tcg_gen_movi_i32(tmp, s->pc); store_reg(s, 14, tmp); /* Sign-extend the 24-bit offset */ offset = (((int32_t)insn) << 8) >> 8; + val = read_pc(s); /* offset * 4 + bit24 * 2 + (thumb bit) */ val += (offset << 2) | ((insn >> 23) & 2) | 1; - /* pipeline offset */ - val += 4; /* protected by ARCH(5); above, near the start of uncond block */ gen_bx_im(s, val); return; @@ -9138,10 +9136,8 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) } else { /* store */ if (i == 15) { - /* special case: r15 = PC + 8 */ - val = (long)s->pc + 4; tmp = tcg_temp_new_i32(); - tcg_gen_movi_i32(tmp, val); + tcg_gen_movi_i32(tmp, read_pc(s)); } else if (user) { tmp = tcg_temp_new_i32(); tmp2 = tcg_const_i32(i); @@ -9207,15 +9203,13 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) int32_t offset; /* branch (and link) */ - val = (int32_t)s->pc; if (insn & (1 << 24)) { tmp = tcg_temp_new_i32(); - tcg_gen_movi_i32(tmp, val); + tcg_gen_movi_i32(tmp, s->pc); store_reg(s, 14, tmp); } offset = sextract32(insn << 2, 0, 26); - val += offset + 4; - gen_jmp(s, val); + gen_jmp(s, read_pc(s) + offset); } break; case 0xc: @@ -9573,12 +9567,7 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) tcg_temp_free_i32(addr); } else if ((insn & (7 << 5)) == 0) { /* Table Branch. */ - if (rn == 15) { - addr = tcg_temp_new_i32(); - tcg_gen_movi_i32(addr, s->pc); - } else { - addr = load_reg(s, rn); - } + addr = load_reg(s, rn); tmp = load_reg(s, rm); tcg_gen_add_i32(addr, addr, tmp); if (insn & (1 << 4)) { @@ -9594,7 +9583,7 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) } tcg_temp_free_i32(addr); tcg_gen_shli_i32(tmp, tmp, 1); - tcg_gen_addi_i32(tmp, tmp, s->pc); + tcg_gen_addi_i32(tmp, tmp, read_pc(s)); store_reg(s, 15, tmp); } else { bool is_lasr = false; @@ -10327,7 +10316,7 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) tcg_gen_movi_i32(cpu_R[14], s->pc | 1); } - offset += s->pc; + offset += read_pc(s); if (insn & (1 << 12)) { /* b/bl */ gen_jmp(s, offset); @@ -10568,7 +10557,7 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) offset |= (insn & (1 << 11)) << 8; /* jump to the offset */ - gen_jmp(s, s->pc + offset); + gen_jmp(s, read_pc(s) + offset); } } else { /* @@ -11062,7 +11051,7 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn) if (insn & (1 << 11)) { rd = (insn >> 8) & 7; /* load pc-relative. Bit 1 of PC is ignored. */ - val = s->pc + 2 + ((insn & 0xff) * 4); + val = read_pc(s) + ((insn & 0xff) * 4); val &= ~(uint32_t)2; addr = tcg_temp_new_i32(); tcg_gen_movi_i32(addr, val); @@ -11449,7 +11438,7 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn) } else { /* PC. bit 1 is ignored. */ tmp = tcg_temp_new_i32(); - tcg_gen_movi_i32(tmp, (s->pc + 2) & ~(uint32_t)2); + tcg_gen_movi_i32(tmp, read_pc(s) & ~(uint32_t)2); } val = (insn & 0xff) * 4; tcg_gen_addi_i32(tmp, tmp, val); @@ -11569,9 +11558,7 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn) tcg_gen_brcondi_i32(TCG_COND_NE, tmp, 0, s->condlabel); tcg_temp_free_i32(tmp); offset = ((insn & 0xf8) >> 2) | (insn & 0x200) >> 3; - val = (uint32_t)s->pc + 2; - val += offset; - gen_jmp(s, val); + gen_jmp(s, read_pc(s) + offset); break; case 15: /* IT, nop-hint. */ @@ -11735,7 +11722,7 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn) arm_skip_unless(s, cond); /* jump to the offset */ - val = (uint32_t)s->pc + 2; + val = read_pc(s); offset = ((int32_t)insn << 24) >> 24; val += offset << 1; gen_jmp(s, val); @@ -11761,9 +11748,9 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn) break; } /* unconditional branch */ - val = (uint32_t)s->pc; + val = read_pc(s); offset = ((int32_t)insn << 21) >> 21; - val += (offset << 1) + 2; + val += offset << 1; gen_jmp(s, val); break; @@ -11787,7 +11774,7 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn) /* 0b1111_0xxx_xxxx_xxxx : BL/BLX prefix */ uint32_t uoffset = ((int32_t)insn << 21) >> 9; - tcg_gen_movi_i32(cpu_R[14], s->pc + 2 + uoffset); + tcg_gen_movi_i32(cpu_R[14], read_pc(s) + uoffset); } break; } From 16e0d8234ef9291747332d2c431e46808a060472 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 15 Aug 2019 09:46:43 +0100 Subject: [PATCH 09/29] target/arm: Introduce add_reg_for_lit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Provide a common routine for the places that require ALIGN(PC, 4) as the base address as opposed to plain PC. The two are always the same for A32, but the difference is meaningful for thumb mode. Signed-off-by: Richard Henderson Reviewed-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20190807045335.1361-5-richard.henderson@linaro.org Signed-off-by: Peter Maydell --- target/arm/translate-vfp.inc.c | 38 ++------ target/arm/translate.c | 166 +++++++++++++++------------------ 2 files changed, 82 insertions(+), 122 deletions(-) diff --git a/target/arm/translate-vfp.inc.c b/target/arm/translate-vfp.inc.c index 092eb5ec53..262d4177e5 100644 --- a/target/arm/translate-vfp.inc.c +++ b/target/arm/translate-vfp.inc.c @@ -941,14 +941,8 @@ static bool trans_VLDR_VSTR_sp(DisasContext *s, arg_VLDR_VSTR_sp *a) offset = -offset; } - if (s->thumb && a->rn == 15) { - /* This is actually UNPREDICTABLE */ - addr = tcg_temp_new_i32(); - tcg_gen_movi_i32(addr, s->pc & ~2); - } else { - addr = load_reg(s, a->rn); - } - tcg_gen_addi_i32(addr, addr, offset); + /* For thumb, use of PC is UNPREDICTABLE. */ + addr = add_reg_for_lit(s, a->rn, offset); tmp = tcg_temp_new_i32(); if (a->l) { gen_aa32_ld32u(s, tmp, addr, get_mem_index(s)); @@ -983,14 +977,8 @@ static bool trans_VLDR_VSTR_dp(DisasContext *s, arg_VLDR_VSTR_dp *a) offset = -offset; } - if (s->thumb && a->rn == 15) { - /* This is actually UNPREDICTABLE */ - addr = tcg_temp_new_i32(); - tcg_gen_movi_i32(addr, s->pc & ~2); - } else { - addr = load_reg(s, a->rn); - } - tcg_gen_addi_i32(addr, addr, offset); + /* For thumb, use of PC is UNPREDICTABLE. */ + addr = add_reg_for_lit(s, a->rn, offset); tmp = tcg_temp_new_i64(); if (a->l) { gen_aa32_ld64(s, tmp, addr, get_mem_index(s)); @@ -1029,13 +1017,8 @@ static bool trans_VLDM_VSTM_sp(DisasContext *s, arg_VLDM_VSTM_sp *a) return true; } - if (s->thumb && a->rn == 15) { - /* This is actually UNPREDICTABLE */ - addr = tcg_temp_new_i32(); - tcg_gen_movi_i32(addr, s->pc & ~2); - } else { - addr = load_reg(s, a->rn); - } + /* For thumb, use of PC is UNPREDICTABLE. */ + addr = add_reg_for_lit(s, a->rn, 0); if (a->p) { /* pre-decrement */ tcg_gen_addi_i32(addr, addr, -(a->imm << 2)); @@ -1112,13 +1095,8 @@ static bool trans_VLDM_VSTM_dp(DisasContext *s, arg_VLDM_VSTM_dp *a) return true; } - if (s->thumb && a->rn == 15) { - /* This is actually UNPREDICTABLE */ - addr = tcg_temp_new_i32(); - tcg_gen_movi_i32(addr, s->pc & ~2); - } else { - addr = load_reg(s, a->rn); - } + /* For thumb, use of PC is UNPREDICTABLE. */ + addr = add_reg_for_lit(s, a->rn, 0); if (a->p) { /* pre-decrement */ tcg_gen_addi_i32(addr, addr, -(a->imm << 2)); diff --git a/target/arm/translate.c b/target/arm/translate.c index 41523c0241..f481b87f99 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -220,6 +220,23 @@ static inline TCGv_i32 load_reg(DisasContext *s, int reg) return tmp; } +/* + * Create a new temp, REG + OFS, except PC is ALIGN(PC, 4). + * This is used for load/store for which use of PC implies (literal), + * or ADD that implies ADR. + */ +static TCGv_i32 add_reg_for_lit(DisasContext *s, int reg, int ofs) +{ + TCGv_i32 tmp = tcg_temp_new_i32(); + + if (reg == 15) { + tcg_gen_movi_i32(tmp, (read_pc(s) & ~3) + ofs); + } else { + tcg_gen_addi_i32(tmp, cpu_R[reg], ofs); + } + return tmp; +} + /* Set a CPU register. The source must be a temporary and will be marked as dead. */ static void store_reg(DisasContext *s, int reg, TCGv_i32 var) @@ -9457,16 +9474,12 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) */ bool wback = extract32(insn, 21, 1); - if (rn == 15) { - if (insn & (1 << 21)) { - /* UNPREDICTABLE */ - goto illegal_op; - } - addr = tcg_temp_new_i32(); - tcg_gen_movi_i32(addr, s->pc & ~3); - } else { - addr = load_reg(s, rn); + if (rn == 15 && (insn & (1 << 21))) { + /* UNPREDICTABLE */ + goto illegal_op; } + + addr = add_reg_for_lit(s, rn, 0); offset = (insn & 0xff) * 4; if ((insn & (1 << 23)) == 0) { offset = -offset; @@ -10667,27 +10680,15 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) store_reg(s, rd, tmp); } else { /* Add/sub 12-bit immediate. */ - if (rn == 15) { - offset = s->pc & ~(uint32_t)3; - if (insn & (1 << 23)) - offset -= imm; - else - offset += imm; - tmp = tcg_temp_new_i32(); - tcg_gen_movi_i32(tmp, offset); - store_reg(s, rd, tmp); + if (insn & (1 << 23)) { + imm = -imm; + } + tmp = add_reg_for_lit(s, rn, imm); + if (rn == 13 && rd == 13) { + /* ADD SP, SP, imm or SUB SP, SP, imm */ + store_sp_checked(s, tmp); } else { - tmp = load_reg(s, rn); - if (insn & (1 << 23)) - tcg_gen_subi_i32(tmp, tmp, imm); - else - tcg_gen_addi_i32(tmp, tmp, imm); - if (rn == 13 && rd == 13) { - /* ADD SP, SP, imm or SUB SP, SP, imm */ - store_sp_checked(s, tmp); - } else { - store_reg(s, rd, tmp); - } + store_reg(s, rd, tmp); } } } @@ -10801,61 +10802,53 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) } } memidx = get_mem_index(s); - if (rn == 15) { - addr = tcg_temp_new_i32(); - /* PC relative. */ - /* s->pc has already been incremented by 4. */ - imm = s->pc & 0xfffffffc; - if (insn & (1 << 23)) - imm += insn & 0xfff; - else - imm -= insn & 0xfff; - tcg_gen_movi_i32(addr, imm); + imm = insn & 0xfff; + if (insn & (1 << 23)) { + /* PC relative or Positive offset. */ + addr = add_reg_for_lit(s, rn, imm); + } else if (rn == 15) { + /* PC relative with negative offset. */ + addr = add_reg_for_lit(s, rn, -imm); } else { addr = load_reg(s, rn); - if (insn & (1 << 23)) { - /* Positive offset. */ - imm = insn & 0xfff; - tcg_gen_addi_i32(addr, addr, imm); - } else { - imm = insn & 0xff; - switch ((insn >> 8) & 0xf) { - case 0x0: /* Shifted Register. */ - shift = (insn >> 4) & 0xf; - if (shift > 3) { - tcg_temp_free_i32(addr); - goto illegal_op; - } - tmp = load_reg(s, rm); - if (shift) - tcg_gen_shli_i32(tmp, tmp, shift); - tcg_gen_add_i32(addr, addr, tmp); - tcg_temp_free_i32(tmp); - break; - case 0xc: /* Negative offset. */ - tcg_gen_addi_i32(addr, addr, -imm); - break; - case 0xe: /* User privilege. */ - tcg_gen_addi_i32(addr, addr, imm); - memidx = get_a32_user_mem_index(s); - break; - case 0x9: /* Post-decrement. */ - imm = -imm; - /* Fall through. */ - case 0xb: /* Post-increment. */ - postinc = 1; - writeback = 1; - break; - case 0xd: /* Pre-decrement. */ - imm = -imm; - /* Fall through. */ - case 0xf: /* Pre-increment. */ - writeback = 1; - break; - default: + imm = insn & 0xff; + switch ((insn >> 8) & 0xf) { + case 0x0: /* Shifted Register. */ + shift = (insn >> 4) & 0xf; + if (shift > 3) { tcg_temp_free_i32(addr); goto illegal_op; } + tmp = load_reg(s, rm); + if (shift) { + tcg_gen_shli_i32(tmp, tmp, shift); + } + tcg_gen_add_i32(addr, addr, tmp); + tcg_temp_free_i32(tmp); + break; + case 0xc: /* Negative offset. */ + tcg_gen_addi_i32(addr, addr, -imm); + break; + case 0xe: /* User privilege. */ + tcg_gen_addi_i32(addr, addr, imm); + memidx = get_a32_user_mem_index(s); + break; + case 0x9: /* Post-decrement. */ + imm = -imm; + /* Fall through. */ + case 0xb: /* Post-increment. */ + postinc = 1; + writeback = 1; + break; + case 0xd: /* Pre-decrement. */ + imm = -imm; + /* Fall through. */ + case 0xf: /* Pre-increment. */ + writeback = 1; + break; + default: + tcg_temp_free_i32(addr); + goto illegal_op; } } @@ -11051,10 +11044,7 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn) if (insn & (1 << 11)) { rd = (insn >> 8) & 7; /* load pc-relative. Bit 1 of PC is ignored. */ - val = read_pc(s) + ((insn & 0xff) * 4); - val &= ~(uint32_t)2; - addr = tcg_temp_new_i32(); - tcg_gen_movi_i32(addr, val); + addr = add_reg_for_lit(s, 15, (insn & 0xff) * 4); tmp = tcg_temp_new_i32(); gen_aa32_ld32u_iss(s, tmp, addr, get_mem_index(s), rd | ISSIs16Bit); @@ -11432,16 +11422,8 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn) * - Add PC/SP (immediate) */ rd = (insn >> 8) & 7; - if (insn & (1 << 11)) { - /* SP */ - tmp = load_reg(s, 13); - } else { - /* PC. bit 1 is ignored. */ - tmp = tcg_temp_new_i32(); - tcg_gen_movi_i32(tmp, read_pc(s) & ~(uint32_t)2); - } val = (insn & 0xff) * 4; - tcg_gen_addi_i32(tmp, tmp, val); + tmp = add_reg_for_lit(s, insn & (1 << 11) ? 13 : 15, val); store_reg(s, rd, tmp); break; From 4818c3743b0e0095fdcecd24457da9b3443730ab Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 15 Aug 2019 09:46:43 +0100 Subject: [PATCH 10/29] target/arm: Remove redundant s->pc & ~1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The thumb bit has already been removed from s->pc, and is always even. Signed-off-by: Richard Henderson Reviewed-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20190807045335.1361-6-richard.henderson@linaro.org Signed-off-by: Peter Maydell --- target/arm/translate.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index f481b87f99..8779d36a86 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -1273,7 +1273,7 @@ static void gen_exception_bkpt_insn(DisasContext *s, int offset, uint32_t syn) /* Force a TB lookup after an instruction that changes the CPU state. */ static inline void gen_lookup_tb(DisasContext *s) { - tcg_gen_movi_i32(cpu_R[15], s->pc & ~1); + tcg_gen_movi_i32(cpu_R[15], s->pc); s->base.is_jmp = DISAS_EXIT; } @@ -7804,7 +7804,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) * self-modifying code correctly and also to take * any pending interrupts immediately. */ - gen_goto_tb(s, 0, s->pc & ~1); + gen_goto_tb(s, 0, s->pc); return; case 7: /* sb */ if ((insn & 0xf) || !dc_isar_feature(aa32_sb, s)) { @@ -7815,7 +7815,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) * for TCG; MB and end the TB instead. */ tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC); - gen_goto_tb(s, 0, s->pc & ~1); + gen_goto_tb(s, 0, s->pc); return; default: goto illegal_op; @@ -10449,7 +10449,7 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) * and also to take any pending interrupts * immediately. */ - gen_goto_tb(s, 0, s->pc & ~1); + gen_goto_tb(s, 0, s->pc); break; case 7: /* sb */ if ((insn & 0xf) || !dc_isar_feature(aa32_sb, s)) { @@ -10460,7 +10460,7 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) * for TCG; MB and end the TB instead. */ tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC); - gen_goto_tb(s, 0, s->pc & ~1); + gen_goto_tb(s, 0, s->pc); break; default: goto illegal_op; From a04159166b880b505ccadc16f2fe84169806883d Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 15 Aug 2019 09:46:44 +0100 Subject: [PATCH 11/29] target/arm: Replace s->pc with s->base.pc_next MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We must update s->base.pc_next when we return from the translate_insn hook to the main translator loop. By incrementing s->base.pc_next immediately after reading the insn word, "pc_next" contains the address of the next instruction throughout translation. All remaining uses of s->pc are referencing the address of the next insn, so this is now a simple global replacement. Remove the "s->pc" field. Signed-off-by: Richard Henderson Reviewed-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20190807045335.1361-7-richard.henderson@linaro.org Signed-off-by: Peter Maydell --- target/arm/translate-a64.c | 51 +++++++++--------- target/arm/translate.c | 103 ++++++++++++++++++------------------- target/arm/translate.h | 1 - 3 files changed, 72 insertions(+), 83 deletions(-) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index a0b557ddce..bc89f2c831 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -255,7 +255,7 @@ static void gen_exception_internal(int excp) static void gen_exception_internal_insn(DisasContext *s, int offset, int excp) { - gen_a64_set_pc_im(s->pc - offset); + gen_a64_set_pc_im(s->base.pc_next - offset); gen_exception_internal(excp); s->base.is_jmp = DISAS_NORETURN; } @@ -263,7 +263,7 @@ static void gen_exception_internal_insn(DisasContext *s, int offset, int excp) static void gen_exception_insn(DisasContext *s, int offset, int excp, uint32_t syndrome, uint32_t target_el) { - gen_a64_set_pc_im(s->pc - offset); + gen_a64_set_pc_im(s->base.pc_next - offset); gen_exception(excp, syndrome, target_el); s->base.is_jmp = DISAS_NORETURN; } @@ -273,7 +273,7 @@ static void gen_exception_bkpt_insn(DisasContext *s, int offset, { TCGv_i32 tcg_syn; - gen_a64_set_pc_im(s->pc - offset); + gen_a64_set_pc_im(s->base.pc_next - offset); tcg_syn = tcg_const_i32(syndrome); gen_helper_exception_bkpt_insn(cpu_env, tcg_syn); tcg_temp_free_i32(tcg_syn); @@ -1238,7 +1238,7 @@ static void disas_uncond_b_imm(DisasContext *s, uint32_t insn) if (insn & (1U << 31)) { /* BL Branch with link */ - tcg_gen_movi_i64(cpu_reg(s, 30), s->pc); + tcg_gen_movi_i64(cpu_reg(s, 30), s->base.pc_next); } /* B Branch / BL Branch with link */ @@ -1271,7 +1271,7 @@ static void disas_comp_b_imm(DisasContext *s, uint32_t insn) tcg_gen_brcondi_i64(op ? TCG_COND_NE : TCG_COND_EQ, tcg_cmp, 0, label_match); - gen_goto_tb(s, 0, s->pc); + gen_goto_tb(s, 0, s->base.pc_next); gen_set_label(label_match); gen_goto_tb(s, 1, addr); } @@ -1302,7 +1302,7 @@ static void disas_test_b_imm(DisasContext *s, uint32_t insn) tcg_gen_brcondi_i64(op ? TCG_COND_NE : TCG_COND_EQ, tcg_cmp, 0, label_match); tcg_temp_free_i64(tcg_cmp); - gen_goto_tb(s, 0, s->pc); + gen_goto_tb(s, 0, s->base.pc_next); gen_set_label(label_match); gen_goto_tb(s, 1, addr); } @@ -1330,7 +1330,7 @@ static void disas_cond_b_imm(DisasContext *s, uint32_t insn) /* genuinely conditional branches */ TCGLabel *label_match = gen_new_label(); arm_gen_test_cc(cond, label_match); - gen_goto_tb(s, 0, s->pc); + gen_goto_tb(s, 0, s->base.pc_next); gen_set_label(label_match); gen_goto_tb(s, 1, addr); } else { @@ -1491,7 +1491,7 @@ static void handle_sync(DisasContext *s, uint32_t insn, * any pending interrupts immediately. */ reset_btype(s); - gen_goto_tb(s, 0, s->pc); + gen_goto_tb(s, 0, s->base.pc_next); return; case 7: /* SB */ @@ -1503,7 +1503,7 @@ static void handle_sync(DisasContext *s, uint32_t insn, * MB and end the TB instead. */ tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC); - gen_goto_tb(s, 0, s->pc); + gen_goto_tb(s, 0, s->base.pc_next); return; default: @@ -2015,7 +2015,7 @@ static void disas_uncond_b_reg(DisasContext *s, uint32_t insn) gen_a64_set_pc(s, dst); /* BLR also needs to load return address */ if (opc == 1) { - tcg_gen_movi_i64(cpu_reg(s, 30), s->pc); + tcg_gen_movi_i64(cpu_reg(s, 30), s->base.pc_next); } break; @@ -2042,7 +2042,7 @@ static void disas_uncond_b_reg(DisasContext *s, uint32_t insn) gen_a64_set_pc(s, dst); /* BLRAA also needs to load return address */ if (opc == 9) { - tcg_gen_movi_i64(cpu_reg(s, 30), s->pc); + tcg_gen_movi_i64(cpu_reg(s, 30), s->base.pc_next); } break; @@ -14030,10 +14030,10 @@ static void disas_a64_insn(CPUARMState *env, DisasContext *s) { uint32_t insn; - s->pc_curr = s->pc; - insn = arm_ldl_code(env, s->pc, s->sctlr_b); + s->pc_curr = s->base.pc_next; + insn = arm_ldl_code(env, s->base.pc_next, s->sctlr_b); s->insn = insn; - s->pc += 4; + s->base.pc_next += 4; s->fp_access_checked = false; @@ -14130,7 +14130,6 @@ static void aarch64_tr_init_disas_context(DisasContextBase *dcbase, int bound, core_mmu_idx; dc->isar = &arm_cpu->isar; - dc->pc = dc->base.pc_first; dc->condjmp = 0; dc->aarch64 = 1; @@ -14203,7 +14202,7 @@ static void aarch64_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu) { DisasContext *dc = container_of(dcbase, DisasContext, base); - tcg_gen_insn_start(dc->pc, 0, 0); + tcg_gen_insn_start(dc->base.pc_next, 0, 0); dc->insn_start = tcg_last_op(); } @@ -14213,7 +14212,7 @@ static bool aarch64_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cpu, DisasContext *dc = container_of(dcbase, DisasContext, base); if (bp->flags & BP_CPU) { - gen_a64_set_pc_im(dc->pc); + gen_a64_set_pc_im(dc->base.pc_next); gen_helper_check_breakpoints(cpu_env); /* End the TB early; it likely won't be executed */ dc->base.is_jmp = DISAS_TOO_MANY; @@ -14224,7 +14223,7 @@ static bool aarch64_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cpu, to for it to be properly cleared -- thus we increment the PC here so that the logic setting tb->size below does the right thing. */ - dc->pc += 4; + dc->base.pc_next += 4; dc->base.is_jmp = DISAS_NORETURN; } @@ -14254,7 +14253,6 @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) disas_a64_insn(env, dc); } - dc->base.pc_next = dc->pc; translator_loop_temp_check(&dc->base); } @@ -14270,7 +14268,7 @@ static void aarch64_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) */ switch (dc->base.is_jmp) { default: - gen_a64_set_pc_im(dc->pc); + gen_a64_set_pc_im(dc->base.pc_next); /* fall through */ case DISAS_EXIT: case DISAS_JUMP: @@ -14287,11 +14285,11 @@ static void aarch64_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) switch (dc->base.is_jmp) { case DISAS_NEXT: case DISAS_TOO_MANY: - gen_goto_tb(dc, 1, dc->pc); + gen_goto_tb(dc, 1, dc->base.pc_next); break; default: case DISAS_UPDATE: - gen_a64_set_pc_im(dc->pc); + gen_a64_set_pc_im(dc->base.pc_next); /* fall through */ case DISAS_EXIT: tcg_gen_exit_tb(NULL, 0); @@ -14303,11 +14301,11 @@ static void aarch64_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) case DISAS_SWI: break; case DISAS_WFE: - gen_a64_set_pc_im(dc->pc); + gen_a64_set_pc_im(dc->base.pc_next); gen_helper_wfe(cpu_env); break; case DISAS_YIELD: - gen_a64_set_pc_im(dc->pc); + gen_a64_set_pc_im(dc->base.pc_next); gen_helper_yield(cpu_env); break; case DISAS_WFI: @@ -14317,7 +14315,7 @@ static void aarch64_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) */ TCGv_i32 tmp = tcg_const_i32(4); - gen_a64_set_pc_im(dc->pc); + gen_a64_set_pc_im(dc->base.pc_next); gen_helper_wfi(cpu_env, tmp); tcg_temp_free_i32(tmp); /* The helper doesn't necessarily throw an exception, but we @@ -14328,9 +14326,6 @@ static void aarch64_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) } } } - - /* Functions above can change dc->pc, so re-align db->pc_next */ - dc->base.pc_next = dc->pc; } static void aarch64_tr_disas_log(const DisasContextBase *dcbase, diff --git a/target/arm/translate.c b/target/arm/translate.c index 8779d36a86..14572b8501 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -1036,7 +1036,7 @@ static inline void gen_blxns(DisasContext *s, int rm) * We do however need to set the PC, because the blxns helper reads it. * The blxns helper may throw an exception. */ - gen_set_pc_im(s, s->pc); + gen_set_pc_im(s, s->base.pc_next); gen_helper_v7m_blxns(cpu_env, var); tcg_temp_free_i32(var); s->base.is_jmp = DISAS_EXIT; @@ -1222,7 +1222,7 @@ static inline void gen_hvc(DisasContext *s, int imm16) * for single stepping.) */ s->svc_imm = imm16; - gen_set_pc_im(s, s->pc); + gen_set_pc_im(s, s->base.pc_next); s->base.is_jmp = DISAS_HVC; } @@ -1237,14 +1237,14 @@ static inline void gen_smc(DisasContext *s) tmp = tcg_const_i32(syn_aa32_smc()); gen_helper_pre_smc(cpu_env, tmp); tcg_temp_free_i32(tmp); - gen_set_pc_im(s, s->pc); + gen_set_pc_im(s, s->base.pc_next); s->base.is_jmp = DISAS_SMC; } static void gen_exception_internal_insn(DisasContext *s, int offset, int excp) { gen_set_condexec(s); - gen_set_pc_im(s, s->pc - offset); + gen_set_pc_im(s, s->base.pc_next - offset); gen_exception_internal(excp); s->base.is_jmp = DISAS_NORETURN; } @@ -1253,7 +1253,7 @@ static void gen_exception_insn(DisasContext *s, int offset, int excp, int syn, uint32_t target_el) { gen_set_condexec(s); - gen_set_pc_im(s, s->pc - offset); + gen_set_pc_im(s, s->base.pc_next - offset); gen_exception(excp, syn, target_el); s->base.is_jmp = DISAS_NORETURN; } @@ -1263,7 +1263,7 @@ static void gen_exception_bkpt_insn(DisasContext *s, int offset, uint32_t syn) TCGv_i32 tcg_syn; gen_set_condexec(s); - gen_set_pc_im(s, s->pc - offset); + gen_set_pc_im(s, s->base.pc_next - offset); tcg_syn = tcg_const_i32(syn); gen_helper_exception_bkpt_insn(cpu_env, tcg_syn); tcg_temp_free_i32(tcg_syn); @@ -1273,7 +1273,7 @@ static void gen_exception_bkpt_insn(DisasContext *s, int offset, uint32_t syn) /* Force a TB lookup after an instruction that changes the CPU state. */ static inline void gen_lookup_tb(DisasContext *s) { - tcg_gen_movi_i32(cpu_R[15], s->pc); + tcg_gen_movi_i32(cpu_R[15], s->base.pc_next); s->base.is_jmp = DISAS_EXIT; } @@ -2909,7 +2909,7 @@ static inline bool use_goto_tb(DisasContext *s, target_ulong dest) { #ifndef CONFIG_USER_ONLY return (s->base.tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK) || - ((s->pc - 1) & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK); + ((s->base.pc_next - 1) & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK); #else return true; #endif @@ -3279,17 +3279,17 @@ static void gen_nop_hint(DisasContext *s, int val) */ case 1: /* yield */ if (!(tb_cflags(s->base.tb) & CF_PARALLEL)) { - gen_set_pc_im(s, s->pc); + gen_set_pc_im(s, s->base.pc_next); s->base.is_jmp = DISAS_YIELD; } break; case 3: /* wfi */ - gen_set_pc_im(s, s->pc); + gen_set_pc_im(s, s->base.pc_next); s->base.is_jmp = DISAS_WFI; break; case 2: /* wfe */ if (!(tb_cflags(s->base.tb) & CF_PARALLEL)) { - gen_set_pc_im(s, s->pc); + gen_set_pc_im(s, s->base.pc_next); s->base.is_jmp = DISAS_WFE; } break; @@ -7240,7 +7240,7 @@ static int disas_coproc_insn(DisasContext *s, uint32_t insn) if (isread) { return 1; } - gen_set_pc_im(s, s->pc); + gen_set_pc_im(s, s->base.pc_next); s->base.is_jmp = DISAS_WFI; return 0; default: @@ -7804,7 +7804,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) * self-modifying code correctly and also to take * any pending interrupts immediately. */ - gen_goto_tb(s, 0, s->pc); + gen_goto_tb(s, 0, s->base.pc_next); return; case 7: /* sb */ if ((insn & 0xf) || !dc_isar_feature(aa32_sb, s)) { @@ -7815,7 +7815,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) * for TCG; MB and end the TB instead. */ tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC); - gen_goto_tb(s, 0, s->pc); + gen_goto_tb(s, 0, s->base.pc_next); return; default: goto illegal_op; @@ -7871,7 +7871,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) int32_t offset; tmp = tcg_temp_new_i32(); - tcg_gen_movi_i32(tmp, s->pc); + tcg_gen_movi_i32(tmp, s->base.pc_next); store_reg(s, 14, tmp); /* Sign-extend the 24-bit offset */ offset = (((int32_t)insn) << 8) >> 8; @@ -8056,7 +8056,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) /* branch link/exchange thumb (blx) */ tmp = load_reg(s, rm); tmp2 = tcg_temp_new_i32(); - tcg_gen_movi_i32(tmp2, s->pc); + tcg_gen_movi_i32(tmp2, s->base.pc_next); store_reg(s, 14, tmp2); gen_bx(s, tmp); break; @@ -9222,7 +9222,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) /* branch (and link) */ if (insn & (1 << 24)) { tmp = tcg_temp_new_i32(); - tcg_gen_movi_i32(tmp, s->pc); + tcg_gen_movi_i32(tmp, s->base.pc_next); store_reg(s, 14, tmp); } offset = sextract32(insn << 2, 0, 26); @@ -9244,7 +9244,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) break; case 0xf: /* swi */ - gen_set_pc_im(s, s->pc); + gen_set_pc_im(s, s->base.pc_next); s->svc_imm = extract32(insn, 0, 24); s->base.is_jmp = DISAS_SWI; break; @@ -10326,7 +10326,7 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) if (insn & (1 << 14)) { /* Branch and link. */ - tcg_gen_movi_i32(cpu_R[14], s->pc | 1); + tcg_gen_movi_i32(cpu_R[14], s->base.pc_next | 1); } offset += read_pc(s); @@ -10449,7 +10449,7 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) * and also to take any pending interrupts * immediately. */ - gen_goto_tb(s, 0, s->pc); + gen_goto_tb(s, 0, s->base.pc_next); break; case 7: /* sb */ if ((insn & 0xf) || !dc_isar_feature(aa32_sb, s)) { @@ -10460,7 +10460,7 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) * for TCG; MB and end the TB instead. */ tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC); - gen_goto_tb(s, 0, s->pc); + gen_goto_tb(s, 0, s->base.pc_next); break; default: goto illegal_op; @@ -11121,7 +11121,7 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn) /* BLX/BX */ tmp = load_reg(s, rm); if (link) { - val = (uint32_t)s->pc | 1; + val = (uint32_t)s->base.pc_next | 1; tmp2 = tcg_temp_new_i32(); tcg_gen_movi_i32(tmp2, val); store_reg(s, 14, tmp2); @@ -11695,7 +11695,7 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn) if (cond == 0xf) { /* swi */ - gen_set_pc_im(s, s->pc); + gen_set_pc_im(s, s->base.pc_next); s->svc_imm = extract32(insn, 0, 8); s->base.is_jmp = DISAS_SWI; break; @@ -11724,7 +11724,7 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn) tcg_gen_andi_i32(tmp, tmp, 0xfffffffc); tmp2 = tcg_temp_new_i32(); - tcg_gen_movi_i32(tmp2, s->pc | 1); + tcg_gen_movi_i32(tmp2, s->base.pc_next | 1); store_reg(s, 14, tmp2); gen_bx(s, tmp); break; @@ -11749,7 +11749,7 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn) tcg_gen_addi_i32(tmp, tmp, offset); tmp2 = tcg_temp_new_i32(); - tcg_gen_movi_i32(tmp2, s->pc | 1); + tcg_gen_movi_i32(tmp2, s->base.pc_next | 1); store_reg(s, 14, tmp2); gen_bx(s, tmp); } else { @@ -11769,16 +11769,16 @@ undef: static bool insn_crosses_page(CPUARMState *env, DisasContext *s) { - /* Return true if the insn at dc->pc might cross a page boundary. + /* Return true if the insn at dc->base.pc_next might cross a page boundary. * (False positives are OK, false negatives are not.) * We know this is a Thumb insn, and our caller ensures we are - * only called if dc->pc is less than 4 bytes from the page + * only called if dc->base.pc_next is less than 4 bytes from the page * boundary, so we cross the page if the first 16 bits indicate * that this is a 32 bit insn. */ - uint16_t insn = arm_lduw_code(env, s->pc, s->sctlr_b); + uint16_t insn = arm_lduw_code(env, s->base.pc_next, s->sctlr_b); - return !thumb_insn_is_16bit(s, s->pc, insn); + return !thumb_insn_is_16bit(s, s->base.pc_next, insn); } static void arm_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) @@ -11790,7 +11790,6 @@ static void arm_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) uint32_t condexec, core_mmu_idx; dc->isar = &cpu->isar; - dc->pc = dc->base.pc_first; dc->condjmp = 0; dc->aarch64 = 0; @@ -11922,7 +11921,7 @@ static void arm_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu) { DisasContext *dc = container_of(dcbase, DisasContext, base); - tcg_gen_insn_start(dc->pc, + tcg_gen_insn_start(dc->base.pc_next, (dc->condexec_cond << 4) | (dc->condexec_mask >> 1), 0); dc->insn_start = tcg_last_op(); @@ -11935,7 +11934,7 @@ static bool arm_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cpu, if (bp->flags & BP_CPU) { gen_set_condexec(dc); - gen_set_pc_im(dc, dc->pc); + gen_set_pc_im(dc, dc->base.pc_next); gen_helper_check_breakpoints(cpu_env); /* End the TB early; it's likely not going to be executed */ dc->base.is_jmp = DISAS_TOO_MANY; @@ -11948,7 +11947,7 @@ static bool arm_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cpu, tb->size below does the right thing. */ /* TODO: Advance PC by correct instruction length to * avoid disassembler error messages */ - dc->pc += 2; + dc->base.pc_next += 2; dc->base.is_jmp = DISAS_NORETURN; } @@ -11959,7 +11958,7 @@ static bool arm_pre_translate_insn(DisasContext *dc) { #ifdef CONFIG_USER_ONLY /* Intercept jump to the magic kernel page. */ - if (dc->pc >= 0xffff0000) { + if (dc->base.pc_next >= 0xffff0000) { /* We always get here via a jump, so know we are not in a conditional execution block. */ gen_exception_internal(EXCP_KERNEL_TRAP); @@ -11994,7 +11993,6 @@ static void arm_post_translate_insn(DisasContext *dc) gen_set_label(dc->condlabel); dc->condjmp = 0; } - dc->base.pc_next = dc->pc; translator_loop_temp_check(&dc->base); } @@ -12008,10 +12006,10 @@ static void arm_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) return; } - dc->pc_curr = dc->pc; - insn = arm_ldl_code(env, dc->pc, dc->sctlr_b); + dc->pc_curr = dc->base.pc_next; + insn = arm_ldl_code(env, dc->base.pc_next, dc->sctlr_b); dc->insn = insn; - dc->pc += 4; + dc->base.pc_next += 4; disas_arm_insn(dc, insn); arm_post_translate_insn(dc); @@ -12077,15 +12075,15 @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) return; } - dc->pc_curr = dc->pc; - insn = arm_lduw_code(env, dc->pc, dc->sctlr_b); - is_16bit = thumb_insn_is_16bit(dc, dc->pc, insn); - dc->pc += 2; + dc->pc_curr = dc->base.pc_next; + insn = arm_lduw_code(env, dc->base.pc_next, dc->sctlr_b); + is_16bit = thumb_insn_is_16bit(dc, dc->base.pc_next, insn); + dc->base.pc_next += 2; if (!is_16bit) { - uint32_t insn2 = arm_lduw_code(env, dc->pc, dc->sctlr_b); + uint32_t insn2 = arm_lduw_code(env, dc->base.pc_next, dc->sctlr_b); insn = insn << 16 | insn2; - dc->pc += 2; + dc->base.pc_next += 2; } dc->insn = insn; @@ -12133,8 +12131,8 @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) * but isn't very efficient). */ if (dc->base.is_jmp == DISAS_NEXT - && (dc->pc - dc->page_start >= TARGET_PAGE_SIZE - || (dc->pc - dc->page_start >= TARGET_PAGE_SIZE - 3 + && (dc->base.pc_next - dc->page_start >= TARGET_PAGE_SIZE + || (dc->base.pc_next - dc->page_start >= TARGET_PAGE_SIZE - 3 && insn_crosses_page(env, dc)))) { dc->base.is_jmp = DISAS_TOO_MANY; } @@ -12179,7 +12177,7 @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) case DISAS_NEXT: case DISAS_TOO_MANY: case DISAS_UPDATE: - gen_set_pc_im(dc, dc->pc); + gen_set_pc_im(dc, dc->base.pc_next); /* fall through */ default: /* FIXME: Single stepping a WFI insn will not halt the CPU. */ @@ -12200,13 +12198,13 @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) switch(dc->base.is_jmp) { case DISAS_NEXT: case DISAS_TOO_MANY: - gen_goto_tb(dc, 1, dc->pc); + gen_goto_tb(dc, 1, dc->base.pc_next); break; case DISAS_JUMP: gen_goto_ptr(); break; case DISAS_UPDATE: - gen_set_pc_im(dc, dc->pc); + gen_set_pc_im(dc, dc->base.pc_next); /* fall through */ default: /* indicate that the hash table must be used to find the next TB */ @@ -12252,15 +12250,12 @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) gen_set_label(dc->condlabel); gen_set_condexec(dc); if (unlikely(is_singlestepping(dc))) { - gen_set_pc_im(dc, dc->pc); + gen_set_pc_im(dc, dc->base.pc_next); gen_singlestep_exception(dc); } else { - gen_goto_tb(dc, 1, dc->pc); + gen_goto_tb(dc, 1, dc->base.pc_next); } } - - /* Functions above can change dc->pc, so re-align db->pc_next */ - dc->base.pc_next = dc->pc; } static void arm_tr_disas_log(const DisasContextBase *dcbase, CPUState *cpu) diff --git a/target/arm/translate.h b/target/arm/translate.h index 53ac50bc02..64304c957e 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -10,7 +10,6 @@ typedef struct DisasContext { DisasContextBase base; const ARMISARegisters *isar; - target_ulong pc; /* The address of the current instruction being translated. */ target_ulong pc_curr; target_ulong page_start; From a767fac802f6fa6220330ea6f408dde2fb41db22 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 15 Aug 2019 09:46:44 +0100 Subject: [PATCH 12/29] target/arm: Replace offset with pc in gen_exception_insn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The offset is variable depending on the instruction set, whereas we have stored values for the current pc and the next pc. Passing in the actual value is clearer in intent. Signed-off-by: Richard Henderson Reviewed-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20190807045335.1361-8-richard.henderson@linaro.org Signed-off-by: Peter Maydell --- target/arm/translate-a64.c | 25 ++++++++++++++----------- target/arm/translate-vfp.inc.c | 6 +++--- target/arm/translate.c | 31 ++++++++++++++++--------------- 3 files changed, 33 insertions(+), 29 deletions(-) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index bc89f2c831..70caf3becb 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -260,10 +260,10 @@ static void gen_exception_internal_insn(DisasContext *s, int offset, int excp) s->base.is_jmp = DISAS_NORETURN; } -static void gen_exception_insn(DisasContext *s, int offset, int excp, +static void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, uint32_t syndrome, uint32_t target_el) { - gen_a64_set_pc_im(s->base.pc_next - offset); + gen_a64_set_pc_im(pc); gen_exception(excp, syndrome, target_el); s->base.is_jmp = DISAS_NORETURN; } @@ -342,7 +342,7 @@ static inline void gen_goto_tb(DisasContext *s, int n, uint64_t dest) void unallocated_encoding(DisasContext *s) { /* Unallocated and reserved encodings are uncategorized */ - gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(), + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), default_exception_el(s)); } @@ -1114,8 +1114,8 @@ static inline bool fp_access_check(DisasContext *s) return true; } - gen_exception_insn(s, 4, EXCP_UDEF, syn_fp_access_trap(1, 0xe, false), - s->fp_excp_el); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, + syn_fp_access_trap(1, 0xe, false), s->fp_excp_el); return false; } @@ -1125,7 +1125,7 @@ static inline bool fp_access_check(DisasContext *s) bool sve_access_check(DisasContext *s) { if (s->sve_excp_el) { - gen_exception_insn(s, 4, EXCP_UDEF, syn_sve_access_trap(), + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_sve_access_trap(), s->sve_excp_el); return false; } @@ -1859,8 +1859,8 @@ static void disas_exc(DisasContext *s, uint32_t insn) switch (op2_ll) { case 1: /* SVC */ gen_ss_advance(s); - gen_exception_insn(s, 0, EXCP_SWI, syn_aa64_svc(imm16), - default_exception_el(s)); + gen_exception_insn(s, s->base.pc_next, EXCP_SWI, + syn_aa64_svc(imm16), default_exception_el(s)); break; case 2: /* HVC */ if (s->current_el == 0) { @@ -1873,7 +1873,8 @@ static void disas_exc(DisasContext *s, uint32_t insn) gen_a64_set_pc_im(s->pc_curr); gen_helper_pre_hvc(cpu_env); gen_ss_advance(s); - gen_exception_insn(s, 0, EXCP_HVC, syn_aa64_hvc(imm16), 2); + gen_exception_insn(s, s->base.pc_next, EXCP_HVC, + syn_aa64_hvc(imm16), 2); break; case 3: /* SMC */ if (s->current_el == 0) { @@ -1885,7 +1886,8 @@ static void disas_exc(DisasContext *s, uint32_t insn) gen_helper_pre_smc(cpu_env, tmp); tcg_temp_free_i32(tmp); gen_ss_advance(s); - gen_exception_insn(s, 0, EXCP_SMC, syn_aa64_smc(imm16), 3); + gen_exception_insn(s, s->base.pc_next, EXCP_SMC, + syn_aa64_smc(imm16), 3); break; default: unallocated_encoding(s); @@ -14064,7 +14066,8 @@ static void disas_a64_insn(CPUARMState *env, DisasContext *s) if (s->btype != 0 && s->guarded_page && !btype_destination_ok(insn, s->bt, s->btype)) { - gen_exception_insn(s, 4, EXCP_UDEF, syn_btitrap(s->btype), + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, + syn_btitrap(s->btype), default_exception_el(s)); return; } diff --git a/target/arm/translate-vfp.inc.c b/target/arm/translate-vfp.inc.c index 262d4177e5..5065d4524c 100644 --- a/target/arm/translate-vfp.inc.c +++ b/target/arm/translate-vfp.inc.c @@ -96,10 +96,10 @@ static bool full_vfp_access_check(DisasContext *s, bool ignore_vfp_enabled) { if (s->fp_excp_el) { if (arm_dc_feature(s, ARM_FEATURE_M)) { - gen_exception_insn(s, 4, EXCP_NOCP, syn_uncategorized(), + gen_exception_insn(s, s->pc_curr, EXCP_NOCP, syn_uncategorized(), s->fp_excp_el); } else { - gen_exception_insn(s, 4, EXCP_UDEF, + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_fp_access_trap(1, 0xe, false), s->fp_excp_el); } @@ -108,7 +108,7 @@ static bool full_vfp_access_check(DisasContext *s, bool ignore_vfp_enabled) if (!s->vfp_enabled && !ignore_vfp_enabled) { assert(!arm_dc_feature(s, ARM_FEATURE_M)); - gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(), + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), default_exception_el(s)); return false; } diff --git a/target/arm/translate.c b/target/arm/translate.c index 14572b8501..60f1b3998a 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -1249,11 +1249,11 @@ static void gen_exception_internal_insn(DisasContext *s, int offset, int excp) s->base.is_jmp = DISAS_NORETURN; } -static void gen_exception_insn(DisasContext *s, int offset, int excp, +static void gen_exception_insn(DisasContext *s, uint32_t pc, int excp, int syn, uint32_t target_el) { gen_set_condexec(s); - gen_set_pc_im(s, s->base.pc_next - offset); + gen_set_pc_im(s, pc); gen_exception(excp, syn, target_el); s->base.is_jmp = DISAS_NORETURN; } @@ -1300,7 +1300,7 @@ static inline void gen_hlt(DisasContext *s, int imm) return; } - gen_exception_insn(s, s->thumb ? 2 : 4, EXCP_UDEF, syn_uncategorized(), + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), default_exception_el(s)); } @@ -3177,7 +3177,8 @@ static bool msr_banked_access_decode(DisasContext *s, int r, int sysm, int rn, undef: /* If we get here then some access check did not pass */ - gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(), exc_target); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, + syn_uncategorized(), exc_target); return false; } @@ -3571,7 +3572,7 @@ static int disas_neon_ls_insn(DisasContext *s, uint32_t insn) * for attempts to execute invalid vfp/neon encodings with FP disabled. */ if (s->fp_excp_el) { - gen_exception_insn(s, 4, EXCP_UDEF, + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_simd_access_trap(1, 0xe, false), s->fp_excp_el); return 0; } @@ -4842,7 +4843,7 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn) * for attempts to execute invalid vfp/neon encodings with FP disabled. */ if (s->fp_excp_el) { - gen_exception_insn(s, 4, EXCP_UDEF, + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_simd_access_trap(1, 0xe, false), s->fp_excp_el); return 0; } @@ -6970,7 +6971,7 @@ static int disas_neon_insn_3same_ext(DisasContext *s, uint32_t insn) } if (s->fp_excp_el) { - gen_exception_insn(s, 4, EXCP_UDEF, + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_simd_access_trap(1, 0xe, false), s->fp_excp_el); return 0; } @@ -7093,7 +7094,7 @@ static int disas_neon_insn_2reg_scalar_ext(DisasContext *s, uint32_t insn) off_rm = vfp_reg_offset(0, rm); } if (s->fp_excp_el) { - gen_exception_insn(s, 4, EXCP_UDEF, + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_simd_access_trap(1, 0xe, false), s->fp_excp_el); return 0; } @@ -7586,7 +7587,7 @@ static void gen_srs(DisasContext *s, * For the UNPREDICTABLE cases we choose to UNDEF. */ if (s->current_el == 1 && !s->ns && mode == ARM_CPU_MODE_MON) { - gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(), 3); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), 3); return; } @@ -7622,7 +7623,7 @@ static void gen_srs(DisasContext *s, } if (undef) { - gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(), + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), default_exception_el(s)); return; } @@ -7713,7 +7714,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) * UsageFault exception. */ if (arm_dc_feature(s, ARM_FEATURE_M)) { - gen_exception_insn(s, 4, EXCP_INVSTATE, syn_uncategorized(), + gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(), default_exception_el(s)); return; } @@ -9250,7 +9251,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) break; default: illegal_op: - gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(), + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), default_exception_el(s)); break; } @@ -10275,7 +10276,7 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) } /* All other insns: NOCP */ - gen_exception_insn(s, 4, EXCP_NOCP, syn_uncategorized(), + gen_exception_insn(s, s->pc_curr, EXCP_NOCP, syn_uncategorized(), default_exception_el(s)); break; } @@ -10939,7 +10940,7 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) } return; illegal_op: - gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(), + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), default_exception_el(s)); } @@ -11763,7 +11764,7 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn) return; illegal_op: undef: - gen_exception_insn(s, 2, EXCP_UDEF, syn_uncategorized(), + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), default_exception_el(s)); } From aee828e7541a5895669ade3a4b6978382b6b094a Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 15 Aug 2019 09:46:44 +0100 Subject: [PATCH 13/29] target/arm: Replace offset with pc in gen_exception_internal_insn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The offset is variable depending on the instruction set. Passing in the actual value is clearer in intent. Signed-off-by: Richard Henderson Reviewed-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20190807045335.1361-9-richard.henderson@linaro.org Signed-off-by: Peter Maydell --- target/arm/translate-a64.c | 8 ++++---- target/arm/translate.c | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index 70caf3becb..2f8eea02e3 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -253,9 +253,9 @@ static void gen_exception_internal(int excp) tcg_temp_free_i32(tcg_excp); } -static void gen_exception_internal_insn(DisasContext *s, int offset, int excp) +static void gen_exception_internal_insn(DisasContext *s, uint64_t pc, int excp) { - gen_a64_set_pc_im(s->base.pc_next - offset); + gen_a64_set_pc_im(pc); gen_exception_internal(excp); s->base.is_jmp = DISAS_NORETURN; } @@ -1924,7 +1924,7 @@ static void disas_exc(DisasContext *s, uint32_t insn) break; } #endif - gen_exception_internal_insn(s, 0, EXCP_SEMIHOST); + gen_exception_internal_insn(s, s->base.pc_next, EXCP_SEMIHOST); } else { unsupported_encoding(s, insn); } @@ -14220,7 +14220,7 @@ static bool aarch64_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cpu, /* End the TB early; it likely won't be executed */ dc->base.is_jmp = DISAS_TOO_MANY; } else { - gen_exception_internal_insn(dc, 0, EXCP_DEBUG); + gen_exception_internal_insn(dc, dc->base.pc_next, EXCP_DEBUG); /* The address covered by the breakpoint must be included in [tb->pc, tb->pc + tb->size) in order to for it to be properly cleared -- thus we diff --git a/target/arm/translate.c b/target/arm/translate.c index 60f1b3998a..c26d3376b3 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -1241,10 +1241,10 @@ static inline void gen_smc(DisasContext *s) s->base.is_jmp = DISAS_SMC; } -static void gen_exception_internal_insn(DisasContext *s, int offset, int excp) +static void gen_exception_internal_insn(DisasContext *s, uint32_t pc, int excp) { gen_set_condexec(s); - gen_set_pc_im(s, s->base.pc_next - offset); + gen_set_pc_im(s, pc); gen_exception_internal(excp); s->base.is_jmp = DISAS_NORETURN; } @@ -1296,7 +1296,7 @@ static inline void gen_hlt(DisasContext *s, int imm) s->current_el != 0 && #endif (imm == (s->thumb ? 0x3c : 0xf000))) { - gen_exception_internal_insn(s, 0, EXCP_SEMIHOST); + gen_exception_internal_insn(s, s->base.pc_next, EXCP_SEMIHOST); return; } @@ -11940,7 +11940,7 @@ static bool arm_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cpu, /* End the TB early; it's likely not going to be executed */ dc->base.is_jmp = DISAS_TOO_MANY; } else { - gen_exception_internal_insn(dc, 0, EXCP_DEBUG); + gen_exception_internal_insn(dc, dc->base.pc_next, EXCP_DEBUG); /* The address covered by the breakpoint must be included in [tb->pc, tb->pc + tb->size) in order to for it to be properly cleared -- thus we From 06bcbda3f64d464b6ecac789bce4bd69f199cd68 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 15 Aug 2019 09:46:44 +0100 Subject: [PATCH 14/29] target/arm: Remove offset argument to gen_exception_bkpt_insn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unlike the other more generic gen_exception{,_internal}_insn interfaces, breakpoints always refer to the current instruction. Signed-off-by: Richard Henderson Reviewed-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20190807045335.1361-10-richard.henderson@linaro.org Signed-off-by: Peter Maydell --- target/arm/translate-a64.c | 7 +++---- target/arm/translate.c | 8 ++++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index 2f8eea02e3..55324333da 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -268,12 +268,11 @@ static void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, s->base.is_jmp = DISAS_NORETURN; } -static void gen_exception_bkpt_insn(DisasContext *s, int offset, - uint32_t syndrome) +static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syndrome) { TCGv_i32 tcg_syn; - gen_a64_set_pc_im(s->base.pc_next - offset); + gen_a64_set_pc_im(s->pc_curr); tcg_syn = tcg_const_i32(syndrome); gen_helper_exception_bkpt_insn(cpu_env, tcg_syn); tcg_temp_free_i32(tcg_syn); @@ -1900,7 +1899,7 @@ static void disas_exc(DisasContext *s, uint32_t insn) break; } /* BRK */ - gen_exception_bkpt_insn(s, 4, syn_aa64_bkpt(imm16)); + gen_exception_bkpt_insn(s, syn_aa64_bkpt(imm16)); break; case 2: if (op2_ll != 0) { diff --git a/target/arm/translate.c b/target/arm/translate.c index c26d3376b3..8bae0c3993 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -1258,12 +1258,12 @@ static void gen_exception_insn(DisasContext *s, uint32_t pc, int excp, s->base.is_jmp = DISAS_NORETURN; } -static void gen_exception_bkpt_insn(DisasContext *s, int offset, uint32_t syn) +static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn) { TCGv_i32 tcg_syn; gen_set_condexec(s); - gen_set_pc_im(s, s->base.pc_next - offset); + gen_set_pc_im(s, s->pc_curr); tcg_syn = tcg_const_i32(syn); gen_helper_exception_bkpt_insn(cpu_env, tcg_syn); tcg_temp_free_i32(tcg_syn); @@ -8140,7 +8140,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) case 1: /* bkpt */ ARCH(5); - gen_exception_bkpt_insn(s, 4, syn_aa32_bkpt(imm16, false)); + gen_exception_bkpt_insn(s, syn_aa32_bkpt(imm16, false)); break; case 2: /* Hypervisor call (v7) */ @@ -11566,7 +11566,7 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn) { int imm8 = extract32(insn, 0, 8); ARCH(5); - gen_exception_bkpt_insn(s, 2, syn_aa32_bkpt(imm8, true)); + gen_exception_bkpt_insn(s, syn_aa32_bkpt(imm8, true)); break; } From 3cb36637157088892e9e33ddb1034bffd1251d3b Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 15 Aug 2019 09:46:45 +0100 Subject: [PATCH 15/29] target/arm: Use unallocated_encoding for aarch32 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Promote this function from aarch64 to fully general use. Use it to unify the code sequences for generating illegal opcode exceptions. Signed-off-by: Richard Henderson Reviewed-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20190807045335.1361-11-richard.henderson@linaro.org Signed-off-by: Peter Maydell --- target/arm/translate-a64.c | 7 ------- target/arm/translate-a64.h | 2 -- target/arm/translate-vfp.inc.c | 3 +-- target/arm/translate.c | 22 ++++++++++++---------- target/arm/translate.h | 2 ++ 5 files changed, 15 insertions(+), 21 deletions(-) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index 55324333da..fc3e5f5c38 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -338,13 +338,6 @@ static inline void gen_goto_tb(DisasContext *s, int n, uint64_t dest) } } -void unallocated_encoding(DisasContext *s) -{ - /* Unallocated and reserved encodings are uncategorized */ - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), - default_exception_el(s)); -} - static void init_tmp_a64_array(DisasContext *s) { #ifdef CONFIG_DEBUG_TCG diff --git a/target/arm/translate-a64.h b/target/arm/translate-a64.h index 9cd2b3d238..12ad8ac6ed 100644 --- a/target/arm/translate-a64.h +++ b/target/arm/translate-a64.h @@ -18,8 +18,6 @@ #ifndef TARGET_ARM_TRANSLATE_A64_H #define TARGET_ARM_TRANSLATE_A64_H -void unallocated_encoding(DisasContext *s); - #define unsupported_encoding(s, insn) \ do { \ qemu_log_mask(LOG_UNIMP, \ diff --git a/target/arm/translate-vfp.inc.c b/target/arm/translate-vfp.inc.c index 5065d4524c..3e8ea80493 100644 --- a/target/arm/translate-vfp.inc.c +++ b/target/arm/translate-vfp.inc.c @@ -108,8 +108,7 @@ static bool full_vfp_access_check(DisasContext *s, bool ignore_vfp_enabled) if (!s->vfp_enabled && !ignore_vfp_enabled) { assert(!arm_dc_feature(s, ARM_FEATURE_M)); - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), - default_exception_el(s)); + unallocated_encoding(s); return false; } diff --git a/target/arm/translate.c b/target/arm/translate.c index 8bae0c3993..cc7d37b787 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -1270,6 +1270,13 @@ static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn) s->base.is_jmp = DISAS_NORETURN; } +void unallocated_encoding(DisasContext *s) +{ + /* Unallocated and reserved encodings are uncategorized */ + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), + default_exception_el(s)); +} + /* Force a TB lookup after an instruction that changes the CPU state. */ static inline void gen_lookup_tb(DisasContext *s) { @@ -1300,8 +1307,7 @@ static inline void gen_hlt(DisasContext *s, int imm) return; } - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), - default_exception_el(s)); + unallocated_encoding(s); } static inline void gen_add_data_offset(DisasContext *s, unsigned int insn, @@ -7623,8 +7629,7 @@ static void gen_srs(DisasContext *s, } if (undef) { - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), - default_exception_el(s)); + unallocated_encoding(s); return; } @@ -9251,8 +9256,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) break; default: illegal_op: - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), - default_exception_el(s)); + unallocated_encoding(s); break; } } @@ -10940,8 +10944,7 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) } return; illegal_op: - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), - default_exception_el(s)); + unallocated_encoding(s); } static void disas_thumb_insn(DisasContext *s, uint32_t insn) @@ -11764,8 +11767,7 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn) return; illegal_op: undef: - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), - default_exception_el(s)); + unallocated_encoding(s); } static bool insn_crosses_page(CPUARMState *env, DisasContext *s) diff --git a/target/arm/translate.h b/target/arm/translate.h index 64304c957e..92ef790be9 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -99,6 +99,8 @@ typedef struct DisasCompare { bool value_global; } DisasCompare; +void unallocated_encoding(DisasContext *s); + /* Share the TCG temporaries common between 32 and 64 bit modes. */ extern TCGv_i32 cpu_NF, cpu_ZF, cpu_CF, cpu_VF; extern TCGv_i64 cpu_exclusive_addr; From 640581a06d14e2d0d3c3ba79b916de6bc43578b0 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 15 Aug 2019 09:46:45 +0100 Subject: [PATCH 16/29] target/arm: Remove helper_double_saturate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace x = double_saturate(y) with x = add_saturate(y, y). There is no need for a separate more specialized helper. Signed-off-by: Richard Henderson Reviewed-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20190807045335.1361-12-richard.henderson@linaro.org Signed-off-by: Peter Maydell --- target/arm/helper.h | 1 - target/arm/op_helper.c | 15 --------------- target/arm/translate.c | 4 ++-- 3 files changed, 2 insertions(+), 18 deletions(-) diff --git a/target/arm/helper.h b/target/arm/helper.h index 132aa1682e..1fb2cb5a77 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -6,7 +6,6 @@ DEF_HELPER_3(add_saturate, i32, env, i32, i32) DEF_HELPER_3(sub_saturate, i32, env, i32, i32) DEF_HELPER_3(add_usaturate, i32, env, i32, i32) DEF_HELPER_3(sub_usaturate, i32, env, i32, i32) -DEF_HELPER_2(double_saturate, i32, env, s32) DEF_HELPER_FLAGS_2(sdiv, TCG_CALL_NO_RWG_SE, s32, s32, s32) DEF_HELPER_FLAGS_2(udiv, TCG_CALL_NO_RWG_SE, i32, i32, i32) DEF_HELPER_FLAGS_1(rbit, TCG_CALL_NO_RWG_SE, i32, i32) diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index 5e1625a1c8..0fd4bd0238 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -135,21 +135,6 @@ uint32_t HELPER(sub_saturate)(CPUARMState *env, uint32_t a, uint32_t b) return res; } -uint32_t HELPER(double_saturate)(CPUARMState *env, int32_t val) -{ - uint32_t res; - if (val >= 0x40000000) { - res = ~SIGNBIT; - env->QF = 1; - } else if (val <= (int32_t)0xc0000000) { - res = SIGNBIT; - env->QF = 1; - } else { - res = val << 1; - } - return res; -} - uint32_t HELPER(add_usaturate)(CPUARMState *env, uint32_t a, uint32_t b) { uint32_t res = a + b; diff --git a/target/arm/translate.c b/target/arm/translate.c index cc7d37b787..34e65cd80c 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -8107,7 +8107,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) tmp = load_reg(s, rm); tmp2 = load_reg(s, rn); if (op1 & 2) - gen_helper_double_saturate(tmp2, cpu_env, tmp2); + gen_helper_add_saturate(tmp2, cpu_env, tmp2, tmp2); if (op1 & 1) gen_helper_sub_saturate(tmp, cpu_env, tmp, tmp2); else @@ -9950,7 +9950,7 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) tmp = load_reg(s, rn); tmp2 = load_reg(s, rm); if (op & 1) - gen_helper_double_saturate(tmp, cpu_env, tmp); + gen_helper_add_saturate(tmp, cpu_env, tmp, tmp); if (op & 2) gen_helper_sub_saturate(tmp, cpu_env, tmp2, tmp); else From b9e758f0b5bc64b5800432c0a436dd1afc98ba33 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Fri, 2 Aug 2019 14:25:26 +0200 Subject: [PATCH 17/29] target/arm/cpu64: Ensure kvm really supports aarch64=off If -cpu ,aarch64=off is used then KVM must also be used, and it and the host must support running the vcpu in 32-bit mode. Also, if -cpu ,aarch64=on is used, then it doesn't matter if kvm is enabled or not. Signed-off-by: Andrew Jones Reviewed-by: Eric Auger Signed-off-by: Peter Maydell --- target/arm/cpu64.c | 12 ++++++------ target/arm/kvm64.c | 9 +++++++++ target/arm/kvm_arm.h | 14 ++++++++++++++ 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index ee55237a9b..ab63115c77 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -426,13 +426,13 @@ static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error **errp) * restriction allows us to avoid fixing up functionality that assumes a * uniform execution state like do_interrupt. */ - if (!kvm_enabled()) { - error_setg(errp, "'aarch64' feature cannot be disabled " - "unless KVM is enabled"); - return; - } - if (value == false) { + if (!kvm_enabled() || !kvm_arm_aarch32_supported(CPU(cpu))) { + error_setg(errp, "'aarch64' feature cannot be disabled " + "unless KVM is enabled and 32-bit EL1 " + "is supported"); + return; + } unset_feature(&cpu->env, ARM_FEATURE_AARCH64); } else { set_feature(&cpu->env, ARM_FEATURE_AARCH64); diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index 22d19c9aec..3d91846beb 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -24,7 +24,9 @@ #include "exec/gdbstub.h" #include "sysemu/sysemu.h" #include "sysemu/kvm.h" +#include "sysemu/kvm_int.h" #include "kvm_arm.h" +#include "hw/boards.h" #include "internals.h" static bool have_guest_debug; @@ -593,6 +595,13 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) return true; } +bool kvm_arm_aarch32_supported(CPUState *cpu) +{ + KVMState *s = KVM_STATE(current_machine->accelerator); + + return kvm_check_extension(s, KVM_CAP_ARM_EL1_32BIT); +} + #define ARM_CPU_ID_MPIDR 3, 0, 0, 0, 5 int kvm_arch_init_vcpu(CPUState *cs) diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h index 2a07333c61..98af1050a7 100644 --- a/target/arm/kvm_arm.h +++ b/target/arm/kvm_arm.h @@ -207,6 +207,15 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf); */ void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu); +/** + * kvm_arm_aarch32_supported: + * @cs: CPUState + * + * Returns: true if the KVM VCPU can enable AArch32 mode + * and false otherwise. + */ +bool kvm_arm_aarch32_supported(CPUState *cs); + /** * kvm_arm_get_max_vm_ipa_size - Returns the number of bits in the * IPA address space supported by KVM @@ -247,6 +256,11 @@ static inline void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu) cpu->host_cpu_probe_failed = true; } +static inline bool kvm_arm_aarch32_supported(CPUState *cs) +{ + return false; +} + static inline int kvm_arm_get_max_vm_ipa_size(MachineState *ms) { return -ENOENT; From ae502508f83e06abddab3ac1a11dd822718472e1 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Fri, 2 Aug 2019 14:25:27 +0200 Subject: [PATCH 18/29] target/arm/cpu: Ensure we can use the pmu with kvm We first convert the pmu property from a static property to one with its own accessors. Then we use the set accessor to check if the PMU is supported when using KVM. Indeed a 32-bit KVM host does not support the PMU, so this check will catch an attempt to use it at property-set time. Signed-off-by: Andrew Jones Reviewed-by: Eric Auger Signed-off-by: Peter Maydell --- target/arm/cpu.c | 30 +++++++++++++++++++++++++----- target/arm/kvm.c | 7 +++++++ target/arm/kvm_arm.h | 14 ++++++++++++++ 3 files changed, 46 insertions(+), 5 deletions(-) diff --git a/target/arm/cpu.c b/target/arm/cpu.c index ec2ab95dbe..2399c14471 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -994,10 +994,6 @@ static Property arm_cpu_has_el3_property = static Property arm_cpu_cfgend_property = DEFINE_PROP_BOOL("cfgend", ARMCPU, cfgend, false); -/* use property name "pmu" to match other archs and virt tools */ -static Property arm_cpu_has_pmu_property = - DEFINE_PROP_BOOL("pmu", ARMCPU, has_pmu, true); - static Property arm_cpu_has_vfp_property = DEFINE_PROP_BOOL("vfp", ARMCPU, has_vfp, true); @@ -1020,6 +1016,29 @@ static Property arm_cpu_pmsav7_dregion_property = pmsav7_dregion, qdev_prop_uint32, uint32_t); +static bool arm_get_pmu(Object *obj, Error **errp) +{ + ARMCPU *cpu = ARM_CPU(obj); + + return cpu->has_pmu; +} + +static void arm_set_pmu(Object *obj, bool value, Error **errp) +{ + ARMCPU *cpu = ARM_CPU(obj); + + if (value) { + if (kvm_enabled() && !kvm_arm_pmu_supported(CPU(cpu))) { + error_setg(errp, "'pmu' feature not supported by KVM on this host"); + return; + } + set_feature(&cpu->env, ARM_FEATURE_PMU); + } else { + unset_feature(&cpu->env, ARM_FEATURE_PMU); + } + cpu->has_pmu = value; +} + static void arm_get_init_svtor(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { @@ -1094,7 +1113,8 @@ void arm_cpu_post_init(Object *obj) } if (arm_feature(&cpu->env, ARM_FEATURE_PMU)) { - qdev_property_add_static(DEVICE(obj), &arm_cpu_has_pmu_property, + cpu->has_pmu = true; + object_property_add_bool(obj, "pmu", arm_get_pmu, arm_set_pmu, &error_abort); } diff --git a/target/arm/kvm.c b/target/arm/kvm.c index fe4f461d4e..bfe3d445e1 100644 --- a/target/arm/kvm.c +++ b/target/arm/kvm.c @@ -162,6 +162,13 @@ void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu) env->features = arm_host_cpu_features.features; } +bool kvm_arm_pmu_supported(CPUState *cpu) +{ + KVMState *s = KVM_STATE(current_machine->accelerator); + + return kvm_check_extension(s, KVM_CAP_ARM_PMU_V3); +} + int kvm_arm_get_max_vm_ipa_size(MachineState *ms) { KVMState *s = KVM_STATE(ms->accelerator); diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h index 98af1050a7..b3106c8600 100644 --- a/target/arm/kvm_arm.h +++ b/target/arm/kvm_arm.h @@ -216,6 +216,15 @@ void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu); */ bool kvm_arm_aarch32_supported(CPUState *cs); +/** + * bool kvm_arm_pmu_supported: + * @cs: CPUState + * + * Returns: true if the KVM VCPU can enable its PMU + * and false otherwise. + */ +bool kvm_arm_pmu_supported(CPUState *cs); + /** * kvm_arm_get_max_vm_ipa_size - Returns the number of bits in the * IPA address space supported by KVM @@ -261,6 +270,11 @@ static inline bool kvm_arm_aarch32_supported(CPUState *cs) return false; } +static inline bool kvm_arm_pmu_supported(CPUState *cs) +{ + return false; +} + static inline int kvm_arm_get_max_vm_ipa_size(MachineState *ms) { return -ENOENT; From 7b351d98709d3f77d6bb18562e1bf228862b0d57 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Fri, 2 Aug 2019 14:25:30 +0200 Subject: [PATCH 19/29] target/arm/helper: zcr: Add build bug next to value range assumption The current implementation of ZCR_ELx matches the architecture, only implementing the lower four bits, with the rest RAZ/WI. This puts a strict limit on ARM_MAX_VQ of 16. Make sure we don't let ARM_MAX_VQ grow without a corresponding update here. Suggested-by: Dave Martin Signed-off-by: Andrew Jones Reviewed-by: Richard Henderson Reviewed-by: Eric Auger Signed-off-by: Peter Maydell --- target/arm/helper.c | 1 + 1 file changed, 1 insertion(+) diff --git a/target/arm/helper.c b/target/arm/helper.c index 24806c16ca..2fd504ea7a 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -5300,6 +5300,7 @@ static void zcr_write(CPUARMState *env, const ARMCPRegInfo *ri, int new_len; /* Bits other than [3:0] are RAZ/WI. */ + QEMU_BUILD_BUG_ON(ARM_MAX_VQ > 16); raw_write(env, ri, value & 0xf); /* From 46417784d21c89446763f2047228977bdc267895 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Fri, 2 Aug 2019 14:25:31 +0200 Subject: [PATCH 20/29] target/arm/cpu: Use div-round-up to determine predicate register array size Unless we're guaranteed to always increase ARM_MAX_VQ by a multiple of four, then we should use DIV_ROUND_UP to ensure we get an appropriate array size. Signed-off-by: Andrew Jones Reviewed-by: Richard Henderson Signed-off-by: Peter Maydell --- target/arm/cpu.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index d12c746085..ab5d58a9d4 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -195,7 +195,7 @@ typedef struct ARMVectorReg { #ifdef TARGET_AARCH64 /* In AArch32 mode, predicate registers do not exist at all. */ typedef struct ARMPredicateReg { - uint64_t p[2 * ARM_MAX_VQ / 8] QEMU_ALIGNED(16); + uint64_t p[DIV_ROUND_UP(2 * ARM_MAX_VQ, 8)] QEMU_ALIGNED(16); } ARMPredicateReg; /* In AArch32 mode, PAC keys do not exist at all. */ From 4ed9d9f894a7ab19f5b927fe0d833effeabac0ce Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Fri, 2 Aug 2019 14:25:34 +0200 Subject: [PATCH 21/29] target/arm/kvm64: Fix error returns A couple return -EINVAL's forgot their '-'s. Signed-off-by: Andrew Jones Reviewed-by: Eric Auger Reviewed-by: Richard Henderson Signed-off-by: Peter Maydell --- target/arm/kvm64.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index 3d91846beb..ddde6268b9 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -854,7 +854,7 @@ int kvm_arch_put_registers(CPUState *cs, int level) write_cpustate_to_list(cpu, true); if (!write_list_to_kvmstate(cpu, level)) { - return EINVAL; + return -EINVAL; } kvm_arm_sync_mpstate_to_kvm(cpu); @@ -995,7 +995,7 @@ int kvm_arch_get_registers(CPUState *cs) } if (!write_kvmstate_to_list(cpu)) { - return EINVAL; + return -EINVAL; } /* Note that it's OK to have registers which aren't in CPUState, * so we can ignore a failure return here. From 30e3537fa5948e6346d23422b8397dee086a434e Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Fri, 2 Aug 2019 14:25:35 +0200 Subject: [PATCH 22/29] target/arm/kvm64: Move the get/put of fpsimd registers out Move the getting/putting of the fpsimd registers out of kvm_arch_get/put_registers() into their own helper functions to prepare for alternatively getting/putting SVE registers. No functional change. Signed-off-by: Andrew Jones Reviewed-by: Eric Auger Reviewed-by: Richard Henderson Signed-off-by: Peter Maydell --- target/arm/kvm64.c | 148 +++++++++++++++++++++++++++------------------ 1 file changed, 88 insertions(+), 60 deletions(-) diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index ddde6268b9..0b004d5d30 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -719,13 +719,53 @@ int kvm_arm_cpreg_level(uint64_t regidx) #define AARCH64_SIMD_CTRL_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U32 | \ KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x)) +static int kvm_arch_put_fpsimd(CPUState *cs) +{ + ARMCPU *cpu = ARM_CPU(cs); + CPUARMState *env = &cpu->env; + struct kvm_one_reg reg; + uint32_t fpr; + int i, ret; + + for (i = 0; i < 32; i++) { + uint64_t *q = aa64_vfp_qreg(env, i); +#ifdef HOST_WORDS_BIGENDIAN + uint64_t fp_val[2] = { q[1], q[0] }; + reg.addr = (uintptr_t)fp_val; +#else + reg.addr = (uintptr_t)q; +#endif + reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]); + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); + if (ret) { + return ret; + } + } + + reg.addr = (uintptr_t)(&fpr); + fpr = vfp_get_fpsr(env); + reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpsr); + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); + if (ret) { + return ret; + } + + reg.addr = (uintptr_t)(&fpr); + fpr = vfp_get_fpcr(env); + reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpcr); + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); + if (ret) { + return ret; + } + + return 0; +} + int kvm_arch_put_registers(CPUState *cs, int level) { struct kvm_one_reg reg; - uint32_t fpr; uint64_t val; - int i; - int ret; + int i, ret; unsigned int el; ARMCPU *cpu = ARM_CPU(cs); @@ -815,33 +855,7 @@ int kvm_arch_put_registers(CPUState *cs, int level) } } - /* Advanced SIMD and FP registers. */ - for (i = 0; i < 32; i++) { - uint64_t *q = aa64_vfp_qreg(env, i); -#ifdef HOST_WORDS_BIGENDIAN - uint64_t fp_val[2] = { q[1], q[0] }; - reg.addr = (uintptr_t)fp_val; -#else - reg.addr = (uintptr_t)q; -#endif - reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]); - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); - if (ret) { - return ret; - } - } - - reg.addr = (uintptr_t)(&fpr); - fpr = vfp_get_fpsr(env); - reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpsr); - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); - if (ret) { - return ret; - } - - fpr = vfp_get_fpcr(env); - reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpcr); - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); + ret = kvm_arch_put_fpsimd(cs); if (ret) { return ret; } @@ -862,14 +876,54 @@ int kvm_arch_put_registers(CPUState *cs, int level) return ret; } +static int kvm_arch_get_fpsimd(CPUState *cs) +{ + ARMCPU *cpu = ARM_CPU(cs); + CPUARMState *env = &cpu->env; + struct kvm_one_reg reg; + uint32_t fpr; + int i, ret; + + for (i = 0; i < 32; i++) { + uint64_t *q = aa64_vfp_qreg(env, i); + reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]); + reg.addr = (uintptr_t)q; + ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®); + if (ret) { + return ret; + } else { +#ifdef HOST_WORDS_BIGENDIAN + uint64_t t; + t = q[0], q[0] = q[1], q[1] = t; +#endif + } + } + + reg.addr = (uintptr_t)(&fpr); + reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpsr); + ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®); + if (ret) { + return ret; + } + vfp_set_fpsr(env, fpr); + + reg.addr = (uintptr_t)(&fpr); + reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpcr); + ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®); + if (ret) { + return ret; + } + vfp_set_fpcr(env, fpr); + + return 0; +} + int kvm_arch_get_registers(CPUState *cs) { struct kvm_one_reg reg; uint64_t val; - uint32_t fpr; unsigned int el; - int i; - int ret; + int i, ret; ARMCPU *cpu = ARM_CPU(cs); CPUARMState *env = &cpu->env; @@ -958,36 +1012,10 @@ int kvm_arch_get_registers(CPUState *cs) env->spsr = env->banked_spsr[i]; } - /* Advanced SIMD and FP registers */ - for (i = 0; i < 32; i++) { - uint64_t *q = aa64_vfp_qreg(env, i); - reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]); - reg.addr = (uintptr_t)q; - ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®); - if (ret) { - return ret; - } else { -#ifdef HOST_WORDS_BIGENDIAN - uint64_t t; - t = q[0], q[0] = q[1], q[1] = t; -#endif - } - } - - reg.addr = (uintptr_t)(&fpr); - reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpsr); - ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®); + ret = kvm_arch_get_fpsimd(cs); if (ret) { return ret; } - vfp_set_fpsr(env, fpr); - - reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpcr); - ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®); - if (ret) { - return ret; - } - vfp_set_fpcr(env, fpr); ret = kvm_get_vcpu_events(cpu); if (ret) { From 191f4bfe8d6cf0c7d5cd7f84cd7076e32e3745dd Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 8 Aug 2019 13:26:10 -0700 Subject: [PATCH 23/29] target/arm: Use tcg_gen_extract_i32 for shifter_out_im Extract is a compact combination of shift + and. Signed-off-by: Richard Henderson Message-id: 20190808202616.13782-2-richard.henderson@linaro.org Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- target/arm/translate.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index 34e65cd80c..14d6b6d4d2 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -605,14 +605,7 @@ static void gen_sar(TCGv_i32 dest, TCGv_i32 t0, TCGv_i32 t1) static void shifter_out_im(TCGv_i32 var, int shift) { - if (shift == 0) { - tcg_gen_andi_i32(cpu_CF, var, 1); - } else { - tcg_gen_shri_i32(cpu_CF, var, shift); - if (shift != 31) { - tcg_gen_andi_i32(cpu_CF, cpu_CF, 1); - } - } + tcg_gen_extract_i32(cpu_CF, var, shift, 1); } /* Shift by immediate. Includes special handling for shift == 0. */ From d1f8755fc93911f5b27246b1da794542d222fa1b Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 8 Aug 2019 13:26:11 -0700 Subject: [PATCH 24/29] target/arm: Use tcg_gen_deposit_i32 for PKHBT, PKHTB Use deposit as the composit operation to merge the bits from the two inputs. Signed-off-by: Richard Henderson Message-id: 20190808202616.13782-3-richard.henderson@linaro.org Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- target/arm/translate.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index 14d6b6d4d2..9c3323509e 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -8754,19 +8754,16 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) shift = (insn >> 7) & 0x1f; if (insn & (1 << 6)) { /* pkhtb */ - if (shift == 0) + if (shift == 0) { shift = 31; + } tcg_gen_sari_i32(tmp2, tmp2, shift); - tcg_gen_andi_i32(tmp, tmp, 0xffff0000); - tcg_gen_ext16u_i32(tmp2, tmp2); + tcg_gen_deposit_i32(tmp, tmp, tmp2, 0, 16); } else { /* pkhbt */ - if (shift) - tcg_gen_shli_i32(tmp2, tmp2, shift); - tcg_gen_ext16u_i32(tmp, tmp); - tcg_gen_andi_i32(tmp2, tmp2, 0xffff0000); + tcg_gen_shli_i32(tmp2, tmp2, shift); + tcg_gen_deposit_i32(tmp, tmp2, tmp, 0, 16); } - tcg_gen_or_i32(tmp, tmp, tmp2); tcg_temp_free_i32(tmp2); store_reg(s, rd, tmp); } else if ((insn & 0x00200020) == 0x00200000) { @@ -9802,19 +9799,16 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) shift = ((insn >> 10) & 0x1c) | ((insn >> 6) & 0x3); if (insn & (1 << 5)) { /* pkhtb */ - if (shift == 0) + if (shift == 0) { shift = 31; + } tcg_gen_sari_i32(tmp2, tmp2, shift); - tcg_gen_andi_i32(tmp, tmp, 0xffff0000); - tcg_gen_ext16u_i32(tmp2, tmp2); + tcg_gen_deposit_i32(tmp, tmp, tmp2, 0, 16); } else { /* pkhbt */ - if (shift) - tcg_gen_shli_i32(tmp2, tmp2, shift); - tcg_gen_ext16u_i32(tmp, tmp); - tcg_gen_andi_i32(tmp2, tmp2, 0xffff0000); + tcg_gen_shli_i32(tmp2, tmp2, shift); + tcg_gen_deposit_i32(tmp, tmp2, tmp, 0, 16); } - tcg_gen_or_i32(tmp, tmp, tmp2); tcg_temp_free_i32(tmp2); store_reg(s, rd, tmp); } else { From 464eaa9571fae5867d9aea7d7209c091c8a50223 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 8 Aug 2019 13:26:12 -0700 Subject: [PATCH 25/29] target/arm: Remove redundant shift tests The immediate shift generator functions already test for, and eliminate, the case of a shift by zero. Signed-off-by: Richard Henderson Message-id: 20190808202616.13782-4-richard.henderson@linaro.org Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- target/arm/translate.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index 9c3323509e..ebc7c67f02 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -8811,8 +8811,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) shift = (insn >> 10) & 3; /* ??? In many cases it's not necessary to do a rotate, a shift is sufficient. */ - if (shift != 0) - tcg_gen_rotri_i32(tmp, tmp, shift * 8); + tcg_gen_rotri_i32(tmp, tmp, shift * 8); op1 = (insn >> 20) & 7; switch (op1) { case 0: gen_sxtb16(tmp); break; @@ -9889,8 +9888,7 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) shift = (insn >> 4) & 3; /* ??? In many cases it's not necessary to do a rotate, a shift is sufficient. */ - if (shift != 0) - tcg_gen_rotri_i32(tmp, tmp, shift * 8); + tcg_gen_rotri_i32(tmp, tmp, shift * 8); op = (insn >> 20) & 7; switch (op) { case 0: gen_sxth(tmp); break; @@ -10617,11 +10615,10 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) case 7: goto illegal_op; default: /* Saturate. */ - if (shift) { - if (op & 1) - tcg_gen_sari_i32(tmp, tmp, shift); - else - tcg_gen_shli_i32(tmp, tmp, shift); + if (op & 1) { + tcg_gen_sari_i32(tmp, tmp, shift); + } else { + tcg_gen_shli_i32(tmp, tmp, shift); } tmp2 = tcg_const_i32(imm); if (op & 4) { @@ -10812,9 +10809,7 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) goto illegal_op; } tmp = load_reg(s, rm); - if (shift) { - tcg_gen_shli_i32(tmp, tmp, shift); - } + tcg_gen_shli_i32(tmp, tmp, shift); tcg_gen_add_i32(addr, addr, tmp); tcg_temp_free_i32(tmp); break; From dd861b3f29be97a9e3cdb9769dcbc0c7d7825185 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 8 Aug 2019 13:26:13 -0700 Subject: [PATCH 26/29] target/arm: Use ror32 instead of open-coding the operation The helper function is more documentary, and also already handles the case of rotate by zero. Signed-off-by: Richard Henderson Message-id: 20190808202616.13782-5-richard.henderson@linaro.org Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- target/arm/translate.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index ebc7c67f02..02ce8d44fa 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -7964,8 +7964,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) /* CPSR = immediate */ val = insn & 0xff; shift = ((insn >> 8) & 0xf) * 2; - if (shift) - val = (val >> shift) | (val << (32 - shift)); + val = ror32(val, shift); i = ((insn & (1 << 22)) != 0); if (gen_set_psr_im(s, msr_mask(s, (insn >> 16) & 0xf, i), i, val)) { @@ -8228,9 +8227,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) /* immediate operand */ val = insn & 0xff; shift = ((insn >> 8) & 0xf) * 2; - if (shift) { - val = (val >> shift) | (val << (32 - shift)); - } + val = ror32(val, shift); tmp2 = tcg_temp_new_i32(); tcg_gen_movi_i32(tmp2, val); if (logic_cc && shift) { From adefba76e8bf10dfb342094d2f5debfeedb1a74d Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 8 Aug 2019 13:26:14 -0700 Subject: [PATCH 27/29] target/arm: Use tcg_gen_rotri_i32 for gen_swap_half Rotate is the more compact and obvious way to swap 16-bit elements of a 32-bit word. Signed-off-by: Richard Henderson Message-id: 20190808202616.13782-6-richard.henderson@linaro.org Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- target/arm/translate.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index 02ce8d44fa..2e16064620 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -444,11 +444,7 @@ static TCGv_i64 gen_muls_i64_i32(TCGv_i32 a, TCGv_i32 b) /* Swap low and high halfwords. */ static void gen_swap_half(TCGv_i32 var) { - TCGv_i32 tmp = tcg_temp_new_i32(); - tcg_gen_shri_i32(tmp, var, 16); - tcg_gen_shli_i32(var, var, 16); - tcg_gen_or_i32(var, var, tmp); - tcg_temp_free_i32(tmp); + tcg_gen_rotri_i32(var, var, 16); } /* Dual 16-bit add. Result placed in t0 and t1 is marked as dead. From 5f8cd06ebcf57420be8fea4574de2e074de46709 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 8 Aug 2019 13:26:15 -0700 Subject: [PATCH 28/29] target/arm: Simplify SMMLA, SMMLAR, SMMLS, SMMLSR All of the inputs to these instructions are 32-bits. Rather than extend each input to 64-bits and then extract the high 32-bits of the output, use tcg_gen_muls2_i32 and other 32-bit generator functions. Signed-off-by: Richard Henderson Message-id: 20190808202616.13782-7-richard.henderson@linaro.org Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- target/arm/translate.c | 72 +++++++++++++++--------------------------- 1 file changed, 26 insertions(+), 46 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index 2e16064620..9e2853fe76 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -376,34 +376,6 @@ static void gen_revsh(TCGv_i32 var) tcg_gen_ext16s_i32(var, var); } -/* Return (b << 32) + a. Mark inputs as dead */ -static TCGv_i64 gen_addq_msw(TCGv_i64 a, TCGv_i32 b) -{ - TCGv_i64 tmp64 = tcg_temp_new_i64(); - - tcg_gen_extu_i32_i64(tmp64, b); - tcg_temp_free_i32(b); - tcg_gen_shli_i64(tmp64, tmp64, 32); - tcg_gen_add_i64(a, tmp64, a); - - tcg_temp_free_i64(tmp64); - return a; -} - -/* Return (b << 32) - a. Mark inputs as dead. */ -static TCGv_i64 gen_subq_msw(TCGv_i64 a, TCGv_i32 b) -{ - TCGv_i64 tmp64 = tcg_temp_new_i64(); - - tcg_gen_extu_i32_i64(tmp64, b); - tcg_temp_free_i32(b); - tcg_gen_shli_i64(tmp64, tmp64, 32); - tcg_gen_sub_i64(a, tmp64, a); - - tcg_temp_free_i64(tmp64); - return a; -} - /* 32x32->64 multiply. Marks inputs as dead. */ static TCGv_i64 gen_mulu_i64_i32(TCGv_i32 a, TCGv_i32 b) { @@ -8857,23 +8829,27 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) (SMMUL, SMMLA, SMMLS) */ tmp = load_reg(s, rm); tmp2 = load_reg(s, rs); - tmp64 = gen_muls_i64_i32(tmp, tmp2); + tcg_gen_muls2_i32(tmp2, tmp, tmp, tmp2); if (rd != 15) { - tmp = load_reg(s, rd); + tmp3 = load_reg(s, rd); if (insn & (1 << 6)) { - tmp64 = gen_subq_msw(tmp64, tmp); + tcg_gen_sub_i32(tmp, tmp, tmp3); } else { - tmp64 = gen_addq_msw(tmp64, tmp); + tcg_gen_add_i32(tmp, tmp, tmp3); } + tcg_temp_free_i32(tmp3); } if (insn & (1 << 5)) { - tcg_gen_addi_i64(tmp64, tmp64, 0x80000000u); + /* + * Adding 0x80000000 to the 64-bit quantity + * means that we have carry in to the high + * word when the low word has the high bit set. + */ + tcg_gen_shri_i32(tmp2, tmp2, 31); + tcg_gen_add_i32(tmp, tmp, tmp2); } - tcg_gen_shri_i64(tmp64, tmp64, 32); - tmp = tcg_temp_new_i32(); - tcg_gen_extrl_i64_i32(tmp, tmp64); - tcg_temp_free_i64(tmp64); + tcg_temp_free_i32(tmp2); store_reg(s, rn, tmp); break; case 0: @@ -10099,22 +10075,26 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) } break; case 5: case 6: /* 32 * 32 -> 32msb (SMMUL, SMMLA, SMMLS) */ - tmp64 = gen_muls_i64_i32(tmp, tmp2); + tcg_gen_muls2_i32(tmp2, tmp, tmp, tmp2); if (rs != 15) { - tmp = load_reg(s, rs); + tmp3 = load_reg(s, rs); if (insn & (1 << 20)) { - tmp64 = gen_addq_msw(tmp64, tmp); + tcg_gen_add_i32(tmp, tmp, tmp3); } else { - tmp64 = gen_subq_msw(tmp64, tmp); + tcg_gen_sub_i32(tmp, tmp, tmp3); } + tcg_temp_free_i32(tmp3); } if (insn & (1 << 4)) { - tcg_gen_addi_i64(tmp64, tmp64, 0x80000000u); + /* + * Adding 0x80000000 to the 64-bit quantity + * means that we have carry in to the high + * word when the low word has the high bit set. + */ + tcg_gen_shri_i32(tmp2, tmp2, 31); + tcg_gen_add_i32(tmp, tmp, tmp2); } - tcg_gen_shri_i64(tmp64, tmp64, 32); - tmp = tcg_temp_new_i32(); - tcg_gen_extrl_i64_i32(tmp, tmp64); - tcg_temp_free_i64(tmp64); + tcg_temp_free_i32(tmp2); break; case 7: /* Unsigned sum of absolute differences. */ gen_helper_usad8(tmp, tmp, tmp2); From 664b7e3b97d6376f3329986c465b3782458b0f8b Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 8 Aug 2019 13:26:16 -0700 Subject: [PATCH 29/29] target/arm: Use tcg_gen_extrh_i64_i32 to extract the high word Separate shift + extract low will result in one extra insn for hosts like RISC-V, MIPS, and Sparc. Signed-off-by: Richard Henderson Message-id: 20190808202616.13782-8-richard.henderson@linaro.org Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- target/arm/translate.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index 9e2853fe76..d948757131 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -1746,8 +1746,7 @@ static int disas_iwmmxt_insn(DisasContext *s, uint32_t insn) if (insn & ARM_CP_RW_BIT) { /* TMRRC */ iwmmxt_load_reg(cpu_V0, wrd); tcg_gen_extrl_i64_i32(cpu_R[rdlo], cpu_V0); - tcg_gen_shri_i64(cpu_V0, cpu_V0, 32); - tcg_gen_extrl_i64_i32(cpu_R[rdhi], cpu_V0); + tcg_gen_extrh_i64_i32(cpu_R[rdhi], cpu_V0); } else { /* TMCRR */ tcg_gen_concat_i32_i64(cpu_V0, cpu_R[rdlo], cpu_R[rdhi]); iwmmxt_store_reg(cpu_V0, wrd); @@ -2792,8 +2791,7 @@ static int disas_dsp_insn(DisasContext *s, uint32_t insn) if (insn & ARM_CP_RW_BIT) { /* MRA */ iwmmxt_load_reg(cpu_V0, acc); tcg_gen_extrl_i64_i32(cpu_R[rdlo], cpu_V0); - tcg_gen_shri_i64(cpu_V0, cpu_V0, 32); - tcg_gen_extrl_i64_i32(cpu_R[rdhi], cpu_V0); + tcg_gen_extrh_i64_i32(cpu_R[rdhi], cpu_V0); tcg_gen_andi_i32(cpu_R[rdhi], cpu_R[rdhi], (1 << (40 - 32)) - 1); } else { /* MAR */ tcg_gen_concat_i32_i64(cpu_V0, cpu_R[rdlo], cpu_R[rdhi]); @@ -5990,8 +5988,7 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn) gen_helper_neon_narrow_high_u16(tmp, cpu_V0); break; case 2: - tcg_gen_shri_i64(cpu_V0, cpu_V0, 32); - tcg_gen_extrl_i64_i32(tmp, cpu_V0); + tcg_gen_extrh_i64_i32(tmp, cpu_V0); break; default: abort(); } @@ -6005,8 +6002,7 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn) break; case 2: tcg_gen_addi_i64(cpu_V0, cpu_V0, 1u << 31); - tcg_gen_shri_i64(cpu_V0, cpu_V0, 32); - tcg_gen_extrl_i64_i32(tmp, cpu_V0); + tcg_gen_extrh_i64_i32(tmp, cpu_V0); break; default: abort(); } @@ -7239,9 +7235,8 @@ static int disas_coproc_insn(DisasContext *s, uint32_t insn) tmp = tcg_temp_new_i32(); tcg_gen_extrl_i64_i32(tmp, tmp64); store_reg(s, rt, tmp); - tcg_gen_shri_i64(tmp64, tmp64, 32); tmp = tcg_temp_new_i32(); - tcg_gen_extrl_i64_i32(tmp, tmp64); + tcg_gen_extrh_i64_i32(tmp, tmp64); tcg_temp_free_i64(tmp64); store_reg(s, rt2, tmp); } else { @@ -7350,8 +7345,7 @@ static void gen_storeq_reg(DisasContext *s, int rlow, int rhigh, TCGv_i64 val) tcg_gen_extrl_i64_i32(tmp, val); store_reg(s, rlow, tmp); tmp = tcg_temp_new_i32(); - tcg_gen_shri_i64(val, val, 32); - tcg_gen_extrl_i64_i32(tmp, val); + tcg_gen_extrh_i64_i32(tmp, val); store_reg(s, rhigh, tmp); }