public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3 0/7] OvmfXen: Set PcdFSBClock at runtime
@ 2021-04-12 13:29 Anthony PERARD
  2021-04-12 13:29 ` [PATCH v3 1/7] OvmfPkg/XenResetVector: Silent a warning from nasm Anthony PERARD
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Anthony PERARD @ 2021-04-12 13:29 UTC (permalink / raw)
  To: devel
  Cc: Laszlo Ersek, Jordan Justen, Ard Biesheuvel, xen-devel,
	Anthony PERARD, Julien Grall

Patch series available in this git branch:
git://xenbits.xen.org/people/aperard/ovmf.git br.apic-timer-freq-v3

Changes in v3:
- typos and codying style

Changes in v2:
- main change is to allow mapping of Xen pages outside of the RAM
  see patch: "OvmfPkg/XenPlatformPei: Map extra physical address"
- that new function allows to map the Xen shared info page (where we can find
  information about tsc frequency) at the highest physical address allowed.

Hi,

OvmfXen uses the APIC timer, but with an hard-coded frequency that may change
as pointed out here:
  https://edk2.groups.io/g/devel/message/45185
  <20190808134423.ybqg3qkpw5ucfzk4@Air-de-Roger>

This series changes that so the frequency is calculated at runtime.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490

There is also one cleanup patch that has nothing to do with the rest.

Cheers,

Anthony PERARD (7):
  OvmfPkg/XenResetVector: Silent a warning from nasm
  MdePkg: Allow PcdFSBClock to by Dynamic
  OvmfPkg/IndustryStandard/Xen: Apply EDK2 coding style to
    XEN_VCPU_TIME_INFO
  OvmfPkg/IndustryStandard: Introduce PageTable.h
  OvmfPkg/XenPlatformPei: Map extra physical address
  OvmfPkg/XenPlatformPei: Calibrate APIC timer frequency
  OvmfPkg/OvmfXen: Set PcdFSBClock

 MdePkg/MdePkg.dec                             |   8 +-
 OvmfPkg/OvmfXen.dsc                           |   4 +-
 OvmfPkg/XenPlatformPei/XenPlatformPei.inf     |   4 +
 .../IndustryStandard/PageTable.h}             | 117 +-------
 OvmfPkg/Include/IndustryStandard/Xen/xen.h    |  17 +-
 .../BaseMemEncryptSevLib/X64/VirtualMemory.h  | 143 +---------
 OvmfPkg/XenPlatformPei/Platform.h             |  10 +
 OvmfPkg/XenPlatformPei/Platform.c             |   1 +
 OvmfPkg/XenPlatformPei/Xen.c                  | 252 ++++++++++++++++++
 OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm    |   2 +-
 10 files changed, 287 insertions(+), 271 deletions(-)
 copy OvmfPkg/{Library/BaseMemEncryptSevLib/X64/VirtualMemory.h => Include/IndustryStandard/PageTable.h} (60%)

-- 
Anthony PERARD


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 1/7] OvmfPkg/XenResetVector: Silent a warning from nasm
  2021-04-12 13:29 [PATCH v3 0/7] OvmfXen: Set PcdFSBClock at runtime Anthony PERARD
@ 2021-04-12 13:29 ` Anthony PERARD
  2021-04-12 13:29 ` [PATCH v3 2/7] MdePkg: Allow PcdFSBClock to by Dynamic Anthony PERARD
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Anthony PERARD @ 2021-04-12 13:29 UTC (permalink / raw)
  To: devel
  Cc: Laszlo Ersek, Jordan Justen, Ard Biesheuvel, xen-devel,
	Anthony PERARD, Julien Grall

To avoid nasm generating a warning, replace the macro by the value
expected to be stored in eax.
  Ia32/XenPVHMain.asm:76: warning: dword data exceeds bounds

Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm b/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm
index 2df0f12e18cb..c761e9d30729 100644
--- a/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm
+++ b/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm
@@ -73,7 +73,7 @@ xenPVHMain:
     ;
     ; parameter for Flat32SearchForBfvBase
     ;
-    mov     eax, ADDR_OF(fourGigabytes)
+    mov     eax, 0   ; ADDR_OF(fourGigabytes)
     add     eax, edx ; add delta
 
     ;
-- 
Anthony PERARD


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 2/7] MdePkg: Allow PcdFSBClock to by Dynamic
  2021-04-12 13:29 [PATCH v3 0/7] OvmfXen: Set PcdFSBClock at runtime Anthony PERARD
  2021-04-12 13:29 ` [PATCH v3 1/7] OvmfPkg/XenResetVector: Silent a warning from nasm Anthony PERARD
@ 2021-04-12 13:29 ` Anthony PERARD
  2021-04-13  0:59   ` 回复: [edk2-devel] " gaoliming
  2021-04-12 13:29 ` [PATCH v3 3/7] OvmfPkg/IndustryStandard/Xen: Apply EDK2 coding style to XEN_VCPU_TIME_INFO Anthony PERARD
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Anthony PERARD @ 2021-04-12 13:29 UTC (permalink / raw)
  To: devel
  Cc: Laszlo Ersek, Jordan Justen, Ard Biesheuvel, xen-devel,
	Anthony PERARD, Julien Grall, Michael D Kinney, Liming Gao,
	Zhiguang Liu, Liming Gao

We are going to want to change the value of PcdFSBClock at run time in
OvmfXen, so move it to the PcdsDynamic section.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
---

Notes:
    CC: Michael D Kinney <michael.d.kinney@intel.com>
    CC: Liming Gao <gaoliming@byosoft.com.cn>
    CC: Zhiguang Liu <zhiguang.liu@intel.com>

 MdePkg/MdePkg.dec | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index f4156920e599..8965e903e093 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2265,10 +2265,6 @@ [PcdsFixedAtBuild,PcdsPatchableInModule]
   # @ValidList  0x80000001 | 8, 16, 32
   gEfiMdePkgTokenSpaceGuid.PcdPort80DataWidth|8|UINT8|0x0000002d
 
-  ## This value is used to configure X86 Processor FSB clock.
-  # @Prompt FSB Clock.
-  gEfiMdePkgTokenSpaceGuid.PcdFSBClock|200000000|UINT32|0x0000000c
-
   ## The maximum printable number of characters. UefLib functions: AsciiPrint(), AsciiErrorPrint(),
   #  PrintXY(), AsciiPrintXY(), Print(), ErrorPrint() base on this PCD value to print characters.
   # @Prompt Maximum Printable Number of Characters.
@@ -2372,5 +2368,9 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
   # @Prompt Boot Timeout (s)
   gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0xffff|UINT16|0x0000002c
 
+  ## This value is used to configure X86 Processor FSB clock.
+  # @Prompt FSB Clock.
+  gEfiMdePkgTokenSpaceGuid.PcdFSBClock|200000000|UINT32|0x0000000c
+
 [UserExtensions.TianoCore."ExtraFiles"]
   MdePkgExtra.uni
-- 
Anthony PERARD


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 3/7] OvmfPkg/IndustryStandard/Xen: Apply EDK2 coding style to XEN_VCPU_TIME_INFO
  2021-04-12 13:29 [PATCH v3 0/7] OvmfXen: Set PcdFSBClock at runtime Anthony PERARD
  2021-04-12 13:29 ` [PATCH v3 1/7] OvmfPkg/XenResetVector: Silent a warning from nasm Anthony PERARD
  2021-04-12 13:29 ` [PATCH v3 2/7] MdePkg: Allow PcdFSBClock to by Dynamic Anthony PERARD
@ 2021-04-12 13:29 ` Anthony PERARD
  2021-04-12 13:30 ` [PATCH v3 4/7] OvmfPkg/IndustryStandard: Introduce PageTable.h Anthony PERARD
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Anthony PERARD @ 2021-04-12 13:29 UTC (permalink / raw)
  To: devel
  Cc: Laszlo Ersek, Jordan Justen, Ard Biesheuvel, xen-devel,
	Anthony PERARD, Julien Grall

We are going to use new fields from the Xen headers. Apply the EDK2
coding style so that the code that is going to use it doesn't look out
of place.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v2:
    - fix case of Tsc* field

 OvmfPkg/Include/IndustryStandard/Xen/xen.h | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/OvmfPkg/Include/IndustryStandard/Xen/xen.h b/OvmfPkg/Include/IndustryStandard/Xen/xen.h
index e55d93263285..79a4e212e7c2 100644
--- a/OvmfPkg/Include/IndustryStandard/Xen/xen.h
+++ b/OvmfPkg/Include/IndustryStandard/Xen/xen.h
@@ -183,10 +183,10 @@ struct vcpu_time_info {
      * The correct way to interact with the version number is similar to
      * Linux's seqlock: see the implementations of read_seqbegin/read_seqretry.
      */
-    UINT32 version;
+    UINT32 Version;
     UINT32 pad0;
-    UINT64 tsc_timestamp;   /* TSC at last update of time vals.  */
-    UINT64 system_time;     /* Time, in nanosecs, since boot.    */
+    UINT64 TscTimestamp;   /* TSC at last update of time vals.  */
+    UINT64 SystemTime;     /* Time, in nanosecs, since boot.    */
     /*
      * Current system time:
      *   system_time +
@@ -194,11 +194,11 @@ struct vcpu_time_info {
      * CPU frequency (Hz):
      *   ((10^9 << 32) / tsc_to_system_mul) >> tsc_shift
      */
-    UINT32 tsc_to_system_mul;
-    INT8   tsc_shift;
+    UINT32 TscToSystemMultiplier;
+    INT8   TscShift;
     INT8   pad1[3];
 }; /* 32 bytes */
-typedef struct vcpu_time_info vcpu_time_info_t;
+typedef struct vcpu_time_info XEN_VCPU_TIME_INFO;
 
 struct vcpu_info {
     /*
@@ -234,7 +234,7 @@ struct vcpu_info {
 #endif /* XEN_HAVE_PV_UPCALL_MASK */
     xen_ulong_t evtchn_pending_sel;
     struct arch_vcpu_info arch;
-    struct vcpu_time_info time;
+    struct vcpu_time_info Time;
 }; /* 64 bytes (x86) */
 #ifndef __XEN__
 typedef struct vcpu_info vcpu_info_t;
@@ -250,7 +250,7 @@ typedef struct vcpu_info vcpu_info_t;
  * of this structure remaining constant.
  */
 struct shared_info {
-    struct vcpu_info vcpu_info[XEN_LEGACY_MAX_VCPUS];
+    struct vcpu_info VcpuInfo[XEN_LEGACY_MAX_VCPUS];
 
     /*
      * A domain can create "event channels" on which it can send and receive
@@ -299,6 +299,7 @@ struct shared_info {
 };
 #ifndef __XEN__
 typedef struct shared_info shared_info_t;
+typedef struct shared_info XEN_SHARED_INFO;
 #endif
 
 /* Turn a plain number into a C UINTN constant. */
-- 
Anthony PERARD


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 4/7] OvmfPkg/IndustryStandard: Introduce PageTable.h
  2021-04-12 13:29 [PATCH v3 0/7] OvmfXen: Set PcdFSBClock at runtime Anthony PERARD
                   ` (2 preceding siblings ...)
  2021-04-12 13:29 ` [PATCH v3 3/7] OvmfPkg/IndustryStandard/Xen: Apply EDK2 coding style to XEN_VCPU_TIME_INFO Anthony PERARD
@ 2021-04-12 13:30 ` Anthony PERARD
  2021-04-13  9:59   ` Laszlo Ersek
  2021-04-12 13:30 ` [PATCH v3 5/7] OvmfPkg/XenPlatformPei: Map extra physical address Anthony PERARD
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Anthony PERARD @ 2021-04-12 13:30 UTC (permalink / raw)
  To: devel
  Cc: Laszlo Ersek, Jordan Justen, Ard Biesheuvel, xen-devel,
	Anthony PERARD, Julien Grall, Brijesh Singh, Tom Lendacky

