From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout02.posteo.de (mout02.posteo.de [185.67.36.66]) by mx.groups.io with SMTP id smtpd.web11.850.1623778292481735943 for ; Tue, 15 Jun 2021 10:31:33 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@posteo.de header.s=2017 header.b=dDnglEOm; spf=pass (domain: posteo.de, ip: 185.67.36.66, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [89.146.220.130]) by mout02.posteo.de (Postfix) with ESMTPS id A0898240100 for ; Tue, 15 Jun 2021 19:31:29 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1623778289; bh=ww7LPAxfvPRfp0za5i2W+1Qhb2OR+OMLNFGvKA979v4=; h=Subject:To:Cc:From:Date:From; b=dDnglEOmJoQf9FTk4arSZpRk75JshM+HWKE7MhURgVKoYSt0DG+OsCsGpyGecBj4T Tls/TK+LuES4LtWT59EQ4s8HtCpZxRaiFUNv/gop+iS8ZxWBPgdwv4y/VvEhFwyMbF igNhpW9ZT+GKMQmEPWEYVx22/ag+XJCo4wo86s1KMukrk5jGC6J6NwIPfyhdoADbO7 EnJs4mLB5H/jkPpCPbJmuVheRtLbCjXSR7yMaalpJem8hbpFIKXrcsuNxuhiFIpjkl i9QZ7pg03Jx6Wdwpk3qfPOff55NW1OflqwUSQvp6tWV0imnZxgQY7ft7NXHowX4HNN m0Jw8/WHY4Ljw== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4G4FjV2jGZz6tmS; Tue, 15 Jun 2021 19:31:26 +0200 (CEST) Subject: Re: [edk2-devel] [PATCH v2 2/3] UefiPayloadPkg: Add PayloadLoaderPeim which can load ELF payload To: "Ni, Ray" , "devel@edk2.groups.io" , "mcb30@ipxe.org" Cc: "Ma, Maurice" , "Dong, Guo" , "You, Benjamin" References: <20210603062259.1390-1-ray.ni@intel.com> <20210603062259.1390-3-ray.ni@intel.com> <812b8f13-e951-5d27-9bd1-61711e6dd840@posteo.de> <486c5ab8-240e-3ac5-5a4a-7f368cb68644@posteo.de> <8eb8db11-90c2-57e0-6868-3532c5af8073@posteo.de> <785b1d37-9314-4909-7d1f-efa343018238@posteo.de> <24a2cc8a-69c5-5961-448c-1d07a0628eda@ipxe.org> <168735878F610E03.10233@groups.io> From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= Message-ID: <50593faa-de78-defb-5e6e-c74a87b29ddd@posteo.de> Date: Tue, 15 Jun 2021 17:31:26 +0000 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: quoted-printable Hey Ray, Sure, thanks a lot for taking the time. I will need a bit longer to get=20 to it, sorry. :) Best regards, Marvin On 15.06.21 16:36, Ni, Ray wrote: > Marvin, > I have sent out https://edk2.groups.io/g/devel/message/76429 to address your f= eedbacks. > > Can I merge the 3 patches first? (we can continue discussing the more-ch= ecks patch.) > > Thanks, > Ray > >> -----Original Message----- >> From: devel@edk2.groups.io On Behalf Of Ni, Ray >> Sent: Thursday, June 10, 2021 7:37 PM >> To: devel@edk2.groups.io; mcb30@ipxe.org; mhaeuser@posteo.de >> Cc: Ma, Maurice ; Dong, Guo ;= You, Benjamin >> Subject: Re: [edk2-devel] [PATCH v2 2/3] UefiPayloadPkg: Add PayloadLoa= derPeim which can load ELF payload >> >> >> >>> -----Original Message----- >>> From: devel@edk2.groups.io On Behalf Of Michael= Brown >>> Sent: Thursday, June 10, 2021 6:43 PM >>> To: devel@edk2.groups.io; mhaeuser@posteo.de; Ni, Ray >>> Cc: Ma, Maurice ; Dong, Guo = ; You, Benjamin >>> Subject: Re: [edk2-devel] [PATCH v2 2/3] UefiPayloadPkg: Add PayloadLo= aderPeim which can load ELF payload >>> >>> On 10/06/2021 11:13, Marvin H=C3=A4user wrote: >>>> On 10.06.21 11:39, Ni, Ray wrote: >>>>>> Maybe for some context, my main issue at first was that the checks = are >>>>>> all proper runtime checks with no ASSERTs at all, so I got confused= how >>>>>> this situation could happen in a realistic scenario. I needed to tr= ace >>>>>> the ParseStatus data flow to understand the idea is basically the s= ame >>>>>> as in the PE library. Code in a way is self-documenting, and this >>>>>> personally gave me a hard time understanding why it is written this= way. >>>>>> But thanks for clarifying your intention! :) >>>>> I assume you are ok with the ParseStatus. >>>>> I will send new version based on mail discussion. Thanks! >>>> I don't need to be okay with anything, I'm not a maintainer nor an >>>> authority. But I gave my opinion, which is that it is dead code that >>>> makes the design/flow harder to understand for a third party, at no >>>> obvious benefit. >>> FWIW, I strongly agree with Marvin on this: having ParseStatus in its >>> current form is a bad idea since it adds no value but does incur a cos= t. >> OK. I can remove that=F0=9F=98=8A >> >> >> >>=20 >>