RE: [PATCH V8 6/8] physmem: Add helper function to destroy CPU AddressSpace

[Salil Mehta via <qemu-devel@nongnu.org>]

>  From: Peter Maydell <peter.maydell@linaro.org>
>  Sent: Monday, May 6, 2024 10:29 AM
>  To: Salil Mehta <salil.mehta@huawei.com>
>  
>  On Mon, 6 May 2024 at 10:06, Salil Mehta <salil.mehta@huawei.com>
>  wrote:
>  >
>  > Hi Peter,
>  >
>  > Thanks for the review.
>  >
>  > >  From: Peter Maydell <peter.maydell@linaro.org>  When do we need to
>  > > destroy a single address space in this way that means  we need to
>  > > keep a count of how many ASes the CPU currently has? The  commit
>  > > message talks about the case when we unrealize the whole CPU
>  > > object, but in that situation you can just throw away all the ASes
>  > > at once (eg  by calling some
>  > >  cpu_destroy_address_spaces() function from
>  cpu_common_unrealizefn()).
>  >
>  >
>  > Yes, maybe, we can destroy all at once from common leg as well. I'd
>  > prefer this to be done from the arch specific function for ARM to
>  > maintain the clarity & symmetry of initialization and
>  > un-initialization legs.  For now, all of these address space destruction is
>  happening in context to the arm_cpu_unrealizefn().
>  >
>  > It’s a kind of trade-off between little more code and clarity but I'm
>  > open to further suggestions.
>  >
>  >
>  > >
>  > >  Also, if we're leaking stuff here by failing to destroy it, is that
>  > > a problem for  existing CPU types like x86 that we can already hotplug?
>  >
>  > No we are not. We are taking care of these in the ARM arch specific
>  > legs within functions arm_cpu_(un)realizefn().
>  
>  How can you be taking care of *x86* CPU types in the Arm unrealize?


Sorry, yes, I missed to reply that clearly. There was indeed a leak with x86 reported
by Phillipe/David last year. In fact, Phillipe floated a patch last year for this.
I thought it was fixed already as part of cpu_common_unrealize() but I just
checked and realized that the below proposed changed still isn’t part of the
mainline

https://lore.kernel.org/qemu-devel/20230918160257.30127-9-philmd@linaro.org/

I can definitely add a common CPU AddressSpace destruction leg as part of this
patch if in case arch specific CPU unrealize does not cleans up its CPU
AddressSpace?


Thanks
Salil.

>  
>  thanks
>  -- PMM