Date   

Re: [PATCH V7 1/1] OvmfPkg: Enable TDX in ResetVector

Yao, Jiewen
 

I strongly recommend to separate SEV and TDX in all context, if it is something SEV or TDX specific.
Then each file has clear ownership.
If it is something generic for both SEV and TDX, it can in one file.

For example, SecPeiTempRam/SecPageTable can be in common file.
But SevSnpSecrets/GhcbBookkeeping should be in SEV file.

Thank you
Yao Jiewen

-----Original Message-----
From: Gerd Hoffmann <kraxel@redhat.com>
Sent: Thursday, September 23, 2021 4:48 PM
To: Xu, Min M <min.m.xu@intel.com>
Cc: devel@edk2.groups.io; Ard Biesheuvel <ardb+tianocore@kernel.org>; Justen,
Jordan L <jordan.l.justen@intel.com>; Brijesh Singh <brijesh.singh@amd.com>;
Erdem Aktas <erdemaktas@google.com>; James Bottomley
<jejb@linux.ibm.com>; Yao, Jiewen <jiewen.yao@intel.com>; Tom Lendacky
<thomas.lendacky@amd.com>
Subject: Re: [PATCH V7 1/1] OvmfPkg: Enable TDX in ResetVector

On Thu, Sep 23, 2021 at 12:38:24AM +0000, Xu, Min M wrote:
On September 22, 2021 3:49 PM, Gerd Hoffmann wrote:
Hi,

+%ifdef ARCH_X64
+;
+; TDX Metadata offset block
+;
+; TdxMetadata.asm is included in ARCH_X64 because Inte TDX is only ;
+available in ARCH_X64. Below block describes the offset of ;
+TdxMetadata block in Ovmf image ; ; GUID :
+e47a6535-984a-4798-865e-4685a7bf8ec2
+;
+tdxMetadataOffsetStart:
+ DD tdxMetadataOffsetStart - TdxMetadataGuid - 16
+ DW tdxMetadataOffsetEnd - tdxMetadataOffsetStart
+ DB 0x35, 0x65, 0x7a, 0xe4, 0x4a, 0x98, 0x98, 0x47
+ DB 0x86, 0x5e, 0x46, 0x85, 0xa7, 0xbf, 0x8e, 0xc2
+tdxMetadataOffsetEnd:
+
+%endif
This should be switched to common ovmf metadata (see patches 4-7 of the
SEV-SNP series).

Min: please have a look at these patches.
Hi, Gerd
I checked the patches 4-7 of the SEV-SNP series. The common
OvmfMetadata is designed for both SEV and TDX, right?
That is the idea, yes.

If so, then it means the SEV and TDX metadata will be mixed in this
OvmfMetadata.
Yes.

I am thinking there will always be different fields for
SEV and TDX. For example, SEV has PcdOvmfSecGhcbPageTable but TDX
doesn't need that page. If the common OvmfMetadata is consumed by
TDX-QEMU, then PcdOvmfSecGhcbPageTableBase will be initialized too.
That doesn't make sense.
We have different range types. OVMF_* are the common areas. SEV_* will
be used by sev only, TDX_* will be used by tdx only. TDX and SEV
entries are allowed to overlap, i.e. PcdOvmfSecGhcbPageTableBase should
have some SEV_* type for sev (I think this needs fixing in the series),
and tdx can use the page for something else by adding an TDX_* entry for
the same range.

I am thinking that SEV and TDX can keep their own Metadata (in
separate files, SevMetadata.asm and TdxMetadata.asm) which are pointed
by the SEV or TDX offsets in the GUID-ed chain in ResetVector.
I'd very much prefer to have a single table to avoid duplication for the
common memory areas and keep the reset vector small.

Having separate SevMetadata.asm + TdxMetadata.asm files (then have
OvmfMetadata.asm include these two) is an option. I think this isn't
needed, we can also just group the entries in OvmfMetadata.asm.

In this case, SEV and TDX can design their own metadata flexibly, for
example, the attribute, the item structure, add/remove/update the
items, etc.
Why have two ways to do the same thing?

take care,
Gerd


Re: [PATCH v3 0/4] AndroidBootImgLib improvements

Leif Lindholm
 

Hi Jeff,

I was about to say "no more issues", and then I went to build
EmbeddedPkg, and it turns out this fails in
Applications/AndroidBootApp due to the missing dependency on
gEfiLoadFile2ProtocolGuid in AndroidBootImgLib.inf.

(Why this doesn't break AndroidFastbootApp build as well is not
immediately obvious to me.)

Would you like to figure out why, or would you prefer me to just fold
in

diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
index affde50f617a..8eefeef4f915 100644
--- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
+++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
@@ -39,6 +39,7 @@ [Packages]
[Protocols]
gAndroidBootImgProtocolGuid
gEfiLoadedImageProtocolGuid
+ gEfiLoadFile2ProtocolGuid

[Guids]
gEfiAcpiTableGuid

?

/
Leif

On Tue, Sep 21, 2021 at 16:32:58 +0000, Jeff Brasen wrote:
Jun/Others,

Any additional comments on this patch series?


Thanks,

Jeff

________________________________
From: Jeff Brasen <jbrasen@nvidia.com>
Sent: Tuesday, September 14, 2021 10:57 AM
To: Leif Lindholm <leif@nuviainc.com>
Cc: devel@edk2.groups.io <devel@edk2.groups.io>; daniel.schaefer@hpe.com <daniel.schaefer@hpe.com>; abner.chang@hpe.com <abner.chang@hpe.com>; ardb+tianocore@kernel.org <ardb+tianocore@kernel.org>; Jun Nie <jun.nie@linaro.org>
Subject: Re: [PATCH v3 0/4] AndroidBootImgLib improvements

So for patch 3: This is only a change if mAndroidBootImg->UpdateDtb == NULL.

This seemed like a bug as we would not add the initrd values nor would we use the fdt from the BootImg if that is where the device tree was sourced from.

It seems like either we should require UpdateDtb to be implemented (which seems to cause greater compatibility issues) or we install the device tree we have if that function isn't implemented.

As far as merging goes I am fine either way. Our particular flow won't hit this path as we don't have a device tree in the bootimg (use the system config table) and we will have the new pcd set, but this seemed like a bug while I looking at this code.


Thanks,

Jeff

________________________________
From: Leif Lindholm <leif@nuviainc.com>
Sent: Tuesday, September 14, 2021 9:00 AM
To: Jeff Brasen <jbrasen@nvidia.com>
Cc: devel@edk2.groups.io <devel@edk2.groups.io>; daniel.schaefer@hpe.com <daniel.schaefer@hpe.com>; abner.chang@hpe.com <abner.chang@hpe.com>; ardb+tianocore@kernel.org <ardb+tianocore@kernel.org>; Jun Nie <jun.nie@linaro.org>
Subject: Re: [PATCH v3 0/4] AndroidBootImgLib improvements

External email: Use caution opening links or attachments


Hi Jeff,

Thanks for this.
This set looks good to me, with a slight question mark wrt behaviour
compatibility with previous versions for 3/4.
(I think it's fine, but I'm a bear of very little brain, and it's been
several years since I reviewed this code, and even longer since I
really interacted with Android.
^
| shameless plug for more EmbeddedPkg reviewer volunteers.)

I've added Jun Nie, who wrote the original version of this code, to
see if he has any comments.

1-2/4 are obviously unproblematic, and I could merge those ahead of
time if preferred. You can add
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
for those if there are any further revisions of the set.

Best Regards,

Leif

On Mon, Sep 13, 2021 at 23:18:47 +0000, Jeff Brasen wrote:
Added support for using loadfile2 approach for passing ramdisk to linux.
Created patch series for general error handling improvments based on
review feedback.
If ACPI tables are in system table or PCD is defined the LoadFile2 method
of passing initrd will be used.

[v3]
-Code review cleanup
-Removed duplicate header file
-Added change to allow FDT to install if UpdateDtb function is not defined
-Added specific ACPI check
-Moved install functions to subfunctions

[v2]
-Added review feedback
-General improvements to error handling

[v1]
- Intial revision


Jeff Brasen (4):
EmbeddedPkg: Remove duplicate libfdt.h include
EmbeddedPkg: AndroidBootImgBoot error handling updates
EmbeddedPkg: Install FDT if UpdateDtb is not present
EmbeddedPkg: Add LoadFile2 for linux initrd

EmbeddedPkg/EmbeddedPkg.dec | 1 +
.../AndroidBootImgLib/AndroidBootImgLib.inf | 4 +
.../AndroidBootImgLib/AndroidBootImgLib.c | 275 +++++++++++++++---
3 files changed, 233 insertions(+), 47 deletions(-)

--
2.17.1


Re: [PATCH 1/1] Qemu: SbsaQemu: Set the DSDT revision value to 2 to use 64-bit math

Leif Lindholm
 

Not just an improvement, but an actual bugfix.

I would propose a subject line change though:
Qemu: SbsaQemu: Set the DSDT revision value to 2 to use 64-bit math
->
Platform/Qemu: fix SbsaQemu DSDT format version

If you're OK with the change, I can fold that in.
Reviewed-by: Leif Lindholm <leif@nuviainc.com>

On Wed, Sep 22, 2021 at 16:37:09 -0600, Rebecca Cran wrote:
Set the DSDT revision value to 2 by using the define from Acpi60.h
EFI_ACPI_6_0_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_REVISION. This
causes the AML interpreter to use full 64-bit integers and math.

Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
---
Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl b/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
index e056d6cdb02e..1bf9fbb99e75 100644
--- a/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
+++ b/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
@@ -6,6 +6,7 @@
* SPDX-License-Identifier: BSD-2-Clause-Patent
**/

+#include <IndustryStandard/Acpi60.h>
#include <IndustryStandard/SbsaQemuAcpi.h>

#define LINK_DEVICE(Uid, LinkName, Irq) \
@@ -25,8 +26,9 @@
Address, Pin, Link, Zero \
}

-DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "LINARO", "SBSAQEMU",
- FixedPcdGet32 (PcdAcpiDefaultOemRevision)) {
+DefinitionBlock ("DsdtTable.aml", "DSDT",
+ EFI_ACPI_6_0_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_REVISION,
+ "LINARO", "SBSAQEMU", FixedPcdGet32 (PcdAcpiDefaultOemRevision)) {
Scope (_SB) {
// UART PL011
Device (COM0) {
--
2.31.1


Re: [edk2-platforms PATCH 0/4] Marvell readmes

Leif Lindholm
 

Hi Marcin,

On Wed, Sep 22, 2021 at 14:46:10 +0200, Marcin Wojtas wrote:
Marcin, when you find the time, could you please do a pass over these
files with Leif's critique in mind?

For all 3 platforms, how about the following update:
- extend the "Summary" section with supported features or interfaces
(or add an extra heading for that).
- "Build" section -> add link to external wiki/.md file
To be clear - I don't mind the actual example build command line, and
maybe listing the expected/available command line options (-D).

- "ARM System Ready certification." - leave intact

Please let me know if that will work for you.
Sure, sounds good.

Best Regards,

Leif


Re: [PATCH v1 0/4] Set default Makefile name

Chris Jones
 

For this patch series:

Reviewed-by: Chris Jones <christopher.jones@...>


Regards,
Chris


From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of PierreGondois via groups.io <pierre.gondois@...>
Sent: Thursday, September 23, 2021 9:58 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>; Bob Feng <bob.c.feng@...>; Liming Gao <gaoliming@...>; Sami Mujawar <Sami.Mujawar@...>
Subject: [edk2-devel] [PATCH v1 0/4] Set default Makefile name
 
From: Pierre Gondois <Pierre.Gondois@...>

A Makefile name is not set in BaseTools when only building modules
or libraries. This patch-set sets a default Makefile name for the
"build" command.

The patch-set also:
- Removes unsused Makefile variables
- Removes hard-coded references to "target.txt" and "tools_def.txt"

The changes can be seen at: https://github.com/PierreARM/edk2/tree/1868_BaseTools_build_py_corrections_v1

Pierre Gondois (4):
  BaseTools/GenMake: Use ToolDefinition as fallback option
  BaseTools/build: Set MakefileName
  BaseTools: Remove Makefile/MakefileName fields
  BaseTools: Remove hard-coded strings for target and tools_def

 BaseTools/Source/Python/AutoGen/GenMake.py            |  8 ++++----
 BaseTools/Source/Python/AutoGen/ModuleAutoGen.py      |  1 -
 BaseTools/Source/Python/GenFds/GenFds.py              |  4 ++--
 .../Source/Python/GenFds/GenFdsGlobalVariable.py      |  4 ++--
 BaseTools/Source/Python/TargetTool/TargetTool.py      |  3 ++-
 BaseTools/Source/Python/Workspace/BuildClassObject.py |  2 --
 BaseTools/Source/Python/Workspace/DscBuildData.py     |  9 ++++-----
 BaseTools/Source/Python/build/build.py                | 11 ++++-------
 8 files changed, 18 insertions(+), 24 deletions(-)

--
2.17.1






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.


[PATCH v1 4/4] BaseTools: Remove hard-coded strings for target and tools_def

PierreGondois
 

From: Pierre Gondois <Pierre.Gondois@arm.com>

The "target.txt" and "tools_def.txt" filenames are hard-coded
at some places when global definitions are available at:
BaseTools/Source/Python/Common/TargetTxtClassObject.py:
DefaultTargetTxtFile
and
BaseTools/Source/Python/Common/ToolDefClassObject.py:
DefaultToolsDefFile

Use these global definitions instead.

Also remove the unused gBuildConfiguration and gToolsDefinition
variables from build.py

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
BaseTools/Source/Python/GenFds/GenFds.py | 4 ++--
BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py | 4 ++--
BaseTools/Source/Python/TargetTool/TargetTool.py | 3 ++-
BaseTools/Source/Python/Workspace/DscBuildData.py | 9 ++++-----
BaseTools/Source/Python/build/build.py | 4 ----
5 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/GenFds.py b/BaseTools/Source/Python/GenFds/GenFds.py
index ae3e776a5540..c34104500059 100644
--- a/BaseTools/Source/Python/GenFds/GenFds.py
+++ b/BaseTools/Source/Python/GenFds/GenFds.py
@@ -20,7 +20,7 @@ from linecache import getlines
from io import BytesIO

import Common.LongFilePathOs as os
-from Common.TargetTxtClassObject import TargetTxtDict
+from Common.TargetTxtClassObject import TargetTxtDict,gDefaultTargetTxtFile
from Common.DataType import *
import Common.GlobalData as GlobalData
from Common import EdkLogger
@@ -207,7 +207,7 @@ def GenFdsApi(FdsCommandDict, WorkSpaceDataBase=None):
GenFdsGlobalVariable.ConfDir = ConfDirectoryPath
if not GlobalData.gConfDirectory:
GlobalData.gConfDirectory = GenFdsGlobalVariable.ConfDir
- BuildConfigurationFile = os.path.normpath(os.path.join(ConfDirectoryPath, "target.txt"))
+ BuildConfigurationFile = os.path.normpath(os.path.join(ConfDirectoryPath, gDefaultTargetTxtFile))
if os.path.isfile(BuildConfigurationFile) == True:
# if no build target given in command line, get it from target.txt
TargetObj = TargetTxtDict()
diff --git a/BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py b/BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py
index 25f9d54874d3..d7668ba681aa 100644
--- a/BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py
+++ b/BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py
@@ -24,7 +24,7 @@ from Common import EdkLogger
from Common.Misc import SaveFileOnChange

from Common.TargetTxtClassObject import TargetTxtDict
-from Common.ToolDefClassObject import ToolDefDict
+from Common.ToolDefClassObject import ToolDefDict,gDefaultToolsDefFile
from AutoGen.BuildEngine import ToolBuildRule
import Common.DataType as DataType
from Common.Misc import PathClass,CreateDirectory
@@ -103,7 +103,7 @@ class GenFdsGlobalVariable:
TargetObj = TargetTxtDict()
ToolDefinitionFile = TargetObj.Target.TargetTxtDictionary[DataType.TAB_TAT_DEFINES_TOOL_CHAIN_CONF]
if ToolDefinitionFile == '':
- ToolDefinitionFile = "Conf/tools_def.txt"
+ ToolDefinitionFile = os.path.join('Conf', gDefaultToolsDefFile)
if os.path.isfile(ToolDefinitionFile):
ToolDefObj = ToolDefDict((os.path.join(os.getenv("WORKSPACE"), "Conf")))
ToolDefinition = ToolDefObj.ToolDef.ToolsDefTxtDatabase
diff --git a/BaseTools/Source/Python/TargetTool/TargetTool.py b/BaseTools/Source/Python/TargetTool/TargetTool.py
index 71222e3cc899..7f2479f0f0ac 100644
--- a/BaseTools/Source/Python/TargetTool/TargetTool.py
+++ b/BaseTools/Source/Python/TargetTool/TargetTool.py
@@ -17,6 +17,7 @@ import Common.BuildToolError as BuildToolError
from Common.DataType import *
from Common.BuildVersion import gBUILD_VERSION
from Common.LongFilePathSupport import OpenLongFilePath as open
+from Common.TargetTxtClassObject import gDefaultTargetTxtFile

# To Do 1.set clean, 2. add item, if the line is disabled.

@@ -25,7 +26,7 @@ class TargetTool():
self.WorkSpace = os.path.normpath(os.getenv('WORKSPACE'))
self.Opt = opt
self.Arg = args[0]
- self.FileName = os.path.normpath(os.path.join(self.WorkSpace, 'Conf', 'target.txt'))
+ self.FileName = os.path.normpath(os.path.join(self.WorkSpace, 'Conf', gDefaultTargetTxtFile))
if os.path.isfile(self.FileName) == False:
print("%s does not exist." % self.FileName)
sys.exit(1)
diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py b/BaseTools/Source/Python/Workspace/DscBuildData.py
index 4d5b1ad4d90a..d1ee0ccaea7e 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -19,8 +19,8 @@ from Common.Misc import *
from types import *
from Common.Expression import *
from CommonDataClass.CommonClass import SkuInfoClass
-from Common.TargetTxtClassObject import TargetTxtDict
-from Common.ToolDefClassObject import ToolDefDict
+from Common.TargetTxtClassObject import TargetTxtDict,gDefaultTargetTxtFile
+from Common.ToolDefClassObject import ToolDefDict,gDefaultToolsDefFile
from .MetaDataTable import *
from .MetaFileTable import *
from .MetaFileParser import *
@@ -3526,12 +3526,11 @@ class DscBuildData(PlatformBuildClassObject):
self._ToolChainFamily = TAB_COMPILER_MSFT
TargetObj = TargetTxtDict()
TargetTxt = TargetObj.Target
- BuildConfigurationFile = os.path.normpath(os.path.join(GlobalData.gConfDirectory, "target.txt"))
+ BuildConfigurationFile = os.path.normpath(os.path.join(GlobalData.gConfDirectory, gDefaultTargetTxtFile))
if os.path.isfile(BuildConfigurationFile) == True:
ToolDefinitionFile = TargetTxt.TargetTxtDictionary[DataType.TAB_TAT_DEFINES_TOOL_CHAIN_CONF]
if ToolDefinitionFile == '':
- ToolDefinitionFile = "tools_def.txt"
- ToolDefinitionFile = os.path.normpath(mws.join(self.WorkspaceDir, 'Conf', ToolDefinitionFile))
+ ToolDefinitionFile = os.path.normpath(mws.join(self.WorkspaceDir, 'Conf', gDefaultToolsDefFile))
if os.path.isfile(ToolDefinitionFile) == True:
ToolDefObj = ToolDefDict((os.path.join(os.getenv("WORKSPACE"), "Conf")))
ToolDefinition = ToolDefObj.ToolDef.ToolsDefTxtDatabase
diff --git a/BaseTools/Source/Python/build/build.py b/BaseTools/Source/Python/build/build.py
index e4969d863f6e..07187c03618a 100755
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -67,10 +67,6 @@ from AutoGen.AutoGen import CalculatePriorityValue
## standard targets of build command
gSupportedTarget = ['all', 'genc', 'genmake', 'modules', 'libraries', 'fds', 'clean', 'cleanall', 'cleanlib', 'run']

-## build configuration file
-gBuildConfiguration = "target.txt"
-gToolsDefinition = "tools_def.txt"
-
TemporaryTablePattern = re.compile(r'^_\d+_\d+_[a-fA-F0-9]+$')
TmpTableDict = {}

--
2.17.1


[PATCH v1 3/4] BaseTools: Remove Makefile/MakefileName fields

PierreGondois
 

From: Pierre Gondois <Pierre.Gondois@arm.com>

The Makefile and MakefilName fields are never set/used. Remove them.
To check this, the following commands can be used:
- grep -rIn "\.Makefile"
- grep -rIn "\.MakefileName"

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
BaseTools/Source/Python/AutoGen/ModuleAutoGen.py | 1 -
BaseTools/Source/Python/Workspace/BuildClassObject.py | 2 --
BaseTools/Source/Python/build/build.py | 2 --
3 files changed, 5 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
index d70b0d7ae828..368a31047e82 100755
--- a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
@@ -254,7 +254,6 @@ class ModuleAutoGen(AutoGen):
self.AutoGenDepSet = set()
self.ReferenceModules = []
self.ConstPcd = {}
- self.Makefile = None
self.FileDependCache = {}

def __init_platform_info__(self):
diff --git a/BaseTools/Source/Python/Workspace/BuildClassObject.py b/BaseTools/Source/Python/Workspace/BuildClassObject.py
index 88a1d1582cd8..ef873720f455 100644
--- a/BaseTools/Source/Python/Workspace/BuildClassObject.py
+++ b/BaseTools/Source/Python/Workspace/BuildClassObject.py
@@ -590,7 +590,6 @@ class PackageBuildClassObject(BuildData):
# @var OutputDirectory: To store value for OutputDirectory
# @var FlashDefinition: To store value for FlashDefinition
# @var BuildNumber: To store value for BuildNumber
-# @var MakefileName: To store value for MakefileName
# @var SkuIds: To store value for SkuIds, it is a set structure as
# { 'SkuName' : SkuId, '!include' : includefilename, ...}
# @var Modules: To store value for Modules, it is a list structure as
@@ -614,7 +613,6 @@ class PlatformBuildClassObject(BuildData):
self.OutputDirectory = ''
self.FlashDefinition = ''
self.BuildNumber = ''
- self.MakefileName = ''

self.SkuIds = {}
self.Modules = []
diff --git a/BaseTools/Source/Python/build/build.py b/BaseTools/Source/Python/build/build.py
index 58081361c38d..e4969d863f6e 100755
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -2186,8 +2186,6 @@ class Build():
Pa.CreateLibModuelDirs()
# Fetch the MakeFileName.
self.MakeFileName = Pa.MakeFileName
- if not self.MakeFileName:
- self.MakeFileName = Pa.MakeFile

Pa.DataPipe.DataContainer = {"LibraryBuildDirectoryList":Pa.LibraryBuildDirectoryList}
Pa.DataPipe.DataContainer = {"ModuleBuildDirectoryList":Pa.ModuleBuildDirectoryList}
--
2.17.1


[PATCH v1 2/4] BaseTools/build: Set MakefileName

PierreGondois
 

From: Pierre Gondois <Pierre.Gondois@arm.com>

Running the following command:
python3 build/build.py -a AARCH64 -t GCC5
-p ArmPlatformPkg/ArmPlatformPkg.dsc -b DEBUG libraries
triggers the following error:
make: *** Build/ArmPlatform/DEBUG_GCC5/AARCH64/MdePkg/Library/
BasePcdLibNull/BasePcdLibNull: Is a directory. Stop.

Indeed, MakefileName is set to en empty string. Setting MakefileName
resolves the error.

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
BaseTools/Source/Python/build/build.py | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/build/build.py b/BaseTools/Source/Python/build/build.py
index 02b489892422..58081361c38d 100755
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -4,7 +4,7 @@
# Copyright (c) 2014, Hewlett-Packard Development Company, L.P.<BR>
# Copyright (c) 2007 - 2021, Intel Corporation. All rights reserved.<BR>
# Copyright (c) 2018, Hewlett Packard Enterprise Development, L.P.<BR>
-# Copyright (c) 2020, ARM Limited. All rights reserved.<BR>
+# Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -1308,6 +1308,9 @@ class Build():
if Target == 'run':
return True

+ # Fetch the MakeFileName.
+ self.MakeFileName = AutoGenObject.MakeFileName
+
# build modules
if BuildModule:
BuildCommand = BuildCommand + [Target]
--
2.17.1


[PATCH v1 1/4] BaseTools/GenMake: Use ToolDefinition as fallback option

PierreGondois
 

From: Pierre Gondois <Pierre.Gondois@arm.com>

Use the value set in tools_def.txt when the makefile type is
not explicitly set via BuildOption. This allows to have a
valid default makefile name instead of an empty string.

Also use GMAKE_FILETYPE instead of hard-coded "gmake".

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
BaseTools/Source/Python/AutoGen/GenMake.py | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py b/BaseTools/Source/Python/AutoGen/GenMake.py
index 961b2ab1c399..e55efff059f9 100755
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -2,7 +2,7 @@
# Create makefile for MS nmake and GNU make
#
# Copyright (c) 2007 - 2021, Intel Corporation. All rights reserved.<BR>
-# Copyright (c) 2020, ARM Limited. All rights reserved.<BR>
+# Copyright (c) 2020 - 2021, Arm Limited. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#

@@ -177,11 +177,11 @@ class BuildFile(object):

MakePath = AutoGenObject.BuildOption.get('MAKE', {}).get('PATH')
if not MakePath:
- self._FileType = ""
- elif "nmake" in MakePath:
+ MakePath = AutoGenObject.ToolDefinition.get('MAKE', {}).get('PATH')
+ if "nmake" in MakePath:
self._FileType = NMAKE_FILETYPE
else:
- self._FileType = "gmake"
+ self._FileType = GMAKE_FILETYPE

if sys.platform == "win32":
self._Platform = WIN32_PLATFORM
--
2.17.1


[PATCH v1 0/4] Set default Makefile name

PierreGondois
 

From: Pierre Gondois <Pierre.Gondois@arm.com>

A Makefile name is not set in BaseTools when only building modules
or libraries. This patch-set sets a default Makefile name for the
"build" command.

The patch-set also:
- Removes unsused Makefile variables
- Removes hard-coded references to "target.txt" and "tools_def.txt"

The changes can be seen at: https://github.com/PierreARM/edk2/tree/1868_BaseTools_build_py_corrections_v1

Pierre Gondois (4):
BaseTools/GenMake: Use ToolDefinition as fallback option
BaseTools/build: Set MakefileName
BaseTools: Remove Makefile/MakefileName fields
BaseTools: Remove hard-coded strings for target and tools_def

BaseTools/Source/Python/AutoGen/GenMake.py | 8 ++++----
BaseTools/Source/Python/AutoGen/ModuleAutoGen.py | 1 -
BaseTools/Source/Python/GenFds/GenFds.py | 4 ++--
.../Source/Python/GenFds/GenFdsGlobalVariable.py | 4 ++--
BaseTools/Source/Python/TargetTool/TargetTool.py | 3 ++-
BaseTools/Source/Python/Workspace/BuildClassObject.py | 2 --
BaseTools/Source/Python/Workspace/DscBuildData.py | 9 ++++-----
BaseTools/Source/Python/build/build.py | 11 ++++-------
8 files changed, 18 insertions(+), 24 deletions(-)

--
2.17.1


Re: [PATCH V7 1/1] OvmfPkg: Enable TDX in ResetVector

Gerd Hoffmann
 

On Thu, Sep 23, 2021 at 12:38:24AM +0000, Xu, Min M wrote:
On September 22, 2021 3:49 PM, Gerd Hoffmann wrote:
Hi,

+%ifdef ARCH_X64
+;
+; TDX Metadata offset block
+;
+; TdxMetadata.asm is included in ARCH_X64 because Inte TDX is only ;
+available in ARCH_X64. Below block describes the offset of ;
+TdxMetadata block in Ovmf image ; ; GUID :
+e47a6535-984a-4798-865e-4685a7bf8ec2
+;
+tdxMetadataOffsetStart:
+ DD tdxMetadataOffsetStart - TdxMetadataGuid - 16
+ DW tdxMetadataOffsetEnd - tdxMetadataOffsetStart
+ DB 0x35, 0x65, 0x7a, 0xe4, 0x4a, 0x98, 0x98, 0x47
+ DB 0x86, 0x5e, 0x46, 0x85, 0xa7, 0xbf, 0x8e, 0xc2
+tdxMetadataOffsetEnd:
+
+%endif
This should be switched to common ovmf metadata (see patches 4-7 of the
SEV-SNP series).

Min: please have a look at these patches.
Hi, Gerd
I checked the patches 4-7 of the SEV-SNP series. The common
OvmfMetadata is designed for both SEV and TDX, right?
That is the idea, yes.

If so, then it means the SEV and TDX metadata will be mixed in this
OvmfMetadata.
Yes.

I am thinking there will always be different fields for
SEV and TDX. For example, SEV has PcdOvmfSecGhcbPageTable but TDX
doesn't need that page. If the common OvmfMetadata is consumed by
TDX-QEMU, then PcdOvmfSecGhcbPageTableBase will be initialized too.
That doesn't make sense.
We have different range types. OVMF_* are the common areas. SEV_* will
be used by sev only, TDX_* will be used by tdx only. TDX and SEV
entries are allowed to overlap, i.e. PcdOvmfSecGhcbPageTableBase should
have some SEV_* type for sev (I think this needs fixing in the series),
and tdx can use the page for something else by adding an TDX_* entry for
the same range.

I am thinking that SEV and TDX can keep their own Metadata (in
separate files, SevMetadata.asm and TdxMetadata.asm) which are pointed
by the SEV or TDX offsets in the GUID-ed chain in ResetVector.
I'd very much prefer to have a single table to avoid duplication for the
common memory areas and keep the reset vector small.

Having separate SevMetadata.asm + TdxMetadata.asm files (then have
OvmfMetadata.asm include these two) is an option. I think this isn't
needed, we can also just group the entries in OvmfMetadata.asm.

In this case, SEV and TDX can design their own metadata flexibly, for
example, the attribute, the item structure, add/remove/update the
items, etc.
Why have two ways to do the same thing?

take care,
Gerd


Re: [PATCH v1 0/2] ACPI 6.4 SBSA generic watchdog renaming

Sami Mujawar
 

Hi Zhichao,

The following patch series have been reviewed and have received r-b from the maintainers.

·         https://edk2.groups.io/g/devel/topic/84925099#79362 (ShellPkg & DynamicTablesPkg)

·         https://edk2.groups.io/g/devel/topic/84868164#79285 (ShellPkg)

·         https://edk2.groups.io/g/devel/topic/84968868#79489 (ShellPkg)


Some of the patch series have DynamicTablesPkg patches as well.
If you agree I can merge the 3 patch series listed above in edk2 master. Kindly let me know.

Regards,

Sami Mujawar


Re: [PATCH v8 08/32] OvmfPkg/ResetVector: use SEV-SNP-validated CPUID values

Gerd Hoffmann
 

Hi,

One issue with that is that the contents of the CPUID page are not part
of guest measurement that will be checked later during attestation (only
the metadata such as page type/location is recorded in the measurement).

[ more details snipped ]
Thanks, that makes sense.

That said, for the !SNP case, additional handling *could* be added to make
use of the CPUID page, but in that case it wouldn't be validated by firmware,
so isn't much better security-wise than asking KVM.
Well, the intention would be more to (a) be able to test the code
without SNP hardware (for example in public CI) and (b) avoid trapping
into kvm if we don't have to.

It is clearly not a priority though, we can look into that once all the
SNP bits are merged in edk2 and qemu.

take care,
Gerd


Re: [PATCH V2] MdeModulePkg/BootManagerMenuApp: Limit string drawing within one line

Gao, Zhichao
 

Hi Ray,
That is one way for platform to customize their own boot option. It can solve the issue. But my point is it should not be used in such case. If EDK2 want a default behavior of the boot option description, we should change at the root instead of using the customize interfaces.

Thanks,
Zhichao

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Thursday, September 23, 2021 3:43 PM
To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io;
gaoliming@byosoft.com.cn
Cc: Wang, Jian J <jian.j.wang@intel.com>
Subject: RE: [edk2-devel] [PATCH V2]
MdeModulePkg/BootManagerMenuApp: Limit string drawing within one line

Another option is to use EfiBootManagerRegisterBootDescriptionHandler()
to alter the boot descriptions.

-----Original Message-----
From: Gao, Zhichao <zhichao.gao@intel.com>
Sent: Thursday, September 23, 2021 11:47 AM
To: devel@edk2.groups.io; gaoliming@byosoft.com.cn; Ni, Ray
<ray.ni@intel.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>
Subject: RE: [edk2-devel] [PATCH V2]
MdeModulePkg/BootManagerMenuApp:
Limit string drawing within one line

Hi Liming,

Yes. Because the design of the BM app is not aimed to display the boot
option over one line. And it is not using the setup browser engine.
That would cause the difference.
If we want to make them align, there are two options:
1. BM app to use the setup browser engine 2. add scroll bar logic for
the boot item

Both above change is not simple and may cause new issues. It would be a
new design other than a bug fix.

Another solution is the patch V1 to limit the boot option description
within 72 characters. Ray pointed out it is not a good solution.

BTW, I would remove the change-id in next patch.

Thanks,
Zhichao

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
gaoliming
Sent: Thursday, September 23, 2021 10:59 AM
To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>; Ni,
Ray <ray.ni@intel.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>
Subject: 回复: [edk2-devel] [PATCH V2]
MdeModulePkg/BootManagerMenuApp: Limit string drawing within one
line

Zhichao:
With this change, the same boot option will be displayed
differently in BootManagerApp and BootManager Page. Is it the designed
behavior?

Besides, please remove change-id from the commit message.

Thanks
Liming
-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Gao,
Zhichao
发送时间: 2021年9月22日 12:50
收件人: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Liming Gao
<gaoliming@byosoft.com.cn>
抄送: Wang, Jian J <jian.j.wang@intel.com>
主题: Re: [edk2-devel] [PATCH V2]
MdeModulePkg/BootManagerMenuApp:
Limit string drawing within one line

Hi Liming,

The solution is different with the first time we discussed on the
Bugzilla. Can
you review if it is OK to you?

Thanks,
Zhichao

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Wednesday, September 22, 2021 11:28 AM
To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@intel.com>; Liming Gao
<gaoliming@byosoft.com.cn>
Subject: RE: [PATCH V2] MdeModulePkg/BootManagerMenuApp:
Limit
string drawing within one line

Reviewed-by: Ray Ni <ray.ni@intel.com>

-----Original Message-----
From: Gao, Zhichao <zhichao.gao@intel.com>
Sent: Thursday, September 9, 2021 3:26 PM
To: devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@intel.com>; Liming Gao
<gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>
Subject: [PATCH V2] MdeModulePkg/BootManagerMenuApp: Limit
string
drawing within one line

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

Limit the draw box always within the screen's column and row.
Limit the string drawing within one line.

Change-Id: Ib7bd63cb07b23875a1e4f37ae80a422e1d5ed54f
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---

V2:

Drop the change in UefiBootManagerLib in V1.

Add the limitation in BootManagerMenuApp instead.


.../BootManagerMenuApp/BootManagerMenu.c | 72
++++++++++++++++++-
1 file changed, 69 insertions(+), 3 deletions(-)

diff --git
a/MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenu.
c
b/MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenu.
c
index 9e729074ec..d4bdeba073 100644
---
a/MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenu.
c
+++
b/MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenu.
c
@@ -1,7 +1,7 @@
/** @file

The application to show the Boot Manager Menu.



-Copyright (c) 2011 - 2018, Intel Corporation. All rights
reserved.<BR>

+Copyright (c) 2011 - 2021, Intel Corporation. All rights
+reserved.<BR>

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



**/

@@ -45,9 +45,56 @@ PrintStringAt (
IN CHAR16 *String

)

{

+ UINTN ScreenWidth;

+ UINTN ScreenRows;

+ CHAR16 *TurncateString;

+ EFI_STATUS Status;

+ UINTN ShowingLength;



gST->ConOut->SetCursorPosition (gST->ConOut, Column, Row);

- return Print (L"%s", String);

+

+ gST->ConOut->QueryMode (

+ gST->ConOut,

+ gST->ConOut->Mode->Mode,

+ &ScreenWidth,

+ &ScreenRows

+ );

+

+ if (Column > (ScreenWidth - 1) || Row > (ScreenRows - 1)) {

+ return 0;

+ }

+

+ if ((StrLen (String) + Column) > (ScreenWidth - 1)) {

+ //

+ // | - ScreenWidth - |

+ // ...Column.....................

+ // TurncateString length should leave one character for
+ draw box
and

+ // require one character for string end.

+ //

+ ShowingLength = ScreenWidth - Column - 1;

+ TurncateString = AllocatePool ((ShowingLength + 1) * sizeof
(CHAR16));

+

+ if (TurncateString == NULL) {

+ return 0;

+ }

+

+ Status = StrnCpyS (TurncateString, ShowingLength + 1,
+ String,
ShowingLength - 3);

+

+ if (EFI_ERROR (Status)) {

+ FreePool (TurncateString);

+ return 0;

+ }

+

+ *(TurncateString + ShowingLength - 3) = L'.';

+ *(TurncateString + ShowingLength - 2) = L'.';

+ *(TurncateString + ShowingLength - 1) = L'.';

+ *(TurncateString + ShowingLength) = L'\0';

+ ShowingLength = Print (L"%s", TurncateString);

+ FreePool (TurncateString);

+ return ShowingLength;

+ } else {

+ return Print (L"%s", String);

+ }

}



/**

@@ -68,7 +115,22 @@ PrintCharAt (
CHAR16 Character

)

{

+ UINTN ScreenWidth;

+ UINTN ScreenRows;

+

gST->ConOut->SetCursorPosition (gST->ConOut, Column, Row);

+

+ gST->ConOut->QueryMode (

+ gST->ConOut,

+ gST->ConOut->Mode->Mode,

+ &ScreenWidth,

+ &ScreenRows

+ );

+

+ if (Column > (ScreenWidth - 1) || Row > (ScreenRows - 1)) {

+ return 0;

+ }

+

return Print (L"%c", Character);

}



@@ -193,7 +255,11 @@ InitializeBootMenuScreen (


MaxPrintRows = Row - 6;

UnSelectableItmes = TITLE_TOKEN_COUNT + 2 +
HELP_TOKEN_COUNT + 2;

- BootMenuData->MenuScreen.Width = MaxStrWidth + 8;

+ if (MaxStrWidth + 8 > Column) {

+ BootMenuData->MenuScreen.Width = Column;

+ } else {

+ BootMenuData->MenuScreen.Width = MaxStrWidth + 8;

+ }

if (BootMenuData->ItemCount + UnSelectableItmes >
MaxPrintRows) {

BootMenuData->MenuScreen.Height = MaxPrintRows;

BootMenuData->ScrollBarControl.HasScrollBar = TRUE;

--
2.31.1.windows.1









Re: [PATCH v1 08/10] DynamicTablesPkg: Update DynamicTablesPkg.ci.yaml

PierreGondois
 

Hi Sami,
Unfortunately this is still necessary,

cf
https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=29900&view=logs&j=216bd3cb-36c2-5579-221e-bd2f77088687&t=156c6dac-d9ee-52ac-8143-8428ed0a9e36

ERROR - EFI coding style error
ERROR - *Error code: 8003
ERROR - *The #ifndef at the start of an include file should use both
prefix and postfix underscore characters, '_'
ERROR - *file: D:\a\1\s\DynamicTablesPkg\Include\Library\AmlLib\AmlLib.h
ERROR - *Line number: 623
ERROR - *The #ifndef name [DISABLE_NEW_DEPRECATED_INTERFACES] does not
follow the rules

Regards,

Pierre

On 9/22/21 4:48 PM, Sami Mujawar via Groups.Io wrote:
Hi Pierre,

On Wed, Jun 23, 2021 at 04:05 AM, PierreGondois wrote:

2- Disable the Ecc check 8003 for the following keyword:
"DISABLE_NEW_DEPRECATED_INTERFACES"
Indeed, this error has been corrected on the latest version of
BaseTools, but is still triggered when using the older python
packages containing the BaseTools.

Can you check if the 8003 error needs to be disabled with latest
Basetools, please? If not can you drop this part from the patch.

Regards,

Sami Mujawar


Re: [PATCH V2] MdeModulePkg/BootManagerMenuApp: Limit string drawing within one line

Ni, Ray
 

Another option is to use EfiBootManagerRegisterBootDescriptionHandler() to alter the boot descriptions.

-----Original Message-----
From: Gao, Zhichao <zhichao.gao@intel.com>
Sent: Thursday, September 23, 2021 11:47 AM
To: devel@edk2.groups.io; gaoliming@byosoft.com.cn; Ni, Ray <ray.ni@intel.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>
Subject: RE: [edk2-devel] [PATCH V2] MdeModulePkg/BootManagerMenuApp: Limit string drawing within one line

Hi Liming,

Yes. Because the design of the BM app is not aimed to display the boot option over one line. And it is not using the setup
browser engine.
That would cause the difference.
If we want to make them align, there are two options:
1. BM app to use the setup browser engine
2. add scroll bar logic for the boot item

Both above change is not simple and may cause new issues. It would be a new design other than a bug fix.

Another solution is the patch V1 to limit the boot option description within 72 characters. Ray pointed out it is not a good
solution.

BTW, I would remove the change-id in next patch.

Thanks,
Zhichao

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
gaoliming
Sent: Thursday, September 23, 2021 10:59 AM
To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray
<ray.ni@intel.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>
Subject: 回复: [edk2-devel] [PATCH V2]
MdeModulePkg/BootManagerMenuApp: Limit string drawing within one line

Zhichao:
With this change, the same boot option will be displayed differently in
BootManagerApp and BootManager Page. Is it the designed behavior?

Besides, please remove change-id from the commit message.

Thanks
Liming
-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Gao,
Zhichao
发送时间: 2021年9月22日 12:50
收件人: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Liming Gao
<gaoliming@byosoft.com.cn>
抄送: Wang, Jian J <jian.j.wang@intel.com>
主题: Re: [edk2-devel] [PATCH V2]
MdeModulePkg/BootManagerMenuApp:
Limit string drawing within one line

Hi Liming,

The solution is different with the first time we discussed on the
Bugzilla. Can
you review if it is OK to you?

Thanks,
Zhichao

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Wednesday, September 22, 2021 11:28 AM
To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@intel.com>; Liming Gao
<gaoliming@byosoft.com.cn>
Subject: RE: [PATCH V2] MdeModulePkg/BootManagerMenuApp: Limit
string drawing within one line

Reviewed-by: Ray Ni <ray.ni@intel.com>

-----Original Message-----
From: Gao, Zhichao <zhichao.gao@intel.com>
Sent: Thursday, September 9, 2021 3:26 PM
To: devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@intel.com>; Liming Gao
<gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>
Subject: [PATCH V2] MdeModulePkg/BootManagerMenuApp: Limit
string
drawing within one line

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

Limit the draw box always within the screen's column and row.
Limit the string drawing within one line.

Change-Id: Ib7bd63cb07b23875a1e4f37ae80a422e1d5ed54f
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---

V2:

Drop the change in UefiBootManagerLib in V1.

Add the limitation in BootManagerMenuApp instead.


.../BootManagerMenuApp/BootManagerMenu.c | 72
++++++++++++++++++-
1 file changed, 69 insertions(+), 3 deletions(-)

diff --git
a/MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenu.
c
b/MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenu.
c
index 9e729074ec..d4bdeba073 100644
---
a/MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenu.
c
+++
b/MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenu.
c
@@ -1,7 +1,7 @@
/** @file

The application to show the Boot Manager Menu.



-Copyright (c) 2011 - 2018, Intel Corporation. All rights
reserved.<BR>

+Copyright (c) 2011 - 2021, Intel Corporation. All rights
+reserved.<BR>

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



**/

@@ -45,9 +45,56 @@ PrintStringAt (
IN CHAR16 *String

)

{

+ UINTN ScreenWidth;

+ UINTN ScreenRows;

+ CHAR16 *TurncateString;

+ EFI_STATUS Status;

+ UINTN ShowingLength;



gST->ConOut->SetCursorPosition (gST->ConOut, Column, Row);

- return Print (L"%s", String);

+

+ gST->ConOut->QueryMode (

+ gST->ConOut,

+ gST->ConOut->Mode->Mode,

+ &ScreenWidth,

+ &ScreenRows

+ );

+

+ if (Column > (ScreenWidth - 1) || Row > (ScreenRows - 1)) {

+ return 0;

+ }

+

+ if ((StrLen (String) + Column) > (ScreenWidth - 1)) {

+ //

+ // | - ScreenWidth - |

+ // ...Column.....................

+ // TurncateString length should leave one character for draw
+ box
and

+ // require one character for string end.

+ //

+ ShowingLength = ScreenWidth - Column - 1;

+ TurncateString = AllocatePool ((ShowingLength + 1) * sizeof
(CHAR16));

+

+ if (TurncateString == NULL) {

+ return 0;

+ }

+

+ Status = StrnCpyS (TurncateString, ShowingLength + 1, String,
ShowingLength - 3);

+

+ if (EFI_ERROR (Status)) {

+ FreePool (TurncateString);

+ return 0;

+ }

+

+ *(TurncateString + ShowingLength - 3) = L'.';

+ *(TurncateString + ShowingLength - 2) = L'.';

+ *(TurncateString + ShowingLength - 1) = L'.';

+ *(TurncateString + ShowingLength) = L'\0';

+ ShowingLength = Print (L"%s", TurncateString);

+ FreePool (TurncateString);

+ return ShowingLength;

+ } else {

+ return Print (L"%s", String);

+ }

}



/**

@@ -68,7 +115,22 @@ PrintCharAt (
CHAR16 Character

)

{

+ UINTN ScreenWidth;

+ UINTN ScreenRows;

+

gST->ConOut->SetCursorPosition (gST->ConOut, Column, Row);

+

+ gST->ConOut->QueryMode (

+ gST->ConOut,

+ gST->ConOut->Mode->Mode,

+ &ScreenWidth,

+ &ScreenRows

+ );

+

+ if (Column > (ScreenWidth - 1) || Row > (ScreenRows - 1)) {

+ return 0;

+ }

+

return Print (L"%c", Character);

}



@@ -193,7 +255,11 @@ InitializeBootMenuScreen (


MaxPrintRows = Row - 6;

UnSelectableItmes = TITLE_TOKEN_COUNT + 2 +
HELP_TOKEN_COUNT + 2;

- BootMenuData->MenuScreen.Width = MaxStrWidth + 8;

+ if (MaxStrWidth + 8 > Column) {

+ BootMenuData->MenuScreen.Width = Column;

+ } else {

+ BootMenuData->MenuScreen.Width = MaxStrWidth + 8;

+ }

if (BootMenuData->ItemCount + UnSelectableItmes > MaxPrintRows) {

BootMenuData->MenuScreen.Height = MaxPrintRows;

BootMenuData->ScrollBarControl.HasScrollBar = TRUE;

--
2.31.1.windows.1









Re: [edk2-libc Patch 1/1] AppPkg/Applications/Python/Python3.6.8: add IA32 support for py3 package creation batch script

Jayaprakash, N
 

Thank you Rebecca.
I have submitted the updated patch for review.

Regards,
JP

-----Original Message-----
From: Rebecca Cran <rebecca@nuviainc.com>
Sent: 23 September 2021 06:59
To: Jayaprakash, N <n.jayaprakash@intel.com>; devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [edk2-libc Patch 1/1] AppPkg/Applications/Python/Python3.6.8: add IA32 support for py3 package creation batch script

You should be able to use the same branch.


--
Rebecca Cran


On 9/21/21 8:33 PM, Jayaprakash, N wrote:
Hi Rebecca,

Can I resubmit the patch after making these changes in the same development branch or does it require creating a new branch?

Regards,
JP

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Rebecca Cran
Sent: 21 September 2021 22:05
To: Jayaprakash, N <n.jayaprakash@intel.com>; devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [edk2-libc Patch 1/1] AppPkg/Applications/Python/Python3.6.8: add IA32 support for py3 package creation batch script

There are several lines with trailing whitespace. Could you fix them please?


> git diff | grep " $"


+echo Invalid command line arguments passed, please see the below usage
instructions



+   mkdir %OUT_FOLDER%\EFI\Tools
+)

+if not exist %OUT_FOLDER%\EFI\StdLib\lib\python36.8 (

+    echo Python EFI package available at %OUT_FOLDER%

+echo Then use this script to create a Python EFI package



On 9/19/21 6:25 AM, Jayaprakash Nevara wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3638

This change is to add IA32 support into py3 EFI package
creation batch script. Enhanced the script take Architecture
as an additional parameter. With this the script can be used
to create deployable Python 3.6.8 EFI package from X64 and IA32 builds
as required by the user

Cc: Rebecca Cran <rebecca@nuviainc.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Jayaprakash N <n.jayaprakash@intel.com>
---
.../Python/Python-3.6.8/Py368ReadMe.txt | 4 +-
.../Python-3.6.8/create_python368_pkg.bat | 62 ++++++++++++-------
2 files changed, 41 insertions(+), 25 deletions(-)

diff --git a/AppPkg/Applications/Python/Python-3.6.8/Py368ReadMe.txt b/AppPkg/Applications/Python/Python-3.6.8/Py368ReadMe.txt
index 94dbccc..8f4fdc6 100644
--- a/AppPkg/Applications/Python/Python-3.6.8/Py368ReadMe.txt
+++ b/AppPkg/Applications/Python/Python-3.6.8/Py368ReadMe.txt
@@ -118,11 +118,11 @@ system as follows:
A script, create_python368_pkg.bat , is provided which facilitates the population
of the target EFI package. Execute this script from within the
AppPkg/Applications/Python/Python-3.6.8 directory, providing the Tool Chain, Target
- Build and destination directory which is the path to the destination directory.
+ Build, Architecture and Directory path to the destination directory.
The appropriate contents of the AppPkg/Applications/Python/Python-3.6.8/Lib and
Python368.efi Application from Build/AppPkg/RELEASE_VS2017/X64/ will be
^^^^^^^^^^^^^^
- copied into the specified destination directory.
+ copied into the specified Destination directory.

Replace "RELEASE_VS2017", in the source path, with values appropriate for your tool chain.

diff --git a/AppPkg/Applications/Python/Python-3.6.8/create_python368_pkg.bat b/AppPkg/Applications/Python/Python-3.6.8/create_python368_pkg.bat
index 6bbdbd9..2bb62b6 100644
--- a/AppPkg/Applications/Python/Python-3.6.8/create_python368_pkg.bat
+++ b/AppPkg/Applications/Python/Python-3.6.8/create_python368_pkg.bat
@@ -2,47 +2,63 @@

set TOOL_CHAIN_TAG=%1
set TARGET=%2
-set OUT_FOLDER=%3
+set ARCH=%3
+set OUT_FOLDER=%4
if "%TOOL_CHAIN_TAG%"=="" goto usage
if "%TARGET%"=="" goto usage
+if "%ARCH%"=="" goto usage
if "%OUT_FOLDER%"=="" goto usage
goto continue

:usage
echo.
+echo Batch Script to create Python EFI Package.
echo.
+echo Invalid command line arguments passed, please see the below usage instructions
echo.
-echo Creates Python EFI Package.
-echo.
-echo "Usage: %0 <ToolChain> <Target> <OutFolder>"
-echo.
-echo ToolChain = one of VS2013x86, VS2015x86, VS2017, VS2019
-echo Target = one of RELEASE, DEBUG
-echo OutFolder = Target folder where package needs to create
-echo.
+echo "Usage: %0 <ToolChain> <Target> <Architecture> <OutFolder>"
echo.
+echo ToolChain = one of VS2013x86, VS2015x86, VS2017, VS2019
+echo Target = one of RELEASE, DEBUG
+echo Architecture = one of IA32, X64
+echo OutFolder = Output directory for creating the package
echo.

goto :eof

:continue
cd ..\..\..\..\
-IF NOT EXIST Build\AppPkg\%TARGET%_%TOOL_CHAIN_TAG%\X64\Python368.efi goto error
-mkdir %OUT_FOLDER%\EFI\Tools
-xcopy Build\AppPkg\%TARGET%_%TOOL_CHAIN_TAG%\X64\Python368.efi %OUT_FOLDER%\EFI\Tools\ /y
-mkdir %OUT_FOLDER%\EFI\StdLib\lib\python36.8
-mkdir %OUT_FOLDER%\EFI\StdLib\etc
-xcopy AppPkg\Applications\Python\Python-3.6.8\Lib\* %OUT_FOLDER%\EFI\StdLib\lib\python36.8\ /Y /S /I
-xcopy StdLib\Efi\StdLib\etc\* %OUT_FOLDER%\EFI\StdLib\etc\ /Y /S /I
-goto all_done
-
-:error
-echo Failed to Create Python 3.6.8 Package, Python368.efi is not available on build location Build\AppPkg\%TARGET%_%TOOL_CHAIN_TAG%\X64\
+if not exist Build\AppPkg\%TARGET%_%TOOL_CHAIN_TAG%\%ARCH%\Python368.efi (
+ goto error
+)

+if not exist %OUT_FOLDER%\EFI\Tools (
+ mkdir %OUT_FOLDER%\EFI\Tools
+)
+xcopy Build\AppPkg\%TARGET%_%TOOL_CHAIN_TAG%\%ARCH%\Python368.efi %OUT_FOLDER%\EFI\Tools\ /y

-:all_done
-exit /b %ec%
-
+if not exist %OUT_FOLDER%\EFI\StdLib\lib\python36.8 (
+ mkdir %OUT_FOLDER%\EFI\StdLib\lib\python36.8
+)
+if not exist %OUT_FOLDER%\EFI\StdLib\etc (
+ mkdir %OUT_FOLDER%\EFI\StdLib\etc
+)
+xcopy AppPkg\Applications\Python\Python-3.6.8\Lib\* %OUT_FOLDER%\EFI\StdLib\lib\python36.8\ /Y /S /I
+xcopy StdLib\Efi\StdLib\etc\* %OUT_FOLDER%\EFI\StdLib\etc\ /Y /S /I
+echo.

+if not x%OUT_FOLDER::=%==x%OUT_FOLDER% (
+ echo Python EFI package available at %OUT_FOLDER%
+) else (
+ echo Python EFI package available at %CD%\%OUT_FOLDER%
+)
+goto all_done

+:error
+echo Failed to Create Python EFI Package
+echo Python368.efi is not available at Build\AppPkg\%TARGET%_%TOOL_CHAIN_TAG%\%ARCH%\
+echo Follow the instructions in Py368ReadMe.txt to build Python interpreter
+echo Then use this script to create a Python EFI package

+:all_done
+exit /b %ERRORLEVEL%



Re: [PATCH V2] MdeModulePkg/BootManagerMenuApp: Limit string drawing within one line

Gao, Zhichao
 

Hi Liming,

Yes. Because the design of the BM app is not aimed to display the boot option over one line. And it is not using the setup browser engine.
That would cause the difference.
If we want to make them align, there are two options:
1. BM app to use the setup browser engine
2. add scroll bar logic for the boot item

Both above change is not simple and may cause new issues. It would be a new design other than a bug fix.

Another solution is the patch V1 to limit the boot option description within 72 characters. Ray pointed out it is not a good solution.

BTW, I would remove the change-id in next patch.

Thanks,
Zhichao

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
gaoliming
Sent: Thursday, September 23, 2021 10:59 AM
To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray
<ray.ni@intel.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>
Subject: 回复: [edk2-devel] [PATCH V2]
MdeModulePkg/BootManagerMenuApp: Limit string drawing within one line

Zhichao:
With this change, the same boot option will be displayed differently in
BootManagerApp and BootManager Page. Is it the designed behavior?

Besides, please remove change-id from the commit message.

Thanks
Liming
-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Gao,
Zhichao
发送时间: 2021年9月22日 12:50
收件人: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Liming Gao
<gaoliming@byosoft.com.cn>
抄送: Wang, Jian J <jian.j.wang@intel.com>
主题: Re: [edk2-devel] [PATCH V2]
MdeModulePkg/BootManagerMenuApp:
Limit string drawing within one line

Hi Liming,

The solution is different with the first time we discussed on the
Bugzilla. Can
you review if it is OK to you?

Thanks,
Zhichao

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Wednesday, September 22, 2021 11:28 AM
To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@intel.com>; Liming Gao
<gaoliming@byosoft.com.cn>
Subject: RE: [PATCH V2] MdeModulePkg/BootManagerMenuApp: Limit
string drawing within one line

Reviewed-by: Ray Ni <ray.ni@intel.com>

-----Original Message-----
From: Gao, Zhichao <zhichao.gao@intel.com>
Sent: Thursday, September 9, 2021 3:26 PM
To: devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@intel.com>; Liming Gao
<gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>
Subject: [PATCH V2] MdeModulePkg/BootManagerMenuApp: Limit
string
drawing within one line

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

Limit the draw box always within the screen's column and row.
Limit the string drawing within one line.

Change-Id: Ib7bd63cb07b23875a1e4f37ae80a422e1d5ed54f
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---

V2:

Drop the change in UefiBootManagerLib in V1.

Add the limitation in BootManagerMenuApp instead.


.../BootManagerMenuApp/BootManagerMenu.c | 72
++++++++++++++++++-
1 file changed, 69 insertions(+), 3 deletions(-)

diff --git
a/MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenu.
c
b/MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenu.
c
index 9e729074ec..d4bdeba073 100644
---
a/MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenu.
c
+++
b/MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenu.
c
@@ -1,7 +1,7 @@
/** @file

The application to show the Boot Manager Menu.



-Copyright (c) 2011 - 2018, Intel Corporation. All rights
reserved.<BR>

+Copyright (c) 2011 - 2021, Intel Corporation. All rights
+reserved.<BR>

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



**/

@@ -45,9 +45,56 @@ PrintStringAt (
IN CHAR16 *String

)

{

+ UINTN ScreenWidth;

+ UINTN ScreenRows;

+ CHAR16 *TurncateString;

+ EFI_STATUS Status;

+ UINTN ShowingLength;



gST->ConOut->SetCursorPosition (gST->ConOut, Column, Row);

- return Print (L"%s", String);

+

+ gST->ConOut->QueryMode (

+ gST->ConOut,

+ gST->ConOut->Mode->Mode,

+ &ScreenWidth,

+ &ScreenRows

+ );

+

+ if (Column > (ScreenWidth - 1) || Row > (ScreenRows - 1)) {

+ return 0;

+ }

+

+ if ((StrLen (String) + Column) > (ScreenWidth - 1)) {

+ //

+ // | - ScreenWidth - |

+ // ...Column.....................

+ // TurncateString length should leave one character for draw
+ box
and

+ // require one character for string end.

+ //

+ ShowingLength = ScreenWidth - Column - 1;

+ TurncateString = AllocatePool ((ShowingLength + 1) * sizeof
(CHAR16));

+

+ if (TurncateString == NULL) {

+ return 0;

+ }

+

+ Status = StrnCpyS (TurncateString, ShowingLength + 1, String,
ShowingLength - 3);

+

+ if (EFI_ERROR (Status)) {

+ FreePool (TurncateString);

+ return 0;

+ }

+

+ *(TurncateString + ShowingLength - 3) = L'.';

+ *(TurncateString + ShowingLength - 2) = L'.';

+ *(TurncateString + ShowingLength - 1) = L'.';

+ *(TurncateString + ShowingLength) = L'\0';

+ ShowingLength = Print (L"%s", TurncateString);

+ FreePool (TurncateString);

+ return ShowingLength;

+ } else {

+ return Print (L"%s", String);

+ }

}



/**

@@ -68,7 +115,22 @@ PrintCharAt (
CHAR16 Character

)

{

+ UINTN ScreenWidth;

+ UINTN ScreenRows;

+

gST->ConOut->SetCursorPosition (gST->ConOut, Column, Row);

+

+ gST->ConOut->QueryMode (

+ gST->ConOut,

+ gST->ConOut->Mode->Mode,

+ &ScreenWidth,

+ &ScreenRows

+ );

+

+ if (Column > (ScreenWidth - 1) || Row > (ScreenRows - 1)) {

+ return 0;

+ }

+

return Print (L"%c", Character);

}



@@ -193,7 +255,11 @@ InitializeBootMenuScreen (


MaxPrintRows = Row - 6;

UnSelectableItmes = TITLE_TOKEN_COUNT + 2 +
HELP_TOKEN_COUNT + 2;

- BootMenuData->MenuScreen.Width = MaxStrWidth + 8;

+ if (MaxStrWidth + 8 > Column) {

+ BootMenuData->MenuScreen.Width = Column;

+ } else {

+ BootMenuData->MenuScreen.Width = MaxStrWidth + 8;

+ }

if (BootMenuData->ItemCount + UnSelectableItmes > MaxPrintRows) {

BootMenuData->MenuScreen.Height = MaxPrintRows;

BootMenuData->ScrollBarControl.HasScrollBar = TRUE;

--
2.31.1.windows.1









[PATCH EDK2 v1 0/1] UefiCpuPkg/CpuMpPei: Remove MigrateGdt declaration

wenyi,xie
 

Main Changes :
1.remove declaration of MigrateGdt

Wenyi Xie (1):
UefiCpuPkg/CpuMpPei: Remove MigrateGdt declaration

UefiCpuPkg/CpuMpPei/CpuMpPei.h | 12 ------------
1 file changed, 12 deletions(-)

--
2.20.1.windows.1


[PATCH EDK2 v1 1/1] UefiCpuPkg/CpuMpPei: Remove MigrateGdt declaration

wenyi,xie
 

The definition of MigrateGdt has been moved to SecMain since
commit f6ec1dd3, so also remove declaration of MigrateGdt left
in CpuMpPei.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Wenyi Xie <xiewenyi2@huawei.com>
---
UefiCpuPkg/CpuMpPei/CpuMpPei.h | 12 ------------
1 file changed, 12 deletions(-)

diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
index c6870656ca64..1dac4e2eb8b1 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
@@ -398,18 +398,6 @@ SecPlatformInformation2 (
OUT EFI_SEC_PLATFORM_INFORMATION_RECORD2 *PlatformInformationRecord2
);

-/**
- Migrates the Global Descriptor Table (GDT) to permanent memory.
-
- @retval EFI_SUCCESS The GDT was migrated successfully.
- @retval EFI_OUT_OF_RESOURCES The GDT could not be migrated due to lack of available memory.
-
-**/
-EFI_STATUS
-MigrateGdt (
- VOID
- );
-
/**
Initializes MP and exceptions handlers.

--
2.20.1.windows.1

8901 - 8920 of 89854