All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Borntraeger <borntraeger@de.ibm.com>
To: Amit Shah <amit.shah@redhat.com>
Cc: virtualization@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, Alan Cox <alan@linux.intel.com>
Subject: Re: [PATCH] virtio_console: Add support for multiple ports for generic guest and host communication
Date: Tue, 29 Sep 2009 14:56:56 +0200	[thread overview]
Message-ID: <200909291456.56723.borntraeger__28247.7151988589$1254229199$gmane$org@de.ibm.com> (raw)
In-Reply-To: <20090929122006.GB3766@amit-x200.redhat.com>

Am Dienstag 29 September 2009 14:20:06 schrieb Amit Shah:
> Christian tested the patch on s390 and found that the output was
> very slow. He tracked it down to put_chars never getting init'ed
> to the final value.
> 
> Signed-off-by: Amit Shah <amit.shah@redhat.com>

Thanks. This fix is 
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> 

I am a bit reluctant to Ack the whole change, since my preference would have
been to not merge virtio serial/console and instead keeping both separate.
We have already managed to clutter all other virtio drivers with tons of 
configuration stuff and feature bits - and every driver uses a different model
for configuration and commands (feature bits, config space, config_change 
indication, extra config virtqueue, commands embedded into the data....).
Using a different device ID for a different use seem like a better way to me.

On the other hand, this patch allows more than one console (I have not tested
this feature) and with this fix applied I dont see any obvious problems.

For the console part I can give a 
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> (console)

Ignoring my preference for having a separate driver and devids, I have no 
opinion about the generic communication stuff - no ack or nack.

