On 30 August 2016 at 17:04, Alan Ott alan@softiron.co.uk wrote:
These patches are for the MarvellYukon driver which is currently part of OpenPlatformPkg.
The impetus was to get this driver to work on a SoftIron Overdrive 1000 board using the AMD Opteron-A (Seattle/Styx) SoC. On this platform, in my testing, edk2 allocates DMA buffers with 64-bit addresses. The Marvell Yukon driver as posted did not support 64-bit addresses, and simply truncated any DMA address to 32-bits. After consulting with Ard Biesheuvel and Leif Lindholm on IRC, it seemed the proper fix was to add support for 64-bit DMA. For this I went back to the original source of this driver (FreeBSD), and brought over the appropriate code.
A couple of patches are basic fixes, but the one titled "Don't re-use DMA buffers" changes how handling of DMA buffers works. This patch makes it work closer to how the FreeBSD implementation works and also adds some required DMA function calls. Its commit message is worth a read.
I don't have a Juno board to test this on, so while this does work on my Overdrive 1000, there's a chance I broke something for other users.
Changes from v1:
- Update to the version of the driver committed in OpenPlatformPkg.
Changes from v2:
- Base on what's currently upstream (Leif Lindholm took patches 1 and 2 from v2, reducing the patch count from 6 to 4).
- Use functions for CopyMem() and SetMem() from EFI Boot Services (gBS->CopyMem(), etc.).
- Add Signoff and Contributed-Under
Changes from v3:
- Change some DEBUG() messages to EFI_D_NET
Changes from v4:
- Move the call to FreeBuffer() in msk_rxeof() to happen regardless of other allocation failures.
- Add patch to free the link object if the link's buffer can't be created.
Changes from v5:
- Use 64-bit DMA in all cases. Do not try to detect it at build time.
- Note, I did not take out the #ifdefs for the 64-bit DMA, so that this can be reversed in the future if it becomes necessary to build for 32-bit DMA only or to detect it. That possibility is uncertain enough at present to preclude removal of 32-bit DMA support entirely.
- Fix linearization bug in 1/5. Previously, applying 1/5 by itself would not build because it was dependent on a variable assignment in 3/5.
Changes from v6:
- Introduce patch #2 to use system memory buffer object for receive queue instead of a DMA buffer.
- Remove the setting of the DMA mapping in patch #1.
Changes from v7:
- Removed duplicate of patch #2 that inadvertently got sent.
Alan Ott (6): Drivers/Net/MarvellYukon: Don't re-use DMA buffers Drivers/Net/MarvellYukon: Use system memory buffer struct for receive queue Drivers/Net/MarvellYukon: Zero allocated memory for DMA receive buffers Drivers/Net/MarvellYukon: Add 64-bit DMA support Drivers/Net/MarvellYukon: Set Dual Address Cycle Attribute Drivers/Net/MarvellYukon: Free link if its DMA buffer can't be allocated
OK, pushed as
972bf6b5263e Drivers/Net/MarvellYukon: Don't re-use DMA buffers 68f2976a59b0 Drivers/Net/MarvellYukon: Use system memory buffer struct for receive queue 98af09fcda4a Drivers/Net/MarvellYukon: Zero allocated memory for DMA receive buffers 94eca77f492b Drivers/Net/MarvellYukon: Add 64-bit DMA support 5125821c7ef5 Drivers/Net/MarvellYukon: Set Dual Address Cycle Attribute 7bc42ad39700 Drivers/Net/MarvellYukon: Free link if its DMA buffer can't be allocated
As discussed, if you have more time to spend on this, there is still room for improvement in terms of: - removing the redundant copy on the RX path - replacing PciIo->AllocateBuffer() with gBS->AllocatePool() [and reduce the max packet size to a more reasonable ~1500)
Thanks a lot for this contribution! Especially the 64-bit PCI support in UEFI is a big deal for AArch64, since X64 usually does not bother to enable 64-bit DMA in UEFI (since RAM always starts at 0x0 on a PC)
Cheers, Ard.