We are going to use the page table structure in yet another place,
collect the types and macro that can be used from another module
rather than making yet another copy.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Tom Lendacky <thomas.lendacky@amd.com>
---

Notes:
    CC: Brijesh Singh <brijesh.singh@amd.com>
    
    v3:
    - fix typos and coding style
    
    v2:
    - new patch

 .../IndustryStandard/PageTable.h}             | 117 +-------------
 .../BaseMemEncryptSevLib/X64/VirtualMemory.h  | 143 +-----------------
 2 files changed, 5 insertions(+), 255 deletions(-)
 copy OvmfPkg/{Library/BaseMemEncryptSevLib/X64/VirtualMemory.h => Include/IndustryStandard/PageTable.h} (60%)

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h b/OvmfPkg/Include/IndustryStandard/PageTable.h
similarity index 60%
copy from OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
copy to OvmfPkg/Include/IndustryStandard/PageTable.h
index 996f94f07ebb..5e797eeea8ef 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
+++ b/OvmfPkg/Include/IndustryStandard/PageTable.h
@@ -1,6 +1,6 @@
 /** @file
 
-  Virtual Memory Management Services to set or clear the memory encryption bit
+  x86_64 Page Tables structures
 
   Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
   Copyright (c) 2017 - 2020, AMD Incorporated. All rights reserved.<BR>
@@ -11,17 +11,10 @@
 
 **/
 
-#ifndef __VIRTUAL_MEMORY__
-#define __VIRTUAL_MEMORY__
+#ifndef PAGE_TABLE_H_
+#define PAGE_TABLE_H_
 
-#include <Library/BaseLib.h>
-#include <Library/BaseMemoryLib.h>
-#include <Library/CacheMaintenanceLib.h>
-#include <Library/DebugLib.h>
-#include <Library/MemoryAllocationLib.h>
-#include <Uefi.h>
-
-#define SYS_CODE64_SEL 0x38
+#include <Base.h>
 
 #pragma pack(1)
 
@@ -165,106 +158,4 @@ typedef union {
 #define PTE_OFFSET(x)               ( (x >> 12) & PAGETABLE_ENTRY_MASK)
 #define PAGING_1G_ADDRESS_MASK_64   0x000FFFFFC0000000ull
 
-#define PAGE_TABLE_POOL_ALIGNMENT   BASE_2MB
-#define PAGE_TABLE_POOL_UNIT_SIZE   SIZE_2MB
-#define PAGE_TABLE_POOL_UNIT_PAGES  \
-  EFI_SIZE_TO_PAGES (PAGE_TABLE_POOL_UNIT_SIZE)
-#define PAGE_TABLE_POOL_ALIGN_MASK  \
-  (~(EFI_PHYSICAL_ADDRESS)(PAGE_TABLE_POOL_ALIGNMENT - 1))
-
-typedef struct {
-  VOID            *NextPool;
-  UINTN           Offset;
-  UINTN           FreePages;
-} PAGE_TABLE_POOL;
-
-/**
-  Return the pagetable memory encryption mask.
-
-  @return  The pagetable memory encryption mask.
-
-**/
-UINT64
-EFIAPI
-InternalGetMemEncryptionAddressMask (
-  VOID
-  );
-
-/**
-  This function clears memory encryption bit for the memory region specified by
-  PhysicalAddress and Length from the current page table context.
-
-  @param[in]  Cr3BaseAddress          Cr3 Base Address (if zero then use
-                                      current CR3)
-  @param[in]  PhysicalAddress         The physical address that is the start
-                                      address of a memory region.
-  @param[in]  Length                  The length of memory region
-  @param[in]  Flush                   Flush the caches before applying the
-                                      encryption mask
-
-  @retval RETURN_SUCCESS              The attributes were cleared for the
-                                      memory region.
-  @retval RETURN_INVALID_PARAMETER    Number of pages is zero.
-  @retval RETURN_UNSUPPORTED          Clearing the memory encyrption attribute
-                                      is not supported
-**/
-RETURN_STATUS
-EFIAPI
-InternalMemEncryptSevSetMemoryDecrypted (
-  IN  PHYSICAL_ADDRESS        Cr3BaseAddress,
-  IN  PHYSICAL_ADDRESS        PhysicalAddress,
-  IN  UINTN                   Length,
-  IN  BOOLEAN                 Flush
-  );
-
-/**
-  This function sets memory encryption bit for the memory region specified by
-  PhysicalAddress and Length from the current page table context.
-
-  @param[in]  Cr3BaseAddress          Cr3 Base Address (if zero then use
-                                      current CR3)
-  @param[in]  PhysicalAddress         The physical address that is the start
-                                      address of a memory region.
-  @param[in]  Length                  The length of memory region
-  @param[in]  Flush                   Flush the caches before applying the
-                                      encryption mask
-
-  @retval RETURN_SUCCESS              The attributes were set for the memory
-                                      region.
-  @retval RETURN_INVALID_PARAMETER    Number of pages is zero.
-  @retval RETURN_UNSUPPORTED          Setting the memory encyrption attribute
-                                      is not supported
-**/
-RETURN_STATUS
-EFIAPI
-InternalMemEncryptSevSetMemoryEncrypted (
-  IN  PHYSICAL_ADDRESS        Cr3BaseAddress,
-  IN  PHYSICAL_ADDRESS        PhysicalAddress,
-  IN  UINTN                   Length,
-  IN  BOOLEAN                 Flush
-  );
-
-/**
-  Returns the encryption state of the specified virtual address range.
-
-  @param[in]  Cr3BaseAddress          Cr3 Base Address (if zero then use
-                                      current CR3)
-  @param[in]  BaseAddress             Base address to check
-  @param[in]  Length                  Length of virtual address range
-
-  @retval MemEncryptSevAddressRangeUnencrypted  Address range is mapped
-                                                unencrypted
-  @retval MemEncryptSevAddressRangeEncrypted    Address range is mapped
-                                                encrypted
-  @retval MemEncryptSevAddressRangeMixed        Address range is mapped mixed
-  @retval MemEncryptSevAddressRangeError        Address range is not mapped
-**/
-MEM_ENCRYPT_SEV_ADDRESS_RANGE_STATE
-EFIAPI
-InternalMemEncryptSevGetAddressRangeState (
-  IN PHYSICAL_ADDRESS         Cr3BaseAddress,
-  IN PHYSICAL_ADDRESS         BaseAddress,
-  IN UINTN                    Length
-  );
-
 #endif
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
index 996f94f07ebb..fe2a0b2826cd 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
@@ -14,6 +14,7 @@
 #ifndef __VIRTUAL_MEMORY__
 #define __VIRTUAL_MEMORY__
 
+#include <IndustryStandard/PageTable.h>
 #include <Library/BaseLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/CacheMaintenanceLib.h>
@@ -23,148 +24,6 @@
 
 #define SYS_CODE64_SEL 0x38
 
-#pragma pack(1)
-
-//
-// Page-Map Level-4 Offset (PML4) and
-// Page-Directory-Pointer Offset (PDPE) entries 4K & 2MB
-//
-
-typedef union {
-  struct {
-    UINT64  Present:1;                // 0 = Not present in memory,
-                                      //   1 = Present in memory
-    UINT64  ReadWrite:1;              // 0 = Read-Only, 1= Read/Write
-    UINT64  UserSupervisor:1;         // 0 = Supervisor, 1=User
-    UINT64  WriteThrough:1;           // 0 = Write-Back caching,
-                                      //   1 = Write-Through caching
-    UINT64  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
-    UINT64  Accessed:1;               // 0 = Not accessed,
-                                      //   1 = Accessed (set by CPU)
-    UINT64  Reserved:1;               // Reserved
-    UINT64  MustBeZero:2;             // Must Be Zero
-    UINT64  Available:3;              // Available for use by system software
-    UINT64  PageTableBaseAddress:40;  // Page Table Base Address
-    UINT64  AvabilableHigh:11;        // Available for use by system software
-    UINT64  Nx:1;                     // No Execute bit
-  } Bits;
-  UINT64    Uint64;
-} PAGE_MAP_AND_DIRECTORY_POINTER;
-
-//
-// Page Table Entry 4KB
-//
-typedef union {
-  struct {
-    UINT64  Present:1;                // 0 = Not present in memory,
-                                      //   1 = Present in memory
-    UINT64  ReadWrite:1;              // 0 = Read-Only, 1= Read/Write
-    UINT64  UserSupervisor:1;         // 0 = Supervisor, 1=User
-    UINT64  WriteThrough:1;           // 0 = Write-Back caching,
-                                      //   1 = Write-Through caching
-    UINT64  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
-    UINT64  Accessed:1;               // 0 = Not accessed,
-                                      //   1 = Accessed (set by CPU)
-    UINT64  Dirty:1;                  // 0 = Not Dirty, 1 = written by
-                                      //   processor on access to page
-    UINT64  PAT:1;                    //
-    UINT64  Global:1;                 // 0 = Not global page, 1 = global page
-                                      //   TLB not cleared on CR3 write
-    UINT64  Available:3;              // Available for use by system software
-    UINT64  PageTableBaseAddress:40;  // Page Table Base Address
-    UINT64  AvabilableHigh:11;        // Available for use by system software
-    UINT64  Nx:1;                     // 0 = Execute Code,
-                                      //   1 = No Code Execution
-  } Bits;
-  UINT64    Uint64;
-} PAGE_TABLE_4K_ENTRY;
-
-//
-// Page Table Entry 2MB
-//
-typedef union {
-  struct {
-    UINT64  Present:1;                // 0 = Not present in memory,
-                                      //   1 = Present in memory
-    UINT64  ReadWrite:1;              // 0 = Read-Only, 1= Read/Write
-    UINT64  UserSupervisor:1;         // 0 = Supervisor, 1=User
-    UINT64  WriteThrough:1;           // 0 = Write-Back caching,
-                                      //   1=Write-Through caching
-    UINT64  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
-    UINT64  Accessed:1;               // 0 = Not accessed,
-                                      //   1 = Accessed (set by CPU)
-    UINT64  Dirty:1;                  // 0 = Not Dirty, 1 = written by
-                                      //   processor on access to page
-    UINT64  MustBe1:1;                // Must be 1
-    UINT64  Global:1;                 // 0 = Not global page, 1 = global page
-                                      //   TLB not cleared on CR3 write
-    UINT64  Available:3;              // Available for use by system software
-    UINT64  PAT:1;                    //
-    UINT64  MustBeZero:8;             // Must be zero;
-    UINT64  PageTableBaseAddress:31;  // Page Table Base Address
-    UINT64  AvabilableHigh:11;        // Available for use by system software
-    UINT64  Nx:1;                     // 0 = Execute Code,
-                                      //   1 = No Code Execution
-  } Bits;
-  UINT64    Uint64;
-} PAGE_TABLE_ENTRY;
-
-//
-// Page Table Entry 1GB
-//
-typedef union {
-  struct {
-    UINT64  Present:1;                // 0 = Not present in memory,
-                                      //   1 = Present in memory
-    UINT64  ReadWrite:1;              // 0 = Read-Only, 1= Read/Write
-    UINT64  UserSupervisor:1;         // 0 = Supervisor, 1=User
-    UINT64  WriteThrough:1;           // 0 = Write-Back caching,
-                                      //   1 = Write-Through caching
-    UINT64  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
-    UINT64  Accessed:1;               // 0 = Not accessed,
-                                      //   1 = Accessed (set by CPU)
-    UINT64  Dirty:1;                  // 0 = Not Dirty, 1 = written by
-                                      //   processor on access to page
-    UINT64  MustBe1:1;                // Must be 1
-    UINT64  Global:1;                 // 0 = Not global page, 1 = global page
-                                      //   TLB not cleared on CR3 write
-    UINT64  Available:3;              // Available for use by system software
-    UINT64  PAT:1;                    //
-    UINT64  MustBeZero:17;            // Must be zero;
-    UINT64  PageTableBaseAddress:22;  // Page Table Base Address
-    UINT64  AvabilableHigh:11;        // Available for use by system software
-    UINT64  Nx:1;                     // 0 = Execute Code,
-                                      //   1 = No Code Execution
-  } Bits;
-  UINT64    Uint64;
-} PAGE_TABLE_1G_ENTRY;
-
-#pragma pack()
-
-#define IA32_PG_P                   BIT0
-#define IA32_PG_RW                  BIT1
-#define IA32_PG_PS                  BIT7
-
-#define PAGING_PAE_INDEX_MASK       0x1FF
-
-#define PAGING_4K_ADDRESS_MASK_64   0x000FFFFFFFFFF000ull
-#define PAGING_2M_ADDRESS_MASK_64   0x000FFFFFFFE00000ull
-#define PAGING_1G_ADDRESS_MASK_64   0x000FFFFFC0000000ull
-
-#define PAGING_L1_ADDRESS_SHIFT     12
-#define PAGING_L2_ADDRESS_SHIFT     21
-#define PAGING_L3_ADDRESS_SHIFT     30
-#define PAGING_L4_ADDRESS_SHIFT     39
-
-#define PAGING_PML4E_NUMBER         4
-
-#define PAGETABLE_ENTRY_MASK        ((1UL << 9) - 1)
-#define PML4_OFFSET(x)              ( (x >> 39) & PAGETABLE_ENTRY_MASK)
-#define PDP_OFFSET(x)               ( (x >> 30) & PAGETABLE_ENTRY_MASK)
-#define PDE_OFFSET(x)               ( (x >> 21) & PAGETABLE_ENTRY_MASK)
-#define PTE_OFFSET(x)               ( (x >> 12) & PAGETABLE_ENTRY_MASK)
-#define PAGING_1G_ADDRESS_MASK_64   0x000FFFFFC0000000ull
-
 #define PAGE_TABLE_POOL_ALIGNMENT   BASE_2MB
 #define PAGE_TABLE_POOL_UNIT_SIZE   SIZE_2MB
 #define PAGE_TABLE_POOL_UNIT_PAGES  \
-- 
Anthony PERARD


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 5/7] OvmfPkg/XenPlatformPei: Map extra physical address
  2021-04-12 13:29 [PATCH v3 0/7] OvmfXen: Set PcdFSBClock at runtime Anthony PERARD
                   ` (3 preceding siblings ...)
  2021-04-12 13:30 ` [PATCH v3 4/7] OvmfPkg/IndustryStandard: Introduce PageTable.h Anthony PERARD
