[PATCH] powerpc: add new required termio functions

[PATCH] powerpc: add new required termio functions

[Michael Neuling]

The "tty: termios locking functions break with new termios type" patch
(f629307c857c030d5a3dd777fee37c8bb395e171) breaks the powerpc compile.
This adds the required API to asm-powerpc.

Signed-off-by: Michael Neuling <mikey@neuling.org>
--
This needs to go up for 2.6.23.

Should we really put these definitions in asm-generic/termios.h as I'm
guessing other architectures are broken too?

coopers@include/ % git grep kernel_termios_to_user_termios_1
asm-arm/termios.h:#define kernel_termios_to_user_termios_1(u, k)
asm-cris/termios.h:#define kernel_termios_to_user_termios_1(u, k)
asm-h8300/termios.h:#define kernel_termios_to_user_termios_1(u, k)
asm-i386/termios.h:#define kernel_termios_to_user_termios_1(u, k)
asm-ia64/termios.h:#define kernel_termios_to_user_termios_1(u, k)
asm-m32r/termios.h:#define kernel_termios_to_user_termios_1(u, k)
asm-m68k/termios.h:#define kernel_termios_to_user_termios_1(u, k)
asm-mips/termios.h:#define kernel_termios_to_user_termios_1(u, k)
asm-v850/termios.h:#define kernel_termios_to_user_termios_1(u, k)
asm-x86_64/termios.h:#define kernel_termios_to_user_termios_1(u, k)

 include/asm-powerpc/termios.h |    3 +++
 1 file changed, 3 insertions(+)

