qemu with hax to log dma reads & writes jcs.org/2018/11/12/vfio

target/arm: Stop assuming DBGDIDR always exists

The AArch32 DBGDIDR defines properties like the number of
breakpoints, watchpoints and context-matching comparators. On an
AArch64 CPU, the register may not even exist if AArch32 is not
supported at EL1.

Currently we hard-code use of DBGDIDR to identify the number of
breakpoints etc; this works for all our TCG CPUs, but will break if
we ever add an AArch64-only CPU. We also have an assert() that the
AArch32 and AArch64 registers match, which currently works only by
luck for KVM because we don't populate either of these ID registers
from the KVM vCPU and so they are both zero.

Clean this up so we have functions for finding the number
of breakpoints, watchpoints and context comparators which look
in the appropriate ID register.

This allows us to drop the "check that AArch64 and AArch32 agree
on the number of breakpoints etc" asserts:
* we no longer look at the AArch32 versions unless that's the
right place to be looking
* it's valid to have a CPU (eg AArch64-only) where they don't match
* we shouldn't have been asserting the validity of ID registers
in a codepath used with KVM anyway

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20200214175116.9164-11-peter.maydell@linaro.org

+57 -19
+7
target/arm/cpu.h
··· 1840 1840 FIELD(ID_DFR0, PERFMON, 24, 4) 1841 1841 FIELD(ID_DFR0, TRACEFILT, 28, 4) 1842 1842 1843 + FIELD(DBGDIDR, SE_IMP, 12, 1) 1844 + FIELD(DBGDIDR, NSUHD_IMP, 14, 1) 1845 + FIELD(DBGDIDR, VERSION, 16, 4) 1846 + FIELD(DBGDIDR, CTX_CMPS, 20, 4) 1847 + FIELD(DBGDIDR, BRPS, 24, 4) 1848 + FIELD(DBGDIDR, WRPS, 28, 4) 1849 + 1843 1850 FIELD(MVFR0, SIMDREG, 0, 4) 1844 1851 FIELD(MVFR0, FPSP, 4, 4) 1845 1852 FIELD(MVFR0, FPDP, 8, 4)
+3 -3
target/arm/debug_helper.c
··· 16 16 { 17 17 CPUARMState *env = &cpu->env; 18 18 uint64_t bcr = env->cp15.dbgbcr[lbn]; 19 - int brps = extract32(cpu->dbgdidr, 24, 4); 20 - int ctx_cmps = extract32(cpu->dbgdidr, 20, 4); 19 + int brps = arm_num_brps(cpu); 20 + int ctx_cmps = arm_num_ctx_cmps(cpu); 21 21 int bt; 22 22 uint32_t contextidr; 23 23 uint64_t hcr_el2; ··· 29 29 * case DBGWCR<n>_EL1.LBN must indicate that breakpoint). 30 30 * We choose the former. 31 31 */ 32 - if (lbn > brps || lbn < (brps - ctx_cmps)) { 32 + if (lbn >= brps || lbn < (brps - ctx_cmps)) { 33 33 return false; 34 34 } 35 35
+5 -16
target/arm/helper.c
··· 6256 6256 }; 6257 6257 6258 6258 /* Note that all these register fields hold "number of Xs minus 1". */ 6259 - brps = extract32(cpu->dbgdidr, 24, 4); 6260 - wrps = extract32(cpu->dbgdidr, 28, 4); 6261 - ctx_cmps = extract32(cpu->dbgdidr, 20, 4); 6259 + brps = arm_num_brps(cpu); 6260 + wrps = arm_num_wrps(cpu); 6261 + ctx_cmps = arm_num_ctx_cmps(cpu); 6262 6262 6263 6263 assert(ctx_cmps <= brps); 6264 6264 6265 - /* The DBGDIDR and ID_AA64DFR0_EL1 define various properties 6266 - * of the debug registers such as number of breakpoints; 6267 - * check that if they both exist then they agree. 6268 - */ 6269 - if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { 6270 - assert(FIELD_EX64(cpu->isar.id_aa64dfr0, ID_AA64DFR0, BRPS) == brps); 6271 - assert(FIELD_EX64(cpu->isar.id_aa64dfr0, ID_AA64DFR0, WRPS) == wrps); 6272 - assert(FIELD_EX64(cpu->isar.id_aa64dfr0, ID_AA64DFR0, CTX_CMPS) 6273 - == ctx_cmps); 6274 - } 6275 - 6276 6265 define_one_arm_cp_reg(cpu, &dbgdidr); 6277 6266 define_arm_cp_regs(cpu, debug_cp_reginfo); 6278 6267 ··· 6280 6269 define_arm_cp_regs(cpu, debug_lpae_cp_reginfo); 6281 6270 } 6282 6271 6283 - for (i = 0; i < brps + 1; i++) { 6272 + for (i = 0; i < brps; i++) { 6284 6273 ARMCPRegInfo dbgregs[] = { 6285 6274 { .name = "DBGBVR", .state = ARM_CP_STATE_BOTH, 6286 6275 .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = i, .opc2 = 4, ··· 6299 6288 define_arm_cp_regs(cpu, dbgregs); 6300 6289 } 6301 6290 6302 - for (i = 0; i < wrps + 1; i++) { 6291 + for (i = 0; i < wrps; i++) { 6303 6292 ARMCPRegInfo dbgregs[] = { 6304 6293 { .name = "DBGWVR", .state = ARM_CP_STATE_BOTH, 6305 6294 .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = i, .opc2 = 6,
+42
target/arm/internals.h
··· 931 931 } 932 932 } 933 933 934 + /** 935 + * arm_num_brps: Return number of implemented breakpoints. 936 + * Note that the ID register BRPS field is "number of bps - 1", 937 + * and we return the actual number of breakpoints. 938 + */ 939 + static inline int arm_num_brps(ARMCPU *cpu) 940 + { 941 + if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { 942 + return FIELD_EX64(cpu->isar.id_aa64dfr0, ID_AA64DFR0, BRPS) + 1; 943 + } else { 944 + return FIELD_EX32(cpu->dbgdidr, DBGDIDR, BRPS) + 1; 945 + } 946 + } 947 + 948 + /** 949 + * arm_num_wrps: Return number of implemented watchpoints. 950 + * Note that the ID register WRPS field is "number of wps - 1", 951 + * and we return the actual number of watchpoints. 952 + */ 953 + static inline int arm_num_wrps(ARMCPU *cpu) 954 + { 955 + if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { 956 + return FIELD_EX64(cpu->isar.id_aa64dfr0, ID_AA64DFR0, WRPS) + 1; 957 + } else { 958 + return FIELD_EX32(cpu->dbgdidr, DBGDIDR, WRPS) + 1; 959 + } 960 + } 961 + 962 + /** 963 + * arm_num_ctx_cmps: Return number of implemented context comparators. 964 + * Note that the ID register CTX_CMPS field is "number of cmps - 1", 965 + * and we return the actual number of comparators. 966 + */ 967 + static inline int arm_num_ctx_cmps(ARMCPU *cpu) 968 + { 969 + if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { 970 + return FIELD_EX64(cpu->isar.id_aa64dfr0, ID_AA64DFR0, CTX_CMPS) + 1; 971 + } else { 972 + return FIELD_EX32(cpu->dbgdidr, DBGDIDR, CTX_CMPS) + 1; 973 + } 974 + } 975 + 934 976 /* Note make_memop_idx reserves 4 bits for mmu_idx, and MO_BSWAP is bit 3. 935 977 * Thus a TCGMemOpIdx, without any MO_ALIGN bits, fits in 8 bits. 936 978 */