Re: [PATCH] target/arm: Check if debug is already initialized

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 5/4/23 09:02, Akihiko Odaki wrote:
> When virtualizing SMP system, kvm_arm_init_debug() will be called
> multiple times. Check if the debug feature is already initialized when the
> function is called; otherwise it will overwrite pointers to memory
> allocated with the previous call and leak it.
> 
> Fixes: e4482ab7e3 ("target-arm: kvm - add support for HW assisted debug")
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>   target/arm/kvm64.c | 23 +++++++++++++++++------
>   1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 1197253d12..d2fce5e582 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -32,7 +32,11 @@
>   #include "hw/acpi/ghes.h"
>   #include "hw/arm/virt.h"
>   
> -static bool have_guest_debug;
> +static enum {
> +    GUEST_DEBUG_UNINITED,
> +    GUEST_DEBUG_INITED,
> +    GUEST_DEBUG_UNAVAILABLE,
> +} guest_debug;
>   
>   /*
>    * Although the ARM implementation of hardware assisted debugging
> @@ -84,8 +88,14 @@ GArray *hw_breakpoints, *hw_watchpoints;
>    */
>   static void kvm_arm_init_debug(CPUState *cs)
>   {
> -    have_guest_debug = kvm_check_extension(cs->kvm_state,
> -                                           KVM_CAP_SET_GUEST_DEBUG);

- Maybe we can merge kvm{,64}.c (see commit 82bf7ae84c
   "target/arm: Remove KVM support for 32-bit Arm hosts")

- Could kvm_arm_init_debug() belong to kvm_arch_init()?
   Then this patch / enum is not required.

- Why we keep a reference to the global kvm_state in CPUState is not
   clear to me.