Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Richard Thomaiyar <richard.marian.thomaiyar@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
Instead of having a build time switch to prevent the FDT configuration
table from being installed, make this behavior dependent on whether we
are passing ACPI tables to the OS. This is done by looking for the
ACPI 2.0 configuration table, and only installing the FDT one if the
ACPI one cannot be found.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Defer FDT configuration table installation until ReadyToBoot is signaled.
This allows any driver to make modifications in the mean time, and will
also allow us to defer the decision of whether to install it in the first
place to later on in the boot.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Disable the PL031 RTC DT node unconditionally rather than only when
the DT will be exposed to the OS. This allows us to defer the decision
whether to expose it to the OS to a later time without creating an
additional dependency on the FDT client code by the RTC driver.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
The ACPI specification, up to and including revision 5.1 Errata A, allows
the DSDT and X_DSDT fields to be both set in the FADT. (Obviously, this
only makes sense if the DSDT address is representable in 4 bytes.)
Starting with 5.1 Errata B, specifically for Mantis 1393
<https://mantis.uefi.org/mantis/view.php?id=1393>, the spec requires at
most one of DSDT and X_DSDT to be set to a nonzero value.
MdeModulePkg/AcpiTableDxe handles this mutual exclusion somewhat
inconsistently.
- If the caller of EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() installs the
tables in "DSDT, FADT" order, then we enforce the exclusion between the
DSDT and X_DSDT fields:
DSDT under 4GB FADT.DSDT FADT.X_DSDT [VARIANT B]
-------------- --------- -----------
yes set clear
no clear set
This behavior conforms to 5.1 Errata B. (And it's not required by
earlier versions of the spec.)
- If the caller passes in the tables in "FADT, DSDT" relative order, then
we do not enforce the exclusion:
DSDT under 4GB FADT.DSDT FADT.X_DSDT [VARIANT A]
-------------- --------- -----------
yes set set
no clear set
This satisfies 5.1 Errata A and earlier, but breaks 5.1 Errata B and
later.
Unify the handling of both relative orders. In particular, check the major
and minor version numbers in the FADT. If the FADT version is strictly
before 5.1, then implement [VARIANT A]. If the FADT version is equal to or
larger than 5.1, then implement [VARIANT B].
We make three observations:
- We can't check the FADT table version precisely against "5.1 Errata B";
erratum levels are not captured in the table. We err in the safe
direction, namely we enforce the exclusion for "5.1" and "5.1 Errata A".
- The same applies to "6.0" versus "6.0 Errata A". Because we cannot
distinguish these two, we consider "6.0" to be "equal to or larger than
5.1", and apply [VARIANT B], enforcing the exclusion.
- While a blanket [VARIANT B] would be simpler, there is a significant
benefit to [VARIANT A], under the spec versions that permit it:
compatibility with a wider range of OSPMs (typically, older ones).
For example, Igor reported about a "DELL R430 system with rev4 FADT
where DSDT and X_DSDT are pointing to the same address". Michael also
reported about several systems that exhibit the same.
Regression tested with the following KVM guests (QEMU built at
ata0def594286d, "Merge remote-tracking branch
'remotes/bonzini/tags/for-upstream' into staging", 2017-01-30):
- OVMF: boot and S3 suspend/resume
- Ia32, Q35, SMM
- Fedlet 20141209
- Ia32X64, Q35, SMM
- Fedora 22
- Windows 7
- Windows 8.1
- Windows 10
- Windows Server 2008 R2
- Windows Server 2012 R2
- Windows Server 2016 Tech Preview 4
- X64, I440FX, no SMM
- Fedora 24
- RHEL-6.7
- RHEL-7.2-ish
- ArmVirtQemu: boot test with virtio-gpu
- AARCH64
- Fedora 24
- RHELSA-7.3
- openSUSE Tumbleweed (4.8.4-based)
This change is connected to ASWG ticket
<https://mantis.uefi.org/mantis/view.php?id=1757>, which is now
closed/fixed.
Cc: Al Stone <ahs3@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Leo Duran <leo.duran@amd.com>
Cc: Michael Tsirkin <mtsirkin@redhat.com>
Cc: Phil Dennis-Jordan <phil@philjordan.eu>
Cc: Star Zeng <star.zeng@intel.com>
Reported-by: Phil Dennis-Jordan <phil@philjordan.eu>
Suggested-by: Igor Mammedov <imammedo@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
This patch incurs no functional changes, it just removes some whitespace,
and also makes sure we always assign
AcpiTableInstance->Fadt3->Dsdt
first, and
AcpiTableInstance->Fadt3->XDsdt
second. The goal is to separate the syntactic changes from the functional
changes implemented by the next patch.
Cc: Al Stone <ahs3@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Leo Duran <leo.duran@amd.com>
Cc: Michael Tsirkin <mtsirkin@redhat.com>
Cc: Phil Dennis-Jordan <phil@philjordan.eu>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Fix the issue of after installing a package the Event information is duplicated. The tool checks if the EVENT information existing in UserExtension or not. If already existing in UserExtension the tool will not add additional information.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hess Chen <hesheng.chen@intel.com>
Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>
Add a checkpoint for UNI file which is missing '"' at the end of a line.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hess Chen <hesheng.chen@intel.com>
Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>
The original solution of getting parameter name is to skip "_" but
now it doesn't ignore "_" anymore.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hess Chen <hesheng.chen@intel.com>
Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>
Some memory attributes are implied by the memory type, e.g., device memory
is always mapped non-executable and cached memory should have the inner
shareable attribute.
In order to prevent unnecessary memory attribute updates of mappings
created early on, make EfiAttributeToArmAttribute() return these implied
attributes in the same way as ArmMmuLib does already. This avoids false
positives when looking for differences between current and desired mapping
attributes.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Include DXE_CORE in the BuildOptions that are set to force 4 KB section
alignment for PE/COFF images in order to allow them to be mapped with
strict memory permissions.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
The primary use case for UncachedMemoryAllocationLib is non-coherent DMA,
which implies that such regions are not used to fetch instructions from.
So let's map them as non-executable, to avoid creating a security hole
when the rest of the platform may be enforcing strict memory permissions
on ordinary allocations.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Uncached pool allocations are aligned to the data cache line length under
the assumption that this is sufficient to prevent cache maintenance from
corrupting adjacent allocations. However, the value to use in such cases
is architecturally called the Cache Writeback Granule (CWG), which is
essentially the maximum Dcache line length rather than the minimum.
Note that this is mostly a cosmetical fix, given that the pool allocation
is turned into a page allocation later, and rounded up accordingly.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
In order to play nice with platforms that use strict memory permission
policies, restore the original mapping attributes when freeing uncached
allocations.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Since shell has consumed the APIs in UefiBootManagerLib.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
The commit is a follow-up of commit 8491e30.
In file MpService.c line 786:
Pte[Index] = (UINT64)((UINTN)PageTable + EFI_PAGE_SIZE * (Index + 1)) |
mAddressEncMask ...
(Where PageTable is of type VOID*, Index is of type UINTN, mAddressEncMask
is of type UINT64 and Pte[Index] is of type UINT64.)
Since in this case, the code logic ensures that the expression will not
exceed the range of UINTN, the commit will remove the explicit type cast
'(UINT64)'.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Jeff Fan <jeff.fan@intel.com>
Now that ARM has grown support for managing memory permissions in
ArmMmuLib, we can enable the non-executable DXE stack for all virt
platforms. Note that this includes the AARCH64 Xen platform as well.
Note that this is not [entirely] redundant: the non-executable stack
is configured before DxeCore is invoked. The image and memory protection
features configured during DXE only take affect when the CPU arch
protocol implementation is registered.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Now that we have the prerequisite functionality available in ArmMmuLib,
wire it up into ArmSetMemoryRegionNoExec, ArmClearMemoryRegionNoExec,
ArmSetMemoryRegionReadOnly and ArmClearMemoryRegionReadOnly. This is
used by the non-executable stack feature that is configured by DxeIpl.
NOTE: The current implementation will not combine RO and XP attributes,
i.e., setting/clearing a region no-exec will unconditionally
clear the read-only attribute, and vice versa. Currently, we
only use ArmSetMemoryRegionNoExec(), so for now, we should be
able to live with this.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
We no longer make use of the ArmMmuLib 'feature' to create aliased
memory ranges with mismatched attributes, and in fact, it was only
wired up in the ARM version to begin with.
So remove the VirtualMask argument from ArmSetMemoryAttributes()'s
prototype, and remove the dead code that referred to it.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
... where it belongs, since AARCH64 already keeps it there, and
non DXE users of ArmMmuLib (such as DxeIpl, for the non-executable
stack) may need its functionality as well.
While at it, rename SetMemoryAttributes to ArmSetMemoryAttributes,
and make any functions that are not exported STATIC. Also, replace
an explicit gBS->AllocatePages() call [which is DXE specific] with
MemoryAllocationLib::AllocatePages().
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
The routines ArmConfigureMmu(), SetMemoryAttributes() [*] and the
various set/clear read-only/no-exec routines are declared as returning
EFI_STATUS in the respective header files, so align the definitions with
that.
* SetMemoryAttributes() is declared in the wrong header (and defined in
ArmMmuLib for AARCH64 and in CpuDxe for ARM)
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Like for AARCH64, enable PE/COFF image and NX memory protection for all
32-bit ARM virt platforms.
Note that this does not [yet] protect EfiLoaderData regions, due to
compatibility issues with GRUB.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Enable the use of strict memory permissions on ARM by processing the
EFI_MEMORY_RO and EFI_MEMORY_XP rather than ignoring them. As before,
calls to CpuArchProtocol::SetMemoryAttributes that only set RO/XP
bits will preserve the cacheability attributes. Permissions attributes
are not preserved when setting the memory type only: the way the memory
permission attributes are defined does not allows for that, and so this
situation does not deviate from other architectures.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Page and section entries in the page tables are updated using the
helper ArmUpdateTranslationTableEntry(), which cleans the page
table entry to the PoC, and invalidates the TLB entry covering
the page described by the entry being updated.
Since we may be updating section entries, we might be leaving stale
TLB entries at this point (for all pages in the section except the
first one), which will be invalidated wholesale at the end of
SetMemoryAttributes(). At that point, all caches are cleaned *and*
invalidated as well.
This cache maintenance is costly and unnecessary. The TLB maintenance
is only necessary if we updated any section entries, since any page
by page entries that have been updated will have been invalidated
individually by ArmUpdateTranslationTableEntry().
So drop the clean/invalidate of the caches, and only perform the
full TLB flush if UpdateSectionEntries() was called, or if sections
were split by UpdatePageEntries(). Finally, make the cache maintenance
on the remapped regions themselves conditional on whether any memory
type attributes were modified.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Currently, any range passed to CpuArchProtocol::SetMemoryAttributes is
fully broken down into page mappings if the start or the size of the
region happens to be misaliged relative to the section size of 1 MB.
This is going to result in memory being wasted on second level page tables
when we enable strict memory permissions, given that we remap the entire
RAM space non-executable (modulo the code bits) when the CpuArchProtocol
is installed.
So refactor the code to iterate over the range in a way that ensures
that all naturally aligned section sized subregions are not broken up.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
The change doesn't impact the functionality.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
https://bugzilla.tianocore.org/show_bug.cgi?id=417
The commit makes the following refinements in VolInfo source codes to
avoid VS2010/VS2012 build failure:
1. Refines coding style for function 'CombinePath' to declare local
variables at the beginning of the function block.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>
https://bugzilla.tianocore.org/show_bug.cgi?id=417
The commit makes the following refinements in GenVtf source codes to
avoid VS2010/VS2012 build failure:
1. Refines coding style to declare local variables at the beginning of a
code block in function 'main'.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>
https://bugzilla.tianocore.org/show_bug.cgi?id=417
The commit makes the following refinements in GenFw source codes to
avoid VS2010/VS2012 build failure:
1. Replaces the uses of 'bool' with 'BOOLEAN' for accordance, and remove
the header file dependency for '<stdbool.h>'.
2. Refines coding style for function 'GetSymName' to declare local
variables at the beginning of the function block.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>
Architectures such as AArch64 may run the OS with 16 KB or 64 KB sized
pages, and for this reason, the UEFI spec mandates a minimal allocation
granularity of 64 KB for regions that may require different memory
attributes at OS runtime.
So make PeiCore's implementation of AllocatePages () take this into
account as well.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
Remove the local definitions for the default and runtime page allocation
granularity macros, and switch to the new MdePkg versions.
Note that this replaces a reference to the 'default' version with the
more correct 'runtime' version, but this matters little in practice.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
Remove the local definitions for the default and runtime page allocation
granularity macros, and switch to the new MdePkg versions.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
The UEFI spec differs between architectures in the minimum alignment
and granularity of page allocations that are visible to the OS as
EFI_MEMORY_RUNTIME regions.
So define macros that carry these values to the respective ProcessorBind.h
header files.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
This PCD holds the address mask for page table entries when memory
encryption is enabled on AMD processors supporting the Secure Encrypted
Virtualization (SEV) feature.
The mask is applied when page tables entries are created or modified.
CC: Jeff Fan <jeff.fan@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leo Duran <leo.duran@amd.com>
Reviewed-by: Jeff Fan <jeff.fan@intel.com>
The patch doesn't impact the functionality.
The rename also fixes the inconsistency between function
header comments and function parameters.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
There are cases that the operands of an expression are all with rank less
than UINT64/INT64 and the result of the expression is explicitly cast to
UINT64/INT64 to fit the target size.
An example will be:
UINT32 a,b;
// a and b can be any unsigned int type with rank less than UINT64, like
// UINT8, UINT16, etc.
UINT64 c;
c = (UINT64) (a + b);
Some static code checkers may warn that the expression result might
overflow within the rank of "int" (integer promotions) and the result is
then cast to a bigger size.
The commit refines codes by the following rules:
1). When the expression is possible to overflow the range of unsigned int/
int:
c = (UINT64)a + b;
2). When the expression will not overflow within the rank of "int", remove
the explicit type casts:
c = a + b;
3). When the expression will be cast to pointer of possible greater size:
UINT32 a,b;
VOID *c;
c = (VOID *)(UINTN)(a + b); --> c = (VOID *)((UINTN)a + b);
4). When one side of a comparison expression contains only operands with
rank less than UINT32:
UINT8 a;
UINT16 b;
UINTN c;
if ((UINTN)(a + b) > c) {...} --> if (((UINT32)a + b) > c) {...}
For rule 4), if we remove the 'UINTN' type cast like:
if (a + b > c) {...}
The VS compiler will complain with warning C4018 (signed/unsigned
mismatch, level 3 warning) due to promoting 'a + b' to type 'int'.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Jeff Fan <jeff.fan@intel.com>
There are cases that the operands of an expression are all with rank less
than UINT64/INT64 and the result of the expression is explicitly cast to
UINT64/INT64 to fit the target size.
An example will be:
UINT32 a,b;
// a and b can be any unsigned int type with rank less than UINT64, like
// UINT8, UINT16, etc.
UINT64 c;
c = (UINT64) (a + b);
Some static code checkers may warn that the expression result might
overflow within the rank of "int" (integer promotions) and the result is
then cast to a bigger size.
The commit refines codes by the following rules:
1). When the expression is possible to overflow the range of unsigned int/
int:
c = (UINT64)a + b;
2). When the expression will not overflow within the rank of "int", remove
the explicit type casts:
c = a + b;
3). When the expression will be cast to pointer of possible greater size:
UINT32 a,b;
VOID *c;
c = (VOID *)(UINTN)(a + b); --> c = (VOID *)((UINTN)a + b);
4). When one side of a comparison expression contains only operands with
rank less than UINT32:
UINT8 a;
UINT16 b;
UINTN c;
if ((UINTN)(a + b) > c) {...} --> if (((UINT32)a + b) > c) {...}
For rule 4), if we remove the 'UINTN' type cast like:
if (a + b > c) {...}
The VS compiler will complain with warning C4018 (signed/unsigned
mismatch, level 3 warning) due to promoting 'a + b' to type 'int'.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Jeff Fan <jeff.fan@intel.com>
There are cases that the operands of an expression are all with rank less
than UINT64/INT64 and the result of the expression is explicitly cast to
UINT64/INT64 to fit the target size.
An example will be:
UINT32 a,b;
// a and b can be any unsigned int type with rank less than UINT64, like
// UINT8, UINT16, etc.
UINT64 c;
c = (UINT64) (a + b);
Some static code checkers may warn that the expression result might
overflow within the rank of "int" (integer promotions) and the result is
then cast to a bigger size.
The commit refines codes by the following rules:
1). When the expression is possible to overflow the range of unsigned int/
int:
c = (UINT64)a + b;
2). When the expression will not overflow within the rank of "int", remove
the explicit type casts:
c = a + b;
3). When the expression will be cast to pointer of possible greater size:
UINT32 a,b;
VOID *c;
c = (VOID *)(UINTN)(a + b); --> c = (VOID *)((UINTN)a + b);
4). When one side of a comparison expression contains only operands with
rank less than UINT32:
UINT8 a;
UINT16 b;
UINTN c;
if ((UINTN)(a + b) > c) {...} --> if (((UINT32)a + b) > c) {...}
For rule 4), if we remove the 'UINTN' type cast like:
if (a + b > c) {...}
The VS compiler will complain with warning C4018 (signed/unsigned
mismatch, level 3 warning) due to promoting 'a + b' to type 'int'.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
There are cases that the operands of an expression are all with rank less
than UINT64/INT64 and the result of the expression is explicitly cast to
UINT64/INT64 to fit the target size.
An example will be:
UINT32 a,b;
// a and b can be any unsigned int type with rank less than UINT64, like
// UINT8, UINT16, etc.
UINT64 c;
c = (UINT64) (a + b);
Some static code checkers may warn that the expression result might
overflow within the rank of "int" (integer promotions) and the result is
then cast to a bigger size.
The commit refines codes by the following rules:
1). When the expression is possible to overflow the range of unsigned int/
int:
c = (UINT64)a + b;
2). When the expression will not overflow within the rank of "int", remove
the explicit type casts:
c = a + b;
3). When the expression will be cast to pointer of possible greater size:
UINT32 a,b;
VOID *c;
c = (VOID *)(UINTN)(a + b); --> c = (VOID *)((UINTN)a + b);
4). When one side of a comparison expression contains only operands with
rank less than UINT32:
UINT8 a;
UINT16 b;
UINTN c;
if ((UINTN)(a + b) > c) {...} --> if (((UINT32)a + b) > c) {...}
For rule 4), if we remove the 'UINTN' type cast like:
if (a + b > c) {...}
The VS compiler will complain with warning C4018 (signed/unsigned
mismatch, level 3 warning) due to promoting 'a + b' to type 'int'.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
There are cases that the operands of an expression are all with rank less
than UINT64/INT64 and the result of the expression is explicitly cast to
UINT64/INT64 to fit the target size.
An example will be:
UINT32 a,b;
// a and b can be any unsigned int type with rank less than UINT64, like
// UINT8, UINT16, etc.
UINT64 c;
c = (UINT64) (a + b);
Some static code checkers may warn that the expression result might
overflow within the rank of "int" (integer promotions) and the result is
then cast to a bigger size.
The commit refines codes by the following rules:
1). When the expression is possible to overflow the range of unsigned int/
int:
c = (UINT64)a + b;
2). When the expression will not overflow within the rank of "int", remove
the explicit type casts:
c = a + b;
3). When the expression will be cast to pointer of possible greater size:
UINT32 a,b;
VOID *c;
c = (VOID *)(UINTN)(a + b); --> c = (VOID *)((UINTN)a + b);
4). When one side of a comparison expression contains only operands with
rank less than UINT32:
UINT8 a;
UINT16 b;
UINTN c;
if ((UINTN)(a + b) > c) {...} --> if (((UINT32)a + b) > c) {...}
For rule 4), if we remove the 'UINTN' type cast like:
if (a + b > c) {...}
The VS compiler will complain with warning C4018 (signed/unsigned
mismatch, level 3 warning) due to promoting 'a + b' to type 'int'.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
There are cases that the operands of an expression are all with rank less
than UINT64/INT64 and the result of the expression is explicitly cast to
UINT64/INT64 to fit the target size.
An example will be:
UINT32 a,b;
// a and b can be any unsigned int type with rank less than UINT64, like
// UINT8, UINT16, etc.
UINT64 c;
c = (UINT64) (a + b);
Some static code checkers may warn that the expression result might
overflow within the rank of "int" (integer promotions) and the result is
then cast to a bigger size.
The commit refines codes by the following rules:
1). When the expression is possible to overflow the range of unsigned int/
int:
c = (UINT64)a + b;
2). When the expression will not overflow within the rank of "int", remove
the explicit type casts:
c = a + b;
3). When the expression will be cast to pointer of possible greater size:
UINT32 a,b;
VOID *c;
c = (VOID *)(UINTN)(a + b); --> c = (VOID *)((UINTN)a + b);
4). When one side of a comparison expression contains only operands with
rank less than UINT32:
UINT8 a;
UINT16 b;
UINTN c;
if ((UINTN)(a + b) > c) {...} --> if (((UINT32)a + b) > c) {...}
For rule 4), if we remove the 'UINTN' type cast like:
if (a + b > c) {...}
The VS compiler will complain with warning C4018 (signed/unsigned
mismatch, level 3 warning) due to promoting 'a + b' to type 'int'.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>
There are cases that the operands of an expression are all with rank less
than UINT64/INT64 and the result of the expression is explicitly cast to
UINT64/INT64 to fit the target size.
An example will be:
UINT32 a,b;
// a and b can be any unsigned int type with rank less than UINT64, like
// UINT8, UINT16, etc.
UINT64 c;
c = (UINT64) (a + b);
Some static code checkers may warn that the expression result might
overflow within the rank of "int" (integer promotions) and the result is
then cast to a bigger size.
The commit refines codes by the following rules:
1). When the expression is possible to overflow the range of unsigned int/
int:
c = (UINT64)a + b;
2). When the expression will not overflow within the rank of "int", remove
the explicit type casts:
c = a + b;
3). When the expression will be cast to pointer of possible greater size:
UINT32 a,b;
VOID *c;
c = (VOID *)(UINTN)(a + b); --> c = (VOID *)((UINTN)a + b);
4). When one side of a comparison expression contains only operands with
rank less than UINT32:
UINT8 a;
UINT16 b;
UINTN c;
if ((UINTN)(a + b) > c) {...} --> if (((UINT32)a + b) > c) {...}
For rule 4), if we remove the 'UINTN' type cast like:
if (a + b > c) {...}
The VS compiler will complain with warning C4018 (signed/unsigned
mismatch, level 3 warning) due to promoting 'a + b' to type 'int'.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Jiewen Yao
Ruiyu Ni
Ard Biesheuvel
Laszlo Ersek
Hess Chen
Dandan Bi
Hao Wu
Leo Duran