-----Original Message-----
From: Andrew Fish <afish@...>
Sent: Friday, June 11, 2021 2:50 AM
To: edk2-devel-groups-io <devel@edk2.groups.io>; xypron.glpk@...
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>; Sunny Wang <Sunny.Wang@...>; Barton Gao <gaojie@...>; G Edhaya Chandran <Edhaya.Chandran@...>; Mike Kinney <michael.d.kinney@...>
Subject: Re: [edk2-devel] Commit a9d1fb58 - uefi-sct/SctPkg: allowable NotifyTpl in CreateEvent

On Jun 10, 2021, at 11:05 AM, Heinrich Schuchardt <xypron.glpk@...> wrote:

On 6/10/21 7:22 PM, Samer El-Haj-Mahmoud wrote:

+ Mike

I agree with Sunny's evaluation. The original EDK2 patch from Jan had good feedback from Mike K.. The exact language from the UEFI spec section 7.1 is:

" Consequently, there is a fourth TPL, TPL_HIGH_LEVEL, designed for use exclusively by the firmware."

Firmware includes drivers. Drivers use CreateEvent().

" This is the highest priority level. It is not interruptible (interrupts are disabled) and is used sparingly by firmware to synchronize operations that need to be accessible from any priority level. For example, it must be possible to signal events while executing at any priority level. Therefore, firmware manipulates the internal event structure while at this priority level."

Given this, I think the following needs to happen:

1. The SCT commit (https://github.com/tianocore/edk2-test/commit/421a6997ef362c6286c4ef87d21d5367a9d1fb58 ) needs to be reverted
2. The SCT change can be re-submitted as a new patch, without adding TPL_HIGH_LEVEL
3. The original EDK2 patch on Jan 6 (https://edk2.groups.io/g/devel/topic/79479680#69853) needs to be re-submitted per Mike's feedback. This means not adding the TPL_HIGH_LEVEL support in CreateEvent

In my opinion, (1) needs to be reverted right way, as it is causing SCT failures on many (all?) EDK2 based systems. This needs to happen before the edk2-test stable tag is created

Table 7-3 TPL Restrictions in UEFI spec 2.9 explicitly has:

Event Notification Levels

TPL_APPLICATION

<= TPL_HIGH_LEVEL

Nowhere does the specification say that CreateEvent() cannot be called
with with TPL_HIGH_LEVEL.

I can understand why Michael suggests that CreateEvent() should not be
callable at TPL_HIGH_LEVEL. But this requires a change request updating
the specification.

This is the other bit of the spec that might be relevant….

7.1 Event, Timer, and Task Priority Services
...
Execution in the boot services environment occurs at different task priority levels, or TPLs. The boot services environment exposes only three of these levels to UEFI applications and drivers:
• TPL_APPLICATION, the lowest priority level
• TPL_CALLBACK, an intermediate priority level
• TPL_NOTIFY, the highest priority level

Thanks,

Andrew Fish

I am missing this step in the action list above.

1) and 2) can be done with a single patch. Just don't revert the line
adding TPL_APPLICATION.

Best regards

Heinrich

Any thought about these suggested next steps?

Thanks,
--Samer

-----Original Message-----
From: Sunny Wang <Sunny.Wang@...>
Sent: Thursday, June 10, 2021 4:26 AM
To: Heinrich Schuchardt <xypron.glpk@...>; Samer El-Haj-Mahmoud
<Samer.El-Haj-Mahmoud@...>; Barton Gao
<gaojie@...>; G Edhaya Chandran
<Edhaya.Chandran@...>
Cc: Sunny Wang <Sunny.Wang@...>
Subject: RE: Commit a9d1fb58

Thanks for the information, Heinrich.

I think we need to further work on this for Mike's comment with your
ongoing edk2 patch.
-
https://edk2.groups.io/g/devel/message/69853?p=,,,20,0,0,0::relevance,,M
deModulePkg%3A+correct+TPL+level+check+CoreCreateEventEx,20,2,0,794
79680
For addressing Mike's comment, we will need to update both of your edk2
and edk2-test patches.
Therefore, I would suggest NOT picking up your edk2-test commit into the
upcoming stable tag/release. What do you guys think?

In other words, the main problem is that SCT should not create an event with
TPL_HIGH_LEVEL, so we need to revert TPL_HIGH_LEVEL related change in
Heinrich's commit https://github.com/tianocore/edk2-
test/commit/421a6997ef362c6286c4ef87d21d5367a9d1fb58.

By the way, I confirmed this issue on two of my AARCH64 platforms (RPi4 and
another one).

Best Regards,
Sunny Wang

-----Original Message-----
From: Heinrich Schuchardt <xypron.glpk@...>
Sent: Thursday, June 10, 2021 1:04 PM
To: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>; Barton
Gao <gaojie@...>; G Edhaya Chandran
<Edhaya.Chandran@...>; Sunny Wang <Sunny.Wang@...>
Subject: Re: Commit a9d1fb58

On 6/10/21 4:10 AM, Samer El-Haj-Mahmoud wrote:

With commit
https://github.com/tianocore/edk2-

test/commit/421a6997ef362c6286c4ef87d21d5367a9d1fb58

<https://github.com/tianocore/edk2-

test/commit/421a6997ef362c6286c4ef87d21d5367a9d1fb58>

, I am seeing the following new failure on some EDK2 based systems:

BS.CreateEvent - Create event with all valid event type and supported TPL.

--

*FAILURE*

EF317ADE-8668-456F-BED9-766056672DFF

/RELEASE_BUILD/edk2-test/uefi-

sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/
BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c:520:Statu
s - Invalid Parameter, EventType - 0x00000100, NotifyTpl -

31

BS.CreateEvent - Create event with all valid event type and supported TPL.

--

*FAILURE*

EF317ADE-8668-456F-BED9-766056672DFF

/RELEASE_BUILD/edk2-test/uefi-

sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/
BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c:520:Statu
s - Invalid Parameter, EventType - 0x00000200, NotifyTpl -

31

Are we sure this is implemented properly in EDK2? I imagine this code is
common in TianoCore/EDK2, and all systems will experience such failure.

I confirmed this on one AARCH64 system. I am trying to confirm on
another AARCH64 as well.

Can anyone confirm on other systems/archs?

If there is going to be a stable tag/release of edk2-test, we may want
to confirm this change does not cause failures in many systems before we
pick it up.

Thanks,

--Samer

An EDK II patch is available since 2021-01-06 via
https://bugzilla.tianocore.org/show_bug.cgi?id=3058
Cf. https://edk2.groups.io/g/devel/message/69851

Best regards

Heinrich

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.