[PATCH 0/2] BUG, more dw8259 ACPI crashes

[PATCH 0/2] BUG, more dw8259 ACPI crashes

[Jeremy Linton]

5.19rc4 crashes with:

[    4.441703] Internal error: Oops: 96000004 [#1] SMP
[    4.446601] Modules linked in:
[    4.449668] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.19.0-0.rc4.33.fc37.aarch64 #1
[    4.457532] Hardware name: Marvell                         Armada 7k/8k Family Board      /Armada 7k/8k Family Board      , BIOS EDK II Jun  4 2019
[    4.470800] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    4.477791] pc : dw8250_handle_irq+0x58/0x190
[    4.482170] lr : dw8250_handle_irq+0x38/0x190
[    4.486544] sp : ffff800008003e40
[    4.489869] x29: ffff800008003e40 x28: ffffbe13be183980 x27: 00000000b46bd3d0
[    4.497036] x26: ffffbe13bdbc3008 x25: ffff6f1447910f80 x24: ffff6f1447910f94
[    4.504204] x23: 0000000000000012 x22: ffff6f144218fc80 x21: 0000000000000000
[    4.511372] x20: 0000000000000007 x19: ffffbe13be7628d0 x18: ffff80000918b528
[    4.518540] x17: ffffb103c0343000 x16: ffff800008004000 x15: 0000000000004000
[    4.525706] x14: 0000000000000000 x13: 0000000000000030 x12: 0101010101010101
[    4.532872] x11: ffffbe13be16f6d8 x10: 0000000000001d90 x9 : ffffbe13bc6b4f0c
[    4.540040] x8 : ffffbe13be185770 x7 : 0000000000000000 x6 : 00000000296b5f4a
[    4.547206] x5 : 00ffffffffffffff x4 : 0000000000010002 x3 : ffffbe13be7628d0
[    4.554373] x2 : 0000000000000007 x1 : 0000000000000000 x0 : 0000000000000000
[    4.561539] Call trace:
[    4.563994]  dw8250_handle_irq+0x58/0x190
[    4.568020]  serial8250_interrupt+0x68/0x140
[    4.572307]  __handle_irq_event_percpu+0x68/0x230
[    4.577034]  handle_irq_event+0x58/0x120
[    4.580973]  handle_fasteoi_irq+0xcc/0x1c4
[    4.585086]  generic_handle_domain_irq+0x38/0x50
[    4.589723]  gic_handle_irq+0x50/0xe0
[    4.593399]  call_on_irq_stack+0x2c/0x38
[    4.597338]  do_interrupt_handler+0xdc/0xe0
[    4.601539]  el1_interrupt+0x34/0x70
[    4.605131]  el1h_64_irq_handler+0x18/0x24
[    4.609244]  el1h_64_irq+0x68/0x6c
[    4.612658]  arch_cpu_idle+0x18/0x2c
[    4.616246]  default_idle_call+0x34/0x18c
[    4.620273]  cpuidle_idle_call+0x154/0x1a0
[    4.624388]  do_idle+0xa4/0xf4
[    4.627455]  cpu_startup_entry+0x34/0x3c
[    4.631394]  kernel_init+0x0/0x150
[    4.634808]  arch_post_acpi_subsys_init+0x0/0x30
[    4.639445]  start_kernel+0x474/0x490
[    4.643121]  __primary_switched+0xc0/0xc8
[    4.647148] Code: 7100305f 1a9f17f5 f100003f 7a400aa4 (b9400817) 
[    4.653267] ---[ end trace 0000000000000000 ]---

On an ACPI enabled Mcbin. This is again the result
of the pdata information not being available on an
ACPI machine. But since there are multiple commits
I've broken this into to fixes. The first
simply reverts the USR register change because it
should be functionality transparent and I don't
have a good way to tie ACPI ids to the hardware
and the second fixes the later patch which uses it
in the interrupt handler.

Jeremy Linton (2):
  Revert "serial: 8250: dw: Move the USR register to pdata"
  serial: 8250: dw: Fix NULL pointer dereference

 drivers/tty/serial/8250/8250_dw.c    | 14 +++++++-------
 drivers/tty/serial/8250/8250_dwlib.h |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

-- 
2.35.1

[PATCH 1/2] Revert "serial: 8250: dw: Move the USR register to pdata"

[Jeremy Linton]

pdata is only setup by DT machines, leaving ACPI machines
with null pdata. Since I don't know the exact mapping of
ACPI ID's to dw 8250 variations I can't add pdata to them
without possibly breaking something. As such the simplest
fix here is probably just to revert this commit.

This reverts commit ffd381445eac2aa624e49bab5a811451e8351008.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 drivers/tty/serial/8250/8250_dw.c    | 9 +++------
 drivers/tty/serial/8250/8250_dwlib.h | 2 +-
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index f57bbd32ef11..53e488beb8c2 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -275,7 +275,7 @@ static int dw8250_handle_irq(struct uart_port *p)
 
 	if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
 		/* Clear the USR */
-		(void)p->serial_in(p, d->pdata->usr_reg);
+		(void)p->serial_in(p, d->usr_reg);
 
 		return 1;
 	}
@@ -447,6 +447,7 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
 			p->serial_out = dw8250_serial_outq;
 			p->flags = UPF_SKIP_TEST | UPF_SHARE_IRQ | UPF_FIXED_TYPE;
 			p->type = PORT_OCTEON;
+			data->usr_reg = OCTEON_UART_USR;
 			data->skip_autocfg = true;
 		}
 #endif
@@ -536,6 +537,7 @@ static int dw8250_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	data->data.dma.fn = dw8250_fallback_dma_filter;
+	data->usr_reg = DW_UART_USR;
 	data->pdata = device_get_match_data(p->dev);
 	p->private_data = &data->data;
 
@@ -738,27 +740,22 @@ static const struct dev_pm_ops dw8250_pm_ops = {
 };
 
 static const struct dw8250_platform_data dw8250_dw_apb = {
-	.usr_reg = DW_UART_USR,
 };
 
 static const struct dw8250_platform_data dw8250_octeon_3860_data = {
-	.usr_reg = OCTEON_UART_USR,
 	.quirks = DW_UART_QUIRK_OCTEON,
 };
 
 static const struct dw8250_platform_data dw8250_armada_38x_data = {
-	.usr_reg = DW_UART_USR,
 	.quirks = DW_UART_QUIRK_ARMADA_38X,
 };
 
 static const struct dw8250_platform_data dw8250_renesas_rzn1_data = {
-	.usr_reg = DW_UART_USR,
 	.cpr_val = 0x00012f32,
 	.quirks = DW_UART_QUIRK_IS_DMA_FC,
 };
 
 static const struct dw8250_platform_data dw8250_starfive_jh7100_data = {
-	.usr_reg = DW_UART_USR,
 	.quirks = DW_UART_QUIRK_SKIP_SET_RATE,
 };
 
diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
index 055bfdc87985..9870ad0d54e4 100644
--- a/drivers/tty/serial/8250/8250_dwlib.h
+++ b/drivers/tty/serial/8250/8250_dwlib.h
@@ -26,7 +26,6 @@ struct dw8250_port_data {
 };
 
 struct dw8250_platform_data {
-	u8 usr_reg;
 	u32 cpr_val;
 	unsigned int quirks;
 };
