On Tue, 8 Nov 2022 at 11:51, Mads Ynddal <m...@ynddal.dk> wrote:
> I also noticed you are adding 1 to the WRPs and BRPs. As I interpret the
> documentation, you should subtract 1 instead, given the value 0 is reserved:
>
> diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
> index dbc3605f6d..80a583cbd1 100644
> --- a/target/arm/hvf/hvf.c
> +++ b/target/arm/hvf/hvf.c
> @@ -39,11 +39,11 @@ static void hvf_arm_init_debug(CPUState *cpu)
> {
> ARMCPU *arm_cpu = ARM_CPU(cpu);
>
> - max_hw_bps = 1 + extract64(arm_cpu->isar.id_aa64dfr0, 12, 4);
> + max_hw_bps = extract64(arm_cpu->isar.id_aa64dfr0, 12, 4) - 1;
> hw_breakpoints =
> g_array_sized_new(true, true, sizeof(HWBreakpoint), max_hw_bps);
>
> - max_hw_wps = 1 + extract64(arm_cpu->isar.id_aa64dfr0, 20, 4);
> + max_hw_wps = extract64(arm_cpu->isar.id_aa64dfr0, 20, 4) - 1;
> hw_watchpoints =
> g_array_sized_new(true, true, sizeof(HWWatchpoint), max_hw_wps);
> return;
>
> But the documentation is a bit ambiguous on that. Maybe we can test it?
Adding 1 is correct -- the field definition is "number of breakpoints - 1",
so the number of bps is "field value + 1". You don't need to open-code this,
though -- there are functions arm_num_brps() and arm_num_wrps()
in target/arm/internals.h that extract the fields from the ID registers
and adjust them to give the actual number.
thanks
-- PMM