@ 2021-04-12 13:30 ` Anthony PERARD
  2021-04-12 13:30 ` [PATCH v3 6/7] OvmfPkg/XenPlatformPei: Calibrate APIC timer frequency Anthony PERARD
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Anthony PERARD @ 2021-04-12 13:30 UTC (permalink / raw)
  To: devel
  Cc: Laszlo Ersek, Jordan Justen, Ard Biesheuvel, xen-devel,
	Anthony PERARD, Julien Grall

Some information available in a Xen guest can be mapped anywhere in
the physical address space and they don't need to be backed by RAM.
For example, the shared info page.

While it's easier to put those pages anywhere, it is better to avoid
mapping it where the RAM is. It might split a nice 1G guest page table
into 4k pages and thus reducing performance of the guest when it
accesses its memory. Also mapping a page like the shared info page and
then unmapping it or mapping it somewhere else would leave a hole in
the RAM that the guest would propably not be able to use anymore.

So the patch introduces a new function which can be used to 1:1
mapping of guest physical memory above 4G during the PEI phase so we
can map the Xen shared pages outside of memory that can be used by
guest, and as high as possible.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v3:
    - fix typos
    v2:
    - new patch

 OvmfPkg/XenPlatformPei/XenPlatformPei.inf |  1 +
 OvmfPkg/XenPlatformPei/Platform.h         |  5 ++
 OvmfPkg/XenPlatformPei/Xen.c              | 71 +++++++++++++++++++++++
 3 files changed, 77 insertions(+)

diff --git a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
index 0ef77db92c03..8790d907d3ec 100644
--- a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
+++ b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
@@ -66,6 +66,7 @@ [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
diff --git a/OvmfPkg/XenPlatformPei/Platform.h b/OvmfPkg/XenPlatformPei/Platform.h
index 7661f4a8de0a..e70ca58078eb 100644
--- a/OvmfPkg/XenPlatformPei/Platform.h
+++ b/OvmfPkg/XenPlatformPei/Platform.h
@@ -127,6 +127,11 @@ XenGetE820Map (
   UINT32 *Count
   );
 
+EFI_STATUS
+PhysicalAddressIdentityMapping (
+  IN EFI_PHYSICAL_ADDRESS AddressToMap
+  );
+
 extern EFI_BOOT_MODE mBootMode;
 
 extern UINT8 mPhysMemAddressWidth;
diff --git a/OvmfPkg/XenPlatformPei/Xen.c b/OvmfPkg/XenPlatformPei/Xen.c
index c41fecdc486e..b2a7d1c21dac 100644
--- a/OvmfPkg/XenPlatformPei/Xen.c
+++ b/OvmfPkg/XenPlatformPei/Xen.c
@@ -17,6 +17,8 @@
 //
 // The Library classes this module consumes
 //
+#include <Library/BaseMemoryLib.h>
+#include <Library/CpuLib.h>
 #include <Library/DebugLib.h>
 #include <Library/HobLib.h>
 #include <Library/MemoryAllocationLib.h>
@@ -25,6 +27,7 @@
 #include <IndustryStandard/E820.h>
 #include <Library/ResourcePublicationLib.h>
 #include <Library/MtrrLib.h>
+#include <IndustryStandard/PageTable.h>
 #include <IndustryStandard/Xen/arch-x86/hvm/start_info.h>
 #include <Library/XenHypercallLib.h>
 #include <IndustryStandard/Xen/memory.h>
@@ -386,3 +389,71 @@ InitializeXen (
 
   return EFI_SUCCESS;
 }
+
+EFI_STATUS
+PhysicalAddressIdentityMapping (
+  IN EFI_PHYSICAL_ADDRESS   AddressToMap
+  )
+{
+  INTN                            Index;
+  PAGE_MAP_AND_DIRECTORY_POINTER  *L4, *L3;
+  PAGE_TABLE_ENTRY                *PageTable;
+
+  DEBUG ((DEBUG_INFO, "Mapping 1:1 of address 0x%lx\n", (UINT64)AddressToMap));
+
+  // L4 / Top level Page Directory Pointers
+
+  L4 = (VOID*)(UINTN)PcdGet32 (PcdOvmfSecPageTablesBase);
+  Index = PML4_OFFSET (AddressToMap);
+
+  if (!L4[Index].Bits.Present) {
+    L3 = AllocatePages (1);
+    if (L3 == NULL) {
+      return EFI_OUT_OF_RESOURCES;
+    }
+
+    ZeroMem (L3, EFI_PAGE_SIZE);
+
+    L4[Index].Bits.ReadWrite = 1;
+    L4[Index].Bits.Accessed = 1;
+    L4[Index].Bits.PageTableBaseAddress = (EFI_PHYSICAL_ADDRESS)L3 >> 12;
+    L4[Index].Bits.Present = 1;
+  }
+
+  // L3 / Next level Page Directory Pointers
+
+  L3 = (VOID*)(EFI_PHYSICAL_ADDRESS)(L4[Index].Bits.PageTableBaseAddress << 12);
+  Index = PDP_OFFSET (AddressToMap);
+
+  if (!L3[Index].Bits.Present) {
+    PageTable = AllocatePages (1);
+    if (PageTable == NULL) {
+      return EFI_OUT_OF_RESOURCES;
+    }
+
+    ZeroMem (PageTable, EFI_PAGE_SIZE);
+
+    L3[Index].Bits.ReadWrite = 1;
+    L3[Index].Bits.Accessed = 1;
+    L3[Index].Bits.PageTableBaseAddress = (EFI_PHYSICAL_ADDRESS)PageTable >> 12;
+    L3[Index].Bits.Present = 1;
+  }
+
+  // L2 / Page Table Entries
+
+  PageTable = (VOID*)(EFI_PHYSICAL_ADDRESS)(L3[Index].Bits.PageTableBaseAddress << 12);
+  Index = PDE_OFFSET (AddressToMap);
+
+  if (!PageTable[Index].Bits.Present) {
+    PageTable[Index].Bits.ReadWrite = 1;
+    PageTable[Index].Bits.Accessed = 1;
+    PageTable[Index].Bits.Dirty = 1;
+    PageTable[Index].Bits.MustBe1 = 1;
+    PageTable[Index].Bits.PageTableBaseAddress = AddressToMap >> 21;
+    PageTable[Index].Bits.Present = 1;
+  }
+
+  CpuFlushTlb ();
+
+  return EFI_SUCCESS;
+}
-- 
Anthony PERARD


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 6/7] OvmfPkg/XenPlatformPei: Calibrate APIC timer frequency
  2021-04-12 13:29 [PATCH v3 0/7] OvmfXen: Set PcdFSBClock at runtime Anthony PERARD
                   ` (4 preceding siblings ...)
  2021-04-12 13:30 ` [PATCH v3 5/7] OvmfPkg/XenPlatformPei: Map extra physical address Anthony PERARD
@ 2021-04-12 13:30 ` Anthony PERARD
  2021-04-13 10:04   ` Laszlo Ersek
  2021-04-12 13:30 ` [PATCH v3 7/7] OvmfPkg/OvmfXen: Set PcdFSBClock Anthony PERARD
  2021-04-13 12:10 ` [edk2-devel] [PATCH v3 0/7] OvmfXen: Set PcdFSBClock at runtime Laszlo Ersek
  7 siblings, 1 reply; 14+ messages in thread
From: Anthony PERARD @ 2021-04-12 13:30 UTC (permalink / raw)
  To: devel
  Cc: Laszlo Ersek, Jordan Justen, Ard Biesheuvel, xen-devel,
	Anthony PERARD, Julien Grall, Roger Pau Monné

Calculate the frequency of the APIC timer that Xen provides.

Even though the frequency is currently hard-coded, it isn't part of
the public ABI that Xen provides and thus may change at any time. OVMF
needs to determine the frequency by an other mean.

Fortunately, Xen provides a way to determines the frequency of the
TSC, so we can use TSC to calibrate the frequency of the APIC timer.
That information is found in the shared_info page which we map and
unmap once done (XenBusDxe is going to map the page somewhere else).

The shared_info page is mapped at the highest physical address allowed
as it doesn't need to be in the RAM, thus there's a call to update the
page table.

The calculated frequency is only logged in this patch, it will be used
in a following patch.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    CC: Roger Pau Monné <roger.pau@citrix.com>
    
    v3:
    - fix debug format strings
    - fix coding style
    
    v2:
    - fix CamelCases
    - Use U64 multiplication and division helpers
    - don't read TscShift from the SharedInfo page again
    - change the location of the shared info page to be outside of the ram
    - check for overflow in XenDelay
    - check for overflow when calculating the calculating APIC frequency

 OvmfPkg/XenPlatformPei/XenPlatformPei.inf |   2 +
 OvmfPkg/XenPlatformPei/Platform.h         |   5 +
 OvmfPkg/XenPlatformPei/Platform.c         |   1 +
 OvmfPkg/XenPlatformPei/Xen.c              | 177 ++++++++++++++++++++++
 4 files changed, 185 insertions(+)

diff --git a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
index 8790d907d3ec..5732d2188871 100644
--- a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
+++ b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
@@ -52,6 +52,7 @@ [LibraryClasses]
   DebugLib
   HobLib
   IoLib
+  LocalApicLib
   PciLib
   ResourcePublicationLib
   PeiServicesLib
@@ -59,6 +60,7 @@ [LibraryClasses]
   MtrrLib
   MemEncryptSevLib
   PcdLib
