Re: [edk2-devel] [PATCH V7 36/37] UefiCpuPkg: Setting initial-count register as the last step

["Lendacky, Thomas" <thomas.lendacky@amd.com>]

On 2/28/22 01:21, Min Xu via groups.io wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3711
> 
> Per SDM, changing the mode of APIC timer (from one-shot to periodic or
> vice versa) by writing to the timer LVT entry does not start the timer.
> To start the timer, it is necessary to write to the initial-count
> register.
> 
> If initial-count is wrote before mode change, it's possible that timer
> expired before the mode change. Thus failing the periodic mode.

I'm replying to this patch since I can't find patch V12 46/47 anywhere in
my email.

I've bisected a regression in the Linux kernel to this patch when an
SEV-SNP guest is booted. The following message is issued in the kernel for
every AP being brought online:

APIC: Stale IRR: 00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000020 ISR: 00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000

Possibly a timing issue involving the mode switch with the interrupt
unmasked. If I leave the interrupt masked and only un-mask it
after the programming of the init-count, then the message goes away.

Thoughts?

Thanks,
Tom

> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Julien Grall <julien@xen.org>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
>   .../Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c    | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> index 2d17177df12b..f26d9c93894f 100644
> --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> @@ -967,11 +967,6 @@ InitializeApicTimer (
>     //
>     InitializeLocalApicSoftwareEnable (TRUE);
>   
> -  //
> -  // Program init-count register.
> -  //
> -  WriteLocalApicReg (XAPIC_TIMER_INIT_COUNT_OFFSET, InitCount);
> -
>     if (DivideValue != 0) {
>       ASSERT (DivideValue <= 128);
>       ASSERT (DivideValue == GetPowerOfTwo32 ((UINT32)DivideValue));
> @@ -996,6 +991,11 @@ InitializeApicTimer (
>     LvtTimer.Bits.Mask   = 0;
>     LvtTimer.Bits.Vector = Vector;
>     WriteLocalApicReg (XAPIC_LVT_TIMER_OFFSET, LvtTimer.Uint32);
> +
> +  //
> +  // Program init-count register.
> +  //
> +  WriteLocalApicReg (XAPIC_TIMER_INIT_COUNT_OFFSET, InitCount);
>   }
>   
>   /**