From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yb1-f177.google.com (mail-yb1-f177.google.com [209.85.219.177]) by mx.groups.io with SMTP id smtpd.web10.6073.1619516893474828314 for ; Tue, 27 Apr 2021 02:48:13 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20161025 header.b=AHAht+EC; spf=pass (domain: gmail.com, ip: 209.85.219.177, mailfrom: adr.her.arc.95@gmail.com) Received: by mail-yb1-f177.google.com with SMTP id p202so24910038ybg.8 for ; Tue, 27 Apr 2021 02:48:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=O3ykZT4ct9zHsMkrXkhmzAripUmsk2H2+ZocUhwt8/M=; b=AHAht+EC6pK3Pq/0mUVDJwjLrBgReaYnuEDn8Jyql966JUddoz0EE9/BpmMwGz9aMU UqC0VoqK+qrjNZ60k1vb/UaTgkKC+SqbCWBGL4G92sA1vb8XUSAFp6OcV6n54AfwleP1 ijyCRaGE+MVBtpWM3aokG95m0zTaXv14N1DB25Ojzxc7DkhR9d5n8jLKCZwhrrLdJ/hO H1v+oDPE3emfGR7maxz/pA3d5JF9MWRDu9iWg9jtmwH0l9CauaN1QeyjOcy+dg2gg8OB d0f7BnR7lXPndPt5AUU+afImC4/3WHqVAIAKybm8nAi7cT6dxpLiOYLY9sJIWFGpL1Gd SPzg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=O3ykZT4ct9zHsMkrXkhmzAripUmsk2H2+ZocUhwt8/M=; b=HnG12gYhOiQav97Q8/AieaPheMgCNu8ZXS+I3FzCiJepo9/Subeok3Yh8n+Lcwdda8 dSMiZ2gu0eT8nyYcWO2WLvAM/p9YBPFYDZRnZVWSsokq2T4TGMO1krZkcs2Fg+gbXaoM HBE6Hz5dpisiM3ivXhejv87lc7QcGWQz1D9ufvMqh6nNqRdYVDdW/gOupjZj+OCpQolT A4Di9w5XqPsUCeJboYmbrR6YYGWY9Sl99qWOoi0Y1MQV1FClFEotDfsuMUNcp7SAewim OhGetFFq0qQPfLkI9El3THJKEKnBr3mQvH/kfF+PN1CwhXDKQBNIXJalRH0SyS64bXVT My2w== X-Gm-Message-State: AOAM5310Z8b1YezWGiXn8yQeMJibJ8+wq22hbspnb5gvOIzG+LyeijAq qpBivSxlu2m0zLfEO4M722RB70DRCWjBSCrYUivDGU3EELVXFHVi X-Google-Smtp-Source: ABdhPJy3PRI+tb5tNVMjliSNSNEdvhOMRT7agcFibctJumZfhxkxeI71C3IRT7fhSErBnn2Ze1cy63mp2hPkJd3LQDE= X-Received: by 2002:a25:d094:: with SMTP id h142mr20439903ybg.477.1619516892710; Tue, 27 Apr 2021 02:48:12 -0700 (PDT) MIME-Version: 1.0 References: <2a2fef8f58a134336f19e049fbeb2c951bffaa52.1619206373.git.adr.her.arc.95@gmail.com> In-Reply-To: From: =?UTF-8?B?QWRyacOhbiBIZXJyZXJh?= Date: Tue, 27 Apr 2021 10:48:01 +0100 Message-ID: Subject: Re: [edk2-devel] [PATCH] ArmPkg/ArmGicLib: fix maximum interrupts supported To: Sami Mujawar Cc: "devel@edk2.groups.io" , Joey Gouly , Ard Biesheuvel , "leif@nuviainc.com" , nd Content-Type: multipart/alternative; boundary="000000000000190d0b05c0f12928" --000000000000190d0b05c0f12928 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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=C3=A1n. On Mon, 26 Apr 2021 at 08:56, Sami Mujawar 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 on behalf of Adri=C3= = =A1n > Herrera via groups.io > *Date: *Saturday, 24 April 2021 at 03:57 > *To: *devel@edk2.groups.io > *Cc: *Adri=C3=A1n Herrera > *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 =3D 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 =3D > 0b11111. > This avoids the overhead of guarding ArmGicLib functions while not > requiring to modify the drivers code. > > Signed-off-by: Adri=C3=A1n Herrera > --- > 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) & 0x1= F) > + 1); > > > + UINT32 ITLinesNumber; > > > + > > > + ITLinesNumber =3D MmioRead32 (GicDistributorBase + ARM_GIC_ICDICTR) & > 0x1F; > > > + return MIN (32 * (ITLinesNumber+ 1), 1020); > > > } > > > > > > VOID > > > -- > 2.30.0 > > > >=20 > > --000000000000190d0b05c0f12928 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Sami,

That patch provides the same s= olution. 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=C3=A1n.

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

Hi Adrian,

=C2=A0

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 re= view and tested-by.

Can you check if this patch covers the proble= ms you describe, please?

=C2=A0

Regards,

=C2=A0

Sami Mujawar

=C2=A0

From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Adri=C3=A1n Herrera via groups.io <adr.her.arc.95=3Dgmail.com@groups.io>
Date: Saturday, 24 April 2021 at 03:57
To: devel= @edk2.groups.io <devel@edk2.groups.io>
Cc: Adri=C3=A1n Herrera <adr.her.arc.95@gmail.com>
Subject: [edk2-devel] [PATCH] ArmPkg/ArmGicLib: fix maximum interru= pts supported

The maximum number of = interrupts supported is determined as
32 * (GICD_TYPER.ITLinesNumber + 1).

When GICD_TYPER.ITLinesNumber =3D 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:
=C2=A0 (1) ArmGicLib functions do not guard against special interrupts. =C2=A0 (2) ArmGicGetMaxNumInterrupts number includes special interrupts. =C2=A0 (2) ArmGicV*Dxe relies on ArmGicGetMaxNumInterrupts, and thus
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 programs special interrupts through ArmGicL= ib.

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

Signed-off-by: Adri=C3=A1n Herrera <adr.her.arc.95@gmail.com>
---
=C2=A0ArmPkg/Drivers/ArmGic/ArmGicLib.c | 5 ++++-
=C2=A01 file changed, 4 insertions(+), 1 deletion(-)

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


=C2=A0=C2=A0 )


=C2=A0{


-=C2=A0 return 32 * ((MmioRead32 (GicDistributorBase + ARM_GIC_ICDICTR) &a= mp; 0x1F) + 1);


+=C2=A0 UINT32 ITLinesNumber;


+


+=C2=A0 ITLinesNumber =3D MmioRead32 (GicDistributorBase + ARM_GIC_ICDICTR= ) & 0x1F;


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


=C2=A0}


=C2=A0


=C2=A0VOID


--
2.30.0





--000000000000190d0b05c0f12928--