Re: [edk2-devel] [Patch 1/1] Maintainers.txt: Update based on active community members
On 2023-10-28 12:23:30, Michael D Kinney wrote: > @@ -497,7 +463,6 @@ F: OvmfPkg/ > W: http://www.tianocore.org/ovmf/ > M: Ard Biesheuvel [ardbiesheuvel] > M: Jiewen Yao [jyao1] > -R: Jordan Justen [jljusten] > R: Gerd Hoffmann [kraxel] > S: Maintained Acked-by: Jordan Justen -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110373): https://edk2.groups.io/g/devel/message/110373 Mute This Topic: https://groups.io/mt/102245264/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2 2/2] Maintainers.txt: Change Jordan Justen to a reviewer for OvmfPkg
Cc: Laszlo Ersek Cc: Ard Biesheuvel Signed-off-by: Jordan Justen --- Maintainers.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Maintainers.txt b/Maintainers.txt index 96e792ab66..e384971238 100644 --- a/Maintainers.txt +++ b/Maintainers.txt @@ -423,9 +423,9 @@ R: Siyuan Fu OvmfPkg F: OvmfPkg/ W: http://www.tianocore.org/ovmf/ -M: Jordan Justen M: Laszlo Ersek M: Ard Biesheuvel +R: Jordan Justen S: Maintained OvmfPkg: bhyve-related modules -- 2.30.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#71264): https://edk2.groups.io/g/devel/message/71264 Mute This Topic: https://groups.io/mt/80389015/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2 1/2] Maintainers.txt: Remove Jordan Justen from EmulatorPkg
Cc: Andrew Fish Cc: Ray Ni Signed-off-by: Jordan Justen Reviewed-by: Andrew Fish --- Maintainers.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/Maintainers.txt b/Maintainers.txt index 2cd356551e..96e792ab66 100644 --- a/Maintainers.txt +++ b/Maintainers.txt @@ -197,7 +197,6 @@ M: Ard Biesheuvel EmulatorPkg F: EmulatorPkg/ W: https://github.com/tianocore/tianocore.github.io/wiki/EmulatorPkg -M: Jordan Justen M: Andrew Fish M: Ray Ni S: Maintained -- 2.30.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#71263): https://edk2.groups.io/g/devel/message/71263 Mute This Topic: https://groups.io/mt/80389012/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] Maintainers.txt: Remove Jordan Justen from OvmfPkg
Cc: Laszlo Ersek Cc: Ard Biesheuvel Signed-off-by: Jordan Justen --- Maintainers.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/Maintainers.txt b/Maintainers.txt index 96e792ab66..e986a3680c 100644 --- a/Maintainers.txt +++ b/Maintainers.txt @@ -423,7 +423,6 @@ R: Siyuan Fu OvmfPkg F: OvmfPkg/ W: http://www.tianocore.org/ovmf/ -M: Jordan Justen M: Laszlo Ersek M: Ard Biesheuvel S: Maintained -- 2.30.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#71077): https://edk2.groups.io/g/devel/message/71077 Mute This Topic: https://groups.io/mt/80325381/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] Maintainers.txt: Remove Jordan Justen from EmulatorPkg
Cc: Andrew Fish Cc: Ray Ni Signed-off-by: Jordan Justen --- Maintainers.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/Maintainers.txt b/Maintainers.txt index 2cd356551e..96e792ab66 100644 --- a/Maintainers.txt +++ b/Maintainers.txt @@ -197,7 +197,6 @@ M: Ard Biesheuvel EmulatorPkg F: EmulatorPkg/ W: https://github.com/tianocore/tianocore.github.io/wiki/EmulatorPkg -M: Jordan Justen M: Andrew Fish M: Ray Ni S: Maintained -- 2.30.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#71076): https://edk2.groups.io/g/devel/message/71076 Mute This Topic: https://groups.io/mt/80325375/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFC PATCH 4/4] BaseTools/PatchCheck.py: Check the committer email address
On 2020-01-02 07:25:53, Philippe Mathieu-Daude wrote:
> To avoid patches committed with incorrect email address,
> use the EmailAddressCheck class on the committer email too.
>
> Cc: Liming Gao
> Cc: Jordan Justen
> Signed-off-by: Philippe Mathieu-Daude
> ---
> RFC because I haven't checked --pretty="%cn <%ce>" works on Windows shell.
>
> BaseTools/Scripts/PatchCheck.py | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
> index f0e661bfd6e3..3baeb3de7ba2 100755
> --- a/BaseTools/Scripts/PatchCheck.py
> +++ b/BaseTools/Scripts/PatchCheck.py
> @@ -560,6 +560,8 @@ class CheckGitCommits:
> else:
> blank_line = True
> print('Checking git commit:', commit)
> +email = self.read_committer_email_address_from_git(commit)
> +self.ok &= EmailAddressCheck(email, 'Committer').ok
> patch = self.read_patch_from_git(commit)
> self.ok &= CheckOnePatch(commit, patch).ok
> if not commits:
> @@ -578,6 +580,10 @@ class CheckGitCommits:
> # Run git to get the commit patch
> return self.run_git('show', '--pretty=email', '--no-textconv',
> commit)
>
> +def read_committer_email_address_from_git(self, commit):
> +# Run git to get the committer email
> +return self.run_git('show', '--pretty="%cn <%ce>"', '--no-patch',
> commit)
I think '--pretty=%cn <%ce>' ought to work without double-quotes
because the argument is separately sent via the subprocess.Popen call.
I'm not certain it will work, but it ought to. :)
-Jordan
> +
> def run_git(self, *args):
> cmd = [ 'git' ]
> cmd += args
> --
> 2.21.0
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52845): https://edk2.groups.io/g/devel/message/52845
Mute This Topic: https://groups.io/mt/69381399/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 1/1] BaseTools/PatchCheck.py: Ignore CR and LF characters in subject length
On 2019-12-19 06:12:56, Philippe Mathieu-Daudé wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=113
>
> Strip the trailing characters before checking the subject line is
> less than 72 characters.
>
> Fixes: e61406708c83f
> Cc: Liming Gao
> Cc: Jordan Justen
> Signed-off-by: Philippe Mathieu-Daude
> ---
> Cc: Yonghong Zhu
> Cc: Zhichao Gao
> ---
> BaseTools/Scripts/PatchCheck.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
> index 2a4e6f603e79..b72e71963ea8 100755
> --- a/BaseTools/Scripts/PatchCheck.py
> +++ b/BaseTools/Scripts/PatchCheck.py
> @@ -196,7 +196,7 @@ class CommitMessageCheck:
> self.error('Empty commit message!')
> return
>
> -if count >= 1 and len(lines[0]) >= 72:
> +if count >= 1 and len(lines[0].strip()) >= 72:
I think we should use rstrip rather than strip. I tried to test this
with rstrip instead, by updating the commit message to have several
whitespace characters at the beginning.
Unfortunately, I think subject_prefix_re is eating more spaces at the
start of the line than it ought to.
The leading whitespace issue is probably a problem for another day, so
if you change it to rstrip, then:
Reviewed-by: Jordan Justen
> self.error('First line of commit message (subject line) ' +
> 'is too long.')
>
> --
> 2.21.0
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52646): https://edk2.groups.io/g/devel/message/52646
Mute This Topic: https://groups.io/mt/68831576/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2] OvmfPkg: Make SOURCE_DEBUG_ENABLE actually need to be set to TRUE
Reviewed-by: Jordan Justen On 2019-10-21 13:13:20, Laszlo Ersek wrote: > From: Peter Jones > > Currently some tests check the value of SOURCE_DEBUG_ENABLE, and some > tests check if it's defined or not. Additionally, in UefiPayloadPkg as > well as some other trees, we define it as FALSE in the .dsc file. > > This patch changes all of the Ovmf platforms to explicitly define it as > FALSE by default, and changes all of the checks to test if the value is > TRUE. > > Signed-off-by: Peter Jones > Message-Id: <20190920184507.909884-1-pjo...@redhat.com> > [ler...@redhat.com: drop Contributed-under line, per TianoCore BZ#1373] > [ler...@redhat.com: replace "!= TRUE" with more idiomatic "== FALSE"] > Cc: Andrew Fish > Cc: Anthony Perard > Cc: Ard Biesheuvel > Cc: Jordan Justen > Cc: Julien Grall > Cc: Leif Lindholm > Cc: Michael Kinney > Cc: Peter Jones > Signed-off-by: Laszlo Ersek > --- > > Notes: > v2: > > - repo: https://github.com/lersek/edk2.git > branch: src_dbg_true_v2 > > - repost the patch in Peter's stead, with the updates requested at > > <9c6d70b5-fcd6-373f-973f-044d1338e47b@redhat.com">http://mid.mail-archive.com/9c6d70b5-fcd6-373f-973f-044d1338e47b@redhat.com> > > - per discussion with the other stewards, it's OK to explicitly resubmit > the patch (noting the original authorship) with the Contributed-under > line removed > > OvmfPkg/OvmfPkgIa32.dsc| 17 + > OvmfPkg/OvmfPkgIa32X64.dsc | 19 ++- > OvmfPkg/OvmfPkgX64.dsc | 19 ++- > OvmfPkg/OvmfXen.dsc| 17 + > OvmfPkg/OvmfPkgIa32.fdf| 2 +- > OvmfPkg/OvmfPkgIa32X64.fdf | 2 +- > OvmfPkg/OvmfPkgX64.fdf | 2 +- > OvmfPkg/OvmfXen.fdf| 2 +- > 8 files changed, 42 insertions(+), 38 deletions(-) > > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > index 66e944436a69..4301e7821902 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -30,6 +30,7 @@ [Defines] ># >DEFINE SECURE_BOOT_ENABLE = FALSE >DEFINE SMM_REQUIRE = FALSE > + DEFINE SOURCE_DEBUG_ENABLE = FALSE >DEFINE TPM2_ENABLE = FALSE >DEFINE TPM2_CONFIG_ENABLE = FALSE > > @@ -157,7 +158,7 @@ [LibraryClasses] > > CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf > > FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf > > -!ifdef $(SOURCE_DEBUG_ENABLE) > +!if $(SOURCE_DEBUG_ENABLE) == TRUE > > PeCoffExtraActionLib|SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/PeCoffExtraActionLibDebug.inf > > DebugCommunicationLib|SourceLevelDebugPkg/Library/DebugCommunicationLibSerialPort/DebugCommunicationLibSerialPort.inf > !else > @@ -225,7 +226,7 @@ [LibraryClasses.common.SEC] > !endif > > ReportStatusCodeLib|MdeModulePkg/Library/PeiReportStatusCodeLib/PeiReportStatusCodeLib.inf > > ExtractGuidedSectionLib|MdePkg/Library/BaseExtractGuidedSectionLib/BaseExtractGuidedSectionLib.inf > -!ifdef $(SOURCE_DEBUG_ENABLE) > +!if $(SOURCE_DEBUG_ENABLE) == TRUE > > DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgentLib.inf > !endif >HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf > @@ -267,7 +268,7 @@ [LibraryClasses.common.PEIM] >PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf > > ResourcePublicationLib|MdePkg/Library/PeiResourcePublicationLib/PeiResourcePublicationLib.inf > > ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiExtractGuidedSectionLib.inf > -!ifdef $(SOURCE_DEBUG_ENABLE) > +!if $(SOURCE_DEBUG_ENABLE) == TRUE > > DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgentLib.inf > !endif > > CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf > @@ -292,7 +293,7 @@ [LibraryClasses.common.DXE_CORE] >DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf > !endif > > ExtractGuidedSectionLib|MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf > -!ifdef $(SOURCE_DEBUG_ENABLE) > +!if $(SOURCE_DEBUG_ENABLE) == TRUE >DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf > !endif > > CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf > @@ -351,7 +352,7 @@ [LibraryClasses.common.DXE_DRIVER] > !else >LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf > !endif > -!ifdef $(SOURCE_DEBUG_ENABLE) > +!if $(
Re: [edk2-devel] [Patch 05/12] MdePkg BaseIoLibIntrinsic: Remove __inline__ attribute for IO functions
On 2019-10-10 09:32:19, Laszlo Ersek wrote: > Hi Liming, Andrew, > > On 10/10/19 14:32, Liming Gao wrote: > > Laszlo: > > > >> -Original Message- > >> From: Laszlo Ersek > >> Sent: Wednesday, October 9, 2019 4:22 AM > >> To: Gao, Liming ; devel@edk2.groups.io; > >> af...@apple.com > >> Subject: Re: [edk2-devel] [Patch 05/12] MdePkg BaseIoLibIntrinsic: Remove > >> __inline__ attribute for IO functions > >> > >> On 10/08/19 16:47, Gao, Liming wrote: > >> > >>> [Liming] I verify GCC5 tool chain. I will verify GCC48/GCC49 and > >>> XCODE5. > >>> > >>> I don’t know the specific reason about __inline__. If there is no > >>> impact on > >>> > >>> other GCC tool chain, I prefer to remove them. > >> > >>> [Liming] This seems the remaining clean up task. So, I prefer to remove > >>> __inline__ if no impact on GCC tool chain. > >> > >> OK. Given your testing with GCC48, I'm fine. > >> > > With this patch set, I verify GCC48/GCC49/GCC5 on Ovmf3264. They can all > > boot to Shell. > > Are they enough? > > Would you guys agree with the following commit message, on this patch? > > """ > __inline__ has no discernible effect with the GCC48 / GCC49 / GCC5 > toolchains, but it breaks the build with CLANG9. > > Remove __inline__. > """ Would it be more accurate to say it didn't have a functional difference? Did we rule out that it might have made a difference in code gen? I guess I wouldn't be surprised if the older non-LTO toolchains didn't make use of it anyway. So, maybe there is no difference in code gen. With LTO the linker is hopefully smarter about auto-inlining anyhow. -Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48853): https://edk2.groups.io/g/devel/message/48853 Mute This Topic: https://groups.io/mt/34309058/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch] EmulatorPkg/TimerLib: Add missing GetTimeInNanoSecond function
On 2019-09-18 01:13:54, Liming Gao wrote:
> From: mjohn4
It looks like the author is not set properly. If you run "git log -1",
then it'll probably show mjohn4 rather than Michael Johnson.
Michael should run:
$ git config --global user.name "Michael Johnson"
After that when git commit it will get the correct author name in the
patch.
Michael, Liming: You can adjust it locally with:
git commit --amend --author="Michael Johnson "
>
> Add GetTimeInNanoSecond, already declared in the TimerLib API,
> to EmulatorPkg implementations of TimerLib.
>
> Cc: Jordan Justen
> Cc: Andrew Fish
> Cc: Ray Ni
> Signed-off-by: Johnson, Michael
To be a valid email address, I think this should either be:
Signed-off-by: "Johnson, Michael"
or
Signed-off-by: Michael Johnson
The second form is more common.
If user.name was set as above then "git commit -s" would add it to the
patch automatically, and correctly.
Aside from all that, it seems like the code matches other
implementations in edk2, so:
Reviewed-by: Jordan Justen
> ---
> .../Library/DxeCoreTimerLib/DxeCoreTimerLib.c | 45 -
> EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c | 45 -
> EmulatorPkg/Library/PeiTimerLib/PeiTimerLib.c | 47
> +-
> 3 files changed, 134 insertions(+), 3 deletions(-)
>
> diff --git a/EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.c
> b/EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.c
> index c331cbba9c..ab0de143c4 100644
> --- a/EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.c
> +++ b/EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.c
> @@ -1,12 +1,13 @@
> /** @file
>A non-functional instance of the Timer Library.
>
> - Copyright (c) 2007 - 2010, Intel Corporation. All rights reserved.
> + Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.
>SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
>
> #include
> +#include
> #include
> #include
> #include
> @@ -119,4 +120,46 @@ GetPerformanceCounterProperties (
>return gEmuThunk->QueryPerformanceFrequency ();
> }
>
> +/**
> + Converts elapsed ticks of performance counter to time in nanoseconds.
> +
> + This function converts the elapsed ticks of running performance counter to
> + time value in unit of nanoseconds.
> +
> + @param Ticks The number of elapsed ticks of running performance
> counter.
> +
> + @return The elapsed time in nanoseconds.
> +
> +**/
> +UINT64
> +EFIAPI
> +GetTimeInNanoSecond (
> + IN UINT64 Ticks
> + )
> +{
> + UINT64 Frequency;
> + UINT64 NanoSeconds;
> + UINT64 Remainder;
> + INTNShift;
> +
> + Frequency = GetPerformanceCounterProperties (NULL, NULL);
> +
> + //
> + // Ticks
> + // Time = - x 1,000,000,000
> + //Frequency
> + //
> + NanoSeconds = MultU64x32 (DivU64x64Remainder (Ticks, Frequency,
> ), 10u);
> +
> + //
> + // Ensure (Remainder * 1,000,000,000) will not overflow 64-bit.
> + // Since 2^29 < 1,000,000,000 = 0x3B9ACA00 < 2^30, Remainder should <
> 2^(64-30) = 2^34,
> + // i.e. highest bit set in Remainder should <= 33.
> + //
> + Shift = MAX (0, HighBitSet64 (Remainder) - 33);
> + Remainder = RShiftU64 (Remainder, (UINTN) Shift);
> + Frequency = RShiftU64 (Frequency, (UINTN) Shift);
> + NanoSeconds += DivU64x64Remainder (MultU64x32 (Remainder, 10u),
> Frequency, NULL);
>
> + return NanoSeconds;
> +}
> diff --git a/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c
> b/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c
> index 14cae4214c..1bbc9e0162 100644
> --- a/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c
> +++ b/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c
> @@ -1,7 +1,7 @@
> /** @file
>A non-functional instance of the Timer Library.
>
> - Copyright (c) 2007 - 2010, Intel Corporation. All rights reserved.
> + Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.
>SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -198,3 +198,46 @@ DxeTimerLibConstructor (
>return EFI_SUCCESS;
> }
>
> +/**
> + Converts elapsed ticks of performance counter to time in nanoseconds.
> +
> + This function converts the elapsed ticks of running performance counter to
> + time value in unit of nanoseconds.
> +
> + @param Ticks The number of elapsed ticks of running performance
> counter.
> +
> + @return The elapsed time in nanoseconds.
> +
> +**/
> +UINT64
> +EFIAPI
> +GetTimeInNanoSecond (
> + IN UINT64 Ticks
> + )
> +{
> + UINT64 Frequency;
> + UINT64 NanoSeconds;
&
Re: [edk2-devel] [RFC PATCH 01/28] OvmfPkg/Sec: Enable cache early to speed up booting
On 2019-08-22 06:46:07, Laszlo Ersek wrote:
> On 08/21/19 23:51, Jordan Justen wrote:
> > On 2019-08-21 07:21:25, Laszlo Ersek wrote:
> >> On 08/19/19 23:35, Lendacky, Thomas wrote:
> >>> From: Tom Lendacky
> >>>
> >>> + //
> >>> + // Enable caching
> >>> + //
> >>> + AsmEnableCache ();
> >>> +
> >>>DEBUG ((EFI_D_INFO,
> >>> "SecCoreStartupWithStack(0x%x, 0x%x)\n",
> >>> (UINT32)(UINTN)BootFv,
> >>>
> >>
> >> This makes me uncomfortable. There used to be problems related to
> >> caching when VFIO device assignment were used. My concern is admittedly
> >> vague, but this is a very brittle area of OVMF-on-KVM. If you asked me
> >> "well what could break here", I'd answer "you never know, and the burden
> >> of proof is not on me". :) Can we make this change conditional on SEV-ES?
> >
> > This was also raised as an issue by Peter for the ACRN hypervisor and
> > Scott for the bhyve hypervisor.
> >
> > I think it is rare for a platform to enable cache at this early of a
> > stage, but it is also rare to decompress a firmware volume at this
> > point.
> >
> > It appears that it could be helpful to figure out how to safely enable
> > cache by default here, since it does seem to be impacting several
> > hypervisors.
>
> I can't think of anything better than "trial and error".
Maybe we could try to detect kvm, and enable caching if !kvm.
Maybe we could enable it during the decompress of the PEI FV and
disable it afterward?
> The issues that
> used to pop up in the past, due to host kernel (KVM) changes,
> particularly in connection with VFIO device assignment, have been
> completely obscure and unpenetrable to me.
Don't we eventually enable caching during the boot, so how is VFIO not
affected by that?
> Even though I've contributed
> at least one KVM patch to mitigate those problems, they remain a mistery
> to me, and I remain unable to *reason* about the problems or the fixes.
If VFIO requires uncached access, then what mechanisms does kvm
support for accessing memory ranges uncached? I thought kvm simply
ignored the cache setting and always enabled caching, because this
section of boot is not particularly slow with kvm. But, if enabling
caching causes issues, then I guess it does something.
Does kvm support mtrr to uncache i/o ranges? I didn't think kvm
supported mtrrs in the past.
I hope we wouldn't have to use paging to disable caching for the
affected regions.
-Jordan
> So I think we could only flip the behavior (enable cache by default) and
> collect bug reports. But that's extremely annoying for end-users, and I
> see "no regressions" as one of my top responsibilities.
>
> Even if we provided an fw_cfg knob to disable the change, using
> QemuFwCfgSecLib, similarly to commit ab081a50e565 ("OvmfPkg:
> PlatformPei: take no-exec DXE settings from the QEMU command line",
> 2015-09-15), such fw_cfg knobs are difficult to use through layered
> products, such as libvirt, proxmox, etc. And of course fw_cfg is only
> available on QEMU.
>
> Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46236): https://edk2.groups.io/g/devel/message/46236
Mute This Topic: https://groups.io/mt/32966275/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFC PATCH 01/28] OvmfPkg/Sec: Enable cache early to speed up booting
On 2019-08-21 07:21:25, Laszlo Ersek wrote: > On 08/19/19 23:35, Lendacky, Thomas wrote: > > From: Tom Lendacky > > > > Currently, the OVMF code relies on the hypervisor to enable the cache > > support on the processor in order to improve the boot speed. However, > > with SEV-ES, the hypervisor is not allowed to change the CR0 register > > to enable caching. > > > > Update the OVMF Sec support to enable caching in order to improve the > > boot speed. > > > > Signed-off-by: Tom Lendacky > > --- > > OvmfPkg/Sec/SecMain.c | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c > > index 3914355cd17b..2448be0cd408 100644 > > --- a/OvmfPkg/Sec/SecMain.c > > +++ b/OvmfPkg/Sec/SecMain.c > > @@ -739,6 +739,11 @@ SecCoreStartupWithStack ( > > > >ProcessLibraryConstructorList (NULL, NULL); > > > > + // > > + // Enable caching > > + // > > + AsmEnableCache (); > > + > >DEBUG ((EFI_D_INFO, > > "SecCoreStartupWithStack(0x%x, 0x%x)\n", > > (UINT32)(UINTN)BootFv, > > > > This makes me uncomfortable. There used to be problems related to > caching when VFIO device assignment were used. My concern is admittedly > vague, but this is a very brittle area of OVMF-on-KVM. If you asked me > "well what could break here", I'd answer "you never know, and the burden > of proof is not on me". :) Can we make this change conditional on SEV-ES? This was also raised as an issue by Peter for the ACRN hypervisor and Scott for the bhyve hypervisor. I think it is rare for a platform to enable cache at this early of a stage, but it is also rare to decompress a firmware volume at this point. It appears that it could be helpful to figure out how to safely enable cache by default here, since it does seem to be impacting several hypervisors. -Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46183): https://edk2.groups.io/g/devel/message/46183 Mute This Topic: https://groups.io/mt/32966275/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V5 00/11] EmulatorPkg: Fix VS20xx IA32 boot and simplify build config
On 2019-08-16 17:57:04, Michael D Kinney wrote: > > Andrew Fish (7): > EmulatorPkg/Unix/Host: Disable inline/optimizations Maybe add XCODE5 in commit message subject? Acked-by: Jordan Justen > EmulatorPkg: Fix XCODE5 lldb issues There was another long line in the EmulatorPkg/Unix/lldbefi.py change. Acked-by: Jordan Justen > EmulatorPkg/Unix/Host: Initialize field in BerkeleyPacketFilter.c Reviewed-by: Jordan Justen > EmulatorPkg/Unix/Host: Remove debug code from BerkeleyPacketFilter.c Reviewed-by: Jordan Justen > EmulatorPkg: Disable TftpDynamicCommand and LogoDxe for XCODE5 Acked-by: Jordan Justen > EmulatorPkg/Sec: Change scope of PpiArray[10] Reviewed-by: Jordan Justen > BaseTools/tools_def.template: Add -gdwarf to XCODE5 X64 Acked-by: Jordan Justen -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46020): https://edk2.groups.io/g/devel/message/46020 Mute This Topic: https://groups.io/mt/32918474/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V4 09/10] EmulatorPkg/Sec: Change local variable scope for XCODE5
On 2019-08-15 19:14:36, Michael D Kinney wrote:
> The local variable PpiArray[10] is declared in middle
> of the SEC module _ModuleEntryPoint(). When building
> for XCODE5 with optimizations enabled, this results in
> corruption and an exception.
It looks like with old code, the scope containing PpiArray was closed,
but a dangling reference to had been made to it's location on the
stack. So, I think the change is good but we should add this extra
detail to the commit message.
-Jordan
> The fix is to move the
> declaration of PpiArray[10] to the standard location
> at the beginning of the function so the storage for
> this local variable is allocated for the entire
> lifetime of the function.
>
> Cc: Jordan Justen
> Cc: Ray Ni
> Cc: Michael D Kinney
> Signed-off-by: Andrew Fish
> ---
> EmulatorPkg/Sec/Sec.c | 16 +++-
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/EmulatorPkg/Sec/Sec.c b/EmulatorPkg/Sec/Sec.c
> index 701032233b..b734d2bb87 100644
> --- a/EmulatorPkg/Sec/Sec.c
> +++ b/EmulatorPkg/Sec/Sec.c
> @@ -75,6 +75,7 @@ _ModuleEntryPoint (
>EFI_PEI_PPI_DESCRIPTOR*SecPpiList;
>UINTN SecReseveredMemorySize;
>UINTN Index;
> + EFI_PEI_PPI_DESCRIPTORPpiArray[10];
>
>EMU_MAGIC_PAGE()->PpiList = PpiList;
>ProcessLibraryConstructorList ();
> @@ -104,16 +105,13 @@ _ModuleEntryPoint (
>SecCoreData->PeiTemporaryRamBase = (VOID
> *)((UINTN)SecCoreData->PeiTemporaryRamBase + SecReseveredMemorySize);
>SecCoreData->PeiTemporaryRamSize -= SecReseveredMemorySize;
> #else
> - {
> -//
> -// When I subtrack from SecCoreData->PeiTemporaryRamBase PEI Core
> crashes? Either there is a bug
> -// or I don't understand temp RAM correctly?
> -//
> -EFI_PEI_PPI_DESCRIPTORPpiArray[10];
> + //
> + // When I subtrack from SecCoreData->PeiTemporaryRamBase PEI Core crashes?
> Either there is a bug
> + // or I don't understand temp RAM correctly?
> + //
>
> -SecPpiList = [0];
> -ASSERT (sizeof (PpiArray) >= SecReseveredMemorySize);
> - }
> + SecPpiList = [0];
> + ASSERT (sizeof (PpiArray) >= SecReseveredMemorySize);
> #endif
>// Copy existing list, and append our entries.
>CopyMem (SecPpiList, PpiList, sizeof (EFI_PEI_PPI_DESCRIPTOR) * Index);
> --
> 2.21.0.windows.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#45846): https://edk2.groups.io/g/devel/message/45846
Mute This Topic: https://groups.io/mt/32894354/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V4 06/10] EmulatorPkg: Fix XCODE5 lldb issues
On 2019-08-16 08:09:55, Kinney, Michael D wrote:
> Jordan,
>
> It is not a typo.
>
> Andrew generated the XCODE specific changes, so they have
> been tested by him.
In that case, is the git author for the patches set to Andrew? If it
was, I would expect to see a line inside the email body with:
From: Andrew Fish
Git does that for patches where the sender doesn't match the patch
author.
You might want to rebase and run:
git commit --amend --author="Andrew Fish " --reset-author
to change the author.
I expect you might want to add Reviewed-by for yourself on these
patches to help speed things along. If Andrew authored the patch, you
reviewed it, and with a quick review it looked good to me, I would
probably add an Acked-by for some of the patches.
-Jordan
> I have also reviewed and tested the XCODE
> changes and verified that all 6 combinations build and boot
> to shell (IA32/X64 for RELEASE/DEBUG/NOOPT). Since they are
> all related to making EmulatorPkg work, we decided to fold
> them into the same patch set that was already being reviewed.
>
> I also verified build and boot to shell for 6 combinations
> on GCC5 (IA32/X64 for RELEASE/DEBUG/NOOPT) and the 12
> combinations of VS2015/VS2017, IA323/X64, RELEASE/DEBUG/NOOPT.
>
> I have been working on some CI experiments using Azure Pipelines.
> Here is a pointer to the build logs for all the combinations
> listed above.
>
> https://dev.azure.com/mikekinney/edk2-ci/_build/results?buildId=312
>
> Mike
>
> > -Original Message-
> > From: Justen, Jordan L
> > Sent: Friday, August 16, 2019 12:41 AM
> > To: Kinney, Michael D ;
> > devel@edk2.groups.io
> > Cc: Ni, Ray ; Andrew Fish
> >
> > Subject: Re: [Patch V4 06/10] EmulatorPkg: Fix XCODE5
> > lldb issues
> >
> > On 2019-08-15 19:14:33, Michael D Kinney wrote:
> > > Fix scripts to support lldb symbolic debugging when
> > using XCODE5 tool
> > > chain.
> > >
> > > Cc: Jordan Justen
> > > Cc: Ray Ni
> > > Cc: Michael D Kinney
> > > Signed-off-by: Andrew Fish
> >
> > Is this a Cc/Signed-off-by typo? (See also, patches 7-
> > 10).
> >
> > This makes me wonder if you are taking advantage of the
> > git commit -s switch to add your Signed-off-by.
> >
> > Also, I'm wondering if you are taking advantage of git-
> > send-email automatically Cc'ing the addresses you
> > listed in the commit message.
> > (I thought it Cc'd for the author and Cc tags, but I
> > wasn't sure about the Signed-off-by tag, and yet I see
> > Andrew was Cc'd.)
> >
> > There's a couple long lines below. You could use \ at
> > the end of the line to split the .sh line. I think the
> > cd can be a separate command in a shell script. (Not in
> > make)
> >
> > I hope someone that uses the XCODE toolchain could
> > review/check the XCODE patches.
> >
> > -Jordan
> >
> > > ---
> > > EmulatorPkg/Unix/lldbefi.py | 8 +---
> > > EmulatorPkg/build.sh| 17 ++---
> > > 2 files changed, 7 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/EmulatorPkg/Unix/lldbefi.py
> > b/EmulatorPkg/Unix/lldbefi.py
> > > index 218326b8cb..099192d8b5 100755
> > > --- a/EmulatorPkg/Unix/lldbefi.py
> > > +++ b/EmulatorPkg/Unix/lldbefi.py
> > > @@ -346,6 +346,7 @@ def TypePrintFormating(debugger):
> > > debugger.HandleCommand("type summary add CHAR8 -
> > -python-function lldbefi.CHAR8_TypeSummary")
> > > debugger.HandleCommand('type summary add --regex
> > "CHAR8
> > > \[[0-9]+\]" --python-function
> > lldbefi.CHAR8_TypeSummary')
> > >
> > > +debugger.HandleCommand('setting set frame-format
> > "frame
> > > + #${frame.index}: ${frame.pc}{
> > > +
> > ${module.file.basename}{:${function.name}()${function.p
> > c-offset}}}{
> > > + at ${line.file.fullpath}:${line.number}}\n"')
> > >
> > > gEmulatorBreakWorkaroundNeeded = True
> > >
> > > @@ -381,15 +382,16 @@ def
> > LoadEmulatorEfiSymbols(frame, bp_loc , internal_dict):
> > > Error = lldb.SBError()
> > > FileNamePtr = frame.FindVariable
> > ("FileName").GetValueAsUnsigned()
> > > FileNameLen = frame.FindVariable
> > > ("FileNameLength").GetValueAsUnsigned()
> > > +
> > > FileName =
> > frame.thread.process.ReadCStringFromMemory
> > (FileNamePtr, FileNameLen, Error)
> > > if not Error.Succ
Re: [edk2-devel] [Patch V4 07/10] EmulatorPkg/Unix/Host: Fix BerkeleyPacketFilter.c issues
On 2019-08-15 19:14:34, Michael D Kinney wrote:
> * Fix uninitialized Private->ReadBuffer.
> * Remove old debug code that generates an exception.
Bulleted lists in the commit message often makes me think they should
be separate patches. Maybe that could improve the commit message
subject line for the patches too. :)
-Jordan
>
> Cc: Jordan Justen
> Cc: Ray Ni
> Cc: Michael D Kinney
> Signed-off-by: Andrew Fish
> ---
> EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c | 5 +
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c
> b/EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c
> index 8d0eb0d197..3013bbc86b 100644
> --- a/EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c
> +++ b/EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c
> @@ -216,6 +216,7 @@ EmuSnpStart (
>}
>
>Status = EFI_SUCCESS;
> + Private->ReadBuffer = NULL;
>if (Private->BpfFd == 0) {
> Status = OpenBpfFileDescriptor (Private, >BpfFd);
> if (EFI_ERROR (Status)) {
> @@ -766,10 +767,6 @@ EmuSnpGetStatus (
>
>Private = EMU_SNP_PRIVATE_DATA_FROM_THIS (This);
>
> - if (TxBuf != NULL) {
> -*((UINT8 **)TxBuf) = (UINT8 *)1;
> - }
> -
>if ( InterruptStatus != NULL ) {
> *InterruptStatus = EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT;
>}
> --
> 2.21.0.windows.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#45800): https://edk2.groups.io/g/devel/message/45800
Mute This Topic: https://groups.io/mt/32894352/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V4 06/10] EmulatorPkg: Fix XCODE5 lldb issues
On 2019-08-15 19:14:33, Michael D Kinney wrote:
> Fix scripts to support lldb symbolic debugging when
> using XCODE5 tool chain.
>
> Cc: Jordan Justen
> Cc: Ray Ni
> Cc: Michael D Kinney
> Signed-off-by: Andrew Fish
Is this a Cc/Signed-off-by typo? (See also, patches 7-10).
This makes me wonder if you are taking advantage of the git commit -s
switch to add your Signed-off-by.
Also, I'm wondering if you are taking advantage of git-send-email
automatically Cc'ing the addresses you listed in the commit message.
(I thought it Cc'd for the author and Cc tags, but I wasn't sure about
the Signed-off-by tag, and yet I see Andrew was Cc'd.)
There's a couple long lines below. You could use \ at the end of the
line to split the .sh line. I think the cd can be a separate command
in a shell script. (Not in make)
I hope someone that uses the XCODE toolchain could review/check the
XCODE patches.
-Jordan
> ---
> EmulatorPkg/Unix/lldbefi.py | 8 +---
> EmulatorPkg/build.sh| 17 ++---
> 2 files changed, 7 insertions(+), 18 deletions(-)
>
> diff --git a/EmulatorPkg/Unix/lldbefi.py b/EmulatorPkg/Unix/lldbefi.py
> index 218326b8cb..099192d8b5 100755
> --- a/EmulatorPkg/Unix/lldbefi.py
> +++ b/EmulatorPkg/Unix/lldbefi.py
> @@ -346,6 +346,7 @@ def TypePrintFormating(debugger):
> debugger.HandleCommand("type summary add CHAR8 --python-function
> lldbefi.CHAR8_TypeSummary")
> debugger.HandleCommand('type summary add --regex "CHAR8 \[[0-9]+\]"
> --python-function lldbefi.CHAR8_TypeSummary')
>
> +debugger.HandleCommand('setting set frame-format "frame #${frame.index}:
> ${frame.pc}{
> ${module.file.basename}{:${function.name}()${function.pc-offset}}}{ at
> ${line.file.fullpath}:${line.number}}\n"')
>
> gEmulatorBreakWorkaroundNeeded = True
>
> @@ -381,15 +382,16 @@ def LoadEmulatorEfiSymbols(frame, bp_loc ,
> internal_dict):
> Error = lldb.SBError()
> FileNamePtr = frame.FindVariable ("FileName").GetValueAsUnsigned()
> FileNameLen = frame.FindVariable ("FileNameLength").GetValueAsUnsigned()
> +
> FileName = frame.thread.process.ReadCStringFromMemory (FileNamePtr,
> FileNameLen, Error)
> if not Error.Success():
> print "!ReadCStringFromMemory() did not find a %d byte C string at
> %x" % (FileNameLen, FileNamePtr)
> # make breakpoint command contiue
> -frame.GetThread().GetProcess().Continue()
> +return False
>
> debugger = frame.thread.process.target.debugger
> if frame.FindVariable ("AddSymbolFlag").GetValueAsUnsigned() == 1:
> -LoadAddress = frame.FindVariable ("LoadAddress").GetValueAsUnsigned()
> +LoadAddress = frame.FindVariable
> ("LoadAddress").GetValueAsUnsigned() - 0x240
>
> debugger.HandleCommand ("target modules add %s" % FileName)
> print "target modules load --slid 0x%x %s" % (LoadAddress, FileName)
> @@ -405,7 +407,7 @@ def LoadEmulatorEfiSymbols(frame, bp_loc , internal_dict):
> print "!lldb.target.RemoveModule (%s) FAILED" % SBModule
>
> # make breakpoint command contiue
> -frame.thread.process.Continue()
> +return False
>
> def GuidToCStructStr (guid, Name=False):
>#
> diff --git a/EmulatorPkg/build.sh b/EmulatorPkg/build.sh
> index 60056e1b6c..35912a7775 100755
> --- a/EmulatorPkg/build.sh
> +++ b/EmulatorPkg/build.sh
> @@ -209,21 +209,8 @@ fi
> if [[ "$RUN_EMULATOR" == "yes" ]]; then
>case `uname` in
> Darwin*)
> - #
> - # On Darwin we can't use dlopen, so we have to load the real PE/COFF
> images.
> - # This .gdbinit script sets a breakpoint that loads symbols for the
> PE/COFFEE
> - # images that get loaded in Host
> - #
> - if [[ "$CLANG_VER" == *-ccc-host-triple* ]]
> - then
> - # only older versions of Xcode support -ccc-host-tripe, for newer
> versions
> - # it is -target
> -cp $WORKSPACE/EmulatorPkg/Unix/lldbefi.py
> "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS/$PROCESSOR"
> -cd $BUILD_ROOT_ARCH; /usr/bin/lldb --source
> $WORKSPACE/EmulatorPkg/Unix/lldbinit Host
> -exit $?
> - else
> -cp $WORKSPACE/EmulatorPkg/Unix/.gdbinit
> "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS/$PROCESSOR"
> - fi
> + cd $BUILD_ROOT_ARCH; /usr/bin/lldb -o "command script import
> $WORKSPACE/EmulatorPkg/Unix/lldbefi.py" -o 'script
> lldb.debugger.SetAsync(True)' -o "run" ./Host
> + exit $?
>;;
>esac
>
> --
> 2.21.0.windows.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#45799): https://edk2.groups.io/g/devel/message/45799
Mute This Topic: https://groups.io/mt/32894351/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V3 0/4] EmulatorPkg: Fix VS20xx IA32 boot and simplify build config
On 2019-08-09 15:44:01, Michael D Kinney wrote: > New in V3 > == > * Fix size value used in call to AsciiStrCpyS() in PosixFileSystem.c > * Fix XCODE5 safe string function build failure in BerkleyPacketFilter.c > * Add NOOPT build target to DSC file. > > New in V2 > = > * Fix size values in safe string function calls. > * Update POSIX sources to use AsciiStrCpyS() and AsciiStrCatS(). > * Verify that no exceptions occur if EMU_MAGIC_PAGE() can not be mapped. An > error message is generated and the host app exits normally. > * Update EmulatorPkg DEC file with a new PcdPeiServicesTablePage default value > that works for Windows/POSIX hosts for both IA32 and X64. > > https://bugzilla.tianocore.org/show_bug.cgi?id=162 > https://bugzilla.tianocore.org/show_bug.cgi?id=2055 > https://bugzilla.tianocore.org/show_bug.cgi?id=2056 > > * Fix VS20xx IA32 boot failure > * Remove UNIX_SEC_BUILD/WIN_SEC_BUILD > * Add -D DISABLE_NEW_DEPRECATED_INTERFACES > > Cc: Jordan Justen > Cc: Andrew Fish > Cc: Ray Ni > Signed-off-by: Michael D Kinney > > Michael D Kinney (4): > EmulatorPkg: Fix VS20xx IA32 boot failure Reviewed-by: Jordan Justen > EmulatorPkg: Remove UNIX_SEC_BUILD/WIN_SEC_BUILD Reviewed-by: Jordan Justen > EmulatorPkg: Add -D DISABLE_NEW_DEPRECATED_INTERFACES Acked-by: Jordan Justen > EmulatorPkg: Add support for NOOPT target Reviewed-by: Jordan Justen Were you able to test this still boots on Linux? -Jordan > > EmulatorPkg/EmuBusDriverDxe/EmuBusDriverDxe.c | 9 +- > EmulatorPkg/EmulatorPkg.dec | 4 +- > EmulatorPkg/EmulatorPkg.dsc | 34 ++--- > EmulatorPkg/FlashMapPei/FlashMapPei.c | 8 +- > EmulatorPkg/Library/SmbiosLib/SmbiosLib.c | 4 +- > .../ThunkProtocolList/ThunkProtocolList.c | 11 +- > EmulatorPkg/Readme.md | 8 +- > EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c | 10 +- > EmulatorPkg/Unix/Host/PosixFileSystem.c | 80 > EmulatorPkg/Unix/Host/X11GraphicsWindow.c | 4 +- > EmulatorPkg/Win/Host/WinFileSystem.c | 116 -- > EmulatorPkg/Win/VS2017/BuildVS.bat| 2 +- > EmulatorPkg/build.sh | 8 +- > 13 files changed, 197 insertions(+), 101 deletions(-) > > -- > 2.21.0.windows.1 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#45310): https://edk2.groups.io/g/devel/message/45310 Mute This Topic: https://groups.io/mt/32816073/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch 3/3] BaseTools/PatchCheck: Disable text conversion in 'git show'
On 2019-08-05 15:01:14, Kinney, Michael D wrote: > Laszlo, > > The context of this change is only to the PatchCheck.py tool. > and how that tool uses git show. > > I agree with the summary of very flexible capabilities in git > to help developers review different types of files. All of > those settings that were added to support UNI files in UTF-16 > file format were very valuable when we had to review the > text changes to those binary files. We should be using UTF-8 > now, and we can even extend PatchCheck.py to flag an error if > a UNI file is in UTF-16 format. Ah. If we are to the point where we want to actively prevent utf-16 in the tree, then this sounds like a good idea. > I still prefer we make this change only to PatchCheck.py to > prevent false positives and print lines of text that can > not be found in a developer's working directory. I prefer > this one time change to PatchCheck.py instead of having to > update .gitattributes whenever the git show features are > extended to convert more binary files to text files. I think it's pretty rare for EDK II to add new binary file types, but I don't feel too strongly on this. I thought adding Laszlo's settings to .gitattributes might amount to solving two issues with one change. Since Laszlo acked this patch, I'll go ahead with: Reviewed-by: Jordan Justen > My expectation is that EDK II Maintainers need to review > if a binary file is ok or not for a repo. I would also be > ok with adding general rules to PatchCheck.py to generate > an error if a binary file is added/updated in one of the > text only repos (edk2, edk2-platforms) and not generate > an error if a binary file is added/updated in a repo that > supports binaries (edk-non-osi). -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#45034): https://edk2.groups.io/g/devel/message/45034 Mute This Topic: https://groups.io/mt/32685494/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch 0/3] BaseTools/PatchCheck: Fix false positives
On 2019-08-01 17:13:11, Michael D Kinney wrote: > https://bugzilla.tianocore.org/show_bug.cgi?id=2044 > > * Add '--no-textconv' to 'git show' command to disable binary->text > conversion. > * Ignore blank lines in patch > * Ignore lines with 'copy from ' and 'copy to ' prefixes > > Cc: Bob Feng > Cc: Liming Gao > Cc: Jordan Justen > Signed-off-by: Michael D Kinney > > Michael D Kinney (3): > BaseTools/PatchCheck: Ignore blank lines in diff > BaseTools/PatchCheck: Add copy from/to keywords patches 1 and 2: Reviewed-by: Jordan Justen > BaseTools/PatchCheck: Disable text conversion in 'git show' > > BaseTools/Scripts/PatchCheck.py | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > -- > 2.21.0.windows.1 > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44865): https://edk2.groups.io/g/devel/message/44865 Mute This Topic: https://groups.io/mt/32685491/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch 3/3] BaseTools/PatchCheck: Disable text conversion in 'git show'
First, I hope we don't add lots of large .pdf files into the source
tree. I see two duplicated > 200k .pdf files in edk2, which seems like
a waste of space in the edk2 tree.
BaseTools/Source/C/BrotliCompress/docs/brotli-comparison-study-2015-09-22.pdf
MdeModulePkg/Library/BrotliCustomDecompressLib/docs/brotli-comparison-study-2015-09-22.pdf
Second, I'm not too sure about this change. It seems like it might
have unintended consequences.
One thought I had is that it might break UTF-16 unicode files diffs,
but luckily I think we've pretty much gotten rid of UTF-16 files at
this point.
I also wonder if adding a .pdf attribute like Laszlo recommends for
.efi might be an alternative solution.
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-09
We could actually add these setting in the git tree in a
.gitattributes file, right? Has this been suggested? I think Laszlo
documented this before we had converted edk2 to git.
-Jordan
On 2019-08-01 17:13:14, Michael D Kinney wrote:
> https://bugzilla.tianocore.org/show_bug.cgi?id=2044
>
> 'git show' is used to extrat the patch contents for analysis.
> Add the flag '--no-textconv' to the 'git show' command to
> disable the conversion from some binary file types to text
> content.
>
> Without this change, binary files such as .pdf files are
> converted to text in the show command and PatchCheck complains
> that the wrong line endings are used in the patch.
>
> Cc: Bob Feng
> Cc: Liming Gao
> Cc: Jordan Justen
> Signed-off-by: Michael D Kinney
> ---
> BaseTools/Scripts/PatchCheck.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
> index 6b07241bfe..2a4e6f603e 100755
> --- a/BaseTools/Scripts/PatchCheck.py
> +++ b/BaseTools/Scripts/PatchCheck.py
> @@ -543,7 +543,7 @@ class CheckGitCommits:
>
> def read_patch_from_git(self, commit):
> # Run git to get the commit patch
> -return self.run_git('show', '--pretty=email', commit)
> +return self.run_git('show', '--pretty=email', '--no-textconv',
> commit)
>
> def run_git(self, *args):
> cmd = [ 'git' ]
> --
> 2.21.0.windows.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#44864): https://edk2.groups.io/g/devel/message/44864
Mute This Topic: https://groups.io/mt/32685494/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 3/3] OvmfPkg/build.sh: remove $ADD_QEMU_HDA
On 2019-07-29 19:24:00, wrote: > On 2019-07-24 16:56, Jordan Justen wrote: > > Reviewed-by: Jordan Justen > > > > On 2019-07-24 14:44:05, Rebecca Cran wrote: > >> $ADD_QEMU_HDA was added because QEMU used to refuse to run without a > >> disk. Since newer versions run without any disks, remove it. > >> > >> Signed-off-by: Rebecca Cran > > Could someone push the series, please? > I pushed 2 & 3 as: 4634fd429e G OvmfPkg/build.sh: remove $ADD_QEMU_HDA 8fed4e47d9 G (origin/master, origin/HEAD) OvmfPkg/build.sh: use newer '-drive if=pflash' syntax when running qemu I still want to look closer at patch 1. -Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44588): https://edk2.groups.io/g/devel/message/44588 Mute This Topic: https://groups.io/mt/32591949/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 2/3] OvmfPkg/build.sh: use newer '-drive if=pflash' syntax when running qemu
Reviewed-by: Jordan Justen On 2019-07-24 14:44:04, Rebecca Cran wrote: > Specify the firmware to use via the newer '-drive if=pflash' syntax > which allows specifying the raw format parameter. This > avoids warnings with newer version of QEMU. > > Signed-off-by: Rebecca Cran > --- > OvmfPkg/build.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh > index 1c28e65404..0219791aa8 100755 > --- a/OvmfPkg/build.sh > +++ b/OvmfPkg/build.sh > @@ -263,7 +263,7 @@ if [[ "$RUN_QEMU" == "yes" ]]; then >fi >ln -sf $FV_DIR/OVMF.fd $QEMU_FIRMWARE_DIR/bios.bin >if [[ "$ENABLE_FLASH" == "yes" ]]; then > -QEMU_COMMAND="$QEMU_COMMAND -pflash $QEMU_FIRMWARE_DIR/bios.bin" > +QEMU_COMMAND="$QEMU_COMMAND -drive > if=pflash,format=raw,file=$QEMU_FIRMWARE_DIR/bios.bin" >else > QEMU_COMMAND="$QEMU_COMMAND -L $QEMU_FIRMWARE_DIR" >fi > -- > 2.22.0 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44357): https://edk2.groups.io/g/devel/message/44357 Mute This Topic: https://groups.io/mt/32591948/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] OvmfPkg/build.sh: remove literal carriage return
Reviewed-by: Jordan Justen 0dd8d7d556df46c503254d37b22b2b34f6ad12f6 But I still forgot to fix up your git author name after applying the patch. :\ On 2019-07-24 18:35:40, wrote: > Signed-off-by: Rebecca Cran > --- > OvmfPkg/build.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh > index 960382f633..78f44e3a96 100755 > --- a/OvmfPkg/build.sh > +++ b/OvmfPkg/build.sh > @@ -40,7 +40,7 @@ ARCH_X64=no > BUILDTARGET=DEBUG > BUILD_OPTIONS= > PLATFORMFILE= > -THREADNUMBER=0 > +THREADNUMBER=0 > LAST_ARG= > RUN_QEMU=no > ENABLE_FLASH=no > -- > 2.22.0 > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44354): https://edk2.groups.io/g/devel/message/44354 Mute This Topic: https://groups.io/mt/32594107/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 3/3] OvmfPkg/build.sh: remove $ADD_QEMU_HDA
Reviewed-by: Jordan Justen On 2019-07-24 14:44:05, Rebecca Cran wrote: > $ADD_QEMU_HDA was added because QEMU used to refuse to run without a > disk. Since newer versions run without any disks, remove it. > > Signed-off-by: Rebecca Cran > --- > OvmfPkg/build.sh | 14 -- > 1 file changed, 14 deletions(-) > > diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh > index 0219791aa8..960382f633 100755 > --- a/OvmfPkg/build.sh > +++ b/OvmfPkg/build.sh > @@ -213,17 +213,6 @@ if [[ "$RUN_QEMU" == "yes" ]]; then >ENABLE_FLASH=yes >;; >esac > - > - ADD_QEMU_HDA=yes > - for arg in "$@" > - do > -case $arg in > - -hd[a-d]|-fd[ab]|-cdrom) > -ADD_QEMU_HDA=no > -break > -;; > -esac > - done > fi > > # > @@ -267,9 +256,6 @@ if [[ "$RUN_QEMU" == "yes" ]]; then >else > QEMU_COMMAND="$QEMU_COMMAND -L $QEMU_FIRMWARE_DIR" >fi > - if [[ "$ADD_QEMU_HDA" == "yes" ]]; then > -QEMU_COMMAND="$QEMU_COMMAND -hda fat:$BUILD_ROOT_ARCH" > - fi >echo Running: $QEMU_COMMAND "$@" >$QEMU_COMMAND "$@" >exit $? > -- > 2.22.0 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44342): https://edk2.groups.io/g/devel/message/44342 Mute This Topic: https://groups.io/mt/32591949/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2] OvmfPkg/build.sh: enable multitheaded build by default
On 2019-07-24 05:21:35, Philippe Mathieu-Daudé wrote: > On 7/23/19 2:32 AM, rebe...@bsdio.com wrote: > > Enable multithreaded builds by default when building OvmfPkg > > using build.sh. > > This can drastically reduce build times. For example, on a > > modern ThreadRipper system the time required to build decreases > > from 3 minutes to 1 minute. > > > > Signed-off-by: Rebecca Cran > > --- > > OvmfPkg/build.sh | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh > > index 4fcbdd2bc9..bacab5e22a 100755 > > --- a/OvmfPkg/build.sh > > +++ b/OvmfPkg/build.sh > > @@ -40,7 +40,7 @@ ARCH_X64=no > > BUILDTARGET=DEBUG > > BUILD_OPTIONS= > > PLATFORMFILE= > > -THREADNUMBER=1 > > +THREADNUMBER=0 > > LAST_ARG= > > RUN_QEMU=no > > ENABLE_FLASH=no > > > > Reviewed-by: Philippe Mathieu-Daude Whoops. Sorry, I pushed this a few days back, but I forgot to reply to notify the list. It's 83e7d5c75e7304aa5172c88eb24fa563445ce043. -Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44341): https://edk2.groups.io/g/devel/message/44341 Mute This Topic: https://groups.io/mt/32565505/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] OvmfPkg/build.sh: use newer '-drive if=pflash' syntax when running qemu
On 2019-07-24 10:39:20, Philippe Mathieu-Daudé wrote: > On 7/24/19 7:26 PM, Jordan Justen wrote: > > Hmm, it looks like qemu 2.0 is when the multiple flash device support > > was added: https://wiki.qemu.org/ChangeLog/2.0 > > > > The build.sh script currently attempts to detect 1.6 and newer for > > flash support. pflash first appeared in 1.1, but kvm support was added > > in 1.6. I think it'd be reasonable to alter the script to not use > > flash for qemu < 2.0, to enable using separate code and vars binaries. > > Where can I find the range of QEMU versions used by EDK2? > I'm trying to understand why it is important to still maintain scripts > for a such old version. OvmfPkg/README mentions as far back as qemu 0.10. It seems it was update from 0.9.1 to 0.10 in 2012. (Oh, cool! OVMF just passed 10 years of being released as open source. Since May of 2009! :) I think we've tried to keep the range of versions that it works with as wide as possible, but I think we could (and perhaps should) discuss raising the minimum supported version of qemu. qemu 2.0 is over 5 years old now. qemu 0.10.0 is over 10 years old. One thing that was originally problematic (for raising the qemu requirement) was finding windows builds of newer qemu releases. The main website seems to link to windows builds of the latest releases now: https://www.qemu.org/download/#windows -Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44340): https://edk2.groups.io/g/devel/message/44340 Mute This Topic: https://groups.io/mt/32580104/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] OvmfPkg/build.sh: use newer '-drive if=pflash' syntax when running qemu
On 2019-07-24 05:27:28, Philippe Mathieu-Daudé wrote: > On 7/24/19 4:54 AM, Jordan Justen wrote: > > On 2019-07-23 18:51:00, wrote: > >> Specify the firmware to use via the newer '-drive if=pflash' syntax > >> which allows specifying the raw format and readonly parameters. This > >> avoids warnings with newer version of QEMU. > >> > >> Signed-off-by: Rebecca Cran > >> --- > >> OvmfPkg/build.sh | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh > >> index a2c4eff2a5..e2890ff762 100755 > >> --- a/OvmfPkg/build.sh > >> +++ b/OvmfPkg/build.sh > >> @@ -268,7 +268,7 @@ if [[ "$RUN_QEMU" == "yes" ]]; then > >>fi > >>ln -sf $FV_DIR/OVMF.fd $QEMU_FIRMWARE_DIR/bios.bin > >>if [[ "$ENABLE_FLASH" == "yes" ]]; then > >> -QEMU_COMMAND="$QEMU_COMMAND -pflash $QEMU_FIRMWARE_DIR/bios.bin" > >> +QEMU_COMMAND="$QEMU_COMMAND -drive > >> if=pflash,format=raw,readonly,file=$QEMU_FIRMWARE_DIR/bios.bin" > > > > It looks like we set ENABLE_FLASH for qemu 1.6 and newer. Does this > > work on 1.6? > > > > Also, I don't think we want readonly. Read-write should allow > > non-volatile variables to persist across multiple boot. > > This flash only contains the CODE, not the non-volatile VARS, using > readonly makes sense to me. That's not the case here. The build.sh support uses a combined code/vars binary. Originally the qemu pflash support only supported a single binary. At this point we can probably assume that a new enough version of qemu is available that supports multiple pflash devices, so build.sh could be upgraded to use the split binaries. Hmm, it looks like qemu 2.0 is when the multiple flash device support was added: https://wiki.qemu.org/ChangeLog/2.0 The build.sh script currently attempts to detect 1.6 and newer for flash support. pflash first appeared in 1.1, but kvm support was added in 1.6. I think it'd be reasonable to alter the script to not use flash for qemu < 2.0, to enable using separate code and vars binaries. -Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44325): https://edk2.groups.io/g/devel/message/44325 Mute This Topic: https://groups.io/mt/32580104/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 2/2] OvmfPkg/build.sh: use $QEMU_FIRMWARE_DIR as QEMU fat root
I'd kind of like to remove the ADD_QEMU_HDA support. I think, way, way back qemu would not boot if no drives were added, so this kind of helped. (OVMF can still load the shell with no drives.) If qemu will still run the firmware without a drive, then maybe maybe we can just dump ADD_QEMU_HDA. -Jordan On 2019-07-23 18:51:01, Rebecca Cran wrote: > The $BUILD_ROOT_ARCH directory has too many files to > work as a fat filesystem. QEMU fails with the message: > > Too many entries in root directory > > Use the $QEMU_FIRMWARE_DIR as the root directory instead. > > Signed-off-by: Rebecca Cran > --- > OvmfPkg/build.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh > index e2890ff762..b01de0fd55 100755 > --- a/OvmfPkg/build.sh > +++ b/OvmfPkg/build.sh > @@ -273,7 +273,7 @@ if [[ "$RUN_QEMU" == "yes" ]]; then > QEMU_COMMAND="$QEMU_COMMAND -L $QEMU_FIRMWARE_DIR" >fi >if [[ "$ADD_QEMU_HDA" == "yes" ]]; then > -QEMU_COMMAND="$QEMU_COMMAND -hda fat:$BUILD_ROOT_ARCH" > +QEMU_COMMAND="$QEMU_COMMAND -hda fat:$QEMU_FIRMWARE_DIR" >fi >echo Running: $QEMU_COMMAND "$@" >$QEMU_COMMAND "$@" > -- > 2.22.0 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44285): https://edk2.groups.io/g/devel/message/44285 Mute This Topic: https://groups.io/mt/32580106/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] OvmfPkg/build.sh: use newer '-drive if=pflash' syntax when running qemu
On 2019-07-23 18:51:00, wrote: > Specify the firmware to use via the newer '-drive if=pflash' syntax > which allows specifying the raw format and readonly parameters. This > avoids warnings with newer version of QEMU. > > Signed-off-by: Rebecca Cran > --- > OvmfPkg/build.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh > index a2c4eff2a5..e2890ff762 100755 > --- a/OvmfPkg/build.sh > +++ b/OvmfPkg/build.sh > @@ -268,7 +268,7 @@ if [[ "$RUN_QEMU" == "yes" ]]; then >fi >ln -sf $FV_DIR/OVMF.fd $QEMU_FIRMWARE_DIR/bios.bin >if [[ "$ENABLE_FLASH" == "yes" ]]; then > -QEMU_COMMAND="$QEMU_COMMAND -pflash $QEMU_FIRMWARE_DIR/bios.bin" > +QEMU_COMMAND="$QEMU_COMMAND -drive > if=pflash,format=raw,readonly,file=$QEMU_FIRMWARE_DIR/bios.bin" It looks like we set ENABLE_FLASH for qemu 1.6 and newer. Does this work on 1.6? Also, I don't think we want readonly. Read-write should allow non-volatile variables to persist across multiple boot. -Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44283): https://edk2.groups.io/g/devel/message/44283 Mute This Topic: https://groups.io/mt/32580104/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] OvmfPkg/build.sh: run qemu _after_ building OvmfPkg
On 2019-07-23 18:20:31, wrote: > Running qemu before building the firmware image doesn't make much sense. > Move things so qemu is run after building OvmfPkg. > > Signed-off-by: Rebecca Cran > --- > OvmfPkg/build.sh | 11 +-- > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh > index 1c28e65404..a2c4eff2a5 100755 > --- a/OvmfPkg/build.sh > +++ b/OvmfPkg/build.sh > @@ -256,6 +256,11 @@ else >echo using prebuilt tools > fi > > +# > +# Build the edk2 OvmfPkg > +# > +echo Running edk2 build for OvmfPkg$Processor > +build -p $PLATFORMFILE $BUILD_OPTIONS -b $BUILDTARGET -t $TARGET_TOOLS -n > $THREADNUMBER > > if [[ "$RUN_QEMU" == "yes" ]]; then >if [[ ! -d $QEMU_FIRMWARE_DIR ]]; then > @@ -275,10 +280,4 @@ if [[ "$RUN_QEMU" == "yes" ]]; then >exit $? We exit after running qemu. In other words, if qemu is present on the command line, then we run qemu, but don't try to build OVMF. One use case for running qemu multiple times without building is to preserve non-volatile variables across multiple qemu boots. -Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44282): https://edk2.groups.io/g/devel/message/44282 Mute This Topic: https://groups.io/mt/32579896/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] OvmfPkg: enable multiprocessor builds when using build.sh
On 2019-07-23 00:44:06, Laszlo Ersek wrote: > On 07/23/19 02:00, Rebecca Cran wrote: > > On 2019-07-22 17:14, Jordan Justen wrote: > >> > >> I was suggesting that if they didn't specify -n as a param to > >> build.sh, then build.sh should not send -n to the edk2 build command. > >> The effect would be for the edk2 build command to check > >> Conf/target.txt. By default, I think target.txt will not set > >> THREADNUMBER, so "0" would still be the result. > >> > >> Yet, it would give them the option to set it in Conf/target.txt. > >> Today, since we always use the -n param, target.txt is always ignored > >> for this parameter. > > > > > > On a related topic, I wonder if we should add a "-j" parameter if we > > build BaseTools for users (e.g. "make -j4 -C BaseTools")? I've found > > that it can be pretty slow without it: on my system adding -j4 reduces > > build time from 55 seconds to 15. Going higher doesn't seem to produce > > much more benefit: -j32 (on a ThreadRipper system) reduces it to 12 seconds. > > > > > > Passing > > -j $(getconf _NPROCESSORS_ONLN) > > to "make" (for building BaseTools) makes sense, IMO. I guess the concern might be that we'll be running a bunch of make invocations in parallel, each trying to spawn a compilation for each thread. O(n^2) compilations. :) In the make man-page for -j: "When make invokes a sub-make, all instances of make will coordinate to run the specified number of jobs at a time;", but I'm not sure if that's how `build -n` is implemented. (With make...) Since python writes the makefiles, it could be used instead of getconf, right? What we need is someone to make the ninja-build backend for BaseTools. :) -Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44222): https://edk2.groups.io/g/devel/message/44222 Mute This Topic: https://groups.io/mt/32553200/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] OvmfPkg: enable multiprocessor builds when using build.sh
On 2019-07-22 13:06:03, Laszlo Ersek wrote: > On 07/22/19 09:11, Jordan Justen wrote: > > > It looks like if we tweaked things more, and omitted adding the -n > > parameter to the build command by default, then it would use the > > Conf/target.txt value, which by default appears to also be 0, so this > > could accomplish the same thing, but also let a user set it in > > target.txt. > > I assume that users prefer passing a simple command line parameter to > editing a text file. IOW I believe "THREADNUMBER=0" would be the best > solution. > TL;DR: I agree with you. :) I think they prefer to do neither, and get the same result as "0". :) I was suggesting that if they didn't specify -n as a param to build.sh, then build.sh should not send -n to the edk2 build command. The effect would be for the edk2 build command to check Conf/target.txt. By default, I think target.txt will not set THREADNUMBER, so "0" would still be the result. Yet, it would give them the option to set it in Conf/target.txt. Today, since we always use the -n param, target.txt is always ignored for this parameter. But, personally, I don't think the Conf directory is useful. I'd like to see us deprecate it entirely, or at least make it optional. Therefore, I think the best use of our time is to just set it to 0 as you suggest. :) -Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44178): https://edk2.groups.io/g/devel/message/44178 Mute This Topic: https://groups.io/mt/32553200/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] OvmfPkg: enable multiprocessor builds when using build.sh
Maybe a commit message tweak would be: OvmfPkg/build.sh: enable multithreaded build by default On 2019-07-21 17:58:16, Rebecca Cran wrote: > When building both BaseTools and OvmfPkg, enable multiprocessor builds, > using up to the number of cores available in the system. This can > drastically reduce build times. > For example, on a modern ThreadRipper system the > time required to build decreases from 3 minutes to 1 minute. > > Signed-off-by: Rebecca Cran > --- > OvmfPkg/build.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh > index 4fcbdd2bc9..5d3a672bd2 100755 > --- a/OvmfPkg/build.sh > +++ b/OvmfPkg/build.sh > @@ -40,7 +40,7 @@ ARCH_X64=no > BUILDTARGET=DEBUG > BUILD_OPTIONS= > PLATFORMFILE= > -THREADNUMBER=1 > +THREADNUMBER=$(getconf _NPROCESSORS_ONLN) Based on OvmfPkg/build.sh --help, I think initializing THREADNUMBER to 0 might have the same effect, but not depend on getconf. Does that work? I'm not sure why I defaulted this to single threaded build way back in 578630802e. It looks like if we tweaked things more, and omitted adding the -n parameter to the build command by default, then it would use the Conf/target.txt value, which by default appears to also be 0, so this could accomplish the same thing, but also let a user set it in target.txt. -Jordan > LAST_ARG= > RUN_QEMU=no > ENABLE_FLASH=no > -- > 2.22.0 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44098): https://edk2.groups.io/g/devel/message/44098 Mute This Topic: https://groups.io/mt/32553200/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] EmulatorPkg/Unix: Convert timezone from seconds to minutes
Fixes and assert seen when running ls under the shell. It appears the assert was added in: commit 99849a906e15ea3a9a0330d69bbae0d21ff49808 ShellPkg/ls: Display the file time in local time. Signed-off-by: Jordan Justen Cc: Andrew Fish Cc: Ray Ni --- For reference: https://linux.die.net/man/3/timezone EmulatorPkg/Unix/Host/EmuThunk.c| 2 +- EmulatorPkg/Unix/Host/PosixFileSystem.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/EmulatorPkg/Unix/Host/EmuThunk.c b/EmulatorPkg/Unix/Host/EmuThunk.c index f1330c8234..665477e223 100644 --- a/EmulatorPkg/Unix/Host/EmuThunk.c +++ b/EmulatorPkg/Unix/Host/EmuThunk.c @@ -363,7 +363,7 @@ SecGetTime ( Time->Minute = tm->tm_min; Time->Second = tm->tm_sec; Time->Nanosecond = 0; - Time->TimeZone = timezone; + Time->TimeZone = timezone / 60; Time->Daylight = (daylight ? EFI_TIME_ADJUST_DAYLIGHT : 0) | (tm->tm_isdst > 0 ? EFI_TIME_IN_DAYLIGHT : 0); diff --git a/EmulatorPkg/Unix/Host/PosixFileSystem.c b/EmulatorPkg/Unix/Host/PosixFileSystem.c index 3149c6c3e0..5b74053498 100644 --- a/EmulatorPkg/Unix/Host/PosixFileSystem.c +++ b/EmulatorPkg/Unix/Host/PosixFileSystem.c @@ -220,7 +220,7 @@ PosixSystemTimeToEfiTime ( Time->Second = tm->tm_sec; Time->Nanosecond = 0; - Time->TimeZone = timezone; + Time->TimeZone = timezone / 60; Time->Daylight = (daylight ? EFI_TIME_ADJUST_DAYLIGHT : 0) | (tm->tm_isdst > 0 ? EFI_TIME_IN_DAYLIGHT : 0); } -- 2.22.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43695): https://edk2.groups.io/g/devel/message/43695 Mute This Topic: https://groups.io/mt/32457224/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] EmulatorPkg: update HOST_TOOLS to xcode5
pushed as 55b9bbf40a1cf9788dd6a7b093851076851fc670 On 2019-06-19 09:57:20, Stephano Cetola wrote: > The last compiler flag change was for Xcode 5.0, not Xcode 3.2. As such > the HOST_TOOLS should be set to XCODE5. > > Also, fix a small typo. > > This fixes bug 447: > > https://bugzilla.tianocore.org/show_bug.cgi?id=447 > > Signed-off-by: Stephano Cetola > --- > EmulatorPkg/build.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/EmulatorPkg/build.sh b/EmulatorPkg/build.sh > index 558b65e88b..c5bf0bd655 100755 > --- a/EmulatorPkg/build.sh > +++ b/EmulatorPkg/build.sh > @@ -60,9 +60,9 @@ case `uname` in > CLANG_VER=$(clang -ccc-host-triple x86_64-pc-win32-macho 2>&1 > >/dev/null) || true > if [[ "$CLANG_VER" == *-ccc-host-triple* ]] > then > -# only older versions of Xcode support -ccc-host-tripe, for newer > versions > +# only older versions of Xcode support -ccc-host-triple, for newer > versions > # it is -target > - HOST_TOOLS=XCODE32 > + HOST_TOOLS=XCODE5 >TARGET_TOOLS=XCODE5 > else >HOST_TOOLS=XCODE32 > -- > 2.17.1 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43694): https://edk2.groups.io/g/devel/message/43694 Mute This Topic: https://groups.io/mt/32125628/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] EmulatorPkg/build.sh: Fix missing usage of -b BUILDTARGET parameter
On 2019-04-10 02:07:34, Jordan Justen wrote:
> Signed-off-by: Jordan Justen
> Cc: Andrew Fish
> Cc: Ray Ni
ping
> ---
> EmulatorPkg/build.sh | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/EmulatorPkg/build.sh b/EmulatorPkg/build.sh
> index 89fd26baca..75561c116a 100755
> --- a/EmulatorPkg/build.sh
> +++ b/EmulatorPkg/build.sh
> @@ -1,7 +1,7 @@
> #!/bin/bash
> #
> # Copyright (c) 2008 - 2011, Apple Inc. All rights reserved.
> -# Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.
> +# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> @@ -188,7 +188,7 @@ do
> done
>
> PLATFORMFILE=$WORKSPACE/EmulatorPkg/EmulatorPkg.dsc
> -BUILD_DIR=$BUILD_OUTPUT_DIR/DEBUG_"$TARGET_TOOLS"
> +BUILD_DIR="$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS"
> BUILD_ROOT_ARCH=$BUILD_DIR/$PROCESSOR
>
> if [[ ! -f `which build` || ! -f `which GenFv` ]];
> @@ -218,11 +218,11 @@ if [[ "$RUN_EMULATOR" == "yes" ]]; then
>then
># only older versions of Xcode support -ccc-host-tripe, for newer
> versions
># it is -target
> -cp $WORKSPACE/EmulatorPkg/Unix/lldbefi.py
> $BUILD_OUTPUT_DIR/DEBUG_"$TARGET_TOOLS"/$PROCESSOR
> +cp $WORKSPACE/EmulatorPkg/Unix/lldbefi.py
> "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS/$PROCESSOR"
> cd $BUILD_ROOT_ARCH; /usr/bin/lldb --source
> $WORKSPACE/EmulatorPkg/Unix/lldbinit Host
> exit $?
>else
> -cp $WORKSPACE/EmulatorPkg/Unix/.gdbinit
> $BUILD_OUTPUT_DIR/DEBUG_"$TARGET_TOOLS"/$PROCESSOR
> +cp $WORKSPACE/EmulatorPkg/Unix/.gdbinit
> "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS/$PROCESSOR"
>fi
>;;
>esac
> @@ -255,7 +255,7 @@ if [[ $HOST_TOOLS == $TARGET_TOOLS ]]; then
> else
>build -p $WORKSPACE/EmulatorPkg/EmulatorPkg.dsc $BUILD_OPTIONS -a
> $PROCESSOR -b $BUILDTARGET -t $HOST_TOOLS -D BUILD_$ARCH_SIZE -D
> UNIX_SEC_BUILD -D SKIP_MAIN_BUILD -n 3 modules
>build -p $WORKSPACE/EmulatorPkg/EmulatorPkg.dsc $BUILD_OPTIONS -a
> $PROCESSOR -b $BUILDTARGET -t $TARGET_TOOLS -D BUILD_$ARCH_SIZE
> $NETWORK_SUPPORT $BUILD_NEW_SHELL $BUILD_FAT -n 3
> - cp $BUILD_OUTPUT_DIR/DEBUG_"$HOST_TOOLS"/$PROCESSOR/Host $BUILD_ROOT_ARCH
> + cp "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$HOST_TOOLS/$PROCESSOR/Host"
> $BUILD_ROOT_ARCH
> fi
> exit $?
>
> --
> 2.20.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#43693): https://edk2.groups.io/g/devel/message/43693
Mute This Topic: https://groups.io/mt/31017060/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] EmulatorPkg: update HOST_TOOLS to xcode5
On 2019-06-19 09:57:20, Stephano Cetola wrote: > The last compiler flag change was for Xcode 5.0, not Xcode 3.2. As such > the HOST_TOOLS should be set to XCODE5. > > Also, fix a small typo. > > This fixes bug 447: > > https://bugzilla.tianocore.org/show_bug.cgi?id=447 > > Signed-off-by: Stephano Cetola > --- > EmulatorPkg/build.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/EmulatorPkg/build.sh b/EmulatorPkg/build.sh > index 558b65e88b..c5bf0bd655 100755 > --- a/EmulatorPkg/build.sh > +++ b/EmulatorPkg/build.sh > @@ -60,9 +60,9 @@ case `uname` in > CLANG_VER=$(clang -ccc-host-triple x86_64-pc-win32-macho 2>&1 > >/dev/null) || true > if [[ "$CLANG_VER" == *-ccc-host-triple* ]] > then > -# only older versions of Xcode support -ccc-host-tripe, for newer > versions > +# only older versions of Xcode support -ccc-host-triple, for newer > versions I don't like to include multiple fixes in one patch, but this is a pretty minor typo. > # it is -target > - HOST_TOOLS=XCODE32 > + HOST_TOOLS=XCODE5 Based on the discussion in the bug, this seems right, so: Reviewed-by: Jordan Justen >TARGET_TOOLS=XCODE5 > else >HOST_TOOLS=XCODE32 > -- > 2.17.1 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#42660): https://edk2.groups.io/g/devel/message/42660 Mute This Topic: https://groups.io/mt/32125628/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V3] EmulatorPkg: don't display the cpu current speed
On 2019-06-11 22:42:13, Liu, Zhiguang wrote: > Thanks for the comments. > I will change the commit message in the next version. > But I don't think this is a issue worth making a major change. > Given that the change is consistent with NT32, will you agree with this > change? Hmm. You are right that NT32 also does this. I don't agree enough to give you a Reviewed-by, but if Andrew or Ray give a Reviewed-by, then I'm ok for us to take the patch. -Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#42336): https://edk2.groups.io/g/devel/message/42336 Mute This Topic: https://groups.io/mt/32013654/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 11/10] OvmfPkg/Csm/LegacyBiosDxe: Correct Legacy16GetTableAddress call for E820 data
On 2019-06-12 23:28:12, Wu, Hao A wrote:
>
> > -Original Message-
> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> > David Woodhouse
> > Sent: Thursday, June 13, 2019 5:43 AM
> >
> > The DX register is supposed to contain the required alignment for the
> > allocation. It was zero, and SeaBIOS doesn't (well, didn't) cope well
> > with that. Set it appropriately, and set BX to indicate the regions
> > it's OK to allocate in too. That was already OK but let's make sure
> > it's initialised properly and not just working by chance.
> >
> > Also actually return an error if the allocation fails. Instead of going
> > all the way through into the CSM and just letting it have a bogus
> > pointer to the E82o data.
'E82o' => 'E820'
> >
> > diff --git a/OvmfPkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
> > b/OvmfPkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
> > index 211750c012..e7766eb2b1 100644
> > --- a/OvmfPkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
> > +++ b/OvmfPkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
> > @@ -928,7 +928,9 @@ GenericLegacyBoot (
> >if (CopySize > Private->Legacy16Table->E820Length) {
> > ZeroMem (, sizeof (EFI_IA32_REGISTER_SET));
> > Regs.X.AX = Legacy16GetTableAddress;
> > +Regs.X.BX = (UINT16) 0x3; // Region
>
>
> According to the spec:
>
> '''
> BX = Allocation region
> 00 = Allocate from either 0xE or 0xF 64 KB blocks.
> Bit 0 = 1 Allocate from 0xF 64 KB block
> Bit 1 = 1 Allocate from 0xE 64 KB block
> '''
>
> I think the value 0 for BX is fine which indicates the allocation can
> happen in either ranges. Not sure if setting BX to 0x11 is a valid
> operation.
Based on the spec you quote, it does seem reasonable to expect that a
CSM should treat 0 the same as 3, but it is possible that some CSM out
there made a silly choice and would blow up on 3.
Since David mentioned that bx==0 works ok with SeaBIOS CSM, then maybe
we should just drop this change? Or, we can add a comment that bx==0
means either the e000 or f000 block?
-Jordan
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#42335): https://edk2.groups.io/g/devel/message/42335
Mute This Topic: https://groups.io/mt/32045939/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V3] EmulatorPkg: don't display the cpu current speed
On 2019-06-11 00:32:27, Zhiguang Liu wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1686
>
> V3: I hope that changing the status of the mCpuSmbiosType4
> wouldn't affect other features except showing CPU speed.
> The value is zero in NT32Pkg.
>
> Cc: Jordan Justen
> Cc: Andrew Fish
> Cc: Ray Ni
> Signed-off-by: Zhiguang Liu
> ---
> EmulatorPkg/CpuRuntimeDxe/Cpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/EmulatorPkg/CpuRuntimeDxe/Cpu.c b/EmulatorPkg/CpuRuntimeDxe/Cpu.c
> index 00e93016af..a5e19b4181 100644
> --- a/EmulatorPkg/CpuRuntimeDxe/Cpu.c
> +++ b/EmulatorPkg/CpuRuntimeDxe/Cpu.c
> @@ -104,7 +104,7 @@ SMBIOS_TABLE_TYPE4 mCpuSmbiosType4 = {
>0, // ExternalClock;
>0, // MaxSpeed;
>0, // CurrentSpeed;
> - 0x41, // Status;
> + 0, // Status;
It looks like bit 6 means the process is populated, and bits[2:0]==1
means the CPU is enabled.
So, it looks like this change will make SMBIOS indicate the the
processor socket is not populated, and bit2[2:0]==0 means that the CPU
status is unknown.
I think the commit message for this patch should have been:
===
EmulatorPkg: Change SMBIOS processor to unpopulated
This change updates the SMBIOS processor information to indicate that
the processor is not populated, and that it's status is unknown.
With this change the processor speed will not be shown in setup.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1686
===
But, I'm not sure I agree we should make this change to fix this bug.
I'm not particularly concerned with this bug, but I wonder if perhaps
the MdeModulePkg should just suppress the item if the speed is 0.
Or, alternately, perhaps we can investigate some methods to attempt to
determine the processor speed. I guess for all OS's, it might be
difficult, but we probably could support finding the processor speed
under the most common environments.
-Jordan
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#42184): https://edk2.groups.io/g/devel/message/42184
Mute This Topic: https://groups.io/mt/32013654/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 0/6] Ovmf: Drop IntelFramework[Module]Pkg dependency
On 2019-06-10 18:43:07, Hao A Wu wrote: > The series is also available at: > https://github.com/hwu25/edk2/tree/ovmf_drop_framework_v2 > > V2 changes: > > * Update to module OvmfPkg/Csm/CsmSupportLib/CsmSupportLib.inf has been > dropped, since the module is proposed to be removed in another series: > https://edk2.groups.io/g/devel/message/42142 I see that https://bugzilla.tianocore.org/show_bug.cgi?id=1811 mentions that there is "missing of reviewer/maintainer of the CSM modules in OvmfPkg", so it will be dropped. Yet, I thought David agreed to maintain that support as recently as May 20: https://edk2.groups.io/g/devel/message/41049 Maybe he changed his mind about it? By the way, I know that Scott (Cc'd) was interested in leveraging CSM support of OVMF for a fork that supports the BSD bhyve hypervisor. -Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#42182): https://edk2.groups.io/g/devel/message/42182 Mute This Topic: https://groups.io/mt/32011839/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch 0/7] Add new CLANG8ELF tool chain for new LLVM/CLANG8
On 2019-04-27 17:55:02, Liming Gao wrote: > >-Original Message- > >From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > >Sent: Saturday, April 27, 2019 12:33 AM > > > > > >This series confuses me. The existing CLANGxx toolchains already use > >GenFw and ELF to PE/COFF conversion, so the name CLANG8ELF is > >misleading. > > > LLVM/CLANG8.0 compiler supports to generate PE image or ELF image. > This tool chain is to generate ELF image and be converted to PE > image. I am investigating another tool chain with CLANG8.0 to > directly generate PE image. To differentiate them, I use the tool > chain name CLANG8ELF and CLANG8PE for them. Assuming CLANG8ELF and CLANG8PE were functional, would both be needed? It kind of sounds like this a half-finished investigation. I'm guessing that if CLANG8PE produces equal or better code, then it would be preferred. Therefore, shouldn't we just finish the investigation, and add a single CLANG8 toolchain at the end? Or, maybe to reflect that it mostly uses the LLVM tool stack we could name it LLVM8. -Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#40985): https://edk2.groups.io/g/devel/message/40985 Mute This Topic: https://groups.io/mt/31354044/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [RFC] Remove PI spec TemporaryRamSupport PPI from PEI Core
Short summary: Despite being part of the Platorm Initialization Specificication, this proposal would remove support for the TemporaryRamSupport PPI from PEI Core. Arguably, this would mean the PEI Core does not support the PI spec, but we do not currently know if any platforms require this PPI. Main question: Does anyone know of a platform that requires this PPI, or does anyone have major concerns with removing it before the PI specification can be updated to remove the PPI? Alternatives: Continue to support the PPI, but we would need to merge a bug-fix patchset which adds some assembly code into the PEI Core. More details: I discussed this topic with the Tianocore Stewards, Vincent and Ray. Although the path forward was not unanimous, it appears that the majority thought we should propose to remove support for the TemporaryRamSupport PPI from the PEI Core. This PPI is defined in MdePkg/Include/Ppi/TemporaryRamSupport.h. I believe it has been present in all versions of the Platorm Initialization Specificication, including the latest, version 1.7. (https://uefi.org/specsandtesttools) The TemporaryRamSupport PPI is defined as "optional" in the PI spec, but I believe this only applied to producers of the PPI (PI platforms). The PEI Core (which is the consumer of this PPI) should arguably support the PPI in order to fully support PI spec based platforms. But, there are some subtle issues with the PPI that make it difficult to implement for both the consumer (PEI Core) and the producer (the platform). In fact, there is a bug currently with the PEI Core's implementation, which I sent a patch series to address on April 10th, with the subject of "[PATCH v2 0/6] Fix PEI Core issue during TemporaryRamMigration". Unfortunately the fix is not very simple, and requires adding assembly code to the PEI Core module. Based on this, we discussed if the TemporaryRamSupport PPI is no longer required by current PI platforms, despite being present in the PI spec. The only known example of it being required was a platform based on the IPF architecture, which is no longer supported by EDK II. Several EDK II sample platforms produce this PPI in EDK II, but only as sample code. Clearly the removal would have to take this into account. There are two replacements for the TemporaryRamSupport PPI, but they do not cover all types of potential hardware. If a platform cannot access main memory at the same time as the Temporary RAM, then it would not be supported by the alternatives. Nevertheless, the two replacements are: 1. Stop producing the PPI, and the PEI Core will automatically copy the required memory from Temporary RAM to the main memory. This will work for platforms that do not require special code to shut down Temporary RAM. 2. Produce the MdePkg/Include/Ppi/TemporaryRamDone.h PPI. This will cause the PEI Core to notify the platform after it has copied memory, and the platform can take special actions to disable Temporary RAM mode if required. Since both of these options will cause the PEI Core to do a memory copy from Temporary RAM to the main memory, there might be platforms that they cannot work with as described above. Thanks for your time, -Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#39471): https://edk2.groups.io/g/devel/message/39471 Mute This Topic: https://groups.io/mt/31319621/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 02/10] MdePkg/PiFirmwareFile: fix undefined behavior in SECTION_SIZE
On 2019-04-17 07:59:41, Laszlo Ersek wrote:
> On 04/17/19 13:44, Andrew Fish wrote:
>
> > Sorry I digressed into the C specification discussion, and did not
> > deal with the patch in general. My point is the original code is legal
> > C code. If you lookup CWE-119 it is written as a restriction on what
> > the C language allows.
> >
> > As I mentioned casting to specific alignment is legal, as is defining
> > a structure that is pragma pack(1) that can make a UINT32 not be 4
> > byte aligned. Thus the cast created a legal UINT32 value. A cast to
> > *(UINT32 *) is different that a cast to (UINT32 *). The rules you
> > quote a triggered by the = and not the cast.
> >
> > Thus this is undefined behavior in C:
> > UINT32 *Ub = (UINT32 *)gSection.Sec.Size;
> > Size = *Ub & 0x00ff;
> >
> > And this is not Undefined behavior:
> > UINT32 NotUb = *(UINT32 *)gSection.Sec.Size & 0x00ff;
>
> I agree the 2nd snippet may not be UB due to alignment violations
> *alone*.
>
> It is still UB due to violating the effective type rules.
>
> > I also had a hard time interpreting what C spec was saying, but
> > talking to the people who write the compiler and ubsan cleared it up
> > for me. It also makes sense when you think about it. If you tell the
> > compiler *(UINT32 *) it can know to generate byte reads if the
> > hardware requires aligned access. If you do a (UINT32 *) that new
> > pointer no longer carries the information about the alignment
> > requirement. Thus the *(UINT32 *) cast is like making a packed
> > structure.
>
> Yes, I think I'm clear on how the alignment information is carried
> around, and when it is lost. In your first example above, due to us
> forming a separate (standalone) pointer, we lose the alignment
> information, and then the assignment is undefined due to an alignment
> violation. While in the second example, the alignment information is not
> lost, and the assignment is not undefined on an alignment basis *alone*.
>
> However: the second assignment is *still* undefined, because it violates
> the effective type rules. Here's a more direct example for the same:
>
> STATIC UINT64 mUint64;
>
> int main(void)
> {
> UINT16 *Uint16Ptr;
>
> Uint16Ptr = (UINT16 *)
> *Uint16Ptr = 1;
> return 0;
> }
>
> The assignment to (*Uint16Ptr) is fine from the alignment perspective,
> but it is nevertheless undefined, because it breaks the effective type
> rules. Namely, UINT16 (the type used for the access) is not compatible
> with UINT64 (the effective type of mUint64).
>
> Normally, we don't care about this situation in edk2 -- in fact we write
> loads of code like the above --, but we get away with that only because
> we force the toolchains to ignore the effective type rules. For GCC in
> particular, the option is "-fno-strict-aliasing".
>
> The only reason I've posted this patch is that "cppcheck" (invoked as a
> part of "RH covscan") doesn't care about "-fno-strict-aliasing". And
> while "cppcheck" happens to report the overrun, and not the type
> punning, the way to remove the warning is to adhere to all the C rules
> in the expression, even though we have "-fno-strict-aliasing" in place.
>
> > I agree the union is a good solution to CWE-119 and it better matches
> > the alignment requirement in the PI spec.
>
> Thank you.
>
> I'll wait a bit longer to see if Jordan accepts this 02/10 patch based
> on the most recent comments, and whether Liming or Mike accepts 04/10.
>
> If Jordan remains unconvinced on SECTION_SIZE (in this 02/10 patch), and
> Liming or Mike are fine with 04/10, I can rework 02/10 to follow 04/10.
>
> If Jordan remains unconvinced but Mike or Liming prefers the union, then
> we have a stalemate and I'll abandon the patch set.
I did have a (slight) concern about adding a typedef to a public
header that wasn't in the spec. It seemed like something that someone
somewhere might not like in case it could interfere with future
versions of the spec. According to Liming, we don't have to worry
about that.
Regarding the UINT32* discussion, I didn't think the union really
would make a difference vs skipping the union and casting the original
struct pointer directly to a UINT32*.
I can see Andrew's point that the union may add some alignment
assumptions to the dereference, so I can see how that does potentially
change something subtle. Maybe on some machines it will allow for more
efficient reading of the data with the (valid) alignment assumption.
I was also not seeing why you were saying it produced *undefined*
results. I don't think it does in our case, but when you point out
that we are aliasing data access, I can see how that quickly gets into
*undefined* territory from a compiler's perspective.
Anyway, given Liming's feedback that it is ok to add the union, I'm ok
with this patch.
-Jordan
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#39271):
Re: [edk2-devel] [PATCH 02/10] MdePkg/PiFirmwareFile: fix undefined behavior in SECTION_SIZE
On 2019-04-16 03:59:48, Laszlo Ersek wrote:
> On 04/16/19 11:04, Jordan Justen wrote:
> > On 2019-04-15 09:15:31, Laszlo Ersek wrote:
> >> On 04/14/19 09:19, Jordan Justen wrote:
> >>> On 2019-04-12 16:31:20, Laszlo Ersek wrote:
> >>>> RH covscan justifiedly reports that accessing
> >>>> "EFI_COMMON_SECTION_HEADER.Size", which is of type UINT8[3], through a
> >>>> (UINT32*), is undefined behavior:
> >>>>
> >>>>> Error: OVERRUN (CWE-119):
> >>>>> edk2-89910a39dcfd/OvmfPkg/Sec/SecMain.c:178: overrun-local: Overrunning
> >>>>> array of 3 bytes at byte offset 3 by dereferencing pointer
> >>>>> "(UINT32 *)((EFI_COMMON_SECTION_HEADER *)(UINTN)Section)->Size".
> >>>>> # 176| Section = (EFI_COMMON_SECTION_HEADER*)(UINTN)
> >>>>> CurrentAddress;
> >>>>> # 177|
> >>>>> # 178|-> Size = SECTION_SIZE (Section);
> >>>>> # 179| if (Size < sizeof (*Section)) {
> >>>>> # 180| return EFI_VOLUME_CORRUPTED;
> >>>>
> >>>> Fix this by introducing EFI_COMMON_SECTION_HEADER_UNION, and expressing
> >>>> SECTION_SIZE() in terms of "EFI_COMMON_SECTION_HEADER_UNION.Uint32".
> >>>>
> >>>> Cc: Liming Gao
> >>>> Cc: Michael D Kinney
> >>>> Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1710
> >>>> Issue: scan-1007.txt
> >>>> Signed-off-by: Laszlo Ersek
> >>>> ---
> >>>> MdePkg/Include/Pi/PiFirmwareFile.h | 10 +-
> >>>> 1 file changed, 9 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/MdePkg/Include/Pi/PiFirmwareFile.h
> >>>> b/MdePkg/Include/Pi/PiFirmwareFile.h
> >>>> index a9f3bcc4eb8e..4fce8298d1c0 100644
> >>>> --- a/MdePkg/Include/Pi/PiFirmwareFile.h
> >>>> +++ b/MdePkg/Include/Pi/PiFirmwareFile.h
> >>>> @@ -229,16 +229,24 @@ typedef struct {
> >>>>///
> >>>>UINT8 Size[3];
> >>>>EFI_SECTION_TYPE Type;
> >>>>///
> >>>>/// Declares the section type.
> >>>>///
> >>>> } EFI_COMMON_SECTION_HEADER;
> >>>>
> >>>> +///
> >>>> +/// Union that permits accessing EFI_COMMON_SECTION_HEADER as a UINT32
> >>>> object.
> >>>> +///
> >>>> +typedef union {
> >>>> + EFI_COMMON_SECTION_HEADER Hdr;
> >>>> + UINT32Uint32;
> >>>> +} EFI_COMMON_SECTION_HEADER_UNION;
> >>>> +
> >>>> typedef struct {
> >>>>///
> >>>>/// A 24-bit unsigned integer that contains the total size of the
> >>>> section in bytes,
> >>>>/// including the EFI_COMMON_SECTION_HEADER.
> >>>>///
> >>>>UINT8 Size[3];
> >>>>
> >>>>EFI_SECTION_TYPE Type;
> >>>> @@ -476,17 +484,17 @@ typedef struct {
> >>>>/// A UINT16 that represents a particular build. Subsequent builds
> >>>> have monotonically
> >>>>/// increasing build numbers relative to earlier builds.
> >>>>///
> >>>>UINT16BuildNumber;
> >>>>CHAR16VersionString[1];
> >>>> } EFI_VERSION_SECTION2;
> >>>>
> >>>> #define SECTION_SIZE(SectionHeaderPtr) \
> >>>> -((UINT32) (*((UINT32 *) ((EFI_COMMON_SECTION_HEADER *) (UINTN)
> >>>> SectionHeaderPtr)->Size) & 0x00ff))
> >>>> +(((EFI_COMMON_SECTION_HEADER_UNION *) (UINTN)
> >>>> (SectionHeaderPtr))->Uint32 & 0x00ff)
> >>>
> >>> Mike, all,
> >>>
> >>> Can we add a typedef for EFI_COMMON_SECTION_HEADER_UNION if it's not
> >>> in the PI spec?
> >>>
> >>> If it's not allowed, I think something like this might work too:
> >>>
> >>> #define SECTION_SIZE(SectionHeaderPtr) \
> >>> (*((UINT32*)(UINTN)(SectionHeaderPtr)) & 0x00ff)
> >>
> >> (Less importantly:)
> >>
> >> It might shut up the static analyzer, but regarding the C standard, it's
> >> equally undefined behavior.
> >
Commit Partitioning - Re: [edk2-devel] [edk2] [PATCH V2] UefiPayloadPkg: Enhance UEFI payload for coreboot and Slim Bootloader
On 2019-04-11 08:51:22, Guo Dong wrote: > CorebootModulePkg and CorebootPayloadPkg originally supports coreboot only. > In order to support other bootloaders, such as Slim Bootloader, they need > be updated to be more generic. > UEFI Payload (UefiPayloadPkg) a converged package from CorebootModulePkg > and CorebootPayloadPkg with following updates: > a. Support both coreboot and Slim Bootloader > b. Removed SataControllerDxe and BaseSerialPortLib16550 to use EDK2 modules > c. Support passing bootloader parameter to UEFI payload, e.g. coreboot >table from coreboot or HOB list from Slim Bootloader > d. Using GraphicsOutputDxe from EDK2 with minor change instead of FbGop > e. Remove the dependency to IntelFrameworkPkg and IntelFrameworkModulePkg >and QuarkSocPkg > f. Use BaseDebugLibSerialPort library as DebugLib > g. Use HPET timer, drop legacy 8254 timer support > h. Use BaseXApicX2ApicLib instead of BaseXApicLib > i. Remove HOB gUefiFrameBufferInfoGuid to use EDK2 graphics HOBs. > j. Other clean ups Why this wasn't split into *at least* 10 patches given the 10 major bullet points listed here? https://github.com/tianocore/tianocore.github.io/wiki/Commit-Partitioning > > On how UefiPayloadPkg could work with coreboot/Slim Bootloader, please > refer UefiPayloadPkg/BuildAndIntegrationInstructions.txt > > Once UefiPayloadPkg is checked-in, CorebootModulePkg and CorebootPayloadPkg > could be retired. > > Signed-off-by: Guo Dong > Reviewed-by: Maurice Ma Same question to Maurice. Maybe something to consider in the future. -Jordan > --- > UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c | > 158 > ++ > UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h | > 30 ++ > UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf | > 58 ++ > UefiPayloadPkg/BlSupportPei/BlSupportPei.c | > 566 > ++ > UefiPayloadPkg/BlSupportPei/BlSupportPei.h | > 39 +++ > UefiPayloadPkg/BlSupportPei/BlSupportPei.inf | > 73 + > UefiPayloadPkg/BuildAndIntegrationInstructions.txt | > 82 > ++ > UefiPayloadPkg/GraphicsOutputDxe/ComponentName.c | > 184 > > UefiPayloadPkg/GraphicsOutputDxe/GraphicsOutput.c| > 739 > +++ > UefiPayloadPkg/GraphicsOutputDxe/GraphicsOutput.h| > 53 + > UefiPayloadPkg/GraphicsOutputDxe/GraphicsOutputDxe.inf | > 53 + > UefiPayloadPkg/Include/Coreboot.h| > 249 > + > UefiPayloadPkg/Include/Guid/AcpiBoardInfoGuid.h | > 29
Re: [edk2-devel] [PATCH 08/10] OvmfPkg: suppress "Value stored to ... is never read" analyzer warnings
Value stored to 'Status' is never read > > # Status = gDS->RemoveMemorySpace ( > > # ^ > > edk2-89910a39dcfd/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c:173:3: > > note: Value stored to 'Status' is never read > > # Status = gDS->RemoveMemorySpace ( > > # ^ > > # 171| // Mark flash region as runtime memory > > # 172| // > > # 173|-> Status = gDS->RemoveMemorySpace ( > > # 174| BaseAddress, > > # 175| Length > > > > Error: CLANG_WARNING: > > edk2-89910a39dcfd/OvmfPkg/SataControllerDxe/SataController.c:231:3: > > warning: Value stored to 'DeviceUDmaMode' is never read > > # DeviceUDmaMode = 0; > > # ^~ > > edk2-89910a39dcfd/OvmfPkg/SataControllerDxe/SataController.c:231:3: > > note: Value stored to 'DeviceUDmaMode' is never read > > # DeviceUDmaMode = 0; > > # ^~ > > # 229| UINT16DeviceUDmaMode; > > # 230| > > # 231|-> DeviceUDmaMode = 0; > > # 232| > > # 233| // > > > > Error: CLANG_WARNING: > > edk2-89910a39dcfd/OvmfPkg/VirtioGpuDxe/Gop.c:65:5: warning: Value stored > > to 'Status' is never read > > #Status = VirtioGpuSetScanout ( > > #^~ > > edk2-89910a39dcfd/OvmfPkg/VirtioGpuDxe/Gop.c:65:5: note: Value stored to > > 'Status' is never read > > #Status = VirtioGpuSetScanout ( > > #^~ > > # 63| // by setting ResourceId=0 for it. > > # 64| // > > # 65|-> Status = VirtioGpuSetScanout ( > > # 66| VgpuGop->ParentBus, // VgpuDev > > # 67| 0, 0, 0, 0, // X, Y, Width, Height > > > > Error: CLANG_WARNING: > > edk2-89910a39dcfd/OvmfPkg/VirtioNetDxe/SnpReceive.c:165:3: warning: > > Value stored to 'RxPtr' is never read > > # RxPtr += sizeof (UINT16); > > # ^~~~ > > edk2-89910a39dcfd/OvmfPkg/VirtioNetDxe/SnpReceive.c:165:3: note: Value > > stored to 'RxPtr' is never read > > # RxPtr += sizeof (UINT16); > > # ^~~~ > > # 163| *Protocol = (UINT16) ((RxPtr[0] << 8) | RxPtr[1]); > > # 164| } > > # 165|-> RxPtr += sizeof (UINT16); > > # 166| > > # 167| Status = EFI_SUCCESS; > > Cc: Anthony Perard > Cc: Ard Biesheuvel > Cc: Jordan Justen > Cc: Julien Grall > Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1710 > Issue: scan-0992.txt > Issue: scan-0996.txt > Issue: scan-0997.txt > Issue: scan-0998.txt > Issue: scan-1000.txt > Issue: scan-1001.txt > Issue: scan-1006.txt > Issue: scan-1011.txt > Issue: scan-1012.txt > Signed-off-by: Laszlo Ersek > --- > OvmfPkg/AcpiPlatformDxe/Xen.c | 5 + > OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c | 5 + > OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.c | 4 > OvmfPkg/Library/VirtioLib/VirtioLib.c | 5 + > OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c | 5 + > OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c| 5 + > OvmfPkg/SataControllerDxe/SataController.c| 5 + > OvmfPkg/VirtioGpuDxe/Gop.c| 6 ++ > OvmfPkg/VirtioNetDxe/SnpReceive.c | 5 + > 9 files changed, 45 insertions(+) > > diff --git a/OvmfPkg/AcpiPlatformDxe/Xen.c b/OvmfPkg/AcpiPlatformDxe/Xen.c > index 357c60d23f4e..c8f275a8ee84 100644 > --- a/OvmfPkg/AcpiPlatformDxe/Xen.c > +++ b/OvmfPkg/AcpiPlatformDxe/Xen.c > @@ -144,16 +144,21 @@ InstallXenTables ( >Fadt2Table = NULL; >Fadt1Table = NULL; >Facs2Table = NULL; >Facs1Table = NULL; >DsdtTable = NULL; >TableHandle = 0; >NumberOfTableEntries = 0; > > + // > + // Suppress "Value stored to ... is never read" analyzer warnings. > + // > + (VOID)NumberOfTableEntries; > + I've seen this solution to shut up the compiler for similar reasons, and it kind of bugs me. It looks like both paths of the if & else initialize NumberOfTableEntries, so maybe we can just drop setting it to 0? I looked at FwhInstance too, and it doesn't look like it is used after being set. So, maybe that should just be dropped? I guess both of your bullet points give arguments allowing the unused set. Rather than adding "Suppress..." comments everywhere, maybe a global macro could be defined for similar us
Re: [edk2-devel] [PATCH 02/10] MdePkg/PiFirmwareFile: fix undefined behavior in SECTION_SIZE
On 2019-04-12 16:31:20, Laszlo Ersek wrote:
> RH covscan justifiedly reports that accessing
> "EFI_COMMON_SECTION_HEADER.Size", which is of type UINT8[3], through a
> (UINT32*), is undefined behavior:
>
> > Error: OVERRUN (CWE-119):
> > edk2-89910a39dcfd/OvmfPkg/Sec/SecMain.c:178: overrun-local: Overrunning
> > array of 3 bytes at byte offset 3 by dereferencing pointer
> > "(UINT32 *)((EFI_COMMON_SECTION_HEADER *)(UINTN)Section)->Size".
> > # 176| Section = (EFI_COMMON_SECTION_HEADER*)(UINTN) CurrentAddress;
> > # 177|
> > # 178|-> Size = SECTION_SIZE (Section);
> > # 179| if (Size < sizeof (*Section)) {
> > # 180| return EFI_VOLUME_CORRUPTED;
>
> Fix this by introducing EFI_COMMON_SECTION_HEADER_UNION, and expressing
> SECTION_SIZE() in terms of "EFI_COMMON_SECTION_HEADER_UNION.Uint32".
>
> Cc: Liming Gao
> Cc: Michael D Kinney
> Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1710
> Issue: scan-1007.txt
> Signed-off-by: Laszlo Ersek
> ---
> MdePkg/Include/Pi/PiFirmwareFile.h | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/MdePkg/Include/Pi/PiFirmwareFile.h
> b/MdePkg/Include/Pi/PiFirmwareFile.h
> index a9f3bcc4eb8e..4fce8298d1c0 100644
> --- a/MdePkg/Include/Pi/PiFirmwareFile.h
> +++ b/MdePkg/Include/Pi/PiFirmwareFile.h
> @@ -229,16 +229,24 @@ typedef struct {
>///
>UINT8 Size[3];
>EFI_SECTION_TYPE Type;
>///
>/// Declares the section type.
>///
> } EFI_COMMON_SECTION_HEADER;
>
> +///
> +/// Union that permits accessing EFI_COMMON_SECTION_HEADER as a UINT32
> object.
> +///
> +typedef union {
> + EFI_COMMON_SECTION_HEADER Hdr;
> + UINT32Uint32;
> +} EFI_COMMON_SECTION_HEADER_UNION;
> +
> typedef struct {
>///
>/// A 24-bit unsigned integer that contains the total size of the section
> in bytes,
>/// including the EFI_COMMON_SECTION_HEADER.
>///
>UINT8 Size[3];
>
>EFI_SECTION_TYPE Type;
> @@ -476,17 +484,17 @@ typedef struct {
>/// A UINT16 that represents a particular build. Subsequent builds have
> monotonically
>/// increasing build numbers relative to earlier builds.
>///
>UINT16BuildNumber;
>CHAR16VersionString[1];
> } EFI_VERSION_SECTION2;
>
> #define SECTION_SIZE(SectionHeaderPtr) \
> -((UINT32) (*((UINT32 *) ((EFI_COMMON_SECTION_HEADER *) (UINTN)
> SectionHeaderPtr)->Size) & 0x00ff))
> +(((EFI_COMMON_SECTION_HEADER_UNION *) (UINTN)
> (SectionHeaderPtr))->Uint32 & 0x00ff)
Mike, all,
Can we add a typedef for EFI_COMMON_SECTION_HEADER_UNION if it's not
in the PI spec?
If it's not allowed, I think something like this might work too:
#define SECTION_SIZE(SectionHeaderPtr) \
(*((UINT32*)(UINTN)(SectionHeaderPtr)) & 0x00ff)
Then again, I see SECTION_SIZE is not in the spec, so maybe it's ok to
add the typedef.
-Jordan
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#39002): https://edk2.groups.io/g/devel/message/39002
Mute This Topic: https://groups.io/mt/31070302/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/5] OvmfPkg/build.sh: Require QEMU 1.6 or newer and always enable flash
On 2019-04-10 07:16:34, Laszlo Ersek wrote: > Hi Jordan, > > I've asked Phil (CC'd) to review this series in my stead. I'll be happy > to provide an ACK when Phil gives his R-b (if I should forget, please > ping me separately). > > In the future, please include a cover letter; a cumulative diffstat > usually helps with the review. (I've had to check all patches to see > that only build.sh is being modified.) I did create a cover letter, but then I deleted it. I thought I was making too much out of it... >From what I gather, you don't find this script useful, and therefore I sometimes wonder if I'm the only one that uses it. :) -Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#38836): https://edk2.groups.io/g/devel/message/38836 Mute This Topic: https://groups.io/mt/31017199/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 3/5] OvmfPkg/build.sh: Move automatic TARGET_TOOLS detection later
On 2019-04-10 08:37:22, Philippe Mathieu-Daudé wrote: > Hi Jordan, > > On 4/10/19 11:34 AM, Jordan Justen wrote: > > If we are building for a non-native ARM, then we may need to select a > > cross-compiler based on the -a paramter. > > I am sorry but I am non-native English and I have hard time to > understand your comment. > > OVMF targets x86 hardware, which is obviously not ARM. >From OvmfPkg/README: "The Open Virtual Machine Firmware (OVMF) project aims to support firmware for Virtual Machines using the edk2 code base." This doesn't appear to indicate any particular architecture or VMM. Nevertheless, ArmVirtPkg was created, so in effect, you are correct. > And I'm not aware of another target architecture. Are you talking > about the host where you build? I mean the target. The patch looks for something like "-a AARCH64" on the command line. If found, we try to build ArmVirtPkg/ArmVirtQemu.dsc instead of OVMF. It's a bit of a hack in that it will try to build the dsc from another package. But, OvmfPkg/build.sh is a little more generic than one might expect. For example, when I want to build MdeModulePkg, I usually just run: OvmfPkg/build.sh -p MdeModulePkg/MdeModulePkg.dsc I find it convenient that the script sets up the build environment automatically, and after the script is done, no changes persist in my shell's environment. > I am trying your series on a Aarch64 host but I fail at passing the > correct cross gcc. I set GCC5_IA32_PREFIX, GCC5_X64_PREFIX and GCC5_BIN > env vars, but the build script still run '"gcc"' instead of my cross one. > > A more generic comment regarding your series: how do you use this script? Yeah, you are right. The comments could be better. I'll try to improve them. Basically, these changes let me build ArmVirtPkg/ArmVirtQemu.dsc on x86_64 Linux by simply added "-a AARCH64" or "-a ARM" parameteres to this script. It also helped setup the qemu command line. Here are the 4 commands I used on x86_64 Linux to build and run ArmVirtPkg: $ OvmfPkg/build.sh -a AARCH64 $ OvmfPkg/build.sh -a AARCH64 qemu -m 1024 -cpu cortex-a57 -M virt -net none $ OvmfPkg/build.sh -a ARM $ OvmfPkg/build.sh -a ARM qemu -m 1024 -cpu cortex-a15 -M virt -net none I think the script might be usable on an ARM based build systems as well, but it might require further tweaks. Ideally, if the build system was AARCH64 based, then: * OvmfPkg/build.sh: (with no params) would cause AARCH64 ArmVirtPkg to be built using a native compiler. * OvmfPkg/build.sh -a ARM: would cause ARM ArmVirtPkg to be built using a cross compiler. * OvmfPkg/build.sh -a X64: would cause X64 OVMF to be built using a cross compiler. -Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#38835): https://edk2.groups.io/g/devel/message/38835 Mute This Topic: https://groups.io/mt/31017201/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 0/6] Fix PEI Core issue during TemporaryRamMigration
On 2019-04-10 09:41:43, Ard Biesheuvel wrote: > On Wed, 10 Apr 2019 at 01:41, Jordan Justen wrote: > > > > https://github.com/jljusten/edk2.git temp-ram-support-v2 > > > > https://github.com/jljusten/edk2/commits/temp-ram-support-v2 > > > > v2: > > * Add AARCH64 and ARM assembly > > Hi Jordan, > > I'm not sure I'm following the reasoning behind this. Did you see the explanation in patch 1 commit message? Could you reply there with questions, or maybe I should try to expand on that? > Does this fix an issue we currently have on ARM systems? Yes, but I don't know of a case where it has been observed on AARCH64/ARM. Nevertheless, as far as I can tell, a similar issue could happen because the current implementation relies on non-spec'd behavior of code gen within a C function. (Not the C calling convention, but what code does with inside the function between calls.) > And how did you build and/or test OVMF for ARM? I built ArmVirtPkg for AARCH64 and ARM on x86_64 Linux with a cross-compiler, and ran it with qemu. -Jordan > > > * Drop IA32 and X64 .S source files > > * Adjust PEI_CORE_TEMPORARY_RAM_TRANSITION pointer in the assembly > >code based on the stack pointer change before & after > >TemporaryRamSupport->TemporaryRamMigration > > * Drop extra cleanup patches for OvmfPkg & EmulatorPkg. These were > >just complicating the series. > > > > This series fixes an issue that, while rare, is possible based on the > > way the TemporaryRamSupport PPI is defined along with how it is used > > by the PEI Core. > > > > Liu Yu reported a boot issue with EmulatorPkg, which I believe was > > caused by this issue. > > > > The details of the issue are described in the commit message of the > > "MdeModulePkg/Core/Pei: Add interface for assembly based > > TemporaryRamSupport" patch. > > > > Testing: > > > > I tested building and booting in several scenarios: > > > > * OVMF IA32 & X64 on Linux > > * ArmVirtPkg AARCH64 & ARM on x86_64 Linux > > * EmulatorPkg IA32 & X64 on Linux > > > > Untested: > > > > * My system does not reproduce the issue that Liu Yu reported with > > EmulatorPkg, so I can't say that I have verified that issue. > > * Building on windows > > * AARCH64/ARM TemporaryRamMigration.asm sources -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#38834): https://edk2.groups.io/g/devel/message/38834 Mute This Topic: https://groups.io/mt/31016921/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 2/5] OvmfPkg/build.sh: Don't automatically -hda qemu parameter
This hasn't worked for me in quite some time. I always add a -cdrom paramter, which causes build.sh to skip this. Signed-off-by: Jordan Justen --- OvmfPkg/build.sh | 3 --- 1 file changed, 3 deletions(-) diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh index c92e90acfa..217abae683 100755 --- a/OvmfPkg/build.sh +++ b/OvmfPkg/build.sh @@ -262,9 +262,6 @@ fi if [[ "$RUN_QEMU" == "yes" ]]; then FIRMWARE_IMAGE=$FV_DIR/OVMF.fd QEMU_COMMAND="$QEMU_COMMAND -pflash $FIRMWARE_IMAGE" - if [[ "$ADD_QEMU_HDA" == "yes" ]]; then -QEMU_COMMAND="$QEMU_COMMAND -hda fat:$BUILD_ROOT_ARCH" - fi echo Running: $QEMU_COMMAND "$@" $QEMU_COMMAND "$@" exit $? -- 2.20.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#38798): https://edk2.groups.io/g/devel/message/38798 Mute This Topic: https://groups.io/mt/31017200/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] OvmfPkg/build.sh: Add AARCH64/ARM build and qemu support
Signed-off-by: Jordan Justen --- OvmfPkg/build.sh | 72 1 file changed, 60 insertions(+), 12 deletions(-) diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh index 812441b9b1..9d787452b5 100755 --- a/OvmfPkg/build.sh +++ b/OvmfPkg/build.sh @@ -35,6 +35,8 @@ fi # Configure defaults for various options # +ARCH_AARCH64=no +ARCH_ARM=no ARCH_IA32=no ARCH_X64=no BUILDTARGET=DEBUG @@ -71,12 +73,14 @@ do else case $LAST_ARG in -a) -if [[ x"$arg" != x"IA32" && x"$arg" != x"X64" ]]; then - echo Unsupported processor architecture: $arg - echo Only IA32 or X64 is supported - exit 1 -fi -eval ARCH_$arg=yes +case $arg in + AARCH64|ARM|IA32|X64) +eval ARCH_$arg=yes +;; + *) +echo Unsupported processor architecture: $arg +exit 1 + esac ;; -b) BUILDTARGET=$arg @@ -99,7 +103,22 @@ do shift done -if [[ "$ARCH_IA32" == "yes" && "$ARCH_X64" == "yes" ]]; then +export GCC5_AARCH64_PREFIX=aarch64-linux-gnu- +export GCC5_ARM_PREFIX=arm-linux-gnueabihf- + +if [[ "$ARCH_AARCH64" == "yes" ]]; then + PROCESSOR=AARCH64 + Processor=AArch64 + BUILD_OPTIONS="$BUILD_OPTIONS -a $PROCESSOR" + PLATFORM_BUILD_DIR=ArmVirtQemu-$PROCESSOR + BUILD_ROOT_ARCH=$PROCESSOR +elif [[ "$ARCH_ARM" == "yes" ]]; then + PROCESSOR=ARM + Processor=Arm + BUILD_OPTIONS="$BUILD_OPTIONS -a $PROCESSOR" + PLATFORM_BUILD_DIR=ArmVirtQemu-$PROCESSOR + BUILD_ROOT_ARCH=$PROCESSOR +elif [[ "$ARCH_IA32" == "yes" && "$ARCH_X64" == "yes" ]]; then PROCESSOR=IA32X64 Processor=Ia32X64 BUILD_OPTIONS="$BUILD_OPTIONS -a IA32 -a X64" @@ -170,6 +189,22 @@ if [ -z "$TARGET_TOOLS" ]; then fi case $PROCESSOR in + AARCH64) +if [ -z "$QEMU_COMMAND" ]; then + # + # The user didn't set the QEMU_COMMAND variable. + # + QEMU_COMMAND=qemu-system-aarch64 +fi +;; + ARM) +if [ -z "$QEMU_COMMAND" ]; then + # + # The user didn't set the QEMU_COMMAND variable. + # + QEMU_COMMAND=qemu-system-aarch64 +fi +;; IA32) if [ -n "$QEMU_COMMAND" ]; then # @@ -203,7 +238,14 @@ case $PROCESSOR in esac if [ -z "$PLATFORMFILE" ]; then - PLATFORMFILE=$WORKSPACE/OvmfPkg/OvmfPkg$Processor.dsc + case $PROCESSOR in +AARCH64|ARM) + PLATFORMFILE=$WORKSPACE/ArmVirtPkg/ArmVirtQemu.dsc + ;; +*) + PLATFORMFILE=$WORKSPACE/OvmfPkg/OvmfPkg$Processor.dsc + ;; + esac fi if [[ "$RUN_QEMU" == "yes" ]]; then @@ -263,8 +305,15 @@ fi if [[ "$RUN_QEMU" == "yes" ]]; then - FIRMWARE_IMAGE=$FV_DIR/OVMF.fd - QEMU_COMMAND="$QEMU_COMMAND -pflash $FIRMWARE_IMAGE" + case $PROCESSOR in +AARCH64|ARM) + FIRMWARE_IMAGE="-bios $FV_DIR/QEMU_EFI.fd" + ;; +*) + FIRMWARE_IMAGE="-pflash $FV_DIR/OVMF.fd" + ;; + esac + QEMU_COMMAND="$QEMU_COMMAND $FIRMWARE_IMAGE" echo Running: $QEMU_COMMAND "$@" $QEMU_COMMAND "$@" exit $? @@ -273,7 +322,6 @@ fi # # Build the edk2 OvmfPkg # -echo Running edk2 build for OvmfPkg$Processor +echo Running edk2 build for $PROCESSOR $PLATFORMFILE build -p $PLATFORMFILE $BUILD_OPTIONS -b $BUILDTARGET -t $TARGET_TOOLS -n $THREADNUMBER exit $? - -- 2.20.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#38800): https://edk2.groups.io/g/devel/message/38800 Mute This Topic: https://groups.io/mt/31017202/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 1/5] OvmfPkg/build.sh: Require QEMU 1.6 or newer and always enable flash
Signed-off-by: Jordan Justen
---
OvmfPkg/build.sh | 19 +++
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh
index 4fcbdd2bc9..c92e90acfa 100755
--- a/OvmfPkg/build.sh
+++ b/OvmfPkg/build.sh
@@ -43,7 +43,6 @@ PLATFORMFILE=
THREADNUMBER=1
LAST_ARG=
RUN_QEMU=no
-ENABLE_FLASH=no
#
# Pick a default tool type for a given OS
@@ -110,7 +109,7 @@ do
break
;;
--enable-flash)
-ENABLE_FLASH=yes
+# Ignore old option. We always enable flash.
;;
*)
BUILD_OPTIONS="$BUILD_OPTIONS $arg"
@@ -210,7 +209,10 @@ if [[ "$RUN_QEMU" == "yes" ]]; then
awk '{print $2}')
case $qemu_version in
1.[6-9].*|[2-9].*.*|[1-9][0-9]*.*.*)
- ENABLE_FLASH=yes
+ ;;
+*)
+ echo qemu 1.6 or newer is required. detected: $qemu_version
+ exit 1
;;
esac
@@ -258,15 +260,8 @@ fi
if [[ "$RUN_QEMU" == "yes" ]]; then
- if [[ ! -d $QEMU_FIRMWARE_DIR ]]; then
-mkdir $QEMU_FIRMWARE_DIR
- fi
- ln -sf $FV_DIR/OVMF.fd $QEMU_FIRMWARE_DIR/bios.bin
- if [[ "$ENABLE_FLASH" == "yes" ]]; then
-QEMU_COMMAND="$QEMU_COMMAND -pflash $QEMU_FIRMWARE_DIR/bios.bin"
- else
-QEMU_COMMAND="$QEMU_COMMAND -L $QEMU_FIRMWARE_DIR"
- fi
+ FIRMWARE_IMAGE=$FV_DIR/OVMF.fd
+ QEMU_COMMAND="$QEMU_COMMAND -pflash $FIRMWARE_IMAGE"
if [[ "$ADD_QEMU_HDA" == "yes" ]]; then
QEMU_COMMAND="$QEMU_COMMAND -hda fat:$BUILD_ROOT_ARCH"
fi
--
2.20.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#38797): https://edk2.groups.io/g/devel/message/38797
Mute This Topic: https://groups.io/mt/31017199/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 3/5] OvmfPkg/build.sh: Move automatic TARGET_TOOLS detection later
If we are building for a non-native ARM, then we may need to select a
cross-compiler based on the -a paramter.
Signed-off-by: Jordan Justen
---
OvmfPkg/build.sh | 99 +---
1 file changed, 51 insertions(+), 48 deletions(-)
diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh
index 217abae683..812441b9b1 100755
--- a/OvmfPkg/build.sh
+++ b/OvmfPkg/build.sh
@@ -40,58 +40,11 @@ ARCH_X64=no
BUILDTARGET=DEBUG
BUILD_OPTIONS=
PLATFORMFILE=
+TARGET_TOOLS=
THREADNUMBER=1
LAST_ARG=
RUN_QEMU=no
-#
-# Pick a default tool type for a given OS
-#
-TARGET_TOOLS=MYTOOLS
-case `uname` in
- CYGWIN*)
-echo Cygwin not fully supported yet.
-;;
- Darwin*)
-Major=$(uname -r | cut -f 1 -d '.')
-# Major is Darwin version, not OS X version.
-# OS X Yosemite 10.10.2 returns 14.
-case $Major in
- [156789])
-echo OvmfPkg requires OS X Snow Leopard 10.6 or newer OS
-exit 1
-;;
- 10)
-TARGET_TOOLS=XCODE32
-;;
- 1[12])
-TARGET_TOOLS=XCLANG
-;;
- *)
-# Mavericks and future assume XCODE5 (clang + lldb)
-TARGET_TOOLS=XCODE5
-;;
-esac
-;;
- Linux*)
-gcc_version=$(gcc -v 2>&1 | tail -1 | awk '{print $3}')
-case $gcc_version in
- [1-3].*|4.[0-7].*)
-echo OvmfPkg requires GCC4.8 or later
-exit 1
-;;
- 4.8.*)
-TARGET_TOOLS=GCC48
-;;
- 4.9.*|6.[0-2].*)
-TARGET_TOOLS=GCC49
-;;
- *)
-TARGET_TOOLS=GCC5
-;;
-esac
-esac
-
#
# Scan command line to override defaults
#
@@ -166,6 +119,56 @@ else
BUILD_ROOT_ARCH=X64
fi
+#
+# Pick a default tool type for a given OS
+#
+if [ -z "$TARGET_TOOLS" ]; then
+ TARGET_TOOLS=MYTOOLS
+ case `uname` in
+CYGWIN*)
+ echo Cygwin not fully supported yet.
+ ;;
+Darwin*)
+ Major=$(uname -r | cut -f 1 -d '.')
+ # Major is Darwin version, not OS X version.
+ # OS X Yosemite 10.10.2 returns 14.
+ case $Major in
+[156789])
+ echo OvmfPkg requires OS X Snow Leopard 10.6 or newer OS
+ exit 1
+ ;;
+10)
+ TARGET_TOOLS=XCODE32
+ ;;
+1[12])
+ TARGET_TOOLS=XCLANG
+ ;;
+ *)
+ # Mavericks and future assume XCODE5 (clang + lldb)
+ TARGET_TOOLS=XCODE5
+ ;;
+ esac
+ ;;
+Linux*)
+ gcc_version=$(gcc -v 2>&1 | tail -1 | awk '{print $3}')
+ case $gcc_version in
+[1-3].*|4.[0-7].*)
+ echo OvmfPkg requires GCC4.8 or later
+ exit 1
+ ;;
+4.8.*)
+ TARGET_TOOLS=GCC48
+ ;;
+4.9.*|6.[0-2].*)
+ TARGET_TOOLS=GCC49
+ ;;
+*)
+ TARGET_TOOLS=GCC5
+ ;;
+ esac
+ esac
+fi
+
case $PROCESSOR in
IA32)
if [ -n "$QEMU_COMMAND" ]; then
--
2.20.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#38799): https://edk2.groups.io/g/devel/message/38799
Mute This Topic: https://groups.io/mt/31017201/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] OvmfPkg/build.sh: Add a cross compiler prefix for AARCH64/ARM
Signed-off-by: Jordan Justen
---
OvmfPkg/build.sh | 26 ++
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh
index 9d787452b5..e1a708942b 100755
--- a/OvmfPkg/build.sh
+++ b/OvmfPkg/build.sh
@@ -103,9 +103,6 @@ do
shift
done
-export GCC5_AARCH64_PREFIX=aarch64-linux-gnu-
-export GCC5_ARM_PREFIX=arm-linux-gnueabihf-
-
if [[ "$ARCH_AARCH64" == "yes" ]]; then
PROCESSOR=AARCH64
Processor=AArch64
@@ -138,6 +135,23 @@ else
BUILD_ROOT_ARCH=X64
fi
+GCC_CROSS_PREFIX=
+gcc_machine=$(gcc -dumpmachine 2>&1 | awk -F - '{print $1}')
+case $gcc_machine in
+ x86_64)
+case $BUILD_ROOT_ARCH in
+ AARCH64)
+ GCC_CROSS_PREFIX=aarch64-linux-gnu-
+;;
+ ARM)
+ GCC_CROSS_PREFIX=arm-linux-gnueabihf-
+;;
+esac
+;;
+esac
+
+GCC=${GCC_CROSS_PREFIX}gcc
+
#
# Pick a default tool type for a given OS
#
@@ -169,7 +183,7 @@ if [ -z "$TARGET_TOOLS" ]; then
esac
;;
Linux*)
- gcc_version=$(gcc -v 2>&1 | tail -1 | awk '{print $3}')
+ gcc_version=$($GCC -v 2>&1 | tail -1 | awk '{print $3}')
case $gcc_version in
[1-3].*|4.[0-7].*)
echo OvmfPkg requires GCC4.8 or later
@@ -188,6 +202,10 @@ if [ -z "$TARGET_TOOLS" ]; then
esac
fi
+if [ -n "${GCC_CROSS_PREFIX}" ]; then
+ export ${TARGET_TOOLS}_${BUILD_ROOT_ARCH}_PREFIX=${GCC_CROSS_PREFIX}
+fi
+
case $PROCESSOR in
AARCH64)
if [ -z "$QEMU_COMMAND" ]; then
--
2.20.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#38801): https://edk2.groups.io/g/devel/message/38801
Mute This Topic: https://groups.io/mt/31017203/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] EmulatorPkg/build.sh: Fix missing usage of -b BUILDTARGET parameter
Signed-off-by: Jordan Justen
Cc: Andrew Fish
Cc: Ray Ni
---
EmulatorPkg/build.sh | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/EmulatorPkg/build.sh b/EmulatorPkg/build.sh
index 89fd26baca..75561c116a 100755
--- a/EmulatorPkg/build.sh
+++ b/EmulatorPkg/build.sh
@@ -1,7 +1,7 @@
#!/bin/bash
#
# Copyright (c) 2008 - 2011, Apple Inc. All rights reserved.
-# Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.
+# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -188,7 +188,7 @@ do
done
PLATFORMFILE=$WORKSPACE/EmulatorPkg/EmulatorPkg.dsc
-BUILD_DIR=$BUILD_OUTPUT_DIR/DEBUG_"$TARGET_TOOLS"
+BUILD_DIR="$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS"
BUILD_ROOT_ARCH=$BUILD_DIR/$PROCESSOR
if [[ ! -f `which build` || ! -f `which GenFv` ]];
@@ -218,11 +218,11 @@ if [[ "$RUN_EMULATOR" == "yes" ]]; then
then
# only older versions of Xcode support -ccc-host-tripe, for newer
versions
# it is -target
-cp $WORKSPACE/EmulatorPkg/Unix/lldbefi.py
$BUILD_OUTPUT_DIR/DEBUG_"$TARGET_TOOLS"/$PROCESSOR
+cp $WORKSPACE/EmulatorPkg/Unix/lldbefi.py
"$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS/$PROCESSOR"
cd $BUILD_ROOT_ARCH; /usr/bin/lldb --source
$WORKSPACE/EmulatorPkg/Unix/lldbinit Host
exit $?
else
-cp $WORKSPACE/EmulatorPkg/Unix/.gdbinit
$BUILD_OUTPUT_DIR/DEBUG_"$TARGET_TOOLS"/$PROCESSOR
+cp $WORKSPACE/EmulatorPkg/Unix/.gdbinit
"$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS/$PROCESSOR"
fi
;;
esac
@@ -255,7 +255,7 @@ if [[ $HOST_TOOLS == $TARGET_TOOLS ]]; then
else
build -p $WORKSPACE/EmulatorPkg/EmulatorPkg.dsc $BUILD_OPTIONS -a $PROCESSOR
-b $BUILDTARGET -t $HOST_TOOLS -D BUILD_$ARCH_SIZE -D UNIX_SEC_BUILD -D
SKIP_MAIN_BUILD -n 3 modules
build -p $WORKSPACE/EmulatorPkg/EmulatorPkg.dsc $BUILD_OPTIONS -a $PROCESSOR
-b $BUILDTARGET -t $TARGET_TOOLS -D BUILD_$ARCH_SIZE $NETWORK_SUPPORT
$BUILD_NEW_SHELL $BUILD_FAT -n 3
- cp $BUILD_OUTPUT_DIR/DEBUG_"$HOST_TOOLS"/$PROCESSOR/Host $BUILD_ROOT_ARCH
+ cp "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$HOST_TOOLS/$PROCESSOR/Host"
$BUILD_ROOT_ARCH
fi
exit $?
--
2.20.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#38794): https://edk2.groups.io/g/devel/message/38794
Mute This Topic: https://groups.io/mt/31017060/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2 5/6] MdeModulePkg/Core/Pei: Add X64 assembly for TemporaryRamMigration
Some compilers may optimize register usage in ways that are incompatible with the TemporaryRamSupport PPI. Using assembly code to call the TemporaryRamMigration function prevents this issue. Signed-off-by: Jordan Justen Cc: Jian J Wang Cc: Hao Wu Cc: Ray Ni Cc: Star Zeng --- .../Dispatcher/X64/TemporaryRamMigration.nasm | 74 +++ MdeModulePkg/Core/Pei/PeiMain.inf | 3 + 2 files changed, 77 insertions(+) create mode 100644 MdeModulePkg/Core/Pei/Dispatcher/X64/TemporaryRamMigration.nasm diff --git a/MdeModulePkg/Core/Pei/Dispatcher/X64/TemporaryRamMigration.nasm b/MdeModulePkg/Core/Pei/Dispatcher/X64/TemporaryRamMigration.nasm new file mode 100644 index 00..5de8acdf34 --- /dev/null +++ b/MdeModulePkg/Core/Pei/Dispatcher/X64/TemporaryRamMigration.nasm @@ -0,0 +1,74 @@ +;-- +; +; Copyright (c) 2018, Intel Corporation. All rights reserved. +; SPDX-License-Identifier: BSD-2-Clause-Patent +; +;-- + +#include + +SECTION .text + +extern ASM_PFX(PeiTemporaryRamMigrated) + +;-- +; VOID +; EFIAPI +; PeiTemporaryRamMigration ( +; IN PEI_CORE_TEMPORARY_RAM_TRANSITION *TempRamTransitionData +; ); +; +; @param[in] RCX Pointer to PEI_CORE_TEMPORARY_RAM_TRANSITION +; +; @return None This routine does not return +;-- +global ASM_PFX(PeiTemporaryRamMigration) +ASM_PFX(PeiTemporaryRamMigration): + +; +; We never return from this call +; +add rsp, 8 + +; +; (rax) Pointer to PEI_CORE_TEMPORARY_RAM_TRANSITION +; +mov rax, rcx + +; +; We store the TempRamTransitionData pointer in rbx. By the X64 +; calling convention we should normally save rbx, but we won't be +; returning to the caller, so we don't need to save it. By the +; same rule, the TemporaryRamMigration PPI call should preserve +; rbx for us. +; +mov rbx, rcx + +; +; Setup parameters and call TemporaryRamSupport->TemporaryRamMigration +; (rcx) PeiServices +; (rdx) TemporaryMemoryBase +; (r8) PermanentMemoryBase +; (r9) CopySize +; +mov rcx, [rax + 0x08] +mov rdx, [rax + 0x10] +mov r8, [rax + 0x18] +mov r9, [rax + 0x20] + +; +; (rbx) Pointer to PEI_CORE_TEMPORARY_RAM_TRANSITION on stack +; +; Adjusted based on stack change made during +; TemporaryRamSupport->TemporaryRamMigration call +; +sub rbx, rsp +call[rax + 0x00] +add rbx, rsp + +; +; Setup parameters and call PeiTemporaryRamMigrated +; (rcx) Pointer to PEI_CORE_TEMPORARY_RAM_TRANSITION +; +mov rcx, rbx +callASM_PFX(PeiTemporaryRamMigrated) diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf b/MdeModulePkg/Core/Pei/PeiMain.inf index 918c4a0df8..fb643a9dd5 100644 --- a/MdeModulePkg/Core/Pei/PeiMain.inf +++ b/MdeModulePkg/Core/Pei/PeiMain.inf @@ -59,6 +59,9 @@ [Sources.Ia32] Dispatcher/Ia32/TemporaryRamMigration.nasm +[Sources.X64] + Dispatcher/X64/TemporaryRamMigration.nasm + [Packages] MdePkg/MdePkg.dec MdeModulePkg/MdeModulePkg.dec -- 2.20.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#38791): https://edk2.groups.io/g/devel/message/38791 Mute This Topic: https://groups.io/mt/31016933/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/6] MdeModulePkg/Core/Pei: Add AARCH64 assembly for TemporaryRamMigration
Signed-off-by: Jordan Justen Cc: Leif Lindholm Cc: Ard Biesheuvel --- .../AArch64/TemporaryRamMigration.S | 63 +++ .../AArch64/TemporaryRamMigration.asm | 63 +++ MdeModulePkg/Core/Pei/PeiMain.inf | 4 ++ 3 files changed, 130 insertions(+) create mode 100644 MdeModulePkg/Core/Pei/Dispatcher/AArch64/TemporaryRamMigration.S create mode 100644 MdeModulePkg/Core/Pei/Dispatcher/AArch64/TemporaryRamMigration.asm diff --git a/MdeModulePkg/Core/Pei/Dispatcher/AArch64/TemporaryRamMigration.S b/MdeModulePkg/Core/Pei/Dispatcher/AArch64/TemporaryRamMigration.S new file mode 100644 index 00..e73932daff --- /dev/null +++ b/MdeModulePkg/Core/Pei/Dispatcher/AArch64/TemporaryRamMigration.S @@ -0,0 +1,63 @@ +#-- +# +# Copyright (c) 2019, Intel Corporation. All rights reserved. +# SPDX-License-Identifier: BSD-2-Clause-Patent +# +#-- + +.text +.align 5 + +GCC_ASM_EXPORT(PeiTemporaryRamMigration) + +#-- +# VOID +# EFIAPI +# PeiTemporaryRamMigration ( +# IN PEI_CORE_TEMPORARY_RAM_TRANSITION *TempRamTransitionData +# ); +# +# @param[in] x0Pointer to PEI_CORE_TEMPORARY_RAM_TRANSITION +# +# @return None This routine does not return +#-- +ASM_PFX(PeiTemporaryRamMigration): + +# +# We store the TempRamTransitionData pointer in x19. By the +# AArch64 calling convention we should normally save x19, but we +# won't be returning to the caller, so we don't need to save it. +# By the same rule, the TemporaryRamMigration PPI call should +# preserve x19 for us. +# +mov x19, x0 + +# +# Setup parameters and call TemporaryRamSupport->TemporaryRamMigration +# (rcx) PeiServices +# (rdx) TemporaryMemoryBase +# (r8) PermanentMemoryBase +# (r9) CopySize +# +ldp x0, x1, [x19, 0x08] +ldp x2, x3, [x19, 0x18] + +# +# (x19) Pointer to PEI_CORE_TEMPORARY_RAM_TRANSITION on stack +# +# Adjusted based on stack change made during +# TemporaryRamSupport->TemporaryRamMigration call +# +ldr x4, [x19] +mov x5, sp +sub x19, x19, x5 +blr x4 +mov x5, sp +add x19, x19, x5 + +# +# Setup parameters and call PeiTemporaryRamMigrated +# (x0) Pointer to PEI_CORE_TEMPORARY_RAM_TRANSITION +# +mov x0, x19 +bl ASM_PFX(PeiTemporaryRamMigrated) diff --git a/MdeModulePkg/Core/Pei/Dispatcher/AArch64/TemporaryRamMigration.asm b/MdeModulePkg/Core/Pei/Dispatcher/AArch64/TemporaryRamMigration.asm new file mode 100644 index 00..fbffd16c52 --- /dev/null +++ b/MdeModulePkg/Core/Pei/Dispatcher/AArch64/TemporaryRamMigration.asm @@ -0,0 +1,63 @@ +;-- +; +; Copyright (c) 2019, Intel Corporation. All rights reserved. +; SPDX-License-Identifier: BSD-2-Clause-Patent +; +;-- + + EXPORT PeiTemporaryRamMigration + AREA PeiCore_LowLevel, CODE, READONLY + +;-- +; VOID +; EFIAPI +; PeiTemporaryRamMigration ( +; IN PEI_CORE_TEMPORARY_RAM_TRANSITION *TempRamTransitionData +; ); +; +; @param[in] x0Pointer to PEI_CORE_TEMPORARY_RAM_TRANSITION +; +; @return None This routine does not return +;-- +PeiTemporaryRamMigration + +; +; We store the TempRamTransitionData pointer in x19. By the +; AArch64 calling convention we should normally save x19, but we +; won't be returning to the caller, so we don't need to save it. +; By the same rule, the TemporaryRamMigration PPI call should +; preserve x19 for us. +; +mov x19, x0 + +; +; Setup parameters and call TemporaryRamSupport->TemporaryRamMigration +; (rcx) PeiServices +; (rdx) TemporaryMemoryBase +; (r8) PermanentMemoryBase +; (r9) CopySize +; +ldp x0, x1, [x19, 0x08] +ldp x2, x3, [x19, 0x18] + +; +; (x19) Pointer to PEI_CORE_TEMPORARY_RAM_TRANSITION on stack +; +; Adjusted based on stack change made during +; TemporaryRamSupport->TemporaryRamMigration call +; +ldr x4, [x19] +mov x5, sp +sub x19, x19, x5 +blr x4 +mov x5, sp +add x19, x19, x5 + +; +; Setup parameters and call PeiTemporaryRamMigrated +; (x0) Pointer to PEI_CORE_TEMPORARY_RAM_TRANSITION +; +mov x0, x19 +bl ASM_PFX(PeiTemporaryRamMigrated) + + END diff --git a/MdeModule
[edk2-devel] [PATCH v2 6/6] MdeModulePkg/Core/Pei: Use code path for assembly based TemporaryRamSupport
There is potential problem with PEI Core's usage of the
TemporaryRamSupport PPI. The issue is described in the previous patch:
"MdeModulePkg/Core/Pei: Add interface for assembly based TemporaryRamSupport"
Now that assembly paths are available for all supported architectures,
we can make use of the assembly based PEI Core code path.
Signed-off-by: Jordan Justen
Cc: Jian J Wang
Cc: Hao Wu
Cc: Ray Ni
Cc: Star Zeng
---
MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 59 ---
1 file changed, 38 insertions(+), 21 deletions(-)
diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
index ba2fd0cae1..116124d6f8 100644
--- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
+++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
@@ -682,6 +682,7 @@ PeiCheckAndSwitchStack (
EFI_PHYSICAL_ADDRESS TempBase2;
UINTN TempSize2;
UINTN Index;
+ PEI_CORE_TEMPORARY_RAM_TRANSITION TempRamTransitionData;
PeiServices = (CONST EFI_PEI_SERVICES **) >Ps;
@@ -816,30 +817,20 @@ PeiCheckAndSwitchStack (
Private = (PEI_CORE_INSTANCE *)((UINTN)(VOID *)Private - StackOffset);
}
- //
- // 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.
- //
- TemporaryRamSupportPpi->TemporaryRamMigration (
-PeiServices,
-TemporaryRamBase,
-(EFI_PHYSICAL_ADDRESS)(UINTN)(TopOfNewStack -
TemporaryStackSize),
-TemporaryRamSize
-);
-
- //
- // Migrate memory pages allocated in pre-memory phase.
- // It could not be called before calling
TemporaryRamSupportPpi->TemporaryRamMigration()
- // as the migrated memory pages may be overridden by
TemporaryRamSupportPpi->TemporaryRamMigration().
- //
- MigrateMemoryPages (Private, TRUE);
+ TempRamTransitionData.TemporaryRamMigration =
+TemporaryRamSupportPpi->TemporaryRamMigration;
+ TempRamTransitionData.PeiServices = PeiServices;
+ TempRamTransitionData.TemporaryMemoryBase = TemporaryRamBase;
+ TempRamTransitionData.PermanentMemoryBase =
+(EFI_PHYSICAL_ADDRESS)(UINTN)(TopOfNewStack - TemporaryStackSize);
+ TempRamTransitionData.CopySize = TemporaryRamSize;
+ TempRamTransitionData.Private = Private;
+ TempRamTransitionData.SecCoreData = SecCoreData;
//
- // Entry PEI Phase 2
+ // Migrate Temporary RAM and enter PEI Phase 2
//
- PeiCore (SecCoreData, NULL, Private);
+ PeiTemporaryRamMigration();
} else {
//
// Migrate memory pages allocated in pre-memory phase.
@@ -952,6 +943,32 @@ PeiCheckAndSwitchStack (
}
}
+VOID
+EFIAPI
+PeiTemporaryRamMigrated (
+ IN VOID *CallbackContext
+ )
+{
+ PEI_CORE_TEMPORARY_RAM_TRANSITION *TempRamTransitionData =
+(PEI_CORE_TEMPORARY_RAM_TRANSITION*)CallbackContext;
+
+ //
+ // Migrate memory pages allocated in pre-memory phase.
+ // It could not be called before calling
TemporaryRamSupportPpi->TemporaryRamMigration()
+ // as the migrated memory pages may be overridden by
TemporaryRamSupportPpi->TemporaryRamMigration().
+ //
+ MigrateMemoryPages (TempRamTransitionData->Private, TRUE);
+
+ //
+ // Entry PEI Phase 2
+ //
+ PeiCore (
+TempRamTransitionData->SecCoreData,
+NULL,
+TempRamTransitionData->Private
+);
+}
+
/**
Conduct PEIM dispatch.
--
2.20.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#38790): https://edk2.groups.io/g/devel/message/38790
Mute This Topic: https://groups.io/mt/31016932/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/6] MdeModulePkg/Core/Pei: Add ARM assembly for TemporaryRamMigration
Signed-off-by: Jordan Justen
Cc: Leif Lindholm
Cc: Ard Biesheuvel
---
.../Dispatcher/Arm/TemporaryRamMigration.S| 68 +++
.../Dispatcher/Arm/TemporaryRamMigration.asm | 68 +++
MdeModulePkg/Core/Pei/PeiMain.inf | 5 ++
3 files changed, 141 insertions(+)
create mode 100644 MdeModulePkg/Core/Pei/Dispatcher/Arm/TemporaryRamMigration.S
create mode 100644
MdeModulePkg/Core/Pei/Dispatcher/Arm/TemporaryRamMigration.asm
diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Arm/TemporaryRamMigration.S
b/MdeModulePkg/Core/Pei/Dispatcher/Arm/TemporaryRamMigration.S
new file mode 100644
index 00..872bbcf27d
--- /dev/null
+++ b/MdeModulePkg/Core/Pei/Dispatcher/Arm/TemporaryRamMigration.S
@@ -0,0 +1,68 @@
+#--
+#
+# Copyright (c) 2019, Intel Corporation. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#--
+
+.text
+.align 5
+
+GCC_ASM_EXPORT(PeiTemporaryRamMigration)
+
+#--
+# VOID
+# EFIAPI
+# PeiTemporaryRamMigration (
+# IN PEI_CORE_TEMPORARY_RAM_TRANSITION *TempRamTransitionData
+# );
+#
+# @param[in] r0Pointer to PEI_CORE_TEMPORARY_RAM_TRANSITION
+#
+# @return None This routine does not return
+#--
+ASM_PFX(PeiTemporaryRamMigration):
+
+#
+# We store the TempRamTransitionData pointer in r6. By the
+# Arm calling convention we should normally save r6, but we
+# won't be returning to the caller, so we don't need to save it.
+# By the same rule, the TemporaryRamMigration PPI call should
+# preserve r6 for us.
+#
+mov r6, r0
+
+#
+# Setup parameters and call TemporaryRamSupport->TemporaryRamMigration
+# (r0) PeiServices
+# (r2,r3) TemporaryMemoryBase
+# (stack) PermanentMemoryBase
+# (stack) CopySize
+#
+add r7, r6, #4
+ldmia r7, {r0-r5}
+stmfd sp!, {r3, r4, r5}
+mov r3, r2
+mov r2, r1
+
+#
+# (r6) Pointer to PEI_CORE_TEMPORARY_RAM_TRANSITION on stack
+#
+# Adjusted based on stack change made during
+# TemporaryRamSupport->TemporaryRamMigration call
+#
+ldr r4, [r6]
+mov r5, sp
+sub r6, r6, r5
+mov lr, pc
+bx r4
+mov r0, sp
+add r6, r6, r0
+add sp, sp, #0xc
+
+#
+# Setup parameters and call PeiTemporaryRamMigrated
+# (r0) Pointer to PEI_CORE_TEMPORARY_RAM_TRANSITION
+#
+mov r0, r6
+bl ASM_PFX(PeiTemporaryRamMigrated)
diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Arm/TemporaryRamMigration.asm
b/MdeModulePkg/Core/Pei/Dispatcher/Arm/TemporaryRamMigration.asm
new file mode 100644
index 00..1cfb0e0dd8
--- /dev/null
+++ b/MdeModulePkg/Core/Pei/Dispatcher/Arm/TemporaryRamMigration.asm
@@ -0,0 +1,68 @@
+;--
+;
+; Copyright (c) 2019, Intel Corporation. All rights reserved.
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+;--
+
+ EXPORT PeiTemporaryRamMigration
+ AREA PeiCore_LowLevel, CODE, READONLY
+
+;--
+; VOID
+; EFIAPI
+; PeiTemporaryRamMigration (
+; IN PEI_CORE_TEMPORARY_RAM_TRANSITION *TempRamTransitionData
+; );
+;
+; @param[in] r0Pointer to PEI_CORE_TEMPORARY_RAM_TRANSITION
+;
+; @return None This routine does not return
+;--
+PeiTemporaryRamMigration
+
+;
+; We store the TempRamTransitionData pointer in r6. By the
+; Arm calling convention we should normally save r6, but we
+; won't be returning to the caller, so we don't need to save it.
+; By the same rule, the TemporaryRamMigration PPI call should
+; preserve r6 for us.
+;
+mov r6, r0
+
+;
+; Setup parameters and call TemporaryRamSupport->TemporaryRamMigration
+; (r0) PeiServices
+; (r2,r3) TemporaryMemoryBase
+; (stack) PermanentMemoryBase
+; (stack) CopySize
+;
+add r7, r6, #4
+ldmia r7, {r0-r5}
+stmfd sp!, {r3, r4, r5}
+mov r3, r2
+mov r2, r1
+
+;
+; (r6) Pointer to PEI_CORE_TEMPORARY_RAM_TRANSITION on stack
+;
+; Adjusted based on stack change made during
+; TemporaryRamSupport->TemporaryRamMigration call
+;
+ldr r4, [r6]
+mov r5, sp
+sub r6, r6, r5
+mov lr, pc
+bx r4
+mov r0, sp
+add r6, r6, r0
+add sp, sp, #0xc
+
+;
+; Setup parameters and call PeiTemporaryRamMigrated
[edk2-devel] [PATCH v2 4/6] MdeModulePkg/Core/Pei: Add IA32 assembly for TemporaryRamMigration
Signed-off-by: Jordan Justen Cc: Jian J Wang Cc: Hao Wu Cc: Ray Ni Cc: Star Zeng --- .../Ia32/TemporaryRamMigration.nasm | 77 +++ MdeModulePkg/Core/Pei/PeiMain.inf | 3 + 2 files changed, 80 insertions(+) create mode 100644 MdeModulePkg/Core/Pei/Dispatcher/Ia32/TemporaryRamMigration.nasm diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Ia32/TemporaryRamMigration.nasm b/MdeModulePkg/Core/Pei/Dispatcher/Ia32/TemporaryRamMigration.nasm new file mode 100644 index 00..8014e3cc95 --- /dev/null +++ b/MdeModulePkg/Core/Pei/Dispatcher/Ia32/TemporaryRamMigration.nasm @@ -0,0 +1,77 @@ +;-- +; +; Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved. +; SPDX-License-Identifier: BSD-2-Clause-Patent +; +;-- + +#include + +SECTION .text + +extern ASM_PFX(PeiTemporaryRamMigrated) + +;-- +; VOID +; EFIAPI +; PeiTemporaryRamMigration ( +; IN PEI_CORE_TEMPORARY_RAM_TRANSITION *TempRamTransitionData +; ); +; +; @param[in] Stack Pointer to PEI_CORE_TEMPORARY_RAM_TRANSITION +; +; @return None This routine does not return +;-- +global ASM_PFX(PeiTemporaryRamMigration) +ASM_PFX(PeiTemporaryRamMigration): + +; +; We never return from this call +; +add esp, 4 + +; +; (eax) Pointer to PEI_CORE_TEMPORARY_RAM_TRANSITION +; +mov eax, [esp] + +; +; We store the TempRamTransitionData pointer in ebx. By the IA32 +; calling convention we should normally save ebx, but we won't be +; returning to the caller, so we don't need to save it. By the +; same rule, the TemporaryRamMigration PPI call should preserve +; ebx for us. +; +mov ebx, [esp] + +; +; Setup parameters and call TemporaryRamSupport->TemporaryRamMigration +; 32-bit PeiServices +; 64-bit TemporaryMemoryBase +; 64-bit PermanentMemoryBase +; 32-bit CopySize +; +pushDWORD [eax + 0x18] ; CopySize +pushDWORD [eax + 0x14] ; PermanentMemoryBase Upper 32-bit +pushDWORD [eax + 0x10] ; PermanentMemoryBase Lower 32-bit +pushDWORD [eax + 0x0c] ; TemporaryMemoryBase Upper 32-bit +pushDWORD [eax + 0x08] ; TemporaryMemoryBase Lower 32-bit +pushDWORD [eax + 0x04] ; PeiServices + +; +; (ebx) Pointer to PEI_CORE_TEMPORARY_RAM_TRANSITION on stack +; +; Adjusted based on stack change made during +; TemporaryRamSupport->TemporaryRamMigration call +; +sub ebx, esp +callDWORD [eax + 0x00] +add ebx, esp +add esp, 0x18 + +; +; Setup parameters and call PeiTemporaryRamMigrated +; 32-bit Pointer to PEI_CORE_TEMPORARY_RAM_TRANSITION +; +pushebx +callASM_PFX(PeiTemporaryRamMigrated) diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf b/MdeModulePkg/Core/Pei/PeiMain.inf index 1be898bb91..918c4a0df8 100644 --- a/MdeModulePkg/Core/Pei/PeiMain.inf +++ b/MdeModulePkg/Core/Pei/PeiMain.inf @@ -56,6 +56,9 @@ Dispatcher/Arm/TemporaryRamMigration.asm | MSFT Dispatcher/Arm/TemporaryRamMigration.S | GCC +[Sources.Ia32] + Dispatcher/Ia32/TemporaryRamMigration.nasm + [Packages] MdePkg/MdePkg.dec MdeModulePkg/MdeModulePkg.dec -- 2.20.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#38789): https://edk2.groups.io/g/devel/message/38789 Mute This Topic: https://groups.io/mt/31016930/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2 1/6] MdeModulePkg/Core/Pei: Add interface for assembly based TemporaryRamSupport
There is potential problem with PEI Core's usage of the
TemporaryRamSupport PPI. When the TemporaryRamMigration function is
called, it returns to C based code after changing the stack to the new
permanent memory copy of the stack. But, the C compiler may have
stored pointers to addresses on the old temporary RAM stack. Even
though the stack is copied to a new permanent memory location, it is
not possible to adjust all pointers that the C compiler may have added
within the stack data.
For this reason, it is only safe to return to assembly code after
calling TemporaryRamMigration. The assembly code can make sure the old
temporary RAM stack is not used before calling a new C function. When
the new function is called, it will use the new permanent memory
stack, so it is safe to use C code again.
This patch add the interface that the assembly function will need. The
PEI_CORE_TEMPORARY_RAM_TRANSITION contains all the data that the
assembly code will need to call the
TemporaryRamSupport->TemporaryRamMigration function, and then the
context that PEI will need after this call when the new C based
PeiTemporaryRamMigrated function is called.
After all assembly code based implementations have been added, PEI
Core will be updated to use the new assembly based code path.
Signed-off-by: Jordan Justen
Cc: Jian J Wang
Cc: Hao Wu
Cc: Ray Ni
Cc: Star Zeng
---
MdeModulePkg/Core/Pei/PeiMain.h | 52 +
1 file changed, 52 insertions(+)
diff --git a/MdeModulePkg/Core/Pei/PeiMain.h b/MdeModulePkg/Core/Pei/PeiMain.h
index 0aed4f4685..6522798059 100644
--- a/MdeModulePkg/Core/Pei/PeiMain.h
+++ b/MdeModulePkg/Core/Pei/PeiMain.h
@@ -1865,4 +1865,56 @@ PeiReinitializeFv (
IN PEI_CORE_INSTANCE *PrivateData
);
+#pragma pack(1)
+typedef struct {
+ /**
+These fields are used by PeiTemporaryRamMigration to call the
+TemporaryRamMigration PPI.
+ **/
+ TEMPORARY_RAM_MIGRATION TemporaryRamMigration;
+ CONST EFI_PEI_SERVICES**PeiServices;
+ EFI_PHYSICAL_ADDRESS TemporaryMemoryBase;
+ EFI_PHYSICAL_ADDRESS PermanentMemoryBase;
+ UINTN CopySize;
+
+ /**
+These fields are used by PeiTemporaryRamMigrated.
+ **/
+ PEI_CORE_INSTANCE *Private;
+ CONST EFI_SEC_PEI_HAND_OFF*SecCoreData;
+} PEI_CORE_TEMPORARY_RAM_TRANSITION;
+#pragma pack()
+
+/**
+ To call the TemporaryRamMigration PPI, we might not be able to rely
+ on C code's handling of the stack. In these cases we use an assembly
+ function to make sure the old stack is not used after the
+ TemporaryRamMigration PPI is used.
+
+ After calling the TemporaryRamMigration PPI, this function calls
+ PeiTemporaryRamMigrated.
+
+ @param TempRamTransitionData
+**/
+VOID
+EFIAPI
+PeiTemporaryRamMigration (
+ IN PEI_CORE_TEMPORARY_RAM_TRANSITION *TempRamTransitionData
+ );
+
+/**
+ After PeiTemporaryRamMigration has called the TemporaryRamMigration
+ PPI, it will call this C based function to allow PEI to continue
+ after the migration using the new stack in the migrated RAM.
+
+ @param CallbackContext Pointer to PEI_CORE_TEMPORARY_RAM_TRANSITION
+ data.
+**/
+VOID
+EFIAPI
+PeiTemporaryRamMigrated (
+ IN VOID *CallbackContext
+ );
+
+
#endif
--
2.20.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#38786): https://edk2.groups.io/g/devel/message/38786
Mute This Topic: https://groups.io/mt/31016924/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2 0/6] Fix PEI Core issue during TemporaryRamMigration
https://github.com/jljusten/edk2.git temp-ram-support-v2 https://github.com/jljusten/edk2/commits/temp-ram-support-v2 v2: * Add AARCH64 and ARM assembly * Drop IA32 and X64 .S source files * Adjust PEI_CORE_TEMPORARY_RAM_TRANSITION pointer in the assembly code based on the stack pointer change before & after TemporaryRamSupport->TemporaryRamMigration * Drop extra cleanup patches for OvmfPkg & EmulatorPkg. These were just complicating the series. This series fixes an issue that, while rare, is possible based on the way the TemporaryRamSupport PPI is defined along with how it is used by the PEI Core. Liu Yu reported a boot issue with EmulatorPkg, which I believe was caused by this issue. The details of the issue are described in the commit message of the "MdeModulePkg/Core/Pei: Add interface for assembly based TemporaryRamSupport" patch. Testing: I tested building and booting in several scenarios: * OVMF IA32 & X64 on Linux * ArmVirtPkg AARCH64 & ARM on x86_64 Linux * EmulatorPkg IA32 & X64 on Linux Untested: * My system does not reproduce the issue that Liu Yu reported with EmulatorPkg, so I can't say that I have verified that issue. * Building on windows * AARCH64/ARM TemporaryRamMigration.asm sources Cc: Liu Yu Cc: Ray Ni Cc: Andrew Fish Cc: Laszlo Ersek Cc: Leif Lindholm Cc: Michael D Kinney Jordan Justen (6): MdeModulePkg/Core/Pei: Add interface for assembly based TemporaryRamSupport MdeModulePkg/Core/Pei: Add AARCH64 assembly for TemporaryRamMigration MdeModulePkg/Core/Pei: Add ARM assembly for TemporaryRamMigration MdeModulePkg/Core/Pei: Add IA32 assembly for TemporaryRamMigration MdeModulePkg/Core/Pei: Add X64 assembly for TemporaryRamMigration MdeModulePkg/Core/Pei: Use code path for assembly based TemporaryRamSupport .../AArch64/TemporaryRamMigration.S | 63 +++ .../AArch64/TemporaryRamMigration.asm | 63 +++ .../Dispatcher/Arm/TemporaryRamMigration.S| 68 .../Dispatcher/Arm/TemporaryRamMigration.asm | 68 MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 59 +- .../Ia32/TemporaryRamMigration.nasm | 77 +++ .../Dispatcher/X64/TemporaryRamMigration.nasm | 74 ++ MdeModulePkg/Core/Pei/PeiMain.h | 52 + MdeModulePkg/Core/Pei/PeiMain.inf | 15 9 files changed, 518 insertions(+), 21 deletions(-) create mode 100644 MdeModulePkg/Core/Pei/Dispatcher/AArch64/TemporaryRamMigration.S create mode 100644 MdeModulePkg/Core/Pei/Dispatcher/AArch64/TemporaryRamMigration.asm create mode 100644 MdeModulePkg/Core/Pei/Dispatcher/Arm/TemporaryRamMigration.S create mode 100644 MdeModulePkg/Core/Pei/Dispatcher/Arm/TemporaryRamMigration.asm create mode 100644 MdeModulePkg/Core/Pei/Dispatcher/Ia32/TemporaryRamMigration.nasm create mode 100644 MdeModulePkg/Core/Pei/Dispatcher/X64/TemporaryRamMigration.nasm -- 2.20.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#38785): https://edk2.groups.io/g/devel/message/38785 Mute This Topic: https://groups.io/mt/31016921/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch] EmulatorPkg/Unix: Rename GdbRun to GdbRun.sh
$ git grep GdbRun
EmulatorPkg/Unix/Host/Host.c: // has a break point script to source the GdbRun
script.
EmulatorPkg/Unix/Xcode/xcode_project32/xcode_project.xcodeproj/default.pbxuser:
command = "set gInXcode=1\nsource ../../../../EmulatorPkg/Unix/GdbRun";
EmulatorPkg/Unix/Xcode/xcode_project64/xcode_project.xcodeproj/default.pbxuser:
command = "set gInXcode=1\nsource ../../../../EmulatorPkg/Unix/GdbRun";
EmulatorPkg/build.sh: /usr/bin/gdb $BUILD_ROOT_ARCH/Host -q
-cd=$BUILD_ROOT_ARCH -x $WORKSPACE/EmulatorPkg/Unix/GdbRun
If those are updated,
Reviewed-by: Jordan Justen
On 2019-04-03 14:54:39, Kinney, Michael D wrote:
> https://bugzilla.tianocore.org/show_bug.cgi?id=1657
>
> Add .sh file extension to the shell script GdbRun so
> changes to this file will pass PatchCheck.py.
>
> Cc: Jordan Justen
> Cc: Andrew Fish
> Cc: Ray Ni
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Michael D Kinney
> ---
> EmulatorPkg/Unix/{GdbRun => GdbRun.sh} | 0
> 1 file changed, 0 insertions(+), 0 deletions(-)
> rename EmulatorPkg/Unix/{GdbRun => GdbRun.sh} (100%)
>
> diff --git a/EmulatorPkg/Unix/GdbRun b/EmulatorPkg/Unix/GdbRun.sh
> similarity index 100%
> rename from EmulatorPkg/Unix/GdbRun
> rename to EmulatorPkg/Unix/GdbRun.sh
> --
> 2.21.0.windows.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#18): https://edk2.groups.io/g/devel/message/18
Mute This Topic: https://groups.io/mt/30886171/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
