[edk2-devel] [PATCH v6 3/9] OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper

Ankur Arora posted 9 patches 3 years, 9 months ago
There is a newer version of this series
[edk2-devel] [PATCH v6 3/9] OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper
Posted by Ankur Arora 3 years, 9 months ago
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 <lersek@redhat.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Aaron Young <aaron.young@oracle.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
 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 UINT8                        CpuStatus
+  );
+
 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 UINT8                        CpuStatus
+  )
+{
+  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
-- 
2.9.3



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#70870): https://edk2.groups.io/g/devel/message/70870
Mute This Topic: https://groups.io/mt/80199903/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v6 3/9] OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper
Posted by Laszlo Ersek 3 years, 9 months ago
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 <lersek@redhat.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Aaron Young <aaron.young@oracle.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
> Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
> ---
>  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 UINT8                        CpuStatus
> +  );
> +
>  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 UINT8                        CpuStatus
> +  )
> +{
> +  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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v6 3/9] OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper
Posted by Ankur Arora 3 years, 9 months ago
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 <lersek@redhat.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> Cc: Aaron Young <aaron.young@oracle.com>
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
>> Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
>> ---
>>   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 UINT8                        CpuStatus
>> +  );
>> +
>>   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 UINT8                        CpuStatus
>> +  )
>> +{
>> +  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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-