Re: [PATCH 2/3] hvf: implement guest debugging on Apple Silicon hosts

[Peter Maydell <peter.maydell@linaro.org>]

On Tue, 8 Nov 2022 at 11:51, Mads Ynddal <mads@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