| Message ID | 20230929124304.13672-9-salil.mehta@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add architecture agnostic code to support vCPU Hotplug | expand |
Salil Mehta <salil.mehta@huawei.com> writes: > vCPU Hot-unplug requires some helper functions to unroll what has happened > during realization of a CPU object. > > Signed-off-by: Salil Mehta <salil.mehta@huawei.com> > --- > gdbstub/gdbstub.c | 13 +++++++++++++ > include/exec/cpu-common.h | 8 ++++++++ > include/exec/gdbstub.h | 1 + > include/hw/core/cpu.h | 1 + > softmmu/physmem.c | 25 +++++++++++++++++++++++++ > 5 files changed, 48 insertions(+) > > diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c > index 349d348c7b..a06ea3adad 100644 > --- a/gdbstub/gdbstub.c > +++ b/gdbstub/gdbstub.c > @@ -491,6 +491,19 @@ void gdb_register_coprocessor(CPUState *cpu, > } > } > > +void gdb_unregister_coprocessor_all(CPUState *cpu) > +{ > + GDBRegisterState *s, *p; > + > + p = cpu->gdb_regs; > + while (p) { > + s = p; > + p = p->next; Maybe add: /* s->xml is static const char so isn't freed */ > + g_free(s); > + } > + cpu->gdb_regs = NULL; > +} > + > static void gdb_process_breakpoint_remove_all(GDBProcess *p) > { > CPUState *cpu = gdb_get_first_cpu_in_process(p); > diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h > index 41788c0bdd..eb56a228a2 100644 > --- a/include/exec/cpu-common.h > +++ b/include/exec/cpu-common.h > @@ -120,6 +120,14 @@ size_t qemu_ram_pagesize_largest(void); > */ > void cpu_address_space_init(CPUState *cpu, int asidx, > const char *prefix, MemoryRegion *mr); > +/** > + * cpu_address_space_destroy: > + * @cpu: CPU for which address space needs to be destroyed > + * @asidx: integer index of this address space > + * > + * Note that with KVM only one address space is supported. > + */ > +void cpu_address_space_destroy(CPUState *cpu, int asidx); > > void cpu_physical_memory_rw(hwaddr addr, void *buf, > hwaddr len, bool is_write); > diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h > index 16a139043f..986d8d2fa5 100644 > --- a/include/exec/gdbstub.h > +++ b/include/exec/gdbstub.h > @@ -27,6 +27,7 @@ typedef int (*gdb_set_reg_cb)(CPUArchState *env, uint8_t *buf, int reg); > void gdb_register_coprocessor(CPUState *cpu, > gdb_get_reg_cb get_reg, gdb_set_reg_cb set_reg, > int num_regs, const char *xml, int > g_pos); Can we have a kdoc comment here describing the function. I suspect this should be split into two patches, one for gdbstub and one for physmem. > +void gdb_unregister_coprocessor_all(CPUState *cpu); > > /** > * gdbserver_start: start the gdb server > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h > index 648b5b3586..65d2ae4581 100644 > --- a/include/hw/core/cpu.h > +++ b/include/hw/core/cpu.h > @@ -355,6 +355,7 @@ struct CPUState { > QSIMPLEQ_HEAD(, qemu_work_item) work_list; > > CPUAddressSpace *cpu_ases; > + int cpu_ases_count; > int num_ases; > AddressSpace *as; > MemoryRegion *memory; > diff --git a/softmmu/physmem.c b/softmmu/physmem.c > index 4f6ca653b3..4dfa0ca66f 100644 > --- a/softmmu/physmem.c > +++ b/softmmu/physmem.c > @@ -761,6 +761,7 @@ void cpu_address_space_init(CPUState *cpu, int asidx, > > if (!cpu->cpu_ases) { > cpu->cpu_ases = g_new0(CPUAddressSpace, cpu->num_ases); > + cpu->cpu_ases_count = cpu->num_ases; > } > > newas = &cpu->cpu_ases[asidx]; > @@ -774,6 +775,30 @@ void cpu_address_space_init(CPUState *cpu, int asidx, > } > } > > +void cpu_address_space_destroy(CPUState *cpu, int asidx) > +{ > + CPUAddressSpace *cpuas; > + > + assert(asidx < cpu->num_ases); > + assert(asidx == 0 || !kvm_enabled()); > + assert(cpu->cpu_ases); > + > + cpuas = &cpu->cpu_ases[asidx]; > + if (tcg_enabled()) { > + memory_listener_unregister(&cpuas->tcg_as_listener); > + } > + > + address_space_destroy(cpuas->as); > + g_free_rcu(cpuas->as, rcu); > + > + if (cpu->cpu_ases_count == 1) { > + g_free(cpu->cpu_ases); > + cpu->cpu_ases = NULL; > + } > + > + cpu->cpu_ases_count--; > +} > + > AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx) > { > /* Return the AddressSpace corresponding to the specified index */
> From: Alex Bennée <alex.bennee@linaro.org> > Sent: Friday, September 29, 2023 3:35 PM > To: Salil Mehta <salil.mehta@huawei.com> > Cc: qemu-devel@nongnu.org; qemu-arm@nongnu.org; maz@kernel.org; jean- > philippe@linaro.org; Jonathan Cameron <jonathan.cameron@huawei.com>; > lpieralisi@kernel.org; peter.maydell@linaro.org; > richard.henderson@linaro.org; imammedo@redhat.com; andrew.jones@linux.dev; > david@redhat.com; philmd@linaro.org; eric.auger@redhat.com; > oliver.upton@linux.dev; pbonzini@redhat.com; mst@redhat.com; > will@kernel.org; gshan@redhat.com; rafael@kernel.org; > linux@armlinux.org.uk; darren@os.amperecomputing.com; > ilkka@os.amperecomputing.com; vishnu@os.amperecomputing.com; > karl.heubaum@oracle.com; miguel.luis@oracle.com; salil.mehta@opnsrc.net; > zhukeqian <zhukeqian1@huawei.com>; wangxiongfeng (C) > <wangxiongfeng2@huawei.com>; wangyanan (Y) <wangyanan55@huawei.com>; > jiakernel2@gmail.com; maobibo@loongson.cn; lixianglai@loongson.cn; Linuxarm > <linuxarm@huawei.com> > Subject: Re: [PATCH 8/9] physmem,gdbstub: Add helper functions to help > *unrealize* vCPU object > > > Salil Mehta <salil.mehta@huawei.com> writes: > > > vCPU Hot-unplug requires some helper functions to unroll what has happened > > during realization of a CPU object. > > > > Signed-off-by: Salil Mehta <salil.mehta@huawei.com> > > --- > > gdbstub/gdbstub.c | 13 +++++++++++++ > > include/exec/cpu-common.h | 8 ++++++++ > > include/exec/gdbstub.h | 1 + > > include/hw/core/cpu.h | 1 + > > softmmu/physmem.c | 25 +++++++++++++++++++++++++ > > 5 files changed, 48 insertions(+) > > > > diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c > > index 349d348c7b..a06ea3adad 100644 > > --- a/gdbstub/gdbstub.c > > +++ b/gdbstub/gdbstub.c > > @@ -491,6 +491,19 @@ void gdb_register_coprocessor(CPUState *cpu, > > } > > } > > > > +void gdb_unregister_coprocessor_all(CPUState *cpu) > > +{ > > + GDBRegisterState *s, *p; > > + > > + p = cpu->gdb_regs; > > + while (p) { > > + s = p; > > + p = p->next; > > Maybe add: > > /* s->xml is static const char so isn't freed */ Sure. > > + g_free(s); > > + } > > + cpu->gdb_regs = NULL; > > +} > > + > > static void gdb_process_breakpoint_remove_all(GDBProcess *p) > > { > > CPUState *cpu = gdb_get_first_cpu_in_process(p); > > diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h > > index 41788c0bdd..eb56a228a2 100644 > > --- a/include/exec/cpu-common.h > > +++ b/include/exec/cpu-common.h > > @@ -120,6 +120,14 @@ size_t qemu_ram_pagesize_largest(void); > > */ > > void cpu_address_space_init(CPUState *cpu, int asidx, > > const char *prefix, MemoryRegion *mr); > > +/** > > + * cpu_address_space_destroy: > > + * @cpu: CPU for which address space needs to be destroyed > > + * @asidx: integer index of this address space > > + * > > + * Note that with KVM only one address space is supported. > > + */ > > +void cpu_address_space_destroy(CPUState *cpu, int asidx); > > > > void cpu_physical_memory_rw(hwaddr addr, void *buf, > > hwaddr len, bool is_write); > > diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h > > index 16a139043f..986d8d2fa5 100644 > > --- a/include/exec/gdbstub.h > > +++ b/include/exec/gdbstub.h > > @@ -27,6 +27,7 @@ typedef int (*gdb_set_reg_cb)(CPUArchState *env, uint8_t *buf, int reg); > > void gdb_register_coprocessor(CPUState *cpu, > > gdb_get_reg_cb get_reg, gdb_set_reg_cb set_reg, > > int num_regs, const char *xml, int > > g_pos); > > Can we have a kdoc comment here describing the function. Ok. > > I suspect this should be split into two patches, one for gdbstub and one > for physmem. No issues. Can do that. > > +void gdb_unregister_coprocessor_all(CPUState *cpu); > > > > /** > > * gdbserver_start: start the gdb server > > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h > > index 648b5b3586..65d2ae4581 100644 > > --- a/include/hw/core/cpu.h > > +++ b/include/hw/core/cpu.h > > @@ -355,6 +355,7 @@ struct CPUState { > > QSIMPLEQ_HEAD(, qemu_work_item) work_list; > > > > CPUAddressSpace *cpu_ases; > > + int cpu_ases_count; > > int num_ases; > > AddressSpace *as; > > MemoryRegion *memory; > > diff --git a/softmmu/physmem.c b/softmmu/physmem.c > > index 4f6ca653b3..4dfa0ca66f 100644 > > --- a/softmmu/physmem.c > > +++ b/softmmu/physmem.c > > @@ -761,6 +761,7 @@ void cpu_address_space_init(CPUState *cpu, int asidx, > > > > if (!cpu->cpu_ases) { > > cpu->cpu_ases = g_new0(CPUAddressSpace, cpu->num_ases); > > + cpu->cpu_ases_count = cpu->num_ases; > > } > > > > newas = &cpu->cpu_ases[asidx]; > > @@ -774,6 +775,30 @@ void cpu_address_space_init(CPUState *cpu, int > asidx, > > } > > } > > > > +void cpu_address_space_destroy(CPUState *cpu, int asidx) > > +{ > > + CPUAddressSpace *cpuas; > > + > > + assert(asidx < cpu->num_ases); > > + assert(asidx == 0 || !kvm_enabled()); > > + assert(cpu->cpu_ases); > > + > > + cpuas = &cpu->cpu_ases[asidx]; > > + if (tcg_enabled()) { > > + memory_listener_unregister(&cpuas->tcg_as_listener); > > + } > > + > > + address_space_destroy(cpuas->as); > > + g_free_rcu(cpuas->as, rcu); > > + > > + if (cpu->cpu_ases_count == 1) { > > + g_free(cpu->cpu_ases); > > + cpu->cpu_ases = NULL; > > + } > > + > > + cpu->cpu_ases_count--; > > +} > > + > > AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx) > > { > > /* Return the AddressSpace corresponding to the specified index */ > > > -- > Alex Bennée > Virtualisation Tech Lead @ Linaro
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index 349d348c7b..a06ea3adad 100644 --- a/gdbstub/gdbstub.c +++ b/gdbstub/gdbstub.c @@ -491,6 +491,19 @@ void gdb_register_coprocessor(CPUState *cpu, } } +void gdb_unregister_coprocessor_all(CPUState *cpu) +{ + GDBRegisterState *s, *p; + + p = cpu->gdb_regs; + while (p) { + s = p; + p = p->next; + g_free(s); + } + cpu->gdb_regs = NULL; +} + static void gdb_process_breakpoint_remove_all(GDBProcess *p) { CPUState *cpu = gdb_get_first_cpu_in_process(p); diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h index 41788c0bdd..eb56a228a2 100644 --- a/include/exec/cpu-common.h +++ b/include/exec/cpu-common.h @@ -120,6 +120,14 @@ size_t qemu_ram_pagesize_largest(void); */ void cpu_address_space_init(CPUState *cpu, int asidx, const char *prefix, MemoryRegion *mr); +/** + * cpu_address_space_destroy: + * @cpu: CPU for which address space needs to be destroyed + * @asidx: integer index of this address space + * + * Note that with KVM only one address space is supported. + */ +void cpu_address_space_destroy(CPUState *cpu, int asidx); void cpu_physical_memory_rw(hwaddr addr, void *buf, hwaddr len, bool is_write); diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h index 16a139043f..986d8d2fa5 100644 --- a/include/exec/gdbstub.h +++ b/include/exec/gdbstub.h @@ -27,6 +27,7 @@ typedef int (*gdb_set_reg_cb)(CPUArchState *env, uint8_t *buf, int reg); void gdb_register_coprocessor(CPUState *cpu, gdb_get_reg_cb get_reg, gdb_set_reg_cb set_reg, int num_regs, const char *xml, int g_pos); +void gdb_unregister_coprocessor_all(CPUState *cpu); /** * gdbserver_start: start the gdb server diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index 648b5b3586..65d2ae4581 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -355,6 +355,7 @@ struct CPUState { QSIMPLEQ_HEAD(, qemu_work_item) work_list; CPUAddressSpace *cpu_ases; + int cpu_ases_count; int num_ases; AddressSpace *as; MemoryRegion *memory; diff --git a/softmmu/physmem.c b/softmmu/physmem.c index 4f6ca653b3..4dfa0ca66f 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -761,6 +761,7 @@ void cpu_address_space_init(CPUState *cpu, int asidx, if (!cpu->cpu_ases) { cpu->cpu_ases = g_new0(CPUAddressSpace, cpu->num_ases); + cpu->cpu_ases_count = cpu->num_ases; } newas = &cpu->cpu_ases[asidx]; @@ -774,6 +775,30 @@ void cpu_address_space_init(CPUState *cpu, int asidx, } } +void cpu_address_space_destroy(CPUState *cpu, int asidx) +{ + CPUAddressSpace *cpuas; + + assert(asidx < cpu->num_ases); + assert(asidx == 0 || !kvm_enabled()); + assert(cpu->cpu_ases); + + cpuas = &cpu->cpu_ases[asidx]; + if (tcg_enabled()) { + memory_listener_unregister(&cpuas->tcg_as_listener); + } + + address_space_destroy(cpuas->as); + g_free_rcu(cpuas->as, rcu); + + if (cpu->cpu_ases_count == 1) { + g_free(cpu->cpu_ases); + cpu->cpu_ases = NULL; + } + + cpu->cpu_ases_count--; +} + AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx) { /* Return the AddressSpace corresponding to the specified index */
vCPU Hot-unplug requires some helper functions to unroll what has happened during realization of a CPU object. Signed-off-by: Salil Mehta <salil.mehta@huawei.com> --- gdbstub/gdbstub.c | 13 +++++++++++++ include/exec/cpu-common.h | 8 ++++++++ include/exec/gdbstub.h | 1 + include/hw/core/cpu.h | 1 + softmmu/physmem.c | 25 +++++++++++++++++++++++++ 5 files changed, 48 insertions(+)