Re: pci_intr_alloc() vs pci_intr_establish() - retry type?
Le mer. 5 déc. 2018 à 08:39, Masanobu SAITOH a écrit : > I suspect Serial ATA AHCI 1.2.1 specification page 111 has the hint. > > Figure 23: Port/CCC and MSI Message Mapping, Example 1 > Figure 24: Port and MSI Message Mapping, Example 2 > > I suspect MSI-X also assume this layout. pci_msix_alloc_map() might > be used. Very likely we need to disable MSICAP.MME bit if MSI happens to have more vectors to force single MSI operation. There is no such bit for MSI-X. Let's work on this off-list, I have some idea to test the interrupt mapping, and actually add support for multiple vectors. Since in your case the thing only fails for IDENTIFY, it looks like the registers mapped via regular BAR work and just interrupt routing is bad. Performance wise it doesn't buy much besides avoiding one register read per interrupt, because the whole ATA subsystem is not MPSAFE and hence the interrupt handlers are serialized via KERNEL_LOCK() anyway. > My SuperMicro A2SDi-H-TP4F's disk is connected to not channel 0 > but channel 6. I also verified that GHC.MRSM is 0. GHC.MRSM is only set if hardware has less MSI/MSI-X vectors than AHCI ports, and fallbacks to single MSI due to this. So it's actually normal for this to be 0. It'd be quite strange for hardware to have less MSI/MSI-X vectors than supported ports. Jaromir
Re: pci_intr_alloc() vs pci_intr_establish() - retry type?
On 2018/12/05 4:53, Jaromír Doleček wrote: I've now disabled MSI-X for ahcisata(4), need to figure what needs to be setup there. Le lun. 3 déc. 2018 à 22:41, Jared McNeill a écrit : IIUC we don't actually have confirmation that ThunderX AHCI works yet.. Nick is having issues with his board. Okay - I misunderstood, thought it was confirmed working. The BAR used for Cavium is slightly suspicious. AHCI spec mentions (v1.3.1 section 2.4 Serial ATA Capability) 0x10 BAR as one of 'legacy' access methods. It specifically only talks about 0x24 as the proper BAR for AHCI, it doesn't mention option to have it elsewhere. Jaromir I suspect Serial ATA AHCI 1.2.1 specification page 111 has the hint. Figure 23: Port/CCC and MSI Message Mapping, Example 1 Figure 24: Port and MSI Message Mapping, Example 2 I suspect MSI-X also assume this layout. pci_msix_alloc_map() might be used. My SuperMicro A2SDi-H-TP4F's disk is connected to not channel 0 but channel 6. I also verified that GHC.MRSM is 0. ahcisata1 at pci0 dev 20 function 0: vendor 8086 product 19c2 (rev. 0x11) ahcisata1: interrupting at msix2 vec 0 ahcisata1: 64-bit DMA ahcisata1: AHCI revision 1.31, 4 ports, 32 slots, CAP 0xc3349f43 X GHC = 8002 atabus8 at ahcisata1 channel 3 atabus9 at ahcisata1 channel 4 atabus10 at ahcisata1 channel 5 atabus11 at ahcisata1 channel 6 -- --- SAITOH Masanobu (msai...@execsw.org msai...@netbsd.org)
Re: pci_intr_alloc() vs pci_intr_establish() - retry type?
I've now disabled MSI-X for ahcisata(4), need to figure what needs to be setup there. Le lun. 3 déc. 2018 à 22:41, Jared McNeill a écrit : > IIUC we don't actually have confirmation that ThunderX AHCI works yet.. > Nick is having issues with his board. Okay - I misunderstood, thought it was confirmed working. The BAR used for Cavium is slightly suspicious. AHCI spec mentions (v1.3.1 section 2.4 Serial ATA Capability) 0x10 BAR as one of 'legacy' access methods. It specifically only talks about 0x24 as the proper BAR for AHCI, it doesn't mention option to have it elsewhere. Jaromir
Re: pci_intr_alloc() vs pci_intr_establish() - retry type?
On 2018/12/04 16:44, Masanobu SAITOH wrote:
On 2018/12/04 4:55, Jaromír Doleček wrote:
Le lun. 3 déc. 2018 à 12:09, Masanobu SAITOH a écrit :
C3000's AHCI has multi-vector MSI-X table and it doesn't work since
Nobember 20th...
Can you try if by chance this code adapted nvme(4) changes anything on
your system?
http://www.netbsd.org/~jdolecek/ahcisata_msixoff.diff
It panics:
[ 1.0427481] panic: extent_alloc_region: bad size
[ 1.0427481] cpu0: Begin traceback...
[ 1.0427481] vpanic() at netbsd:vpanic+0x16f
[ 1.0427481] snprintf() at netbsd:snprintf
[ 1.0427481] extent_alloc_region() at netbsd:extent_alloc_region+0x29e
[ 1.0427481] bus_space_reserve() at netbsd:bus_space_reserve+0x95
[ 1.0427481] bus_space_map() at netbsd:bus_space_map+0x85
[ 1.0427481] ahci_pci_attach() at netbsd:ahci_pci_attach+0xde
[ 1.0427481] config_attach_loc() at netbsd:config_attach_loc+0x1a8
[ 1.0427481] config_found_sm_loc() at netbsd:config_found_sm_loc+0x48
[ 1.0427481] pci_probe_device() at netbsd:pci_probe_device+0x582
[ 1.0427481] pci_enumerate_bus() at netbsd:pci_enumerate_bus+0x197
[ 1.0427481] pcirescan() at netbsd:pcirescan+0x47
[ 1.0427481] pciattach() at netbsd:pciattach+0x193
[ 1.0427481] config_attach_loc() at netbsd:config_attach_loc+0x1a8
[ 1.0427481] config_found_sm_loc() at netbsd:config_found_sm_loc+0x48
[ 1.0427481] mp_pci_scan() at netbsd:mp_pci_scan+0xa3
[ 1.0427481] mainbus_attach() at netbsd:mainbus_attach+0x332
[ 1.0427481] config_attach_loc() at netbsd:config_attach_loc+0x1a8
[ 1.0427481] cpu_configure() at netbsd:cpu_configure+0x2b
[ 1.0427481] main() at netbsd:main+0x331
[ 1.0427481] cpu0: End traceback...
[ 1.0427481] fatal breakpoint trap in supervisor mode
[ 1.0427481] trap type 1 code 0 rip 0x8021de45 cs 0x8 rflags 0x202
cr2 0 ilevel 0x8 rsp 0x81d027f0
[ 1.0427481] curlwp 0x81858900 pid 0.1 lowest kstack
0x81cfe2c0
Stopped in pid 0.1 (system) at netbsd:breakpoint+0x5: leave
pcictl dump shows:
PCI configuration registers:
Common header:
0x00: 0x19c28086 0x02b00147 0x01060110 0x
Vendor Name: Intel (0x8086)
Device Name: C3000 SATA Controller 1 (0x19c2)
Command register: 0x0147
I/O space accesses: on
Memory space accesses: on
Bus mastering: on
Special cycles: off
MWI transactions: off
Palette snooping: off
Parity error checking: on
Address/data stepping: off
System error (SERR): on
Fast back-to-back transactions: off
Interrupt disable: off
Status register: 0x02b0
Immediate Readiness: off
Interrupt status: inactive
Capability List support: on
66 MHz capable: on
User Definable Features (UDF) support: off
Fast back-to-back capable: on
Data parity error detected: off
DEVSEL timing: medium (0x1)
Slave signaled Target Abort: off
Master received Target Abort: off
Master received Master Abort: off
Asserted System Error (SERR): off
Parity error detected: off
Class Name: mass storage (0x01)
Subclass Name: SATA (0x06)
Interface Name: AHCI 1.0 (0x01)
Revision ID: 0x10
BIST: 0x00
Header Type: 0x00 (0x00)
Latency Timer: 0x00
Cache Line Size: 0bytes (0x00)
Type 0 ("normal" device) header:
0x10: 0xdffb4000 0xdffbd000 0xe071 0xe061
0x20: 0xe021 0xdffbc000 0x 0x72708086
0x30: 0x 0x0080 0x 0x010e
Base address register at 0x10
type: 32-bit nonprefetchable memory
base: 0xdffb4000
Base address register at 0x14
type: 32-bit nonprefetchable memory
base: 0xdffbd000
Base address register at 0x18
type: I/O
base: 0xe070
Base address register at 0x1c
type: I/O
base: 0xe060
Base address register at 0x20
type: I/O
base: 0xe020
Base address register at 0x24
type: 32-bit nonprefetchable memory
base: 0xdffbc000
Cardbus CIS Pointer: 0x
Subsystem vendor ID: 0x8086
Subsystem ID: 0x7270
Expansion ROM Base Address Register: 0x
base: 0x
Expansion ROM Enable: off
Validation Status: Validation not supported
Validation Details: 0x0
Capability list pointer: 0x80
Reserved @ 0x38: 0x
Maximum Latency: 0x00
Minimum Grant: 0x00
Interrupt pin: 0x01 (pin A)
Interrupt line: 0x0e
Capability register at 0x80
type: 0x05 (MSI)
Capability register at 0x70
type: 0x01 (Power Management)
Capability register at 0xa8
type: 0x12 (SATA)
Capability register at 0xd0
type: 0x11 (MSI-X)
PCI Power Management Capabilities Register
Capabilities register: 0x4003
Version: 1.2
PME# clock: off
Device specific initialization: off
3.3V auxiliary current: self-powered
D1 power management state support: off
D2 power management state support: off
PME# s
Re: pci_intr_alloc() vs pci_intr_establish() - retry type?
On 2018/12/04 4:55, Jaromír Doleček wrote:
Le lun. 3 déc. 2018 à 12:09, Masanobu SAITOH a écrit :
C3000's AHCI has multi-vector MSI-X table and it doesn't work since
Nobember 20th...
Can you try if by chance this code adapted nvme(4) changes anything on
your system?
http://www.netbsd.org/~jdolecek/ahcisata_msixoff.diff
It panics:
[ 1.0427481] panic: extent_alloc_region: bad size
[ 1.0427481] cpu0: Begin traceback...
[ 1.0427481] vpanic() at netbsd:vpanic+0x16f
[ 1.0427481] snprintf() at netbsd:snprintf
[ 1.0427481] extent_alloc_region() at netbsd:extent_alloc_region+0x29e
[ 1.0427481] bus_space_reserve() at netbsd:bus_space_reserve+0x95
[ 1.0427481] bus_space_map() at netbsd:bus_space_map+0x85
[ 1.0427481] ahci_pci_attach() at netbsd:ahci_pci_attach+0xde
[ 1.0427481] config_attach_loc() at netbsd:config_attach_loc+0x1a8
[ 1.0427481] config_found_sm_loc() at netbsd:config_found_sm_loc+0x48
[ 1.0427481] pci_probe_device() at netbsd:pci_probe_device+0x582
[ 1.0427481] pci_enumerate_bus() at netbsd:pci_enumerate_bus+0x197
[ 1.0427481] pcirescan() at netbsd:pcirescan+0x47
[ 1.0427481] pciattach() at netbsd:pciattach+0x193
[ 1.0427481] config_attach_loc() at netbsd:config_attach_loc+0x1a8
[ 1.0427481] config_found_sm_loc() at netbsd:config_found_sm_loc+0x48
[ 1.0427481] mp_pci_scan() at netbsd:mp_pci_scan+0xa3
[ 1.0427481] mainbus_attach() at netbsd:mainbus_attach+0x332
[ 1.0427481] config_attach_loc() at netbsd:config_attach_loc+0x1a8
[ 1.0427481] cpu_configure() at netbsd:cpu_configure+0x2b
[ 1.0427481] main() at netbsd:main+0x331
[ 1.0427481] cpu0: End traceback...
[ 1.0427481] fatal breakpoint trap in supervisor mode
[ 1.0427481] trap type 1 code 0 rip 0x8021de45 cs 0x8 rflags 0x202
cr2 0 ilevel 0x8 rsp 0x81d027f0
[ 1.0427481] curlwp 0x81858900 pid 0.1 lowest kstack
0x81cfe2c0
Stopped in pid 0.1 (system) at netbsd:breakpoint+0x5: leave
pcictl dump shows:
PCI configuration registers:
Common header:
0x00: 0x19c28086 0x02b00147 0x01060110 0x
Vendor Name: Intel (0x8086)
Device Name: C3000 SATA Controller 1 (0x19c2)
Command register: 0x0147
I/O space accesses: on
Memory space accesses: on
Bus mastering: on
Special cycles: off
MWI transactions: off
Palette snooping: off
Parity error checking: on
Address/data stepping: off
System error (SERR): on
Fast back-to-back transactions: off
Interrupt disable: off
Status register: 0x02b0
Immediate Readiness: off
Interrupt status: inactive
Capability List support: on
66 MHz capable: on
User Definable Features (UDF) support: off
Fast back-to-back capable: on
Data parity error detected: off
DEVSEL timing: medium (0x1)
Slave signaled Target Abort: off
Master received Target Abort: off
Master received Master Abort: off
Asserted System Error (SERR): off
Parity error detected: off
Class Name: mass storage (0x01)
Subclass Name: SATA (0x06)
Interface Name: AHCI 1.0 (0x01)
Revision ID: 0x10
BIST: 0x00
Header Type: 0x00 (0x00)
Latency Timer: 0x00
Cache Line Size: 0bytes (0x00)
Type 0 ("normal" device) header:
0x10: 0xdffb4000 0xdffbd000 0xe071 0xe061
0x20: 0xe021 0xdffbc000 0x 0x72708086
0x30: 0x 0x0080 0x 0x010e
Base address register at 0x10
type: 32-bit nonprefetchable memory
base: 0xdffb4000
Base address register at 0x14
type: 32-bit nonprefetchable memory
base: 0xdffbd000
Base address register at 0x18
type: I/O
base: 0xe070
Base address register at 0x1c
type: I/O
base: 0xe060
Base address register at 0x20
type: I/O
base: 0xe020
Base address register at 0x24
type: 32-bit nonprefetchable memory
base: 0xdffbc000
Cardbus CIS Pointer: 0x
Subsystem vendor ID: 0x8086
Subsystem ID: 0x7270
Expansion ROM Base Address Register: 0x
base: 0x
Expansion ROM Enable: off
Validation Status: Validation not supported
Validation Details: 0x0
Capability list pointer: 0x80
Reserved @ 0x38: 0x
Maximum Latency: 0x00
Minimum Grant: 0x00
Interrupt pin: 0x01 (pin A)
Interrupt line: 0x0e
Capability register at 0x80
type: 0x05 (MSI)
Capability register at 0x70
type: 0x01 (Power Management)
Capability register at 0xa8
type: 0x12 (SATA)
Capability register at 0xd0
type: 0x11 (MSI-X)
PCI Power Management Capabilities Register
Capabilities register: 0x4003
Version: 1.2
PME# clock: off
Device specific initialization: off
3.3V auxiliary current: self-powered
D1 power management state support: off
D2 power management state support: off
PME# support D0: off
PME# support D1: off
Re: pci_intr_alloc() vs pci_intr_establish() - retry type?
On Mon, 3 Dec 2018, Jaromír Doleček wrote: ahcisata(4) works for me on two different computers in MSI mode. I don't have any ahcisata device which supports MSI-X, but it is working on Cavium which has MSI-X. Perhaps some different BAR wierdness than Cavium, or simply the boards using MSI-X should use the "cavium" BAR? IIUC we don't actually have confirmation that ThunderX AHCI works yet.. Nick is having issues with his board.
Re: pci_intr_alloc() vs pci_intr_establish() - retry type?
Le lun. 3 déc. 2018 à 12:09, Masanobu SAITOH a écrit : > C3000's AHCI has multi-vector MSI-X table and it doesn't work since > Nobember 20th... Can you try if by chance this code adapted nvme(4) changes anything on your system? http://www.netbsd.org/~jdolecek/ahcisata_msixoff.diff ahcisata(4) works for me on two different computers in MSI mode. I don't have any ahcisata device which supports MSI-X, but it is working on Cavium which has MSI-X. Perhaps some different BAR wierdness than Cavium, or simply the boards using MSI-X should use the "cavium" BAR? So another thing to try would be to make ahci_pci_abar() return AHCI_PCI_ABAR_CAVIUM always and see if it changes things. Also you can try rev 1.44 of ahcisata_pci.c with AHCISATA_DISABLE_MSIX to see if it works if forced to MSI only. I wonder whether there is something we still need to adjust during general initialization, I see this on my hardware: re(4) - on my system works when attached either MSI or MSI-X nvme(4) - on my system works only when attached via MSI-X (regardless of the PCI_CAP_MSIX), doesn't work when forced to MSI ahcisata(4) works in MSI on my system, as MSI-X for skrll@, MSI-X doesn't work for you Jaromir
Re: pci_intr_alloc() vs pci_intr_establish() - retry type?
On 2018/12/01 5:27, Jared McNeill wrote:
On Fri, 30 Nov 2018, Martin Husemann wrote:
On Fri, Nov 30, 2018 at 02:22:37PM -0400, Jared McNeill wrote:
The driver can call pci_intr_type on the handle returned by pci_intr_alloc
to see what kind of interrupt (INTx/MSI/MSI-X) was negotiated.
Yeah, but the fear was that some hardware could either do a single INT
or (lots of) MSI{X}, and now you end up with (lots of) INT returned
from pci_intr_alloc() and can just use the first.
Does this not happen for real world hardware, or is the driver supposed to
check with pci_*_count() upfront?
I don't think pci_intr_alloc will ever return more than one INTx..
C3000's AHCI has multi-vector MSI-X table and it doesn't work since
Nobember 20th...
[ 1.0426355] ahcisata0 at pci0 dev 19 function 0: vendor 8086 product 19b2
(rev. 0x10)
[ 1.0426355] allocated pic msix5 type edge pin 0 level 6 to cpu0 slot 21 idt
entry 110
[ 1.0426355] ahcisata0: interrupting at msix5 vec 0
[ 1.0426355] ahcisata0: 64-bit DMA
[ 1.0426355] ahcisata0: AHCI revision 1.31, 1 port, 32 slots, CAP
0xc3369f40
[ 1.0426355] atabus0 at ahcisata0 channel 3
[ 1.0426355] ahcisata1 at pci0 dev 20 function 0: vendor 8086 product 19c2
(rev. 0x10)
[ 1.0426355] allocated pic msix6 type edge pin 0 level 6 to cpu0 slot 22 idt
entry 111
[ 1.0426355] ahcisata1: interrupting at msix6 vec 0
[ 1.0426355] ahcisata1: 64-bit DMA
[ 1.0426355] ahcisata1: AHCI revision 1.31, 2 ports, 32 slots, CAP
0xc3369f41
[ 1.0426355] atabus1 at ahcisata1 channel 4
[ 1.0426355] atabus2 at ahcisata1 channel 5
(snip)
[ 3.4536958] wd0 at atabus2 drive 0
[ 6.4561682] wd0: IDENTIFY failed
[ 6.4561682] wd0: fixing 0 sector size
[ 6.4561682] wd0: secperunit and ncylinders are zero
[ 9.4686496] wd0(ahcisata1:5:0): using PIO mode 0
(snip)
[ 9.6688146] boot device:
--
---
SAITOH Masanobu (msai...@execsw.org
msai...@netbsd.org)
Re: pci_intr_alloc() vs pci_intr_establish() - retry type?
On Fri, 30 Nov 2018, Martin Husemann wrote:
On Fri, Nov 30, 2018 at 02:22:37PM -0400, Jared McNeill wrote:
The driver can call pci_intr_type on the handle returned by pci_intr_alloc
to see what kind of interrupt (INTx/MSI/MSI-X) was negotiated.
Yeah, but the fear was that some hardware could either do a single INT
or (lots of) MSI{X}, and now you end up with (lots of) INT returned
from pci_intr_alloc() and can just use the first.
Does this not happen for real world hardware, or is the driver supposed to
check with pci_*_count() upfront?
I don't think pci_intr_alloc will ever return more than one INTx..
Re: pci_intr_alloc() vs pci_intr_establish() - retry type?
On Fri, Nov 30, 2018 at 02:22:37PM -0400, Jared McNeill wrote:
> The driver can call pci_intr_type on the handle returned by pci_intr_alloc
> to see what kind of interrupt (INTx/MSI/MSI-X) was negotiated.
Yeah, but the fear was that some hardware could either do a single INT
or (lots of) MSI{X}, and now you end up with (lots of) INT returned
from pci_intr_alloc() and can just use the first.
Does this not happen for real world hardware, or is the driver supposed to
check with pci_*_count() upfront?
Martin
Re: pci_intr_alloc() vs pci_intr_establish() - retry type?
On Wed, 28 Nov 2018, Mouse wrote: This reminds me - how does a driver know if any kind of MSI support is even available? it shouldn't need to. Okay, then I'm missing something. I've seen hardware where enabling MSI is something device-specific (a value in a register, typically), meaning _something_ MD, presumably the driver, has to be involved if MSI is to be used. And how can a driver do this without knowing whether it's supported? The driver can call pci_intr_type on the handle returned by pci_intr_alloc to see what kind of interrupt (INTx/MSI/MSI-X) was negotiated.
Re: pci_intr_alloc() vs pci_intr_establish() - retry type?
On Wed, Nov 28, 2018 at 12:34:42PM -0500, Michael wrote: > > Or, for that matter, if it can use one IRQ or - say - 8 MSI. Right, this is pretty common, isn't it? -- Thor Lancelot Simon t...@panix.com "Whether or not there's hope for change is not the question. If you want to be a free person, you don't stand up for human rights because it will work, but because it is right." --Andrei Sakharov
Re: pci_intr_alloc() vs pci_intr_establish() - retry type?
Hello, On Wed, 28 Nov 2018 07:48:09 -0500 (EST) Mouse wrote: > >> This reminds me - how does a driver know if any kind of MSI support > >> is even available? > > it shouldn't need to. > > Okay, then I'm missing something. I've seen hardware where enabling > MSI is something device-specific (a value in a register, typically), > meaning _something_ MD, presumably the driver, has to be involved if > MSI is to be used. Or, for that matter, if it can use one IRQ or - say - 8 MSI. have fun Michael
Re: pci_intr_alloc() vs pci_intr_establish() - retry type?
Jared McNeill wrote: >On Wed, 28 Nov 2018, Jaromír Doleček wrote: > >> pci_intr_alloc() checks what the device claims to support down the >> call stack, reading the PCI config space. I assume there some >> negotiation between the PCI bus and the device. I hope device can't >> claim to support e.g. MSI-X if the bus doesn't. > >Device claiming to support MSI-X is fine. The bus should only set >PCI_FLAGS_MSIX_OKAY in its pcibus_attach_args pba_flags in the case that >MSI-X is supported, and then the MD code will filter it out: I traced through what one of my non-MSI machines does at boot. The wm(4) driver asks for MSI-X interrupts, the MD PCI code gives it an INTx interrupt, the driver sets things up correctly to use the returned interrupt, it doesn't loop in the driver retrying the pci_intr_alloc().
Re: pci_intr_alloc() vs pci_intr_establish() - retry type?
>> This reminds me - how does a driver know if any kind of MSI support >> is even available? > it shouldn't need to. Okay, then I'm missing something. I've seen hardware where enabling MSI is something device-specific (a value in a register, typically), meaning _something_ MD, presumably the driver, has to be involved if MSI is to be used. And how can a driver do this without knowing whether it's supported? /~\ The ASCII Mouse \ / Ribbon Campaign X Against HTMLmo...@rodents-montreal.org / \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B
Re: pci_intr_alloc() vs pci_intr_establish() - retry type?
On Wed, 28 Nov 2018, Jaromír Doleček wrote: pci_intr_alloc() checks what the device claims to support down the call stack, reading the PCI config space. I assume there some negotiation between the PCI bus and the device. I hope device can't claim to support e.g. MSI-X if the bus doesn't. Device claiming to support MSI-X is fine. The bus should only set PCI_FLAGS_MSIX_OKAY in its pcibus_attach_args pba_flags in the case that MSI-X is supported, and then the MD code will filter it out: https://nxr.netbsd.org/xref/src/sys/arch/arm/pci/pci_msi_machdep.c#90 https://nxr.netbsd.org/xref/src/sys/arch/x86/pci/pci_msi_machdep.c#256
re: pci_intr_alloc() vs pci_intr_establish() - retry type?
> This reminds me - how does a driver know if any kind of MSI support is > even available? it shouldn't need to. unless it has specific need to avoid or prefer a particular type of interrupt, the MD pci needs to deal with this. eg, whatever the pci_chipset_tag_t the driver attches with, should enable the pci(9) backends to DTRT. pci_intr_alloc(9) is already generic enough to enable all this (in that it can be told to prefer anything, or to just work.) ie, drivers that use pci_intr_alloc() without asking for any particular type _should_ work with any controller. i can't really comment on the actual question -- about whether etablish can fail or not after alloc. i think it would be good if "alloc" implies that "establish" will work, but without looking closely at all the cases (ie, drivers :), i can't be sure it isn't really necessary. .mrg.
Re: pci_intr_alloc() vs pci_intr_establish() - retry type?
Le mer. 28 nov. 2018 à 00:42, Taylor R Campbell a écrit : > > Date: Tue, 27 Nov 2018 21:14:04 + > > From: Robert Swindells > > > > It looks to me that drivers try MSI and/or MSIX first based on the > > device type not on whether the host controller can handle it. pci_intr_alloc() checks what the device claims to support down the call stack, reading the PCI config space. I assume there some negotiation between the PCI bus and the device. I hope device can't claim to support e.g. MSI-X if the bus doesn't. > Can we make the same API work for MSI, MSI-X, and INTx interrupts, so > that we don't have to remember two or three different APIs? Maybe > just pass a subset of PCI_INTR_INTx|PCI_INTR_MSI|PCI_INTR_MSIX to say > which ones to try or something? Yes, it would be the goal to have some shared API to do this fallback logic. But I need to understand whether it's actually necessary. It would be much simpler if we can rely on establish to work regardless of type of interrupt. Especially in 1/1/1 case, I don't see in the code a path where it would be possible for MSI/MSI-X establish to fail, but e.g. INTx one to not fail too. For sure, the MD allocation can fail due to some memory shortage or because we run out of interrupts, supported by the port. I don't see how establishing 1 MSI/MSI-X interrupt could fail, but then establishing 1 INTx could succeed. So perhaps it's okay to just to try to establish once and remove all those fallbacks. Unless the driver does something fancy, like try to use ncpu MSI interrupts and falling back to 1 INTx interrupt. I'd be in favour of simplifying the code to do just that in xhci(4) and ahcisata(4) - opinions? Jaromir
Re: pci_intr_alloc() vs pci_intr_establish() - retry type?
Hello, On Tue, 27 Nov 2018 21:34:53 +0100 Jaromír Doleček wrote: > I see several drivers (e.g. xhci(4), ahcisata(4), bge(4), nvme(4)) > retry the pci_intr_alloc()+pci_intr_establish() with 'lower' types > when pci_intr_establish() fails. > > Is this a real case, can it actualy happen that pci_intr_alloc() > returns the interrupt handlers, but pci_intr_establish() would fail > for it? This reminds me - how does a driver know if any kind of MSI support is even available? The reason I'm asking - on G5 macs we can support MSI and I'm planning to add code for that eventually. ( Also on Iyonix, but that's got more important problems for now. ) Wether support is available would essentially depend on which host bridge the device in question is connected to. PCI-X and PCIe slots in G5s could support MSI, the AGP slot found in some G5s could not, and no G3 or G4 with whatever type of PCI slots can support MSI. On PCI-X G5s the situation is a bit weird - they have the same OpenPIC as G4s, with IRQ lines wired to each slot, and on top of that another OpenPIC which lives in the HT/PCI(-X|e) hostbridge, which can respond to MSIs from anything living on a bus it controls. PCIe G5s have only the latter. have fun Michael
Re: pci_intr_alloc() vs pci_intr_establish() - retry type?
> Date: Tue, 27 Nov 2018 21:14:04 + > From: Robert Swindells > > It looks to me that drivers try MSI and/or MSIX first based on the > device type not on whether the host controller can handle it. Can we make the same API work for MSI, MSI-X, and INTx interrupts, so that we don't have to remember two or three different APIs? Maybe just pass a subset of PCI_INTR_INTx|PCI_INTR_MSI|PCI_INTR_MSIX to say which ones to try or something?
Re: pci_intr_alloc() vs pci_intr_establish() - retry type?
Jaromir Dolecek wrote: >I see several drivers (e.g. xhci(4), ahcisata(4), bge(4), nvme(4)) >retry the pci_intr_alloc()+pci_intr_establish() with 'lower' types >when pci_intr_establish() fails. > >Is this a real case, can it actualy happen that pci_intr_alloc() >returns the interrupt handlers, but pci_intr_establish() would fail >for it? > >If it's not a real case, it would be nice to remove all that boilerplate code. I have several amd64 systems that don't do MSI/MSIX. It looks to me that drivers try MSI and/or MSIX first based on the device type not on whether the host controller can handle it. I can add some debug to see what is going on. Robert Swindells
pci_intr_alloc() vs pci_intr_establish() - retry type?
I see several drivers (e.g. xhci(4), ahcisata(4), bge(4), nvme(4)) retry the pci_intr_alloc()+pci_intr_establish() with 'lower' types when pci_intr_establish() fails. Is this a real case, can it actualy happen that pci_intr_alloc() returns the interrupt handlers, but pci_intr_establish() would fail for it? If it's not a real case, it would be nice to remove all that boilerplate code. Jaromir
