fix(c9xx): don't flush dcache when invalidating

[gilbsgilbs]

The data cache invalidation function for c9xx CPUs uses dcache.cipa instruction. According to T-Head extension specification[1] section 3.1.5, this instruction also performs a cache clean along with the invalidation.

On top of being incorrect, this leads to a serious issue on the designware ethernet driver, where stalled cache may get flushed each time we handle a new received packet[2]. As a result, received packet are randomly corrupted with old cached data. This can easily be reproduced by sending an ARP request to the device during a TFTP transfer. The last TFTP block is treated as the ARP reply we just sent, which makes U-Boot hang on the block.

Always use dcache.ipa instruction to invalidate dcache. Replace existing usages of dcache.ipa with our implementation.

Note that this fix is slightly intrusive as it changes the cache invalidation behavior in all drivers. However, I have not noticed any side-effect during my tests.

[1] https://github.com/T-head-Semi/thead-extension-spec/releases/download/2.3.0/xthead-2023-11-10-2.3.0.pdf

[2]

thead-u-boot/drivers/net/designware.c

Line 475 in 918a8c8

invalidate_dcache_range(data_start, data_end);

[RevySR]

Suggest using dcache.cipa instead of dcache.ipa

Can you give me a test case? I need to validate it.

[gilbsgilbs]

Suggest using dcache.cipa instead of dcache.ipa

Do you mean dcache.cipa instead of dcache.cpa? If so, I think that would indeed make sense. Otherwise, I'm not sure I understand what you mean as this is exactly what this PR refrains from doing. dcache.cipa flushes the cache which we don't want to do when just invalidating.

Can you give me a test case? I need to validate it.

Do you mean a programmatic test case? Not sure how to write that. If you just want to reproduce the issue, download any big file over tftp on the Lichee Pi 4A and wait for an ARP request to arrive (or trigger one yourself using arping for instance). When that happens, you'll see that U-Boot hangs on some TFTP block, then timeouts, then the missed block is requested and sent over again. After this patch, the timeout no longer happens, it just downloads the file smoothly.

[RevySR]

Suggest using dcache.cipa instead of dcache.ipa

Do you mean dcache.cipa instead of dcache.cpa? If so, I think that would indeed make sense. Otherwise, I'm not sure I understand what you mean as this is exactly what this PR refrains from doing. dcache.cipa flushes the cache which we don't want to do when just invalidating.

Talked to the original author and he suggested using dcache.cipa instead of dcache.ipa here.

[gilbsgilbs]

Talked to the original author and he suggested using dcache.cipa instead of dcache.ipa here.

Sorry, I'm not sure we're on the same page. Do you expect something from me or do you have something to check with the original author?

To make my point clear: before this PR, invalidate_dcache_range was already using dcache.cipa. I'm arguing that this was wrong and leading to bugs. This PR replaces it with dcache.ipa instead.

For example, when handling an ARP request, U-Boot modifies the packet in-place to build its ARP reply:

thead-u-boot/net/arp.c

Lines 164 to 170 in 918a8c8

	debug_cond(DEBUG_DEV_PKT, "Got ARP REQUEST, return our IP\n");
	eth_hdr_size = net_update_ether(et, et->et_src, PROT_ARP);
	arp->ar_op = htons(ARPOP_REPLY);
	memcpy(&arp->ar_tha, &arp->ar_sha, ARP_HLEN);
	net_copy_ip(&arp->ar_tpa, &arp->ar_spa);
	memcpy(&arp->ar_sha, net_ethaddr, ARP_HLEN);
	net_copy_ip(&arp->ar_spa, &net_ip);

As the TH1520 isn't DMA-coherent, this garbage data in cache may be flushed after the ethernet device has written new packets at this same memory address. That's why the designware ethernet driver invalidates the buffer's dcache before handing over a new packet to the system:

thead-u-boot/drivers/net/designware.c

Line 475 in 918a8c8

invalidate_dcache_range(data_start, data_end);

Therefore, if we clean or flush the cache here (instead of just invalidating), what we are actually flushing is the old garbage data that U-Boot had previously written before the ethernet device wrote the new packet. This is not what we want.

Generally, there is no guarantee that U-Boot won't tamper with DMA buffers, so when a buffer is marked as invalid by some driver, it should not be flushed (like dcache.cipa does) but invalidated (like dcache.ipa does). At least that's my rationale.

[gilbsgilbs]

@RevySR 👋 . Have you been able to reproduce the issue? Do you need some help or details?