[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 04/17] common-user: Move syscall error detection into safe
Re: [PATCH v5 04/17] common-user: Move syscall error detection into safe_syscall_base
Mon, 22 Nov 2021 13:21:07 +0100
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0
On 11/22/21 12:55 PM, Peter Maydell wrote:
- /* This is the entry point for making a system call. The calling
+ * This is the entry point for making a system call. The calling
* convention here is that of a C varargs function with the
* first argument an 'int *' to the signal_pending flag, the
* second one the system call number (as a 'long'), and all further
* arguments being syscall arguments (also 'long').
This comment text needs updating to mention the new errnop argument.
(Applies to all the similar comments in the files for the other archs.)
+ * The syscall calling convention isn't the same as the C one:
Looks like the indent here is wrong ?
Irritatingly, these files are a mix of tabs/spaces.
* we enter with x0 == *signal_pending
- * x1 == syscall number
- * x2 ... x7, (stack) == syscall arguments
+ * x1 == errno
"int* address of errno"
Arg, fixed some of these, but clearly. not all.
/* code path for having successfully executed the syscall */
+ cmn x0, #4095
+ b.cs 1f
Shouldn't this be going to label 0f ? We need to do the 'neg',
and unless I'm misreading the diff there's currently no path
of execution that gets to that.
Oops, rebase error, where the fix landed in the next patch.
+ * r2 == syscall number
+ * r3, [sp+0] ... [sp+16] == syscall arguments
* and return the result in r0
Don't we wind up with a potential issue here with 64-bit arguments
due to the calling convention wanting to put those in aligned
memory/register locations? Previously because we had just two
extra arguments the arguments started at r2 and had the same
alignment behaviour as the syscall wants for them starting at
r0; but now we start at r3 so if for instance the first argument
is 64-bit it will be in [sp+0][sp+4] but should go in r0:r1
(Stopped reviewing here because if we need to change the
way we call these functions there's no point my reviewing
the fine detail of the asm.)
Oof. I missed that detail. Yes, that is a problem (I think arm is the only such
supported host). I think the best solution would be to *not* pass in &errno, but to have
the assembly tail-call to
long safe_syscall_errno_tail(int value)
errno = value;
Which is probably more efficient in any case. I'll re-work this.
- Re: [PATCH v5 02/17] linux-user/signal.c: Create a common rewind_if_in_safe_syscall, (continued)
- [PATCH v5 07/17] linux-user: Remove HAVE_SAFE_SYSCALL and hostdep.h, Richard Henderson, 2021/11/17
- [PATCH v5 08/17] common-user: Adjust system call return on FreeBSD, Richard Henderson, 2021/11/17
- [PATCH v5 09/17] *-user: Rename TARGET_ERESTARTSYS to QEMU_ERESTARTSYS, Richard Henderson, 2021/11/17
- [PATCH v5 11/17] bsd-user: Create special-errno.h, Richard Henderson, 2021/11/17
- [PATCH v5 12/17] linux-user: Create special-errno.h, Richard Henderson, 2021/11/17