+  SafeIntLib
   XenHypercallLib
 
 [Pcd]
diff --git a/OvmfPkg/XenPlatformPei/Platform.h b/OvmfPkg/XenPlatformPei/Platform.h
index e70ca58078eb..77d88fc159d7 100644
--- a/OvmfPkg/XenPlatformPei/Platform.h
+++ b/OvmfPkg/XenPlatformPei/Platform.h
@@ -132,6 +132,11 @@ PhysicalAddressIdentityMapping (
   IN EFI_PHYSICAL_ADDRESS AddressToMap
   );
 
+VOID
+CalibrateLapicTimer (
+  VOID
+  );
+
 extern EFI_BOOT_MODE mBootMode;
 
 extern UINT8 mPhysMemAddressWidth;
diff --git a/OvmfPkg/XenPlatformPei/Platform.c b/OvmfPkg/XenPlatformPei/Platform.c
index 717fd0ab1a45..e9511eb40c62 100644
--- a/OvmfPkg/XenPlatformPei/Platform.c
+++ b/OvmfPkg/XenPlatformPei/Platform.c
@@ -448,6 +448,7 @@ InitializeXenPlatform (
   InitializeRamRegions ();
 
   InitializeXen ();
+  CalibrateLapicTimer ();
 
   if (mBootMode != BOOT_ON_S3_RESUME) {
     ReserveEmuVariableNvStore ();
diff --git a/OvmfPkg/XenPlatformPei/Xen.c b/OvmfPkg/XenPlatformPei/Xen.c
index b2a7d1c21dac..8b06bebd7731 100644
--- a/OvmfPkg/XenPlatformPei/Xen.c
+++ b/OvmfPkg/XenPlatformPei/Xen.c
@@ -21,8 +21,10 @@
 #include <Library/CpuLib.h>
 #include <Library/DebugLib.h>
 #include <Library/HobLib.h>
+#include <Library/LocalApicLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/PcdLib.h>
+#include <Library/SafeIntLib.h>
 #include <Guid/XenInfo.h>
 #include <IndustryStandard/E820.h>
 #include <Library/ResourcePublicationLib.h>
@@ -457,3 +459,178 @@ PhysicalAddressIdentityMapping (
 
   return EFI_SUCCESS;
 }
+
+STATIC
+EFI_STATUS
+MapSharedInfoPage (
+  IN VOID *PagePtr
+  )
+{
+  xen_add_to_physmap_t  Parameters;
+  INTN                  ReturnCode;
+
+  Parameters.domid = DOMID_SELF;
+  Parameters.space = XENMAPSPACE_shared_info;
+  Parameters.idx = 0;
+  Parameters.gpfn = (UINTN)PagePtr >> EFI_PAGE_SHIFT;
+  ReturnCode = XenHypercallMemoryOp (XENMEM_add_to_physmap, &Parameters);
+  if (ReturnCode != 0) {
+    return EFI_NO_MAPPING;
+  }
+  return EFI_SUCCESS;
+}
+
+STATIC
+VOID
+UnmapXenPage (
+  IN VOID *PagePtr
+  )
+{
+  xen_remove_from_physmap_t Parameters;
+  INTN                      ReturnCode;
+
+  Parameters.domid = DOMID_SELF;
+  Parameters.gpfn = (UINTN)PagePtr >> EFI_PAGE_SHIFT;
+  ReturnCode = XenHypercallMemoryOp (XENMEM_remove_from_physmap, &Parameters);
+  ASSERT (ReturnCode == 0);
+}
+
+
+STATIC
+UINT64
+GetCpuFreq (
+  IN XEN_VCPU_TIME_INFO *VcpuTime
+  )
+{
+  UINT32 Version;
+  UINT32 TscToSystemMultiplier;
+  INT8   TscShift;
+  UINT64 CpuFreq;
+
+  do {
+    Version = VcpuTime->Version;
+    MemoryFence ();
+    TscToSystemMultiplier = VcpuTime->TscToSystemMultiplier;
+    TscShift = VcpuTime->TscShift;
+    MemoryFence ();
+  } while (((Version & 1) != 0) && (Version != VcpuTime->Version));
+
+  CpuFreq = DivU64x32 (LShiftU64 (1000000000ULL, 32), TscToSystemMultiplier);
+  if (TscShift >= 0) {
+      CpuFreq = RShiftU64 (CpuFreq, TscShift);
+  } else {
+      CpuFreq = LShiftU64 (CpuFreq, -TscShift);
+  }
+  return CpuFreq;
+}
+
+STATIC
+VOID
+XenDelay (
+  IN XEN_VCPU_TIME_INFO *VcpuTimeInfo,
+  IN UINT64             DelayNs
+  )
+{
+  UINT64        Tick;
+  UINT64        CpuFreq;
+  UINT64        Delay;
+  UINT64        DelayTick;
+  UINT64        NewTick;
+  RETURN_STATUS Status;
+
+  Tick = AsmReadTsc ();
+
+  CpuFreq = GetCpuFreq (VcpuTimeInfo);
+  Status = SafeUint64Mult (DelayNs, CpuFreq, &Delay);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR,
+      "XenDelay (%lu ns): delay too big in relation to CPU freq %lu Hz\n",
+      DelayNs, CpuFreq));
+    ASSERT_EFI_ERROR (Status);
+    CpuDeadLoop ();
+  }
+
+  DelayTick = DivU64x32 (Delay, 1000000000);
+
+  NewTick = Tick + DelayTick;
+
+  //
+  // Check for overflow
+  //
+  if (NewTick < Tick) {
+    //
+    // Overflow, wait for TSC to also overflow
+    //
+    while (AsmReadTsc () >= Tick) {
+      CpuPause ();
+    }
+  }
+
+  while (AsmReadTsc () <= NewTick) {
+    CpuPause ();
+  }
+}
+
+
+/**
+  Calculate the frequency of the Local Apic Timer
+**/
+VOID
+CalibrateLapicTimer (
+  VOID
+  )
+{
+  XEN_SHARED_INFO       *SharedInfo;
+  XEN_VCPU_TIME_INFO    *VcpuTimeInfo;
+  UINT32                TimerTick, TimerTick2, DiffTimer;
+  UINT64                TscTick, TscTick2;
+  UINT64                Freq;
+  UINT64                Dividend;
+  EFI_STATUS            Status;
+
+
+  SharedInfo = (VOID*)((1ULL << mPhysMemAddressWidth) - EFI_PAGE_SIZE);
+  Status = PhysicalAddressIdentityMapping ((EFI_PHYSICAL_ADDRESS)SharedInfo);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR,
+      "Failed to add page table entry for Xen shared info page: %r\n",
+      Status));
+    ASSERT_EFI_ERROR (Status);
+    return;
+  }
+
+  Status = MapSharedInfoPage (SharedInfo);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Failed to map Xen's shared info page: %r\n",
+      Status));
+    ASSERT_EFI_ERROR (Status);
+    return;
+  }
+
+  VcpuTimeInfo = &SharedInfo->VcpuInfo[0].Time;
+
+  InitializeApicTimer (1, MAX_UINT32, TRUE, 0);
+  DisableApicTimerInterrupt ();
+
+  TimerTick = GetApicTimerCurrentCount ();
+  TscTick = AsmReadTsc ();
+  XenDelay (VcpuTimeInfo, 1000000ULL);
+  TimerTick2 = GetApicTimerCurrentCount ();
+  TscTick2 = AsmReadTsc ();
+
+
+  DiffTimer = TimerTick - TimerTick2;
+  Status = SafeUint64Mult (GetCpuFreq (VcpuTimeInfo), DiffTimer, &Dividend);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "overflow while calculating APIC frequency\n"));
+    DEBUG ((DEBUG_ERROR, "CPU freq: %lu Hz; APIC timer tick count for 1 ms: %u\n",
+      GetCpuFreq (VcpuTimeInfo), DiffTimer));
+    ASSERT_EFI_ERROR (Status);
+    CpuDeadLoop ();
+  }
+
+  Freq = DivU64x64Remainder (Dividend, TscTick2 - TscTick, NULL);
+  DEBUG ((DEBUG_INFO, "APIC Freq % 8lu Hz\n", Freq));
+
+  UnmapXenPage (SharedInfo);
+}
-- 
Anthony PERARD


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 7/7] OvmfPkg/OvmfXen: Set PcdFSBClock
  2021-04-12 13:29 [PATCH v3 0/7] OvmfXen: Set PcdFSBClock at runtime Anthony PERARD
                   ` (5 preceding siblings ...)
  2021-04-12 13:30 ` [PATCH v3 6/7] OvmfPkg/XenPlatformPei: Calibrate APIC timer frequency Anthony PERARD
@ 2021-04-12 13:30 ` Anthony PERARD
  2021-04-13 10:09   ` Laszlo Ersek
  2021-04-13 12:10 ` [edk2-devel] [PATCH v3 0/7] OvmfXen: Set PcdFSBClock at runtime Laszlo Ersek
  7 siblings, 1 reply; 14+ messages in thread
From: Anthony PERARD @ 2021-04-12 13:30 UTC (permalink / raw)
  To: devel
  Cc: Laszlo Ersek, Jordan Justen, Ard Biesheuvel, xen-devel,
	Anthony PERARD, Julien Grall

Update gEfiMdePkgTokenSpaceGuid.PcdFSBClock so it can have the correct
value when SecPeiDxeTimerLibCpu start to use it for the APIC timer.

Currently, nothing appear to use the value in PcdFSBClock before
XenPlatformPei had a chance to set it even though TimerLib is included
in modules run before XenPlatformPei.

XenPlatformPei doesn't use any of the functions that would use that
value. No other modules in the PEI phase seems to use the TimerLib
before PcdFSBClock is set. There are currently two other modules in
the PEI phase that needs the TimerLib:
- S3Resume2Pei, but only because LocalApicLib needs it, but nothing is
  using the value from PcdFSBClock.
- CpuMpPei, but I believe it only runs after XenPlatformPei

Before the PEI phase, there's the SEC phase, and SecMain needs
TimerLib because of LocalApicLib. And it initialise the APIC timers
for the debug agent. But I don't think any of the DebugLib that
OvmfXen could use are actually using the *Delay functions in TimerLib,
and so would not use the value from PcdFSBClock which would be
uninitialised.

A simple runtime test showed that TimerLib doesn't use PcdFSBClock
value before it is set.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v3:
    - cast Freq in assert
    - fix typos
    - use correct assert to check Status
    v2:
    - keep the default value of PcdFSBClock because that is part of the syntax.

 OvmfPkg/OvmfXen.dsc                       | 4 +---
 OvmfPkg/XenPlatformPei/XenPlatformPei.inf | 1 +
 OvmfPkg/XenPlatformPei/Xen.c              | 4 ++++
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 86abe277c349..e535503e385d 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -447,9 +447,6 @@ [PcdsFixedAtBuild]
   # Point to the MdeModulePkg/Application/UiApp/UiApp.inf
   gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
 
-  ## Xen vlapic's frequence is 100 MHz
-  gEfiMdePkgTokenSpaceGuid.PcdFSBClock|100000000
-
   # We populate DXE IPL tables with 1G pages preferably on Xen
   gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable|TRUE
 
@@ -476,6 +473,7 @@ [PcdsDynamicDefault]
   gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base|0x0
   gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size|0x800000000
 
+  gEfiMdePkgTokenSpaceGuid.PcdFSBClock|100000000
   gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
 
   # Set video resolution for text setup.
diff --git a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
index 5732d2188871..87dd4b24679a 100644
--- a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
+++ b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
@@ -85,6 +85,7 @@ [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode
   gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable
   gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask
+  gEfiMdePkgTokenSpaceGuid.PcdFSBClock
   gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy
   gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress
 
diff --git a/OvmfPkg/XenPlatformPei/Xen.c b/OvmfPkg/XenPlatformPei/Xen.c
index 8b06bebd7731..2dc9f9ff8f3c 100644
--- a/OvmfPkg/XenPlatformPei/Xen.c
+++ b/OvmfPkg/XenPlatformPei/Xen.c
@@ -632,5 +632,9 @@ CalibrateLapicTimer (
   Freq = DivU64x64Remainder (Dividend, TscTick2 - TscTick, NULL);
   DEBUG ((DEBUG_INFO, "APIC Freq % 8lu Hz\n", Freq));
 
+  ASSERT ((UINT32)Freq <= MAX_UINT32);
+  Status = PcdSet32S (PcdFSBClock, Freq);
+  ASSERT_EFI_ERROR (Status);
+
   UnmapXenPage (SharedInfo);
 }
-- 
Anthony PERARD


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* 回复: [edk2-devel] [PATCH v3 2/7] MdePkg: Allow PcdFSBClock to by Dynamic
  2021-04-12 13:29 ` [PATCH v3 2/7] MdePkg: Allow PcdFSBClock to by Dynamic Anthony PERARD
