[PATCH 0/2] Add DxeResetSystemLibBhyve

[PATCH 0/2] Add DxeResetSystemLibBhyve

[Rebecca Cran]

Add DxeResetSystemLibBhyve and update BhyvePkg/BhyvePkgX64.dsc to use it.

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>

Rebecca Cran (2):
  OvmfPkg: Add DxeResetSystemLibBhyve
  BhyvePkg: Update BhyvePkgX64.dsc to use DxeResetSystemLibBhyve

 BhyvePkg/BhyvePkgX64.dsc                      |  2 +-
 OvmfPkg/Include/IndustryStandard/Bhyve.h      |  2 +
 .../ResetSystemLib/DxeResetShutdownBhyve.c    | 43 +++++++++++++++++++
 .../ResetSystemLib/DxeResetSystemLibBhyve.inf | 38 ++++++++++++++++
 4 files changed, 84 insertions(+), 1 deletion(-)
 create mode 100644 OvmfPkg/Library/ResetSystemLib/DxeResetShutdownBhyve.c
 create mode 100644 OvmfPkg/Library/ResetSystemLib/DxeResetSystemLibBhyve.inf

-- 
2.20.1

[PATCH 1/2] OvmfPkg: Add DxeResetSystemLibBhyve

[Rebecca Cran]

Introduce the DxeResetSystemLibBhyve library to support powering off
bhyve guests.

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
 OvmfPkg/Include/IndustryStandard/Bhyve.h      |  2 +
 .../ResetSystemLib/DxeResetShutdownBhyve.c    | 43 +++++++++++++++++++
 .../ResetSystemLib/DxeResetSystemLibBhyve.inf | 38 ++++++++++++++++
 3 files changed, 83 insertions(+)
 create mode 100644 OvmfPkg/Library/ResetSystemLib/DxeResetShutdownBhyve.c
 create mode 100644 OvmfPkg/Library/ResetSystemLib/DxeResetSystemLibBhyve.inf

diff --git a/OvmfPkg/Include/IndustryStandard/Bhyve.h b/OvmfPkg/Include/IndustryStandard/Bhyve.h
index 02ce5587ee..9745d62688 100644
--- a/OvmfPkg/Include/IndustryStandard/Bhyve.h
+++ b/OvmfPkg/Include/IndustryStandard/Bhyve.h
@@ -13,4 +13,6 @@
 
 #define BHYVE_ACPI_TIMER_IO_ADDR 0x408
 
+#define BHYVE_PM_VALUE 0x408
+
 #endif // __BHYVE_H__
diff --git a/OvmfPkg/Library/ResetSystemLib/DxeResetShutdownBhyve.c b/OvmfPkg/Library/ResetSystemLib/DxeResetShutdownBhyve.c
new file mode 100644
index 0000000000..cb30d75ee2
--- /dev/null
+++ b/OvmfPkg/Library/ResetSystemLib/DxeResetShutdownBhyve.c
@@ -0,0 +1,43 @@
+/** @file
+  DXE Reset System Library Shutdown API implementation for bhyve.
+
+  Copyright (C) 2020, Rebecca Cran <rebecca@bsdio.com>
+  Copyright (C) 2020, Red Hat, Inc.
+  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Base.h>                   // BIT13
+
+#include <Library/BaseLib.h>        // CpuDeadLoop()
+#include <Library/IoLib.h>          // IoOr16()
+#include <Library/ResetSystemLib.h> // ResetShutdown()
+#include <OvmfPlatforms.h>          // BHYVE_PM_VALUE
+
+EFI_STATUS
+EFIAPI
+DxeResetInit (
+  IN EFI_HANDLE       ImageHandle,
+  IN EFI_SYSTEM_TABLE *SystemTable
+  )
+{
+  return EFI_SUCCESS;
+}
+
+/**
+  Calling this function causes the system to enter a power state equivalent
+  to the ACPI G2/S5 or G3 states.
+
+  System shutdown should not return, if it returns, it means the system does
+  not support shut down reset.
+**/
+VOID
+EFIAPI
+ResetShutdown (
+  VOID
+  )
+{
+  IoBitFieldWrite16 (BHYVE_PM_VALUE, 10, 13, 5);
+  IoOr16 (BHYVE_PM_VALUE, BIT13);
+  CpuDeadLoop ();
+}
diff --git a/OvmfPkg/Library/ResetSystemLib/DxeResetSystemLibBhyve.inf b/OvmfPkg/Library/ResetSystemLib/DxeResetSystemLibBhyve.inf
new file mode 100644
index 0000000000..c7923a4755
--- /dev/null
+++ b/OvmfPkg/Library/ResetSystemLib/DxeResetSystemLibBhyve.inf
@@ -0,0 +1,38 @@
+## @file
+#  DXE library instance for ResetSystem library class for bhyve
+#
+#  Copyright (C) 2020, Red Hat, Inc.
+#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 1.29
+  BASE_NAME                      = DxeResetSystemLibBhyve
+  FILE_GUID                      = 88a12688-98f5-44a6-bf4b-7894706a2d11
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = ResetSystemLib|DXE_DRIVER DXE_RUNTIME_DRIVER SMM_CORE DXE_SMM_DRIVER UEFI_DRIVER UEFI_APPLICATION
+  CONSTRUCTOR                    = DxeResetInit
+
+#
+# The following information is for reference only and not required by the build
+# tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64
+#
+
+[Sources]
+  DxeResetShutdownBhyve.c
+  ResetSystemLib.c
+
+[Packages]
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  IoLib
+  TimerLib
-- 
2.20.1

Re: [edk2-devel] [PATCH 1/2] OvmfPkg: Add DxeResetSystemLibBhyve

[Laszlo Ersek]

On 04/23/20 05:02, Rebecca Cran wrote:
> Introduce the DxeResetSystemLibBhyve library to support powering off
> bhyve guests.

(1) Please replace all "DxeResetSystemLibBhyve" references in the patch
(code, commit message, subject line, file names) with
"BaseResetSystemLibBhyve".

The reason is that the library instance you are introducing is suitable
for all firmware phases and module types. We don't perform any action
that would preclude a particular firmware phase. And we also don't have
phase-specific opportunities for performance improvements. And so it
makes sense to make this a BASE library, and support the broadest
collection of module types.

> 
> Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>  OvmfPkg/Include/IndustryStandard/Bhyve.h      |  2 +
>  .../ResetSystemLib/DxeResetShutdownBhyve.c    | 43 +++++++++++++++++++
>  .../ResetSystemLib/DxeResetSystemLibBhyve.inf | 38 ++++++++++++++++
>  3 files changed, 83 insertions(+)
>  create mode 100644 OvmfPkg/Library/ResetSystemLib/DxeResetShutdownBhyve.c
>  create mode 100644 OvmfPkg/Library/ResetSystemLib/DxeResetSystemLibBhyve.inf
> 
> diff --git a/OvmfPkg/Include/IndustryStandard/Bhyve.h b/OvmfPkg/Include/IndustryStandard/Bhyve.h
> index 02ce5587ee..9745d62688 100644
> --- a/OvmfPkg/Include/IndustryStandard/Bhyve.h
> +++ b/OvmfPkg/Include/IndustryStandard/Bhyve.h
> @@ -13,4 +13,6 @@
>  
>  #define BHYVE_ACPI_TIMER_IO_ADDR 0x408
>  
> +#define BHYVE_PM_VALUE 0x408
> +
>  #endif // __BHYVE_H__
> diff --git a/OvmfPkg/Library/ResetSystemLib/DxeResetShutdownBhyve.c b/OvmfPkg/Library/ResetSystemLib/DxeResetShutdownBhyve.c
> new file mode 100644
> index 0000000000..cb30d75ee2
> --- /dev/null
> +++ b/OvmfPkg/Library/ResetSystemLib/DxeResetShutdownBhyve.c
> @@ -0,0 +1,43 @@
> +/** @file
> +  DXE Reset System Library Shutdown API implementation for bhyve.
> +
> +  Copyright (C) 2020, Rebecca Cran <rebecca@bsdio.com>
> +  Copyright (C) 2020, Red Hat, Inc.
> +  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <Base.h>                   // BIT13
> +
> +#include <Library/BaseLib.h>        // CpuDeadLoop()
> +#include <Library/IoLib.h>          // IoOr16()
> +#include <Library/ResetSystemLib.h> // ResetShutdown()
> +#include <OvmfPlatforms.h>          // BHYVE_PM_VALUE

(2) Note that you are adding BHYVE_PM_VALUE to
"OvmfPkg/Include/IndustryStandard/Bhyve.h" (very correctly), and
presently, "OvmfPlatforms.h" does not include "Bhyve.h".

Therefore I suggest replacing the above #include with one for
"IndustryStandard/Bhyve.h".

(And then please re-sort the #include directives.)

> +
> +EFI_STATUS
> +EFIAPI
> +DxeResetInit (
> +  IN EFI_HANDLE       ImageHandle,
> +  IN EFI_SYSTEM_TABLE *SystemTable
> +  )
> +{
> +  return EFI_SUCCESS;
> +}
> +

(3) A CONSTRUCTOR does not look necessary for this library instance.

> +/**
> +  Calling this function causes the system to enter a power state equivalent
> +  to the ACPI G2/S5 or G3 states.
> +
> +  System shutdown should not return, if it returns, it means the system does
> +  not support shut down reset.
> +**/
> +VOID
> +EFIAPI
> +ResetShutdown (
> +  VOID
> +  )
> +{
> +  IoBitFieldWrite16 (BHYVE_PM_VALUE, 10, 13, 5);
> +  IoOr16 (BHYVE_PM_VALUE, BIT13);
> +  CpuDeadLoop ();
> +}
> diff --git a/OvmfPkg/Library/ResetSystemLib/DxeResetSystemLibBhyve.inf b/OvmfPkg/Library/ResetSystemLib/DxeResetSystemLibBhyve.inf
> new file mode 100644
> index 0000000000..c7923a4755
> --- /dev/null
> +++ b/OvmfPkg/Library/ResetSystemLib/DxeResetSystemLibBhyve.inf
> @@ -0,0 +1,38 @@
> +## @file
> +#  DXE library instance for ResetSystem library class for bhyve

