Re: [edk2-devel] [PATCH 1/6] UefiCpuPkg/LocalApicTimerDxe: Duplicate OvmfPkg/LocalApicTimerDxe driver

["Laszlo Ersek" <lersek@redhat.com>]

On 1/15/24 09:59, Pedro Falcato wrote:
> On Mon, Jan 15, 2024 at 8:04 AM Ni, Ray <ray.ni@intel.com> wrote:
>>
>> This commit only duplicates the OvmfPkg/LocalApicTimerDxe.
>> Following commits will enhance the driver.
> 
> Hi,
> 
> Please describe why you're doing this change. i.e what's your use case
> for LocalApicTimerDxe,

Right, I have the same question -- although, admittedly, I've not been
CC'd on the cover letter (0/6), and the reason could be captured there.
(I'm not going to check my edk2-devel folder again today, only finishing
up my personal INBOX. So tomorrow when I check edk2-devel, I might
actually find the reason in the cover letter.)

On a second thought, I'm (retroactively?) surprised that
LocalApicTimerDxe was (apparently?) first introduced under OvmfPkg --
i.e., for VMs? That's not impossible, but feels a bit strange.

> and why are you duplicating this instead of
> moving OvmfPkg's (why do we need to maintain 2 separate versions of
> what is essentially the same driver)?

Not wanting the NestedInterruptTplLib dependency / functionality in
UefiCpuPkg's instance of this driver seems justified (patch 2). Also,
patch 4 calculates the timer frequency based on CPUID; might not be
straightforward to use on VMs without prior verification.

The tricky parts to review in this series are the math in patch 4 (such
code is usually prone to integer overflows, so that's my concern by
default there), plus the big comment in patch 6.

I'll try to look at them later.

Meta-comment for Ray: patches like patch#1 are best formatted with the
option "--find-copies-harder" (try it please, the output speaks for itself).

Thanks
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113847): https://edk2.groups.io/g/devel/message/113847
Mute This Topic: https://groups.io/mt/103734961/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-