From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: Laszlo Ersek <lersek@redhat.com>,
edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Anthony Perard <anthony.perard@citrix.com>,
Ard Biesheuvel <ard.biesheuvel@arm.com>,
Jordan Justen <jordan.l.justen@intel.com>,
Julien Grall <julien@xen.org>, Rebecca Cran <rebecca@bsdio.com>
Subject: Re: [PATCH 6/6] OvmfPkg/ResetSystemLib: introduce the DxeResetSystemLib instance
Date: Fri, 17 Apr 2020 18:23:14 +0200 [thread overview]
Message-ID: <4cc7f027-5595-c7ea-5bd5-1c79c23734a8@redhat.com> (raw)
In-Reply-To: <20200417153751.7110-7-lersek@redhat.com>
On 4/17/20 5:37 PM, Laszlo Ersek wrote:
> The BaseResetSystemLib instance is not suitable for OS runtime, because
> its ResetShutdown() implementation calls PciRead16 (OVMF_HOSTBRIDGE_DID).
> On q35, this boils down to a memory-mapped config space access -- but we
> never ask the OS to map MMCONFIG for runtime.
>
> There are at least three alternatives to approach this:
>
> (1) Investigate "MdePkg/Library/DxeRuntimePciExpressLib", which offers
> some kind of runtime mapping for MMCONFIG.
>
> (2) Consume PciCf8Lib directly, rather than PciLib, in ResetSystemLib.
> Then we'll read OVMF_HOSTBRIDGE_DID from the config space with IO port
> accesses on q35 too, not just on i440fx. IO ports don't depend on page
> tables.
>
> (3) In the lib constructor, cache "mAcpiPmBaseAddress" based on
> "PcdOvmfHostBridgePciDevId" (which is set by PlatformPei). Then the
> host bridge type will be known at runtime without PCI config space
> accesses.
>
> This patch follows approach (3), in order to mirror AcpiTimerLib.
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
>
> Notes:
>
> * This patch is best viewed with "git show --find-copies-harder -C43".
>
> * PCDs are not usable in the DXE_CORE, as the PCD PPI is gone, and the PCD
> protocol is not available yet. (The DXE_CORE does consume ResetSystemLib
> in practice, when OVMF is built with -D SOURCE_DEBUG_ENABLE.)
>
> * The bug is not easy to trigger in common setups, because e.g. the Linux
> guest cannot easily be convinced to use the EFI runtime service for
> poweroff. One way to trigger the bug is to (a) patch OVMF as follows:
>
>> diff --git a/OvmfPkg/Library/ResetSystemLib/ResetSystemLib.c b/OvmfPkg/Library/ResetSystemLib/ResetSystemLib.c
>> index fe51f53d1df2..1edc4349ad20 100644
>> --- a/OvmfPkg/Library/ResetSystemLib/ResetSystemLib.c
>> +++ b/OvmfPkg/Library/ResetSystemLib/ResetSystemLib.c
>> @@ -28,6 +28,7 @@ ResetCold (
>> VOID
>> )
>> {
>> + ResetShutdown ();
>> IoWrite8 (0xCF9, BIT2 | BIT1); // 1st choice: PIIX3 RCR, RCPU|SRST
>> MicroSecondDelay (50);
>
> (b) boot a Linux guest with "reboot=efi" on q35, and (c) reboot the
> guest with the "reboot" command. Then the guest kernel will log:
>
>> reboot: Restarting system
>> reboot: machine restart
>> ------------[ cut here ]------------
>> [Firmware Bug]: Page fault caused by firmware at PA: 0xb0000002
>> WARNING: CPU: 0 PID: 1362 at arch/x86/platform/efi/quirks.c:738
>> efi_recover_from_page_fault+0x2a/0xc8
>> Modules linked in: ip_set nfnetlink sunrpc vfat fat intel_rapl_msr
>> intel_rapl_common kvm_intel kvm irqbypass crct10dif_pclmul crc32_pclmul
>> bochs_drm drm_vram_helper ttm ghash_clmulni_intel drm_kms_helper
>> iTCO_wdt iTCO_vendor
>> CPU: 0 PID: 1362 Comm: reboot Not tainted 5.3.6-200.fc30.x86_64 #1
>> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0
>> 02/06/2015
>> RIP: 0010:efi_recover_from_page_fault+0x2a/0xc8
>> Code: 0f 1f 44 00 00 8b 15 35 c2 c1 01 85 d2 74 09 48 81 ff ff 0f 00 00
>> 77 01 c3 53 48 89 fe 48 c7 c7 58 ba 12 aa 50 e8 74 f0 00 00 <0f> 0b 83
>> 3d 0d c2 c1 01 0a 0f 84 8f 00 00 00 48 8b 05 70 c8 4a 01
>> RSP: 0018:ffffaf3a402539d8 EFLAGS: 00010286
>> RAX: 0000000000000000 RBX: ffff8a3c33711f40 RCX: 00000000000003b2
>> RDX: 0000000000000001 RSI: 0000000000000096 RDI: 0000000000000246
>> RBP: ffffaf3a40253a88 R08: 0000000000000000 R09: 00000000000003b2
>> R10: 0000000000000001 R11: ffffffffa9ee3800 R12: 00000000b0000002
>> R13: 0000000000000000 R14: 000000000000000b R15: 0000000000000001
>> FS: 00007fbca9cf8940(0000) GS:ffff8a3c3ae00000(0000)
>> knlGS:0000000000000000
>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: fffffffeff8a67a1 CR3: 0000000072868006 CR4: 00000000001606f0
>> Call Trace:
>> no_context+0x15b/0x380
>> ? do_user_addr_fault+0x12e/0x440
>> do_page_fault+0x31/0x110
>> async_page_fault+0x3e/0x50
>> RIP: 0010:0xfffffffeff8a67a1
>> Code: Bad RIP value.
>> RSP: 0018:ffffaf3a40253b30 EFLAGS: 00010046
>> RAX: 00000000b0000002 RBX: 0000000000000000 RCX: 00000000b0000002
>> RDX: 00000000b0000000 RSI: 0000000000000000 RDI: fffffffeff8a80f0
>> RBP: ffffaf3a40253b60 R08: fffffffeff8aadb8 R09: 0000000000000000
>> R10: ffffffffaa5762c0 R11: ffffffffa9ee3800 R12: 0000000000000000
>> R13: 0000000000000000 R14: 0000000000000046 R15: 0000000000000000
>> ? efi_call+0x58/0x90
>> ? virt_efi_reset_system+0x8d/0x100
>> ? efi_reboot+0x85/0xb8
>> ? native_machine_emergency_restart+0x9f/0x250
>> ? native_apic_msr_read+0x16/0x20
>> ? disconnect_bsp_APIC+0x8c/0xd0
>> ? __do_sys_reboot+0x1d2/0x210
>> ? __fput+0x168/0x250
>> ? do_writev+0x6b/0x110
>> ? do_syscall_64+0x5f/0x1a0
>> ? entry_SYSCALL_64_after_hwframe+0x44/0xa9
>> ---[ end trace d5bee708166d198b ]---
>> efi: efi_reset_system() buggy! Reboot through BIOS
>
> 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>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2675
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> OvmfPkg/OvmfPkgIa32.dsc | 6 +++
> OvmfPkg/OvmfPkgIa32X64.dsc | 6 +++
> OvmfPkg/OvmfPkgX64.dsc | 6 +++
> OvmfPkg/OvmfXen.dsc | 4 ++
> OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf | 2 +-
> OvmfPkg/Library/ResetSystemLib/{BaseResetSystemLib.inf => DxeResetSystemLib.inf} | 21 +++++----
> OvmfPkg/Library/ResetSystemLib/{BaseResetShutdown.c => DxeResetShutdown.c} | 49 ++++++++++++--------
> 7 files changed, 66 insertions(+), 28 deletions(-)
>
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index cd0ed34e0e5a..d5e90c001370 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -314,6 +314,7 @@ [LibraryClasses.common.DXE_CORE]
> [LibraryClasses.common.DXE_RUNTIME_DRIVER]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> @@ -331,6 +332,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
> [LibraryClasses.common.UEFI_DRIVER]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> @@ -346,6 +348,7 @@ [LibraryClasses.common.UEFI_DRIVER]
> [LibraryClasses.common.DXE_DRIVER]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> @@ -383,6 +386,7 @@ [LibraryClasses.common.DXE_DRIVER]
> [LibraryClasses.common.UEFI_APPLICATION]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> @@ -396,6 +400,7 @@ [LibraryClasses.common.UEFI_APPLICATION]
> [LibraryClasses.common.DXE_SMM_DRIVER]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAllocationLib.inf
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> @@ -417,6 +422,7 @@ [LibraryClasses.common.DXE_SMM_DRIVER]
> [LibraryClasses.common.SMM_CORE]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> SmmCorePlatformHookLib|MdeModulePkg/Library/SmmCorePlatformHookLibNull/SmmCorePlatformHookLibNull.inf
> MemoryAllocationLib|MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/PiSmmCoreMemoryAllocationLib.inf
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 3c377c6e858e..066f49aeaee0 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -318,6 +318,7 @@ [LibraryClasses.common.DXE_CORE]
> [LibraryClasses.common.DXE_RUNTIME_DRIVER]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> @@ -335,6 +336,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
> [LibraryClasses.common.UEFI_DRIVER]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> @@ -350,6 +352,7 @@ [LibraryClasses.common.UEFI_DRIVER]
> [LibraryClasses.common.DXE_DRIVER]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> @@ -387,6 +390,7 @@ [LibraryClasses.common.DXE_DRIVER]
> [LibraryClasses.common.UEFI_APPLICATION]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> @@ -400,6 +404,7 @@ [LibraryClasses.common.UEFI_APPLICATION]
> [LibraryClasses.common.DXE_SMM_DRIVER]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAllocationLib.inf
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> @@ -421,6 +426,7 @@ [LibraryClasses.common.DXE_SMM_DRIVER]
> [LibraryClasses.common.SMM_CORE]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> SmmCorePlatformHookLib|MdeModulePkg/Library/SmmCorePlatformHookLibNull/SmmCorePlatformHookLibNull.inf
> MemoryAllocationLib|MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/PiSmmCoreMemoryAllocationLib.inf
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 701a7ccea987..ac510522a9ff 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -318,6 +318,7 @@ [LibraryClasses.common.DXE_CORE]
> [LibraryClasses.common.DXE_RUNTIME_DRIVER]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> @@ -335,6 +336,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
> [LibraryClasses.common.UEFI_DRIVER]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> @@ -350,6 +352,7 @@ [LibraryClasses.common.UEFI_DRIVER]
> [LibraryClasses.common.DXE_DRIVER]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> @@ -387,6 +390,7 @@ [LibraryClasses.common.DXE_DRIVER]
> [LibraryClasses.common.UEFI_APPLICATION]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> @@ -400,6 +404,7 @@ [LibraryClasses.common.UEFI_APPLICATION]
> [LibraryClasses.common.DXE_SMM_DRIVER]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAllocationLib.inf
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> @@ -421,6 +426,7 @@ [LibraryClasses.common.DXE_SMM_DRIVER]
> [LibraryClasses.common.SMM_CORE]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> SmmCorePlatformHookLib|MdeModulePkg/Library/SmmCorePlatformHookLibNull/SmmCorePlatformHookLibNull.inf
> MemoryAllocationLib|MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/PiSmmCoreMemoryAllocationLib.inf
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
> index 86b24d1716b9..f6214bd3465a 100644
> --- a/OvmfPkg/OvmfXen.dsc
> +++ b/OvmfPkg/OvmfXen.dsc
> @@ -288,6 +288,7 @@ [LibraryClasses.common.DXE_CORE]
>
> [LibraryClasses.common.DXE_RUNTIME_DRIVER]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> @@ -304,6 +305,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>
> [LibraryClasses.common.UEFI_DRIVER]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> @@ -318,6 +320,7 @@ [LibraryClasses.common.UEFI_DRIVER]
>
> [LibraryClasses.common.DXE_DRIVER]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> @@ -341,6 +344,7 @@ [LibraryClasses.common.DXE_DRIVER]
>
> [LibraryClasses.common.UEFI_APPLICATION]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> + ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> diff --git a/OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf b/OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
> index 0772780b2dc2..35d317f1e0b3 100644
> --- a/OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
> +++ b/OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
> @@ -12,7 +12,7 @@ [Defines]
> FILE_GUID = 66564872-21d4-4d2a-a68b-1e844f980820
> MODULE_TYPE = BASE
> VERSION_STRING = 1.0
> - LIBRARY_CLASS = ResetSystemLib
> + LIBRARY_CLASS = ResetSystemLib|SEC PEI_CORE PEIM DXE_CORE
>
> #
> # The following information is for reference only and not required by the build
> diff --git a/OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf b/OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> similarity index 43%
> copy from OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
> copy to OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> index 0772780b2dc2..a9b4ce90000a 100644
> --- a/OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
> +++ b/OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.inf
> @@ -1,18 +1,20 @@
> ## @file
> -# Base library instance for ResetSystem library class for OVMF
> +# DXE library instance for ResetSystem library class for OVMF
> #
> +# 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 = 0x00010005
> - BASE_NAME = BaseResetSystemLib
> - FILE_GUID = 66564872-21d4-4d2a-a68b-1e844f980820
> - MODULE_TYPE = BASE
> + INF_VERSION = 1.29
> + BASE_NAME = DxeResetSystemLib
> + FILE_GUID = bc7835ea-4094-41fe-b770-bad9e6c479b2
> + MODULE_TYPE = DXE_DRIVER
> VERSION_STRING = 1.0
> - LIBRARY_CLASS = ResetSystemLib
> + 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
> @@ -22,7 +24,7 @@ [Defines]
> #
>
> [Sources]
> - BaseResetShutdown.c
> + DxeResetShutdown.c
> ResetSystemLib.c
>
> [Packages]
> @@ -34,5 +36,8 @@ [LibraryClasses]
> BaseLib
> DebugLib
> IoLib
> - PciLib
> + PcdLib
> TimerLib
> +
> +[Pcd]
> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId ## CONSUMES
> diff --git a/OvmfPkg/Library/ResetSystemLib/BaseResetShutdown.c b/OvmfPkg/Library/ResetSystemLib/DxeResetShutdown.c
> similarity index 57%
> copy from OvmfPkg/Library/ResetSystemLib/BaseResetShutdown.c
> copy to OvmfPkg/Library/ResetSystemLib/DxeResetShutdown.c
> index 21c80e43230c..5a75c32df361 100644
> --- a/OvmfPkg/Library/ResetSystemLib/BaseResetShutdown.c
> +++ b/OvmfPkg/Library/ResetSystemLib/DxeResetShutdown.c
> @@ -1,5 +1,5 @@
> /** @file
> - Base Reset System Library Shutdown API implementation for OVMF.
> + DXE Reset System Library Shutdown API implementation for OVMF.
>
> Copyright (C) 2020, Red Hat, Inc.
> Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> @@ -11,41 +11,52 @@
> #include <Library/BaseLib.h> // CpuDeadLoop()
> #include <Library/DebugLib.h> // ASSERT()
> #include <Library/IoLib.h> // IoOr16()
> -#include <Library/PciLib.h> // PciRead16()
> +#include <Library/PcdLib.h> // PcdGet16()
> #include <Library/ResetSystemLib.h> // ResetShutdown()
> -#include <OvmfPlatforms.h> // OVMF_HOSTBRIDGE_DID
> +#include <OvmfPlatforms.h> // PIIX4_PMBA_VALUE
>
> -/**
> - Calling this function causes the system to enter a power state equivalent
> - to the ACPI G2/S5 or G3 states.
> +STATIC UINT16 mAcpiPmBaseAddress;
>
> - System shutdown should not return, if it returns, it means the system does
> - not support shut down reset.
> -**/
> -VOID
> +EFI_STATUS
> EFIAPI
> -ResetShutdown (
> - VOID
> +DxeResetInit (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> )
> {
> - UINT16 AcpiPmBaseAddress;
> UINT16 HostBridgeDevId;
>
> - AcpiPmBaseAddress = 0;
> - HostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID);
> + HostBridgeDevId = PcdGet16 (PcdOvmfHostBridgePciDevId);
> switch (HostBridgeDevId) {
> case INTEL_82441_DEVICE_ID:
> - AcpiPmBaseAddress = PIIX4_PMBA_VALUE;
> + mAcpiPmBaseAddress = PIIX4_PMBA_VALUE;
> break;
> case INTEL_Q35_MCH_DEVICE_ID:
> - AcpiPmBaseAddress = ICH9_PMBASE_VALUE;
> + mAcpiPmBaseAddress = ICH9_PMBASE_VALUE;
> break;
> default:
> ASSERT (FALSE);
> CpuDeadLoop ();
> + return EFI_UNSUPPORTED;
> }
>
> - IoBitFieldWrite16 (AcpiPmBaseAddress + 4, 10, 13, 0);
> - IoOr16 (AcpiPmBaseAddress + 4, BIT13);
> + 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 (mAcpiPmBaseAddress + 4, 10, 13, 0);
> + IoOr16 (mAcpiPmBaseAddress + 4, BIT13);
> CpuDeadLoop ();
> }
>
next prev parent reply other threads:[~2020-04-17 16:23 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-17 15:37 [PATCH 0/6] OvmfPkg/ResetSystemLib: clean up, refactor, fix Laszlo Ersek
2020-04-17 15:37 ` [PATCH 1/6] OvmfPkg/ResetSystemLib: wrap long lines Laszlo Ersek
2020-04-17 16:01 ` Philippe Mathieu-Daudé
2020-04-17 15:37 ` [PATCH 2/6] OvmfPkg/ResetSystemLib: clean up library dependencies Laszlo Ersek
2020-04-17 16:02 ` Philippe Mathieu-Daudé
2020-04-17 15:37 ` [PATCH 3/6] OvmfPkg/ResetSystemLib: improve coding style in ResetSystem() Laszlo Ersek
2020-04-17 16:02 ` Philippe Mathieu-Daudé
2020-04-17 15:37 ` [PATCH 4/6] OvmfPkg/ResetSystemLib: factor out ResetShutdown() Laszlo Ersek
2020-04-17 16:05 ` Philippe Mathieu-Daudé
2020-04-17 15:37 ` [PATCH 5/6] OvmfPkg/ResetSystemLib: rename to BaseResetSystemLib Laszlo Ersek
2020-04-21 17:27 ` [edk2-devel] " Laszlo Ersek
2020-04-21 18:08 ` Philippe Mathieu-Daudé
2020-04-17 15:37 ` [PATCH 6/6] OvmfPkg/ResetSystemLib: introduce the DxeResetSystemLib instance Laszlo Ersek
2020-04-17 16:23 ` Philippe Mathieu-Daudé [this message]
2020-04-17 15:59 ` [PATCH 0/6] OvmfPkg/ResetSystemLib: clean up, refactor, fix Ard Biesheuvel
2020-04-17 16:19 ` Philippe Mathieu-Daudé
2020-04-20 9:48 ` Laszlo Ersek
2020-04-20 10:17 ` Philippe Mathieu-Daudé
2020-04-20 9:46 ` Laszlo Ersek
2020-04-21 17:49 ` Rebecca Cran
2020-04-22 20:35 ` [edk2-devel] " Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4cc7f027-5595-c7ea-5bd5-1c79c23734a8@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox