Re: [edk2-devel] [PATCH v6 3/9] OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper
On 2021-01-29 6:36 p.m., Laszlo Ersek wrote: On 01/29/21 01:59, Ankur Arora wrote: Add QemuCpuhpWriteCpuStatus() which will be used to update the QEMU CPU status register. On error, it hangs in a similar fashion as other helper functions. Cc: Laszlo Ersek Cc: Jordan Justen Cc: Ard Biesheuvel Cc: Igor Mammedov Cc: Boris Ostrovsky Cc: Aaron Young Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132 Signed-off-by: Ankur Arora --- OvmfPkg/CpuHotplugSmm/QemuCpuhp.h | 6 ++ OvmfPkg/CpuHotplugSmm/QemuCpuhp.c | 22 ++ 2 files changed, 28 insertions(+) diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h index 8adaa0ad91f0..804809846890 100644 --- a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h +++ b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h @@ -30,6 +30,12 @@ QemuCpuhpReadCpuStatus ( IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo ); +VOID +QemuCpuhpWriteCpuStatus ( + IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo, + IN UINT8CpuStatus + ); + UINT32 QemuCpuhpReadCommandData ( IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c index f871e50c377b..ed44264de934 100644 --- a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c +++ b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c @@ -67,6 +67,28 @@ QemuCpuhpReadCpuStatus ( return CpuStatus; } +VOID +QemuCpuhpWriteCpuStatus ( + IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo, + IN UINT8CpuStatus + ) +{ + EFI_STATUS Status; + + Status = MmCpuIo->Io.Write ( + MmCpuIo, + MM_IO_UINT8, + ICH9_CPU_HOTPLUG_BASE + QEMU_CPUHP_R_CPU_STAT, + 1, + &CpuStatus + ); + if (EFI_ERROR (Status)) { +DEBUG ((DEBUG_ERROR, "%a: %r\n", __FUNCTION__, Status)); +ASSERT (FALSE); +CpuDeadLoop (); + } +} + UINT32 QemuCpuhpReadCommandData ( IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo The code is fine, but please move the new function (both declaration and definition) between QemuCpuhpWriteCpuSelector() and QemuCpuhpWriteCommand(). Reason: the pre-patch order of the functions matches the order of the register descriptions in QEMU's "docs/specs/acpi_cpu_hotplug.txt". There, we first have a section called "read access", then another called "write access". And in each section, registers are listed in increasing offset order, within the hotplug register block. Will fix. Thanks Ankur Thanks! Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#71048): https://edk2.groups.io/g/devel/message/71048 Mute This Topic: https://groups.io/mt/80199903/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v6 3/9] OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper
On 01/29/21 01:59, Ankur Arora wrote: > Add QemuCpuhpWriteCpuStatus() which will be used to update the QEMU > CPU status register. On error, it hangs in a similar fashion as > other helper functions. > > Cc: Laszlo Ersek > Cc: Jordan Justen > Cc: Ard Biesheuvel > Cc: Igor Mammedov > Cc: Boris Ostrovsky > Cc: Aaron Young > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132 > Signed-off-by: Ankur Arora > --- > OvmfPkg/CpuHotplugSmm/QemuCpuhp.h | 6 ++ > OvmfPkg/CpuHotplugSmm/QemuCpuhp.c | 22 ++ > 2 files changed, 28 insertions(+) > > diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h > b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h > index 8adaa0ad91f0..804809846890 100644 > --- a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h > +++ b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h > @@ -30,6 +30,12 @@ QemuCpuhpReadCpuStatus ( >IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo >); > > +VOID > +QemuCpuhpWriteCpuStatus ( > + IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo, > + IN UINT8CpuStatus > + ); > + > UINT32 > QemuCpuhpReadCommandData ( >IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo > diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c > b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c > index f871e50c377b..ed44264de934 100644 > --- a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c > +++ b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c > @@ -67,6 +67,28 @@ QemuCpuhpReadCpuStatus ( >return CpuStatus; > } > > +VOID > +QemuCpuhpWriteCpuStatus ( > + IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo, > + IN UINT8CpuStatus > + ) > +{ > + EFI_STATUS Status; > + > + Status = MmCpuIo->Io.Write ( > + MmCpuIo, > + MM_IO_UINT8, > + ICH9_CPU_HOTPLUG_BASE + QEMU_CPUHP_R_CPU_STAT, > + 1, > + &CpuStatus > + ); > + if (EFI_ERROR (Status)) { > +DEBUG ((DEBUG_ERROR, "%a: %r\n", __FUNCTION__, Status)); > +ASSERT (FALSE); > +CpuDeadLoop (); > + } > +} > + > UINT32 > QemuCpuhpReadCommandData ( >IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo > The code is fine, but please move the new function (both declaration and definition) between QemuCpuhpWriteCpuSelector() and QemuCpuhpWriteCommand(). Reason: the pre-patch order of the functions matches the order of the register descriptions in QEMU's "docs/specs/acpi_cpu_hotplug.txt". There, we first have a section called "read access", then another called "write access". And in each section, registers are listed in increasing offset order, within the hotplug register block. Thanks! Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#70918): https://edk2.groups.io/g/devel/message/70918 Mute This Topic: https://groups.io/mt/80199903/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-