Re: [edk2-devel] [PATCH V2 4/4] OvmfPkg/ResetVector: Update ResetVector to support Tdx

[Min Xu]

On July 23, 2021 9:35 PM, Lendacky, Thomas wrote:
> On 7/22/21 5:58 PM, Xu, Min M wrote:
> > On July 23, 2021 1:08 AM, Tom Lendacky wrote:
> >> On 7/22/21 12:52 AM, Min Xu wrote:
> >>> RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
> >>>
> >>> diff --git a/OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
> >>> b/OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
> >>> index c6d0d898bcd1..2206ca719593 100644
> >>> --- a/OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
> >>> +++ b/OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
> >>> @@ -17,6 +17,9 @@ Transition32FlatTo64Flat:
> >>>
> >>>  OneTimeCall SetCr3ForPageTables64
> >>>
> >>> +cmp dword[TDX_WORK_AREA], 0x47584454 ; 'TDXG'
> >>> +jz  TdxTransition32FlatTo64Flat
> >>> +
> >>
> >> Is the memory area guaranteed to be zeroed for legacy guests?
> >> Hopefully, this won't trip up a non-TDX guest with a false match (highly
> unlikely, though).
> >>
> > TDX_WORK_AREA is piece of TdxMailbox which is located in the MEMFD
> > started from PcdOvmfSecGhcbBackupBase. In Td guest, this memory region
> > is initialized to all-0 by host VMM. In legacy guests, I am not sure
> > what's the initialized value it is. So 'TDXG' is checked to guarantee it is 
> > Td-
> guest or not.
> > Since Tdx re-use the memory region (PcdOvmfSecGhcbBackupBase) as the
> > TDX_WORK_AREA, and @Tom Lendacky you should be the original owner of
> > PcdOvmfSecGhcbBackupBase, can this area be cleared in the beginning of
> > ResetVector in legacy guests? Or I should better create a TDX specific
> > work area in MEMFD to guarantee the Td And Non-Td check?
> 
> I believe PcdOvmfSecGhcbBackupBase can be cleared early. For SEV-ES, it isn't
> shared with the hypervisor, so clearing it before activating the pagetables 
> can
> be done (it will be treated as encrypted before paging is enabled and mapped
> as encrypted after paging is enabled) and for a legacy guest the mapping
> doesn't matter. It isn't required to be cleared today, so if you do add
> something, be sure to put a comment in there about why it's being done. No
> need for a new area.
> 
> The possibility of random data being there that matches 'TDXG' is extremely
> low. But better safe than sorry, I guess.
>
Thanks Tom. I will update it in the next version.
>

Thanks!
Xu, Min


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78147): https://edk2.groups.io/g/devel/message/78147
Mute This Topic: https://groups.io/mt/84373830/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 3/3] BaseTools: Drop check for distutils.utils

[Cole]

distutils.utils is no longer used anywhere, so this check can
be dropped.

Signed-off-by: Cole Robinson 
---
 BaseTools/Tests/RunTests.py | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/BaseTools/Tests/RunTests.py b/BaseTools/Tests/RunTests.py
index 81af736cd8..934683a446 100644
--- a/BaseTools/Tests/RunTests.py
+++ b/BaseTools/Tests/RunTests.py
@@ -13,13 +13,6 @@ import os
 import sys

 import unittest

 

-try:

-import distutils.util

-except ModuleNotFoundError:

-sys.exit('''

-Python reported: "No module named 'distutils.util"

-''')

-

 import TestTools

 

 def GetCTestSuite():

-- 
2.31.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78146): https://edk2.groups.io/g/devel/message/78146
Mute This Topic: https://groups.io/mt/84409131/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 1/3] build: Fix python3.10 threading DeprecationWarnings

[Cole]

threading camelCase functions have preferred alternatives since
python2.6. python3.10 has started emitting DeprecationWarnings
for them

Signed-off-by: Cole Robinson 
---
 BaseTools/Source/Python/build/build.py | 48 +-
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/BaseTools/Source/Python/build/build.py 
b/BaseTools/Source/Python/build/build.py
index 3e4d83409f..02b4898924 100755
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -197,7 +197,7 @@ def ReadMessage(From, To, ExitFlag,MemTo=None):
 To(LineStr)

 else:

 break

-if ExitFlag.isSet():

+if ExitFlag.is_set():

 break

 

 class MakeSubProc(Popen):

@@ -241,8 +241,8 @@ def LaunchCommand(Command, WorkingDir,ModuleAuto = None):
 EndOfProcedure.clear()

 if Proc.stdout:

 StdOutThread = Thread(target=ReadMessage, args=(Proc.stdout, 
EdkLogger.info, EndOfProcedure,Proc.ProcOut))

-StdOutThread.setName("STDOUT-Redirector")

-StdOutThread.setDaemon(False)

+StdOutThread.name = "STDOUT-Redirector"

+StdOutThread.daemon = False

 StdOutThread.start()

 

 

@@ -433,8 +433,8 @@ class BuildTask:
 @staticmethod

 def StartScheduler(MaxThreadNumber, ExitFlag):

 SchedulerThread = Thread(target=BuildTask.Scheduler, 
args=(MaxThreadNumber, ExitFlag))

-SchedulerThread.setName("Build-Task-Scheduler")

-SchedulerThread.setDaemon(False)

+SchedulerThread.name = "Build-Task-Scheduler"

+SchedulerThread.daemon = False

 SchedulerThread.start()

 # wait for the scheduler to be started, especially useful in Linux

 while not BuildTask.IsOnGoing():

@@ -456,7 +456,7 @@ class BuildTask:
 # indicated to do so, or there's error in running thread

 #

 while (len(BuildTask._PendingQueue) > 0 or 
len(BuildTask._ReadyQueue) > 0 \

-   or not ExitFlag.isSet()) and not 
BuildTask._ErrorFlag.isSet():

+   or not ExitFlag.is_set()) and not 
BuildTask._ErrorFlag.is_set():

 EdkLogger.debug(EdkLogger.DEBUG_8, "Pending Queue (%d), Ready 
Queue (%d)"

 % (len(BuildTask._PendingQueue), 
len(BuildTask._ReadyQueue)))

 

@@ -474,7 +474,7 @@ class BuildTask:
 BuildTask._PendingQueueLock.release()

 

 # launch build thread until the maximum number of threads is 
reached

-while not BuildTask._ErrorFlag.isSet():

+while not BuildTask._ErrorFlag.is_set():

 # empty ready queue, do nothing further

 if len(BuildTask._ReadyQueue) == 0:

 break

@@ -498,12 +498,12 @@ class BuildTask:
 time.sleep(0.01)

 

 # wait for all running threads exit

-if BuildTask._ErrorFlag.isSet():

+if BuildTask._ErrorFlag.is_set():

 EdkLogger.quiet("\nWaiting for all build threads exit...")

-# while not BuildTask._ErrorFlag.isSet() and \

+# while not BuildTask._ErrorFlag.is_set() and \

 while len(BuildTask._RunningQueue) > 0:

 EdkLogger.verbose("Waiting for thread ending...(%d)" % 
len(BuildTask._RunningQueue))

