RE: [PATCH V5 8/9] physmem: Add helper function to destroy CPU AddressSpace

[Salil Mehta]

> From: Gavin Shan <gshan@redhat.com>
> Sent: Thursday, October 12, 2023 1:18 AM
> To: Salil Mehta <salil.mehta@opnsrc.net>; Salil Mehta
> <salil.mehta@huawei.com>; qemu-devel@nongnu.org; qemu-arm@nongnu.org
> Cc: 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; rafael@kernel.org;
> alex.bennee@linaro.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; 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 V5 8/9] physmem: Add helper function to destroy CPU
> AddressSpace
> 
> Hi Salil,
> 
> On 10/12/23 10:04, Salil Mehta wrote:
> > On 12/10/2023 00:31, Gavin Shan wrote:
> >> On 10/12/23 05:43, Salil Mehta wrote:
> 
> [...]
> 
> >>> +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);
> >>> +
> >>
> >> The two asserts on @asidx and @cpu->cpu_ases can be combined
> >> to one so that these 3 asserts can be combined to two.
> >>
> >>         /* Only one address space is supported by KVM */
> >>         assert(asidx == 0 || !kvm_enabled());
> >>         assert(asidx >= 0 && asidx < cpu->cpu_ases_count)
> >
> > We can do that.
> >
> > I am not in favor to remove  'assert(cpu->cpu_ases);' as this can save
> lot of debugging.
> >
> 
> Ok, It's fine to keep 'assert(cpu->cpu_ases)', but 'assert(asidx >= 0)' is
> still needed? For example, the wrong chunk of memory will be release when
> @asidx is smaller than zero, which is out-of-bound to @cpu->cpu_ases[]

Yes, of course, we can keep that.

Thanks
Salil.