Stop calling set_syscall_retval() from linux_set_syscall_retval()

[trasz]

Summary

Stop calling set_syscall_retval() from linux_set_syscall_retval().
The former clobbers some registers that shouldn't be touched.

[kib]

sys/amd64/linux/linux_sysvec.c

224

So why return instead of break ?

239

Why do this twice ?

246

And you are trying to remove this macro in other review ? Why do you put incomplete change to review ?

[trasz]

sys/amd64/linux/linux_sysvec.c

224

Because that's the end of the fast path for this function. The set_syscall_retval() does the same. It makes actual sense there, since it doesn't need PCB_FULL_IRET, but I've tried to keep two routines as similar as possible.

239

This one is needed; the ones we do unconditionally are to be removed (eventually). Again, same as with set_syscall_retval().

246

It is complete - it applies to CURRENT. Otherwise this patch would depend on the other one, otherwise unrelated.

[kib]

sys/amd64/linux/linux_sysvec.c

224

But you still set FULL_IRET, which completely undermines the meaning of the 'fast path'.

239

I do not see this patch as committable until the 'eventually' part is resolved. set_syscall_retval() does not set FULL_IRET twice.

246

I do not understand this: you cannot get proper review until one of the patches is resolved.

[trasz]

Fix multiple issues.

[trasz]

sys/amd64/linux/linux_sysvec.c

224

True. The plan was to keep it as close to set_syscall_retval() as possible, and later just remove the problematic calls to set_pcb_flags(). But you're probably right that the code should fit the current state of things, not the one we'd like to have in the future.

[kib]

sys/amd64/linux/linux_sysvec.c

245

This paragraph does not make much sense in the context of linux_set_syscall_retval.

249

Isn't this half of the comment relevant to all paths ending in set(FULL_IRET) ? I suggest to move it right above set_pcb_flags() instead of the wishful thinking that is currently at lines 2490250.

[trasz]

sys/amd64/linux/linux_sysvec.c

245

True. I've reworded this somewhat; it still gives a useful hint.

[trasz]

Fix stuff.

[kib]

sys/amd64/linux/linux_sysvec.c

217–225

Why is this assignment needed ?

[trasz]

Remove unnecessary assignment.

[trasz]

sys/amd64/linux/linux_sysvec.c

217–225

It's not; I just didn't quite understood what it did and wanted to be extra safe there. Removed.

[kib]

sys/amd64/linux/linux_sysvec.c

217

May be move this comment down as the first sentence before 'require full context restore ...'. It would avoid requiring to read everything to understand that comment.

244

I think it would be more up to point if you say there '... to get all registers except %rcx and %r11 restored ...'.