@@ -35,6 +34,7 @@ struct dw8250_data {
 	struct dw8250_port_data	data;
 	const struct dw8250_platform_data *pdata;
 
+	u8			usr_reg;
 	int			msr_mask_on;
 	int			msr_mask_off;
 	struct clk		*clk;
-- 
2.35.1

[PATCH 2/2] serial: 8250: dw: Fix NULL pointer dereference

[Jeremy Linton]

Similarly to e6a08c6949cb, ACPI machines don't have
a pdata setup, leading to a null pointer dereference
when dw8250_handle_irq runs.

Fixes: aa63d786cea2 ("serial: 8250: dw: Add support for DMA flow controlling devices")
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 drivers/tty/serial/8250/8250_dw.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 53e488beb8c2..06f80090cbb9 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -237,10 +237,13 @@ static int dw8250_handle_irq(struct uart_port *p)
 	struct dw8250_data *d = to_dw8250_data(p->private_data);
 	unsigned int iir = p->serial_in(p, UART_IIR);
 	bool rx_timeout = (iir & 0x3f) == UART_IIR_RX_TIMEOUT;
-	unsigned int quirks = d->pdata->quirks;
+	unsigned int quirks = 0;
 	unsigned int status;
 	unsigned long flags;
 
+	if (d->pdata)
+		quirks = d->pdata->quirks;
+
 	/*
 	 * There are ways to get Designware-based UARTs into a state where
 	 * they are asserting UART_IIR_RX_TIMEOUT but there is no actual
-- 
2.35.1

Re: [PATCH 1/2] Revert "serial: 8250: dw: Move the USR register to pdata"

[Greg KH]

On Tue, Jun 28, 2022 at 07:02:31PM -0500, Jeremy Linton wrote:
> pdata is only setup by DT machines, leaving ACPI machines
> with null pdata. Since I don't know the exact mapping of
> ACPI ID's to dw 8250 variations I can't add pdata to them
> without possibly breaking something. As such the simplest
> fix here is probably just to revert this commit.
> 
> This reverts commit ffd381445eac2aa624e49bab5a811451e8351008.
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>

Didn't checkpatch complain that you need a blank line before your
signed-off-by line?

thanks,

greg k-h

Re: [PATCH 0/2] BUG, more dw8259 ACPI crashes

[Miquel Raynal]

Hi Jeremy,

jeremy.linton@arm.com wrote on Tue, 28 Jun 2022 19:02:30 -0500:

> 5.19rc4 crashes with:
> 
> [    4.441703] Internal error: Oops: 96000004 [#1] SMP
> [    4.446601] Modules linked in:
> [    4.449668] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.19.0-0.rc4.33.fc37.aarch64 #1
> [    4.457532] Hardware name: Marvell                         Armada 7k/8k Family Board      /Armada 7k/8k Family Board      , BIOS EDK II Jun  4 2019
> [    4.470800] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [    4.477791] pc : dw8250_handle_irq+0x58/0x190
> [    4.482170] lr : dw8250_handle_irq+0x38/0x190
> [    4.486544] sp : ffff800008003e40
> [    4.489869] x29: ffff800008003e40 x28: ffffbe13be183980 x27: 00000000b46bd3d0
> [    4.497036] x26: ffffbe13bdbc3008 x25: ffff6f1447910f80 x24: ffff6f1447910f94
> [    4.504204] x23: 0000000000000012 x22: ffff6f144218fc80 x21: 0000000000000000
> [    4.511372] x20: 0000000000000007 x19: ffffbe13be7628d0 x18: ffff80000918b528
> [    4.518540] x17: ffffb103c0343000 x16: ffff800008004000 x15: 0000000000004000
> [    4.525706] x14: 0000000000000000 x13: 0000000000000030 x12: 0101010101010101
> [    4.532872] x11: ffffbe13be16f6d8 x10: 0000000000001d90 x9 : ffffbe13bc6b4f0c
> [    4.540040] x8 : ffffbe13be185770 x7 : 0000000000000000 x6 : 00000000296b5f4a
> [    4.547206] x5 : 00ffffffffffffff x4 : 0000000000010002 x3 : ffffbe13be7628d0
> [    4.554373] x2 : 0000000000000007 x1 : 0000000000000000 x0 : 0000000000000000
> [    4.561539] Call trace:
> [    4.563994]  dw8250_handle_irq+0x58/0x190
> [    4.568020]  serial8250_interrupt+0x68/0x140
> [    4.572307]  __handle_irq_event_percpu+0x68/0x230
> [    4.577034]  handle_irq_event+0x58/0x120
> [    4.580973]  handle_fasteoi_irq+0xcc/0x1c4
> [    4.585086]  generic_handle_domain_irq+0x38/0x50
> [    4.589723]  gic_handle_irq+0x50/0xe0
> [    4.593399]  call_on_irq_stack+0x2c/0x38
> [    4.597338]  do_interrupt_handler+0xdc/0xe0
> [    4.601539]  el1_interrupt+0x34/0x70
> [    4.605131]  el1h_64_irq_handler+0x18/0x24
> [    4.609244]  el1h_64_irq+0x68/0x6c
> [    4.612658]  arch_cpu_idle+0x18/0x2c
> [    4.616246]  default_idle_call+0x34/0x18c
> [    4.620273]  cpuidle_idle_call+0x154/0x1a0
> [    4.624388]  do_idle+0xa4/0xf4
> [    4.627455]  cpu_startup_entry+0x34/0x3c
> [    4.631394]  kernel_init+0x0/0x150
> [    4.634808]  arch_post_acpi_subsys_init+0x0/0x30
> [    4.639445]  start_kernel+0x474/0x490
> [    4.643121]  __primary_switched+0xc0/0xc8
> [    4.647148] Code: 7100305f 1a9f17f5 f100003f 7a400aa4 (b9400817) 
> [    4.653267] ---[ end trace 0000000000000000 ]---
> 
> On an ACPI enabled Mcbin. This is again the result
> of the pdata information not being available on an
> ACPI machine. But since there are multiple commits
> I've broken this into to fixes. The first
> simply reverts the USR register change because it
> should be functionality transparent and I don't
> have a good way to tie ACPI ids to the hardware
> and the second fixes the later patch which uses it
> in the interrupt handler.

Sorry for the breakage, the move of the USR register to pdata is not
really necessary anyway, it came up rather late in the review process,
and the other fix lgtm, so:

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

> 
> Jeremy Linton (2):
>   Revert "serial: 8250: dw: Move the USR register to pdata"
>   serial: 8250: dw: Fix NULL pointer dereference
> 
>  drivers/tty/serial/8250/8250_dw.c    | 14 +++++++-------
>  drivers/tty/serial/8250/8250_dwlib.h |  2 +-
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 


Thanks,
Miquèl

Re: [PATCH 2/2] serial: 8250: dw: Fix NULL pointer dereference

[Ilpo Järvinen]

Hi,

Does cd16044d7c38 ("serial: 8250: dw: enable using pdata with ACPI") work 
for you? (Currently in tty-linus branch).

-- 
 i.


On Tue, 28 Jun 2022, Jeremy Linton wrote:

> Similarly to e6a08c6949cb, ACPI machines don't have
> a pdata setup, leading to a null pointer dereference
> when dw8250_handle_irq runs.
> 
> Fixes: aa63d786cea2 ("serial: 8250: dw: Add support for DMA flow controlling devices")
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  drivers/tty/serial/8250/8250_dw.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index 53e488beb8c2..06f80090cbb9 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -237,10 +237,13 @@ static int dw8250_handle_irq(struct uart_port *p)
>  	struct dw8250_data *d = to_dw8250_data(p->private_data);
>  	unsigned int iir = p->serial_in(p, UART_IIR);
>  	bool rx_timeout = (iir & 0x3f) == UART_IIR_RX_TIMEOUT;
> -	unsigned int quirks = d->pdata->quirks;
> +	unsigned int quirks = 0;
>  	unsigned int status;
>  	unsigned long flags;
>  
> +	if (d->pdata)
> +		quirks = d->pdata->quirks;
> +
>  	/*
>  	 * There are ways to get Designware-based UARTs into a state where
>  	 * they are asserting UART_IIR_RX_TIMEOUT but there is no actual
> 

Re: [PATCH 0/2] BUG, more dw8259 ACPI crashes

[Jeremy Linton]

Hi,

On 6/28/22 19:02, Jeremy Linton wrote:
> 5.19rc4 crashes with:
> 
> [    4.441703] Internal error: Oops: 96000004 [#1] SMP
> [    4.446601] Modules linked in:
> [    4.449668] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.19.0-0.rc4.33.fc37.aarch64 #1
> [    4.457532] Hardware name: Marvell                         Armada 7k/8k Family Board      /Armada 7k/8k Family Board      , BIOS EDK II Jun  4 2019
> [    4.470800] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [    4.477791] pc : dw8250_handle_irq+0x58/0x190
> [    4.482170] lr : dw8250_handle_irq+0x38/0x190
> [    4.486544] sp : ffff800008003e40
> [    4.489869] x29: ffff800008003e40 x28: ffffbe13be183980 x27: 00000000b46bd3d0
> [    4.497036] x26: ffffbe13bdbc3008 x25: ffff6f1447910f80 x24: ffff6f1447910f94
> [    4.504204] x23: 0000000000000012 x22: ffff6f144218fc80 x21: 0000000000000000
> [    4.511372] x20: 0000000000000007 x19: ffffbe13be7628d0 x18: ffff80000918b528
> [    4.518540] x17: ffffb103c0343000 x16: ffff800008004000 x15: 0000000000004000
> [    4.525706] x14: 0000000000000000 x13: 0000000000000030 x12: 0101010101010101
> [    4.532872] x11: ffffbe13be16f6d8 x10: 0000000000001d90 x9 : ffffbe13bc6b4f0c
> [    4.540040] x8 : ffffbe13be185770 x7 : 0000000000000000 x6 : 00000000296b5f4a
> [    4.547206] x5 : 00ffffffffffffff x4 : 0000000000010002 x3 : ffffbe13be7628d0
> [    4.554373] x2 : 0000000000000007 x1 : 0000000000000000 x0 : 0000000000000000
> [    4.561539] Call trace:
> [    4.563994]  dw8250_handle_irq+0x58/0x190
> [    4.568020]  serial8250_interrupt+0x68/0x140
> [    4.572307]  __handle_irq_event_percpu+0x68/0x230
> [    4.577034]  handle_irq_event+0x58/0x120
> [    4.580973]  handle_fasteoi_irq+0xcc/0x1c4
> [    4.585086]  generic_handle_domain_irq+0x38/0x50
> [    4.589723]  gic_handle_irq+0x50/0xe0
> [    4.593399]  call_on_irq_stack+0x2c/0x38
> [    4.597338]  do_interrupt_handler+0xdc/0xe0
> [    4.601539]  el1_interrupt+0x34/0x70
> [    4.605131]  el1h_64_irq_handler+0x18/0x24
> [    4.609244]  el1h_64_irq+0x68/0x6c
> [    4.612658]  arch_cpu_idle+0x18/0x2c
> [    4.616246]  default_idle_call+0x34/0x18c
> [    4.620273]  cpuidle_idle_call+0x154/0x1a0
> [    4.624388]  do_idle+0xa4/0xf4
> [    4.627455]  cpu_startup_entry+0x34/0x3c
> [    4.631394]  kernel_init+0x0/0x150
> [    4.634808]  arch_post_acpi_subsys_init+0x0/0x30
> [    4.639445]  start_kernel+0x474/0x490
> [    4.643121]  __primary_switched+0xc0/0xc8
> [    4.647148] Code: 7100305f 1a9f17f5 f100003f 7a400aa4 (b9400817)
> [    4.653267] ---[ end trace 0000000000000000 ]---
> 
> On an ACPI enabled Mcbin. This is again the result
> of the pdata information not being available on an
> ACPI machine. But since there are multiple commits
> I've broken this into to fixes. The first
> simply reverts the USR register change because it
> should be functionality transparent and I don't
> have a good way to tie ACPI ids to the hardware
> and the second fixes the later patch which uses it
> in the interrupt handler.

Hi,

I missed cd16044d7c38 ("serial: 8250: dw: enable using pdata with ACPI") 
in tty-linus, which fixes this. So please ignore this set, sorry about 
the noise.


Thanks,

Re: [PATCH 1/2] Revert "serial: 8250: dw: Move the USR register to pdata"

[Jeremy Linton]

Hi,

On 6/29/22 01:08, Greg KH wrote:
> On Tue, Jun 28, 2022 at 07:02:31PM -0500, Jeremy Linton wrote:
>> pdata is only setup by DT machines, leaving ACPI machines
>> with null pdata. Since I don't know the exact mapping of
>> ACPI ID's to dw 8250 variations I can't add pdata to them
>> without possibly breaking something. As such the simplest
>> fix here is probably just to revert this commit.
>>
>> This reverts commit ffd381445eac2aa624e49bab5a811451e8351008.
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> 
> Didn't checkpatch complain that you need a blank line before your
> signed-off-by line?

No, it, and I apparently thought that the revert was part of the tagging.

Lets kill this set anyway, as the patch you already have to add pdata to 
ACPI in tty-linus solves all these problems. I guess I missed that 
posting, so sorry about the noise.

Thanks,