@@ -2613,8 +2613,14 @@ static void tcg_commit(MemoryListener *listener)
*
* That said, the listener is also called during realize, before
* all of the tcg machinery for run-on is initialized: thus halt_cond.
+ * Similarly, the listener can also be triggered during the *unrealize*
+ * operation. In such a case, we should avoid using `run_on_cpu` because the
+ * TCG vCPU thread might already be terminated. As a result, the CPU work
+ * will never get processed, and `tcg_commit_cpu` will not be called. This
+ * means that operations like `tlb_flush()` might not be executed,
+ * potentially leading to inconsistencies.
*/
- if (cpu->halt_cond) {
+ if (cpu->halt_cond && !cpu->unplug) {
async_run_on_cpu(cpu, tcg_commit_cpu, RUN_ON_CPU_HOST_PTR(cpuas));
} else {
tcg_commit_cpu(cpu, RUN_ON_CPU_HOST_PTR(cpuas));
@@ -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 */
@@ -2642,6 +2662,98 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
acc->parent_realize(dev, errp);
}
+#ifndef CONFIG_USER_ONLY
+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;
+
+ /* rock 'n' un-roll, whatever happened in the arm_cpu_realizefn cleanly */
+ destroy_cpreg_list(cpu);
+ arm_cpu_unregister_gdb_regs(cpu);
+ unregister_cp_regs_for_features(cpu);
+
+ if (cpu->sau_sregion && arm_feature(env, ARM_FEATURE_M_SECURITY)) {
+ g_free(env->sau.rbar);
+ g_free(env->sau.rlar);
+ }
+
+ if (arm_feature(env, ARM_FEATURE_PMSA) &&
+ arm_feature(env, ARM_FEATURE_V7) &&
+ cpu->pmsav7_dregion) {
+ if (arm_feature(env, ARM_FEATURE_V8)) {
+ g_free(env->pmsav8.rbar[M_REG_NS]);
+ g_free(env->pmsav8.rlar[M_REG_NS]);
+ if (arm_feature(env, ARM_FEATURE_M_SECURITY)) {
+ g_free(env->pmsav8.rbar[M_REG_S]);
+ g_free(env->pmsav8.rlar[M_REG_S]);
+ }
+ } else {
+ g_free(env->pmsav7.drbar);
+ g_free(env->pmsav7.drsr);
+ g_free(env->pmsav7.dracr);
+ }
+ if (cpu->pmsav8r_hdregion) {
+ g_free(env->pmsav8.hprbar);
+ g_free(env->pmsav8.hprlar);
+ }
+ }
+
+ if (arm_feature(env, ARM_FEATURE_PMU)) {
+ if (!kvm_enabled()) {
+ arm_unregister_pre_el_change_hooks(cpu);
+ arm_unregister_el_change_hooks(cpu);
+ }
+
+ if (cpu->pmu_timer) {
+ timer_del(cpu->pmu_timer);
+ }
+ }
+
+ cpu_remove_sync(CPU(dev));
+
+ /*
+ * We are intentionally destroying the CPU address space after the vCPU
+ * threads have been joined. This ensures that for TCG, any pending TLB
+ * flushes associated with the CPU are completed. The destruction of the
+ * address space also removes associated listeners, and joining threads
+ * after the address space no longer exists can lead to race conditions with
+ * already queued work for this CPU, which may result in a segmentation
+ * fault (SEGV) in `tcg_commit_cpu()`.
+ *
+ * Alternatively, Peter Maydell has suggested moving the CPU address space
+ * destruction to `cpu_common_unrealize()`, which would be called in the
+ * context of `parent_unrealize()`. This would also address the race
+ * condition in TCG.
+ *
+ * RFC: Question: Any additional thoughts or feedback on this approach would
+ * be appreciated?
+ */
+ has_secure = cpu->has_el3 || arm_feature(env, ARM_FEATURE_M_SECURITY);
+ cpu_address_space_destroy(cs, ARMASIdx_NS);
+ if (cpu->tag_memory != NULL) {
+ cpu_address_space_destroy(cs, ARMASIdx_TagNS);
+ if (has_secure) {
+ cpu_address_space_destroy(cs, ARMASIdx_TagS);
+ }
+ }
+ if (has_secure) {
+ cpu_address_space_destroy(cs, ARMASIdx_S);
+ }
+
+ acc->parent_unrealize(dev);
+
+ timer_del(cpu->gt_timer[GTIMER_PHYS]);
+ timer_del(cpu->gt_timer[GTIMER_VIRT]);
+ timer_del(cpu->gt_timer[GTIMER_HYP]);
+ timer_del(cpu->gt_timer[GTIMER_SEC]);
+ timer_del(cpu->gt_timer[GTIMER_HYPVIRT]);
+}
+#endif
+
static ObjectClass *arm_cpu_class_by_name(const char *cpu_model)
{
ObjectClass *oc;
@@ -2745,7 +2857,10 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
device_class_set_parent_realize(dc, arm_cpu_realizefn,
&acc->parent_realize);
-
+#ifndef CONFIG_USER_ONLY
+ device_class_set_parent_unrealize(dc, arm_cpu_unrealizefn,
+ &acc->parent_unrealize);
+#endif
device_class_set_props(dc, arm_cpu_properties);
resettable_class_set_parent_phases(rc, NULL, arm_cpu_reset_hold, NULL,
@@ -1128,6 +1128,7 @@ struct ARMCPUClass {
const ARMCPUInfo *info;
DeviceRealize parent_realize;
+ DeviceUnrealize parent_unrealize;
ResettablePhases parent_phases;
};
@@ -3293,6 +3294,13 @@ static inline AddressSpace *arm_addressspace(CPUState *cs, MemTxAttrs attrs)
*/
void arm_register_pre_el_change_hook(ARMCPU *cpu, ARMELChangeHookFn *hook,
void *opaque);
+/**
+ * arm_unregister_pre_el_change_hook:
+ * unregister all pre EL change hook functions. Generally called during
+ * unrealize'ing leg
+ */
+void arm_unregister_pre_el_change_hooks(ARMCPU *cpu);
+
/**
* arm_register_el_change_hook:
* Register a hook function which will be called immediately after this
@@ -3305,6 +3313,12 @@ void arm_register_pre_el_change_hook(ARMCPU *cpu, ARMELChangeHookFn *hook,
*/
void arm_register_el_change_hook(ARMCPU *cpu, ARMELChangeHookFn *hook, void
*opaque);
+/**
+ * arm_unregister_el_change_hook:
+ * unregister all EL change hook functions. Generally called during
+ * unrealize'ing leg
+ */
+void arm_unregister_el_change_hooks(ARMCPU *cpu);
/**
* arm_rebuild_hflags:
@@ -595,3 +595,9 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
}
#endif /* CONFIG_TCG */
}
+
+void arm_cpu_unregister_gdb_regs(ARMCPU *cpu)
+{
+ CPUState *cs = CPU(cpu);
+ gdb_unregister_coprocessor_all(cs);
+}
@@ -264,6 +264,19 @@ void init_cpreg_list(ARMCPU *cpu)
g_list_free(keys);
}
+void destroy_cpreg_list(ARMCPU *cpu)
+{
+ assert(cpu->cpreg_indexes);
+ assert(cpu->cpreg_values);
+ assert(cpu->cpreg_vmstate_indexes);
+ assert(cpu->cpreg_vmstate_values);
+
+ g_free(cpu->cpreg_indexes);
+ g_free(cpu->cpreg_values);
+ g_free(cpu->cpreg_vmstate_indexes);
+ g_free(cpu->cpreg_vmstate_values);
+}
+
static bool arm_pan_enabled(CPUARMState *env)
{
if (is_a64(env)) {
@@ -9985,6 +9998,18 @@ void register_cp_regs_for_features(ARMCPU *cpu)
#endif
}
+void unregister_cp_regs_for_features(ARMCPU *cpu)
+{
+ CPUARMState *env = &cpu->env;
+ if (arm_feature(env, ARM_FEATURE_M)) {
+ /* M profile has no coprocessor registers */
+ return;
+ }
+
+ /* empty it all. unregister all the coprocessor registers */
+ g_hash_table_remove_all(cpu->cp_regs);
+}
+
/*
* Private utility function for define_one_arm_cp_reg_with_opaque():
* add a single reginfo struct to the hash table.
@@ -367,9 +367,12 @@ void arm_cpu_register(const ARMCPUInfo *info);
void aarch64_cpu_register(const ARMCPUInfo *info);
void register_cp_regs_for_features(ARMCPU *cpu);
+void unregister_cp_regs_for_features(ARMCPU *cpu);
void init_cpreg_list(ARMCPU *cpu);
+void destroy_cpreg_list(ARMCPU *cpu);
void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu);
+void arm_cpu_unregister_gdb_regs(ARMCPU *cpu);
void arm_translate_init(void);
void arm_cpu_register_gdb_commands(ARMCPU *cpu);
@@ -1983,6 +1983,11 @@ int kvm_arch_init_vcpu(CPUState *cs)
int kvm_arch_destroy_vcpu(CPUState *cs)
{
+ /* vCPUs which are yet to be realized will not have handler */
+ if (cs->thread_id) {
+ qemu_del_vm_change_state_handler(cs->vmcse);
+ }
+
return 0;
}