[RFC PATCH 1/1] fix get_ & put_ chars in hvc_vio to fix IN/OUT _BUF assumptions

[Ryan Arnold]

Greetings Milton, et al.

On Fri, 2005-12-16 at 23:02 -0600, Milton Miller wrote: 
> > +
> > +/*
> > + * This is a design shortcoming, the number '16' is a vio required 
> > buffer
> > + * size.  This should be changeable per architecture, but hvc_struct 
> > relies
> > + * upon it and that struct is used by all hvc_console backend 
> > drivers.  This
> > + * needs to be fixed.
> > + */
> 
> This is a bit strong.  vio requires inbuf to be at least 16, and will 
> process upto 16 in outbound.   They could be bigger, it will only cause 
> the hvc_driver to loop.  Outbound couuld be smaller, but it would 
> reduce the efficency.

I wrote this comment.  In retrospect it is definitely too strong.  

None-the-less I feel the need to address a few dangerous assumptions in
the hvc_vio code based upon the size of N_INBUF and N_OUTBUF.

The firmware function plpar_hcall_norets(H_PUT_TERM_CHAR, ...) will not
gracefully accept a 'count' parameter exceeding '16'.  It will generate
an H_Parameter error.  The current hvconsole.c hvc_put_chars() function
does not account for this and is prone to breakage should N_OUTBUF have
a size greater than '16'.

Furthermore, the hvc_console driver asks its back-ends to read 'count'
number of characters based upon the amount of room left in the flip
buffer.  This value is less-than-or-equal-to the size of N_INBUF.

Presently the hvc_vio driver makes an erroneous assumption that it can
forward this request directly to vio firmware and ask for a particular
number of bytes to be read into its recv buffer.

This is not how plpar_hcall(H_GET_TERM_CHAR,...) works.  You don't ask
it to read a number of bytes.  After is invoked it tells you how many
bytes it read, up to the firmware specified maximum of 16.  Should the
amount requested (count) be less than the number read from firmware we'd
over-write the flip buffer.

We've not noticed any problems thus-far because we currently allow the
tty to drain the flip buffer before it gets too full (after every 64
bytes read).  Draining after 64 bytes is unnecessary considering that
the flip buffer is 512 bytes so I've removed it.

To fix the back-end problem I've directed the back-end drivers to return
-EAGAIN if they can't satisfy the get request size-constraint.  This
signals the hvc_console driver front-end that it should allow the tty to
clear the flip buffer to make room for a larger 'get'.  This allows us
to pack the flip buffer to be nearly full.

As requested, I've also removed the front-end function
__hvc_write_kernel() because it is no longer necessary.

The following patch is against powerpc.git

Ryan S. Arnold <rsa at us.ibm.com>
IBM Linux Technology Center

Signed-off-by: "Ryan S. Arnold" <rsa at us.ibm.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: powerpc.git.hvc_console.patch
Type: text/x-patch
Size: 4325 bytes
Desc: not available
Url : http://ozlabs.org/pipermail/linuxppc64-dev/attachments/20060104/bd56f19f/attachment.bin