-EdkLogger.debug(EdkLogger.DEBUG_8, "Threads [%s]" % ", 
".join(Th.getName() for Th in threading.enumerate()))

+EdkLogger.debug(EdkLogger.DEBUG_8, "Threads [%s]" % ", 
".join(Th.name for Th in threading.enumerate()))

 # avoid tense loop

 time.sleep(0.1)

 except BaseException as X:

@@ -531,7 +531,7 @@ class BuildTask:
 #

 @staticmethod

 def IsOnGoing():

-return not BuildTask._SchedulerStopped.isSet()

+return not BuildTask._SchedulerStopped.is_set()

 

 ## Abort the build

 @staticmethod

@@ -547,7 +547,7 @@ class BuildTask:
 #

 @staticmethod

 def HasError():

-return BuildTask._ErrorFlag.isSet()

+return BuildTask._ErrorFlag.is_set()

 

 ## Get error message in running thread

 #

@@ -644,7 +644,7 @@ class BuildTask:
 # TRICK: hide the output of threads left running, so that the user 
can

 #catch the error message easily

 #

-if not BuildTask._ErrorFlag.isSet():

+if not BuildTask._ErrorFlag.is_set():

 GlobalData.gBuildingModule = "%s [%s, %s, %s]" % 
(str(self.BuildItem.BuildObject),

   
self.BuildItem.BuildObject.Arch,

   
self.BuildItem.BuildObject.ToolChain,

@@ -653,7 +653,7 @@ class BuildTask:
 EdkLogger.SetLevel(EdkLogger.ERROR)

 

[edk2-devel] [PATCH 2/3] python: Replace distutils.utils.split_quotes with shlex.split

[Cole]

distutils is deprecated and may be removed in python 3.12.
Use shlex.split which has been around since python 2.3.

shlex.split does not split on all the ASCII control characters that
split_quoted will[1], but for edk2 usage I don't think that matters.

[1] 
https://stackoverflow.com/questions/54999301/what-is-the-difference-between-distutils-util-split-quoted-and-shlex-split

Signed-off-by: Cole Robinson 
---
 BaseTools/Source/Python/AutoGen/UniClassObject.py | 4 ++--
 BaseTools/Source/Python/UPT/Library/UniClassObject.py | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/UniClassObject.py 
b/BaseTools/Source/Python/AutoGen/UniClassObject.py
index 883c2356e0..b16330e368 100644
--- a/BaseTools/Source/Python/AutoGen/UniClassObject.py
+++ b/BaseTools/Source/Python/AutoGen/UniClassObject.py
@@ -12,7 +12,7 @@
 #

 from __future__ import print_function

 import Common.LongFilePathOs as os, codecs, re

-import distutils.util

+import shlex

 import Common.EdkLogger as EdkLogger

 from io import BytesIO

 from Common.BuildToolError import *

@@ -233,7 +233,7 @@ class UniFileClassObject(object):
 # Get Language definition

 #

 def GetLangDef(self, File, Line):

-Lang = distutils.util.split_quoted((Line.split(u"//")[0]))

+Lang = shlex.split(Line.split(u"//")[0])

 if len(Lang) != 3:

 try:

 FileIn = 
UniFileClassObject.OpenUniFile(LongFilePath(File.Path))

diff --git a/BaseTools/Source/Python/UPT/Library/UniClassObject.py 
b/BaseTools/Source/Python/UPT/Library/UniClassObject.py
index d25f300146..8c44dc2252 100644
--- a/BaseTools/Source/Python/UPT/Library/UniClassObject.py
+++ b/BaseTools/Source/Python/UPT/Library/UniClassObject.py
@@ -14,7 +14,7 @@ from __future__ import print_function
 # Import Modules

 #

 import os, codecs, re

-import distutils.util

+import shlex

 from Logger import ToolError

 from Logger import Log as EdkLogger

 from Logger import StringTable as ST

@@ -320,7 +320,7 @@ class UniFileClassObject(object):
 # Get Language definition

 #

 def GetLangDef(self, File, Line):

-Lang = distutils.util.split_quoted((Line.split(u"//")[0]))

+Lang = shlex.split(Line.split(u"//")[0])

 if len(Lang) != 3:

 try:

 FileIn = codecs.open(File.Path, mode='rb', 
encoding='utf_8').readlines()

-- 
2.31.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78145): https://edk2.groups.io/g/devel/message/78145
Mute This Topic: https://groups.io/mt/84409130/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 0/3] BaseTools: fix some python DeprecationWarnings

[Cole]

This addresses some python DeprecationWarnings that are popping up
with python 3.10

Cole Robinson (3):
  build: Fix python3.10 threading DeprecationWarnings
  python: Replace distutils.utils.split_quotes with shlex.split
  BaseTools: Drop check for distutils.utils

 .../Source/Python/AutoGen/UniClassObject.py   |  4 +-
 .../Python/UPT/Library/UniClassObject.py  |  4 +-
 BaseTools/Source/Python/build/build.py| 48 +--
 BaseTools/Tests/RunTests.py   |  7 ---
 4 files changed, 28 insertions(+), 35 deletions(-)

-- 
2.31.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78143): https://edk2.groups.io/g/devel/message/78143
Mute This Topic: https://groups.io/mt/84409128/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

[Cheng-Chieh Huang via groups.io]

Hi,

This is not about UefiPayload. I did not have GCC5 in my environment, so I
use a newer GCC to compile. However, in newer gcc (let's say GCC8), it
generates floating point instructions in each function and system will
crash as you mentioned that we did not handle it in UEFI.

I uploaded this patch because many people from our group use newer GCC and
it will be more convenient if we can disable it with a flag (otherwise, we
need to maintain a local patch). I guess the real fix (bigger work) is for
EDK2 to support the newer GCC version in its toolchain (create something
like *_GCC8_*).

Back to the question, I think it should be no harm to add these flags to
tool_defs.txt if EDK2 did not expect the compiler to generate MMX/SSE
instructions in the first place.

--
Cheng-Chieh

On Sat, Jul 24, 2021 at 12:46 AM Kinney, Michael D <
michael.d.kin...@intel.com> wrote:

> Can you provide details on why this flag was needed in the UefiPayloadPkg
> use case?
>
>
>
> If the newer versions of the GCC compilers are injecting use of MMX/SSE
> instructions from optimization of C code, then we need to unconditionally
> disable that usage in tools_def.txt.  The only place MMX/SSE instructions
> can be used is in NASM files with proper save/restore of the MMX/SSE state.
>
>
>
> Here is an example with use of xmm0 that is saved/restored on the stack.
>
>
>
>
> https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseMemoryLibSse2/X64/CopyMem.nasm
>
>
>
> Mike
>
>
>
> *From:* Cheng-Chieh Huang 
> *Sent:* Friday, July 23, 2021 4:17 AM
> *To:* gaoliming 
> *Cc:* devel@edk2.groups.io; Kinney, Michael D 
> *Subject:* Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add
> DISABLE_MMX_SSE to avoid generating floating points operation
>
>
>
> I got what you mean by "Tools_def.txt doesn’t support build flag" now. I
> thought the parser does support some sort of if/else condition, but it
> seems to be not the case. Alternatively, we can do this during the copy
> (from template to tools_def.txt), but everytime we need to recreate
> tools_def.txt which is not ideal).
>
>
>
> In that case, I think we should just roll back to only doing this in
> UefiPayloadPkg.dsc. If other packs want to add these flags, they will need
> to do it on their own.
>
>
>
> On Fri, Jul 23, 2021 at 6:29 PM gaoliming 
> wrote:
>
> How do you add this support in tools_def? Can you give the proposal for it?
>
>
>
> Thanks
>
> Liming
>
> *发件人**:* Cheng-Chieh Huang 
> *发送时间:* 2021年7月22日 10:35
> *收件人:* gaoliming 
> *抄送:* devel@edk2.groups.io; Kinney, Michael D 
> *主题:* Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE
> to avoid generating floating points operation
>
>
>
> I mean, I will submit a patch to support DISABLE_GCC_MMX_SSE in tools_def.
> What do you think?
>
>
>
> --
>
> Cheng-Chieh
>
>
>
> On Thu, Jul 22, 2021 at 9:28 AM gaoliming 
> wrote:
>
> Tools_def.txt doesn’t support build flag DISABLE_GCC_MMX_SSE. If this
> flag is moved into BaseTools\Conf\tools_def.template, -mno-mmx -mno-sse
> option will be the default GCC options. That means all platforms will apply
> them.
>
>
>
> Thanks
>
> Liming
>
> *发件人**:* devel@edk2.groups.io  *代表 *Cheng-Chieh
> Huang via groups.io
> *发送时间:* 2021年7月22日 1:43
> *收件人:* Kinney, Michael D 
> *抄送:* devel@edk2.groups.io
> *主题:* Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE
> to avoid generating floating points operation
>
>
>
> Yes, we can. I will drop this patch for this  uefipayload batch and send
> another one for support DISABLE_GCC_MMX_SSE in tools_de.txt.
>
>
>
> --
>
> Cheng-Chieh
>
> On Thu, Jul 22, 2021, 12:35 AM Kinney, Michael D <
> michael.d.kin...@intel.com> wrote:
>
> Are those flags needed for all packages that build with GCC?
>
> Should this be moved into tools_def.txt?
>
> Mike
>
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of
> Cheng-Chieh Huang via groups.io
> > Sent: Wednesday, July 21, 2021 6:23 AM
> > To: devel@edk2.groups.io
> > Cc: Cheng-Chieh Huang 
> > Subject: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE
> to avoid generating floating points operation
> >
> > This will allow we compile payload using gcc8
> >
> > Signed-off-by: Cheng-Chieh Huang 
> > ---
> >  UefiPayloadPkg/UefiPayloadPkg.dsc | 5 +
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc
> b/UefiPayloadPkg/UefiPayloadPkg.dsc
> > index 8aa5f18cd35c..fa41c5a24af5 100644
> > --- a/UefiPayloadPkg/UefiPayloadPkg.dsc
> > +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
> > @@ -30,6 +30,8 @@ [Defines]
> >DEFINE PS2_KEYBOARD_ENABLE  = FALSE
> >DEFINE UNIVERSAL_PAYLOAD= FALSE
> >
> > +  DEFINE DISABLE_MMX_SSE  = FALSE
> > +
> >#
> ># SBL:  UEFI payload for Slim Bootloader
> ># COREBOOT: UEFI payload for coreboot
> > @@ -96,6 +98,9 @@ [BuildOptions]
> >*_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
> >  !if $(BOOTLOADER) == "LINUXBOOT"

Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

[Michael D Kinney]

As long as those flags are supported or ignored by the GCC5 compilers, then 
there should be not harm in adding them to the GCC5 profile.

Mike

From: Cheng-Chieh Huang 
Sent: Friday, July 23, 2021 10:17 AM
To: Kinney, Michael D 
Cc: gaoliming ; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to 
avoid generating floating points operation

Hi,

This is not about UefiPayload. I did not have GCC5 in my environment, so I use 
a newer GCC to compile. However, in newer gcc (let's say GCC8), it generates 
floating point instructions in each function and system will crash as you 
mentioned that we did not handle it in UEFI.

I uploaded this patch because many people from our group use newer GCC and it 
will be more convenient if we can disable it with a flag (otherwise, we need to 
maintain a local patch). I guess the real fix (bigger work) is for EDK2 to 
support the newer GCC version in its toolchain (create something like *_GCC8_*).

Back to the question, I think it should be no harm to add these flags to 
tool_defs.txt if EDK2 did not expect the compiler to generate MMX/SSE 
instructions in the first place.

--
Cheng-Chieh

On Sat, Jul 24, 2021 at 12:46 AM Kinney, Michael D 
mailto:michael.d.kin...@intel.com>> wrote:
Can you provide details on why this flag was needed in the UefiPayloadPkg use 
case?

If the newer versions of the GCC compilers are injecting use of MMX/SSE 
instructions from optimization of C code, then we need to unconditionally 
disable that usage in tools_def.txt.  The only place MMX/SSE instructions can 
be used is in NASM files with proper save/restore of the MMX/SSE state.

Here is an example with use of xmm0 that is saved/restored on the stack.

https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseMemoryLibSse2/X64/CopyMem.nasm

Mike

From: Cheng-Chieh Huang mailto:chengch...@google.com>>
Sent: Friday, July 23, 2021 4:17 AM
To: gaoliming mailto:gaolim...@byosoft.com.cn>>
Cc: devel@edk2.groups.io; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Subject: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to 
avoid generating floating points operation

I got what you mean by "Tools_def.txt doesn’t support build flag" now. I 
thought the parser does support some sort of if/else condition, but it seems to 
be not the case. Alternatively, we can do this during the copy (from template 
to tools_def.txt), but everytime we need to recreate tools_def.txt which is not 
ideal).

In that case, I think we should just roll back to only doing this in 
UefiPayloadPkg.dsc. If other packs want to add these flags, they will need to 
do it on their own.

On Fri, Jul 23, 2021 at 6:29 PM gaoliming 
mailto:gaolim...@byosoft.com.cn>> wrote:
How do you add this support in tools_def? Can you give the proposal for it?

Thanks
Liming
发件人: Cheng-Chieh Huang mailto:chengch...@google.com>>
发送时间: 2021年7月22日 10:35
收件人: gaoliming mailto:gaolim...@byosoft.com.cn>>
抄送: devel@edk2.groups.io; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
主题: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to 
avoid generating floating points operation

I mean, I will submit a patch to support DISABLE_GCC_MMX_SSE in tools_def. What 
do you think?

--
Cheng-Chieh

On Thu, Jul 22, 2021 at 9:28 AM gaoliming 
mailto:gaolim...@byosoft.com.cn>> wrote:
Tools_def.txt doesn’t support build flag DISABLE_GCC_MMX_SSE. If this flag is 
moved into BaseTools\Conf\tools_def.template, -mno-mmx -mno-sse option will be 
the default GCC options. That means all platforms will apply them.

Thanks
Liming
发件人: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> 代表 Cheng-Chieh Huang via 
groups.io
发送时间: 2021年7月22日 1:43
收件人: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
抄送: devel@edk2.groups.io
主题: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to 
avoid generating floating points operation

Yes, we can. I will drop this patch for this  uefipayload batch and send 
another one for support DISABLE_GCC_MMX_SSE in tools_de.txt.

--
Cheng-Chieh
On Thu, Jul 22, 2021, 12:35 AM Kinney, Michael D 
mailto:michael.d.kin...@intel.com>> wrote:
Are those flags needed for all packages that build with GCC?

Should this be moved into tools_def.txt?

Mike

> -Original Message-
> From: devel@edk2.groups.io 
> mailto:devel@edk2.groups.io>> On Behalf Of Cheng-Chieh 
> Huang via groups.io
> Sent: Wednesday, July 21, 2021 6:23 AM
> To: devel@edk2.groups.io
> Cc: Cheng-Chieh Huang mailto:chengch...@google.com>>
> Subject: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to 
> avoid generating floating points operation
>
> This will allow we compile payload using gcc8
>
> Signed-off-by: Cheng-Chieh Huang 
> 

Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

[Michael D Kinney]

Can you provide details on why this flag was needed in the UefiPayloadPkg use 
case?

If the newer versions of the GCC compilers are injecting use of MMX/SSE 
instructions from optimization of C code, then we need to unconditionally 
disable that usage in tools_def.txt.  The only place MMX/SSE instructions can 
be used is in NASM files with proper save/restore of the MMX/SSE state.

Here is an example with use of xmm0 that is saved/restored on the stack.

https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseMemoryLibSse2/X64/CopyMem.nasm

Mike

From: Cheng-Chieh Huang 
Sent: Friday, July 23, 2021 4:17 AM
To: gaoliming 
Cc: devel@edk2.groups.io; Kinney, Michael D 
Subject: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to 
avoid generating floating points operation

I got what you mean by "Tools_def.txt doesn’t support build flag" now. I 
thought the parser does support some sort of if/else condition, but it seems to 
be not the case. Alternatively, we can do this during the copy (from template 
to tools_def.txt), but everytime we need to recreate tools_def.txt which is not 
ideal).

In that case, I think we should just roll back to only doing this in 
UefiPayloadPkg.dsc. If other packs want to add these flags, they will need to 
do it on their own.

On Fri, Jul 23, 2021 at 6:29 PM gaoliming 
mailto:gaolim...@byosoft.com.cn>> wrote:
How do you add this support in tools_def? Can you give the proposal for it?

Thanks
Liming
发件人: Cheng-Chieh Huang mailto:chengch...@google.com>>
发送时间: 2021年7月22日 10:35
收件人: gaoliming mailto:gaolim...@byosoft.com.cn>>
抄送: devel@edk2.groups.io; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
主题: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to 
avoid generating floating points operation

I mean, I will submit a patch to support DISABLE_GCC_MMX_SSE in tools_def. What 
do you think?

--
Cheng-Chieh

On Thu, Jul 22, 2021 at 9:28 AM gaoliming 
mailto:gaolim...@byosoft.com.cn>> wrote:
Tools_def.txt doesn’t support build flag DISABLE_GCC_MMX_SSE. If this flag is 
moved into BaseTools\Conf\tools_def.template, -mno-mmx -mno-sse option will be 
the default GCC options. That means all platforms will apply them.

Thanks
Liming
发件人: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> 代表 Cheng-Chieh Huang via 
groups.io
发送时间: 2021年7月22日 1:43
收件人: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
抄送: devel@edk2.groups.io
主题: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to 
avoid generating floating points operation

Yes, we can. I will drop this patch for this  uefipayload batch and send 
another one for support DISABLE_GCC_MMX_SSE in tools_de.txt.

--
Cheng-Chieh
On Thu, Jul 22, 2021, 12:35 AM Kinney, Michael D 
mailto:michael.d.kin...@intel.com>> wrote:
Are those flags needed for all packages that build with GCC?

Should this be moved into tools_def.txt?

Mike

> -Original Message-
> From: devel@edk2.groups.io 
> mailto:devel@edk2.groups.io>> On Behalf Of Cheng-Chieh 
> Huang via groups.io
> Sent: Wednesday, July 21, 2021 6:23 AM
> To: devel@edk2.groups.io
> Cc: Cheng-Chieh Huang mailto:chengch...@google.com>>
> Subject: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to 
> avoid generating floating points operation
>
> This will allow we compile payload using gcc8
>
> Signed-off-by: Cheng-Chieh Huang 
> mailto:chengch...@google.com>>
> ---
>  UefiPayloadPkg/UefiPayloadPkg.dsc | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc 
> b/UefiPayloadPkg/UefiPayloadPkg.dsc
> index 8aa5f18cd35c..fa41c5a24af5 100644
> --- a/UefiPayloadPkg/UefiPayloadPkg.dsc
> +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
> @@ -30,6 +30,8 @@ [Defines]
>DEFINE PS2_KEYBOARD_ENABLE  = FALSE
>DEFINE UNIVERSAL_PAYLOAD= FALSE
>
> +  DEFINE DISABLE_MMX_SSE  = FALSE
> +
>#
># SBL:  UEFI payload for Slim Bootloader
># COREBOOT: UEFI payload for coreboot
> @@ -96,6 +98,9 @@ [BuildOptions]
>*_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
>  !if $(BOOTLOADER) == "LINUXBOOT"
>*_*_*_CC_FLAGS = -D LINUXBOOT_PAYLOAD
> +!endif
> +!if $(DISABLE_MMX_SSE)
> +  *_*_*_CC_FLAGS = -mno-mmx -mno-sse
>  !endif
>GCC:*_UNIXGCC_*_CC_FLAGS   = -DMDEPKG_NDEBUG
>GCC:RELEASE_*_*_CC_FLAGS   = -DMDEPKG_NDEBUG
> --
> 2.32.0.402.g57bb445576-goog
>
>
>
>
>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78140): https://edk2.groups.io/g/devel/message/78140
Mute This Topic: https://groups.io/mt/84401664/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]

Re: [edk2-devel] [PATCH 1/1] EmbeddedPkg/VirtualRealTimeClockLib : Fix SetTime issues

[Pete Batard]

Hi Sunny,

Good catch for both these issues. Thanks for fixing them.

With this:

On 2021.07.23 10:04, Sunny Wang wrote:

This patch fixes two issues below:
1. SCT SetTime_Func failures.
- https://github.com/pftf/RPi4/issues/164
2. Using shell time and date commands to set time can't work.

The problem is that gRT->SetTime always returns EFI_INVALID_PARAMETER
error status.

The root cause is that LibSetTime() sets RtcEpochSeconds variable with
inconsistent attributes. One is without EFI_VARIABLE_NON_VOLATILE,
the other one is with EFI_VARIABLE_NON_VOLATILE. That caused that the
variable driver returns EFI_INVALID_PARAMETER. Per UEFI spec, if a
preexisting variable is rewritten with different attributes,
SetVariable() shall not modify the variable and shall return
EFI_INVALID_PARAMETER.

Therefore, the solution is to add EFI_VARIABLE_NON_VOLATILE attribute
to the first EfiSetVariable() call to make two calls consistent.

By the way, this patch also fix a minor issue with a debug message.

Cc: Samer El-Haj-Mahmoud 
Cc: Sami Mujawar 
Cc: Jeremy Linton 
Cc: Ard Biesheuvel 
Cc: Pete Batard 
Cc: Leif Lindholm 

Signed-off-by: Sunny Wang 
---
  .../VirtualRealTimeClockLib/VirtualRealTimeClockLib.c   | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git 
a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c 
b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
index de6fbb40e6..c10c91bc75 100644
--- a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
+++ b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
@@ -4,7 +4,7 @@
   *
   *  Coypright (c) 2019, Pete Batard 
   *  Copyright (c) 2018, Andrei Warkentin 
- *  Copyright (c) 2011-2014, ARM Ltd. All rights reserved.
+ *  Copyright (c) 2011-2021, ARM Ltd. All rights reserved.
   *  Copyright (c) 2008-2010, Apple Inc. All rights reserved.
   *  Copyright (c) Microsoft Corporation. All rights reserved.
   *
@@ -96,7 +96,7 @@ LibGetTime (
  EfiSetVariable (
(CHAR16 *)mEpochVariableName,
,
-  EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
+  EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS,
sizeof (EpochSeconds),

);
@@ -324,7 +324,7 @@ LibSetTime (
  DEBUG ((
DEBUG_ERROR,
"LibSetTime: Failed to save %s variable to non-volatile storage, Status = 
%r\n",
-  mDaylightVariableName,
+  mEpochVariableName,
Status
));
  return Status;



Reviewed-by: Pete Batard 
Tested-by: Pete Batard 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78139): https://edk2.groups.io/g/devel/message/78139
Mute This Topic: https://groups.io/mt/84397263/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platform PATCH v1 1/1] Platform/RaspberryPi: Make SetVariable return EFI_UNSUPPORTED at runtime

[Ard Biesheuvel]

On Fri, 23 Jul 2021 at 11:15, Sunny Wang  wrote:
>
> The RPi does not support storing UEFI NV variables at runtime. For now,
> gRT->SetVariable at runtime returns EFI_OUT_OF_RESOURCES which is not a
> proper error and would cause FWTS failures. Therefore, this patch is
> to make gRT->SetVariable at runtime return EFI_UNSUPPORTED.
>
> For more information, please check the issues below:
>-https://github.com/pftf/RPi4/issues/6
>-https://github.com/pftf/RPi4/issues/93
>-https://github.com/pftf/RPi4/issues/163
>
> I also tested this with the ACS 3.0 FWTS. All the failures
> reported in issue 93 and 163 can be fixed by this patch.
>
> Cc: Samer El-Haj-Mahmoud 
> Cc: Sami Mujawar 
> Cc: Jeremy Linton 
> Cc: Ard Biesheuvel 
> Cc: Pete Batard 
> Cc: Leif Lindholm 
>
> Signed-off-by: Sunny Wang 

This looks ok to me, but we should also expose this fact via the
EFI_RT_PROPERTIES_TABLE, so that the OS can anticipate this result.

> ---
>  .../Drivers/VarBlockServiceDxe/VarBlockService.c | 12 
>  1 file changed, 12 insertions(+)
>
> diff --git 
> a/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c 
> b/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
> index 572309439a..16d4d4f178 100644
> --- a/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
> +++ b/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
> @@ -2,6 +2,7 @@
>   *
>   *  Copyright (c) 2018, Andrei Warkentin 
>   *  Copyright (c) 2006-2014, Intel Corporation. All rights reserved.
> + *  Copyright (c) 2021, ARM Limited. All rights reserved.
>   *
>   *  SPDX-License-Identifier: BSD-2-Clause-Patent
>   *
> @@ -596,6 +597,7 @@ FvbProtocolWrite (
>  EFI_DEVICE_ERROR  - The block device is not functioning correctly and
>  could not be written
>  EFI_INVALID_PARAMETER - NumBytes or Buffer are NULL
> +EFI_UNSUPPORTED This function is not supported at runtime
>
>  --*/
>  {
> @@ -605,6 +607,16 @@ FvbProtocolWrite (
>EFI_STATUS Status;
>EFI_STATUS ReturnStatus;
>
> +  //
> +  // The current variables support relies on modifying RPI_EFI.FD on SD
> +  // card, which works fine at boot time. However, at runtime, the SD
> +  // controller is exposed via ACPI and subsequently owned by the OS.
> +  // Therefore, we need to direclty return EFI_UNSUPPORTED.
> +  //
> +  if (EfiAtRuntime ()) {
> +   return EFI_UNSUPPORTED;
> +  }
> +
>//
>// Check for invalid conditions.
>//
> --
> 2.31.0.windows.1
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78138): https://edk2.groups.io/g/devel/message/78138
Mute This Topic: https://groups.io/mt/84397460/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] ArmVirt and Self-Updating Code

[Ard Biesheuvel]

On Fri, 23 Jul 2021 at 16:27, Marvin Häuser  wrote:
>
>
>
> On 23.07.21 16:09, Ard Biesheuvel wrote:
> > On Fri, 23 Jul 2021 at 12:47, Marvin Häuser  wrote:
> >> On 23.07.21 12:13, Ard Biesheuvel wrote:
> >>> On Fri, 23 Jul 2021 at 11:55, Marvin Häuser  wrote:
...
>  2) emit a GOT, which ends up being converted to PE/COFF Relocations (->
>  self-relocation), for global data that cannot be referenced relatively?
>  Is there any way to know/force that no symbol in GOT is accessed up
>  until the end of the self-relocation process?
>
> Do you maybe have one final comment regarding that second question,
> please? :)

The RELA section is not converted into PE/COFF relocations. This would
not achieve a lot, given that no prior PE/COFF loader exists to
process them. There is a snippet of asm code in the startup code that
processes the R_AARCH64_RELATIVE relocation entries before calling
into C code.

This also gives us the guarantee that no GOT indirections are
dereferenced, given that our asm code simply does not do that.

> Let's drop "GOT" and make it "any instruction that requires prior
> relocation to function correctly".
>

The thing to keep in mind here is that R_AARCH64_RELATIVE relocations
never target instructions, but only memory locations that carry
absolute addresses. This could be locations in .rodata or .data
(global vars carrying pointer values), or GOT entries.

> >>> It is not really a GOT. Actually, a GOT is undesirable, as it forces
> >>> global variables to be referenced via an absolute address, even when a
> >>> relative reference could be used.
> >> Hmm, the GCC docs say a GOT is used for "all constant addresses" (I took
> >> it as "absolute"?), it is kind of vague. I understood it this way:
> >> 1) no-pie emits relocations that can target the .text and .data sections
> >> for instructions that embed and variables that hold an absolute address
> >> (I thought this was RELA?)
> >> 2) pie emits a GOT such that there are no relocations as described in
> >> 1), because all absolute addresses are indirected by GOT (just GOT
> >> references are relocated)
> >>
> > Correct. And this works really well for shared libraries, where all
> > text and data sections can be shared between processes, as they will
> > not be modified by the loader. All locations targeted by relocations
> > will be nicely lumped together in the GOT.
> >
> > However, for bare metal style programs, there is no sharing, and there
> > is no advantage to lumping anything together. It is much better to use
> > relative references where possible, and simply apply relocations
> > wherever needed across the text and data sections,
> >
> >> If I understood the process right, but the term (GOT) is wrong, sorry,
> >> that is what I gathered from the docs. :)
> >> I have a x86 + PE background, so ARM + ELF is a bit of a learning curve...
> >>
> > The GOT is a special data structure used for implicit variable
> > accesses, i.e., global vars used in the code. Statically initialized
> > pointer variables are the other category, which are not code, and for
> > which the same considerations do not apply, given that the right value
> > simply needs to be stored in the variable before the program starts.
> >
> >>> For instance, a statically initialized pointer always carries an
> >>> absolute address, and so it always needs an entry in the RELA table
> >>>
> >>> E.g.,
> >>>
> >>> int foo = 10; // external linkage
> >>> static int *bar = 
> >>>
> >>> In this case, there is no way to use relative addressing because the
> >>> address of foo is taken at build time.
> >>>
> >>> However, if bar would be something like
> >>>
> >>> static int *bar() { return  }
> >>>
> >>> the address is only taken at runtime, and the compiler can use a
> >>> relative reference instead, and no RELA entry is needed. With a GOT,
> >>> we force the compiler to allocate a variable that holds the absolute
> >>> address, which we would prefer to avoid.
> >> And this is not forced by whatever table -fpie uses, as per my
> >> understanding above?
> >>
> > The selection of 'code model' as it is called is controlled by GCC's
> > -mcmodel= argument, which defaults to 'small' on AArch64, regardless
> > of whether you use PIC/PIE or not.
>
> Aha, makes sense, thanks!
>
> Best regards,
> Marvin
>
> >> “Now, StandaloneMmPkg has similar (self-)relocation code 
> >> too:https://github.com/tianocore/edk2/blob/17143c4837393d42c484b42d1789b85b2cff1aaf/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c#L379-L386
> >>
> >> Because I cannot find such elsewhere, I assume it must be for the same 
> >> ARM virtualised environment as above.
> > No.
> >
> >> The binary it applies the Relocations to is documented to be the 
> >> Standalone MM core, but in fact SecCore is located:
> >>
> >> 

Re: [edk2-devel] ArmVirt and Self-Updating Code

[Marvin Häuser]

On 23.07.21 16:09, Ard Biesheuvel wrote:

On Fri, 23 Jul 2021 at 12:47, Marvin Häuser  wrote:

On 23.07.21 12:13, Ard Biesheuvel wrote:

On Fri, 23 Jul 2021 at 11:55, Marvin Häuser  wrote:

On 22.07.21 17:14, Ard Biesheuvel wrote:

On Thu, 22 Jul 2021 at 16:54, Bret Barkelew  wrote:

Expanding audience to the full dev list…

See below…



- Bret



From: Thomas Abraham
Sent: Wednesday, July 7, 2021 11:07 PM
To: Bret Barkelew; Ard Biesheuvel (TianoCore); Lindholm, Leif; Laszlo Ersek; 
Marvin Häuser; Sami Mujawar
Cc: nd
Subject: [EXTERNAL] RE: ArmVirt and Self-Updating Code



+ Sami



From: Bret Barkelew
Sent: Thursday, July 8, 2021 11:05 AM
To: Thomas Abraham; Ard Biesheuvel 
(TianoCore); Lindholm, Leif; Laszlo 
Ersek; Marvin Häuser
Subject: ArmVirt and Self-Updating Code



All,



Marvin asked me a question on the UEFI Talkbox Discord that’s a little beyond 
my ken…



“There is self-relocating code in ArmVirtPkg:

https://github.com/tianocore/edk2/blob/17143c4837393d42c484b42d1789b85b2cff1aaf/ArmVirtPkg/PrePi/PrePi.c#L133-L165

According to comments in the ASM, it seems like this is for Linux-based RAM 
boot (I saw further stuff for KVM, so it makes sense I guess?). It seems 
unfortunate it cannot be mapped into a known address range so that 
self-relocation is not necessary, but that's out of my scope to understand.


"Mapping" implies that the MMU is on, but this code boots with the MMU
off. Unlike x86, ARM does not define any physical address ranges that
are guaranteed to be backed by DRAM, so a portable image either needs
to be fully position independent, or carry the metadata it needs to
relocate itself as it is invoked.

And I understood it right that the idea is to use "-fpie" to
1) have all control flow instructions be position-independent (i.e.
jumps, calls, etc; ARM docs don't spill it out, but vaguely imply this
always is possible?), and

The primary reason to use -fpie and PIE linking is to ensure that the
resulting ELF executable contains a RELA section that describes every
location in the binary where a memory address is stored that needs to
be updated according to the actual placement in memory. The side
effect of -fpie is that position independent global references are
emitted (i.e., ADRP/ADD instructions which are relative to the program
counter). However, the AArch64 compiler uses those by default anyway,
so for this it is not strictly needed.


2) emit a GOT, which ends up being converted to PE/COFF Relocations (->
self-relocation), for global data that cannot be referenced relatively?
Is there any way to know/force that no symbol in GOT is accessed up
until the end of the self-relocation process?


Do you maybe have one final comment regarding that second question, 
please? :)
Let's drop "GOT" and make it "any instruction that requires prior 
relocation to function correctly".



It is not really a GOT. Actually, a GOT is undesirable, as it forces
global variables to be referenced via an absolute address, even when a
relative reference could be used.

Hmm, the GCC docs say a GOT is used for "all constant addresses" (I took
it as "absolute"?), it is kind of vague. I understood it this way:
1) no-pie emits relocations that can target the .text and .data sections
for instructions that embed and variables that hold an absolute address
(I thought this was RELA?)
2) pie emits a GOT such that there are no relocations as described in
1), because all absolute addresses are indirected by GOT (just GOT
references are relocated)


Correct. And this works really well for shared libraries, where all
text and data sections can be shared between processes, as they will
not be modified by the loader. All locations targeted by relocations
will be nicely lumped together in the GOT.

However, for bare metal style programs, there is no sharing, and there
is no advantage to lumping anything together. It is much better to use
relative references where possible, and simply apply relocations
wherever needed across the text and data sections,


If I understood the process right, but the term (GOT) is wrong, sorry,
that is what I gathered from the docs. :)
I have a x86 + PE background, so ARM + ELF is a bit of a learning curve...


The GOT is a special data structure used for implicit variable
accesses, i.e., global vars used in the code. Statically initialized
pointer variables are the other category, which are not code, and for
which the same considerations do not apply, given that the right value
simply needs to be stored in the variable before the program starts.


For instance, a statically initialized pointer always carries an
absolute address, and so it always needs an entry in the RELA table

E.g.,

int foo = 10; // external linkage
static int *bar = 

In this case, there is no way to use relative addressing because the
address of foo is taken at build time.

However, if bar would be something like

static int *bar() { return  }

the address is only taken at runtime, and the compiler can use a

Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

[Cheng-Chieh Huang via groups.io]

I got what you mean by "Tools_def.txt doesn’t support build flag" now. I
thought the parser does support some sort of if/else condition, but it
seems to be not the case. Alternatively, we can do this during the copy
(from template to tools_def.txt), but everytime we need to recreate
tools_def.txt which is not ideal).

In that case, I think we should just roll back to only doing this in
UefiPayloadPkg.dsc. If other packs want to add these flags, they will need
to do it on their own.

On Fri, Jul 23, 2021 at 6:29 PM gaoliming  wrote:

> How do you add this support in tools_def? Can you give the proposal for it?
>
>
>
> Thanks
>
> Liming
>
> *发件人:* Cheng-Chieh Huang 
> *发送时间:* 2021年7月22日 10:35
> *收件人:* gaoliming 
> *抄送:* devel@edk2.groups.io; Kinney, Michael D 
> *主题:* Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE
> to avoid generating floating points operation
>
>
>
> I mean, I will submit a patch to support DISABLE_GCC_MMX_SSE in tools_def.
> What do you think?
>
>
>
> --
>
> Cheng-Chieh
>
>
>
> On Thu, Jul 22, 2021 at 9:28 AM gaoliming 
> wrote:
>
> Tools_def.txt doesn’t support build flag DISABLE_GCC_MMX_SSE. If this
> flag is moved into BaseTools\Conf\tools_def.template, -mno-mmx -mno-sse
> option will be the default GCC options. That means all platforms will apply
> them.
>
>
>
> Thanks
>
> Liming
>
> *发件人:* devel@edk2.groups.io  *代表 *Cheng-Chieh Huang
> via groups.io
> *发送时间:* 2021年7月22日 1:43
> *收件人:* Kinney, Michael D 
> *抄送:* devel@edk2.groups.io
> *主题:* Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE
> to avoid generating floating points operation
>
>
>
> Yes, we can. I will drop this patch for this  uefipayload batch and send
> another one for support DISABLE_GCC_MMX_SSE in tools_de.txt.
>
>
>
> --
>
> Cheng-Chieh
>
> On Thu, Jul 22, 2021, 12:35 AM Kinney, Michael D <
> michael.d.kin...@intel.com> wrote:
>
> Are those flags needed for all packages that build with GCC?
>
> Should this be moved into tools_def.txt?
>
> Mike
>
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of
> Cheng-Chieh Huang via groups.io
> > Sent: Wednesday, July 21, 2021 6:23 AM
> > To: devel@edk2.groups.io
> > Cc: Cheng-Chieh Huang 
> > Subject: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE
> to avoid generating floating points operation
> >
> > This will allow we compile payload using gcc8
> >
> > Signed-off-by: Cheng-Chieh Huang 
> > ---
> >  UefiPayloadPkg/UefiPayloadPkg.dsc | 5 +
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc
> b/UefiPayloadPkg/UefiPayloadPkg.dsc
> > index 8aa5f18cd35c..fa41c5a24af5 100644
> > --- a/UefiPayloadPkg/UefiPayloadPkg.dsc
> > +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
> > @@ -30,6 +30,8 @@ [Defines]
> >DEFINE PS2_KEYBOARD_ENABLE  = FALSE
> >DEFINE UNIVERSAL_PAYLOAD= FALSE
> >
> > +  DEFINE DISABLE_MMX_SSE  = FALSE
> > +
> >#
> ># SBL:  UEFI payload for Slim Bootloader
> ># COREBOOT: UEFI payload for coreboot
> > @@ -96,6 +98,9 @@ [BuildOptions]
> >*_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
> >  !if $(BOOTLOADER) == "LINUXBOOT"
> >*_*_*_CC_FLAGS = -D LINUXBOOT_PAYLOAD
> > +!endif
> > +!if $(DISABLE_MMX_SSE)
> > +  *_*_*_CC_FLAGS = -mno-mmx -mno-sse
> >  !endif
> >GCC:*_UNIXGCC_*_CC_FLAGS   = -DMDEPKG_NDEBUG
> >GCC:RELEASE_*_*_CC_FLAGS   = -DMDEPKG_NDEBUG
> > --
> > 2.32.0.402.g57bb445576-goog
> >
> >
> >
> >
> >
>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78135): https://edk2.groups.io/g/devel/message/78135
Mute This Topic: https://groups.io/mt/84401664/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] ArmVirt and Self-Updating Code

[Ard Biesheuvel]

On Fri, 23 Jul 2021 at 12:47, Marvin Häuser  wrote:
>
> On 23.07.21 12:13, Ard Biesheuvel wrote:
> > On Fri, 23 Jul 2021 at 11:55, Marvin Häuser  wrote:
> >> On 22.07.21 17:14, Ard Biesheuvel wrote:
> >>> On Thu, 22 Jul 2021 at 16:54, Bret Barkelew  
> >>> wrote:
>  Expanding audience to the full dev list…
> 
>  See below…
> 
> 
> 
>  - Bret
> 
> 
> 
>  From: Thomas Abraham
>  Sent: Wednesday, July 7, 2021 11:07 PM
>  To: Bret Barkelew; Ard Biesheuvel (TianoCore); Lindholm, Leif; Laszlo 
>  Ersek; Marvin Häuser; Sami Mujawar
>  Cc: nd
>  Subject: [EXTERNAL] RE: ArmVirt and Self-Updating Code
> 
> 
> 
>  + Sami
> 
> 
> 
>  From: Bret Barkelew
>  Sent: Thursday, July 8, 2021 11:05 AM
>  To: Thomas Abraham; Ard Biesheuvel 
>  (TianoCore); Lindholm, 
>  Leif; Laszlo Ersek; Marvin 
>  Häuser
>  Subject: ArmVirt and Self-Updating Code
> 
> 
> 
>  All,
> 
> 
> 
>  Marvin asked me a question on the UEFI Talkbox Discord that’s a little 
>  beyond my ken…
> 
> 
> 
>  “There is self-relocating code in ArmVirtPkg:
> 
>  https://github.com/tianocore/edk2/blob/17143c4837393d42c484b42d1789b85b2cff1aaf/ArmVirtPkg/PrePi/PrePi.c#L133-L165
> 
>  According to comments in the ASM, it seems like this is for Linux-based 
>  RAM boot (I saw further stuff for KVM, so it makes sense I guess?). It 
>  seems unfortunate it cannot be mapped into a known address range so that 
>  self-relocation is not necessary, but that's out of my scope to 
>  understand.
> 
> >>> "Mapping" implies that the MMU is on, but this code boots with the MMU
> >>> off. Unlike x86, ARM does not define any physical address ranges that
> >>> are guaranteed to be backed by DRAM, so a portable image either needs
> >>> to be fully position independent, or carry the metadata it needs to
> >>> relocate itself as it is invoked.
> >> And I understood it right that the idea is to use "-fpie" to
> >> 1) have all control flow instructions be position-independent (i.e.
> >> jumps, calls, etc; ARM docs don't spill it out, but vaguely imply this
> >> always is possible?), and
> > The primary reason to use -fpie and PIE linking is to ensure that the
> > resulting ELF executable contains a RELA section that describes every
> > location in the binary where a memory address is stored that needs to
> > be updated according to the actual placement in memory. The side
> > effect of -fpie is that position independent global references are
> > emitted (i.e., ADRP/ADD instructions which are relative to the program
> > counter). However, the AArch64 compiler uses those by default anyway,
> > so for this it is not strictly needed.
> >
> >> 2) emit a GOT, which ends up being converted to PE/COFF Relocations (->
> >> self-relocation), for global data that cannot be referenced relatively?
> >> Is there any way to know/force that no symbol in GOT is accessed up
> >> until the end of the self-relocation process?
> >>
> > It is not really a GOT. Actually, a GOT is undesirable, as it forces
> > global variables to be referenced via an absolute address, even when a
> > relative reference could be used.
>
> Hmm, the GCC docs say a GOT is used for "all constant addresses" (I took
> it as "absolute"?), it is kind of vague. I understood it this way:
> 1) no-pie emits relocations that can target the .text and .data sections
> for instructions that embed and variables that hold an absolute address
> (I thought this was RELA?)
> 2) pie emits a GOT such that there are no relocations as described in
> 1), because all absolute addresses are indirected by GOT (just GOT
> references are relocated)
>

Correct. And this works really well for shared libraries, where all
text and data sections can be shared between processes, as they will
not be modified by the loader. All locations targeted by relocations
will be nicely lumped together in the GOT.

However, for bare metal style programs, there is no sharing, and there
is no advantage to lumping anything together. It is much better to use
relative references where possible, and simply apply relocations
wherever needed across the text and data sections,

> If I understood the process right, but the term (GOT) is wrong, sorry,
> that is what I gathered from the docs. :)
> I have a x86 + PE background, so ARM + ELF is a bit of a learning curve...
>

The GOT is a special data structure used for implicit variable
accesses, i.e., global vars used in the code. Statically initialized
pointer variables are the other category, which are not code, and for
which the same considerations do not apply, given that the right value
simply needs to be stored in the variable before the program starts.

> > For instance, a statically initialized pointer always carries an
> > absolute address, and so it always needs an entry in the RELA table
> >
> > E.g.,
> >
> > int foo = 

Re: [edk2-devel] [PATCH 3/3] IntelSiliconPkg: Add IgdOpRegion30.h to support IGD OpRegion v3.0

[Digant H Solanki]

Gentle reminder to review this patch pls. Thanks..!!

-Original Message-
From: Solanki, Digant H  
Sent: Thursday, July 22, 2021 5:17 PM
To: devel@edk2.groups.io
Cc: Solanki, Digant H ; Ni, Ray ; 
Chaganty, Rangasai V ; S, Ashraf Ali 

Subject: [PATCH 3/3] IntelSiliconPkg: Add IgdOpRegion30.h to support IGD 
OpRegion v3.0

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3426

- There are many OpRegion fields obsoleted in MBOX1
- MBOX2 is re-purposed for Backlight related fields for dual LFP.
- Backlight related fields moved to MBOX2 from MBOX3 and some fields are 
obsoleted in MBOX3.

Signed-off-by: Digant H Solanki 
Cc: Ray Ni 
Cc: Rangasai V Chaganty 
Cc: Ashraf Ali S 
---
 Silicon/Intel/IntelSiliconPkg/Include/IndustryStandard/IgdOpRegion30.h | 101 
+
 1 file changed, 101 insertions(+)