@ 2021-04-13  0:59   ` gaoliming
  2021-04-13 10:18     ` Laszlo Ersek
  0 siblings, 1 reply; 14+ messages in thread
From: gaoliming @ 2021-04-13  0:59 UTC (permalink / raw)
  To: devel, anthony.perard
  Cc: 'Laszlo Ersek', 'Jordan Justen',
	'Ard Biesheuvel', xen-devel, 'Julien Grall',
	'Michael D Kinney', 'Zhiguang Liu',
	'Liming Gao'

Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Anthony
> PERARD via groups.io
> 发送时间: 2021年4月12日 21:30
> 收件人: devel@edk2.groups.io
> 抄送: Laszlo Ersek <lersek@redhat.com>; Jordan Justen
> <jordan.l.justen@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
> xen-devel@lists.xenproject.org; Anthony PERARD
> <anthony.perard@citrix.com>; Julien Grall <julien@xen.org>; Michael D
> Kinney <michael.d.kinney@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com>; Liming
> Gao <liming.gao@intel.com>
> 主题: [edk2-devel] [PATCH v3 2/7] MdePkg: Allow PcdFSBClock to by
> Dynamic
> 
> We are going to want to change the value of PcdFSBClock at run time in
> OvmfXen, so move it to the PcdsDynamic section.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Reviewed-by: Liming Gao <liming.gao@intel.com>
> ---
> 
> Notes:
>     CC: Michael D Kinney <michael.d.kinney@intel.com>
>     CC: Liming Gao <gaoliming@byosoft.com.cn>
>     CC: Zhiguang Liu <zhiguang.liu@intel.com>
> 
>  MdePkg/MdePkg.dec | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> index f4156920e599..8965e903e093 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -2265,10 +2265,6 @@ [PcdsFixedAtBuild,PcdsPatchableInModule]
>    # @ValidList  0x80000001 | 8, 16, 32
>    gEfiMdePkgTokenSpaceGuid.PcdPort80DataWidth|8|UINT8|0x0000002d
> 
> -  ## This value is used to configure X86 Processor FSB clock.
> -  # @Prompt FSB Clock.
> -
> gEfiMdePkgTokenSpaceGuid.PcdFSBClock|200000000|UINT32|0x0000000c
> -
>    ## The maximum printable number of characters. UefLib functions:
> AsciiPrint(), AsciiErrorPrint(),
>    #  PrintXY(), AsciiPrintXY(), Print(), ErrorPrint() base on this PCD
value to
> print characters.
>    # @Prompt Maximum Printable Number of Characters.
> @@ -2372,5 +2368,9 @@ [PcdsFixedAtBuild, PcdsPatchableInModule,
> PcdsDynamic, PcdsDynamicEx]
>    # @Prompt Boot Timeout (s)
> 
> gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0xffff|UINT16|0x0000
> 002c
> 
> +  ## This value is used to configure X86 Processor FSB clock.
> +  # @Prompt FSB Clock.
> +
> gEfiMdePkgTokenSpaceGuid.PcdFSBClock|200000000|UINT32|0x0000000c
> +
>  [UserExtensions.TianoCore."ExtraFiles"]
>    MdePkgExtra.uni
> --
> Anthony PERARD
> 
> 
> 
> 
> 




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 4/7] OvmfPkg/IndustryStandard: Introduce PageTable.h
  2021-04-12 13:30 ` [PATCH v3 4/7] OvmfPkg/IndustryStandard: Introduce PageTable.h Anthony PERARD
@ 2021-04-13  9:59   ` Laszlo Ersek
  0 siblings, 0 replies; 14+ messages in thread
From: Laszlo Ersek @ 2021-04-13  9:59 UTC (permalink / raw)
  To: Anthony PERARD, devel
  Cc: Jordan Justen, Ard Biesheuvel, xen-devel, Julien Grall,
	Brijesh Singh, Tom Lendacky

On 04/12/21 15:30, Anthony PERARD wrote:
> We are going to use the page table structure in yet another place,
> collect the types and macro that can be used from another module
> rather than making yet another copy.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> Acked-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> 
> Notes:
>     CC: Brijesh Singh <brijesh.singh@amd.com>
>     
>     v3:
>     - fix typos and coding style
>     
>     v2:
>     - new patch
> 
>  .../IndustryStandard/PageTable.h}             | 117 +-------------
>  .../BaseMemEncryptSevLib/X64/VirtualMemory.h  | 143 +-----------------
>  2 files changed, 5 insertions(+), 255 deletions(-)
>  copy OvmfPkg/{Library/BaseMemEncryptSevLib/X64/VirtualMemory.h => Include/IndustryStandard/PageTable.h} (60%)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

