Re: [edk2-devel] [PATCH v1 4/4] OvmfPkg: Do not check VbeShim.h formatting with Uncrustify
On 03/22/22 02:58, mikub...@linux.microsoft.com wrote:
> From: Michael Kubacki
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3875
>
> The following files:
> OvmfPkg/Bhyve/BhyveRfbDxe/VbeShim.h
> OvmfPkg/QemuVideoDxe/VbeShim.h
>
> Are auto generated by the following generators:
> OvmfPkg/Bhyve/BhyveRfbDxe/VbeShim.sh
> OvmfPkg/QemuVideoDxe/VbeShim.sh
>
> Therefore, Uncrustify causes a file update to produce a very large
> diff due to formatting changes.
>
> This change does the following:
> 1. Reverts the Uncrustify changes applied to the files in commit
> ac0a286f4d74.
> 2. Uses a new UncrustifyCheck CI plugin configuration option to
> exclude the files from future formatting checks.
>
> Cc: Ard Biesheuvel
> Cc: Jiewen Yao
> Cc: Jordan Justen
> Cc: Gerd Hoffmann
> Cc: Rebecca Cran
> Cc: Peter Grehan
> Cc: Laszlo Ersek
> Signed-off-by: Michael Kubacki
> ---
> OvmfPkg/OvmfPkg.ci.yaml | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/OvmfPkg/OvmfPkg.ci.yaml b/OvmfPkg/OvmfPkg.ci.yaml
> index 7459b84d8090..ff022242b018 100644
> --- a/OvmfPkg/OvmfPkg.ci.yaml
> +++ b/OvmfPkg/OvmfPkg.ci.yaml
> @@ -97,5 +97,10 @@
> ], # words to extend to the dictionary for this package
> "IgnoreStandardPaths": [], # Standard Plugin defined paths that
> should be ignore
> "AdditionalIncludePaths": [] # Additional paths to spell check
> (wildcards supported)
> +},
> +
> +# options defined in .pytool/Plugin/UncrustifyCheck
> +"UncrustifyCheck": {
> + "IgnoreFiles": ["VbeShim.h"]
> }
> }
>
Acked-by: Laszlo Ersek
(Didn't expect the plugin / new option to take relative pathnames / last
components as well, but it's very handy in this case.)
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87817): https://edk2.groups.io/g/devel/message/87817
Mute This Topic: https://groups.io/mt/89944186/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 3/4] OvmfPkg: Revert Uncrustify formatting in VbeShim.h files
On 03/22/22 02:58, mikub...@linux.microsoft.com wrote: > From: Michael Kubacki > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3875 > > Reverts changes automatically applied by Uncrustify in commit > ac0a286. > > Cc: Ard Biesheuvel > Cc: Jiewen Yao > Cc: Jordan Justen > Cc: Gerd Hoffmann > Cc: Rebecca Cran > Cc: Peter Grehan > Cc: Laszlo Ersek > Signed-off-by: Michael Kubacki > --- > OvmfPkg/Bhyve/BhyveRfbDxe/VbeShim.h | 1811 ++-- > OvmfPkg/QemuVideoDxe/VbeShim.h | 1389 --- > 2 files changed, 1599 insertions(+), 1601 deletions(-) Acked-by: Laszlo Ersek Thank you Michael. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#87816): https://edk2.groups.io/g/devel/message/87816 Mute This Topic: https://groups.io/mt/89944185/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V9 14/47] UefiCpuPkg: Enable Tdx support in MpInitLib
Hi, Ray Can you help to review this patch? https://edk2.groups.io/g/devel/message/87690 Thanks Min -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#87815): https://edk2.groups.io/g/devel/message/87815 Mute This Topic: https://groups.io/mt/89859015/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V9 02/47] MdePkg: Update Cpuid.h for Tdx
Hi, Liming Can you help to review this patch? https://edk2.groups.io/g/devel/message/87678 Thanks Min -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#87814): https://edk2.groups.io/g/devel/message/87814 Mute This Topic: https://groups.io/mt/89858996/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v1 4/4] OvmfPkg: Do not check VbeShim.h formatting with Uncrustify
From: Michael Kubacki
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3875
The following files:
OvmfPkg/Bhyve/BhyveRfbDxe/VbeShim.h
OvmfPkg/QemuVideoDxe/VbeShim.h
Are auto generated by the following generators:
OvmfPkg/Bhyve/BhyveRfbDxe/VbeShim.sh
OvmfPkg/QemuVideoDxe/VbeShim.sh
Therefore, Uncrustify causes a file update to produce a very large
diff due to formatting changes.
This change does the following:
1. Reverts the Uncrustify changes applied to the files in commit
ac0a286f4d74.
2. Uses a new UncrustifyCheck CI plugin configuration option to
exclude the files from future formatting checks.
Cc: Ard Biesheuvel
Cc: Jiewen Yao
Cc: Jordan Justen
Cc: Gerd Hoffmann
Cc: Rebecca Cran
Cc: Peter Grehan
Cc: Laszlo Ersek
Signed-off-by: Michael Kubacki
---
OvmfPkg/OvmfPkg.ci.yaml | 5 +
1 file changed, 5 insertions(+)
diff --git a/OvmfPkg/OvmfPkg.ci.yaml b/OvmfPkg/OvmfPkg.ci.yaml
index 7459b84d8090..ff022242b018 100644
--- a/OvmfPkg/OvmfPkg.ci.yaml
+++ b/OvmfPkg/OvmfPkg.ci.yaml
@@ -97,5 +97,10 @@
], # words to extend to the dictionary for this package
"IgnoreStandardPaths": [], # Standard Plugin defined paths that
should be ignore
"AdditionalIncludePaths": [] # Additional paths to spell check
(wildcards supported)
+},
+
+# options defined in .pytool/Plugin/UncrustifyCheck
+"UncrustifyCheck": {
+ "IgnoreFiles": ["VbeShim.h"]
}
}
--
2.28.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87813): https://edk2.groups.io/g/devel/message/87813
Mute This Topic: https://groups.io/mt/89944186/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v1 2/4] .pytool/Plugin/UncrustifyCheck: Add ignore file support
From: Michael Kubacki
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3880
Currently UncrustifyCheck.py provides the following exclusion
options:
1. Override the type of files UncrustifyCheck operates against by
default (.c and .h files). Using the "IgnoreStandardPaths"
configuration option.
2. By default, UncrustifyCheck skips files in git submodules and
ignored by git (the "SkipGitExclusions" configuration option can
override this behavior).
The goal of UncrustifyCheck is to provide consistent formatting
across the codebase. In some rare circumstances, maintainers might
need to exclude a specific file (or file pattern) within their
package. For example, a small set of auto-generated files from
another repository.
This change adds a new configuration option that can be specified
in a package CI YAML file to describe a list of files within the
package that should be ignored by UncrustifyCheck.
The configuration option is called "IgnoreFiles" and it uses similar
syntax to git ignore to ignore a list of files.
Cc: Michael D Kinney
Cc: Liming Gao
Cc: Sean Brogan
Cc: Bret Barkelew
Signed-off-by: Michael Kubacki
---
.pytool/Plugin/UncrustifyCheck/Readme.md | 7 ++
.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py | 24
2 files changed, 31 insertions(+)
diff --git a/.pytool/Plugin/UncrustifyCheck/Readme.md
b/.pytool/Plugin/UncrustifyCheck/Readme.md
index 0c46fd241a7a..efe7a573e4fa 100644
--- a/.pytool/Plugin/UncrustifyCheck/Readme.md
+++ b/.pytool/Plugin/UncrustifyCheck/Readme.md
@@ -41,6 +41,7 @@ The plugin can be configured with a few optional
configuration options.
"AdditionalIncludePaths": [], # Additional paths to check formatting
(wildcards supported).
"AuditOnly": False, # Don't fail the build if there are
errors. Just log them.
"ConfigFilePath": "", # Custom path to an Uncrustify config file.
+ "IgnoreFiles": [],# A list of file patterns to ignore.
"IgnoreStandardPaths": [],# Standard Plugin defined paths that
should be ignored.
"OutputFileDiffs": True, # Output chunks of formatting diffs in the
test case log.
# This can significantly slow down the
plugin on very large packages.
@@ -67,6 +68,12 @@ the test as skipped. This allows visibility into the
failures without breaking t
When specified in the config file, this is a package relative path to the
Uncrustify configuration file.
+### `IgnoreFiles`
+
+This option supports .gitignore file and folder matching strings including
wildcards.
+
+The files specified by this configuration option will not be processed by
Uncrustify.
+
### `IgnoreStandardPaths`
This plugin by default will check the below standard paths. A package
configuration file can specify any of these paths
diff --git a/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py
b/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py
index dc133fecc4b2..e8370381032f 100644
--- a/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py
+++ b/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py
@@ -13,11 +13,13 @@ import os
import pathlib
import shutil
import timeit
+from collections.abc import Callable
from edk2toolext.environment import version_aggregator
from edk2toolext.environment.plugin_manager import PluginManager
from edk2toolext.environment.plugintypes.ci_build_plugin import ICiBuildPlugin
from edk2toolext.environment.plugintypes.uefi_helper_plugin import
HelperFunctions
from edk2toolext.environment.var_dict import VarDict
+from edk2toollib.gitignore_parser import parse_gitignore_lines
from edk2toollib.log.junit_report_format import JunitReportTestCase
from edk2toollib.uefi.edk2.path_utilities import Edk2Path
from edk2toollib.utility_functions import RunCmd
@@ -273,6 +275,15 @@ class UncrustifyCheck(ICiBuildPlugin):
f"-c {self._app_config_file} -F {self._app_input_file_path}
--if-changed --suffix {UncrustifyCheck.FORMATTED_FILE_EXTENSION}",
outstream=output)
self._app_output = output.getvalue().strip().splitlines()
+def _get_files_ignored_in_config(self) -> Callable[[str], bool]:
+
+Returns a function that returns true if a given file string path is
ignored in the plugin configuration file and false otherwise.
+"""
+ignored_files = []
+if "IgnoreFiles" in self._package_config:
+ignored_files = self._package_config["IgnoreFiles"]
+return parse_gitignore_lines(ignored_files,
os.path.join(self._abs_workspace_path, "nofile.txt"), self._abs_workspace_path)
+
def _get_git_ignored_paths(self) -> List[str]:
Returns a list of file absolute path strings to all files ignored in
this git repository.
@@ -464,6 +475,19 @@ class UncrustifyCheck(ICiBuildPlugin):
self._abs_file_paths_to_format.extend(
[str(path.resolve()) for path in
pathlib.Path(self._abs_package_path).r
[edk2-devel] [PATCH v1 1/4] .pytool/Plugin/UncrustifyCheck: Update func to return absolute paths
From: Michael Kubacki Currently, UncrustifyCheck._get_git_ignored_paths() is documented to return a list of absolute file paths but it currently returns a list of relative file paths. This change updates the function to return a list of absolute file paths. The result is later compared to the list of absolute file paths for files to run against Uncrustify. Cc: Michael D Kinney Cc: Liming Gao Cc: Sean Brogan Cc: Bret Barkelew Signed-off-by: Michael Kubacki --- .pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py b/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py index 82db7a5a438b..dc133fecc4b2 100644 --- a/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py +++ b/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py @@ -292,7 +292,11 @@ class UncrustifyCheck(ICiBuildPlugin): f"An error occurred reading git ignore settings. This will prevent Uncrustify from running against the expected set of files.") # Note: This will potentially be a large list, but at least sorted -return outstream_buffer.getvalue().strip().splitlines() +rel_paths = outstream_buffer.getvalue().strip().splitlines() +abs_paths = [] +for path in rel_paths: +abs_paths.append(os.path.join(self._abs_workspace_path, path)) +return abs_paths def _get_git_submodule_paths(self) -> List[str]: """ -- 2.28.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#87810): https://edk2.groups.io/g/devel/message/87810 Mute This Topic: https://groups.io/mt/89944182/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v1 0/4] UncrustifyCheck Ignore Support
From: Michael Kubacki REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3880 REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3875 This patch series: 1. Updates ignore handling in UncrustifyCheck including adding a new CI plugin configuration option to ignore files in a package. 2. Reverts Uncrustify changes as requested for files in OvmfPkg. 3. Uses the new ignore file option to ignore the files that had Uncrustify formatting reverted in OvmfPkg. Cc: Michael D Kinney Cc: Liming Gao Cc: Sean Brogan Cc: Bret Barkelew Cc: Ard Biesheuvel Cc: Jiewen Yao Cc: Jordan Justen Cc: Gerd Hoffmann Cc: Rebecca Cran Cc: Peter Grehan Cc: Laszlo Ersek Signed-off-by: Michael Kubacki Michael Kubacki (4): .pytool/Plugin/UncrustifyCheck: Update func to return absolute paths .pytool/Plugin/UncrustifyCheck: Add ignore file support OvmfPkg: Revert Uncrustify formatting in VbeShim.h files OvmfPkg: Do not check VbeShim.h formatting with Uncrustify .pytool/Plugin/UncrustifyCheck/Readme.md |7 + .pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py | 30 +- OvmfPkg/Bhyve/BhyveRfbDxe/VbeShim.h | 1811 ++-- OvmfPkg/OvmfPkg.ci.yaml |5 + OvmfPkg/QemuVideoDxe/VbeShim.h| 1389 --- 5 files changed, 1640 insertions(+), 1602 deletions(-) -- 2.28.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#87809): https://edk2.groups.io/g/devel/message/87809 Mute This Topic: https://groups.io/mt/89944179/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdeModulePkg/NvmExpressDxe: fix check for Cap.Css
Will do tomorrow (UTC+1), thanks for reviewing :)
Am 22. März 2022 01:28:45 UTC schrieb "Wu, Hao A" :
>The change is good to me.
>
>Could you help to apply this change to the code in
>MdeModulePkg\Bus\Pci\NvmExpressPei\ as well?
>Really sorry for missing the above comment in previous reply.
>
>Best Regards,
>Hao Wu
>
>> -Original Message-
>> From: devel@edk2.groups.io On Behalf Of Mara
>> Sophie Grosch via groups.io
>> Sent: Tuesday, March 22, 2022 1:06 AM
>> To: devel@edk2.groups.io; Wu, Hao A
>> Subject: [edk2-devel] [PATCH] MdeModulePkg/NvmExpressDxe: fix check for
>> Cap.Css
>>
>> Fix the check for NVMe command set being supported by the controller.
>>
>> Was problematic with qemu (6.2.0, Debian 1:6.2+dfsg-3), which sets 0xC1 in
>> that
>> register, making the OVMF think the NVMe controller does not support NVMe.
>>
>> Uncovered by commit 9dd14fc91c174eae87fd122c7ac70073a363527f, which
>> changed the number of bits included in the Css register from 4 to 8.
>>
>> Signed-off-by: Mara Sophie Grosch
>> ---
>> MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
>> b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
>> index d87212ffb2..b90c48731c 100644
>> --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
>> +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
>> @@ -761,7 +761,7 @@ NvmeControllerInit (
>> return Status;
>> }
>>
>> - if (Private->Cap.Css != 0x01) {
>> + if ((Private->Cap.Css & BIT0) == 0) {
>> DEBUG ((DEBUG_INFO, "NvmeControllerInit: the controller doesn't support
>> NVMe command set\n"));
>> return EFI_UNSUPPORTED;
>> }
>> --
>> 2.35.1
>>
>>
>>
>>
>
>
>
>
>
>
--
Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87808): https://edk2.groups.io/g/devel/message/87808
Mute This Topic: https://groups.io/mt/89833412/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] Event: TianoCore Bug Triage - APAC / NAMO - 03/22/2022 #cal-reminder
BEGIN:VCALENDAR VERSION:2.0 PRODID:-//Groups.io Inc//Groups.io Calendar//EN METHOD:PUBLISH REFRESH-INTERVAL;VALUE=DURATION:PT1H X-PUBLISHED-TTL:PT1H CALSCALE:GREGORIAN BEGIN:VTIMEZONE TZID:America/Los_Angeles LAST-MODIFIED:20220317T223602Z TZURL:http://tzurl.org/zoneinfo-outlook/America/Los_Angeles X-LIC-LOCATION:America/Los_Angeles BEGIN:DAYLIGHT TZNAME:PDT TZOFFSETFROM:-0800 TZOFFSETTO:-0700 DTSTART:19700308T02 RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=2SU END:DAYLIGHT BEGIN:STANDARD TZNAME:PST TZOFFSETFROM:-0700 TZOFFSETTO:-0800 DTSTART:19701101T02 RRULE:FREQ=YEARLY;BYMONTH=11;BYDAY=1SU END:STANDARD END:VTIMEZONE BEGIN:VEVENT X-GIOIDS:Event:1238717 UID:mlda.1580078539586725120.r...@groups.io DTSTAMP:20220322T013001Z ORGANIZER;CN=Liming Gao:mailto:gaolim...@byosoft.com.cn DTSTART:20220323T013000Z DTEND:20220323T023000Z SUMMARY:TianoCore Bug Triage - APAC / NAMO DESCRIPTION:TianoCore Bug Triage - APAC / NAMO\n\nHosted by Liming Gao\n\ n \n\nMicrosoft Teams meeting\n\n*Join on your computer or mobile a pp*\n\nClick here to join the meeting ( https://teams.microsoft.com/l/mee tup-join/19%3ameeting_OTUyZTg2NjgtNDhlNS00ODVlLTllYTUtYzg1OTNjNjdiZjFh%40 thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255 d%22%2c%22Oid%22%3a%22b286b53a-1218-4db3-bfc9-3d4c5aa7669e%22%7d )\n\n*Jo in with a video conferencing device*\n\nte...@conf.intel.com\n\nVideo Con ference ID: 116 062 094 0\n\nAlternate VTC dialing instructions ( https:/ /conf.intel.com/teams/?conf=1160620940&ivr=teams&d=conf.intel.com&test=te st_call )\n\n*Or call in (audio only)*\n\n+1 916-245-6934\,\,77463821# ( tel:+19162456934\,\,77463821# ) United States\, Sacramento\n\nPhone Confe rence ID: 774 638 21#\n\nFind a local number ( https://dialin.teams.micro soft.com/d195d438-2daa-420e-b9ea-da26f9d1d6d5?id=77463821 ) | Reset PIN ( https://mysettings.lync.com/pstnconferencing )\n\nLearn More ( https://a ka.ms/JoinTeamsMeeting ) | Meeting options ( https://teams.microsoft.com/ meetingOptions/?organizerId=b286b53a-1218-4db3-bfc9-3d4c5aa7669e&tenantId =46c98d88-e344-4ed4-8496-4ed7712e255d&threadId=19_meeting_OTUyZTg2NjgtNDh lNS00ODVlLTllYTUtYzg1OTNjNjdiZjFh@thread.v2&messageId=0&language=en-US ) LOCATION:https://teams.microsoft.com/l/meetup-join/19%3ameeting_OTk1YzJhN 2UtOGQwNi00NjY4LWEwMTktY2JiODRlYTY1NmY0%40thread.v2/0?context=%7b%22Tid%2 2%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%226e4ce4c4- 1242-431b-9a51-92cd01a5df3c%22%7d SEQUENCE:2 END:VEVENT END:VCALENDAR invite.ics Description: application/ics
Re: [edk2-devel] [PATCH] MdeModulePkg/NvmExpressDxe: fix check for Cap.Css
The change is good to me.
Could you help to apply this change to the code in
MdeModulePkg\Bus\Pci\NvmExpressPei\ as well?
Really sorry for missing the above comment in previous reply.
Best Regards,
Hao Wu
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of Mara
> Sophie Grosch via groups.io
> Sent: Tuesday, March 22, 2022 1:06 AM
> To: devel@edk2.groups.io; Wu, Hao A
> Subject: [edk2-devel] [PATCH] MdeModulePkg/NvmExpressDxe: fix check for
> Cap.Css
>
> Fix the check for NVMe command set being supported by the controller.
>
> Was problematic with qemu (6.2.0, Debian 1:6.2+dfsg-3), which sets 0xC1 in
> that
> register, making the OVMF think the NVMe controller does not support NVMe.
>
> Uncovered by commit 9dd14fc91c174eae87fd122c7ac70073a363527f, which
> changed the number of bits included in the Css register from 4 to 8.
>
> Signed-off-by: Mara Sophie Grosch
> ---
> MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
> b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
> index d87212ffb2..b90c48731c 100644
> --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
> +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
> @@ -761,7 +761,7 @@ NvmeControllerInit (
> return Status;
> }
>
> - if (Private->Cap.Css != 0x01) {
> + if ((Private->Cap.Css & BIT0) == 0) {
> DEBUG ((DEBUG_INFO, "NvmeControllerInit: the controller doesn't support
> NVMe command set\n"));
> return EFI_UNSUPPORTED;
> }
> --
> 2.35.1
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87806): https://edk2.groups.io/g/devel/message/87806
Mute This Topic: https://groups.io/mt/89833412/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] Now: Tools, CI, Code base construction meeting series - 03/21/2022 #cal-notice
*Tools, CI, Code base construction meeting series* *When:* 03/21/2022 4:30pm to 5:30pm (UTC-07:00) America/Los Angeles *Where:* https://github.com/tianocore/edk2/discussions/2614 View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=1463508 ) *Description:* TianoCore community, Microsoft and Intel will be hosting a series of open meetings to discuss build, CI, tools, and other related topics. If you are interested, have ideas/opinions please join us. These meetings will be Monday 4:30pm Pacific Time on Microsoft Teams. MS Teams Link in following discussion: * https://github.com/tianocore/edk2/discussions/2614 Anyone is welcome to join. * tianocore/edk2: EDK II (github.com) * tianocore/edk2-basetools: EDK II BaseTools Python tools as a PIP module (github.com) https://github.com/tianocore/edk2-basetools * tianocore/edk2-pytool-extensions: Extensions to the edk2 build system allowing for a more robust and plugin based build system and tool execution environment (github.com) https://github.com/tianocore/edk2-pytool-extensions * tianocore/edk2-pytool-library: Python library package that supports UEFI development (github.com) https://github.com/tianocore/edk2-pytool-library MS Teams Browser Clients * https://docs.microsoft.com/en-us/microsoftteams/get-clients?tabs=Windows#browser-client -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#87805): https://edk2.groups.io/g/devel/message/87805 Mute This Topic: https://groups.io/mt/89941485/21656 Mute #cal-notice:https://edk2.groups.io/g/devel/mutehashtag/cal-notice Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 1/2] Add filesystem drivers
Hi Pedro/Patrick Thank you, I'll adjust it for that :) Kind regards, Sean -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#87804): https://edk2.groups.io/g/devel/message/87804 Mute This Topic: https://groups.io/mt/89924170/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2 3/3] BaseTools: Scripts/efi_lldb.py: Add lldb EFI commands and pretty Print
https://bugzilla.tianocore.org/show_bug.cgi?id=3500
Use efi_debugging.py Python Classes to implement EFI gdb commands:
efi_symbols, guid, table, hob, and devicepath
You can attach to any standard gdb or kdp remote server and get EFI
symbols. No modifications of EFI are required.
Example usage:
OvmfPkg/build.sh qemu -gdb tcp::9000
lldb -o "gdb-remote localhost:9000" -o "command script import efi_lldb.py"
Note you may also have to teach lldb about QEMU:
-o "settings set plugin.process.gdb-remote.target-definition-file
x86_64_target_definition.py"
Cc: Leif Lindholm
Cc: Michael D Kinney
Cc: Hao A Wu
Cc: Bob Feng
Cc: Liming Gao
Cc: Yuwei Chen
Signed-off-by: Rebecca Cran
---
BaseTools/Scripts/efi_lldb.py | 1044
1 file changed, 1044 insertions(+)
diff --git a/BaseTools/Scripts/efi_lldb.py b/BaseTools/Scripts/efi_lldb.py
new file mode 100755
index ..089b6ba58ab8
--- /dev/null
+++ b/BaseTools/Scripts/efi_lldb.py
@@ -0,0 +1,1044 @@
+#!/usr/bin/python3
+'''
+Copyright (c) Apple Inc. 2021
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+Example usage:
+OvmfPkg/build.sh qemu -gdb tcp::9000
+lldb -o "gdb-remote localhost:9000" -o "command script import efi_lldb.py"
+'''
+
+import optparse
+import shlex
+import subprocess
+import uuid
+import sys
+import os
+from pathlib import Path
+from efi_debugging import EfiDevicePath, EfiConfigurationTable, EfiTpl
+from efi_debugging import EfiHob, GuidNames, EfiStatusClass, EfiBootMode
+from efi_debugging import PeTeImage, patch_ctypes
+
+try:
+# Just try for LLDB in case PYTHONPATH is already correctly setup
+import lldb
+except ImportError:
+try:
+env = os.environ.copy()
+env['LLDB_DEFAULT_PYTHON_VERSION'] = str(sys.version_info.major)
+lldb_python_path = subprocess.check_output(
+["xcrun", "lldb", "-P"], env=env).decode("utf-8").strip()
+sys.path.append(lldb_python_path)
+import lldb
+except ValueError:
+print("Couldn't find LLDB.framework from lldb -P")
+print("PYTHONPATH should match the currently selected lldb")
+sys.exit(-1)
+
+
+class LldbFileObject(object):
+'''
+Class that fakes out file object to abstract lldb from the generic code.
+For lldb this is memory so we don't have a concept of the end of the file.
+'''
+
+def __init__(self, process):
+# _exe_ctx is lldb.SBExecutionContext
+self._process = process
+self._offset = 0
+self._SBError = lldb.SBError()
+
+def tell(self):
+return self._offset
+
+def read(self, size=-1):
+if size == -1:
+# arbitrary default size
+size = 0x100
+
+data = self._process.ReadMemory(self._offset, size, self._SBError)
+if self._SBError.fail:
+raise MemoryError(
+f'lldb could not read memory 0x{size:x} '
+f' bytes from 0x{self._offset:08x}')
+else:
+return data
+
+def readable(self):
+return True
+
+def seek(self, offset, whence=0):
+if whence == 0:
+self._offset = offset
+elif whence == 1:
+self._offset += offset
+else:
+# whence == 2 is seek from end
+raise NotImplementedError
+
+def seekable(self):
+return True
+
+def write(self, data):
+result = self._process.WriteMemory(self._offset, data, self._SBError)
+if self._SBError.fail:
+raise MemoryError(
+f'lldb could not write memory to 0x{self._offset:08x}')
+return result
+
+def writable(self):
+return True
+
+def truncate(self, size=None):
+raise NotImplementedError
+
+def flush(self):
+raise NotImplementedError
+
+def fileno(self):
+raise NotImplementedError
+
+
+class EfiSymbols:
+"""
+Class to manage EFI Symbols
+You need to pass file, and exe_ctx to load symbols.
+You can print(EfiSymbols()) to see the currently loaded symbols
+"""
+
+loaded = {}
+stride = None
+range = None
+verbose = False
+
+def __init__(self, target=None):
+if target:
+EfiSymbols.target = target
+EfiSymbols._file = LldbFileObject(target.process)
+
+@ classmethod
+def __str__(cls):
+return ''.join(f'{pecoff}\n' for (pecoff, _) in cls.loaded.values())
+
+@ classmethod
+def configure_search(cls, stride, range, verbose=False):
+cls.stride = stride
+cls.range = range
+cls.verbose = verbose
+
+@ classmethod
+def clear(cls):
+cls.loaded = {}
+
+@ classmethod
+def add_symbols_for_pecoff(cls, pecoff):
+'''Tell lldb the location of the .text and .data sections.'''
+
+if pecoff.LoadAddress in cls.loaded:
+return 'Already Loaded: '
+
+module = cls.target.AddModule(None, None, str(pecoff.CodeViewUuid))
+if not module:
+
[edk2-devel] [PATCH v2 0/3] BaseTools: Add support for gdb and lldb
This patch set adds debugging support for gdb and lldb. It also adds generic debugging classes that use a file like object to make it easy to import into any debugger that supports Python. Changes from v1 to v2: - Moved scripts from the root of the repo into BaseTools/Scripts. - Fixed typo of "RISCV" as "RISKV". Testing: - Tested gdb on Ubuntu and lldb on macOS for IA32 and X64. - Tested gdb on openSUSE for AARCH64. Rebecca Cran (3): BaseTools: efi_debugging.py: Add debugger agnostic dbg Python Classes BaseTools: Scripts/efi_gdb.py: Add gdb EFI commands and pretty Print BaseTools: Scripts/efi_lldb.py: Add lldb EFI commands and pretty Print BaseTools/Scripts/efi_debugging.py | 2185 BaseTools/Scripts/efi_gdb.py | 918 BaseTools/Scripts/efi_lldb.py | 1044 ++ 3 files changed, 4147 insertions(+) create mode 100755 BaseTools/Scripts/efi_debugging.py create mode 100755 BaseTools/Scripts/efi_gdb.py create mode 100755 BaseTools/Scripts/efi_lldb.py -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#87802): https://edk2.groups.io/g/devel/message/87802 Mute This Topic: https://groups.io/mt/89937670/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/3] BaseTools: Scripts/efi_gdb.py: Add gdb EFI commands and pretty Print
https://bugzilla.tianocore.org/show_bug.cgi?id=3500
Use efi_debugging.py Python Classes to implement EFI gdb commands:
(gdb) help efi
Commands for debugging EFI. efi
List of efi subcommands:
efi devicepath -- Display an EFI device path.
efi guid -- Display info about EFI GUID's.
efi hob -- Dump EFI HOBs. Type 'hob -h' for more info.
efi symbols -- Load Symbols for EFI. Type 'efi_symbols -h' for more info.
efi table -- Dump EFI System Tables. Type 'table -h' for more info.
This module is coded against a generic gdb remote serial stub. It should
work with QEMU, JTAG debugger, or a generic EFI gdb remote serial stub.
No modifications of EFI is required to load symbols.
Example usage:
OvmfPkg/build.sh qemu -gdb tcp::9000
gdb -ex "target remote localhost:9000" -ex "source efi_gdb.py"
Cc: Leif Lindholm
Cc: Michael D Kinney
Cc: Hao A Wu
Cc: Bob Feng
Cc: Liming Gao
Cc: Yuwei Chen
Signed-off-by: Rebecca Cran
---
BaseTools/Scripts/efi_gdb.py | 918
1 file changed, 918 insertions(+)
diff --git a/BaseTools/Scripts/efi_gdb.py b/BaseTools/Scripts/efi_gdb.py
new file mode 100755
index ..e9bae8e9b913
--- /dev/null
+++ b/BaseTools/Scripts/efi_gdb.py
@@ -0,0 +1,918 @@
+#!/usr/bin/python3
+'''
+Copyright 2021 (c) Apple Inc. All rights reserved.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+EFI gdb commands based on efi_debugging classes.
+
+Example usage:
+OvmfPkg/build.sh qemu -gdb tcp::9000
+gdb -ex "target remote localhost:9000" -ex "source efi_gdb.py"
+
+(gdb) help efi
+Commands for debugging EFI. efi
+
+List of efi subcommands:
+
+efi devicepath -- Display an EFI device path.
+efi guid -- Display info about EFI GUID's.
+efi hob -- Dump EFI HOBs. Type 'hob -h' for more info.
+efi symbols -- Load Symbols for EFI. Type 'efi_symbols -h' for more info.
+efi table -- Dump EFI System Tables. Type 'table -h' for more info.
+
+This module is coded against a generic gdb remote serial stub. It should work
+with QEMU, JTAG debugger, or a generic EFI gdb remote serial stub.
+
+If you are debugging with QEMU or a JTAG hardware debugger you can insert
+a CpuDeadLoop(); in your code, attach with gdb, and then `p Index=1` to
+step past. If you have a debug stub in EFI you can use CpuBreakpoint();.
+'''
+
+from gdb.printing import RegexpCollectionPrettyPrinter
+from gdb.printing import register_pretty_printer
+import gdb
+import os
+import sys
+import uuid
+import optparse
+import shlex
+
+# gdb will not import from the same path as this script.
+# so lets fix that for gdb...
+sys.path.append(os.path.dirname(os.path.abspath(__file__)))
+
+from efi_debugging import PeTeImage, patch_ctypes# noqa: E402
+from efi_debugging import EfiHob, GuidNames, EfiStatusClass # noqa: E402
+from efi_debugging import EfiBootMode, EfiDevicePath # noqa: E402
+from efi_debugging import EfiConfigurationTable, EfiTpl # noqa: E402
+
+
+class GdbFileObject(object):
+'''Provide a file like object required by efi_debugging'''
+
+def __init__(self):
+self.inferior = gdb.selected_inferior()
+self.offset = 0
+
+def tell(self):
+return self.offset
+
+def read(self, size=-1):
+if size == -1:
+# arbitrary default size
+size = 0x100
+
+try:
+data = self.inferior.read_memory(self.offset, size)
+except MemoryError:
+data = bytearray(size)
+assert False
+if len(data) != size:
+raise MemoryError(
+f'gdb could not read memory 0x{size:x}'
++ f' bytes from 0x{self.offset:08x}')
+else:
+# convert memoryview object to a bytestring.
+return data.tobytes()
+
+def readable(self):
+return True
+
+def seek(self, offset, whence=0):
+if whence == 0:
+self.offset = offset
+elif whence == 1:
+self.offset += offset
+else:
+# whence == 2 is seek from end
+raise NotImplementedError
+
+def seekable(self):
+return True
+
+def write(self, data):
+self.inferior.write_memory(self.offset, data)
+return len(data)
+
+def writable(self):
+return True
+
+def truncate(self, size=None):
+raise NotImplementedError
+
+def flush(self):
+raise NotImplementedError
+
+def fileno(self):
+raise NotImplementedError
+
+
+class EfiSymbols:
+"""Class to manage EFI Symbols"""
+
+loaded = {}
+stride = None
+range = None
+verbose = False
+
+def __init__(self, file=None):
+EfiSymbols.file = file if file else GdbFileObject()
+
+@ classmethod
+def __str__(cls):
+return ''.join(f'{value}\n' for value in cls.loaded.values())
+
+@ classmethod
+def configure_search(cls, stride, range=None, verbose=False):
+cls.stride = stride
+cls.range = range
+cls.verbose = verbose
+
+@ classmethod
+def
[edk2-devel] [PATCH v2 1/3] BaseTools: efi_debugging.py: Add debugger agnostic dbg Python Classes
Add Scripts/efi_debugging.py to provide debugger agnostic debugging
utility Python classes.
Cc: Leif Lindholm
Cc: Michael D Kinney
Cc: Hao A Wu
Cc: Bob Feng
Cc: Liming Gao
Cc: Yuwei Chen
Signed-off-by: Rebecca Cran
---
BaseTools/Scripts/efi_debugging.py | 2185
1 file changed, 2185 insertions(+)
diff --git a/BaseTools/Scripts/efi_debugging.py
b/BaseTools/Scripts/efi_debugging.py
new file mode 100755
index ..af975f4e6ceb
--- /dev/null
+++ b/BaseTools/Scripts/efi_debugging.py
@@ -0,0 +1,2185 @@
+#!/usr/bin/python3
+'''
+Copyright (c) Apple Inc. 2021
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+Class that abstracts PE/COFF debug info parsing via a Python file like
+object. You can port this code into an arbitrary debugger by invoking
+the classes and passing in a file like object that abstracts the debugger
+reading memory.
+
+If you run this file directly it will parse the passed in PE/COFF files
+for debug info:
+python3 ./efi_pefcoff.py DxeCore.efi
+IA32`/DxeCore.dll load = 0x
+EntryPoint = 0x30d2 TextAddress = 0x0240 DataAddress = 0x42c0
+.text0x0240 (0x04080) flags:0x6020
+.data0x42C0 (0x001C0) flags:0xC040
+.reloc 0x4480 (0x00240) flags:0x4240
+
+Note: PeCoffClass uses virtual addresses and not file offsets.
+ It needs to work when images are loaded into memory.
+ as long as virtual address map to file addresses this
+ code can process binary files.
+
+Note: This file can also contain generic worker functions (like GuidNames)
+ that abstract debugger agnostic services to the debugger.
+
+This file should never import debugger specific modules.
+'''
+
+import sys
+import os
+import uuid
+import struct
+import re
+from ctypes import c_char, c_uint8, c_uint16, c_uint32, c_uint64, c_void_p
+from ctypes import ARRAY, sizeof
+from ctypes import Structure, LittleEndianStructure
+
+#
+# The empty LittleEndianStructure must have _fields_ assigned prior to use or
+# sizeof(). Anything that is size UINTN may need to get adjusted.
+#
+# The issue is ctypes matches our local machine, not the machine we are
+# trying to debug. Call patch_ctypes() passing in the byte width from the
+# debugger python to make sure you are in sync.
+#
+# Splitting out the _field_ from the Structure (LittleEndianStructure) class
+# allows it to be patched.
+#
+
+
+class EFI_LOADED_IMAGE_PROTOCOL(LittleEndianStructure):
+pass
+
+
+EFI_LOADED_IMAGE_PROTOCOL_fields_ = [
+('Revision', c_uint32),
+('ParentHandle', c_void_p),
+('SystemTable', c_void_p),
+('DeviceHandle', c_void_p),
+('FilePath', c_void_p),
+('Reserved', c_void_p),
+('LoadOptionsSize', c_uint32),
+('LoadOptions', c_void_p),
+('ImageBase', c_void_p),
+('ImageSize', c_uint64),
+('ImageCodeType', c_uint32),
+('ImageDataType', c_uint32),
+('Unload',c_void_p),
+]
+
+
+class EFI_GUID(LittleEndianStructure):
+_fields_ = [
+('Data1', c_uint32),
+('Data2', c_uint16),
+('Data3', c_uint16),
+('Data4', ARRAY(c_uint8, 8))
+]
+
+
+class EFI_SYSTEM_TABLE_POINTER(LittleEndianStructure):
+_fields_ = [
+('Signature', c_uint64),
+('EfiSystemTableBase',c_uint64),
+('Crc32', c_uint32)
+]
+
+
+class EFI_DEBUG_IMAGE_INFO_NORMAL(LittleEndianStructure):
+pass
+
+
+EFI_DEBUG_IMAGE_INFO_NORMAL_fields_ = [
+('ImageInfoType', c_uint32),
+('LoadedImageProtocolInstance', c_void_p),
+('ImageHandle', c_void_p)
+]
+
+
+class EFI_DEBUG_IMAGE_INFO(LittleEndianStructure):
+pass
+
+
+EFI_DEBUG_IMAGE_INFO_fields_ = [
+('NormalImage', c_void_p),
+]
+
+
+class EFI_DEBUG_IMAGE_INFO_TABLE_HEADER(LittleEndianStructure):
+pass
+
+
+EFI_DEBUG_IMAGE_INFO_TABLE_HEADER_fields_ = [
+('UpdateStatus', c_uint32),
+('TableSize', c_uint32),
+('EfiDebugImageInfoTable',c_void_p),
+]
+
+
+class EFI_TABLE_HEADER(LittleEndianStructure):
+_fields_ = [
+('Signature', c_uint64),
+('Revision', c_uint32),
+('HeaderSize',c_uint32),
+('CRC32', c_uint32),
+('Reserved', c_uint32),
+]
+
+
+class EFI_CONFIGURATION_TABLE(LittleEndianStructure):
+pass
+
+
+EFI_CONFIGURATION_TABLE_fields_ = [
+('VendorGuid',EFI_GUID),
+('VendorTable', c_void_p)
+]
+
+
+class EFI_SYSTEM_TABLE(LittleEndianStructure):
+pass
+
+
Re: [edk2-devel][PATCH 1/2] MdeModulePkg: StackOffset must be aligned to a 16-byte boundary in X64
Good day,
Thanks for the update!
On 21.03.22 13:43, Kuo, Ted wrote:
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3865
For X64, StackOffset must be aligned to a 16-byte boundary as well as
old stack and new stack. Otherwise, it'll get wrong data from Private
pointer after switching from old stack to new stack.
Cc: Michael D Kinney
Cc: Dandan Bi
Cc: Liming Gao
Cc: Debkumar De
Cc: Harry Han
Cc: Catharine West
Cc: Jian J Wang
Cc: Marvin Häuser
Signed-off-by: Ted Kuo
---
MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
index 3552feda8f..8a2c1ec779 100644
--- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
+++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
@@ -823,6 +823,19 @@ PeiCheckAndSwitchStack (
(VOID **)&TemporaryRamSupportPpi
);
if (!EFI_ERROR (Status)) {
+ //
+ // For X64, StackOffset must be aligned to a 16-byte boundary.
Otherwise, it'll get wrong data
+ // from Private pointer after switching to new stack.
+ //
+ if ((sizeof (UINTN) == sizeof (UINT64)) && ((StackOffset & 0x0F) == 8)) {
+if (StackOffsetPositive == TRUE) {
+ StackOffset -= 8;
+} else {
+ StackOffset += 8;
+}
+Private->StackOffset = StackOffset;
+ }
+
Hmm, the overall design (not your patch) looks very broken to me. So, if
the PPI exists, it's responsible for the migration of the stack, but it
is neither passed where to migrate the stack to, nor does it return
where it did migrate it to. This means the StackOffset calculated here
may be out-of-sync with what actually happens in the PPI, e.g., if the
PPI code is changed. There also is no detailed explanation for the
memory layout with FSP separate stack vs bootloader shared stack, so I
cannot really give detailed comments quickly. *Sigh*.
Anyhow, as for the patch, I do not understand a few things:
1) Maybe most important of all, what even is broken? Which address is
not 16-Byte-aligned to cause this issue in the first place?
2) Why do you align StackOffset? Like yes, if the old top of the stack
and the offset to the new top of the stack are both 16-Byte-aligned,
then the new top of the stack is 16-Byte-aligned too. However,
StackOffset is more of a by-product and TopOfNewStack remains holding
the old value. I just don't really understand the idea of this approach.
3) This only works when StackOffset is guaranteed to be 8-Byte-aligned
(is it?). As we are dealing with the *top* of the stack (which should be
4K-aligned even for memory protection!), what would be wrong with just
aligning down and up instead?
(Same question for the second patch to the FSP code)
4) The next patch performs a similar alignment operation (as mentioned
before). However, while this patch aligns the *top* of the stack, the
FSP patch aligns the *bottom* of the stack. This may or may not be
correct based on your premises. Can you maybe document why this is
correct, or even better, try to align the top of the stack in FSP as
well? (By transitivity, if you align the top correctly, the bottom
should be aligned correctly as well, as every nested call must preserve
the alignment invariant)
5) Why does this only happen when the PPI is found? Would that not risk
an unaligned stack if the PPI is not provided, the same way it is
unaligned now?
6) The comment explicitly mentions X64, but checks only for 64-bit
pointer size. So this should affect AArch64 and RISC-V-64 as well. Are
they guaranteed to function correctly after this patch (especially with
the PPI sync guarantees mentioned earlier)?
7) This only updates FSP code, similar to 5), but to QEMU and friends
continue to work?
Thanks!
Best regards,
Marvin
//
// Heap Offset
//
@@ -852,7 +865,10 @@ PeiCheckAndSwitchStack (
// Temporary Ram Support PPI is provided by platform, it will copy
// temporary memory to permanent memory and do stack switching.
// After invoking Temporary Ram Support PPI, the following code's
- // stack is in permanent memory.
+ // stack is in permanent memory. For X64, the bit3:0 of the new stack
+ // produced by TemporaryRamMigration must be aligned with the bit3:0 of
+ // the old stack. Otherwise, it'll break the original stack alignment
+ // after switching to new stack.
//
TemporaryRamSupportPpi->TemporaryRamMigration (
PeiServices,
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87798): https://edk2.groups.io/g/devel/message/87798
Mute This Topic: https://groups.io/mt/89926603/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-
[edk2-devel] [PATCH] MdeModulePkg/NvmExpressDxe: fix check for Cap.Css
Fix the check for NVMe command set being supported by the controller.
Was problematic with qemu (6.2.0, Debian 1:6.2+dfsg-3), which sets 0xC1
in that register, making the OVMF think the NVMe controller does not
support NVMe.
Uncovered by commit 9dd14fc91c174eae87fd122c7ac70073a363527f, which
changed the number of bits included in the Css register from 4 to 8.
Signed-off-by: Mara Sophie Grosch
---
MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
index d87212ffb2..b90c48731c 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
@@ -761,7 +761,7 @@ NvmeControllerInit (
return Status;
}
- if (Private->Cap.Css != 0x01) {
+ if ((Private->Cap.Css & BIT0) == 0) {
DEBUG ((DEBUG_INFO, "NvmeControllerInit: the controller doesn't support NVMe
command set\n"));
return EFI_UNSUPPORTED;
}
--
2.35.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87797): https://edk2.groups.io/g/devel/message/87797
Mute This Topic: https://groups.io/mt/89833412/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] UefiPayloadPkg: Fix architecture in the build instruction
Signed-off-by: Akihiko Odaki --- UefiPayloadPkg/BuildAndIntegrationInstructions.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UefiPayloadPkg/BuildAndIntegrationInstructions.txt b/UefiPayloadPkg/BuildAndIntegrationInstructions.txt index 61d31f1020..01b3d7f992 100644 --- a/UefiPayloadPkg/BuildAndIntegrationInstructions.txt +++ b/UefiPayloadPkg/BuildAndIntegrationInstructions.txt @@ -38,7 +38,7 @@ B. HOW TO BUILD > edksetup.bat For pure X64 build: - > build -a x64 -p UefiPayloadPkg\UefiPayloadPkg.dsc -b -t + > build -a X64 -p UefiPayloadPkg\UefiPayloadPkg.dsc -b -t -D BOOTLOADER= For X64 build with IA32 entry point: -- 2.35.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#87795): https://edk2.groups.io/g/devel/message/87795 Mute This Topic: https://groups.io/mt/89930528/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel][PATCH 2/2] IntelFsp2Pkg: Ensure new stack is aligned to old stack for X64
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3865
Ensure bit3:0 of NewStack is aligned with bit3:0 of OldStack for X64
before switching stack. Otherwise, RSP may not be aligned to a 16-byte
boundary after returning from SecTemporaryRamSupport.
Cc: Chasel Chiu
Cc: Nate DeSimone
Cc: Star Zeng
Cc: Ashraf Ali S
Signed-off-by: Ted Kuo
---
IntelFsp2Pkg/FspSecCore/SecMain.c | 8
1 file changed, 8 insertions(+)
diff --git a/IntelFsp2Pkg/FspSecCore/SecMain.c
b/IntelFsp2Pkg/FspSecCore/SecMain.c
index d376fb8361..f93e2d2ff7 100644
--- a/IntelFsp2Pkg/FspSecCore/SecMain.c
+++ b/IntelFsp2Pkg/FspSecCore/SecMain.c
@@ -258,6 +258,14 @@ SecTemporaryRamSupport (
NewStack = (VOID *)(UINTN)PermanentMemoryBase;
}
+ //
+ // Ensure bit3:0 of NewStack is aligned with bit3:0 of OldStack for X64
before switching stack.
+ // Otherwise, RSP may not be aligned to a 16-byte boundary after returning
from SecTemporaryRamSupport.
+ //
+ if ((sizeof (UINTN) == sizeof (UINT64)) && (((UINTN)NewStack & 0x0F) !=
((UINTN)OldStack & 0x0F))) {
+NewStack = (VOID *)((UINTN)NewStack - 8);
+ }
+
//
// Migrate Heap
//
--
2.16.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87793): https://edk2.groups.io/g/devel/message/87793
Mute This Topic: https://groups.io/mt/89926608/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel][PATCH 1/2] MdeModulePkg: StackOffset must be aligned to a 16-byte boundary in X64
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3865
For X64, StackOffset must be aligned to a 16-byte boundary as well as
old stack and new stack. Otherwise, it'll get wrong data from Private
pointer after switching from old stack to new stack.
Cc: Michael D Kinney
Cc: Dandan Bi
Cc: Liming Gao
Cc: Debkumar De
Cc: Harry Han
Cc: Catharine West
Cc: Jian J Wang
Cc: Marvin Häuser
Signed-off-by: Ted Kuo
---
MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
index 3552feda8f..8a2c1ec779 100644
--- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
+++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
@@ -823,6 +823,19 @@ PeiCheckAndSwitchStack (
(VOID **)&TemporaryRamSupportPpi
);
if (!EFI_ERROR (Status)) {
+ //
+ // For X64, StackOffset must be aligned to a 16-byte boundary.
Otherwise, it'll get wrong data
+ // from Private pointer after switching to new stack.
+ //
+ if ((sizeof (UINTN) == sizeof (UINT64)) && ((StackOffset & 0x0F) == 8)) {
+if (StackOffsetPositive == TRUE) {
+ StackOffset -= 8;
+} else {
+ StackOffset += 8;
+}
+Private->StackOffset = StackOffset;
+ }
+
//
// Heap Offset
//
@@ -852,7 +865,10 @@ PeiCheckAndSwitchStack (
// Temporary Ram Support PPI is provided by platform, it will copy
// temporary memory to permanent memory and do stack switching.
// After invoking Temporary Ram Support PPI, the following code's
- // stack is in permanent memory.
+ // stack is in permanent memory. For X64, the bit3:0 of the new stack
+ // produced by TemporaryRamMigration must be aligned with the bit3:0 of
+ // the old stack. Otherwise, it'll break the original stack alignment
+ // after switching to new stack.
//
TemporaryRamSupportPpi->TemporaryRamMigration (
PeiServices,
--
2.16.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87792): https://edk2.groups.io/g/devel/message/87792
Mute This Topic: https://groups.io/mt/89926603/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel][PATCH 0/2] Ensure RSP is aligned to a 16-byte boundary for PEI 64bit
The changes ensure the same stack alignment is kept before and after swithing stack in X64. Ted Kuo (2): MdeModulePkg: StackOffset must be aligned to a 16-byte boundary in X64 IntelFsp2Pkg: Ensure new stack is aligned to old stack for X64 IntelFsp2Pkg/FspSecCore/SecMain.c | 8 MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 18 +- 2 files changed, 25 insertions(+), 1 deletion(-) -- 2.16.2.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#87791): https://edk2.groups.io/g/devel/message/87791 Mute This Topic: https://groups.io/mt/89926595/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 1/2] MdeModulePkg/UefiBootManagerLib: Add Pcd to prioritize internal devices
From: Matt DeVillier
Add Pcd to allow configuring internal devices to be prioritized over
internally-attached USB devices, such as card readers, over SATA, NVMe
and eMMC drives.
Cc: Jian J Wang
Cc: Liming Gao
Signed-off-by: Matt DeVillier
Signed-off-by: Sean Rhodes
---
MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 10 --
.../Library/UefiBootManagerLib/UefiBootManagerLib.inf | 1 +
MdeModulePkg/MdeModulePkg.dec | 4
MdeModulePkg/MdeModulePkg.uni | 4
4 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 962892d38f..28c3cff471 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -2214,8 +2214,14 @@ BmEnumerateBootOptions (
//
// Skip the fixed block io then the removable block io
//
- if (BlkIo->Media->RemovableMedia == ((Removable == 0) ? FALSE : TRUE)) {
-continue;
+ if (FixedPcdGetBool (PcdPrioritizeInternal)) {
+if (BlkIo->Media->RemovableMedia == (Removable == 0)) {
+ continue;
+}
+ } else {
+if (BlkIo->Media->RemovableMedia == ((Removable == 0) ? FALSE : TRUE))
{
+ continue;
+}
}
Description = BmGetBootDescription (Handles[Index]);
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
index fe05d5f1cc..207420e334 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
+++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
@@ -119,3 +119,4 @@
gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile ##
CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdDriverHealthConfigureForm ##
SOMETIMES_CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxRepairCount ##
CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdPrioritizeInternal
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 40601c9583..8b8b1f7033 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1563,6 +1563,10 @@
# @Prompt SD/MMC Host Controller Operations Timeout (us).
gEfiMdeModulePkgTokenSpaceGuid.PcdSdMmcGenericTimeoutValue|100|UINT32|0x0031
+ ## Inditicates if internal devices will be prioritized over external devices.
+ # @prompt Prioritize internal devices.
+ gEfiMdeModulePkgTokenSpaceGuid.PcdPrioritizeInternal|FALSE|BOOLEAN|0x0032
+
[PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
## This PCD defines the Console output row. The default value is 25
according to UEFI spec.
# This PCD could be set to 0 then console output would be at max column and
max row.
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
index b070f15ff2..7b8606f091 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -1177,6 +1177,10 @@
#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSdMmcGenericTimeoutValue_HELP
#language en-US "Indicates the default timeout value for SD/MMC Host Controller
operations in microseconds."
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdPrioritizeInternal_PROMPT
#language en-US "Enable booting from internal devices before external devices."
+
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdPrioritizeInternal_HELP
#language en-US "Indicates the if internal devices will be prioritized in boot
order."
+
#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdCodRelocationDevPath_PROMPT
#language en-US "Capsule On Disk relocation device path."
#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdCodRelocationDevPath_HELP
#language en-US "Full device path of platform specific device to store
Capsule On Disk temp relocation file."
--
2.32.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87789): https://edk2.groups.io/g/devel/message/87789
Mute This Topic: https://groups.io/mt/89924426/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 2/2] UefiPayloadPkg: Hookup Prioritise Internal build option
Hook PRIORITISE_INTERNAL build option to PrioritiseInternal PCD. Cc: Guo Dong Cc: Ray Ni Cc: Maurice Ma Cc: Benjamin You Signed-off-by: Sean Rhodes --- UefiPayloadPkg/UefiPayloadPkg.dsc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc index 14a8d157a2..187e3aafc8 100644 --- a/UefiPayloadPkg/UefiPayloadPkg.dsc +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc @@ -35,6 +35,7 @@ DEFINE SMM_SUPPORT = FALSE DEFINE ABOVE_4G_MEMORY = TRUE DEFINE BOOT_MANAGER_ESCAPE = FALSE + DEFINE PRIORITIZE_INTERNAL = FALSE DEFINE SD_MMC_TIMEOUT = 100 # # SBL: UEFI payload for Slim Bootloader @@ -401,8 +402,9 @@ !if $(PERFORMANCE_MEASUREMENT_ENABLE) gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask | 0x1 !endif - gEfiMdeModulePkgTokenSpaceGuid.PcdSdMmcGenericTimeoutValue|$(SD_MMC_TIMEOUT) + gEfiMdeModulePkgTokenSpaceGuid.PcdSdMmcGenericTimeoutValue|$(SD_MMC_TIMEOUT) + gEfiMdeModulePkgTokenSpaceGuid.PcdPrioritizeInternal|$(PRIORITISE_INTERNAL) gUefiPayloadPkgTokenSpaceGuid.PcdDispatchModuleAbove4GMemory|$(ABOVE_4G_MEMORY) gUefiPayloadPkgTokenSpaceGuid.PcdBootManagerEscape|$(BOOT_MANAGER_ESCAPE) -- 2.32.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#87790): https://edk2.groups.io/g/devel/message/87790 Mute This Topic: https://groups.io/mt/89924427/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 15/18] UefiPayloadPkg/Library/PlatformBootManagerLib: Remove broken VGA detection
Hey Guo Can I chase when this patch will be merged? Many thanks Sean -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#87788): https://edk2.groups.io/g/devel/message/87788 Mute This Topic: https://groups.io/mt/89056153/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 2/2] UefiPayloadPkg: Use new filesystem drivers
From: Patrick Rudolph Cc: Guo Dong Cc: Ray Ni Cc: Maurice Ma Cc: Benjamin You Signed-off-by: Patrick Rudolph --- UefiPayloadPkg/UefiPayloadPkg.fdf | 15 +++ 1 file changed, 15 insertions(+) diff --git a/UefiPayloadPkg/UefiPayloadPkg.fdf b/UefiPayloadPkg/UefiPayloadPkg.fdf index c7b04978ad..d50a31fd06 100644 --- a/UefiPayloadPkg/UefiPayloadPkg.fdf +++ b/UefiPayloadPkg/UefiPayloadPkg.fdf @@ -206,6 +206,21 @@ INF MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf !endif INF FatPkg/EnhancedFatDxe/Fat.inf +# +# Filesystem drivers +# +!if $(ARCH) == IA32 +INF RuleOverride=BINARY USE = IA32 FSDrivers/exfat.inf +INF RuleOverride=BINARY USE = IA32 FSDrivers/ext2.inf +INF RuleOverride=BINARY USE = IA32 FSDrivers/ntfs.inf +INF RuleOverride=BINARY USE = IA32 FSDrivers/ext4.inf +!else +INF RuleOverride=BINARY USE = X64 FSDrivers/exfat.inf +INF RuleOverride=BINARY USE = X64 FSDrivers/ext2.inf +INF RuleOverride=BINARY USE = X64 FSDrivers/ntfs.inf +INF RuleOverride=BINARY USE = X64 FSDrivers/ext4.inf +!endif + # # SD/eMMC Support # -- 2.32.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#87786): https://edk2.groups.io/g/devel/message/87786 Mute This Topic: https://groups.io/mt/89924169/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] 回复: [edk2-devel] [PATCH 43/43] MdeModulePkg/BmBoot: skip secondary eMMC entries
Hi Liming This is how the entries appear for multiple MMC drives. Can you advise how you'd like me to adjust the patch? Thanks Sean -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#87785): https://edk2.groups.io/g/devel/message/87785 Mute This Topic: https://groups.io/mt/88963038/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/2] SecurityPkg: Add RNG support
Hi Jiewen/Jian Would you be able to review please? Thanks Sean -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#87784): https://edk2.groups.io/g/devel/message/87784 Mute This Topic: https://groups.io/mt/89353220/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] UefiPayloadPkg: Fix PciHostBridgeLib
Hi Can I follow up on this patch being merged? Thanks Sean -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#87783): https://edk2.groups.io/g/devel/message/87783 Mute This Topic: https://groups.io/mt/88968466/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 4/5] UefiPayloadPkg: Add SmmStoreLib
From: Patrick Rudolph Implement all of the FVB protocol functions on top of the SmmStore as a library. The library consumes the introduced gEfiSmmStoreInfoHobGuid. The SMI handler uses a fixed communication buffer in reserved DRAM. To initiate a transaction you must write to the I/O APM_CNT port. Tests on Intel(R) Xeon(R) E-2288G CPU @ 3.70G showed that the SMI isn't triggered with a probability of 1:40 of all cases when called in a tight loop. The CPU continues running and the SMI is triggeres asynchronously a few clock cycles later. coreboot only handels synchronous APM request and does nothing on asynchronous APM triggers. As there's no livesign from SMM it's impossible to tell if the handler has run. Just wait a bit and try again to trigger a synchronous SMI. Tests confirmed that out of 5 million tries the SMI is now always handled. When a synchronous SMI happens with the correct write to the APM_CNT port, the ebx register is checked first that it doesn't point to SMRAM. If it doesn't it's used to read in the arguments that define a SmmStore transaction. The SMI handler will only operate on a predefined and memory mapped region in the boot media. Cc: Guo Dong Cc: Ray Ni Cc: Maurice Ma Cc: Benjamin You Cc: Sean Rhodes Signed-off-by: Patrick Rudolph --- UefiPayloadPkg/Include/Library/SmmStoreLib.h | 120 + UefiPayloadPkg/Library/SmmStoreLib/SmmStore.c | 473 ++ UefiPayloadPkg/Library/SmmStoreLib/SmmStore.h | 81 +++ .../Library/SmmStoreLib/SmmStoreLib.inf | 41 ++ .../Library/SmmStoreLib/X64/SmmStore.nasm | 48 ++ UefiPayloadPkg/UefiPayloadPkg.dsc | 1 + 6 files changed, 764 insertions(+) create mode 100644 UefiPayloadPkg/Include/Library/SmmStoreLib.h create mode 100644 UefiPayloadPkg/Library/SmmStoreLib/SmmStore.c create mode 100644 UefiPayloadPkg/Library/SmmStoreLib/SmmStore.h create mode 100644 UefiPayloadPkg/Library/SmmStoreLib/SmmStoreLib.inf create mode 100644 UefiPayloadPkg/Library/SmmStoreLib/X64/SmmStore.nasm diff --git a/UefiPayloadPkg/Include/Library/SmmStoreLib.h b/UefiPayloadPkg/Include/Library/SmmStoreLib.h new file mode 100644 index 00..6ade375b1d --- /dev/null +++ b/UefiPayloadPkg/Include/Library/SmmStoreLib.h @@ -0,0 +1,120 @@ +/** @file SmmStoreLib.h + + Copyright (c) 2022, 9elements GmbH + + SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#ifndef SMM_STORE_LIB_H_ +#define SMM_STORE_LIB_H_ + +#include +#include +#include + +#define SMMSTORE_COMBUF_SIZE 16 + +/** + Get the SmmStore block size + + @param BlockSizeThe pointer to store the block size in. + +**/ +EFI_STATUS +SmmStoreLibGetBlockSize ( + OUT UINTN *BlockSize + ); + +/** + Get the SmmStore number of blocks + + @param NumBlocksThe pointer to store the number of blocks in. + +**/ +EFI_STATUS +SmmStoreLibGetNumBlocks ( + OUT UINTN *NumBlocks + ); + +/** + Get the SmmStore MMIO address + + @param MmioAddressThe pointer to store the address in. + +**/ +EFI_STATUS +SmmStoreLibGetMmioAddress ( + OUT EFI_PHYSICAL_ADDRESS *MmioAddress + ); + +/** + Read from SmmStore + + @param[in] Lba The starting logical block index to read from. + @param[in] Offset Offset into the block at which to begin reading. + @param[in] NumBytes On input, indicates the requested read size. On + output, indicates the actual number of bytes read + @param[in] Buffer Pointer to the buffer to read into. + +**/ +EFI_STATUS +SmmStoreLibRead ( + INEFI_LBA Lba, + INUINTNOffset, + INUINTN*NumBytes, + INUINT8*Buffer + ); + +/** + Write to SmmStore + + @param[in] Lba The starting logical block index to write to. + @param[in] Offset Offset into the block at which to begin writing. + @param[in] NumBytes On input, indicates the requested write size. On + output, indicates the actual number of bytes written + @param[in] Buffer Pointer to the data to write. + +**/ +EFI_STATUS +SmmStoreLibWrite ( + INEFI_LBA Lba, + INUINTNOffset, + INUINTN*NumBytes, + INUINT8*Buffer + ); + +/** + Erase a block using the SmmStore + + @param LbaThe logical block index to erase. + +**/ +EFI_STATUS +SmmStoreLibEraseBlock ( + IN EFI_LBA Lba + ); + +/** + Initializes SmmStore support + + @retval EFI_WRITE_PROTECTED The SmmStore is not present. + @retval EFI_UNSUPPORTED The SmmStoreInfo HOB wasn't found. + @retval EFI_SUCCESS The SmmStore is supported. + +**/ +EFI_STATUS +SmmStoreLibInitialize ( + VOID + ); + +/** + Denitializes SmmStore support +**/ +VOID +EFIAPI +SmmStoreLibDeinitialize ( + VOID + ); + +#endif /* SMM_STORE_LIB_H_ */ diff --git a/UefiPayloadPkg/Library/SmmStoreLib/SmmStore.c b/UefiPayloadPkg/Library/SmmStoreLib/SmmStore.c new file mode 100644 index 00..25426a4d1a --- /dev/null +++ b/UefiPayloadPkg/Library/SmmStoreLib/SmmStore.c @@ -0,0 +1,473 @@ +/
[edk2-devel] [PATCH 1/5] UefiPayloadPkg/UefiPayloadPkg.ci.yaml: Remove duplicated entry
From: Patrick Rudolph Remove a duplicated entry to fix a CI error. Cc: Guo Dong Cc: Ray Ni Cc: Maurice Ma Cc: Benjamin You Cc: Sean Rhodes Signed-off-by: Patrick Rudolph --- UefiPayloadPkg/UefiPayloadPkg.ci.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/UefiPayloadPkg/UefiPayloadPkg.ci.yaml b/UefiPayloadPkg/UefiPayloadPkg.ci.yaml index 93f4d3603d..909379eaac 100644 --- a/UefiPayloadPkg/UefiPayloadPkg.ci.yaml +++ b/UefiPayloadPkg/UefiPayloadPkg.ci.yaml @@ -20,7 +20,6 @@ "IgnoreFiles": [ "Include/Coreboot.h", "Library/CbParseLib/CbParseLib.c", -"Library/CbParseLib/CbParseLib.c", "PayloadLoaderPeim/ElfLib/ElfCommon.h", "PayloadLoaderPeim/ElfLib/Elf32.h", "PayloadLoaderPeim/ElfLib/Elf64.h" -- 2.32.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#87780): https://edk2.groups.io/g/devel/message/87780 Mute This Topic: https://groups.io/mt/89923797/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 5/5] UefiPayloadPkg: Add support for Firmware Volume Block Protocol
This adds support for FVB in order to support a platform independent
and non-volatile variable store on UefiPayloadPkg. The variable store
makes use the SmmStoreLib to provide an unauthenticed variable store.
Since commit bc744f5893fc4d53275ed26dd8d968011c6a09c1 coreboot supports
the SMMSTORE v2 feature. It implements a SMI handler that is able to
write, read and erase pages in the boot media (SPI flash).
The communication is done using a fixed communication buffer that is
allocated in CBMEM. The existence of this optional feature is advertised
by a coreboot table.
When the SMMSTORE feature is not available the variable emulation is used
by setting PcdEmuVariableNvModeEnable to TRUE.
The DXE component provides runtime services and takes care of virtual to
physical mapping the communication buffers between SMM and OS.
The contents of the variable store can be accessed and modified by any
priviledged application. As authentication is done by runtime services
only the store shouldn't be used to store authenticated variables.
Tested on Linux and Windows 10 on real hardware.
Currently this cannot be tested on coreboot and qemu as it doesn't support
the SMMSTORE on qemu.
Cc: Guo Dong
Cc: Ray Ni
Cc: Maurice Ma
Cc: Benjamin You
Signed-off-by: Patrick Rudolph
Signed-off-by: Sean Rhodes
---
.../SmmStoreFvb/SmmStoreFvbRuntime.c | 282 ++
.../SmmStoreFvb/SmmStoreFvbRuntime.h | 111 +++
.../SmmStoreFvb/SmmStoreFvbRuntimeDxe.c | 849 ++
.../SmmStoreFvb/SmmStoreFvbRuntimeDxe.inf | 66 ++
UefiPayloadPkg/UefiPayloadPkg.dsc | 24 +-
UefiPayloadPkg/UefiPayloadPkg.fdf | 4 +
6 files changed, 1330 insertions(+), 6 deletions(-)
create mode 100644 UefiPayloadPkg/SmmStoreFvb/SmmStoreFvbRuntime.c
create mode 100644 UefiPayloadPkg/SmmStoreFvb/SmmStoreFvbRuntime.h
create mode 100644 UefiPayloadPkg/SmmStoreFvb/SmmStoreFvbRuntimeDxe.c
create mode 100644 UefiPayloadPkg/SmmStoreFvb/SmmStoreFvbRuntimeDxe.inf
diff --git a/UefiPayloadPkg/SmmStoreFvb/SmmStoreFvbRuntime.c
b/UefiPayloadPkg/SmmStoreFvb/SmmStoreFvbRuntime.c
new file mode 100644
index 00..08d5c408fb
--- /dev/null
+++ b/UefiPayloadPkg/SmmStoreFvb/SmmStoreFvbRuntime.c
@@ -0,0 +1,282 @@
+/** @file SmmStoreFvbRuntime.c
+
+ Copyright (c) 2022, 9elements GmbH
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "SmmStoreFvbRuntime.h"
+
+STATIC EFI_EVENT mSmmStoreVirtualAddrChangeEvent;
+
+//
+// Global variable declarations
+//
+SMMSTORE_INSTANCE *mSmmStoreInstance;
+
+SMMSTORE_INSTANCE mSmmStoreInstanceTemplate = {
+ SMMSTORE_SIGNATURE, // Signature
+ NULL, // Handle ... NEED TO BE FILLED
+ {
+FvbGetAttributes, // GetAttributes
+FvbSetAttributes, // SetAttributes
+FvbGetPhysicalAddress, // GetPhysicalAddress
+FvbGetBlockSize, // GetBlockSize
+FvbRead, // Read
+FvbWrite, // Write
+FvbEraseBlocks,// EraseBlocks
+NULL, // ParentHandle
+ }, // FvbProtoccol
+ 0, // BlockSize ... NEED TO BE FILLED
+ 0, // LastBlock ... NEED TO BE FILLED
+ 0, // MmioAddress ... NEED TO BE FILLED
+ {
+{
+ {
+HARDWARE_DEVICE_PATH,
+HW_MEMMAP_DP,
+{
+ (UINT8)(sizeof (MEMMAP_DEVICE_PATH)),
+ (UINT8)(sizeof (MEMMAP_DEVICE_PATH) >> 8)
+}
+ },
+ EfiMemoryMappedIO,
+ (EFI_PHYSICAL_ADDRESS)0, // NEED TO BE FILLED
+ (EFI_PHYSICAL_ADDRESS)0, // NEED TO BE FILLED
+},
+{
+ END_DEVICE_PATH_TYPE,
+ END_ENTIRE_DEVICE_PATH_SUBTYPE,
+ {
+END_DEVICE_PATH_LENGTH,
+0
+ }
+}
+ } // DevicePath
+};
+
+/**
+ Initialize the SmmStore instance.
+
+
+ @param[in] FvBase The physical MMIO base address of the FV
containing
+ the variable store.
+
+ @param[in] NumberofBlocks Number of blocks within the FV.
+ @param[in] BlockSize The size in bytes of one block within the FV.
+ @param[in, out] Instance The SmmStore instace to initialize
+
+**/
+STATIC
+EFI_STATUS
+SmmStoreInitInstance (
+ IN EFI_PHYSICAL_ADDRESS FvBase,
+ IN UINTN NumberofBlocks,
+ IN UINTN BlockSize,
+ IN OUT SMMSTORE_INSTANCE *Instance
+ )
+{
+ EFI_STATUS Status;
+ FV_MEMMAP_DEVICE_PATH *FvDevicePath;
+
+ ASSERT (Instance != NULL);
+
+ Instance->BlockSize = BlockSize;
+ Instance->LastBlock = NumberofBlocks - 1;
+ Instance->MmioAddress = FvBase;
+
+ FvDevicePath= &Instance->DevicePath;
+ FvDevicePath->MemMapDevPath.StartingAddress = FvBase;
+ FvDevicePath->MemMapDevPath.EndingAddress = FvBase + BlockSize *
NumberofBlocks - 1;
+
+ Status = FvbInitialize (Instance);
+ if (EFI_ERROR (Status)) {
+return Status;
+ }
+
+ Status = gBS->InstallMul
[edk2-devel] [PATCH 3/5] UefiPayloadPkg: Add SmmStoreInfoGuid
From: Patrick Rudolph
Add a new InfoHob that contains the SmmStore information passed from
coreboot tables when the SMMSTOREV2 feature is enabled.
This will be used to implement the FVB in top of the MM installed by
coreboot.
Cc: Guo Dong
Cc: Ray Ni
Cc: Maurice Ma
Cc: Benjamin You
Cc: Sean Rhodes
Signed-off-by: Patrick Rudolph
---
.../Include/Guid/SmmStoreInfoGuid.h | 27
.../Include/Library/SmmStoreParseLib.h| 29 +
.../Library/CbParseLib/CbParseLib.c | 43 +++
.../Library/SblParseLib/SblParseLib.c | 18
.../UefiPayloadEntry/UefiPayloadEntry.c | 13 ++
.../UefiPayloadEntry/UefiPayloadEntry.h | 2 +
.../UefiPayloadEntry/UefiPayloadEntry.inf | 1 +
UefiPayloadPkg/UefiPayloadPkg.dec | 2 +
8 files changed, 135 insertions(+)
create mode 100644 UefiPayloadPkg/Include/Guid/SmmStoreInfoGuid.h
create mode 100644 UefiPayloadPkg/Include/Library/SmmStoreParseLib.h
diff --git a/UefiPayloadPkg/Include/Guid/SmmStoreInfoGuid.h
b/UefiPayloadPkg/Include/Guid/SmmStoreInfoGuid.h
new file mode 100644
index 00..d5dcb8218f
--- /dev/null
+++ b/UefiPayloadPkg/Include/Guid/SmmStoreInfoGuid.h
@@ -0,0 +1,27 @@
+/** @file
+ This file defines the hob structure for coreboot's SmmStore.
+
+ Copyright (c) 2022, 9elements GmbH
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef SMMSTORE_GUID_H_
+#define SMMSTORE_GUID_H_
+
+///
+/// System Table Information GUID
+///
+extern EFI_GUID gEfiSmmStoreInfoHobGuid;
+
+typedef struct {
+ UINT64ComBuffer;
+ UINT32ComBufferSize;
+ UINT32NumBlocks;
+ UINT32BlockSize;
+ UINT64MmioAddress;
+ UINT8 ApmCmd;
+ UINT8 Reserved0[3];
+} SMMSTORE_INFO;
+
+#endif // SMMSTORE_GUID_H_
diff --git a/UefiPayloadPkg/Include/Library/SmmStoreParseLib.h
b/UefiPayloadPkg/Include/Library/SmmStoreParseLib.h
new file mode 100644
index 00..0b5b8f8aba
--- /dev/null
+++ b/UefiPayloadPkg/Include/Library/SmmStoreParseLib.h
@@ -0,0 +1,29 @@
+/** @file
+ This library will parse the coreboot table in memory and extract those
required
+ information.
+
+ Copyright (c) 2021, Star Labs Systems. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef SMM_STORE_PARSE_LIB_H_
+#define SMM_STORE_PARSE_LIB_H_
+
+#include
+
+/**
+ Find the SmmStore HOB.
+
+ @param SmmStoreInfo Pointer to the SMMSTORE_INFO structure
+
+ @retval RETURN_SUCCESS Successfully find the Smm store buffer
information.
+ @retval RETURN_NOT_FOUND Failed to find the Smm store buffer information .
+**/
+RETURN_STATUS
+EFIAPI
+ParseSmmStoreInfo (
+ OUT SMMSTORE_INFO *SmmStoreInfo
+ );
+
+#endif // SMM_STORE_PARSE_LIB_H_
diff --git a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
index 8a353f77f6..5c7efec3ec 100644
--- a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
+++ b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
@@ -604,3 +605,45 @@ ParseMiscInfo (
{
return RETURN_SUCCESS;
}
+
+/**
+ Find the SmmStore HOB.
+
+ @param SmmStoreInfo Pointer to the SMMSTORE_INFO structure
+
+ @retval RETURN_SUCCESS Successfully find the Smm store buffer
information.
+ @retval RETURN_NOT_FOUND Failed to find the Smm store buffer information .
+**/
+RETURN_STATUS
+EFIAPI
+ParseSmmStoreInfo (
+ OUT SMMSTORE_INFO *SmmStoreInfo
+ )
+{
+ struct cb_smmstorev2 *CbSSRec;
+
+ if (SmmStoreInfo == NULL) {
+return RETURN_INVALID_PARAMETER;
+ }
+
+ CbSSRec = FindCbTag (CB_TAG_SMMSTOREV2);
+ if (CbSSRec == NULL) {
+return RETURN_NOT_FOUND;
+ }
+
+ DEBUG ((DEBUG_INFO, "Found Smm Store information\n"));
+ DEBUG ((DEBUG_INFO, "block size: 0x%x\n", CbSSRec->block_size));
+ DEBUG ((DEBUG_INFO, "number of blocks: 0x%x\n", CbSSRec->num_blocks));
+ DEBUG ((DEBUG_INFO, "communication buffer: 0x%x\n", CbSSRec->com_buffer));
+ DEBUG ((DEBUG_INFO, "communication buffer size: 0x%x\n",
CbSSRec->com_buffer_size));
+ DEBUG ((DEBUG_INFO, "MMIO address of store: 0x%x\n", CbSSRec->mmap_addr));
+
+ SmmStoreInfo->ComBuffer = CbSSRec->com_buffer;
+ SmmStoreInfo->ComBufferSize = CbSSRec->com_buffer_size;
+ SmmStoreInfo->BlockSize = CbSSRec->block_size;
+ SmmStoreInfo->NumBlocks = CbSSRec->num_blocks;
+ SmmStoreInfo->MmioAddress = CbSSRec->mmap_addr;
+ SmmStoreInfo->ApmCmd= CbSSRec->apm_cmd;
+
+ return RETURN_SUCCESS;
+}
diff --git a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
b/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
index d88238bfdc..6875fa9934 100644
--- a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
+++ b/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
@@ -289,3 +290,20 @@ ParseMiscInfo (
return Status;
}
+
+/**
+ Find the SmmStore HOB.
+
+ @p
[edk2-devel] [PATCH 2/5] UefiPayloadPkg/Include/Coreboot: Add headers for SMMSTOREv2 table
From: Patrick Rudolph
Since commit bc744f5893fc4d53275ed26dd8d968011c6a09c1 coreboot supports
the SMMSTORE v2 feature. It implements a SMI handler that is able to
write, read and erase pages in the boot media (SPI flash).
The existence of this optional feature is advertised by a coreboot table.
Add the tag and headers to parse the table.
Cc: Guo Dong
Cc: Ray Ni
Cc: Maurice Ma
Cc: Benjamin You
Cc: Sean Rhodes
Signed-off-by: Patrick Rudolph
---
UefiPayloadPkg/Include/Coreboot.h | 13 +
1 file changed, 13 insertions(+)
diff --git a/UefiPayloadPkg/Include/Coreboot.h
b/UefiPayloadPkg/Include/Coreboot.h
index a3e1109fe8..617e5e9bc0 100644
--- a/UefiPayloadPkg/Include/Coreboot.h
+++ b/UefiPayloadPkg/Include/Coreboot.h
@@ -236,6 +236,19 @@ struct cb_cbmem_tab {
UINT64cbmem_tab;
};
+#define CB_TAG_SMMSTOREV2 0x0039
+struct cb_smmstorev2 {
+ UINT32tag;
+ UINT32size;
+ UINT32num_blocks; /* Number of writeable blocks in Smm */
+ UINT32block_size; /* Size of a block in byte. Default: 64 KiB */
+ UINT32mmap_addr; /* MMIO address of the store for read only access
*/
+ UINT32com_buffer; /* Physical address of the communication buffer */
+ UINT32com_buffer_size; /* Size of the communication buffer in byte */
+ UINT8 apm_cmd; /* The command byte to write to the APM I/O port
*/
+ UINT8 unused[3]; /* Set to zero */
+};
+
/* Helpful macros */
#define MEM_RANGE_COUNT(_rec) \
--
2.32.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87779): https://edk2.groups.io/g/devel/message/87779
Mute This Topic: https://groups.io/mt/89923796/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiPayloadPkg: Fix build error
The patch looks good to me, and it has been confirmed by several others from the coreboot community. However, I don't have the capability to merge. Thanks Sean -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#8): https://edk2.groups.io/g/devel/message/8 Mute This Topic: https://groups.io/mt/89710183/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
