linux-kernel - Re: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250905190621.GA1306997@bhelgaas>
Date: Fri, 5 Sep 2025 14:06:21 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Devendra K Verma <devendra.verma@....com>
Cc: bhelgaas@...gle.com, mani@...nel.org, vkoul@...nel.org,
	dmaengine@...r.kernel.org, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org, michal.simek@....com
Subject: Re: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode

On Fri, Sep 05, 2025 at 03:46:59PM +0530, Devendra K Verma wrote:
> AMD MDB IP supports Linked List (LL) mode as well as non-LL mode.
> The current code does not have the mechanisms to enable the
> DMA transactions using the non-LL mode. The following two cases
> are added with this patch:
> - When a valid physical base address is not configured via the
>   Xilinx VSEC capability then the IP can still be used in non-LL
>   mode. The default mode for all the DMA transactions and for all
>   the DMA channels then is non-LL mode.
> - When a valid physical base address is configured but the client
>   wants to use the non-LL mode for DMA transactions then also the
>   flexibility is provided via the peripheral_config struct member of
>   dma_slave_config. In this case the channels can be individually
>   configured in non-LL mode. This use case is desirable for single
>   DMA transfer of a chunk, this saves the effort of preparing the
>   Link List.

> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -223,8 +223,28 @@ static int dw_edma_device_config(struct dma_chan *dchan,
>  				 struct dma_slave_config *config)
>  {
>  	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> +	int nollp = 0;
> +
> +	if (WARN_ON(config->peripheral_config &&
> +		    config->peripheral_size != sizeof(int)))
> +		return -EINVAL;
>  
>  	memcpy(&chan->config, config, sizeof(*config));
> +
> +	/*
> +	 * When there is no valid LLP base address available
> +	 * then the default DMA ops will use the non-LL mode.
> +	 * Cases where LL mode is enabled and client wants
> +	 * to use the non-LL mode then also client can do
> +	 * so via the providing the peripheral_config param.

s/via the/via/

> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> @@ -224,6 +224,15 @@ static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
>  	pdata->phys_addr = off;
>  }
>  
> +static u64 dw_edma_get_phys_addr(struct pci_dev *pdev,
> +				 struct dw_edma_pcie_data *pdata,
> +				 enum pci_barno bar)
> +{
> +	if (pdev->vendor == PCI_VENDOR_ID_XILINX)
> +		return pdata->phys_addr;
> +	return pci_bus_address(pdev, bar);

This doesn't seem right.  pci_bus_address() returns pci_bus_addr_t, so
pdata->phys_addr should also be a pci_bus_addr_t, and the function
should return pci_bus_addr_t.

A pci_bus_addr_t is not a "phys_addr"; it is an address that is valid
on the PCI side of a PCI host bridge, which may be different than the
CPU physical address on the CPU side of the bridge because of things
like IOMMUs.

Seems like the struct dw_edma_region.paddr should be renamed to
something like "bus_addr" and made into a pci_bus_addr_t.

Bjorn