From: Gerd Hoffmann <kraxel@redhat.com> To: Ralf Baechle <ralf@linux-mips.org> Cc: linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, linux-api@vger.kernel.org Subject: Re: [PATCH v3 0/3] preadv & pwritev syscalls. Date: Tue, 16 Dec 2008 17:48:58 +0100 [thread overview] Message-ID: <4947DBFA.9050108@redhat.com> (raw) In-Reply-To: <20081216160502.GA15331@linux-mips.org> Ralf Baechle wrote: > On Mon, Dec 15, 2008 at 09:57:24PM +0100, Gerd Hoffmann wrote: > >>> It fixes the alignment issue but still won't work; on MIPS 32-bit userspace >>> will pass the 64-bit argument in two registers but the 64-bit kernel code >>> will assume it to be passed in a single registers. It'd be ugly but passing >>> a pointer to a 64-bit argument would solve the issue; something like this: >>> >>> sys_preadv(unsigned long fd, const struct iovec __user *vec, >>> unsigned long vlen, loff_t __user *pos); >>> compat_sys_preadv(unsigned long fd, const struct compat_iovec __user *vec, >>> unsigned long vlen, loff_t __user *pos); >> Suggestion from the s390 front was to explicitly pass high and low part >> of pos as two arguments. A bit ugly too, but should work fine as well >> and it avoids the user pointer dereference. What do you think about this? > > That's what the wrapper which you deleted, was doing ;-) So yes, I like > it. Yep, but I'm trying to find a way to have it work without per-arch wrappers ... > It just raises one new problem, endianess - are arguments being passed > as low/high or high/low? On MIPS we've been solving the issue with the > merge_64() macro which is defined depending on the byte order: > > #ifdef __MIPSEB__ > #define merge_64(r1, r2) ((((r1) & 0xffffffffUL) << 32) + ((r2) & 0xffffffffUL)) > #endif > #ifdef __MIPSEL__ > #define merge_64(r1, r2) ((((r2) & 0xffffffffUL) << 32) + ((r1) & 0xffffffffUL)) > #endif > > The actual syscall wrapper could use it like: > > asmlinkage int compat_sys_pwritev(unsigned long fd, > const struct compat_iovec __user *vec, > unsigned a3, unsigned a4, unsigned long vlen) > { > loff_t offset = merge_64(a3, a4); > ... i.e. the ordering of the splitted argument depends on the os endianness? What is the reason for this? I'd prefer to have the ordering coded explicitly instead, like this: asmlinkage int compat_sys_pwritev(unsigned long fd, const struct compat_iovec __user *vec, unsigned long vlen, unsigned pos_low, unsigned pos_high) { loff_t pos = pos_low | (loff_t)pos_high << 32; [ ... ] cheers, Gerd
WARNING: multiple messages have this Message-ID (diff)
From: Gerd Hoffmann <kraxel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> To: Ralf Baechle <ralf-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arch-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Subject: Re: [PATCH v3 0/3] preadv & pwritev syscalls. Date: Tue, 16 Dec 2008 17:48:58 +0100 [thread overview] Message-ID: <4947DBFA.9050108@redhat.com> (raw) In-Reply-To: <20081216160502.GA15331-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org> Ralf Baechle wrote: > On Mon, Dec 15, 2008 at 09:57:24PM +0100, Gerd Hoffmann wrote: > >>> It fixes the alignment issue but still won't work; on MIPS 32-bit userspace >>> will pass the 64-bit argument in two registers but the 64-bit kernel code >>> will assume it to be passed in a single registers. It'd be ugly but passing >>> a pointer to a 64-bit argument would solve the issue; something like this: >>> >>> sys_preadv(unsigned long fd, const struct iovec __user *vec, >>> unsigned long vlen, loff_t __user *pos); >>> compat_sys_preadv(unsigned long fd, const struct compat_iovec __user *vec, >>> unsigned long vlen, loff_t __user *pos); >> Suggestion from the s390 front was to explicitly pass high and low part >> of pos as two arguments. A bit ugly too, but should work fine as well >> and it avoids the user pointer dereference. What do you think about this? > > That's what the wrapper which you deleted, was doing ;-) So yes, I like > it. Yep, but I'm trying to find a way to have it work without per-arch wrappers ... > It just raises one new problem, endianess - are arguments being passed > as low/high or high/low? On MIPS we've been solving the issue with the > merge_64() macro which is defined depending on the byte order: > > #ifdef __MIPSEB__ > #define merge_64(r1, r2) ((((r1) & 0xffffffffUL) << 32) + ((r2) & 0xffffffffUL)) > #endif > #ifdef __MIPSEL__ > #define merge_64(r1, r2) ((((r2) & 0xffffffffUL) << 32) + ((r1) & 0xffffffffUL)) > #endif > > The actual syscall wrapper could use it like: > > asmlinkage int compat_sys_pwritev(unsigned long fd, > const struct compat_iovec __user *vec, > unsigned a3, unsigned a4, unsigned long vlen) > { > loff_t offset = merge_64(a3, a4); > ... i.e. the ordering of the splitted argument depends on the os endianness? What is the reason for this? I'd prefer to have the ordering coded explicitly instead, like this: asmlinkage int compat_sys_pwritev(unsigned long fd, const struct compat_iovec __user *vec, unsigned long vlen, unsigned pos_low, unsigned pos_high) { loff_t pos = pos_low | (loff_t)pos_high << 32; [ ... ] cheers, Gerd -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2008-12-16 16:49 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2008-12-15 11:36 [PATCH v3 0/3] preadv & pwritev syscalls Gerd Hoffmann 2008-12-15 11:36 ` Gerd Hoffmann 2008-12-15 11:36 ` [PATCH v3 1/3] Add missing accounting calls to compat_sys_{readv,writev} Gerd Hoffmann 2008-12-15 11:36 ` [PATCH v3 2/3] Add preadv and pwritev system calls Gerd Hoffmann 2008-12-15 11:36 ` [PATCH v3 3/3] MIPS: Add preadv(2) and pwritev(2) syscalls Gerd Hoffmann 2008-12-15 11:36 ` Gerd Hoffmann 2008-12-15 16:03 ` [PATCH v3 0/3] preadv & pwritev syscalls Ralf Baechle 2008-12-15 16:03 ` Ralf Baechle 2008-12-15 20:02 ` David Miller 2008-12-15 20:57 ` Gerd Hoffmann 2008-12-16 16:05 ` Ralf Baechle 2008-12-16 16:25 ` Kyle McMartin 2008-12-16 16:48 ` Gerd Hoffmann [this message] 2008-12-16 16:48 ` Gerd Hoffmann 2008-12-16 17:02 ` Kyle McMartin 2008-12-16 21:03 ` Arnd Bergmann 2008-12-16 21:34 ` Gerd Hoffmann 2008-12-16 21:34 ` Gerd Hoffmann 2008-12-16 22:39 ` Heiko Carstens 2008-12-16 22:39 ` Heiko Carstens
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=4947DBFA.9050108@redhat.com \ --to=kraxel@redhat.com \ --cc=linux-api@vger.kernel.org \ --cc=linux-arch@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=ralf@linux-mips.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.