Re: [edk2] [PATCH v4 4/8] OvmfPkg: Include MdeModulePkg/FrameBufferLib in OvmfPkg

On 10/11/16 07:50, Ruiyu Ni wrote: > One of the following patches will change QemuVideoDxe driver > to use the new FrameBufferLib. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ruiyu Ni > Cc: Laszlo Ersek > Cc: Jordan Justen

Re: [edk2] [PATCH v4 6/8] OvmfPkg: QemuVideoDxe uses MdeModulePkg/FrameBufferLib

On 10/11/16 07:50, Ruiyu Ni wrote: > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ruiyu Ni > Cc: Laszlo Ersek > Cc: Jordan Justen > --- > OvmfPkg/QemuVideoDxe/Gop.c| 47 >

Re: [edk2] [Patch V4] BaseTools: support the NOOPT target with the GCC tool chains

On 10/10/2016 8:00 PM, Yonghong Zhu wrote: Update the tools_def.template to add NOOPT support with GCC tool chains. Also disable -flto and -Os in NOOPT target for GCC5. Cc: Liming Gao Cc: Laszlo Ersek Contributed-under: TianoCore Contribution

Re: [edk2] [PATCH V2 00/50] Add capsule update and recovery sample.

It seems we should rename *MdeModulePkg* to *SampleModulePkg* to elimination the confusingJust kidding. Ok. I will leave MdeModulePkg position question to Mike Kinney. I think we have plan to do something before. It just does not happen yet. I appreciate your detail feedback on usage model

Re: [edk2] [Patch V4] BaseTools: support the NOOPT target with the GCC tool chains

On 10/11/16 17:16, Bruce Cran wrote: > On 10/10/2016 8:00 PM, Yonghong Zhu wrote: > >> Update the tools_def.template to add NOOPT support with GCC tool chains. >> Also disable -flto and -Os in NOOPT target for GCC5. >> >> Cc: Liming Gao >> Cc: Laszlo Ersek

Re: [edk2] [PATCH v4 2/8] MdeModulePkg: Add FrameBufferBltLib library instance

On 10/11/16 07:50, Ruiyu Ni wrote: > This library provides interfaces to perform UEFI Graphics > Output Protocol Video BLT operations. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ruiyu Ni > Reviewed-by: Feng Tian > Cc:

Re: [edk2] RFC: ProtocolLib for cross DXE and SMM Protocol and Handle Services

Mike, > I agree that accessing DXE protocols in an SMI handler is not allowed. > > It is legal for an SMM Driver to access DXE content in the SMM Driver Entry > Point. To digress from the original thread a bit.. There's legal from a PI perspective but for the situations that warrant stricter

Re: [edk2] [PATCH V2 09/50] MdeModulePkg/FmpAuthenticationLib: Add FmpAuthenticationLib instance.

In ExecuteFmpAuthenticationHandler and other functions you use asserts to handle parameter validation. I didn't see in the caller any protections against bad parameters so on builds with ASSERT disabled this code will not be safe. [Jiewen] The code uses 4 ASSERT. ASSERT (Image != NULL); //

Re: [edk2] [PATCH v4 5/8] ArmVirtPkg: Include MdeModulePkg/FrameBufferLib in ArmVirtPkg

On 10/11/16 07:50, Ruiyu Ni wrote: > One of the following patches will change QemuVideoDxe driver > to use the new FrameBufferLib. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ruiyu Ni > Cc: Laszlo Ersek > Cc: Ard Biesheuvel

Re: [edk2] [PATCH 17/45] MdeModulePkg/CapsuleApp: Add CapsuleApp application.

Jaben, How about moving the contents in ShellPkg\Include\Protocol\ to MdePkg\Include\Protocol\? All industry standard protocols are defined in MdePkg, except the Shell related protocols. Thanks/Ray From: Carsey, Jaben Sent: Tuesday, October 4, 2016 4:38 AM To: Rothman, Michael A

Re: [edk2] [PATCH 17/45] MdeModulePkg/CapsuleApp: Add CapsuleApp application.