> ---
>  drivers/char/virtio_console.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index 37513e8..598bc0d 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -571,6 +571,14 @@ int init_port_console(struct virtio_console_port
>  *port) * The Host's telling us this port is a console port. Hook it
>  	 * up with an hvc console.
>  	 *
> +	 * We had set the virtio_cons put_chars implementation to
> +	 * put_chars for early_init. Now that we're done with the
> +	 * early init phase, replace it with our cons_put_chars
> +	 * implementation.
> +	 */
> +	virtio_cons.put_chars = cons_put_chars;
> +
> +	/*
>  	 * To set up and manage our virtual console, we call
>  	 * hvc_alloc().
>  	 *

  reply	other threads:[~2009-09-29 12:56 UTC|newest]

Thread overview: 113+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-11 14:13 Multiple ports support for virtio_console; major number for dev Amit Shah
2009-09-11 14:13 ` [PATCH] virtio_console: Add support for multiple ports for generic guest and host communication Amit Shah
2009-09-11 14:13 ` Amit Shah
2009-09-11 16:00   ` Alan Cox
2009-09-11 16:00   ` Alan Cox
2009-09-11 16:38     ` Amit Shah
2009-09-11 17:26       ` Anthony Liguori
2009-09-11 17:33         ` Amit Shah
2009-09-11 17:33         ` Amit Shah
2009-09-11 17:35           ` Anthony Liguori
2009-09-16 10:23             ` Alan Cox
2009-09-16 22:20               ` Anthony Liguori
2009-09-17 13:06                 ` Markus Armbruster
2009-09-17 13:15                   ` Alan Cox
2009-09-17 13:41                     ` Amit Shah
2009-09-17 13:41                     ` Amit Shah
2009-09-17 15:57                       ` Alan Cox
2009-09-17 15:57                       ` Alan Cox
2009-09-17 16:46                         ` Amit Shah
2009-09-17 16:46                         ` Amit Shah
2009-09-17 13:15                   ` Alan Cox
2009-09-17 13:06                 ` Markus Armbruster
2009-09-18 16:00                 ` Alan Cox
2009-09-18 17:55                   ` Anthony Liguori
2009-09-18 17:55                   ` Anthony Liguori
2009-09-18 17:57                     ` H. Peter Anvin
2009-09-18 18:02                       ` Anthony Liguori
2009-09-18 18:02                       ` Anthony Liguori
2009-09-19  7:04                       ` Amit Shah
2009-09-19  7:04                       ` Amit Shah
2009-09-18 17:57                     ` H. Peter Anvin
2009-09-18 20:11                     ` Gerd Hoffmann
2009-09-18 20:11                     ` Gerd Hoffmann
2009-09-19  7:01                     ` Amit Shah
2009-09-19  7:01                     ` Amit Shah
2009-09-21 10:06                     ` Alan Cox
2009-09-21 10:06                     ` Alan Cox
2009-09-18 16:00                 ` Alan Cox
2009-09-16 22:20               ` Anthony Liguori
2009-09-16 10:23             ` Alan Cox
2009-09-11 17:35           ` Anthony Liguori
2009-09-11 17:26       ` Anthony Liguori
2009-09-11 16:38     ` Amit Shah
2009-09-15 12:37     ` Amit Shah
2009-09-15 12:37     ` Amit Shah
2009-09-15 12:57       ` Anthony Liguori
2009-09-15 12:57       ` Anthony Liguori
2009-09-15 13:03         ` Amit Shah
2009-09-15 13:03         ` Amit Shah
2009-09-15 13:37           ` Anthony Liguori
2009-09-15 13:37           ` Anthony Liguori
2009-09-15 14:08         ` Gerd Hoffmann
2009-09-15 14:08           ` Gerd Hoffmann
2009-09-15 14:09           ` Anthony Liguori
2009-09-15 14:09             ` Anthony Liguori
2009-09-22  2:44     ` Rusty Russell
2009-09-22  2:44     ` Rusty Russell
2009-09-22 15:45       ` Amit Shah
2009-09-22 15:45       ` Amit Shah
2009-09-29  9:24         ` Amit Shah
2009-09-29  9:24         ` Amit Shah
2009-09-29 10:09           ` Christian Borntraeger
2009-09-29 10:33             ` Amit Shah
2009-09-29 11:02               ` Christian Borntraeger
2009-09-29 11:40                 ` Christian Borntraeger
2009-09-29 11:40                 ` Christian Borntraeger
2009-09-29 11:02               ` Christian Borntraeger
2009-09-29 10:33             ` Amit Shah
2009-09-29 10:09           ` Christian Borntraeger
2009-09-29 12:03           ` Christian Borntraeger
2009-09-29 12:03           ` Christian Borntraeger
2009-09-29 12:20             ` Amit Shah
2009-09-29 12:20             ` Amit Shah
2009-09-29 12:56               ` Christian Borntraeger [this message]
2009-09-29 12:56               ` Christian Borntraeger
2009-09-29 13:09                 ` Amit Shah
2009-09-29 13:31                   ` Christian Borntraeger
2009-09-30 17:13                     ` Amit Shah
2009-09-30 17:13                     ` Amit Shah
2009-10-01  8:17                       ` Christian Borntraeger
2009-10-01  8:17                       ` Christian Borntraeger
2009-10-01  8:47                         ` Amit Shah
2009-10-01  8:47                         ` Amit Shah
2009-10-01  8:55                           ` Christian Borntraeger
2009-10-01  8:55                           ` Christian Borntraeger
2009-10-01  9:00                       ` Christian Borntraeger
2009-10-01 10:04                         ` Amit Shah
2009-10-01 10:04                         ` Amit Shah
2009-10-01  9:00                       ` Christian Borntraeger
2009-10-01 10:28                       ` Christian Borntraeger
2009-10-01 10:28                       ` Christian Borntraeger
2009-10-01 10:42                         ` Amit Shah
2009-10-01 10:42                         ` Amit Shah
2009-10-01 11:58                         ` Amit Shah
2009-10-01 11:58                         ` Amit Shah
2009-10-01 12:04                           ` Christian Borntraeger
2009-10-01 13:50                             ` Amit Shah
2009-10-01 13:50                             ` Amit Shah
2009-10-01 12:04                           ` Christian Borntraeger
2009-10-05 14:05                         ` Amit Shah
2009-10-05 14:05                           ` Amit Shah
2009-10-06  6:49                           ` Christian Borntraeger
2009-10-06  6:49                             ` Christian Borntraeger
2009-10-06  6:55                             ` Amit Shah
2009-10-06  6:55                               ` Amit Shah
2009-09-29 13:31                   ` Christian Borntraeger
2009-09-29 13:09                 ` Amit Shah
2009-09-29 13:11   ` Rusty Russell
2009-09-29 13:24     ` Amit Shah
2009-09-29 13:24     ` Amit Shah
2009-09-29 13:11   ` Rusty Russell
  -- strict thread matches above, loose matches on Subject: below --
2009-09-09  8:11 Multiple Port Support for virtio-console Amit Shah
2009-09-09  8:12 ` [PATCH] virtio_console: Add support for multiple ports for generic guest and host communication Amit Shah
2009-09-09  8:12 ` Amit Shah

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='200909291456.56723.borntraeger__28247.7151988589$1254229199$gmane$org@de.ibm.com' \
    --to=borntraeger@de.ibm.com \
    --cc=alan@linux.intel.com \
    --cc=amit.shah@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=virtualization@lists.linux-foundation.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: link
Be 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.