> In my version, I added the sw breakpoints to `hvf_vcpu_state` so they would
> follow each CPU, but it's effectively a copy of the global set of breakpoints.
Ah, I missed this field, and it seems the proper one to use. I'll switch
to this.
> Moving it to `hvf_arch_update_guest_debug` would probably be best. Having it
> in
> `hvf_arch_init_vcpu` would mean that it's always enabled. In some
> corner-cases,
> that might have an adverse effect.
I agree, I'll move it to 'hvf_arch_update_guest_debug()'.
> 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?
I tested this again and indeed adding 1 is correct. Thanks Peter for
also pointing out 'arm_num_brps()' and 'arm_num_wrps()', I'll switch to
those.