Jaben, The reason I propose to move the Shell protocol definitions to MdePkg is that all applications that wants to use the ARGV can use the ShellParameter protocol, without depending on ShellPkg. Thanks/Ray From: Ni, Ruiyu Sent: Tuesday, October 11, 2016 4:08 PM To: Carsey, Jaben

Re: [edk2] [PATCH 17/45] MdeModulePkg/CapsuleApp: Add CapsuleApp application.

Yes, I think that is a very good idea. Shell spec defines EFI_SHELL_PROTOCOL, EFI_SHELL_DYNAMIC_COMMAND_PROTOCOL, EFI_SHELL_PARAMETERS_PROTOCOL. These should be treated as industry stand and we can move these 3 to MdePkg. SHELL_ENVIRONMENT_2_PROTOCOL and SHELL_INTERFACE_PROTOCOL should be still

Re: [edk2] [PATCH v4 8/8] ArmVirtPkg: Remove unused BltLib reference

On 10/11/16 07:50, Ruiyu Ni wrote: > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ruiyu Ni > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > --- > ArmVirtPkg/ArmVirtQemu.dsc | 5 + >

Re: [edk2] [PATCH 17/45] MdeModulePkg/CapsuleApp: Add CapsuleApp application.

Yes, I think that is a very good idea. Shell spec defines EFI_SHELL_PROTOCOL, EFI_SHELL_DYNAMIC_COMMAND_PROTOCOL, EFI_SHELL_PARAMETERS_PROTOCOL. These should be treated as industry stand and we can move these 3 to MdePkg. SHELL_ENVIRONMENT_2_PROTOCOL and SHELL_INTERFACE_PROTOCOL should be still

Re: [edk2] [PATCH V2 00/50] Add capsule update and recovery sample.

Comment individual package name - [SampleSystemFirmwareUpdatePkg] I do not think it is clear enough. To clarify the feature included: 1) It is for both firmware update and recovery. 2) It supports both system firmware update and device firmware update. If we want to move to a

Re: [edk2] [PATCH v4 7/8] OvmfPkg: Remove unused BltLib reference

On 10/11/16 07:50, Ruiyu Ni wrote: > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ruiyu Ni > Cc: Jordan Justen > Cc: Laszlo Ersek > --- > OvmfPkg/OvmfPkgIa32.dsc| 5 + >

Re: [edk2] [PATCH 3/3] ArmPlatformPkg: Remove UINTN cast when setting BaudRate.

On 10/11/16 12:23, Evan Lloyd wrote: > Hi Leif. > Please feel free to fix the space change as you see fit and proper, > as it was just incidental tidying up. I would simply drop that hunk for now. While I personally prefer the no-space form, and stick with it consistently in all code I write,

Re: [edk2] [PATCH 3/3] ArmPlatformPkg: Remove UINTN cast when setting BaudRate.

On Tue, Oct 11, 2016 at 10:23:12AM +, Evan Lloyd wrote: > Please feel free to fix the space change as you see fit and proper, > as it was just incidental tidying up. Thanks. > It would be good to have a discussion about the general position, > though. > There are, I am sure, sound reasons

Re: [edk2] [Patch V4] BaseTools: support the NOOPT target with the GCC tool chains

On 10/11/16 04:00, Yonghong Zhu wrote: > Update the tools_def.template to add NOOPT support with GCC tool chains. > Also disable -flto and -Os in NOOPT target for GCC5. > > Cc: Liming Gao > Cc: Laszlo Ersek > Contributed-under: TianoCore Contribution

Re: [edk2] [PATCH v4 2/8] MdeModulePkg: Add FrameBufferBltLib library instance

On 10/11/16 07:50, Ruiyu Ni wrote: > This library provides interfaces to perform UEFI Graphics > Output Protocol Video BLT operations. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ruiyu Ni > Reviewed-by: Feng Tian > Cc:

Re: [edk2] [PATCH 3/3] ArmPlatformPkg: Remove UINTN cast when setting BaudRate.

Thanks, Leif. Some interesting points, and I agree and will strive to comply. I only point out that there MAY be a general pressure to not bother "tidying" where trivia are observed. That is, of course, difficult to prove or quantify. Regards, Evan >-Original Message- >From: Leif

