[edk2,3/5] ArmPkg/ArmGicDxe: Expose HardwareInterrupt2 protocol

Message ID 20170911152335.72672-4-evan.lloyd@arm.com
State Superseded
Headers show
Series
  • Untitled series #3948
Related show

Commit Message

Evan Lloyd Sept. 11, 2017, 3:23 p.m.
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>


The existing HardwareInterrupt protocol lacked a means to configure the
level/edge properties of an interrupt.  The new HardwareInterrupt2
protocol introduced this capability.
This patch updates the GIC drivers to provide the new interfaces.
The changes comprise:
  Update to use HardwareInterrupt2 protocol
  Additions to register info in ArmGicLib.h
  Added new functionality (GetTriggerType and SetTriggerType)

The requirement for this change derives from a problem detected on ARM
Juno boards, but the change is of generic (ARM) relevance.

This commit is in response to review on the mailing list and, as
suggested there, rolls Girish's updates onto Ard's original example.

NOTE: At this point the GICv3 code is not tested.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Signed-off-by: Girish Pathak <girish.pathak@arm.com>

Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>

Tested-by: Girish Pathak <girish.pathak@arm.com>

---
 ArmPkg/Drivers/ArmGic/ArmGicDxe.inf       |   3 +-
 ArmPkg/Drivers/ArmGic/ArmGicDxe.h         |  25 +++-
 ArmPkg/Include/Library/ArmGicLib.h        |  12 +-
 ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c   |  44 ++++++-
 ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c | 139 +++++++++++++++++++-
 ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 138 ++++++++++++++++++-
 6 files changed, 354 insertions(+), 7 deletions(-)

-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm Sept. 14, 2017, 5 p.m. | #1
On Mon, Sep 11, 2017 at 04:23:33PM +0100, evan.lloyd@arm.com wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> 

> The existing HardwareInterrupt protocol lacked a means to configure the

> level/edge properties of an interrupt.  The new HardwareInterrupt2

> protocol introduced this capability.

> This patch updates the GIC drivers to provide the new interfaces.

> The changes comprise:

>   Update to use HardwareInterrupt2 protocol

>   Additions to register info in ArmGicLib.h

>   Added new functionality (GetTriggerType and SetTriggerType)

> 

> The requirement for this change derives from a problem detected on ARM

> Juno boards, but the change is of generic (ARM) relevance.

> 

> This commit is in response to review on the mailing list and, as

> suggested there, rolls Girish's updates onto Ard's original example.

> 

> NOTE: At this point the GICv3 code is not tested.


Is this still the case?
I don't see how the code can be merged while that holds true.

> Contributed-under: TianoCore Contribution Agreement 1.0

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> Signed-off-by: Girish Pathak <girish.pathak@arm.com>

> Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>

> Tested-by: Girish Pathak <girish.pathak@arm.com>

> ---

>  ArmPkg/Drivers/ArmGic/ArmGicDxe.inf       |   3 +-

>  ArmPkg/Drivers/ArmGic/ArmGicDxe.h         |  25 +++-

>  ArmPkg/Include/Library/ArmGicLib.h        |  12 +-

>  ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c   |  44 ++++++-

>  ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c | 139 +++++++++++++++++++-

>  ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 138 ++++++++++++++++++-

>  6 files changed, 354 insertions(+), 7 deletions(-)

> 

> diff --git a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf

> index e554301c4b28022c805f69242cf6ee979d19abc2..d5921533fb68fa32c3e0705b05930700ee81da07 100644

> --- a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf

> +++ b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf

> @@ -1,7 +1,7 @@

>  #/** @file

>  #

>  #  Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>

> -#  Copyright (c) 2012 - 2015, ARM Ltd. All rights reserved.<BR>

> +#  Copyright (c) 2012 - 2017, ARM Ltd. All rights reserved.<BR>

>  #

>  #  This program and the accompanying materials

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

> @@ -48,6 +48,7 @@ [LibraryClasses]

>  

>  [Protocols]

>    gHardwareInterruptProtocolGuid

> +  gHardwareInterrupt2ProtocolGuid

>    gEfiCpuArchProtocolGuid

>  

>  [Pcd.common]

> diff --git a/ArmPkg/Drivers/ArmGic/ArmGicDxe.h b/ArmPkg/Drivers/ArmGic/ArmGicDxe.h

> index 1018f2004e75d879a72c2d6bf37b64051e720d12..cefa4c2d4e4a05c54e51642db0f471e9a338afb6 100644

> --- a/ArmPkg/Drivers/ArmGic/ArmGicDxe.h

> +++ b/ArmPkg/Drivers/ArmGic/ArmGicDxe.h

> @@ -1,6 +1,6 @@

>  /*++

>  

> -Copyright (c) 2013-2014, ARM Ltd. All rights reserved.<BR>

> +Copyright (c) 2013-2017, ARM Ltd. All rights reserved.<BR>

>  

>  This program and the accompanying materials

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

> @@ -24,6 +24,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.

>  

>  #include <Protocol/Cpu.h>

>  #include <Protocol/HardwareInterrupt.h>

> +#include <Protocol/HardwareInterrupt2.h>

>  

>  extern UINTN                        mGicNumInterrupts;

>  extern HARDWARE_INTERRUPT_HANDLER  *gRegisteredInterruptHandlers;

> @@ -34,6 +35,7 @@ extern HARDWARE_INTERRUPT_HANDLER  *gRegisteredInterruptHandlers;

>  EFI_STATUS

>  InstallAndRegisterInterruptService (

>    IN EFI_HARDWARE_INTERRUPT_PROTOCOL   *InterruptProtocol,

> +  IN EFI_HARDWARE_INTERRUPT2_PROTOCOL  *Interrupt2Protocol,

>    IN EFI_CPU_INTERRUPT_HANDLER          InterruptHandler,

>    IN EFI_EVENT_NOTIFY                   ExitBootServicesEvent

>    );

> @@ -64,4 +66,25 @@ GicV3DxeInitialize (

>    IN EFI_SYSTEM_TABLE   *SystemTable

>    );

>  

> +

> +// Shared code

> +

> +/**

> +  Calculate GICD_ICFGRn base address and corresponding bit

> +  field Int_config[1] of the GIC distributor register.

> +

> +  @param Source       Hardware source of the interrupt.

> +  @param RegAddress   Corresponding GICD_ICFGRn base address.

> +  @param Config1Bit   Bit number of F Int_config[1] bit in the register.

> +

> +  @retval EFI_SUCCESS       Source interrupt supported.

> +  @retval EFI_UNSUPPORTED   Source interrupt is not supported.

> +**/

> +EFI_STATUS

> +GicGetDistributorIcfgBaseAndBit (

> +  IN HARDWARE_INTERRUPT_SOURCE             Source,

> +  OUT UINTN                               *RegAddress,

> +  OUT UINTN                               *Config1Bit

> +  );

> +

>  #endif

> diff --git a/ArmPkg/Include/Library/ArmGicLib.h b/ArmPkg/Include/Library/ArmGicLib.h

> index f7b546895d116f81c65a853fcdb067ec7601b2da..1c8d8cf6a7c39e2b5e2e36feb3e5433f29f488e2 100644

> --- a/ArmPkg/Include/Library/ArmGicLib.h

> +++ b/ArmPkg/Include/Library/ArmGicLib.h

> @@ -1,6 +1,6 @@

>  /** @file

>  *

> -*  Copyright (c) 2011-2015, ARM Limited. All rights reserved.

> +*  Copyright (c) 2011-2017, ARM Limited. All rights reserved.

>  *

>  *  This program and the accompanying materials

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

> @@ -51,10 +51,18 @@

>  #define ARM_GIC_ICDDCR_ARE      (1 << 4) // Affinity Routing Enable (ARE)

>  #define ARM_GIC_ICDDCR_DS       (1 << 6) // Disable Security (DS)

>  

> +// GICD_ICDICFR bits

> +#define ARM_GIC_ICDICFR_WIDTH            32   // ICDICFR is a 32 bit register

> +#define ARM_GIC_ICDICFR_BYTES            (ARM_GIC_ICDICFR_WIDTH / 8)

> +#define ARM_GIC_ICDICFR_F_WIDTH          2    // Each F field is 2 bits

> +#define ARM_GIC_ICDICFR_F_STRIDE         16   // (32/2) F fields per register

> +#define ARM_GIC_ICDICFR_F_CONFIG1_BIT    1    // Bit number within F field

> +#define ARM_GIC_ICDICFR_LEVEL_TRIGGERED  0x0  // Level triggered interrupt

> +#define ARM_GIC_ICDICFR_EDGE_TRIGGERED   0x1  // Edge triggered interrupt

> +

>  

>  // GIC Redistributor

>  

> -


Spurious whitespace change.

>  #define ARM_GICR_CTLR_FRAME_SIZE    SIZE_64KB

>  #define ARM_GICR_SGI_PPI_FRAME_SIZE SIZE_64KB

>  

> diff --git a/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c b/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c

> index 88cb455b75bb8e8cb22157643a392403ce93129d..13a7fae07b856025ab1c0eac97d07ec4c4df20a9 100644

> --- a/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c

> +++ b/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c

> @@ -1,6 +1,6 @@

>  /*++

>  

> -Copyright (c) 2013-2014, ARM Ltd. All rights reserved.<BR>

> +Copyright (c) 2013-2017, ARM Ltd. All rights reserved.<BR>

>  

>  This program and the accompanying materials

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

> @@ -43,6 +43,46 @@ UINTN mGicNumInterrupts                 = 0;

>  

>  HARDWARE_INTERRUPT_HANDLER  *gRegisteredInterruptHandlers = NULL;

>  

> +

> +/**

> +  Calculate GICD_ICFGRn base address and corresponding bit

> +  field Int_config[1] of the GIC distributor register.

> +

> +  @param Source       Hardware source of the interrupt.

> +  @param RegAddress   Corresponding GICD_ICFGRn base address.

> +  @param Config1Bit   Bit number of F Int_config[1] bit in the register.

> +

> +  @retval EFI_SUCCESS       Source interrupt supported.

> +  @retval EFI_UNSUPPORTED   Source interrupt is not supported.

> +**/

> +EFI_STATUS

> +GicGetDistributorIcfgBaseAndBit (

> +  IN HARDWARE_INTERRUPT_SOURCE             Source,

> +  OUT UINTN                               *RegAddress,

> +  OUT UINTN                               *Config1Bit

> +  )

> +{

> +  UINTN                  RegIndex;

> +  UINTN                  Field;

> +

> +  if (Source >= mGicNumInterrupts) {

> +    ASSERT(Source < mGicNumInterrupts);

> +    return EFI_UNSUPPORTED;

> +  }

> +

> +  RegIndex = Source / ARM_GIC_ICDICFR_F_STRIDE;  // NOTE: truncation is significant

> +  Field = Source % ARM_GIC_ICDICFR_F_STRIDE;

> +  *RegAddress = PcdGet64 (PcdGicDistributorBase)

> +                  + ARM_GIC_ICDICFR

> +                  + (ARM_GIC_ICDICFR_BYTES * RegIndex);


Should these continuation lines really be indented?

> +  *Config1Bit = ((Field * ARM_GIC_ICDICFR_F_WIDTH)

> +                 + ARM_GIC_ICDICFR_F_CONFIG1_BIT);

> +

> +  return EFI_SUCCESS;

> +}

> +

> +

> +

