riscv cbo: fix cache underflushing

[br]

Summary

Fix cache underflushing

Reported by: jhb

[jrtc27]

sys/riscv/riscv/cbo.c

63

Invalidating outside the given range is dangerous; any adjacent data risks getting corrupted. Though arm64 seems to do that in its assembly too...

[br]

Indeed. I do find however that this actually fixes my issue with if_xae when tag alignment requirement for TX/RX buffers is downgraded to lower values.

[mhorne]

I think I need some more context to understand this diff.

In D54531#1246777, @br wrote:

Indeed. I do find however that this actually fixes my issue with if_xae when tag alignment requirement for TX/RX buffers is downgraded to lower values.

Is this not a driver bug?

sys/riscv/riscv/cbo.c

83

What is the difference between __align_down and masking to the line size?

[jrtc27]

sys/riscv/riscv/cbo.c

83

None, except that jhb@ is on a question to replace a bunch of vm_offset_t with void * or similar, and __align_down works with pointer types, but masking would require casting to vm_offset_t (or vm_pointer_t aka uintptr_t for CheriBSD).

[jhb]

sys/riscv/riscv/cbo.c

63

That is an old bug FWIW rather than part of br's change here. I agree though that invalidating (which discards dirty data) outside of the range is somewhat dubious. The thing that probably fixes if_xae is fixing the end address. Previously it did not do a flush of a partial line at the end of the range and now it does. I could see why this might matter for a NIC driver if it is only flushing the length of a received packet vs rounding up that length to a cache line (which the device driver can't really know and probably shouldn't' have to know).

[jrtc27]

sys/riscv/riscv/cbo.c

63

You'd have to have an unaligned start for it to matter. But yes, if it's doing cache operations on specific ranges then the old code could have missed a line. But then that means it's doing the exact thing that's dodgy, namely trying to invalidate only part of a line...

[jhb]

sys/riscv/riscv/cbo.c

63

I wonder if it's doing the "normal" NIC driver thing of biasing the initial receive buffer pointer by 2 so that the IP header is aligned. That is, you take the 2K mbuf cluster, but you m_adj(m, 2) before loading it into the RX ring so that the 14 byte Ethernet header ends such that the IP header starts at byte 16. In that case, it really is safe to flush those 2 extra bytes. And that is a pretty common pattern in NIC drivers.