Re: [edk2] [PATCH V2 03/50] MdeModulePkg/Include: Add FmpAuthenticationLib header.

HI Sean We need support PKCS7 authentication for capsule BIOS update, and we need support RSA2048SHA256 authentication for recovery image. They are using same format, and the only difference is cert type. That is why we choose *registration*. A platform has the flexibility to choose 1 or more

Re: [edk2] [PATCH] MdeModulePkg/MdeModulePkg.dec: Fix EBC build failure of PciBus driver

On 10/11/16 07:01, Ruiyu Ni wrote: > When PciBus is built as EBC, PcdPciDegradeResourceForOptionRom does > not have associated value resulting build failure. > The patch sets the default value to TRUE, covering the EBC ARCH. > > Contributed-under: TianoCore Contribution Agreement 1.0 >

Re: [edk2] [PATCH V2 06/50] MdeModulePkg/CapsuleLib: Add ProcessCapsules() API.

HI Sean We choose to process capsule twice purposely - for security consideration, as I mentioned in the comment section. We did design review in detail in Intel technical sync meeting. And it is agreed by Mike Kinney and Vincent Zimmer. To resolve your concern: 1) For example windows

Re: [edk2] [PATCH V2 09/50] MdeModulePkg/FmpAuthenticationLib: Add FmpAuthenticationLib instance.

In ExecuteFmpAuthenticationHandler and other functions you use asserts to handle parameter validation. I didn't see in the caller any protections against bad parameters so on builds with ASSERT disabled this code will not be safe. Can you explain how you are using monotonic count? Your

Re: [edk2] [PATCH 3/3] ArmPlatformPkg: Remove UINTN cast when setting BaudRate.

