Andrew Jones <drjones@xxxxxxxxxx> writes:
> On Thu, May 18, 2017 at 05:02:08PM +0100, Alex Bennée wrote:
>> Currently this is a fairly simple test that reads the debug register
>> state and attempts to write/read-back the watchpoint and breakpoint
>> registers.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@xxxxxxxxxx>
>> ---
>> arm/Makefile.common | 1 +
>> arm/debugtest.c | 671 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 672 insertions(+)
>> create mode 100644 arm/debugtest.c
>>
>> diff --git a/arm/Makefile.common b/arm/Makefile.common
>> index 74c7394..1d7dec1 100644
>> --- a/arm/Makefile.common
>> +++ b/arm/Makefile.common
>> @@ -15,6 +15,7 @@ tests-common += $(TEST_DIR)/pci-test.flat
>> tests-common += $(TEST_DIR)/pmu.flat
>> tests-common += $(TEST_DIR)/gic.flat
>> tests-common += $(TEST_DIR)/psci.flat
>> +tests-common += $(TEST_DIR)/debugtest.flat
>>
>> tests-all = $(tests-common) $(tests)
>> all: $(tests-all)
>> diff --git a/arm/debugtest.c b/arm/debugtest.c
>> new file mode 100644
>> index 0000000..4400bf3
>> --- /dev/null
>> +++ b/arm/debugtest.c
>> @@ -0,0 +1,671 @@
>> +/*
>> + * Test guest access to debug registers.
>> + *
>> + * Copyright (C) 2015, Linaro Ltd, Alex Bennée <alex.bennee@xxxxxxxxxx>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2.
>> + */
>> +
>> +#include <libcflat.h>
>> +#include <asm/psci.h>
>> +#include <asm/smp.h>
>> +#include <asm/cpumask.h>
>> +#include <asm/barrier.h>
>> +#include "asm/sysreg.h"
>> +#include "asm/processor.h"
>
> nit: mixing <> and ""
>
>> +
>> +
>> +
>
> nit: excessive blank lines
>
>> +#if defined(__arm__)
>> +
>> +#define ID_DFR0 __ACCESS_CP15(c0, 0, c1, 2)
>> +
>> +/* __ACCESS_CP14(CRn, Op1, CRm, Op2) */
>> +#define DBGDIDR __ACCESS_CP14(c0, 0, c0, 0)
>> +#define DBGDSCR __ACCESS_CP14(c0, 0, c2, 2)
>> +
>> +#define DBGBCR0 __ACCESS_CP14(c0, 0, c0, 5)
>> +#define DBGBCR1 __ACCESS_CP14(c0, 0, c1, 5)
>> +#define DBGBCR2 __ACCESS_CP14(c0, 0, c2, 5)
>> +#define DBGBCR3 __ACCESS_CP14(c0, 0, c3, 5)
>> +#define DBGBCR4 __ACCESS_CP14(c0, 0, c4, 5)
>> +#define DBGBCR5 __ACCESS_CP14(c0, 0, c5, 5)
>> +#define DBGBCR6 __ACCESS_CP14(c0, 0, c6, 5)
>> +#define DBGBCR7 __ACCESS_CP14(c0, 0, c7, 5)
>> +#define DBGBCR8 __ACCESS_CP14(c0, 0, c8, 5)
>> +#define DBGBCR9 __ACCESS_CP14(c0, 0, c9, 5)
>> +#define DBGBCR10 __ACCESS_CP14(c0, 0, c10, 5)
>> +#define DBGBCR11 __ACCESS_CP14(c0, 0, c11, 5)
>> +#define DBGBCR12 __ACCESS_CP14(c0, 0, c12, 5)
>> +#define DBGBCR13 __ACCESS_CP14(c0, 0, c13, 5)
>> +#define DBGBCR14 __ACCESS_CP14(c0, 0, c14, 5)
>> +#define DBGBCR15 __ACCESS_CP14(c0, 0, c15, 5)
>> +#define DBGBVR0 __ACCESS_CP14(c0, 0, c0, 4)
>> +#define DBGBVR1 __ACCESS_CP14(c0, 0, c1, 4)
>> +#define DBGBVR2 __ACCESS_CP14(c0, 0, c2, 4)
>> +#define DBGBVR3 __ACCESS_CP14(c0, 0, c3, 4)
>> +#define DBGBVR4 __ACCESS_CP14(c0, 0, c4, 4)
>> +#define DBGBVR5 __ACCESS_CP14(c0, 0, c5, 4)
>> +#define DBGBVR6 __ACCESS_CP14(c0, 0, c6, 4)
>> +#define DBGBVR7 __ACCESS_CP14(c0, 0, c7, 4)
>> +#define DBGBVR8 __ACCESS_CP14(c0, 0, c8, 4)
>> +#define DBGBVR9 __ACCESS_CP14(c0, 0, c9, 4)
>> +#define DBGBVR10 __ACCESS_CP14(c0, 0, c10, 4)
>> +#define DBGBVR11 __ACCESS_CP14(c0, 0, c11, 4)
>> +#define DBGBVR12 __ACCESS_CP14(c0, 0, c12, 4)
>> +#define DBGBVR13 __ACCESS_CP14(c0, 0, c13, 4)
>> +#define DBGBVR14 __ACCESS_CP14(c0, 0, c14, 4)
>> +#define DBGBVR15 __ACCESS_CP14(c0, 0, c15, 4)
>> +#define DBGWCR0 __ACCESS_CP14(c0, 0, c0, 7)
>> +#define DBGWCR1 __ACCESS_CP14(c0, 0, c1, 7)
>> +#define DBGWCR2 __ACCESS_CP14(c0, 0, c2, 7)
>> +#define DBGWCR3 __ACCESS_CP14(c0, 0, c3, 7)
>> +#define DBGWCR4 __ACCESS_CP14(c0, 0, c4, 7)
>> +#define DBGWCR5 __ACCESS_CP14(c0, 0, c5, 7)
>> +#define DBGWCR6 __ACCESS_CP14(c0, 0, c6, 7)
>> +#define DBGWCR7 __ACCESS_CP14(c0, 0, c7, 7)
>> +#define DBGWCR8 __ACCESS_CP14(c0, 0, c8, 7)
>> +#define DBGWCR9 __ACCESS_CP14(c0, 0, c9, 7)
>> +#define DBGWCR10 __ACCESS_CP14(c0, 0, c10, 7)
>> +#define DBGWCR11 __ACCESS_CP14(c0, 0, c11, 7)
>> +#define DBGWCR12 __ACCESS_CP14(c0, 0, c12, 7)
>> +#define DBGWCR13 __ACCESS_CP14(c0, 0, c13, 7)
>> +#define DBGWCR14 __ACCESS_CP14(c0, 0, c14, 7)
>> +#define DBGWCR15 __ACCESS_CP14(c0, 0, c15, 7)
>> +#define DBGWVR0 __ACCESS_CP14(c0, 0, c0, 6)
>> +#define DBGWVR1 __ACCESS_CP14(c0, 0, c1, 6)
>> +#define DBGWVR2 __ACCESS_CP14(c0, 0, c2, 6)
>> +#define DBGWVR3 __ACCESS_CP14(c0, 0, c3, 6)
>> +#define DBGWVR4 __ACCESS_CP14(c0, 0, c4, 6)
>> +#define DBGWVR5 __ACCESS_CP14(c0, 0, c5, 6)
>> +#define DBGWVR6 __ACCESS_CP14(c0, 0, c6, 6)
>> +#define DBGWVR7 __ACCESS_CP14(c0, 0, c7, 6)
>> +#define DBGWVR8 __ACCESS_CP14(c0, 0, c8, 6)
>> +#define DBGWVR9 __ACCESS_CP14(c0, 0, c9, 6)
>> +#define DBGWVR10 __ACCESS_CP14(c0, 0, c10, 6)
>> +#define DBGWVR11 __ACCESS_CP14(c0, 0, c11, 6)
>> +#define DBGWVR12 __ACCESS_CP14(c0, 0, c12, 6)
>> +#define DBGWVR13 __ACCESS_CP14(c0, 0, c13, 6)
>> +#define DBGWVR14 __ACCESS_CP14(c0, 0, c14, 6)
>> +#define DBGWVR15 __ACCESS_CP14(c0, 0, c15, 6)
>> +
>> +#define read_debug_bcr(reg) read_sysreg(DBGBCR ## reg)
>> +#define read_debug_bvr(reg) read_sysreg(DBGBVR ## reg)
>> +#define read_debug_wcr(reg) read_sysreg(DBGWCR ## reg)
>> +#define read_debug_wvr(reg) read_sysreg(DBGWVR ## reg)
>> +
>> +#define write_debug_bcr(val, reg) write_sysreg(val, DBGBCR ## reg)
>> +#define write_debug_bvr(val, reg) write_sysreg(val, DBGBVR ## reg)
>> +#define write_debug_wcr(val, reg) write_sysreg(val, DBGWCR ## reg)
>> +#define write_debug_wvr(val, reg) write_sysreg(val, DBGWVR ## reg)
>> +
>> +#define PRIxDBGADDR PRIx32
>> +
>> +struct dbgregs {
>> + uint32_t dbgbcr;
>> + uint32_t dbgbvr;
>> + uint32_t dbgwcr;
>> + uint32_t dbgwvr;
>> +};
>> +
>> +#elif defined(__aarch64__)
>> +
>> +#define read_debug_bcr(reg) read_sysreg(dbgbcr ## reg ## _el1)
>> +#define read_debug_bvr(reg) read_sysreg(dbgbvr ## reg ## _el1)
>> +#define read_debug_wcr(reg) read_sysreg(dbgwcr ## reg ## _el1)
>> +#define read_debug_wvr(reg) read_sysreg(dbgwvr ## reg ## _el1)
>> +
>> +#define write_debug_bcr(val, reg) write_sysreg(val, dbgbcr ## reg ## _el1)
>> +#define write_debug_bvr(val, reg) write_sysreg(val, dbgbvr ## reg ## _el1)
>> +#define write_debug_wcr(val, reg) write_sysreg(val, dbgwcr ## reg ## _el1)
>> +#define write_debug_wvr(val, reg) write_sysreg(val, dbgwvr ## reg ## _el1)
>> +
>> +#define PRIxDBGADDR PRIx64
>> +
>> +struct dbgregs {
>> + uint32_t dbgbcr;
>> + uint64_t dbgbvr;
>> + uint32_t dbgwcr;
>> + uint64_t dbgwvr;
>> +};
>> +
>> +#endif
>> +
>> +#define MIN(a, b) ((a) < (b) ? (a) : (b))
>
> MIN() is already defined in libcflat.h
>
>> +
>> +static cpumask_t smp_test_complete;
>> +static int nbp, nwp;
>> +
>> +static void read_dbgb(int n, struct dbgregs *array)
>> +{
>> + switch (n-1) {
>> + case 15:
>> + array[15].dbgbcr = read_debug_bcr(15);
>> + array[15].dbgbvr = read_debug_bvr(15);
>> + case 14:
>> + array[14].dbgbcr = read_debug_bcr(14);
>> + array[14].dbgbvr = read_debug_bvr(14);
>> + case 13:
>> + array[13].dbgbcr = read_debug_bcr(13);
>> + array[13].dbgbvr = read_debug_bvr(13);
>> + case 12:
>> + array[12].dbgbcr = read_debug_bcr(12);
>> + array[12].dbgbvr = read_debug_bvr(12);
>> + case 11:
>> + array[11].dbgbcr = read_debug_bcr(11);
>> + array[11].dbgbvr = read_debug_bvr(11);
>> + case 10:
>> + array[10].dbgbcr = read_debug_bcr(10);
>> + array[10].dbgbvr = read_debug_bvr(10);
>> + case 9:
>> + array[9].dbgbcr = read_debug_bcr(9);
>> + array[9].dbgbvr = read_debug_bvr(9);
>> + case 8:
>> + array[8].dbgbcr = read_debug_bcr(8);
>> + array[8].dbgbvr = read_debug_bvr(8);
>> + case 7:
>> + array[7].dbgbcr = read_debug_bcr(7);
>> + array[7].dbgbvr = read_debug_bvr(7);
>> + case 6:
>> + array[6].dbgbcr = read_debug_bcr(6);
>> + array[6].dbgbvr = read_debug_bvr(6);
>> + case 5:
>> + array[5].dbgbcr = read_debug_bcr(5);
>> + array[5].dbgbvr = read_debug_bvr(5);
>> + case 4:
>> + array[4].dbgbcr = read_debug_bcr(4);
>> + array[4].dbgbvr = read_debug_bvr(4);
>> + case 3:
>> + array[3].dbgbcr = read_debug_bcr(3);
>> + array[3].dbgbvr = read_debug_bvr(3);
>> + case 2:
>> + array[2].dbgbcr = read_debug_bcr(2);
>> + array[2].dbgbvr = read_debug_bvr(2);
>> + case 1:
>> + array[1].dbgbcr = read_debug_bcr(1);
>> + array[1].dbgbvr = read_debug_bvr(1);
>> + case 0:
>> + array[0].dbgbcr = read_debug_bcr(0);
>> + array[0].dbgbvr = read_debug_bvr(0);
>> + break;
>> + default:
>> + break;
>> + }
>> +}
>> +
>> +static void write_dbgb(int n, struct dbgregs *array)
>> +{
>> + switch (n-1) {
>> + case 15:
>> + write_debug_bcr(array[15].dbgbcr, 15);
>> + write_debug_bvr(array[15].dbgbvr, 15);
>> + case 14:
>> + write_debug_bcr(array[14].dbgbcr, 14);
>> + write_debug_bvr(array[14].dbgbvr, 14);
>> + case 13:
>> + write_debug_bcr(array[13].dbgbcr, 13);
>> + write_debug_bvr(array[13].dbgbvr, 13);
>> + case 12:
>> + write_debug_bcr(array[12].dbgbcr, 12);
>> + write_debug_bvr(array[12].dbgbvr, 12);
>> + case 11:
>> + write_debug_bcr(array[11].dbgbcr, 11);
>> + write_debug_bvr(array[11].dbgbvr, 11);
>> + case 10:
>> + write_debug_bcr(array[10].dbgbcr, 10);
>> + write_debug_bvr(array[10].dbgbvr, 10);
>> + case 9:
>> + write_debug_bcr(array[9].dbgbcr, 9);
>> + write_debug_bvr(array[9].dbgbvr, 9);
>> + case 8:
>> + write_debug_bcr(array[8].dbgbcr, 8);
>> + write_debug_bvr(array[8].dbgbvr, 8);
>> + case 7:
>> + write_debug_bcr(array[7].dbgbcr, 7);
>> + write_debug_bvr(array[7].dbgbvr, 7);
>> + case 6:
>> + write_debug_bcr(array[6].dbgbcr, 6);
>> + write_debug_bvr(array[6].dbgbvr, 6);
>> + case 5:
>> + write_debug_bcr(array[5].dbgbcr, 5);
>> + write_debug_bvr(array[5].dbgbvr, 5);
>> + case 4:
>> + write_debug_bcr(array[4].dbgbcr, 4);
>> + write_debug_bvr(array[4].dbgbvr, 4);
>> + case 3:
>> + write_debug_bcr(array[3].dbgbcr, 3);
>> + write_debug_bvr(array[3].dbgbvr, 3);
>> + case 2:
>> + write_debug_bcr(array[2].dbgbcr, 2);
>> + write_debug_bvr(array[2].dbgbvr, 2);
>> + case 1:
>> + write_debug_bcr(array[1].dbgbcr, 1);
>> + write_debug_bvr(array[1].dbgbvr, 1);
>> + case 0:
>> + write_debug_bcr(array[0].dbgbcr, 0);
>> + write_debug_bvr(array[0].dbgbvr, 0);
>> + break;
>> + default:
>> + break;
>> + }
>> +}
>> +
>> +static void read_dbgw(int n, struct dbgregs *array)
>> +{
>> + switch (n-1) {
>> + case 15:
>> + array[15].dbgwcr = read_debug_wcr(15);
>> + array[15].dbgwvr = read_debug_wvr(15);
>> + case 14:
>> + array[14].dbgwcr = read_debug_wcr(14);
>> + array[14].dbgwvr = read_debug_wvr(14);
>> + case 13:
>> + array[13].dbgwcr = read_debug_wcr(13);
>> + array[13].dbgwvr = read_debug_wvr(13);
>> + case 12:
>> + array[12].dbgwcr = read_debug_wcr(12);
>> + array[12].dbgwvr = read_debug_wvr(12);
>> + case 11:
>> + array[11].dbgwcr = read_debug_wcr(11);
>> + array[11].dbgwvr = read_debug_wvr(11);
>> + case 10:
>> + array[10].dbgwcr = read_debug_wcr(10);
>> + array[10].dbgwvr = read_debug_wvr(10);
>> + case 9:
>> + array[9].dbgwcr = read_debug_wcr(9);
>> + array[9].dbgwvr = read_debug_wvr(9);
>> + case 8:
>> + array[8].dbgwcr = read_debug_wcr(8);
>> + array[8].dbgwvr = read_debug_wvr(8);
>> + case 7:
>> + array[7].dbgwcr = read_debug_wcr(7);
>> + array[7].dbgwvr = read_debug_wvr(7);
>> + case 6:
>> + array[6].dbgwcr = read_debug_wcr(6);
>> + array[6].dbgwvr = read_debug_wvr(6);
>> + case 5:
>> + array[5].dbgwcr = read_debug_wcr(5);
>> + array[5].dbgwvr = read_debug_wvr(5);
>> + case 4:
>> + array[4].dbgwcr = read_debug_wcr(4);
>> + array[4].dbgwvr = read_debug_wvr(4);
>> + case 3:
>> + array[3].dbgwcr = read_debug_wcr(3);
>> + array[3].dbgwvr = read_debug_wvr(3);
>> + case 2:
>> + array[2].dbgwcr = read_debug_wcr(2);
>> + array[2].dbgwvr = read_debug_wvr(2);
>> + case 1:
>> + array[1].dbgwcr = read_debug_wcr(1);
>> + array[1].dbgwvr = read_debug_wvr(1);
>> + case 0:
>> + array[0].dbgwcr = read_debug_wcr(0);
>> + array[0].dbgwvr = read_debug_wvr(0);
>> + break;
>> + default:
>> + break;
>> + }
>> +}
>> +
>> +static void write_dbgw(int n, struct dbgregs *array)
>> +{
>> + switch (n-1) {
>> + case 15:
>> + write_debug_wcr(array[15].dbgwcr, 15);
>> + write_debug_wvr(array[15].dbgwvr, 15);
>> + case 14:
>> + write_debug_wcr(array[14].dbgwcr, 14);
>> + write_debug_wvr(array[14].dbgwvr, 14);
>> + case 13:
>> + write_debug_wcr(array[13].dbgwcr, 13);
>> + write_debug_wvr(array[13].dbgwvr, 13);
>> + case 12:
>> + write_debug_wcr(array[12].dbgwcr, 12);
>> + write_debug_wvr(array[12].dbgwvr, 12);
>> + case 11:
>> + write_debug_wcr(array[11].dbgwcr, 11);
>> + write_debug_wvr(array[11].dbgwvr, 11);
>> + case 10:
>> + write_debug_wcr(array[10].dbgwcr, 10);
>> + write_debug_wvr(array[10].dbgwvr, 10);
>> + case 9:
>> + write_debug_wcr(array[9].dbgwcr, 9);
>> + write_debug_wvr(array[9].dbgwvr, 9);
>> + case 8:
>> + write_debug_wcr(array[8].dbgwcr, 8);
>> + write_debug_wvr(array[8].dbgwvr, 8);
>> + case 7:
>> + write_debug_wcr(array[7].dbgwcr, 7);
>> + write_debug_wvr(array[7].dbgwvr, 7);
>> + case 6:
>> + write_debug_wcr(array[6].dbgwcr, 6);
>> + write_debug_wvr(array[6].dbgwvr, 6);
>> + case 5:
>> + write_debug_wcr(array[5].dbgwcr, 5);
>> + write_debug_wvr(array[5].dbgwvr, 5);
>> + case 4:
>> + write_debug_wcr(array[4].dbgwcr, 4);
>> + write_debug_wvr(array[4].dbgwvr, 4);
>> + case 3:
>> + write_debug_wcr(array[3].dbgwcr, 3);
>> + write_debug_wvr(array[3].dbgwvr, 3);
>> + case 2:
>> + write_debug_wcr(array[2].dbgwcr, 2);
>> + write_debug_wvr(array[2].dbgwvr, 2);
>> + case 1:
>> + write_debug_wcr(array[1].dbgwcr, 1);
>> + write_debug_wvr(array[1].dbgwvr, 1);
>> + case 0:
>> + write_debug_wcr(array[0].dbgwcr, 0);
>> + write_debug_wvr(array[0].dbgwvr, 0);
>> + break;
>> + default:
>> + break;
>> + }
>> +}
>
> Any reason not to implement the above functions with
> loops? E.g.
>
> while (--n >= 0) {
> write_debug_wcr(array[n].dbgwcr, n);
> write_debug_wvr(array[n].dbgwvr, n);
> }
Well we are using macro pasting here so we need to instantiate all
possible asm fragments.
>
>> +
>> +static void dump_regs(int cpu, struct dbgregs *array)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < nbp; i++)
>> + printf("CPU%d: B%d 0x%08x:0x%" PRIxDBGADDR "\n", cpu, i,
>> + array[i].dbgbcr, array[i].dbgbvr);
>> + for (i = 0; i < nwp; i++)
>> + printf("CPU%d: W%d 0x%08x:0x%" PRIxDBGADDR "\n", cpu, i,
>> + array[i].dbgwcr, array[i].dbgwvr);
>> +}
>> +
>> +static int compare_regs(struct dbgregs *initial, struct dbgregs *current)
>> +{
>> + int i, errors = 0;
>> +
>> + for (i = 0; i < nbp; i++) {
>> + if (initial[i].dbgbcr != current[i].dbgbcr) {
>> + printf("ERROR DBGBCR%d: %" PRIx32 "!=%" PRIx32 "\n",
>> + i, initial[i].dbgbcr, current[i].dbgbcr);
>> + errors++;
>> + }
>> + if (initial[i].dbgbvr != current[i].dbgbvr) {
>> + printf("ERROR DBGBVR%d: %" PRIxDBGADDR "!=%" PRIxDBGADDR "\n",
>> + i, initial[i].dbgbvr, current[i].dbgbvr);
>> + errors++;
>> + }
>> + }
>> +
>> + for (i = 0; i < nwp; i++) {
>> + if (initial[i].dbgwcr != current[i].dbgwcr) {
>> + printf("ERROR DBGWCR%d: %" PRIx32 "!=%" PRIx32 "\n",
>> + i, initial[i].dbgwcr, current[i].dbgwcr);
>> + errors++;
>> + }
>> + if (initial[i].dbgwvr != current[i].dbgwvr) {
>> + printf("ERROR DBWVR%d: %" PRIxDBGADDR "!=%" PRIxDBGADDR "\n",
>> + i, initial[i].dbgwvr, current[i].dbgwvr);
>> + errors++;
>> + }
>> + }
>
> Shouldn't these error messages also output the CPU?
>
>> +
>> + return errors;
>> +}
>> +
>> +static void test_debug_regs(void)
>> +{
>> + int errors = 0;
>> + int cpu = smp_processor_id();
>> + int i;
>> + struct dbgregs initial[16], current[16];
>> +
>> + printf("CPU%d online\n", cpu);
>> +
>> + printf("Reading Initial Values\n");
>> + read_dbgb(nbp, &initial[0]);
>> + read_dbgw(nwp, &initial[0]);
>> + dump_regs(cpu, &initial[0]);
>> +
>> + printf("Setting simple BPs and WPs\n");
>> + for (i = 0; i < nbp; i++) {
>> + initial[i].dbgbcr = (0x3 << 1) | (0xf << 5) ; /* E=0 */
>> + initial[i].dbgbvr = ~0x3;
>> + }
>> + for (i = 0; i < nwp; i++) {
>> + initial[i].dbgwcr = (0x3 << 1) | (0x3 << 3); /* E=0 */
>> + initial[i].dbgwvr = ~0x7;
>> + }
>> + write_dbgb(nbp, &initial[0]);
>> + write_dbgw(nwp, &initial[0]);
>> +
>> + printf("Reading values back\n");
>
> The above informational printfs will print for each cpu and in whatever
> order the cpus are run, potentially giving strangely ordered output. Do
> we really need them?
>
>> + read_dbgb(nbp, ¤t[0]);
>> + read_dbgw(nwp, ¤t[0]);
>> +
>> + errors += compare_regs(&initial[0], ¤t[0]);
>> +
>> + report("CPU%d: Done - Errors: %d\n", errors == 0, cpu, errors);
>> +
>> + if (errors)
>> + dump_regs(cpu, ¤t[0]);
>> +
>> + cpumask_set_cpu(cpu, &smp_test_complete);
>> + if (cpu != 0)
>> + halt();
>> +}
>> +
>> +static char *split_var(char *s, long *val)
>> +{
>> + char *p;
>> +
>> + p = strchr(s, '=');
>> + if (!p)
>> + return NULL;
>> +
>> + *val = atol(p+1);
>> + *p = '\0';
>> +
>> + return s;
>> +}
>
> We have parse_keyval() in lib/util.c. It isn't an awesome design,
> but it's similar to this old split_var() does. I think Radim has
> plans to improve on it, at which time we'll fix this test up too.
> In short, let's use parse_keyval here for now and not add this
> function.
>
>> +
>> +#if defined(__arm__)
>> +static void read_debug_feature_regs(void)
>> +{
>> + uint32_t id_dfr0 = read_sysreg(ID_DFR0);
>> + uint32_t dbgdidr = read_sysreg(DBGDIDR);
>> + uint32_t dbgdscr = read_sysreg(DBGDSCR);
>> + char *pmu, *dm_m, *mmt_m, *cpt_m, *mmd_m, *cpsd_m, *cpd_m;
>> +
>> + printf("ID_DFR0: 0x%" PRIx32 "\n", id_dfr0);
>> +
>> + /* PMU support */
>> + switch ((id_dfr0 >> 24) & 0xf) {
>> + case 0:
>> + pmu = "No PMUv2";
>> + break;
>> + case 1:
>> + pmu = "PMUv1";
>> + break;
>> + case 2:
>> + pmu = "PMUv2";
>> + break;
>> + case 3:
>> + pmu = "No PMU Extensions";
>> + break;
>> + default:
>> + pmu = "Invalid";
>> + break;
>> + }
>> + printf(" PMU: %s\n", pmu);
>> +
>> + /* M-profile debug model */
>> + switch ((id_dfr0 >> 20) & 0xf) {
>> + case 0:
>> + dm_m = "Not supported";
>> + break;
>> + case 1:
>> + dm_m = "M-profile debug with memory mapped access";
>> + break;
>> + default:
>> + dm_m = "Invalid";
>> + break;
>> + }
>> + printf(" M-profile debug: %s\n", dm_m);
>> +
>> + /* Memory mapped trace model */
>> + switch ((id_dfr0 >> 16) & 0xf) {
>> + case 0:
>> + mmt_m = "Not supported";
>> + break;
>> + case 1:
>> + mmt_m = "Supported with memory mapped access";
>> + break;
>> + default:
>> + mmt_m = "Invalid";
>> + break;
>> + }
>> + printf(" Trace Model: %s\n", mmt_m);
>> +
>> + /* Co-processor trace model */
>> + switch ((id_dfr0 >> 12) & 0xf) {
>> + case 0:
>> + cpt_m = "Not supported";
>> + break;
>> + case 1:
>> + cpt_m = "Supported with CP14access";
>> + break;
>> + default:
>> + cpt_m = "Invalid";
>> + break;
>> + }
>> + printf(" Trace Model: %s\n", cpt_m);
>> +
>> + /* Memory mapped debug model, A and R profiles */
>> + switch ((id_dfr0 >> 8) & 0xf) {
>> + case 0:
>> + mmd_m = "Not supported or pre-ARMv6";
>> + break;
>> + case 4:
>> + mmd_m = "Supported v7 Debug Architecture";
>> + break;
>> + case 5:
>> + mmd_m = "Supported v7.1 Debug Architecture";
>> + break;
>> + default:
>> + mmd_m = "Invalid";
>> + break;
>> + }
>> + printf(" Debug Model: %s\n", mmd_m);
>> +
>> + /* Co-processor secure debug model */
>> + switch ((id_dfr0 >> 4) & 0xf) {
>> + case 0:
>> + cpsd_m = "Not supported";
>> + break;
>> + case 2:
>> + cpsd_m = "Supported v6 Debug Architecture";
>> + break;
>> + case 3:
>> + cpsd_m = "Supported v6.1 Debug Architecture";
>> + break;
>> + case 4:
>> + cpsd_m = "Supported v7 Debug Architecture";
>> + break;
>> + case 5:
>> + cpsd_m = "Supported v7.1 Debug Architecture";
>> + break;
>> + default:
>> + cpsd_m = "Invalid";
>> + break;
>> + }
>> + printf(" Co-processor Secure Debug Model: %s\n", cpsd_m);
>> +
>> + /* Co-processor debug model */
>> + switch ((id_dfr0 >> 0) & 0xf) {
>> + case 0:
>> + cpd_m = "Not supported";
>> + break;
>> + case 2:
>> + cpd_m = "Supported v6 Debug Architecture";
>> + break;
>> + case 3:
>> + cpd_m = "Supported v6.1 Debug Architecture";
>> + break;
>> + case 4:
>> + cpd_m = "Supported v7 Debug Architecture";
>> + break;
>> + case 5:
>> + cpd_m = "Supported v7.1 Debug Architecture";
>> + break;
>> + default:
>> + cpd_m = "Invalid";
>> + break;
>> + }
>> + printf(" Co-processor Debug Model: %s\n", cpd_m);
>> +
>> + /* DBGIDR, num registers */
>> + nwp = (dbgdidr >> 28) + 1;
>> + /* BRPs == 0 is reserved, else it is BRPs + 1 */
>> + nbp = ((dbgdidr >> 24) & 0xf);
>> + if (nbp > 0)
>> + nbp += 1;
>> +
>> + printf("DBGDIDR: 0x%" PRIx32 " nwp:%d nbp:%d\n", dbgdidr, nwp, nbp);
>> +
> nit: extra blank line
>> + printf("DBGDSCR: 0x%" PRIx32 "\n", dbgdscr);
>
> You might want to consider report_info() instead of printfs here.
>
>> +
>> +}
>> +#elif defined(__aarch64__)
>> +static void read_debug_feature_regs(void)
>> +{
>> + uint64_t id = read_sysreg(id_aa64dfr0_el1);
>> +
>> + /* Check the number of break/watch points */
>> + nbp = ((id >> 12) & 0xf);
>> + if (nbp > 0)
>> + nbp -= 1;
>> +
>> + nwp = ((id >> 20) & 0xf);
>> + if (nwp > 0)
>> + nwp -= 1;
>> +
>> + printf("ID_AA64_DFR0_EL1: 0x%" PRIx64 " nwp:%d nbp:%d\n", id, nwp, nbp);
>
> report_info()?
>
>> +}
>> +#endif
>> +
>> +int main(int argc, char **argv)
>> +{
>> + int cpu, i;
>> +
>> + read_debug_feature_regs();
>> +
>> + for (i = 0; i < argc; ++i) {
>
> should start at i = 1, argv[0] is program name
>
>> + char *var;
>> + long val;
>> +
>> + var = split_var(argv[i], &val);
>> +
>> + if (var) {
>> + int tmp;
>> +
>> + if (strcmp(var, "nbp") == 0) {
>> + tmp = MIN(nbp, val);
>> + nbp = tmp;
>> + }
>> + if (strcmp(var, "nwp") == 0) {
>> + tmp = MIN(nwp, val);
>> + nwp = tmp;
>> + }
>> + }
>> + }
>
> Should probably report_info() the values of nbp and nwp
> here as they're variable and have the MIN() thing and all
> modifying them.
>
>> +
>> + for_each_present_cpu(cpu) {
>> + if (cpu == 0)
>> + continue;
>> + smp_boot_secondary(cpu, test_debug_regs);
>> + }
>> +
>> + test_debug_regs();
>> +
>> + while (!cpumask_full(&smp_test_complete))
>> + cpu_relax();
>> +
>> + return report_summary();
>> +}
>> --
>> 2.13.0
>>
>
> Thanks,
> drew
--
Alex Bennée
