Re: [edk2] [question] A piece of log.txt showed in uefi shell is not the same as showed in notepad++
Andrew Fish,
thank you,in this case,you are right.
but I think there may be other case that the user data just contain some
nosense-data
such as { UINT16 MyData[]={0x000c,0x000d,0x000d,0x000e} } to the editor,
the uefi editor may also meet this question,the editor may do not need output
this,
but the editor should make sure other visible chars can output normal...
I don't know if it is a bug or not.
Thank you very much.
krishna.
在 2019-01-24 12:13:50，"Andrew Fish" 写道：
On Jan 23, 2019, at 7:17 PM, krishnaLee wrote:
Hi,
I dumped a small log.txt from my work,but the log.txt showed in uefi shell,is
not the same as it showed in notepad++.
I had upload the log here:
https://github.com/krishna116/test/blob/master/log.zip
it seems the log showed in uefi shell had missed some strings...I don't know
why,please help,
Krishna,
Your log.txt looks like a normal UTF-16 Unicode file. The leading FF FE is the
Byte order mark for UTF-16 Little Endian [1]. The file looks like it has CR
line endings [2] so maybe that is confusing your editor? There seems to be a CR
LF at the end, so the mixture of line ending may even be more likely confusing
the editor. The byte order mark should let your editor know it is 16-bit
Unicode and the correct endian. Hope this helps...
$ hexdump -C /Users/andrewfish/Downloads/log/log.txt
ff fe 20 00 20 00 20 00 4d 00 58 00 32 00 35 00 |.. . . .M.X.2.5.|
0010 4c 00 31 00 32 00 38 00 37 00 35 00 46 00 20 00 |L.1.2.8.7.5.F. .|
0020 20 00 20 00 20 00 49 00 44 00 3a 00 30 00 78 00 | . . .I.D.:.0.x.|
0030 43 00 32 00 32 00 30 00 31 00 38 00 20 00 20 00 |C.2.2.0.1.8. . .|
0040 20 00 20 00 53 00 69 00 7a 00 65 00 3a 00 20 00 | . .S.i.z.e.:. .|
0050 31 00 36 00 33 00 38 00 34 00 4b 00 42 00 20 00 |1.6.3.8.4.K.B. .|
0060 28 00 31 00 33 00 31 00 30 00 37 00 32 00 4b 00 |(.1.3.1.0.7.2.K.|
0070 62 00 29 00 20 00 20 00 20 00 20 00 20 00 20 00 |b.). . . . . . .|
0080 20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 | . . . . . . . .|
*
00c0 20 00 20 00 20 00 20 00 0d 00 0d 00 20 00 20 00 | . . . . . .|
00d0 20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 | . . . . . . . .|
*
0190 20 00 20 00 0d 00 0d 00 20 00 20 00 20 00 20 00 | . . . . . .|
01a0 4d 00 58 00 32 00 35 00 4c 00 36 00 34 00 37 00 |M.X.2.5.L.6.4.7.|
01b0 33 00 45 00 20 00 20 00 20 00 20 00 49 00 44 00 |3.E. . . . .I.D.|
01c0 3a 00 30 00 78 00 43 00 32 00 32 00 30 00 31 00 |:.0.x.C.2.2.0.1.|
01d0 37 00 20 00 20 00 20 00 20 00 53 00 69 00 7a 00 |7. . . . .S.i.z.|
01e0 65 00 3a 00 20 00 38 00 31 00 39 00 32 00 4b 00 |e.:. .8.1.9.2.K.|
01f0 42 00 20 00 28 00 36 00 35 00 35 00 33 00 36 00 |B. .(.6.5.5.3.6.|
0200 4b 00 62 00 29 00 20 00 20 00 20 00 20 00 20 00 |K.b.). . . . . .|
0210 20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 | . . . . . . . .|
*
0260 0d 00 0d 00 20 00 20 00 20 00 20 00 20 00 20 00 | . . . . . .|
0270 20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 | . . . . . . . .|
*
0320 20 00 20 00 20 00 20 00 20 00 20 00 0d 00 0d 00 | . . . . . .|
0330 53 00 50 00 49 00 20 00 42 00 41 00 52 00 3a 00 |S.P.I. .B.A.R.:.|
0340 20 00 46 00 45 00 30 00 31 00 30 00 30 00 30 00 | .F.E.0.1.0.0.0.|
0350 30 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 |0. . . . . . . .|
0360 20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 | . . . . . . . .|
*
03f0 20 00 20 00 20 00 20 00 0d 00 0d 00 20 00 20 00 | . . . . . .|
0400 20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 | . . . . . . . .|
*
04c0 20 00 20 00 0d 00 0d 00 46 00 50 00 54 00 20 00 | . .F.P.T. .|
04d0 4f 00 70 00 65 00 72 00 61 00 74 00 69 00 6f 00 |O.p.e.r.a.t.i.o.|
04e0 6e 00 20 00 53 00 75 00 63 00 63 00 65 00 73 00 |n. .S.u.c.c.e.s.|
04f0 73 00 66 00 75 00 6c 00 2e 00 20 00 20 00 20 00 |s.f.u.l... . . .|
0500 20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 | . . . . . . . .|
*
0590 0d 00 0d 00 20 00 20 00 20 00 20 00 20 00 20 00 | . . . . . .|
05a0 20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 | . . . . . . . .|
*
0650 20 00 20 00 20 00 20 00 20 00 20 00 0d 00 0d 00 | . . . . . .|
0660 0d 00 0a 00 ||
0664
[1] https://en.wikipedia.org/wiki/Byte_order_mark#UTF-16
[2] https://en.wikipedia.org/wiki/Newline
Thanks,
Andrew Fish
thank you,
by krishna.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [question] A piece of log.txt showed in uefi shell is not the same as showed in notepad++
> On Jan 23, 2019, at 7:17 PM, krishnaLee wrote: > > Hi, > I dumped a small log.txt from my work,but the log.txt showed in uefi shell,is > not the same as it showed in notepad++. > I had upload the log here: > https://github.com/krishna116/test/blob/master/log.zip > > > it seems the log showed in uefi shell had missed some strings...I don't know > why,please help, > Krishna, Your log.txt looks like a normal UTF-16 Unicode file. The leading FF FE is the Byte order mark for UTF-16 Little Endian [1]. The file looks like it has CR line endings [2] so maybe that is confusing your editor? There seems to be a CR LF at the end, so the mixture of line ending may even be more likely confusing the editor. The byte order mark should let your editor know it is 16-bit Unicode and the correct endian. Hope this helps... $ hexdump -C /Users/andrewfish/Downloads/log/log.txt ff fe 20 00 20 00 20 00 4d 00 58 00 32 00 35 00 |.. . . .M.X.2.5.| 0010 4c 00 31 00 32 00 38 00 37 00 35 00 46 00 20 00 |L.1.2.8.7.5.F. .| 0020 20 00 20 00 20 00 49 00 44 00 3a 00 30 00 78 00 | . . .I.D.:.0.x.| 0030 43 00 32 00 32 00 30 00 31 00 38 00 20 00 20 00 |C.2.2.0.1.8. . .| 0040 20 00 20 00 53 00 69 00 7a 00 65 00 3a 00 20 00 | . .S.i.z.e.:. .| 0050 31 00 36 00 33 00 38 00 34 00 4b 00 42 00 20 00 |1.6.3.8.4.K.B. .| 0060 28 00 31 00 33 00 31 00 30 00 37 00 32 00 4b 00 |(.1.3.1.0.7.2.K.| 0070 62 00 29 00 20 00 20 00 20 00 20 00 20 00 20 00 |b.). . . . . . .| 0080 20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 | . . . . . . . .| * 00c0 20 00 20 00 20 00 20 00 0d 00 0d 00 20 00 20 00 | . . . . . .| 00d0 20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 | . . . . . . . .| * 0190 20 00 20 00 0d 00 0d 00 20 00 20 00 20 00 20 00 | . . . . . .| 01a0 4d 00 58 00 32 00 35 00 4c 00 36 00 34 00 37 00 |M.X.2.5.L.6.4.7.| 01b0 33 00 45 00 20 00 20 00 20 00 20 00 49 00 44 00 |3.E. . . . .I.D.| 01c0 3a 00 30 00 78 00 43 00 32 00 32 00 30 00 31 00 |:.0.x.C.2.2.0.1.| 01d0 37 00 20 00 20 00 20 00 20 00 53 00 69 00 7a 00 |7. . . . .S.i.z.| 01e0 65 00 3a 00 20 00 38 00 31 00 39 00 32 00 4b 00 |e.:. .8.1.9.2.K.| 01f0 42 00 20 00 28 00 36 00 35 00 35 00 33 00 36 00 |B. .(.6.5.5.3.6.| 0200 4b 00 62 00 29 00 20 00 20 00 20 00 20 00 20 00 |K.b.). . . . . .| 0210 20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 | . . . . . . . .| * 0260 0d 00 0d 00 20 00 20 00 20 00 20 00 20 00 20 00 | . . . . . .| 0270 20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 | . . . . . . . .| * 0320 20 00 20 00 20 00 20 00 20 00 20 00 0d 00 0d 00 | . . . . . .| 0330 53 00 50 00 49 00 20 00 42 00 41 00 52 00 3a 00 |S.P.I. .B.A.R.:.| 0340 20 00 46 00 45 00 30 00 31 00 30 00 30 00 30 00 | .F.E.0.1.0.0.0.| 0350 30 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 |0. . . . . . . .| 0360 20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 | . . . . . . . .| * 03f0 20 00 20 00 20 00 20 00 0d 00 0d 00 20 00 20 00 | . . . . . .| 0400 20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 | . . . . . . . .| * 04c0 20 00 20 00 0d 00 0d 00 46 00 50 00 54 00 20 00 | . .F.P.T. .| 04d0 4f 00 70 00 65 00 72 00 61 00 74 00 69 00 6f 00 |O.p.e.r.a.t.i.o.| 04e0 6e 00 20 00 53 00 75 00 63 00 63 00 65 00 73 00 |n. .S.u.c.c.e.s.| 04f0 73 00 66 00 75 00 6c 00 2e 00 20 00 20 00 20 00 |s.f.u.l... . . .| 0500 20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 | . . . . . . . .| * 0590 0d 00 0d 00 20 00 20 00 20 00 20 00 20 00 20 00 | . . . . . .| 05a0 20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 | . . . . . . . .| * 0650 20 00 20 00 20 00 20 00 20 00 20 00 0d 00 0d 00 | . . . . . .| 0660 0d 00 0a 00 || 0664 [1] https://en.wikipedia.org/wiki/Byte_order_mark#UTF-16 [2] https://en.wikipedia.org/wiki/Newline Thanks, Andrew Fish > > thank you, > by krishna. > > > > > > ___ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [question] A piece of log.txt showed in uefi shell is not the same as showed in notepad++
Hi, I dumped a small log.txt from my work,but the log.txt showed in uefi shell,is not the same as it showed in notepad++. I had upload the log here: https://github.com/krishna116/test/blob/master/log.zip it seems the log showed in uefi shell had missed some strings...I don't know why,please help, thank you, by krishna. ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] Drop CSM support in OvmfPkg?
David, I think we got an agreement here to move CSM components in OvmfPkg. I prefer we firstly clone the required CSM components in OvmfPkg right no. Finally I can remove the IntelFrameworkModulePkg/IntelFrameworkPkg in one patch. (I say "finally" because OVMF CSM dependency is not the only case that prevent removing the two framework packages.) Would you like to do the clone? Or if you are busy, I can do that. Thanks, Ray > -Original Message- > From: David Woodhouse [mailto:dw...@infradead.org] > Sent: Wednesday, January 23, 2019 5:49 PM > To: Laszlo Ersek ; Ni, Ray ; Gerd > Hoffmann ; Richardson, Brian > > Cc: Justen, Jordan L ; edk2-devel@lists.01.org; > Kevin O'Connor ; Anthony Perard > > Subject: Re: Drop CSM support in OvmfPkg? > > On Wed, 2019-01-23 at 10:46 +0100, Laszlo Ersek wrote: > > I'm fine if we move the generic CSM components into OvmfPkg, however I'm > > going to ask David to assume reviewer responsibilities for them. > > > > Given the current format of "Maintainers.txt", we couldn't spell out the > > exact pathnames of the CSM components, so we'd add a line like > > > > R: David Woodhouse > > > > under OvmfPkg. There is "prior art" for this pattern, see: > > > > R: Anthony Perard > > R: Julien Grall > > > > Because Anthony and Julien are the authority on Xen-related code under > > OvmfPkg. (See commit 337fe6a06eda, "Maintainers.txt: add Xen reviewers > > to OvmfPkg", 2017-09-26.) > > > > > > If we keep CSM support in OvmfPkg in any form at all, then I would > > prefer holding all the related stuff in the core edk2 repository (with > > the above Reviewership), over requiring people to deal with multiple > > repositories. I agree (from experience) that PACKAGES_PATH / multiple > > workspaces work fine, but in this case I think keeping one shared > > history is an advantage. > > This all makes sense to me. ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU
On 01/23/19 15:02, Ard Biesheuvel wrote:
> On Wed, 23 Jan 2019 at 10:55, Laszlo Ersek wrote:
>>
>> On 01/23/19 10:26, Ard Biesheuvel wrote:
>>> On Wed, 23 Jan 2019 at 10:14, Laszlo Ersek wrote:
On 01/22/19 16:37, Ard Biesheuvel wrote:
>>
> Is SetUefiImageMemoryAttributes() being
> called to remap the memory R-X ?
No, it is not; the grub binary in question doesn't have the required
section alignment (... I hope at least that that's what your question
refers to):
> ProtectUefiImageCommon - 0x3E6C54C0
> - 0x00013BEEF000 - 0x00030600
> ProtectUefiImageCommon - Section Alignment(0x200) is
incorrect
>>>
>>> This is puzzling, given that the exact same binary works on Mustang.
>>
>> And even on the original (unspecified) hardware, the same binary works
>> frequently. My understanding is that there are five VMs executing reboot
>> loops in parallel, on the same host, and 4 out of 5 may hit the issue in
>> a reasonable time period (300 reboots or so).
>>
>>> So when loaded, GRUB should cover the following regions:
>>>
>>> 0x13beef - 0x13bf00 (0x11000)
>>> 0x13bf00 - 0x13bf01f600 (0x1f600)
>>>
>>> where neither covers a 2 MB block fully, which means that the TLB
>>> entry that we are hitting is stale.
>>>
>>> Since ProtectUefiImageCommon() does not do anything in this case, the
>>> stale translation must be the result of
>>> PcdDxeNxMemoryProtectionPolicy, which either sets the wrong
>>> permissions for EfiLoaderCode (relying on ProtectUefiImageCommon), or
>>> we don't flush the TLBs correctly after updating the permissions when
>>> converting the memory from EfiConventionalMemory to EfiLoaderCode
>>>
>>> Are you using the default value for PcdDxeNxMemoryProtectionPolicy?
>>
>> Yes, we have
>>
>> ArmVirtPkg/ArmVirt.dsc.inc:
>> gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC0007FD1
>>
>> from commit 1acd7c54a724 ("ArmVirtPkg AARCH64: enable NX memory
>> protection for all platforms", 2017-03-01).
>>
>> The binary is from the RPM
>> "edk2-aarch64-20180508gitee3198e672e2-5.el8+1789+f0947240.noarch", which
>> is basically upstream ee3198e672e2 plus a small number of backports and
>> downstream customizations.
>>
>
> This might help:
>
> diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> index b7173e00b039..4c0b4b4efbd5 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> +++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> @@ -138,7 +138,7 @@ ASM_FUNC(ArmUpdateTranslationTableEntry)
>
> ASM_FUNC(ArmInvalidateTlb)
> EL1_OR_EL2_OR_EL3(x0)
> -1: tlbi vmalle1
> +1: tlbi vmalle1is
> b 4f
> 2: tlbi alle2
> b 4f
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> index 90192df24f55..d54b1c19accf 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> @@ -34,7 +34,7 @@
>
>// flush the TLBs
>.if \el == 1
> - tlbi vmalle1
> + tlbi vmalle1is
>.else
>tlbi alle\el
>.endif
>
"Invalidate all EL1&0 regime stage 1 TLB entries for the current VMID
>>>on all PEs in the same Inner Shareable domain<<<".
I'll soon try to build a new pkg with this applied, for testing by the
reporter. Many thanks!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] EDK II Network Stack Issue
On 1/22/19 6:34 PM, Laszlo Ersek wrote: Okay, summary: - forget the MdePkg and StdLib DSC files, build only AppPkg (or whatever platform DSC it is that produces your UEFI application) - make sure PcdDebugPropertyMask has bitmask 0x06 set when you build AppPkg (or the appropriate platform DSC in question); either modify the same DSC for this, or use the --pcd command line switch - make sure PcdDebugPrintErrorLevel has all the DEBUG_* bits set from "StdLib/EfiSocketLib/Socket.h" that you care about; the methods for setting this PCD are identical to those seen in the previous point. Hope this helps, Laszlo Hello Laszlo, debug prints are working now - thanks for your advice! Greetings, Karin ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] Network Stack Budgeting
On 01/23/19 17:27, Tomas Pilar (tpilar) wrote: > >> The set of devices connected during BDS is platform policy. It is not >> the "network stack" that calls Snp.Start(), but the platform BDS code >> that calls gBS->ConnectController() on the device, possibly without a >> good reason (e.g. without the device being present in a UEFI boot >> option). The network stack only "reacts" to that connection request. > Indeed, but even if a SNP handle is present as a boot option in a boot > manager, surely the Snp.Start() should be deferred until the user > actually chooses to boot from that handle. Yes, I agree. As far as I remember, UefiBootManagerLib is compatible with that idea (it makes sure the device path is connected before the boot is attempted). So again, the deferral you suggest applies to platform BDS. > A workaround that we have in the legacy implementation doesn't start > the underlying hardware datapath until the platform tries to send the > first packet (since PXE boot is always initiated by the client) but > that is a horrible hack that should not be necessary. The distinction > between Snp.Initialize() and Snp.Start() is there exactly for that > reason, no? That's a good question; if someone finds the answer, please let me too know! :) > In other words, ConnectController() should not immediately result in > Snp.Start() being called. Hmm, now that you put it like this, it's hard for me to comment on this specific statement. I'm unsure if the higher level network drivers can do their jobs (i.e. just bind the device) without calling Snp.Start(). I haven't implemented anything higher level than SNP; I'm not sure about MNP etc. internals. I do admit your original point makes a lot more sense to me now. Can you capture a call stack when Snp.Start() is invoked for the very first time (which, IIUC, is a call that should not happen, in your opinion)? Thanks Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] parallelism in the module-level, generated GNUmakefile's
On 01/23/19 17:36, Gao, Liming wrote: > Laszlo: By design, BaseTools generates GNUMakefile with the complete > dependency. So, the generated GNUMakefile should support parallelism > run. But, I don't verify this functionality. Have you found any > limitation to forbid parallelism run in module GNUMakefile? No, I haven't, and that is what surprised me :) I *expected* problems, but didn't find any. Given that BaseTools / "build" does not offer any way at all to build a single generated GNUMakefile with "make -j", such parallel builds of said GNUMakefiles have never been tested. Therefore, I expected them to break, under "make -j". When they didn't, I was surprised. :) The question is if we can *rely* on BaseTools to generate the GNUMakefiles with complete dependencies. In other words, if we now start building them with "make -j" (via the outer make), and run into an error due to missing dependencies, can we report a TianoCore BZ about that? Will it be in scope? Is "complete dependencies" an explicit goal? If the answer is "yes", I will happily take that answer. If the answer is "no", then BaseTools should use .NOTPARALLEL in the GNUMakefiles (or else "build" should invoke the inner "make" with an explicit "-j1" flag -- as I've learned today). Thank you! Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] ShellPkg/TftpDynamicCommand: Return proper status
Tftp command always returned "SHELL_NOT_FOUND" which is treated as an
error by callers. Add missing line to clean the ShellStatus on
successful operation. If operation has failed, return the error status
if available.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Vladimir Olovyannikov
---
ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
index ba753a279b00..88e3988a554e 100644
--- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
+++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
@@ -548,6 +548,8 @@ RunTftp (
goto NextHandle;
}
+ShellStatus = SHELL_SUCCESS;
+
NextHandle:
CloseProtocolAndDestroyServiceChild (
@@ -575,6 +577,10 @@ RunTftp (
FreePool (Handles);
}
+ if ((ShellStatus != SHELL_SUCCESS) && (EFI_ERROR(Status))) {
+ShellStatus = Status & ~MAX_BIT;
+ }
+
return ShellStatus;
}
--
2.20.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] History question about Base.h and its alternate parallel name space.... Should we change it?
Hi Andrew,
I think it makes sense for new code to use the
UEFI type names and for existing code to remain unchanged.
>From looking at UefiBaseTypes.h, I do not think we should
move all types from that file into Base.h. If we really
wanted to do that, we could simple add #include of
UefiBaseTypes.h to Base.h.
Moving a few lines from UefiBaseTypes.h to Base.h at the
bottom of Base.h with a comment block that explains why
the UEFI specific types/defines are available from Base.h
would be another way to add a subset of types/defines.
That way, if we decide to move more in the future, there
is a place to move them that is easy to review.
Do you want to propose the specific types/defines that
should be moved?
Best regards,
Mike
> -Original Message-
> From: af...@apple.com [mailto:af...@apple.com]
> Sent: Tuesday, January 22, 2019 5:00 PM
> To: Kinney, Michael D
> Cc: Felix Poludov ; edk2-
> de...@lists.01.org
> Subject: Re: [edk2] History question about Base.h and
> its alternate parallel name space Should we change
> it?
>
>
>
> > On Jan 22, 2019, at 9:49 AM, Kinney, Michael D
> wrote:
> >
> > Andrew,
> >
> > If we move all of those, then what are the code review
> rules for a lib of type BASE?
> >
> > Is there a preference to use the current BASE types?
> Under what conditions are EFI type names allowed?
> >
> > Is the a preference to stop using the current BASE
> types all together?
> >
>
> Mike,
>
> it seems messy to add types to Base.h to support Base
> Libs. So for example IPv4_ADDRESS and IPv6_ADDRESS got
> sucked into Base.h since an MdePkg Base Lib got added
> that used it. That does not really seem to scale to Base
> Libs in other packages. Thus I guess the preference
> would be to use the common EFI_* names. I'd not advocate
> removing the old names as that is going to break other
> folks Base LIbs. We could do the cleanup in TianoCore.
> If we want to deprecate the old names form, I guess that
> could be an ifdef to allow some depreciation in the
> future?
>
> Thanks,
>
> Andrew Fish
>
> > Thanks,
> >
> > Mike
> >
> >> -Original Message-
> >> From: af...@apple.com [mailto:af...@apple.com]
> >> Sent: Friday, January 18, 2019 9:24 AM
> >> To: Felix Poludov
> >> Cc: Kinney, Michael D ;
> >> edk2-devel@lists.01.org
> >> Subject: Re: [edk2] History question about Base.h and
> >> its alternate parallel name space Should we
> change
> >> it?
> >>
> >>
> >>
> >>> On Jan 18, 2019, at 9:08 AM, Felix Polyudov
> >> wrote:
> >>>
> >>> Mike,
> >>>
> >>> I think EFI_GUID and EFI_STATUS should cover most of
> >> the use cases.
> >>>
> >>
> >> I think I missed a couple in my original mail
> >>
> >> But here are the typedef and #define names that get
> >> remapped (or redone) in
> >> MdePkg/Include/Uefi/UefiBaseType.h
> >>
> >> typedef struct {
> >> UINT32 Data1;
> >> UINT16 Data2;
> >> UINT16 Data3;
> >> UINT8 Data4[8];
> >> } GUID;
> >>
> >> typedef struct {
> >> UINT8 Addr[4];
> >> } IPv4_ADDRESS;
> >>
> >> typedef struct {
> >> UINT8 Addr[16];
> >> } IPv6_ADDRESS;
> >>
> >> typedef UINT64 PHYSICAL_ADDRESS;
> >> typedef UINTN RETURN_STATUS;
> >>
> >> #define ENCODE_ERROR(StatusCode)
> >> ((RETURN_STATUS)(MAX_BIT | (StatusCode)))
> >> #define ENCODE_WARNING(StatusCode)
> >> ((RETURN_STATUS)(StatusCode))
> >> #define RETURN_ERROR(StatusCode)
> >> (((INTN)(RETURN_STATUS)(StatusCode)) < 0)
> >> #define RETURN_SUCCESS 0
> >> #define RETURN_LOAD_ERRORENCODE_ERROR (1)
> >> #define RETURN_INVALID_PARAMETER ENCODE_ERROR (2)
> >>
> >>
> >>
> >> I think the cleanup would be as simple as moving
> things
> >> from MdePkg/Include/Uefi/UefiBaseType.h to
> >> MdePkg/Include/Base.h.
> >>
> >> So:
> >>
> >> typedef struct {
> >> UINT32 Data1;
> >> UINT16 Data2;
> >> UINT16 Data3;
> >> UINT8 Data4[8];
> >> } GUID;
> >>
> >> Becomes:
> >>
> >> typedef struct {
> >> UINT32 Data1;
> >> UINT16 Data2;
> >> UINT16 Data3;
> >> UINT8 Data4[8];
> >> } GUID, EFI_GUID;
> >>
> >> Thanks,
> >>
> >> Andrew Fish
> >>
> >>
> >>> -Original Message-
> >>> From: Kinney, Michael D
> >> [mailto:michael.d.kin...@intel.com]
> >>> Sent: Thursday, January 17, 2019 3:04 PM
> >>> To: Felix Polyudov; 'Andrew Fish'; Kinney, Michael D
> >>> Cc: edk2-devel@lists.01.org
> >>> Subject: RE: [edk2] History question about Base.h
> and
> >> its alternate parallel name space Should we
> change
> >> it?
> >>>
> >>> Felix,
> >>>
> >>> Is there a specific set that would have the most
> >> benefit?
> >>>
> >>> Is EFI_GUID sufficient?
> >>>
> >>> Mike
> >>>
> -Original Message-
> From: Felix Polyudov [mailto:fel...@ami.com]
> Sent: Wednesday, January 16, 2019 3:10 PM
> To: 'Andrew Fish' ; Kinney,
> Michael
> >> D
>
> Cc: edk2-devel@lists.01.org
> Subject: RE: [edk2] History question about Base.h
> and
> its alternate parallel name space Should we
> >> change
> it?
>
> I agree with
Re: [edk2] Network Stack Budgeting
> On Jan 23, 2019, at 8:27 AM, Tomas Pilar (tpilar) > wrote: > > >> The set of devices connected during BDS is platform policy. It is not >> the "network stack" that calls Snp.Start(), but the platform BDS code >> that calls gBS->ConnectController() on the device, possibly without a >> good reason (e.g. without the device being present in a UEFI boot >> option). The network stack only "reacts" to that connection request. > Indeed, but even if a SNP handle is present as a boot option in a boot > manager, surely the Snp.Start() should be deferred until the user actually > chooses to boot from that handle. > Tom, You don't need to call gBS->ConnectController() on all possible boot options, just the one you are currently trying to boot. I mostly muck around in a non edk2 BDS, but in general what you do in a BDS is: 1) Connect your graphics console(s) (usually involves ConOut NVRAM) 3) Connect any serial consoles (ConIn/ConOut NVRAM). 2) Connect any built in keyboard, maybe SPI etc. (ConIn NVRAM). 4) Connect any hot pluggable console devices (Connect any existing USB HID devices). 5) Connect any other device required in the boot path (like the example entropy device. The platform can have policy to force values into ConIn/ConOut. For example on a laptop maybe you always want the built in keyboard active. As you attempt to boot a boot option you can recursively connect the device path for that boot option and attempt to boot it. If that option fails you can fall back to the next boot option and try to connect that device path and boot. Thus you don't need to start things before you are ready. If you launch Firmware Setup that usually does a Connect All. The same things happen when you launch the Shell. Also some drivers connect extra stuff. For example when you try to connect a specific PCI device all the PCI IO handles get created. This is just how you have to enumerate PCI. But the recursive connect should only happen on the PCI IO handle you care about. Thanks, Andrew Fish > A workaround that we have in the legacy implementation doesn't start the > underlying hardware datapath until the platform tries to send the first > packet (since PXE boot is always initiated by the client) but that is a > horrible hack that should not be necessary. The distinction between > Snp.Initialize() and Snp.Start() is there exactly for that reason, no? > > In other words, ConnectController() should not immediately result in > Snp.Start() being called. > > Cheers, > Tom > > > ___ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] parallelism in the module-level, generated GNUmakefile's
Laszlo:
By design, BaseTools generates GNUMakefile with the complete dependency. So,
the generated GNUMakefile should support parallelism run. But, I don't verify
this functionality. Have you found any limitation to forbid parallelism run in
module GNUMakefile?
Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo
> Ersek
> Sent: Tuesday, January 22, 2019 8:55 PM
> To: Gao, Liming
> Cc: edk2-devel-01
> Subject: [edk2] parallelism in the module-level, generated GNUmakefile's
>
> Hi Liming,
>
> I'd like to ask a question about parallel ("multi threaded") make.
>
> The use case is the following:
>
> (1) The edk2 "build" BaseTools utility itself is invoked from a
> *higher-level* Makefile that is outside of the edk2 project.
>
> (2) The outer make process is invoked with "make -jN".
>
> (3) The "build" utility is invoked in isolation. That is, at most *one*
> instance of the "build" utility runs at any given time, despite the
> outer make receiving "-jN". This guarantees that there is no corruption
> in $WORKSPACE/Conf and so on.
>
> (4) The "build" utility is invoked with the "-m" option, to build a
> single module INF.
>
> (5) The "-n" parameter of the "build" utility is not used (and it would
> be useless anyway, since "-n" only parallelizes the build *between* INF
> files, and not *within* INF files, and here we run "build" on a single
> INF file, with "-m").
>
>
> As a result, when the "build" utility invokes the "make" program, on the
> GNUmakefile that was generated by "GenMake.py", there is already a
> higher-level make process that is the (indirect) ancestor of the new
> make process. This is important because the higher level make process
> sets some options in the MAKEFLAGS environment variable, and the inner
> make process inherits those:
>
>outer make --> sets MAKEFLAGS in the environment
>|
>|
>build \
> -a ARCH \
> -p PLATFORM_DSC \
> -t TOOLCHAIN_TAG \
> -b TARGET \
> -m MODULE_INF
>|
>|
>inner make --> inheirts MAKEFLAGS in the environment
>
> Due to (2), the *inner* make inherits the following MAKEFLAGS:
>
> --jobserver-fds=3,4 -j
>
> The "-j" and "--jobserver-fds" options cause the inner make -- which is
> started by "build" -- to communicate with the *outer* make process. The
> goal of this communication is to ensure that no more than 4 jobs are
> active at any given time.
>
> The important part is that, if the "job server" (provided by the outer
> make) *allows* the inner make to run two or more recipes in parallel,
> from the generated GNUMakefile, then the inner make *will do that*. It
> will launch multiple "nasm", "gcc", "iasl" etc commands in parallel.
>
> In my testing, this happens to work fine -- the build completes okay. My
> question is:
>
> - Is this behavior *intentional*?
>
> - In other words: does "GenMake.py" *intentionally* generate
> "GNUmakefile" so that it is compatible with "make -j"? Or does it work
> only by chance, on my end?
>
>
> If the answer is "by chance only", then that is 100% fine with me; I am
> not requesting that we add "make -j" compatibility to "GenMake.py".
> Instead, I'd suggest that we expose this limitation *explicitly* in
> "GenMake.py". For that, the ".NOTPARALLEL" target can be used, and it
> really has to be placed into the generated GNUMakefile:
>
> https://www.gnu.org/software/make/manual/make.html#index-_002eNOTPARALLEL
>
> If we do that, then the inner make will safely ignore the inherited "-j"
> option, for the targets that are listed as prerequisites of .NOTPARALLEL.
>
> And then the problem becomes: how can we collect the full set of
> GNUMakefile targest (so that we can list them all in .NOTPARALLEL)?
> There are many types of build products, from iasl, nasm, VfrCompile,
> etc, beyond the most common "object file from C source".
>
> Thanks!
> Laszlo
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] Network Stack Budgeting
> The set of devices connected during BDS is platform policy. It is not > the "network stack" that calls Snp.Start(), but the platform BDS code > that calls gBS->ConnectController() on the device, possibly without a > good reason (e.g. without the device being present in a UEFI boot > option). The network stack only "reacts" to that connection request. Indeed, but even if a SNP handle is present as a boot option in a boot manager, surely the Snp.Start() should be deferred until the user actually chooses to boot from that handle. A workaround that we have in the legacy implementation doesn't start the underlying hardware datapath until the platform tries to send the first packet (since PXE boot is always initiated by the client) but that is a horrible hack that should not be necessary. The distinction between Snp.Initialize() and Snp.Start() is there exactly for that reason, no? In other words, ConnectController() should not immediately result in Snp.Start() being called. Cheers, Tom ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 2/5] ArmPkg/ArmMmuLib AARCH64: get rid of needless TLB invalidation
On Wed, 23 Jan 2019 at 17:13, Leif Lindholm wrote:
>
> On Wed, Jan 23, 2019 at 04:55:28PM +0100, Ard Biesheuvel wrote:
> > On Wed, 23 Jan 2019 at 16:46, Leif Lindholm
> > wrote:
> > >
> > > On Mon, Jan 07, 2019 at 08:15:01AM +0100, Ard Biesheuvel wrote:
> > > > Currently, we always invalidate the TLBs entirely after making
> > > > any modification to the page tables. Now that we have introduced
> > > > strict memory permissions in quite a number of places, such
> > > > modifications occur much more often, and it is better for performance
> > > > to flush only those TLB entries that are actually affected by
> > > > the changes.
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Ard Biesheuvel
> > > > ---
> > > > ArmPkg/Include/Library/ArmMmuLib.h | 3 ++-
> > > > ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S| 6 +++---
> > > > ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 16
> > > > +++-
> > > > ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S | 14
> > > > --
> > > > 4 files changed, 20 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > > > b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > > > index d66df3e17a02..e1fabfcbea14 100644
> > > > --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > > > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > > > @@ -129,13 +129,14 @@ STATIC
> > > > VOID
> > > > ReplaceLiveEntry (
> > > >IN UINT64 *Entry,
> > > > - IN UINT64 Value
> > > > + IN UINT64 Value,
> > > > + IN UINT64 Address
> > > >)
> > > > {
> > > >if (!ArmMmuEnabled ()) {
> > > > *Entry = Value;
> > > >} else {
> > > > -ArmReplaceLiveTranslationEntry (Entry, Value);
> > > > +ArmReplaceLiveTranslationEntry (Entry, Value, Address);
> > > >}
> > > > }
> > > >
> > > > @@ -296,7 +297,8 @@ GetBlockEntryListFromAddress (
> > > >
> > > > // Fill the BlockEntry with the new TranslationTable
> > > > ReplaceLiveEntry (BlockEntry,
> > > > - ((UINTN)TranslationTable &
> > > > TT_ADDRESS_MASK_DESCRIPTION_TABLE) | TableAttributes |
> > > > TT_TYPE_TABLE_ENTRY);
> > > > + (UINTN)TranslationTable | TableAttributes |
> > > > TT_TYPE_TABLE_ENTRY,
> > > > + RegionStart);
> > >
> >
> > /me pages in the data ...
> >
> > > OK, this whole patch took a few times around the loop before I think I
> > > caught on what was happening.
> > >
> > > I think I'm down to the only things confusing me being:
> > > - The name "Address" to refer to something that is always the start
> > > address of a 4KB-aligned translation region.
> > > Is this because the function will be usable in other contexts in
> > > later patches?
> >
> > I could change it to VirtualAddress if you prefer.
> > It is only passed
> > for TLB maintenance which is only needed at page granularity, and the
> > low bits are shifted out anyway.
>
> Yeah, exactly. It would just be nice if the name reflected that. Not
> sure VirtualAddress does. VirtualBase? PageBase?
>
Well, the argument passed in is called RegionStart, so let's just
stick with that.
> > > - Why drop the & TT_ADDRESS_MASK_DESCRIPTION_TABLE bit here?
> > > Was it just always pointless and you decided to drop it while you
> > > were at it?
> >
> > IIRC yes. It is a newly allocated page, so the masking does not do anything.
>
> Yeah, that's fair enough.
> Just made me wonder if anything unobvious was going on :)
>
> /
> Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 2/5] ArmPkg/ArmMmuLib AARCH64: get rid of needless TLB invalidation
On Wed, Jan 23, 2019 at 05:16:57PM +0100, Ard Biesheuvel wrote:
> On Wed, 23 Jan 2019 at 17:13, Leif Lindholm wrote:
> >
> > On Wed, Jan 23, 2019 at 04:55:28PM +0100, Ard Biesheuvel wrote:
> > > On Wed, 23 Jan 2019 at 16:46, Leif Lindholm
> > > wrote:
> > > >
> > > > On Mon, Jan 07, 2019 at 08:15:01AM +0100, Ard Biesheuvel wrote:
> > > > > Currently, we always invalidate the TLBs entirely after making
> > > > > any modification to the page tables. Now that we have introduced
> > > > > strict memory permissions in quite a number of places, such
> > > > > modifications occur much more often, and it is better for performance
> > > > > to flush only those TLB entries that are actually affected by
> > > > > the changes.
> > > > >
> > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > Signed-off-by: Ard Biesheuvel
> > > > > ---
> > > > > ArmPkg/Include/Library/ArmMmuLib.h | 3 ++-
> > > > > ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S| 6 +++---
> > > > > ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 16
> > > > > +++-
> > > > > ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S | 14
> > > > > --
> > > > > 4 files changed, 20 insertions(+), 19 deletions(-)
> > > > >
> > > > > diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > > > > b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > > > > index d66df3e17a02..e1fabfcbea14 100644
> > > > > --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > > > > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > > > > @@ -129,13 +129,14 @@ STATIC
> > > > > VOID
> > > > > ReplaceLiveEntry (
> > > > >IN UINT64 *Entry,
> > > > > - IN UINT64 Value
> > > > > + IN UINT64 Value,
> > > > > + IN UINT64 Address
> > > > >)
> > > > > {
> > > > >if (!ArmMmuEnabled ()) {
> > > > > *Entry = Value;
> > > > >} else {
> > > > > -ArmReplaceLiveTranslationEntry (Entry, Value);
> > > > > +ArmReplaceLiveTranslationEntry (Entry, Value, Address);
> > > > >}
> > > > > }
> > > > >
> > > > > @@ -296,7 +297,8 @@ GetBlockEntryListFromAddress (
> > > > >
> > > > > // Fill the BlockEntry with the new TranslationTable
> > > > > ReplaceLiveEntry (BlockEntry,
> > > > > - ((UINTN)TranslationTable &
> > > > > TT_ADDRESS_MASK_DESCRIPTION_TABLE) | TableAttributes |
> > > > > TT_TYPE_TABLE_ENTRY);
> > > > > + (UINTN)TranslationTable | TableAttributes |
> > > > > TT_TYPE_TABLE_ENTRY,
> > > > > + RegionStart);
> > > >
> > >
> > > /me pages in the data ...
> > >
> > > > OK, this whole patch took a few times around the loop before I think I
> > > > caught on what was happening.
> > > >
> > > > I think I'm down to the only things confusing me being:
> > > > - The name "Address" to refer to something that is always the start
> > > > address of a 4KB-aligned translation region.
> > > > Is this because the function will be usable in other contexts in
> > > > later patches?
> > >
> > > I could change it to VirtualAddress if you prefer.
> > > It is only passed
> > > for TLB maintenance which is only needed at page granularity, and the
> > > low bits are shifted out anyway.
> >
> > Yeah, exactly. It would just be nice if the name reflected that. Not
> > sure VirtualAddress does. VirtualBase? PageBase?
> >
>
> Well, the argument passed in is called RegionStart, so let's just
> stick with that.
Sure. With that:
Reviewed-by: Leif Lindholm
> > > > - Why drop the & TT_ADDRESS_MASK_DESCRIPTION_TABLE bit here?
> > > > Was it just always pointless and you decided to drop it while you
> > > > were at it?
> > >
> > > IIRC yes. It is a newly allocated page, so the masking does not do
> > > anything.
> >
> > Yeah, that's fair enough.
> > Just made me wonder if anything unobvious was going on :)
> >
> > /
> > Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 2/5] ArmPkg/ArmMmuLib AARCH64: get rid of needless TLB invalidation
On Wed, Jan 23, 2019 at 04:55:28PM +0100, Ard Biesheuvel wrote:
> On Wed, 23 Jan 2019 at 16:46, Leif Lindholm wrote:
> >
> > On Mon, Jan 07, 2019 at 08:15:01AM +0100, Ard Biesheuvel wrote:
> > > Currently, we always invalidate the TLBs entirely after making
> > > any modification to the page tables. Now that we have introduced
> > > strict memory permissions in quite a number of places, such
> > > modifications occur much more often, and it is better for performance
> > > to flush only those TLB entries that are actually affected by
> > > the changes.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Ard Biesheuvel
> > > ---
> > > ArmPkg/Include/Library/ArmMmuLib.h | 3 ++-
> > > ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S| 6 +++---
> > > ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 16
> > > +++-
> > > ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S | 14
> > > --
> > > 4 files changed, 20 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > > b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > > index d66df3e17a02..e1fabfcbea14 100644
> > > --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > > @@ -129,13 +129,14 @@ STATIC
> > > VOID
> > > ReplaceLiveEntry (
> > >IN UINT64 *Entry,
> > > - IN UINT64 Value
> > > + IN UINT64 Value,
> > > + IN UINT64 Address
> > >)
> > > {
> > >if (!ArmMmuEnabled ()) {
> > > *Entry = Value;
> > >} else {
> > > -ArmReplaceLiveTranslationEntry (Entry, Value);
> > > +ArmReplaceLiveTranslationEntry (Entry, Value, Address);
> > >}
> > > }
> > >
> > > @@ -296,7 +297,8 @@ GetBlockEntryListFromAddress (
> > >
> > > // Fill the BlockEntry with the new TranslationTable
> > > ReplaceLiveEntry (BlockEntry,
> > > - ((UINTN)TranslationTable & TT_ADDRESS_MASK_DESCRIPTION_TABLE)
> > > | TableAttributes | TT_TYPE_TABLE_ENTRY);
> > > + (UINTN)TranslationTable | TableAttributes |
> > > TT_TYPE_TABLE_ENTRY,
> > > + RegionStart);
> >
>
> /me pages in the data ...
>
> > OK, this whole patch took a few times around the loop before I think I
> > caught on what was happening.
> >
> > I think I'm down to the only things confusing me being:
> > - The name "Address" to refer to something that is always the start
> > address of a 4KB-aligned translation region.
> > Is this because the function will be usable in other contexts in
> > later patches?
>
> I could change it to VirtualAddress if you prefer.
> It is only passed
> for TLB maintenance which is only needed at page granularity, and the
> low bits are shifted out anyway.
Yeah, exactly. It would just be nice if the name reflected that. Not
sure VirtualAddress does. VirtualBase? PageBase?
> > - Why drop the & TT_ADDRESS_MASK_DESCRIPTION_TABLE bit here?
> > Was it just always pointless and you decided to drop it while you
> > were at it?
>
> IIRC yes. It is a newly allocated page, so the masking does not do anything.
Yeah, that's fair enough.
Just made me wonder if anything unobvious was going on :)
/
Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] Network Stack Budgeting
Hi, On 01/23/19 11:55, Tomas Pilar (tpilar) wrote: > Hi, > > Recently I have done some performance improvements to my network > driver. I am however finding that on some platforms, it's becoming > impossible to boot if the network cable has a lot of traffic on it > that is not filtered by the NIC itself (broadcast, multicast or > directed unicast). It would seem the platform hangs in the DXE phase > trying to process (drop) all the packets and not progressing through > the boot order. > > I am wondering if anyone has seen similar behaviour. Does the network > stack have any budgeting? > > Ideally this would be fixed by the network stack not calling > Snp.Start() until in the BDS phase but it seems most platforms just > call Snp.Start() immediately following the driver probe. this is a platform BDS issue, on those platforms where you see the symptom. The set of devices connected during BDS is platform policy. It is not the "network stack" that calls Snp.Start(), but the platform BDS code that calls gBS->ConnectController() on the device, possibly without a good reason (e.g. without the device being present in a UEFI boot option). The network stack only "reacts" to that connection request. It is convenient for a platform BDS implementation to just connect all drivers to all devices, regardless of what devices are actually going to be booted. Unfortunately, as the number of devices grows (or the traffic grows, as you say), the boot performance worsens. In OVMF and ArmVirtQemu, we saw pretty dramatic gains when we switched from the simple+convenient approach to a bit smarter/selective approach. And once we had fixed all the regressions too :) , it was a net win. (Regressions because, once you switch the policy from "blanket connect" to "specific connect", it is easy to forget about devices that are not really "boot" devices, but you still might want to connect them in every case. Consider a USB keyboard, or a hardware entropy device, for example. After the switch, you have to connect those individually.) Thanks, Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 2/5] ArmPkg/ArmMmuLib AARCH64: get rid of needless TLB invalidation
On Wed, 23 Jan 2019 at 16:46, Leif Lindholm wrote:
>
> On Mon, Jan 07, 2019 at 08:15:01AM +0100, Ard Biesheuvel wrote:
> > Currently, we always invalidate the TLBs entirely after making
> > any modification to the page tables. Now that we have introduced
> > strict memory permissions in quite a number of places, such
> > modifications occur much more often, and it is better for performance
> > to flush only those TLB entries that are actually affected by
> > the changes.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel
> > ---
> > ArmPkg/Include/Library/ArmMmuLib.h | 3 ++-
> > ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S| 6 +++---
> > ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 16
> > +++-
> > ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S | 14
> > --
> > 4 files changed, 20 insertions(+), 19 deletions(-)
> >
> > diff --git a/ArmPkg/Include/Library/ArmMmuLib.h
> > b/ArmPkg/Include/Library/ArmMmuLib.h
> > index fb7fd006417c..d2725810f1c6 100644
> > --- a/ArmPkg/Include/Library/ArmMmuLib.h
> > +++ b/ArmPkg/Include/Library/ArmMmuLib.h
> > @@ -59,7 +59,8 @@ VOID
> > EFIAPI
> > ArmReplaceLiveTranslationEntry (
> >IN UINT64 *Entry,
> > - IN UINT64 Value
> > + IN UINT64 Value,
> > + IN UINT64 Address
> >);
> >
> > EFI_STATUS
> > diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> > b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> > index b7173e00b039..175fb58206b6 100644
> > --- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> > +++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> > @@ -124,15 +124,15 @@ ASM_FUNC(ArmSetMAIR)
> > // IN VOID *MVA// X1
> > // );
> > ASM_FUNC(ArmUpdateTranslationTableEntry)
> > - dc civac, x0 // Clean and invalidate data line
> > - dsb sy
> > + dsb nshst
> > + lsr x1, x1, #12
> > EL1_OR_EL2_OR_EL3(x0)
> > 1: tlbivaae1, x1 // TLB Invalidate VA , EL1
> > b 4f
> > 2: tlbivae2, x1 // TLB Invalidate VA , EL2
> > b 4f
> > 3: tlbivae3, x1 // TLB Invalidate VA , EL3
> > -4: dsb sy
> > +4: dsb nsh
> > isb
> > ret
> >
> > diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > index d66df3e17a02..e1fabfcbea14 100644
> > --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > @@ -129,13 +129,14 @@ STATIC
> > VOID
> > ReplaceLiveEntry (
> >IN UINT64 *Entry,
> > - IN UINT64 Value
> > + IN UINT64 Value,
> > + IN UINT64 Address
> >)
> > {
> >if (!ArmMmuEnabled ()) {
> > *Entry = Value;
> >} else {
> > -ArmReplaceLiveTranslationEntry (Entry, Value);
> > +ArmReplaceLiveTranslationEntry (Entry, Value, Address);
> >}
> > }
> >
> > @@ -296,7 +297,8 @@ GetBlockEntryListFromAddress (
> >
> > // Fill the BlockEntry with the new TranslationTable
> > ReplaceLiveEntry (BlockEntry,
> > - ((UINTN)TranslationTable & TT_ADDRESS_MASK_DESCRIPTION_TABLE) |
> > TableAttributes | TT_TYPE_TABLE_ENTRY);
> > + (UINTN)TranslationTable | TableAttributes | TT_TYPE_TABLE_ENTRY,
> > + RegionStart);
>
/me pages in the data ...
> OK, this whole patch took a few times around the loop before I think I
> caught on what was happening.
>
> I think I'm down to the only things confusing me being:
> - The name "Address" to refer to something that is always the start
> address of a 4KB-aligned translation region.
> Is this because the function will be usable in other contexts in
> later patches?
I could change it to VirtualAddress if you prefer. It is only passed
for TLB maintenance which is only needed at page granularity, and the
low bits are shifted out anyway.
> - Why drop the & TT_ADDRESS_MASK_DESCRIPTION_TABLE bit here?
> Was it just always pointless and you decided to drop it while you
> were at it?
>
IIRC yes. It is a newly allocated page, so the masking does not do anything.
> /
> Leif
>
> >}
> > } else {
> >if (IndexLevel != PageLevel) {
> > @@ -375,6 +377,8 @@ UpdateRegionMapping (
> >*BlockEntry &= BlockEntryMask;
> >*BlockEntry |= (RegionStart & TT_ADDRESS_MASK_BLOCK_ENTRY) |
> > Attributes | Type;
> >
> > + ArmUpdateTranslationTableEntry (BlockEntry, (VOID *)RegionStart);
> > +
> >// Go to the next BlockEntry
> >RegionStart += BlockEntrySize;
> >RegionLength -= BlockEntrySize;
> > @@ -487,9 +491,6 @@ ArmSetMemoryAttributes (
> > return Status;
> >}
> >
> > - // Invalidate all TLB entries so changes are synced
> > - ArmInvalidateTlb ();
> > -
> >return EFI_SUCCESS;
> > }
> >
> > @@ -512,9 +513,6 @@ SetMemoryRegionAttribute (
> > return Status;
> >}
> >
> > - //
Re: [edk2] A VM failed to boot when I changed the gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size
Hi,
(adding Alex, Gerd, Dave)
On 01/23/19 12:40, Wuzongyong (Euler Dept) wrote:
>
> Hi,
>
> Recently I do a test with edk2 on rhel platform.
Cool :) I don't frequently see RHEL-related reports on this list.
> I resized the gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size value to 1TB for
> supporting multiple GPUs passthrough which have large bars .
Out of curiosity, did you modify the PCD directly (that is, by changing
the DSC file, or using the --pcd build option), or else, did you use the
QEMU switch
-fw_cfg name=opt/ovmf/X-PciMmio64Mb,string=1048576
that was described in commit 7e5b1b670c38 ("OvmfPkg: PlatformPei:
determine the 64-bit PCI host aperture for X64 DXE", 2016-03-23)?
> But when I started a VM with a virtio nic and booted from the changed OVMF,
Ah, OK, you wrote "changed OVMF". I guess you modified the DSC then.
So, for a bit more convenience, see above: the aperture size can be set
on the QEMU command line too. The fw_cfg file name carries "X-" in the
name so that it's clear that the knob is experimental and might change
in the future incompatibly.
> it seems the VM failed to boot before loading ipxe.
> The uefi error log is like this:
>
> X64 Exception Type - 0E(#PF - Page-Fault) CPU Apic ID -
> ExceptionData - 000B I:0 R:1 U:0 W:1 P:1 PK:0 S:0
> RIP - BE4AD7F7, CS - 0038, RFLAGS - 00010206
> RAX - , RCX - 0014, RDX - 0114
> RBX - BE4BEFE0, RSP - BFEDE6F8, RBP - BE4BEFF0
> RSI - BE4BEFF0, RDI - BE4BEFE0
> R8 - , R9 - , R10 -
> R11 - BE4BB900, R12 - BE4BEFD0, R13 - 0060
> R14 - 0084, R15 - 0070
> DS - 0030, ES - 0030, FS - 0030
> GS - 0030, SS - 0030
> CR0 - 80010033, CR2 - 0114, CR3 - BF6BA000
> CR4 - 0668, CR8 -
> DR0 - , DR1 - , DR2 -
> DR3 - , DR6 - 0FF0, DR7 - 0400
> GDTR - BF6A8A98 0047, LDTR -
> IDTR - BF29E018 0FFF, TR -
> FXSAVE_STATE - BFEDE350
> Find image 1af41000.efidrv (ImageBase=BE499000,
> EntryPoint=BE49F1EB)
"1af41000.efidrv" is the iPXE UEFI option ROM for the virtio-net NIC.
QEMU loads the combined BIOS+UEFI option ROM from an external file into
the ROM BAR of the virtio-net NIC, and then the PciBusDxe driver built
into OVMF makes sure the driver is dispatched. (The dispatch is deferred
until after EndOfDxe, in BDS.)
The external file that provides this ROM image may be known under
different pathnames. On RHEL7 for example, it is installed as
"/usr/share/ipxe/1af41000.rom", as part of the ipxe-roms-qemu package;
however QEMU loads it through the symlink
"/usr/share/qemu-kvm/pxe-virtio.rom" that is part of the qemu-kvm-rhev
package.
If you use upstream QEMU, then the file is
"$prefix/share/qemu/efi-virtio.rom".
(1) Either way, if you'd like to check whether this issue is specific to
the iPXE option ROM, you could prevent QEMU from loading the ROM image
into the NIC's ROM BAR, and retest. The QEMU option for that is
-device virtio-net-pci,[some properties,]romfile=''
The corresponging domain XML element is
In this case, virtio-net NICs will be bound by OVMF's built-in
virtio-net driver (OvmfPkg/VirtioNetDxe).
> But when I decrease the PcdPciMmio64Size to 10 * 32 GB, the VM booted
> successfully.
> I'm not familiar with uefi, could you please point out what wrong I have done?
(2) One important factor to check (on your host) is:
$ grep 'address sizes' /proc/cpuinfo
Because, the size that you choose for the 64-bit PCI MMIO aperture
influences the total address space size (the "address width") for which
the DXE IPL PEIM in OVMF has to create page tables (1:1
virtual->physical mapping). When using an aperture >= 1TB, this address
width is at least 40.
And, if you use KVM with nested paging enabled ("ept" on Intel, "npt" on
AMD), *but* the physical address size of your processor is *smaller*
than 40 (i.e. smaller than the address width required by the guest),
then accesses to high addresses in the guest will silently fail (usually
with very bad results).
(3) If you confirm that your physical CPU has a phys addr size that is
large enough, then another test could be to enable 1GB pages in the VCPU
model. Pass
-cpu [whatever model you already use],+pdpe1gb
The equivalent libvirt domain XML fragment would be
(4) Assuming you are testing upstream OVMF, if you rebuild it with the
following switch:
--pcd gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel=0x8040004F
and then boot it on QEMU with
-global isa-debugcon.iobase=0x402 -debugcon file:ovmf.log
then I might be
Re: [edk2] [PATCH 2/5] ArmPkg/ArmMmuLib AARCH64: get rid of needless TLB invalidation
On Mon, Jan 07, 2019 at 08:15:01AM +0100, Ard Biesheuvel wrote:
> Currently, we always invalidate the TLBs entirely after making
> any modification to the page tables. Now that we have introduced
> strict memory permissions in quite a number of places, such
> modifications occur much more often, and it is better for performance
> to flush only those TLB entries that are actually affected by
> the changes.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel
> ---
> ArmPkg/Include/Library/ArmMmuLib.h | 3 ++-
> ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S| 6 +++---
> ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 16
> +++-
> ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S | 14 --
> 4 files changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/ArmPkg/Include/Library/ArmMmuLib.h
> b/ArmPkg/Include/Library/ArmMmuLib.h
> index fb7fd006417c..d2725810f1c6 100644
> --- a/ArmPkg/Include/Library/ArmMmuLib.h
> +++ b/ArmPkg/Include/Library/ArmMmuLib.h
> @@ -59,7 +59,8 @@ VOID
> EFIAPI
> ArmReplaceLiveTranslationEntry (
>IN UINT64 *Entry,
> - IN UINT64 Value
> + IN UINT64 Value,
> + IN UINT64 Address
>);
>
> EFI_STATUS
> diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> index b7173e00b039..175fb58206b6 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> +++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> @@ -124,15 +124,15 @@ ASM_FUNC(ArmSetMAIR)
> // IN VOID *MVA// X1
> // );
> ASM_FUNC(ArmUpdateTranslationTableEntry)
> - dc civac, x0 // Clean and invalidate data line
> - dsb sy
> + dsb nshst
> + lsr x1, x1, #12
> EL1_OR_EL2_OR_EL3(x0)
> 1: tlbivaae1, x1 // TLB Invalidate VA , EL1
> b 4f
> 2: tlbivae2, x1 // TLB Invalidate VA , EL2
> b 4f
> 3: tlbivae3, x1 // TLB Invalidate VA , EL3
> -4: dsb sy
> +4: dsb nsh
> isb
> ret
>
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> index d66df3e17a02..e1fabfcbea14 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> @@ -129,13 +129,14 @@ STATIC
> VOID
> ReplaceLiveEntry (
>IN UINT64 *Entry,
> - IN UINT64 Value
> + IN UINT64 Value,
> + IN UINT64 Address
>)
> {
>if (!ArmMmuEnabled ()) {
> *Entry = Value;
>} else {
> -ArmReplaceLiveTranslationEntry (Entry, Value);
> +ArmReplaceLiveTranslationEntry (Entry, Value, Address);
>}
> }
>
> @@ -296,7 +297,8 @@ GetBlockEntryListFromAddress (
>
> // Fill the BlockEntry with the new TranslationTable
> ReplaceLiveEntry (BlockEntry,
> - ((UINTN)TranslationTable & TT_ADDRESS_MASK_DESCRIPTION_TABLE) |
> TableAttributes | TT_TYPE_TABLE_ENTRY);
> + (UINTN)TranslationTable | TableAttributes | TT_TYPE_TABLE_ENTRY,
> + RegionStart);
OK, this whole patch took a few times around the loop before I think I
caught on what was happening.
I think I'm down to the only things confusing me being:
- The name "Address" to refer to something that is always the start
address of a 4KB-aligned translation region.
Is this because the function will be usable in other contexts in
later patches?
- Why drop the & TT_ADDRESS_MASK_DESCRIPTION_TABLE bit here?
Was it just always pointless and you decided to drop it while you
were at it?
/
Leif
>}
> } else {
>if (IndexLevel != PageLevel) {
> @@ -375,6 +377,8 @@ UpdateRegionMapping (
>*BlockEntry &= BlockEntryMask;
>*BlockEntry |= (RegionStart & TT_ADDRESS_MASK_BLOCK_ENTRY) |
> Attributes | Type;
>
> + ArmUpdateTranslationTableEntry (BlockEntry, (VOID *)RegionStart);
> +
>// Go to the next BlockEntry
>RegionStart += BlockEntrySize;
>RegionLength -= BlockEntrySize;
> @@ -487,9 +491,6 @@ ArmSetMemoryAttributes (
> return Status;
>}
>
> - // Invalidate all TLB entries so changes are synced
> - ArmInvalidateTlb ();
> -
>return EFI_SUCCESS;
> }
>
> @@ -512,9 +513,6 @@ SetMemoryRegionAttribute (
> return Status;
>}
>
> - // Invalidate all TLB entries so changes are synced
> - ArmInvalidateTlb ();
> -
>return EFI_SUCCESS;
> }
>
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> index 90192df24f55..d40c19b2e3e5 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> @@ -32,11 +32,12 @@
>dmb sy
>dcivac, x0
>
> - // flush the TLBs
> + // flush translations for the target address from the TLBs
> + lsr x2,
Re: [edk2] [PATCH] SD/eMMC : Fix Command Argument for SD/eMMC R/W operation.
Well, if we don't hear back, I can just commit it anyway before the
end of the week. One question/comment inline below:
On Tue, Jan 22, 2019 at 04:29:50AM +, Meenakshi Aggarwal wrote:
> Hi Jun, Haojian,
>
> Please review the patch.
>
> Thanks,
> Meenakshi
>
> > -Original Message-
> > From: Leif Lindholm
> > Sent: Thursday, January 17, 2019 4:54 PM
> > To: Meenakshi Aggarwal
> > Cc: ard.biesheu...@linaro.org; edk2-devel@lists.01.org; Jun Nie
> > ; Haojian Zhuang
> > Subject: Re: [PATCH] SD/eMMC : Fix Command Argument for SD/eMMC R/W
> > operation.
> >
> > Jun, Haojian - any comments?
> >
> > On Wed, Jan 16, 2019 at 06:51:36PM +0530, Meenakshi Aggarwal wrote:
> > > Issue : SD read failure for high capacity cards e.g. 64 GB i Reason :
> > > Command argument value exceeds 32 bit for block number 0x3787FFF and
> > > cant be fit into 32 bit wide SD host controller register.
> > >
> > > Fix :
> > > AccessMode bits [29:30] of OCR is a valid definition to calculate data
> > > address for eMMC cards.
> > >
> > > For SD cards, data address is calculated on the basis of card capacity
> > > status bit[30] of OCR.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Meenakshi Aggarwal
> > > ---
> > > EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c | 19 ++-
> > > 1 file changed, 14 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
> > > b/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
> > > index a2b9232..625a59e 100644
> > > --- a/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
> > > +++ b/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
> > > @@ -148,12 +148,21 @@ MmcTransferBlock (
> > >MmcHostInstance = MMC_HOST_INSTANCE_FROM_BLOCK_IO_THIS (This);
> > >MmcHost = MmcHostInstance->MmcHost;
> > >
> > > - //Set command argument based on the card access mode (Byte mode or
> > > Block mode)
> > > - if ((MmcHostInstance->CardInfo.OCRData.AccessMode &
> > MMC_OCR_ACCESS_MASK) ==
> > > - MMC_OCR_ACCESS_SECTOR) {
> > > -CmdArg = Lba;
> > > + if (MmcHostInstance->CardInfo.CardType != EMMC_CARD) {
> > > +//Set command argument based on the card capacity (SDSC or SDXC/SDHC)
> > > +if (MmcHostInstance->CardInfo.OCRData.AccessMode & BIT1) {
What is BIT1? Can we add a #define for it?
(The comment _nearly_ but not quite explains it to me.)
/
Leif
> > > + CmdArg = Lba;
> > > +} else {
> > > + CmdArg = Lba * This->Media->BlockSize;
> > > +}
> > >} else {
> > > -CmdArg = Lba * This->Media->BlockSize;
> > > +//Set command argument based on the card access mode (Byte mode or
> > Block mode)
> > > +if ((MmcHostInstance->CardInfo.OCRData.AccessMode &
> > MMC_OCR_ACCESS_MASK) ==
> > > +MMC_OCR_ACCESS_SECTOR) {
> > > + CmdArg = Lba;
> > > +} else {
> > > + CmdArg = Lba * This->Media->BlockSize;
> > > +}
> > >}
> > >
> > >Status = MmcHost->SendCommand (MmcHost, Cmd, CmdArg);
> > > --
> > > 1.9.1
> > >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] Network Stack Budgeting
Hi Ting, Currently we see it on DELL 13G platforms. However, in my experience most platforms will call Snp.Start() immediatelly following the ConnectController() way before the boot manager is entered. Cheers, Tom On 23/01/2019 14:14, Ye, Ting wrote: > Hi Tom, > > As I known it is up to platform BDS when to connect network stack, or even > not to connect network stack. For example, in fast boot process, network > stack will not be connected thus Snp.Start() has no chance to be called. > May I know which platforms you see this issue? > > Thanks, > Ting > > > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Tomas > Pilar (tpilar) > Sent: Wednesday, January 23, 2019 6:56 PM > To: edk2-devel@lists.01.org > Subject: [edk2] Network Stack Budgeting > > Hi, > > Recently I have done some performance improvements to my network driver. I am > however finding that on some platforms, it's becoming impossible to boot if > the network cable has a lot of traffic on it that is not filtered by the NIC > itself (broadcast, multicast or directed unicast). It would seem the platform > hangs in the DXE phase trying to process (drop) all the packets and not > progressing through the boot order. > > I am wondering if anyone has seen similar behaviour. Does the network stack > have any budgeting? > > Ideally this would be fixed by the network stack not calling Snp.Start() > until in the BDS phase but it seems most platforms just call Snp.Start() > immediately following the driver probe. > > Cheers, > Tom > > > ___ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V2 10/15] ArmVirtXen: Use merged variable driver for emulated NV mode
On 2019/1/23 20:15, Julien Grall wrote:
On 23/01/2019 01:41, Zeng, Star wrote:
Hi Julien,
Hi Star,
On 2019/1/22 12:30, Zeng, Star wrote:
On 2019/1/22 3:40, Ard Biesheuvel wrote:
On Mon, 21 Jan 2019 at 14:36, Julien Grall
wrote:
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
index a8bb9cf25ebd..adaf6ccb48b0 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
@@ -236,14 +236,16 @@ VariableClassAddressChangeEvent (
{
UINTN Index;
- EfiConvertPointer (0x0, (VOID **)
>FvbInstance->GetBlockSize);
- EfiConvertPointer (0x0, (VOID **)
>FvbInstance->GetPhysicalAddress);
- EfiConvertPointer (0x0, (VOID **)
>FvbInstance->GetAttributes);
- EfiConvertPointer (0x0, (VOID **)
>FvbInstance->SetAttributes);
- EfiConvertPointer (0x0, (VOID **)
>FvbInstance->Read);
- EfiConvertPointer (0x0, (VOID **)
>FvbInstance->Write);
- EfiConvertPointer (0x0, (VOID **)
>FvbInstance->EraseBlocks);
- EfiConvertPointer (0x0, (VOID **)
>FvbInstance);
+ if (mVariableModuleGlobal->FvbInstance != NULL) {
+ EfiConvertPointer (0x0, (VOID **)
>FvbInstance->GetBlockSize);
+ EfiConvertPointer (0x0, (VOID **)
>FvbInstance->GetPhysicalAddress);
+ EfiConvertPointer (0x0, (VOID **)
>FvbInstance->GetAttributes);
+ EfiConvertPointer (0x0, (VOID **)
>FvbInstance->SetAttributes);
+ EfiConvertPointer (0x0, (VOID **)
>FvbInstance->Read);
+ EfiConvertPointer (0x0, (VOID **)
>FvbInstance->Write);
+ EfiConvertPointer (0x0, (VOID **)
>FvbInstance->EraseBlocks);
+ EfiConvertPointer (0x0, (VOID **)
>FvbInstance);
+ }
EfiConvertPointer (0x0, (VOID **)
>PlatformLangCodes);
EfiConvertPointer (0x0, (VOID **)
>LangCodes);
EfiConvertPointer (0x0, (VOID **)
>PlatformLang);
Thanks Ard. I integrated it into the patch 10 of V4.
Repo: g...@github.com:lzeng14/edk2.git
Branch: MergedVariableDriver_EmuNvMode_V4
Julien, could you help take a try?
Sorry for a little push. Are you able to have a quick try? :)
Sorry for the late, I didn't have time yesterday to test at it.
I tried the new branch and was able to boot a Linux guest using UEFI.
Feel free to add my tags to the series:
Tested-by: Julien Grall
Acked-by: Julien Grall
It is really very helpful to make the patch higher quality.
Many thanks to you, Ard and Laszlo for the feedback.
Thanks all of you.
Star
Best regards,
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] Network Stack Budgeting
Hi Tom, As I known it is up to platform BDS when to connect network stack, or even not to connect network stack. For example, in fast boot process, network stack will not be connected thus Snp.Start() has no chance to be called. May I know which platforms you see this issue? Thanks, Ting -Original Message- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Tomas Pilar (tpilar) Sent: Wednesday, January 23, 2019 6:56 PM To: edk2-devel@lists.01.org Subject: [edk2] Network Stack Budgeting Hi, Recently I have done some performance improvements to my network driver. I am however finding that on some platforms, it's becoming impossible to boot if the network cable has a lot of traffic on it that is not filtered by the NIC itself (broadcast, multicast or directed unicast). It would seem the platform hangs in the DXE phase trying to process (drop) all the packets and not progressing through the boot order. I am wondering if anyone has seen similar behaviour. Does the network stack have any budgeting? Ideally this would be fixed by the network stack not calling Snp.Start() until in the BDS phase but it seems most platforms just call Snp.Start() immediately following the driver probe. Cheers, Tom ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU
On Wed, 23 Jan 2019 at 10:55, Laszlo Ersek wrote:
>
> On 01/23/19 10:26, Ard Biesheuvel wrote:
> > On Wed, 23 Jan 2019 at 10:14, Laszlo Ersek wrote:
> >> On 01/22/19 16:37, Ard Biesheuvel wrote:
>
> >>> Is SetUefiImageMemoryAttributes() being
> >>> called to remap the memory R-X ?
> >>
> >> No, it is not; the grub binary in question doesn't have the required
> >> section alignment (... I hope at least that that's what your question
> >> refers to):
> >>
> >>> ProtectUefiImageCommon - 0x3E6C54C0
> >>> - 0x00013BEEF000 - 0x00030600
> >>> ProtectUefiImageCommon - Section Alignment(0x200) is
> >> incorrect
> >>
> >
> > This is puzzling, given that the exact same binary works on Mustang.
>
> And even on the original (unspecified) hardware, the same binary works
> frequently. My understanding is that there are five VMs executing reboot
> loops in parallel, on the same host, and 4 out of 5 may hit the issue in
> a reasonable time period (300 reboots or so).
>
> > So when loaded, GRUB should cover the following regions:
> >
> > 0x13beef - 0x13bf00 (0x11000)
> > 0x13bf00 - 0x13bf01f600 (0x1f600)
> >
> > where neither covers a 2 MB block fully, which means that the TLB
> > entry that we are hitting is stale.
> >
> > Since ProtectUefiImageCommon() does not do anything in this case, the
> > stale translation must be the result of
> > PcdDxeNxMemoryProtectionPolicy, which either sets the wrong
> > permissions for EfiLoaderCode (relying on ProtectUefiImageCommon), or
> > we don't flush the TLBs correctly after updating the permissions when
> > converting the memory from EfiConventionalMemory to EfiLoaderCode
> >
> > Are you using the default value for PcdDxeNxMemoryProtectionPolicy?
>
> Yes, we have
>
> ArmVirtPkg/ArmVirt.dsc.inc:
> gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC0007FD1
>
> from commit 1acd7c54a724 ("ArmVirtPkg AARCH64: enable NX memory
> protection for all platforms", 2017-03-01).
>
> The binary is from the RPM
> "edk2-aarch64-20180508gitee3198e672e2-5.el8+1789+f0947240.noarch", which
> is basically upstream ee3198e672e2 plus a small number of backports and
> downstream customizations.
>
This might help:
diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
index b7173e00b039..4c0b4b4efbd5 100644
--- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
+++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
@@ -138,7 +138,7 @@ ASM_FUNC(ArmUpdateTranslationTableEntry)
ASM_FUNC(ArmInvalidateTlb)
EL1_OR_EL2_OR_EL3(x0)
-1: tlbi vmalle1
+1: tlbi vmalle1is
b 4f
2: tlbi alle2
b 4f
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
index 90192df24f55..d54b1c19accf 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
@@ -34,7 +34,7 @@
// flush the TLBs
.if \el == 1
- tlbi vmalle1
+ tlbi vmalle1is
.else
tlbi alle\el
.endif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] Drop CSM support in OvmfPkg?
On Wed, 23 Jan 2019 at 10:55, Laszlo Er > > That's going to be a hard thing to keep from happening over time, as > > this is valid C :(sek wrote: > > Hi All, > > On 01/23/19 04:43, Ni, Ray wrote: > >> -Original Message- > >> From: David Woodhouse > >> Sent: Wednesday, January 23, 2019 12:23 AM > >> To: Ni, Ray ; Gerd Hoffmann ; > >> Laszlo Ersek ; Richardson, Brian > >> > >> Cc: Justen, Jordan L ; edk2-devel@lists.01.org; > >> Kevin O'Connor ; Anthony Perard > >> > >> Subject: Re: Drop CSM support in OvmfPkg? > >> > >> On Tue, 2019-01-22 at 16:13 +, Ni, Ray wrote: > >>> David, > >>> I'd like to re-start the discussion. > >>> Could you please kindly explain the background/reason of adding CSM > >>> support in OVMF? > >>> Maybe knowing the reason can help to make further decisions of > >>> whether to > >>> A. keep it outside OvmfPkg > >>> B. keep it inside OvmfPkg > >>> C. maybe have a chance to just remove the CSM support after > >>> revisiting > >> > >> > >> The idea was to make it simple to have a single firmware image for > >> virtual machines which would support both UEFI and Legacy boot for > >> guests simultaneously. > >> > >> In libvirt there has been an alternative approach, where the BIOS image > >> is switched between OVMF and SeaBIOS according to the configuration of > >> the guest VM. > >> > >> That's fine for libvirt, but in situations where VM hosting is provided > >> as a service, it becomes quite painful to manage the 'UEFI' vs. > >> 'Legacy' flags on guest images and then switch firmware images > >> accordingly. A one-size-fits-all BIOS using OVMF+CSM is very much > >> preferable. > > > > David, > > Thanks for sharing. I now understand that you do have a need of > > CSM + UEFI OVMF image. > > A very straightforward idea is to move all COM components you needed > > into OvmfPkg. But Laszlo as the OvmfPkg owner may disagree with this. > > So maybe you could set up another (github) repo and clone all the CSM > > components > > there. > > EDKII build tool supports to build firmware from multiple repos. > > That's how we can have edk2-platforms and to-be-created edk2-app. > > In practical, you could create a new csm repo. > > Laszlo/Gerd who don't care about CSM can just build OVMF image from edk2 > > repo. > > You can build the OVMF image from edk2 and csm repo. > > > > We can have a call if you are ok. I can explain how that can work in > > details. > > I'm fine if we move the generic CSM components into OvmfPkg, however I'm > going to ask David to assume reviewer responsibilities for them. > > Given the current format of "Maintainers.txt", we couldn't spell out the > exact pathnames of the CSM components, so we'd add a line like > > R: David Woodhouse > > under OvmfPkg. There is "prior art" for this pattern, see: > > R: Anthony Perard > R: Julien Grall > > Because Anthony and Julien are the authority on Xen-related code under > OvmfPkg. (See commit 337fe6a06eda, "Maintainers.txt: add Xen reviewers > to OvmfPkg", 2017-09-26.) > > > If we keep CSM support in OvmfPkg in any form at all, then I would > prefer holding all the related stuff in the core edk2 repository (with > the above Reviewership), over requiring people to deal with multiple > repositories. I agree (from experience) that PACKAGES_PATH / multiple > workspaces work fine, but in this case I think keeping one shared > history is an advantage. > I don't have an opinion on whether we should keep CSM or not in OvmfPkg. But if we do, I agree we should just move all the pieces we rely on and that are getting dropped into OvmfPkg/ proper, rather than keeping them on the side somewhere. ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V2 10/15] ArmVirtXen: Use merged variable driver for emulated NV mode
On 23/01/2019 01:41, Zeng, Star wrote:
Hi Julien,
Hi Star,
On 2019/1/22 12:30, Zeng, Star wrote:
On 2019/1/22 3:40, Ard Biesheuvel wrote:
On Mon, 21 Jan 2019 at 14:36, Julien Grall wrote:
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
index a8bb9cf25ebd..adaf6ccb48b0 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
@@ -236,14 +236,16 @@ VariableClassAddressChangeEvent (
{
UINTN Index;
- EfiConvertPointer (0x0, (VOID **)
>FvbInstance->GetBlockSize);
- EfiConvertPointer (0x0, (VOID **)
>FvbInstance->GetPhysicalAddress);
- EfiConvertPointer (0x0, (VOID **)
>FvbInstance->GetAttributes);
- EfiConvertPointer (0x0, (VOID **)
>FvbInstance->SetAttributes);
- EfiConvertPointer (0x0, (VOID **) >FvbInstance->Read);
- EfiConvertPointer (0x0, (VOID **)
>FvbInstance->Write);
- EfiConvertPointer (0x0, (VOID **)
>FvbInstance->EraseBlocks);
- EfiConvertPointer (0x0, (VOID **) >FvbInstance);
+ if (mVariableModuleGlobal->FvbInstance != NULL) {
+ EfiConvertPointer (0x0, (VOID **)
>FvbInstance->GetBlockSize);
+ EfiConvertPointer (0x0, (VOID **)
>FvbInstance->GetPhysicalAddress);
+ EfiConvertPointer (0x0, (VOID **)
>FvbInstance->GetAttributes);
+ EfiConvertPointer (0x0, (VOID **)
>FvbInstance->SetAttributes);
+ EfiConvertPointer (0x0, (VOID **)
>FvbInstance->Read);
+ EfiConvertPointer (0x0, (VOID **)
>FvbInstance->Write);
+ EfiConvertPointer (0x0, (VOID **)
>FvbInstance->EraseBlocks);
+ EfiConvertPointer (0x0, (VOID **) >FvbInstance);
+ }
EfiConvertPointer (0x0, (VOID **)
>PlatformLangCodes);
EfiConvertPointer (0x0, (VOID **) >LangCodes);
EfiConvertPointer (0x0, (VOID **) >PlatformLang);
Thanks Ard. I integrated it into the patch 10 of V4.
Repo: g...@github.com:lzeng14/edk2.git
Branch: MergedVariableDriver_EmuNvMode_V4
Julien, could you help take a try?
Sorry for a little push. Are you able to have a quick try? :)
Sorry for the late, I didn't have time yesterday to test at it.
I tried the new branch and was able to boot a Linux guest using UEFI.
Feel free to add my tags to the series:
Tested-by: Julien Grall
Acked-by: Julien Grall
Many thanks to you, Ard and Laszlo for the feedback.
Best regards,
--
Julien Grall
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch_1] make EDK Driver Support BH720+EMMC chip
Mike:
Please refer to
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process
and contribute the patch.
And, please work on the patch in edk2 master instead of branch.
Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Mike
> Li (WH)
> Sent: Wednesday, January 23, 2019 5:29 PM
> To: Xiaoguang Yu (WH) ; Ernest Zhang(WH)
> ; Andy Dai (WH)
>
> Cc: edk2-devel@lists.01.org; Shirley Her (SC)
> Subject: [edk2] [patch_1] make EDK Driver Support BH720+EMMC chip
>
>
> Hi, all
>
> The following modifications are made to enable EDK Driver Support BH720 CHIP.
>
> ---
> /c/MyWorkspace/edk2-vUDK2018/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
>2019-01-10
> 14:35:21.342736200 -0800
> +++ /c/yx/edk2/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> 2019-01-21 15:36:12.195715300 -0800
> @@ -4,6 +4,7 @@
>It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
> + Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
>Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD
> License
> @@ -16,8 +17,6 @@
> **/
> #include "SdMmcPciHcDxe.h"
> -int g_deviceId = 0;
> -
> /**
>Dump the content of SD/MMC host controller's Capability Register.
> @@ -47,7 +46,8 @@ DumpCapabilityReg (
>DEBUG ((DEBUG_INFO, " Voltage 3.3 %a\n", Capability->Voltage33 ?
> "TRUE" : "FALSE"));
>DEBUG ((DEBUG_INFO, " Voltage 3.0 %a\n", Capability->Voltage30 ?
> "TRUE" : "FALSE"));
>DEBUG ((DEBUG_INFO, " Voltage 1.8 %a\n", Capability->Voltage18 ?
> "TRUE" : "FALSE"));
> - DEBUG ((DEBUG_INFO, " 64-bit Sys Bus%a\n", Capability->SysBus64 ?
> "TRUE" : "FALSE"));
> + DEBUG ((DEBUG_INFO, " V4 64-bit Sys Bus %a\n", Capability->SysBus64V4 ?
> "TRUE" : "FALSE"));
> + DEBUG ((DEBUG_INFO, " V3 64-bit Sys Bus %a\n", Capability->SysBus64V3 ?
> "TRUE" : "FALSE"));
>DEBUG ((DEBUG_INFO, " Async Interrupt %a\n", Capability->AsyncInt ?
> "TRUE" : "FALSE"));
>DEBUG ((DEBUG_INFO, " SlotType "));
>if (Capability->SlotType == 0x00) {
> @@ -419,9 +419,39 @@ SdMmcHcWaitMmioSet (
> }
> /**
> + Get the controller version information from the specified slot.
> +
> + @param[in] PciIo The PCI IO protocol instance.
> + @param[in] SlotThe slot number of the SD card to send the
> command to.
> + @param[out] Version The buffer to store the version information.
> +
> + @retval EFI_SUCCESS The operation executes successfully.
> + @retval Others The operation fails.
> +
> +**/
> +EFI_STATUS
> +SdMmcHcGetControllerVersion (
> + IN EFI_PCI_IO_PROTOCOL *PciIo,
> + IN UINT8Slot,
> + OUTUINT16 *Version
> + )
> +{
> + EFI_STATUSStatus;
> +
> + Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_CTRL_VER, TRUE, sizeof
> (UINT16), Version);
> + if (EFI_ERROR (Status)) {
> +return Status;
> + }
> +
> + *Version &= 0xFF;
> +
> + return EFI_SUCCESS;
> +}
> +
> +/**
>Software reset the specified SD/MMC host controller and enable all
> interrupts.
> - @param[in] PciIo The PCI IO protocol instance.
> + @param[in] PrivateA pointer to the SD_MMC_HC_PRIVATE_DATA instance.
>@param[in] Slot The slot number of the SD card to send the
> command to.
>@retval EFI_SUCCESS The software reset executes successfully.
> @@ -430,18 +460,38 @@ SdMmcHcWaitMmioSet (
> **/
> EFI_STATUS
> SdMmcHcReset (
> - IN EFI_PCI_IO_PROTOCOL*PciIo,
> + IN SD_MMC_HC_PRIVATE_DATA *Private,
>IN UINT8 Slot
>)
> {
>EFI_STATUSStatus;
>UINT8 SwReset;
> + EFI_PCI_IO_PROTOCOL *PciIo;
> +
> + //
> + // Notify the SD/MMC override protocol that we are about to reset
> + // the SD/MMC host controller.
> + //
> + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> +Status = mOverride->NotifyPhase (
> + Private->ControllerHandle,
> + Slot,
> + EdkiiSdMmcResetPre,
> + NULL);
> +if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_WARN,
> +"%a: SD/MMC pre reset notifier callback failed - %r\n",
> +__FUNCTION__, Status));
> + return Status;
> +}
> + }
> - SwReset = 0xFF;
> - Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_SW_RST, FALSE, sizeof
> (SwReset), );
> + PciIo = Private->PciIo;
> + SwReset = BIT0;
> + Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_SW_RST, sizeof (SwReset),
> );
>if (EFI_ERROR (Status)) {
> -DEBUG ((DEBUG_ERROR, "SdMmcHcReset: write full 1 fails: %r\n", Status));
> +DEBUG ((DEBUG_ERROR, "SdMmcHcReset: write SW Reset for All fails:
[edk2] A VM failed to boot when I changed the gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size
Hi, Recently I do a test with edk2 on rhel platform. I resized the gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size value to 1TB for supporting multiple GPUs passthrough which have large bars . But when I started a VM with a virtio nic and booted from the changed OVMF, it seems the VM failed to boot before loading ipxe. The uefi error log is like this: X64 Exception Type - 0E(#PF - Page-Fault) CPU Apic ID - ExceptionData - 000B I:0 R:1 U:0 W:1 P:1 PK:0 S:0 RIP - BE4AD7F7, CS - 0038, RFLAGS - 00010206 RAX - , RCX - 0014, RDX - 0114 RBX - BE4BEFE0, RSP - BFEDE6F8, RBP - BE4BEFF0 RSI - BE4BEFF0, RDI - BE4BEFE0 R8 - , R9 - , R10 - R11 - BE4BB900, R12 - BE4BEFD0, R13 - 0060 R14 - 0084, R15 - 0070 DS - 0030, ES - 0030, FS - 0030 GS - 0030, SS - 0030 CR0 - 80010033, CR2 - 0114, CR3 - BF6BA000 CR4 - 0668, CR8 - DR0 - , DR1 - , DR2 - DR3 - , DR6 - 0FF0, DR7 - 0400 GDTR - BF6A8A98 0047, LDTR - IDTR - BF29E018 0FFF, TR - FXSAVE_STATE - BFEDE350 Find image 1af41000.efidrv (ImageBase=BE499000, EntryPoint=BE49F1EB) But when I decrease the PcdPciMmio64Size to 10 * 32 GB, the VM booted successfully. I'm not familiar with uefi, could you please point out what wrong I have done? Thanks, Zongyong Wu ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v1 1/1] ShellPkg/UefiShellAcpiViewCommandLib: Add support for PPTT
Added the acpiview parser for the PPTT table.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Krzysztof Koch
---
The changes can be seen at:
https://github.com/KrzysztofKoch1/edk2/tree/woa_390_pptt_acpiview_v1
Notes:
v1:
- added a parser for PPTT in acpiview [Krzysztof]
ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h|
23 +-
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c |
362
ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c |
4 +-
ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf |
3 +-
ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni |
3 +-
5 files changed, 391 insertions(+), 4 deletions(-)
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
index
ecf7dae9038a4ebcb3e3764964f0c16ca3ef51f6..a42450c1431be343870dabb4e03f64ed1cf78afc
100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
@@ -1,7 +1,7 @@
/** @file
Header file for ACPI parser
- Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
+ Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
which accompanies this distribution. The full text of the license may be
found at
@@ -655,6 +655,27 @@ ParseAcpiMcfg (
IN UINT8 AcpiTableRevision
);
+/**
+ This function parses the ACPI PPTT table.
+ When trace is enabled this function parses the PPTT table and
+ traces the ACPI table fields.
+
+ This function also performs validation of the ACPI table fields.
+
+ @param [in] Trace If TRUE, trace the ACPI fields.
+ @param [in] PtrPointer to the start of the buffer.
+ @param [in] AcpiTableLengthLength of the ACPI table.
+ @param [in] AcpiTableRevision Revision of the ACPI table.
+**/
+VOID
+EFIAPI
+ParseAcpiPptt (
+ IN BOOLEAN Trace,
+ IN UINT8* Ptr,
+ IN UINT32 AcpiTableLength,
+ IN UINT8 AcpiTableRevision
+ );
+
/**
This function parses the ACPI RSDP table.
diff --git
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
new file mode 100644
index
..d97ddf8e925d30917f888bf33e2c1346cd330663
--- /dev/null
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
@@ -0,0 +1,362 @@
+/** @file
+ PPTT table parser
+
+ Copyright (c) 2019, ARM Limited. All rights reserved.
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD
License
+ which accompanies this distribution. The full text of the license may be
found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+ @par Reference(s):
+- ACPI 6.2 Specification - Errata A, September 2017
+**/
+
+#include
+#include
+#include "AcpiParser.h"
+
+// Local variables
+STATIC CONST UINT8* ProcessorTopologyStructureType;
+STATIC CONST UINT8* ProcessorTopologyStructureLength;
+STATIC CONST UINT32* NumberOfPrivateResources;
+STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
+
+/**
+ An ACPI_PARSER array describing the ACPI PPTT Table.
+**/
+STATIC CONST ACPI_PARSER PpttParser[] = {
+ PARSE_ACPI_HEADER ()
+};
+
+/**
+ This function validates the Cache Type Structure (Type 1) Line size field.
+
+ @param [in] Ptr Pointer to the start of the field data.
+ @param [in] Context Pointer to context specific information e.g. this
+ could be a pointer to the ACPI table header.
+**/
+STATIC
+VOID
+EFIAPI
+ValidateCacheLineSize (
+ IN UINT8* Ptr,
+ IN VOID* Context
+ )
+{
+#if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
+ // Reference: ARM Architecture Reference Manual ARMv8 (D.a)
+ // Section D12.2.25: CCSIDR_EL1, Current Cache Size ID Register
+ // LineSize, bits [2:0]
+ // (Log2(Number of bytes in cache line)) - 4.
+
+ UINT16 LineSize;
+ LineSize = *(UINT16*)Ptr;
+
+ if ((LineSize < 16) || (LineSize > 2048)) {
+IncrementErrorCount ();
+Print (
+ L"\nERROR: The cache line size must be between 16 and 2048 bytes"
+L" on ARM Platforms."
+ );
+return;
+ }
+
+ if ((LineSize & (LineSize - 1)) != 0) {
+IncrementErrorCount ();
+Print (L"\nERROR: The cache line size is not a power of 2.");
+ }
+#endif
+}
+
+/**
+ This function validates the Cache Type Structure (Type 1) Attributes field.
+
+ @param [in] Ptr Pointer to the start of the field data.
[edk2] Network Stack Budgeting
Hi, Recently I have done some performance improvements to my network driver. I am however finding that on some platforms, it's becoming impossible to boot if the network cable has a lot of traffic on it that is not filtered by the NIC itself (broadcast, multicast or directed unicast). It would seem the platform hangs in the DXE phase trying to process (drop) all the packets and not progressing through the boot order. I am wondering if anyone has seen similar behaviour. Does the network stack have any budgeting? Ideally this would be fixed by the network stack not calling Snp.Start() until in the BDS phase but it seems most platforms just call Snp.Start() immediately following the driver probe. Cheers, Tom ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch_3] make EDK Driver Support BH720+EMMC chip
Hi, all
The following modifications are made to enable EDK Driver Support BH720 CHIP.
--- /c/yx/edk2/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c 2019-01-21
15:36:12.188734400 -0800
+++ /c/MyWorkspace/edk2-vUDK2018/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
2018-08-13 22:40:04.802226200 -0700
@@ -1,7 +1,6 @@
/** @file
This file provides some helper functions which are specific for SD card
device.
- Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -319,9 +318,116 @@ SdCardSetRca (
return Status;
}
+/**
+ Send command SEND_CSD to the SD device to get the data of the CSD register.
+
+ Refer to SD Physical Layer Simplified Spec 4.1 Section 4.7 for details.
+
+ @param[in] PassThru A pointer to the EFI_SD_MMC_PASS_THRU_PROTOCOL
instance.
+ @param[in] Slot The slot number of the SD card to send the command
to.
+ @param[in] Rca The relative device address of selected device.
+ @param[out] Csd The buffer to store the content of the CSD
register.
+Note the caller should ignore the lowest byte of
this
+buffer as the content of this byte is meaningless
even
+if the operation succeeds.
+
+ @retval EFI_SUCCESS The operation is done correctly.
+ @retval OthersThe operation fails.
+
+**/
+EFI_STATUS
+SdCardGetCsd (
+ IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru,
+ IN UINT8 Slot,
+ IN UINT16 Rca,
+ OUT SD_CSD *Csd
+ )
+{
+ EFI_SD_MMC_COMMAND_BLOCK SdMmcCmdBlk;
+ EFI_SD_MMC_STATUS_BLOCK SdMmcStatusBlk;
+ EFI_SD_MMC_PASS_THRU_COMMAND_PACKET Packet;
+ EFI_STATUSStatus;
+
+ ZeroMem (, sizeof (SdMmcCmdBlk));
+ ZeroMem (, sizeof (SdMmcStatusBlk));
+ ZeroMem (, sizeof (Packet));
+ Packet.SdMmcCmdBlk=
+ Packet.SdMmcStatusBlk =
+ Packet.Timeout= SD_MMC_HC_GENERIC_TIMEOUT;
+ SdMmcCmdBlk.CommandIndex = SD_SEND_CSD;
+ SdMmcCmdBlk.CommandType = SdMmcCommandTypeAc;
+ SdMmcCmdBlk.ResponseType = SdMmcResponseTypeR2;
+ SdMmcCmdBlk.CommandArgument = (UINT32)Rca << 16;
+ Status = SdMmcPassThruPassThru (PassThru, Slot, , NULL);
+ if (!EFI_ERROR (Status)) {
+//
+// For details, refer to SD Host Controller Simplified Spec 3.0 Table 2-12.
+//
+CopyMem (((UINT8*)Csd) + 1, , sizeof (SD_CSD) - 1);
+ }
+
+ return Status;
+}
+
+/**
+ Send command SEND_CSD to the SD device to get the data of the CSD register.
+
+ Refer to SD Physical Layer Simplified Spec 4.1 Section 4.7 for details.
+
+ @param[in] PassThru A pointer to the EFI_SD_MMC_PASS_THRU_PROTOCOL
instance.
+ @param[in] Slot The slot number of the SD card to send the command
to.
+ @param[in] Rca The relative device address of selected device.
+ @param[out] Scr The buffer to store the content of the SCR
register.
+
+ @retval EFI_SUCCESS The operation is done correctly.
+ @retval OthersThe operation fails.
+
+**/
+EFI_STATUS
+SdCardGetScr (
+ IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru,
+ IN UINT8 Slot,
+ IN UINT16 Rca,
+ OUT SD_SCR *Scr
+ )
+{
+ EFI_SD_MMC_COMMAND_BLOCK SdMmcCmdBlk;
+ EFI_SD_MMC_STATUS_BLOCK SdMmcStatusBlk;
+ EFI_SD_MMC_PASS_THRU_COMMAND_PACKET Packet;
+ EFI_STATUSStatus;
+
+ ZeroMem (, sizeof (SdMmcCmdBlk));
+ ZeroMem (, sizeof (SdMmcStatusBlk));
+ ZeroMem (, sizeof (Packet));
+
+ Packet.SdMmcCmdBlk=
+ Packet.SdMmcStatusBlk =
+ Packet.Timeout= SD_MMC_HC_GENERIC_TIMEOUT;
+
+ SdMmcCmdBlk.CommandIndex = SD_APP_CMD;
+ SdMmcCmdBlk.CommandType = SdMmcCommandTypeAc;
+ SdMmcCmdBlk.ResponseType = SdMmcResponseTypeR1;
+ SdMmcCmdBlk.CommandArgument = (UINT32)Rca << 16;
+
+ Status = SdMmcPassThruPassThru (PassThru, Slot, , NULL);
+ if (EFI_ERROR (Status)) {
+return Status;
+ }
+
+ SdMmcCmdBlk.CommandIndex = SD_SEND_SCR;
+ SdMmcCmdBlk.CommandType = SdMmcCommandTypeAdtc;
+ SdMmcCmdBlk.ResponseType = SdMmcResponseTypeR1;
+
+ Packet.InDataBuffer = Scr;
+ Packet.InTransferLength = sizeof (SD_SCR);
+
+ Status = SdMmcPassThruPassThru (PassThru, Slot, , NULL);
+
+ return Status;
+}
/**
Send command SELECT_DESELECT_CARD to the SD device to select/deselect it.
@@ -785,8 +891,8 @@ SdCardSetBusMode (
UINT8BusWidth;
UINT8AccessMode;
UINT8HostCtrl1;
+ UINT8HostCtrl2;
UINT8SwitchResp[64];
- SD_MMC_BUS_MODE Timing;
Re: [edk2] [patch_2] make EDK Driver Support BH720+EMMC chip
Hi, all
The following modifications are made to enable EDK Driver Support BH720 CHIP.
--- /c/yx/edk2/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
2019-01-21 15:36:12.186739600 -0800
+++
/c/MyWorkspace/edk2-vUDK2018/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
2019-01-10 14:34:23.072538900 -0800
@@ -1,7 +1,6 @@
/** @file
This file provides some helper functions which are specific for EMMC device.
- Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -527,17 +526,35 @@ EmmcTuningClkForHs200 (
if (EFI_ERROR (Status)) {
return Status;
}
+
+ if(BhtHostPciSupport(PciIo)){
+ //set data transfer with 4bit
+ Status = SdMmcHcSetBusWidth (PciIo, Slot, 4);
+ //enable hardware tuning
+ HostCtrl2 = (~0x10);
+ Status = SdMmcHcAndMmio (PciIo, Slot,
0x110,sizeof (HostCtrl2), );
+
+Status = EmmcSendTuningBlk (PassThru, Slot, 4);
+if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "EmmcTuningClkForHs200:
Send tuning block fails with %r\n", Status));
+ return Status;
+}
+
+ }
//
// Ask the device to send a sequence of tuning blocks till the tuning
procedure is done.
//
Retry = 0;
do {
-Status = EmmcSendTuningBlk (PassThru, Slot, BusWidth);
-if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "EmmcTuningClkForHs200: Send tuning block fails
with %r\n", Status));
- return Status;
-}
-
+ if(!BhtHostPciSupport(PciIo)){
+ Status = EmmcSendTuningBlk (PassThru, Slot, BusWidth);
+ if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "EmmcTuningClkForHs200: Send tuning
block fails with %r\n", Status));
+return Status;
+ }
+ } else {
+ gBS->Stall(5000);
+ }
Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, TRUE, sizeof
(HostCtrl2), );
if (EFI_ERROR (Status)) {
return Status;
@@ -548,6 +565,10 @@ EmmcTuningClkForHs200 (
}
if ((HostCtrl2 & (BIT6 | BIT7)) == BIT7) {
+if(BhtHostPciSupport(PciIo)){
+ //set data transfer with default
+ Status = SdMmcHcSetBusWidth (PciIo, Slot,
BusWidth);
+ }
return EFI_SUCCESS;
}
} while (++Retry < 40);
@@ -652,7 +673,6 @@ EmmcSwitchBusWidth (
@param[in] Slot The slot number of the SD card to send the command
to.
@param[in] RcaThe relative device address to be assigned.
@param[in] HsTiming The value to be written to HS_TIMING field of
EXT_CSD register.
- @param[in] Timing The bus mode timing indicator.
@param[in] ClockFreq The max clock frequency to be set, the unit is MHz.
@retval EFI_SUCCESS The operation is done correctly.
@@ -666,7 +686,6 @@ EmmcSwitchClockFreq (
IN UINT8 Slot,
IN UINT16 Rca,
IN UINT8 HsTiming,
- IN SD_MMC_BUS_MODETiming,
IN UINT32 ClockFreq
)
{
@@ -708,28 +727,7 @@ EmmcSwitchClockFreq (
//
// Convert the clock freq unit from MHz to KHz.
//
- Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000,
Private->BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
- if (EFI_ERROR (Status)) {
-return Status;
- }
-
- if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
-Status = mOverride->NotifyPhase (
- Private->ControllerHandle,
- Slot,
- EdkiiSdMmcSwitchClockFreqPost,
-
- );
-if (EFI_ERROR (Status)) {
- DEBUG ((
-DEBUG_ERROR,
-"%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
-__FUNCTION__,
-Status
-));
- return Status;
-}
- }
+ Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000,
Private->Capability[Slot]);
return Status;
}
@@ -764,13 +762,10 @@ EmmcSwitchToHighSpeed (
IN UINT8 BusWidth
)
{
- EFI_STATUS Status;
- UINT8 HsTiming;
- UINT8 HostCtrl1;
- SD_MMC_BUS_MODE Timing;
- SD_MMC_HC_PRIVATE_DATA *Private;
-
- Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru);
+ EFI_STATUS Status;
+ UINT8 HsTiming;
+ UINT8 HostCtrl1;
+ UINT8 HostCtrl2;
Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU
On 01/23/19 10:26, Ard Biesheuvel wrote:
> On Wed, 23 Jan 2019 at 10:14, Laszlo Ersek wrote:
>> On 01/22/19 16:37, Ard Biesheuvel wrote:
>>> Is SetUefiImageMemoryAttributes() being
>>> called to remap the memory R-X ?
>>
>> No, it is not; the grub binary in question doesn't have the required
>> section alignment (... I hope at least that that's what your question
>> refers to):
>>
>>> ProtectUefiImageCommon - 0x3E6C54C0
>>> - 0x00013BEEF000 - 0x00030600
>>> ProtectUefiImageCommon - Section Alignment(0x200) is
>> incorrect
>>
>
> This is puzzling, given that the exact same binary works on Mustang.
And even on the original (unspecified) hardware, the same binary works
frequently. My understanding is that there are five VMs executing reboot
loops in parallel, on the same host, and 4 out of 5 may hit the issue in
a reasonable time period (300 reboots or so).
> So when loaded, GRUB should cover the following regions:
>
> 0x13beef - 0x13bf00 (0x11000)
> 0x13bf00 - 0x13bf01f600 (0x1f600)
>
> where neither covers a 2 MB block fully, which means that the TLB
> entry that we are hitting is stale.
>
> Since ProtectUefiImageCommon() does not do anything in this case, the
> stale translation must be the result of
> PcdDxeNxMemoryProtectionPolicy, which either sets the wrong
> permissions for EfiLoaderCode (relying on ProtectUefiImageCommon), or
> we don't flush the TLBs correctly after updating the permissions when
> converting the memory from EfiConventionalMemory to EfiLoaderCode
>
> Are you using the default value for PcdDxeNxMemoryProtectionPolicy?
Yes, we have
ArmVirtPkg/ArmVirt.dsc.inc:
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC0007FD1
from commit 1acd7c54a724 ("ArmVirtPkg AARCH64: enable NX memory
protection for all platforms", 2017-03-01).
The binary is from the RPM
"edk2-aarch64-20180508gitee3198e672e2-5.el8+1789+f0947240.noarch", which
is basically upstream ee3198e672e2 plus a small number of backports and
downstream customizations.
Thanks!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] Drop CSM support in OvmfPkg?
On Wed, 2019-01-23 at 10:46 +0100, Laszlo Ersek wrote: > I'm fine if we move the generic CSM components into OvmfPkg, however I'm > going to ask David to assume reviewer responsibilities for them. > > Given the current format of "Maintainers.txt", we couldn't spell out the > exact pathnames of the CSM components, so we'd add a line like > > R: David Woodhouse > > under OvmfPkg. There is "prior art" for this pattern, see: > > R: Anthony Perard > R: Julien Grall > > Because Anthony and Julien are the authority on Xen-related code under > OvmfPkg. (See commit 337fe6a06eda, "Maintainers.txt: add Xen reviewers > to OvmfPkg", 2017-09-26.) > > > If we keep CSM support in OvmfPkg in any form at all, then I would > prefer holding all the related stuff in the core edk2 repository (with > the above Reviewership), over requiring people to deal with multiple > repositories. I agree (from experience) that PACKAGES_PATH / multiple > workspaces work fine, but in this case I think keeping one shared > history is an advantage. This all makes sense to me. smime.p7s Description: S/MIME cryptographic signature ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] Drop CSM support in OvmfPkg?
Hi All, On 01/23/19 04:43, Ni, Ray wrote: >> -Original Message- >> From: David Woodhouse >> Sent: Wednesday, January 23, 2019 12:23 AM >> To: Ni, Ray ; Gerd Hoffmann ; >> Laszlo Ersek ; Richardson, Brian >> >> Cc: Justen, Jordan L ; edk2-devel@lists.01.org; >> Kevin O'Connor ; Anthony Perard >> >> Subject: Re: Drop CSM support in OvmfPkg? >> >> On Tue, 2019-01-22 at 16:13 +, Ni, Ray wrote: >>> David, >>> I'd like to re-start the discussion. >>> Could you please kindly explain the background/reason of adding CSM >>> support in OVMF? >>> Maybe knowing the reason can help to make further decisions of >>> whether to >>> A. keep it outside OvmfPkg >>> B. keep it inside OvmfPkg >>> C. maybe have a chance to just remove the CSM support after >>> revisiting >> >> >> The idea was to make it simple to have a single firmware image for >> virtual machines which would support both UEFI and Legacy boot for >> guests simultaneously. >> >> In libvirt there has been an alternative approach, where the BIOS image >> is switched between OVMF and SeaBIOS according to the configuration of >> the guest VM. >> >> That's fine for libvirt, but in situations where VM hosting is provided >> as a service, it becomes quite painful to manage the 'UEFI' vs. >> 'Legacy' flags on guest images and then switch firmware images >> accordingly. A one-size-fits-all BIOS using OVMF+CSM is very much >> preferable. > > David, > Thanks for sharing. I now understand that you do have a need of > CSM + UEFI OVMF image. > A very straightforward idea is to move all COM components you needed > into OvmfPkg. But Laszlo as the OvmfPkg owner may disagree with this. > So maybe you could set up another (github) repo and clone all the CSM > components > there. > EDKII build tool supports to build firmware from multiple repos. > That's how we can have edk2-platforms and to-be-created edk2-app. > In practical, you could create a new csm repo. > Laszlo/Gerd who don't care about CSM can just build OVMF image from edk2 repo. > You can build the OVMF image from edk2 and csm repo. > > We can have a call if you are ok. I can explain how that can work in details. I'm fine if we move the generic CSM components into OvmfPkg, however I'm going to ask David to assume reviewer responsibilities for them. Given the current format of "Maintainers.txt", we couldn't spell out the exact pathnames of the CSM components, so we'd add a line like R: David Woodhouse under OvmfPkg. There is "prior art" for this pattern, see: R: Anthony Perard R: Julien Grall Because Anthony and Julien are the authority on Xen-related code under OvmfPkg. (See commit 337fe6a06eda, "Maintainers.txt: add Xen reviewers to OvmfPkg", 2017-09-26.) If we keep CSM support in OvmfPkg in any form at all, then I would prefer holding all the related stuff in the core edk2 repository (with the above Reviewership), over requiring people to deal with multiple repositories. I agree (from experience) that PACKAGES_PATH / multiple workspaces work fine, but in this case I think keeping one shared history is an advantage. Thanks, Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH v2 1/4] Marvell/Armada7k8k: Shift PEI stack base
Hi Leif, śr., 23 sty 2019 o 10:42 Leif Lindholm napisał(a): > > On Wed, Jan 23, 2019 at 09:28:40AM +0100, Marcin Wojtas wrote: > > wt., 22 sty 2019 o 22:10 Leif Lindholm > > napisał(a): > > > > > > On Tue, Jan 22, 2019 at 09:56:14PM +0100, Marcin Wojtas wrote: > > > > > > > I think I gave my suggestion for the resolution of this problem > > > > > > > (with > > > > > > > moving StackBase to 0x0540 as the alternative) in my previous > > > > > > > reply. > > > > > > > > > > > > > > > > > > > Yes, and I answered, presenting the alternative memory map with > > > > > > additional 64kB "cut out" on top of 20MB "hole" of memory, which I'm > > > > > > not fancy, given available space inside the 20MB chunk. > > > > > > > > > > Please go back and reread my first and my second email. > > > > > Then please point out where I have, other than as an alternative > > > > > solution, suggested growing the cutout size. > > > > > > > > > > Then perhaps we can rewind this conversation and try again? > > > > > > > > Ok. So would it be sufficient to replace > > > > gMarvellTokenSpaceGuid.PcdSecureRegionBase with two sets of separate > > > > PCDs for ARM-TF runtime services and OPTEE leaving the PEI stack base > > > > @0x43f? > > > > > > That would be lovely, thank you :) > > > > > > (Although your reference to wanting to keep the PEI stack area out of > > > the hands of the operating system might mean that you want three? I'll > > > leave that to your discretion.) > > > > > > > PEI stack has its own PCDs: > >gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase > >gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize > > > > I want to keep it simple (and btw aligned with U-Boot booting the > > mainline DTB with single 20MB reserved memory area), so what I intend > > to do is to limit reserved region in Armada7k8kMemoryInitPeiLib.c with > > PcdArmTFRegionBase (@0x400) up to PcdOpteeRegionBase + > > PcdOpteeRegionSize (@0x540). > > I am totally online with you wanting to align the reservation of 20MB > of RAM with U-Boot. > > If you want to remove the 2MB gap between ARM-TF and Optee from use by > the OS, you need to reserve that 2MB window. Not pretend that it forms > part of an adjacent region that you also happen to want to keep out of > the hands of the OS. > > The point of the source code is not wiggling the correct signal lines > to create an expected behaviour. Were that the case, we'd be hacking > programs directly into binary. > The point of the source code is to describe what is being done such > that someone else can come in and understand it. > > Saving 15 (or 30, or whatever) lines of boilerplate text by making the > code misleading is not a win. > > You want to solve this by making PcdCPUCorePrimaryStackSize 2MB? > Fine. It's not misleading, and you could always shrink it if you need > the remainder for something else. > > You want to solve this by setting up a third reserved area of > (2MB - PcdCPUCorePrimaryStackSize)? > Fine. > > You want to solve this by making the source code say that a memory > region is simultaneously reserved for Secure world and where our > Non-secure stack resides? > Not fine. That is what I mean by semantic sense. > Thank you for your input. I will explicitly handle each region then. Best regards, Marcin ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH v2 1/4] Marvell/Armada7k8k: Shift PEI stack base
On Wed, Jan 23, 2019 at 09:28:40AM +0100, Marcin Wojtas wrote: > wt., 22 sty 2019 o 22:10 Leif Lindholm napisał(a): > > > > On Tue, Jan 22, 2019 at 09:56:14PM +0100, Marcin Wojtas wrote: > > > > > > I think I gave my suggestion for the resolution of this problem > > > > > > (with > > > > > > moving StackBase to 0x0540 as the alternative) in my previous > > > > > > reply. > > > > > > > > > > > > > > > > Yes, and I answered, presenting the alternative memory map with > > > > > additional 64kB "cut out" on top of 20MB "hole" of memory, which I'm > > > > > not fancy, given available space inside the 20MB chunk. > > > > > > > > Please go back and reread my first and my second email. > > > > Then please point out where I have, other than as an alternative > > > > solution, suggested growing the cutout size. > > > > > > > > Then perhaps we can rewind this conversation and try again? > > > > > > Ok. So would it be sufficient to replace > > > gMarvellTokenSpaceGuid.PcdSecureRegionBase with two sets of separate > > > PCDs for ARM-TF runtime services and OPTEE leaving the PEI stack base > > > @0x43f? > > > > That would be lovely, thank you :) > > > > (Although your reference to wanting to keep the PEI stack area out of > > the hands of the operating system might mean that you want three? I'll > > leave that to your discretion.) > > > > PEI stack has its own PCDs: >gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase >gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize > > I want to keep it simple (and btw aligned with U-Boot booting the > mainline DTB with single 20MB reserved memory area), so what I intend > to do is to limit reserved region in Armada7k8kMemoryInitPeiLib.c with > PcdArmTFRegionBase (@0x400) up to PcdOpteeRegionBase + > PcdOpteeRegionSize (@0x540). I am totally online with you wanting to align the reservation of 20MB of RAM with U-Boot. If you want to remove the 2MB gap between ARM-TF and Optee from use by the OS, you need to reserve that 2MB window. Not pretend that it forms part of an adjacent region that you also happen to want to keep out of the hands of the OS. The point of the source code is not wiggling the correct signal lines to create an expected behaviour. Were that the case, we'd be hacking programs directly into binary. The point of the source code is to describe what is being done such that someone else can come in and understand it. Saving 15 (or 30, or whatever) lines of boilerplate text by making the code misleading is not a win. You want to solve this by making PcdCPUCorePrimaryStackSize 2MB? Fine. It's not misleading, and you could always shrink it if you need the remainder for something else. You want to solve this by setting up a third reserved area of (2MB - PcdCPUCorePrimaryStackSize)? Fine. You want to solve this by making the source code say that a memory region is simultaneously reserved for Secure world and where our Non-secure stack resides? Not fine. That is what I mean by semantic sense. Best Regards, Leif ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] IntelFsp2Pkg: FSP can utilize bootloader stack
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1485
Current FSP utilizes pre-allocated temporary memory from
boot loader for both heap and stack. To reduce overall
temporary memory usage FSP may share the same stack with
boot loader and only needs a smaller memory for heap,
no separate memory required for stack.
Setting PcdFspHeapSizePercentage to 0 to enable FSP sharing
stack with boot loader, in this case boot loader stack
has to be large enough for FSP to use. Default is 50
(half memory heap and half memory stack) for backward
compatible with original model.
Test: Verified on internal platform and booting successfully
with both modes.
Cc: Nate DeSimone
Cc: Star Zeng
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu
---
IntelFsp2Pkg/FspSecCore/SecMain.c | 86
++
IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf| 3 ++-
IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm | 52
++--
IntelFsp2Pkg/FspSecCore/Ia32/ReadEsp.nasm | 28
IntelFsp2Pkg/IntelFsp2Pkg.dec | 4
5 files changed, 154 insertions(+), 19 deletions(-)
diff --git a/IntelFsp2Pkg/FspSecCore/SecMain.c
b/IntelFsp2Pkg/FspSecCore/SecMain.c
index 70460a3c8b..b0b5dda711 100644
--- a/IntelFsp2Pkg/FspSecCore/SecMain.c
+++ b/IntelFsp2Pkg/FspSecCore/SecMain.c
@@ -1,6 +1,6 @@
/** @file
- Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.
+ Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
which accompanies this distribution. The full text of the license may be
found at
@@ -38,6 +38,19 @@ UINT64 mIdtEntryTemplate = 0x8e08ffe4ULL;
/**
+ Return value of esp
+
+ @return value of esp
+
+**/
+UINT32
+EFIAPI
+AsmReadEsp (
+ VOID
+ );
+
+/**
+
Entry point to the C language phase of SEC. After the SEC assembly
code has initialized some temporary memory and set up the stack,
the control is transferred to this function.
@@ -83,7 +96,26 @@ SecStartup (
//
InitializeFloatingPointUnits ();
+ //
+ // Scenario 1 memory map when running on bootloader stack
+ //
+ // |---|>
+ // |Idt Table |
+ // |---|
+ // |PeiService Pointer |
+ // |---|
+ // | |
+ // | |
+ // | Heap |
+ // | |
+ // | |
+ // |---|> TempRamBase
+ // |Bootloader stack |
+ // |---|
+ //
+ // Scenario 2 memory map when running FSP on a separate stack
+ //
// |---|>
// |Idt Table |
// |---|
@@ -135,11 +167,19 @@ SecStartup (
SecCoreData.BootFirmwareVolumeSize = (UINT32)((EFI_FIRMWARE_VOLUME_HEADER
*)BootFirmwareVolume)->FvLength;
SecCoreData.TemporaryRamBase = (VOID*)(UINTN) TempRamBase;
- SecCoreData.TemporaryRamSize = SizeOfRam;
- SecCoreData.PeiTemporaryRamBase= SecCoreData.TemporaryRamBase;
- SecCoreData.PeiTemporaryRamSize= SecCoreData.TemporaryRamSize * PcdGet8
(PcdFspHeapSizePercentage) / 100;
- SecCoreData.StackBase =
(VOID*)(UINTN)((UINTN)SecCoreData.TemporaryRamBase +
SecCoreData.PeiTemporaryRamSize);
- SecCoreData.StackSize = SecCoreData.TemporaryRamSize -
SecCoreData.PeiTemporaryRamSize;
+ if (PcdGet8 (PcdFspHeapSizePercentage) == 0) {
+SecCoreData.TemporaryRamSize = SizeOfRam; // stack size that is
going to be copied to the permanent memory
+SecCoreData.PeiTemporaryRamBase= SecCoreData.TemporaryRamBase;
+SecCoreData.PeiTemporaryRamSize= SecCoreData.TemporaryRamSize;
+SecCoreData.StackBase = (VOID *)GetFspEntryStack(); // Share
the same boot loader stack
+SecCoreData.StackSize = 0;
+ } else {
+SecCoreData.TemporaryRamSize = SizeOfRam;
+SecCoreData.PeiTemporaryRamBase= SecCoreData.TemporaryRamBase;
+SecCoreData.PeiTemporaryRamSize= SecCoreData.TemporaryRamSize *
PcdGet8 (PcdFspHeapSizePercentage) / 100;
+SecCoreData.StackBase =
(VOID*)(UINTN)((UINTN)SecCoreData.TemporaryRamBase +
SecCoreData.PeiTemporaryRamSize);
+SecCoreData.StackSize = SecCoreData.TemporaryRamSize -
SecCoreData.PeiTemporaryRamSize;
+ }
DEBUG ((DEBUG_INFO, "Fsp BootFirmwareVolumeBase - 0x%x\n",
SecCoreData.BootFirmwareVolumeBase));
DEBUG ((DEBUG_INFO, "Fsp BootFirmwareVolumeSize - 0x%x\n",
SecCoreData.BootFirmwareVolumeSize));
@@ -194,15 +234,37 @@ SecTemporaryRamSupport (
UINTN HeapSize;
UINTN StackSize;
- HeapSize = CopySize * PcdGet8 (PcdFspHeapSizePercentage) / 100 ;
- StackSize =
[edk2] [patch_1] make EDK Driver Support BH720+EMMC chip
Hi, all
The following modifications are made to enable EDK Driver Support BH720 CHIP.
---
/c/MyWorkspace/edk2-vUDK2018/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
2019-01-10 14:35:21.342736200 -0800
+++ /c/yx/edk2/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
2019-01-21 15:36:12.195715300 -0800
@@ -4,6 +4,7 @@
It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
+ Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -16,8 +17,6 @@
**/
#include "SdMmcPciHcDxe.h"
-int g_deviceId = 0;
-
/**
Dump the content of SD/MMC host controller's Capability Register.
@@ -47,7 +46,8 @@ DumpCapabilityReg (
DEBUG ((DEBUG_INFO, " Voltage 3.3 %a\n", Capability->Voltage33 ?
"TRUE" : "FALSE"));
DEBUG ((DEBUG_INFO, " Voltage 3.0 %a\n", Capability->Voltage30 ?
"TRUE" : "FALSE"));
DEBUG ((DEBUG_INFO, " Voltage 1.8 %a\n", Capability->Voltage18 ?
"TRUE" : "FALSE"));
- DEBUG ((DEBUG_INFO, " 64-bit Sys Bus%a\n", Capability->SysBus64 ?
"TRUE" : "FALSE"));
+ DEBUG ((DEBUG_INFO, " V4 64-bit Sys Bus %a\n", Capability->SysBus64V4 ?
"TRUE" : "FALSE"));
+ DEBUG ((DEBUG_INFO, " V3 64-bit Sys Bus %a\n", Capability->SysBus64V3 ?
"TRUE" : "FALSE"));
DEBUG ((DEBUG_INFO, " Async Interrupt %a\n", Capability->AsyncInt ?
"TRUE" : "FALSE"));
DEBUG ((DEBUG_INFO, " SlotType "));
if (Capability->SlotType == 0x00) {
@@ -419,9 +419,39 @@ SdMmcHcWaitMmioSet (
}
/**
+ Get the controller version information from the specified slot.
+
+ @param[in] PciIo The PCI IO protocol instance.
+ @param[in] SlotThe slot number of the SD card to send the
command to.
+ @param[out] Version The buffer to store the version information.
+
+ @retval EFI_SUCCESS The operation executes successfully.
+ @retval Others The operation fails.
+
+**/
+EFI_STATUS
+SdMmcHcGetControllerVersion (
+ IN EFI_PCI_IO_PROTOCOL *PciIo,
+ IN UINT8Slot,
+ OUTUINT16 *Version
+ )
+{
+ EFI_STATUSStatus;
+
+ Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_CTRL_VER, TRUE, sizeof
(UINT16), Version);
+ if (EFI_ERROR (Status)) {
+return Status;
+ }
+
+ *Version &= 0xFF;
+
+ return EFI_SUCCESS;
+}
+
+/**
Software reset the specified SD/MMC host controller and enable all
interrupts.
- @param[in] PciIo The PCI IO protocol instance.
+ @param[in] PrivateA pointer to the SD_MMC_HC_PRIVATE_DATA instance.
@param[in] Slot The slot number of the SD card to send the command
to.
@retval EFI_SUCCESS The software reset executes successfully.
@@ -430,18 +460,38 @@ SdMmcHcWaitMmioSet (
**/
EFI_STATUS
SdMmcHcReset (
- IN EFI_PCI_IO_PROTOCOL*PciIo,
+ IN SD_MMC_HC_PRIVATE_DATA *Private,
IN UINT8 Slot
)
{
EFI_STATUSStatus;
UINT8 SwReset;
+ EFI_PCI_IO_PROTOCOL *PciIo;
+
+ //
+ // Notify the SD/MMC override protocol that we are about to reset
+ // the SD/MMC host controller.
+ //
+ if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
+Status = mOverride->NotifyPhase (
+ Private->ControllerHandle,
+ Slot,
+ EdkiiSdMmcResetPre,
+ NULL);
+if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN,
+"%a: SD/MMC pre reset notifier callback failed - %r\n",
+__FUNCTION__, Status));
+ return Status;
+}
+ }
- SwReset = 0xFF;
- Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_SW_RST, FALSE, sizeof
(SwReset), );
+ PciIo = Private->PciIo;
+ SwReset = BIT0;
+ Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_SW_RST, sizeof (SwReset),
);
if (EFI_ERROR (Status)) {
-DEBUG ((DEBUG_ERROR, "SdMmcHcReset: write full 1 fails: %r\n", Status));
+DEBUG ((DEBUG_ERROR, "SdMmcHcReset: write SW Reset for All fails: %r\n",
Status));
return Status;
}
@@ -450,7 +500,7 @@ SdMmcHcReset (
Slot,
SD_MMC_HC_SW_RST,
sizeof (SwReset),
- 0xFF,
+ BIT0,
0x00,
SD_MMC_HC_GENERIC_TIMEOUT
);
@@ -458,10 +508,33 @@ SdMmcHcReset (
DEBUG ((DEBUG_INFO, "SdMmcHcReset: reset done with %r\n", Status));
return Status;
}
+
//
// Enable all interrupt after reset all.
//
Status = SdMmcHcEnableInterrupt (PciIo, Slot);
+ if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_INFO, "SdMmcHcReset: SdMmcHcEnableInterrupt done with %r\n",
+ Status));
+return Status;
+ }
+
+ //
+ // Notify the SD/MMC override protocol that we have just reset
+ // the SD/MMC host controller.
+ //
+ if (mOverride !=
Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU
On Wed, 23 Jan 2019 at 10:14, Laszlo Ersek wrote:
>
> On 01/22/19 16:37, Ard Biesheuvel wrote:
> > On Tue, 22 Jan 2019 at 16:09, Laszlo Ersek wrote:
> >>
> >> Performing thread-necromancy on a patch from 2015, which is today known
> >> as commit b7de7e3cab3f. Also CC'ing Christoffer and Marc.
> >>
> >> Question at the bottom.
> >>
> >> On 12/07/15 08:06, Ard Biesheuvel wrote:
> >>> From: "Cohen, Eugene"
> >>>
> >>> This patch updates the ArmPkg variant of InvalidateInstructionCacheRange
> >>> to
> >>> flush the data cache only to the point of unification (PoU). This improves
> >>> performance and also allows invalidation in scenarios where it would be
> >>> inappropriate to flush to the point of coherency (like when executing code
> >>> from L2 configured as cache-as-ram).
> >>>
> >>> Contributed-under: TianoCore Contribution Agreement 1.0
> >>> Signed-off-by: Eugene Cohen
> >>>
> >>> Added AARCH64 and ARM/GCC implementations of the above.
> >>>
> >>> Contributed-under: TianoCore Contribution Agreement 1.0
> >>> Signed-off-by: Ard Biesheuvel
> >>> ---
> >>> ArmPkg/Include/Library/ArmLib.h| 8
> >>> +++-
> >>> ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c | 2 +-
> >>> ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 6 ++
> >>> ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S | 6 ++
> >>> ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm | 5 +
> >>> 5 files changed, 25 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/ArmPkg/Include/Library/ArmLib.h
> >>> b/ArmPkg/Include/Library/ArmLib.h
> >>> index 9622444ec63f..85fa1f600ba9 100644
> >>> --- a/ArmPkg/Include/Library/ArmLib.h
> >>> +++ b/ArmPkg/Include/Library/ArmLib.h
> >>> @@ -183,12 +183,18 @@ ArmInvalidateDataCacheEntryByMVA (
> >>>
> >>> VOID
> >>> EFIAPI
> >>> -ArmCleanDataCacheEntryByMVA (
> >>> +ArmCleanDataCacheEntryToPoUByMVA(
> >>>IN UINTN Address
> >>>);
> >>>
> >>> VOID
> >>> EFIAPI
> >>> +ArmCleanDataCacheEntryByMVA(
> >>> +IN UINTN Address
> >>> +);
> >>> +
> >>> +VOID
> >>> +EFIAPI
> >>> ArmCleanInvalidateDataCacheEntryByMVA (
> >>>IN UINTN Address
> >>>);
> >>> diff --git
> >>> a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> >>> b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> >>> index feab4497ac1b..1045f9068f4d 100644
> >>> --- a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> >>> +++ b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> >>> @@ -64,7 +64,7 @@ InvalidateInstructionCacheRange (
> >>>IN UINTN Length
> >>>)
> >>> {
> >>> - CacheRangeOperation (Address, Length, ArmCleanDataCacheEntryByMVA);
> >>> + CacheRangeOperation (Address, Length,
> >>> ArmCleanDataCacheEntryToPoUByMVA);
> >>>ArmInvalidateInstructionCache ();
> >>>return Address;
> >>> }
> >>> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> >>> b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> >>> index c530d19e897e..db21f73f0ed7 100644
> >>> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> >>> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> >>> @@ -22,6 +22,7 @@
> >>> GCC_ASM_EXPORT (ArmInvalidateInstructionCache)
> >>> GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryByMVA)
> >>> GCC_ASM_EXPORT (ArmCleanDataCacheEntryByMVA)
> >>> +GCC_ASM_EXPORT (ArmCleanDataCacheEntryToPoUByMVA)
> >>> GCC_ASM_EXPORT (ArmCleanInvalidateDataCacheEntryByMVA)
> >>> GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryBySetWay)
> >>> GCC_ASM_EXPORT (ArmCleanDataCacheEntryBySetWay)
> >>> @@ -72,6 +73,11 @@ ASM_PFX(ArmCleanDataCacheEntryByMVA):
> >>>ret
> >>>
> >>>
> >>> +ASM_PFX(ArmCleanDataCacheEntryToPoUByMVA):
> >>> + dc cvau, x0// Clean single data cache line to PoU
> >>> + ret
> >>> +
> >>> +
> >>> ASM_PFX(ArmCleanInvalidateDataCacheEntryByMVA):
> >>>dc civac, x0 // Clean and invalidate single data cache line
> >>>ret
> >>> diff --git a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> >>> b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> >>> index 5f030d92de31..7de1b11ef818 100644
> >>> --- a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> >>> +++ b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> >>> @@ -19,6 +19,7 @@
> >>> GCC_ASM_EXPORT (ArmInvalidateInstructionCache)
> >>> GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryByMVA)
> >>> GCC_ASM_EXPORT (ArmCleanDataCacheEntryByMVA)
> >>> +GCC_ASM_EXPORT (ArmCleanDataCacheEntryToPoUByMVA)
> >>> GCC_ASM_EXPORT (ArmCleanInvalidateDataCacheEntryByMVA)
> >>> GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryBySetWay)
> >>> GCC_ASM_EXPORT (ArmCleanDataCacheEntryBySetWay)
> >>> @@ -69,6 +70,11 @@ ASM_PFX(ArmCleanDataCacheEntryByMVA):
> >>>bx lr
> >>>
> >>>
> >>> +ASM_PFX(ArmCleanDataCacheEntryToPoUByMVA):
> >>> + mcr p15, 0, r0, c7, c11, 1 @clean single data cache line to PoU
> >>> + bx lr
> >>> +
> >>> +
> >>>
Re: [edk2] Drop CSM support in OvmfPkg?
On Wed, 2019-01-23 at 07:12 +0100, Gerd Hoffmann wrote: > > A one-size-fits-all BIOS using OVMF+CSM is very much > > preferable. > > Building a one-size-fits-all BIOS is pretty much impossible due to CSM > being incompatible with secure boot. Booting with CSM is incompatible with Secure Boot, of course. But it doesn't prevent Secure Boot unless we actually use the CSM for booting, surely? I'm interested in what happened to generic CSM support. Has Intel really ditched CSM completely from all other TianoCore builds? Nobody else will ever be able to build a Tiano-based UEFI firmware again, unless they also squirrel away a copy of the code before it disappears? The code really is otherwise being deleted? Or is this an "open source regression", with code that was in the public repository now disappearing and only being maintained in private? smime.p7s Description: S/MIME cryptographic signature ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 3/3] FatPkg: Add GPT check in FatPei to support Capsule-on-Disk feature.
Hi Ray and Hao
Do you have any comments for this patch?
-Original Message-
From: Zhang, Chao B
Sent: Wednesday, January 23, 2019 2:22 PM
To: Chen, Chen A ; edk2-devel@lists.01.org
Cc: Ni, Ray
Subject: RE: [PATCH 3/3] FatPkg: Add GPT check in FatPei to support
Capsule-on-Disk feature.
Reviewed-by : Chao Zhang
-Original Message-
From: Chen, Chen A
Sent: Thursday, January 17, 2019 10:03 AM
To: edk2-devel@lists.01.org
Cc: Chen, Chen A ; Ni, Ray ; Zhang,
Chao B
Subject: [PATCH 3/3] FatPkg: Add GPT check in FatPei to support Capsule-on-Disk
feature.
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1470
This feature is used for finding GPT partition, follow the following step to
check.
1) Check Protective MBR.
2) Check GPT primary/backup header.
3) Check GPT primary/backup entry array.
Cc: Ruiyu Ni
Cc: Zhang Chao B
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chen A Chen
---
FatPkg/FatPei/FatLitePeim.h | 1 +
FatPkg/FatPei/FatPei.inf| 3 +
FatPkg/FatPei/Gpt.c | 546
3 files changed, 550 insertions(+)
create mode 100644 FatPkg/FatPei/Gpt.c
diff --git a/FatPkg/FatPei/FatLitePeim.h b/FatPkg/FatPei/FatLitePeim.h index
fbf887da5f..afb429c56e 100644
--- a/FatPkg/FatPei/FatLitePeim.h
+++ b/FatPkg/FatPei/FatLitePeim.h
@@ -27,6 +27,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
EXPRESS OR IMPLIED.
#include
#include
#include
+#include
#include
#include
#include
diff --git a/FatPkg/FatPei/FatPei.inf b/FatPkg/FatPei/FatPei.inf index
829e87fe92..dd0869f7cd 100644
--- a/FatPkg/FatPei/FatPei.inf
+++ b/FatPkg/FatPei/FatPei.inf
@@ -31,6 +31,7 @@
[Sources]
Mbr.c
+ Gpt.c
Eltorito.c
Part.c
FatLiteApi.c
@@ -49,6 +50,7 @@
[LibraryClasses]
PcdLib
BaseMemoryLib
+ MemoryAllocationLib
PeimEntryPoint
BaseLib
DebugLib
@@ -61,6 +63,7 @@
gRecoveryOnFatIdeDiskGuid ## SOMETIMES_CONSUMES ##
UNDEFINED
gRecoveryOnFatFloppyDiskGuid## SOMETIMES_CONSUMES ##
UNDEFINED
gRecoveryOnFatNvmeDiskGuid ## SOMETIMES_CONSUMES ##
UNDEFINED
+ gEfiPartTypeUnusedGuid ## SOMETIMES_CONSUMES ##
UNDEFINED
[Ppis]
diff --git a/FatPkg/FatPei/Gpt.c b/FatPkg/FatPei/Gpt.c new file mode 100644
index 00..d1f4c1c8b5
--- /dev/null
+++ b/FatPkg/FatPei/Gpt.c
@@ -0,0 +1,546 @@
+/** @file
+ Routines supporting partition discovery and
+ logical device reading
+
+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+
+This program and the accompanying materials are licensed and made
+available under the terms and conditions of the BSD License which
+accompanies this distribution. The full text of the license may be
+found at http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include
+#include
+#include
+#include "FatLitePeim.h"
+
+//
+// Assumption: 'a' and 'blocksize' are all UINT32 or UINT64.
+// If 'a' and 'blocksize' are not the same type, should use DivU64xU32 to
calculate.
+//
+#define EFI_SIZE_TO_BLOCKS(a, blocksize) (((a) / (blocksize)) + (((a)
+% (blocksize)) ? 1 : 0))
+
+//
+// GPT Partition Entry Status
+//
+typedef struct {
+ BOOLEAN OutOfRange;
+ BOOLEAN Overlap;
+ BOOLEAN OsSpecific;
+} EFI_PARTITION_ENTRY_STATUS;
+
+/**
+ Check if the CRC field in the Partition table header is valid
+
+ @param[in] BlockIo Parent BlockIo interface
+ @param[in] DiskIo Disk Io Protocol.
+ @param[in] PartHeader Partition table header structure
+
+ @retval TRUE the CRC is valid
+ @retval FALSE the CRC is invalid
+
+**/
+BOOLEAN
+PartitionCheckGptHeaderCRC (
+ IN EFI_PARTITION_TABLE_HEADER *PartHeader
+ )
+{
+ UINT32 GptHdrCrc;
+ UINT32 Crc;
+
+ GptHdrCrc = PartHeader->Header.CRC32;
+
+ //
+ // Set CRC field to zero when doing calcuation //
+ PartHeader->Header.CRC32 = 0;
+
+ Crc = CalculateCrc32 (PartHeader, PartHeader->Header.HeaderSize);
+
+ //
+ // Restore Header CRC
+ //
+ PartHeader->Header.CRC32 = GptHdrCrc;
+
+ return (GptHdrCrc == Crc);
+}
+
+
+/**
+ Check if the CRC field in the Partition table header is valid
+ for Partition entry array.
+
+ @param[in] BlockIo Parent BlockIo interface
+ @param[in] DiskIo Disk Io Protocol.
+ @param[in] PartHeader Partition table header structure
+
+ @retval TRUE the CRC is valid
+ @retval FALSE the CRC is invalid
+
+**/
+BOOLEAN
+PartitionCheckGptEntryArrayCRC (
+ IN EFI_PARTITION_TABLE_HEADER *PartHeader,
+ IN EFI_PARTITION_ENTRY*PartEntry
+ )
+{
+ UINT32 Crc;
+ UINTN Size;
+
+ Size = (UINTN)MultU64x32(PartHeader->NumberOfPartitionEntries,
+ PartHeader->SizeOfPartitionEntry);
+ Crc = CalculateCrc32 (PartEntry, Size);
+
+ return (BOOLEAN)
Re: [edk2] [platforms: PATCH v2 1/4] Marvell/Armada7k8k: Shift PEI stack base
Hi Leif, wt., 22 sty 2019 o 22:10 Leif Lindholm napisał(a): > > On Tue, Jan 22, 2019 at 09:56:14PM +0100, Marcin Wojtas wrote: > > > > > I think I gave my suggestion for the resolution of this problem (with > > > > > moving StackBase to 0x0540 as the alternative) in my previous > > > > > reply. > > > > > > > > > > > > > Yes, and I answered, presenting the alternative memory map with > > > > additional 64kB "cut out" on top of 20MB "hole" of memory, which I'm > > > > not fancy, given available space inside the 20MB chunk. > > > > > > Please go back and reread my first and my second email. > > > Then please point out where I have, other than as an alternative > > > solution, suggested growing the cutout size. > > > > > > Then perhaps we can rewind this conversation and try again? > > > > Ok. So would it be sufficient to replace > > gMarvellTokenSpaceGuid.PcdSecureRegionBase with two sets of separate > > PCDs for ARM-TF runtime services and OPTEE leaving the PEI stack base > > @0x43f? > > That would be lovely, thank you :) > > (Although your reference to wanting to keep the PEI stack area out of > the hands of the operating system might mean that you want three? I'll > leave that to your discretion.) > PEI stack has its own PCDs: gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize I want to keep it simple (and btw aligned with U-Boot booting the mainline DTB with single 20MB reserved memory area), so what I intend to do is to limit reserved region in Armada7k8kMemoryInitPeiLib.c with PcdArmTFRegionBase (@0x400) up to PcdOpteeRegionBase + PcdOpteeRegionSize (@0x540). Best regards, Marcin ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
