1. Patchew
  2. EDK2
  3. View series

[edk2-devel] [PATCH] ArmPkg/ArmGicLib: fix maximum interrupts supported

Adrián Herrera posted 1 patch 3 years ago
Failed in applying to current master (apply log)
ArmPkg/Drivers/ArmGic/ArmGicLib.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
[edk2-devel] [PATCH] ArmPkg/ArmGicLib: fix maximum interrupts supported
Posted by Adrián Herrera 3 years ago 
The maximum number of interrupts supported is determined as
32 * (GICD_TYPER.ITLinesNumber + 1).

When GICD_TYPER.ITLinesNumber = 0b11111, the maximum number of
interrupts supported is 1024. However, both GICv2 and GICv3 reserve
INTIDs 1020-1023 for special purposes.

This results in runtime crashes because:
  (1) ArmGicLib functions do not guard against special interrupts.
  (2) ArmGicGetMaxNumInterrupts number includes special interrupts.
  (2) ArmGicV*Dxe relies on ArmGicGetMaxNumInterrupts, and thus
      programs special interrupts through ArmGicLib.

ArmGicGetMaxNumInterrupts now does not include special interrupts, that
is, it reports 1020 instead of 1024 when GICD_TYPER.ITLinesNumber = 0b11111.
This avoids the overhead of guarding ArmGicLib functions while not
requiring to modify the drivers code.

Signed-off-by: Adrián Herrera <adr.her.arc.95@gmail.com>
---
 ArmPkg/Drivers/ArmGic/ArmGicLib.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
index 6b01c88206..dff1401e9c 100644
--- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
+++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
@@ -120,7 +120,10 @@ ArmGicGetMaxNumInterrupts (
   IN  INTN          GicDistributorBase

   )

 {

-  return 32 * ((MmioRead32 (GicDistributorBase + ARM_GIC_ICDICTR) & 0x1F) + 1);

+  UINT32 ITLinesNumber;

+

+  ITLinesNumber = MmioRead32 (GicDistributorBase + ARM_GIC_ICDICTR) & 0x1F;

+  return MIN (32 * (ITLinesNumber+ 1), 1020);

 }

 

 VOID

-- 
2.30.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74401): https://edk2.groups.io/g/devel/message/74401
Mute This Topic: https://groups.io/mt/82327316/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] ArmPkg/ArmGicLib: fix maximum interrupts supported
Posted by Sami Mujawar 3 years ago 
Hi Adrian,

I believe there is already a similar patch on the mailing list at https://edk2.groups.io/g/devel/message/72596. This patch is pending review and tested-by.
Can you check if this patch covers the problems you describe, please?

Regards,

Sami Mujawar

From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Adrián Herrera via groups.io <adr.her.arc.95=gmail.com@groups.io>
Date: Saturday, 24 April 2021 at 03:57
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Adrián Herrera <adr.her.arc.95@gmail.com>
Subject: [edk2-devel] [PATCH] ArmPkg/ArmGicLib: fix maximum interrupts supported
The maximum number of interrupts supported is determined as
32 * (GICD_TYPER.ITLinesNumber + 1).

When GICD_TYPER.ITLinesNumber = 0b11111, the maximum number of
interrupts supported is 1024. However, both GICv2 and GICv3 reserve
INTIDs 1020-1023 for special purposes.

This results in runtime crashes because:
  (1) ArmGicLib functions do not guard against special interrupts.
  (2) ArmGicGetMaxNumInterrupts number includes special interrupts.
  (2) ArmGicV*Dxe relies on ArmGicGetMaxNumInterrupts, and thus
      programs special interrupts through ArmGicLib.

ArmGicGetMaxNumInterrupts now does not include special interrupts, that
is, it reports 1020 instead of 1024 when GICD_TYPER.ITLinesNumber = 0b11111.
This avoids the overhead of guarding ArmGicLib functions while not
requiring to modify the drivers code.

Signed-off-by: Adrián Herrera <adr.her.arc.95@gmail.com>
---
 ArmPkg/Drivers/ArmGic/ArmGicLib.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
index 6b01c88206..dff1401e9c 100644
--- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
+++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
@@ -120,7 +120,10 @@ ArmGicGetMaxNumInterrupts (
   IN  INTN          GicDistributorBase


   )


 {


-  return 32 * ((MmioRead32 (GicDistributorBase + ARM_GIC_ICDICTR) & 0x1F) + 1);


+  UINT32 ITLinesNumber;


+


+  ITLinesNumber = MmioRead32 (GicDistributorBase + ARM_GIC_ICDICTR) & 0x1F;


+  return MIN (32 * (ITLinesNumber+ 1), 1020);


 }





 VOID


--
2.30.0







-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74414): https://edk2.groups.io/g/devel/message/74414
Mute This Topic: https://groups.io/mt/82327316/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] ArmPkg/ArmGicLib: fix maximum interrupts supported
Posted by Adrián Herrera 2 years, 12 months ago 
Hi Sami,

That patch provides the same solution. I'll be sure to check the mailing
list before sending the next time.
Any benefit on marking this as duplicate? If so, how could I do it?

Thank you,
Adrián.

On Mon, 26 Apr 2021 at 08:56, Sami Mujawar <Sami.Mujawar@arm.com> wrote:

> Hi Adrian,
>
>
>
> I believe there is already a similar patch on the mailing list at
> https://edk2.groups.io/g/devel/message/72596. This patch is pending
> review and tested-by.
>
> Can you check if this patch covers the problems you describe, please?
>
>
>
> Regards,
>
>
>
> Sami Mujawar
>
>
>
> *From: *devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Adrián
> Herrera via groups.io <adr.her.arc.95=gmail.com@groups.io>
> *Date: *Saturday, 24 April 2021 at 03:57
> *To: *devel@edk2.groups.io <devel@edk2.groups.io>
> *Cc: *Adrián Herrera <adr.her.arc.95@gmail.com>
> *Subject: *[edk2-devel] [PATCH] ArmPkg/ArmGicLib: fix maximum interrupts
> supported
>
> The maximum number of interrupts supported is determined as
> 32 * (GICD_TYPER.ITLinesNumber + 1).
>
> When GICD_TYPER.ITLinesNumber = 0b11111, the maximum number of
> interrupts supported is 1024. However, both GICv2 and GICv3 reserve
> INTIDs 1020-1023 for special purposes.
>
> This results in runtime crashes because:
>   (1) ArmGicLib functions do not guard against special interrupts.
>   (2) ArmGicGetMaxNumInterrupts number includes special interrupts.
>   (2) ArmGicV*Dxe relies on ArmGicGetMaxNumInterrupts, and thus
>       programs special interrupts through ArmGicLib.
>
> ArmGicGetMaxNumInterrupts now does not include special interrupts, that
> is, it reports 1020 instead of 1024 when GICD_TYPER.ITLinesNumber =
> 0b11111.
> This avoids the overhead of guarding ArmGicLib functions while not
> requiring to modify the drivers code.
>
> Signed-off-by: Adrián Herrera <adr.her.arc.95@gmail.com>
> ---
>  ArmPkg/Drivers/ArmGic/ArmGicLib.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
> b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
> index 6b01c88206..dff1401e9c 100644
> --- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
> +++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
> @@ -120,7 +120,10 @@ ArmGicGetMaxNumInterrupts (
>    IN  INTN          GicDistributorBase
>
>
>    )
>
>
>  {
>
>
> -  return 32 * ((MmioRead32 (GicDistributorBase + ARM_GIC_ICDICTR) & 0x1F)
> + 1);
>
>
> +  UINT32 ITLinesNumber;
>
>
> +
>
>
> +  ITLinesNumber = MmioRead32 (GicDistributorBase + ARM_GIC_ICDICTR) &
> 0x1F;
>
>
> +  return MIN (32 * (ITLinesNumber+ 1), 1020);
>
>
>  }
>
>
>
>
>
>  VOID
>
>
> --
> 2.30.0
>
>
>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74445): https://edk2.groups.io/g/devel/message/74445
Mute This Topic: https://groups.io/mt/82327316/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] ArmPkg/ArmGicLib: fix maximum interrupts supported
Posted by Sami Mujawar 2 years, 12 months ago 
Hi Adrian,

I think the mailing list discussions here should be sufficient.
However, it would be great if you can test that the earlier patch works and reply with a tested-by.

Regards,

Sami Mujawar

From: Adrián Herrera Arcila <adr.her.arc.95@gmail.com>
Date: Tuesday, 27 April 2021 at 10:48
To: Sami Mujawar <Sami.Mujawar@arm.com>
Cc: devel@edk2.groups.io <devel@edk2.groups.io>, Joey Gouly <Joey.Gouly@arm.com>, Ard Biesheuvel <ardb+tianocore@kernel.org>, leif@nuviainc.com <leif@nuviainc.com>, nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH] ArmPkg/ArmGicLib: fix maximum interrupts supported
Hi Sami,

That patch provides the same solution. I'll be sure to check the mailing list before sending the next time.
Any benefit on marking this as duplicate? If so, how could I do it?

Thank you,
Adrián.

On Mon, 26 Apr 2021 at 08:56, Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>> wrote:
Hi Adrian,

I believe there is already a similar patch on the mailing list at https://edk2.groups.io/g/devel/message/72596. This patch is pending review and tested-by.
Can you check if this patch covers the problems you describe, please?

Regards,

Sami Mujawar

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> on behalf of Adrián Herrera via groups.io<http://groups.io> <adr.her.arc.95=gmail.com@groups.io<mailto:gmail.com@groups.io>>
Date: Saturday, 24 April 2021 at 03:57
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Cc: Adrián Herrera <adr.her.arc.95@gmail.com<mailto:adr.her.arc.95@gmail.com>>
Subject: [edk2-devel] [PATCH] ArmPkg/ArmGicLib: fix maximum interrupts supported
The maximum number of interrupts supported is determined as
32 * (GICD_TYPER.ITLinesNumber + 1).

When GICD_TYPER.ITLinesNumber = 0b11111, the maximum number of
interrupts supported is 1024. However, both GICv2 and GICv3 reserve
INTIDs 1020-1023 for special purposes.

This results in runtime crashes because:
  (1) ArmGicLib functions do not guard against special interrupts.
  (2) ArmGicGetMaxNumInterrupts number includes special interrupts.
  (2) ArmGicV*Dxe relies on ArmGicGetMaxNumInterrupts, and thus
      programs special interrupts through ArmGicLib.

ArmGicGetMaxNumInterrupts now does not include special interrupts, that
is, it reports 1020 instead of 1024 when GICD_TYPER.ITLinesNumber = 0b11111.
This avoids the overhead of guarding ArmGicLib functions while not
requiring to modify the drivers code.

Signed-off-by: Adrián Herrera <adr.her.arc.95@gmail.com<mailto:adr.her.arc.95@gmail.com>>
---
 ArmPkg/Drivers/ArmGic/ArmGicLib.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
index 6b01c88206..dff1401e9c 100644
--- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
+++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
@@ -120,7 +120,10 @@ ArmGicGetMaxNumInterrupts (
   IN  INTN          GicDistributorBase


   )


 {


-  return 32 * ((MmioRead32 (GicDistributorBase + ARM_GIC_ICDICTR) & 0x1F) + 1);


+  UINT32 ITLinesNumber;


+


+  ITLinesNumber = MmioRead32 (GicDistributorBase + ARM_GIC_ICDICTR) & 0x1F;


+  return MIN (32 * (ITLinesNumber+ 1), 1020);


 }





 VOID


--
2.30.0






-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74448): https://edk2.groups.io/g/devel/message/74448
Mute This Topic: https://groups.io/mt/82327316/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.