Index: linux-2.6-ozlabs/include/asm-powerpc/termios.h
===================================================================
--- linux-2.6-ozlabs.orig/include/asm-powerpc/termios.h
+++ linux-2.6-ozlabs/include/asm-powerpc/termios.h
@@ -80,6 +80,9 @@ struct termio {
 
 #include <asm-generic/termios.h>
 
+#define user_termios_to_kernel_termios_1(k, u) copy_from_user(k, u, sizeof(struct termios))
+#define kernel_termios_to_user_termios_1(u, k) copy_to_user(u, k, sizeof(struct termios))
+
 #endif	/* __KERNEL__ */
 
 #endif	/* _ASM_POWERPC_TERMIOS_H */

Re: [PATCH] powerpc: add new required termio functions

[Linus Torvalds]

On Wed, 12 Sep 2007, Michael Neuling wrote:
>
> The "tty: termios locking functions break with new termios type" patch
> (f629307c857c030d5a3dd777fee37c8bb395e171) breaks the powerpc compile.

Really?

It shouldn't. The use of kernel_termios_to_user_termios_1() is conditional 
on the architecture having a define for TCGETS2, and I think they match 
up. I see:

	[torvalds@woody linux]$ git grep -l kernel_termios_to_user_termios_1 include | wc -l
	10
	[torvalds@woody linux]$ git grep -l TCGETS2 include | wc -l
	10

and in neither case is ppc in that list of architecures.

So maybe you just read the patch without actually testing whether it 
actually broke powerpc?

Or is something subtler going on?

			Linus

Re: [PATCH] powerpc: add new required termio functions

[Tony Breeds]

On Tue, Sep 11, 2007 at 07:17:42PM -0700, Linus Torvalds wrote:
 
> Really?
> 
> It shouldn't. The use of kernel_termios_to_user_termios_1() is conditional 
> on the architecture having a define for TCGETS2, and I think they match 
> up. I see:
> 
> 	[torvalds@woody linux]$ git grep -l kernel_termios_to_user_termios_1 include | wc -l
> 	10
> 	[torvalds@woody linux]$ git grep -l TCGETS2 include | wc -l
> 	10
> 
> and in neither case is ppc in that list of architecures.
> 
> So maybe you just read the patch without actually testing whether it 
> actually broke powerpc?
> 
> Or is something subtler going on?

As far as I can see TIOCSLCKTRMIOS and TIOCGLCKTRMIOS aren't protected
by TCGETS2 guards.  Do they need to be ...  Perhaps


From: Tony Breeds <tony@bakeyournoodle.com>

Add Guards around TIOCSLCKTRMIOS and TIOCGLCKTRMIOS.

Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>

---

 drivers/char/tty_ioctl.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/char/tty_ioctl.c b/drivers/char/tty_ioctl.c
index 4a8969c..3ee73cf 100644
--- a/drivers/char/tty_ioctl.c
+++ b/drivers/char/tty_ioctl.c
@@ -795,6 +795,19 @@ int n_tty_ioctl(struct tty_struct * tty, struct file * file,
 			if (L_ICANON(tty))
 				retval = inq_canon(tty);
 			return put_user(retval, (unsigned int __user *) arg);
+#ifndef TCGETS2
+		case TIOCGLCKTRMIOS:
+			if (kernel_termios_to_user_termios((struct termios __user *)arg, real_tty->termios_locked))
+				return -EFAULT;
+			return 0;
+
+		case TIOCSLCKTRMIOS:
+			if (!capable(CAP_SYS_ADMIN))
+				return -EPERM;
+			if (user_termios_to_kernel_termios(real_tty->termios_locked, (struct termios __user *) arg))
+				return -EFAULT;
+			return 0;
+#else
 		case TIOCGLCKTRMIOS:
 			if (kernel_termios_to_user_termios_1((struct termios __user *)arg, real_tty->termios_locked))
 				return -EFAULT;
@@ -806,6 +819,7 @@ int n_tty_ioctl(struct tty_struct * tty, struct file * file,
 			if (user_termios_to_kernel_termios_1(real_tty->termios_locked, (struct termios __user *) arg))
 				return -EFAULT;
 			return 0;
+#endif
 
 		case TIOCPKT:
 		{

Yours Tony

  linux.conf.au        http://linux.conf.au/ || http://lca2008.linux.org.au/
  Jan 28 - Feb 02 2008 The Australian Linux Technical Conference!

Re: [PATCH] powerpc: add new required termio functions

[Michael Neuling]

> On Wed, 12 Sep 2007, Michael Neuling wrote:
> >
> > The "tty: termios locking functions break with new termios type" patch
> > (f629307c857c030d5a3dd777fee37c8bb395e171) breaks the powerpc compile.
> 
> Really?
> 
> It shouldn't. The use of kernel_termios_to_user_termios_1() is conditional 
> on the architecture having a define for TCGETS2, and I think they match 
> up. I see:
> 
> 	[torvalds@woody linux]$ git grep -l kernel_termios_to_user_termios_1 in
clude | wc -l
> 	10
> 	[torvalds@woody linux]$ git grep -l TCGETS2 include | wc -l
> 	10
> 
> and in neither case is ppc in that list of architecures.
> 
> So maybe you just read the patch without actually testing whether it 
> actually broke powerpc?

Not, I actually compiled it.

> Or is something subtler going on?

Looks like those new calls are not protected by the TCGETS2 define.
Adding those ifdefs seems like the correct fix.  

Mikey

Re: [PATCH] powerpc: add new required termio functions

[Geoff Levand]

Michael Neuling wrote:
> The "tty: termios locking functions break with new termios type" patch
> (f629307c857c030d5a3dd777fee37c8bb395e171) breaks the powerpc compile.
> This adds the required API to asm-powerpc.
> 
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> --
> This needs to go up for 2.6.23.
> 
> Should we really put these definitions in asm-generic/termios.h as I'm
> guessing other architectures are broken too?

I think it would be better to do so, as that is where we pickup the defs for
the original kernel_termios_to_user_termios and user_termios_to_kernel_termios.

-Geoff

Re: [PATCH] powerpc: add new required termio functions

[Heiko Carstens]

On Wed, Sep 12, 2007 at 12:04:39PM +1000, Michael Neuling wrote:
> The "tty: termios locking functions break with new termios type" patch
> (f629307c857c030d5a3dd777fee37c8bb395e171) breaks the powerpc compile.
> [...]
> I'm guessing other architectures are broken too?

FWIW, the above quoted patch breaks s390 as well.

Re: [PATCH] powerpc: add new required termio functions

[Andrew Morton]

On Wed, 12 Sep 2007 12:20:32 +0200 Heiko Carstens <heiko.carstens@de.ibm.com> wrote:

> On Wed, Sep 12, 2007 at 12:04:39PM +1000, Michael Neuling wrote:
> > The "tty: termios locking functions break with new termios type" patch
> > (f629307c857c030d5a3dd777fee37c8bb395e171) breaks the powerpc compile.
> > [...]
> > I'm guessing other architectures are broken too?
> 
> FWIW, the above quoted patch breaks s390 as well.

Does this fix it?

diff -puN drivers/char/tty_ioctl.c~powerpc-add-new-required-termio-functions drivers/char/tty_ioctl.c
--- a/drivers/char/tty_ioctl.c~powerpc-add-new-required-termio-functions
+++ a/drivers/char/tty_ioctl.c
@@ -795,6 +795,19 @@ int n_tty_ioctl(struct tty_struct * tty,
 			if (L_ICANON(tty))
 				retval = inq_canon(tty);
 			return put_user(retval, (unsigned int __user *) arg);
+#ifndef TCGETS2
+		case TIOCGLCKTRMIOS:
+			if (kernel_termios_to_user_termios((struct termios __user *)arg, real_tty->termios_locked))
+				return -EFAULT;
+			return 0;
+
+		case TIOCSLCKTRMIOS:
+			if (!capable(CAP_SYS_ADMIN))
+				return -EPERM;
+			if (user_termios_to_kernel_termios(real_tty->termios_locked, (struct termios __user *) arg))
+				return -EFAULT;
+			return 0;
+#else
 		case TIOCGLCKTRMIOS:
 			if (kernel_termios_to_user_termios_1((struct termios __user *)arg, real_tty->termios_locked))
 				return -EFAULT;
@@ -806,6 +819,7 @@ int n_tty_ioctl(struct tty_struct * tty,
 			if (user_termios_to_kernel_termios_1(real_tty->termios_locked, (struct termios __user *) arg))
 				return -EFAULT;
 			return 0;
+#endif
 
 		case TIOCPKT:
 		{
_

Re: [PATCH] powerpc: add new required termio functions

[Michael Neuling]

In message <20070912040109.3f6a9149.akpm@linux-foundation.org> you wrote:
> On Wed, 12 Sep 2007 12:20:32 +0200 Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> 
> > On Wed, Sep 12, 2007 at 12:04:39PM +1000, Michael Neuling wrote:
> > > The "tty: termios locking functions break with new termios type" patch
> > > (f629307c857c030d5a3dd777fee37c8bb395e171) breaks the powerpc compile.
> > > [...]
> > > I'm guessing other architectures are broken too?
> > 
> > FWIW, the above quoted patch breaks s390 as well.
> 
> Does this fix it?

FYI it does fix powerpc, but I suspect you were asking about s390 here.

Mikey

> 
> diff -puN drivers/char/tty_ioctl.c~powerpc-add-new-required-termio-functions 
drivers/char/tty_ioctl.c
> --- a/drivers/char/tty_ioctl.c~powerpc-add-new-required-termio-functions
> +++ a/drivers/char/tty_ioctl.c
> @@ -795,6 +795,19 @@ int n_tty_ioctl(struct tty_struct * tty,
>  			if (L_ICANON(tty))
>  				retval = inq_canon(tty);
>  			return put_user(retval, (unsigned int __user *) arg);
> +#ifndef TCGETS2
> +		case TIOCGLCKTRMIOS:
> +			if (kernel_termios_to_user_termios((struct termios __us
er *)arg, real_tty->termios_locked))
> +				return -EFAULT;
> +			return 0;
> +
> +		case TIOCSLCKTRMIOS:
> +			if (!capable(CAP_SYS_ADMIN))
> +				return -EPERM;
> +			if (user_termios_to_kernel_termios(real_tty->termios_lo
cked, (struct termios __user *) arg))
> +				return -EFAULT;
> +			return 0;
> +#else
>  		case TIOCGLCKTRMIOS:
>  			if (kernel_termios_to_user_termios_1((struct termios __
user *)arg, real_tty->termios_locked))
>  				return -EFAULT;
> @@ -806,6 +819,7 @@ int n_tty_ioctl(struct tty_struct * tty,
>  			if (user_termios_to_kernel_termios_1(real_tty->termios_
locked, (struct termios __user *) arg))
>  				return -EFAULT;
>  			return 0;
> +#endif
>  
>  		case TIOCPKT:
>  		{
> _
> 

Re: [PATCH] powerpc: add new required termio functions

[Christoph Hellwig]

On Wed, Sep 12, 2007 at 04:01:09AM -0700, Andrew Morton wrote:
> On Wed, 12 Sep 2007 12:20:32 +0200 Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> 
> > On Wed, Sep 12, 2007 at 12:04:39PM +1000, Michael Neuling wrote:
> > > The "tty: termios locking functions break with new termios type" patch
> > > (f629307c857c030d5a3dd777fee37c8bb395e171) breaks the powerpc compile.
> > > [...]
> > > I'm guessing other architectures are broken too?
> > 
> > FWIW, the above quoted patch breaks s390 as well.
> 
> Does this fix it?

I might be missing something, but the the right fix is probably to
apply the arch patches from Alan to powerpc and s390.  We don't want to
be left over without all the nice termios features on these platforms,
do we?

Re: [PATCH] powerpc: add new required termio functions

[Heiko Carstens]

On Wed, Sep 12, 2007 at 12:34:09PM +0100, Christoph Hellwig wrote:
> On Wed, Sep 12, 2007 at 04:01:09AM -0700, Andrew Morton wrote:
> > On Wed, 12 Sep 2007 12:20:32 +0200 Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> > 
> > > On Wed, Sep 12, 2007 at 12:04:39PM +1000, Michael Neuling wrote:
> > > > The "tty: termios locking functions break with new termios type" patch
> > > > (f629307c857c030d5a3dd777fee37c8bb395e171) breaks the powerpc compile.
> > > > [...]
> > > > I'm guessing other architectures are broken too?
> > > 
> > > FWIW, the above quoted patch breaks s390 as well.
> > 
> > Does this fix it?

Yes, it does.

> I might be missing something, but the the right fix is probably to
> apply the arch patches from Alan to powerpc and s390.  We don't want to
> be left over without all the nice termios features on these platforms,
> do we?

But not in rc6 timeframe, I would guess?

Re: [PATCH] powerpc: add new required termio functions

[Christoph Hellwig]

On Wed, Sep 12, 2007 at 01:52:57PM +0200, Heiko Carstens wrote:
> > I might be missing something, but the the right fix is probably to
> > apply the arch patches from Alan to powerpc and s390.  We don't want to
> > be left over without all the nice termios features on these platforms,
> > do we?
> 
> But not in rc6 timeframe, I would guess?

Ask Alan..

Re: [PATCH] powerpc: add new required termio functions

[Alan Cox]

On Wed, Sep 12, 2007 at 02:49:10PM +0100, Christoph Hellwig wrote:
> On Wed, Sep 12, 2007 at 01:52:57PM +0200, Heiko Carstens wrote:
> > > I might be missing something, but the the right fix is probably to
> > > apply the arch patches from Alan to powerpc and s390.  We don't want to
> > > be left over without all the nice termios features on these platforms,
> > > do we?
> > 
> > But not in rc6 timeframe, I would guess?
> 
> Ask Alan..

Well i've been chasing people for many months on the 390 side so if they missed
a release tough ...

Alan


-- 
--
		"A wiki is where ideas go to die"
				-- Becky Hogge