Update the DTS and ACPI descriptions with correct information about the routing of legacy interrupts.
For DT, this comes down to updating the interrupt-map with distinct sets of 4 GIC interrupt lines per PCIe slot.
For ACPI, we need to update the PNP0A08 node and add three companion devices, one for each slot, whose _PRT methods describe the legacy interrupt routing of each respective slot. The _PRT method at the root of the PCI ACPI hierarchy is updated to map INTA (which is shared by all functions of the bridge device) to GIC interrupt #320. With this change, the boot log is free of warnings and non-MSI capable devices work as expected.
Tested on Cello with xhci_hcd.quirks=64 passed on the kernel command line, in which case the xhci interrupt is routed to GIC interrupt #324
Ard Biesheuvel (2): Platforms/AMD: correct legacy PCI interrupt routing in DSDT Platforms/AMD: correct legacy PCI interrupt routing in DTS
Platforms/AMD/Styx/AcpiTables/Dsdt.asl | 63 +++++++++++--------- Platforms/AMD/Styx/OverdriveBoard/FdtBlob/styx-overdrive.dtb | Bin 7973 -> 8123 bytes Platforms/AMD/Styx/OverdriveBoard/FdtBlob/styx-overdrive.dts | 20 +++++-- 3 files changed, 49 insertions(+), 34 deletions(-)
The _PRT method in the PCI0 object describes something that resembles the legacy interrupt routing of the first slot only, but applies it to all PCI-PCI bridges, which means the wrong interrupt is reported for devices in slots 2 and 3. Since most devices support MSI, this is not actually a big deal, but it would be nice to fix this nonetheless.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org --- Platforms/AMD/Styx/AcpiTables/Dsdt.asl | 63 +++++++++++--------- 1 file changed, 34 insertions(+), 29 deletions(-)
diff --git a/Platforms/AMD/Styx/AcpiTables/Dsdt.asl b/Platforms/AMD/Styx/AcpiTables/Dsdt.asl index 3bfa26acea07..4741bb487cc7 100644 --- a/Platforms/AMD/Styx/AcpiTables/Dsdt.asl +++ b/Platforms/AMD/Styx/AcpiTables/Dsdt.asl @@ -508,40 +508,45 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "AMDINC", "SEATTLE ", 3) Name (_SEG, 0x00) // _SEG: PCI Segment Name (_BBN, 0x00) // _BBN: BIOS Bus Number Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute - Name (_PRT, Package (0x04) // _PRT: PCI Routing Table + Name (_PRT, Package () // _PRT: PCI Routing Table { - Package (0x04) - { - 0xFFFF, - 0x00, - 0x00, - 0x0140 - }, + // INTA of the bridge device itself + Package () { 0x2FFFF, 0x0, 0x0, 0x140 } + })
- Package (0x04) + Device (EXP1) + { + Name (_ADR, 0x20001) // _ADR: Address + Name (_PRT, Package () // _PRT: PCI Routing Table { - 0xFFFF, - 0x01, - 0x00, - 0x0141 - }, - - Package (0x04) + Package () { 0xFFFF, 0x0, 0x0, 0x140 }, + Package () { 0xFFFF, 0x1, 0x0, 0x141 }, + Package () { 0xFFFF, 0x2, 0x0, 0x142 }, + Package () { 0xFFFF, 0x3, 0x0, 0x143 } + }) // _PRT + } + Device (EXP2) + { + Name (_ADR, 0x20002) // _ADR: Address + Name (_PRT, Package () // _PRT: PCI Routing Table { - 0xFFFF, - 0x02, - 0x00, - 0x0142 - }, - - Package (0x04) + Package () { 0xFFFF, 0x0, 0x0, 0x144 }, + Package () { 0xFFFF, 0x1, 0x0, 0x145 }, + Package () { 0xFFFF, 0x2, 0x0, 0x146 }, + Package () { 0xFFFF, 0x3, 0x0, 0x147 } + }) // _PRT + } + Device (EXP3) + { + Name (_ADR, 0x20003) // _ADR: Address + Name (_PRT, Package () // _PRT: PCI Routing Table { - 0xFFFF, - 0x03, - 0x00, - 0x0143 - } - }) // _PRT + Package () { 0xFFFF, 0x0, 0x0, 0x148 }, + Package () { 0xFFFF, 0x1, 0x0, 0x149 }, + Package () { 0xFFFF, 0x2, 0x0, 0x14A }, + Package () { 0xFFFF, 0x3, 0x0, 0x14B } + }) // _PRT + }
Method (_CRS, 0, Serialized) // _CRS: Current Resource Settings {
On Sat, Apr 08, 2017 at 06:14:52PM +0100, Ard Biesheuvel wrote:
The _PRT method in the PCI0 object describes something that resembles the legacy interrupt routing of the first slot only, but applies it to all PCI-PCI bridges, which means the wrong interrupt is reported for devices in slots 2 and 3. Since most devices support MSI, this is not actually a big deal, but it would be nice to fix this nonetheless.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org
Ok, fully caught up on discussion now, this looks correct as per termination of discussion until we get any form of definitive answer from AMD or other ACPI PCI expert.
Reviewed-by: Graeme Gregory graeme.gregory@linaro.org
Platforms/AMD/Styx/AcpiTables/Dsdt.asl | 63 +++++++++++--------- 1 file changed, 34 insertions(+), 29 deletions(-)
diff --git a/Platforms/AMD/Styx/AcpiTables/Dsdt.asl b/Platforms/AMD/Styx/AcpiTables/Dsdt.asl index 3bfa26acea07..4741bb487cc7 100644 --- a/Platforms/AMD/Styx/AcpiTables/Dsdt.asl +++ b/Platforms/AMD/Styx/AcpiTables/Dsdt.asl @@ -508,40 +508,45 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "AMDINC", "SEATTLE ", 3) Name (_SEG, 0x00) // _SEG: PCI Segment Name (_BBN, 0x00) // _BBN: BIOS Bus Number Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
Name (_PRT, Package (0x04) // _PRT: PCI Routing Table
Name (_PRT, Package () // _PRT: PCI Routing Table {
Package (0x04){0xFFFF,0x00,0x00,0x0140},
// INTA of the bridge device itselfPackage () { 0x2FFFF, 0x0, 0x0, 0x140 }})
Package (0x04)
Device (EXP1){Name (_ADR, 0x20001) // _ADR: AddressName (_PRT, Package () // _PRT: PCI Routing Table {
0xFFFF,0x01,0x00,0x0141},Package (0x04)
Package () { 0xFFFF, 0x0, 0x0, 0x140 },Package () { 0xFFFF, 0x1, 0x0, 0x141 },Package () { 0xFFFF, 0x2, 0x0, 0x142 },Package () { 0xFFFF, 0x3, 0x0, 0x143 }}) // _PRT}Device (EXP2){Name (_ADR, 0x20002) // _ADR: AddressName (_PRT, Package () // _PRT: PCI Routing Table {
0xFFFF,0x02,0x00,0x0142},Package (0x04)
Package () { 0xFFFF, 0x0, 0x0, 0x144 },Package () { 0xFFFF, 0x1, 0x0, 0x145 },Package () { 0xFFFF, 0x2, 0x0, 0x146 },Package () { 0xFFFF, 0x3, 0x0, 0x147 }}) // _PRT}Device (EXP3){Name (_ADR, 0x20003) // _ADR: AddressName (_PRT, Package () // _PRT: PCI Routing Table {
0xFFFF,0x03,0x00,0x0143}}) // _PRT
Package () { 0xFFFF, 0x0, 0x0, 0x148 },Package () { 0xFFFF, 0x1, 0x0, 0x149 },Package () { 0xFFFF, 0x2, 0x0, 0x14A },Package () { 0xFFFF, 0x3, 0x0, 0x14B }}) // _PRT}Method (_CRS, 0, Serialized) // _CRS: Current Resource Settings { -- 2.9.3
The interrupt map in the PCIe node describes something that resembles the legacy interrupt routing of the first slot only, but applies it to all PCI-PCI bridges, which means the wrong interrupt is reported for devices in slots 2 and 3. Since most devices support MSI, this is not actually a big deal, but it would be nice to fix this nonetheless.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org --- Platforms/AMD/Styx/OverdriveBoard/FdtBlob/styx-overdrive.dtb | Bin 7973 -> 8123 bytes Platforms/AMD/Styx/OverdriveBoard/FdtBlob/styx-overdrive.dts | 20 +++++++++++++++----- 2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/Platforms/AMD/Styx/OverdriveBoard/FdtBlob/styx-overdrive.dtb b/Platforms/AMD/Styx/OverdriveBoard/FdtBlob/styx-overdrive.dtb index ba4de560d279d79fde086c6e3f79626bd17a108f..8b538d72a243244cbf0895cc7094a605a1403338 100644 GIT binary patch delta 1674 zcmcIkT}V_x6rQ>3y1#eb|DSfn4Xs^M(GaOnawE_nLZ3qFAwoj7G#3&Kd{9u(mq0v) zZ~ZF0wdxi`8ZHr`4^mG-gkX?KuwhVw522ki_g-7`2EH_KX68HRJAZTU%;d@0#`rsV z>l<Swvy8Fhj71i3jib0woG3V?AE+Olj<0hVm$G~q+6-q8jwv=~z_kG7o^j(7sR$RL zq~W!pc`wU<!Io874#BD{&|!H#R3%iF66<zB$KF`QOIMJTmoZ}0iIvAvG7jmn9A9Pz zcE!pWbu#Al=tg!LdI}%AUg1tseQ{xnP-!aV@1fPyp<`Px_3~GcF!$j+XYTirH=UU# zbneU0YU$-)VZl-=B4{>?u>G=hqpj1bXS;9pOKcOKTdy{;ZGN;09Yka^V;m#f2N;vj zs5<;=Jc@f7l!0)KK!0!o+H57f9Y$;|yce=|FXU_iJ_t2D3?=q5o$4Co2&C*D?7vg4 z7d{m`d@qD$r>;+R9^VHqWxwjPF8?b>!f{MQ_rWcP#-4ZRv41)S<;A=n0fvhDLKCU> zBsEK&)9FkRz|jwJXDs@Hv26t_vk4bu@)eXNyd2Q&jByu?i+NtmYdE`6A2pXDnKR}_ zbJToD1M>(DYhVpQOaUI@6W0Lwlxu)|j%WaViYpW#s>UtT`ikIwI8){_aPi{>A>nT3 zA7R8@%U5B}9YgJBcQu~?ucx_uHJR#4Ub@zPgCgM2KK=&ovF;tb1w)?tz1J@9s`!wd zh80iuK3FO5K3HSEeUM<(cM%)P`M_nx0%%YRcD#l`81)B2beMD_Nz^YVtPPMwYYS@W zRUsjN&G#~V_Xl_%m;)^$dK26Yock{sdV-Xp;lL>{hOXD$QEMo~g45E2hmR&EQKQs_ ze<q_B!ia))Q7gU5%McBR>*&q4s9N%VbF-r>DgJ7Oi8oq$i}s+(!3AHZ&CqF)J^fea x>6y&aXG9TMw9IsVw5L<romo4({bx>v8~F-Mh0jY;4$P79%#%nh&*X|v@ZWp>4_g2L
delta 1741 zcmcIkT}abW6u)<$x-U2PshhdbP^qzHS|cmh2Ps7QW>8Scu(8^lOv%DPBdFKtlm!W? zCG{YgE<q^Fpcfwk1p|A^9{K_!9}=X8?3}y(Yvw=jr32^w&pqdNzV5x}|EzXsU-^vj z=U2vxw=l+njCox6El0K>n~{-}M#kI=qvhXtH$Pt)548jW!2nX)%9QxcN7gha<J%(9 zun#f5Gs)(7OBz6SqHD+`Q`9grV>x2QMI<7d3i)E@EA6{8dj(_QP`u>5Ft0eWc>uLU z66&#f`MtEyLtzpQDeKmw9LM!j884GS4a=ddZ+)Q*DjmzFimZ=S^jdUC6{JypDw&K~ zsB}h^S_WnMIM%Z!V@xfRF{?*9@k#iuerTCR3C~8H1b)~&3~NgG2bj@BWNh`?PCg4$ z+Adu8<aFD~n>vt~?tKwvayt16)aRCn2$C&V*e>NJ(6%UR^XeQ5u7#lP>`rdt+kNOD zBI8YSFJp!t#*FnWt;4(Kew2wQ1MO?!v#!{3Qt19@laQlt<X0hNut7}k<ToI|-EdD| zDpTEag9n!MR_uS#P$PV{{>IzjuE8wp<Ilp|VXx7V@kto}m1D}dOGLMW(Ila3O>*cC z(-q@<R*!?A%Im5~wS}Y9CUs7y^MW==^xCD<m_}Izck;@-bBwK!RU$14gz87nPdGpD zF0YK6L2IVfFV<t`GKavBkCfkoa&KzRqU7R%xL_&2jxR&dx`VGm%vy%pn^r$h!7FQB z#cH%I9zAs~+)fdYGG!^~vGc6G3#x22>&GMON!t-U3D<3j4WPXCji8=v2o-Xiz>fMH zU@654h^Pkx&PyePoX&y+xTDb_mX4IHP0)r`x*zJ5Scfe6c0s?>$#21gvr$Ap3=OWM z|0P3d0cBV$+z;agt<@u$8Z=npv|?cJ{(~XZr0c>b#AuY&$Y>X}(q9!v+3l`=gAxrd zbwJ(=7b5Yfcz$Uej-KKR+UVy`7@~Q2RCrKiANecu=veCMxWEur%1q}+GU?W4md<YD znZ2G`{sT^XniNV6On7da;_XpiIMLdXOvGX>$y7s871xojtvT7!+L5}vX*d4`{O1_>
diff --git a/Platforms/AMD/Styx/OverdriveBoard/FdtBlob/styx-overdrive.dts b/Platforms/AMD/Styx/OverdriveBoard/FdtBlob/styx-overdrive.dts index 36474a26c9c6..b462910b3bf0 100644 --- a/Platforms/AMD/Styx/OverdriveBoard/FdtBlob/styx-overdrive.dts +++ b/Platforms/AMD/Styx/OverdriveBoard/FdtBlob/styx-overdrive.dts @@ -261,11 +261,21 @@ bus-range = <0x0 0x7f>; msi-parent = <0x4>; reg = <0x0 0xf0000000 0x0 0x10000000>; - interrupt-map-mask = <0xf800 0x0 0x0 0x7>; - interrupt-map = <0x1000 0x0 0x0 0x1 0x1 0x0 0x0 0x0 0x120 0x1>, - <0x1000 0x0 0x0 0x2 0x1 0x0 0x0 0x0 0x121 0x1>, - <0x1000 0x0 0x0 0x3 0x1 0x0 0x0 0x0 0x122 0x1>, - <0x1000 0x0 0x0 0x4 0x1 0x0 0x0 0x0 0x123 0x1>; + interrupt-map-mask = <0xff00 0x0 0x0 0x7>; + interrupt-map = <0x1100 0x0 0x0 0x1 0x1 0x0 0x0 0x0 0x120 0x1>, + <0x1100 0x0 0x0 0x2 0x1 0x0 0x0 0x0 0x121 0x1>, + <0x1100 0x0 0x0 0x3 0x1 0x0 0x0 0x0 0x122 0x1>, + <0x1100 0x0 0x0 0x4 0x1 0x0 0x0 0x0 0x123 0x1>, + + <0x1200 0x0 0x0 0x1 0x1 0x0 0x0 0x0 0x124 0x1>, + <0x1200 0x0 0x0 0x2 0x1 0x0 0x0 0x0 0x125 0x1>, + <0x1200 0x0 0x0 0x3 0x1 0x0 0x0 0x0 0x126 0x1>, + <0x1200 0x0 0x0 0x4 0x1 0x0 0x0 0x0 0x127 0x1>, + + <0x1300 0x0 0x0 0x1 0x1 0x0 0x0 0x0 0x128 0x1>, + <0x1300 0x0 0x0 0x2 0x1 0x0 0x0 0x0 0x129 0x1>, + <0x1300 0x0 0x0 0x3 0x1 0x0 0x0 0x0 0x12a 0x1>, + <0x1300 0x0 0x0 0x4 0x1 0x0 0x0 0x0 0x12b 0x1>; dma-coherent; dma-ranges = <0x43000000 0x0 0x0 0x0 0x0 0x100 0x0>; ranges = <0x1000000 0x0 0x00000000 0x0 0xefff0000 0x00 0x00010000>, /* I/O Memory (size=64K) */
On Sat, Apr 08, 2017 at 06:14:51PM +0100, Ard Biesheuvel wrote:
Update the DTS and ACPI descriptions with correct information about the routing of legacy interrupts.
For DT, this comes down to updating the interrupt-map with distinct sets of 4 GIC interrupt lines per PCIe slot.
For ACPI, we need to update the PNP0A08 node and add three companion devices, one for each slot, whose _PRT methods describe the legacy interrupt routing of each respective slot. The _PRT method at the root of the PCI ACPI hierarchy is updated to map INTA (which is shared by all functions of the bridge device) to GIC interrupt #320. With this change, the boot log is free of warnings and non-MSI capable devices work as expected.
Tested on Cello with xhci_hcd.quirks=64 passed on the kernel command line, in which case the xhci interrupt is routed to GIC interrupt #324
Even if this is not proven to be fully resolved, it certainly seems like an improvement - so: Reviewed-by: Leif Lindholm leif.lindholm@linaro.org
Ard Biesheuvel (2): Platforms/AMD: correct legacy PCI interrupt routing in DSDT Platforms/AMD: correct legacy PCI interrupt routing in DTS
Platforms/AMD/Styx/AcpiTables/Dsdt.asl | 63 +++++++++++--------- Platforms/AMD/Styx/OverdriveBoard/FdtBlob/styx-overdrive.dtb | Bin 7973 -> 8123 bytes Platforms/AMD/Styx/OverdriveBoard/FdtBlob/styx-overdrive.dts | 20 +++++-- 3 files changed, 49 insertions(+), 34 deletions(-)
-- 2.9.3
On 11 April 2017 at 19:40, Leif Lindholm leif.lindholm@linaro.org wrote:
On Sat, Apr 08, 2017 at 06:14:51PM +0100, Ard Biesheuvel wrote:
Update the DTS and ACPI descriptions with correct information about the routing of legacy interrupts.
For DT, this comes down to updating the interrupt-map with distinct sets of 4 GIC interrupt lines per PCIe slot.
For ACPI, we need to update the PNP0A08 node and add three companion devices, one for each slot, whose _PRT methods describe the legacy interrupt routing of each respective slot. The _PRT method at the root of the PCI ACPI hierarchy is updated to map INTA (which is shared by all functions of the bridge device) to GIC interrupt #320. With this change, the boot log is free of warnings and non-MSI capable devices work as expected.
Tested on Cello with xhci_hcd.quirks=64 passed on the kernel command line, in which case the xhci interrupt is routed to GIC interrupt #324
Even if this is not proven to be fully resolved, it certainly seems like an improvement - so: Reviewed-by: Leif Lindholm leif.lindholm@linaro.org
Since Alan confirmed that legacy interrupts work for him on slots other than the first one, both on DT and ACPI, I have pushed these patches.
-
Ard Biesheuvel -
graeme.gregory＠linaro.org -
Leif Lindholm