> 
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h b/OvmfPkg/Include/IndustryStandard/PageTable.h
> similarity index 60%
> copy from OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
> copy to OvmfPkg/Include/IndustryStandard/PageTable.h
> index 996f94f07ebb..5e797eeea8ef 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
> +++ b/OvmfPkg/Include/IndustryStandard/PageTable.h
> @@ -1,6 +1,6 @@
>  /** @file
>  
> -  Virtual Memory Management Services to set or clear the memory encryption bit
> +  x86_64 Page Tables structures
>  
>    Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>    Copyright (c) 2017 - 2020, AMD Incorporated. All rights reserved.<BR>
> @@ -11,17 +11,10 @@
>  
>  **/
>  
> -#ifndef __VIRTUAL_MEMORY__
> -#define __VIRTUAL_MEMORY__
> +#ifndef PAGE_TABLE_H_
> +#define PAGE_TABLE_H_
>  
> -#include <Library/BaseLib.h>
> -#include <Library/BaseMemoryLib.h>
> -#include <Library/CacheMaintenanceLib.h>
> -#include <Library/DebugLib.h>
> -#include <Library/MemoryAllocationLib.h>
> -#include <Uefi.h>
> -
> -#define SYS_CODE64_SEL 0x38
> +#include <Base.h>
>  
>  #pragma pack(1)
>  
> @@ -165,106 +158,4 @@ typedef union {
>  #define PTE_OFFSET(x)               ( (x >> 12) & PAGETABLE_ENTRY_MASK)
>  #define PAGING_1G_ADDRESS_MASK_64   0x000FFFFFC0000000ull
>  
> -#define PAGE_TABLE_POOL_ALIGNMENT   BASE_2MB
> -#define PAGE_TABLE_POOL_UNIT_SIZE   SIZE_2MB
> -#define PAGE_TABLE_POOL_UNIT_PAGES  \
> -  EFI_SIZE_TO_PAGES (PAGE_TABLE_POOL_UNIT_SIZE)
> -#define PAGE_TABLE_POOL_ALIGN_MASK  \
> -  (~(EFI_PHYSICAL_ADDRESS)(PAGE_TABLE_POOL_ALIGNMENT - 1))
> -
> -typedef struct {
> -  VOID            *NextPool;
> -  UINTN           Offset;
> -  UINTN           FreePages;
> -} PAGE_TABLE_POOL;
> -
> -/**
> -  Return the pagetable memory encryption mask.
> -
> -  @return  The pagetable memory encryption mask.
> -
> -**/
> -UINT64
> -EFIAPI
> -InternalGetMemEncryptionAddressMask (
> -  VOID
> -  );
> -
> -/**
> -  This function clears memory encryption bit for the memory region specified by
> -  PhysicalAddress and Length from the current page table context.
> -
> -  @param[in]  Cr3BaseAddress          Cr3 Base Address (if zero then use
> -                                      current CR3)
> -  @param[in]  PhysicalAddress         The physical address that is the start
> -                                      address of a memory region.
> -  @param[in]  Length                  The length of memory region
> -  @param[in]  Flush                   Flush the caches before applying the
> -                                      encryption mask
> -
> -  @retval RETURN_SUCCESS              The attributes were cleared for the
> -                                      memory region.
> -  @retval RETURN_INVALID_PARAMETER    Number of pages is zero.
> -  @retval RETURN_UNSUPPORTED          Clearing the memory encyrption attribute
> -                                      is not supported
> -**/
> -RETURN_STATUS
> -EFIAPI
> -InternalMemEncryptSevSetMemoryDecrypted (
> -  IN  PHYSICAL_ADDRESS        Cr3BaseAddress,
> -  IN  PHYSICAL_ADDRESS        PhysicalAddress,
> -  IN  UINTN                   Length,
> -  IN  BOOLEAN                 Flush
> -  );
> -
> -/**
> -  This function sets memory encryption bit for the memory region specified by
> -  PhysicalAddress and Length from the current page table context.
> -
> -  @param[in]  Cr3BaseAddress          Cr3 Base Address (if zero then use
> -                                      current CR3)
> -  @param[in]  PhysicalAddress         The physical address that is the start
> -                                      address of a memory region.
> -  @param[in]  Length                  The length of memory region
> -  @param[in]  Flush                   Flush the caches before applying the
> -                                      encryption mask
> -
> -  @retval RETURN_SUCCESS              The attributes were set for the memory
> -                                      region.
> -  @retval RETURN_INVALID_PARAMETER    Number of pages is zero.
> -  @retval RETURN_UNSUPPORTED          Setting the memory encyrption attribute
> -                                      is not supported
> -**/
> -RETURN_STATUS
> -EFIAPI
> -InternalMemEncryptSevSetMemoryEncrypted (
> -  IN  PHYSICAL_ADDRESS        Cr3BaseAddress,
> -  IN  PHYSICAL_ADDRESS        PhysicalAddress,
> -  IN  UINTN                   Length,
> -  IN  BOOLEAN                 Flush
> -  );
> -
> -/**
> -  Returns the encryption state of the specified virtual address range.
> -
> -  @param[in]  Cr3BaseAddress          Cr3 Base Address (if zero then use
> -                                      current CR3)
> -  @param[in]  BaseAddress             Base address to check
> -  @param[in]  Length                  Length of virtual address range
> -
> -  @retval MemEncryptSevAddressRangeUnencrypted  Address range is mapped
> -                                                unencrypted
> -  @retval MemEncryptSevAddressRangeEncrypted    Address range is mapped
> -                                                encrypted
> -  @retval MemEncryptSevAddressRangeMixed        Address range is mapped mixed
> -  @retval MemEncryptSevAddressRangeError        Address range is not mapped
> -**/
> -MEM_ENCRYPT_SEV_ADDRESS_RANGE_STATE
> -EFIAPI
> -InternalMemEncryptSevGetAddressRangeState (
> -  IN PHYSICAL_ADDRESS         Cr3BaseAddress,
> -  IN PHYSICAL_ADDRESS         BaseAddress,
> -  IN UINTN                    Length
> -  );
> -
>  #endif
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
> index 996f94f07ebb..fe2a0b2826cd 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
> @@ -14,6 +14,7 @@
>  #ifndef __VIRTUAL_MEMORY__
>  #define __VIRTUAL_MEMORY__
>  
> +#include <IndustryStandard/PageTable.h>
>  #include <Library/BaseLib.h>
>  #include <Library/BaseMemoryLib.h>
>  #include <Library/CacheMaintenanceLib.h>
> @@ -23,148 +24,6 @@
>  
>  #define SYS_CODE64_SEL 0x38
>  
> -#pragma pack(1)
> -
> -//
> -// Page-Map Level-4 Offset (PML4) and
> -// Page-Directory-Pointer Offset (PDPE) entries 4K & 2MB
> -//
> -
> -typedef union {
> -  struct {
> -    UINT64  Present:1;                // 0 = Not present in memory,
> -                                      //   1 = Present in memory
> -    UINT64  ReadWrite:1;              // 0 = Read-Only, 1= Read/Write
> -    UINT64  UserSupervisor:1;         // 0 = Supervisor, 1=User
> -    UINT64  WriteThrough:1;           // 0 = Write-Back caching,
> -                                      //   1 = Write-Through caching
> -    UINT64  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
> -    UINT64  Accessed:1;               // 0 = Not accessed,
> -                                      //   1 = Accessed (set by CPU)
> -    UINT64  Reserved:1;               // Reserved
> -    UINT64  MustBeZero:2;             // Must Be Zero
> -    UINT64  Available:3;              // Available for use by system software
> -    UINT64  PageTableBaseAddress:40;  // Page Table Base Address
> -    UINT64  AvabilableHigh:11;        // Available for use by system software
> -    UINT64  Nx:1;                     // No Execute bit
> -  } Bits;
> -  UINT64    Uint64;
> -} PAGE_MAP_AND_DIRECTORY_POINTER;
> -
> -//
> -// Page Table Entry 4KB
> -//
> -typedef union {
> -  struct {
> -    UINT64  Present:1;                // 0 = Not present in memory,
> -                                      //   1 = Present in memory
> -    UINT64  ReadWrite:1;              // 0 = Read-Only, 1= Read/Write
> -    UINT64  UserSupervisor:1;         // 0 = Supervisor, 1=User
> -    UINT64  WriteThrough:1;           // 0 = Write-Back caching,
> -                                      //   1 = Write-Through caching
> -    UINT64  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
> -    UINT64  Accessed:1;               // 0 = Not accessed,
> -                                      //   1 = Accessed (set by CPU)
> -    UINT64  Dirty:1;                  // 0 = Not Dirty, 1 = written by
> -                                      //   processor on access to page
> -    UINT64  PAT:1;                    //
> -    UINT64  Global:1;                 // 0 = Not global page, 1 = global page
> -                                      //   TLB not cleared on CR3 write
> -    UINT64  Available:3;              // Available for use by system software
> -    UINT64  PageTableBaseAddress:40;  // Page Table Base Address
> -    UINT64  AvabilableHigh:11;        // Available for use by system software
> -    UINT64  Nx:1;                     // 0 = Execute Code,
> -                                      //   1 = No Code Execution
> -  } Bits;
> -  UINT64    Uint64;
> -} PAGE_TABLE_4K_ENTRY;
> -
> -//
> -// Page Table Entry 2MB
> -//
> -typedef union {
> -  struct {
> -    UINT64  Present:1;                // 0 = Not present in memory,
> -                                      //   1 = Present in memory
> -    UINT64  ReadWrite:1;              // 0 = Read-Only, 1= Read/Write
> -    UINT64  UserSupervisor:1;         // 0 = Supervisor, 1=User
> -    UINT64  WriteThrough:1;           // 0 = Write-Back caching,
> -                                      //   1=Write-Through caching
> -    UINT64  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
> -    UINT64  Accessed:1;               // 0 = Not accessed,
> -                                      //   1 = Accessed (set by CPU)
> -    UINT64  Dirty:1;                  // 0 = Not Dirty, 1 = written by
> -                                      //   processor on access to page
> -    UINT64  MustBe1:1;                // Must be 1
> -    UINT64  Global:1;                 // 0 = Not global page, 1 = global page
> -                                      //   TLB not cleared on CR3 write
> -    UINT64  Available:3;              // Available for use by system software
> -    UINT64  PAT:1;                    //
> -    UINT64  MustBeZero:8;             // Must be zero;
> -    UINT64  PageTableBaseAddress:31;  // Page Table Base Address
> -    UINT64  AvabilableHigh:11;        // Available for use by system software
> -    UINT64  Nx:1;                     // 0 = Execute Code,
> -                                      //   1 = No Code Execution
> -  } Bits;
> -  UINT64    Uint64;
> -} PAGE_TABLE_ENTRY;
> -
> -//
> -// Page Table Entry 1GB
> -//
> -typedef union {
> -  struct {
> -    UINT64  Present:1;                // 0 = Not present in memory,
> -                                      //   1 = Present in memory
> -    UINT64  ReadWrite:1;              // 0 = Read-Only, 1= Read/Write
> -    UINT64  UserSupervisor:1;         // 0 = Supervisor, 1=User
> -    UINT64  WriteThrough:1;           // 0 = Write-Back caching,
> -                                      //   1 = Write-Through caching
> -    UINT64  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
> -    UINT64  Accessed:1;               // 0 = Not accessed,
> -                                      //   1 = Accessed (set by CPU)
> -    UINT64  Dirty:1;                  // 0 = Not Dirty, 1 = written by
> -                                      //   processor on access to page
> -    UINT64  MustBe1:1;                // Must be 1
> -    UINT64  Global:1;                 // 0 = Not global page, 1 = global page
> -                                      //   TLB not cleared on CR3 write
> -    UINT64  Available:3;              // Available for use by system software
> -    UINT64  PAT:1;                    //
> -    UINT64  MustBeZero:17;            // Must be zero;
> -    UINT64  PageTableBaseAddress:22;  // Page Table Base Address
> -    UINT64  AvabilableHigh:11;        // Available for use by system software
> -    UINT64  Nx:1;                     // 0 = Execute Code,
> -                                      //   1 = No Code Execution
> -  } Bits;
> -  UINT64    Uint64;
> -} PAGE_TABLE_1G_ENTRY;
> -
> -#pragma pack()
> -
> -#define IA32_PG_P                   BIT0
> -#define IA32_PG_RW                  BIT1
> -#define IA32_PG_PS                  BIT7
> -
> -#define PAGING_PAE_INDEX_MASK       0x1FF
> -
> -#define PAGING_4K_ADDRESS_MASK_64   0x000FFFFFFFFFF000ull
> -#define PAGING_2M_ADDRESS_MASK_64   0x000FFFFFFFE00000ull
> -#define PAGING_1G_ADDRESS_MASK_64   0x000FFFFFC0000000ull
> -
> -#define PAGING_L1_ADDRESS_SHIFT     12
> -#define PAGING_L2_ADDRESS_SHIFT     21
> -#define PAGING_L3_ADDRESS_SHIFT     30
> -#define PAGING_L4_ADDRESS_SHIFT     39
> -
> -#define PAGING_PML4E_NUMBER         4
> -
> -#define PAGETABLE_ENTRY_MASK        ((1UL << 9) - 1)
> -#define PML4_OFFSET(x)              ( (x >> 39) & PAGETABLE_ENTRY_MASK)
> -#define PDP_OFFSET(x)               ( (x >> 30) & PAGETABLE_ENTRY_MASK)
> -#define PDE_OFFSET(x)               ( (x >> 21) & PAGETABLE_ENTRY_MASK)
> -#define PTE_OFFSET(x)               ( (x >> 12) & PAGETABLE_ENTRY_MASK)
> -#define PAGING_1G_ADDRESS_MASK_64   0x000FFFFFC0000000ull
> -
>  #define PAGE_TABLE_POOL_ALIGNMENT   BASE_2MB
>  #define PAGE_TABLE_POOL_UNIT_SIZE   SIZE_2MB
>  #define PAGE_TABLE_POOL_UNIT_PAGES  \
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 6/7] OvmfPkg/XenPlatformPei: Calibrate APIC timer frequency
  2021-04-12 13:30 ` [PATCH v3 6/7] OvmfPkg/XenPlatformPei: Calibrate APIC timer frequency Anthony PERARD
@ 2021-04-13 10:04   ` Laszlo Ersek
  0 siblings, 0 replies; 14+ messages in thread
From: Laszlo Ersek @ 2021-04-13 10:04 UTC (permalink / raw)
  To: Anthony PERARD, devel
  Cc: Jordan Justen, Ard Biesheuvel, xen-devel, Julien Grall,
	Roger Pau Monné

On 04/12/21 15:30, Anthony PERARD wrote:
> Calculate the frequency of the APIC timer that Xen provides.
> 
> Even though the frequency is currently hard-coded, it isn't part of
> the public ABI that Xen provides and thus may change at any time. OVMF
> needs to determine the frequency by an other mean.
> 
> Fortunately, Xen provides a way to determines the frequency of the
> TSC, so we can use TSC to calibrate the frequency of the APIC timer.
> That information is found in the shared_info page which we map and
> unmap once done (XenBusDxe is going to map the page somewhere else).
> 
> The shared_info page is mapped at the highest physical address allowed
> as it doesn't need to be in the RAM, thus there's a call to update the
> page table.
> 
> The calculated frequency is only logged in this patch, it will be used
> in a following patch.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     CC: Roger Pau Monné <roger.pau@citrix.com>
>     
>     v3:
>     - fix debug format strings
>     - fix coding style

Thanks for the updates.

Laszlo

