* [PATCH] ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge
@ 2018-11-14 19:27 Ard Biesheuvel
2018-11-14 20:00 ` Leif Lindholm
0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2018-11-14 19:27 UTC (permalink / raw)
To: edk2-devel
Fix a typo in the 32-bit ARM version of the GICv3 driver, which uses
the wrong system register encoding to access ICC_IAR1, and attempted
to access ICC_IAR0 instead. This results in boot time hangs both
under QEMU emulation and on real hardware.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S | 2 +-
ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.asm | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S b/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S
index a72f3c865163..c308d2fa3e2f 100644
--- a/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S
+++ b/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S
@@ -66,7 +66,7 @@ ASM_FUNC(ArmGicV3EndOfInterrupt)
// VOID
// );
ASM_FUNC(ArmGicV3AcknowledgeInterrupt)
- mrc p15, 0, r0, c12, c8, 0 //ICC_IAR1
+ mrc p15, 0, r0, c12, c12, 0 //ICC_IAR1
bx lr
//VOID
diff --git a/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.asm b/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.asm
index 4228fb59be54..222047d1ad43 100644
--- a/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.asm
+++ b/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.asm
@@ -66,7 +66,7 @@
// VOID
// );
RVCT_ASM_EXPORT ArmGicV3AcknowledgeInterrupt
- mrc p15, 0, r0, c12, c8, 0 //ICC_IAR1
+ mrc p15, 0, r0, c12, c12, 0 //ICC_IAR1
bx lr
//VOID
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge
2018-11-14 19:27 [PATCH] ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge Ard Biesheuvel
@ 2018-11-14 20:00 ` Leif Lindholm
2018-11-14 22:11 ` Laszlo Ersek
0 siblings, 1 reply; 6+ messages in thread
From: Leif Lindholm @ 2018-11-14 20:00 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel
On Wed, Nov 14, 2018 at 11:27:24AM -0800, Ard Biesheuvel wrote:
> Fix a typo in the 32-bit ARM version of the GICv3 driver, which uses
> the wrong system register encoding to access ICC_IAR1, and attempted
> to access ICC_IAR0 instead. This results in boot time hangs both
> under QEMU emulation and on real hardware.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
I would say given how long we've gone without finding this, it's not
justifiable to push this before the stable tag is made - so please
hold off on pushing it until we open the flood gates.
/
Leif
> ---
> ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S | 2 +-
> ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.asm | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S b/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S
> index a72f3c865163..c308d2fa3e2f 100644
> --- a/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S
> +++ b/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S
> @@ -66,7 +66,7 @@ ASM_FUNC(ArmGicV3EndOfInterrupt)
> // VOID
> // );
> ASM_FUNC(ArmGicV3AcknowledgeInterrupt)
> - mrc p15, 0, r0, c12, c8, 0 //ICC_IAR1
> + mrc p15, 0, r0, c12, c12, 0 //ICC_IAR1
> bx lr
>
> //VOID
> diff --git a/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.asm b/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.asm
> index 4228fb59be54..222047d1ad43 100644
> --- a/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.asm
> +++ b/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.asm
> @@ -66,7 +66,7 @@
> // VOID
> // );
> RVCT_ASM_EXPORT ArmGicV3AcknowledgeInterrupt
> - mrc p15, 0, r0, c12, c8, 0 //ICC_IAR1
> + mrc p15, 0, r0, c12, c12, 0 //ICC_IAR1
> bx lr
>
> //VOID
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge
2018-11-14 20:00 ` Leif Lindholm
@ 2018-11-14 22:11 ` Laszlo Ersek
2018-11-15 12:37 ` Ard Biesheuvel
0 siblings, 1 reply; 6+ messages in thread
From: Laszlo Ersek @ 2018-11-14 22:11 UTC (permalink / raw)
To: Leif Lindholm, Ard Biesheuvel; +Cc: edk2-devel
On 11/14/18 21:00, Leif Lindholm wrote:
> On Wed, Nov 14, 2018 at 11:27:24AM -0800, Ard Biesheuvel wrote:
>> Fix a typo in the 32-bit ARM version of the GICv3 driver, which uses
>> the wrong system register encoding to access ICC_IAR1, and attempted
>> to access ICC_IAR0 instead. This results in boot time hangs both
>> under QEMU emulation and on real hardware.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>
> I would say given how long we've gone without finding this,
Right, that makes me curious -- what has changed now? What exposed this bug?
Thanks!
Laszlo
> it's not
> justifiable to push this before the stable tag is made - so please
> hold off on pushing it until we open the flood gates.
>
> /
> Leif
>
>> ---
>> ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S | 2 +-
>> ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.asm | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S b/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S
>> index a72f3c865163..c308d2fa3e2f 100644
>> --- a/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S
>> +++ b/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S
>> @@ -66,7 +66,7 @@ ASM_FUNC(ArmGicV3EndOfInterrupt)
>> // VOID
>> // );
>> ASM_FUNC(ArmGicV3AcknowledgeInterrupt)
>> - mrc p15, 0, r0, c12, c8, 0 //ICC_IAR1
>> + mrc p15, 0, r0, c12, c12, 0 //ICC_IAR1
>> bx lr
>>
>> //VOID
>> diff --git a/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.asm b/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.asm
>> index 4228fb59be54..222047d1ad43 100644
>> --- a/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.asm
>> +++ b/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.asm
>> @@ -66,7 +66,7 @@
>> // VOID
>> // );
>> RVCT_ASM_EXPORT ArmGicV3AcknowledgeInterrupt
>> - mrc p15, 0, r0, c12, c8, 0 //ICC_IAR1
>> + mrc p15, 0, r0, c12, c12, 0 //ICC_IAR1
>> bx lr
>>
>> //VOID
>> --
>> 2.17.1
>>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge
2018-11-14 22:11 ` Laszlo Ersek
@ 2018-11-15 12:37 ` Ard Biesheuvel
2018-11-15 12:44 ` Laszlo Ersek
0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2018-11-15 12:37 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: Leif Lindholm, edk2-devel@lists.01.org
On Wed, 14 Nov 2018 at 14:11, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 11/14/18 21:00, Leif Lindholm wrote:
> > On Wed, Nov 14, 2018 at 11:27:24AM -0800, Ard Biesheuvel wrote:
> >> Fix a typo in the 32-bit ARM version of the GICv3 driver, which uses
> >> the wrong system register encoding to access ICC_IAR1, and attempted
> >> to access ICC_IAR0 instead. This results in boot time hangs both
> >> under QEMU emulation and on real hardware.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >
> > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> >
> > I would say given how long we've gone without finding this,
>
> Right, that makes me curious -- what has changed now? What exposed this bug?
>
I was regression testing a EFI workaround I put in the kernel for
GICv3, which was apparently the first time anyone tried running
EDK2/ARM on a GICv3 system (which is one of the reasons I wanted to
get you one of the Socionext SynQuacer boards: it has a GICv3 with
GICv2 compatibility and support for 32-bit guests, but sadly, we still
don't have any with fixed silicon)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge
2018-11-15 12:37 ` Ard Biesheuvel
@ 2018-11-15 12:44 ` Laszlo Ersek
2018-11-15 13:09 ` Ard Biesheuvel
0 siblings, 1 reply; 6+ messages in thread
From: Laszlo Ersek @ 2018-11-15 12:44 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Leif Lindholm, edk2-devel@lists.01.org
On 11/15/18 13:37, Ard Biesheuvel wrote:
> On Wed, 14 Nov 2018 at 14:11, Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> On 11/14/18 21:00, Leif Lindholm wrote:
>>> On Wed, Nov 14, 2018 at 11:27:24AM -0800, Ard Biesheuvel wrote:
>>>> Fix a typo in the 32-bit ARM version of the GICv3 driver, which uses
>>>> the wrong system register encoding to access ICC_IAR1, and attempted
>>>> to access ICC_IAR0 instead. This results in boot time hangs both
>>>> under QEMU emulation and on real hardware.
>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>
>>> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>>>
>>> I would say given how long we've gone without finding this,
>>
>> Right, that makes me curious -- what has changed now? What exposed this bug?
>>
>
> I was regression testing a EFI workaround I put in the kernel for
> GICv3, which was apparently the first time anyone tried running
> EDK2/ARM on a GICv3 system (which is one of the reasons I wanted to
> get you one of the Socionext SynQuacer boards: it has a GICv3 with
> GICv2 compatibility and support for 32-bit guests, but sadly, we still
> don't have any with fixed silicon)
>
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge
2018-11-15 12:44 ` Laszlo Ersek
@ 2018-11-15 13:09 ` Ard Biesheuvel
0 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2018-11-15 13:09 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: Leif Lindholm, edk2-devel@lists.01.org
On Thu, 15 Nov 2018 at 04:44, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 11/15/18 13:37, Ard Biesheuvel wrote:
> > On Wed, 14 Nov 2018 at 14:11, Laszlo Ersek <lersek@redhat.com> wrote:
> >>
> >> On 11/14/18 21:00, Leif Lindholm wrote:
> >>> On Wed, Nov 14, 2018 at 11:27:24AM -0800, Ard Biesheuvel wrote:
> >>>> Fix a typo in the 32-bit ARM version of the GICv3 driver, which uses
> >>>> the wrong system register encoding to access ICC_IAR1, and attempted
> >>>> to access ICC_IAR0 instead. This results in boot time hangs both
> >>>> under QEMU emulation and on real hardware.
> >>>>
> >>>> Contributed-under: TianoCore Contribution Agreement 1.1
> >>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >>>
> >>> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> >>>
> >>> I would say given how long we've gone without finding this,
> >>
> >> Right, that makes me curious -- what has changed now? What exposed this bug?
> >>
> >
> > I was regression testing a EFI workaround I put in the kernel for
> > GICv3, which was apparently the first time anyone tried running
> > EDK2/ARM on a GICv3 system (which is one of the reasons I wanted to
> > get you one of the Socionext SynQuacer boards: it has a GICv3 with
> > GICv2 compatibility and support for 32-bit guests, but sadly, we still
> > don't have any with fixed silicon)
> >
>
> Thanks!
> Laszlo
Pushed as 66127011a544b90e800eb3619e84c2f94a354903
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-11-15 13:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-14 19:27 [PATCH] ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge Ard Biesheuvel
2018-11-14 20:00 ` Leif Lindholm
2018-11-14 22:11 ` Laszlo Ersek
2018-11-15 12:37 ` Ard Biesheuvel
2018-11-15 12:44 ` Laszlo Ersek
2018-11-15 13:09 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox