Re: [edk2-devel] [PATCH V2 28/28] OvmfPkg: Add LocalApicTimerDxe

["Min Xu" <min.m.xu@intel.com>]

On October 12, 2021 9:02 PM, Gerd Hoffmann wrote:
> On Tue, Oct 05, 2021 at 11:39:39AM +0800, Min Xu wrote:
> > RFC： https://bugzilla.tianocore.org/show_bug.cgi?id=3429
> >
> > TDX guest supports LocalApicTimer. But in current OvmfPkg the
> > supported timer is 8254TimerDxe. So
> > gUefiOvmfPkgTokenSpaceGuid.PcdTimerSelector
> > is introduced to select the running Timer. The Timer driver will check
> > the TimerSelector in its entry point. The default Timer is 8254.
> 
> Hmm.
> 
> We already have a local apic timer implementation (XenTimerDxe).  Works fine
> with kvm, microvm already uses that.  See commit 76602f45dcd9
> ("OvmfPkg/Microvm: use XenTimerDxe (lapic timer)").
> 
> So, first I'd suggest to just use that (maybe rename the thing to avoid confusion
> as it isn't really Xen specific).
Thanks for reminder. We can use XenTimerDxe as the LocalApicTimerDxe in Tdx guest. There will be a separate patch to rename XenTimerDxe to LocalApicTimerDxe in the next version.
> 
> Next question is whenever there is a need for a runtime switch.  I doubt it is
> possible to create a virtual machine without lapic, so switching ovmf from 8254
> (aka pit) to lapic unconditionally should work fine.
> Quick smoke test (patch below) shows no obvious problems.
> 
I am not quite sure if there will be any side effect if we switch ovmf (X64) from 8254 to lapic unconditionally. Quick smoke test does show no obvious problems (EDK2 CI shows no error either). But since 8254 timer has already been used in OvmfPkgX64, then there always a reason why 8254 is used.
I am thinking if it is a more secure way to introduce PcdTimerSelector (to select timer in run-time) this time.
We can revisit this proposal (switch ovmf from 8254 to lapic unconditionally) when we are pretty sure there is no side effect in the future.

Thanks.
Min