On Sun, 27 Nov 2022 at 13:21, <tobias.roeh...@rwth-aachen.de> wrote:
>
> From: Tobias Röhmel <tobias.roeh...@rwth-aachen.de>
>
> Signed-off-by: Tobias Röhmel <tobias.roeh...@rwth-aachen.de>
Only some minor fixes needed in this one.
> ---
> target/arm/cpu.c | 24 +++-
> target/arm/cpu.h | 6 +
> target/arm/helper.c | 299 +++++++++++++++++++++++++++++++++++++++++++
> target/arm/machine.c | 28 ++++
> 4 files changed, 356 insertions(+), 1 deletion(-)
> @@ -2001,8 +2009,9 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
> **errp)
> */
> if (!cpu->has_mpu) {
> cpu->pmsav7_dregion = 0;
> + cpu->pmsav8r_hdregion = 0;
> }
> - if (cpu->pmsav7_dregion == 0) {
> + if ((cpu->pmsav7_dregion == 0) && (cpu->pmsav8r_hdregion == 0)) {
> cpu->has_mpu = false;
> }
This leaves the door open for a CPU with 0 dregions but some hdregions.
That I think doesn't make sense, so we should just disallow it.
We can do that by combining these two if()s into one:
if (!cpu->has_mpu || cpu->pmsav7_dregion == 0) {
cpu->has_mpu = false;
cpu->pmsav7_dregion = 0;
cpu->pmsav8r_hdregion = 0;
}
> @@ -2030,6 +2039,19 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
> **errp)
> env->pmsav7.dracr = g_new0(uint32_t, nr);
> }
> }
> +
> + if (cpu->pmsav8r_hdregion > 0xFF) {
Lowercase for hex digits, please (matches the check on pmsav7_dregion).
> + error_setg(errp, "PMSAv8 MPU EL2 #regions invalid %" PRIu32,
> + cpu->pmsav8r_hdregion);
> + return;
> + }
> +
> + if (cpu->pmsav8r_hdregion) {
> + env->pmsav8.hprbar = g_new0(uint32_t,
> + cpu->pmsav8r_hdregion);
> + env->pmsav8.hprlar = g_new0(uint32_t,
> + cpu->pmsav8r_hdregion);
> + }
> }
>
> if (arm_feature(env, ARM_FEATURE_M_SECURITY)) {
> +static const ARMCPRegInfo pmsav8r_cp_reginfo[] = {
> + { .name = "PRBAR",
> + .cp = 15, .opc1 = 0, .crn = 6, .crm = 3, .opc2 = 0,
> + .access = PL1_RW, .type = ARM_CP_ALIAS | ARM_CP_NO_RAW,
> + .accessfn = access_tvm_trvm,
> + .readfn = prbar_read, .writefn = prbar_write },
> + { .name = "PRLAR",
> + .cp = 15, .opc1 = 0, .crn = 6, .crm = 3, .opc2 = 1,
> + .access = PL1_RW, .type = ARM_CP_ALIAS | ARM_CP_NO_RAW,
> + .accessfn = access_tvm_trvm,
> + .readfn = prlar_read, .writefn = prlar_write },
> + { .name = "PRSELR", .resetvalue = 0,
> + .cp = 15, .opc1 = 0, .crn = 6, .crm = 2, .opc2 = 1,
> + .access = PL1_RW, .accessfn = access_tvm_trvm,
> + .writefn = prselr_write,
> + .fieldoffset = offsetof(CPUARMState, pmsav7.rnr[M_REG_NS]) },
> + { .name = "HPRBAR", .resetvalue = 0,
> + .cp = 15, .opc1 = 4, .crn = 6, .crm = 3, .opc2 = 0,
> + .access = PL2_RW, .type = ARM_CP_ALIAS | ARM_CP_NO_RAW,
> + .readfn = hprbar_read, .writefn = hprbar_write },
> + { .name = "HPRLAR",
> + .cp = 15, .opc1 = 4, .crn = 6, .crm = 3, .opc2 = 1,
> + .access = PL2_RW, .type = ARM_CP_ALIAS | ARM_CP_NO_RAW,
> + .readfn = hprlar_read, .writefn = hprlar_write },
These 4 registers should be just ARM_CP_NO_RAW, not
ARM_CP_ALIAS | ARM_CP_NO_RAW.
> + { .name = "HPRSELR", .resetvalue = 0,
> + .cp = 15, .opc1 = 4, .crn = 6, .crm = 2, .opc2 = 1,
> + .access = PL2_RW,
> + .writefn = hprselr_write,
> + .fieldoffset = offsetof(CPUARMState, pmsav8.hprselr) },
> + { .name = "HPRENR",
> + .cp = 15, .opc1 = 4, .crn = 6, .crm = 1, .opc2 = 1,
> + .access = PL2_RW, .type = ARM_CP_ALIAS,
> + .readfn = hprenr_read, .writefn = hprenr_write },
> +};
> +
> static const ARMCPRegInfo pmsav7_cp_reginfo[] = {
> /* Reset for all these registers is handled in arm_cpu_reset(),
> * because the PMSAv7 is also used by M-profile CPUs, which do
> @@ -8166,6 +8382,13 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> .access = PL1_R, .type = ARM_CP_CONST,
> .resetvalue = cpu->pmsav7_dregion << 8
> };
> + /* HMPUIR is specific to PMSA V8 */
> + ARMCPRegInfo id_hmpuir_reginfo = {
> + .name = "HMPUIR",
> + .cp = 15, .opc1 = 4, .crn = 0, .crm = 0, .opc2 = 4,
> + .access = PL2_R, .type = ARM_CP_CONST,
> + .resetvalue = cpu->pmsav8r_hdregion
> + };
> static const ARMCPRegInfo crn0_wi_reginfo = {
> .name = "CRN0_WI", .cp = 15, .crn = 0, .crm = CP_ANY,
> .opc1 = CP_ANY, .opc2 = CP_ANY, .access = PL1_W,
> @@ -8208,6 +8431,71 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> define_arm_cp_regs(cpu, id_cp_reginfo);
> if (!arm_feature(env, ARM_FEATURE_PMSA)) {
> define_one_arm_cp_reg(cpu, &id_tlbtr_reginfo);
> + } else if (arm_feature(env, ARM_FEATURE_PMSA) &&
> + arm_feature(env, ARM_FEATURE_V8)) {
> + uint32_t i = 0;
> + g_autofree char *tmp_string_pr;
> + g_autofree char *tmp_string_hpr;
> +
> + define_one_arm_cp_reg(cpu, &id_mpuir_reginfo);
> + define_one_arm_cp_reg(cpu, &id_hmpuir_reginfo);
> + define_arm_cp_regs(cpu, pmsav8r_cp_reginfo);
> +
> + /* Register alias is only valid for first 32 indexes */
> + for (i = 0; i < MIN(cpu->pmsav7_dregion, 32); ++i) {
> + uint8_t crm = 0b1000 | extract32(i, 1, 3);
> + uint8_t opc1 = extract32(i, 4, 1);
> + uint8_t opc2 = extract32(i, 0, 1) << 2;
> +
> + tmp_string_pr = g_strdup_printf("PRBAR%u", i);
> + ARMCPRegInfo tmp_prbarn_reginfo = {
> + .name = tmp_string_pr, .type = ARM_CP_ALIAS |
> ARM_CP_NO_RAW,
All these registers hould be just ARM_CP_NO_RAW, not
ARM_CP_ALIAS | ARM_CP_NO_RAW.
> + .cp = 15, .opc1 = opc1, .crn = 6, .crm = crm, .opc2 =
> opc2,
> + .access = PL1_RW, .resetvalue = 0,
> + .accessfn = access_tvm_trvm,
> + .writefn = pmsav8r_regn_write, .readfn =
> pmsav8r_regn_read
> + };
> + define_one_arm_cp_reg(cpu, &tmp_prbarn_reginfo);
This is still leaking the name strings. If you don't want to
use g_autofree then you must free the strings yourself.
> +
> + opc2 = extract32(i, 0, 1) << 2 | 0x1;
> + tmp_string_pr = g_strdup_printf("PRLAR%u", i);
> + ARMCPRegInfo tmp_prlarn_reginfo = {
> + .name = tmp_string_pr, .type = ARM_CP_ALIAS |
> ARM_CP_NO_RAW,
> + .cp = 15, .opc1 = opc1, .crn = 6, .crm = crm, .opc2 =
> opc2,
> + .access = PL1_RW, .resetvalue = 0,
> + .accessfn = access_tvm_trvm,
> + .writefn = pmsav8r_regn_write, .readfn =
> pmsav8r_regn_read
> + };
> + define_one_arm_cp_reg(cpu, &tmp_prlarn_reginfo);
> + }
> +
> + /* Register alias is only valid for first 32 indexes */
> + for (i = 0; i < MIN(cpu->pmsav8r_hdregion, 32); ++i) {
> + uint8_t crm = 0b1000 | extract32(i, 1, 3);
> + uint8_t opc1 = 0b100 | extract32(i, 4, 1);
> + uint8_t opc2 = extract32(i, 0, 1) << 2;
> +
> + tmp_string_hpr = g_strdup_printf("HPRBAR%u", i);
> + ARMCPRegInfo tmp_hprbarn_reginfo = {
> + .name = tmp_string_hpr,
> + .type = ARM_CP_ALIAS | ARM_CP_NO_RAW,
> + .cp = 15, .opc1 = opc1, .crn = 6, .crm = crm, .opc2 =
> opc2,
> + .access = PL2_RW, .resetvalue = 0,
> + .writefn = pmsav8r_regn_write, .readfn =
> pmsav8r_regn_read
> + };
> + define_one_arm_cp_reg(cpu, &tmp_hprbarn_reginfo);
> +
> + opc2 = extract32(i, 0, 1) << 2 | 0x1;
> + tmp_string_hpr = g_strdup_printf("HPRLAR%u", i);
> + ARMCPRegInfo tmp_hprlarn_reginfo = {
> + .name = tmp_string_hpr,
> + .type = ARM_CP_ALIAS | ARM_CP_NO_RAW,
> + .cp = 15, .opc1 = opc1, .crn = 6, .crm = crm, .opc2 =
> opc2,
> + .access = PL2_RW, .resetvalue = 0,
> + .writefn = pmsav8r_regn_write, .readfn =
> pmsav8r_regn_read
> + };
> + define_one_arm_cp_reg(cpu, &tmp_hprlarn_reginfo);
> + }
> } else if (arm_feature(env, ARM_FEATURE_V7)) {
> define_one_arm_cp_reg(cpu, &id_mpuir_reginfo);
> }
thanks
-- PMM
