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 for Android: free password hash cracker in your pocket

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

Message-ID:
 <SA1PR12MB81200F594C9B842C563F3DFE950EA@SA1PR12MB8120.namprd12.prod.outlook.com>
Date: Wed, 10 Sep 2025 12:30:39 +0000
From: "Verma, Devendra" <Devendra.Verma@....com>
To: Bjorn Helgaas <helgaas@...nel.org>
CC: "bhelgaas@...gle.com" <bhelgaas@...gle.com>, "mani@...nel.org"
	<mani@...nel.org>, "vkoul@...nel.org" <vkoul@...nel.org>,
	"dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "Simek,
 Michal" <michal.simek@....com>
Subject: RE: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode

[AMD Official Use Only - AMD Internal Distribution Only]

Thank you Bjorn for reviewing the patch.
Please check my response inline.

Regards,
Devendra

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@...nel.org>
> Sent: Saturday, September 6, 2025 00:36
> To: Verma, Devendra <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; Simek, Michal <michal.simek@....com>
> Subject: Re: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> 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

In case of AMD, it is not an address that is accessible from host via PCI, it is the device
side DDR offset of physical address which is not known to host,that is why the VSEC capability
is used to let know host of the DDR offset to correctly programming the LLP of DMA controller.
Without programming the LLP controller will not know from where to start reading the LL for
DMA processing. DMA controller requires the physical address of LL present on its side of DDR.