ARM queue for 2.10: all M profile bugfixes...

thanks

-- PMM

The following changes since commit 25dd0e77898c3e10796d4cbeb35e8af5ba6ce975:

Merge remote-tracking branch 'remotes/mjt/tags/trivial-patches-fetch' into staging (2017-07-31 11:27:43 +0100)

are available in the git repository at:

git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20170731

for you to fetch changes up to 89cbc3778a3d61761e2231e740269218c9a8a41d:

hw/mps2_scc: fix incorrect properties (2017-07-31 13:11:56 +0100)

* fix broken properties on MPS2 SCC device

* fix MPU trace handling of write vs exec

* fix MPU M profile bugs:

- not handling system space or PPB region correctly

- not resetting state

- not migrating MPU_RNR

Peter Maydell (6):

target/arm: Correct MPU trace handling of write vs execute

target/arm: Don't do MPU lookups for addresses in M profile PPB region

target/arm: Don't allow guest to make System space executable for M profile

target/arm: Rename cp15.c6_rgnr to pmsav7.rnr

target/arm: Move PMSAv7 reset into arm_cpu_reset() so M profile MPUs get reset

target/arm: Migrate MPU_RNR register state for M profile cores

Philippe Mathieu-Daudé (1):

hw/mps2_scc: fix incorrect properties

target/arm/cpu.h | 3 +--

hw/intc/armv7m_nvic.c | 14 +++++-----

hw/misc/mps2-scc.c | 4 +--

target/arm/cpu.c | 14 ++++++++++

target/arm/helper.c | 71 ++++++++++++++++++++++++++++++++++-----------------

target/arm/machine.c | 30 +++++++++++++++++++++-

6 files changed, 101 insertions(+), 35 deletions(-)

Almost all of the PMSAv7 state is in the pmsav7 substruct of

the ARM CPU state structure. The exception is the region

number register, which is in cp15.c6_rgnr. This exception

is a bit odd for M profile, which otherwise generally does

not store state in the cp15 substruct.

Rename cp15.c6_rgnr to pmsav7.rnr accordingly.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Message-id: 1501153150-19984-4-git-send-email-peter.maydell@linaro.org

target/arm/cpu.h | 3 +--

hw/intc/armv7m_nvic.c | 14 +++++++-------

target/arm/helper.c | 6 +++---

target/arm/machine.c | 2 +-

4 files changed, 12 insertions(+), 13 deletions(-)

@@ -XXX,XX +XXX,XX @@ typedef struct CPUARMState {

uint64_t par_el[4];

};

- uint32_t c6_rgnr;

-

uint32_t c9_insn; /* Cache lockdown registers. */

uint32_t c9_data;

uint64_t c9_pmcr; /* performance monitor control register */

@@ -XXX,XX +XXX,XX @@ typedef struct CPUARMState {

uint32_t *drbar;

uint32_t *drsr;

uint32_t *dracr;

+ uint32_t rnr;

} pmsav7;

void *nvic;

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c

--- a/hw/intc/armv7m_nvic.c

+++ b/hw/intc/armv7m_nvic.c

@@ -XXX,XX +XXX,XX @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset)

case 0xd94: /* MPU_CTRL */

return cpu->env.v7m.mpu_ctrl;

case 0xd98: /* MPU_RNR */

- return cpu->env.cp15.c6_rgnr;

+ return cpu->env.pmsav7.rnr;

case 0xd9c: /* MPU_RBAR */

case 0xda4: /* MPU_RBAR_A1 */

case 0xdac: /* MPU_RBAR_A2 */

case 0xdb4: /* MPU_RBAR_A3 */

{

- int region = cpu->env.cp15.c6_rgnr;

+ int region = cpu->env.pmsav7.rnr;

if (region >= cpu->pmsav7_dregion) {

return 0;

@@ -XXX,XX +XXX,XX @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset)

case 0xdb0: /* MPU_RASR_A2 */

case 0xdb8: /* MPU_RASR_A3 */

{

- int region = cpu->env.cp15.c6_rgnr;

+ int region = cpu->env.pmsav7.rnr;

if (region >= cpu->pmsav7_dregion) {

return 0;

@@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value)

PRIu32 "/%" PRIu32 "\n",

value, cpu->pmsav7_dregion);

} else {

- cpu->env.cp15.c6_rgnr = value;

+ cpu->env.pmsav7.rnr = value;

}

