Page MenuHomeFreeBSD

Optimize RISC-V copyin(9)/copyout(9) routines
ClosedPublic

Authored by mhorne063_gmail.com on Jan 15 2019, 11:44 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, May 7, 4:14 AM
Unknown Object (File)
Mon, Apr 22, 11:18 PM
Unknown Object (File)
Apr 1 2024, 2:27 AM
Unknown Object (File)
Jan 30 2024, 6:50 AM
Unknown Object (File)
Dec 22 2023, 10:30 PM
Unknown Object (File)
Dec 19 2023, 7:00 AM
Unknown Object (File)
Dec 13 2023, 12:05 AM
Unknown Object (File)
Nov 11 2023, 12:09 PM
Subscribers

Details

Summary

The existing copyin(9) and copyout(9) routines on RISC-V perform only a
simple byte-by-byte copy. Improve their performance by performing
word-sized copies where possible.

Overall approach: For best performance, all load's and stores must occur
on their native boundary, i.e. a 64-bit load must occur on a 64-bit
aligned address. Misaligned loads and stores are possible, but they
require trapping into the SBI, which introduces an even bigger overhead.
Therefore, we will perform word-sized loads and stores only where
possible.

For cases where the source and destination addresses are not aligned to
each other, we have no choice but to do a byte-by- byte copy for the
entire thing. So long as this is not the case, then we can perform word
copy for some or all of the buffer. In some cases this will require byte
copy at the beginning or end to account for addresses that are not
initially word-aligned or any remainder due to buffer length.

Test Plan

Boots successfully to the login prompt.

I will perform a few simple tests to get some numbers on the performance improvement.

I could also try running the copyin tests, but I'm not sure if I'm set up to do that yet.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/riscv/include/riscvreg.h
159 ↗(On Diff #52874)

XLEN is defined to be the width of the CPU's general purpose registers - what's the reason for adding a new constant?

sys/riscv/include/riscvreg.h
159 ↗(On Diff #52874)

Whoops, I had a comment about this but didn't submit it.

The spec uses XLEN to refer to the GPR width in bits. In my opinion, defining it here in bytes is a miss-use of the name.
A quick grep shows that it isn't referenced by any existing code, so rather than using it in mine I added the new constant.

Perhaps we could adjust it to XLEN = 64, and rename REG_SIZE to XLEN_BYTES? Having a proper XLEN in bits would be useful for eventually making some existing constants width-agnostic.

sys/riscv/include/riscvreg.h
159 ↗(On Diff #52874)

Ahh, I see. Indeed, we don't use XLEN, but there is the compiler-provided __riscv_xlen which has a couple of usages.

I like the suggestion of fixing XLEN's value (probably just define XLEN to be __riscv_xlen?) and introducing XLEN_BYTES.

Adjust XLEN constant names and values.

You can add your copyright to this file if you feel so inclined.

This revision is now accepted and ready to land.Jan 18 2019, 8:33 PM

You can add your copyright to this file if you feel so inclined.

Sure, might as well :)

This revision now requires review to proceed.Jan 18 2019, 8:44 PM
This revision was not accepted when it landed; it landed in state Needs Review.Jan 21 2019, 7:39 PM
This revision was automatically updated to reflect the committed changes.