[v4,09/35] OvmfPkg/OvmfXen: use a TimerLib instance that depends only on the CPU

Commit Message

Anthony PERARD July 29, 2019, 3:39 p.m. UTC

The ACPI Timer isn't present in a PVH guest, but local APIC works on
both PVH and HVM.

Note that the use of SecPeiDxeTimerLibCpu might be an issue with a
driver of type DXE_RUNTIME_DRIVER. I've attempted to find out which of
the DXE_RUNTIME_DRIVER uses the TimerLib at runtime. I've done that by
replacing the TimerLib evaluation in
[LibraryClasses.common.DXE_RUNTIME_DRIVER] by a different one and
checking every module that uses it (with the --report-file=report
build option).

ResetSystemRuntimeDxe is calling the TimerLib API at runtime to do the
operation "EfiResetCold", so this may never complete if the OS have
disabled the Local APIC Timer.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1689
Signed-off-by: Anthony PERARD <[e-mail]>
Acked-by: Laszlo Ersek <[e-mail]>
---

Notes:
    v4:
    - reworded the first sentence, use "not present" instead of "don't work".
    
    v3:
    - cleanup .dsc, leave only one TimerLib resolution
    - Added a note in the commit message regarding the use of the local apic
      by runtime drivers

 OvmfPkg/OvmfXen.dsc | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

Comments

Laszlo Ersek July 30, 2019, 9:41 a.m. UTC | #1

On 07/29/19 17:39, Anthony PERARD wrote:
> The ACPI Timer isn't present in a PVH guest, but local APIC works on
> both PVH and HVM.
> 
> Note that the use of SecPeiDxeTimerLibCpu might be an issue with a
> driver of type DXE_RUNTIME_DRIVER. I've attempted to find out which of
> the DXE_RUNTIME_DRIVER uses the TimerLib at runtime. I've done that by
> replacing the TimerLib evaluation in
> [LibraryClasses.common.DXE_RUNTIME_DRIVER] by a different one and
> checking every module that uses it (with the --report-file=report
> build option).
> 
> ResetSystemRuntimeDxe is calling the TimerLib API at runtime to do the
> operation "EfiResetCold", so this may never complete if the OS have
> disabled the Local APIC Timer.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1689
> Signed-off-by: Anthony PERARD <[e-mail]>
> Acked-by: Laszlo Ersek <[e-mail]>
> ---
> 
> Notes:
>     v4:
>     - reworded the first sentence, use "not present" instead of "don't work".

Plus two typo fixes.

Acked-by: Laszlo Ersek <[e-mail]>

Thanks
Laszlo

Roger Pau Monné Aug. 7, 2019, 2:25 p.m. UTC | #2

On Mon, Jul 29, 2019 at 04:39:18PM +0100, Anthony PERARD wrote:
> The ACPI Timer isn't present in a PVH guest, but local APIC works on
> both PVH and HVM.
> 
> Note that the use of SecPeiDxeTimerLibCpu might be an issue with a
> driver of type DXE_RUNTIME_DRIVER. I've attempted to find out which of
> the DXE_RUNTIME_DRIVER uses the TimerLib at runtime. I've done that by
> replacing the TimerLib evaluation in
> [LibraryClasses.common.DXE_RUNTIME_DRIVER] by a different one and
> checking every module that uses it (with the --report-file=report
> build option).
> 
> ResetSystemRuntimeDxe is calling the TimerLib API at runtime to do the
> operation "EfiResetCold", so this may never complete if the OS have
> disabled the Local APIC Timer.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1689
> Signed-off-by: Anthony PERARD <[e-mail]>
> Acked-by: Laszlo Ersek <[e-mail]>
> ---
> 
> Notes:
>     v4:
>     - reworded the first sentence, use "not present" instead of "don't work".

Thanks, I'm afraid I cannot comment on the change itself, but the
comment message is accurate now :).

Roger.

Patch

diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 7619a89382..b40d39e003 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -104,7 +104,7 @@  [SkuIds]
 ################################################################################
 [LibraryClasses]
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
-  TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
+  TimerLib|MdePkg/Library/SecPeiDxeTimerLibCpu/SecPeiDxeTimerLibCpu.inf
   PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
   BaseMemoryLib|MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
   BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
@@ -202,7 +202,6 @@  [LibraryClasses.common]
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
 
 [LibraryClasses.common.SEC]
-  TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
   QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
 !ifdef $(DEBUG_ON_SERIAL_PORT)
   DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
@@ -281,7 +280,6 @@  [LibraryClasses.common.DXE_CORE]
 
 [LibraryClasses.common.DXE_RUNTIME_DRIVER]
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
-  TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
   HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
   DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
   MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
@@ -298,7 +296,6 @@  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
 
 [LibraryClasses.common.UEFI_DRIVER]
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
-  TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
   HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
   DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
   MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
@@ -313,7 +310,6 @@  [LibraryClasses.common.UEFI_DRIVER]
 
 [LibraryClasses.common.DXE_DRIVER]
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
-  TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
   HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
   MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
   ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
@@ -337,7 +333,6 @@  [LibraryClasses.common.DXE_DRIVER]
 
 [LibraryClasses.common.UEFI_APPLICATION]
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
-  TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
   HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
   MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
   ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf