From ede4877e3bbb2c9234dc45de16c2d5d6be4aa97b Mon Sep 17 00:00:00 2001
From: Mikhail Krichanov <mikhailkrichanov@gmail.com>
Date: Thu, 5 Sep 2024 15:54:33 +0300
Subject: [PATCH] Ring3: Fixed buggy timer interrupt handling for X64.

---
 OvmfPkg/OvmfPkgIa32X64.dsc                    |  1 +
 .../X64/ArchExceptionHandler.c                | 48 ++++++++++---------
 .../X64/ArchInterruptDefs.h                   |  2 +-
 .../X64/ExceptionHandlerAsm.nasm              | 24 +++-------
 4 files changed, 35 insertions(+), 40 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index e05a82140a..9d1c45648c 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -611,6 +611,7 @@
     gEfiMdePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x00000003
   !endif
   gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|TRUE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdEnableUserSpace|TRUE
 
 ################################################################################
 #
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
index a1275b53df..0d8edb6b81 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
@@ -208,21 +208,23 @@ ArchSetupExceptionStack (
   // be filled by processor during task switching.
   //
   TssBase = (UINTN)Tss;
-
+  //
+  // Last byte of bitmap must be followed by a byte with all bits set.
+  //
   TssDesc->Uint128.Uint64   = 0;
   TssDesc->Uint128.Uint64_1 = 0;
-  TssDesc->Bits.LimitLow    = (UINT16)(CPU_TSS_SIZE - 1);
+  TssDesc->Bits.LimitLow    = (UINT16)(CPU_TSS_SIZE - 2);
   TssDesc->Bits.BaseLow     = (UINT16)TssBase;
   TssDesc->Bits.BaseMidl    = (UINT8)(TssBase >> 16);
   TssDesc->Bits.Type        = IA32_GDT_TYPE_TSS;
   TssDesc->Bits.DPL         = 3;
   TssDesc->Bits.P           = 1;
-  TssDesc->Bits.LimitHigh   = (CPU_TSS_SIZE - 1) >> 16;
+  TssDesc->Bits.LimitHigh   = (CPU_TSS_SIZE - 2) >> 16;
   TssDesc->Bits.BaseMidh    = (UINT8)(TssBase >> 24);
   TssDesc->Bits.BaseHigh    = (UINT32)(TssBase >> 32);
 
   //
-  // Fixup exception task descriptor and task-state segment
+  // Set I/O Permission Bit Map
   //
   ZeroMem (Tss, sizeof (*Tss));
   //
@@ -230,6 +232,26 @@ ArchSetupExceptionStack (
   //
   StackTop = StackTop - CPU_STACK_ALIGNMENT + 1;
   StackTop = (UINTN)ALIGN_POINTER (StackTop, CPU_STACK_ALIGNMENT);
+
+  Tss->RSP0             = StackTop;
+  Tss->IOMapBaseAddress = sizeof (IA32_TASK_STATE_SEGMENT);
+  //
+  // Allow access to gUartBase = 0x3F8 and Offsets: 0x01, 0x03, 0x04, 0x05, 0x06
+  //
+  IOBitMap = (UINT8 *)((UINTN)Tss + Tss->IOMapBaseAddress);
+  for (Index = 0; Index < IO_BIT_MAP_SIZE; ++Index) {
+    if ((Index * 8) == 0x3F8) {
+      *IOBitMap = 0x84;
+    } else {
+      *IOBitMap = 0xFF;
+    }
+
+    ++IOBitMap;
+  }
+
+  //
+  // Fixup IST and task-state segment
+  //
   IdtTable = (IA32_IDT_GATE_DESCRIPTOR  *)Idtr.Base;
   for (Index = 0; Index < CPU_STACK_SWITCH_EXCEPTION_NUMBER; ++Index) {
     //
@@ -256,24 +278,6 @@ ArchSetupExceptionStack (
   //
   AsmWriteGdtr (&Gdtr);
 
-  //
-  // Set I/O Permission Bit Map
-  //
-  Tss->IOMapBaseAddress = sizeof (IA32_TASK_STATE_SEGMENT);
-  //
-  // Allow access to gUartBase = 0x3F8 and Offsets: 0x01, 0x03, 0x04, 0x05, 0x06
-  //
-  IOBitMap = (UINT8 *)((UINTN)Tss + Tss->IOMapBaseAddress);
-  for (Index = 0; Index < IO_BIT_MAP_SIZE; ++Index) {
-    if ((Index * 8) == 0x3F8) {
-      *IOBitMap = 0x84;
-    } else {
-      *IOBitMap = 0xFF;
-    }
-
-    ++IOBitMap;
-  }
-
   //
   // Load current task
   //
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchInterruptDefs.h b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchInterruptDefs.h
index 39f19f3b3b..21e469e358 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchInterruptDefs.h
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchInterruptDefs.h
@@ -41,7 +41,7 @@ typedef struct {
 //
 // 0x81 is needed to allow Ring3 code access to Uart in I/O Permission Bit Map.
 //
-#define IO_BIT_MAP_SIZE    0x81
+#define IO_BIT_MAP_SIZE    (ALIGN_VALUE (0x81, 16))
 #define CPU_TSS_SIZE       (sizeof (IA32_TASK_STATE_SEGMENT) + IO_BIT_MAP_SIZE)
 
 #endif
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
index a8c69026db..81d86fb533 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
@@ -207,20 +207,11 @@ HasErrorCode:
     mov     rax, gs
     push    rax
 
-    ; Check whether Ring3 process was interrupted.
-    and     rax, 3
-    cmp     rax, 3
-    jne     SkipHook
-    mov     rax, cr2
-    cmp     rax, 0xFFFFFFFFFFFFFFF8
-    jne     SkipHook
-    mov     rcx, 32
-SkipHook:
-    mov     rax, ss
-    mov     ds, rax
-    mov     es, rax
-    mov     fs, rax
-    mov     gs, rax
+    mov     ax, ss
+    mov     ds, ax
+    mov     es, ax
+    mov     fs, ax
+    mov     gs, ax
 
     mov     [rbp + 8], rcx               ; save vector number
 
@@ -426,7 +417,7 @@ CetDone:
 ;; UINT64  R8, R9, R10, R11, R12, R13, R14, R15;
     pop     rdi
     pop     rsi
-    add     rsp, 8               ; not for rbp
+    add     rsp, 8           ; not for rbp
     pop     qword [rbp + 48] ; for rsp
     pop     rbx
     pop     rdx
@@ -445,9 +436,8 @@ CetDone:
     push    rcx
     mov     rcx, ds
     and     rcx, 3
-    cmp     rcx, 3
     pop     rcx
-    je      ReturnToRing3
+    jnz     ReturnToRing3
 
     mov     rsp, rbp
     pop     rbp