|
Re: 回复: [edk2-devel] [edk2-devel202105 PATCH v2 1/1] ArmPkg/ArmGic: Fix maximum number of interrupts in GICv3
Hi All,
I have pushed this change to edk2 master at
cfa6ffb113f2..e1999b264f1f
Regards,
Sami Mujawar
toggle quoted messageShow quoted text
On 27/05/2021 10:19 AM, Sami Mujawar
via groups.io wrote:
Hi
Laszlo, Liming,
Apologies
for not doing it earlier. I was not sure if it was within my
right to merge the change.
I
will merge this in the next 2 hours.
Regards,
Sami
Mujawar
Hi Liming,
On 05/27/21 04:32, gaoliming wrote:
> If no objection, I will merge this patch today. Then,
tomorrow, I will create stable tag 202105.
yes, please do that -- TBH, I thought Sami would merge it
sooner, as
Sami does have maintainer access through DynamicTablesPkg
and
StandaloneMmPkg.
Thanks!
Laszlo
>
> Thanks
> Liming
>> -----邮件原件-----
>> 发件人:
devel@edk2.groups.io <devel@edk2.groups.io>
代表
gaoliming
>> 发送时间:
2021年5月26日 10:22
>> 收件人:
devel@edk2.groups.io; lersek@...;
>> sami.mujawar@...
>> 抄送:
ardb@...; leif@...; Matteo.Carlini@...;
>> Andreas.Sandberg@...; joey.gouly@...;
nd@...
>> 主题:
回复:
[edk2-devel] [edk2-devel202105 PATCH v2 1/1] ArmPkg/ArmGic:
>> Fix maximum number of interrupts in GICv3
>>
>> Laszlo, Ard, Sami:
>> I am OK to merge this patch for stable tag
202105.
>>
>> Thanks
>> Liming
>>> -----邮件原件-----
>>> 发件人:
devel@edk2.groups.io <devel@edk2.groups.io>
代表
Laszlo
>> Ersek
>>> 发送时间:
2021年5月25日 19:55
>>> 收件人: devel@edk2.groups.io;
sami.mujawar@...
>>> 抄送: ardb@...;
leif@...; Matteo.Carlini@...;
>>> Andreas.Sandberg@...; joey.gouly@...;
nd@...
>>> 主题:
Re: [edk2-devel] [edk2-devel202105 PATCH v2 1/1]
ArmPkg/ArmGic:
>>> Fix maximum number of interrupts in GICv3
>>>
>>> Hi Sami,
>>>
>>> On 05/24/21 15:01, Sami Mujawar wrote:
>>>> From: Andreas Sandberg
<andreas.sandberg@...>
>>>>
>>>> Bugzilla: 3415 (https://bugzilla.tianocore.org/show_bug.cgi?id=3415)
>>>>
>>>> The GICv3 architecture supports up to 1020
ordinary interrupt
>>>> lines. The actual number of interrupts
supported is described by the
>>>> ITLinesNumber field in the GICD_TYPER
register. The total number of
>>>> implemented registers is normally
calculated as
>>>> 32*(ITLinesNumber+1). However, maximum
value (0x1f) is a special case
>>>> since that would indicate that 1024
interrupts are implemented.
>>>>
>>>> Add handling for this special case in
ArmGicGetMaxNumInterrupts.
>>>>
>>>> Signed-off-by: Andreas Sandberg
<andreas.sandberg@...>
>>>> Signed-off-by: Joey Gouly
<joey.gouly@...>
>>>> Signed-off-by: Sami Mujawar
<sami.mujawar@...>
>>>> Reviewed-by: Ard Biesheuvel
<ardb@...>
>>>> ---
>>>> The changes can be seen at:
>>>>
https://github.com/samimujawar/edk2/tree/1396_gic_max_num_intr_v2
>>>>
>>>> Notes:
>>>> v2:
>>>> - Fix comment style.
>>> [Laszlo]
>>>> - Updated comment style.
>>> [Sami]
>>>>
>>>> ArmPkg/Drivers/ArmGic/ArmGicLib.c | 11
+++++++++--
>>>> 1 file changed, 9 insertions(+), 2
deletions(-)
>>>
>>> I think this patch should be merged really
soon, as long as Ard agrees.
>>>
>>> Thanks,
>>> Laszlo
>>>
>>>>
>>>> diff --git
a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
>>> b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
>>>> index
>>>
>>
6b01c88206ad8adef3100dd44c0d57660db77783..bd4b5edb903f3846f4f0e43
>>> 1f93e001f01cd9e7d 100644
>>>> --- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
>>>> +++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
>>>> @@ -1,6 +1,6 @@
>>>> /** @file
>>>> *
>>>> -* Copyright (c) 2011-2018, ARM Limited.
All rights reserved.
>>>> +* Copyright (c) 2011-2021, Arm Limited.
All rights reserved.
>>>> *
>>>> * SPDX-License-Identifier:
BSD-2-Clause-Patent
>>>> *
>>>> @@ -120,7 +120,14 @@
ArmGicGetMaxNumInterrupts (
>>>> IN INTN GicDistributorBase
>>>> )
>>>> {
>>>> - return 32 * ((MmioRead32
(GicDistributorBase + ARM_GIC_ICDICTR) &
>>> 0x1F) + 1);
>>>> + UINTN ItLines;
>>>> +
>>>> + ItLines = MmioRead32 (GicDistributorBase
+ ARM_GIC_ICDICTR) &
>>> 0x1F;
>>>> +
>>>> + //
>>>> + // Interrupt ID 1020-1023 are reserved.
>>>> + //
>>>> + return (ItLines == 0x1f) ? 1020 : 32 *
(ItLines + 1);
>>>> }
>>>>
>>>> VOID
>>>>
>>>
>>>
>>>
>>>
>>>
>>
>>
>>
>>
>>
>>
>>
>
>
>
|
|
|
Re: [PATCH v1] OvmfPkg: Add build options for 8MB and 16MB X64 OVMF images
On 05/26/21 19:08, Devon Bautista wrote: Currently, the largest volume size for building OVMF images is 4MB. With
the growth of the Linuxboot project, maintainers have had to maintain a
fork containing this patch which allows larger image sizes in order for
Linuxboot developers/users to have enough space to experiment with
and test including their own Linux kernel in the DXE section of OVMF
firmware. Testing using OVMF is valuable since it allows testing in QEMU
and thus does not require any hardware to do so.
This patch allows specifying '-D FD_SIZE_8MB' or '-D FD_SIZE_16MB' to
the OVMF build script in order to add the ability to build 8MB or 16MB
x86_64 (X64) OVMF images, respectively.
Signed-off-by: Devon Bautista <dbautista@...>
---
OvmfPkg/OvmfPkgDefines.fdf.inc | 34 ++++++++++++++++++++++++++++++++++
OvmfPkg/OvmfPkgX64.dsc | 10 +++++++++-
OvmfPkg/VarStore.fdf.inc | 16 ++++++++--------
3 files changed, 51 insertions(+), 9 deletions(-)
diff --git a/OvmfPkg/OvmfPkgDefines.fdf.inc b/OvmfPkg/OvmfPkgDefines.fdf.inc
index 35fd454b97..da37758934 100644
--- a/OvmfPkg/OvmfPkgDefines.fdf.inc
+++ b/OvmfPkg/OvmfPkgDefines.fdf.inc
@@ -66,6 +66,40 @@ DEFINE SECFV_OFFSET = 0x003CC000
DEFINE SECFV_SIZE = 0x34000
!endif
+!if $(FD_SIZE_IN_KB) == 8192
+DEFINE VARS_SIZE = 0x84000
+DEFINE VARS_BLOCKS = 0x84
+DEFINE VARS_LIVE_SIZE = 0x40000
+DEFINE VARS_SPARE_SIZE = 0x42000
+
+DEFINE FW_BASE_ADDRESS = 0xFF800000
+DEFINE FW_SIZE = 0x00800000
+DEFINE FW_BLOCKS = 0x800
+DEFINE CODE_BASE_ADDRESS = 0xFF884000
+DEFINE CODE_SIZE = 0x0077C000
+DEFINE CODE_BLOCKS = 0x77C
+DEFINE FVMAIN_SIZE = 0x00748000
+DEFINE SECFV_OFFSET = 0x007CC000
+DEFINE SECFV_SIZE = 0x34000
+!endif
+
+!if $(FD_SIZE_IN_KB) == 16384
+DEFINE VARS_SIZE = 0x84000
+DEFINE VARS_BLOCKS = 0x84
+DEFINE VARS_LIVE_SIZE = 0x40000
+DEFINE VARS_SPARE_SIZE = 0x42000
+
+DEFINE FW_BASE_ADDRESS = 0xFF000000
+DEFINE FW_SIZE = 0x01000000
+DEFINE FW_BLOCKS = 0x1000
+DEFINE CODE_BASE_ADDRESS = 0xFF084000
+DEFINE CODE_SIZE = 0x00F7C000
+DEFINE CODE_BLOCKS = 0xF7C
+DEFINE FVMAIN_SIZE = 0x00F48000
+DEFINE SECFV_OFFSET = 0x00FCC000
+DEFINE SECFV_SIZE = 0x34000
+!endif
+
SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress = $(FW_BASE_ADDRESS)
SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize = $(FW_SIZE)
SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareBlockSize = $(BLOCK_SIZE)
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 999738dc39..28351e2f56 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -66,11 +66,19 @@
!else
!ifdef $(FD_SIZE_4MB)
DEFINE FD_SIZE_IN_KB = 4096
+!else
+!ifdef $(FD_SIZE_8MB)
+ DEFINE FD_SIZE_IN_KB = 8192
+!else
+!ifdef $(FD_SIZE_16MB)
+ DEFINE FD_SIZE_IN_KB = 16384
!else
DEFINE FD_SIZE_IN_KB = 4096
!endif
!endif
!endif
+!endif
+!endif
[BuildOptions]
GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG
@@ -501,7 +509,7 @@
gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
!endif
!endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if $(FD_SIZE_IN_KB) == 4096 || $(FD_SIZE_IN_KB) == 8196 || $(FD_SIZE_IN_KB) == 16384
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
!if $(NETWORK_TLS_ENABLE) == FALSE
diff --git a/OvmfPkg/VarStore.fdf.inc b/OvmfPkg/VarStore.fdf.inc
index a1e524e393..70db929478 100644
--- a/OvmfPkg/VarStore.fdf.inc
+++ b/OvmfPkg/VarStore.fdf.inc
@@ -11,7 +11,7 @@
!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
0x00000000|0x0000e000
!endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 8192) || ($(FD_SIZE_IN_KB) == 16384)
0x00000000|0x00040000
!endif
#NV_VARIABLE_STORE
@@ -29,7 +29,7 @@ DATA = {
# FvLength: 0x20000
0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00,
!endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 8192) || ($(FD_SIZE_IN_KB) == 16384)
# FvLength: 0x84000
0x00, 0x40, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00,
!endif
@@ -41,7 +41,7 @@ DATA = {
# CheckSum
0x19, 0xF9,
!endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 8192) || ($(FD_SIZE_IN_KB) == 16384)
# CheckSum
0xAF, 0xB8,
!endif
@@ -51,7 +51,7 @@ DATA = {
# Blockmap[0]: 0x20 Blocks * 0x1000 Bytes / Block
0x20, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00,
!endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 8192) || ($(FD_SIZE_IN_KB) == 16384)
# Blockmap[0]: 0x84 Blocks * 0x1000 Bytes / Block
0x84, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00,
!endif
@@ -70,7 +70,7 @@ DATA = {
# This can speed up the Variable Dispatch a bit.
0xB8, 0xDF, 0x00, 0x00,
!endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 8192) || ($(FD_SIZE_IN_KB) == 16384)
# Size: 0x40000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) -
# 0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0x3ffb8
# This can speed up the Variable Dispatch a bit.
@@ -83,7 +83,7 @@ DATA = {
!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
0x0000e000|0x00001000
!endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 8192) || ($(FD_SIZE_IN_KB) == 16384)
0x00040000|0x00001000
!endif
#NV_EVENT_LOG
@@ -91,7 +91,7 @@ DATA = {
!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
0x0000f000|0x00001000
!endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 8192) || ($(FD_SIZE_IN_KB) == 16384)
0x00041000|0x00001000
!endif
#NV_FTW_WORKING
@@ -109,7 +109,7 @@ DATA = {
!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
0x00010000|0x00010000
!endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 8192) || ($(FD_SIZE_IN_KB) == 16384)
0x00042000|0x00042000
!endif
#NV_FTW_SPARE
I'm providing minimal feedback here just to get this review off my plate as quickly as possible. Sorry, I'm collapsing under my TODO list. (1) Every such change is compatibility breaking, so we *must* use the opportunity at once to *significantly increase* the non-volatile variable store size as well. We need to discuss this question with OS vendors and hardware platform vendors on this list, to see what physical flash sizes are expected in the future, and we must add a good safety margin on top of that. The primary concern is with the dbx variable growing without bounds over time. Once we introduce a new FD_SIZE_IN_KB option, we're stuck with its varstore layout forever, so we'd better get it right and future-proof at once. (2) [FD.MEMFD] should immediately benefit from this change, even if your downstream populates FVMAIN_COMPACT with something else than PEIFV and DXEFV. First, we're almost out of (uncompressed) DXEFV space again. Second, especially the confidential computing technologies have been gobbling up the nice, low, free space in FD.MEMFD the way a kid with a sweet tooth empties a cookie jar. This change is already compat breaking, so I'd like to see *some* proposal (separate patches) for enlarging *and pushing up* PEIFV and DXEFV. (3) Unfortunately, I have to agree that introducing *both* a 8MB option *and* a 16MB option is justified, per QEMU commit 0657c657eb37 ("hw/i386/pc: add max combined fw size as machine configuration option", 2020-12-09). However, please add each option in a separate patch. (4) Dumping a bunch of magic numbers on reviewers is unhelpful. I'll need to sit down with a calculator and go through the patch with a magnifying glass. Please support that work by creating a commit message (summary table) similar to the one in commit b24fca05751f ("OvmfPkg: introduce 4MB flash image (mainly) for Windows HCK", 2017-05-05). (5) Modifying *only* "OvmfPkg/OvmfPkgX64.dsc" doesn't seem right, there are other DSC files (platforms) in OvmfPkg that would benefit. Without much thinking for now, I'd say the new options should be available in each DSC (platform description), even the 32-bit ones. I'm extremely annoyed by the general trend that the firmware (the OS under the OS) keeps growing. Because of that, Linuxboot is a fantastic project. I'd like OVMF to support the development of Linuxboot. I welcome this patch for that reason. But I'd also like OVMF to benefit from this change even when it is built with a traditional -- and regrettably, ever-growing -- DXE phase. I welcome this patch for that reason too. Thank you, Laszlo
|
|
|
Re: [RESEND PATCH RFC v3 00/22] Add AMD Secure Nested Paging (SEV-SNP) support
On 05/27/21 01:10, Brijesh Singh wrote: (I missed adding devel@edk2.groups.io, resending the series)
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
SEV-SNP builds upon existing SEV and SEV-ES functionality while adding
new hardware-based memory protections. SEV-SNP adds strong memory integrity
protection to help prevent malicious hypervisor-based attacks like data
replay, memory re-mapping and more in order to create an isolated memory
encryption environment.
This series provides the basic building blocks to support booting the SEV-SNP
VMs, it does not cover all the security enhancement introduced by the SEV-SNP
such as interrupt protection.
Many of the integrity guarantees of SEV-SNP are enforced through a new
structure called the Reverse Map Table (RMP). Adding a new page to SEV-SNP
VM requires a 2-step process. First, the hypervisor assigns a page to the
guest using the new RMPUPDATE instruction. This transitions the page to
guest-invalid. Second, the guest validates the page using the new PVALIDATE
instruction. The SEV-SNP VMs can use the new "Page State Change Request NAE"
defined in the GHCB specification to ask hypervisor to add or remove page
from the RMP table.
Each page assigned to the SEV-SNP VM can either be validated or unvalidated,
as indicated by the Validated flag in the page's RMP entry. There are two
approaches that can be taken for the page validation: Pre-validation and
Lazy Validation.
Under pre-validation, the pages are validated prior to first use. And under
lazy validation, pages are validated when first accessed. An access to a
unvalidated page results in a #VC exception, at which time the exception
handler may validate the page. Lazy validation requires careful tracking of
the validated pages to avoid validating the same GPA more than once. The
recently introduced "Unaccepted" memory type can be used to communicate the
unvalidated memory ranges to the Guest OS.
At this time we only support the pre-validation. OVMF detects all the available
system RAM in the PEI phase. When SEV-SNP is enabled, the memory is validated
before it is made available to the EDK2 core.
This series does not implements the following SEV-SNP features yet:
* CPUID filtering
* Lazy validation
* Interrupt security
The series builds on SNP pre-patch posted here: https://tinyurl.com/pu6admks
Additional resources
---------------------
SEV-SNP whitepaper
https://www.amd.com/system/files/TechDocs/SEV-SNP-strengthening-vm-isolation-with-integrity-protection-and-more.pdf
APM 2: https://www.amd.com/system/files/TechDocs/24593.pdf (section 15.36)
The complete source is available at
https://github.com/AMDESE/ovmf/tree/sev-snp-rfc-2
GHCB spec:
https://developer.amd.com/wp-content/resources/56421.pdf
SEV-SNP firmware specification:
https://developer.amd.com/sev/
Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Changes since v2:
* Add support for the AP creation.
* Use the module-scoping override to make AmdSevDxe use the IO port for PCI reads.
* Use the reserved memory type for CPUID and Secrets page.
*
Changes since v1:
* Drop the interval tree support to detect the pre-validated overlap region.
* Use an array to keep track of pre-validated regions.
* Add support to query the Hypervisor feature and verify that SNP feature is supported.
* Introduce MemEncryptSevClearMmioPageEncMask() to clear the C-bit from MMIO ranges.
* Pull the SevSecretDxe and SevSecretPei into OVMF package build.
* Extend the SevSecretDxe to expose confidential computing blob location through
EFI configuration table.
Brijesh Singh (21):
UefiCpuPkg: Define the SEV-SNP specific dynamic PCDs
OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
OvmfPkg/MemEncryptSevLib: extend the workarea to include SNP enabled
field
OvmfPkg/MemEncryptSevLib: extend Es Workarea to include hv features
OvmfPkg: reserve Secrets page in MEMFD
OvmfPkg: reserve CPUID page for the SEV-SNP guest
OvmfPkg/ResetVector: validate the data pages used in SEC phase
OvmfPkg/ResetVector: invalidate the GHCB page
OvmfPkg: add library to support registering GHCB GPA
OvmfPkg/PlatformPei: register GHCB gpa for the SEV-SNP guest
UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is
enabled
OvmfPkg/AmdSevDxe: do not use extended PCI config space
OvmfPkg/MemEncryptSevLib: add support to validate system RAM
OvmfPkg/BaseMemEncryptSevLib: skip the pre-validated system RAM
OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI
phase
OvmfPkg/SecMain: pre-validate the memory used for decompressing Fv
OvmfPkg/PlatformPei: validate the system RAM when SNP is active
OvmfPkg/MemEncryptSevLib: Change the page state in the RMP table
OvmfPkg/MemEncryptSevLib: skip page state change for Mmio address
OvmfPkg/AmdSev: expose the SNP reserved pages through configuration
table
MdePkg/GHCB: increase the GHCB protocol max version
Tom Lendacky (1):
UefiCpuPkg/MpInitLib: Use SEV-SNP AP Creation NAE event to launch APs
OvmfPkg/OvmfPkg.dec | 21 ++
UefiCpuPkg/UefiCpuPkg.dec | 11 +
OvmfPkg/AmdSev/AmdSevX64.dsc | 5 +-
OvmfPkg/Bhyve/BhyveX64.dsc | 5 +-
OvmfPkg/OvmfPkgIa32.dsc | 2 +
OvmfPkg/OvmfPkgIa32X64.dsc | 7 +-
OvmfPkg/OvmfPkgX64.dsc | 8 +-
OvmfPkg/OvmfXen.dsc | 5 +-
OvmfPkg/OvmfPkgX64.fdf | 17 +-
OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf | 4 +
OvmfPkg/AmdSev/SecretPei/SecretPei.inf | 1 +
.../DxeMemEncryptSevLib.inf | 3 +
.../PeiMemEncryptSevLib.inf | 7 +
.../SecMemEncryptSevLib.inf | 3 +
.../GhcbRegisterLib/GhcbRegisterLib.inf | 33 +++
OvmfPkg/PlatformPei/PlatformPei.inf | 5 +
OvmfPkg/ResetVector/ResetVector.inf | 4 +
OvmfPkg/Sec/SecMain.inf | 3 +
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 4 +
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 4 +
MdePkg/Include/Register/Amd/Ghcb.h | 2 +-
.../Guid/ConfidentialComputingSecret.h | 18 ++
OvmfPkg/Include/Library/GhcbRegisterLib.h | 27 ++
OvmfPkg/Include/Library/MemEncryptSevLib.h | 31 +-
.../X64/SnpPageStateChange.h | 31 ++
.../BaseMemEncryptSevLib/X64/VirtualMemory.h | 19 ++
UefiCpuPkg/Library/MpInitLib/MpLib.h | 19 ++
OvmfPkg/AmdSev/SecretDxe/SecretDxe.c | 22 ++
OvmfPkg/AmdSev/SecretPei/SecretPei.c | 15 +-
.../DxeMemEncryptSevLibInternal.c | 27 ++
.../Ia32/MemEncryptSevLib.c | 17 ++
.../PeiMemEncryptSevLibInternal.c | 27 ++
.../SecMemEncryptSevLibInternal.c | 19 ++
.../X64/DxeSnpSystemRamValidate.c | 40 +++
.../X64/PeiDxeVirtualMemory.c | 167 ++++++++++-
.../X64/PeiSnpSystemRamValidate.c | 126 ++++++++
.../X64/SecSnpSystemRamValidate.c | 36 +++
.../X64/SnpPageStateChangeInternal.c | 230 +++++++++++++++
.../Library/GhcbRegisterLib/GhcbRegisterLib.c | 97 +++++++
OvmfPkg/PlatformPei/AmdSev.c | 81 ++++++
OvmfPkg/PlatformPei/MemDetect.c | 12 +
OvmfPkg/Sec/SecMain.c | 106 +++++++
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 11 +-
.../MpInitLib/Ia32/SevSnpRmpAdjustInternal.c | 31 ++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 274 ++++++++++++++++--
.../MpInitLib/X64/SevSnpRmpAdjustInternal.c | 44 +++
OvmfPkg/FvmainCompactScratchEnd.fdf.inc | 5 +
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 23 ++
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 227 +++++++++++++++
OvmfPkg/ResetVector/ResetVector.nasmb | 6 +
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 1 +
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 51 ++++
52 files changed, 1956 insertions(+), 38 deletions(-)
create mode 100644 OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.inf
create mode 100644 OvmfPkg/Include/Library/GhcbRegisterLib.h
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
create mode 100644 OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/SevSnpRmpAdjustInternal.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/SevSnpRmpAdjustInternal.c
I'm confirming that this series is in my review queue. However, I may need unusually long time to get to it. Thanks for your patience. Thanks Laszlo
|
|
|
Re: [PATCH v1 0/8] Measured SEV boot with kernel/initrd/cmdline
On 05/25/21 07:31, Dov Murik wrote: Booting with SEV prevented the loading of kernel, initrd, and kernel
command-line via QEMU fw_cfg interface because they arrive from the VMM
which is untrusted in SEV.
However, in some cases the kernel, initrd, and cmdline are not secret
but should not be modified by the host. In such a case, we want to
verify inside the trusted VM that the kernel, initrd, and cmdline are
indeed the ones expected by the Guest Owner, and only if that is the
case go on and boot them up (removing the need for grub inside OVMF in
that mode).
This patch series declares a new page in MEMFD which will contain the
hashes of these three blobs (kernel, initrd, cmdline), each under its
own GUID entry. This tables of hashes is populated by QEMU before
launch, and encrypted as part of the initial VM memory; this makes sure
theses hashes are part of the SEV measurement (which has to be approved
by the Guest Owner for secret injection, for example). Note that this
requires a new QEMU patch which will be submitted soon.
OVMF parses the table of hashes populated by QEMU (patch 5), and as it
reads the fw_cfg blobs from QEMU, it will verify each one against the
expected hash (kernel and initrd verifiers are introduced in patch 6,
and command-line verifier is introduced in patches 7+8). This is all
done inside the trusted VM context. If all the hashes are correct, boot
of the kernel is allowed to continue.
Any attempt by QEMU to modify the kernel, initrd, cmdline (including
dropping one of them), or to modify the OVMF code that verifies those
hashes, will cause the initial SEV measurement to change and therefore
will be detectable by the Guest Owner during launch before secret
injection.
Cc: Laszlo Ersek <lersek@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ashish Kalra <ashish.kalra@...>
Cc: Brijesh Singh <brijesh.singh@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: James Bottomley <jejb@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Min Xu <min.m.xu@...>
Cc: Tom Lendacky <thomas.lendacky@...>
James Bottomley (8):
OvmfPkg/AmdSev/SecretDxe: fix header comment to generic naming
OvmfPkg: PlatformBootManagerLibGrub: Allow executing kernel via fw_cfg
OvmfPkg/AmdSev: add a page to the MEMFD for firmware config hashes
OvmfPkg/QemuKernelLoaderFsDxe: Add ability to verify loaded items
OvmfPkg/AmdSev: Add library to find encrypted hashes for the FwCfg
device
OvmfPkg/AmdSev: Add firmware file plugin to verifier
OvmfPkg: GenericQemuLoadImageLib: Allow verifying fw_cfg command line
OvmfPkg/AmdSev: add SevQemuLoadImageLib
OvmfPkg/OvmfPkg.dec | 10 ++
OvmfPkg/AmdSev/AmdSevX64.dsc | 9 +-
OvmfPkg/AmdSev/AmdSevX64.fdf | 3 +
OvmfPkg/AmdSev/Library/SevFwCfgVerifier/SevFwCfgVerifier.inf | 30 +++++
OvmfPkg/AmdSev/Library/SevHashFinderLib/SevHashFinderLib.inf | 34 ++++++
OvmfPkg/AmdSev/Library/SevQemuLoadImageLib/SevQemuLoadImageLib.inf | 30 +++++
OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.inf | 2 +
OvmfPkg/ResetVector/ResetVector.inf | 2 +
OvmfPkg/AmdSev/Include/Library/SevHashFinderLib.h | 47 ++++++++
OvmfPkg/Include/Library/QemuFwCfgLib.h | 35 ++++++
OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.h | 11 ++
OvmfPkg/AmdSev/Library/SevFwCfgVerifier/SevFwCfgVerifier.c | 60 ++++++++++
OvmfPkg/AmdSev/Library/SevHashFinderLib/SevHashFinderLib.c | 126 ++++++++++++++++++++
OvmfPkg/AmdSev/Library/SevQemuLoadImageLib/SevQemuLoadImageLib.c | 52 ++++++++
OvmfPkg/AmdSev/SecretDxe/SecretDxe.c | 2 +-
OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c | 29 +++++
OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c | 5 +
OvmfPkg/Library/PlatformBootManagerLibGrub/QemuKernel.c | 50 ++++++++
OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c | 31 +++++
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 20 ++++
OvmfPkg/ResetVector/ResetVector.nasmb | 2 +
21 files changed, 587 insertions(+), 3 deletions(-)
create mode 100644 OvmfPkg/AmdSev/Library/SevFwCfgVerifier/SevFwCfgVerifier.inf
create mode 100644 OvmfPkg/AmdSev/Library/SevHashFinderLib/SevHashFinderLib.inf
create mode 100644 OvmfPkg/AmdSev/Library/SevQemuLoadImageLib/SevQemuLoadImageLib.inf
create mode 100644 OvmfPkg/AmdSev/Include/Library/SevHashFinderLib.h
create mode 100644 OvmfPkg/AmdSev/Library/SevFwCfgVerifier/SevFwCfgVerifier.c
create mode 100644 OvmfPkg/AmdSev/Library/SevHashFinderLib/SevHashFinderLib.c
create mode 100644 OvmfPkg/AmdSev/Library/SevQemuLoadImageLib/SevQemuLoadImageLib.c
create mode 100644 OvmfPkg/Library/PlatformBootManagerLibGrub/QemuKernel.c
I'm confirming that this series is in my review queue. However, I may need unusually long time to get to it. Thanks for your patience. Thanks Laszlo
|
|
|
Re: [PATCH 15/43] OvmfPkg/XenAcpiPlatformDxe: remove the InstallAcpiTable() helper function
Philippe Mathieu-Daudé <philmd@...>
On 5/26/21 10:14 PM, Laszlo Ersek wrote: The InstallAcpiTable() helper function buys us nothing. Reduce code
complexity by removing the function.
This patch is best viewed with "git show -b".
Cc: Anthony Perard <anthony.perard@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Julien Grall <julien@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Signed-off-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/XenAcpiPlatformDxe/AcpiPlatform.h | 9 ---
OvmfPkg/XenAcpiPlatformDxe/AcpiPlatform.c | 30 ++--------
OvmfPkg/XenAcpiPlatformDxe/Xen.c | 60 ++++++++++----------
3 files changed, 36 insertions(+), 63 deletions(-) Reviewed-by: Philippe Mathieu-Daude <philmd@...>
|
|
|
Re: [edk2-devel202105 PATCH v2 1/1] ArmPkg/ArmGic: Fix maximum number of interrupts in GICv3
Philippe Mathieu-Daudé <philmd@...>
On 5/24/21 3:01 PM, Sami Mujawar wrote: From: Andreas Sandberg <andreas.sandberg@...>
Bugzilla: 3415 (https://bugzilla.tianocore.org/show_bug.cgi?id=3415)
The GICv3 architecture supports up to 1020 ordinary interrupt
lines. The actual number of interrupts supported is described by the
ITLinesNumber field in the GICD_TYPER register. The total number of
implemented registers is normally calculated as
32*(ITLinesNumber+1). However, maximum value (0x1f) is a special case
since that would indicate that 1024 interrupts are implemented.
Add handling for this special case in ArmGicGetMaxNumInterrupts.
Signed-off-by: Andreas Sandberg <andreas.sandberg@...>
Signed-off-by: Joey Gouly <joey.gouly@...>
Signed-off-by: Sami Mujawar <sami.mujawar@...>
Reviewed-by: Ard Biesheuvel <ardb@...>
---
The changes can be seen at:
https://github.com/samimujawar/edk2/tree/1396_gic_max_num_intr_v2
Notes:
v2:
- Fix comment style. [Laszlo]
- Updated comment style. [Sami]
ArmPkg/Drivers/ArmGic/ArmGicLib.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daude <philmd@...>
|
|
|
Re: 回复: [edk2-devel] [edk2-devel202105 PATCH v2 1/1] ArmPkg/ArmGic: Fix maximum number of interrupts in GICv3
Hi Laszlo, Liming,
Apologies for not doing it earlier. I was not sure if it was within my right to merge the change.
I will merge this in the next 2 hours.
Regards,
Sami Mujawar
From:
Laszlo Ersek <lersek@...>
Date: Thursday, 27 May 2021 at 09:50
To: gaoliming <gaoliming@...>, devel@edk2.groups.io <devel@edk2.groups.io>, Sami Mujawar <Sami.Mujawar@...>
Cc: ardb@... <ardb@...>, leif@... <leif@...>, Matteo Carlini <Matteo.Carlini@...>, Andreas Sandberg <Andreas.Sandberg@...>, Joey Gouly <Joey.Gouly@...>, nd <nd@...>
Subject: Re: 回复: [edk2-devel] [edk2-devel202105 PATCH v2 1/1] ArmPkg/ArmGic: Fix maximum number of interrupts in GICv3
toggle quoted messageShow quoted text
On 05/27/21 04:32, gaoliming wrote:
> If no objection, I will merge this patch today. Then, tomorrow, I will create stable tag 202105.
yes, please do that -- TBH, I thought Sami would merge it sooner, as
Sami does have maintainer access through DynamicTablesPkg and
StandaloneMmPkg.
Thanks!
Laszlo
>
> Thanks
> Liming
>> ----- 邮件原件-----
>> 发件人: devel@edk2.groups.io <devel@edk2.groups.io>
代表 gaoliming
>> 发送时间: 2021 年5 月26 日
10:22
>> 收件人: devel@edk2.groups.io; lersek@...;
>> sami.mujawar@...
>> 抄送: ardb@...; leif@...; Matteo.Carlini@...;
>> Andreas.Sandberg@...; joey.gouly@...; nd@...
>> 主题:
回复: [edk2-devel] [edk2-devel202105 PATCH v2 1/1] ArmPkg/ArmGic:
>> Fix maximum number of interrupts in GICv3
>>
>> Laszlo, Ard, Sami:
>> I am OK to merge this patch for stable tag 202105.
>>
>> Thanks
>> Liming
>>> ----- 邮件原件-----
>>> 发件人: devel@edk2.groups.io <devel@edk2.groups.io>
代表 Laszlo
>> Ersek
>>> 发送时间: 2021 年5 月25 日
19:55
>>> 收件人: devel@edk2.groups.io; sami.mujawar@...
>>> 抄送: ardb@...; leif@...; Matteo.Carlini@...;
>>> Andreas.Sandberg@...; joey.gouly@...; nd@...
>>> 主题: Re: [edk2-devel] [edk2-devel202105 PATCH v2 1/1] ArmPkg/ArmGic:
>>> Fix maximum number of interrupts in GICv3
>>>
>>> Hi Sami,
>>>
>>> On 05/24/21 15:01, Sami Mujawar wrote:
>>>> From: Andreas Sandberg <andreas.sandberg@...>
>>>>
>>>> Bugzilla: 3415 ( https://bugzilla.tianocore.org/show_bug.cgi?id=3415)
>>>>
>>>> The GICv3 architecture supports up to 1020 ordinary interrupt
>>>> lines. The actual number of interrupts supported is described by the
>>>> ITLinesNumber field in the GICD_TYPER register. The total number of
>>>> implemented registers is normally calculated as
>>>> 32*(ITLinesNumber+1). However, maximum value (0x1f) is a special case
>>>> since that would indicate that 1024 interrupts are implemented.
>>>>
>>>> Add handling for this special case in ArmGicGetMaxNumInterrupts.
>>>>
>>>> Signed-off-by: Andreas Sandberg <andreas.sandberg@...>
>>>> Signed-off-by: Joey Gouly <joey.gouly@...>
>>>> Signed-off-by: Sami Mujawar <sami.mujawar@...>
>>>> Reviewed-by: Ard Biesheuvel <ardb@...>
>>>> ---
>>>> The changes can be seen at:
>>>>
https://github.com/samimujawar/edk2/tree/1396_gic_max_num_intr_v2
>>>>
>>>> Notes:
>>>> v2:
>>>> - Fix comment style.
>>> [Laszlo]
>>>> - Updated comment style.
>>> [Sami]
>>>>
>>>> ArmPkg/Drivers/ArmGic/ArmGicLib.c | 11 +++++++++--
>>>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>> I think this patch should be merged really soon, as long as Ard agrees.
>>>
>>> Thanks,
>>> Laszlo
>>>
>>>>
>>>> diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
>>> b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
>>>> index
>>>
>> 6b01c88206ad8adef3100dd44c0d57660db77783..bd4b5edb903f3846f4f0e43
>>> 1f93e001f01cd9e7d 100644
>>>> --- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
>>>> +++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
>>>> @@ -1,6 +1,6 @@
>>>> /** @file
>>>> *
>>>> -* Copyright (c) 2011-2018, ARM Limited. All rights reserved.
>>>> +* Copyright (c) 2011-2021, Arm Limited. All rights reserved.
>>>> *
>>>> * SPDX-License-Identifier: BSD-2-Clause-Patent
>>>> *
>>>> @@ -120,7 +120,14 @@ ArmGicGetMaxNumInterrupts (
>>>> IN INTN GicDistributorBase
>>>> )
>>>> {
>>>> - return 32 * ((MmioRead32 (GicDistributorBase + ARM_GIC_ICDICTR) &
>>> 0x1F) + 1);
>>>> + UINTN ItLines;
>>>> +
>>>> + ItLines = MmioRead32 (GicDistributorBase + ARM_GIC_ICDICTR) &
>>> 0x1F;
>>>> +
>>>> + //
>>>> + // Interrupt ID 1020-1023 are reserved.
>>>> + //
>>>> + return (ItLines == 0x1f) ? 1020 : 32 * (ItLines + 1);
>>>> }
>>>>
>>>> VOID
>>>>
>>>
>>>
>>>
>>>
>>>
>>
>>
>>
>>
>>
>>
>>
>
>
>
|
|
|
Re: [PATCH 05/43] OvmfPkg/README: bump minimum QEMU version to 1.7.1, machine types to 1.7
On 05/27/21 10:24, Philippe Mathieu-Daudé wrote: On 5/26/21 10:14 PM, Laszlo Ersek wrote:
Due to switching to the QemuFwCfgAcpiPlatformDxe driver earlier in this
series, require QEMU version 1.7.1 in the "OvmfPkg/README" file, and
require 1.7 or later machine types too.
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Signed-off-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/README | 43 ++++----------------
1 file changed, 8 insertions(+), 35 deletions(-) :))
Reviewed-by: Philippe Mathieu-Daude <philmd@...>
Time flies, doesn't it :) Thank you! Laszlo
|
|
|
Re: [PATCH 00/43] OvmfPkg: remove Xen support from OvmfPkg*.dsc, in favor of OvmfXen.dsc
On 05/27/21 09:34, Ard Biesheuvel wrote: On Wed, 26 May 2021 at 22:15, Laszlo Ersek <lersek@...> wrote:
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Repo: https://pagure.io/lersek/edk2.git
Branch: xen_split_bz_2122
This patch set removes dynamic Xen enlightenment from the following
platforms:
OvmfPkg/OvmfPkgIa32.dsc
OvmfPkg/OvmfPkgIa32X64.dsc
OvmfPkg/OvmfPkgX64.dsc
In Xen guests, the following platform should be used:
OvmfPkg/OvmfXen.dsc
Please see more details / references in the bugzilla ticket.
NOOPT build savings:
- Ia32: PEIFV 1536 bytes, DXEFV 130288 bytes
- Ia32X64: PEIFV 1536 bytes, DXEFV 140912 bytes
- X64: PEIFV 1664 bytes, DXEFV 140912 bytes
- Xen: PEIFV 256 bytes, DXEFV 69504 bytes
Functional testing:
- Booted a Fedora guest on OvmfPkgIa32X64 on QEMU/KVM, compared verbose
logs before-after. Memory allocations were satisfied at different
addresses, as expected, plus the Xen drivers were absent. No
differences otherwise.
- Booted a RHEL guest on ArmVirtQemu on AARCH64. Memory allocations were
satisfied at different addresses, as expected.
- Xen regression-testing was not done; I'm requesting feedback.
Build testing / bisectability: at every stage, the series builds with
the following script:
#!/bin/bash
set -e -u -C
build -b DEBUG -t GCC5 -p ArmVirtPkg/ArmVirtKvmTool.dsc -a AARCH64
build -b DEBUG -t GCC5 -p ArmVirtPkg/ArmVirtKvmTool.dsc -a ARM
build -b NOOPT -t GCC5 -p ArmVirtPkg/ArmVirtQemu.dsc -a AARCH64
build -b NOOPT -t GCC5 -p ArmVirtPkg/ArmVirtQemu.dsc -a ARM
build -b NOOPT -t GCC5 -p ArmVirtPkg/ArmVirtQemuKernel.dsc -a AARCH64
build -b NOOPT -t GCC5 -p ArmVirtPkg/ArmVirtQemuKernel.dsc -a ARM
build -b NOOPT -t GCC5 -p ArmVirtPkg/ArmVirtXen.dsc -a AARCH64
build -b NOOPT -t GCC5 -p ArmVirtPkg/ArmVirtXen.dsc -a ARM
build -b NOOPT -t GCC5 -p OvmfPkg/AmdSev/AmdSevX64.dsc -a X64
build -b NOOPT -t GCC5 -p OvmfPkg/Bhyve/BhyveX64.dsc -a X64
build -b NOOPT -t GCC5 -p OvmfPkg/OvmfPkgIa32.dsc -a IA32
build -b NOOPT -t GCC5 -p OvmfPkg/OvmfPkgIa32X64.dsc -a IA32 -a X64
build -b NOOPT -t GCC5 -p OvmfPkg/OvmfPkgX64.dsc -a X64
build -b NOOPT -t GCC5 -p OvmfPkg/OvmfXen.dsc -a X64 The patches in the series were formatted with the following options, for
posting:
--stat=1000 --stat-graph-width=20 --find-copies-harder -U6
(The option "--find-copies-harder" is not the best for presenting every
single patch in the series, in isolation, but taken globally for the
entire series, it is the most helpful option.)
Some patches advance with really small steps, in order to cut down on a
subsequent "meaty" patch. Personally I don't like reviewing code
movement patches, so I did my best to (a) keep that to a minimum, and
(b) present it as unintrusively as possible.
The CC list is a bit long; the reason is that I kept touching up
"Maintainers.txt", and the "OvmfPkg/Bhyve" and "OvmfPkg/AmdSev"
platforms as well (whenever it made sense).
Thanks for taking this on.
For the series,
Reviewed-by: Ard Biesheuvel <ardb@...> Thank you, Ard! I hope the large patch count (with the small patch bodies and the not-so-short commit messages) didn't get on your nerves! I really did make an effort to find a good balance. E.g. in the "clean up and sort #includes & INF files" patches. Thank you again! Laszlo
|
|
|
Re: 回复: [edk2-devel] [edk2-devel202105 PATCH v2 1/1] ArmPkg/ArmGic: Fix maximum number of interrupts in GICv3
Hi Liming, On 05/27/21 04:32, gaoliming wrote: If no objection, I will merge this patch today. Then, tomorrow, I will create stable tag 202105. yes, please do that -- TBH, I thought Sami would merge it sooner, as Sami does have maintainer access through DynamicTablesPkg and StandaloneMmPkg. Thanks! Laszlo
Thanks
Liming
-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 gaoliming
发送时间: 2021年5月26日 10:22
收件人: devel@edk2.groups.io; lersek@...;
sami.mujawar@...
抄送: ardb@...; leif@...; Matteo.Carlini@...;
Andreas.Sandberg@...; joey.gouly@...; nd@...
主题: 回复: [edk2-devel] [edk2-devel202105 PATCH v2 1/1] ArmPkg/ArmGic:
Fix maximum number of interrupts in GICv3
Laszlo, Ard, Sami:
I am OK to merge this patch for stable tag 202105.
Thanks
Liming
-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Laszlo Ersek
发送时间: 2021年5月25日 19:55
收件人: devel@edk2.groups.io; sami.mujawar@...
抄送: ardb@...; leif@...; Matteo.Carlini@...;
Andreas.Sandberg@...; joey.gouly@...; nd@...
主题: Re: [edk2-devel] [edk2-devel202105 PATCH v2 1/1] ArmPkg/ArmGic:
Fix maximum number of interrupts in GICv3
Hi Sami,
On 05/24/21 15:01, Sami Mujawar wrote:
From: Andreas Sandberg <andreas.sandberg@...>
Bugzilla: 3415 (https://bugzilla.tianocore.org/show_bug.cgi?id=3415)
The GICv3 architecture supports up to 1020 ordinary interrupt
lines. The actual number of interrupts supported is described by the
ITLinesNumber field in the GICD_TYPER register. The total number of
implemented registers is normally calculated as
32*(ITLinesNumber+1). However, maximum value (0x1f) is a special case
since that would indicate that 1024 interrupts are implemented.
Add handling for this special case in ArmGicGetMaxNumInterrupts.
Signed-off-by: Andreas Sandberg <andreas.sandberg@...>
Signed-off-by: Joey Gouly <joey.gouly@...>
Signed-off-by: Sami Mujawar <sami.mujawar@...>
Reviewed-by: Ard Biesheuvel <ardb@...>
---
The changes can be seen at:
https://github.com/samimujawar/edk2/tree/1396_gic_max_num_intr_v2
Notes:
v2:
- Fix comment style. [Laszlo]
- Updated comment style. [Sami]
ArmPkg/Drivers/ArmGic/ArmGicLib.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
I think this patch should be merged really soon, as long as Ard agrees.
Thanks,
Laszlo
diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
index 6b01c88206ad8adef3100dd44c0d57660db77783..bd4b5edb903f3846f4f0e43
1f93e001f01cd9e7d 100644
--- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
+++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
@@ -1,6 +1,6 @@
/** @file
*
-* Copyright (c) 2011-2018, ARM Limited. All rights reserved.
+* Copyright (c) 2011-2021, Arm Limited. All rights reserved.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*
@@ -120,7 +120,14 @@ ArmGicGetMaxNumInterrupts (
IN INTN GicDistributorBase
)
{
- return 32 * ((MmioRead32 (GicDistributorBase + ARM_GIC_ICDICTR) & 0x1F) + 1);
+ UINTN ItLines;
+
+ ItLines = MmioRead32 (GicDistributorBase + ARM_GIC_ICDICTR) & 0x1F;
+
+ //
+ // Interrupt ID 1020-1023 are reserved.
+ //
+ return (ItLines == 0x1f) ? 1020 : 32 * (ItLines + 1);
}
VOID
|
|
|
Re: [PATCH 37/43] OvmfPkg/SmbiosPlatformDxe: return EFI_NOT_FOUND if there is no SMBIOS data
Philippe Mathieu-Daudé <philmd@...>
On 5/26/21 10:14 PM, Laszlo Ersek wrote: According to the function-top comment, SmbiosTablePublishEntry() is
supposed to return an error code if no SMBIOS data is found, from either
GetXenSmbiosTables() or GetQemuSmbiosTables(). Currently the function
returns EFI_SUCCESS in this case however (propagated from
gBS->LocateProtocol()). Make the return code match the documentation.
(This issue is not too important, but it gets in the way of splitting the
entry point function next.)
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Signed-off-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 1 +
1 file changed, 1 insertion(+) Reviewed-by: Philippe Mathieu-Daude <philmd@...>
|
|
|
Re: [PATCH 24/43] OvmfPkg/Bhyve: make "PcdPciDisableBusEnumeration" Fixed-at-Build
Philippe Mathieu-Daudé <philmd@...>
On 5/26/21 10:14 PM, Laszlo Ersek wrote: The Bhyve platform specifies the dynamic access method for
"PcdPciDisableBusEnumeration" needlessly.
After the DSC file sets the PCD to TRUE by default, the PCD is never
written again. In particular, the
"OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf" file references the PCD
superfluously.
Make the PCD Fixed-At-Build, and remove the PCD reference from the INF
file.
(Note that further simplifications are possible in
"OvmfPkg/Bhyve/AcpiPlatformDxe", but those are out of scope for this patch
series.)
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Peter Grehan <grehan@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Cc: Rebecca Cran <rebecca@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Signed-off-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/Bhyve/BhyveX64.dsc | 2 +-
OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf | 1 -
2 files changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daude <philmd@...>
|
|
|
Re: [PATCH 19/43] OvmfPkg/OvmfXen: make "PcdPciDisableBusEnumeration" Fixed-at-Build
Philippe Mathieu-Daudé <philmd@...>
On 5/26/21 10:14 PM, Laszlo Ersek wrote: The OvmfXen platform specifies the dynamic access method for
"PcdPciDisableBusEnumeration" needlessly.
After the DSC file sets the PCD to TRUE by default, the InitializeXen()
function in XenPlatformPei superfluously sets the PCD to TRUE again. There
are no other writes to the PCD in the platform.
Make the PCD Fixed-At-Build, and remove the access (in fact, the whole
InitializeXen() function) from XenPlatformPei.
Cc: Anthony Perard <anthony.perard@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Julien Grall <julien@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Signed-off-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/OvmfXen.dsc | 2 +-
OvmfPkg/XenPlatformPei/XenPlatformPei.inf | 1 -
OvmfPkg/XenPlatformPei/Platform.h | 5 -----
OvmfPkg/XenPlatformPei/Platform.c | 1 -
OvmfPkg/XenPlatformPei/Xen.c | 20 --------------------
5 files changed, 1 insertion(+), 28 deletions(-) Reviewed-by: Philippe Mathieu-Daude <philmd@...>
|
|
|
Re: [PATCH 10/43] OvmfPkg/AcpiPlatformDxe: consolidate #includes and [LibraryClasses]
#includes
Philippe Mathieu-Daudé <philmd@...>
On 5/26/21 10:14 PM, Laszlo Ersek wrote: - #include only such public headers in "AcpiPlatform.h" that are required
by the function declarations and type definitions introduced in
"AcpiPlatform.h". Don't use "AcpiPlatform.h" as a convenience #include
file.
- In every file, list every necessary public #include individually, with
an example identifier that's actually consumed.
- Remove unnecessary lib classes, add unlisted lib classes.
- Remove unnecessary #include directives, add unlisted #include
directives.
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Signed-off-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf | 1 -
OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf | 2 ++
OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h | 12 ++----------
OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c | 5 +++++
OvmfPkg/AcpiPlatformDxe/BootScript.c | 7 ++++---
OvmfPkg/AcpiPlatformDxe/EntryPoint.c | 6 +++++-
OvmfPkg/AcpiPlatformDxe/PciDecoding.c | 4 +++-
OvmfPkg/AcpiPlatformDxe/Qemu.c | 14 +++++++-------
OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 19 ++++++++++---------
OvmfPkg/AcpiPlatformDxe/Xen.c | 4 +++-
10 files changed, 41 insertions(+), 33 deletions(-) Reviewed-by: Philippe Mathieu-Daude <philmd@...>
|
|
|
Re: [PATCH 17/43] OvmfPkg/Bhyve/AcpiPlatformDxe: fix file path typo in comment
Philippe Mathieu-Daudé <philmd@...>
On 5/26/21 10:14 PM, Laszlo Ersek wrote: The built-in ACPI tables for Bhyve are located in the
"OvmfPkg/Bhyve/AcpiTables" module, not in the "OvmfPkg/AcpiTables" module.
Correct the typo in a code comment.
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Peter Grehan <grehan@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Cc: Rebecca Cran <rebecca@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Signed-off-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daude <philmd@...>
|
|
|
Re: [PATCH 16/43] OvmfPkg/XenAcpiPlatformDxe: remove OVMF's built-in ACPI tables
Philippe Mathieu-Daudé <philmd@...>
On 5/26/21 10:14 PM, Laszlo Ersek wrote: Xen is an advanced hypervisor; no Xen guest can function correctly without
the hypervisor's dynamically provided ACPI tables. Remove the built-in
(fallback) tables from XenAcpiPlatformDxe -- and the OvmfXen platform.
Remove any dependencies from XenAcpiPlatformDxe that are no longer needed.
Cc: Anthony Perard <anthony.perard@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Julien Grall <julien@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Signed-off-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/OvmfXen.dsc | 1 -
OvmfPkg/OvmfXen.fdf | 7 -
OvmfPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf | 2 -
OvmfPkg/XenAcpiPlatformDxe/AcpiPlatform.c | 202 --------------------
4 files changed, 212 deletions(-) Reviewed-by: Philippe Mathieu-Daude <philmd@...>
|
|
|
Re: [PATCH 14/43] OvmfPkg/XenAcpiPlatformDxe: remove QEMU fw_cfg dependency
Philippe Mathieu-Daudé <philmd@...>
On 5/26/21 10:14 PM, Laszlo Ersek wrote: The QemuDetected() function wraps QemuFwCfgIsAvailable(); it always fails
on Xen. Because of that, we can eliminate the QemuDetected() call itself
from the Xen ACPI platform driver, and then the rest of "Qemu.c" becomes
useless -- the workhorse function of that source file is
QemuInstallAcpiTable(), which we no longer call.
Remove any dependencies that are no longer needed by the
XenAcpiPlatformDxe driver.
Cc: Anthony Perard <anthony.perard@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Julien Grall <julien@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Signed-off-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf | 9 -
OvmfPkg/XenAcpiPlatformDxe/AcpiPlatform.h | 14 -
OvmfPkg/XenAcpiPlatformDxe/AcpiPlatform.c | 9 +-
OvmfPkg/XenAcpiPlatformDxe/Qemu.c | 511 --------------------
4 files changed, 1 insertion(+), 542 deletions(-) Reviewed-by: Philippe Mathieu-Daude <philmd@...>
|
|
|
Re: [PATCH 13/43] OvmfPkg/XenAcpiPlatformDxe: remove the QEMU ACPI linker/loader client
Philippe Mathieu-Daudé <philmd@...>
On 5/26/21 10:14 PM, Laszlo Ersek wrote: The root of the QEMU ACPI linker/loader client in XenAcpiPlatformDxe is
the InstallQemuFwCfgTables() function. This function always fails on Xen,
due to its top-most QemuFwCfgFindFile() call.
Remove the InstallQemuFwCfgTables() function call from XenAcpiPlatformDxe,
along with all dependencies that now become unused.
Cc: Anthony Perard <anthony.perard@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Julien Grall <julien@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Signed-off-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf | 6 -
OvmfPkg/XenAcpiPlatformDxe/AcpiPlatform.h | 51 -
OvmfPkg/XenAcpiPlatformDxe/AcpiPlatform.c | 2 +-
OvmfPkg/XenAcpiPlatformDxe/BootScript.c | 269 -----
OvmfPkg/XenAcpiPlatformDxe/PciDecoding.c | 194 ----
OvmfPkg/XenAcpiPlatformDxe/QemuFwCfgAcpi.c | 1196 --------------------
6 files changed, 1 insertion(+), 1717 deletions(-) Reviewed-by: Philippe Mathieu-Daude <philmd@...>
|
|
|
Re: [PATCH 38/43] OvmfPkg/SmbiosPlatformDxe: locate SMBIOS protocol in InstallAllStructures()
Philippe Mathieu-Daudé <philmd@...>
On 5/26/21 10:14 PM, Laszlo Ersek wrote: Locate the SMBIOS protocol internally to the InstallAllStructures()
function. This has no performance impact (InstallAllStructures() is only
called once), but moving the code from the entry point function makes the
latter smaller. And that will be useful when we split the entry point
function to two versions.
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Signed-off-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 30 +++++++++-----------
1 file changed, 14 insertions(+), 16 deletions(-) Reviewed-by: Philippe Mathieu-Daude <philmd@...>
|
|
|
Re: [PATCH 36/43] OvmfPkg/SmbiosPlatformDxe: clean up #includes and INF
#includes
Philippe Mathieu-Daudé <philmd@...>
On 5/26/21 10:14 PM, Laszlo Ersek wrote: - Sort all sections in the INF file.
- Remove unused packages (MdeModulePkg) and lib classes (BaseMemoryLib)
from the INF file.
- Restrict some lib classes (BaseLib, HobLib) and GUIDs (gEfiXenInfoGuid)
to IA32 and X64, in the INF file; only the IA32/X64 Xen implementation
requires these.
- Don't make "SmbiosPlatformDxe.h" #include everything just as a
convenience. Spell out directly needed #includes in every file (annotate
each with an example identifier consumed), drop unused #includes.
- Keep #includes sorted.
- Remove the leading underscore from the #include guard macro name in
"SmbiosPlatformDxe.h".
Cc: Anthony Perard <anthony.perard@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Julien Grall <julien@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Signed-off-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf | 20 ++++++++++----------
OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.h | 15 +++------------
OvmfPkg/SmbiosPlatformDxe/Qemu.c | 8 +++++---
OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 6 ++++++
OvmfPkg/SmbiosPlatformDxe/X86Xen.c | 6 ++++--
5 files changed, 28 insertions(+), 27 deletions(-) Reviewed-by: Philippe Mathieu-Daude <philmd@...>
|
|
|