>     
>     v2:
>     - fix CamelCases
>     - Use U64 multiplication and division helpers
>     - don't read TscShift from the SharedInfo page again
>     - change the location of the shared info page to be outside of the ram
>     - check for overflow in XenDelay
>     - check for overflow when calculating the calculating APIC frequency
> 
>  OvmfPkg/XenPlatformPei/XenPlatformPei.inf |   2 +
>  OvmfPkg/XenPlatformPei/Platform.h         |   5 +
>  OvmfPkg/XenPlatformPei/Platform.c         |   1 +
>  OvmfPkg/XenPlatformPei/Xen.c              | 177 ++++++++++++++++++++++
>  4 files changed, 185 insertions(+)
> 
> diff --git a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
> index 8790d907d3ec..5732d2188871 100644
> --- a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
> +++ b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
> @@ -52,6 +52,7 @@ [LibraryClasses]
>    DebugLib
>    HobLib
>    IoLib
> +  LocalApicLib
>    PciLib
>    ResourcePublicationLib
>    PeiServicesLib
> @@ -59,6 +60,7 @@ [LibraryClasses]
>    MtrrLib
>    MemEncryptSevLib
>    PcdLib
> +  SafeIntLib
>    XenHypercallLib
>  
>  [Pcd]
> diff --git a/OvmfPkg/XenPlatformPei/Platform.h b/OvmfPkg/XenPlatformPei/Platform.h
> index e70ca58078eb..77d88fc159d7 100644
> --- a/OvmfPkg/XenPlatformPei/Platform.h
> +++ b/OvmfPkg/XenPlatformPei/Platform.h
> @@ -132,6 +132,11 @@ PhysicalAddressIdentityMapping (
>    IN EFI_PHYSICAL_ADDRESS AddressToMap
>    );
>  
> +VOID
> +CalibrateLapicTimer (
> +  VOID
> +  );
> +
>  extern EFI_BOOT_MODE mBootMode;
>  
>  extern UINT8 mPhysMemAddressWidth;
> diff --git a/OvmfPkg/XenPlatformPei/Platform.c b/OvmfPkg/XenPlatformPei/Platform.c
> index 717fd0ab1a45..e9511eb40c62 100644
> --- a/OvmfPkg/XenPlatformPei/Platform.c
> +++ b/OvmfPkg/XenPlatformPei/Platform.c
> @@ -448,6 +448,7 @@ InitializeXenPlatform (
>    InitializeRamRegions ();
>  
>    InitializeXen ();
> +  CalibrateLapicTimer ();
>  
>    if (mBootMode != BOOT_ON_S3_RESUME) {
>      ReserveEmuVariableNvStore ();
> diff --git a/OvmfPkg/XenPlatformPei/Xen.c b/OvmfPkg/XenPlatformPei/Xen.c
> index b2a7d1c21dac..8b06bebd7731 100644
> --- a/OvmfPkg/XenPlatformPei/Xen.c
> +++ b/OvmfPkg/XenPlatformPei/Xen.c
> @@ -21,8 +21,10 @@
>  #include <Library/CpuLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/HobLib.h>
> +#include <Library/LocalApicLib.h>
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/PcdLib.h>
> +#include <Library/SafeIntLib.h>
>  #include <Guid/XenInfo.h>
>  #include <IndustryStandard/E820.h>
>  #include <Library/ResourcePublicationLib.h>
> @@ -457,3 +459,178 @@ PhysicalAddressIdentityMapping (
>  
>    return EFI_SUCCESS;
>  }
> +
> +STATIC
> +EFI_STATUS
> +MapSharedInfoPage (
> +  IN VOID *PagePtr
> +  )
> +{
> +  xen_add_to_physmap_t  Parameters;
> +  INTN                  ReturnCode;
> +
> +  Parameters.domid = DOMID_SELF;
> +  Parameters.space = XENMAPSPACE_shared_info;
> +  Parameters.idx = 0;
> +  Parameters.gpfn = (UINTN)PagePtr >> EFI_PAGE_SHIFT;
> +  ReturnCode = XenHypercallMemoryOp (XENMEM_add_to_physmap, &Parameters);
> +  if (ReturnCode != 0) {
> +    return EFI_NO_MAPPING;
> +  }
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC
> +VOID
> +UnmapXenPage (
> +  IN VOID *PagePtr
> +  )
> +{
> +  xen_remove_from_physmap_t Parameters;
> +  INTN                      ReturnCode;
> +
> +  Parameters.domid = DOMID_SELF;
> +  Parameters.gpfn = (UINTN)PagePtr >> EFI_PAGE_SHIFT;
> +  ReturnCode = XenHypercallMemoryOp (XENMEM_remove_from_physmap, &Parameters);
> +  ASSERT (ReturnCode == 0);
> +}
> +
> +
> +STATIC
> +UINT64
> +GetCpuFreq (
> +  IN XEN_VCPU_TIME_INFO *VcpuTime
> +  )
> +{
> +  UINT32 Version;
> +  UINT32 TscToSystemMultiplier;
> +  INT8   TscShift;
> +  UINT64 CpuFreq;
> +
> +  do {
> +    Version = VcpuTime->Version;
> +    MemoryFence ();
> +    TscToSystemMultiplier = VcpuTime->TscToSystemMultiplier;
> +    TscShift = VcpuTime->TscShift;
> +    MemoryFence ();
> +  } while (((Version & 1) != 0) && (Version != VcpuTime->Version));
> +
> +  CpuFreq = DivU64x32 (LShiftU64 (1000000000ULL, 32), TscToSystemMultiplier);
> +  if (TscShift >= 0) {
> +      CpuFreq = RShiftU64 (CpuFreq, TscShift);
> +  } else {
> +      CpuFreq = LShiftU64 (CpuFreq, -TscShift);
> +  }
> +  return CpuFreq;
> +}
> +
> +STATIC
> +VOID
> +XenDelay (
> +  IN XEN_VCPU_TIME_INFO *VcpuTimeInfo,
> +  IN UINT64             DelayNs
> +  )
> +{
> +  UINT64        Tick;
> +  UINT64        CpuFreq;
> +  UINT64        Delay;
> +  UINT64        DelayTick;
> +  UINT64        NewTick;
> +  RETURN_STATUS Status;
> +
> +  Tick = AsmReadTsc ();
> +
> +  CpuFreq = GetCpuFreq (VcpuTimeInfo);
> +  Status = SafeUint64Mult (DelayNs, CpuFreq, &Delay);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR,
> +      "XenDelay (%lu ns): delay too big in relation to CPU freq %lu Hz\n",
> +      DelayNs, CpuFreq));
> +    ASSERT_EFI_ERROR (Status);
> +    CpuDeadLoop ();
> +  }
> +
> +  DelayTick = DivU64x32 (Delay, 1000000000);
> +
> +  NewTick = Tick + DelayTick;
> +
> +  //
> +  // Check for overflow
> +  //
> +  if (NewTick < Tick) {
> +    //
> +    // Overflow, wait for TSC to also overflow
> +    //
> +    while (AsmReadTsc () >= Tick) {
> +      CpuPause ();
> +    }
> +  }
> +
> +  while (AsmReadTsc () <= NewTick) {
> +    CpuPause ();
> +  }
> +}
> +
> +
> +/**
> +  Calculate the frequency of the Local Apic Timer
> +**/
> +VOID
> +CalibrateLapicTimer (
> +  VOID
> +  )
> +{
> +  XEN_SHARED_INFO       *SharedInfo;
> +  XEN_VCPU_TIME_INFO    *VcpuTimeInfo;
> +  UINT32                TimerTick, TimerTick2, DiffTimer;
> +  UINT64                TscTick, TscTick2;
> +  UINT64                Freq;
> +  UINT64                Dividend;
> +  EFI_STATUS            Status;
> +
> +
> +  SharedInfo = (VOID*)((1ULL << mPhysMemAddressWidth) - EFI_PAGE_SIZE);
> +  Status = PhysicalAddressIdentityMapping ((EFI_PHYSICAL_ADDRESS)SharedInfo);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR,
> +      "Failed to add page table entry for Xen shared info page: %r\n",
> +      Status));
> +    ASSERT_EFI_ERROR (Status);
> +    return;
> +  }
> +
> +  Status = MapSharedInfoPage (SharedInfo);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Failed to map Xen's shared info page: %r\n",
> +      Status));
> +    ASSERT_EFI_ERROR (Status);
> +    return;
> +  }
> +
> +  VcpuTimeInfo = &SharedInfo->VcpuInfo[0].Time;
> +
> +  InitializeApicTimer (1, MAX_UINT32, TRUE, 0);
> +  DisableApicTimerInterrupt ();
> +
> +  TimerTick = GetApicTimerCurrentCount ();
> +  TscTick = AsmReadTsc ();
> +  XenDelay (VcpuTimeInfo, 1000000ULL);
> +  TimerTick2 = GetApicTimerCurrentCount ();
> +  TscTick2 = AsmReadTsc ();
> +
> +
> +  DiffTimer = TimerTick - TimerTick2;
> +  Status = SafeUint64Mult (GetCpuFreq (VcpuTimeInfo), DiffTimer, &Dividend);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "overflow while calculating APIC frequency\n"));
> +    DEBUG ((DEBUG_ERROR, "CPU freq: %lu Hz; APIC timer tick count for 1 ms: %u\n",
> +      GetCpuFreq (VcpuTimeInfo), DiffTimer));
> +    ASSERT_EFI_ERROR (Status);
> +    CpuDeadLoop ();
> +  }
> +
> +  Freq = DivU64x64Remainder (Dividend, TscTick2 - TscTick, NULL);
> +  DEBUG ((DEBUG_INFO, "APIC Freq % 8lu Hz\n", Freq));
> +
> +  UnmapXenPage (SharedInfo);
> +}
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 7/7] OvmfPkg/OvmfXen: Set PcdFSBClock
  2021-04-12 13:30 ` [PATCH v3 7/7] OvmfPkg/OvmfXen: Set PcdFSBClock Anthony PERARD
@ 2021-04-13 10:09   ` Laszlo Ersek
  0 siblings, 0 replies; 14+ messages in thread
From: Laszlo Ersek @ 2021-04-13 10:09 UTC (permalink / raw)
  To: Anthony PERARD, devel
  Cc: Jordan Justen, Ard Biesheuvel, xen-devel, Julien Grall