break;

case 0xd9c: /* MPU_RBAR */

@@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value)

region, cpu->pmsav7_dregion);

return;

}

- cpu->env.cp15.c6_rgnr = region;

+ cpu->env.pmsav7.rnr = region;

} else {

- region = cpu->env.cp15.c6_rgnr;

+ region = cpu->env.pmsav7.rnr;

}

if (region >= cpu->pmsav7_dregion) {

@@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value)

case 0xdb0: /* MPU_RASR_A2 */

case 0xdb8: /* MPU_RASR_A3 */

{

- int region = cpu->env.cp15.c6_rgnr;

+ int region = cpu->env.pmsav7.rnr;

if (region >= cpu->pmsav7_dregion) {

return;

diff --git a/target/arm/helper.c b/target/arm/helper.c

--- a/target/arm/helper.c

+++ b/target/arm/helper.c

@@ -XXX,XX +XXX,XX @@ static uint64_t pmsav7_read(CPUARMState *env, const ARMCPRegInfo *ri)

return 0;

}

- u32p += env->cp15.c6_rgnr;

+ u32p += env->pmsav7.rnr;

return *u32p;

}

@@ -XXX,XX +XXX,XX @@ static void pmsav7_write(CPUARMState *env, const ARMCPRegInfo *ri,

return;

}

- u32p += env->cp15.c6_rgnr;

+ u32p += env->pmsav7.rnr;

tlb_flush(CPU(cpu)); /* Mappings may have changed - purge! */

*u32p = value;

}

@@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo pmsav7_cp_reginfo[] = {

.readfn = pmsav7_read, .writefn = pmsav7_write, .resetfn = pmsav7_reset },

{ .name = "RGNR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 2, .opc2 = 0,

.access = PL1_RW,

- .fieldoffset = offsetof(CPUARMState, cp15.c6_rgnr),

+ .fieldoffset = offsetof(CPUARMState, pmsav7.rnr),

.writefn = pmsav7_rgnr_write },

REGINFO_SENTINEL

};

diff --git a/target/arm/machine.c b/target/arm/machine.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/machine.c

+++ b/target/arm/machine.c

@@ -XXX,XX +XXX,XX @@ static bool pmsav7_rgnr_vmstate_validate(void *opaque, int version_id)

{

ARMCPU *cpu = opaque;

- return cpu->env.cp15.c6_rgnr < cpu->pmsav7_dregion;

+ return cpu->env.pmsav7.rnr < cpu->pmsav7_dregion;

}

static const VMStateDescription vmstate_pmsav7 = {

2.7.4

The PMSAv7 region number register is migrated for R profile

cores using the cpreg scheme, but M profile doesn't use

cpregs, and so we weren't migrating the MPU_RNR register state

at all. Fix that by adding a migration subsection for the

M profile case.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Message-id: 1501153150-19984-6-git-send-email-peter.maydell@linaro.org

target/arm/machine.c | 28 ++++++++++++++++++++++++++++

1 file changed, 28 insertions(+)

diff --git a/target/arm/machine.c b/target/arm/machine.c

--- a/target/arm/machine.c

+++ b/target/arm/machine.c

@@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_pmsav7 = {

}

};

+static bool pmsav7_rnr_needed(void *opaque)

+{

+ ARMCPU *cpu = opaque;

+ CPUARMState *env = &cpu->env;

+

+ /* For R profile cores pmsav7.rnr is migrated via the cpreg

+ * "RGNR" definition in helper.h. For M profile we have to

+ * migrate it separately.

+ */

+ return arm_feature(env, ARM_FEATURE_M);

+}

+

