On Tue, 23 Apr 2019 at 00:37, Michael D Kinney <michael.d.kin...@intel.com> wrote: > > Ard, > > I only see one potential issue. > > The size of the buffer copied is based on the FwResourceCount field. > The actual size of based on the FwResourceCountMax field. Your patch > does not set FwResourceCountMax to FwResourceCount, so this may confuse > the OS consumers that may think the buffer allocated for ESRT is larger > that is actually is. > > /// > /// The number of firmware resources in the table, must not be zero. > /// > UINT32 FwResourceCount; > /// > /// The maximum number of resource array entries that can be within the > table > /// without reallocating the table, must not be zero. > /// > UINT32 FwResourceCountMax; >
The OS never sees our copy of the table. That copy is for internal use only by the DxeCapsuleLibFmp implementation. > The simplest fix is to use FwResourceCountMax instead of FwResourceCount > for the copy size. The other option is to set FwResourceCountMax to > FwResourceCountMax after the copy. > Given the above, I'll go with the latter. It is unlikely to matter, but it is more correct, and prevents any potential confusion in the future. > The rest of the patch looks good. With one of the two changes above: > > Reviewed-by: Michael D Kinney <michael.d.kin...@intel.com> > Thanks. > > > -----Original Message----- > > From: devel@edk2.groups.io > > [mailto:devel@edk2.groups.io] On Behalf Of Ard > > Biesheuvel > > Sent: Monday, April 22, 2019 3:03 PM > > To: Wu, Hao A <hao.a...@intel.com> > > Cc: devel@edk2.groups.io; Wang, Jian J > > <jian.j.w...@intel.com>; Kinney, Michael D > > <michael.d.kin...@intel.com> > > Subject: Re: [edk2-devel] [PATCH v2] > > MdeModulePkg/DxeCapsuleLibFmp: avoid ESRT accesses at > > runtime > > > > On Mon, 22 Apr 2019 at 09:14, Wu, Hao A > > <hao.a...@intel.com> wrote: > > > > > > > -----Original Message----- > > > > From: devel@edk2.groups.io > > [mailto:devel@edk2.groups.io] On Behalf Of Ard > > > > Biesheuvel > > > > Sent: Saturday, April 20, 2019 6:35 PM > > > > To: devel@edk2.groups.io > > > > Cc: Wu, Hao A; Wang, Jian J; Kinney, Michael D; Ard > > Biesheuvel > > > > Subject: [edk2-devel] [PATCH v2] > > MdeModulePkg/DxeCapsuleLibFmp: avoid > > > > ESRT accesses at runtime > > > > > > Hello Ard, > > > > > > It seems to me v2 patch makes a copy of the ESRT for > > runtime usage (rather > > > than avoid using ESRT), so the title of the commit > > may need update as > > > well. > > > > > > Could you help to update the patch subject when you > > push the change? > > > > > > Other than that, the patch looks good to me. But I > > would like to see if > > > Mike has additional comments on this: > > > > > > Acked-by: Hao Wu <hao.a...@intel.com> > > > > > > > Thanks Hao. I will modify the subject to 'clone ESRT > > for runtime access' > > > > Mike: any comments? > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#39377): https://edk2.groups.io/g/devel/message/39377 Mute This Topic: https://groups.io/mt/31254157/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
