Re: [PATCH V2 08/10] physmem: Add helper function to destroy CPU AddressSpace

[Gavin Shan]

On 9/30/23 10:19, Salil Mehta wrote:
> Virtual CPU Hot-unplug leads to unrealization of a CPU object. This also
> involves destruction of the CPU AddressSpace. Add common function to help
> destroy the CPU AddressSpace.
>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
>   include/exec/cpu-common.h |  8 ++++++++
>   include/hw/core/cpu.h     |  1 +
>   softmmu/physmem.c         | 25 +++++++++++++++++++++++++
>   3 files changed, 34 insertions(+)
>
> 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/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;

@num_ases and @cpu_ases_count are duplicate to each other to some extent. The
real problem is @cpu_ases is allocated at once and we need to make the 
allocation
sparse. In that way, each CPU address space is independent and can be destroyed
independently. The sparse allocation for the CPU address space can be done in
cpu_address_space_init() like below:

#define CPU_ADDRESS_SPACE_MAX 8

struct CPUState {
    CPUAddressSpace *cpu_ases[CPU_ADDRESS_SPACE_MAX];
}

void cpu_address_space_init(CPUState *cpu, int asidx, const char *prefix, 
MemoryRegion *mr)
{
    assert(asidx < CPU_ADDRESS_SPACE_MAX);

    /* The address space has been initialized ? */
    if (cpu->cpu_ases[asidx]) {
        return;
    }

    cpu->cpu_ases[asidx] = g_new0(CPUAddressSpace, 1);

}

void cpu_address_destroy(CPUState *cpu, int asidx)
{
    assert(asidx < CPU_ADDRESS_SPACE_MAX);

    /* The address space isn't initialized? */
    if (!cpu->cpu_ases[asidx]) {
        return;
    }

    :
    g_free(cpu->cpu_ases[asidx]);
    cpu->cpu_ases[asidx] = NULL;
    
}

> 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 */

Thanks,
Gavin