+static const VMStateDescription vmstate_pmsav7_rnr = {

+ .name = "cpu/pmsav7-rnr",

+ .version_id = 1,

+ .minimum_version_id = 1,

+ .needed = pmsav7_rnr_needed,

+ .fields = (VMStateField[]) {

+ VMSTATE_UINT32(env.pmsav7.rnr, ARMCPU),

+ VMSTATE_END_OF_LIST()

+ }

+};

+

static int get_cpsr(QEMUFile *f, void *opaque, size_t size,

VMStateField *field)

{

@@ -XXX,XX +XXX,XX @@ const VMStateDescription vmstate_arm_cpu = {

&vmstate_iwmmxt,

&vmstate_m,

&vmstate_thumb2ee,

+ /* pmsav7_rnr must come before pmsav7 so that we have the

+ * region number before we test it in the VMSTATE_VALIDATE

+ * in vmstate_pmsav7.

+ */

+ &vmstate_pmsav7_rnr,

&vmstate_pmsav7,

NULL

}

2.7.4

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Message-id: 20170729234930.725-1-f4bug@amsat.org

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

hw/misc/mps2-scc.c | 4 ++--

1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/misc/mps2-scc.c b/hw/misc/mps2-scc.c

--- a/hw/misc/mps2-scc.c

+++ b/hw/misc/mps2-scc.c

@@ -XXX,XX +XXX,XX @@ static Property mps2_scc_properties[] = {

/* Values for various read-only ID registers (which are specific

* to the board model or FPGA image)

- DEFINE_PROP_UINT32("scc-cfg4", MPS2SCC, aid, 0),

+ DEFINE_PROP_UINT32("scc-cfg4", MPS2SCC, cfg4, 0),

DEFINE_PROP_UINT32("scc-aid", MPS2SCC, aid, 0),

- DEFINE_PROP_UINT32("scc-id", MPS2SCC, aid, 0),

+ DEFINE_PROP_UINT32("scc-id", MPS2SCC, id, 0),

/* These are the initial settings for the source clocks on the board.

* In hardware they can be configured via a config file read by the

* motherboard configuration controller to suit the FPGA image.

2.7.4

The M profile PMSAv7 specification says that if the address being looked

up is in the PPB region (0xe0000000 - 0xe00fffff) then we do not use

the MPU regions but always use the default memory map. Implement this

(we were previously behaving like an R profile PMSAv7, which does not

special case this).

target/arm/helper.c | 17 ++++++++++++++++-

1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c

--- a/target/arm/helper.c

+++ b/target/arm/helper.c

@@ -XXX,XX +XXX,XX @@ static bool pmsav7_use_background_region(ARMCPU *cpu,

+static inline bool m_is_ppb_region(CPUARMState *env, uint32_t address)

+{

+ /* True if address is in the M profile PPB region 0xe0000000 - 0xe00fffff */

+ return arm_feature(env, ARM_FEATURE_M) &&

+ extract32(address, 20, 12) == 0xe00;

+}

+

static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,

int access_type, ARMMMUIdx mmu_idx,

hwaddr *phys_ptr, int *prot, uint32_t *fsr)

@@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,

*phys_ptr = address;

*prot = 0;

- if (regime_translation_disabled(env, mmu_idx)) { /* MPU disabled */

+ if (regime_translation_disabled(env, mmu_idx) ||

+ m_is_ppb_region(env, address)) {

+ /* MPU disabled or M profile PPB access: use default memory map.

+ * The other case which uses the default memory map in the

+ * v7M ARM ARM pseudocode is exception vector reads from the vector

+ * table. In QEMU those accesses are done in arm_v7m_load_vector(),

+ * which always does a direct read using address_space_ldl(), rather

+ * than going via this function, so we don't need to check that here.

+ */

get_phys_addr_pmsav7_default(env, mmu_idx, address, prot);

} else { /* MPU enabled */

for (n = (int)cpu->pmsav7_dregion - 1; n >= 0; n--) {

2.7.4

When the PMSAv7 implementation was originally added it was for R profile

CPUs only, and reset was handled using the cpreg .resetfn hooks.

Unfortunately for M profile cores this doesn't work, because they do

not register any cpregs. Move the reset handling into arm_cpu_reset(),

where it will work for both R profile and M profile cores.

target/arm/cpu.c | 14 ++++++++++++++

target/arm/helper.c | 28 ++++++++++++----------------

2 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c

--- a/target/arm/cpu.c

+++ b/target/arm/cpu.c

@@ -XXX,XX +XXX,XX @@ static void arm_cpu_reset(CPUState *s)

env->vfp.xregs[ARM_VFP_FPEXC] = 0;

#endif

+

+ if (arm_feature(env, ARM_FEATURE_PMSA) &&

+ arm_feature(env, ARM_FEATURE_V7)) {

+ if (cpu->pmsav7_dregion > 0) {

+ memset(env->pmsav7.drbar, 0,

+ sizeof(*env->pmsav7.drbar) * cpu->pmsav7_dregion);

+ memset(env->pmsav7.drsr, 0,

+ sizeof(*env->pmsav7.drsr) * cpu->pmsav7_dregion);

+ memset(env->pmsav7.dracr, 0,

+ sizeof(*env->pmsav7.dracr) * cpu->pmsav7_dregion);

+ }

+ env->pmsav7.rnr = 0;

+ }

+

set_flush_to_zero(1, &env->vfp.standard_fp_status);

set_flush_inputs_to_zero(1, &env->vfp.standard_fp_status);

set_default_nan_mode(1, &env->vfp.standard_fp_status);

diff --git a/target/arm/helper.c b/target/arm/helper.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/helper.c

+++ b/target/arm/helper.c

@@ -XXX,XX +XXX,XX @@ static void pmsav7_write(CPUARMState *env, const ARMCPRegInfo *ri,

*u32p = value;

-static void pmsav7_reset(CPUARMState *env, const ARMCPRegInfo *ri)

- ARMCPU *cpu = arm_env_get_cpu(env);

- uint32_t *u32p = *(uint32_t **)raw_ptr(env, ri);

-

- if (!u32p) {

- return;

- }

-

- memset(u32p, 0, sizeof(*u32p) * cpu->pmsav7_dregion);

static void pmsav7_rgnr_write(CPUARMState *env, const ARMCPRegInfo *ri,

uint64_t value)

{

@@ -XXX,XX +XXX,XX @@ static void pmsav7_rgnr_write(CPUARMState *env, const ARMCPRegInfo *ri,

static const ARMCPRegInfo pmsav7_cp_reginfo[] = {

+ /* Reset for all these registers is handled in arm_cpu_reset(),

+ * because the PMSAv7 is also used by M-profile CPUs, which do

+ * not register cpregs but still need the state to be reset.

+ */

{ .name = "DRBAR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 1, .opc2 = 0,

.access = PL1_RW, .type = ARM_CP_NO_RAW,

.fieldoffset = offsetof(CPUARMState, pmsav7.drbar),

- .readfn = pmsav7_read, .writefn = pmsav7_write, .resetfn = pmsav7_reset },

+ .readfn = pmsav7_read, .writefn = pmsav7_write,

+ .resetfn = arm_cp_reset_ignore },

{ .name = "DRSR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 1, .opc2 = 2,

.access = PL1_RW, .type = ARM_CP_NO_RAW,

.fieldoffset = offsetof(CPUARMState, pmsav7.drsr),

- .readfn = pmsav7_read, .writefn = pmsav7_write, .resetfn = pmsav7_reset },

+ .readfn = pmsav7_read, .writefn = pmsav7_write,

+ .resetfn = arm_cp_reset_ignore },

{ .name = "DRACR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 1, .opc2 = 4,

.access = PL1_RW, .type = ARM_CP_NO_RAW,

.fieldoffset = offsetof(CPUARMState, pmsav7.dracr),

- .readfn = pmsav7_read, .writefn = pmsav7_write, .resetfn = pmsav7_reset },