diff mbox series

[2/2] tty: serial: qcom-geni-serial: check for valid size before starting dma

Message ID 20230307155543.31021-2-srinivas.kandagatla@linaro.org
State New
Headers show
Series [1/2] tty: serial: qcom-geni-serial: check correct dma address before unprep | expand

Commit Message

Srinivas Kandagatla March 7, 2023, 3:55 p.m. UTC 
Check if there are valid length of bytes to transfer before starting dma.

without this check we can see below kernel warning when we try to map a zero size buffers.

------------[ cut here ]------------
WARNING: CPU: 0 PID: 0 at drivers/iommu/dma-iommu.c:1046 iommu_dma_unmap_page+0xe0/0xfc
Modules linked in:
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W          6.3.0-rc1-dirty #347
Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
pstate: 804000c5 (Nzcv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : iommu_dma_unmap_page+0xe0/0xfc
lr : iommu_dma_unmap_page+0x38/0xfc
...
Call trace:
 iommu_dma_unmap_page+0xe0/0xfc
 dma_unmap_page_attrs+0x30/0x1ec
 geni_se_tx_dma_unprep+0x58/0x80
 qcom_geni_serial_isr+0x350/0x750
 __handle_irq_event_percpu+0x58/0x148
 handle_irq_event_percpu+0x18/0x4c
 handle_irq_event+0x48/0x88
 handle_fasteoi_irq+0xb0/0x130
 generic_handle_domain_irq+0x2c/0x44
 gic_handle_irq+0xd4/0x140
 call_on_irq_stack+0x24/0x4c
 do_interrupt_handler+0x80/0x84
 el1_interrupt+0x38/0x6c
 el1h_64_irq_handler+0x18/0x24
 el1h_64_irq+0x64/0x68
 cpuidle_enter_state+0x1e4/0x310
 cpuidle_enter+0x3c/0x54
 call_cpuidle+0x1c/0x40
 do_idle+0x204/0x260
 cpu_startup_entry+0x28/0x2c
 kernel_init+0x0/0x12c
 arch_post_acpi_subsys_init+0x0/0x8
 start_kernel+0x3cc/0x74c
 __primary_switched+0xbc/0xc4

Fixes: 2aaa43c70778 ("tty: serial: qcom-geni-serial: add support for serial engine DMA")
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/tty/serial/qcom_geni_serial.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Konrad Dybcio March 7, 2023, 4:20 p.m. UTC | #1 
On 7.03.2023 16:55, Srinivas Kandagatla wrote:
> Check if there are valid length of bytes to transfer before starting dma.
> 
> without this check we can see below kernel warning when we try to map a zero size buffers.
> 
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 0 at drivers/iommu/dma-iommu.c:1046 iommu_dma_unmap_page+0xe0/0xfc
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W          6.3.0-rc1-dirty #347
> Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
> pstate: 804000c5 (Nzcv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : iommu_dma_unmap_page+0xe0/0xfc
> lr : iommu_dma_unmap_page+0x38/0xfc
> ...
> Call trace:
>  iommu_dma_unmap_page+0xe0/0xfc
>  dma_unmap_page_attrs+0x30/0x1ec
>  geni_se_tx_dma_unprep+0x58/0x80
>  qcom_geni_serial_isr+0x350/0x750
>  __handle_irq_event_percpu+0x58/0x148
>  handle_irq_event_percpu+0x18/0x4c
>  handle_irq_event+0x48/0x88
>  handle_fasteoi_irq+0xb0/0x130
>  generic_handle_domain_irq+0x2c/0x44
>  gic_handle_irq+0xd4/0x140
>  call_on_irq_stack+0x24/0x4c
>  do_interrupt_handler+0x80/0x84
>  el1_interrupt+0x38/0x6c
>  el1h_64_irq_handler+0x18/0x24
>  el1h_64_irq+0x64/0x68
>  cpuidle_enter_state+0x1e4/0x310
>  cpuidle_enter+0x3c/0x54
>  call_cpuidle+0x1c/0x40
>  do_idle+0x204/0x260
>  cpu_startup_entry+0x28/0x2c
>  kernel_init+0x0/0x12c
>  arch_post_acpi_subsys_init+0x0/0x8
>  start_kernel+0x3cc/0x74c
>  __primary_switched+0xbc/0xc4
> 
> Fixes: 2aaa43c70778 ("tty: serial: qcom-geni-serial: add support for serial engine DMA")
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
S-o-b but no C-d-b? Weird..

> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
Anyway, the change is good!

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  drivers/tty/serial/qcom_geni_serial.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 5972b5c317d3..bb63a00f4c07 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -637,6 +637,9 @@ static void qcom_geni_serial_start_tx_dma(struct uart_port *uport)
>  
>  	xmit_size = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
>  
> +	if (!xmit_size)
> +		return;
> +
>  	qcom_geni_serial_setup_tx(uport, xmit_size);
>  
>  	ret = geni_se_tx_dma_prep(&port->se, &xmit->buf[xmit->tail],
Bartosz Golaszewski March 7, 2023, 4:43 p.m. UTC | #2 
On Tue, 7 Mar 2023 at 17:20, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
>
>
> On 7.03.2023 16:55, Srinivas Kandagatla wrote:
> > Check if there are valid length of bytes to transfer before starting dma.
> >
> > without this check we can see below kernel warning when we try to map a zero size buffers.
> >
> > ------------[ cut here ]------------
> > WARNING: CPU: 0 PID: 0 at drivers/iommu/dma-iommu.c:1046 iommu_dma_unmap_page+0xe0/0xfc
> > Modules linked in:
> > CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W          6.3.0-rc1-dirty #347
> > Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
> > pstate: 804000c5 (Nzcv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > pc : iommu_dma_unmap_page+0xe0/0xfc
> > lr : iommu_dma_unmap_page+0x38/0xfc
> > ...
> > Call trace:
> >  iommu_dma_unmap_page+0xe0/0xfc
> >  dma_unmap_page_attrs+0x30/0x1ec
> >  geni_se_tx_dma_unprep+0x58/0x80
> >  qcom_geni_serial_isr+0x350/0x750
> >  __handle_irq_event_percpu+0x58/0x148
> >  handle_irq_event_percpu+0x18/0x4c
> >  handle_irq_event+0x48/0x88
> >  handle_fasteoi_irq+0xb0/0x130
> >  generic_handle_domain_irq+0x2c/0x44
> >  gic_handle_irq+0xd4/0x140
> >  call_on_irq_stack+0x24/0x4c
> >  do_interrupt_handler+0x80/0x84
> >  el1_interrupt+0x38/0x6c
> >  el1h_64_irq_handler+0x18/0x24
> >  el1h_64_irq+0x64/0x68
> >  cpuidle_enter_state+0x1e4/0x310
> >  cpuidle_enter+0x3c/0x54
> >  call_cpuidle+0x1c/0x40
> >  do_idle+0x204/0x260
> >  cpu_startup_entry+0x28/0x2c
> >  kernel_init+0x0/0x12c
> >  arch_post_acpi_subsys_init+0x0/0x8
> >  start_kernel+0x3cc/0x74c
> >  __primary_switched+0xbc/0xc4
> >
> > Fixes: 2aaa43c70778 ("tty: serial: qcom-geni-serial: add support for serial engine DMA")
> > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> S-o-b but no C-d-b? Weird..
>

It was supposed to be Reviewed-by actually.

Bart

> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > ---
> Anyway, the change is good!
>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>
> Konrad
> >  drivers/tty/serial/qcom_geni_serial.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> > index 5972b5c317d3..bb63a00f4c07 100644
> > --- a/drivers/tty/serial/qcom_geni_serial.c
> > +++ b/drivers/tty/serial/qcom_geni_serial.c
> > @@ -637,6 +637,9 @@ static void qcom_geni_serial_start_tx_dma(struct uart_port *uport)
> >
> >       xmit_size = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
> >
> > +     if (!xmit_size)
> > +             return;
> > +
> >       qcom_geni_serial_setup_tx(uport, xmit_size);
> >
> >       ret = geni_se_tx_dma_prep(&port->se, &xmit->buf[xmit->tail],
Srinivas Kandagatla March 7, 2023, 4:48 p.m. UTC | #3 
On 07/03/2023 16:20, Konrad Dybcio wrote:
> 
> 
> On 7.03.2023 16:55, Srinivas Kandagatla wrote:
>> Check if there are valid length of bytes to transfer before starting dma.
>>
>> without this check we can see below kernel warning when we try to map a zero size buffers.
>>
>> ------------[ cut here ]------------
>> WARNING: CPU: 0 PID: 0 at drivers/iommu/dma-iommu.c:1046 iommu_dma_unmap_page+0xe0/0xfc
>> Modules linked in:
>> CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W          6.3.0-rc1-dirty #347
>> Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
>> pstate: 804000c5 (Nzcv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> pc : iommu_dma_unmap_page+0xe0/0xfc
>> lr : iommu_dma_unmap_page+0x38/0xfc
>> ...
>> Call trace:
>>   iommu_dma_unmap_page+0xe0/0xfc
>>   dma_unmap_page_attrs+0x30/0x1ec
>>   geni_se_tx_dma_unprep+0x58/0x80
>>   qcom_geni_serial_isr+0x350/0x750
>>   __handle_irq_event_percpu+0x58/0x148
>>   handle_irq_event_percpu+0x18/0x4c
>>   handle_irq_event+0x48/0x88
>>   handle_fasteoi_irq+0xb0/0x130
>>   generic_handle_domain_irq+0x2c/0x44
>>   gic_handle_irq+0xd4/0x140
>>   call_on_irq_stack+0x24/0x4c
>>   do_interrupt_handler+0x80/0x84
>>   el1_interrupt+0x38/0x6c
>>   el1h_64_irq_handler+0x18/0x24
>>   el1h_64_irq+0x64/0x68
>>   cpuidle_enter_state+0x1e4/0x310
>>   cpuidle_enter+0x3c/0x54
>>   call_cpuidle+0x1c/0x40
>>   do_idle+0x204/0x260
>>   cpu_startup_entry+0x28/0x2c
>>   kernel_init+0x0/0x12c
>>   arch_post_acpi_subsys_init+0x0/0x8
>>   start_kernel+0x3cc/0x74c
>>   __primary_switched+0xbc/0xc4
>>
>> Fixes: 2aaa43c70778 ("tty: serial: qcom-geni-serial: add support for serial engine DMA")
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> S-o-b but no C-d-b? Weird..
> 
Yes, I did mess up this part.
thanks for spotting this, Will fix this in next spin!

--srini
>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>> ---
> Anyway, the change is good!
> 
> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> 
> Konrad
>>   drivers/tty/serial/qcom_geni_serial.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
>> index 5972b5c317d3..bb63a00f4c07 100644
>> --- a/drivers/tty/serial/qcom_geni_serial.c
>> +++ b/drivers/tty/serial/qcom_geni_serial.c
>> @@ -637,6 +637,9 @@ static void qcom_geni_serial_start_tx_dma(struct uart_port *uport)
>>   
>>   	xmit_size = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
>>   
>> +	if (!xmit_size)
>> +		return;
>> +
>>   	qcom_geni_serial_setup_tx(uport, xmit_size);
>>   
>>   	ret = geni_se_tx_dma_prep(&port->se, &xmit->buf[xmit->tail],
diff mbox series

Patch

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 5972b5c317d3..bb63a00f4c07 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -637,6 +637,9 @@  static void qcom_geni_serial_start_tx_dma(struct uart_port *uport)
 
 	xmit_size = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
 
+	if (!xmit_size)
+		return;
+
 	qcom_geni_serial_setup_tx(uport, xmit_size);
 
 	ret = geni_se_tx_dma_prep(&port->se, &xmit->buf[xmit->tail],