| Message ID | 1466605935-19217-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | c7fefb690661f2e38afcb8200bd318ecf38ab961 |
| Headers | show |
On 06/22/16 16:32, Ard Biesheuvel wrote: > Reassign all interrupts to non-secure Group-1 if the GIC has its DS > (Disable Security) bit set. In this case, it is safe to assume that we > own the GIC, and that no other firmware has performed any configuration > yet, which means it is up to us to reconfigure the interrupts so they > can be taken by the non-secure firmware. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > > This is the EDK2 counterpart of Peter's patch against the Linux kernel > > 7c9b973061b0 ("irqchip/gic-v3: Configure all interrupts as non-secure Group-1") > > which tweaks the GICv3 driver so that it works with the GICv3 emulation in > QEMU, which only emulates a single GIC security state when running without > the security extensions. > > ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 16 ++++++++++++++++ > ArmPkg/Include/Library/ArmGicLib.h | 5 +++-- > 2 files changed, 19 insertions(+), 2 deletions(-) > > diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c > index 50fa56262eaf..106c669fcb87 100644 > --- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c > +++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c > @@ -297,6 +297,22 @@ GicV3DxeInitialize ( > MpId = ArmReadMpidr (); > CpuTarget = MpId & (ARM_CORE_AFF0 | ARM_CORE_AFF1 | ARM_CORE_AFF2 | ARM_CORE_AFF3); > > + if ((MmioRead32 (mGicDistributorBase + ARM_GIC_ICDDCR) & ARM_GIC_ICDDCR_DS) != 0) { > + // > + // If the Disable Security (DS) control bit is set, we are dealing with a > + // GIC that has only one security state. In this case, let's assume we are > + // executing in non-secure state (which is appropriate for DXE modules) > + // and that no other firmware has performed any configuration on the GIC. > + // This means we need to reconfigure all interrupts to non-secure Group 1 > + // first. > + // > + MmioWrite32 (mGicRedistributorsBase + ARM_GICR_CTLR_FRAME_SIZE + ARM_GIC_ICDISR, 0xffffffff); > + > + for (Index = 32; Index < mGicNumInterrupts; Index += 32) { > + MmioWrite32 (mGicDistributorBase + ARM_GIC_ICDISR + Index / 8, 0xffffffff); > + } > + } > + > // Route the SPIs to the primary CPU. SPIs start at the INTID 32 > for (Index = 0; Index < (mGicNumInterrupts - 32); Index++) { > MmioWrite32 (mGicDistributorBase + ARM_GICD_IROUTER + (Index * 8), CpuTarget | ARM_GICD_IROUTER_IRM); > diff --git a/ArmPkg/Include/Library/ArmGicLib.h b/ArmPkg/Include/Library/ArmGicLib.h > index 10c4a9d72eb2..4364f3ffef46 100644 > --- a/ArmPkg/Include/Library/ArmGicLib.h > +++ b/ArmPkg/Include/Library/ArmGicLib.h > @@ -47,8 +47,9 @@ > // GICv3 specific registers > #define ARM_GICD_IROUTER 0x6100 // Interrupt Routing Registers > > -// the Affinity Routing Enable (ARE) bit in GICD_CTLR > -#define ARM_GIC_ICDDCR_ARE (1 << 4) > +// GICD_CTLR bits > +#define ARM_GIC_ICDDCR_ARE (1 << 4) // Affinity Routing Enable (ARE) > +#define ARM_GIC_ICDDCR_DS (1 << 6) // Disable Security (DS) > > // > // GIC Redistributor > Drew tested this patch; the result is positive: http://thread.gmane.org/gmane.comp.emulators.qemu/418780/focus=421731 Based on that, I'm synthetizing: Tested-by: Drew Jones <drjones@redhat.com> Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 23 June 2016 at 15:25, Laszlo Ersek <lersek@redhat.com> wrote: > On 06/22/16 16:32, Ard Biesheuvel wrote: >> Reassign all interrupts to non-secure Group-1 if the GIC has its DS >> (Disable Security) bit set. In this case, it is safe to assume that we >> own the GIC, and that no other firmware has performed any configuration >> yet, which means it is up to us to reconfigure the interrupts so they >> can be taken by the non-secure firmware. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> >> This is the EDK2 counterpart of Peter's patch against the Linux kernel >> >> 7c9b973061b0 ("irqchip/gic-v3: Configure all interrupts as non-secure Group-1") >> >> which tweaks the GICv3 driver so that it works with the GICv3 emulation in >> QEMU, which only emulates a single GIC security state when running without >> the security extensions. >> >> ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 16 ++++++++++++++++ >> ArmPkg/Include/Library/ArmGicLib.h | 5 +++-- >> 2 files changed, 19 insertions(+), 2 deletions(-) >> >> diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c >> index 50fa56262eaf..106c669fcb87 100644 >> --- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c >> +++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c >> @@ -297,6 +297,22 @@ GicV3DxeInitialize ( >> MpId = ArmReadMpidr (); >> CpuTarget = MpId & (ARM_CORE_AFF0 | ARM_CORE_AFF1 | ARM_CORE_AFF2 | ARM_CORE_AFF3); >> >> + if ((MmioRead32 (mGicDistributorBase + ARM_GIC_ICDDCR) & ARM_GIC_ICDDCR_DS) != 0) { >> + // >> + // If the Disable Security (DS) control bit is set, we are dealing with a >> + // GIC that has only one security state. In this case, let's assume we are >> + // executing in non-secure state (which is appropriate for DXE modules) >> + // and that no other firmware has performed any configuration on the GIC. >> + // This means we need to reconfigure all interrupts to non-secure Group 1 >> + // first. >> + // >> + MmioWrite32 (mGicRedistributorsBase + ARM_GICR_CTLR_FRAME_SIZE + ARM_GIC_ICDISR, 0xffffffff); >> + >> + for (Index = 32; Index < mGicNumInterrupts; Index += 32) { >> + MmioWrite32 (mGicDistributorBase + ARM_GIC_ICDISR + Index / 8, 0xffffffff); >> + } >> + } >> + >> // Route the SPIs to the primary CPU. SPIs start at the INTID 32 >> for (Index = 0; Index < (mGicNumInterrupts - 32); Index++) { >> MmioWrite32 (mGicDistributorBase + ARM_GICD_IROUTER + (Index * 8), CpuTarget | ARM_GICD_IROUTER_IRM); >> diff --git a/ArmPkg/Include/Library/ArmGicLib.h b/ArmPkg/Include/Library/ArmGicLib.h >> index 10c4a9d72eb2..4364f3ffef46 100644 >> --- a/ArmPkg/Include/Library/ArmGicLib.h >> +++ b/ArmPkg/Include/Library/ArmGicLib.h >> @@ -47,8 +47,9 @@ >> // GICv3 specific registers >> #define ARM_GICD_IROUTER 0x6100 // Interrupt Routing Registers >> >> -// the Affinity Routing Enable (ARE) bit in GICD_CTLR >> -#define ARM_GIC_ICDDCR_ARE (1 << 4) >> +// GICD_CTLR bits >> +#define ARM_GIC_ICDDCR_ARE (1 << 4) // Affinity Routing Enable (ARE) >> +#define ARM_GIC_ICDDCR_DS (1 << 6) // Disable Security (DS) >> >> // >> // GIC Redistributor >> > > Drew tested this patch; the result is positive: > > http://thread.gmane.org/gmane.comp.emulators.qemu/418780/focus=421731 > > Based on that, I'm synthetizing: > > Tested-by: Drew Jones <drjones@redhat.com> > Thanks! @Leif: any concerns? _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Thu, Jun 23, 2016 at 03:26:53PM +0200, Ard Biesheuvel wrote: > On 23 June 2016 at 15:25, Laszlo Ersek <lersek@redhat.com> wrote: > > On 06/22/16 16:32, Ard Biesheuvel wrote: > >> Reassign all interrupts to non-secure Group-1 if the GIC has its DS > >> (Disable Security) bit set. In this case, it is safe to assume that we > >> own the GIC, and that no other firmware has performed any configuration > >> yet, which means it is up to us to reconfigure the interrupts so they > >> can be taken by the non-secure firmware. > >> > >> Contributed-under: TianoCore Contribution Agreement 1.0 > >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >> --- > >> > >> This is the EDK2 counterpart of Peter's patch against the Linux kernel > >> > >> 7c9b973061b0 ("irqchip/gic-v3: Configure all interrupts as non-secure Group-1") > >> > >> which tweaks the GICv3 driver so that it works with the GICv3 emulation in > >> QEMU, which only emulates a single GIC security state when running without > >> the security extensions. > >> > >> ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 16 ++++++++++++++++ > >> ArmPkg/Include/Library/ArmGicLib.h | 5 +++-- > >> 2 files changed, 19 insertions(+), 2 deletions(-) > >> > >> diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c > >> index 50fa56262eaf..106c669fcb87 100644 > >> --- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c > >> +++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c > >> @@ -297,6 +297,22 @@ GicV3DxeInitialize ( > >> MpId = ArmReadMpidr (); > >> CpuTarget = MpId & (ARM_CORE_AFF0 | ARM_CORE_AFF1 | ARM_CORE_AFF2 | ARM_CORE_AFF3); > >> > >> + if ((MmioRead32 (mGicDistributorBase + ARM_GIC_ICDDCR) & ARM_GIC_ICDDCR_DS) != 0) { > >> + // > >> + // If the Disable Security (DS) control bit is set, we are dealing with a > >> + // GIC that has only one security state. In this case, let's assume we are > >> + // executing in non-secure state (which is appropriate for DXE modules) > >> + // and that no other firmware has performed any configuration on the GIC. > >> + // This means we need to reconfigure all interrupts to non-secure Group 1 > >> + // first. > >> + // > >> + MmioWrite32 (mGicRedistributorsBase + ARM_GICR_CTLR_FRAME_SIZE + ARM_GIC_ICDISR, 0xffffffff); > >> + > >> + for (Index = 32; Index < mGicNumInterrupts; Index += 32) { > >> + MmioWrite32 (mGicDistributorBase + ARM_GIC_ICDISR + Index / 8, 0xffffffff); > >> + } > >> + } > >> + > >> // Route the SPIs to the primary CPU. SPIs start at the INTID 32 > >> for (Index = 0; Index < (mGicNumInterrupts - 32); Index++) { > >> MmioWrite32 (mGicDistributorBase + ARM_GICD_IROUTER + (Index * 8), CpuTarget | ARM_GICD_IROUTER_IRM); > >> diff --git a/ArmPkg/Include/Library/ArmGicLib.h b/ArmPkg/Include/Library/ArmGicLib.h > >> index 10c4a9d72eb2..4364f3ffef46 100644 > >> --- a/ArmPkg/Include/Library/ArmGicLib.h > >> +++ b/ArmPkg/Include/Library/ArmGicLib.h > >> @@ -47,8 +47,9 @@ > >> // GICv3 specific registers > >> #define ARM_GICD_IROUTER 0x6100 // Interrupt Routing Registers > >> > >> -// the Affinity Routing Enable (ARE) bit in GICD_CTLR > >> -#define ARM_GIC_ICDDCR_ARE (1 << 4) > >> +// GICD_CTLR bits > >> +#define ARM_GIC_ICDDCR_ARE (1 << 4) // Affinity Routing Enable (ARE) > >> +#define ARM_GIC_ICDDCR_DS (1 << 6) // Disable Security (DS) > >> > >> // > >> // GIC Redistributor > >> > > > > Drew tested this patch; the result is positive: > > > > http://thread.gmane.org/gmane.comp.emulators.qemu/418780/focus=421731 > > > > Based on that, I'm synthetizing: > > > > Tested-by: Drew Jones <drjones@redhat.com> > > > > Thanks! > > @Leif: any concerns? Nope. Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 23 June 2016 at 15:51, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Thu, Jun 23, 2016 at 03:26:53PM +0200, Ard Biesheuvel wrote: >> On 23 June 2016 at 15:25, Laszlo Ersek <lersek@redhat.com> wrote: >> > On 06/22/16 16:32, Ard Biesheuvel wrote: >> >> Reassign all interrupts to non-secure Group-1 if the GIC has its DS >> >> (Disable Security) bit set. In this case, it is safe to assume that we >> >> own the GIC, and that no other firmware has performed any configuration >> >> yet, which means it is up to us to reconfigure the interrupts so they >> >> can be taken by the non-secure firmware. >> >> >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >> --- >> >> >> >> This is the EDK2 counterpart of Peter's patch against the Linux kernel >> >> >> >> 7c9b973061b0 ("irqchip/gic-v3: Configure all interrupts as non-secure Group-1") >> >> >> >> which tweaks the GICv3 driver so that it works with the GICv3 emulation in >> >> QEMU, which only emulates a single GIC security state when running without >> >> the security extensions. >> >> >> >> ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 16 ++++++++++++++++ >> >> ArmPkg/Include/Library/ArmGicLib.h | 5 +++-- >> >> 2 files changed, 19 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c >> >> index 50fa56262eaf..106c669fcb87 100644 >> >> --- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c >> >> +++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c >> >> @@ -297,6 +297,22 @@ GicV3DxeInitialize ( >> >> MpId = ArmReadMpidr (); >> >> CpuTarget = MpId & (ARM_CORE_AFF0 | ARM_CORE_AFF1 | ARM_CORE_AFF2 | ARM_CORE_AFF3); >> >> >> >> + if ((MmioRead32 (mGicDistributorBase + ARM_GIC_ICDDCR) & ARM_GIC_ICDDCR_DS) != 0) { >> >> + // >> >> + // If the Disable Security (DS) control bit is set, we are dealing with a >> >> + // GIC that has only one security state. In this case, let's assume we are >> >> + // executing in non-secure state (which is appropriate for DXE modules) >> >> + // and that no other firmware has performed any configuration on the GIC. >> >> + // This means we need to reconfigure all interrupts to non-secure Group 1 >> >> + // first. >> >> + // >> >> + MmioWrite32 (mGicRedistributorsBase + ARM_GICR_CTLR_FRAME_SIZE + ARM_GIC_ICDISR, 0xffffffff); >> >> + >> >> + for (Index = 32; Index < mGicNumInterrupts; Index += 32) { >> >> + MmioWrite32 (mGicDistributorBase + ARM_GIC_ICDISR + Index / 8, 0xffffffff); >> >> + } >> >> + } >> >> + >> >> // Route the SPIs to the primary CPU. SPIs start at the INTID 32 >> >> for (Index = 0; Index < (mGicNumInterrupts - 32); Index++) { >> >> MmioWrite32 (mGicDistributorBase + ARM_GICD_IROUTER + (Index * 8), CpuTarget | ARM_GICD_IROUTER_IRM); >> >> diff --git a/ArmPkg/Include/Library/ArmGicLib.h b/ArmPkg/Include/Library/ArmGicLib.h >> >> index 10c4a9d72eb2..4364f3ffef46 100644 >> >> --- a/ArmPkg/Include/Library/ArmGicLib.h >> >> +++ b/ArmPkg/Include/Library/ArmGicLib.h >> >> @@ -47,8 +47,9 @@ >> >> // GICv3 specific registers >> >> #define ARM_GICD_IROUTER 0x6100 // Interrupt Routing Registers >> >> >> >> -// the Affinity Routing Enable (ARE) bit in GICD_CTLR >> >> -#define ARM_GIC_ICDDCR_ARE (1 << 4) >> >> +// GICD_CTLR bits >> >> +#define ARM_GIC_ICDDCR_ARE (1 << 4) // Affinity Routing Enable (ARE) >> >> +#define ARM_GIC_ICDDCR_DS (1 << 6) // Disable Security (DS) >> >> >> >> // >> >> // GIC Redistributor >> >> >> > >> > Drew tested this patch; the result is positive: >> > >> > http://thread.gmane.org/gmane.comp.emulators.qemu/418780/focus=421731 >> > >> > Based on that, I'm synthetizing: >> > >> > Tested-by: Drew Jones <drjones@redhat.com> >> > >> >> Thanks! >> >> @Leif: any concerns? > > Nope. > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> Thanks all. I committed this as c7fefb690661 (but I forgot to add Drew's T-b, sorry about that) -- Ard. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c index 50fa56262eaf..106c669fcb87 100644 --- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c +++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c @@ -297,6 +297,22 @@ GicV3DxeInitialize ( MpId = ArmReadMpidr (); CpuTarget = MpId & (ARM_CORE_AFF0 | ARM_CORE_AFF1 | ARM_CORE_AFF2 | ARM_CORE_AFF3); + if ((MmioRead32 (mGicDistributorBase + ARM_GIC_ICDDCR) & ARM_GIC_ICDDCR_DS) != 0) { + // + // If the Disable Security (DS) control bit is set, we are dealing with a + // GIC that has only one security state. In this case, let's assume we are + // executing in non-secure state (which is appropriate for DXE modules) + // and that no other firmware has performed any configuration on the GIC. + // This means we need to reconfigure all interrupts to non-secure Group 1 + // first. + // + MmioWrite32 (mGicRedistributorsBase + ARM_GICR_CTLR_FRAME_SIZE + ARM_GIC_ICDISR, 0xffffffff); + + for (Index = 32; Index < mGicNumInterrupts; Index += 32) { + MmioWrite32 (mGicDistributorBase + ARM_GIC_ICDISR + Index / 8, 0xffffffff); + } + } + // Route the SPIs to the primary CPU. SPIs start at the INTID 32 for (Index = 0; Index < (mGicNumInterrupts - 32); Index++) { MmioWrite32 (mGicDistributorBase + ARM_GICD_IROUTER + (Index * 8), CpuTarget | ARM_GICD_IROUTER_IRM); diff --git a/ArmPkg/Include/Library/ArmGicLib.h b/ArmPkg/Include/Library/ArmGicLib.h index 10c4a9d72eb2..4364f3ffef46 100644 --- a/ArmPkg/Include/Library/ArmGicLib.h +++ b/ArmPkg/Include/Library/ArmGicLib.h @@ -47,8 +47,9 @@ // GICv3 specific registers #define ARM_GICD_IROUTER 0x6100 // Interrupt Routing Registers -// the Affinity Routing Enable (ARE) bit in GICD_CTLR -#define ARM_GIC_ICDDCR_ARE (1 << 4) +// GICD_CTLR bits +#define ARM_GIC_ICDDCR_ARE (1 << 4) // Affinity Routing Enable (ARE) +#define ARM_GIC_ICDDCR_DS (1 << 6) // Disable Security (DS) // // GIC Redistributor
Reassign all interrupts to non-secure Group-1 if the GIC has its DS (Disable Security) bit set. In this case, it is safe to assume that we own the GIC, and that no other firmware has performed any configuration yet, which means it is up to us to reconfigure the interrupts so they can be taken by the non-secure firmware. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- This is the EDK2 counterpart of Peter's patch against the Linux kernel 7c9b973061b0 ("irqchip/gic-v3: Configure all interrupts as non-secure Group-1") which tweaks the GICv3 driver so that it works with the GICv3 emulation in QEMU, which only emulates a single GIC security state when running without the security extensions. ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 16 ++++++++++++++++ ArmPkg/Include/Library/ArmGicLib.h | 5 +++-- 2 files changed, 19 insertions(+), 2 deletions(-) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel