On Fri, Feb 23, 2024 at 12:38:18PM +0100, Niklas Schnelle wrote: > > Although I doubt that generating long TLP from byte writes is > > really necessary. > > I might have gotten confused but I think these are not byte writes. > Remember that the count is in terms of the number of bits sized > quantities to copy so "count == 1" is 4/8 bytes here. Right. There seem to be two callers of this API in the kernel, one is calling with a constant size and wants a large TLP Another seems to want memcpy_to_io with a guarenteed 32/64 bit store. > > IIRC you were merging at most 4 writes. > > So better to do a single 32bit write instead. > > (Unless you have misaligned source data - unlikely.) > > > > While write-combining to generate long TLP is probably mostly > > safe for PCIe targets, there are some that will only handle > > TLP for single 32bit data items. > > Which might be why the code is explicitly requesting 4 byte copies. > > So it may be entirely wrong to write-combine anything except > > the generic memcpy_toio(). > > On anything other than s390x this should only do write-combine if the > memory mapping allows it, no? Meaning a driver that can't handle larger > TLPs really shouldn't use ioremap_wc() then. Right. > On s390x one could argue that our version of __iowriteXX_copy() is > strictly speaking not correct in that zpci_memcpy_toio() doesn't really > use XX bit writes which is why for us memcpy_toio() was actually a > better fit indeed. On the other hand doing 32 bit PCI stores (an s390x > thing) can't combine multiple stores into a single TLP which these > functions are used for and which has much more use cases than forcing a > copy loop with 32/64 bit sized writes which would also be a lot slower > on s390x than an aligned zpci_memcpy_toio(). mlx5 will definitely not work right if __iowrite64_copy() results in anything smaller than 32/64 bit PCIe TLPs. Jason
