| 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
| ||
|
Message-ID: <20230328180338.bpxvjwx4tn4ter3f@mobilestation>
Date: Tue, 28 Mar 2023 21:03:38 +0300
From: Serge Semin <fancer.lancer@...il.com>
To: Joy Chakraborty <joychakr@...gle.com>
Cc: Mark Brown <broonie@...nel.org>, linux-spi@...r.kernel.org,
linux-kernel@...r.kernel.org, manugautam@...gle.com,
rohitner@...gle.com
Subject: Re: [PATCH v4 2/2] spi: dw: Add dma controller capability checks
On Mon, Mar 27, 2023 at 04:34:05AM +0000, Joy Chakraborty wrote:
> Check capabilities of DMA controller during init to make sure it is
> capable of handling MEM2DEV for tx channel, DEV2MEM for rx channel
> and store addr_width capabilities to check per transfer to make sure the
> bits/word requirement can be met for that transfer.
>
> Signed-off-by: Joy Chakraborty <joychakr@...gle.com>
> ---
> drivers/spi/spi-dw-dma.c | 54 ++++++++++++++++++++++++++++++++--------
> drivers/spi/spi-dw.h | 1 +
> 2 files changed, 44 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/spi/spi-dw-dma.c b/drivers/spi/spi-dw-dma.c
> index b3a88bb75907..f47483ec369f 100644
> --- a/drivers/spi/spi-dw-dma.c
> +++ b/drivers/spi/spi-dw-dma.c
> @@ -23,6 +23,8 @@
> #define DW_SPI_TX_BUSY 1
> #define DW_SPI_TX_BURST_LEVEL 16
>
> +static enum dma_slave_buswidth dw_spi_dma_convert_width(u8 n_bytes);
> +
> static bool dw_spi_dma_chan_filter(struct dma_chan *chan, void *param)
> {
> struct dw_dma_slave *s = param;
> @@ -72,12 +74,15 @@ static void dw_spi_dma_maxburst_init(struct dw_spi *dws)
> dw_writel(dws, DW_SPI_DMATDLR, dws->txburst);
> }
>
> -static void dw_spi_dma_sg_burst_init(struct dw_spi *dws)
> +static int dw_spi_dma_caps_init(struct dw_spi *dws)
> {
> + int ret;
> struct dma_slave_caps tx = {0}, rx = {0};
1. Preserve the reverse xmas tree order please (driver local convention).
2. The zero-initialization can be dropped since the function will halt
on further procedures if any of the dma_get_slave_caps() calls fail.
Meanwhile if all of them are successfully executed the capability
structures will be sanely initialized.
>
> - dma_get_slave_caps(dws->txchan, &tx);
> - dma_get_slave_caps(dws->rxchan, &rx);
> + ret = dma_get_slave_caps(dws->txchan, &tx);
if (ret)
return ret;
< newline
> + ret |= dma_get_slave_caps(dws->rxchan, &rx);
> + if (ret)
> + return ret;
No OR-ing the errnos please. They aren't bitfields.
>
> if (tx.max_sg_burst > 0 && rx.max_sg_burst > 0)
> dws->dma_sg_burst = min(tx.max_sg_burst, rx.max_sg_burst);
> @@ -87,6 +92,18 @@ static void dw_spi_dma_sg_burst_init(struct dw_spi *dws)
> dws->dma_sg_burst = rx.max_sg_burst;
> else
> dws->dma_sg_burst = 0;
> +
> + /*
> + * Assuming both channels belong to the same DMA controller hence the
> + * address width capabilities most likely would be the same.
> + */
> + dws->dma_addr_widths = tx.dst_addr_widths & rx.src_addr_widths;
> +
> + if (!(tx.directions & BIT(DMA_MEM_TO_DEV) &&
> + rx.directions & BIT(DMA_DEV_TO_MEM)))
> + return -ENXIO;
Please move this to be right after the capabilities are retrieved.
There is no point in the SG-burst and addr-width data initializations
if a DMA-controller isn't suitable for the Tx/Rx DMAs.
-Serge(y)
> +
> + return 0;
> }
>
> static int dw_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
> @@ -95,6 +112,7 @@ static int dw_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
> struct dw_dma_slave dma_rx = { .src_id = 0 }, *rx = &dma_rx;
> struct pci_dev *dma_dev;
> dma_cap_mask_t mask;
> + int ret = -EBUSY;
>
> /*
> * Get pci device for DMA controller, currently it could only
> @@ -124,20 +142,24 @@ static int dw_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
>
> init_completion(&dws->dma_completion);
>
> - dw_spi_dma_maxburst_init(dws);
> + ret = dw_spi_dma_caps_init(dws);
> + if (ret)
> + goto free_txchan;
>
> - dw_spi_dma_sg_burst_init(dws);
> + dw_spi_dma_maxburst_init(dws);
>
> pci_dev_put(dma_dev);
>
> return 0;
> -
> +free_txchan:
> + dma_release_channel(dws->txchan);
> + dws->txchan = NULL;
> free_rxchan:
> dma_release_channel(dws->rxchan);
> dws->rxchan = NULL;
> err_exit:
> pci_dev_put(dma_dev);
> - return -EBUSY;
> + return ret;
> }
>
> static int dw_spi_dma_init_generic(struct device *dev, struct dw_spi *dws)
> @@ -163,12 +185,16 @@ static int dw_spi_dma_init_generic(struct device *dev, struct dw_spi *dws)
>
> init_completion(&dws->dma_completion);
>
> - dw_spi_dma_maxburst_init(dws);
> + ret = dw_spi_dma_caps_init(dws);
> + if (ret)
> + goto free_txchan;
>
> - dw_spi_dma_sg_burst_init(dws);
> + dw_spi_dma_maxburst_init(dws);
>
> return 0;
> -
> +free_txchan:
> + dma_release_channel(dws->txchan);
> + dws->txchan = NULL;
> free_rxchan:
> dma_release_channel(dws->rxchan);
> dws->rxchan = NULL;
> @@ -202,8 +228,14 @@ static bool dw_spi_can_dma(struct spi_controller *master,
> struct spi_device *spi, struct spi_transfer *xfer)
> {
> struct dw_spi *dws = spi_controller_get_devdata(master);
> + enum dma_slave_buswidth dma_bus_width;
> +
> + if (xfer->len <= dws->fifo_len)
> + return false;
> +
> + dma_bus_width = dw_spi_dma_convert_width(dws->n_bytes);
>
> - return xfer->len > dws->fifo_len;
> + return dws->dma_addr_widths & BIT(dma_bus_width);
> }
>
> static enum dma_slave_buswidth dw_spi_dma_convert_width(u8 n_bytes)
> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> index 9e8eb2b52d5c..3962e6dcf880 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
> @@ -190,6 +190,7 @@ struct dw_spi {
> struct dma_chan *rxchan;
> u32 rxburst;
> u32 dma_sg_burst;
> + u32 dma_addr_widths;
> unsigned long dma_chan_busy;
> dma_addr_t dma_addr; /* phy address of the Data register */
> const struct dw_spi_dma_ops *dma_ops;
> --
> 2.40.0.348.gf938b09366-goog
>
Powered by blists - more mailing lists