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

[Salil Mehta]

Hi Alex,

>  From: Alex Bennée <alex.bennee@linaro.org>
>  Sent: Friday, August 16, 2024 4:37 PM
>  To: Salil Mehta <salil.mehta@huawei.com>
>  
>  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


The current master already has arch-agnostic patch-set. I've applied the
RFC V3 to the latest and complied. I did not see this issue?

I've create a new branch for your reference.

https://github.com/salil-mehta/qemu/tree/virt-cpuhp-armv8/rfc-v4-rc4

Please let me know if this works for you?


Thanks
Salil.



>  
>  --
>  Alex Bennée
>  Virtualisation Tech Lead @ Linaro