* [Qemu-devel] [PATCH 01/13] arm: Use the mmu_idx we're passed in arm_cpu_do_unaligned_access()
2017-04-25 12:06 [Qemu-devel] [PATCH 00/13] armv7m: Implement MPU support Peter Maydell
@ 2017-04-25 12:06 ` Peter Maydell
2017-05-02 22:05 ` Alistair Francis
2017-04-25 12:06 ` [Qemu-devel] [PATCH 02/13] arm: Add support for M profile CPUs having different MMU index semantics Peter Maydell
` (12 subsequent siblings)
13 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2017-04-25 12:06 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée, Alistair Francis
When identifying the DFSR format for an alignment fault, use
the mmu index that we are passed, rather than calling cpu_mmu_index()
to get the mmu index for the current CPU state. This doesn't actually
make any difference since the only cases where the current MMU index
differs from the index used for the load are the "unprivileged
load/store" instructions, and in that case the mmu index may
differ but the translation regime is the same (apart from the
"use from Hyp mode" case which is UNPREDICTABLE).
However it's the more logical thing to do.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/op_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index 156b825..de24815 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -208,7 +208,7 @@ void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
/* the DFSR for an alignment fault depends on whether we're using
* the LPAE long descriptor format, or the short descriptor format
*/
- if (arm_s1_regime_using_lpae_format(env, cpu_mmu_index(env, false))) {
+ if (arm_s1_regime_using_lpae_format(env, mmu_idx)) {
env->exception.fsr = (1 << 9) | 0x21;
} else {
env->exception.fsr = 0x1;
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 01/13] arm: Use the mmu_idx we're passed in arm_cpu_do_unaligned_access()
2017-04-25 12:06 ` [Qemu-devel] [PATCH 01/13] arm: Use the mmu_idx we're passed in arm_cpu_do_unaligned_access() Peter Maydell
@ 2017-05-02 22:05 ` Alistair Francis
2017-05-13 22:54 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 36+ messages in thread
From: Alistair Francis @ 2017-05-02 22:05 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, qemu-devel@nongnu.org Developers, Alistair Francis,
Alex Bennée, Patch Tracking
On Tue, Apr 25, 2017 at 5:06 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> When identifying the DFSR format for an alignment fault, use
> the mmu index that we are passed, rather than calling cpu_mmu_index()
> to get the mmu index for the current CPU state. This doesn't actually
> make any difference since the only cases where the current MMU index
> differs from the index used for the load are the "unprivileged
> load/store" instructions, and in that case the mmu index may
> differ but the translation regime is the same (apart from the
> "use from Hyp mode" case which is UNPREDICTABLE).
> However it's the more logical thing to do.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
Thanks,
Alistair
> ---
> target/arm/op_helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
> index 156b825..de24815 100644
> --- a/target/arm/op_helper.c
> +++ b/target/arm/op_helper.c
> @@ -208,7 +208,7 @@ void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
> /* the DFSR for an alignment fault depends on whether we're using
> * the LPAE long descriptor format, or the short descriptor format
> */
> - if (arm_s1_regime_using_lpae_format(env, cpu_mmu_index(env, false))) {
> + if (arm_s1_regime_using_lpae_format(env, mmu_idx)) {
> env->exception.fsr = (1 << 9) | 0x21;
> } else {
> env->exception.fsr = 0x1;
> --
> 2.7.4
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 01/13] arm: Use the mmu_idx we're passed in arm_cpu_do_unaligned_access()
2017-05-02 22:05 ` Alistair Francis
@ 2017-05-13 22:54 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-05-13 22:54 UTC (permalink / raw)
To: Alistair Francis, Peter Maydell
Cc: qemu-arm, Alex Bennée, qemu-devel@nongnu.org Developers,
Patch Tracking
On 05/02/2017 07:05 PM, Alistair Francis wrote:
> On Tue, Apr 25, 2017 at 5:06 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> When identifying the DFSR format for an alignment fault, use
>> the mmu index that we are passed, rather than calling cpu_mmu_index()
>> to get the mmu index for the current CPU state. This doesn't actually
>> make any difference since the only cases where the current MMU index
>> differs from the index used for the load are the "unprivileged
>> load/store" instructions, and in that case the mmu index may
>> differ but the translation regime is the same (apart from the
>> "use from Hyp mode" case which is UNPREDICTABLE).
>> However it's the more logical thing to do.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
> Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Thanks,
>
> Alistair
>
>> ---
>> target/arm/op_helper.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
>> index 156b825..de24815 100644
>> --- a/target/arm/op_helper.c
>> +++ b/target/arm/op_helper.c
>> @@ -208,7 +208,7 @@ void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
>> /* the DFSR for an alignment fault depends on whether we're using
>> * the LPAE long descriptor format, or the short descriptor format
>> */
>> - if (arm_s1_regime_using_lpae_format(env, cpu_mmu_index(env, false))) {
>> + if (arm_s1_regime_using_lpae_format(env, mmu_idx)) {
>> env->exception.fsr = (1 << 9) | 0x21;
>> } else {
>> env->exception.fsr = 0x1;
>> --
>> 2.7.4
>>
>>
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 02/13] arm: Add support for M profile CPUs having different MMU index semantics
2017-04-25 12:06 [Qemu-devel] [PATCH 00/13] armv7m: Implement MPU support Peter Maydell
2017-04-25 12:06 ` [Qemu-devel] [PATCH 01/13] arm: Use the mmu_idx we're passed in arm_cpu_do_unaligned_access() Peter Maydell
@ 2017-04-25 12:06 ` Peter Maydell
2017-05-02 22:23 ` Alistair Francis
2017-04-25 12:07 ` [Qemu-devel] [PATCH 03/13] arm: Use different ARMMMUIdx values for M profile Peter Maydell
` (11 subsequent siblings)
13 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2017-04-25 12:06 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée, Alistair Francis
The M profile CPU's MPU has an awkward corner case which we
would like to implement with a different MMU index.
We can avoid having to bump the number of MMU modes ARM
uses, because some of our existing MMU indexes are only
used by non-M-profile CPUs, so we can borrow one.
To avoid that getting too confusing, clean up the code
to try to keep the two meanings of the index separate.
Instead of ARMMMUIdx enum values being identical to core QEMU
MMU index values, they are now the core index values with some
high bits set. Any particular CPU always uses the same high
bits (so eventually A profile cores and M profile cores will
use different bits). New functions arm_to_core_mmu_idx()
and core_to_arm_mmu_idx() convert between the two.
In general core index values are stored in 'int' types, and
ARM values are stored in ARMMMUIdx types.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/cpu.h | 71 ++++++++++++++++-----
target/arm/translate.h | 2 +-
target/arm/helper.c | 151 ++++++++++++++++++++++++---------------------
target/arm/op_helper.c | 3 +-
target/arm/translate-a64.c | 18 ++++--
target/arm/translate.c | 10 +--
6 files changed, 156 insertions(+), 99 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 1055bfe..e1f4856 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2037,6 +2037,16 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
* for the accesses done as part of a stage 1 page table walk, rather than
* having to walk the stage 2 page table over and over.)
*
+ * The ARMMMUIdx and the mmu index value used by the core QEMU TLB code
+ * are not quite the same -- different CPU types (most notably M profile
+ * vs A/R profile) would like to use MMU indexes with different semantics,
+ * but since we don't ever need to use all of those in a single CPU we
+ * can avoid setting NB_MMU_MODES to more than 8. The lower bits of
+ * ARMMMUIdx are the core TLB mmu index, and the higher bits are always
+ * the same for any particular CPU.
+ * Variables of type ARMMUIdx are always full values, and the core
+ * index values are in variables of type 'int'.
+ *
* Our enumeration includes at the end some entries which are not "true"
* mmu_idx values in that they don't have corresponding TLBs and are only
* valid for doing slow path page table walks.
@@ -2045,28 +2055,61 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
* of the AT/ATS operations.
* The values used are carefully arranged to make mmu_idx => EL lookup easy.
*/
+#define ARM_MMU_IDX_A 0x10 /* A profile (and M profile, for the moment) */
+#define ARM_MMU_IDX_NOTLB 0x20 /* does not have a TLB */
+
+#define ARM_MMU_IDX_TYPE_MASK (~0x7)
+#define ARM_MMU_IDX_COREIDX_MASK 0x7
+
typedef enum ARMMMUIdx {
- ARMMMUIdx_S12NSE0 = 0,
- ARMMMUIdx_S12NSE1 = 1,
- ARMMMUIdx_S1E2 = 2,
- ARMMMUIdx_S1E3 = 3,
- ARMMMUIdx_S1SE0 = 4,
- ARMMMUIdx_S1SE1 = 5,
- ARMMMUIdx_S2NS = 6,
+ ARMMMUIdx_S12NSE0 = 0 | ARM_MMU_IDX_A,
+ ARMMMUIdx_S12NSE1 = 1 | ARM_MMU_IDX_A,
+ ARMMMUIdx_S1E2 = 2 | ARM_MMU_IDX_A,
+ ARMMMUIdx_S1E3 = 3 | ARM_MMU_IDX_A,
+ ARMMMUIdx_S1SE0 = 4 | ARM_MMU_IDX_A,
+ ARMMMUIdx_S1SE1 = 5 | ARM_MMU_IDX_A,
+ ARMMMUIdx_S2NS = 6 | ARM_MMU_IDX_A,
/* Indexes below here don't have TLBs and are used only for AT system
* instructions or for the first stage of an S12 page table walk.
*/
- ARMMMUIdx_S1NSE0 = 7,
- ARMMMUIdx_S1NSE1 = 8,
+ ARMMMUIdx_S1NSE0 = 0 | ARM_MMU_IDX_NOTLB,
+ ARMMMUIdx_S1NSE1 = 1 | ARM_MMU_IDX_NOTLB,
} ARMMMUIdx;
+/* Bit macros for the core-mmu-index values for each index,
+ * for use when calling tlb_flush_by_mmuidx() and friends.
+ */
+typedef enum ARMMMUIdxBit {
+ ARMMMUIdxBit_S12NSE0 = 1 << 0,
+ ARMMMUIdxBit_S12NSE1 = 1 << 1,
+ ARMMMUIdxBit_S1E2 = 1 << 2,
+ ARMMMUIdxBit_S1E3 = 1 << 3,
+ ARMMMUIdxBit_S1SE0 = 1 << 4,
+ ARMMMUIdxBit_S1SE1 = 1 << 5,
+ ARMMMUIdxBit_S2NS = 1 << 6,
+} ARMMMUIdxBit;
+
#define MMU_USER_IDX 0
+static inline int arm_to_core_mmu_idx(ARMMMUIdx mmu_idx)
+{
+ return mmu_idx & ARM_MMU_IDX_COREIDX_MASK;
+}
+
+static inline ARMMMUIdx core_to_arm_mmu_idx(CPUARMState *env, int mmu_idx)
+{
+ return mmu_idx | ARM_MMU_IDX_A;
+}
+
/* Return the exception level we're running at if this is our mmu_idx */
static inline int arm_mmu_idx_to_el(ARMMMUIdx mmu_idx)
{
- assert(mmu_idx < ARMMMUIdx_S2NS);
- return mmu_idx & 3;
+ switch (mmu_idx & ARM_MMU_IDX_TYPE_MASK) {
+ case ARM_MMU_IDX_A:
+ return mmu_idx & 3;
+ default:
+ g_assert_not_reached();
+ }
}
/* Determine the current mmu_idx to use for normal loads/stores */
@@ -2075,7 +2118,7 @@ static inline int cpu_mmu_index(CPUARMState *env, bool ifetch)
int el = arm_current_el(env);
if (el < 2 && arm_is_secure_below_el3(env)) {
- return ARMMMUIdx_S1SE0 + el;
+ return arm_to_core_mmu_idx(ARMMMUIdx_S1SE0 + el);
}
return el;
}
@@ -2471,7 +2514,7 @@ static inline uint32_t arm_regime_tbi1(CPUARMState *env, ARMMMUIdx mmu_idx)
static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
target_ulong *cs_base, uint32_t *flags)
{
- ARMMMUIdx mmu_idx = cpu_mmu_index(env, false);
+ ARMMMUIdx mmu_idx = core_to_arm_mmu_idx(env, cpu_mmu_index(env, false));
if (is_a64(env)) {
*pc = env->pc;
*flags = ARM_TBFLAG_AARCH64_STATE_MASK;
@@ -2496,7 +2539,7 @@ static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
<< ARM_TBFLAG_XSCALE_CPAR_SHIFT);
}
- *flags |= (mmu_idx << ARM_TBFLAG_MMUIDX_SHIFT);
+ *flags |= (arm_to_core_mmu_idx(mmu_idx) << ARM_TBFLAG_MMUIDX_SHIFT);
/* The SS_ACTIVE and PSTATE_SS bits correspond to the state machine
* states defined in the ARM ARM for software singlestep:
diff --git a/target/arm/translate.h b/target/arm/translate.h
index 629dab9..6b2cc34 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -88,7 +88,7 @@ static inline int arm_dc_feature(DisasContext *dc, int feature)
static inline int get_mem_index(DisasContext *s)
{
- return s->mmu_idx;
+ return arm_to_core_mmu_idx(s->mmu_idx);
}
/* Function used to determine the target exception EL when otherwise not known
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 8a3e448..520adcc 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -571,9 +571,9 @@ static void tlbiall_nsnh_write(CPUARMState *env, const ARMCPRegInfo *ri,
CPUState *cs = ENV_GET_CPU(env);
tlb_flush_by_mmuidx(cs,
- (1 << ARMMMUIdx_S12NSE1) |
- (1 << ARMMMUIdx_S12NSE0) |
- (1 << ARMMMUIdx_S2NS));
+ ARMMMUIdxBit_S12NSE1 |
+ ARMMMUIdxBit_S12NSE0 |
+ ARMMMUIdxBit_S2NS);
}
static void tlbiall_nsnh_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -582,9 +582,9 @@ static void tlbiall_nsnh_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
CPUState *cs = ENV_GET_CPU(env);
tlb_flush_by_mmuidx_all_cpus_synced(cs,
- (1 << ARMMMUIdx_S12NSE1) |
- (1 << ARMMMUIdx_S12NSE0) |
- (1 << ARMMMUIdx_S2NS));
+ ARMMMUIdxBit_S12NSE1 |
+ ARMMMUIdxBit_S12NSE0 |
+ ARMMMUIdxBit_S2NS);
}
static void tlbiipas2_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -605,7 +605,7 @@ static void tlbiipas2_write(CPUARMState *env, const ARMCPRegInfo *ri,
pageaddr = sextract64(value << 12, 0, 40);
- tlb_flush_page_by_mmuidx(cs, pageaddr, (1 << ARMMMUIdx_S2NS));
+ tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdxBit_S2NS);
}
static void tlbiipas2_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -621,7 +621,7 @@ static void tlbiipas2_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
pageaddr = sextract64(value << 12, 0, 40);
tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr,
- (1 << ARMMMUIdx_S2NS));
+ ARMMMUIdxBit_S2NS);
}
static void tlbiall_hyp_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -629,7 +629,7 @@ static void tlbiall_hyp_write(CPUARMState *env, const ARMCPRegInfo *ri,
{
CPUState *cs = ENV_GET_CPU(env);
- tlb_flush_by_mmuidx(cs, (1 << ARMMMUIdx_S1E2));
+ tlb_flush_by_mmuidx(cs, ARMMMUIdxBit_S1E2);
}
static void tlbiall_hyp_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -637,7 +637,7 @@ static void tlbiall_hyp_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
{
CPUState *cs = ENV_GET_CPU(env);
- tlb_flush_by_mmuidx_all_cpus_synced(cs, (1 << ARMMMUIdx_S1E2));
+ tlb_flush_by_mmuidx_all_cpus_synced(cs, ARMMMUIdxBit_S1E2);
}
static void tlbimva_hyp_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -646,7 +646,7 @@ static void tlbimva_hyp_write(CPUARMState *env, const ARMCPRegInfo *ri,
CPUState *cs = ENV_GET_CPU(env);
uint64_t pageaddr = value & ~MAKE_64BIT_MASK(0, 12);
- tlb_flush_page_by_mmuidx(cs, pageaddr, (1 << ARMMMUIdx_S1E2));
+ tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdxBit_S1E2);
}
static void tlbimva_hyp_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -656,7 +656,7 @@ static void tlbimva_hyp_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t pageaddr = value & ~MAKE_64BIT_MASK(0, 12);
tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr,
- (1 << ARMMMUIdx_S1E2));
+ ARMMMUIdxBit_S1E2);
}
static const ARMCPRegInfo cp_reginfo[] = {
@@ -2596,9 +2596,9 @@ static void vttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
/* Accesses to VTTBR may change the VMID so we must flush the TLB. */
if (raw_read(env, ri) != value) {
tlb_flush_by_mmuidx(cs,
- (1 << ARMMMUIdx_S12NSE1) |
- (1 << ARMMMUIdx_S12NSE0) |
- (1 << ARMMMUIdx_S2NS));
+ ARMMMUIdxBit_S12NSE1 |
+ ARMMMUIdxBit_S12NSE0 |
+ ARMMMUIdxBit_S2NS);
raw_write(env, ri, value);
}
}
@@ -2957,12 +2957,12 @@ static void tlbi_aa64_vmalle1_write(CPUARMState *env, const ARMCPRegInfo *ri,
if (arm_is_secure_below_el3(env)) {
tlb_flush_by_mmuidx(cs,
- (1 << ARMMMUIdx_S1SE1) |
- (1 << ARMMMUIdx_S1SE0));
+ ARMMMUIdxBit_S1SE1 |
+ ARMMMUIdxBit_S1SE0);
} else {
tlb_flush_by_mmuidx(cs,
- (1 << ARMMMUIdx_S12NSE1) |
- (1 << ARMMMUIdx_S12NSE0));
+ ARMMMUIdxBit_S12NSE1 |
+ ARMMMUIdxBit_S12NSE0);
}
}
@@ -2974,12 +2974,12 @@ static void tlbi_aa64_vmalle1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
if (sec) {
tlb_flush_by_mmuidx_all_cpus_synced(cs,
- (1 << ARMMMUIdx_S1SE1) |
- (1 << ARMMMUIdx_S1SE0));
+ ARMMMUIdxBit_S1SE1 |
+ ARMMMUIdxBit_S1SE0);
} else {
tlb_flush_by_mmuidx_all_cpus_synced(cs,
- (1 << ARMMMUIdx_S12NSE1) |
- (1 << ARMMMUIdx_S12NSE0));
+ ARMMMUIdxBit_S12NSE1 |
+ ARMMMUIdxBit_S12NSE0);
}
}
@@ -2995,18 +2995,18 @@ static void tlbi_aa64_alle1_write(CPUARMState *env, const ARMCPRegInfo *ri,
if (arm_is_secure_below_el3(env)) {
tlb_flush_by_mmuidx(cs,
- (1 << ARMMMUIdx_S1SE1) |
- (1 << ARMMMUIdx_S1SE0));
+ ARMMMUIdxBit_S1SE1 |
+ ARMMMUIdxBit_S1SE0);
} else {
if (arm_feature(env, ARM_FEATURE_EL2)) {
tlb_flush_by_mmuidx(cs,
- (1 << ARMMMUIdx_S12NSE1) |
- (1 << ARMMMUIdx_S12NSE0) |
- (1 << ARMMMUIdx_S2NS));
+ ARMMMUIdxBit_S12NSE1 |
+ ARMMMUIdxBit_S12NSE0 |
+ ARMMMUIdxBit_S2NS);
} else {
tlb_flush_by_mmuidx(cs,
- (1 << ARMMMUIdx_S12NSE1) |
- (1 << ARMMMUIdx_S12NSE0));
+ ARMMMUIdxBit_S12NSE1 |
+ ARMMMUIdxBit_S12NSE0);
}
}
}
@@ -3017,7 +3017,7 @@ static void tlbi_aa64_alle2_write(CPUARMState *env, const ARMCPRegInfo *ri,
ARMCPU *cpu = arm_env_get_cpu(env);
CPUState *cs = CPU(cpu);
- tlb_flush_by_mmuidx(cs, (1 << ARMMMUIdx_S1E2));
+ tlb_flush_by_mmuidx(cs, ARMMMUIdxBit_S1E2);
}
static void tlbi_aa64_alle3_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -3026,7 +3026,7 @@ static void tlbi_aa64_alle3_write(CPUARMState *env, const ARMCPRegInfo *ri,
ARMCPU *cpu = arm_env_get_cpu(env);
CPUState *cs = CPU(cpu);
- tlb_flush_by_mmuidx(cs, (1 << ARMMMUIdx_S1E3));
+ tlb_flush_by_mmuidx(cs, ARMMMUIdxBit_S1E3);
}
static void tlbi_aa64_alle1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -3042,17 +3042,17 @@ static void tlbi_aa64_alle1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
if (sec) {
tlb_flush_by_mmuidx_all_cpus_synced(cs,
- (1 << ARMMMUIdx_S1SE1) |
- (1 << ARMMMUIdx_S1SE0));
+ ARMMMUIdxBit_S1SE1 |
+ ARMMMUIdxBit_S1SE0);
} else if (has_el2) {
tlb_flush_by_mmuidx_all_cpus_synced(cs,
- (1 << ARMMMUIdx_S12NSE1) |
- (1 << ARMMMUIdx_S12NSE0) |
- (1 << ARMMMUIdx_S2NS));
+ ARMMMUIdxBit_S12NSE1 |
+ ARMMMUIdxBit_S12NSE0 |
+ ARMMMUIdxBit_S2NS);
} else {
tlb_flush_by_mmuidx_all_cpus_synced(cs,
- (1 << ARMMMUIdx_S12NSE1) |
- (1 << ARMMMUIdx_S12NSE0));
+ ARMMMUIdxBit_S12NSE1 |
+ ARMMMUIdxBit_S12NSE0);
}
}
@@ -3061,7 +3061,7 @@ static void tlbi_aa64_alle2is_write(CPUARMState *env, const ARMCPRegInfo *ri,
{
CPUState *cs = ENV_GET_CPU(env);
- tlb_flush_by_mmuidx_all_cpus_synced(cs, (1 << ARMMMUIdx_S1E2));
+ tlb_flush_by_mmuidx_all_cpus_synced(cs, ARMMMUIdxBit_S1E2);
}
static void tlbi_aa64_alle3is_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -3069,7 +3069,7 @@ static void tlbi_aa64_alle3is_write(CPUARMState *env, const ARMCPRegInfo *ri,
{
CPUState *cs = ENV_GET_CPU(env);
- tlb_flush_by_mmuidx_all_cpus_synced(cs, (1 << ARMMMUIdx_S1E3));
+ tlb_flush_by_mmuidx_all_cpus_synced(cs, ARMMMUIdxBit_S1E3);
}
static void tlbi_aa64_vae1_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -3086,12 +3086,12 @@ static void tlbi_aa64_vae1_write(CPUARMState *env, const ARMCPRegInfo *ri,
if (arm_is_secure_below_el3(env)) {
tlb_flush_page_by_mmuidx(cs, pageaddr,
- (1 << ARMMMUIdx_S1SE1) |
- (1 << ARMMMUIdx_S1SE0));
+ ARMMMUIdxBit_S1SE1 |
+ ARMMMUIdxBit_S1SE0);
} else {
tlb_flush_page_by_mmuidx(cs, pageaddr,
- (1 << ARMMMUIdx_S12NSE1) |
- (1 << ARMMMUIdx_S12NSE0));
+ ARMMMUIdxBit_S12NSE1 |
+ ARMMMUIdxBit_S12NSE0);
}
}
@@ -3106,7 +3106,7 @@ static void tlbi_aa64_vae2_write(CPUARMState *env, const ARMCPRegInfo *ri,
CPUState *cs = CPU(cpu);
uint64_t pageaddr = sextract64(value << 12, 0, 56);
- tlb_flush_page_by_mmuidx(cs, pageaddr, (1 << ARMMMUIdx_S1E2));
+ tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdxBit_S1E2);
}
static void tlbi_aa64_vae3_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -3120,7 +3120,7 @@ static void tlbi_aa64_vae3_write(CPUARMState *env, const ARMCPRegInfo *ri,
CPUState *cs = CPU(cpu);
uint64_t pageaddr = sextract64(value << 12, 0, 56);
- tlb_flush_page_by_mmuidx(cs, pageaddr, (1 << ARMMMUIdx_S1E3));
+ tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdxBit_S1E3);
}
static void tlbi_aa64_vae1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -3133,12 +3133,12 @@ static void tlbi_aa64_vae1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
if (sec) {
tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr,
- (1 << ARMMMUIdx_S1SE1) |
- (1 << ARMMMUIdx_S1SE0));
+ ARMMMUIdxBit_S1SE1 |
+ ARMMMUIdxBit_S1SE0);
} else {
tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr,
- (1 << ARMMMUIdx_S12NSE1) |
- (1 << ARMMMUIdx_S12NSE0));
+ ARMMMUIdxBit_S12NSE1 |
+ ARMMMUIdxBit_S12NSE0);
}
}
@@ -3149,7 +3149,7 @@ static void tlbi_aa64_vae2is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t pageaddr = sextract64(value << 12, 0, 56);
tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr,
- (1 << ARMMMUIdx_S1E2));
+ ARMMMUIdxBit_S1E2);
}
static void tlbi_aa64_vae3is_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -3159,7 +3159,7 @@ static void tlbi_aa64_vae3is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t pageaddr = sextract64(value << 12, 0, 56);
tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr,
- (1 << ARMMMUIdx_S1E3));
+ ARMMMUIdxBit_S1E3);
}
static void tlbi_aa64_ipas2e1_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -3181,7 +3181,7 @@ static void tlbi_aa64_ipas2e1_write(CPUARMState *env, const ARMCPRegInfo *ri,
pageaddr = sextract64(value << 12, 0, 48);
- tlb_flush_page_by_mmuidx(cs, pageaddr, (1 << ARMMMUIdx_S2NS));
+ tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdxBit_S2NS);
}
static void tlbi_aa64_ipas2e1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -3197,7 +3197,7 @@ static void tlbi_aa64_ipas2e1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
pageaddr = sextract64(value << 12, 0, 48);
tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr,
- (1 << ARMMMUIdx_S2NS));
+ ARMMMUIdxBit_S2NS);
}
static CPAccessResult aa64_zva_access(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -7049,6 +7049,17 @@ static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
return &env->cp15.tcr_el[regime_el(env, mmu_idx)];
}
+/* Convert a possible stage1+2 MMU index into the appropriate
+ * stage 1 MMU index
+ */
+static inline ARMMMUIdx stage_1_mmu_idx(ARMMMUIdx mmu_idx)
+{
+ if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
+ mmu_idx += (ARMMMUIdx_S1NSE0 - ARMMMUIdx_S12NSE0);
+ }
+ return mmu_idx;
+}
+
/* Returns TBI0 value for current regime el */
uint32_t arm_regime_tbi0(CPUARMState *env, ARMMMUIdx mmu_idx)
{
@@ -7056,11 +7067,9 @@ uint32_t arm_regime_tbi0(CPUARMState *env, ARMMMUIdx mmu_idx)
uint32_t el;
/* For EL0 and EL1, TBI is controlled by stage 1's TCR, so convert
- * a stage 1+2 mmu index into the appropriate stage 1 mmu index.
- */
- if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
- mmu_idx += ARMMMUIdx_S1NSE0;
- }
+ * a stage 1+2 mmu index into the appropriate stage 1 mmu index.
+ */
+ mmu_idx = stage_1_mmu_idx(mmu_idx);
tcr = regime_tcr(env, mmu_idx);
el = regime_el(env, mmu_idx);
@@ -7079,11 +7088,9 @@ uint32_t arm_regime_tbi1(CPUARMState *env, ARMMMUIdx mmu_idx)
uint32_t el;
/* For EL0 and EL1, TBI is controlled by stage 1's TCR, so convert
- * a stage 1+2 mmu index into the appropriate stage 1 mmu index.
- */
- if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
- mmu_idx += ARMMMUIdx_S1NSE0;
- }
+ * a stage 1+2 mmu index into the appropriate stage 1 mmu index.
+ */
+ mmu_idx = stage_1_mmu_idx(mmu_idx);
tcr = regime_tcr(env, mmu_idx);
el = regime_el(env, mmu_idx);
@@ -7129,9 +7136,7 @@ static inline bool regime_using_lpae_format(CPUARMState *env,
* on whether the long or short descriptor format is in use. */
bool arm_s1_regime_using_lpae_format(CPUARMState *env, ARMMMUIdx mmu_idx)
{
- if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
- mmu_idx += ARMMMUIdx_S1NSE0;
- }
+ mmu_idx = stage_1_mmu_idx(mmu_idx);
return regime_using_lpae_format(env, mmu_idx);
}
@@ -8385,7 +8390,7 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address,
int ret;
ret = get_phys_addr(env, address, access_type,
- mmu_idx + ARMMMUIdx_S1NSE0, &ipa, attrs,
+ stage_1_mmu_idx(mmu_idx), &ipa, attrs,
prot, page_size, fsr, fi);
/* If S1 fails or S2 is disabled, return early. */
@@ -8406,7 +8411,7 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address,
/*
* For non-EL2 CPUs a stage1+stage2 translation is just stage 1.
*/
- mmu_idx += ARMMMUIdx_S1NSE0;
+ mmu_idx = stage_1_mmu_idx(mmu_idx);
}
}
@@ -8482,7 +8487,8 @@ bool arm_tlb_fill(CPUState *cs, vaddr address,
int ret;
MemTxAttrs attrs = {};
- ret = get_phys_addr(env, address, access_type, mmu_idx, &phys_addr,
+ ret = get_phys_addr(env, address, access_type,
+ core_to_arm_mmu_idx(env, mmu_idx), &phys_addr,
&attrs, &prot, &page_size, fsr, fi);
if (!ret) {
/* Map a single [sub]page. */
@@ -8507,10 +8513,11 @@ hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
bool ret;
uint32_t fsr;
ARMMMUFaultInfo fi = {};
+ ARMMMUIdx mmu_idx = core_to_arm_mmu_idx(env, cpu_mmu_index(env, false));
*attrs = (MemTxAttrs) {};
- ret = get_phys_addr(env, addr, 0, cpu_mmu_index(env, false), &phys_addr,
+ ret = get_phys_addr(env, addr, 0, mmu_idx, &phys_addr,
attrs, &prot, &page_size, &fsr, &fi);
if (ret) {
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index de24815..2a85666 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -194,6 +194,7 @@ void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
int target_el;
bool same_el;
uint32_t syn;
+ ARMMMUIdx arm_mmu_idx = core_to_arm_mmu_idx(env, mmu_idx);
if (retaddr) {
/* now we have a real cpu fault */
@@ -208,7 +209,7 @@ void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
/* the DFSR for an alignment fault depends on whether we're using
* the LPAE long descriptor format, or the short descriptor format
*/
- if (arm_s1_regime_using_lpae_format(env, mmu_idx)) {
+ if (arm_s1_regime_using_lpae_format(env, arm_mmu_idx)) {
env->exception.fsr = (1 << 9) | 0x21;
} else {
env->exception.fsr = 0x1;
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 24de30d..a82ab49 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -101,21 +101,27 @@ void a64_translate_init(void)
offsetof(CPUARMState, exclusive_high), "exclusive_high");
}
-static inline ARMMMUIdx get_a64_user_mem_index(DisasContext *s)
+static inline int get_a64_user_mem_index(DisasContext *s)
{
- /* Return the mmu_idx to use for A64 "unprivileged load/store" insns:
+ /* Return the core mmu_idx to use for A64 "unprivileged load/store" insns:
* if EL1, access as if EL0; otherwise access at current EL
*/
+ ARMMMUIdx useridx;
+
switch (s->mmu_idx) {
case ARMMMUIdx_S12NSE1:
- return ARMMMUIdx_S12NSE0;
+ useridx = ARMMMUIdx_S12NSE0;
+ break;
case ARMMMUIdx_S1SE1:
- return ARMMMUIdx_S1SE0;
+ useridx = ARMMMUIdx_S1SE0;
+ break;
case ARMMMUIdx_S2NS:
g_assert_not_reached();
default:
- return s->mmu_idx;
+ useridx = s->mmu_idx;
+ break;
}
+ return arm_to_core_mmu_idx(useridx);
}
void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
@@ -11212,7 +11218,7 @@ void gen_intermediate_code_a64(ARMCPU *cpu, TranslationBlock *tb)
dc->be_data = ARM_TBFLAG_BE_DATA(tb->flags) ? MO_BE : MO_LE;
dc->condexec_mask = 0;
dc->condexec_cond = 0;
- dc->mmu_idx = ARM_TBFLAG_MMUIDX(tb->flags);
+ dc->mmu_idx = core_to_arm_mmu_idx(env, ARM_TBFLAG_MMUIDX(tb->flags));
dc->tbi0 = ARM_TBFLAG_TBI0(tb->flags);
dc->tbi1 = ARM_TBFLAG_TBI1(tb->flags);
dc->current_el = arm_mmu_idx_to_el(dc->mmu_idx);
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 0b5a0bc..8d509a2 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -145,9 +145,9 @@ static void disas_set_da_iss(DisasContext *s, TCGMemOp memop, ISSInfo issinfo)
disas_set_insn_syndrome(s, syn);
}
-static inline ARMMMUIdx get_a32_user_mem_index(DisasContext *s)
+static inline int get_a32_user_mem_index(DisasContext *s)
{
- /* Return the mmu_idx to use for A32/T32 "unprivileged load/store"
+ /* Return the core mmu_idx to use for A32/T32 "unprivileged load/store"
* insns:
* if PL2, UNPREDICTABLE (we choose to implement as if PL0)
* otherwise, access as if at PL0.
@@ -156,11 +156,11 @@ static inline ARMMMUIdx get_a32_user_mem_index(DisasContext *s)
case ARMMMUIdx_S1E2: /* this one is UNPREDICTABLE */
case ARMMMUIdx_S12NSE0:
case ARMMMUIdx_S12NSE1:
- return ARMMMUIdx_S12NSE0;
+ return arm_to_core_mmu_idx(ARMMMUIdx_S12NSE0);
case ARMMMUIdx_S1E3:
case ARMMMUIdx_S1SE0:
case ARMMMUIdx_S1SE1:
- return ARMMMUIdx_S1SE0;
+ return arm_to_core_mmu_idx(ARMMMUIdx_S1SE0);
case ARMMMUIdx_S2NS:
default:
g_assert_not_reached();
@@ -11816,7 +11816,7 @@ void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb)
dc->be_data = ARM_TBFLAG_BE_DATA(tb->flags) ? MO_BE : MO_LE;
dc->condexec_mask = (ARM_TBFLAG_CONDEXEC(tb->flags) & 0xf) << 1;
dc->condexec_cond = ARM_TBFLAG_CONDEXEC(tb->flags) >> 4;
- dc->mmu_idx = ARM_TBFLAG_MMUIDX(tb->flags);
+ dc->mmu_idx = core_to_arm_mmu_idx(env, ARM_TBFLAG_MMUIDX(tb->flags));
dc->current_el = arm_mmu_idx_to_el(dc->mmu_idx);
#if !defined(CONFIG_USER_ONLY)
dc->user = (dc->current_el == 0);
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 02/13] arm: Add support for M profile CPUs having different MMU index semantics
2017-04-25 12:06 ` [Qemu-devel] [PATCH 02/13] arm: Add support for M profile CPUs having different MMU index semantics Peter Maydell
@ 2017-05-02 22:23 ` Alistair Francis
2017-05-30 13:56 ` Peter Maydell
0 siblings, 1 reply; 36+ messages in thread
From: Alistair Francis @ 2017-05-02 22:23 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, qemu-devel@nongnu.org Developers, Alistair Francis,
Alex Bennée, Patch Tracking
On Tue, Apr 25, 2017 at 5:06 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> The M profile CPU's MPU has an awkward corner case which we
> would like to implement with a different MMU index.
>
> We can avoid having to bump the number of MMU modes ARM
> uses, because some of our existing MMU indexes are only
> used by non-M-profile CPUs, so we can borrow one.
> To avoid that getting too confusing, clean up the code
> to try to keep the two meanings of the index separate.
>
> Instead of ARMMMUIdx enum values being identical to core QEMU
> MMU index values, they are now the core index values with some
> high bits set. Any particular CPU always uses the same high
> bits (so eventually A profile cores and M profile cores will
> use different bits). New functions arm_to_core_mmu_idx()
> and core_to_arm_mmu_idx() convert between the two.
>
> In general core index values are stored in 'int' types, and
> ARM values are stored in ARMMMUIdx types.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
This looks ok to me. I'm a little worried it will make SMMU or some
ARMv9 (or whatever the next thing is) work more complicated as we have
this.
Thanks,
Alistair
> ---
> target/arm/cpu.h | 71 ++++++++++++++++-----
> target/arm/translate.h | 2 +-
> target/arm/helper.c | 151 ++++++++++++++++++++++++---------------------
> target/arm/op_helper.c | 3 +-
> target/arm/translate-a64.c | 18 ++++--
> target/arm/translate.c | 10 +--
> 6 files changed, 156 insertions(+), 99 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 1055bfe..e1f4856 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -2037,6 +2037,16 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
> * for the accesses done as part of a stage 1 page table walk, rather than
> * having to walk the stage 2 page table over and over.)
> *
> + * The ARMMMUIdx and the mmu index value used by the core QEMU TLB code
> + * are not quite the same -- different CPU types (most notably M profile
> + * vs A/R profile) would like to use MMU indexes with different semantics,
> + * but since we don't ever need to use all of those in a single CPU we
> + * can avoid setting NB_MMU_MODES to more than 8. The lower bits of
> + * ARMMMUIdx are the core TLB mmu index, and the higher bits are always
> + * the same for any particular CPU.
> + * Variables of type ARMMUIdx are always full values, and the core
> + * index values are in variables of type 'int'.
> + *
> * Our enumeration includes at the end some entries which are not "true"
> * mmu_idx values in that they don't have corresponding TLBs and are only
> * valid for doing slow path page table walks.
> @@ -2045,28 +2055,61 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
> * of the AT/ATS operations.
> * The values used are carefully arranged to make mmu_idx => EL lookup easy.
> */
> +#define ARM_MMU_IDX_A 0x10 /* A profile (and M profile, for the moment) */
> +#define ARM_MMU_IDX_NOTLB 0x20 /* does not have a TLB */
> +
> +#define ARM_MMU_IDX_TYPE_MASK (~0x7)
> +#define ARM_MMU_IDX_COREIDX_MASK 0x7
> +
> typedef enum ARMMMUIdx {
> - ARMMMUIdx_S12NSE0 = 0,
> - ARMMMUIdx_S12NSE1 = 1,
> - ARMMMUIdx_S1E2 = 2,
> - ARMMMUIdx_S1E3 = 3,
> - ARMMMUIdx_S1SE0 = 4,
> - ARMMMUIdx_S1SE1 = 5,
> - ARMMMUIdx_S2NS = 6,
> + ARMMMUIdx_S12NSE0 = 0 | ARM_MMU_IDX_A,
> + ARMMMUIdx_S12NSE1 = 1 | ARM_MMU_IDX_A,
> + ARMMMUIdx_S1E2 = 2 | ARM_MMU_IDX_A,
> + ARMMMUIdx_S1E3 = 3 | ARM_MMU_IDX_A,
> + ARMMMUIdx_S1SE0 = 4 | ARM_MMU_IDX_A,
> + ARMMMUIdx_S1SE1 = 5 | ARM_MMU_IDX_A,
> + ARMMMUIdx_S2NS = 6 | ARM_MMU_IDX_A,
> /* Indexes below here don't have TLBs and are used only for AT system
> * instructions or for the first stage of an S12 page table walk.
> */
> - ARMMMUIdx_S1NSE0 = 7,
> - ARMMMUIdx_S1NSE1 = 8,
> + ARMMMUIdx_S1NSE0 = 0 | ARM_MMU_IDX_NOTLB,
> + ARMMMUIdx_S1NSE1 = 1 | ARM_MMU_IDX_NOTLB,
> } ARMMMUIdx;
>
> +/* Bit macros for the core-mmu-index values for each index,
> + * for use when calling tlb_flush_by_mmuidx() and friends.
> + */
> +typedef enum ARMMMUIdxBit {
> + ARMMMUIdxBit_S12NSE0 = 1 << 0,
> + ARMMMUIdxBit_S12NSE1 = 1 << 1,
> + ARMMMUIdxBit_S1E2 = 1 << 2,
> + ARMMMUIdxBit_S1E3 = 1 << 3,
> + ARMMMUIdxBit_S1SE0 = 1 << 4,
> + ARMMMUIdxBit_S1SE1 = 1 << 5,
> + ARMMMUIdxBit_S2NS = 1 << 6,
> +} ARMMMUIdxBit;
> +
> #define MMU_USER_IDX 0
>
> +static inline int arm_to_core_mmu_idx(ARMMMUIdx mmu_idx)
> +{
> + return mmu_idx & ARM_MMU_IDX_COREIDX_MASK;
> +}
> +
> +static inline ARMMMUIdx core_to_arm_mmu_idx(CPUARMState *env, int mmu_idx)
> +{
> + return mmu_idx | ARM_MMU_IDX_A;
> +}
> +
> /* Return the exception level we're running at if this is our mmu_idx */
> static inline int arm_mmu_idx_to_el(ARMMMUIdx mmu_idx)
> {
> - assert(mmu_idx < ARMMMUIdx_S2NS);
> - return mmu_idx & 3;
> + switch (mmu_idx & ARM_MMU_IDX_TYPE_MASK) {
> + case ARM_MMU_IDX_A:
> + return mmu_idx & 3;
> + default:
> + g_assert_not_reached();
> + }
> }
>
> /* Determine the current mmu_idx to use for normal loads/stores */
> @@ -2075,7 +2118,7 @@ static inline int cpu_mmu_index(CPUARMState *env, bool ifetch)
> int el = arm_current_el(env);
>
> if (el < 2 && arm_is_secure_below_el3(env)) {
> - return ARMMMUIdx_S1SE0 + el;
> + return arm_to_core_mmu_idx(ARMMMUIdx_S1SE0 + el);
> }
> return el;
> }
> @@ -2471,7 +2514,7 @@ static inline uint32_t arm_regime_tbi1(CPUARMState *env, ARMMMUIdx mmu_idx)
> static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
> target_ulong *cs_base, uint32_t *flags)
> {
> - ARMMMUIdx mmu_idx = cpu_mmu_index(env, false);
> + ARMMMUIdx mmu_idx = core_to_arm_mmu_idx(env, cpu_mmu_index(env, false));
> if (is_a64(env)) {
> *pc = env->pc;
> *flags = ARM_TBFLAG_AARCH64_STATE_MASK;
> @@ -2496,7 +2539,7 @@ static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
> << ARM_TBFLAG_XSCALE_CPAR_SHIFT);
> }
>
> - *flags |= (mmu_idx << ARM_TBFLAG_MMUIDX_SHIFT);
> + *flags |= (arm_to_core_mmu_idx(mmu_idx) << ARM_TBFLAG_MMUIDX_SHIFT);
>
> /* The SS_ACTIVE and PSTATE_SS bits correspond to the state machine
> * states defined in the ARM ARM for software singlestep:
> diff --git a/target/arm/translate.h b/target/arm/translate.h
> index 629dab9..6b2cc34 100644
> --- a/target/arm/translate.h
> +++ b/target/arm/translate.h
> @@ -88,7 +88,7 @@ static inline int arm_dc_feature(DisasContext *dc, int feature)
>
> static inline int get_mem_index(DisasContext *s)
> {
> - return s->mmu_idx;
> + return arm_to_core_mmu_idx(s->mmu_idx);
> }
>
> /* Function used to determine the target exception EL when otherwise not known
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 8a3e448..520adcc 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -571,9 +571,9 @@ static void tlbiall_nsnh_write(CPUARMState *env, const ARMCPRegInfo *ri,
> CPUState *cs = ENV_GET_CPU(env);
>
> tlb_flush_by_mmuidx(cs,
> - (1 << ARMMMUIdx_S12NSE1) |
> - (1 << ARMMMUIdx_S12NSE0) |
> - (1 << ARMMMUIdx_S2NS));
> + ARMMMUIdxBit_S12NSE1 |
> + ARMMMUIdxBit_S12NSE0 |
> + ARMMMUIdxBit_S2NS);
> }
>
> static void tlbiall_nsnh_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -582,9 +582,9 @@ static void tlbiall_nsnh_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
> CPUState *cs = ENV_GET_CPU(env);
>
> tlb_flush_by_mmuidx_all_cpus_synced(cs,
> - (1 << ARMMMUIdx_S12NSE1) |
> - (1 << ARMMMUIdx_S12NSE0) |
> - (1 << ARMMMUIdx_S2NS));
> + ARMMMUIdxBit_S12NSE1 |
> + ARMMMUIdxBit_S12NSE0 |
> + ARMMMUIdxBit_S2NS);
> }
>
> static void tlbiipas2_write(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -605,7 +605,7 @@ static void tlbiipas2_write(CPUARMState *env, const ARMCPRegInfo *ri,
>
> pageaddr = sextract64(value << 12, 0, 40);
>
> - tlb_flush_page_by_mmuidx(cs, pageaddr, (1 << ARMMMUIdx_S2NS));
> + tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdxBit_S2NS);
> }
>
> static void tlbiipas2_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -621,7 +621,7 @@ static void tlbiipas2_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
> pageaddr = sextract64(value << 12, 0, 40);
>
> tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr,
> - (1 << ARMMMUIdx_S2NS));
> + ARMMMUIdxBit_S2NS);
> }
>
> static void tlbiall_hyp_write(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -629,7 +629,7 @@ static void tlbiall_hyp_write(CPUARMState *env, const ARMCPRegInfo *ri,
> {
> CPUState *cs = ENV_GET_CPU(env);
>
> - tlb_flush_by_mmuidx(cs, (1 << ARMMMUIdx_S1E2));
> + tlb_flush_by_mmuidx(cs, ARMMMUIdxBit_S1E2);
> }
>
> static void tlbiall_hyp_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -637,7 +637,7 @@ static void tlbiall_hyp_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
> {
> CPUState *cs = ENV_GET_CPU(env);
>
> - tlb_flush_by_mmuidx_all_cpus_synced(cs, (1 << ARMMMUIdx_S1E2));
> + tlb_flush_by_mmuidx_all_cpus_synced(cs, ARMMMUIdxBit_S1E2);
> }
>
> static void tlbimva_hyp_write(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -646,7 +646,7 @@ static void tlbimva_hyp_write(CPUARMState *env, const ARMCPRegInfo *ri,
> CPUState *cs = ENV_GET_CPU(env);
> uint64_t pageaddr = value & ~MAKE_64BIT_MASK(0, 12);
>
> - tlb_flush_page_by_mmuidx(cs, pageaddr, (1 << ARMMMUIdx_S1E2));
> + tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdxBit_S1E2);
> }
>
> static void tlbimva_hyp_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -656,7 +656,7 @@ static void tlbimva_hyp_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
> uint64_t pageaddr = value & ~MAKE_64BIT_MASK(0, 12);
>
> tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr,
> - (1 << ARMMMUIdx_S1E2));
> + ARMMMUIdxBit_S1E2);
> }
>
> static const ARMCPRegInfo cp_reginfo[] = {
> @@ -2596,9 +2596,9 @@ static void vttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> /* Accesses to VTTBR may change the VMID so we must flush the TLB. */
> if (raw_read(env, ri) != value) {
> tlb_flush_by_mmuidx(cs,
> - (1 << ARMMMUIdx_S12NSE1) |
> - (1 << ARMMMUIdx_S12NSE0) |
> - (1 << ARMMMUIdx_S2NS));
> + ARMMMUIdxBit_S12NSE1 |
> + ARMMMUIdxBit_S12NSE0 |
> + ARMMMUIdxBit_S2NS);
> raw_write(env, ri, value);
> }
> }
> @@ -2957,12 +2957,12 @@ static void tlbi_aa64_vmalle1_write(CPUARMState *env, const ARMCPRegInfo *ri,
>
> if (arm_is_secure_below_el3(env)) {
> tlb_flush_by_mmuidx(cs,
> - (1 << ARMMMUIdx_S1SE1) |
> - (1 << ARMMMUIdx_S1SE0));
> + ARMMMUIdxBit_S1SE1 |
> + ARMMMUIdxBit_S1SE0);
> } else {
> tlb_flush_by_mmuidx(cs,
> - (1 << ARMMMUIdx_S12NSE1) |
> - (1 << ARMMMUIdx_S12NSE0));
> + ARMMMUIdxBit_S12NSE1 |
> + ARMMMUIdxBit_S12NSE0);
> }
> }
>
> @@ -2974,12 +2974,12 @@ static void tlbi_aa64_vmalle1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
>
> if (sec) {
> tlb_flush_by_mmuidx_all_cpus_synced(cs,
> - (1 << ARMMMUIdx_S1SE1) |
> - (1 << ARMMMUIdx_S1SE0));
> + ARMMMUIdxBit_S1SE1 |
> + ARMMMUIdxBit_S1SE0);
> } else {
> tlb_flush_by_mmuidx_all_cpus_synced(cs,
> - (1 << ARMMMUIdx_S12NSE1) |
> - (1 << ARMMMUIdx_S12NSE0));
> + ARMMMUIdxBit_S12NSE1 |
> + ARMMMUIdxBit_S12NSE0);
> }
> }
>
> @@ -2995,18 +2995,18 @@ static void tlbi_aa64_alle1_write(CPUARMState *env, const ARMCPRegInfo *ri,
>
> if (arm_is_secure_below_el3(env)) {
> tlb_flush_by_mmuidx(cs,
> - (1 << ARMMMUIdx_S1SE1) |
> - (1 << ARMMMUIdx_S1SE0));
> + ARMMMUIdxBit_S1SE1 |
> + ARMMMUIdxBit_S1SE0);
> } else {
> if (arm_feature(env, ARM_FEATURE_EL2)) {
> tlb_flush_by_mmuidx(cs,
> - (1 << ARMMMUIdx_S12NSE1) |
> - (1 << ARMMMUIdx_S12NSE0) |
> - (1 << ARMMMUIdx_S2NS));
> + ARMMMUIdxBit_S12NSE1 |
> + ARMMMUIdxBit_S12NSE0 |
> + ARMMMUIdxBit_S2NS);
> } else {
> tlb_flush_by_mmuidx(cs,
> - (1 << ARMMMUIdx_S12NSE1) |
> - (1 << ARMMMUIdx_S12NSE0));
> + ARMMMUIdxBit_S12NSE1 |
> + ARMMMUIdxBit_S12NSE0);
> }
> }
> }
> @@ -3017,7 +3017,7 @@ static void tlbi_aa64_alle2_write(CPUARMState *env, const ARMCPRegInfo *ri,
> ARMCPU *cpu = arm_env_get_cpu(env);
> CPUState *cs = CPU(cpu);
>
> - tlb_flush_by_mmuidx(cs, (1 << ARMMMUIdx_S1E2));
> + tlb_flush_by_mmuidx(cs, ARMMMUIdxBit_S1E2);
> }
>
> static void tlbi_aa64_alle3_write(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -3026,7 +3026,7 @@ static void tlbi_aa64_alle3_write(CPUARMState *env, const ARMCPRegInfo *ri,
> ARMCPU *cpu = arm_env_get_cpu(env);
> CPUState *cs = CPU(cpu);
>
> - tlb_flush_by_mmuidx(cs, (1 << ARMMMUIdx_S1E3));
> + tlb_flush_by_mmuidx(cs, ARMMMUIdxBit_S1E3);
> }
>
> static void tlbi_aa64_alle1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -3042,17 +3042,17 @@ static void tlbi_aa64_alle1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
>
> if (sec) {
> tlb_flush_by_mmuidx_all_cpus_synced(cs,
> - (1 << ARMMMUIdx_S1SE1) |
> - (1 << ARMMMUIdx_S1SE0));
> + ARMMMUIdxBit_S1SE1 |
> + ARMMMUIdxBit_S1SE0);
> } else if (has_el2) {
> tlb_flush_by_mmuidx_all_cpus_synced(cs,
> - (1 << ARMMMUIdx_S12NSE1) |
> - (1 << ARMMMUIdx_S12NSE0) |
> - (1 << ARMMMUIdx_S2NS));
> + ARMMMUIdxBit_S12NSE1 |
> + ARMMMUIdxBit_S12NSE0 |
> + ARMMMUIdxBit_S2NS);
> } else {
> tlb_flush_by_mmuidx_all_cpus_synced(cs,
> - (1 << ARMMMUIdx_S12NSE1) |
> - (1 << ARMMMUIdx_S12NSE0));
> + ARMMMUIdxBit_S12NSE1 |
> + ARMMMUIdxBit_S12NSE0);
> }
> }
>
> @@ -3061,7 +3061,7 @@ static void tlbi_aa64_alle2is_write(CPUARMState *env, const ARMCPRegInfo *ri,
> {
> CPUState *cs = ENV_GET_CPU(env);
>
> - tlb_flush_by_mmuidx_all_cpus_synced(cs, (1 << ARMMMUIdx_S1E2));
> + tlb_flush_by_mmuidx_all_cpus_synced(cs, ARMMMUIdxBit_S1E2);
> }
>
> static void tlbi_aa64_alle3is_write(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -3069,7 +3069,7 @@ static void tlbi_aa64_alle3is_write(CPUARMState *env, const ARMCPRegInfo *ri,
> {
> CPUState *cs = ENV_GET_CPU(env);
>
> - tlb_flush_by_mmuidx_all_cpus_synced(cs, (1 << ARMMMUIdx_S1E3));
> + tlb_flush_by_mmuidx_all_cpus_synced(cs, ARMMMUIdxBit_S1E3);
> }
>
> static void tlbi_aa64_vae1_write(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -3086,12 +3086,12 @@ static void tlbi_aa64_vae1_write(CPUARMState *env, const ARMCPRegInfo *ri,
>
> if (arm_is_secure_below_el3(env)) {
> tlb_flush_page_by_mmuidx(cs, pageaddr,
> - (1 << ARMMMUIdx_S1SE1) |
> - (1 << ARMMMUIdx_S1SE0));
> + ARMMMUIdxBit_S1SE1 |
> + ARMMMUIdxBit_S1SE0);
> } else {
> tlb_flush_page_by_mmuidx(cs, pageaddr,
> - (1 << ARMMMUIdx_S12NSE1) |
> - (1 << ARMMMUIdx_S12NSE0));
> + ARMMMUIdxBit_S12NSE1 |
> + ARMMMUIdxBit_S12NSE0);
> }
> }
>
> @@ -3106,7 +3106,7 @@ static void tlbi_aa64_vae2_write(CPUARMState *env, const ARMCPRegInfo *ri,
> CPUState *cs = CPU(cpu);
> uint64_t pageaddr = sextract64(value << 12, 0, 56);
>
> - tlb_flush_page_by_mmuidx(cs, pageaddr, (1 << ARMMMUIdx_S1E2));
> + tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdxBit_S1E2);
> }
>
> static void tlbi_aa64_vae3_write(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -3120,7 +3120,7 @@ static void tlbi_aa64_vae3_write(CPUARMState *env, const ARMCPRegInfo *ri,
> CPUState *cs = CPU(cpu);
> uint64_t pageaddr = sextract64(value << 12, 0, 56);
>
> - tlb_flush_page_by_mmuidx(cs, pageaddr, (1 << ARMMMUIdx_S1E3));
> + tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdxBit_S1E3);
> }
>
> static void tlbi_aa64_vae1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -3133,12 +3133,12 @@ static void tlbi_aa64_vae1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
>
> if (sec) {
> tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr,
> - (1 << ARMMMUIdx_S1SE1) |
> - (1 << ARMMMUIdx_S1SE0));
> + ARMMMUIdxBit_S1SE1 |
> + ARMMMUIdxBit_S1SE0);
> } else {
> tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr,
> - (1 << ARMMMUIdx_S12NSE1) |
> - (1 << ARMMMUIdx_S12NSE0));
> + ARMMMUIdxBit_S12NSE1 |
> + ARMMMUIdxBit_S12NSE0);
> }
> }
>
> @@ -3149,7 +3149,7 @@ static void tlbi_aa64_vae2is_write(CPUARMState *env, const ARMCPRegInfo *ri,
> uint64_t pageaddr = sextract64(value << 12, 0, 56);
>
> tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr,
> - (1 << ARMMMUIdx_S1E2));
> + ARMMMUIdxBit_S1E2);
> }
>
> static void tlbi_aa64_vae3is_write(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -3159,7 +3159,7 @@ static void tlbi_aa64_vae3is_write(CPUARMState *env, const ARMCPRegInfo *ri,
> uint64_t pageaddr = sextract64(value << 12, 0, 56);
>
> tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr,
> - (1 << ARMMMUIdx_S1E3));
> + ARMMMUIdxBit_S1E3);
> }
>
> static void tlbi_aa64_ipas2e1_write(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -3181,7 +3181,7 @@ static void tlbi_aa64_ipas2e1_write(CPUARMState *env, const ARMCPRegInfo *ri,
>
> pageaddr = sextract64(value << 12, 0, 48);
>
> - tlb_flush_page_by_mmuidx(cs, pageaddr, (1 << ARMMMUIdx_S2NS));
> + tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdxBit_S2NS);
> }
>
> static void tlbi_aa64_ipas2e1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -3197,7 +3197,7 @@ static void tlbi_aa64_ipas2e1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
> pageaddr = sextract64(value << 12, 0, 48);
>
> tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr,
> - (1 << ARMMMUIdx_S2NS));
> + ARMMMUIdxBit_S2NS);
> }
>
> static CPAccessResult aa64_zva_access(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -7049,6 +7049,17 @@ static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
> return &env->cp15.tcr_el[regime_el(env, mmu_idx)];
> }
>
> +/* Convert a possible stage1+2 MMU index into the appropriate
> + * stage 1 MMU index
> + */
> +static inline ARMMMUIdx stage_1_mmu_idx(ARMMMUIdx mmu_idx)
> +{
> + if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
> + mmu_idx += (ARMMMUIdx_S1NSE0 - ARMMMUIdx_S12NSE0);
> + }
> + return mmu_idx;
> +}
> +
> /* Returns TBI0 value for current regime el */
> uint32_t arm_regime_tbi0(CPUARMState *env, ARMMMUIdx mmu_idx)
> {
> @@ -7056,11 +7067,9 @@ uint32_t arm_regime_tbi0(CPUARMState *env, ARMMMUIdx mmu_idx)
> uint32_t el;
>
> /* For EL0 and EL1, TBI is controlled by stage 1's TCR, so convert
> - * a stage 1+2 mmu index into the appropriate stage 1 mmu index.
> - */
> - if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
> - mmu_idx += ARMMMUIdx_S1NSE0;
> - }
> + * a stage 1+2 mmu index into the appropriate stage 1 mmu index.
> + */
> + mmu_idx = stage_1_mmu_idx(mmu_idx);
>
> tcr = regime_tcr(env, mmu_idx);
> el = regime_el(env, mmu_idx);
> @@ -7079,11 +7088,9 @@ uint32_t arm_regime_tbi1(CPUARMState *env, ARMMMUIdx mmu_idx)
> uint32_t el;
>
> /* For EL0 and EL1, TBI is controlled by stage 1's TCR, so convert
> - * a stage 1+2 mmu index into the appropriate stage 1 mmu index.
> - */
> - if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
> - mmu_idx += ARMMMUIdx_S1NSE0;
> - }
> + * a stage 1+2 mmu index into the appropriate stage 1 mmu index.
> + */
> + mmu_idx = stage_1_mmu_idx(mmu_idx);
>
> tcr = regime_tcr(env, mmu_idx);
> el = regime_el(env, mmu_idx);
> @@ -7129,9 +7136,7 @@ static inline bool regime_using_lpae_format(CPUARMState *env,
> * on whether the long or short descriptor format is in use. */
> bool arm_s1_regime_using_lpae_format(CPUARMState *env, ARMMMUIdx mmu_idx)
> {
> - if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
> - mmu_idx += ARMMMUIdx_S1NSE0;
> - }
> + mmu_idx = stage_1_mmu_idx(mmu_idx);
>
> return regime_using_lpae_format(env, mmu_idx);
> }
> @@ -8385,7 +8390,7 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address,
> int ret;
>
> ret = get_phys_addr(env, address, access_type,
> - mmu_idx + ARMMMUIdx_S1NSE0, &ipa, attrs,
> + stage_1_mmu_idx(mmu_idx), &ipa, attrs,
> prot, page_size, fsr, fi);
>
> /* If S1 fails or S2 is disabled, return early. */
> @@ -8406,7 +8411,7 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address,
> /*
> * For non-EL2 CPUs a stage1+stage2 translation is just stage 1.
> */
> - mmu_idx += ARMMMUIdx_S1NSE0;
> + mmu_idx = stage_1_mmu_idx(mmu_idx);
> }
> }
>
> @@ -8482,7 +8487,8 @@ bool arm_tlb_fill(CPUState *cs, vaddr address,
> int ret;
> MemTxAttrs attrs = {};
>
> - ret = get_phys_addr(env, address, access_type, mmu_idx, &phys_addr,
> + ret = get_phys_addr(env, address, access_type,
> + core_to_arm_mmu_idx(env, mmu_idx), &phys_addr,
> &attrs, &prot, &page_size, fsr, fi);
> if (!ret) {
> /* Map a single [sub]page. */
> @@ -8507,10 +8513,11 @@ hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
> bool ret;
> uint32_t fsr;
> ARMMMUFaultInfo fi = {};
> + ARMMMUIdx mmu_idx = core_to_arm_mmu_idx(env, cpu_mmu_index(env, false));
>
> *attrs = (MemTxAttrs) {};
>
> - ret = get_phys_addr(env, addr, 0, cpu_mmu_index(env, false), &phys_addr,
> + ret = get_phys_addr(env, addr, 0, mmu_idx, &phys_addr,
> attrs, &prot, &page_size, &fsr, &fi);
>
> if (ret) {
> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
> index de24815..2a85666 100644
> --- a/target/arm/op_helper.c
> +++ b/target/arm/op_helper.c
> @@ -194,6 +194,7 @@ void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
> int target_el;
> bool same_el;
> uint32_t syn;
> + ARMMMUIdx arm_mmu_idx = core_to_arm_mmu_idx(env, mmu_idx);
>
> if (retaddr) {
> /* now we have a real cpu fault */
> @@ -208,7 +209,7 @@ void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
> /* the DFSR for an alignment fault depends on whether we're using
> * the LPAE long descriptor format, or the short descriptor format
> */
> - if (arm_s1_regime_using_lpae_format(env, mmu_idx)) {
> + if (arm_s1_regime_using_lpae_format(env, arm_mmu_idx)) {
> env->exception.fsr = (1 << 9) | 0x21;
> } else {
> env->exception.fsr = 0x1;
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 24de30d..a82ab49 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -101,21 +101,27 @@ void a64_translate_init(void)
> offsetof(CPUARMState, exclusive_high), "exclusive_high");
> }
>
> -static inline ARMMMUIdx get_a64_user_mem_index(DisasContext *s)
> +static inline int get_a64_user_mem_index(DisasContext *s)
> {
> - /* Return the mmu_idx to use for A64 "unprivileged load/store" insns:
> + /* Return the core mmu_idx to use for A64 "unprivileged load/store" insns:
> * if EL1, access as if EL0; otherwise access at current EL
> */
> + ARMMMUIdx useridx;
> +
> switch (s->mmu_idx) {
> case ARMMMUIdx_S12NSE1:
> - return ARMMMUIdx_S12NSE0;
> + useridx = ARMMMUIdx_S12NSE0;
> + break;
> case ARMMMUIdx_S1SE1:
> - return ARMMMUIdx_S1SE0;
> + useridx = ARMMMUIdx_S1SE0;
> + break;
> case ARMMMUIdx_S2NS:
> g_assert_not_reached();
> default:
> - return s->mmu_idx;
> + useridx = s->mmu_idx;
> + break;
> }
> + return arm_to_core_mmu_idx(useridx);
> }
>
> void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
> @@ -11212,7 +11218,7 @@ void gen_intermediate_code_a64(ARMCPU *cpu, TranslationBlock *tb)
> dc->be_data = ARM_TBFLAG_BE_DATA(tb->flags) ? MO_BE : MO_LE;
> dc->condexec_mask = 0;
> dc->condexec_cond = 0;
> - dc->mmu_idx = ARM_TBFLAG_MMUIDX(tb->flags);
> + dc->mmu_idx = core_to_arm_mmu_idx(env, ARM_TBFLAG_MMUIDX(tb->flags));
> dc->tbi0 = ARM_TBFLAG_TBI0(tb->flags);
> dc->tbi1 = ARM_TBFLAG_TBI1(tb->flags);
> dc->current_el = arm_mmu_idx_to_el(dc->mmu_idx);
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index 0b5a0bc..8d509a2 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -145,9 +145,9 @@ static void disas_set_da_iss(DisasContext *s, TCGMemOp memop, ISSInfo issinfo)
> disas_set_insn_syndrome(s, syn);
> }
>
> -static inline ARMMMUIdx get_a32_user_mem_index(DisasContext *s)
> +static inline int get_a32_user_mem_index(DisasContext *s)
> {
> - /* Return the mmu_idx to use for A32/T32 "unprivileged load/store"
> + /* Return the core mmu_idx to use for A32/T32 "unprivileged load/store"
> * insns:
> * if PL2, UNPREDICTABLE (we choose to implement as if PL0)
> * otherwise, access as if at PL0.
> @@ -156,11 +156,11 @@ static inline ARMMMUIdx get_a32_user_mem_index(DisasContext *s)
> case ARMMMUIdx_S1E2: /* this one is UNPREDICTABLE */
> case ARMMMUIdx_S12NSE0:
> case ARMMMUIdx_S12NSE1:
> - return ARMMMUIdx_S12NSE0;
> + return arm_to_core_mmu_idx(ARMMMUIdx_S12NSE0);
> case ARMMMUIdx_S1E3:
> case ARMMMUIdx_S1SE0:
> case ARMMMUIdx_S1SE1:
> - return ARMMMUIdx_S1SE0;
> + return arm_to_core_mmu_idx(ARMMMUIdx_S1SE0);
> case ARMMMUIdx_S2NS:
> default:
> g_assert_not_reached();
> @@ -11816,7 +11816,7 @@ void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb)
> dc->be_data = ARM_TBFLAG_BE_DATA(tb->flags) ? MO_BE : MO_LE;
> dc->condexec_mask = (ARM_TBFLAG_CONDEXEC(tb->flags) & 0xf) << 1;
> dc->condexec_cond = ARM_TBFLAG_CONDEXEC(tb->flags) >> 4;
> - dc->mmu_idx = ARM_TBFLAG_MMUIDX(tb->flags);
> + dc->mmu_idx = core_to_arm_mmu_idx(env, ARM_TBFLAG_MMUIDX(tb->flags));
> dc->current_el = arm_mmu_idx_to_el(dc->mmu_idx);
> #if !defined(CONFIG_USER_ONLY)
> dc->user = (dc->current_el == 0);
> --
> 2.7.4
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 02/13] arm: Add support for M profile CPUs having different MMU index semantics
2017-05-02 22:23 ` Alistair Francis
@ 2017-05-30 13:56 ` Peter Maydell
0 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2017-05-30 13:56 UTC (permalink / raw)
To: Alistair Francis
Cc: qemu-arm, qemu-devel@nongnu.org Developers, Alex Bennée,
Patch Tracking
On 2 May 2017 at 23:23, Alistair Francis <alistair.francis@xilinx.com> wrote:
> On Tue, Apr 25, 2017 at 5:06 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> The M profile CPU's MPU has an awkward corner case which we
>> would like to implement with a different MMU index.
>>
>> We can avoid having to bump the number of MMU modes ARM
>> uses, because some of our existing MMU indexes are only
>> used by non-M-profile CPUs, so we can borrow one.
>> To avoid that getting too confusing, clean up the code
>> to try to keep the two meanings of the index separate.
>>
>> Instead of ARMMMUIdx enum values being identical to core QEMU
>> MMU index values, they are now the core index values with some
>> high bits set. Any particular CPU always uses the same high
>> bits (so eventually A profile cores and M profile cores will
>> use different bits). New functions arm_to_core_mmu_idx()
>> and core_to_arm_mmu_idx() convert between the two.
>>
>> In general core index values are stored in 'int' types, and
>> ARM values are stored in ARMMMUIdx types.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
> This looks ok to me. I'm a little worried it will make SMMU or some
> ARMv9 (or whatever the next thing is) work more complicated as we have
> this.
Mmm, that's always a risk, but I think it will be pretty
easy to undo this if we need to.
thanks
-- PMM
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 03/13] arm: Use different ARMMMUIdx values for M profile
2017-04-25 12:06 [Qemu-devel] [PATCH 00/13] armv7m: Implement MPU support Peter Maydell
2017-04-25 12:06 ` [Qemu-devel] [PATCH 01/13] arm: Use the mmu_idx we're passed in arm_cpu_do_unaligned_access() Peter Maydell
2017-04-25 12:06 ` [Qemu-devel] [PATCH 02/13] arm: Add support for M profile CPUs having different MMU index semantics Peter Maydell
@ 2017-04-25 12:07 ` Peter Maydell
2017-04-25 12:07 ` [Qemu-devel] [PATCH 04/13] arm: Clean up handling of no-MPU PMSA CPUs Peter Maydell
` (10 subsequent siblings)
13 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2017-04-25 12:07 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée, Alistair Francis
Make M profile use completely separate ARMMMUIdx values from
those that A profile CPUs use. This is a prelude to adding
support for the MPU and for v8M, which together will require
6 MMU indexes which don't map cleanly onto the A profile
uses:
non secure User
non secure Privileged
non secure Privileged, execution priority < 0
secure User
secure Privileged
secure Privileged, execution priority < 0
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/cpu.h | 21 +++++++++++++++++++--
target/arm/helper.c | 5 +++++
target/arm/translate.c | 3 +++
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index e1f4856..253565b 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2055,8 +2055,9 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
* of the AT/ATS operations.
* The values used are carefully arranged to make mmu_idx => EL lookup easy.
*/
-#define ARM_MMU_IDX_A 0x10 /* A profile (and M profile, for the moment) */
+#define ARM_MMU_IDX_A 0x10 /* A profile */
#define ARM_MMU_IDX_NOTLB 0x20 /* does not have a TLB */
+#define ARM_MMU_IDX_M 0x40 /* M profile */
#define ARM_MMU_IDX_TYPE_MASK (~0x7)
#define ARM_MMU_IDX_COREIDX_MASK 0x7
@@ -2069,6 +2070,8 @@ typedef enum ARMMMUIdx {
ARMMMUIdx_S1SE0 = 4 | ARM_MMU_IDX_A,
ARMMMUIdx_S1SE1 = 5 | ARM_MMU_IDX_A,
ARMMMUIdx_S2NS = 6 | ARM_MMU_IDX_A,
+ ARMMMUIdx_MUser = 0 | ARM_MMU_IDX_M,
+ ARMMMUIdx_MPriv = 1 | ARM_MMU_IDX_M,
/* Indexes below here don't have TLBs and are used only for AT system
* instructions or for the first stage of an S12 page table walk.
*/
@@ -2087,6 +2090,8 @@ typedef enum ARMMMUIdxBit {
ARMMMUIdxBit_S1SE0 = 1 << 4,
ARMMMUIdxBit_S1SE1 = 1 << 5,
ARMMMUIdxBit_S2NS = 1 << 6,
+ ARMMMUIdxBit_MUser = 1 << 0,
+ ARMMMUIdxBit_MPriv = 1 << 1,
} ARMMMUIdxBit;
#define MMU_USER_IDX 0
@@ -2098,7 +2103,11 @@ static inline int arm_to_core_mmu_idx(ARMMMUIdx mmu_idx)
static inline ARMMMUIdx core_to_arm_mmu_idx(CPUARMState *env, int mmu_idx)
{
- return mmu_idx | ARM_MMU_IDX_A;
+ if (arm_feature(env, ARM_FEATURE_M)) {
+ return mmu_idx | ARM_MMU_IDX_M;
+ } else {
+ return mmu_idx | ARM_MMU_IDX_A;
+ }
}
/* Return the exception level we're running at if this is our mmu_idx */
@@ -2107,6 +2116,8 @@ static inline int arm_mmu_idx_to_el(ARMMMUIdx mmu_idx)
switch (mmu_idx & ARM_MMU_IDX_TYPE_MASK) {
case ARM_MMU_IDX_A:
return mmu_idx & 3;
+ case ARM_MMU_IDX_M:
+ return mmu_idx & 1;
default:
g_assert_not_reached();
}
@@ -2117,6 +2128,12 @@ static inline int cpu_mmu_index(CPUARMState *env, bool ifetch)
{
int el = arm_current_el(env);
+ if (arm_feature(env, ARM_FEATURE_M)) {
+ ARMMMUIdx mmu_idx = el == 0 ? ARMMMUIdx_MUser : ARMMMUIdx_MPriv;
+
+ return arm_to_core_mmu_idx(mmu_idx);
+ }
+
if (el < 2 && arm_is_secure_below_el3(env)) {
return arm_to_core_mmu_idx(ARMMMUIdx_S1SE0 + el);
}
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 520adcc..791332c 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6992,6 +6992,8 @@ static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx)
case ARMMMUIdx_S1SE1:
case ARMMMUIdx_S1NSE0:
case ARMMMUIdx_S1NSE1:
+ case ARMMMUIdx_MPriv:
+ case ARMMMUIdx_MUser:
return 1;
default:
g_assert_not_reached();
@@ -7008,6 +7010,8 @@ static inline bool regime_is_secure(CPUARMState *env, ARMMMUIdx mmu_idx)
case ARMMMUIdx_S1NSE1:
case ARMMMUIdx_S1E2:
case ARMMMUIdx_S2NS:
+ case ARMMMUIdx_MPriv:
+ case ARMMMUIdx_MUser:
return false;
case ARMMMUIdx_S1E3:
case ARMMMUIdx_S1SE0:
@@ -7146,6 +7150,7 @@ static inline bool regime_is_user(CPUARMState *env, ARMMMUIdx mmu_idx)
switch (mmu_idx) {
case ARMMMUIdx_S1SE0:
case ARMMMUIdx_S1NSE0:
+ case ARMMMUIdx_MUser:
return true;
default:
return false;
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 8d509a2..ac905dd 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -161,6 +161,9 @@ static inline int get_a32_user_mem_index(DisasContext *s)
case ARMMMUIdx_S1SE0:
case ARMMMUIdx_S1SE1:
return arm_to_core_mmu_idx(ARMMMUIdx_S1SE0);
+ case ARMMMUIdx_MUser:
+ case ARMMMUIdx_MPriv:
+ return arm_to_core_mmu_idx(ARMMMUIdx_MUser);
case ARMMMUIdx_S2NS:
default:
g_assert_not_reached();
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH 04/13] arm: Clean up handling of no-MPU PMSA CPUs
2017-04-25 12:06 [Qemu-devel] [PATCH 00/13] armv7m: Implement MPU support Peter Maydell
` (2 preceding siblings ...)
2017-04-25 12:07 ` [Qemu-devel] [PATCH 03/13] arm: Use different ARMMMUIdx values for M profile Peter Maydell
@ 2017-04-25 12:07 ` Peter Maydell
2017-05-02 22:24 ` Alistair Francis
2017-05-13 22:35 ` Philippe Mathieu-Daudé
2017-04-25 12:07 ` [Qemu-devel] [PATCH 05/13] arm: Don't clear ARM_FEATURE_PMSA for no-mpu configs Peter Maydell
` (9 subsequent siblings)
13 siblings, 2 replies; 36+ messages in thread
From: Peter Maydell @ 2017-04-25 12:07 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée, Alistair Francis
ARM CPUs come in two flavours:
* proper MMU ("VMSA")
* only an MPU ("PMSA")
For PMSA, the MPU may be implemented, or not (in which case there
is default "always acts the same" behaviour, but it isn't guest
programmable).
QEMU is a bit confused about how we indicate this: we have an
ARM_FEATURE_MPU, but it's not clear whether this indicates
"PMSA, not VMSA" or "PMSA and MPU present" , and sometimes we
use it for one purpose and sometimes the other.
Currently trying to implement a PMSA-without-MPU core won't
work correctly because we turn off the ARM_FEATURE_MPU bit
and then a lot of things which should still exist get
turned off too.
As the first step in cleaning this up, rename the feature
bit to ARM_FEATURE_PMSA, which indicates a PMSA CPU (with
or without MPU).
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/cpu.h | 2 +-
target/arm/cpu.c | 12 ++++++------
target/arm/helper.c | 12 ++++++------
target/arm/machine.c | 2 +-
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 253565b..0718955 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1179,7 +1179,7 @@ enum arm_features {
ARM_FEATURE_V6K,
ARM_FEATURE_V7,
ARM_FEATURE_THUMB2,
- ARM_FEATURE_MPU, /* Only has Memory Protection Unit, not full MMU. */
+ ARM_FEATURE_PMSA, /* no MMU; may have Memory Protection Unit */
ARM_FEATURE_VFP3,
ARM_FEATURE_VFP_FP16,
ARM_FEATURE_NEON,
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index b357aee..f17e279 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -586,7 +586,7 @@ static void arm_cpu_post_init(Object *obj)
&error_abort);
}
- if (arm_feature(&cpu->env, ARM_FEATURE_MPU)) {
+ if (arm_feature(&cpu->env, ARM_FEATURE_PMSA)) {
qdev_property_add_static(DEVICE(obj), &arm_cpu_has_mpu_property,
&error_abort);
if (arm_feature(&cpu->env, ARM_FEATURE_V7)) {
@@ -682,7 +682,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
if (arm_feature(env, ARM_FEATURE_V7) &&
!arm_feature(env, ARM_FEATURE_M) &&
- !arm_feature(env, ARM_FEATURE_MPU)) {
+ !arm_feature(env, ARM_FEATURE_PMSA)) {
/* v7VMSA drops support for the old ARMv5 tiny pages, so we
* can use 4K pages.
*/
@@ -758,10 +758,10 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
}
if (!cpu->has_mpu) {
- unset_feature(env, ARM_FEATURE_MPU);
+ unset_feature(ARM_FEATURE_PMSA);
}
- if (arm_feature(env, ARM_FEATURE_MPU) &&
+ if (arm_feature(env, ARM_FEATURE_PMSA) &&
arm_feature(env, ARM_FEATURE_V7)) {
uint32_t nr = cpu->pmsav7_dregion;
@@ -861,7 +861,7 @@ static void arm946_initfn(Object *obj)
cpu->dtb_compatible = "arm,arm946";
set_feature(&cpu->env, ARM_FEATURE_V5);
- set_feature(&cpu->env, ARM_FEATURE_MPU);
+ set_feature(&cpu->env, ARM_FEATURE_PMSA);
set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
cpu->midr = 0x41059461;
cpu->ctr = 0x0f004006;
@@ -1073,7 +1073,7 @@ static void cortex_r5_initfn(Object *obj)
set_feature(&cpu->env, ARM_FEATURE_THUMB_DIV);
set_feature(&cpu->env, ARM_FEATURE_ARM_DIV);
set_feature(&cpu->env, ARM_FEATURE_V7MP);
- set_feature(&cpu->env, ARM_FEATURE_MPU);
+ set_feature(&cpu->env, ARM_FEATURE_PMSA);
cpu->midr = 0x411fc153; /* r1p3 */
cpu->id_pfr0 = 0x0131;
cpu->id_pfr1 = 0x001;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 791332c..404bfdb 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -485,7 +485,7 @@ static void contextidr_write(CPUARMState *env, const ARMCPRegInfo *ri,
{
ARMCPU *cpu = arm_env_get_cpu(env);
- if (raw_read(env, ri) != value && !arm_feature(env, ARM_FEATURE_MPU)
+ if (raw_read(env, ri) != value && !arm_feature(env, ARM_FEATURE_PMSA)
&& !extended_addresses_enabled(env)) {
/* For VMSA (when not using the LPAE long descriptor page table
* format) this register includes the ASID, so do a TLB flush.
@@ -4615,7 +4615,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
define_arm_cp_regs(cpu, v6k_cp_reginfo);
}
if (arm_feature(env, ARM_FEATURE_V7MP) &&
- !arm_feature(env, ARM_FEATURE_MPU)) {
+ !arm_feature(env, ARM_FEATURE_PMSA)) {
define_arm_cp_regs(cpu, v7mp_cp_reginfo);
}
if (arm_feature(env, ARM_FEATURE_V7)) {
@@ -4969,7 +4969,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
}
}
- if (arm_feature(env, ARM_FEATURE_MPU)) {
+ if (arm_feature(env, ARM_FEATURE_PMSA)) {
if (arm_feature(env, ARM_FEATURE_V6)) {
/* PMSAv6 not implemented */
assert(arm_feature(env, ARM_FEATURE_V7));
@@ -5131,7 +5131,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
define_arm_cp_regs(cpu, id_pre_v8_midr_cp_reginfo);
}
define_arm_cp_regs(cpu, id_cp_reginfo);
- if (!arm_feature(env, ARM_FEATURE_MPU)) {
+ if (!arm_feature(env, ARM_FEATURE_PMSA)) {
define_one_arm_cp_reg(cpu, &id_tlbtr_reginfo);
} else if (arm_feature(env, ARM_FEATURE_V7)) {
define_one_arm_cp_reg(cpu, &id_mpuir_reginfo);
@@ -8442,7 +8442,7 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address,
/* pmsav7 has special handling for when MPU is disabled so call it before
* the common MMU/MPU disabled check below.
*/
- if (arm_feature(env, ARM_FEATURE_MPU) &&
+ if (arm_feature(env, ARM_FEATURE_PMSA) &&
arm_feature(env, ARM_FEATURE_V7)) {
*page_size = TARGET_PAGE_SIZE;
return get_phys_addr_pmsav7(env, address, access_type, mmu_idx,
@@ -8457,7 +8457,7 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address,
return 0;
}
- if (arm_feature(env, ARM_FEATURE_MPU)) {
+ if (arm_feature(env, ARM_FEATURE_PMSA)) {
/* Pre-v7 MPU */
*page_size = TARGET_PAGE_SIZE;
return get_phys_addr_pmsav5(env, address, access_type, mmu_idx,
diff --git a/target/arm/machine.c b/target/arm/machine.c
index d8094a8..ac6b758 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -142,7 +142,7 @@ static bool pmsav7_needed(void *opaque)
ARMCPU *cpu = opaque;
CPUARMState *env = &cpu->env;
- return arm_feature(env, ARM_FEATURE_MPU) &&
+ return arm_feature(env, ARM_FEATURE_PMSA) &&
arm_feature(env, ARM_FEATURE_V7);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 04/13] arm: Clean up handling of no-MPU PMSA CPUs
2017-04-25 12:07 ` [Qemu-devel] [PATCH 04/13] arm: Clean up handling of no-MPU PMSA CPUs Peter Maydell
@ 2017-05-02 22:24 ` Alistair Francis
2017-05-13 22:35 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 36+ messages in thread
From: Alistair Francis @ 2017-05-02 22:24 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, qemu-devel@nongnu.org Developers, Alistair Francis,
Alex Bennée, Patch Tracking
On Tue, Apr 25, 2017 at 5:07 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> ARM CPUs come in two flavours:
> * proper MMU ("VMSA")
> * only an MPU ("PMSA")
> For PMSA, the MPU may be implemented, or not (in which case there
> is default "always acts the same" behaviour, but it isn't guest
> programmable).
>
> QEMU is a bit confused about how we indicate this: we have an
> ARM_FEATURE_MPU, but it's not clear whether this indicates
> "PMSA, not VMSA" or "PMSA and MPU present" , and sometimes we
> use it for one purpose and sometimes the other.
>
> Currently trying to implement a PMSA-without-MPU core won't
> work correctly because we turn off the ARM_FEATURE_MPU bit
> and then a lot of things which should still exist get
> turned off too.
>
> As the first step in cleaning this up, rename the feature
> bit to ARM_FEATURE_PMSA, which indicates a PMSA CPU (with
> or without MPU).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
Thanks,
Alistair
> ---
> target/arm/cpu.h | 2 +-
> target/arm/cpu.c | 12 ++++++------
> target/arm/helper.c | 12 ++++++------
> target/arm/machine.c | 2 +-
> 4 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 253565b..0718955 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -1179,7 +1179,7 @@ enum arm_features {
> ARM_FEATURE_V6K,
> ARM_FEATURE_V7,
> ARM_FEATURE_THUMB2,
> - ARM_FEATURE_MPU, /* Only has Memory Protection Unit, not full MMU. */
> + ARM_FEATURE_PMSA, /* no MMU; may have Memory Protection Unit */
> ARM_FEATURE_VFP3,
> ARM_FEATURE_VFP_FP16,
> ARM_FEATURE_NEON,
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index b357aee..f17e279 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -586,7 +586,7 @@ static void arm_cpu_post_init(Object *obj)
> &error_abort);
> }
>
> - if (arm_feature(&cpu->env, ARM_FEATURE_MPU)) {
> + if (arm_feature(&cpu->env, ARM_FEATURE_PMSA)) {
> qdev_property_add_static(DEVICE(obj), &arm_cpu_has_mpu_property,
> &error_abort);
> if (arm_feature(&cpu->env, ARM_FEATURE_V7)) {
> @@ -682,7 +682,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>
> if (arm_feature(env, ARM_FEATURE_V7) &&
> !arm_feature(env, ARM_FEATURE_M) &&
> - !arm_feature(env, ARM_FEATURE_MPU)) {
> + !arm_feature(env, ARM_FEATURE_PMSA)) {
> /* v7VMSA drops support for the old ARMv5 tiny pages, so we
> * can use 4K pages.
> */
> @@ -758,10 +758,10 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
> }
>
> if (!cpu->has_mpu) {
> - unset_feature(env, ARM_FEATURE_MPU);
> + unset_feature(ARM_FEATURE_PMSA);
> }
>
> - if (arm_feature(env, ARM_FEATURE_MPU) &&
> + if (arm_feature(env, ARM_FEATURE_PMSA) &&
> arm_feature(env, ARM_FEATURE_V7)) {
> uint32_t nr = cpu->pmsav7_dregion;
>
> @@ -861,7 +861,7 @@ static void arm946_initfn(Object *obj)
>
> cpu->dtb_compatible = "arm,arm946";
> set_feature(&cpu->env, ARM_FEATURE_V5);
> - set_feature(&cpu->env, ARM_FEATURE_MPU);
> + set_feature(&cpu->env, ARM_FEATURE_PMSA);
> set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
> cpu->midr = 0x41059461;
> cpu->ctr = 0x0f004006;
> @@ -1073,7 +1073,7 @@ static void cortex_r5_initfn(Object *obj)
> set_feature(&cpu->env, ARM_FEATURE_THUMB_DIV);
> set_feature(&cpu->env, ARM_FEATURE_ARM_DIV);
> set_feature(&cpu->env, ARM_FEATURE_V7MP);
> - set_feature(&cpu->env, ARM_FEATURE_MPU);
> + set_feature(&cpu->env, ARM_FEATURE_PMSA);
> cpu->midr = 0x411fc153; /* r1p3 */
> cpu->id_pfr0 = 0x0131;
> cpu->id_pfr1 = 0x001;
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 791332c..404bfdb 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -485,7 +485,7 @@ static void contextidr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> {
> ARMCPU *cpu = arm_env_get_cpu(env);
>
> - if (raw_read(env, ri) != value && !arm_feature(env, ARM_FEATURE_MPU)
> + if (raw_read(env, ri) != value && !arm_feature(env, ARM_FEATURE_PMSA)
> && !extended_addresses_enabled(env)) {
> /* For VMSA (when not using the LPAE long descriptor page table
> * format) this register includes the ASID, so do a TLB flush.
> @@ -4615,7 +4615,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> define_arm_cp_regs(cpu, v6k_cp_reginfo);
> }
> if (arm_feature(env, ARM_FEATURE_V7MP) &&
> - !arm_feature(env, ARM_FEATURE_MPU)) {
> + !arm_feature(env, ARM_FEATURE_PMSA)) {
> define_arm_cp_regs(cpu, v7mp_cp_reginfo);
> }
> if (arm_feature(env, ARM_FEATURE_V7)) {
> @@ -4969,7 +4969,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> }
> }
>
> - if (arm_feature(env, ARM_FEATURE_MPU)) {
> + if (arm_feature(env, ARM_FEATURE_PMSA)) {
> if (arm_feature(env, ARM_FEATURE_V6)) {
> /* PMSAv6 not implemented */
> assert(arm_feature(env, ARM_FEATURE_V7));
> @@ -5131,7 +5131,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> define_arm_cp_regs(cpu, id_pre_v8_midr_cp_reginfo);
> }
> define_arm_cp_regs(cpu, id_cp_reginfo);
> - if (!arm_feature(env, ARM_FEATURE_MPU)) {
> + if (!arm_feature(env, ARM_FEATURE_PMSA)) {
> define_one_arm_cp_reg(cpu, &id_tlbtr_reginfo);
> } else if (arm_feature(env, ARM_FEATURE_V7)) {
> define_one_arm_cp_reg(cpu, &id_mpuir_reginfo);
> @@ -8442,7 +8442,7 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address,
> /* pmsav7 has special handling for when MPU is disabled so call it before
> * the common MMU/MPU disabled check below.
> */
> - if (arm_feature(env, ARM_FEATURE_MPU) &&
> + if (arm_feature(env, ARM_FEATURE_PMSA) &&
> arm_feature(env, ARM_FEATURE_V7)) {
> *page_size = TARGET_PAGE_SIZE;
> return get_phys_addr_pmsav7(env, address, access_type, mmu_idx,
> @@ -8457,7 +8457,7 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address,
> return 0;
> }
>
> - if (arm_feature(env, ARM_FEATURE_MPU)) {
> + if (arm_feature(env, ARM_FEATURE_PMSA)) {
> /* Pre-v7 MPU */
> *page_size = TARGET_PAGE_SIZE;
> return get_phys_addr_pmsav5(env, address, access_type, mmu_idx,
> diff --git a/target/arm/machine.c b/target/arm/machine.c
> index d8094a8..ac6b758 100644
> --- a/target/arm/machine.c
> +++ b/target/arm/machine.c
> @@ -142,7 +142,7 @@ static bool pmsav7_needed(void *opaque)
> ARMCPU *cpu = opaque;
> CPUARMState *env = &cpu->env;
>
> - return arm_feature(env, ARM_FEATURE_MPU) &&
> + return arm_feature(env, ARM_FEATURE_PMSA) &&
> arm_feature(env, ARM_FEATURE_V7);
> }
>
> --
> 2.7.4
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 04/13] arm: Clean up handling of no-MPU PMSA CPUs
2017-04-25 12:07 ` [Qemu-devel] [PATCH 04/13] arm: Clean up handling of no-MPU PMSA CPUs Peter Maydell
2017-05-02 22:24 ` Alistair Francis
@ 2017-05-13 22:35 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-05-13 22:35 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel
Cc: Alistair Francis, Alex Bennée, patches
Hi Peter,
On 04/25/2017 09:07 AM, Peter Maydell wrote:
> ARM CPUs come in two flavours:
> * proper MMU ("VMSA")
> * only an MPU ("PMSA")
> For PMSA, the MPU may be implemented, or not (in which case there
> is default "always acts the same" behaviour, but it isn't guest
> programmable).
>
> QEMU is a bit confused about how we indicate this: we have an
> ARM_FEATURE_MPU, but it's not clear whether this indicates
> "PMSA, not VMSA" or "PMSA and MPU present" , and sometimes we
> use it for one purpose and sometimes the other.
>
> Currently trying to implement a PMSA-without-MPU core won't
> work correctly because we turn off the ARM_FEATURE_MPU bit
> and then a lot of things which should still exist get
> turned off too.
>
> As the first step in cleaning this up, rename the feature
> bit to ARM_FEATURE_PMSA, which indicates a PMSA CPU (with
> or without MPU).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> target/arm/cpu.h | 2 +-
> target/arm/cpu.c | 12 ++++++------
> target/arm/helper.c | 12 ++++++------
> target/arm/machine.c | 2 +-
> 4 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 253565b..0718955 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -1179,7 +1179,7 @@ enum arm_features {
> ARM_FEATURE_V6K,
> ARM_FEATURE_V7,
> ARM_FEATURE_THUMB2,
> - ARM_FEATURE_MPU, /* Only has Memory Protection Unit, not full MMU. */
> + ARM_FEATURE_PMSA, /* no MMU; may have Memory Protection Unit */
> ARM_FEATURE_VFP3,
> ARM_FEATURE_VFP_FP16,
> ARM_FEATURE_NEON,
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index b357aee..f17e279 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -586,7 +586,7 @@ static void arm_cpu_post_init(Object *obj)
> &error_abort);
> }
>
> - if (arm_feature(&cpu->env, ARM_FEATURE_MPU)) {
> + if (arm_feature(&cpu->env, ARM_FEATURE_PMSA)) {
> qdev_property_add_static(DEVICE(obj), &arm_cpu_has_mpu_property,
> &error_abort);
> if (arm_feature(&cpu->env, ARM_FEATURE_V7)) {
> @@ -682,7 +682,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>
> if (arm_feature(env, ARM_FEATURE_V7) &&
> !arm_feature(env, ARM_FEATURE_M) &&
> - !arm_feature(env, ARM_FEATURE_MPU)) {
> + !arm_feature(env, ARM_FEATURE_PMSA)) {
> /* v7VMSA drops support for the old ARMv5 tiny pages, so we
> * can use 4K pages.
> */
> @@ -758,10 +758,10 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
> }
>
> if (!cpu->has_mpu) {
> - unset_feature(env, ARM_FEATURE_MPU);
> + unset_feature(ARM_FEATURE_PMSA);
With the missing 'env' argument:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> }
>
> - if (arm_feature(env, ARM_FEATURE_MPU) &&
> + if (arm_feature(env, ARM_FEATURE_PMSA) &&
> arm_feature(env, ARM_FEATURE_V7)) {
> uint32_t nr = cpu->pmsav7_dregion;
>
> @@ -861,7 +861,7 @@ static void arm946_initfn(Object *obj)
>
> cpu->dtb_compatible = "arm,arm946";
> set_feature(&cpu->env, ARM_FEATURE_V5);
> - set_feature(&cpu->env, ARM_FEATURE_MPU);
> + set_feature(&cpu->env, ARM_FEATURE_PMSA);
> set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
> cpu->midr = 0x41059461;
> cpu->ctr = 0x0f004006;
> @@ -1073,7 +1073,7 @@ static void cortex_r5_initfn(Object *obj)
> set_feature(&cpu->env, ARM_FEATURE_THUMB_DIV);
> set_feature(&cpu->env, ARM_FEATURE_ARM_DIV);
> set_feature(&cpu->env, ARM_FEATURE_V7MP);
> - set_feature(&cpu->env, ARM_FEATURE_MPU);
> + set_feature(&cpu->env, ARM_FEATURE_PMSA);
> cpu->midr = 0x411fc153; /* r1p3 */
> cpu->id_pfr0 = 0x0131;
> cpu->id_pfr1 = 0x001;
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 791332c..404bfdb 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -485,7 +485,7 @@ static void contextidr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> {
> ARMCPU *cpu = arm_env_get_cpu(env);
>
> - if (raw_read(env, ri) != value && !arm_feature(env, ARM_FEATURE_MPU)
> + if (raw_read(env, ri) != value && !arm_feature(env, ARM_FEATURE_PMSA)
> && !extended_addresses_enabled(env)) {
> /* For VMSA (when not using the LPAE long descriptor page table
> * format) this register includes the ASID, so do a TLB flush.
> @@ -4615,7 +4615,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> define_arm_cp_regs(cpu, v6k_cp_reginfo);
> }
> if (arm_feature(env, ARM_FEATURE_V7MP) &&
> - !arm_feature(env, ARM_FEATURE_MPU)) {
> + !arm_feature(env, ARM_FEATURE_PMSA)) {
> define_arm_cp_regs(cpu, v7mp_cp_reginfo);
> }
> if (arm_feature(env, ARM_FEATURE_V7)) {
> @@ -4969,7 +4969,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> }
> }
>
> - if (arm_feature(env, ARM_FEATURE_MPU)) {
> + if (arm_feature(env, ARM_FEATURE_PMSA)) {
> if (arm_feature(env, ARM_FEATURE_V6)) {
> /* PMSAv6 not implemented */
> assert(arm_feature(env, ARM_FEATURE_V7));
> @@ -5131,7 +5131,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> define_arm_cp_regs(cpu, id_pre_v8_midr_cp_reginfo);
> }
> define_arm_cp_regs(cpu, id_cp_reginfo);
> - if (!arm_feature(env, ARM_FEATURE_MPU)) {
> + if (!arm_feature(env, ARM_FEATURE_PMSA)) {
> define_one_arm_cp_reg(cpu, &id_tlbtr_reginfo);
> } else if (arm_feature(env, ARM_FEATURE_V7)) {
> define_one_arm_cp_reg(cpu, &id_mpuir_reginfo);
> @@ -8442,7 +8442,7 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address,
> /* pmsav7 has special handling for when MPU is disabled so call it before
> * the common MMU/MPU disabled check below.
> */
> - if (arm_feature(env, ARM_FEATURE_MPU) &&
> + if (arm_feature(env, ARM_FEATURE_PMSA) &&
> arm_feature(env, ARM_FEATURE_V7)) {
> *page_size = TARGET_PAGE_SIZE;
> return get_phys_addr_pmsav7(env, address, access_type, mmu_idx,
> @@ -8457,7 +8457,7 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address,
> return 0;
> }
>
> - if (arm_feature(env, ARM_FEATURE_MPU)) {
> + if (arm_feature(env, ARM_FEATURE_PMSA)) {
> /* Pre-v7 MPU */
> *page_size = TARGET_PAGE_SIZE;
> return get_phys_addr_pmsav5(env, address, access_type, mmu_idx,
> diff --git a/target/arm/machine.c b/target/arm/machine.c
> index d8094a8..ac6b758 100644
> --- a/target/arm/machine.c
> +++ b/target/arm/machine.c
> @@ -142,7 +142,7 @@ static bool pmsav7_needed(void *opaque)
> ARMCPU *cpu = opaque;
> CPUARMState *env = &cpu->env;
>
> - return arm_feature(env, ARM_FEATURE_MPU) &&
> + return arm_feature(env, ARM_FEATURE_PMSA) &&
> arm_feature(env, ARM_FEATURE_V7);
> }
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 05/13] arm: Don't clear ARM_FEATURE_PMSA for no-mpu configs
2017-04-25 12:06 [Qemu-devel] [PATCH 00/13] armv7m: Implement MPU support Peter Maydell
` (3 preceding siblings ...)
2017-04-25 12:07 ` [Qemu-devel] [PATCH 04/13] arm: Clean up handling of no-MPU PMSA CPUs Peter Maydell
@ 2017-04-25 12:07 ` Peter Maydell
2017-05-02 22:24 ` Alistair Francis
2017-05-13 22:37 ` Philippe Mathieu-Daudé
2017-04-25 12:07 ` [Qemu-devel] [PATCH 06/13] arm: Don't let no-MPU PMSA cores write to SCTLR.M Peter Maydell
` (8 subsequent siblings)
13 siblings, 2 replies; 36+ messages in thread
From: Peter Maydell @ 2017-04-25 12:07 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée, Alistair Francis
Fix the handling of QOM properties for PMSA CPUs with no MPU:
Allow no-MPU to be specified by either:
* has-mpu = false
* pmsav7_dregion = 0
and make setting one imply the other. Don't clear the PMSA
feature bit in this situation.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/cpu.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index f17e279..8e57498 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -757,8 +757,14 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
cpu->id_pfr1 &= ~0xf000;
}
+ /* MPU can be configured out of a PMSA CPU either by setting has-mpu
+ * to false or by setting pmsav7-dregion to 0.
+ */
if (!cpu->has_mpu) {
- unset_feature(ARM_FEATURE_PMSA);
+ cpu->pmsav7_dregion = 0;
+ }
+ if (cpu->pmsav7_dregion == 0) {
+ cpu->has_mpu = false;
}
if (arm_feature(env, ARM_FEATURE_PMSA) &&
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 05/13] arm: Don't clear ARM_FEATURE_PMSA for no-mpu configs
2017-04-25 12:07 ` [Qemu-devel] [PATCH 05/13] arm: Don't clear ARM_FEATURE_PMSA for no-mpu configs Peter Maydell
@ 2017-05-02 22:24 ` Alistair Francis
2017-05-13 22:37 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 36+ messages in thread
From: Alistair Francis @ 2017-05-02 22:24 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, qemu-devel@nongnu.org Developers, Alistair Francis,
Alex Bennée, Patch Tracking
On Tue, Apr 25, 2017 at 5:07 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> Fix the handling of QOM properties for PMSA CPUs with no MPU:
>
> Allow no-MPU to be specified by either:
> * has-mpu = false
> * pmsav7_dregion = 0
> and make setting one imply the other. Don't clear the PMSA
> feature bit in this situation.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
Thanks,
Alistair
> ---
> target/arm/cpu.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index f17e279..8e57498 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -757,8 +757,14 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
> cpu->id_pfr1 &= ~0xf000;
> }
>
> + /* MPU can be configured out of a PMSA CPU either by setting has-mpu
> + * to false or by setting pmsav7-dregion to 0.
> + */
> if (!cpu->has_mpu) {
> - unset_feature(ARM_FEATURE_PMSA);
> + cpu->pmsav7_dregion = 0;
> + }
> + if (cpu->pmsav7_dregion == 0) {
> + cpu->has_mpu = false;
> }
>
> if (arm_feature(env, ARM_FEATURE_PMSA) &&
> --
> 2.7.4
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 05/13] arm: Don't clear ARM_FEATURE_PMSA for no-mpu configs
2017-04-25 12:07 ` [Qemu-devel] [PATCH 05/13] arm: Don't clear ARM_FEATURE_PMSA for no-mpu configs Peter Maydell
2017-05-02 22:24 ` Alistair Francis
@ 2017-05-13 22:37 ` Philippe Mathieu-Daudé
2017-05-30 14:00 ` Peter Maydell
1 sibling, 1 reply; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-05-13 22:37 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel
Cc: Alistair Francis, Alex Bennée, patches
On 04/25/2017 09:07 AM, Peter Maydell wrote:
> Fix the handling of QOM properties for PMSA CPUs with no MPU:
>
> Allow no-MPU to be specified by either:
> * has-mpu = false
> * pmsav7_dregion = 0
> and make setting one imply the other. Don't clear the PMSA
> feature bit in this situation.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> target/arm/cpu.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index f17e279..8e57498 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -757,8 +757,14 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
> cpu->id_pfr1 &= ~0xf000;
> }
>
> + /* MPU can be configured out of a PMSA CPU either by setting has-mpu
> + * to false or by setting pmsav7-dregion to 0.
> + */
> if (!cpu->has_mpu) {
> - unset_feature(ARM_FEATURE_PMSA);
Oh, fixed here.
> + cpu->pmsav7_dregion = 0;
> + }
> + if (cpu->pmsav7_dregion == 0) {
> + cpu->has_mpu = false;
> }
>
> if (arm_feature(env, ARM_FEATURE_PMSA) &&
>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 05/13] arm: Don't clear ARM_FEATURE_PMSA for no-mpu configs
2017-05-13 22:37 ` Philippe Mathieu-Daudé
@ 2017-05-30 14:00 ` Peter Maydell
0 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2017-05-30 14:00 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-arm, QEMU Developers, Alistair Francis, Alex Bennée, patches
On 13 May 2017 at 23:37, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On 04/25/2017 09:07 AM, Peter Maydell wrote:
>>
>> Fix the handling of QOM properties for PMSA CPUs with no MPU:
>>
>> Allow no-MPU to be specified by either:
>> * has-mpu = false
>> * pmsav7_dregion = 0
>> and make setting one imply the other. Don't clear the PMSA
>> feature bit in this situation.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> target/arm/cpu.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>> index f17e279..8e57498 100644
>> --- a/target/arm/cpu.c
>> +++ b/target/arm/cpu.c
>> @@ -757,8 +757,14 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
>> **errp)
>> cpu->id_pfr1 &= ~0xf000;
>> }
>>
>> + /* MPU can be configured out of a PMSA CPU either by setting has-mpu
>> + * to false or by setting pmsav7-dregion to 0.
>> + */
>> if (!cpu->has_mpu) {
>> - unset_feature(ARM_FEATURE_PMSA);
>
>
> Oh, fixed here.
Ah, yes, I was wondering why I hadn't noticed an obvious
compile error. I'll fix up patch 4 so we don't break bisection.
thanks
-- PMM
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 06/13] arm: Don't let no-MPU PMSA cores write to SCTLR.M
2017-04-25 12:06 [Qemu-devel] [PATCH 00/13] armv7m: Implement MPU support Peter Maydell
` (4 preceding siblings ...)
2017-04-25 12:07 ` [Qemu-devel] [PATCH 05/13] arm: Don't clear ARM_FEATURE_PMSA for no-mpu configs Peter Maydell
@ 2017-04-25 12:07 ` Peter Maydell
2017-05-03 21:30 ` Alistair Francis
2017-04-25 12:07 ` [Qemu-devel] [PATCH 07/13] arm: Remove unnecessary check on cpu->pmsav7_dregion Peter Maydell
` (7 subsequent siblings)
13 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2017-04-25 12:07 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée, Alistair Francis
If the CPU is a PMSA config with no MPU implemented, then the
SCTLR.M bit should be RAZ/WI, so that the guest can never
turn on the non-existent MPU.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/helper.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 404bfdb..f0f25c8 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3258,6 +3258,11 @@ static void sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
return;
}
+ if (arm_feature(env, ARM_FEATURE_PMSA) && !cpu->has_mpu) {
+ /* M bit is RAZ/WI for PMSA with no MPU implemented */
+ value &= ~SCTLR_M;
+ }
+
raw_write(env, ri, value);
/* ??? Lots of these bits are not implemented. */
/* This may enable/disable the MMU, so do a TLB flush. */
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 06/13] arm: Don't let no-MPU PMSA cores write to SCTLR.M
2017-04-25 12:07 ` [Qemu-devel] [PATCH 06/13] arm: Don't let no-MPU PMSA cores write to SCTLR.M Peter Maydell
@ 2017-05-03 21:30 ` Alistair Francis
2017-05-13 22:38 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
0 siblings, 1 reply; 36+ messages in thread
From: Alistair Francis @ 2017-05-03 21:30 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, qemu-devel@nongnu.org Developers, Alistair Francis,
Alex Bennée, Patch Tracking
On Tue, Apr 25, 2017 at 5:07 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> If the CPU is a PMSA config with no MPU implemented, then the
> SCTLR.M bit should be RAZ/WI, so that the guest can never
> turn on the non-existent MPU.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
Thanks,
Alistair
> ---
> target/arm/helper.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 404bfdb..f0f25c8 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -3258,6 +3258,11 @@ static void sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> return;
> }
>
> + if (arm_feature(env, ARM_FEATURE_PMSA) && !cpu->has_mpu) {
> + /* M bit is RAZ/WI for PMSA with no MPU implemented */
> + value &= ~SCTLR_M;
> + }
> +
> raw_write(env, ri, value);
> /* ??? Lots of these bits are not implemented. */
> /* This may enable/disable the MMU, so do a TLB flush. */
> --
> 2.7.4
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH 06/13] arm: Don't let no-MPU PMSA cores write to SCTLR.M
2017-05-03 21:30 ` Alistair Francis
@ 2017-05-13 22:38 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-05-13 22:38 UTC (permalink / raw)
To: Alistair Francis, Peter Maydell
Cc: qemu-arm, qemu-devel@nongnu.org Developers, Patch Tracking
On 05/03/2017 06:30 PM, Alistair Francis wrote:
> On Tue, Apr 25, 2017 at 5:07 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> If the CPU is a PMSA config with no MPU implemented, then the
>> SCTLR.M bit should be RAZ/WI, so that the guest can never
>> turn on the non-existent MPU.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
> Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> Thanks,
>
> Alistair
>
>> ---
>> target/arm/helper.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index 404bfdb..f0f25c8 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -3258,6 +3258,11 @@ static void sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>> return;
>> }
>>
>> + if (arm_feature(env, ARM_FEATURE_PMSA) && !cpu->has_mpu) {
>> + /* M bit is RAZ/WI for PMSA with no MPU implemented */
>> + value &= ~SCTLR_M;
>> + }
>> +
>> raw_write(env, ri, value);
>> /* ??? Lots of these bits are not implemented. */
>> /* This may enable/disable the MMU, so do a TLB flush. */
>> --
>> 2.7.4
>>
>>
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 07/13] arm: Remove unnecessary check on cpu->pmsav7_dregion
2017-04-25 12:06 [Qemu-devel] [PATCH 00/13] armv7m: Implement MPU support Peter Maydell
` (5 preceding siblings ...)
2017-04-25 12:07 ` [Qemu-devel] [PATCH 06/13] arm: Don't let no-MPU PMSA cores write to SCTLR.M Peter Maydell
@ 2017-04-25 12:07 ` Peter Maydell
2017-05-13 22:41 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2017-04-25 12:07 ` [Qemu-devel] [PATCH 08/13] armv7m: Improve "-d mmu" tracing for PMSAv7 MPU Peter Maydell
` (6 subsequent siblings)
13 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2017-04-25 12:07 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée, Alistair Francis
Now that we enforce both:
* pmsav7_dregion == 0 implies has_mpu == false
* PMSA with has_mpu == false means SCTLR.M cannot be set
we can remove a check on pmsav7_dregion from get_phys_addr_pmsav7(),
because we can only reach this code path if the MPU is enabled
(and so region_translation_disabled() returned false).
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/helper.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index f0f25c8..5c044d0 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8227,8 +8227,7 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
}
if (n == -1) { /* no hits */
- if (cpu->pmsav7_dregion &&
- (is_user || !(regime_sctlr(env, mmu_idx) & SCTLR_BR))) {
+ if (is_user || !(regime_sctlr(env, mmu_idx) & SCTLR_BR)) {
/* background fault */
*fsr = 0;
return true;
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH 07/13] arm: Remove unnecessary check on cpu->pmsav7_dregion
2017-04-25 12:07 ` [Qemu-devel] [PATCH 07/13] arm: Remove unnecessary check on cpu->pmsav7_dregion Peter Maydell
@ 2017-05-13 22:41 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-05-13 22:41 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Alistair Francis, patches
On 04/25/2017 09:07 AM, Peter Maydell wrote:
> Now that we enforce both:
> * pmsav7_dregion == 0 implies has_mpu == false
> * PMSA with has_mpu == false means SCTLR.M cannot be set
> we can remove a check on pmsav7_dregion from get_phys_addr_pmsav7(),
> because we can only reach this code path if the MPU is enabled
> (and so region_translation_disabled() returned false).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> target/arm/helper.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index f0f25c8..5c044d0 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -8227,8 +8227,7 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
> }
>
> if (n == -1) { /* no hits */
> - if (cpu->pmsav7_dregion &&
> - (is_user || !(regime_sctlr(env, mmu_idx) & SCTLR_BR))) {
> + if (is_user || !(regime_sctlr(env, mmu_idx) & SCTLR_BR)) {
> /* background fault */
> *fsr = 0;
> return true;
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 08/13] armv7m: Improve "-d mmu" tracing for PMSAv7 MPU
2017-04-25 12:06 [Qemu-devel] [PATCH 00/13] armv7m: Implement MPU support Peter Maydell
` (6 preceding siblings ...)
2017-04-25 12:07 ` [Qemu-devel] [PATCH 07/13] arm: Remove unnecessary check on cpu->pmsav7_dregion Peter Maydell
@ 2017-04-25 12:07 ` Peter Maydell
2017-05-03 21:30 ` Alistair Francis
2017-04-25 12:07 ` [Qemu-devel] [PATCH 09/13] armv7m: Implement M profile default memory map Peter Maydell
` (5 subsequent siblings)
13 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2017-04-25 12:07 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée, Alistair Francis
From: Michael Davidsaver <mdavidsaver@gmail.com>
Improve the "-d mmu" tracing for the PMSAv7 MPU translation
process as an aid in debugging guest MPU configurations:
* fix a missing newline for a guest-error log
* report the region number with guest-error or unimp
logs of bad region register values
* add a log message for the overall result of the lookup
* print "0x" prefix for hex values
Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
[PMM: a little tidyup, report region number in all messages
rather than just one]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/helper.c | 39 +++++++++++++++++++++++++++------------
1 file changed, 27 insertions(+), 12 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 5c044d0..9e1ed1c 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8169,16 +8169,18 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
}
if (!rsize) {
- qemu_log_mask(LOG_GUEST_ERROR, "DRSR.Rsize field can not be 0");
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "DRSR[%d]: Rsize field cannot be 0\n", n);
continue;
}
rsize++;
rmask = (1ull << rsize) - 1;
if (base & rmask) {
- qemu_log_mask(LOG_GUEST_ERROR, "DRBAR %" PRIx32 " misaligned "
- "to DRSR region size, mask = %" PRIx32,
- base, rmask);
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "DRBAR[%d]: 0x%" PRIx32 " misaligned "
+ "to DRSR region size, mask = 0x%" PRIx32 "\n",
+ n, base, rmask);
continue;
}
@@ -8215,9 +8217,10 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
}
}
if (rsize < TARGET_PAGE_BITS) {
- qemu_log_mask(LOG_UNIMP, "No support for MPU (sub)region"
+ qemu_log_mask(LOG_UNIMP,
+ "DRSR[%d]: No support for MPU (sub)region "
"alignment of %" PRIu32 " bits. Minimum is %d\n",
- rsize, TARGET_PAGE_BITS);
+ n, rsize, TARGET_PAGE_BITS);
continue;
}
if (srdis) {
@@ -8251,8 +8254,8 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
break;
default:
qemu_log_mask(LOG_GUEST_ERROR,
- "Bad value for AP bits in DRACR %"
- PRIx32 "\n", ap);
+ "DRACR[%d]: Bad value for AP bits: 0x%"
+ PRIx32 "\n", n, ap);
}
} else { /* Priv. mode AP bits decoding */
switch (ap) {
@@ -8269,8 +8272,8 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
break;
default:
qemu_log_mask(LOG_GUEST_ERROR,
- "Bad value for AP bits in DRACR %"
- PRIx32 "\n", ap);
+ "DRACR[%d]: Bad value for AP bits: 0x%"
+ PRIx32 "\n", n, ap);
}
}
@@ -8448,9 +8451,21 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address,
*/
if (arm_feature(env, ARM_FEATURE_PMSA) &&
arm_feature(env, ARM_FEATURE_V7)) {
+ bool ret;
*page_size = TARGET_PAGE_SIZE;
- return get_phys_addr_pmsav7(env, address, access_type, mmu_idx,
- phys_ptr, prot, fsr);
+ ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx,
+ phys_ptr, prot, fsr);
+ qemu_log_mask(CPU_LOG_MMU, "PMSAv7 MPU lookup for %s at 0x%08" PRIx32
+ " mmu_idx %u -> %s (prot %c%c%c)\n",
+ access_type == 1 ? "reading" :
+ (access_type == 2 ? "writing" : "execute"),
+ (uint32_t)address, mmu_idx,
+ ret ? "Miss" : "Hit",
+ *prot & PAGE_READ ? 'r' : '-',
+ *prot & PAGE_WRITE ? 'w' : '-',
+ *prot & PAGE_EXEC ? 'x' : '-');
+
+ return ret;
}
if (regime_translation_disabled(env, mmu_idx)) {
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 08/13] armv7m: Improve "-d mmu" tracing for PMSAv7 MPU
2017-04-25 12:07 ` [Qemu-devel] [PATCH 08/13] armv7m: Improve "-d mmu" tracing for PMSAv7 MPU Peter Maydell
@ 2017-05-03 21:30 ` Alistair Francis
2017-05-13 22:52 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
0 siblings, 1 reply; 36+ messages in thread
From: Alistair Francis @ 2017-05-03 21:30 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, qemu-devel@nongnu.org Developers, Alistair Francis,
Alex Bennée, Patch Tracking
On Tue, Apr 25, 2017 at 5:07 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> From: Michael Davidsaver <mdavidsaver@gmail.com>
>
> Improve the "-d mmu" tracing for the PMSAv7 MPU translation
> process as an aid in debugging guest MPU configurations:
> * fix a missing newline for a guest-error log
> * report the region number with guest-error or unimp
> logs of bad region register values
> * add a log message for the overall result of the lookup
> * print "0x" prefix for hex values
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
> [PMM: a little tidyup, report region number in all messages
> rather than just one]
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
Thanks,
Alistair
> ---
> target/arm/helper.c | 39 +++++++++++++++++++++++++++------------
> 1 file changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 5c044d0..9e1ed1c 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -8169,16 +8169,18 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
> }
>
> if (!rsize) {
> - qemu_log_mask(LOG_GUEST_ERROR, "DRSR.Rsize field can not be 0");
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "DRSR[%d]: Rsize field cannot be 0\n", n);
> continue;
> }
> rsize++;
> rmask = (1ull << rsize) - 1;
>
> if (base & rmask) {
> - qemu_log_mask(LOG_GUEST_ERROR, "DRBAR %" PRIx32 " misaligned "
> - "to DRSR region size, mask = %" PRIx32,
> - base, rmask);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "DRBAR[%d]: 0x%" PRIx32 " misaligned "
> + "to DRSR region size, mask = 0x%" PRIx32 "\n",
> + n, base, rmask);
> continue;
> }
>
> @@ -8215,9 +8217,10 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
> }
> }
> if (rsize < TARGET_PAGE_BITS) {
> - qemu_log_mask(LOG_UNIMP, "No support for MPU (sub)region"
> + qemu_log_mask(LOG_UNIMP,
> + "DRSR[%d]: No support for MPU (sub)region "
> "alignment of %" PRIu32 " bits. Minimum is %d\n",
> - rsize, TARGET_PAGE_BITS);
> + n, rsize, TARGET_PAGE_BITS);
> continue;
> }
> if (srdis) {
> @@ -8251,8 +8254,8 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
> break;
> default:
> qemu_log_mask(LOG_GUEST_ERROR,
> - "Bad value for AP bits in DRACR %"
> - PRIx32 "\n", ap);
> + "DRACR[%d]: Bad value for AP bits: 0x%"
> + PRIx32 "\n", n, ap);
> }
> } else { /* Priv. mode AP bits decoding */
> switch (ap) {
> @@ -8269,8 +8272,8 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
> break;
> default:
> qemu_log_mask(LOG_GUEST_ERROR,
> - "Bad value for AP bits in DRACR %"
> - PRIx32 "\n", ap);
> + "DRACR[%d]: Bad value for AP bits: 0x%"
> + PRIx32 "\n", n, ap);
> }
> }
>
> @@ -8448,9 +8451,21 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address,
> */
> if (arm_feature(env, ARM_FEATURE_PMSA) &&
> arm_feature(env, ARM_FEATURE_V7)) {
> + bool ret;
> *page_size = TARGET_PAGE_SIZE;
> - return get_phys_addr_pmsav7(env, address, access_type, mmu_idx,
> - phys_ptr, prot, fsr);
> + ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx,
> + phys_ptr, prot, fsr);
> + qemu_log_mask(CPU_LOG_MMU, "PMSAv7 MPU lookup for %s at 0x%08" PRIx32
> + " mmu_idx %u -> %s (prot %c%c%c)\n",
> + access_type == 1 ? "reading" :
> + (access_type == 2 ? "writing" : "execute"),
> + (uint32_t)address, mmu_idx,
> + ret ? "Miss" : "Hit",
> + *prot & PAGE_READ ? 'r' : '-',
> + *prot & PAGE_WRITE ? 'w' : '-',
> + *prot & PAGE_EXEC ? 'x' : '-');
> +
> + return ret;
> }
>
> if (regime_translation_disabled(env, mmu_idx)) {
> --
> 2.7.4
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH 08/13] armv7m: Improve "-d mmu" tracing for PMSAv7 MPU
2017-05-03 21:30 ` Alistair Francis
@ 2017-05-13 22:52 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-05-13 22:52 UTC (permalink / raw)
To: Alistair Francis, Peter Maydell
Cc: qemu-arm, qemu-devel@nongnu.org Developers, Patch Tracking
On 05/03/2017 06:30 PM, Alistair Francis wrote:
> On Tue, Apr 25, 2017 at 5:07 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> From: Michael Davidsaver <mdavidsaver@gmail.com>
>>
>> Improve the "-d mmu" tracing for the PMSAv7 MPU translation
>> process as an aid in debugging guest MPU configurations:
>> * fix a missing newline for a guest-error log
>> * report the region number with guest-error or unimp
>> logs of bad region register values
>> * add a log message for the overall result of the lookup
>> * print "0x" prefix for hex values
>>
>> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
>> [PMM: a little tidyup, report region number in all messages
>> rather than just one]
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
> Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Thanks,
>
> Alistair
>
>> ---
>> target/arm/helper.c | 39 +++++++++++++++++++++++++++------------
>> 1 file changed, 27 insertions(+), 12 deletions(-)
>>
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index 5c044d0..9e1ed1c 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -8169,16 +8169,18 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
>> }
>>
>> if (!rsize) {
>> - qemu_log_mask(LOG_GUEST_ERROR, "DRSR.Rsize field can not be 0");
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "DRSR[%d]: Rsize field cannot be 0\n", n);
>> continue;
>> }
>> rsize++;
>> rmask = (1ull << rsize) - 1;
>>
>> if (base & rmask) {
>> - qemu_log_mask(LOG_GUEST_ERROR, "DRBAR %" PRIx32 " misaligned "
>> - "to DRSR region size, mask = %" PRIx32,
>> - base, rmask);
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "DRBAR[%d]: 0x%" PRIx32 " misaligned "
>> + "to DRSR region size, mask = 0x%" PRIx32 "\n",
>> + n, base, rmask);
>> continue;
>> }
>>
>> @@ -8215,9 +8217,10 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
>> }
>> }
>> if (rsize < TARGET_PAGE_BITS) {
>> - qemu_log_mask(LOG_UNIMP, "No support for MPU (sub)region"
>> + qemu_log_mask(LOG_UNIMP,
>> + "DRSR[%d]: No support for MPU (sub)region "
>> "alignment of %" PRIu32 " bits. Minimum is %d\n",
>> - rsize, TARGET_PAGE_BITS);
>> + n, rsize, TARGET_PAGE_BITS);
>> continue;
>> }
>> if (srdis) {
>> @@ -8251,8 +8254,8 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
>> break;
>> default:
>> qemu_log_mask(LOG_GUEST_ERROR,
>> - "Bad value for AP bits in DRACR %"
>> - PRIx32 "\n", ap);
>> + "DRACR[%d]: Bad value for AP bits: 0x%"
>> + PRIx32 "\n", n, ap);
>> }
>> } else { /* Priv. mode AP bits decoding */
>> switch (ap) {
>> @@ -8269,8 +8272,8 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
>> break;
>> default:
>> qemu_log_mask(LOG_GUEST_ERROR,
>> - "Bad value for AP bits in DRACR %"
>> - PRIx32 "\n", ap);
>> + "DRACR[%d]: Bad value for AP bits: 0x%"
>> + PRIx32 "\n", n, ap);
>> }
>> }
>>
>> @@ -8448,9 +8451,21 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address,
>> */
>> if (arm_feature(env, ARM_FEATURE_PMSA) &&
>> arm_feature(env, ARM_FEATURE_V7)) {
>> + bool ret;
>> *page_size = TARGET_PAGE_SIZE;
>> - return get_phys_addr_pmsav7(env, address, access_type, mmu_idx,
>> - phys_ptr, prot, fsr);
>> + ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx,
>> + phys_ptr, prot, fsr);
>> + qemu_log_mask(CPU_LOG_MMU, "PMSAv7 MPU lookup for %s at 0x%08" PRIx32
>> + " mmu_idx %u -> %s (prot %c%c%c)\n",
>> + access_type == 1 ? "reading" :
>> + (access_type == 2 ? "writing" : "execute"),
>> + (uint32_t)address, mmu_idx,
>> + ret ? "Miss" : "Hit",
>> + *prot & PAGE_READ ? 'r' : '-',
>> + *prot & PAGE_WRITE ? 'w' : '-',
>> + *prot & PAGE_EXEC ? 'x' : '-');
>> +
>> + return ret;
>> }
>>
>> if (regime_translation_disabled(env, mmu_idx)) {
>> --
>> 2.7.4
>>
>>
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 09/13] armv7m: Implement M profile default memory map
2017-04-25 12:06 [Qemu-devel] [PATCH 00/13] armv7m: Implement MPU support Peter Maydell
` (7 preceding siblings ...)
2017-04-25 12:07 ` [Qemu-devel] [PATCH 08/13] armv7m: Improve "-d mmu" tracing for PMSAv7 MPU Peter Maydell
@ 2017-04-25 12:07 ` Peter Maydell
2017-05-30 14:56 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2017-04-25 12:07 ` [Qemu-devel] [PATCH 10/13] arm: All M profile cores are PMSA Peter Maydell
` (4 subsequent siblings)
13 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2017-04-25 12:07 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée, Alistair Francis
From: Michael Davidsaver <mdavidsaver@gmail.com>
Add support for the M profile default memory map which is used
if the MPU is not present or disabled.
The main differences in behaviour from implementing this
correctly are that we set the PAGE_EXEC attribute on
the right regions of memory, such that device regions
are not executable.
Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
[PMM: rephrased comment and commit message; don't mark
the flash memory region as not-writable]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/helper.c | 35 ++++++++++++++++++++++++++---------
1 file changed, 26 insertions(+), 9 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 9e1ed1c..51662ad 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8129,18 +8129,35 @@ static inline void get_phys_addr_pmsav7_default(CPUARMState *env,
ARMMMUIdx mmu_idx,
int32_t address, int *prot)
{
- *prot = PAGE_READ | PAGE_WRITE;
- switch (address) {
- case 0xF0000000 ... 0xFFFFFFFF:
- if (regime_sctlr(env, mmu_idx) & SCTLR_V) { /* hivecs execing is ok */
+ if (!arm_feature(env, ARM_FEATURE_M)) {
+ *prot = PAGE_READ | PAGE_WRITE;
+ switch (address) {
+ case 0xF0000000 ... 0xFFFFFFFF:
+ if (regime_sctlr(env, mmu_idx) & SCTLR_V) {
+ /* hivecs execing is ok */
+ *prot |= PAGE_EXEC;
+ }
+ break;
+ case 0x00000000 ... 0x7FFFFFFF:
*prot |= PAGE_EXEC;
+ break;
+ }
+ } else {
+ /* Default system address map for M profile cores.
+ * The architecture specifies which regions are execute-never;
+ * at the MPU level no other checks are defined.
+ */
+ switch (address) {
+ case 0x00000000 ... 0x1fffffff: /* ROM */
+ case 0x20000000 ... 0x3fffffff: /* SRAM */
+ case 0x60000000 ... 0x7fffffff: /* RAM */
+ case 0x80000000 ... 0x9fffffff: /* RAM */
+ *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
+ break;
+ default: /* Peripheral, 2x Device, and System */
+ *prot = PAGE_READ | PAGE_WRITE;
}
- break;
- case 0x00000000 ... 0x7FFFFFFF:
- *prot |= PAGE_EXEC;
- break;
}
-
}
static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH 09/13] armv7m: Implement M profile default memory map
2017-04-25 12:07 ` [Qemu-devel] [PATCH 09/13] armv7m: Implement M profile default memory map Peter Maydell
@ 2017-05-30 14:56 ` Philippe Mathieu-Daudé
2017-05-30 15:11 ` Peter Maydell
0 siblings, 1 reply; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-05-30 14:56 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Alistair Francis, patches
Hi Peter,
On 04/25/2017 09:07 AM, Peter Maydell wrote:
> From: Michael Davidsaver <mdavidsaver@gmail.com>
>
> Add support for the M profile default memory map which is used
> if the MPU is not present or disabled.
>
> The main differences in behaviour from implementing this
> correctly are that we set the PAGE_EXEC attribute on
> the right regions of memory, such that device regions
> are not executable.
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
> [PMM: rephrased comment and commit message; don't mark
> the flash memory region as not-writable]
"not-writable by system caches" maybe to clarify?
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> target/arm/helper.c | 35 ++++++++++++++++++++++++++---------
> 1 file changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 9e1ed1c..51662ad 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -8129,18 +8129,35 @@ static inline void get_phys_addr_pmsav7_default(CPUARMState *env,
> ARMMMUIdx mmu_idx,
> int32_t address, int *prot)
> {
> - *prot = PAGE_READ | PAGE_WRITE;
> - switch (address) {
> - case 0xF0000000 ... 0xFFFFFFFF:
> - if (regime_sctlr(env, mmu_idx) & SCTLR_V) { /* hivecs execing is ok */
> + if (!arm_feature(env, ARM_FEATURE_M)) {
> + *prot = PAGE_READ | PAGE_WRITE;
> + switch (address) {
> + case 0xF0000000 ... 0xFFFFFFFF:
> + if (regime_sctlr(env, mmu_idx) & SCTLR_V) {
> + /* hivecs execing is ok */
> + *prot |= PAGE_EXEC;
> + }
> + break;
> + case 0x00000000 ... 0x7FFFFFFF:
> *prot |= PAGE_EXEC;
I checked at Table B3-1 on the ARMv7-M Architecture Reference Manual I
got at https://static.docs.arm.com/ddi0403/e/DDI0403E_B_armv7m_arm.pdf
and the on-chip peripheral address space at 0x40000000 is eXecute Never.
> + break;
> + }
> + } else {
> + /* Default system address map for M profile cores.
> + * The architecture specifies which regions are execute-never;
> + * at the MPU level no other checks are defined.
> + */
> + switch (address) {
> + case 0x00000000 ... 0x1fffffff: /* ROM */
> + case 0x20000000 ... 0x3fffffff: /* SRAM */
> + case 0x60000000 ... 0x7fffffff: /* RAM */
> + case 0x80000000 ... 0x9fffffff: /* RAM */
> + *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> + break;
> + default: /* Peripheral, 2x Device, and System */
> + *prot = PAGE_READ | PAGE_WRITE;
This body is correct, however what do you think about using cases with
comments instead of 'default'? This would be clearer.
> }
> - break;
> - case 0x00000000 ... 0x7FFFFFFF:
> - *prot |= PAGE_EXEC;
> - break;
> }
> -
> }
>
> static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
>
Regards,
Phil.
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH 09/13] armv7m: Implement M profile default memory map
2017-05-30 14:56 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
@ 2017-05-30 15:11 ` Peter Maydell
2017-06-02 5:10 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2017-05-30 15:11 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-arm, QEMU Developers, Alistair Francis, patches
On 30 May 2017 at 15:56, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Hi Peter,
>
> On 04/25/2017 09:07 AM, Peter Maydell wrote:
>>
>> From: Michael Davidsaver <mdavidsaver@gmail.com>
>>
>> Add support for the M profile default memory map which is used
>> if the MPU is not present or disabled.
>>
>> The main differences in behaviour from implementing this
>> correctly are that we set the PAGE_EXEC attribute on
>> the right regions of memory, such that device regions
>> are not executable.
>>
>> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
>> [PMM: rephrased comment and commit message; don't mark
>> the flash memory region as not-writable]
>
>
> "not-writable by system caches" maybe to clarify?
(Note that this is a comment describing something I
deleted from Michael's original patch.)
No, by 'not-writable' here I mean "not writeable by the CPU".
>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> target/arm/helper.c | 35 ++++++++++++++++++++++++++---------
>> 1 file changed, 26 insertions(+), 9 deletions(-)
>>
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index 9e1ed1c..51662ad 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -8129,18 +8129,35 @@ static inline void
>> get_phys_addr_pmsav7_default(CPUARMState *env,
>> ARMMMUIdx mmu_idx,
>> int32_t address, int
>> *prot)
>> {
>> - *prot = PAGE_READ | PAGE_WRITE;
>> - switch (address) {
>> - case 0xF0000000 ... 0xFFFFFFFF:
>> - if (regime_sctlr(env, mmu_idx) & SCTLR_V) { /* hivecs execing is
>> ok */
>> + if (!arm_feature(env, ARM_FEATURE_M)) {
>> + *prot = PAGE_READ | PAGE_WRITE;
>> + switch (address) {
>> + case 0xF0000000 ... 0xFFFFFFFF:
>> + if (regime_sctlr(env, mmu_idx) & SCTLR_V) {
>> + /* hivecs execing is ok */
>> + *prot |= PAGE_EXEC;
>> + }
>> + break;
>> + case 0x00000000 ... 0x7FFFFFFF:
>
>> *prot |= PAGE_EXEC;
>
> I checked at Table B3-1 on the ARMv7-M Architecture Reference Manual I got
> at https://static.docs.arm.com/ddi0403/e/DDI0403E_B_armv7m_arm.pdf and the
> on-chip peripheral address space at 0x40000000 is eXecute Never.
This is the arm of the if() that deals with R profile, and R profile's
background region is completely different to M profile. (In any
case the patch shouldn't change the behaviour there.)
>> + break;
>> + }
>> + } else {
>> + /* Default system address map for M profile cores.
>> + * The architecture specifies which regions are execute-never;
>> + * at the MPU level no other checks are defined.
>> + */
>> + switch (address) {
>> + case 0x00000000 ... 0x1fffffff: /* ROM */
>> + case 0x20000000 ... 0x3fffffff: /* SRAM */
>> + case 0x60000000 ... 0x7fffffff: /* RAM */
>> + case 0x80000000 ... 0x9fffffff: /* RAM */
>> + *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
>> + break;
>> + default: /* Peripheral, 2x Device, and System */
>> + *prot = PAGE_READ | PAGE_WRITE;
>
>
> This body is correct, however what do you think about using cases with
> comments instead of 'default'? This would be clearer.
Yeah, we could do that. I think this is one of those cases where
I opted to go with how Michael had already written the code rather
than rewriting it.
/* Default system address map for M profile cores.
* The architecture specifies which regions are execute-never;
* at the MPU level no other checks are defined.
*/
switch (address) {
case 0x00000000 ... 0x1fffffff: /* ROM */
case 0x20000000 ... 0x3fffffff: /* SRAM */
case 0x60000000 ... 0x7fffffff: /* RAM */
case 0x80000000 ... 0x9fffffff: /* RAM */
*prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
break;
case 0x40000000 ... 0x5fffffff: /* Peripheral */
case 0xa0000000 ... 0xbfffffff: /* Device */
case 0xc0000000 ... 0xdfffffff: /* Device */
case 0xe0000000 ... 0xffffffff: /* System */
*prot = PAGE_READ | PAGE_WRITE;
break;
default:
g_assert_not_reached();
}
would be the explicit version.
>> }
>> - break;
>> - case 0x00000000 ... 0x7FFFFFFF:
>> - *prot |= PAGE_EXEC;
>> - break;
>> }
>> -
>> }
>>
>> static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
thanks
-- PMM
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH 09/13] armv7m: Implement M profile default memory map
2017-05-30 15:11 ` Peter Maydell
@ 2017-06-02 5:10 ` Philippe Mathieu-Daudé
2017-06-02 9:00 ` Peter Maydell
0 siblings, 1 reply; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-06-02 5:10 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, QEMU Developers, Alistair Francis, patches
Hi Peter,
On 05/30/2017 12:11 PM, Peter Maydell wrote:
> On 30 May 2017 at 15:56, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> Hi Peter,
>>
>> On 04/25/2017 09:07 AM, Peter Maydell wrote:
>>>
>>> From: Michael Davidsaver <mdavidsaver@gmail.com>
>>>
>>> Add support for the M profile default memory map which is used
>>> if the MPU is not present or disabled.
>>>
>>> The main differences in behaviour from implementing this
>>> correctly are that we set the PAGE_EXEC attribute on
>>> the right regions of memory, such that device regions
>>> are not executable.
>>>
>>> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
>>> [PMM: rephrased comment and commit message; don't mark
>>> the flash memory region as not-writable]
>>
>>
>> "not-writable by system caches" maybe to clarify?
>
> (Note that this is a comment describing something I
> deleted from Michael's original patch.)
> No, by 'not-writable' here I mean "not writeable by the CPU".
>
Ok!
>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>> target/arm/helper.c | 35 ++++++++++++++++++++++++++---------
>>> 1 file changed, 26 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>>> index 9e1ed1c..51662ad 100644
>>> --- a/target/arm/helper.c
>>> +++ b/target/arm/helper.c
>>> @@ -8129,18 +8129,35 @@ static inline void
>>> get_phys_addr_pmsav7_default(CPUARMState *env,
>>> ARMMMUIdx mmu_idx,
>>> int32_t address, int
>>> *prot)
>>> {
>>> - *prot = PAGE_READ | PAGE_WRITE;
>>> - switch (address) {
>>> - case 0xF0000000 ... 0xFFFFFFFF:
>>> - if (regime_sctlr(env, mmu_idx) & SCTLR_V) { /* hivecs execing is
>>> ok */
>>> + if (!arm_feature(env, ARM_FEATURE_M)) {
>>> + *prot = PAGE_READ | PAGE_WRITE;
>>> + switch (address) {
>>> + case 0xF0000000 ... 0xFFFFFFFF:
>>> + if (regime_sctlr(env, mmu_idx) & SCTLR_V) {
>>> + /* hivecs execing is ok */
>>> + *prot |= PAGE_EXEC;
>>> + }
>>> + break;
>>> + case 0x00000000 ... 0x7FFFFFFF:
>>
>>> *prot |= PAGE_EXEC;
>>
>> I checked at Table B3-1 on the ARMv7-M Architecture Reference Manual I got
>> at https://static.docs.arm.com/ddi0403/e/DDI0403E_B_armv7m_arm.pdf and the
>> on-chip peripheral address space at 0x40000000 is eXecute Never.
>
> This is the arm of the if() that deals with R profile, and R profile's
Oh I completely misunderstood that if() indeed. R and also A I suppose.
> background region is completely different to M profile. (In any
> case the patch shouldn't change the behaviour there.)
>
>>> + break;
>>> + }
>>> + } else {
>>> + /* Default system address map for M profile cores.
>>> + * The architecture specifies which regions are execute-never;
>>> + * at the MPU level no other checks are defined.
>>> + */
>>> + switch (address) {
>>> + case 0x00000000 ... 0x1fffffff: /* ROM */
>>> + case 0x20000000 ... 0x3fffffff: /* SRAM */
>>> + case 0x60000000 ... 0x7fffffff: /* RAM */
>>> + case 0x80000000 ... 0x9fffffff: /* RAM */
>>> + *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
>>> + break;
>>> + default: /* Peripheral, 2x Device, and System */
>>> + *prot = PAGE_READ | PAGE_WRITE;
>>
>>
>> This body is correct, however what do you think about using cases with
>> comments instead of 'default'? This would be clearer.
>
> Yeah, we could do that. I think this is one of those cases where
> I opted to go with how Michael had already written the code rather
> than rewriting it.
>
> /* Default system address map for M profile cores.
> * The architecture specifies which regions are execute-never;
> * at the MPU level no other checks are defined.
> */
> switch (address) {
> case 0x00000000 ... 0x1fffffff: /* ROM */
> case 0x20000000 ... 0x3fffffff: /* SRAM */
> case 0x60000000 ... 0x7fffffff: /* RAM */
> case 0x80000000 ... 0x9fffffff: /* RAM */
> *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> break;
> case 0x40000000 ... 0x5fffffff: /* Peripheral */
> case 0xa0000000 ... 0xbfffffff: /* Device */
> case 0xc0000000 ... 0xdfffffff: /* Device */
> case 0xe0000000 ... 0xffffffff: /* System */
> *prot = PAGE_READ | PAGE_WRITE;
> break;
> default:
> g_assert_not_reached();
> }
>
> would be the explicit version.
>
I see you added that before your pull request, thank!
For what it's worth:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> }
>>> - break;
>>> - case 0x00000000 ... 0x7FFFFFFF:
>>> - *prot |= PAGE_EXEC;
>>> - break;
>>> }
>>> -
>>> }
>>>
>>> static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH 09/13] armv7m: Implement M profile default memory map
2017-06-02 5:10 ` Philippe Mathieu-Daudé
@ 2017-06-02 9:00 ` Peter Maydell
0 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2017-06-02 9:00 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-arm, QEMU Developers, Alistair Francis, patches
On 2 June 2017 at 06:10, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On 05/30/2017 12:11 PM, Peter Maydell wrote:
>> This is the arm of the if() that deals with R profile, and R profile's
>
>
> Oh I completely misunderstood that if() indeed. R and also A I suppose.
A profile is never PMSA -- arguably VMSA (MMU) vs PMSA (MPU)) is the defining
distinction between A profile and R profile cores.
thanks
-- PMM
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 10/13] arm: All M profile cores are PMSA
2017-04-25 12:06 [Qemu-devel] [PATCH 00/13] armv7m: Implement MPU support Peter Maydell
` (8 preceding siblings ...)
2017-04-25 12:07 ` [Qemu-devel] [PATCH 09/13] armv7m: Implement M profile default memory map Peter Maydell
@ 2017-04-25 12:07 ` Peter Maydell
2017-05-13 22:40 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2017-04-25 12:07 ` [Qemu-devel] [PATCH 11/13] armv7m: Classify faults as MemManage or BusFault Peter Maydell
` (3 subsequent siblings)
13 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2017-04-25 12:07 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée, Alistair Francis
All M profile CPUs are PMSA, so set the feature bit.
(We haven't actually implemented the M profile MPU register
interface yet, but setting this feature bit gives us closer
to correct behaviour for the MPU-disabled case.)
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/cpu.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 8e57498..df8b835 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -543,6 +543,14 @@ static void arm_cpu_post_init(Object *obj)
{
ARMCPU *cpu = ARM_CPU(obj);
+ /* M profile implies PMSA. We have to do this here rather than
+ * in realize with the other feature-implication checks because
+ * we look at the PMSA bit to see if we should add some properties.
+ */
+ if (arm_feature(&cpu->env, ARM_FEATURE_M)) {
+ set_feature(&cpu->env, ARM_FEATURE_PMSA);
+ }
+
if (arm_feature(&cpu->env, ARM_FEATURE_CBAR) ||
arm_feature(&cpu->env, ARM_FEATURE_CBAR_RO)) {
qdev_property_add_static(DEVICE(obj), &arm_cpu_reset_cbar_property,
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH 10/13] arm: All M profile cores are PMSA
2017-04-25 12:07 ` [Qemu-devel] [PATCH 10/13] arm: All M profile cores are PMSA Peter Maydell
@ 2017-05-13 22:40 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-05-13 22:40 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Alistair Francis, patches
On 04/25/2017 09:07 AM, Peter Maydell wrote:
> All M profile CPUs are PMSA, so set the feature bit.
> (We haven't actually implemented the M profile MPU register
> interface yet, but setting this feature bit gives us closer
> to correct behaviour for the MPU-disabled case.)
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> target/arm/cpu.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 8e57498..df8b835 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -543,6 +543,14 @@ static void arm_cpu_post_init(Object *obj)
> {
> ARMCPU *cpu = ARM_CPU(obj);
>
> + /* M profile implies PMSA. We have to do this here rather than
> + * in realize with the other feature-implication checks because
> + * we look at the PMSA bit to see if we should add some properties.
> + */
> + if (arm_feature(&cpu->env, ARM_FEATURE_M)) {
> + set_feature(&cpu->env, ARM_FEATURE_PMSA);
> + }
> +
> if (arm_feature(&cpu->env, ARM_FEATURE_CBAR) ||
> arm_feature(&cpu->env, ARM_FEATURE_CBAR_RO)) {
> qdev_property_add_static(DEVICE(obj), &arm_cpu_reset_cbar_property,
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 11/13] armv7m: Classify faults as MemManage or BusFault
2017-04-25 12:06 [Qemu-devel] [PATCH 00/13] armv7m: Implement MPU support Peter Maydell
` (9 preceding siblings ...)
2017-04-25 12:07 ` [Qemu-devel] [PATCH 10/13] arm: All M profile cores are PMSA Peter Maydell
@ 2017-04-25 12:07 ` Peter Maydell
2017-05-30 14:58 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2017-04-25 12:07 ` [Qemu-devel] [PATCH 12/13] arm: add MPU support to M profile CPUs Peter Maydell
` (2 subsequent siblings)
13 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2017-04-25 12:07 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée, Alistair Francis
From: Michael Davidsaver <mdavidsaver@gmail.com>
General logic is that operations stopped by the MPU are MemManage,
and those which go through the MPU and are caught by the unassigned
handle are BusFault. Distinguish these by looking at the
exception.fsr values, and set the CFSR bits and (if appropriate)
fill in the BFAR or MMFAR with the exception address.
Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
[PMM: i-side faults do not set BFAR/MMFAR, only d-side;
added some CPU_LOG_INT logging]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/helper.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 42 insertions(+), 3 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 51662ad..49b6d01 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6342,10 +6342,49 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
break;
case EXCP_PREFETCH_ABORT:
case EXCP_DATA_ABORT:
- /* TODO: if we implemented the MPU registers, this is where we
- * should set the MMFAR, etc from exception.fsr and exception.vaddress.
+ /* Note that for M profile we don't have a guest facing FSR, but
+ * the env->exception.fsr will be populated by the code that
+ * raises the fault, in the A profile short-descriptor format.
*/
- armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM);
+ switch (env->exception.fsr & 0xf) {
+ case 0x8: /* External Abort */
+ switch (cs->exception_index) {
+ case EXCP_PREFETCH_ABORT:
+ env->v7m.cfsr |= R_V7M_CFSR_PRECISERR_MASK;
+ qemu_log_mask(CPU_LOG_INT, "...with CFSR.PRECISERR\n");
+ break;
+ case EXCP_DATA_ABORT:
+ env->v7m.cfsr |=
+ (R_V7M_CFSR_IBUSERR_MASK | R_V7M_CFSR_BFARVALID_MASK);
+ env->v7m.bfar = env->exception.vaddress;
+ qemu_log_mask(CPU_LOG_INT,
+ "...with CFSR.IBUSERR and BFAR 0x%x\n",
+ env->v7m.bfar);
+ break;
+ }
+ armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_BUS);
+ break;
+ default:
+ /* All other FSR values are either MPU faults or "can't happen
+ * for M profile" cases.
+ */
+ switch (cs->exception_index) {
+ case EXCP_PREFETCH_ABORT:
+ env->v7m.cfsr |= R_V7M_CFSR_IACCVIOL_MASK;
+ qemu_log_mask(CPU_LOG_INT, "...with CFSR.IACCVIOL\n");
+ break;
+ case EXCP_DATA_ABORT:
+ env->v7m.cfsr |=
+ (R_V7M_CFSR_DACCVIOL_MASK | R_V7M_CFSR_MMARVALID_MASK);
+ env->v7m.mmfar = env->exception.vaddress;
+ qemu_log_mask(CPU_LOG_INT,
+ "...with CFSR.DACCVIOL and MMFAR 0x%x\n",
+ env->v7m.mmfar);
+ break;
+ }
+ armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM);
+ break;
+ }
break;
case EXCP_BKPT:
if (semihosting_enabled()) {
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH 11/13] armv7m: Classify faults as MemManage or BusFault
2017-04-25 12:07 ` [Qemu-devel] [PATCH 11/13] armv7m: Classify faults as MemManage or BusFault Peter Maydell
@ 2017-05-30 14:58 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-05-30 14:58 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Alistair Francis, patches
On 04/25/2017 09:07 AM, Peter Maydell wrote:
> From: Michael Davidsaver <mdavidsaver@gmail.com>
>
> General logic is that operations stopped by the MPU are MemManage,
> and those which go through the MPU and are caught by the unassigned
> handle are BusFault. Distinguish these by looking at the
> exception.fsr values, and set the CFSR bits and (if appropriate)
> fill in the BFAR or MMFAR with the exception address.
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
> [PMM: i-side faults do not set BFAR/MMFAR, only d-side;
> added some CPU_LOG_INT logging]
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> target/arm/helper.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 51662ad..49b6d01 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6342,10 +6342,49 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
> break;
> case EXCP_PREFETCH_ABORT:
> case EXCP_DATA_ABORT:
> - /* TODO: if we implemented the MPU registers, this is where we
> - * should set the MMFAR, etc from exception.fsr and exception.vaddress.
> + /* Note that for M profile we don't have a guest facing FSR, but
> + * the env->exception.fsr will be populated by the code that
> + * raises the fault, in the A profile short-descriptor format.
> */
> - armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM);
> + switch (env->exception.fsr & 0xf) {
> + case 0x8: /* External Abort */
> + switch (cs->exception_index) {
> + case EXCP_PREFETCH_ABORT:
> + env->v7m.cfsr |= R_V7M_CFSR_PRECISERR_MASK;
> + qemu_log_mask(CPU_LOG_INT, "...with CFSR.PRECISERR\n");
> + break;
> + case EXCP_DATA_ABORT:
> + env->v7m.cfsr |=
> + (R_V7M_CFSR_IBUSERR_MASK | R_V7M_CFSR_BFARVALID_MASK);
> + env->v7m.bfar = env->exception.vaddress;
> + qemu_log_mask(CPU_LOG_INT,
> + "...with CFSR.IBUSERR and BFAR 0x%x\n",
> + env->v7m.bfar);
> + break;
> + }
> + armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_BUS);
> + break;
> + default:
> + /* All other FSR values are either MPU faults or "can't happen
> + * for M profile" cases.
> + */
> + switch (cs->exception_index) {
> + case EXCP_PREFETCH_ABORT:
> + env->v7m.cfsr |= R_V7M_CFSR_IACCVIOL_MASK;
> + qemu_log_mask(CPU_LOG_INT, "...with CFSR.IACCVIOL\n");
> + break;
> + case EXCP_DATA_ABORT:
> + env->v7m.cfsr |=
> + (R_V7M_CFSR_DACCVIOL_MASK | R_V7M_CFSR_MMARVALID_MASK);
> + env->v7m.mmfar = env->exception.vaddress;
> + qemu_log_mask(CPU_LOG_INT,
> + "...with CFSR.DACCVIOL and MMFAR 0x%x\n",
> + env->v7m.mmfar);
> + break;
> + }
> + armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM);
> + break;
> + }
> break;
> case EXCP_BKPT:
> if (semihosting_enabled()) {
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 12/13] arm: add MPU support to M profile CPUs
2017-04-25 12:06 [Qemu-devel] [PATCH 00/13] armv7m: Implement MPU support Peter Maydell
` (10 preceding siblings ...)
2017-04-25 12:07 ` [Qemu-devel] [PATCH 11/13] armv7m: Classify faults as MemManage or BusFault Peter Maydell
@ 2017-04-25 12:07 ` Peter Maydell
2017-04-25 12:07 ` [Qemu-devel] [PATCH 13/13] arm: Implement HFNMIENA support for M profile MPU Peter Maydell
2017-05-30 14:05 ` [Qemu-devel] [Qemu-arm] [PATCH 00/13] armv7m: Implement MPU support Peter Maydell
13 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2017-04-25 12:07 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée, Alistair Francis
From: Michael Davidsaver <mdavidsaver@gmail.com>
The M series MPU is almost the same as the already implemented R
profile MPU (v7 PMSA). So all we need to implement here is the MPU
register interface in the system register space.
This implementation has the same restriction as the R profile MPU
that it doesn't permit regions to be sized down smaller than 1K.
We also do not yet implement support for MPU_CTRL.HFNMIENA; this
bit should if zero disable use of the MPU when running HardFault,
NMI or with FAULTMASK set to 1 (ie at an execution priority of
less than zero) -- if the MPU is enabled we don't treat these
cases any differently.
Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
[PMM: Keep all the bits in mpu_ctrl field, rather than
using SCTLR bits for them; drop broken HFNMIENA support;
various cleanup]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/cpu.h | 6 +++
hw/intc/armv7m_nvic.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++
target/arm/helper.c | 25 +++++++++++-
target/arm/machine.c | 5 ++-
4 files changed, 137 insertions(+), 3 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 0718955..bbdd064 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -418,6 +418,7 @@ typedef struct CPUARMState {
uint32_t dfsr; /* Debug Fault Status Register */
uint32_t mmfar; /* MemManage Fault Address */
uint32_t bfar; /* BusFault Address */
+ unsigned mpu_ctrl; /* MPU_CTRL (some bits kept in sctlr_el[1]) */
int exception;
} v7m;
@@ -1166,6 +1167,11 @@ FIELD(V7M_DFSR, DWTTRAP, 2, 1)
FIELD(V7M_DFSR, VCATCH, 3, 1)
FIELD(V7M_DFSR, EXTERNAL, 4, 1)
+/* v7M MPU_CTRL bits */
+FIELD(V7M_MPU_CTRL, ENABLE, 0, 1)
+FIELD(V7M_MPU_CTRL, HFNMIENA, 1, 1)
+FIELD(V7M_MPU_CTRL, PRIVDEFENA, 2, 1)
+
/* If adding a feature bit which corresponds to a Linux ELF
* HWCAP bit, remember to update the feature-bit-to-hwcap
* mapping in linux-user/elfload.c:get_elf_hwcap().
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 32ffa0b..26a4b2d 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -19,6 +19,7 @@
#include "hw/arm/arm.h"
#include "hw/arm/armv7m_nvic.h"
#include "target/arm/cpu.h"
+#include "exec/exec-all.h"
#include "qemu/log.h"
#include "trace.h"
@@ -528,6 +529,39 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset)
case 0xd70: /* ISAR4. */
return 0x01310102;
/* TODO: Implement debug registers. */
+ case 0xd90: /* MPU_TYPE */
+ /* Unified MPU; if the MPU is not present this value is zero */
+ return cpu->pmsav7_dregion << 8;
+ break;
+ case 0xd94: /* MPU_CTRL */
+ return cpu->env.v7m.mpu_ctrl;
+ case 0xd98: /* MPU_RNR */
+ return cpu->env.cp15.c6_rgnr;
+ 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;
+
+ if (region >= cpu->pmsav7_dregion) {
+ return 0;
+ }
+ return (cpu->env.pmsav7.drbar[region] & 0x1f) | (region & 0xf);
+ }
+ case 0xda0: /* MPU_RASR */
+ case 0xda8: /* MPU_RASR_A1 */
+ case 0xdb0: /* MPU_RASR_A2 */
+ case 0xdb8: /* MPU_RASR_A3 */
+ {
+ int region = cpu->env.cp15.c6_rgnr;
+
+ if (region >= cpu->pmsav7_dregion) {
+ return 0;
+ }
+ return ((cpu->env.pmsav7.dracr[region] & 0xffff) << 16) |
+ (cpu->env.pmsav7.drsr[region] & 0xffff);
+ }
default:
qemu_log_mask(LOG_GUEST_ERROR, "NVIC: Bad read offset 0x%x\n", offset);
return 0;
@@ -627,6 +661,76 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value)
qemu_log_mask(LOG_UNIMP,
"NVIC: Aux fault status registers unimplemented\n");
break;
+ case 0xd90: /* MPU_TYPE */
+ return; /* RO */
+ case 0xd94: /* MPU_CTRL */
+ if ((value &
+ (R_V7M_MPU_CTRL_HFNMIENA_MASK | R_V7M_MPU_CTRL_ENABLE_MASK))
+ == R_V7M_MPU_CTRL_HFNMIENA_MASK) {
+ qemu_log_mask(LOG_GUEST_ERROR, "MPU_CTRL: HFNMIENA and !ENABLE is "
+ "UNPREDICTABLE\n");
+ }
+ cpu->env.v7m.mpu_ctrl = value & (R_V7M_MPU_CTRL_ENABLE_MASK |
+ R_V7M_MPU_CTRL_HFNMIENA_MASK |
+ R_V7M_MPU_CTRL_PRIVDEFENA_MASK);
+ tlb_flush(CPU(cpu));
+ break;
+ case 0xd98: /* MPU_RNR */
+ if (value >= cpu->pmsav7_dregion) {
+ qemu_log_mask(LOG_GUEST_ERROR, "MPU region out of range %"
+ PRIu32 "/%" PRIu32 "\n",
+ value, cpu->pmsav7_dregion);
+ } else {
+ cpu->env.cp15.c6_rgnr = value;
+ }
+ break;
+ case 0xd9c: /* MPU_RBAR */
+ case 0xda4: /* MPU_RBAR_A1 */
+ case 0xdac: /* MPU_RBAR_A2 */
+ case 0xdb4: /* MPU_RBAR_A3 */
+ {
+ int region;
+
+ if (value & (1 << 4)) {
+ /* VALID bit means use the region number specified in this
+ * value and also update MPU_RNR.REGION with that value.
+ */
+ region = extract32(value, 0, 4);
+ if (region >= cpu->pmsav7_dregion) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "MPU region out of range %u/%" PRIu32 "\n",
+ region, cpu->pmsav7_dregion);
+ return;
+ }
+ cpu->env.cp15.c6_rgnr = region;
+ } else {
+ region = cpu->env.cp15.c6_rgnr;
+ }
+
+ if (region >= cpu->pmsav7_dregion) {
+ return;
+ }
+
+ cpu->env.pmsav7.drbar[region] = value & ~0x1f;
+ tlb_flush(CPU(cpu));
+ break;
+ }
+ case 0xda0: /* MPU_RASR */
+ case 0xda8: /* MPU_RASR_A1 */
+ case 0xdb0: /* MPU_RASR_A2 */
+ case 0xdb8: /* MPU_RASR_A3 */
+ {
+ int region = cpu->env.cp15.c6_rgnr;
+
+ if (region >= cpu->pmsav7_dregion) {
+ return;
+ }
+
+ cpu->env.pmsav7.drsr[region] = value & 0xff3f;
+ cpu->env.pmsav7.dracr[region] = (value >> 16) & 0x173f;
+ tlb_flush(CPU(cpu));
+ break;
+ }
case 0xf00: /* Software Triggered Interrupt Register */
{
/* user mode can only write to STIR if CCR.USERSETMPEND permits it */
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 49b6d01..5bf706d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -7076,6 +7076,10 @@ static inline uint32_t regime_sctlr(CPUARMState *env, ARMMMUIdx mmu_idx)
static inline bool regime_translation_disabled(CPUARMState *env,
ARMMMUIdx mmu_idx)
{
+ if (arm_feature(env, ARM_FEATURE_M)) {
+ return !(env->v7m.mpu_ctrl & R_V7M_MPU_CTRL_ENABLE_MASK);
+ }
+
if (mmu_idx == ARMMMUIdx_S2NS) {
return (env->cp15.hcr_el2 & HCR_VM) == 0;
}
@@ -8199,6 +8203,25 @@ static inline void get_phys_addr_pmsav7_default(CPUARMState *env,
}
}
+static bool pmsav7_use_background_region(ARMCPU *cpu,
+ ARMMMUIdx mmu_idx, bool is_user)
+{
+ /* Return true if we should use the default memory map as a
+ * "background" region if there are no hits against any MPU regions.
+ */
+ CPUARMState *env = &cpu->env;
+
+ if (is_user) {
+ return false;
+ }
+
+ if (arm_feature(env, ARM_FEATURE_M)) {
+ return env->v7m.mpu_ctrl & R_V7M_MPU_CTRL_PRIVDEFENA_MASK;
+ } else {
+ return regime_sctlr(env, mmu_idx) & SCTLR_BR;
+ }
+}
+
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)
@@ -8286,7 +8309,7 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
}
if (n == -1) { /* no hits */
- if (is_user || !(regime_sctlr(env, mmu_idx) & SCTLR_BR)) {
+ if (!pmsav7_use_background_region(cpu, mmu_idx, is_user)) {
/* background fault */
*fsr = 0;
return true;
diff --git a/target/arm/machine.c b/target/arm/machine.c
index ac6b758..1a40469 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -99,8 +99,8 @@ static bool m_needed(void *opaque)
static const VMStateDescription vmstate_m = {
.name = "cpu/m",
- .version_id = 3,
- .minimum_version_id = 3,
+ .version_id = 4,
+ .minimum_version_id = 4,
.needed = m_needed,
.fields = (VMStateField[]) {
VMSTATE_UINT32(env.v7m.vecbase, ARMCPU),
@@ -112,6 +112,7 @@ static const VMStateDescription vmstate_m = {
VMSTATE_UINT32(env.v7m.dfsr, ARMCPU),
VMSTATE_UINT32(env.v7m.mmfar, ARMCPU),
VMSTATE_UINT32(env.v7m.bfar, ARMCPU),
+ VMSTATE_UINT32(env.v7m.mpu_ctrl, ARMCPU),
VMSTATE_INT32(env.v7m.exception, ARMCPU),
VMSTATE_END_OF_LIST()
}
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH 13/13] arm: Implement HFNMIENA support for M profile MPU
2017-04-25 12:06 [Qemu-devel] [PATCH 00/13] armv7m: Implement MPU support Peter Maydell
` (11 preceding siblings ...)
2017-04-25 12:07 ` [Qemu-devel] [PATCH 12/13] arm: add MPU support to M profile CPUs Peter Maydell
@ 2017-04-25 12:07 ` Peter Maydell
2017-05-30 14:05 ` [Qemu-devel] [Qemu-arm] [PATCH 00/13] armv7m: Implement MPU support Peter Maydell
13 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2017-04-25 12:07 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée, Alistair Francis
Implement HFNMIENA support for the M profile MPU. This bit controls
whether the MPU is treated as enabled when executing at execution
priorities of less than zero (in NMI, HardFault or with the FAULTMASK
bit set).
Doing this requires us to use a different MMU index for "running
at execution priority < 0", because we will have different
access permissions for that case versus the normal case.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/cpu.h | 24 +++++++++++++++++++++++-
target/arm/helper.c | 18 +++++++++++++++++-
target/arm/translate.c | 1 +
3 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index bbdd064..2e873e8 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2043,6 +2043,18 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
* for the accesses done as part of a stage 1 page table walk, rather than
* having to walk the stage 2 page table over and over.)
*
+ * R profile CPUs have an MPU, but can use the same set of MMU indexes
+ * as A profile. They only need to distinguish NS EL0 and NS EL1 (and
+ * NS EL2 if we ever model a Cortex-R52).
+ *
+ * M profile CPUs are rather different as they do not have a true MMU.
+ * They have the following different MMU indexes:
+ * User
+ * Privileged
+ * Execution priority negative (this is like privileged, but the
+ * MPU HFNMIENA bit means that it may have different access permission
+ * check results to normal privileged code, so can't share a TLB).
+ *
* The ARMMMUIdx and the mmu index value used by the core QEMU TLB code
* are not quite the same -- different CPU types (most notably M profile
* vs A/R profile) would like to use MMU indexes with different semantics,
@@ -2078,6 +2090,7 @@ typedef enum ARMMMUIdx {
ARMMMUIdx_S2NS = 6 | ARM_MMU_IDX_A,
ARMMMUIdx_MUser = 0 | ARM_MMU_IDX_M,
ARMMMUIdx_MPriv = 1 | ARM_MMU_IDX_M,
+ ARMMMUIdx_MNegPri = 2 | ARM_MMU_IDX_M,
/* Indexes below here don't have TLBs and are used only for AT system
* instructions or for the first stage of an S12 page table walk.
*/
@@ -2098,6 +2111,7 @@ typedef enum ARMMMUIdxBit {
ARMMMUIdxBit_S2NS = 1 << 6,
ARMMMUIdxBit_MUser = 1 << 0,
ARMMMUIdxBit_MPriv = 1 << 1,
+ ARMMMUIdxBit_MNegPri = 1 << 2,
} ARMMMUIdxBit;
#define MMU_USER_IDX 0
@@ -2123,7 +2137,7 @@ static inline int arm_mmu_idx_to_el(ARMMMUIdx mmu_idx)
case ARM_MMU_IDX_A:
return mmu_idx & 3;
case ARM_MMU_IDX_M:
- return mmu_idx & 1;
+ return mmu_idx == ARMMMUIdx_MUser ? 0 : 1;
default:
g_assert_not_reached();
}
@@ -2137,6 +2151,14 @@ static inline int cpu_mmu_index(CPUARMState *env, bool ifetch)
if (arm_feature(env, ARM_FEATURE_M)) {
ARMMMUIdx mmu_idx = el == 0 ? ARMMMUIdx_MUser : ARMMMUIdx_MPriv;
+ /* Execution priority is negative if FAULTMASK is set or
+ * we're in a HardFault or NMI handler.
+ */
+ if ((env->v7m.exception > 0 && env->v7m.exception <= 3)
+ || env->daif & PSTATE_F) {
+ return arm_to_core_mmu_idx(ARMMMUIdx_MNegPri);
+ }
+
return arm_to_core_mmu_idx(mmu_idx);
}
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 5bf706d..3aae52a 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -7037,6 +7037,7 @@ static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx)
case ARMMMUIdx_S1NSE0:
case ARMMMUIdx_S1NSE1:
case ARMMMUIdx_MPriv:
+ case ARMMMUIdx_MNegPri:
case ARMMMUIdx_MUser:
return 1;
default:
@@ -7055,6 +7056,7 @@ static inline bool regime_is_secure(CPUARMState *env, ARMMMUIdx mmu_idx)
case ARMMMUIdx_S1E2:
case ARMMMUIdx_S2NS:
case ARMMMUIdx_MPriv:
+ case ARMMMUIdx_MNegPri:
case ARMMMUIdx_MUser:
return false;
case ARMMMUIdx_S1E3:
@@ -7077,7 +7079,21 @@ static inline bool regime_translation_disabled(CPUARMState *env,
ARMMMUIdx mmu_idx)
{
if (arm_feature(env, ARM_FEATURE_M)) {
- return !(env->v7m.mpu_ctrl & R_V7M_MPU_CTRL_ENABLE_MASK);
+ switch (env->v7m.mpu_ctrl &
+ (R_V7M_MPU_CTRL_ENABLE_MASK | R_V7M_MPU_CTRL_HFNMIENA_MASK)) {
+ case R_V7M_MPU_CTRL_ENABLE_MASK:
+ /* Enabled, but not for HardFault and NMI */
+ return mmu_idx == ARMMMUIdx_MNegPri;
+ case R_V7M_MPU_CTRL_ENABLE_MASK | R_V7M_MPU_CTRL_HFNMIENA_MASK:
+ /* Enabled for all cases */
+ return false;
+ case 0:
+ default:
+ /* HFNMIENA set and ENABLE clear is UNPREDICTABLE, but
+ * we warned about that in armv7m_nvic.c when the guest set it.
+ */
+ return true;
+ }
}
if (mmu_idx == ARMMMUIdx_S2NS) {
diff --git a/target/arm/translate.c b/target/arm/translate.c
index ac905dd..ae6646c 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -163,6 +163,7 @@ static inline int get_a32_user_mem_index(DisasContext *s)
return arm_to_core_mmu_idx(ARMMMUIdx_S1SE0);
case ARMMMUIdx_MUser:
case ARMMMUIdx_MPriv:
+ case ARMMMUIdx_MNegPri:
return arm_to_core_mmu_idx(ARMMMUIdx_MUser);
case ARMMMUIdx_S2NS:
default:
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH 00/13] armv7m: Implement MPU support
2017-04-25 12:06 [Qemu-devel] [PATCH 00/13] armv7m: Implement MPU support Peter Maydell
` (12 preceding siblings ...)
2017-04-25 12:07 ` [Qemu-devel] [PATCH 13/13] arm: Implement HFNMIENA support for M profile MPU Peter Maydell
@ 2017-05-30 14:05 ` Peter Maydell
2017-05-30 16:02 ` Alistair Francis
13 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2017-05-30 14:05 UTC (permalink / raw)
To: qemu-arm, QEMU Developers
Cc: Alistair Francis, patches, Philippe Mathieu-Daudé, Alex Bennée
On 25 April 2017 at 13:06, Peter Maydell <peter.maydell@linaro.org> wrote:
> This patchset implements support for the MPU in our v7M cores.
> Support is on the same level as that for the R profile MPU: it works,
> but regions smaller than 1K in size are not supported. It likely
> has some missing corner-case features.
> I wanted to get this patchset out to the list before I go off
> on my break; I will come back and follow up on review comments
> when I get back in June.
The patchset has had review for about half the patches, and
no this-needs-fixing comments on the others, so I propose to
put it into target-arm.next.
Let me know if anybody would prefer me to hold off so they
can review the remaining patches in the set (2, 3, 9, 11, 12, 13).
thanks
-- PMM
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH 00/13] armv7m: Implement MPU support
2017-05-30 14:05 ` [Qemu-devel] [Qemu-arm] [PATCH 00/13] armv7m: Implement MPU support Peter Maydell
@ 2017-05-30 16:02 ` Alistair Francis
0 siblings, 0 replies; 36+ messages in thread
From: Alistair Francis @ 2017-05-30 16:02 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, QEMU Developers, patches, Alex Bennée,
Philippe Mathieu-Daudé,
Alistair Francis
On Tue, May 30, 2017 at 7:05 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 25 April 2017 at 13:06, Peter Maydell <peter.maydell@linaro.org> wrote:
>> This patchset implements support for the MPU in our v7M cores.
>> Support is on the same level as that for the R profile MPU: it works,
>> but regions smaller than 1K in size are not supported. It likely
>> has some missing corner-case features.
>
>> I wanted to get this patchset out to the list before I go off
>> on my break; I will come back and follow up on review comments
>> when I get back in June.
>
> The patchset has had review for about half the patches, and
> no this-needs-fixing comments on the others, so I propose to
> put it into target-arm.next.
>
> Let me know if anybody would prefer me to hold off so they
> can review the remaining patches in the set (2, 3, 9, 11, 12, 13).
All good from me, I'm happy if it's just applied.
Thanks,
Alistair
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 36+ messages in thread