On 04/12/21 15:30, Anthony PERARD wrote:
> Update gEfiMdePkgTokenSpaceGuid.PcdFSBClock so it can have the correct
> value when SecPeiDxeTimerLibCpu start to use it for the APIC timer.
> 
> Currently, nothing appear to use the value in PcdFSBClock before
> XenPlatformPei had a chance to set it even though TimerLib is included
> in modules run before XenPlatformPei.
> 
> XenPlatformPei doesn't use any of the functions that would use that
> value. No other modules in the PEI phase seems to use the TimerLib
> before PcdFSBClock is set. There are currently two other modules in
> the PEI phase that needs the TimerLib:
> - S3Resume2Pei, but only because LocalApicLib needs it, but nothing is
>   using the value from PcdFSBClock.
> - CpuMpPei, but I believe it only runs after XenPlatformPei
> 
> Before the PEI phase, there's the SEC phase, and SecMain needs
> TimerLib because of LocalApicLib. And it initialise the APIC timers
> for the debug agent. But I don't think any of the DebugLib that
> OvmfXen could use are actually using the *Delay functions in TimerLib,
> and so would not use the value from PcdFSBClock which would be
> uninitialised.
> 
> A simple runtime test showed that TimerLib doesn't use PcdFSBClock
> value before it is set.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     v3:
>     - cast Freq in assert
>     - fix typos
>     - use correct assert to check Status
>     v2:
>     - keep the default value of PcdFSBClock because that is part of the syntax.
> 
>  OvmfPkg/OvmfXen.dsc                       | 4 +---
>  OvmfPkg/XenPlatformPei/XenPlatformPei.inf | 1 +
>  OvmfPkg/XenPlatformPei/Xen.c              | 4 ++++
>  3 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
> index 86abe277c349..e535503e385d 100644
> --- a/OvmfPkg/OvmfXen.dsc
> +++ b/OvmfPkg/OvmfXen.dsc
> @@ -447,9 +447,6 @@ [PcdsFixedAtBuild]
>    # Point to the MdeModulePkg/Application/UiApp/UiApp.inf
>    gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
>  
> -  ## Xen vlapic's frequence is 100 MHz
> -  gEfiMdePkgTokenSpaceGuid.PcdFSBClock|100000000
> -
>    # We populate DXE IPL tables with 1G pages preferably on Xen
>    gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable|TRUE
>  
> @@ -476,6 +473,7 @@ [PcdsDynamicDefault]
>    gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base|0x0
>    gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size|0x800000000
>  
> +  gEfiMdePkgTokenSpaceGuid.PcdFSBClock|100000000
>    gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
>  
>    # Set video resolution for text setup.
> diff --git a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
> index 5732d2188871..87dd4b24679a 100644
> --- a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
> +++ b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
> @@ -85,6 +85,7 @@ [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode
>    gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable
>    gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask
> +  gEfiMdePkgTokenSpaceGuid.PcdFSBClock
>    gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress
>  
> diff --git a/OvmfPkg/XenPlatformPei/Xen.c b/OvmfPkg/XenPlatformPei/Xen.c
> index 8b06bebd7731..2dc9f9ff8f3c 100644
> --- a/OvmfPkg/XenPlatformPei/Xen.c
> +++ b/OvmfPkg/XenPlatformPei/Xen.c
> @@ -632,5 +632,9 @@ CalibrateLapicTimer (
>    Freq = DivU64x64Remainder (Dividend, TscTick2 - TscTick, NULL);
>    DEBUG ((DEBUG_INFO, "APIC Freq % 8lu Hz\n", Freq));
>  
> +  ASSERT ((UINT32)Freq <= MAX_UINT32);
> +  Status = PcdSet32S (PcdFSBClock, Freq);

You misunderstood my request for the explicit cast. I meant that for the
"Freq" argument of the PcdSet32S() function call. Adding it inside the
ASSERT() makes the assertion a tautology, hence, useless. Whereas,
casting Freq to UINT32 *after* the ASSERT() is (a) safe, (b) shuts up a
potential "truncation warning" from visual studio (i.e. when the cast
happens implicitly, due to passing the UINT64 Freq as the 2nd, UINT32,
parameter of PcdSet32S()).

But, I'll fix this up for you when I merge v3; no need to send v4.

Thanks
Laszlo

> +  ASSERT_EFI_ERROR (Status);
> +
>    UnmapXenPage (SharedInfo);
>  }
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 回复: [edk2-devel] [PATCH v3 2/7] MdePkg: Allow PcdFSBClock to by Dynamic
  2021-04-13  0:59   ` 回复: [edk2-devel] " gaoliming
@ 2021-04-13 10:18     ` Laszlo Ersek
  0 siblings, 0 replies; 14+ messages in thread
From: Laszlo Ersek @ 2021-04-13 10:18 UTC (permalink / raw)
  To: gaoliming, devel, anthony.perard
  Cc: 'Jordan Justen', 'Ard Biesheuvel', xen-devel,
	'Julien Grall', 'Michael D Kinney',
	'Zhiguang Liu', 'Liming Gao'

On 04/13/21 02:59, gaoliming wrote:
> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

Thanks, I've updated the email address in your existent R-b on the patch.

Laszlo

> 
>> -----邮件原件-----
>> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Anthony
>> PERARD via groups.io
>> 发送时间: 2021年4月12日 21:30
>> 收件人: devel@edk2.groups.io
>> 抄送: Laszlo Ersek <lersek@redhat.com>; Jordan Justen
>> <jordan.l.justen@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
>> xen-devel@lists.xenproject.org; Anthony PERARD
>> <anthony.perard@citrix.com>; Julien Grall <julien@xen.org>; Michael D
>> Kinney <michael.d.kinney@intel.com>; Liming Gao
>> <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com>; Liming
>> Gao <liming.gao@intel.com>
>> 主题: [edk2-devel] [PATCH v3 2/7] MdePkg: Allow PcdFSBClock to by
>> Dynamic
>>
>> We are going to want to change the value of PcdFSBClock at run time in
>> OvmfXen, so move it to the PcdsDynamic section.
>>
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490
>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>> Reviewed-by: Liming Gao <liming.gao@intel.com>
>> ---
>>
>> Notes:
>>     CC: Michael D Kinney <michael.d.kinney@intel.com>
>>     CC: Liming Gao <gaoliming@byosoft.com.cn>
>>     CC: Zhiguang Liu <zhiguang.liu@intel.com>
>>
>>  MdePkg/MdePkg.dec | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
>> index f4156920e599..8965e903e093 100644
>> --- a/MdePkg/MdePkg.dec
>> +++ b/MdePkg/MdePkg.dec
>> @@ -2265,10 +2265,6 @@ [PcdsFixedAtBuild,PcdsPatchableInModule]
>>    # @ValidList  0x80000001 | 8, 16, 32
>>    gEfiMdePkgTokenSpaceGuid.PcdPort80DataWidth|8|UINT8|0x0000002d
>>
>> -  ## This value is used to configure X86 Processor FSB clock.
>> -  # @Prompt FSB Clock.
>> -
>> gEfiMdePkgTokenSpaceGuid.PcdFSBClock|200000000|UINT32|0x0000000c
>> -
>>    ## The maximum printable number of characters. UefLib functions:
>> AsciiPrint(), AsciiErrorPrint(),
>>    #  PrintXY(), AsciiPrintXY(), Print(), ErrorPrint() base on this PCD
> value to
>> print characters.
>>    # @Prompt Maximum Printable Number of Characters.
>> @@ -2372,5 +2368,9 @@ [PcdsFixedAtBuild, PcdsPatchableInModule,
>> PcdsDynamic, PcdsDynamicEx]
>>    # @Prompt Boot Timeout (s)
>>
>> gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0xffff|UINT16|0x0000
>> 002c
>>
>> +  ## This value is used to configure X86 Processor FSB clock.
>> +  # @Prompt FSB Clock.
>> +
>> gEfiMdePkgTokenSpaceGuid.PcdFSBClock|200000000|UINT32|0x0000000c
>> +
>>  [UserExtensions.TianoCore."ExtraFiles"]
>>    MdePkgExtra.uni
>> --
>> Anthony PERARD
>>
>>
>>
>> 
>>
> 
> 
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v3 0/7] OvmfXen: Set PcdFSBClock at runtime
  2021-04-12 13:29 [PATCH v3 0/7] OvmfXen: Set PcdFSBClock at runtime Anthony PERARD
                   ` (6 preceding siblings ...)
  2021-04-12 13:30 ` [PATCH v3 7/7] OvmfPkg/OvmfXen: Set PcdFSBClock Anthony PERARD
@ 2021-04-13 12:10 ` Laszlo Ersek
  7 siblings, 0 replies; 14+ messages in thread
From: Laszlo Ersek @ 2021-04-13 12:10 UTC (permalink / raw)
  To: devel, anthony.perard
  Cc: Jordan Justen, Ard Biesheuvel, xen-devel, Julien Grall

On 04/12/21 15:29, Anthony PERARD via groups.io wrote:
> Patch series available in this git branch:
> git://xenbits.xen.org/people/aperard/ovmf.git br.apic-timer-freq-v3
> 
> Changes in v3:
> - typos and codying style

Merged in commit range 16136f218d54..71cdb91f3133, via <https://github.com/tianocore/edk2/pull/1560>, with the following updates:

1:  cc0477d6a5d6 = 1:  29280c7084ee OvmfPkg/XenResetVector: Silent a warning from nasm
2:  4459ca235222 ! 2:  44ad51d6b19a MdePkg: Allow PcdFSBClock to by Dynamic
    @@ -8,8 +8,8 @@
         Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490
         Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
         Reviewed-by: Laszlo Ersek <lersek@redhat.com>
    -    Reviewed-by: Liming Gao <liming.gao@intel.com>
         Message-Id: <20210412133003.146438-3-anthony.perard@citrix.com>
    +    Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
     
     diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
     --- a/MdePkg/MdePkg.dec
3:  d2b81d656c53 = 3:  896e68984118 OvmfPkg/IndustryStandard/Xen: Apply EDK2 coding style to XEN_VCPU_TIME_INFO
4:  e2bbc17dc92b ! 4:  9d6861494aaa OvmfPkg/IndustryStandard: Introduce PageTable.h
    @@ -10,6 +10,7 @@
         Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
         Acked-by: Tom Lendacky <thomas.lendacky@amd.com>
         Message-Id: <20210412133003.146438-5-anthony.perard@citrix.com>
    +    Reviewed-by: Laszlo Ersek <lersek@redhat.com>
     
     diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h b/OvmfPkg/Include/IndustryStandard/PageTable.h
     similarity index 60%
5:  158053f2ea02 = 5:  51e0bd28bba9 OvmfPkg/XenPlatformPei: Map extra physical address
6:  a37eba1a4ef4 = 6:  c75c6405128e OvmfPkg/XenPlatformPei: Calibrate APIC timer frequency
7:  7dfd174c2dc6 ! 7:  71cdb91f3133 OvmfPkg/OvmfXen: Set PcdFSBClock
    @@ -31,6 +31,7 @@
         Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
         Reviewed-by: Laszlo Ersek <lersek@redhat.com>
         Message-Id: <20210412133003.146438-8-anthony.perard@citrix.com>
    +    [lersek@redhat.com: cast Freq to UINT32 for PcdSet32S(), not for ASSERT()]
     
     diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
     --- a/OvmfPkg/OvmfXen.dsc
    @@ -73,8 +74,8 @@
        Freq = DivU64x64Remainder (Dividend, TscTick2 - TscTick, NULL);
        DEBUG ((DEBUG_INFO, "APIC Freq % 8lu Hz\n", Freq));
      
    -+  ASSERT ((UINT32)Freq <= MAX_UINT32);
    -+  Status = PcdSet32S (PcdFSBClock, Freq);
    ++  ASSERT (Freq <= MAX_UINT32);
    ++  Status = PcdSet32S (PcdFSBClock, (UINT32)Freq);
     +  ASSERT_EFI_ERROR (Status);
     +
        UnmapXenPage (SharedInfo);

Thanks
Laszlo

> 
> Changes in v2:
> - main change is to allow mapping of Xen pages outside of the RAM
>   see patch: "OvmfPkg/XenPlatformPei: Map extra physical address"
> - that new function allows to map the Xen shared info page (where we can find
>   information about tsc frequency) at the highest physical address allowed.
> 
> Hi,
> 
> OvmfXen uses the APIC timer, but with an hard-coded frequency that may change
> as pointed out here:
>   https://edk2.groups.io/g/devel/message/45185
>   <20190808134423.ybqg3qkpw5ucfzk4@Air-de-Roger>
> 
> This series changes that so the frequency is calculated at runtime.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490
> 
> There is also one cleanup patch that has nothing to do with the rest.
> 
> Cheers,
> 
> Anthony PERARD (7):
>   OvmfPkg/XenResetVector: Silent a warning from nasm
>   MdePkg: Allow PcdFSBClock to by Dynamic
>   OvmfPkg/IndustryStandard/Xen: Apply EDK2 coding style to
>     XEN_VCPU_TIME_INFO
>   OvmfPkg/IndustryStandard: Introduce PageTable.h
>   OvmfPkg/XenPlatformPei: Map extra physical address
>   OvmfPkg/XenPlatformPei: Calibrate APIC timer frequency
>   OvmfPkg/OvmfXen: Set PcdFSBClock
> 
>  MdePkg/MdePkg.dec                             |   8 +-
>  OvmfPkg/OvmfXen.dsc                           |   4 +-
>  OvmfPkg/XenPlatformPei/XenPlatformPei.inf     |   4 +
>  .../IndustryStandard/PageTable.h}             | 117 +-------
>  OvmfPkg/Include/IndustryStandard/Xen/xen.h    |  17 +-
>  .../BaseMemEncryptSevLib/X64/VirtualMemory.h  | 143 +---------
>  OvmfPkg/XenPlatformPei/Platform.h             |  10 +
>  OvmfPkg/XenPlatformPei/Platform.c             |   1 +
>  OvmfPkg/XenPlatformPei/Xen.c                  | 252 ++++++++++++++++++
>  OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm    |   2 +-
>  10 files changed, 287 insertions(+), 271 deletions(-)
>  copy OvmfPkg/{Library/BaseMemEncryptSevLib/X64/VirtualMemory.h => Include/IndustryStandard/PageTable.h} (60%)
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-04-13 12:10 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-04-12 13:29 [PATCH v3 0/7] OvmfXen: Set PcdFSBClock at runtime Anthony PERARD
2021-04-12 13:29 ` [PATCH v3 1/7] OvmfPkg/XenResetVector: Silent a warning from nasm Anthony PERARD
2021-04-12 13:29 ` [PATCH v3 2/7] MdePkg: Allow PcdFSBClock to by Dynamic Anthony PERARD
2021-04-13  0:59   ` 回复: [edk2-devel] " gaoliming
2021-04-13 10:18     ` Laszlo Ersek
2021-04-12 13:29 ` [PATCH v3 3/7] OvmfPkg/IndustryStandard/Xen: Apply EDK2 coding style to XEN_VCPU_TIME_INFO Anthony PERARD
2021-04-12 13:30 ` [PATCH v3 4/7] OvmfPkg/IndustryStandard: Introduce PageTable.h Anthony PERARD
2021-04-13  9:59   ` Laszlo Ersek
2021-04-12 13:30 ` [PATCH v3 5/7] OvmfPkg/XenPlatformPei: Map extra physical address Anthony PERARD
2021-04-12 13:30 ` [PATCH v3 6/7] OvmfPkg/XenPlatformPei: Calibrate APIC timer frequency Anthony PERARD
2021-04-13 10:04   ` Laszlo Ersek
2021-04-12 13:30 ` [PATCH v3 7/7] OvmfPkg/OvmfXen: Set PcdFSBClock Anthony PERARD
2021-04-13 10:09   ` Laszlo Ersek
2021-04-13 12:10 ` [edk2-devel] [PATCH v3 0/7] OvmfXen: Set PcdFSBClock at runtime Laszlo Ersek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox