Unit tests for the math calculations would help with reviews too. Mike
> -----Original Message----- > From: Laszlo Ersek <ler...@redhat.com> > Sent: Tuesday, January 16, 2024 2:03 AM > To: Kinney, Michael D <michael.d.kin...@intel.com>; Pedro Falcato > <pedro.falc...@gmail.com>; devel@edk2.groups.io; Ni, Ray > <ray...@intel.com> > Cc: Desimone, Nathaniel L <nathaniel.l.desim...@intel.com>; Kumar, Rahul > R <rahul.r.ku...@intel.com>; Gerd Hoffmann <kra...@redhat.com> > Subject: Re: [edk2-devel] [PATCH 1/6] UefiCpuPkg/LocalApicTimerDxe: > Duplicate OvmfPkg/LocalApicTimerDxe driver > > On 1/15/24 19:10, Kinney, Michael D wrote: > > Hi Ray, > > > > I think nesting may be possible in physical platforms, but very hard > > to induce. > > > > One option is to consolidate to a single LocalApicTimerDxe > > implementation in the UefiCpuPkg, but allow the platform DSC to either > > specify a Null NestedInterruptTplLib for physical platforms or the > > full one from the OvmfPkg for VM use cases. > > > > The other changes could be included for OvmfPkg use cases. If the VM > > does not support the CPUID leafs, then the PCD value should be used. > > All of this sounds great! > > (And I do think that *some* nesting should not be a problem, on either > physical or virtual platforms, as (IIRC) the interrupt handler stack can > accommodate a certain level of reentrancy. It's the "storm" that is a > problem, but that should never occur on physical machines, I reckon.) > > Assuming a v2 is coming up, my advance request would be for Ray to > re-review the math in patch #4, before posting v2, focusing on integer > overflows, and SafeIntLib (if needed). I've not looked at the patch in > detail yet, but I reviewed something similar not so long ago [1]. The > latter frequency calculation code had been quite commonly used in edk2, > and I needed hours to review just that one patch. Plus, the review > turned up a number of issues to fix. So, preferably such a patch should > not only prevent any integer overflows, but also clarify, in comments, > why overflows are impossible, and/or how they are prevented. > > [1] https://edk2.groups.io/g/devel/message/111613 > http://mid.mail-archive.com/2e42db7c-a927-f47b-7d80- > 632895b11...@redhat.com > > Thanks! > Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113922): https://edk2.groups.io/g/devel/message/113922 Mute This Topic: https://groups.io/mt/103734961/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