diff --git 
a/Silicon/Intel/IntelSiliconPkg/Include/IndustryStandard/IgdOpRegion30.h 
b/Silicon/Intel/IntelSiliconPkg/Include/IndustryStandard/IgdOpRegion30.h
new file mode 100644
index 00..c9948ab55f
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Include/IndustryStandard/IgdOpRegion
+++ 30.h
@@ -0,0 +1,101 @@
+/** @file
+  IGD OpRegion definition from Intel Integrated Graphics Device 
+OpRegion
+  Specification based on version 3.0.
+
+  Copyright (c) 2021, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+#ifndef _IGD_OPREGION_3_0_H_
+#define _IGD_OPREGION_3_0_H_
+
+#include "IgdOpRegion.h"
+
+#define IGD_OPREGION_HEADER_MBOX2_VER_3_0 BIT5
+
+#pragma pack(1)
+///
+/// OpRegion Mailbox 1 - Public ACPI Methods /// Offset 0x100, Size 
+0x100 /// typedef struct {
+  UINT32 DRDY;  ///< Offset 0x100 Driver Readiness
+  UINT32 CSTS;  ///< Offset 0x104 Status
+  UINT32 CEVT;  ///< Offset 0x108 Current Event
+  UINT8  RM11[0x14];///< Offset 0x10C Reserved Must be Zero
+  UINT32 DIDL[8];   ///< Offset 0x120 Supported Display Devices ID List
+  UINT32 CPDL[8];   ///< Offset 0x140 obsolete
+  UINT32 CADL[8];   ///< Offset 0x160 obsolete
+  UINT32 NADL[8];   ///< Offset 0x180 obsolete
+  UINT32 ASLP;  ///< Offset 0x1A0 ASL Sleep Time Out
+  UINT32 TIDX;  ///< Offset 0x1A4 obsolete
+  UINT32 CHPD;  ///< Offset 0x1A8 obsolete
+  UINT32 CLID;  ///< Offset 0x1AC Current Lid State Indicator
+  UINT32 CDCK;  ///< Offset 0x1B0 Current Docking State Indicator
+  UINT32 SXSW;  ///< Offset 0x1B4 obsolete
+  UINT32 EVTS;  ///< Offset 0x1B8 obsolete
+  UINT32 CNOT;  ///< Offset 0x1BC obsolete
+  UINT32 NRDY;  ///< Offset 0x1C0 Driver Status
+  UINT8  DID2[0x1C];///< Offset 0x1C4 Extended Supported Devices ID List 
(DOD)
+  UINT8  CPD2[0x1C];///< Offset 0x1E0 obsolete
+  UINT8  RM12[4];   ///< Offset 0x1FC - 0x1FF Reserved Must be zero
+} IGD_OPREGION_MBOX1_VER_3_0;
+
+///
+/// OpRegion Mailbox 2 - Backlight communication /// Offset 0x200, Size 
+0x100 /// typedef struct {
+  UINT32 BCL1;  ///< Offset 0x200 Backlight Brightness for LFP1
+  UINT32 BCL2;  ///< Offset 0x204 Backlight Brightness for LFP2
+  UINT32 CBL1;  ///< Offset 0x208 Current User Brightness Level for 
LFP1
+  UINT32 CBL2;  ///< Offset 0x20C Current User Brightness Level for 
LFP2
+  UINT32 BCM1[0x1E];///< Offset 0x210 Backlight Brightness Levels Duty 
Cycle Mapping Table for LFP1
+  UINT32 BCM2[0x1E];///< Offset 0x288 Backlight Brightness Levels Duty 
Cycle Mapping Table for LFP2
+} IGD_OPREGION_MBOX2_VER_3_0;
+
+///
+/// OpRegion Mailbox 3 - BIOS/Driver Notification - ASLE Support /// 
+Offset 0x300, Size 0x100 /// typedef struct {
+  UINT32 ARDY;  ///< Offset 0x300 obsolete
+  UINT32 ASLC;  ///< Offset 0x304 obsolete
+  UINT32 TCHE;  ///< Offset 0x308 obsolete
+  UINT32 ALSI;  ///< Offset 0x30C obsolete
+  UINT32 BCLP;  ///< Offset 0x310 obsoleted in ver 3.0, moved to 
Mailbox 2.
+  UINT32 PFIT;  ///< Offset 0x314 obsolete
+  UINT32 CBLV;  ///< Offset 0x318 obsoleted in ver 3.0, moved to 
Mailbox 2.
+  UINT16 BCLM[0x14];///< Offset 0x31C obsoleted in ver 3.0, moved to 
Mailbox 2.
+  UINT32 CPFM;  ///< Offset 0x344 obsolete
+  UINT32 EPFM;  ///< Offset 0x348 obsolete
+  UINT8  PLUT[0x4A];///< Offset 0x34C obsolete
+  UINT32 PFMB;  ///< Offset 0x396 obsolete
+  UINT32 CCDV;  ///< Offset 0x39A obsolete
+  UINT32 PCFT;  ///< Offset 0x39E obsolete
+  UINT32 SROT;  ///< Offset 0x3A2 obsolete
+  UINT32 IUER;  ///< Offset 0x3A6 obsolete
+  UINT64 FDSS;  ///< Offset 0x3AA obsolete
+  UINT32 FDSP;  ///< Offset 0x3B2 obsolete
+  UINT32 STAT;  ///< Offset 0x3B6 obsolete
+  UINT64 RVDA;  ///< Offset 0x3BA Physical address of Raw VBT data. 
Added from Spec Version 0.90 to support VBT greater than 6KB.
+  

Re: [edk2-devel] [PATCH] UefiCpuPkg: ResetVector Tool Support for Python 3

[Ashraf Ali S]

Hi., 
Based on Ray Feedback.

I have sent the patchset includes 1 and 2.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3506

Regards,
Ashraf Ali S
Intel Technology India Pvt. Ltd. 

-Original Message-
From: Ni, Ray  
Sent: Friday, July 23, 2021 8:03 AM
To: S, Ashraf Ali ; devel@edk2.groups.io
Cc: Kumar, Rahul1 ; De, Debkumar 
; Han, Harry ; West, Catharine 
; V, Sangeetha 
Subject: RE: [PATCH] UefiCpuPkg: ResetVector Tool Support for Python 3

Ashraf,
I assume that your previous ResetVector patch can be dropped, and we just 
review this one that supports python3 first, right?
Since this patch is for the feedbacks of patch separation, can you help to do 
it further?
1. only do minimal update to the python script to make it python3 capable. No 
additional printing.
2. add additional printing in 2nd patch
3. submit new .raw binary in a new patch serial because this binary update is 
not because of python3 but the out-of-dated binary.

To make it clear, 1 and 2 are two patches that belong to the same patch set.
3 is a separate patch.

Thanks,
Ray


> -Original Message-
> From: S, Ashraf Ali 
> Sent: Friday, July 23, 2021 12:41 AM
> To: devel@edk2.groups.io
> Cc: S, Ashraf Ali ; Ni, Ray 
> ; Kumar, Rahul1 ; De, 
> Debkumar ; Han, Harry ; 
> West, Catharine ; V, Sangeetha 
> 
> Subject: [PATCH] UefiCpuPkg: ResetVector Tool Support for Python 3
> 
> [edk2-devel] [PATCH]
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3506
> 
> Build Scrips for Reset Vector currently based on Python 2 which is 
> already EOL, needs to modify the build script based on Python 3, 
> update the Binary accordingly.
> 
> Change-Id: I2cfef08177951f2f29ee168ac4a9de9b10769ea1
> Cc: Ray Ni 
> Cc: Rahul Kumar 
> Cc: Debkumar De 
> Cc: Harry Han 
> Cc: Catharine West 
> Cc: Sangeetha V 
> Signed-off-by: Ashraf Ali S 
> ---
>  .../Vtf0/Bin/ResetVector.ia32.port80.raw  | Bin 516 -> 484 bytes
>  .../ResetVector/Vtf0/Bin/ResetVector.ia32.raw | Bin 484 -> 468 bytes
>  .../Vtf0/Bin/ResetVector.ia32.serial.raw  | Bin 884 -> 868 bytes
>  .../Vtf0/Bin/ResetVector.x64.port80.raw   | Bin 28676 -> 28676 bytes
>  .../ResetVector/Vtf0/Bin/ResetVector.x64.raw  | Bin 28676 -> 28676 bytes
>  .../Vtf0/Bin/ResetVector.x64.serial.raw   | Bin 28676 -> 28676 bytes
>  UefiCpuPkg/ResetVector/Vtf0/Build.py  |  11 +++
>  .../Vtf0/Tools/FixupForRawSection.py  |   4 ++--
>  8 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git 
> a/UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.ia32.port80.raw
> b/UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.ia32.port80.raw
> index 
> 2c6ff655ded2a5855ca8f4428d559a7727eb6983..79b23c047bdc6e552d77d5c9e9ae
> ae21ff04d91d 100644 GIT binary patch delta 410
> zcmZo+dBQ9-0SF8a=rRZ}FxWCMF#Invo+zYJ-&~=>PEYMB8#X*^*s
> zI*-2o*Lifq#%B#L{TUe;3~zVd>wJ;c9c#dNqsaO-vqOwyxZ<^$|Sx+*`qBE-KP
> zRw#MZ?IF_m@c;k+44fxR?lK-MVJf=bP$9%z%Jy2m^*||G=ZV*+3=ec3YyDQrw
> zhLV39>OTT4_yTke(6spG0}_@eiX$2-m<39tfuvB0QMW|nV~~MBTOFDYFc(>?{CR!5
> z`2b5=qlIr@Ka2ph)3jn)CK3=F06%+4CG<$;o!=g(0n4LMA4`}djk7m=n
> z@tSo9&>Du9B|zggh&^lgwVUBX-))iIZU6Ps_!-61b|^D2IPfbSNPCqzIiFFU^R?Xs
> zd4>r<#gi8>%0vL2z`!uOpJBgKz-zAkjsdS((>jm5W_tbeb@R)JfB*l#TmvLJAN+p?
> a3S}hl`Z9zAvN|lpjbXxs*L#qpCjbB+6v6TU
> 
> delta 442
> zcmX9)O-lk%6n)b;mb6f61Fgs7G?G=i3EW`h#0jTXjjt=xN`<_@e*RfKhRH@
> zRthehQp z2#|b@Yi!yt8wAow*Da-71;Y*UMJdG%{oGQ>0fSK3#P_%@6n3VVmKY~2sF%gXydlkT
> zz2J+}fsf;~_pRoa+J(fR`Ut;~>qat}3#muERkA!QyT~Xg^M>rg%^bM|McBYs`8V0A
> zcP(O;ogKlFmCBIg5bqJ
> zBhR=?>3OE6Mw4r>-vk>Al5t4>DR50tqp6HM5M^V1To7n?Y1=u`A{@A7c!=ymHGRlZ
> zJ}anuVpcRdDYzN0P#%MY-J^!^vR_OvBRp9GvF1f*Ah$29X~lhJI8j|qcKWL;$
> mb+{6FrzA&7=!a5r41gb~Ws5tejhbe+Ol`#xF!g`tAAbQ#M!vED
> 
> diff --git a/UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.ia32.raw 
> b/UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.ia32.raw
> index 
> e34780a3a2c9b22bd10a1d5a405e344faaff94f3..ce7faa502b858e99908bcdb397b7
> 76258205e1d5 100644 GIT binary patch delta 421 
> zcmaFDe1%zP0uUG;&}9%{V6bIiVEA8TJW)uczPUn$q4ZSeVde;h(!;MgckBm(
> zbsl}`)Ec)Rmn=Zm!NSOdlzMb@vG9g56a50n@+A7C%iRr%2sA^z>KLdmOc50S=)
> z|NsAI;5=D!m+@c;Q_=N?3L)lFw%6jV2TIvGPrN>5c%buG>$g&-l7BD10Br{v65o74
> z!m|EEaYRD}vp|V7kQ6F0>XvAH3^E94n?v<|1pAKd)~$A7DvqwD658)#cwFVZ(U1
> z(K^7DfuU5M*;(VYJW#Upa9X2vSX3z=volBY0S*4`(QKMGUbF51+Qaa&258)`-3%Z4
> zZtt%9ub0NpD4w=MnSsH9U+F;FtJKNSjMDY5-6qI0OaQ6_1rZE@G=l)pF$@fo_h
> zFuI>%zf-_#uKkVyuUXSNkGy7j{quG6%Zz{j|G(S NFyZwchzJ{m0sv1?z^?!R
> 
> literal 484
> zcmX9*OG_g`5Uz2YXi!K{AwfI@4Wb8^4I;k92Z}6+5k#W02QLkK9j9Rq9_
> zqIePigaaNjIzCSdixLS)RFt&2cybqA?5!~cU5~H6s_N>tZQD+`9S{Z>1OTb`GBa#G
> zt*_G(GaClinx^RkGo#yGe2LyNvnlO${H9mTj3XK78TZswjJl#0BPWZ(PsE3m63x5<
> zkjV2pUL={H-<6y`Ayi}y>qBYR=+mmu*E{2X*HV!;FJ=@olMU=1D z>r@*9G|11z5fTzEKTW^U3gc6Jd}Rz>i=xwezWi&|RKrFLb)7MgiLyt(A5Nap
> z{K@){_&;%jkXDHiVLej|v^%t)8c;mepB%?^+SRc((Td402KNZ-pIe~y>R7ebhG=Mi
> zG0>h98oCZ15CogOAHb`XKiHDrNJxngrv+CGHM`_x1(RWLh67mGTp&(0SUJnJ3Rcm&
> z65UvCM_?B@w(a-w1uqM*d0DnQmyjJzmTIyi$x?vuV|+aEM~V$8raq+ TrM$1pedcB-0FmP|Qr7 
> diff --git 
> 

Re: [edk2-devel] [PATCH V2 4/4] OvmfPkg/ResetVector: Update ResetVector to support Tdx

[Lendacky, Thomas via groups.io]

On 7/22/21 5:58 PM, Xu, Min M wrote:
> On July 23, 2021 1:08 AM, Tom Lendacky wrote:
>> On 7/22/21 12:52 AM, Min Xu wrote:
>>> RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
>>>
>>> diff --git a/OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
>>> b/OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
>>> index c6d0d898bcd1..2206ca719593 100644
>>> --- a/OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
>>> +++ b/OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
>>> @@ -17,6 +17,9 @@ Transition32FlatTo64Flat:
>>>
>>>  OneTimeCall SetCr3ForPageTables64
>>>
>>> +cmp dword[TDX_WORK_AREA], 0x47584454 ; 'TDXG'
>>> +jz  TdxTransition32FlatTo64Flat
>>> +
>>
>> Is the memory area guaranteed to be zeroed for legacy guests? Hopefully,
>> this won't trip up a non-TDX guest with a false match (highly unlikely, 
>> though).
>>
> TDX_WORK_AREA is piece of TdxMailbox which is located in the MEMFD started
> from PcdOvmfSecGhcbBackupBase. In Td guest, this memory region is initialized
> to all-0 by host VMM. In legacy guests, I am not sure what's the initialized 
> value
> it is. So 'TDXG' is checked to guarantee it is Td-guest or not. 
> Since Tdx re-use the memory region (PcdOvmfSecGhcbBackupBase) as the
> TDX_WORK_AREA, and @Tom Lendacky you should be the original owner of
> PcdOvmfSecGhcbBackupBase, can this area be cleared in the beginning of
> ResetVector in legacy guests? Or I should better create a TDX specific work
> area in MEMFD to guarantee the Td And Non-Td check?

I believe PcdOvmfSecGhcbBackupBase can be cleared early. For SEV-ES, it
isn't shared with the hypervisor, so clearing it before activating the
pagetables can be done (it will be treated as encrypted before paging is
enabled and mapped as encrypted after paging is enabled) and for a legacy
guest the mapping doesn't matter. It isn't required to be cleared today,
so if you do add something, be sure to put a comment in there about why
it's being done. No need for a new area.

The possibility of random data being there that matches 'TDXG' is
extremely low. But better safe than sorry, I guess.

Thanks,
Tom

>>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78131): https://edk2.groups.io/g/devel/message/78131
Mute This Topic: https://groups.io/mt/84373830/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] ArmVirt and Self-Updating Code

[Marvin Häuser]

On 23.07.21 12:13, Ard Biesheuvel wrote:

On Fri, 23 Jul 2021 at 11:55, Marvin Häuser  wrote:

On 22.07.21 17:14, Ard Biesheuvel wrote:

On Thu, 22 Jul 2021 at 16:54, Bret Barkelew  wrote:

Expanding audience to the full dev list…

See below…



- Bret



From: Thomas Abraham
Sent: Wednesday, July 7, 2021 11:07 PM
To: Bret Barkelew; Ard Biesheuvel (TianoCore); Lindholm, Leif; Laszlo Ersek; 
Marvin Häuser; Sami Mujawar
Cc: nd
Subject: [EXTERNAL] RE: ArmVirt and Self-Updating Code



+ Sami



From: Bret Barkelew
Sent: Thursday, July 8, 2021 11:05 AM
To: Thomas Abraham; Ard Biesheuvel 
(TianoCore); Lindholm, Leif; Laszlo 
Ersek; Marvin Häuser
Subject: ArmVirt and Self-Updating Code



All,



Marvin asked me a question on the UEFI Talkbox Discord that’s a little beyond 
my ken…



“There is self-relocating code in ArmVirtPkg:

https://github.com/tianocore/edk2/blob/17143c4837393d42c484b42d1789b85b2cff1aaf/ArmVirtPkg/PrePi/PrePi.c#L133-L165

According to comments in the ASM, it seems like this is for Linux-based RAM 
boot (I saw further stuff for KVM, so it makes sense I guess?). It seems 
unfortunate it cannot be mapped into a known address range so that 
self-relocation is not necessary, but that's out of my scope to understand.


"Mapping" implies that the MMU is on, but this code boots with the MMU
off. Unlike x86, ARM does not define any physical address ranges that
are guaranteed to be backed by DRAM, so a portable image either needs
to be fully position independent, or carry the metadata it needs to
relocate itself as it is invoked.

And I understood it right that the idea is to use "-fpie" to
1) have all control flow instructions be position-independent (i.e.
jumps, calls, etc; ARM docs don't spill it out, but vaguely imply this
always is possible?), and

The primary reason to use -fpie and PIE linking is to ensure that the
resulting ELF executable contains a RELA section that describes every
location in the binary where a memory address is stored that needs to
be updated according to the actual placement in memory. The side
effect of -fpie is that position independent global references are
emitted (i.e., ADRP/ADD instructions which are relative to the program
counter). However, the AArch64 compiler uses those by default anyway,
so for this it is not strictly needed.


2) emit a GOT, which ends up being converted to PE/COFF Relocations (->
self-relocation), for global data that cannot be referenced relatively?
Is there any way to know/force that no symbol in GOT is accessed up
until the end of the self-relocation process?


It is not really a GOT. Actually, a GOT is undesirable, as it forces
global variables to be referenced via an absolute address, even when a
relative reference could be used.


Hmm, the GCC docs say a GOT is used for "all constant addresses" (I took 
it as "absolute"?), it is kind of vague. I understood it this way:
1) no-pie emits relocations that can target the .text and .data sections 
for instructions that embed and variables that hold an absolute address 
(I thought this was RELA?)
2) pie emits a GOT such that there are no relocations as described in 
1), because all absolute addresses are indirected by GOT (just GOT 
references are relocated)


If I understood the process right, but the term (GOT) is wrong, sorry, 
that is what I gathered from the docs. :)

I have a x86 + PE background, so ARM + ELF is a bit of a learning curve...


For instance, a statically initialized pointer always carries an
absolute address, and so it always needs an entry in the RELA table

E.g.,

int foo = 10; // external linkage
static int *bar = 

In this case, there is no way to use relative addressing because the
address of foo is taken at build time.

However, if bar would be something like

static int *bar() { return  }

the address is only taken at runtime, and the compiler can use a
relative reference instead, and no RELA entry is needed. With a GOT,
we force the compiler to allocate a variable that holds the absolute
address, which we would prefer to avoid.


And this is not forced by whatever table -fpie uses, as per my 
understanding above?



“Now, StandaloneMmPkg has similar (self-)relocation code 
too:https://github.com/tianocore/edk2/blob/17143c4837393d42c484b42d1789b85b2cff1aaf/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c#L379-L386

Because I cannot find such elsewhere, I assume it must be for the same ARM 
virtualised environment as above.

No.


The binary it applies the Relocations to is documented to be the Standalone MM 
core, but in fact SecCore is located:

https://github.com/tianocore/edk2/blob/17143c4837393d42c484b42d1789b85b2cff1aaf/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c#L131-L158

As per your comments below, I think SecCore should not be located here.
Is the Standalone MM core of *type* SecCore in the FFS (without *being*
SecCore)? This confused me the most.


If the FFS SecCore section type is 

回复: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

[gaoliming]

How do you add this support in tools_def? Can you give the proposal for it?

 

Thanks

Liming

发件人: Cheng-Chieh Huang  
发送时间: 2021年7月22日 10:35
收件人: gaoliming 
抄送: devel@edk2.groups.io; Kinney, Michael D 
主题: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to 
avoid generating floating points operation

 

I mean, I will submit a patch to support DISABLE_GCC_MMX_SSE in tools_def. What 
do you think?

 

--

Cheng-Chieh

 

On Thu, Jul 22, 2021 at 9:28 AM gaoliming mailto:gaolim...@byosoft.com.cn> > wrote:

Tools_def.txt doesn’t support build flag DISABLE_GCC_MMX_SSE. If this flag is 
moved into BaseTools\Conf\tools_def.template, -mno-mmx -mno-sse option will be 
the default GCC options. That means all platforms will apply them. 

 

Thanks

Liming

发件人: devel@edk2.groups.io   mailto:devel@edk2.groups.io> > 代表 Cheng-Chieh Huang via groups.io 
 
发送时间: 2021年7月22日 1:43
收件人: Kinney, Michael D mailto:michael.d.kin...@intel.com> >
抄送: devel@edk2.groups.io  
主题: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to 
avoid generating floating points operation

 

Yes, we can. I will drop this patch for this  uefipayload batch and send 
another one for support DISABLE_GCC_MMX_SSE in tools_de.txt.

 

--

Cheng-Chieh 

On Thu, Jul 22, 2021, 12:35 AM Kinney, Michael D mailto:michael.d.kin...@intel.com> > wrote:

Are those flags needed for all packages that build with GCC?

Should this be moved into tools_def.txt?

Mike

> -Original Message-
> From: devel@edk2.groups.io   
> mailto:devel@edk2.groups.io> > On Behalf Of 
> Cheng-Chieh Huang via groups.io  
> Sent: Wednesday, July 21, 2021 6:23 AM
> To: devel@edk2.groups.io  
> Cc: Cheng-Chieh Huang mailto:chengch...@google.com> >
> Subject: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to 
> avoid generating floating points operation
> 
> This will allow we compile payload using gcc8
> 
> Signed-off-by: Cheng-Chieh Huang   >
> ---
>  UefiPayloadPkg/UefiPayloadPkg.dsc | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc 
> b/UefiPayloadPkg/UefiPayloadPkg.dsc
> index 8aa5f18cd35c..fa41c5a24af5 100644
> --- a/UefiPayloadPkg/UefiPayloadPkg.dsc
> +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
> @@ -30,6 +30,8 @@ [Defines]
>DEFINE PS2_KEYBOARD_ENABLE  = FALSE
>DEFINE UNIVERSAL_PAYLOAD= FALSE
> 
> +  DEFINE DISABLE_MMX_SSE  = FALSE
> +
>#
># SBL:  UEFI payload for Slim Bootloader
># COREBOOT: UEFI payload for coreboot
> @@ -96,6 +98,9 @@ [BuildOptions]
>*_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
>  !if $(BOOTLOADER) == "LINUXBOOT"
>*_*_*_CC_FLAGS = -D LINUXBOOT_PAYLOAD
> +!endif
> +!if $(DISABLE_MMX_SSE)
> +  *_*_*_CC_FLAGS = -mno-mmx -mno-sse
>  !endif
>GCC:*_UNIXGCC_*_CC_FLAGS   = -DMDEPKG_NDEBUG
>GCC:RELEASE_*_*_CC_FLAGS   = -DMDEPKG_NDEBUG
> --
> 2.32.0.402.g57bb445576-goog
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78129): https://edk2.groups.io/g/devel/message/78129
Mute This Topic: https://groups.io/mt/84398177/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] ArmVirt and Self-Updating Code

[Ard Biesheuvel]

On Fri, 23 Jul 2021 at 11:55, Marvin Häuser  wrote:
>
> On 22.07.21 17:14, Ard Biesheuvel wrote:
> > On Thu, 22 Jul 2021 at 16:54, Bret Barkelew  
> > wrote:
> >> Expanding audience to the full dev list…
> >>
> >> See below…
> >>
> >>
> >>
> >> - Bret
> >>
> >>
> >>
> >> From: Thomas Abraham
> >> Sent: Wednesday, July 7, 2021 11:07 PM
> >> To: Bret Barkelew; Ard Biesheuvel (TianoCore); Lindholm, Leif; Laszlo 
> >> Ersek; Marvin Häuser; Sami Mujawar
> >> Cc: nd
> >> Subject: [EXTERNAL] RE: ArmVirt and Self-Updating Code
> >>
> >>
> >>
> >> + Sami
> >>
> >>
> >>
> >> From: Bret Barkelew
> >> Sent: Thursday, July 8, 2021 11:05 AM
> >> To: Thomas Abraham; Ard Biesheuvel 
> >> (TianoCore); Lindholm, Leif; 
> >> Laszlo Ersek; Marvin Häuser
> >> Subject: ArmVirt and Self-Updating Code
> >>
> >>
> >>
> >> All,
> >>
> >>
> >>
> >> Marvin asked me a question on the UEFI Talkbox Discord that’s a little 
> >> beyond my ken…
> >>
> >>
> >>
> >> “There is self-relocating code in ArmVirtPkg:
> >>
> >> https://github.com/tianocore/edk2/blob/17143c4837393d42c484b42d1789b85b2cff1aaf/ArmVirtPkg/PrePi/PrePi.c#L133-L165
> >>
> >> According to comments in the ASM, it seems like this is for Linux-based 
> >> RAM boot (I saw further stuff for KVM, so it makes sense I guess?). It 
> >> seems unfortunate it cannot be mapped into a known address range so that 
> >> self-relocation is not necessary, but that's out of my scope to understand.
> >>
> > "Mapping" implies that the MMU is on, but this code boots with the MMU
> > off. Unlike x86, ARM does not define any physical address ranges that
> > are guaranteed to be backed by DRAM, so a portable image either needs
> > to be fully position independent, or carry the metadata it needs to
> > relocate itself as it is invoked.
>
> And I understood it right that the idea is to use "-fpie" to
> 1) have all control flow instructions be position-independent (i.e.
> jumps, calls, etc; ARM docs don't spill it out, but vaguely imply this
> always is possible?), and

The primary reason to use -fpie and PIE linking is to ensure that the
resulting ELF executable contains a RELA section that describes every
location in the binary where a memory address is stored that needs to
be updated according to the actual placement in memory. The side
effect of -fpie is that position independent global references are
emitted (i.e., ADRP/ADD instructions which are relative to the program
counter). However, the AArch64 compiler uses those by default anyway,
so for this it is not strictly needed.

> 2) emit a GOT, which ends up being converted to PE/COFF Relocations (->
> self-relocation), for global data that cannot be referenced relatively?
> Is there any way to know/force that no symbol in GOT is accessed up
> until the end of the self-relocation process?
>

It is not really a GOT. Actually, a GOT is undesirable, as it forces
global variables to be referenced via an absolute address, even when a
relative reference could be used.

For instance, a statically initialized pointer always carries an
absolute address, and so it always needs an entry in the RELA table

E.g.,

int foo = 10; // external linkage
static int *bar = 

In this case, there is no way to use relative addressing because the
address of foo is taken at build time.

However, if bar would be something like

static int *bar() { return  }

the address is only taken at runtime, and the compiler can use a
relative reference instead, and no RELA entry is needed. With a GOT,
we force the compiler to allocate a variable that holds the absolute
address, which we would prefer to avoid.

> >> “Now, StandaloneMmPkg has similar (self-)relocation code 
> >> too:https://github.com/tianocore/edk2/blob/17143c4837393d42c484b42d1789b85b2cff1aaf/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c#L379-L386
> >>
> >> Because I cannot find such elsewhere, I assume it must be for the same ARM 
> >> virtualised environment as above.
> > No.
> >
> >> The binary it applies the Relocations to is documented to be the 
> >> Standalone MM core, but in fact SecCore is located:
> >>
> >> https://github.com/tianocore/edk2/blob/17143c4837393d42c484b42d1789b85b2cff1aaf/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c#L131-L158
>
> As per your comments below, I think SecCore should not be located here.
> Is the Standalone MM core of *type* SecCore in the FFS (without *being*
> SecCore)? This confused me the most.
>

If the FFS SecCore section type is used here, it does not mean that
the image is a SEC image in the strict PI sense.

Perhaps we were just too lazy to add a new type to the FFS spec?

> >> “This yields the following questions to me:
> >>
> >> 1) What even invokes Standalone MM on ARM? It is documented it is spawned 
> >> during SEC, but I could not find any actual invocation.
> >>
> > It is not spawned by the normal world code that runs UEFI. It is a
> > secure world component that runs in a completely 

Re: [edk2-devel] ArmVirt and Self-Updating Code

[Marvin Häuser]

On 22.07.21 17:14, Ard Biesheuvel wrote:

On Thu, 22 Jul 2021 at 16:54, Bret Barkelew  wrote:

Expanding audience to the full dev list…

See below…



- Bret



From: Thomas Abraham
Sent: Wednesday, July 7, 2021 11:07 PM
To: Bret Barkelew; Ard Biesheuvel (TianoCore); Lindholm, Leif; Laszlo Ersek; 
Marvin Häuser; Sami Mujawar
Cc: nd
Subject: [EXTERNAL] RE: ArmVirt and Self-Updating Code



+ Sami



From: Bret Barkelew
Sent: Thursday, July 8, 2021 11:05 AM
To: Thomas Abraham; Ard Biesheuvel 
(TianoCore); Lindholm, Leif; Laszlo 
Ersek; Marvin Häuser
Subject: ArmVirt and Self-Updating Code



All,



Marvin asked me a question on the UEFI Talkbox Discord that’s a little beyond 
my ken…



“There is self-relocating code in ArmVirtPkg:

https://github.com/tianocore/edk2/blob/17143c4837393d42c484b42d1789b85b2cff1aaf/ArmVirtPkg/PrePi/PrePi.c#L133-L165

According to comments in the ASM, it seems like this is for Linux-based RAM 
boot (I saw further stuff for KVM, so it makes sense I guess?). It seems 
unfortunate it cannot be mapped into a known address range so that 
self-relocation is not necessary, but that's out of my scope to understand.


"Mapping" implies that the MMU is on, but this code boots with the MMU
off. Unlike x86, ARM does not define any physical address ranges that
are guaranteed to be backed by DRAM, so a portable image either needs
to be fully position independent, or carry the metadata it needs to
relocate itself as it is invoked.


And I understood it right that the idea is to use "-fpie" to
1) have all control flow instructions be position-independent (i.e. 
jumps, calls, etc; ARM docs don't spill it out, but vaguely imply this 
always is possible?), and
2) emit a GOT, which ends up being converted to PE/COFF Relocations (-> 
self-relocation), for global data that cannot be referenced relatively? 
Is there any way to know/force that no symbol in GOT is accessed up 
until the end of the self-relocation process?



“Now, StandaloneMmPkg has similar (self-)relocation code 
too:https://github.com/tianocore/edk2/blob/17143c4837393d42c484b42d1789b85b2cff1aaf/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c#L379-L386

Because I cannot find such elsewhere, I assume it must be for the same ARM 
virtualised environment as above.

No.


The binary it applies the Relocations to is documented to be the Standalone MM 
core, but in fact SecCore is located:

https://github.com/tianocore/edk2/blob/17143c4837393d42c484b42d1789b85b2cff1aaf/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c#L131-L158


As per your comments below, I think SecCore should not be located here. 
Is the Standalone MM core of *type* SecCore in the FFS (without *being* 
SecCore)? This confused me the most.



“This yields the following questions to me:

1) What even invokes Standalone MM on ARM? It is documented it is spawned 
during SEC, but I could not find any actual invocation.


It is not spawned by the normal world code that runs UEFI. It is a
secure world component that runs in a completely different execution
context (TrustZone). The code does run with the MMU enabled from the
start, but running from an a priori fixed offset was considered to be
a security hazard, so we added self relocation support.

The alternative would have been to add metadata to the StMmCore
component that can be interpreted by the secure world component that
loads it, but this would go beyond any existing specs, and make
portability more problematic.


2) Why does Standalone MM (self-)relocation locate SecCore? Should it not 
already have been relocated with the code from ArmPlatformPkg? Is Standalone MM 
embedded into ARM SecCore?


No and no. Standalone MM has nothing to do with the code that runs as
part of UEFI itself. ArmPlatformPkg is completely separate from
StandaloneMmPkg.


3) Why is SecCore the only module relocated? Are all others guaranteed to be 
"properly" loaded?


SecCore contains a PE/COFF loader, so all subsequent modules are
loaded normally. This is similar to the ArmVirtQemuKernel
self-relocating SEC module, which only relocates itself in this
manner, and relies on standard PE/COFF metadata for loading other
modules.


Interesting... this definitely is vastly different from the x86 side of 
things. I think most things became very clear. Thanks a lot!



4) Is there maybe some high-level documented about the ARM boot flow? It seems 
to be significantly different from the x86 routes quite vastly.”


trustedfirmware.org may have some useful documentation.


I'll check it some time, hopefully this weekend. Thanks!

Best regards,
Marvin


Hoping that one of you could get me closer to an answer for him. Also happy to 
take this to the greater mailing list, but thought I’d avoid churn.



Thanks in advance!

- Bret








-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78127): 

[edk2-devel] [edk2-platform PATCH v1 1/1] Platform/RaspberryPi: Make SetVariable return EFI_UNSUPPORTED at runtime

[Sunny Wang]

The RPi does not support storing UEFI NV variables at runtime. For now,
gRT->SetVariable at runtime returns EFI_OUT_OF_RESOURCES which is not a
proper error and would cause FWTS failures. Therefore, this patch is
to make gRT->SetVariable at runtime return EFI_UNSUPPORTED.

For more information, please check the issues below:
   -https://github.com/pftf/RPi4/issues/6
   -https://github.com/pftf/RPi4/issues/93
   -https://github.com/pftf/RPi4/issues/163

I also tested this with the ACS 3.0 FWTS. All the failures
reported in issue 93 and 163 can be fixed by this patch.

Cc: Samer El-Haj-Mahmoud 
Cc: Sami Mujawar 
Cc: Jeremy Linton 
Cc: Ard Biesheuvel 
Cc: Pete Batard 
Cc: Leif Lindholm 

Signed-off-by: Sunny Wang 
---
 .../Drivers/VarBlockServiceDxe/VarBlockService.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c 
b/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
index 572309439a..16d4d4f178 100644
--- a/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
+++ b/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
@@ -2,6 +2,7 @@
  *
  *  Copyright (c) 2018, Andrei Warkentin 
  *  Copyright (c) 2006-2014, Intel Corporation. All rights reserved.
+ *  Copyright (c) 2021, ARM Limited. All rights reserved.
  *
  *  SPDX-License-Identifier: BSD-2-Clause-Patent
  *
@@ -596,6 +597,7 @@ FvbProtocolWrite (
 EFI_DEVICE_ERROR  - The block device is not functioning correctly and
 could not be written
 EFI_INVALID_PARAMETER - NumBytes or Buffer are NULL
+EFI_UNSUPPORTED This function is not supported at runtime
 
 --*/
 {
@@ -605,6 +607,16 @@ FvbProtocolWrite (
   EFI_STATUS Status;
   EFI_STATUS ReturnStatus;
 
+  //
+  // The current variables support relies on modifying RPI_EFI.FD on SD
+  // card, which works fine at boot time. However, at runtime, the SD 
+  // controller is exposed via ACPI and subsequently owned by the OS.
+  // Therefore, we need to direclty return EFI_UNSUPPORTED.
+  //
+  if (EfiAtRuntime ()) {
+   return EFI_UNSUPPORTED;
+  }
+
   //
   // Check for invalid conditions.
   //
-- 
2.31.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78126): https://edk2.groups.io/g/devel/message/78126
Mute This Topic: https://groups.io/mt/84397460/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platform][PATCH v1 1/1] EmbeddedPkg/VirtualRealTimeClockLib : Fix SetTime issues

[Sunny Wang]

Sorry, please ignore this one. I sent the patch with the wrong subject. This 
change is made in edk2 rather than the edk2-platform.
Please review the later one "[edk2-devel] [PATCH 1/1] 
EmbeddedPkg/VirtualRealTimeClockLib : Fix SetTime issues". Thanks!
https://edk2.groups.io/g/devel/topic/patch_1_1/84397263

Best Regards,
Sunny Wang

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Sunny Wang via 
groups.io
Sent: Friday, July 23, 2021 5:01 PM
To: devel@edk2.groups.io
Cc: Sunny Wang ; Samer El-Haj-Mahmoud 
; Sami Mujawar ; Jeremy 
Linton ; Ard Biesheuvel ; 
Pete Batard ; Leif Lindholm ; Sunny Wang 

Subject: [edk2-devel] [edk2-platform][PATCH v1 1/1] 
EmbeddedPkg/VirtualRealTimeClockLib : Fix SetTime issues

This patch fixes two issues below:
1. SCT SetTime_Func failures.
   - https://github.com/pftf/RPi4/issues/164
2. Using shell time and date commands to set time can't work.

The problem is that gRT->SetTime always returns EFI_INVALID_PARAMETER
error status.

The root cause is that LibSetTime() sets RtcEpochSeconds variable with
inconsistent attributes. One is without EFI_VARIABLE_NON_VOLATILE,
the other one is with EFI_VARIABLE_NON_VOLATILE. That caused that the
variable driver returns EFI_INVALID_PARAMETER. Per UEFI spec, if a
preexisting variable is rewritten with different attributes,
SetVariable() shall not modify the variable and shall return
EFI_INVALID_PARAMETER.

Therefore, the solution is to add EFI_VARIABLE_NON_VOLATILE attribute
to the first EfiSetVariable() call to make two calls consistent.

By the way, this patch also fix a minor issue with a debug message.

Cc: Samer El-Haj-Mahmoud 
Cc: Sami Mujawar 
Cc: Jeremy Linton 
Cc: Ard Biesheuvel 
Cc: Pete Batard 
Cc: Leif Lindholm 

Signed-off-by: Sunny Wang 
---
 .../VirtualRealTimeClockLib/VirtualRealTimeClockLib.c   | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git 
a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c 
b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
index de6fbb40e6..c10c91bc75 100644
--- a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
+++ b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
@@ -4,7 +4,7 @@
  *
  *  Coypright (c) 2019, Pete Batard 
  *  Copyright (c) 2018, Andrei Warkentin 
- *  Copyright (c) 2011-2014, ARM Ltd. All rights reserved.
+ *  Copyright (c) 2011-2021, ARM Ltd. All rights reserved.
  *  Copyright (c) 2008-2010, Apple Inc. All rights reserved.
  *  Copyright (c) Microsoft Corporation. All rights reserved.
  *
@@ -96,7 +96,7 @@ LibGetTime (
 EfiSetVariable (
   (CHAR16 *)mEpochVariableName,
   ,
-  EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
+  EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS,
   sizeof (EpochSeconds),
   
   );
@@ -324,7 +324,7 @@ LibSetTime (
 DEBUG ((
   DEBUG_ERROR,
   "LibSetTime: Failed to save %s variable to non-volatile storage, Status 
= %r\n",
-  mDaylightVariableName,
+  mEpochVariableName,
   Status
   ));
 return Status;
--
2.31.0.windows.1



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78123): https://edk2.groups.io/g/devel/message/78123
Mute This Topic: https://groups.io/mt/84397232/5985097
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [sunny.w...@arm.com]
-=-=-=-=-=-=


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.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78125): https://edk2.groups.io/g/devel/message/78125
Mute This Topic: https://groups.io/mt/84397232/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 1/1] EmbeddedPkg/VirtualRealTimeClockLib : Fix SetTime issues

[Sunny Wang]

This patch fixes two issues below:
1. SCT SetTime_Func failures.
   - https://github.com/pftf/RPi4/issues/164
2. Using shell time and date commands to set time can't work.

The problem is that gRT->SetTime always returns EFI_INVALID_PARAMETER
error status.

The root cause is that LibSetTime() sets RtcEpochSeconds variable with
inconsistent attributes. One is without EFI_VARIABLE_NON_VOLATILE,
the other one is with EFI_VARIABLE_NON_VOLATILE. That caused that the
variable driver returns EFI_INVALID_PARAMETER. Per UEFI spec, if a
preexisting variable is rewritten with different attributes,
SetVariable() shall not modify the variable and shall return
EFI_INVALID_PARAMETER.

Therefore, the solution is to add EFI_VARIABLE_NON_VOLATILE attribute
to the first EfiSetVariable() call to make two calls consistent.

By the way, this patch also fix a minor issue with a debug message.

Cc: Samer El-Haj-Mahmoud 
Cc: Sami Mujawar 
Cc: Jeremy Linton 
Cc: Ard Biesheuvel 
Cc: Pete Batard 
Cc: Leif Lindholm 

Signed-off-by: Sunny Wang 
---
 .../VirtualRealTimeClockLib/VirtualRealTimeClockLib.c   | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git 
a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c 
b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
index de6fbb40e6..c10c91bc75 100644
--- a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
+++ b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
@@ -4,7 +4,7 @@
  *
  *  Coypright (c) 2019, Pete Batard 
  *  Copyright (c) 2018, Andrei Warkentin 
- *  Copyright (c) 2011-2014, ARM Ltd. All rights reserved.
+ *  Copyright (c) 2011-2021, ARM Ltd. All rights reserved.
  *  Copyright (c) 2008-2010, Apple Inc. All rights reserved.
  *  Copyright (c) Microsoft Corporation. All rights reserved.
  *
@@ -96,7 +96,7 @@ LibGetTime (
 EfiSetVariable (
   (CHAR16 *)mEpochVariableName,
   ,
-  EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
+  EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS,
   sizeof (EpochSeconds),
   
   );
@@ -324,7 +324,7 @@ LibSetTime (
 DEBUG ((
   DEBUG_ERROR,
   "LibSetTime: Failed to save %s variable to non-volatile storage, Status 
= %r\n",
-  mDaylightVariableName,
+  mEpochVariableName,
   Status
   ));
 return Status;
-- 
2.31.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78124): https://edk2.groups.io/g/devel/message/78124
Mute This Topic: https://groups.io/mt/84397263/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [edk2-platform][PATCH v1 1/1] EmbeddedPkg/VirtualRealTimeClockLib : Fix SetTime issues

[Sunny Wang]

This patch fixes two issues below:
1. SCT SetTime_Func failures.
   - https://github.com/pftf/RPi4/issues/164
2. Using shell time and date commands to set time can't work.

The problem is that gRT->SetTime always returns EFI_INVALID_PARAMETER
error status.

The root cause is that LibSetTime() sets RtcEpochSeconds variable with
inconsistent attributes. One is without EFI_VARIABLE_NON_VOLATILE,
the other one is with EFI_VARIABLE_NON_VOLATILE. That caused that the
variable driver returns EFI_INVALID_PARAMETER. Per UEFI spec, if a
preexisting variable is rewritten with different attributes,
SetVariable() shall not modify the variable and shall return
EFI_INVALID_PARAMETER.

Therefore, the solution is to add EFI_VARIABLE_NON_VOLATILE attribute
to the first EfiSetVariable() call to make two calls consistent.

By the way, this patch also fix a minor issue with a debug message.

Cc: Samer El-Haj-Mahmoud 
Cc: Sami Mujawar 
Cc: Jeremy Linton 
Cc: Ard Biesheuvel 
Cc: Pete Batard 
Cc: Leif Lindholm 

Signed-off-by: Sunny Wang 
---
 .../VirtualRealTimeClockLib/VirtualRealTimeClockLib.c   | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git 
a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c 
b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
index de6fbb40e6..c10c91bc75 100644
--- a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
+++ b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
@@ -4,7 +4,7 @@
  *
  *  Coypright (c) 2019, Pete Batard 
  *  Copyright (c) 2018, Andrei Warkentin 
- *  Copyright (c) 2011-2014, ARM Ltd. All rights reserved.
+ *  Copyright (c) 2011-2021, ARM Ltd. All rights reserved.
  *  Copyright (c) 2008-2010, Apple Inc. All rights reserved.
  *  Copyright (c) Microsoft Corporation. All rights reserved.
  *
@@ -96,7 +96,7 @@ LibGetTime (
 EfiSetVariable (
   (CHAR16 *)mEpochVariableName,
   ,
-  EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
+  EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS,
   sizeof (EpochSeconds),
   
   );
@@ -324,7 +324,7 @@ LibSetTime (
 DEBUG ((
   DEBUG_ERROR,
   "LibSetTime: Failed to save %s variable to non-volatile storage, Status 
= %r\n",
-  mDaylightVariableName,
+  mEpochVariableName,
   Status
   ));
 return Status;
-- 
2.31.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78123): https://edk2.groups.io/g/devel/message/78123
Mute This Topic: https://groups.io/mt/84397232/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 2/2] UefiCpuPkg: ResetVector Tool additional debug prints

[Ashraf Ali S]

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3506

Before executing the nasm command, added print statement to know what
commands are executing.
before printing the output file need check the status of command which
is executed. if the status is 0 then only print the output file name.

Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Debkumar De 
Cc: Harry Han 
Cc: Catharine West 
Cc: Sangeetha V 
Signed-off-by: Ashraf Ali S 
---
 UefiCpuPkg/ResetVector/Vtf0/Build.py | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/ResetVector/Vtf0/Build.py 
b/UefiCpuPkg/ResetVector/Vtf0/Build.py
index 55f4edd87b..b791d32762 100644
--- a/UefiCpuPkg/ResetVector/Vtf0/Build.py
+++ b/UefiCpuPkg/ResetVector/Vtf0/Build.py
@@ -32,9 +32,12 @@ for arch in ('ia32', 'x64'):
 '-o', output,
 'Vtf0.nasmb',
 )
+print(f"Command : {' '.join(commandLine)}")
 ret = RunCommand(commandLine)
+if ret != 0:
+print(f"something went wrong while executing {commandLine[-1]}")
+sys.exit()
 print('\tASM\t' + output)
-if ret != 0: sys.exit(ret)
 
 commandLine = (
 'python',
-- 
2.30.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78122): https://edk2.groups.io/g/devel/message/78122
Mute This Topic: https://groups.io/mt/84397007/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 1/2] UefiCpuPkg: ResetVector Tool Support for Python 3

[Ashraf Ali S]

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3506

Build Scrips for Reset Vector currently based on Python 2
which is already EOL, needs to modify the build script based on
Python 3

Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Debkumar De 
Cc: Harry Han 
Cc: Catharine West 
Cc: Sangeetha V 

Signed-off-by: Ashraf Ali S 
---
 UefiCpuPkg/ResetVector/Vtf0/Build.py| 6 +++---
 UefiCpuPkg/ResetVector/Vtf0/Tools/FixupForRawSection.py | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/UefiCpuPkg/ResetVector/Vtf0/Build.py 
b/UefiCpuPkg/ResetVector/Vtf0/Build.py
index 343c53b5ff..55f4edd87b 100644
--- a/UefiCpuPkg/ResetVector/Vtf0/Build.py
+++ b/UefiCpuPkg/ResetVector/Vtf0/Build.py
@@ -1,7 +1,7 @@
 ## @file
 #  Automate the process of building the various reset vector types
 #
-#  Copyright (c) 2009, Intel Corporation. All rights reserved.
+#  Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -33,7 +33,7 @@ for arch in ('ia32', 'x64'):
 'Vtf0.nasmb',
 )
 ret = RunCommand(commandLine)
-print '\tASM\t' + output
+print('\tASM\t' + output)
 if ret != 0: sys.exit(ret)
 
 commandLine = (
@@ -41,7 +41,7 @@ for arch in ('ia32', 'x64'):
 'Tools/FixupForRawSection.py',
 output,
 )
-print '\tFIXUP\t' + output
+print('\tFIXUP\t' + output)
 ret = RunCommand(commandLine)
 if ret != 0: sys.exit(ret)
 
diff --git a/UefiCpuPkg/ResetVector/Vtf0/Tools/FixupForRawSection.py 
b/UefiCpuPkg/ResetVector/Vtf0/Tools/FixupForRawSection.py
index c77438a0ce..de771eba22 100644
--- a/UefiCpuPkg/ResetVector/Vtf0/Tools/FixupForRawSection.py
+++ b/UefiCpuPkg/ResetVector/Vtf0/Tools/FixupForRawSection.py
@@ -1,7 +1,7 @@
 ## @file
 #  Apply fixup to VTF binary image for FFS Raw section
 #
-#  Copyright (c) 2008, Intel Corporation. All rights reserved.
+#  Copyright (c) 2008 - 2021, Intel Corporation. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -15,6 +15,6 @@ c = ((len(d) + 4 + 7) & ~7) - 4
 if c > len(d):
 c -= len(d)
 f = open(sys.argv[1], 'wb')
-f.write('\x90' * c)
+f.write(b'\x90' * c)
 f.write(d)
 f.close()
-- 
2.30.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78121): https://edk2.groups.io/g/devel/message/78121
Mute This Topic: https://groups.io/mt/84397005/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH EDK2 v1 1/1] MdeModulePkg: Modify PCD default value

