linux-kernel - Re: Trying to fix ITE-887x parallel/serial driver bugs (including unhandled IRQs)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 12 Apr 2010 17:05:26 -0400
From:	Joseph Krahn <joseph.krahn@...il.com>
To:	Alan Cox <alan@...rguk.ukuu.org.uk>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: Trying to fix ITE-887x parallel/serial driver bugs (including 
	unhandled IRQs)

On Mon, Apr 12, 2010 at 12:43 PM, Alan Cox <alan@...rguk.ukuu.org.uk> wrote:
>> Both drivers (8250_pci.c and parport_pc.c) probe randomly for the
>> chips control I/O port, instead of using the standard PNP-configured
>> BAR, and they do so independently, stepping on the previous drivers
>> configuration attempt. I think the two drivers should be merged into
>> parport_serial.c because this is a combo chip. However, different
>
> That seems reasonable.
>
> I don't think your description is accurate entirely. The device is picked
> up by PCi scans and the INTCBAR is then set up by Linux having checked
> for free address ranges. It's hardly 'random probing' and it isn't a
> normal BAR or guaranteed to have been configured by anything beforehand.
Yes, but the drivers don't use INTCBAR. They scan through a herd-coded
list of possible port ranges:

static int __devinit sio_ite_8872_probe(struct pci_dev *pdev, int autoirq,
                                         int autodma,
                                         const struct parport_pc_via_data *via)
{
        short inta_addr[6] = { 0x2A0, 0x2C0, 0x220, 0x240, 0x1E0 };
...
        for (i = 0; i < 5; i++) {
                base_res = request_region(inta_addr[i], 32, "it887x");
                if (base_res) {
...

>
>> The IT887x chips include an interrupt controller that maps external
>> IRQ inputs to serialized IRQs. Apparently, the I/O ports only present
>> a standard interrupt interface when wired for serialized IRQs. For
>> normal PCI interrupts, it needs a custom interrupt handler to
>> communicate directly with the interrupt-controller port. The current
>
> I guess the obvious thing to do would be to provide one. The serial code
> already supports several such things. What is actually needed or is the
> 'special' handler simply shared IRQ support in which case it ought t just
> work.
The interrupt controller (INTC) provides IRQ status bits in a single
I/O port byte. I only have the datasheet for IT8875F, which is 1
parallel and no serial ports, so my understanding of UART IRQs is sort
of reverse engineering. Polling the UART IIR register shows that it
acts like a normal 16550 UART when set to SERIRQ mode, but I get no
IRQs generated. Setting to standard PCI INTA interrupts, the IRQs are
generated, but the UART IIR registers do not set the IRQ bits.

One possibility is to install custom io_serial_in/out functions. When
the value of IIR is requested, the INTC can be checked, and modify the
result to what it should be for a normal UART. That is a bit of a
hack, but makes it much easier to fit into the existing code.
>
>> serial driver attempts to disable IRQs, and let the core driver revert
>> to polling. However, it does this incorrectly, and can produce
>> unhandled IRQs. (Maybe it was tested on a system with Ser. IRQ?) To
>> avoid extra interrupt-handler code, especially for a less common chip,
>> is it OK to intentionally provide polling-only support?
>
> I'd strongly prefer it worked as well as possible if the the
> documentation to fix it exists. What is involved ?
>
> Alan
>
It is easy to fix the code to use INTCBAR to configure the INTC I/O
port. It is probably not too hard to get a proper driver, if I can
find other users to test different hardware configurations. For
hardware wired to use the chip's serialized IRQ, it probably behaves
like standard PC-style chips, and no special IRQ code is needed. It
would be nice to verify this. Maybe a good approach would be to add
support IRQs, but add a module parameter to disable it.

Joe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/