| Message ID | 20181029045708.6292-2-ming.huang@linaro.org |
|---|---|
| State | Accepted |
| Commit | b66e38b50134728614bbca2a2449a36a5dc2bd91 |
| Headers | show |
| Series | Fix Gic interrupt routing modes bug | expand |
(+ Marc) On 29 October 2018 at 05:57, Ming Huang <ming.huang@linaro.org> wrote: > As GicV3 Spec, Interrupt Routing Modes should be 0 for > routing the SPIs to the primary CPU. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ming Huang <ming.huang@linaro.org> > --- > ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c > index 01154848f443..1558db31713a 100644 > --- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c > +++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c > @@ -469,7 +469,7 @@ GicV3DxeInitialize ( > for (Index = 0; Index < (mGicNumInterrupts - 32); Index++) { > MmioWrite32 ( > mGicDistributorBase + ARM_GICD_IROUTER + (Index * 8), > - CpuTarget | ARM_GICD_IROUTER_IRM > + CpuTarget > ); > } > } > -- > 2.18.0 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 7 November 2018 at 16:07, Marc Zyngier <marc.zyngier@arm.com> wrote: > On 07/11/18 14:48, Ard Biesheuvel wrote: >> (+ Marc) >> >> >> On 29 October 2018 at 05:57, Ming Huang <ming.huang@linaro.org> wrote: >>> As GicV3 Spec, Interrupt Routing Modes should be 0 for >>> routing the SPIs to the primary CPU. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.1 >>> Signed-off-by: Ming Huang <ming.huang@linaro.org> >>> --- >>> ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c >>> index 01154848f443..1558db31713a 100644 >>> --- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c >>> +++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c >>> @@ -469,7 +469,7 @@ GicV3DxeInitialize ( >>> for (Index = 0; Index < (mGicNumInterrupts - 32); Index++) { >>> MmioWrite32 ( >>> mGicDistributorBase + ARM_GICD_IROUTER + (Index * 8), >>> - CpuTarget | ARM_GICD_IROUTER_IRM >>> + CpuTarget > > That's a very odd thing indeed. IRM being set means "broadcast to all > CPUs, ignoring the affinity", and there are a number of problems with that: > > - There is no guarantee that it is actually implemented > - Setting this bit *and* the affinity is like saying "yes" and "no" at > the same time > > So as far as I can see, this patch is correct, assuming that CpuTarget > is set to something sensible. What is a bit odd is the commit message, > which doesn't really explain the problem. How about something like: > > <commit> > Setting the GICD_IROUTERn.IMR and GICD_IROUTERn.{Aff3, Aff2, Aff1, Aff0} > at the same time is nonsensical (see 8.9.13 in the GICv3 spec, which > says of GICD_IROUTERn.IMR that "When this bit is set to 1, > GICD_IROUTER<n>.{Aff3, Aff2, Aff1, Aff0} are UNKNOWN"). There is also no > guarantee that IMR is implemented (see GICD_TYPER.No1N which indicates > whether the implementation supports this or not). > > Let's thus not set this bit, as we want all SPIs to be delivered to the > same CPU, and not be broadcast to all of them. > </commit> > > Otherwise: > Acked-by: Marc Zyngier <marc.zyngier@arm.com> > Thanks Marc Pushed as 0adc6eae9480..b66e38b50134, with the commit log updated as per your suggestion _______________________________________________ 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 01154848f443..1558db31713a 100644 --- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c +++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c @@ -469,7 +469,7 @@ GicV3DxeInitialize ( for (Index = 0; Index < (mGicNumInterrupts - 32); Index++) { MmioWrite32 ( mGicDistributorBase + ARM_GICD_IROUTER + (Index * 8), - CpuTarget | ARM_GICD_IROUTER_IRM + CpuTarget ); } }
As GicV3 Spec, Interrupt Routing Modes should be 0 for routing the SPIs to the primary CPU. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ming Huang <ming.huang@linaro.org> --- ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.18.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel