On Wed, Apr 15, 2020 at 11:17 AM Gustavo Pimentel <Gustavo.Pimentel@xxxxxxxxxxxx> wrote: > > Hi Alan, > > > > I like your approach, it separates the PCIe glue logic from the eDMA > > > itself. > > > I would suggest that pcitest would have multiple options that could be > > > triggered, for instance: > > > 1 - Execute Endpoint DMA (read/write) remotely with Linked List feature > > > (from the Root Complex side) > > > 2 - Execute Endpoint DMA (read/write) remotely without Linked List > > > feature (from the Root Complex side) > > > 3 - Execute Endpoint DMA (read/write) locally with Linked List feature > > > 4 - Execute Endpoint DMA (read/write) locally without Linked List > > > feature > > > > > > > I have all of the above four use cases in mind as well. At the moment, > > only #4 is possible with pcitest. > > > > Use case #3 would need a new command line option for pcitest such as -L > > to let its user specify linked list operationwhen used with dma in > > conjunction with the existing -D option. > > > > Use cases #1 and #2 would need another new command line option such as -R > > to specify remotely initiated dma operation in conjunction with -D option. > > > > New code in pci-epf-test and pci_endpoint_test drivers would be needed > > to support use cases #1, #2, and #3. However, use case #4 should be > > possible without modification to pci-epf-test or pci_endpoint_test as long > > as the dmaengine channels become available on the endpoint side. > > I would suggest something like this: > > -L option, local DMA triggering > -R option, remote DMA triggering > -W <n> option, to select the DMA write channel n => (0 ... 7) to be > used > -R <n> option, to select the DMA read channel n => (0 ... 7) to be > used > -K option, to use or not the linked list feature (K presence enables > the LL use) > -T <n> option, to select which type of DMA transfer to be used => (n = 0 > - scatter-gather mode, 1 - cyclic mode) > -N <n> option, to define the number of cyclic transfers to perform in > total > -C <n> option, to define the size of each chunk to be used > -t <time> option, to define a timeout for the DMA operation That looks like a more complete set of command line options. > > Also, the use of this options (especially when using the remote DMA > option) should be checked through the pci_epc_get_features(), which means > probably we need to pass the EP features capabilities to the > pci_endpoint_test Driver, perhaps using some sets of registers on located > on BAR0 or other. That is a great point. There may be changes required below pci-epf-test in the endpoint framework stack. > > > At the moment, pci-epf-test grabs the first available dma channel on the > > endpoint side and uses it for either read, write, or copy operation. it is not > > possible at the moment to specify which dma channel to use on the pcitest > > command line. This may be possible by modifying the command line option > > -D to also specify the name of one or more dma channels. > > I'm assuming that behavior is due to your code, right? I'm not seen that > behavior on the Kernel tree. > Check my previous suggestion, it should be something similar to what is > been done while you select the MSI/MSI-X interrupt to trigger. I believe this behavior exists in the kernel tree because the call to dma_request_chan_by_mask() always specifies channel zero. The user of pcitest has no way of specifying which one of the available dma channels to use. > > > Also, pci-epf-test grabs the dma channel at bind time and holds on to it > > until unloaded. This denies the use of the dma channel to others on the > > endpoint side. However, it seems possible to grab and release the dma > > channel only for the duration of each read, write, or copy test. These are > > improvements that can come over time. It is great that pci-epf-test was > > recently updated to include support for dma operations which makes such > > improvements possible. > > Check my previous suggestion. I think by having a timeout for the DMA > operation we can provide a way to release the dma channel. > Or we could provide some kind of heart beat, once again through some > register in a BAR. I believe this behavior exists in the kernel tree because the call to dma_request_chan_by_mask() happens during the execution of pci_epf_test_bind() and the call to dma_release_channel() happens during the execution of pci_epf_test_unbind(). As long as pci-epf-test is bound, I cannot use another program such as dmatest from the endpoint-side command prompt to exercise the same channel. What I was suggesting is perhaps pci-epf-test can be modified to acquire and release the channel on each call to pci_epf_test_read(), ...write(), or ...copy() when the pcitest user specifies -D option. > > > > Relative to the implementation of the options 3 and 4, I wonder if the > > > linked list memory space and size could be set through the DT or by the > > > configfs available on the pci-epf-test driver. > > > > > > > Although these options could be set through DT or by configfs, another > > option is to enable the user of pcitest to specify such parameters on > > the command line when invoking each test from the host side. > > That would be an easy and quick solution, but so far as I know there is a > movement in the Kernel to avoid any configuration through module > parameters. So I'm afraid that you have to choose by DT or configfs > strategy. Kishon can help you on this matter, by telling you what he > prefers. Thanks for that reminder. I will check before getting too invested in a specific implementation. Just to clarify, I was suggesting giving the user of pcitest a way to specify which one of the available dma channel to use on each invocation of pcitest, not what dma channels are available on the endpoint side. I assumed the strategy for which dma channels do become present and available on endpoint would be by DT or configfs. > > Regards, > Gustavo > >