HI Peter, > From: Peter Maydell <peter.mayd...@linaro.org> > Sent: Monday, August 19, 2024 2:47 PM > To: Salil Mehta <salil.me...@huawei.com> > > On Mon, 19 Aug 2024 at 13:58, Salil Mehta <salil.me...@huawei.com> > wrote: > > > > Hi Peter, > > > > > From: Peter Maydell <peter.mayd...@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
