RE: [PATCH RFC V3 24/29] target/arm: Add support of *unrealize* ARMCPU during vCPU Hot-unplug

[Salil Mehta]

HI Peter,

>  From: Peter Maydell <peter.maydell@linaro.org>
>  Sent: Monday, August 19, 2024 2:47 PM
>  To: Salil Mehta <salil.mehta@huawei.com>
>  
>  On Mon, 19 Aug 2024 at 13:58, Salil Mehta <salil.mehta@huawei.com>
>  wrote:
>  >
>  > Hi Peter,
>  >
>  > >  From: Peter Maydell <peter.maydell@linaro.org>
>  > >
>  > >  We shouldn't need to explicitly call cpu_address_space_destroy()
>  > > from a  target-specific unrealize anyway: we can do it all from the
>  > > base class (and I  think this would fix some leaks in current code
>  > > for targets that hot-unplug,  though I should check that). Otherwise
>  > > you need to duplicate all the logic for  figuring out which address
>  > > spaces we created in realize, which is fragile and  not necessary
>  > > when all we want to do is "delete every address space the  CPU object
>  has"
>  > >  and we want to do that for every target architecture always.
>  >
>  >
>  > Agreed but I would suggest to make it optional i.e. in case
>  > architecture want to release to from its code. It should be allowed.
>  > This also ensures clarity of the flows,
>  >
>  > https://lore.kernel.org/qemu-
>  devel/a308e1f4f06f4e3ab6ab51f353601f43@hu
>  > awei.com/
>  
>  Do you have any concrete examples where a target arch would want to
>  explicitly release an AS from its own code? 


No, I don’t have but some of the reasons I thought were:

1. Order of destruction of address space. Can it be different than what will; 
be assumed in the loop?
2. What if something needs to be done or handled before destroying each address 
space?
3. the flow


Unless there's a real use case for
>  doing that, I think that "common code always does the cleanup of the ASes,
>  nothing else ever does" is a simple design rule that avoids the need for
>  target-specific code and means we don't need complicated handling for
>  "some of the ASes in cpu->cpu_ases are live and some have been
>  released":
>  either the CPU is realized and they're all valid, or else we're in the 
> process of
>  unrealizing the CPU and we get rid of them all at once.

I don’t have hard opinions on this. You can share the patch. I'll test with my 
branch
of vCPU hotplug

Thanks
Salil.

>  
>  thanks
>  -- PMM