RE: [PATCH 0/8] Adds CPU hot-plug support to Loongarch

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

Hello,

> From: lixianglai <lixianglai@loongson.cn>
> Sent: Thursday, July 27, 2023 3:14 AM
> To: Gavin Shan <gshan@redhat.com>; qemu-devel@nongnu.org; Salil Mehta
> <salil.mehta@huawei.com>; zhukeqian <zhukeqian1@huawei.com>; Bibo Mao
> <maobibo@loongson.cn>
> Subject: Re: [PATCH 0/8] Adds CPU hot-plug support to Loongarch
> 
> Hi Gavin and Salil,
> 
> On 7/27/23 8:57 AM, Gavin Shan wrote:
> > Hi Xianglai,
> >
> > On 7/20/23 17:15, xianglai li wrote:
> >> Hello everyone, We refer to the implementation of ARM CPU
> >> Hot-Plug to add GED-based CPU Hot-Plug support to Loongarch.
> >>
> >> The first 4 patches are changes to the QEMU common code,
> >> including adding GED support for CPU Hot-Plug, updating
> >> the ACPI table creation process, and adding
> >> qdev_disconnect_gpio_out_named
> >> and cpu_address_space_destroy interfaces to release resources
> >> when CPU un-plug.
> >>
> >> The last four patches are Loongarch architecture-related,
> >> and the modifications include the definition of the hook
> >> function related to the CPU Hot-(UN)Plug, the allocation
> >> and release of CPU resources when CPU Hot-(UN)Plug,
> >> the creation process of updating the ACPI table,
> >> and finally the custom switch for the CPU Hot-Plug.
> >>
> >
> > [...]
> >
> > It seems the design for ARM64 hotplug has been greatly referred, but the authors
> > are missed to be copied if you were referring to the following repository. There
> > will be conflicts between those two patchsets as I can see and I'm not sure how
> > it can be resolved. In theory, one patchset needs to be rebased on another one
> > since they're sharing large amount of codes.
> >
> >   https://github.com/salil-mehta/qemu.git
> >   (branch: virt-cpuhp-armv8/rfc-v1-port11052023.dev-1)
> >
> > I took a quick scan on this series. Loongarch doesn't have ARM64's constraint due
> > to vGIC3 where all vCPUs's file descriptor needs to be in place. It means the possible
> > and not yet present vCPU needs to be visible to KVM. Without this constraint, the
> > implementation is simplified a lot.
> 
> We have great respect and gratitude to Salil and his team for their work
> and contributions to CPU HotPlug,


Many thanks! Really appreciate this.


> which greatly reduced the difficulty of developing CPU HotPlug in
> Loongarch.


We are glad that this work is useful to other companies as well this
was one of our goal.


> So, We plan to rebase the next version of patch based on their code.


Great. Thanks for clarifying this as accountability is important
even though we are working in a collaborative environment.

As such, I am planning to send the RFC V2 in 2 weeks of time and
will make sure that the patches which are required by your patch-set
are formed in such a way that they can be independently accepted
w.r.t rest of the ARM64 architecture specific patch-set.


> Hi Salil,
> 
> I estimate that it will take quite some time for your patch series to
> merge in,
> 
> if allowed, can you merge your patch series changes to the common code
> section into the community first,
> 
> so that our code can be rebase and merged.


Sure, as clarified above, something similar, will create a patch-set
which will have patches which can be independently accepted/Ack'ed
and consumed even before the complete patch-set.

Although I think, even in current form most patches have been arranged
in such a way. But I will doubly check again before I float RFC V2.


> > Besides, we found the vCPU hotplug isn't working for TCG when Salil's
> > series was
> > tested. I'm not sure if we have same issue with this series, or TCG
> > isn't a concern
> > to us at all?
> 
> At present, QEMU only supports TCG under Loongarch,
> 
> and we test CPU HotPlug is also carried out under QEMU TCG,
> 
> and CPU HotPlug can work normally when testing.
> 
> Of course, we are also very happy to see you testing CPU hotplug under
> Loongarch,
> 
> which can find more problems and help us improve our patch.


We know that. There are some trivial fixes we already have, I will push
them as well for the completion. We were not sure if anybody needs them
as usage of vCPU Hotplug under 'accel=tcg' is highly unlikely except for
testing cases. Please let us know if you have any?


Many thanks!
Salil.