1. Save/restore ICR high 32bit value and check Delivery Status before sending IPI. It could be fix the interrupted issue between ICR high/low writes by SMI handler.

2. Save/restore CPU Interrupt state around sending IPI. It could avoid sending IPI be interrupted by CPU interrupt handler.
3. Add note for SetApicMode() API that must not be called from an interrupt handler or SMI handler.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
Reviewed-by: Kinney, Michael <michael.d.kinney@intel.com>
Reviewed-by: Mudusuru, Giri <giri.p.mudusuru@intel.com>


git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@15652 6f19259b-4bc3-4df7-8a09-765794883524

3 files changed, 76 insertions, 2 deletions

diff --git a/UefiCpuPkg/Include/Library/LocalApicLib.h b/UefiCpuPkg/Include/Library/LocalApicLib.h
index 896020487..b92b99e11 100644
--- a/UefiCpuPkg/Include/Library/LocalApicLib.h
+++ b/UefiCpuPkg/Include/Library/LocalApicLib.h
@@ -4,7 +4,7 @@
   Local APIC library assumes local APIC is enabled. It does not

   handles cases where local APIC is disabled.

 

-  Copyright (c) 2010 - 2013, Intel Corporation. All rights reserved.<BR>

+  Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.<BR>

   This program and the accompanying materials

   are licensed and made available under the terms and conditions of the BSD License

   which accompanies this distribution.  The full text of the license may be found at

@@ -68,6 +68,9 @@ GetApicMode (
   If the specified local APIC mode can't be set as current, then ASSERT.

 

   @param ApicMode APIC mode to be set.

+

+  @note  This API must not be called from an interrupt handler or SMI handler.

+         It may result in unpredictable behavior.

 **/

 VOID

 EFIAPI

diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
index 6bf9c43dc..6f8cd2eb9 100644
--- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
+++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
@@ -139,18 +139,47 @@ SendIpi (
   )

 {

   LOCAL_APIC_ICR_LOW IcrLowReg;

+  UINT32             IcrHigh;

+  BOOLEAN            InterruptState;

 

   ASSERT (GetApicMode () == LOCAL_APIC_MODE_XAPIC);

   ASSERT (ApicId <= 0xff);

 

+  InterruptState = SaveAndDisableInterrupts ();

+

+  //

+  // Save existing contents of ICR high 32 bits

+  //

+  IcrHigh = ReadLocalApicReg (XAPIC_ICR_HIGH_OFFSET);

+

+  //

+  // Wait for DeliveryStatus clear in case a previous IPI

+  //  is still being sent

+  //

+  do {

+    IcrLowReg.Uint32 = ReadLocalApicReg (XAPIC_ICR_LOW_OFFSET);

+  } while (IcrLowReg.Bits.DeliveryStatus != 0);

+

   //

   // For xAPIC, the act of writing to the low doubleword of the ICR causes the IPI to be sent.

   //

   WriteLocalApicReg (XAPIC_ICR_HIGH_OFFSET, ApicId << 24);

   WriteLocalApicReg (XAPIC_ICR_LOW_OFFSET, IcrLow);

+

+  //

+  // Wait for DeliveryStatus clear again

+  //

   do {

     IcrLowReg.Uint32 = ReadLocalApicReg (XAPIC_ICR_LOW_OFFSET);

   } while (IcrLowReg.Bits.DeliveryStatus != 0);

+

+  //

+  // And restore old contents of ICR high

+  //

+  WriteLocalApicReg (XAPIC_ICR_HIGH_OFFSET, IcrHigh);

+

+  SetInterruptState (InterruptState);

+

 }

 

 //

@@ -193,6 +222,9 @@ GetApicMode (
   If the specified local APIC mode can't be set as current, then ASSERT.

 

   @param ApicMode APIC mode to be set.

+

+  @note  This API must not be called from an interrupt handler or SMI handler.

+         It may result in unpredictable behavior.

 **/

 VOID

 EFIAPI

diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
index ec54d01eb..bf318e06e 100644
--- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
+++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
@@ -180,19 +180,55 @@ SendIpi (
   UINT64             MsrValue;

   LOCAL_APIC_ICR_LOW IcrLowReg;

   UINTN              LocalApciBaseAddress;

+  UINT32             IcrHigh;

+  BOOLEAN            InterruptState;

 

+  //

+  // Legacy APIC or X2APIC?

+  //

   if (GetApicMode () == LOCAL_APIC_MODE_XAPIC) {

     ASSERT (ApicId <= 0xff);

 

+    InterruptState = SaveAndDisableInterrupts ();

+

     //

-    // For xAPIC, the act of writing to the low doubleword of the ICR causes the IPI to be sent.

+    // Get base address of this LAPIC

     //

     LocalApciBaseAddress = GetLocalApicBaseAddress();

+

+    //

+    // Save existing contents of ICR high 32 bits

+    //

+    IcrHigh = MmioRead32 (LocalApciBaseAddress + XAPIC_ICR_HIGH_OFFSET);

+

+    //

+    // Wait for DeliveryStatus clear in case a previous IPI

+    //  is still being sent

+    //

+    do {

+      IcrLowReg.Uint32 = MmioRead32 (LocalApciBaseAddress + XAPIC_ICR_LOW_OFFSET);

+    } while (IcrLowReg.Bits.DeliveryStatus != 0);

+

+    //

+    // For xAPIC, the act of writing to the low doubleword of the ICR causes the IPI to be sent.

+    //

     MmioWrite32 (LocalApciBaseAddress + XAPIC_ICR_HIGH_OFFSET, ApicId << 24);

     MmioWrite32 (LocalApciBaseAddress + XAPIC_ICR_LOW_OFFSET, IcrLow);

+

+    //

+    // Wait for DeliveryStatus clear again

+    //

     do {

       IcrLowReg.Uint32 = MmioRead32 (LocalApciBaseAddress + XAPIC_ICR_LOW_OFFSET);

     } while (IcrLowReg.Bits.DeliveryStatus != 0);

+

+    //

+    // And restore old contents of ICR high

+    //

+    MmioWrite32 (LocalApciBaseAddress + XAPIC_ICR_HIGH_OFFSET, IcrHigh);

+

+    SetInterruptState (InterruptState);

+

   } else {

     //

     // For x2APIC, A single MSR write to the Interrupt Command Register is required for dispatching an 

@@ -242,6 +278,9 @@ GetApicMode (
   If the specified local APIC mode can't be set as current, then ASSERT.

 

   @param ApicMode APIC mode to be set.

+

+  @note  This API must not be called from an interrupt handler or SMI handler.

+         It may result in unpredictable behavior.

 **/

 VOID

 EFIAPI