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

[Philippe Mathieu-Daudé]

Hi Francesco,

On 3/4/23 21:16, 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>
> ---
>   accel/hvf/hvf-accel-ops.c | 115 +++++++
>   accel/hvf/hvf-all.c       |  23 ++
>   include/sysemu/hvf.h      |  34 ++
>   include/sysemu/hvf_int.h  |   1 +
>   target/arm/hvf/hvf.c      | 709 +++++++++++++++++++++++++++++++++++++-
>   target/i386/hvf/hvf.c     |  33 ++
>   6 files changed, 913 insertions(+), 2 deletions(-)

Huge patch (hard to digest).

>   static void hvf_accel_ops_class_init(ObjectClass *oc, void *data)
>   {
>       AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
> @@ -473,6 +582,12 @@ static void hvf_accel_ops_class_init(ObjectClass *oc, void 
> *data)
>       ops->synchronize_post_init = hvf_cpu_synchronize_post_init;
>       ops->synchronize_state = hvf_cpu_synchronize_state;
>       ops->synchronize_pre_loadvm = hvf_cpu_synchronize_pre_loadvm;

Consider splitting in 3;

1: add registers,

> +    ops->insert_breakpoint = hvf_insert_breakpoint;
> +    ops->remove_breakpoint = hvf_remove_breakpoint;
> +    ops->remove_all_breakpoints = hvf_remove_all_breakpoints;

2: breakpoint handlers (and stubs)

> +    ops->update_guest_debug = hvf_update_guest_debug;
> +    ops->supports_guest_debug = hvf_arch_supports_guest_debug;

3: guest_debug handlers (and stubs).

>   };
>   static const TypeInfo hvf_accel_ops_type = {
>       .name = ACCEL_OPS_NAME("hvf"),

Regards,

Phil.