Re: [PATCH 07/11] nds32: Linux Syscall Interface

[Vincent Chen <vincentc at andestech dot com>]

On Wed, May 09, 2018 at 10:57:25AM -0300, Adhemerval Zanella wrote:
> 
> 
> On 09/05/2018 10:46, Vincent Chen wrote:
> > On Wed, May 09, 2018 at 05:27:16AM +0800, Adhemerval Zanella wrote:
> >>
> >>
> >> On 06/05/2018 11:41, vincentc wrote:
> >>> This patch contains the Linux system call interface, as well as the
> 
> [...]
> 
> >>> +#include <errno.h>
> >>> +#include <stdarg.h>
> >>> +#include <sysdep.h>
> >>> +long int syscall (long int __sysno, ...)
> >>> +{
> >>> +
> >>> +	int result;
> >>> +	unsigned long arg1,arg2,arg3,arg4,arg5,arg6;
> >>> +	va_list arg;
> >>> +	va_start (arg, __sysno);
> >>> +	arg1 = va_arg (arg, unsigned long);
> >>> +	arg2 = va_arg (arg, unsigned long);
> >>> +	arg3 = va_arg (arg, unsigned long);
> >>> +	arg4 = va_arg (arg, unsigned long);
> >>> +	arg5 = va_arg (arg, unsigned long);
> >>> +	arg6 = va_arg (arg, unsigned long);
> >>> +	va_end (arg);
> >>> +	__asm__ volatile ( "" ::: "memory" );
> >>> +	result = INLINE_SYSCALL_NCS(__sysno,6,arg1,arg2,arg3,arg4,arg5,arg6);
> >>> +	return result;
> >>> +}
> >>
> >> Maybe we could it as a generic version, the only points I am not sure if why
> >> you need a compiler memory barrier here.
> > 
> > The compiler memory barrier is used to ensuring that the input argument has been
> > stored to the targeted register before issuing syscall in compile time.
> 
> My understanding is INLINE_SYSCALL_NCS is already issues the syscall with a asm 
> volatile, so it should prevent any instruction reordering from compiler to mess
> with expected kernel register allocation.  Are you seeing a miscompile without
> the barrier?

I apologize for giving you incorrect information. After I rechecking the git log
of this file, I think this memory barrier is not needed anymore. Originally, we
need to push the 6-th argument to the stack before invoking the system call. In
order to make sure that the value of 6-th argument is correct in stack, we need
a memory barrier which is located between storing 6-th argument to register $r6
and pushing register $r6 to stack. However, the above procedure was not needed
after we modified system call ABI. So, I think the memory barrier is not needed
now. I will remove it in the next version patch.


Thanks for your suggestions.

Best regards
Vincent Chen