OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 293 ++++++++++++------ OvmfPkg/CpuHotplugSmm/QemuCpuhp.c | 58 +++- OvmfPkg/CpuHotplugSmm/QemuCpuhp.h | 6 + .../Include/IndustryStandard/QemuCpuHotplug.h | 2 + OvmfPkg/SmmControl2Dxe/SmiFeatures.c | 21 +- 5 files changed, 280 insertions(+), 100 deletions(-)
[ Resending to the correct edk2 alias this time. ] Hi, This series adds support for CPU hot-unplug with OVMF. Please see this in conjunction with the QEMU v2 series posted here: https://lore.kernel.org/qemu-devel/20201207140739.3829993-1-imammedo@redhat.com/ In particular, would be glad for comments on Patch 4, specifically where we should be ejecting the CPU. Right now the ejection happens in UnplugCpus() (called from CpuHotplugMmi()): + QemuCpuhpWriteCpuSelector (mMmCpuIo, RemoveApicId); + QemuCpuhpWriteCpuStatus (mMmCpuIo, QEMU_CPUHP_STAT_EJECTED); That is way too early -- given that the actual unplug will happen in SmmCpuUpdate() and given that the BSPHandler() would have waited for the APs multiple times before then. Another possibility is that the actual ejection be deferred to the _EJ0 method after the return from the SMI. But, that seems like a hack. Additionally, Igor points out here that this approach has problems: https://lore.kernel.org/qemu-devel/20201204170939.1815522-1-imammedo@redhat.com/ Please review. Thanks Ankur Ankur Arora (5): OvmfPkg/CpuHotplugSmm: move CPU Hotplug into PlugCpus() OvmfPkg/CpuHotplugSmm: handle fw_remove OvmfPkg/CpuHotplugSmm: add CpuStatus helper function OvmfPkg/CpuHotplugSmm: handle CPU hot-unplug OvmfPkg/SmmControl2Dxe: negotiate ICH9_LPC_SMI_F_CPU_HOT_UNPLUG OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 293 ++++++++++++------ OvmfPkg/CpuHotplugSmm/QemuCpuhp.c | 58 +++- OvmfPkg/CpuHotplugSmm/QemuCpuhp.h | 6 + .../Include/IndustryStandard/QemuCpuHotplug.h | 2 + OvmfPkg/SmmControl2Dxe/SmiFeatures.c | 21 +- 5 files changed, 280 insertions(+), 100 deletions(-) -- 2.25.4 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#68460): https://edk2.groups.io/g/devel/message/68460 Mute This Topic: https://groups.io/mt/78802720/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi Ankur, On 12/08/20 06:34, Ankur Arora wrote: > [ Resending to the correct edk2 alias this time. ] > > Hi, > > This series adds support for CPU hot-unplug with OVMF. > > Please see this in conjunction with the QEMU v2 series posted here: > https://lore.kernel.org/qemu-devel/20201207140739.3829993-1-imammedo@redhat.com/ > > In particular, would be glad for comments on Patch 4, specifically > where we should be ejecting the CPU. > > Right now the ejection happens in UnplugCpus() (called from > CpuHotplugMmi()): > + QemuCpuhpWriteCpuSelector (mMmCpuIo, RemoveApicId); > + QemuCpuhpWriteCpuStatus (mMmCpuIo, QEMU_CPUHP_STAT_EJECTED); > > That is way too early -- given that the actual unplug will happen > in SmmCpuUpdate() and given that the BSPHandler() would have waited > for the APs multiple times before then. > > Another possibility is that the actual ejection be deferred to the > _EJ0 method after the return from the SMI. But, that seems like a > hack. Additionally, Igor points out here that this approach has problems: > https://lore.kernel.org/qemu-devel/20201204170939.1815522-1-imammedo@redhat.com/ I've filed: https://bugzilla.tianocore.org/show_bug.cgi?id=3132 Can you please register an account in the TianoCore Bugzilla at <https://bugzilla.tianocore.org/>, and assign the above ticket to yourself? And then, the URL of the new BZ ticket should be included in every commit message, like this: Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132 just above your S-o-b. No need to repost just because of this; I'll review the RFC series later. Thanks, Laszlo > > Please review. > > Thanks > Ankur > > Ankur Arora (5): > OvmfPkg/CpuHotplugSmm: move CPU Hotplug into PlugCpus() > OvmfPkg/CpuHotplugSmm: handle fw_remove > OvmfPkg/CpuHotplugSmm: add CpuStatus helper function > OvmfPkg/CpuHotplugSmm: handle CPU hot-unplug > OvmfPkg/SmmControl2Dxe: negotiate ICH9_LPC_SMI_F_CPU_HOT_UNPLUG > > OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 293 ++++++++++++------ > OvmfPkg/CpuHotplugSmm/QemuCpuhp.c | 58 +++- > OvmfPkg/CpuHotplugSmm/QemuCpuhp.h | 6 + > .../Include/IndustryStandard/QemuCpuHotplug.h | 2 + > OvmfPkg/SmmControl2Dxe/SmiFeatures.c | 21 +- > 5 files changed, 280 insertions(+), 100 deletions(-) > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#69328): https://edk2.groups.io/g/devel/message/69328 Mute This Topic: https://groups.io/mt/78802720/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 12/21/20 15:44, Laszlo Ersek wrote: > Hi Ankur, > > On 12/08/20 06:34, Ankur Arora wrote: >> [ Resending to the correct edk2 alias this time. ] >> >> Hi, >> >> This series adds support for CPU hot-unplug with OVMF. >> >> Please see this in conjunction with the QEMU v2 series posted here: >> https://lore.kernel.org/qemu-devel/20201207140739.3829993-1-imammedo@redhat.com/ >> >> In particular, would be glad for comments on Patch 4, specifically >> where we should be ejecting the CPU. >> >> Right now the ejection happens in UnplugCpus() (called from >> CpuHotplugMmi()): >> + QemuCpuhpWriteCpuSelector (mMmCpuIo, RemoveApicId); >> + QemuCpuhpWriteCpuStatus (mMmCpuIo, QEMU_CPUHP_STAT_EJECTED); >> >> That is way too early -- given that the actual unplug will happen >> in SmmCpuUpdate() and given that the BSPHandler() would have waited >> for the APs multiple times before then. >> >> Another possibility is that the actual ejection be deferred to the >> _EJ0 method after the return from the SMI. But, that seems like a >> hack. Additionally, Igor points out here that this approach has problems: >> https://lore.kernel.org/qemu-devel/20201204170939.1815522-1-imammedo@redhat.com/ > > I've filed: > > https://bugzilla.tianocore.org/show_bug.cgi?id=3132 > > Can you please register an account in the TianoCore Bugzilla at > <https://bugzilla.tianocore.org/>, and assign the above ticket to yourself? > > And then, the URL of the new BZ ticket should be included in every > commit message, like this: > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132 > > just above your S-o-b. > > No need to repost just because of this; I'll review the RFC series later. I've also listed <https://bugzilla.tianocore.org/show_bug.cgi?id=3132> under <https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning#edk2-stable202102-tag-planning>. Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#69332): https://edk2.groups.io/g/devel/message/69332 Mute This Topic: https://groups.io/mt/78802720/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Mon, 21 Dec 2020 15:44:35 +0100 "Laszlo Ersek" <lersek@redhat.com> wrote: > Hi Ankur, > > On 12/08/20 06:34, Ankur Arora wrote: > > [ Resending to the correct edk2 alias this time. ] > > > > Hi, > > > > This series adds support for CPU hot-unplug with OVMF. > > > > Please see this in conjunction with the QEMU v2 series posted here: > > https://lore.kernel.org/qemu-devel/20201207140739.3829993-1-imammedo@redhat.com/ > > > > In particular, would be glad for comments on Patch 4, specifically > > where we should be ejecting the CPU. > > > > Right now the ejection happens in UnplugCpus() (called from > > CpuHotplugMmi()): > > + QemuCpuhpWriteCpuSelector (mMmCpuIo, RemoveApicId); > > + QemuCpuhpWriteCpuStatus (mMmCpuIo, QEMU_CPUHP_STAT_EJECTED); > > > > That is way too early -- given that the actual unplug will happen > > in SmmCpuUpdate() and given that the BSPHandler() would have waited > > for the APs multiple times before then. > > > > Another possibility is that the actual ejection be deferred to the > > _EJ0 method after the return from the SMI. But, that seems like a > > hack. Additionally, Igor points out here that this approach has problems: > > https://lore.kernel.org/qemu-devel/20201204170939.1815522-1-imammedo@redhat.com/ > > I've filed: > > https://bugzilla.tianocore.org/show_bug.cgi?id=3132 > > Can you please register an account in the TianoCore Bugzilla at > <https://bugzilla.tianocore.org/>, and assign the above ticket to yourself? > > And then, the URL of the new BZ ticket should be included in every > commit message, like this: > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132 > > just above your S-o-b. > > No need to repost just because of this; I'll review the RFC series later. BTW: meanwhile, QEMU part got merged so one doesn't need to hunt for it anymore. If something is broken there, we will have to fix it on top. > > Thanks, > Laszlo > > > > > Please review. > > > > Thanks > > Ankur > > > > Ankur Arora (5): > > OvmfPkg/CpuHotplugSmm: move CPU Hotplug into PlugCpus() > > OvmfPkg/CpuHotplugSmm: handle fw_remove > > OvmfPkg/CpuHotplugSmm: add CpuStatus helper function > > OvmfPkg/CpuHotplugSmm: handle CPU hot-unplug > > OvmfPkg/SmmControl2Dxe: negotiate ICH9_LPC_SMI_F_CPU_HOT_UNPLUG > > > > OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 293 ++++++++++++------ > > OvmfPkg/CpuHotplugSmm/QemuCpuhp.c | 58 +++- > > OvmfPkg/CpuHotplugSmm/QemuCpuhp.h | 6 + > > .../Include/IndustryStandard/QemuCpuHotplug.h | 2 + > > OvmfPkg/SmmControl2Dxe/SmiFeatures.c | 21 +- > > 5 files changed, 280 insertions(+), 100 deletions(-) > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#69333): https://edk2.groups.io/g/devel/message/69333 Mute This Topic: https://groups.io/mt/78802720/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 12/21/20 16:46, Igor Mammedov wrote: > On Mon, 21 Dec 2020 15:44:35 +0100 > "Laszlo Ersek" <lersek@redhat.com> wrote: > >> Hi Ankur, >> >> On 12/08/20 06:34, Ankur Arora wrote: >>> [ Resending to the correct edk2 alias this time. ] >>> >>> Hi, >>> >>> This series adds support for CPU hot-unplug with OVMF. >>> >>> Please see this in conjunction with the QEMU v2 series posted here: >>> https://lore.kernel.org/qemu-devel/20201207140739.3829993-1-imammedo@redhat.com/ >>> >>> In particular, would be glad for comments on Patch 4, specifically >>> where we should be ejecting the CPU. >>> >>> Right now the ejection happens in UnplugCpus() (called from >>> CpuHotplugMmi()): >>> + QemuCpuhpWriteCpuSelector (mMmCpuIo, RemoveApicId); >>> + QemuCpuhpWriteCpuStatus (mMmCpuIo, QEMU_CPUHP_STAT_EJECTED); >>> >>> That is way too early -- given that the actual unplug will happen >>> in SmmCpuUpdate() and given that the BSPHandler() would have waited >>> for the APs multiple times before then. >>> >>> Another possibility is that the actual ejection be deferred to the >>> _EJ0 method after the return from the SMI. But, that seems like a >>> hack. Additionally, Igor points out here that this approach has problems: >>> https://lore.kernel.org/qemu-devel/20201204170939.1815522-1-imammedo@redhat.com/ >> >> I've filed: >> >> https://bugzilla.tianocore.org/show_bug.cgi?id=3132 >> >> Can you please register an account in the TianoCore Bugzilla at >> <https://bugzilla.tianocore.org/>, and assign the above ticket to yourself? >> >> And then, the URL of the new BZ ticket should be included in every >> commit message, like this: >> >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132 >> >> just above your S-o-b. >> >> No need to repost just because of this; I'll review the RFC series later. > BTW: > > meanwhile, QEMU part got merged so one doesn't need to hunt for it anymore. > If something is broken there, we will have to fix it on top. Thank you very much for the info! Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#69338): https://edk2.groups.io/g/devel/message/69338 Mute This Topic: https://groups.io/mt/78802720/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 2020-12-21 6:44 a.m., Laszlo Ersek wrote: > Hi Ankur, > > On 12/08/20 06:34, Ankur Arora wrote: >> [ Resending to the correct edk2 alias this time. ] >> >> Hi, >> >> This series adds support for CPU hot-unplug with OVMF. >> >> Please see this in conjunction with the QEMU v2 series posted here: >> https://lore.kernel.org/qemu-devel/20201207140739.3829993-1-imammedo@redhat.com/ >> >> In particular, would be glad for comments on Patch 4, specifically >> where we should be ejecting the CPU. >> >> Right now the ejection happens in UnplugCpus() (called from >> CpuHotplugMmi()): >> + QemuCpuhpWriteCpuSelector (mMmCpuIo, RemoveApicId); >> + QemuCpuhpWriteCpuStatus (mMmCpuIo, QEMU_CPUHP_STAT_EJECTED); >> >> That is way too early -- given that the actual unplug will happen >> in SmmCpuUpdate() and given that the BSPHandler() would have waited >> for the APs multiple times before then. >> >> Another possibility is that the actual ejection be deferred to the >> _EJ0 method after the return from the SMI. But, that seems like a >> hack. Additionally, Igor points out here that this approach has problems: >> https://lore.kernel.org/qemu-devel/20201204170939.1815522-1-imammedo@redhat.com/ > > I've filed: > > https://bugzilla.tianocore.org/show_bug.cgi?id=3132 > > Can you please register an account in the TianoCore Bugzilla at > <https://bugzilla.tianocore.org/>, and assign the above ticket to yourself? > > And then, the URL of the new BZ ticket should be included in every > commit message, like this: > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132 > > just above your S-o-b. Sure. Will do. > No need to repost just because of this; I'll review the RFC series later. Actually could you hold off before reviewing the RFC series. I'll be sending a v2 series shortly. That does the actual unplug via SmmCpuFeaturesRendezvousExit() (I had described the approach elsewhere in this thread.) Thanks Ankur > Thanks, > Laszlo > >> >> Please review. >> >> Thanks >> Ankur >> >> Ankur Arora (5): >> OvmfPkg/CpuHotplugSmm: move CPU Hotplug into PlugCpus() >> OvmfPkg/CpuHotplugSmm: handle fw_remove >> OvmfPkg/CpuHotplugSmm: add CpuStatus helper function >> OvmfPkg/CpuHotplugSmm: handle CPU hot-unplug >> OvmfPkg/SmmControl2Dxe: negotiate ICH9_LPC_SMI_F_CPU_HOT_UNPLUG >> >> OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 293 ++++++++++++------ >> OvmfPkg/CpuHotplugSmm/QemuCpuhp.c | 58 +++- >> OvmfPkg/CpuHotplugSmm/QemuCpuhp.h | 6 + >> .../Include/IndustryStandard/QemuCpuHotplug.h | 2 + >> OvmfPkg/SmmControl2Dxe/SmiFeatures.c | 21 +- >> 5 files changed, 280 insertions(+), 100 deletions(-) >> > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#69337): https://edk2.groups.io/g/devel/message/69337 Mute This Topic: https://groups.io/mt/78802720/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 12/21/20 20:09, Ankur Arora wrote: > On 2020-12-21 6:44 a.m., Laszlo Ersek wrote: >> Hi Ankur, >> >> On 12/08/20 06:34, Ankur Arora wrote: >>> [ Resending to the correct edk2 alias this time. ] >>> >>> Hi, >>> >>> This series adds support for CPU hot-unplug with OVMF. >>> >>> Please see this in conjunction with the QEMU v2 series posted here: >>> >>> https://lore.kernel.org/qemu-devel/20201207140739.3829993-1-imammedo@redhat.com/ >>> >>> >>> In particular, would be glad for comments on Patch 4, specifically >>> where we should be ejecting the CPU. >>> >>> Right now the ejection happens in UnplugCpus() (called from >>> CpuHotplugMmi()): >>> + QemuCpuhpWriteCpuSelector (mMmCpuIo, RemoveApicId); >>> + QemuCpuhpWriteCpuStatus (mMmCpuIo, QEMU_CPUHP_STAT_EJECTED); >>> >>> That is way too early -- given that the actual unplug will happen >>> in SmmCpuUpdate() and given that the BSPHandler() would have waited >>> for the APs multiple times before then. >>> >>> Another possibility is that the actual ejection be deferred to the >>> _EJ0 method after the return from the SMI. But, that seems like a >>> hack. Additionally, Igor points out here that this approach has >>> problems: >>> >>> https://lore.kernel.org/qemu-devel/20201204170939.1815522-1-imammedo@redhat.com/ >>> >> >> I've filed: >> >> https://bugzilla.tianocore.org/show_bug.cgi?id=3132 >> >> Can you please register an account in the TianoCore Bugzilla at >> <https://bugzilla.tianocore.org/>, and assign the above ticket to >> yourself? >> >> And then, the URL of the new BZ ticket should be included in every >> commit message, like this: >> >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132 >> >> just above your S-o-b. > > Sure. Will do. > >> No need to repost just because of this; I'll review the RFC series later. > > Actually could you hold off before reviewing the RFC series. I'll be > sending > a v2 series shortly. That does the actual unplug via > SmmCpuFeaturesRendezvousExit() > (I had described the approach elsewhere in this thread.) Sure, I can de-queue the RFC series. I planned to review it anyway in the new year; I've just done some admin stuff around the feature now. Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#69339): https://edk2.groups.io/g/devel/message/69339 Mute This Topic: https://groups.io/mt/78802720/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi Ankur, On 12/08/20 06:34, Ankur Arora wrote: > [ Resending to the correct edk2 alias this time. ] > > Hi, > > This series adds support for CPU hot-unplug with OVMF. > > Please see this in conjunction with the QEMU v2 series posted here: > https://lore.kernel.org/qemu-devel/20201207140739.3829993-1-imammedo@redhat.com/ > > In particular, would be glad for comments on Patch 4, specifically > where we should be ejecting the CPU. > > Right now the ejection happens in UnplugCpus() (called from > CpuHotplugMmi()): > + QemuCpuhpWriteCpuSelector (mMmCpuIo, RemoveApicId); > + QemuCpuhpWriteCpuStatus (mMmCpuIo, QEMU_CPUHP_STAT_EJECTED); > > That is way too early -- given that the actual unplug will happen > in SmmCpuUpdate() and given that the BSPHandler() would have waited > for the APs multiple times before then. > > Another possibility is that the actual ejection be deferred to the > _EJ0 method after the return from the SMI. But, that seems like a > hack. Additionally, Igor points out here that this approach has problems: > https://lore.kernel.org/qemu-devel/20201204170939.1815522-1-imammedo@redhat.com/ > > Please review. thanks for the patches; I'm confirming I've got them. I'll need a non-trivial amount of time before I come to these patches (and to the QEMU patches posted by Igor). I'm working very busily on <https://bugzilla.tianocore.org/show_bug.cgi?id=3097> and my brain is full of other stuff. We had the reverse situation earlier this year, I think, when -- in relation to hotplug -- Igor was occupied with a more pressing QEMU development (NUMA I think?), for a significant amount of time. What's important is that I want to do both Igor's patches and your patches *justice*, with my review, and at this time I just cannot sit down with them alone for a day. These patches deserve a deep looking-at, rather than a skim, and I cannot afford the former at the moment. I prefer doing a (hopefully) thorough review, later, to rushing a review now. I hope that's tolerable. Thank you, Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#68646): https://edk2.groups.io/g/devel/message/68646 Mute This Topic: https://groups.io/mt/78802720/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 2020-12-10 1:21 a.m., Laszlo Ersek wrote:
> Hi Ankur,
>
> On 12/08/20 06:34, Ankur Arora wrote:
>> [ Resending to the correct edk2 alias this time. ]
>>
>> Hi,
>>
>> This series adds support for CPU hot-unplug with OVMF.
>>
>> Please see this in conjunction with the QEMU v2 series posted here:
>> https://lore.kernel.org/qemu-devel/20201207140739.3829993-1-imammedo@redhat.com/
>>
>> In particular, would be glad for comments on Patch 4, specifically
>> where we should be ejecting the CPU.
>>
>> Right now the ejection happens in UnplugCpus() (called from
>> CpuHotplugMmi()):
>> + QemuCpuhpWriteCpuSelector (mMmCpuIo, RemoveApicId);
>> + QemuCpuhpWriteCpuStatus (mMmCpuIo, QEMU_CPUHP_STAT_EJECTED);
>>
>> That is way too early -- given that the actual unplug will happen
>> in SmmCpuUpdate() and given that the BSPHandler() would have waited
>> for the APs multiple times before then.
>>
>> Another possibility is that the actual ejection be deferred to the
>> _EJ0 method after the return from the SMI. But, that seems like a
>> hack. Additionally, Igor points out here that this approach has problems:
>> https://lore.kernel.org/qemu-devel/20201204170939.1815522-1-imammedo@redhat.com/
>>
>> Please review.
>
> thanks for the patches; I'm confirming I've got them.
>
> I'll need a non-trivial amount of time before I come to these patches
> (and to the QEMU patches posted by Igor).
>
> I'm working very busily on
> <https://bugzilla.tianocore.org/show_bug.cgi?id=3097> and my brain is
> full of other stuff.
Thanks for letting me know. I empathize with not wanting to context
switch out all of your built up virtio-fs/ARM state.
>
> We had the reverse situation earlier this year, I think, when -- in
> relation to hotplug -- Igor was occupied with a more pressing QEMU
> development (NUMA I think?), for a significant amount of time.
>
> What's important is that I want to do both Igor's patches and your
> patches *justice*, with my review, and at this time I just cannot sit
> down with them alone for a day. These patches deserve a deep looking-at,
> rather than a skim, and I cannot afford the former at the moment. I
> prefer doing a (hopefully) thorough review, later, to rushing a review now.
I'll look forward to it. Anyway I think a deep look at these patches might
be wasted at the current stage. In particular there's a glaring hole in this
patch set which is how to handle the actual unplug (setting of
QEMU_CPUHP_STAT_EJECTED).
That's one thing I would be glad for a comment on: not right away, please
come back to this when you have thinking room.
So the problem is that my current approach -- setting QEMU_CPUHP_STAT_EJECTED
via the CpuHotplugMmi() handler definitely doesn't work given that it removes
an AP immediately while the SMI handler is still using it.
The two alternatives are:
- do this in SmmCpuFeaturesLib::SmmCpuFeaturesRendezvousExit() while exiting
SMI. That means that the only thing we will not do on the AP being unplugged
is restoring debug registers and a bunch of MSRs. Which AFAICS would be
okay, since the next time this AP is plugged in it will start from a clean
slate anyway.
- Qemu marks the hot-unplug when QEMU_CPUHP_STAT_EJECTED is set and defers it
until the SMI exit.
AFAICS, both ought to work. But, assuming it works (I haven't tried it out yet)
the first seems cleaner.
Ankur
>
> I hope that's tolerable.
>
> Thank you,
> Laszlo
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#68669): https://edk2.groups.io/g/devel/message/68669
Mute This Topic: https://groups.io/mt/78802720/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Thu, 10 Dec 2020 12:08:13 -0800 "Ankur Arora" <ankur.a.arora@oracle.com> wrote: > On 2020-12-10 1:21 a.m., Laszlo Ersek wrote: > > Hi Ankur, > > > > On 12/08/20 06:34, Ankur Arora wrote: > >> [ Resending to the correct edk2 alias this time. ] > >> > >> Hi, > >> > >> This series adds support for CPU hot-unplug with OVMF. > >> > >> Please see this in conjunction with the QEMU v2 series posted here: > >> https://lore.kernel.org/qemu-devel/20201207140739.3829993-1-imammedo@redhat.com/ > >> > >> In particular, would be glad for comments on Patch 4, specifically > >> where we should be ejecting the CPU. > >> > >> Right now the ejection happens in UnplugCpus() (called from > >> CpuHotplugMmi()): > >> + QemuCpuhpWriteCpuSelector (mMmCpuIo, RemoveApicId); > >> + QemuCpuhpWriteCpuStatus (mMmCpuIo, QEMU_CPUHP_STAT_EJECTED); > >> > >> That is way too early -- given that the actual unplug will happen > >> in SmmCpuUpdate() and given that the BSPHandler() would have waited > >> for the APs multiple times before then. > >> > >> Another possibility is that the actual ejection be deferred to the > >> _EJ0 method after the return from the SMI. But, that seems like a > >> hack. Additionally, Igor points out here that this approach has problems: > >> https://lore.kernel.org/qemu-devel/20201204170939.1815522-1-imammedo@redhat.com/ > >> > >> Please review. > > > > thanks for the patches; I'm confirming I've got them. > > > > I'll need a non-trivial amount of time before I come to these patches > > (and to the QEMU patches posted by Igor). > > > > I'm working very busily on > > <https://bugzilla.tianocore.org/show_bug.cgi?id=3097> and my brain is > > full of other stuff. > > Thanks for letting me know. I empathize with not wanting to context > switch out all of your built up virtio-fs/ARM state. > > > > > We had the reverse situation earlier this year, I think, when -- in > > relation to hotplug -- Igor was occupied with a more pressing QEMU > > development (NUMA I think?), for a significant amount of time. > > > > What's important is that I want to do both Igor's patches and your > > patches *justice*, with my review, and at this time I just cannot sit > > down with them alone for a day. These patches deserve a deep looking-at, > > rather than a skim, and I cannot afford the former at the moment. I > > prefer doing a (hopefully) thorough review, later, to rushing a review now. > > I'll look forward to it. Anyway I think a deep look at these patches might > be wasted at the current stage. In particular there's a glaring hole in this > patch set which is how to handle the actual unplug (setting of > QEMU_CPUHP_STAT_EJECTED). > > That's one thing I would be glad for a comment on: not right away, please > come back to this when you have thinking room. > > So the problem is that my current approach -- setting QEMU_CPUHP_STAT_EJECTED > via the CpuHotplugMmi() handler definitely doesn't work given that it removes > an AP immediately while the SMI handler is still using it. > > The two alternatives are: > - do this in SmmCpuFeaturesLib::SmmCpuFeaturesRendezvousExit() while exiting > SMI. That means that the only thing we will not do on the AP being unplugged > is restoring debug registers and a bunch of MSRs. Which AFAICS would be > okay, since the next time this AP is plugged in it will start from a clean > slate anyway. > > - Qemu marks the hot-unplug when QEMU_CPUHP_STAT_EJECTED is set and defers it > until the SMI exit. I don't like implementing workarounds on hw side for guest software sake. (it's occasionally done but only if there is no way to fix guest side, for example closed sources OS. So there shall be very good reason to do so) > AFAICS, both ought to work. But, assuming it works (I haven't tried it out yet) > the first seems cleaner. > > Ankur > > > > > > I hope that's tolerable. > > > > Thank you, > > Laszlo > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#68718): https://edk2.groups.io/g/devel/message/68718 Mute This Topic: https://groups.io/mt/78802720/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.