>  /**

>    Register Handler for the specified interrupt source.

>  

> @@ -88,6 +128,7 @@ RegisterInterruptSource (

>  EFI_STATUS

>  InstallAndRegisterInterruptService (

>    IN EFI_HARDWARE_INTERRUPT_PROTOCOL   *InterruptProtocol,

> +  IN EFI_HARDWARE_INTERRUPT2_PROTOCOL  *Interrupt2Protocol,

>    IN EFI_CPU_INTERRUPT_HANDLER          InterruptHandler,

>    IN EFI_EVENT_NOTIFY                   ExitBootServicesEvent

>    )

> @@ -105,6 +146,7 @@ InstallAndRegisterInterruptService (

>    Status = gBS->InstallMultipleProtocolInterfaces (

>                    &gHardwareInterruptHandle,

>                    &gHardwareInterruptProtocolGuid, InterruptProtocol,

> +                  &gHardwareInterrupt2ProtocolGuid, Interrupt2Protocol,

>                    NULL

>                    );

>    if (EFI_ERROR (Status)) {

> diff --git a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c

> index 50ec90207b515d849cbf64f0a4b0d639b3868e60..41db2277132d47dda1a047b73eaf63323b5b9aaf 100644

> --- a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c

> +++ b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c

> @@ -29,6 +29,7 @@ Abstract:

>  #define ARM_GIC_DEFAULT_PRIORITY  0x80

>  

>  extern EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV2Protocol;

> +extern EFI_HARDWARE_INTERRUPT2_PROTOCOL gHardwareInterrupt2V2Protocol;

>  

>  STATIC UINT32 mGicInterruptInterfaceBase;

>  STATIC UINT32 mGicDistributorBase;

> @@ -184,7 +185,7 @@ GicV2IrqInterruptHandler (

>      // Call the registered interrupt handler.

>      InterruptHandler (GicInterrupt, SystemContext);

>    } else {

> -    DEBUG ((EFI_D_ERROR, "Spurious GIC interrupt: 0x%x\n", GicInterrupt));

> +    DEBUG ((DEBUG_ERROR, "Spurious GIC interrupt: 0x%x\n", GicInterrupt));


I don't see anything changing here other than changing the EFI_D_ERROR
to DEBUG_ERROR. Preferably leave such non-functional changes out of
functional patches, even if it leaves the file inconsistent.
If a DEBUG line has a functional modification, clearly it should be
updated to the new form at the same time.

>      GicV2EndOfInterrupt (&gHardwareInterruptV2Protocol, GicInterrupt);

>    }

>  }

> @@ -200,6 +201,141 @@ EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV2Protocol = {

>    GicV2EndOfInterrupt

>  };

>  

> +/**

> +  Get interrupt trigger type of an interrupt

> +

> +  @param This          Instance pointer for this protocol

> +  @param Source        Hardware source of the interrupt.

> +  @param TriggerType   Returns interrupt trigger type.

> +

> +  @retval EFI_SUCCESS       Source interrupt supported.

> +  @retval EFI_UNSUPPORTED   Source interrupt is not supported.

> +**/

> +STATIC

> +EFI_STATUS

> +EFIAPI

> +GicV2GetTriggerType (

> +  IN  EFI_HARDWARE_INTERRUPT2_PROTOCOL      *This,

> +  IN  HARDWARE_INTERRUPT_SOURCE              Source,

> +  OUT EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE  *TriggerType

> +  )

> +{

> +  UINTN                   RegAddress;

> +  UINTN                   Config1Bit;

> +  EFI_STATUS              Status;

> +

> +  Status = GicGetDistributorIcfgBaseAndBit (

> +              Source,

> +              &RegAddress,

> +              &Config1Bit

> +              );

> +

> +  if (EFI_ERROR (Status)) {

> +    return Status;

> +  }

> +

> +  if (MmioBitFieldRead32 (RegAddress, Config1Bit, Config1Bit) == 0) {


Why not

  if (MmioRead32 (RegAddress) & (1 << Config1Bit) == 0) {

?

> +     *TriggerType = EFI_HARDWARE_INTERRUPT2_TRIGGER_LEVEL_HIGH;

> +  } else {

> +     *TriggerType = EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING;

> +  }

> +

> +  return EFI_SUCCESS;

> +}

> +

> +/**

> +  Set interrupt trigger type of an interrupt

> +

> +  @param This          Instance pointer for this protocol

> +  @param Source        Hardware source of the interrupt.

> +  @param TriggerType   Interrupt trigger type.

> +

> +  @retval EFI_SUCCESS       Source interrupt supported.

> +  @retval EFI_UNSUPPORTED   Source interrupt is not supported.

> +**/

> +STATIC

> +EFI_STATUS

> +EFIAPI

> +GicV2SetTriggerType (

> +  IN  EFI_HARDWARE_INTERRUPT2_PROTOCOL      *This,

> +  IN  HARDWARE_INTERRUPT_SOURCE             Source,

> +  IN  EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE  TriggerType

> +  )

> +{

> +  UINTN                   RegAddress;

> +  UINTN                   Config1Bit;

> +  UINT32                  Value;

> +  EFI_STATUS              Status;

> +  BOOLEAN                 SourceEnabled;

> +

> +  if (TriggerType != EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING

> +     && TriggerType != EFI_HARDWARE_INTERRUPT2_TRIGGER_LEVEL_HIGH) {


One more space indentation.
Could benefit from added parentheses:

  if ((TriggerType != EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING) &&
      (TriggerType != EFI_HARDWARE_INTERRUPT2_TRIGGER_LEVEL_HIGH)) {

> +          DEBUG ((DEBUG_ERROR, "Invalid interrupt trigger type: %d\n", \

> +                  TriggerType));

> +          ASSERT (FALSE);

> +          return EFI_UNSUPPORTED;

> +  }

> +

> +  Status = GicGetDistributorIcfgBaseAndBit (

> +             Source,

> +             &RegAddress,

> +             &Config1Bit

> +             );

> +

> +  if (EFI_ERROR (Status)) {

> +    return Status;

> +  }

> +

> +  Status = GicV2GetInterruptSourceState (

> +             (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,

> +             Source,

> +             &SourceEnabled

> +             );

> +

> +  if (EFI_ERROR (Status)) {

> +    return Status;

> +  }

> +

> +  Value = (TriggerType == EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING)

> +          ?  ARM_GIC_ICDICFR_EDGE_TRIGGERED

> +          :  ARM_GIC_ICDICFR_LEVEL_TRIGGERED;

> +

> +  // Before changing the value, we must disable the interrupt,

> +  // otherwise GIC behavior is UNPREDICTABLE.

> +  if (SourceEnabled) {

> +    GicV2DisableInterruptSource (

> +      (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,

> +      Source

> +      );

> +  }

> +

> +  MmioAndThenOr32 (

> +    RegAddress,

> +    ~(0x1 << Config1Bit),

> +    Value << Config1Bit

> +    );

> +

> +  // Restore interrupt state

> +  if (SourceEnabled) {

> +    GicV2EnableInterruptSource (

> +      (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,

> +      Source

> +      );

> +  }

> +

> +  return EFI_SUCCESS;

> +}

> +

> +EFI_HARDWARE_INTERRUPT2_PROTOCOL gHardwareInterrupt2V2Protocol = {

> +  (HARDWARE_INTERRUPT2_REGISTER)RegisterInterruptSource,

> +  (HARDWARE_INTERRUPT2_ENABLE)GicV2EnableInterruptSource,

> +  (HARDWARE_INTERRUPT2_DISABLE)GicV2DisableInterruptSource,

> +  (HARDWARE_INTERRUPT2_INTERRUPT_STATE)GicV2GetInterruptSourceState,

> +  (HARDWARE_INTERRUPT2_END_OF_INTERRUPT)GicV2EndOfInterrupt,

> +  GicV2GetTriggerType,

> +  GicV2SetTriggerType

> +};

> +

>  /**

>    Shutdown our hardware

>  

> @@ -324,6 +460,7 @@ GicV2DxeInitialize (

>  

>    Status = InstallAndRegisterInterruptService (

>               &gHardwareInterruptV2Protocol,

> +             &gHardwareInterrupt2V2Protocol,

>               GicV2IrqInterruptHandler,

>               GicV2ExitBootServicesEvent

>               );

> diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c

> index 69b2d8d794e151e25f06cbea079e2796d9793a43..0c1d5b53119e8cad7ae67c57e9efaa51c1386be6 100644

> --- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c

> +++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c

> @@ -19,6 +19,7 @@

>  #define ARM_GIC_DEFAULT_PRIORITY  0x80

>  

>  extern EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV3Protocol;

> +extern EFI_HARDWARE_INTERRUPT2_PROTOCOL gHardwareInterrupt2V3Protocol;

>  

>  STATIC UINTN mGicDistributorBase;

>  STATIC UINTN mGicRedistributorsBase;

> @@ -177,7 +178,7 @@ GicV3IrqInterruptHandler (

>      // Call the registered interrupt handler.

>      InterruptHandler (GicInterrupt, SystemContext);

>    } else {

> -    DEBUG ((EFI_D_ERROR, "Spurious GIC interrupt: 0x%x\n", GicInterrupt));

> +    DEBUG ((DEBUG_ERROR, "Spurious GIC interrupt: 0x%x\n", GicInterrupt));


Non-functional change, please leave out.

>      GicV3EndOfInterrupt (&gHardwareInterruptV3Protocol, GicInterrupt);

>    }

>  }

> @@ -193,6 +194,140 @@ EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV3Protocol = {

>    GicV3EndOfInterrupt

>  };

>  

> +/**

> +  Get interrupt trigger type of an interrupt

> +

> +  @param This          Instance pointer for this protocol

> +  @param Source        Hardware source of the interrupt.

> +  @param TriggerType   Returns interrupt trigger type.

> +

> +  @retval EFI_SUCCESS       Source interrupt supported.

> +  @retval EFI_UNSUPPORTED   Source interrupt is not supported.

> +**/

> +STATIC

> +EFI_STATUS

> +EFIAPI

> +GicV3GetTriggerType (

> +  IN  EFI_HARDWARE_INTERRUPT2_PROTOCOL      *This,

> +  IN  HARDWARE_INTERRUPT_SOURCE             Source,

> +  OUT EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE  *TriggerType

> +  )

> +{

> +  UINTN                   RegAddress;

> +  UINTN                   Config1Bit;

> +  EFI_STATUS              Status;

> +

> +  Status = GicGetDistributorIcfgBaseAndBit (

> +             Source,

> +             &RegAddress,

> +             &Config1Bit

> +             );

> +

> +  if (EFI_ERROR (Status)) {

> +    return Status;

> +  }

> +

> +  if (MmioBitFieldRead32 (RegAddress, Config1Bit, Config1Bit) == 0) {


  if (MmioRead32 (RegAddress) & (1 << Config1Bit) == 0) {

?

> +     *TriggerType = EFI_HARDWARE_INTERRUPT2_TRIGGER_LEVEL_HIGH;

> +  } else {

> +     *TriggerType = EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING;

> +  }

> +

> +  return EFI_SUCCESS;

> +}

> +

> +/**

> +  Set interrupt trigger type of an interrupt

> +

> +  @param This          Instance pointer for this protocol

> +  @param Source        Hardware source of the interrupt.

> +  @param TriggerType   Interrupt trigger type.

> +

> +  @retval EFI_SUCCESS       Source interrupt supported.

> +  @retval EFI_UNSUPPORTED   Source interrupt is not supported.

> +**/

> +STATIC

> +EFI_STATUS

> +EFIAPI

> +GicV3SetTriggerType (

> +  IN  EFI_HARDWARE_INTERRUPT2_PROTOCOL      *This,

> +  IN  HARDWARE_INTERRUPT_SOURCE             Source,

> +  IN  EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE  TriggerType

> +  )

> +{

> +  UINTN                   RegAddress;

> +  UINTN                   Config1Bit;

> +  UINT32                  Value;

> +  EFI_STATUS              Status;

> +  BOOLEAN                 SourceEnabled;

> +

> +  if (TriggerType != EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING

> +     && TriggerType != EFI_HARDWARE_INTERRUPT2_TRIGGER_LEVEL_HIGH) {


Missing indentation space, and could do with added parentheses).

/
    Leif

> +          DEBUG ((DEBUG_ERROR, "Invalid interrupt trigger type: %d\n", \

> +                 TriggerType));

> +          ASSERT (FALSE);

> +          return EFI_UNSUPPORTED;

> +  }

> +

> +  Status = GicGetDistributorIcfgBaseAndBit (

> +             Source,

> +             &RegAddress,

> +             &Config1Bit

> +             );

> +

> +  if (EFI_ERROR (Status)) {

> +    return Status;

> +  }

> +

> +  Status = GicV3GetInterruptSourceState (

> +             (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,

> +             Source,

> +             &SourceEnabled

> +             );

> +

> +  if (EFI_ERROR (Status)) {

> +    return Status;

> +  }

> +

> +  Value = (TriggerType == EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING)

> +          ?  ARM_GIC_ICDICFR_EDGE_TRIGGERED

> +          :  ARM_GIC_ICDICFR_LEVEL_TRIGGERED;

> +

> +  // Before changing the value, we must disable the interrupt,

> +  // otherwise GIC behavior is UNPREDICTABLE.

> +  if (SourceEnabled) {

> +    GicV3DisableInterruptSource (

> +      (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,

> +      Source

> +      );

> +  }

> +

> +  MmioAndThenOr32 (

> +    RegAddress,

> +    ~(0x1 << Config1Bit),

> +    Value << Config1Bit

> +    );

> +  // Restore interrupt state

> +  if (SourceEnabled) {

> +    GicV3EnableInterruptSource (

> +      (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,

> +      Source

> +      );

> +  }

> +

> +  return EFI_SUCCESS;

> +}

> +

> +EFI_HARDWARE_INTERRUPT2_PROTOCOL gHardwareInterrupt2V3Protocol = {

> +  (HARDWARE_INTERRUPT2_REGISTER)RegisterInterruptSource,

> +  (HARDWARE_INTERRUPT2_ENABLE)GicV3EnableInterruptSource,

> +  (HARDWARE_INTERRUPT2_DISABLE)GicV3DisableInterruptSource,

> +  (HARDWARE_INTERRUPT2_INTERRUPT_STATE)GicV3GetInterruptSourceState,

> +  (HARDWARE_INTERRUPT2_END_OF_INTERRUPT)GicV3EndOfInterrupt,

> +  GicV3GetTriggerType,

> +  GicV3SetTriggerType

> +};

> +

>  /**

>    Shutdown our hardware

>  

> @@ -354,6 +489,7 @@ GicV3DxeInitialize (

>  

>    Status = InstallAndRegisterInterruptService (

>               &gHardwareInterruptV3Protocol,

> +             &gHardwareInterrupt2V3Protocol,

>               GicV3IrqInterruptHandler,

>               GicV3ExitBootServicesEvent

>               );

> -- 

> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch

diff --git a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
index e554301c4b28022c805f69242cf6ee979d19abc2..d5921533fb68fa32c3e0705b05930700ee81da07 100644
--- a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
+++ b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
@@ -1,7 +1,7 @@ 
 #/** @file
 #
 #  Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
-#  Copyright (c) 2012 - 2015, ARM Ltd. All rights reserved.<BR>
+#  Copyright (c) 2012 - 2017, ARM Ltd. All rights reserved.<BR>
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -48,6 +48,7 @@  [LibraryClasses]
 
 [Protocols]
   gHardwareInterruptProtocolGuid
+  gHardwareInterrupt2ProtocolGuid
   gEfiCpuArchProtocolGuid
 
 [Pcd.common]
diff --git a/ArmPkg/Drivers/ArmGic/ArmGicDxe.h b/ArmPkg/Drivers/ArmGic/ArmGicDxe.h
index 1018f2004e75d879a72c2d6bf37b64051e720d12..cefa4c2d4e4a05c54e51642db0f471e9a338afb6 100644
--- a/ArmPkg/Drivers/ArmGic/ArmGicDxe.h
+++ b/ArmPkg/Drivers/ArmGic/ArmGicDxe.h
@@ -1,6 +1,6 @@ 
 /*++
 
-Copyright (c) 2013-2014, ARM Ltd. All rights reserved.<BR>
+Copyright (c) 2013-2017, ARM Ltd. All rights reserved.<BR>
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
@@ -24,6 +24,7 @@  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 #include <Protocol/Cpu.h>
 #include <Protocol/HardwareInterrupt.h>
+#include <Protocol/HardwareInterrupt2.h>
 
 extern UINTN                        mGicNumInterrupts;
 extern HARDWARE_INTERRUPT_HANDLER  *gRegisteredInterruptHandlers;
@@ -34,6 +35,7 @@  extern HARDWARE_INTERRUPT_HANDLER  *gRegisteredInterruptHandlers;
 EFI_STATUS
 InstallAndRegisterInterruptService (
   IN EFI_HARDWARE_INTERRUPT_PROTOCOL   *InterruptProtocol,
+  IN EFI_HARDWARE_INTERRUPT2_PROTOCOL  *Interrupt2Protocol,
   IN EFI_CPU_INTERRUPT_HANDLER          InterruptHandler,
   IN EFI_EVENT_NOTIFY                   ExitBootServicesEvent
   );
@@ -64,4 +66,25 @@  GicV3DxeInitialize (
   IN EFI_SYSTEM_TABLE   *SystemTable
   );
 
+
+// Shared code
+
+/**
+  Calculate GICD_ICFGRn base address and corresponding bit
+  field Int_config[1] of the GIC distributor register.
+
+  @param Source       Hardware source of the interrupt.
+  @param RegAddress   Corresponding GICD_ICFGRn base address.
+  @param Config1Bit   Bit number of F Int_config[1] bit in the register.
+
+  @retval EFI_SUCCESS       Source interrupt supported.
+  @retval EFI_UNSUPPORTED   Source interrupt is not supported.
+**/
+EFI_STATUS
+GicGetDistributorIcfgBaseAndBit (
+  IN HARDWARE_INTERRUPT_SOURCE             Source,
+  OUT UINTN                               *RegAddress,
+  OUT UINTN                               *Config1Bit
+  );
+
 #endif
diff --git a/ArmPkg/Include/Library/ArmGicLib.h b/ArmPkg/Include/Library/ArmGicLib.h
index f7b546895d116f81c65a853fcdb067ec7601b2da..1c8d8cf6a7c39e2b5e2e36feb3e5433f29f488e2 100644
--- a/ArmPkg/Include/Library/ArmGicLib.h
+++ b/ArmPkg/Include/Library/ArmGicLib.h
@@ -1,6 +1,6 @@ 
 /** @file
 *
-*  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
+*  Copyright (c) 2011-2017, ARM Limited. All rights reserved.
 *
 *  This program and the accompanying materials
 *  are licensed and made available under the terms and conditions of the BSD License
@@ -51,10 +51,18 @@ 
 #define ARM_GIC_ICDDCR_ARE      (1 << 4) // Affinity Routing Enable (ARE)
 #define ARM_GIC_ICDDCR_DS       (1 << 6) // Disable Security (DS)
 
+// GICD_ICDICFR bits
+#define ARM_GIC_ICDICFR_WIDTH            32   // ICDICFR is a 32 bit register
+#define ARM_GIC_ICDICFR_BYTES            (ARM_GIC_ICDICFR_WIDTH / 8)
+#define ARM_GIC_ICDICFR_F_WIDTH          2    // Each F field is 2 bits
+#define ARM_GIC_ICDICFR_F_STRIDE         16   // (32/2) F fields per register
+#define ARM_GIC_ICDICFR_F_CONFIG1_BIT    1    // Bit number within F field
+#define ARM_GIC_ICDICFR_LEVEL_TRIGGERED  0x0  // Level triggered interrupt
+#define ARM_GIC_ICDICFR_EDGE_TRIGGERED   0x1  // Edge triggered interrupt
+
 
 // GIC Redistributor
 
-
 #define ARM_GICR_CTLR_FRAME_SIZE    SIZE_64KB
 #define ARM_GICR_SGI_PPI_FRAME_SIZE SIZE_64KB
 
diff --git a/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c b/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c
index 88cb455b75bb8e8cb22157643a392403ce93129d..13a7fae07b856025ab1c0eac97d07ec4c4df20a9 100644
--- a/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c
+++ b/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c
@@ -1,6 +1,6 @@ 
 /*++
 
-Copyright (c) 2013-2014, ARM Ltd. All rights reserved.<BR>
+Copyright (c) 2013-2017, ARM Ltd. All rights reserved.<BR>
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
@@ -43,6 +43,46 @@  UINTN mGicNumInterrupts                 = 0;
 
 HARDWARE_INTERRUPT_HANDLER  *gRegisteredInterruptHandlers = NULL;
 
+
+/**
+  Calculate GICD_ICFGRn base address and corresponding bit
+  field Int_config[1] of the GIC distributor register.
+
+  @param Source       Hardware source of the interrupt.
+  @param RegAddress   Corresponding GICD_ICFGRn base address.
+  @param Config1Bit   Bit number of F Int_config[1] bit in the register.
+
+  @retval EFI_SUCCESS       Source interrupt supported.
+  @retval EFI_UNSUPPORTED   Source interrupt is not supported.
+**/
+EFI_STATUS
+GicGetDistributorIcfgBaseAndBit (
+  IN HARDWARE_INTERRUPT_SOURCE             Source,
+  OUT UINTN                               *RegAddress,
+  OUT UINTN                               *Config1Bit
+  )
+{
+  UINTN                  RegIndex;
+  UINTN                  Field;
+
+  if (Source >= mGicNumInterrupts) {
+    ASSERT(Source < mGicNumInterrupts);
+    return EFI_UNSUPPORTED;
+  }
+
+  RegIndex = Source / ARM_GIC_ICDICFR_F_STRIDE;  // NOTE: truncation is significant
+  Field = Source % ARM_GIC_ICDICFR_F_STRIDE;
+  *RegAddress = PcdGet64 (PcdGicDistributorBase)
+                  + ARM_GIC_ICDICFR
+                  + (ARM_GIC_ICDICFR_BYTES * RegIndex);
+  *Config1Bit = ((Field * ARM_GIC_ICDICFR_F_WIDTH)
+                 + ARM_GIC_ICDICFR_F_CONFIG1_BIT);
+
+  return EFI_SUCCESS;
+}
+
+
+
 /**
   Register Handler for the specified interrupt source.
 
@@ -88,6 +128,7 @@  RegisterInterruptSource (
 EFI_STATUS
 InstallAndRegisterInterruptService (
   IN EFI_HARDWARE_INTERRUPT_PROTOCOL   *InterruptProtocol,
+  IN EFI_HARDWARE_INTERRUPT2_PROTOCOL  *Interrupt2Protocol,
   IN EFI_CPU_INTERRUPT_HANDLER          InterruptHandler,
   IN EFI_EVENT_NOTIFY                   ExitBootServicesEvent
   )
@@ -105,6 +146,7 @@  InstallAndRegisterInterruptService (
   Status = gBS->InstallMultipleProtocolInterfaces (
                   &gHardwareInterruptHandle,
                   &gHardwareInterruptProtocolGuid, InterruptProtocol,
+                  &gHardwareInterrupt2ProtocolGuid, Interrupt2Protocol,
                   NULL
                   );
   if (EFI_ERROR (Status)) {
diff --git a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
index 50ec90207b515d849cbf64f0a4b0d639b3868e60..41db2277132d47dda1a047b73eaf63323b5b9aaf 100644
--- a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
+++ b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
@@ -29,6 +29,7 @@  Abstract:
 #define ARM_GIC_DEFAULT_PRIORITY  0x80
 
 extern EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV2Protocol;
+extern EFI_HARDWARE_INTERRUPT2_PROTOCOL gHardwareInterrupt2V2Protocol;
 
 STATIC UINT32 mGicInterruptInterfaceBase;
 STATIC UINT32 mGicDistributorBase;
@@ -184,7 +185,7 @@  GicV2IrqInterruptHandler (
     // Call the registered interrupt handler.
     InterruptHandler (GicInterrupt, SystemContext);
   } else {
-    DEBUG ((EFI_D_ERROR, "Spurious GIC interrupt: 0x%x\n", GicInterrupt));
+    DEBUG ((DEBUG_ERROR, "Spurious GIC interrupt: 0x%x\n", GicInterrupt));
     GicV2EndOfInterrupt (&gHardwareInterruptV2Protocol, GicInterrupt);
   }
 }
@@ -200,6 +201,141 @@  EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV2Protocol = {
   GicV2EndOfInterrupt
 };
 
+/**
+  Get interrupt trigger type of an interrupt
+
+  @param This          Instance pointer for this protocol
+  @param Source        Hardware source of the interrupt.
+  @param TriggerType   Returns interrupt trigger type.
+
+  @retval EFI_SUCCESS       Source interrupt supported.
+  @retval EFI_UNSUPPORTED   Source interrupt is not supported.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+GicV2GetTriggerType (
+  IN  EFI_HARDWARE_INTERRUPT2_PROTOCOL      *This,
+  IN  HARDWARE_INTERRUPT_SOURCE              Source,
+  OUT EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE  *TriggerType
+  )
+{
+  UINTN                   RegAddress;
+  UINTN                   Config1Bit;
+  EFI_STATUS              Status;
+
+  Status = GicGetDistributorIcfgBaseAndBit (
+              Source,
+              &RegAddress,
+              &Config1Bit
+              );
+
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  if (MmioBitFieldRead32 (RegAddress, Config1Bit, Config1Bit) == 0) {
+     *TriggerType = EFI_HARDWARE_INTERRUPT2_TRIGGER_LEVEL_HIGH;
+  } else {
+     *TriggerType = EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING;
+  }
+
+  return EFI_SUCCESS;
+}
+
+/**
+  Set interrupt trigger type of an interrupt
+
+  @param This          Instance pointer for this protocol
+  @param Source        Hardware source of the interrupt.
+  @param TriggerType   Interrupt trigger type.
+
+  @retval EFI_SUCCESS       Source interrupt supported.
+  @retval EFI_UNSUPPORTED   Source interrupt is not supported.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+GicV2SetTriggerType (
+  IN  EFI_HARDWARE_INTERRUPT2_PROTOCOL      *This,
+  IN  HARDWARE_INTERRUPT_SOURCE             Source,
+  IN  EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE  TriggerType
+  )
+{
+  UINTN                   RegAddress;
+  UINTN                   Config1Bit;
+  UINT32                  Value;
+  EFI_STATUS              Status;
+  BOOLEAN                 SourceEnabled;
+
+  if (TriggerType != EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING
+     && TriggerType != EFI_HARDWARE_INTERRUPT2_TRIGGER_LEVEL_HIGH) {
+          DEBUG ((DEBUG_ERROR, "Invalid interrupt trigger type: %d\n", \
+                  TriggerType));
+          ASSERT (FALSE);
+          return EFI_UNSUPPORTED;
+  }
+
+  Status = GicGetDistributorIcfgBaseAndBit (
+             Source,
+             &RegAddress,
+             &Config1Bit
+             );
+
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Status = GicV2GetInterruptSourceState (
+             (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,
+             Source,
+             &SourceEnabled
+             );
+
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Value = (TriggerType == EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING)
+          ?  ARM_GIC_ICDICFR_EDGE_TRIGGERED
+          :  ARM_GIC_ICDICFR_LEVEL_TRIGGERED;
+
+  // Before changing the value, we must disable the interrupt,
+  // otherwise GIC behavior is UNPREDICTABLE.
+  if (SourceEnabled) {
+    GicV2DisableInterruptSource (
+      (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,
+      Source
+      );
+  }
+
+  MmioAndThenOr32 (
+    RegAddress,
+    ~(0x1 << Config1Bit),
+    Value << Config1Bit
+    );
+
+  // Restore interrupt state
+  if (SourceEnabled) {
+    GicV2EnableInterruptSource (
+      (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,
+      Source
+      );
+  }
+
+  return EFI_SUCCESS;
+}
+
+EFI_HARDWARE_INTERRUPT2_PROTOCOL gHardwareInterrupt2V2Protocol = {
+  (HARDWARE_INTERRUPT2_REGISTER)RegisterInterruptSource,
+  (HARDWARE_INTERRUPT2_ENABLE)GicV2EnableInterruptSource,
+  (HARDWARE_INTERRUPT2_DISABLE)GicV2DisableInterruptSource,
+  (HARDWARE_INTERRUPT2_INTERRUPT_STATE)GicV2GetInterruptSourceState,
+  (HARDWARE_INTERRUPT2_END_OF_INTERRUPT)GicV2EndOfInterrupt,
+  GicV2GetTriggerType,
+  GicV2SetTriggerType
+};
+
 /**
   Shutdown our hardware
 
@@ -324,6 +460,7 @@  GicV2DxeInitialize (
 
   Status = InstallAndRegisterInterruptService (
              &gHardwareInterruptV2Protocol,
+             &gHardwareInterrupt2V2Protocol,
              GicV2IrqInterruptHandler,
              GicV2ExitBootServicesEvent
              );
diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
index 69b2d8d794e151e25f06cbea079e2796d9793a43..0c1d5b53119e8cad7ae67c57e9efaa51c1386be6 100644
--- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
+++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
@@ -19,6 +19,7 @@ 
 #define ARM_GIC_DEFAULT_PRIORITY  0x80
 
 extern EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV3Protocol;
+extern EFI_HARDWARE_INTERRUPT2_PROTOCOL gHardwareInterrupt2V3Protocol;
 
 STATIC UINTN mGicDistributorBase;
 STATIC UINTN mGicRedistributorsBase;
@@ -177,7 +178,7 @@  GicV3IrqInterruptHandler (
     // Call the registered interrupt handler.
     InterruptHandler (GicInterrupt, SystemContext);
   } else {
-    DEBUG ((EFI_D_ERROR, "Spurious GIC interrupt: 0x%x\n", GicInterrupt));
+    DEBUG ((DEBUG_ERROR, "Spurious GIC interrupt: 0x%x\n", GicInterrupt));
     GicV3EndOfInterrupt (&gHardwareInterruptV3Protocol, GicInterrupt);
   }
 }
@@ -193,6 +194,140 @@  EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV3Protocol = {
   GicV3EndOfInterrupt
 };
 
+/**
+  Get interrupt trigger type of an interrupt
+
+  @param This          Instance pointer for this protocol
+  @param Source        Hardware source of the interrupt.
+  @param TriggerType   Returns interrupt trigger type.
+
+  @retval EFI_SUCCESS       Source interrupt supported.
+  @retval EFI_UNSUPPORTED   Source interrupt is not supported.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+GicV3GetTriggerType (
+  IN  EFI_HARDWARE_INTERRUPT2_PROTOCOL      *This,
+  IN  HARDWARE_INTERRUPT_SOURCE             Source,
+  OUT EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE  *TriggerType
+  )
+{
+  UINTN                   RegAddress;
+  UINTN                   Config1Bit;
+  EFI_STATUS              Status;
+
+  Status = GicGetDistributorIcfgBaseAndBit (
+             Source,
+             &RegAddress,
+             &Config1Bit
+             );
+
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  if (MmioBitFieldRead32 (RegAddress, Config1Bit, Config1Bit) == 0) {
+     *TriggerType = EFI_HARDWARE_INTERRUPT2_TRIGGER_LEVEL_HIGH;
+  } else {
+     *TriggerType = EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING;
+  }
+
+  return EFI_SUCCESS;
+}
+
+/**
+  Set interrupt trigger type of an interrupt
+
+  @param This          Instance pointer for this protocol
+  @param Source        Hardware source of the interrupt.
+  @param TriggerType   Interrupt trigger type.
+
+  @retval EFI_SUCCESS       Source interrupt supported.
+  @retval EFI_UNSUPPORTED   Source interrupt is not supported.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+GicV3SetTriggerType (
+  IN  EFI_HARDWARE_INTERRUPT2_PROTOCOL      *This,
+  IN  HARDWARE_INTERRUPT_SOURCE             Source,
+  IN  EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE  TriggerType
+  )
+{
+  UINTN                   RegAddress;
+  UINTN                   Config1Bit;
+  UINT32                  Value;
+  EFI_STATUS              Status;
+  BOOLEAN                 SourceEnabled;
+
+  if (TriggerType != EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING
+     && TriggerType != EFI_HARDWARE_INTERRUPT2_TRIGGER_LEVEL_HIGH) {
+          DEBUG ((DEBUG_ERROR, "Invalid interrupt trigger type: %d\n", \
+                 TriggerType));
+          ASSERT (FALSE);
+          return EFI_UNSUPPORTED;
+  }
+
+  Status = GicGetDistributorIcfgBaseAndBit (
+             Source,
+             &RegAddress,
+             &Config1Bit
+             );
+
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Status = GicV3GetInterruptSourceState (
+             (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,
+             Source,
+             &SourceEnabled
+             );
+
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Value = (TriggerType == EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING)
+          ?  ARM_GIC_ICDICFR_EDGE_TRIGGERED
+          :  ARM_GIC_ICDICFR_LEVEL_TRIGGERED;
+
+  // Before changing the value, we must disable the interrupt,
+  // otherwise GIC behavior is UNPREDICTABLE.
+  if (SourceEnabled) {
+    GicV3DisableInterruptSource (
+      (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,
+      Source
+      );
+  }
+
+  MmioAndThenOr32 (
+    RegAddress,
+    ~(0x1 << Config1Bit),
+    Value << Config1Bit
+    );
+  // Restore interrupt state
+  if (SourceEnabled) {
+    GicV3EnableInterruptSource (
+      (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,
+      Source
+      );
+  }
+
+  return EFI_SUCCESS;
+}
+
+EFI_HARDWARE_INTERRUPT2_PROTOCOL gHardwareInterrupt2V3Protocol = {
+  (HARDWARE_INTERRUPT2_REGISTER)RegisterInterruptSource,
+  (HARDWARE_INTERRUPT2_ENABLE)GicV3EnableInterruptSource,
+  (HARDWARE_INTERRUPT2_DISABLE)GicV3DisableInterruptSource,
+  (HARDWARE_INTERRUPT2_INTERRUPT_STATE)GicV3GetInterruptSourceState,
+  (HARDWARE_INTERRUPT2_END_OF_INTERRUPT)GicV3EndOfInterrupt,
+  GicV3GetTriggerType,
+  GicV3SetTriggerType
+};
+
 /**
   Shutdown our hardware
 
@@ -354,6 +489,7 @@  GicV3DxeInitialize (
 
   Status = InstallAndRegisterInterruptService (
              &gHardwareInterruptV3Protocol,
+             &gHardwareInterrupt2V3Protocol,
              GicV3IrqInterruptHandler,
              GicV3ExitBootServicesEvent
              );