Hi Leif. Please feel free to fix the space change as you see fit and proper, as it was just incidental tidying up. It would be good to have a discussion about the general position, though. There are, I am sure, sound reasons for not rolling these things together (and I knew that, and shouldn't

Re: [edk2] [PATCH v3 00/10] enhance mmc

On 11 October 2016 at 17:24, Ard Biesheuvel wrote: > Hi Ryan, > > On 11 October 2016 at 17:22, Ryan Harkin wrote: > [...] >> And OpenPlatformPkg was taken from my repo, which only carries one >> patch essential for TC2 booting: >> c22a665

Re: [edk2] [PATCH v3 00/10] enhance mmc

Hi Haojian, So I've tested this v3 patchset and while it's much improved, I still get hangs on TC2. The good news, MMC identification and driver initialisation succeeds in this version and the board boots a bit further. Unfortunately, with a RELEASE build, the board hangs at a later point: when

Re: [edk2] [PATCH v3 00/10] enhance mmc

On 11 October 2016 at 17:27, Ryan Harkin wrote: > On 11 October 2016 at 17:24, Ard Biesheuvel wrote: >> Hi Ryan, >> >> On 11 October 2016 at 17:22, Ryan Harkin wrote: >> [...] >>> And OpenPlatformPkg was taken from my

Re: [edk2] RFC: ProtocolLib for cross DXE and SMM Protocol and Handle Services

> -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Cohen, > Eugene > Sent: Tuesday, October 11, 2016 8:18 AM > To: Kinney, Michael D ; Gao, Liming > ; > Laszlo Ersek ;

Re: [edk2] [PATCH] MdeModulePkg/MdeModulePkg.dec: Fix EBC build failure of PciBus driver

On 11 October 2016 at 11:52, Laszlo Ersek wrote: > On 10/11/16 07:01, Ruiyu Ni wrote: >> When PciBus is built as EBC, PcdPciDegradeResourceForOptionRom does >> not have associated value resulting build failure. >> The patch sets the default value to TRUE, covering the EBC ARCH.

Re: [edk2] [PATCH v3 00/10] enhance mmc

On 11 October 2016 at 17:28, Ard Biesheuvel wrote: > On 11 October 2016 at 17:27, Ryan Harkin wrote: >> On 11 October 2016 at 17:24, Ard Biesheuvel >> wrote: >>> Hi Ryan, >>> >>> On 11 October 2016 at 17:22, Ryan

Re: [edk2] [PATCH v3 00/10] enhance mmc

Hi Ryan, On 11 October 2016 at 17:22, Ryan Harkin wrote: [...] > And OpenPlatformPkg was taken from my repo, which only carries one > patch essential for TC2 booting: > c22a665 2016-01-29 HACK: Platforms/ARM: TC2: set >

Re: [edk2] ArmPlatformPkg: Allocate VRAM as RuntimeServicesData

Hi Evan, This was sent to the list with no subject line and I wasn't on CC, so I didn't see it. Are you still using this patch and want it in, i.e. does it need review and test? Cheers, Ryan. On 4 March 2016 at 15:57, wrote: > Code at:

Re: [edk2] [PATCH V3 0/3] Add PcdRecoveryFileName PCD.

Jiewen, This update looks good. The series of 3 patches Reviewed-by: Michael Kinney Mike > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jiewen > Yao > Sent: Monday, October 10, 2016 6:04 PM > To:

Re: [edk2] ArmPlatformPkg: Allocate VRAM as RuntimeServicesData

On 11 October 2016 at 18:44, Ryan Harkin wrote: > Hi Evan, > > This was sent to the list with no subject line and I wasn't on CC, so > I didn't see it. > > Are you still using this patch and want it in, i.e. does it need > review and test? > I'm sure it works, but I don't

Re: [edk2] [PATCH] MdeModulePkg/MdeModulePkg.dec: Fix EBC build failure of PciBus driver

I agree with Ard. Building PciBusDxe driver as EBC ARCH is supported from tool perspective, but doesn't make much sense in real world. So the patch is just to resolve the build failure in EBC ARCH. Thanks/Ray > -Original Message- > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]

Re: [edk2] [Patch 1/2] BaseTools/GenFds: Support FDF sections in any order

This patch is fine to me. Reviewed-by: Yonghong Zhu Best Regards, Zhu Yonghong -Original Message- From: Kinney, Michael D Sent: Saturday, October 08, 2016 5:33 AM To: edk2-devel@lists.01.org Cc: Steele, Kelly ; Zhu, Yonghong

Re: [edk2] [PATCH v3 00/10] enhance mmc

On 12 October 2016 at 01:03, Ryan Harkin wrote: > On 11 October 2016 at 17:28, Ard Biesheuvel wrote: >> On 11 October 2016 at 17:27, Ryan Harkin wrote: >>> On 11 October 2016 at 17:24, Ard Biesheuvel

Re: [edk2] [PATCH] IntelSiliconPkg: Fixing syntax bug in IGD_OPREGION_HEADER

Reviewed-by: jiewen@intel.com > -Original Message- > From: Mudusuru, Giri P > Sent: Wednesday, October 12, 2016 1:07 PM > To: edk2-devel@lists.01.org > Cc: Yao, Jiewen > Subject: [edk2] [PATCH] IntelSiliconPkg: Fixing syntax bug in > IGD_OPREGION_HEADER > >

Re: [edk2] ArmPlatformPkg: Allocate VRAM as RuntimeServicesData

On 11 Oct 2016 20:17, "Ard Biesheuvel" wrote: > > On 11 October 2016 at 18:44, Ryan Harkin wrote: > > Hi Evan, > > > > This was sent to the list with no subject line and I wasn't on CC, so > > I didn't see it. > > > > Are you still using this

[edk2] [PATCH] IntelSiliconPkg: Fixing syntax bug in IGD_OPREGION_HEADER

Added missing ; for DVER in IGD_OPREGION_HEADER Cc: Jiewen Yao Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Giri P Mudusuru --- IntelSiliconPkg/Include/IndustryStandard/IgdOpRegion.h | 2 +- 1 file changed, 1

Re: [edk2] [PATCH v3 00/10] enhance mmc

On 12 Oct 2016 00:33, "Haojian Zhuang" wrote: > > On 12 October 2016 at 01:03, Ryan Harkin wrote: > > On 11 October 2016 at 17:28, Ard Biesheuvel wrote: > >> On 11 October 2016 at 17:27, Ryan Harkin

[edk2] [Patch] SecurityPkg OpalPasswordSmm: Fix S3 resume failure.

Changes includes: 1.Check SMM device list before update it to avoid duplicate creation. 2.Clean up the configuration buffer before use it in S3 resume phase. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Eric Dong Cc: Feng Tian

Re: [edk2] [Patch] SecurityPkg OpalPasswordSmm: Fix S3 resume failure.

Looks good to me Reviewed-by: Feng Tian Thanks Feng -Original Message- From: Dong, Eric Sent: Tuesday, October 11, 2016 4:02 PM To: edk2-devel@lists.01.org Cc: Tian, Feng Subject: [Patch] SecurityPkg OpalPasswordSmm: Fix S3 resume failure.

Re: [edk2] [PATCH V2 00/50] Add capsule update and recovery sample.

Jiewen, I responded to Mikes email which has similar statement. As for the example modules you listed that are in MdeModulePkg, I don't think they should be in MdeModulePkg. We should be working to minimize the core of the src tree (mde*) to only what is needed and then use other modules,

[edk2] [patch] MdeModulePkg/SetupBrowser: Send discard info to driver when fail to submit

When fail to submit data and user discard the change, we should send the discard info to river with EFI_BROWSER_ACTION_CHANGED callback. Cc: Liming Gao Cc: Eric Dong Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Dandan Bi

Re: [edk2] [PATCH V2 00/50] Add capsule update and recovery sample.

Mike, Comments inline. I'll reply with code review comments to the individual patches. Thanks Sean > -Original Message- > From: Kinney, Michael D [mailto:michael.d.kin...@intel.com] > Sent: Monday, October 10, 2016 4:29 PM > To: Sean Brogan ; Yao, Jiewen >

Re: [edk2] [PATCH V2 01/50] MdeModulePkg/Include: Add EDKII system FMP capsule header.

This file is for your implementation. I would suggest removing it from MdeModulePkg and into your new package. > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Jiewen Yao > Sent: Friday, September 30, 2016 5:21 AM > To:

Re: [edk2] [PATCH V2 02/50] MdeModulePkg/Include: Add EdkiiSystemCapsuleLib definition.

I would suggest moving this to the "new" package. > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Jiewen Yao > Sent: Friday, September 30, 2016 5:21 AM > To: edk2-devel@lists.01.org > Cc: Michael D Kinney ; Feng

Re: [edk2] [PATCH V2 07/50] MdeModulePkg/MdeModulePkg.dec: Add capsule related definition.

This would all go in the new package instead of MdeModulePkg. Thanks Sean > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Jiewen Yao > Sent: Friday, September 30, 2016 5:21 AM > To: edk2-devel@lists.01.org > Cc: Michael D Kinney

Re: [edk2] [PATCH V2 04/50] MdeModulePkg/Include: Add IniParsingLib header.

This is specific to your implementation and would again suggest moving to your new package. > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Jiewen Yao > Sent: Friday, September 30, 2016 5:21 AM > To: edk2-devel@lists.01.org > Cc:

Re: [edk2] [PATCH V2 03/50] MdeModulePkg/Include: Add FmpAuthenticationLib header.

I think this library and the design of registering different auth handlers is not the right design for FMP auth verification. This isn't something that needs extension thru registration. This is a controlled environment. I also don't think the capsule runtime should be using these auth

Re: [edk2] [PATCH V2 05/50] MdeModulePkg/Include: Add PlatformFlashAccessLib header.

My suggestion is to move to implementation specific package. > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Jiewen Yao > Sent: Friday, September 30, 2016 5:21 AM > To: edk2-devel@lists.01.org > Cc: Michael D Kinney

Re: [edk2] [PATCH V2 06/50] MdeModulePkg/CapsuleLib: Add ProcessCapsules() API.

> -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Jiewen Yao > Sent: Friday, September 30, 2016 5:21 AM > To: edk2-devel@lists.01.org > Cc: Michael D Kinney ; Feng Tian > ; Chao Zhang

Re: [edk2] [PATCH V2 06/50] MdeModulePkg/CapsuleLib: Add ProcessCapsules() API.

Comment about calling ProcessCapsules twice will break in some scenarios. For example windows capsule update will stage multiple capsules at once. If it mixes capsules from both stages and you use memory to preserve capsule contents you will lose your non system capsule because of the reboot.