Skip to content

Commit 97e7971

Browse files
ij-intelgregkh
authored andcommitted
serial: 8250: Toggle IER bits on only after irq has been set up
[ Upstream commit 039d492 ] Invoking TIOCVHANGUP on 8250_mid port on Ice Lake-D and then reopening the port triggers these faults during serial8250_do_startup(): DMAR: DRHD: handling fault status reg 3 DMAR: [DMA Write NO_PASID] Request device [00:1a.0] fault addr 0x0 [fault reason 0x05] PTE Write access is not set If the IRQ hasn't been set up yet, the UART will have zeroes in its MSI address/data registers. Disabling the IRQ at the interrupt controller won't stop the UART from performing a DMA write to the address programmed in its MSI address register (zero) when it wants to signal an interrupt. The UARTs (in Ice Lake-D) implement PCI 2.1 style MSI without masking capability, so there is no way to mask the interrupt at the source PCI function level, except disabling the MSI capability entirely, but that would cause it to fall back to INTx# assertion, and the PCI specification prohibits disabling the MSI capability as a way to mask a function's interrupt service request. The MSI address register is zeroed by the hangup as the irq is freed. The interrupt is signalled during serial8250_do_startup() performing a THRE test that temporarily toggles THRI in IER. The THRE test currently occurs before UART's irq (and MSI address) is properly set up. Refactor serial8250_do_startup() such that irq is set up before the THRE test. The current irq setup code is intermixed with the timer setup code. As THRE test must be performed prior to the timer setup, extract it into own function and call it only after the THRE test. The ->setup_timer() needs to be part of the struct uart_8250_ops in order to not create circular dependency between 8250 and 8250_base modules. Fixes: 40b36da ("[PATCH] 8250 UART backup timer") Reported-by: Lennert Buytenhek <[email protected]> Tested-by: Lennert Buytenhek <[email protected]> Reviewed-by: Andy Shevchenko <[email protected]> Signed-off-by: Ilpo Järvinen <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]> Signed-off-by: Sasha Levin <[email protected]>
1 parent 7375945 commit 97e7971

File tree

3 files changed

+17
-8
lines changed

3 files changed

+17
-8
lines changed

drivers/tty/serial/8250/8250_core.c

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -298,10 +298,9 @@ static void serial8250_backup_timeout(struct timer_list *t)
298298
jiffies + uart_poll_timeout(&up->port) + HZ / 5);
299299
}
300300

301-
static int univ8250_setup_irq(struct uart_8250_port *up)
301+
static void univ8250_setup_timer(struct uart_8250_port *up)
302302
{
303303
struct uart_port *port = &up->port;
304-
int retval = 0;
305304

306305
/*
307306
* The above check will only give an accurate result the first time
@@ -322,10 +321,16 @@ static int univ8250_setup_irq(struct uart_8250_port *up)
322321
*/
323322
if (!port->irq)
324323
mod_timer(&up->timer, jiffies + uart_poll_timeout(port));
325-
else
326-
retval = serial_link_irq_chain(up);
324+
}
327325

328-
return retval;
326+
static int univ8250_setup_irq(struct uart_8250_port *up)
327+
{
328+
struct uart_port *port = &up->port;
329+
330+
if (port->irq)
331+
return serial_link_irq_chain(up);
332+
333+
return 0;
329334
}
330335

331336
static void univ8250_release_irq(struct uart_8250_port *up)
@@ -381,6 +386,7 @@ static struct uart_ops univ8250_port_ops;
381386
static const struct uart_8250_ops univ8250_driver_ops = {
382387
.setup_irq = univ8250_setup_irq,
383388
.release_irq = univ8250_release_irq,
389+
.setup_timer = univ8250_setup_timer,
384390
};
385391

386392
static struct uart_8250_port serial8250_ports[UART_NR];

drivers/tty/serial/8250/8250_port.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2303,6 +2303,10 @@ int serial8250_do_startup(struct uart_port *port)
23032303
if (port->irq && (up->port.flags & UPF_SHARE_IRQ))
23042304
up->port.irqflags |= IRQF_SHARED;
23052305

2306+
retval = up->ops->setup_irq(up);
2307+
if (retval)
2308+
goto out;
2309+
23062310
if (port->irq && !(up->port.flags & UPF_NO_THRE_TEST)) {
23072311
unsigned char iir1;
23082312

@@ -2345,9 +2349,7 @@ int serial8250_do_startup(struct uart_port *port)
23452349
}
23462350
}
23472351

2348-
retval = up->ops->setup_irq(up);
2349-
if (retval)
2350-
goto out;
2352+
up->ops->setup_timer(up);
23512353

23522354
/*
23532355
* Now, initialize the UART

include/linux/serial_8250.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ struct uart_8250_port;
7474
struct uart_8250_ops {
7575
int (*setup_irq)(struct uart_8250_port *);
7676
void (*release_irq)(struct uart_8250_port *);
77+
void (*setup_timer)(struct uart_8250_port *);
7778
};
7879

7980
struct uart_8250_em485 {

0 commit comments

Comments
 (0)