[wanghuiqiang via groups.io]

Dear all,

Any comments for this change?

Thanks!

-Original Message-
From: wanghuiqiang 
Sent: Monday, July 19, 2021 2:37 PM
To: 'devel@edk2.groups.io' ; 'ray...@intel.com' 
; xiewenyi (A) ; 'Wang, Jian J' 
; 'Wu, Hao A' ; 'Ard Biesheuvel' 
; 'Leif Lindholm' ; 'sami.muja...@arm.com' 

Cc: Songdongkuang 
Subject: RE: [edk2-devel] [PATCH EDK2 v1 1/1] MdeModulePkg: Modify PCD default 
value

+ Ard, Leif, Sami.
All OSes which use 64KB page size and the PCIe resource allocation rely on BIOS 
initialization will encounter VF BAR resource allocation issue.  

Thanks！

-Original Message-
From: wanghuiqiang 
Sent: Monday, July 19, 2021 2:02 PM
To: devel@edk2.groups.io; 'ray...@intel.com' ; xiewenyi (A) 
; Wang, Jian J ; Wu, Hao A 

Cc: Songdongkuang 
Subject: RE: [edk2-devel] [PATCH EDK2 v1 1/1] MdeModulePkg: Modify PCD default 
value

Hi Ray,

52 bit address ranges are supported only when using the 64KB translation 
granule for ARM64 platform, this is another scenario using 64K page size. We 
need set PcdSrIovSystemPageSize to 0x10 in such case to make BIOS resource 
assignment still appropriate even PCI DSM#5 function set as 0 which means the 
operating system must preserve PCI resource assignments made by firmware at 
boot time.  
Please let me know if you have any other concern for this patch.

Thanks!

-Original Message-
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ni, Ray
Sent: Monday, July 19, 2021 10:35 AM
To: devel@edk2.groups.io; xiewenyi (A) ; Wang, Jian J 
; Wu, Hao A 
Cc: Songdongkuang ; wanghuiqiang 

Subject: Re: [edk2-devel] [PATCH EDK2 v1 1/1] MdeModulePkg: Modify PCD default 
value

Wenyi,
Can you explain a bit more about "if 52 bit physical address need to be 
supported, page size should also be set to 64KB alignment"?

Can the platform DSC override this value instead of changing the default value 
in MdeModulePkg.dec which impacts all platforms?

Thanks,
Ray

-Original Message-
From: devel@edk2.groups.io  On Behalf Of wenyi,xie via 
groups.io
Sent: Thursday, July 15, 2021 8:25 PM
To: devel@edk2.groups.io; Wang, Jian J ; Wu, Hao A 

Cc: songdongku...@huawei.com; wanghuiqi...@huawei.com; xiewen...@huawei.com
Subject: [edk2-devel] [PATCH EDK2 v1 1/1] MdeModulePkg: Modify PCD default value

From: "wenyi.xie" 

The default value of PcdSrIovSystemPageSize is 0x1, it means the memory BAR is 
4KB alignment. When page size of OS is set to 64KB, as the resource partitions 
are different between OS and BIOS, it will cause pcie failture. And if 52 bit 
physical address need to be supported, page size should also be set to 64KB 
alignment.
So modify the default vaule of PcdSrIovSystemPageSize to 0x10 can meet the 
requirement above. And even if the OS is 4KB alignment, new value of PCD is 
compatible for this situation.

Cc: Jian J Wang 
Cc: Hao A Wu 
Signed-off-by: Wenyi Xie 
---
 MdeModulePkg/MdeModulePkg.dec | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec 
index ad84421cf3..426ea1b6cc 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1853,7 +1853,7 @@
   #  BIT0 set indicates 4KB alignment
   #  BIT1 set indicates 8KB alignment
   # @Prompt SRIOV system page size.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSystemPageSize|0x1|UINT32|0x1047
+
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSystemPageSize|0x10|UINT32|0x10
+ 47
 
   ## SMBIOS version.
   # @Prompt SMBIOS version.
--
2.20.1.windows.1













-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78120): https://edk2.groups.io/g/devel/message/78120
Mute This Topic: https://groups.io/mt/84223844/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [EXT] RE: [PATCH V2 3/4] NXP/LS1046aFrwyPkg: Enable ConfigurationManager on LS1046AFRWY

[Vikas Singh]

Sunny, Thank you for reviewing my code. Here are my remarks. PSB

-Original Message-
From: Sunny Wang  
Sent: Monday, July 12, 2021 4:08 PM
To: Vikas Singh ; devel@edk2.groups.io
Cc: Sami Mujawar ; l...@nuviainc.com; Meenakshi Aggarwal 
; Samer El-Haj-Mahmoud 
; Varun Sethi ; arokia.samy 
; kuldip dwivedi 
; Ard Biesheuvel ; 
Vikas Singh ; White Weng ; Ran Wang 
; Sunny Wang 
Subject: [EXT] RE: [PATCH V2 3/4] NXP/LS1046aFrwyPkg: Enable 
ConfigurationManager on LS1046AFRWY

Caution: EXT Email

Hi Vikas,

Just have two comments below. Others look good to me.
1. In LS1046aFrwyPkg.dsc, since you already include 
DynamicTablesPkg/DynamicTables.dsc.inc, I think we need to remove 
DynamicTableFactoryDxe.inf code block. Could you check this?
[[VIKAS]] No, DynamicTableFactoryDxe.inf code block Is required to override, or 
I would say to choose what we want for our platform from DynamicTablesPkg.
This block can expose different "inf" files for different platforms as per the 
need basis.
 
2. Remove " DEFINE DYNAMIC_ACPI_ENABLE = TRUE ". In our offline discussion, 
this would cause some build problems with the current CM implementation 
(Multiple NXP platforms share One CM folder).
[[VIKAS]] I don't think "DEFINE DYNAMIC_ACPI_ENABLE = TRUE" should cause any 
problem while building other platforms. In fact this flag is to avoid any build 
issue with other platforms. CM is common for all layer scape platforms but it 
will be only visible to any platform if and only if "DEFINE DYNAMIC_ACPI_ENABLE 
= TRUE". Else platform must be building in old native mode. Could you share any 
scenario or something that I have missed out, will check this implementation 
and try to rectify.

Best Regards,
Sunny Wang

-Original Message-
From: Vikas Singh 
Sent: Friday, June 18, 2021 11:28 PM
To: devel@edk2.groups.io
Cc: Sami Mujawar ; l...@nuviainc.com; Meenakshi Aggarwal 
(meenakshi.aggar...@nxp.com) ; Samer El-Haj-Mahmoud 
; V Sethi (v.se...@nxp.com) ; 
arokia.samy ; kuldip.dwiv...@puresoftware.com; 
Ard Biesheuvel ; vikas.si...@nxp.com; Sunny Wang 

Subject: [PATCH V2 3/4] NXP/LS1046aFrwyPkg: Enable ConfigurationManager on 
LS1046AFRWY

This patch enables the use of ConfigurationManager (CM) and its services to 
leverage the Dynamic ACPI support for NXP's LS1046aFrwy platform.

Signed-off-by: Vikas Singh 
---
 Platform/NXP/LS1046aFrwyPkg/Include/Platform.h | 152   
Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc |  28   
Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.fdf |  13 ++
 Silicon/NXP/LS1046A/LS1046A.dsc.inc|  11 ++
 4 files changed, 204 insertions(+)

diff --git a/Platform/NXP/LS1046aFrwyPkg/Include/Platform.h 
b/Platform/NXP/LS1046aFrwyPkg/Include/Platform.h
new file mode 100644
index 00..3c68d65cd3
--- /dev/null
+++ b/Platform/NXP/LS1046aFrwyPkg/Include/Platform.h
@@ -0,0 +1,152 @@
+/** @file

+ *  Platform headers

+ *

+ *  Copyright 2021 NXP

+ *  Copyright 2021 Puresoftware Ltd

+ *

+ *  SPDX-License-Identifier: BSD-2-Clause-Patent

+ *

+**/

+

+

+#ifndef LS1046AFRWY_PLATFORM_H

+#define LS1046AFRWY_PLATFORM_H

+

+#define EFI_ACPI_ARM_OEM_REVISION0x

+

+// Soc defines

+#define PLAT_SOC_NAME"LS1046AFRWY"

+

+// Gic

+#define GIC_VERSION  2

+#define GICD_BASE0x141

+#define GICC_BASE0x142f000

+#define GICH_BASE0x144

+#define GICV_BASE0x146

+

+// UART

+#define UART0_BASE   0x21C0500

+#define UART0_IT 86

+#define UART0_LENGTH 0x100

+#define SPCR_FLOW_CONTROL_NONE   0

+

+// Timer

+#define TIMER_BLOCK_COUNT1

+#define TIMER_FRAME_COUNT4

+#define TIMER_WATCHDOG_COUNT 1

+#define TIMER_BASE_ADDRESS   0x23E // a.k.a CNTControlBase

+#define TIMER_READ_BASE_ADDRESS  0x23F // a.k.a CNTReadBase

+#define TIMER_SEC_IT 29

+#define TIMER_NON_SEC_IT 30

+#define TIMER_VIRT_IT27

+#define TIMER_HYP_IT 26

+#define TIMER_FRAME0_IT  78

+#define TIMER_FRAME1_IT  79

+#define TIMER_FRAME2_IT  92

+

+// Mcfg

+#define LS1046A_PCI_SEG0_CONFIG_BASE 0x40

+#define LS1046A_PCI_SEG0 0x0

+#define LS1046A_PCI_SEG_BUSNUM_MIN   0x0

+#define LS1046A_PCI_SEG_BUSNUM_MAX   0xff

+#define LS1046A_PCI_SEG1_CONFIG_BASE 0x48

+#define LS1046A_PCI_SEG2_CONFIG_BASE 0x50

+#define LS1046A_PCI_SEG1 0x1

+#define LS1046A_PCI_SEG2 0x2

+

+// Platform specific info needed by Configuration Manager

+

+#define CFG_MGR_TABLE_ID  SIGNATURE_64 ('L','S','1','0','4','6',' ',' 
+')

+

+// Specify the OEM defined tables

+#define OEM_ACPI_TABLES 0

+

+#define PLAT_PCI_SEG0   LS1046A_PCI_SEG0

+#define PLAT_PCI_SEG1_CONFIG_BASE   LS1046A_PCI_SEG1_CONFIG_BASE

+#define