Re: [PATCH RFC V3 24/29] target/arm: Add support of *unrealize* ARMCPU during vCPU Hot-unplug

["Alex Bennée" <alex.bennee@linaro.org>]

Salil Mehta <salil.mehta@huawei.com> writes:

> vCPU Hot-unplug will result in QOM CPU object unrealization which will do away
> with all the vCPU thread creations, allocations, registrations that happened
> as part of the realization process. This change introduces the ARM CPU unrealize
> function taking care of exactly that.
>
> Note, initialized KVM vCPUs are not destroyed in host KVM but their Qemu context
> is parked at the QEMU KVM layer.
>
> Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> Reported-by: Vishnu Pajjuri <vishnu@os.amperecomputing.com>
> [VP: Identified CPU stall issue & suggested probable fix]
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
>  target/arm/cpu.c       | 101 +++++++++++++++++++++++++++++++++++++++++
>  target/arm/cpu.h       |  14 ++++++
>  target/arm/gdbstub.c   |   6 +++
>  target/arm/helper.c    |  25 ++++++++++
>  target/arm/internals.h |   3 ++
>  target/arm/kvm.c       |   5 ++
>  6 files changed, 154 insertions(+)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index c92162fa97..a3dc669309 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -157,6 +157,16 @@ void arm_register_pre_el_change_hook(ARMCPU *cpu, ARMELChangeHookFn *hook,
>      QLIST_INSERT_HEAD(&cpu->pre_el_change_hooks, entry, node);
>  }
>  
> +void arm_unregister_pre_el_change_hooks(ARMCPU *cpu)
> +{
> +    ARMELChangeHook *entry, *next;
> +
> +    QLIST_FOREACH_SAFE(entry, &cpu->pre_el_change_hooks, node, next) {
> +        QLIST_REMOVE(entry, node);
> +        g_free(entry);
> +    }
> +}
> +
>  void arm_register_el_change_hook(ARMCPU *cpu, ARMELChangeHookFn *hook,
>                                   void *opaque)
>  {
> @@ -168,6 +178,16 @@ void arm_register_el_change_hook(ARMCPU *cpu, ARMELChangeHookFn *hook,
>      QLIST_INSERT_HEAD(&cpu->el_change_hooks, entry, node);
>  }
>  
> +void arm_unregister_el_change_hooks(ARMCPU *cpu)
> +{
> +    ARMELChangeHook *entry, *next;
> +
> +    QLIST_FOREACH_SAFE(entry, &cpu->el_change_hooks, node, next) {
> +        QLIST_REMOVE(entry, node);
> +        g_free(entry);
> +    }
> +}
> +
>  static void cp_reg_reset(gpointer key, gpointer value, gpointer opaque)
>  {
>      /* Reset a single ARMCPRegInfo register */
> @@ -2552,6 +2572,85 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>      acc->parent_realize(dev, errp);
>  }
>  
> +static void arm_cpu_unrealizefn(DeviceState *dev)
> +{
> +    ARMCPUClass *acc = ARM_CPU_GET_CLASS(dev);
> +    ARMCPU *cpu = ARM_CPU(dev);
> +    CPUARMState *env = &cpu->env;
> +    CPUState *cs = CPU(dev);
> +    bool has_secure;
> +
> +    has_secure = cpu->has_el3 || arm_feature(env, ARM_FEATURE_M_SECURITY);
> +
> +    /* rock 'n' un-roll, whatever happened in the arm_cpu_realizefn cleanly */
> +    cpu_address_space_destroy(cs, ARMASIdx_NS);

On current master this will fail:

../../target/arm/cpu.c: In function ‘arm_cpu_unrealizefn’:
../../target/arm/cpu.c:2626:5: error: implicit declaration of function ‘cpu_address_space_destroy’ [-Werror=implicit-function-declaration]
 2626 |     cpu_address_space_destroy(cs, ARMASIdx_NS);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~
../../target/arm/cpu.c:2626:5: error: nested extern declaration of ‘cpu_address_space_destroy’ [-Werror=nested-externs]
cc1: all warnings being treated as errors

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro