> This patch makes me uncomfortable. I understand what it intends to do,
> and the intent is not wrong, but we're again treating "volatile UINT32"
> as atomic, and non-reorderable against the
> InterlockedCompareExchange32(). This kind of "optimization" is what
> people write cautionary tales
On 2/1/2024 9:09 AM, Yao, Jiewen via groups.io wrote:
Hi Nhi
Would you please:
1) File an issue in Bugzilla - https://bugzilla.tianocore.org/
2) Share with us the usage of this new API.
We are trying to understand why it is needed.
Hi Jiewen,
Sorry for late response. I've just been back from
volatile means do not optimize into a register. The compiler must perform the
actual memory access every time.
Mike
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of Ni, Ray
> Sent: Sunday, February 18, 2024 7:06 PM
> To: Zhou, Jianfeng ; devel@edk2.groups.io
> Cc: Laszlo
> -PleB->Uint64 = LocalPleB.Uint64;
>
> +*(volatile UINT64 *)&(PleB->Uint64) = LocalPleB.Uint64;
Jianfeng,
If "volatile" is to tell compiler to generate single "mov" instruction for
the assignment.
I am curious how it works in 32bit env where the general purpose registers are
32bit
> + STATIC CONST UINT32 CpusPerHob = 128;
1. Can you refer to
https://github.com/tianocore/edk2/blob/9979c887ef54023c9e262242db02c92d11b7f1e5/UefiCpuPkg/CpuMpPei/CpuMpPei.c#L572
to avoid
hardcode 128?
The MP_HAND_OFF structure change may cause 128 an invalid value.
-=-=-=-=-=-=-=-=-=-=-=-
> - MpHandOff = GetMpHandOffHob (0);
> - if (MpHandOff == NULL) {
> + MaxLogicalProcessorNumber = 0;
> + HaveMpHandOff = FALSE;
1. Can we cache the first MpHandOff instance? It can be used as an indicator to
replace "HaveMpHandOff",
and also can speed up the HOB enumeration in
SwitchApContext() can cache the first instance of MpHandOffHob so that the
second enumeration can avoid searching
for the MpHandOffHob from the top of HOB list.
> VOID
> SwitchApContext (
> - IN MP_HAND_OFF *MpHandOff
> + VOID
>)
> {
> - UINTN Index;
> - UINT32 BspNumber;
> +
Since MpHandOff HOBs are created by the PEI instance of MpInitLib, I am ok that
the consumer (DXE instance)
assumes the HOB instances are created in the order of ProcessorIndex.
Please update the patch accordingly with the change of GetMpHandOffHob() I
commented to patch #1.
Thanks,
Ray
>
>EFI_HOB_GUID_TYPE *GuidHob;
>MP_HAND_OFF*MpHandOff;
>
> - MpHandOff = NULL;
> - GuidHob = GetFirstGuidHob ();
> - if (GuidHob != NULL) {
> + for (GuidHob = GetFirstGuidHob ();
> + GuidHob != NULL;
> + GuidHob = GetNextGuidHob (, GET_NEXT_HOB
> (GuidHob)))
> +
Hi Laszlo,
I have tested:
1. Follow edk2 series patch 3 changes to
BaseTools/Source/Python/AutoGen/GenC.py.
2. Follow edk2-platforms series patch 5 changes to
edk2-platforms/Platform/Loongson/LoongArchQemuPkg/Sec/SecMain.c.
After the changes, I have built and tested, and it works fine.
The current implementation of loongarch NorFlashQemuLib is to
parse fdt and think that the first flash address resolved to
the NVRAM space, with the evolution of qemu code, loongarch
uses the first flash to store UEFI code and the second flash
as NVRAM space, so NorFlashQemuLib needs to be able to
The current implementation of loongarch NorFlashQemuLib is to
parse fdt and think that the first flash address resolved to
the NVRAM space, with the evolution of qemu code, loongarch
uses the first flash to store UEFI code and the second flash
as NVRAM space, so NorFlashQemuLib needs to be able to
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4679
Update PatchCheck.py to evaluate all the files modified in each commit
and generate an error if:
* A commit adds/modifies files in multiple package directories
* A commit adds/modifies files in multiple non-package directories
* A commit
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4679
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4680
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4693
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4694
Fix a collection of PatchCheck issues and add a new PatchCheck
featue
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4680
* Reject patches that match Author email "devel@edk2.groups.io"
* Update the current check for " via Groups.Io" to perform a
case insensitive match. It appears that groups.io has changed the
format of this string to use all lower case.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4694
If no Cc tags are detected in a commit message, then generate an
error. All patches sent for review are required to provide the set
of maintainers and reviewers responsible for the directories/files
modified. The set of maintainers and
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4693
Commit signatures are checked and error messages are logged but
errors are not captured and returned from find_signatures() in the
CommitMessageCheck class. This causes signature errors to be
silently ignored by CI.
Update logic in
Hello Mike,
Thanks for this proposal.
The tag in the commit log works fine for me.
This leaves the issue you raised that a contributor might add this tag
themselves when sending the patch but this is something we should be
able to catch in review.
--
Ard.
On Sun, 18 Feb 2024 at 04:36,
18 matches
Mail list logo
