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

[Peter Maydell]

On Mon, 3 Apr 2023 at 20:17, <francesco.cagnin@gmail.com> wrote:
>
> From: Francesco Cagnin <fcagnin@quarkslab.com>
>
> Support is added for single-stepping, software breakpoints, hardware
> breakpoints and watchpoints. The code has been structured like the KVM
> counterpart (and many parts are basically identical).
>
> Guests can be debugged through the gdbstub.
>
> While guest debugging is enabled, the guest can still read and write the
> DBG*_EL1 registers but they don't have any effect.
>
> Signed-off-by: Francesco Cagnin <fcagnin@quarkslab.com>

> +static void hvf_arm_init_debug(CPUState *cpu)
> +{
> +    ARMCPU *arm_cpu = ARM_CPU(cpu);
> +
> +    max_hw_bps = arm_num_brps(arm_cpu);
> +    hw_breakpoints =
> +        g_array_sized_new(true, true, sizeof(HWBreakpoint), max_hw_bps);
> +
> +    max_hw_wps = arm_num_wrps(arm_cpu);
> +    hw_watchpoints =
> +        g_array_sized_new(true, true, sizeof(HWWatchpoint), max_hw_wps);
> +}

This function gets called per-CPU but it allocates
memory that is per-VM because it's stored in a global.
That means that in an SMP guest the function gets called
multiple times and leaks all but one of the allocations.

The fix for this is to have the function be called
from a general initialization function, not from the
vcpu init function. Compare this (not yet in master)
patch which fixes the same bug for KVM:
20230405153644.25300-1-akihiko.odaki@daynix.com/">https://lore.kernel.org/qemu-devel/20230405153644.25300-1-akihiko.odaki@daynix.com/

Other than that, the structure of the patch looks OK, but
I think you need to identify the cause of the problems
with SMP setups that you mention in the cover letter,
since they suggest that there's a bug lurking in here
somewhere.

thanks
-- PMM