BaseMemoryLibSse2: Take advantage of write combining buffers
The current SSE2 implementation of the ZeroMem(), SetMem(),
SetMem16(), SetMem32 and SetMem64 functions is writing 16 bytes per 16
bytes. It hurts the performances so bad that this is even slower than
a simple 'rep stos' (4% slower) in regular DRAM.
To take full advantages of the 'movntdq' instruction it is better to
"queue" a total of 64 bytes in the write combining buffers. This
patch implement such a change. Below is a table where I measured
(with 'rdtsc') the time to write an entire 100MB RAM buffer. These
functions operate almost two times faster.
Jan Bobek [Thu, 1 Oct 2020 16:15:07 +0000 (00:15 +0800)]
MdePkg/BaseLib: Fix invalid memory access in AArch64 SetJump/LongJump
Correct the memory offsets used in REG_ONE/REG_PAIR macros to
synchronize them with definition of the BASE_LIBRARY_JUMP_BUFFER
structure on AArch64.
The REG_ONE macro declares only a single 64-bit register be
read/written; however, the subsequent offset is 16 bytes larger,
creating an unused memory gap in the middle of the structure and
causing SetJump/LongJump functions to read/write 8 bytes of memory
past the end of the jump buffer struct.
Signed-off-by: Jan Bobek <jbobek@nvidia.com> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com> Acked-by: Michael D Kinney <michael.d.kinney@intel.com> Acked-by: Liming Gao <gaoliming@byosoft.com.cn>
C tool may add the additional INC include path. They should have
high priority than the common INC include path.
This fix is to resolve the structure PCD issue to refer to the same
header file defined in BaseTools and MdePkg. The one in MdePkg should
be used.
MdePkg Acpi10.h definition depends on PACKED.
When structure PCD refers to Acpi10.h, build will fail,
because PACKED definition is missing in BaseTools BaseTypes.h.
C source tools include BaseTools BaseTypes.h. They don't include MdePkg Base.h.
When C source tools include MdePkg Acpi10.h, they also need PACKED definition.
So, add PACKED definition into BaseTools BaseTypes.h.
Cc: Bob Feng <bob.c.feng@intel.com> Cc: Yuwei Chen <yuwei.chen@intel.com> Cc: Michael Kinney <michael.d.kinney@intel.com> Signed-off-by: Liming Gao <gaoliming@byosoft.com.cn> Reviewed-by: Bob Feng <bob.c.feng@intel.com> Tested-by: Garrett Kirkendall <garrett.kirkendall@amd.com>
MdeModulePkg/Core/Dxe: expose SP and CRYPTO capabilities in UEFI memmap
OSs are now capable of treating SP and CRYPTO memory as true capabilities
and therefore these should be exposed. This requires usage of a separate
ACCESS_MASK to hide all page-access permission capabilities.
Change in masking and hiding of SP and CRYPTO was introduced in 3bd5c994c879f78e8e3d5346dc3b627f199291aa
Operating systems are capable of treating SP and CRYPTO memory capabilities
and not as attributes. This means that these capabilites cannot be hidden
from OSs. For this reason, the SP and CRYPTO bits should be separated from
the bitmask that we use for hiding the page-access attributes.
Common mask for ATTRIBUTES was introduced in 3bd5c994c879f78e8e3d5346dc3b627f199291aa
SMBIOS 3.4 spec adds new memory device types (DDR5, LPDDR5)
Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Zhiguang Liu <zhiguang.liu@intel.com> Signed-off-by: Sanyo Wang <sanyo.wang@intel.com> Reviewed-by: Zhiguang Liu <zhiguang.liu@intel.com> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn> Reviewed-by: Sean Brogan <sean.brogan@microsoft.com>
NetworkPkg: Fix possible infinite loop in HTTP msg body parser
When an HTTP server sends a non-chunked body data with no
Content-Length header, the HttpParserMessageBody in DxeHttpLib
gets confused and never sets the Char pointer beyond the body start.
This causes "for" loop to never break because the condition of
"Char >= Body + BodyLength" is never satisfied.
Use BodyLength as the ContentLength for the parser when
ContentLength is absent in HTTP response headers.
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2941
Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com> Cc: Jiaxin Wu <jiaxin.wu@intel.com> Cc: Siyuan Fu <siyuan.fu@intel.com> Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
When Http->Configure() is invoked with HttpConfigData set to
NULL to reset the EFI HTTP instance, TLS child instance is
destroyed but HttpInstance->TlsChildHandle is not set to
NULL. After reconfiguring HTTP through Http->Configure()
and sending the HTTP request to HTTPS URL, TLS child
instance is not recreated because
HttpInstance->TlsChildHandle is not NULL.
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com> Cc: Wu Jiaxin <jiaxin.wu@intel.com> Cc: Fu Siyuan <siyuan.fu@intel.com> Cc: Nickle Wang <nickle.wang@hpe.com> Signed-off-by: Abner Chang <abner.chang@hpe.com> Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com> Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Abner Chang [Thu, 8 Oct 2020 00:53:07 +0000 (08:53 +0800)]
RedfishPkg: Initial commit of RedfishPkg.
Initial version of RedfishPkg.
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com> Signed-off-by: Fu Siyuan <siyuan.fu@intel.com> Signed-off-by: Wang Fan <fan.wang@intel.com> Signed-off-by: Chang Abner <abner.chang@hpe.com> Cc: Nickle Wang <nickle.wang@hpe.com> Cc: Andrew Fish <afish@apple.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Leif Lindholm <leif@nuviainc.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Acked-by: Laszlo Ersek <lersek@redhat.com> Acked-by: Leif Lindholm <leif@nuviainc.com> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Introduce an http client utilizing EDK2 HTTP protocol, to
allow fast image downloading from http/https servers.
HTTP download speed is usually faster than tftp.
The client is based on the same approach as tftp dynamic command, and
uses the same UEFI Shell command line parameters. This makes it easy
integrating http into existing UEFI Shell scripts.
Note that to enable HTTP download, feature Pcd
gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections must
be set to TRUE.
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2860
Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Zhichao Gao <zhichao.gao@intel.com> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com> Cc: Jiaxin Wu <jiaxin.wu@intel.com> Cc: Siyuan Fu <siyuan.fu@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Liming Gao <liming.gao@intel.com> Cc: Nd <nd@arm.com> Reviewed-by: Zhichao Gao <zhichao.gao@intel.com> Tested-by: Laszlo Ersek <lersek@redhat.com>
Crypto algorithms, hkdf-sha256/hmac-sha256/aes, are needed to verify
integrity of variable data, derive hmac and encryption keys, and
encrypt/decrypt varible. Replacing null version source code with real
implementations makes sure we can enable full functionalities of protected
variable later.
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Guomin Jiang <guomin.jiang@intel.com> Cc: Nishant C Mistry <nishant.c.mistry@intel.com> Signed-off-by: Jian J Wang <jian.j.wang@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Timeouts in the XhciDxe driver are taking longer than
expected due to the timeout loops not accounting for
code execution time. As en example, 5 second timeouts
have been observed to take around 36 seconds to complete.
Use SetTimer and Create/CheckEvent from Boot Services to
determine when timeout occurred.
Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Hao A Wu <hao.a.wu@intel.com> Cc: Ray Ni <ray.ni@intel.com> Signed-off-by: Patrick Henz <patrick.henz@hpe.com> Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Add CI status badges for the EmulatorPkg CI builds with
SECURE_BOOT_ENABLE=TRUE for IA32/X64 and DEBUG/RELEASE/NOOPT.
Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Andrew Fish <afish@apple.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Divneil Rai Wadhawan <divneil.r.wadhawan@intel.com> Cc: Sean Brogan <sean.brogan@microsoft.com> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com> Reviewed-by: Sean Brogan <sean.brogan@microsoft.com> Reviewed-by: Ray Ni <ray.ni@intel.com>
Add EmulatorPkg CI builds for SECURE_BOOT_ENABLE=TRUE
for IA32/X64 and DEBUG/RELEASE/NOOPT. Label these as
FULL builds, so if additional build options are added
in the future, they can be added to these FULL builds.
Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Andrew Fish <afish@apple.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Divneil Rai Wadhawan <divneil.r.wadhawan@intel.com> Cc: Sean Brogan <sean.brogan@microsoft.com> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com> Reviewed-by: Sean Brogan <sean.brogan@microsoft.com> Reviewed-by: Ray Ni <ray.ni@intel.com>
Currently, the build tool try to read the section alignment
from efi file if the section alignment type is Auto.
If there is no efi generated, the section alignment will
be set to zero. This behavior causes the Makefile to be different
between the full build and the incremental build.
Since the Genffs can auto get the section alignment from
efi file during Genffs procedure, the build tool can just set section
alignment as zero. This change can make the autogen makefile
consistent for the full build and the incremental build.
If a module add a new PCD, the pcd token number will be
reassigned. The new Pcd token number should be updated
to all module's autogen files. CanSkip can only detect a
single module's change but not others. CanSkip block the
pcd token number update in incremental build, so this
patch is going to remove this call.
Currently, When doing the Incremental build, the directory
macros extended to absolute path in output Makefile, which
is inconsistent with the output of Clean build.
When we do macro replacement, we can't replace macro due to
inconsistent path case, which results in inconsistent display
of incremental build and clean build in makefile.Therefore,
the path is converted to achieve the correct macro replacement.
The content in Guid.xref depends on the files under the corresponding
ffs folder.(refer to the commit 5e9256cd7f54ffd6f1fd9837df92a911fcd2d7c2)
To make Guid.xref update in the incremental build,
clean the files under that ffs folder before generating files in it.
After changing the name of the include source file,
when doing incremental build, the previous source file
is not covered in the. DEPs file, and a build error occurs.
The root cause is that the build tools filter out some dependency
files, which are listed in inf source section, from the deps_target file.
Add those files back to deps_target file to resolve the above problem.
When calculating memory regions and store the information in the
gSystemMemory in file WinHost.c, the code below will cause overflow,
because _wtoi (MemorySizeStr) return an int value and SIZE_1MB is
also an int value, if MemorySizeStr is lager for example 2048, then
result of multiplication will overflow.
for (Index = 0, Done = FALSE; !Done; Index++) {
//
// Save the size of the memory and make a Unicode filename SystemMemory00
//
gSystemMemory[Index].Size = _wtoi (MemorySizeStr) * SIZE_1MB;
Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Andrew Fish <afish@apple.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Signed-off-by: Wenyi Xie <xiewenyi2@huawei.com>
In QEMU commit range 4abf70a661a5..69699f3055a5 (later fixed up in QEMU
commit 4318432ccd3f), Phil implemented a QEMU facility for exposing the
host-side TLS cipher suite configuration to OVMF. The purpose is to
control the permitted ciphers in the guest's UEFI HTTPS boot. This
complements the forwarding of the host-side crypto policy from the host to
the guest -- the other facet was the set of CA certificates (for which
p11-kit patches had been upstreamed, on the host side).
Mention the new command line options in "OvmfPkg/README".
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> Cc: Gary Lin <glin@suse.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2852 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Gary Lin <glin@suse.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200922091827.12617-1-lersek@redhat.com> Acked-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
NOTE: this patch does not introduce a cross-domain mapping; it only maps
the name "gaoliming" in Liming's new email address to "Liming Gao" (see
the Author field on commit aad9cba85fb7).
SECURE_BOOT_ENABLE feature flag is introduced to enable Secure Boot.
The following gets enabled with this patch:
* Secure Boot Menu in "Device Manager" for enrolling keys
* Storage space for Authenticated Variables
* Authenticated execution of 3rd party images
Signed-off-by: Divneil Rai Wadhawan <divneil.r.wadhawan@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com>
Changes OpenSSL to no longer depend on TimerLib and instead use RngLib.
This allows platforms to decide for themsevles what sort of entropy source
they provide to OpenSSL and TlsLib.
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com> Acked-by: Ard Biesheuvel <ard.biesheuvel@arm.com> Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com> Signed-off-by: Matthew Carlson <matthewfcarlson@gmail.com>
Matthew Carlson [Fri, 31 Jul 2020 20:41:53 +0000 (13:41 -0700)]
ArmVirtPkg: Add RngLib based on TimerLib for CryptoPkg
Updates the DSC for the ArmVirtPkg platform to add a RngLib that uses the
TimerLib. This is due to a later change that adds TimerLib as a dependency
for OpenSSL. The TimerLib based RngLib mimics the behavior of OpenSSL
previously and it is recommended to switch to a better source of
entropy than the system's performance counter.
Matthew Carlson [Fri, 31 Jul 2020 20:39:48 +0000 (13:39 -0700)]
OvmfPkg: Add RngLib based on TimerLib for Crypto
Updates the DSC's for Ovmf based platforms to add a RngLib that uses the
TimerLib. This is due to a later change that adds TimerLib as a dependency
for OpenSSL. The TimerLib based RngLib mimics the behavior of OpenSSL
previously and it is recommended to switch to a better source of
entropy than the system's performance counter.
Matthew Carlson [Tue, 28 Jul 2020 01:36:24 +0000 (18:36 -0700)]
MdePkg: TimerRngLib: Added RngLib that uses TimerLib
Added a new RngLib that provides random numbers from the TimerLib
using the performance counter. This is meant to be used for OpenSSL
to replicate past behavior. This should not be used in production as
a real source of entropy.
The value of EFI_BLOCK_IO_PROTOCOL_REVISION3 is currently
0x00020031. However, the value assigned in the UEFI Specification
2.8B is ((2<<16) | (31)) which is 0x0002001F.
Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Zhiguang Liu <zhiguang.liu@intel.com> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn> Reviewed-by: Zhiguang Liu <zhiguang.liu@intel.com>
OvmfPkg: drop redundant VendorID check in VirtioMmioDeviceLib
There is a DEBUG warning printout in VirtioMmioDeviceLib if the current
device's VendorID does not match the traditional 16-bit Red Hat PCIe
vendor ID used with virtio-pci. The virtio-mmio vendor ID is 32-bit and
has no connection to the PCIe registry.
Most specifically, this causes a bunch of noise when booting an AArch64
QEMU platform, since QEMU's virtio-mmio implementation used 'QEMU' as
the vendor ID:
VirtioMmioInit: Warning:
The VendorId (0x554D4551) does not match the VirtIo VendorId (0x1AF4).
UefiPayloadPkg: Support variable size MMCONF space
The default size is still 256MiB, but will be overwritten by
UefiPayloadPkg with the real MMCONF size.
e.g.: On embedded AMD platforms the MMCONF window size is usually
only 64MiB.
Fixes crash on platforms not exposing 256 buses.
Tested on:
* AMD Stoney Ridge
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> Signed-off-by: Marcello Sylvester Bauer <marcello.bauer@9elements.com> Cc: Patrick Rudolph <patrick.rudolph@9elements.com> Cc: Christian Walter <christian.walter@9elements.com> Cc: Maurice Ma <maurice.ma@intel.com> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com> Cc: Benjamin You <benjamin.you@intel.com> Reviewed-by: Guo Dong <guo.dong@intel.com>
MdePkg: PciExpressLib support variable size MMCONF
Add support for arbitrary sized MMCONF by introducing a new PCD.
Add a return value to point out invalid PCI addresses.
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> Signed-off-by: Marcello Sylvester Bauer <marcello.bauer@9elements.com> Cc: Patrick Rudolph <patrick.rudolph@9elements.com> Cc: Christian Walter <christian.walter@9elements.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Patrick Rudolph [Mon, 11 May 2020 12:55:39 +0000 (14:55 +0200)]
UefiPayloadPkg: Store the size of the MMCONF window
Store the real size of the Pcie Memory Mapped Address Space.
This change is necessary to support variable size of MMCONF spaces.
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> Signed-off-by: Marcello Sylvester Bauer <marcello.bauer@9elements.com> Cc: Patrick Rudolph <patrick.rudolph@9elements.com> Cc: Christian Walter <christian.walter@9elements.com> Cc: Maurice Ma <maurice.ma@intel.com> Cc: Guo Dong <guo.dong@intel.com> Cc: Benjamin You <benjamin.you@intel.com> Reviewed-by: Guo Dong <guo.dong@intel.com> Reviewed-by: Maurice Ma <maurice.ma@intel.com>
Matthew Carlson [Tue, 15 Sep 2020 23:02:33 +0000 (07:02 +0800)]
BaseTools: update IASL extdep to more modern version
The IASL extdep is used for CI only and a recent fork of the ACPICA
repo was made to make nuget builds more regular and easier to audit.
https://dev.azure.com/projectmu/_git/acpica
Cc: Bob Feng <bob.c.feng@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Yuwei Chen <yuwei.chen@intel.com> Cc: Sean Brogan <sean.brogan@microsoft.com> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn> Signed-off-by: Matthew Carlson <matthewfcarlson@gmail.com>
MpServices Ppi can be replaced by MpServices2 Ppi and MpServices2
Ppi is mandatory for RegisterCpuFeaturesLib functionality,
basing on this we can drop MpServices Ppi usage from the library
and the constraint that both Ppis must be installed.
Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com> Reviewed-by: Eric Dong <eric.dong@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com>
EmulatorPkg:Change DEC_VERSION to DEC_SPECIFICATION
edk2 DEC specification document only knows about DEC_SPECIFICATION,
so using DEC_VERSION in [Defines] section in EmulatorPkg.dec is not
correct.
Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Andrew Fish <afish@apple.com> Cc: Ray Ni <ray.ni@intel.com> Signed-off-by: Wenyi Xie <xiewenyi2@huawei.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
The BaseSerialPortLib16550 does fallback to a fixed address UART defined
by PcdSerialRegisterBase and does not initialize if it is zero. Do not
assume a serial port at 0x3F8, otherwise it could cause errors during
initialization of a non-existent serial port on non legacy platforms.
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> Signed-off-by: Marcello Sylvester Bauer <marcello.bauer@9elements.com> Reviewed-by: Maurice Ma <maurice.ma@intel.com> Reviewed-by: Guo Dong <guo.dong@intel.com>
EmbeddedPkg/TimeBaseLib: remove the SEC_PER_MONTH, SEC_PER_YEAR macros
The SEC_PER_MONTH and SEC_PER_YEAR macros are wrong: they both evaluate to
0 (of type "int"). They are also unused (they could never be used for
division, for example); so remove them. The macros were originally
introduced in commit 0f4386e775c7 ("ArmPlatformPkg/PL031RealTimeClockLib:
Implement PL031 RTC drive", 2011-06-11).
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> Cc: Leif Lindholm <leif@nuviainc.com> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com> Cc: Philippe Mathieu-Daudé <philmd@redhat.com> Reported-by: Maciej Rabeda <maciej.rabeda@linux.intel.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200904154541.23340-1-lersek@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Leif Lindholm <leif@nuviainc.com>
Disable SHA1 base on the MACRO DISABLE_SHA1_DEPRECATED_INTERFACES.
SHA1 is deprecated function and the MACRO is used to remove the whole
implementation of the SHA1. For the platforms that do not need SHA1
for security, the MACRO should works for DxeImageVerificationLib as
well.
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Min Xu <min.m.xu@intel.com> Cc: Qi Zhang <qi1.zhang@intel.com> Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com> Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2951
Previously false positive Ecc issue whose error code is 10014 was reported
under Linux OS. So we disabled it in EccCheck plugin for edk2 open CI.
As the bug is fixed, we need to revert the change and re-enable it.
Cc: Sean Brogan <sean.brogan@microsoft.com> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
The DxeImageVerificationHandler() function currently checks whether
"SecDataDir" has enough room for "WinCertificate->dwLength". However, for
advancing "OffSet", "WinCertificate->dwLength" is aligned to the next
multiple of 8. If "WinCertificate->dwLength" is large enough, the
alignment will return 0, and "OffSet" will be stuck at the same value.
Check whether "SecDataDir" has room left for both
"WinCertificate->dwLength" and the alignment.
Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Min Xu <min.m.xu@intel.com> Cc: Wenyi Xie <xiewenyi2@huawei.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2215 Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200901091221.20948-4-lersek@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Wenyi Xie <xiewenyi2@huawei.com> Reviewed-by: Min M Xu <min.m.xu@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
SecurityPkg/DxeImageVerificationLib: assign WinCertificate after size check
Currently the (SecDataDirLeft <= sizeof (WIN_CERTIFICATE)) check only
guards the de-referencing of the "WinCertificate" pointer. It does not
guard the calculation of the pointer itself:
This is wrong; if we don't know for sure that we have enough room for a
WIN_CERTIFICATE, then even creating such a pointer, not just
de-referencing it, may invoke undefined behavior.
Move the pointer calculation after the size check.
Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Min Xu <min.m.xu@intel.com> Cc: Wenyi Xie <xiewenyi2@huawei.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2215 Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200901091221.20948-3-lersek@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Wenyi Xie <xiewenyi2@huawei.com> Reviewed-by: Min M Xu <min.m.xu@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
are used multiple times in DxeImageVerificationHandler(). Introduce helper
variables for them: "SecDataDirEnd" and "SecDataDirLeft", respectively.
This saves us multiple calculations and significantly simplifies the code.
Note that all three summands above have type UINT32, therefore the new
variables are also of type UINT32.
This patch does not change behavior.
(Note that the code already handles the case when the
SecDataDir->VirtualAddress + SecDataDir->Size
UINT32 addition overflows -- namely, in that case, the certificate loop is
never entered, and the corruption check right after the loop fires.)
Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Min Xu <min.m.xu@intel.com> Cc: Wenyi Xie <xiewenyi2@huawei.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2215 Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200901091221.20948-2-lersek@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Wenyi Xie <xiewenyi2@huawei.com> Reviewed-by: Min M Xu <min.m.xu@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Zhiguang Liu <zhiguang.liu@intel.com> Signed-off-by: Paul G <paul.grimes@amd.com> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Laszlo Ersek [Wed, 26 Aug 2020 22:21:29 +0000 (00:21 +0200)]
OvmfPkg/CpuHotplugSmm: fix CPU hotplug race just after SMI broadcast
The "virsh setvcpus" (plural) command may hot-plug several VCPUs in quick
succession -- it means a series of "device_add" QEMU monitor commands,
back-to-back.
If a "device_add" occurs *just after* ACPI raises the broadcast SMI, then:
- the CPU_FOREACH() loop in QEMU's ich9_apm_ctrl_changed() cannot make the
SMI pending for the new CPU -- at that time, the new CPU doesn't even
exist yet,
- OVMF will find the new CPU however (in the CPU hotplug register block),
in QemuCpuhpCollectApicIds().
As a result, when the firmware sends an INIT-SIPI-SIPI to the new CPU in
SmbaseRelocate(), expecting it to boot into SMM (due to the pending SMI),
the new CPU instead boots straight into the post-RSM (normal mode) "pen",
skipping its initial SMI handler.
The CPU halts nicely in the pen, but its SMBASE is never relocated, and
the SMRAM message exchange with the BSP falls apart -- the BSP gets stuck
in the following loop:
//
// Wait until the hot-added CPU is just about to execute RSM.
//
while (Context->AboutToLeaveSmm == 0) {
CpuPause ();
}
because the new CPU's initial SMI handler never sets the flag to nonzero.
Fix this by sending a directed SMI to the new CPU just before sending it
the INIT-SIPI-SIPI. The various scenarios are documented in the code --
the cases affected by the patch are documented under point (2).
Note that this is not considered a security patch, as for a malicious
guest OS, the issue is not exploitable -- the symptom is a hang on the
BSP, in the above-noted loop in SmbaseRelocate(). Instead, the patch fixes
behavior for a benign guest OS.
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> Cc: Igor Mammedov <imammedo@redhat.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Philippe Mathieu-Daudé <philmd@redhat.com> Fixes: 51a6fb41181529e4b50ea13377425bda6bb69ba6
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2929 Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200826222129.25798-3-lersek@redhat.com> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
Laszlo Ersek [Wed, 26 Aug 2020 22:21:28 +0000 (00:21 +0200)]
OvmfPkg/CpuHotplugSmm: fix CPU hotplug race just before SMI broadcast
The "virsh setvcpus" (plural) command may hot-plug several VCPUs in quick
succession -- it means a series of "device_add" QEMU monitor commands,
back-to-back.
If a "device_add" occurs *just before* ACPI raises the broadcast SMI,
then:
- OVMF processes the hot-added CPU well.
- However, QEMU's post-SMI ACPI loop -- which clears the pending events
for the hot-added CPUs that were collected before raising the SMI -- is
unaware of the stray CPU. Thus, the pending event is not cleared for it.
As a result of the stuck event, at the next hot-plug, OVMF tries to re-add
(relocate for the 2nd time) the already-known CPU. At that time, the AP is
already in the normal edk2 SMM busy-wait however, so it doesn't respond to
the exchange that the BSP intends to do in SmbaseRelocate(). Thus the VM
gets stuck in SMM.
(Because of the above symptom, this is not considered a security patch; it
doesn't seem exploitable by a malicious guest OS.)
In CpuHotplugMmi(), skip the supposedly hot-added CPU if it's already
known. The post-SMI ACPI loop will clear the pending event for it this
time.
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> Cc: Igor Mammedov <imammedo@redhat.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Philippe Mathieu-Daudé <philmd@redhat.com> Fixes: bc498ac4ca7590479cfd91ad1bb8a36286b0dc21
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2929 Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200826222129.25798-2-lersek@redhat.com> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
Abner Chang [Thu, 6 Aug 2020 02:34:21 +0000 (10:34 +0800)]
EmbeddedPkg/libfdt: Add strncmp macro to use AsciiStrnCmp
For the implementation which utilizes libfdt provided in EmbeddedPkg
however it uses strncmp function in the libfdt helper library,
libfdt_env.h should provide the macro implied with edk2 strncmp
implementation.
The example is RISC-V OpenSBI library. edk2 RISC-V port uses OpenSBI
library and incorporate with edk2 libfdt. edk2 libfdt_env.h provides
the necessary macros to build OpenSBI which uses fdt functions in edk2
environment. However, OpenSBI also has libfdt helper library that uses
strncmp function which is not defined in edk2 libfdt_env.h. This commit
addresses the build issue caused by missing strncmp macro in
libfdt_env.h.
The ICH9_LPC_SMI_F_BROADCAST and ICH9_LPC_SMI_F_CPU_HOTPLUG feature flags
cause QEMU to behave as follows:
BROADCAST CPU_HOTPLUG use case / behavior
--------- ----------- ------------------------------------------------
clear clear OVMF built without SMM_REQUIRE; or very old OVMF
(from before commit a316d7ac91d3 / 2017-02-07).
QEMU permits CPU hotplug operations, and does
not cause the OS to inject an SMI upon hotplug.
Firmware is not expected to be aware of hotplug
events.
clear set Invalid feature set; QEMU rejects the feature
negotiation.
set clear OVMF after a316d7ac91d3 / 2017-02-07, built with
SMM_REQUIRE, but no support for CPU hotplug.
QEMU gracefully refuses hotplug operations.
set set OVMF after a316d7ac91d3 / 2017-02-07, built with
SMM_REQUIRE, and supporting CPU hotplug. QEMU
permits CPU hotplug operations, and causes the
OS to inject an SMI upon hotplug. Firmware is
expected to deal with hotplug events.
Negotiate ICH9_LPC_SMI_F_CPU_HOTPLUG -- but only if SEV is disabled, as
OvmfPkg/CpuHotplugSmm can't deal with SEV yet.
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> Cc: Igor Mammedov <imammedo@redhat.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Liran Alon <liran.alon@oracle.com> Cc: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200714184305.9814-1-lersek@redhat.com> Acked-by: Ard Biesheuvel <ard.biesheuvel@arm.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
MdePkg: UefiFileHandleLib: fix buffer overrun in FileHandleReadLine()
If the size of the supplied buffer in FileHandleReadLine(), module
UefiFileHandleLib.c, was not 0, but was not enough to fit in
the line, the size is increased, and then the Buffer of the new
size is zeroed. This size is always larger than the supplied buffer size,
causing supplied buffer overrun. Fix the issue by using the
supplied buffer size in ZeroMem().
Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Liming Gao <liming.gao@intel.com> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Message-Id: <20200702023113.10517-1-vladimir.olovyannikov@broadcom.com> Reviewed-by: Zhiguang Liu <zhiguang.liu@intel.com>
[lersek@redhat.com: remove stray space character from subject line]
Shenglei Zhang [Fri, 21 Aug 2020 08:26:24 +0000 (16:26 +0800)]
.pytool/EccCheck: Disable Ecc error code 10014 for open CI
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2920
Ecc issues whose error code is 10014, can't be correctly handled
under Linux OS, resulting from a bug in Ecc tool.
So we need to disable it before ecc tool is repaired.
Cc: Sean Brogan <sean.brogan@microsoft.com> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Liming Gao <liming.gao@intel.com> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>