Differential D17265

amd64: even up copyin/copyout with memcpy + other cleanup
ClosedPublic
Actions

Authored by mjg on Sep 21 2018, 12:37 AM.

Details

Reviewers

kib

Commits

rS338858: amd64: even up copyin/copyout with memcpy + other cleanup

Summary

_fault handlers for both primitives are identical, provide just one
change the copying scheme to match memcpy (in particular jump avoidance for the most common case of multiply of 8)
stop re-reading pcb address on exit, just store it locally (in r9)

This last bit can be applied to all other primitives as well. Side point is that there is a way to get rid of the need of setting onfault handler by looking up RIP at page fault and comparing it against userspace access routines. I have a prototype which encloses rep sections with labels for identification.

Regardless of the above there is more cleanup to be done here.

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

mjg created this revision.Sep 21 2018, 12:37 AM

Herald added a subscriber: imp. · View Herald TranscriptSep 21 2018, 12:37 AM

Harbormaster completed remote builds in B19710: Diff 48287.Sep 21 2018, 12:37 AM

Not setting pcb_onfault has more consequences than only comparing addresses in trap(). But I will comment more on the actual patch.

sys/amd64/amd64/support.S
416	This should be jbe and not jle (signed vs unsigned), I believe.
461	This can be movl $EFAULT,%eax.

mjg updated this revision to Diff 48307.Sep 21 2018, 11:46 AM

In D17265#368054, @kib wrote:

Not setting pcb_onfault has more consequences than only comparing addresses in trap(). But I will comment more on the actual patch.

If you mean it needs a place to land to handle recovery, this is covered with a table:

CNAME(onfault_table_nosmap):
       .quad ot_copyout_nosmap_start
       .quad ot_copyout_nosmap_end
       .quad copy_fault
       .quad ot_copyin_nosmap_start
       .quad ot_copyin_nosmap_end
       .quad copy_fault
       .quad 0

I'm mostly toying with this idea. I have a working prototype which I'll possibly propose after 12 ships. I don't know where the general idea originates from, NetBSD and Linux have a variant.

In D17265#368078, @mjg wrote:
In D17265#368054, @kib wrote:

Not setting pcb_onfault has more consequences than only comparing addresses in trap(). But I will comment more on the actual patch.

If you mean it needs a place to land to handle recovery, this is covered with a table:
CNAME(onfault_table_nosmap):
       .quad ot_copyout_nosmap_start
       .quad ot_copyout_nosmap_end
       .quad copy_fault
       .quad ot_copyin_nosmap_start
       .quad ot_copyin_nosmap_end
       .quad copy_fault
       .quad 0
I'm mostly toying with this idea. I have a working prototype which I'll possibly propose after 12 ships. I don't know where the general idea originates from, NetBSD and Linux have a variant.

I mean that there are more places which make a decision based on pcb_onfault != 0 than just a fault landing code. Grep for pcb_onfault.

This revision is now accepted and ready to land.Sep 21 2018, 11:57 AM

Closed by commit rS338858: amd64: even up copyin/copyout with memcpy + other cleanup (authored by mjg). · Explain WhySep 21 2018, 3:01 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

sys/

amd64/

support.S

56 lines

Diff 48307

View Options

amd64: even up copyin/copyout with memcpy + other cleanupClosedPublicActions