* [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