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 | 52 +++++++------------- 1 file changed, 19 insertions(+), 33 deletions(-)
diff --git a/Platforms/AMD/Styx/AcpiTables/Dsdt.asl b/Platforms/AMD/Styx/AcpiTables/Dsdt.asl index 3bfa26acea07..49ef55fc7df6 100644 --- a/Platforms/AMD/Styx/AcpiTables/Dsdt.asl +++ b/Platforms/AMD/Styx/AcpiTables/Dsdt.asl @@ -508,39 +508,25 @@ 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 - { - Package (0x04) - { - 0xFFFF, - 0x00, - 0x00, - 0x0140 - }, - - Package (0x04) - { - 0xFFFF, - 0x01, - 0x00, - 0x0141 - }, - - Package (0x04) - { - 0xFFFF, - 0x02, - 0x00, - 0x0142 - }, - - Package (0x04) - { - 0xFFFF, - 0x03, - 0x00, - 0x0143 - } + Name (_PRT, Package () // _PRT: PCI Routing Table + { + // slot 1: dev 2 fn 1 + Package () { 0x20001, 0x0, 0x0, 0x140 }, + Package () { 0x20001, 0x1, 0x0, 0x141 }, + Package () { 0x20001, 0x2, 0x0, 0x142 }, + Package () { 0x20001, 0x3, 0x0, 0x143 }, + + // slot 2: dev 2 fn 2 + Package () { 0x20002, 0x0, 0x0, 0x144 }, + Package () { 0x20002, 0x1, 0x0, 0x145 }, + Package () { 0x20002, 0x2, 0x0, 0x146 }, + Package () { 0x20002, 0x3, 0x0, 0x147 }, + + // slot 3: dev 2 fn 3 + Package () { 0x20003, 0x0, 0x0, 0x148 }, + Package () { 0x20003, 0x1, 0x0, 0x149 }, + Package () { 0x20003, 0x2, 0x0, 0x14a }, + Package () { 0x20003, 0x3, 0x0, 0x14b } }) // _PRT
Method (_CRS, 0, Serialized) // _CRS: Current Resource Settings
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 Fri, Apr 07, 2017 at 02:28:50PM +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
Considering exlanation on linux-acpi thread this looks fine to me.
Reviewed-by: Graeme Gregory graeme.gregory@linaro.org
Platforms/AMD/Styx/AcpiTables/Dsdt.asl | 52 +++++++------------- 1 file changed, 19 insertions(+), 33 deletions(-)
diff --git a/Platforms/AMD/Styx/AcpiTables/Dsdt.asl b/Platforms/AMD/Styx/AcpiTables/Dsdt.asl index 3bfa26acea07..49ef55fc7df6 100644 --- a/Platforms/AMD/Styx/AcpiTables/Dsdt.asl +++ b/Platforms/AMD/Styx/AcpiTables/Dsdt.asl @@ -508,39 +508,25 @@ 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{Package (0x04){0xFFFF,0x00,0x00,0x0140},Package (0x04){0xFFFF,0x01,0x00,0x0141},Package (0x04){0xFFFF,0x02,0x00,0x0142},Package (0x04){0xFFFF,0x03,0x00,0x0143}
Name (_PRT, Package () // _PRT: PCI Routing Table{// slot 1: dev 2 fn 1Package () { 0x20001, 0x0, 0x0, 0x140 },Package () { 0x20001, 0x1, 0x0, 0x141 },Package () { 0x20001, 0x2, 0x0, 0x142 },Package () { 0x20001, 0x3, 0x0, 0x143 },// slot 2: dev 2 fn 2Package () { 0x20002, 0x0, 0x0, 0x144 },Package () { 0x20002, 0x1, 0x0, 0x145 },Package () { 0x20002, 0x2, 0x0, 0x146 },Package () { 0x20002, 0x3, 0x0, 0x147 },// slot 3: dev 2 fn 3Package () { 0x20003, 0x0, 0x0, 0x148 },Package () { 0x20003, 0x1, 0x0, 0x149 },Package () { 0x20003, 0x2, 0x0, 0x14a },Package () { 0x20003, 0x3, 0x0, 0x14b } }) // _PRTMethod (_CRS, 0, Serialized) // _CRS: Current Resource Settings -- 2.9.3
On 10 April 2017 at 11:05, graeme.gregory@linaro.org wrote:
On Fri, Apr 07, 2017 at 02:28:50PM +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
Considering exlanation on linux-acpi thread this looks fine to me.
Reviewed-by: Graeme Gregory graeme.gregory@linaro.org
Uhm, actually is not. As Bjorn and others pointed out, the function part in the PRT entries must be 0xffff in all cased (according to the spec), and so this approach doesn't fly.
I have sent a v2 that does the right thing, and works without changes to the kernel. There was some confusion on my part due to the fact that I was still receiving some errors (which are also fixed in v2), but the actual problem was already solved by Bjorn's suggested approach (confirmed on Cello and Overdrive using devices that are not MSI capable (or can be made so))
Platforms/AMD/Styx/AcpiTables/Dsdt.asl | 52 +++++++------------- 1 file changed, 19 insertions(+), 33 deletions(-)
diff --git a/Platforms/AMD/Styx/AcpiTables/Dsdt.asl b/Platforms/AMD/Styx/AcpiTables/Dsdt.asl index 3bfa26acea07..49ef55fc7df6 100644 --- a/Platforms/AMD/Styx/AcpiTables/Dsdt.asl +++ b/Platforms/AMD/Styx/AcpiTables/Dsdt.asl @@ -508,39 +508,25 @@ 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{Package (0x04){0xFFFF,0x00,0x00,0x0140},Package (0x04){0xFFFF,0x01,0x00,0x0141},Package (0x04){0xFFFF,0x02,0x00,0x0142},Package (0x04){0xFFFF,0x03,0x00,0x0143}
Name (_PRT, Package () // _PRT: PCI Routing Table{// slot 1: dev 2 fn 1Package () { 0x20001, 0x0, 0x0, 0x140 },Package () { 0x20001, 0x1, 0x0, 0x141 },Package () { 0x20001, 0x2, 0x0, 0x142 },Package () { 0x20001, 0x3, 0x0, 0x143 },// slot 2: dev 2 fn 2Package () { 0x20002, 0x0, 0x0, 0x144 },Package () { 0x20002, 0x1, 0x0, 0x145 },Package () { 0x20002, 0x2, 0x0, 0x146 },Package () { 0x20002, 0x3, 0x0, 0x147 },// slot 3: dev 2 fn 3Package () { 0x20003, 0x0, 0x0, 0x148 },Package () { 0x20003, 0x1, 0x0, 0x149 },Package () { 0x20003, 0x2, 0x0, 0x14a },Package () { 0x20003, 0x3, 0x0, 0x14b } }) // _PRT Method (_CRS, 0, Serialized) // _CRS: Current Resource Settings-- 2.9.3
On Mon, Apr 10, 2017 at 11:08:31AM +0100, Ard Biesheuvel wrote:
On 10 April 2017 at 11:05, graeme.gregory@linaro.org wrote:
On Fri, Apr 07, 2017 at 02:28:50PM +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
Considering exlanation on linux-acpi thread this looks fine to me.
Reviewed-by: Graeme Gregory graeme.gregory@linaro.org
Uhm, actually is not. As Bjorn and others pointed out, the function part in the PRT entries must be 0xffff in all cased (according to the spec), and so this approach doesn't fly.
I have sent a v2 that does the right thing, and works without changes to the kernel. There was some confusion on my part due to the fact that I was still receiving some errors (which are also fixed in v2), but the actual problem was already solved by Bjorn's suggested approach (confirmed on Cello and Overdrive using devices that are not MSI capable (or can be made so))
yeah sorry what I get for reading half in gmail and half in mutt, followed the wrong rabbit hole to thread end.
Sorry, re-looking at thread and v2 now!
Graeme
Platforms/AMD/Styx/AcpiTables/Dsdt.asl | 52 +++++++------------- 1 file changed, 19 insertions(+), 33 deletions(-)
diff --git a/Platforms/AMD/Styx/AcpiTables/Dsdt.asl b/Platforms/AMD/Styx/AcpiTables/Dsdt.asl index 3bfa26acea07..49ef55fc7df6 100644 --- a/Platforms/AMD/Styx/AcpiTables/Dsdt.asl +++ b/Platforms/AMD/Styx/AcpiTables/Dsdt.asl @@ -508,39 +508,25 @@ 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{Package (0x04){0xFFFF,0x00,0x00,0x0140},Package (0x04){0xFFFF,0x01,0x00,0x0141},Package (0x04){0xFFFF,0x02,0x00,0x0142},Package (0x04){0xFFFF,0x03,0x00,0x0143}
Name (_PRT, Package () // _PRT: PCI Routing Table{// slot 1: dev 2 fn 1Package () { 0x20001, 0x0, 0x0, 0x140 },Package () { 0x20001, 0x1, 0x0, 0x141 },Package () { 0x20001, 0x2, 0x0, 0x142 },Package () { 0x20001, 0x3, 0x0, 0x143 },// slot 2: dev 2 fn 2Package () { 0x20002, 0x0, 0x0, 0x144 },Package () { 0x20002, 0x1, 0x0, 0x145 },Package () { 0x20002, 0x2, 0x0, 0x146 },Package () { 0x20002, 0x3, 0x0, 0x147 },// slot 3: dev 2 fn 3Package () { 0x20003, 0x0, 0x0, 0x148 },Package () { 0x20003, 0x1, 0x0, 0x149 },Package () { 0x20003, 0x2, 0x0, 0x14a },Package () { 0x20003, 0x3, 0x0, 0x14b } }) // _PRT Method (_CRS, 0, Serialized) // _CRS: Current Resource Settings-- 2.9.3
-
Ard Biesheuvel -
Graeme Gregory -
graeme.gregory＠linaro.org