Re: [PATCH 0/6] OvmfPkg/ResetSystemLib: clean up, refactor, fix

["Ard Biesheuvel" <ard.biesheuvel@arm.com>]

On 4/17/20 5:37 PM, Laszlo Ersek wrote:
> Ref:    https://bugzilla.tianocore.org/show_bug.cgi?id=2675
> Repo:   https://pagure.io/lersek/edk2.git
> Branch: rsl_cleanup
> 
> Rebecca's
> 
>    [PATCH 02/13] OvmfPkg: support powering off bhyve guests
> 
> at
> 
>    https://edk2.groups.io/g/devel/message/57450
>    http://mid.mail-archive.com/e4e9b29189b83076e1d1a0b9c989938f5226cab6.1586991816.git.rebecca@bsdio.com
> 
> made me realize OvmfPkg/ResetSystemLib should be refreshed. This will
> let us add a simple bhyve-specific instance (later), and also allows us
> to fix a long time dormant bug (now).
> 
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Julien Grall <julien@xen.org>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Cc: Rebecca Cran <rebecca@bsdio.com>
> 
> Thanks,
> Laszlo
> 
> Laszlo Ersek (6):
>    OvmfPkg/ResetSystemLib: wrap long lines
>    OvmfPkg/ResetSystemLib: clean up library dependencies
>    OvmfPkg/ResetSystemLib: improve coding style in ResetSystem()
>    OvmfPkg/ResetSystemLib: factor out ResetShutdown()
>    OvmfPkg/ResetSystemLib: rename to BaseResetSystemLib
>    OvmfPkg/ResetSystemLib: introduce the DxeResetSystemLib instance
> 

For the series,

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

One nit: putting a diff block inside the commit log [6/6] doesn't help 
legibility a lot, and the issue of not being able to access memory that 
is not mapped for runtime is so basic that it doesn't require that level 
of detail to describe a reproducer and the Linux kernel log output when 
the issue is triggered.


>   OvmfPkg/OvmfPkgIa32.dsc                                                       |  8 +-
>   OvmfPkg/OvmfPkgIa32X64.dsc                                                    |  8 +-
>   OvmfPkg/OvmfPkgX64.dsc                                                        |  8 +-
>   OvmfPkg/OvmfXen.dsc                                                           |  6 +-
>   OvmfPkg/Library/ResetSystemLib/{ResetSystemLib.inf => BaseResetSystemLib.inf} | 12 ++-
>   OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf                          | 43 ++++++++++
>   OvmfPkg/Library/ResetSystemLib/BaseResetShutdown.c                            | 51 ++++++++++++
>   OvmfPkg/Library/ResetSystemLib/DxeResetShutdown.c                             | 62 +++++++++++++++
>   OvmfPkg/Library/ResetSystemLib/ResetSystemLib.c                               | 84 +++++---------------
>   9 files changed, 209 insertions(+), 73 deletions(-)
>   rename OvmfPkg/Library/ResetSystemLib/{ResetSystemLib.inf => BaseResetSystemLib.inf} (65%)
>   create mode 100644 OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
>   create mode 100644 OvmfPkg/Library/ResetSystemLib/BaseResetShutdown.c
>   create mode 100644 OvmfPkg/Library/ResetSystemLib/DxeResetShutdown.c
>