Update GenericWatchdogDxe to disable watchdog interaction after exiting
boot services. Also, move the mEfiExitBootServicesEvent event to the top
of the file with the other static variables.
Signed-off-by: Rebecca Cran <rebecca@os.amperecomputing.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
The calculation of the timer period was broken. Introduce a global
mTimerPeriod so the calculation can be removed. Since mTimerFrequencyHz
is only used in one place, remove the global and make it a local
variable. Do the same with mNumTimerTicks.
Signed-off-by: Rebecca Cran <rebecca@os.amperecomputing.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
The generic watchdog offset register is 48 bits wide, and can be set by
performing two 32-bit writes.
Add support for writing the high 16 bits of the offset register and
update the signature of the WatchdogWriteOffsetRegister function to take
a UINT64 value.
Signed-off-by: Rebecca Cran <rebecca@os.amperecomputing.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
The PERFORMANCE_DESCRIBE_FASTCHANNEL Scmi command is available
since SCMI v2.0 and allows to query information about the supported
fast-channels of the Scmi performance protocol.
Add support for this command.
Also move SCMI_MESSAGE_ID_PERFORMANCE enum definition up in the file
to use it in SCMI_PERFORMANCE_DESCRIBE_FASTCHANNEL function
declaration.
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
GetNextEntryAttribute() is currently applying a 64-bit mask
(TT_ATTRIBUTES_MASK) to a 32-bit descriptor value (EntryType).
The original descriptor was 64 bits containing the upper and
lower attributes which are included in TT_ATTRIBUTES_MASK.
The PrevEntryAttribute parameter is also a UINT32, but passed to
PageAttributeToGcdAttribute() for a UINT64 parameter where the
function checks masks in the upper 32 bits of the integer value:
PageAttributeToGcdAttribute (*PrevEntryAttribute)
...
STATIC
UINT64
PageAttributeToGcdAttribute (
IN UINT64 PageAttributes
)
...
if ((PageAttributes & (TT_PXN_MASK | TT_UXN_MASK)) != 0) {
GcdAttributes |= EFI_MEMORY_XP;
}
...
#define TT_PXN_MASK BIT53
#define TT_UXN_MASK BIT54 // EL1&0
This change removes UINT32 intermediary values. For EntryType,
eliminating an unncessary cast. For EntryAttribute, preserving the
upper and lower attributes for evaluation in
PageAttributeToGcdAttribute().
This also resolves the following compiler warning previously present
on Visual Studio for the assignment to the previously 32-bit local
variables.
'=': conversion from 'UINT64' to 'UINT32', possible loss of data
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
Add EFI_NOT_READY return if the CPU can not be enabled because the
processor is already on.
This can occur in normal use if the CPU is still being turned off from
a previous call when this is called again.
Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
Much of the MMU logic was written without function headers. This patch
adds function headers where absent and updates function headers which
do not match the EDK2 standard.
Signed-off-by: Taylor Beebe <t@taylorbeebe.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
There are ASSERTs present in the MMU logic to ensure various
functions return successfully, but these ASSERTs may be ignored
on release builds causing unsafe behavior. This patch updates
the logic to handle unexpected return values and branch safely.
Signed-off-by: Taylor Beebe <t@taylorbeebe.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
This patch updates the GetMemoryRegion() function to handle the case
where there is no mapping for the requested address.
The original logic for the ARM would hit an ASSERT after
GetMemoryRegionPage() returned EFI_SUCCESS but did not update The
RegionLength parameter.
The original logic for the AARCH64 would never initialize the
RegionLength parameter to zero and return EFI_SUCCESS after
traversing an unknown number of pages.
To fix this, update the logic for both architecture to return
EFI_NO_MAPPING if the BaseAddress being checked is unmapped.
Signed-off-by: Taylor Beebe <t@taylorbeebe.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
This patch applies Uncrustify to the following files:
ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.c
ArmPkg/Include/IndustryStandard/ArmStdSmc.h
Signed-off-by: Taylor Beebe <t@taylorbeebe.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4464
This change introduced the MM communicate support in PEI phase for ARM
based platforms. Similar to the DXE counterpart, `PcdMmBufferBase` is
used as communicate buffer and SMC will be invoked to communicate to
TrustZone when MMI is requested.
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Co-authored-by: Ronny Hansen <hansen.ronny@microsoft.com>
Co-authored-by: Shriram Masanamuthu Chinnathurai <shriramma@microsoft.com>
Co-authored-by: Preshit Harlikar <pharlikar@microsoft.com>
Signed-off-by: Kun Qin <kuqin@microsoft.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
Now that we have a sane API to set and clear memory permissions that
works the same on ARM and AArch64, we no longer have a need for the
individual set/clear no-access/read-only/no-exec helpers so let's drop
them.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Now that ArmSetMemoryAttributes() permits a mask to be provided, we can
simplify the implementation the UEFI memory attribute protocol
substantially, and just pass on the requested mask to be set or cleared
directly.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>
Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
Implement the newly defined PPI that permits the PEI core and DXE IPL to
manage memory permissions on ranges of DRAM, for doing things like
mapping the stack non-executable, or granting executable permissions to
shadowed PEIMs.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>
Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
Currently, ArmSetMemoryAttributes () takes a combination of
EFI_MEMORY_xx constants describing the memory type and permission
attributes that should be set on a region of memory. In cases where the
memory type is omitted, we assume that the memory permissions being set
are final, and that existing memory permissions can be discarded.
This is problematic, because we aim to map memory non-executable
(EFI_MEMORY_XP) by default, and only relax this requirement for code
regions that are mapped read-only (EFI_MEMORY_RO). Currently, setting
one permission clears the other, and so code managing these permissions
has to be aware of the existing permissions in order to be able to
preserve them, and this is not always tractable (e.g., the UEFI memory
attribute protocol implements an abstraction that promises to preserve
memory permissions that it is not operating on explicitly).
So let's add an AttributeMask parameter to ArmSetMemoryAttributes(),
which is permitted to be non-zero if no memory type is being provided,
in which case only memory permission attributes covered in the mask will
be affected by the update.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>
Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
The ArmGicAcknowledgeInterrupt () returns the value returned by the
Interrupt Acknowledge Register and the InterruptID separately in an out
parameter.
The function documents the following: 'InterruptId is returned
separately from the register value because in the GICv2 the register
value contains the CpuId and InterruptId while in the GICv3 the register
value is only the InterruptId.'
This function skips setting the InterruptId in the out parameter for
GICv3. Although the return value from the function is the InterruptId
for GICv3, this breaks the function usage model as the caller expects
the InterruptId in the out parameter for the function. e.g. The caller
may end up using the InterruptID which could potentially be an
uninitialised variable value.
Therefore, set the InterruptID in the function out parameter for GICv3
as well.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
According to the GIC architecture version 3 and 4 specification, the
maximum number of INTID bits supported in the CPU interface is 24.
Considering this the RegShift variable is not required to be more than 8
bits. Therefore, make the RegShift variable type to UINT8. Also add
necessary typecasts when calculating the RegOffset and RegShift values.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
GICD_SGIR is a 32-bit register, of which INTID is bits [3:0] and Bits
[14:4] is RES0. Since SgiId parameter in the function ArmGicSendSgiTo ()
is UINT8, mask unused bits of SgiId before writing to the GICD_SGIR
register to prevent accidental setting of the RES0 bits.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
The IrqInterruptHandler () and ExitBootServicesEvent () function
declarations were unused. Therefore, remove these declarations.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
The EIOR register of the Gic CPU interface is a 32 bit register.
However, the HARDWARE_INTERRUPT_SOURCE used to represent the interrupt
source (Interrupt ID) is typedefed as UINTN, see
EmbeddedPkg\Include\Protocol\HardwareInterrupt.h
Therfore, typecast the interrupt ID (Source) value to UINT32 before
setting the EOIR register. Also, add an assert to check that the value
does not exceed 32 bits.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Although the maximum interrupt ID on GicV2 is 10bit and for GicV3/4 is
24bit, and that the IAR and EOIR registers of the Gic CPU interface are
32 bit; the typedef HARDWARE_INTERRUPT_SOURCE is defined as UINTN in
EmbeddedPkg\Include\Protocol\HardwareInterrupt.h
Therefore, use UINTN for Gic Interrupt variables and use appropriate
typecasts wherever needed.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
The CPU Interface Identification Register (GICC_IIDR) is a 32-bit
register. Since ArmGicGetInterfaceIdentification () returns the value
read from the GICC_IIDR register, update the return type for this
function to UINT32.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
According to edk2 coding standard specification, Non-Boolean comparisons
must use a compare operator (==, !=, >, < >=, <=). See Section 5.7.2.1
at https://edk2-docs.gitbook.io/
edk-ii-c-coding-standards-specification/5_source_files/ 57_c_programming
Therefore, fix the comparison in ArmGicEnableDistributor()
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
The Software Generated Interrupt Register (GICD_SGIR) is a 32 bit
register with the following bit assignment:
TargetListFilter, bits [25:24]
CPUTargetList, bits [23:16]
NSATT, bit [15]
SGIINTID, bits [3:0]
Therefore, modify the TargetListFilter, CPUTargetList, SGI Interrupt ID
parameters of the ArmGicSendSgiTo () to use UINT8 instead of INTN.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
The data type used by variables representing the
GicInterruptInterfaceBase has been inconsistently used in the ArmGic
driver and the library. The PCD defined for the GIC Interrupt interface
base address is UINT64. However, the data types for the variables used
is UINTN, INTN, and at some places UINT32.
Therefore, update the data types to use UINTN and add necessary
typecasts when reading values from the PCD. This should then be
consistent across AArch32 and AArch64 builds.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
The data type used by variables representing the GicDistributorBase has
been inconsistently used in the ArmGic driver and the library. The PCD
defined for the GIC Distributor base address is UINT64. However, the
data types for the variables used is UINTN, INTN, and at some places
UINT32.
Therefore, update the data types to use UINTN and add necessary
typecasts when reading values from the PCD. This should then be
consistent across AArch32 and AArch64 builds.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
The GIC v2 base addresses can be 64bit, don't limit to 32 on 64bit
machines.
Signed-off-by: Neil Jones <neil.jones@blaize.com>
Reviewed-by: Pedro Falcato <pedro.falcato@gmail.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
The DXE core implementation of PcdDxeNxMemoryProtectionPolicy already
contains an assertion that EfiConventionalMemory and EfiBootServicesData
are subjected to the same policy when it comes to the use of NX
permissions. The reason for this is that we may otherwise end up with
unbounded recursion in the page table code, given that allocating a page
table would then involve a permission attribute change, and this could
result in the need for a block entry to be split, which would trigger
the allocation of a page table recursively.
For the same reason, a shortcut exists in ApplyMemoryProtectionPolicy()
where, instead of setting the memory attributes unconditionally, we
compare the NX policies and avoid touching the page tables if they are
the same for the old and the new memory types. Without this shortcut, we
may end up in a situation where, as the CPU arch protocol DXE driver is
ramping up, the same unbounded recursion is triggered, due to the fact
that the NX policy for EfiConventionalMemory has not been applied yet.
To break this cycle, let's remap all EfiConventionalMemory regions
according to the NX policy for EfiBootServicesData before exposing the
CPU arch protocol to the DXE core and other drivers. This ensures that
creating EfiBootServicesData allocations does not result in memory
attribute changes, and therefore no recursion.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4463
When the AARCH64 CpuDxe attempts to SyncCacheConfig() with the GCD, it
collects the page attributes as:
EntryAttribute = Entry & TT_ATTR_INDX_MASK
However, TT_ATTR_INDX_MASK only masks the cacheability attributes and
drops the memory protections attributes. Importantly, it also drops the
TT_AF (access flag) which is now wired up in EDK2 to represent
EFI_MEMORY_RP, so by default all SystemMem pages will report as
EFI_MEMORY_RP to the GCD. The GCD currently drops that silently, because
the Capabilities field in the GCD does not support EFI_MEMORY_RP by
default.
However, some ranges may support EFI_MEMORY_RP and incorrectly mark
those ranges as read protected. In conjunction with another change on
the mailing list (see: https://edk2.groups.io/g/devel/topic/98505340),
this causes an access flag fault incorrectly. See the linked BZ below
for full details.
This patch exposes all memory protections attributes to the GCD layer so
it can correctly set pages as EFI_MEMORY[RP|XP|RO] when it initially
syncs.
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Taylor Beebe <t@taylorbeebe.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Signed-off-by: Oliver Smith-Denny <osde@linux.microsoft.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
__FUNCTION__ is a pre-standard extension that gcc and Visual C++ among
others support, while __func__ was standardized in C99.
Since it's more standard, replace __FUNCTION__ with __func__ throughout
ArmPkg.
Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
Expose the protocol introduced in v2.10 that permits the caller to
manage mapping permissions in the page tables.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
In preparation for introducing an implementation of the EFI memory
attributes protocol that is shared between ARM and AArch64, unify the
existing code that converts a page table descriptor into a
EFI_MEMORY_xx bitfield, so it can be called from the generic code.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Implement support for read-protected memory by wiring it up to the
access flag in the page table descriptor. The resulting mapping is
implicitly non-writable and non-executable as well, but this is good
enough for implementing this attribute, as we never rely on write or
execute permissions without read permissions.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Split the ARM permission fields in the short descriptors into an access
flag and AP[2:1] as per the recommendation in the ARM ARM. This makes
the access flag available separately, which allows us to implement
EFI_MEMORY_RP memory analogous to how it will be implemented for
AArch64.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
The section-to-page attribute conversion takes the shareability and
execute-never attributes into account, whereas the page-to-section
counterpart does not. The result is that GetMemoryRegionPage () -which
takes a section attribute argument (via *RegionAttributes) that is
ostensibly based on the first page in the range, but differs from the
actual page attributes when converted back- may return with a
RegionLength of zero. This is incorrect, and confuses code that scans a
region by calling GetMemoryRegion () in sequence.
So fix the conversion, and ASSERT () on a non-zero region length.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Large page support on 32-bit ARM is essentially a glorified contiguous
bit where 16 consecutive entries describing a contiguous range with the
same attributes are presented in a way that permits the TLB to cache its
translation with a single entry.
This was never wired up completely, and does not add a lot of value in
EFI, where the page granularity is 4k and we expect to be able to set RO
and XP permissions on individual pages.
Given that large page support complicates the handling of the XN bit at
the page level (which is in a different place depending on whether the
page is small or large), let's just rip it out.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
As per the SCMI specification, section CLOCK_DESCRIBE_RATES mentions
that the value of num_rates_flags[11:0] in the response must be 3 if
the return format is the triplet. Due to the buggy firmware, this was
not noticed for long time. The firmware is now fixed resulting in
ClockDescribeRates() to fail with "Buffer Too Small" error as the
RequiredArraySize gets miscalculated as 72 instead of 24.
Fix the issue by reusing the logic for both the return format which
must work if num_rates_flags has correct value as expected from the
specification.
Cc: Girish Pathak <girish.pathak@arm.com>
Cc: Jeff Brasen <jbrasen@nvidia.com>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
Tested-by: Pierre Gondois <pierre.gondois@arm.com>
Reported-by: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Tested-by: Sami Mujawar <sami.mujawar@arm.com>
Add support for EFI_MP_SERVICES_PROTOCOL during the DXE phase under
AArch64.
PSCI_CPU_ON is called to power on the core, the supplied procedure is
executed and PSCI_CPU_OFF is called to power off the core.
Fixes contributed by Ard Biesheuvel.
Signed-off-by: Rebecca Cran <rebecca@quicinc.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
Tested-by: Kun Qin <kun.qin@microsoft.com>
In an effort to clean the documentation of the above
package, remove duplicated words, and fix a typo while at it.
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
Reviewed-by: Sami Mujawar <sami.muajwar@arm.com>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
The ARM_PROCESSOR_TABLE pseudo-ACPI table (which carries a ACPI-table
like header but is published as a EFI config table) is not described in
any relevant spec, and is not known to be relied upon by any OS. Let's
just get rid of it.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Tested-by: Sami Mujawar <sami.mujawar@arm.com>
The issue appears to have been introduced by:
41fb5d46 : ArmPkg/ArmGic: Use the GIC Redistributor instead of GIC Distributor for GICv3
The changes to ArmGicIsInterruptEnabled() introduced the error where the Boolean
result is assigned to Interrupts, but then the bit position check is performed
again (against the computed Boolean result instead of the interrupt mask) during
the return statement.
Fix removes erroneous test and relies on boolean test made at return.
Signed-off-by: Robbie King <robbiek@xsightlabs.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
RVCT is obsolete and no longer used.
Remove support for it.
Signed-off-by: Rebecca Cran <quic_rcran@quicinc.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
We never run any code at EL0, and so it would seem that any access
permissions set for EL0 (via the AP[1] attribute in the page tables) are
irrelevant. We currently set EL0 and EL1 permissions to the same value
arbitrarily.
However, this causes problems on hardware like the Apple M1 running the
MacOS hypervisor framework, which enters EL1 with SCTLR_EL1.SPAN
enabled, causing the Privileged Access Never (PAN) feature to be enabled
on any exception taken to EL1, including the IRQ exceptions that handle
our timer interrupt. When PAN is enabled, EL1 has no access to any
mappings that are also accessible to EL0, causing the firmware to crash
if it attempts to access such a mapping.
Even though it is debatable whether or not SCTLR_EL1.SPAN should be
disabled at entry or whether the firmware should put all UNKNOWN bits in
all system registers in a consistent state (which it should), using EL0
permissions serves no purpose whatsoever so let's fix that regardless.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Tested-by: Alexander Graf <agraf@csgraf.de>
Acked-by: Leif Lindholm <leif@nuviainc.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3751
Current MM communicate routine from ArmPkg would conduct few checks prior
to proceeding with SMC calls. However, the inspection step is different
from PI specification.
This patch updated MM communicate input argument inspection routine to
assure that "if the `MessageLength` is zero, or too large for the MM
implementation to manage, the MM implementation must update the
`MessageLength` to reflect the size of the `Data` buffer that it can
tolerate", as described by `EFI_MM_COMMUNICATION_PROTOCOL.Communicate()`
section in PI specification.
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3751
Current MM communicate routine from ArmPkg would conduct few checks prior
to proceeding with SMC calls. However, the inspection step is different
from PI specification.
This patch updated MM communicate input argument inspection routine to
assure `CommSize` represents "the size of the data buffer being passed
in" instead of the size of the data being used from data buffer, as
described by section `EFI_MM_COMMUNICATION2_PROTOCOL.Communicate()` in PI
specification.
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3751
Current MM communicate routine from ArmPkg would conduct few checks prior
to proceeding with SMC calls. However, the inspection step is different
from PI specification.
This patch updated MM communicate input argument inspection routine to
assure that return code `EFI_INVALID_PARAMETER` represents "the
`CommBuffer**` parameters do not refer to the same location in memory",
as described by `EFI_MM_COMMUNICATION2_PROTOCOL.Communicate()` section
in PI specification.
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Current MM communicate2 function from ArmPkg described input arguments
`CommBufferPhysical`, `CommBufferVirtual` and `CommSize` as input only,
which mismatches with the "input and output type" as in PI specification.
This change updated function descriptions of MM communite2 to match input
argument types.
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3737
Apply uncrustify changes to .c/.h files in the ArmPkg package
Cc: Andrew Fish <afish@apple.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Andrew Fish <afish@apple.com>
PcdPciIoTranslation PCD is relocated to MdePkg and leveraged by
both ARM and RISC-V arch. This patch removes the one from ArmPkg
and address the corresponding changes required for other modules
under ArmVirtPkg.
Signed-off-by: Abner Chang <abner.chang@hpe.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
Cc: Sunil V L <sunilvl@ventanamicro.com>
Reviewed-by: Daniel Schaefer <daniel.schaefer@hpe.com>
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Currently, at ExitBootServices() time, the GICv3 driver signals
End-Of-Interrupt (EOI) on all interrupt lines that are supported by the
interrupt controller. This appears to have been carried over from the
GICv2 version, but has been turned into something that violates the GIC
spec, and may trigger SError exceptions on some implementations.
Marc puts it as follows:
The GIC interrupt state machine is pretty strict. An interrupt can
only be deactivated (with or without prior priority drop) if it has
been acknowledged first. In GIC speak, this means that only the
following sequences are valid:
With EOImode==0:
x = ICC_IAR{0,1}_EL1;
ICC_EOIR{0,1}_EL1 = x;
With EOImode==1:
x = ICC_IAR{0,1}_EL1;
ICC_EOIR{0,1}_EL1 = x;
ICC_DIR_EL1 = x;
Any write to ICC_EOIR{0,1}_EL1 that isn't the direct consequence of
the same value being read from ICC_IAR{0,1}_EL1, and with the correct
nesting, breaks the state machine and leads to unpredictable results
that affects *all* interrupts in the system (most likely, the priority
system is dead). See Figure 4-3 ("Interrupt handling state machine")
in Arm IHI 0069F for a description of the acceptable transitions.
Additionally, on implementations that have ICC_CTLR_EL1.SEIS==1, a
SError may be generated to signal the error. See the various
<quote>
IMPLEMENTATION_DEFINED "SError ....";
</quote>
that are all over the pseudocode contained in the same architecture
spec. Needless to say, this is pretty final for any SW that would do
silly things on such implementations (which do exist).
Given that in our implementation, every signalled interrupt is acked,
handled and EOId in sequence, there is no reason to EOI all interrupts
at ExitBootServices() time in the first place, so let's just drop this
code. This fixes an issue reported by Marc where an SError is triggered
by this code, bringing down the system.
Reported-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Acked-by: Marc Zyngier <maz@kernel.org>
Tested-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
Rebecca Cran
Pierre Gondois
Michael Kubacki
Jeff Brasen via groups.io
Taylor Beebe
Kun Qin
Ard Biesheuvel
Sami Mujawar
Neil Jones
Oliver Smith-Denny
Rebecca Cran
Sudeep Holla
Rebecca Cran
Robbie King
Kun Qin
Abner Chang