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.

Hi,  Salil

On 2023/7/27 pm 10:51, Salil Mehta wrote:
> 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.
I am sorry for the late reply.

Thanks very much,We look forward to joining the community with your 
patch series.

>
>>> 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?
No, we don't have it yet.
>
> Many thanks!
> Salil.
>