(4) Please update this comment too.

> +#
> +#  Copyright (C) 2020, Red Hat, Inc.
> +#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent

(5) I think your (C) notice is needed here.

> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 1.29
> +  BASE_NAME                      = DxeResetSystemLibBhyve
> +  FILE_GUID                      = 88a12688-98f5-44a6-bf4b-7894706a2d11
> +  MODULE_TYPE                    = DXE_DRIVER

(6) Flip this to BASE please.

> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = ResetSystemLib|DXE_DRIVER DXE_RUNTIME_DRIVER SMM_CORE DXE_SMM_DRIVER UEFI_DRIVER UEFI_APPLICATION

(7) Please drop the module type restrictions (the part starting with "|").

Thanks!
Laszlo

> +  CONSTRUCTOR                    = DxeResetInit
> +
> +#
> +# The following information is for reference only and not required by the build
> +# tools.
> +#
> +#  VALID_ARCHITECTURES           = IA32 X64
> +#
> +
> +[Sources]
> +  DxeResetShutdownBhyve.c
> +  ResetSystemLib.c
> +
> +[Packages]
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  OvmfPkg/OvmfPkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  IoLib
> +  TimerLib
> 

[PATCH 2/2] BhyvePkg: Update BhyvePkgX64.dsc to use DxeResetSystemLibBhyve

[Rebecca Cran]

bhyve uses a different method for powering off guests than OVMF, so
use the new DxeResetSystemLibBhyve library to support that.

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
Cc: Peter Grehan <grehan@freebsd.org>
---
 BhyvePkg/BhyvePkgX64.dsc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BhyvePkg/BhyvePkgX64.dsc b/BhyvePkg/BhyvePkgX64.dsc
index 51e565994c..554adeb9b9 100644
--- a/BhyvePkg/BhyvePkgX64.dsc
+++ b/BhyvePkg/BhyvePkgX64.dsc
@@ -173,7 +173,7 @@
   DebugAgentLib|MdeModulePkg/Library/DebugAgentLibNull/DebugAgentLibNull.inf
 !endif
 
-  ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
+  ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLibBhyve.inf
   LocalApicLib|UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
   IoApicLib|PcAtChipsetPkg/Library/BaseIoApicLib/BaseIoApicLib.inf
   DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
-- 
2.20.1

Re: [edk2-devel] [PATCH 2/2] BhyvePkg: Update BhyvePkgX64.dsc to use DxeResetSystemLibBhyve

[Laszlo Ersek]

On 04/23/20 05:02, Rebecca Cran wrote:
> bhyve uses a different method for powering off guests than OVMF, so
> use the new DxeResetSystemLibBhyve library to support that.
> 
> Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
> Cc: Peter Grehan <grehan@freebsd.org>
> ---
>  BhyvePkg/BhyvePkgX64.dsc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/BhyvePkg/BhyvePkgX64.dsc b/BhyvePkg/BhyvePkgX64.dsc
> index 51e565994c..554adeb9b9 100644
> --- a/BhyvePkg/BhyvePkgX64.dsc
> +++ b/BhyvePkg/BhyvePkgX64.dsc
> @@ -173,7 +173,7 @@
>    DebugAgentLib|MdeModulePkg/Library/DebugAgentLibNull/DebugAgentLibNull.inf
>  !endif
>  
> -  ResetSystemLib|OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> +  ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLibBhyve.inf
>    LocalApicLib|UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
>    IoApicLib|PcAtChipsetPkg/Library/BaseIoApicLib/BaseIoApicLib.inf
>    DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
> 

This patch should be submitted later, because it depends on both your
BhyvePkg series, and on patch#1 in this series.

Further, under patch#1, I'm going to recommend to call this instance
BaseResetSystemLibBhyve, not "Dxe...". I'll explain why under patch#1.

Thanks
Laszlo

Re: [PATCH 0/2] Add DxeResetSystemLibBhyve

[Rebecca Cran]

On 4/22/20 9:02 PM, Rebecca Cran wrote:
> Add DxeResetSystemLibBhyve and update BhyvePkg/BhyvePkgX64.dsc to use it.
>
> Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
>
> Rebecca Cran (2):
>    OvmfPkg: Add DxeResetSystemLibBhyve
>    BhyvePkg: Update BhyvePkgX64.dsc to use DxeResetSystemLibBhyve

This series depends on the series "Add BhyvePkg, to support the bhyve 
hypervisor".


-- 
Rebecca Cran