| Message ID | 20230307155543.31021-1-srinivas.kandagatla@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] tty: serial: qcom-geni-serial: check correct dma address before unprep | expand |
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],
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 --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index d69592e5e2ec..5972b5c317d3 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -596,7 +596,7 @@ static void qcom_geni_serial_stop_tx_dma(struct uart_port *uport) if (!qcom_geni_serial_main_active(uport)) return; - if (port->rx_dma_addr) { + if (port->tx_dma_addr) { geni_se_tx_dma_unprep(&port->se, port->tx_dma_addr, port->tx_remaining); port->tx_dma_addr = 0;
looks like there was a typo while checking validatity of tx_dma_addr, the code was checking rx instead of tx. This can potentially lead to memory leak, this patch fixes the typo. Fixes: 2aaa43c70778 ("tty: serial: qcom-geni-serial: add support for serial engine DMA") Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> --- drivers/tty/serial/qcom_geni_serial.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)