[Qemu-devel] [PATCH] spapr: select default vty

[Qemu-devel] [PATCH] spapr: select default vty

[Laurent Vivier]

SLOF uses the "/chosen/linux,stdout-path" variable to
choose its console. This variable is provided by QEMU.
QEMU selects the spapr-vty using the "reg" property:
it takes the vty with the lowest reg number.
This patch allows the user to define "linux,stdout-path"
from the command line by adding a keyword 'default' to
the spapr-vty device.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/char/spapr_vty.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/char/spapr_vty.c b/hw/char/spapr_vty.c
index 0fa416ca6b..aa56a9a6cb 100644
--- a/hw/char/spapr_vty.c
+++ b/hw/char/spapr_vty.c
@@ -15,6 +15,7 @@ typedef struct VIOsPAPRVTYDevice {
     CharBackend chardev;
     uint32_t in, out;
     uint8_t buf[VTERM_BUFSIZE];
+    bool is_default;
 } VIOsPAPRVTYDevice;
 
 #define TYPE_VIO_SPAPR_VTY_DEVICE "spapr-vty"
@@ -153,6 +154,7 @@ void spapr_vty_create(VIOsPAPRBus *bus, Chardev *chardev)
 static Property spapr_vty_properties[] = {
     DEFINE_SPAPR_PROPERTIES(VIOsPAPRVTYDevice, sdev),
     DEFINE_PROP_CHR("chardev", VIOsPAPRVTYDevice, chardev),
+    DEFINE_PROP_BOOL("default", VIOsPAPRVTYDevice, is_default, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -194,11 +196,13 @@ static const TypeInfo spapr_vty_info = {
 VIOsPAPRDevice *spapr_vty_get_default(VIOsPAPRBus *bus)
 {
     VIOsPAPRDevice *sdev, *selected;
+    VIOsPAPRVTYDevice *dev;
     BusChild *kid;
 
     /*
      * To avoid the console bouncing around we want one VTY to be
-     * the "default". We haven't really got anything to go on, so
+     * the "default". If the user doesn't provide the information
+     * we haven't really got anything to go on, so
      * arbitrarily choose the one with the lowest reg value.
      */
 
@@ -213,6 +217,13 @@ VIOsPAPRDevice *spapr_vty_get_default(VIOsPAPRBus *bus)
 
         sdev = VIO_SPAPR_DEVICE(iter);
 
+        /* The user can provide the default console to use */
+
+        dev = VIO_SPAPR_VTY_DEVICE(sdev);
+        if (dev->is_default) {
+            return sdev;
+        }
+
         /* First VTY we've found, so it is selected for now */
         if (!selected) {
             selected = sdev;
-- 
2.13.5

Re: [Qemu-devel] [PATCH] spapr: select default vty

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2887 bytes --]

On Tue, Sep 26, 2017 at 02:17:50PM +0200, Laurent Vivier wrote:
> SLOF uses the "/chosen/linux,stdout-path" variable to
> choose its console. This variable is provided by QEMU.
> QEMU selects the spapr-vty using the "reg" property:
> it takes the vty with the lowest reg number.
> This patch allows the user to define "linux,stdout-path"
> from the command line by adding a keyword 'default' to
> the spapr-vty device.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

I'm a bit dubious about the worth of this.  With your SLOF fixes it
should still be possible to redirect output correctly using -prom-env,
yes?  Using a secondary vty is a pretty niche case, so I'm not
convinced we need extra properties just to make it simpler.
> ---
>  hw/char/spapr_vty.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/char/spapr_vty.c b/hw/char/spapr_vty.c
> index 0fa416ca6b..aa56a9a6cb 100644
> --- a/hw/char/spapr_vty.c
> +++ b/hw/char/spapr_vty.c
> @@ -15,6 +15,7 @@ typedef struct VIOsPAPRVTYDevice {
>      CharBackend chardev;
>      uint32_t in, out;
>      uint8_t buf[VTERM_BUFSIZE];
> +    bool is_default;
>  } VIOsPAPRVTYDevice;
>  
>  #define TYPE_VIO_SPAPR_VTY_DEVICE "spapr-vty"
> @@ -153,6 +154,7 @@ void spapr_vty_create(VIOsPAPRBus *bus, Chardev *chardev)
>  static Property spapr_vty_properties[] = {
>      DEFINE_SPAPR_PROPERTIES(VIOsPAPRVTYDevice, sdev),
>      DEFINE_PROP_CHR("chardev", VIOsPAPRVTYDevice, chardev),
> +    DEFINE_PROP_BOOL("default", VIOsPAPRVTYDevice, is_default, false),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -194,11 +196,13 @@ static const TypeInfo spapr_vty_info = {
>  VIOsPAPRDevice *spapr_vty_get_default(VIOsPAPRBus *bus)
>  {
>      VIOsPAPRDevice *sdev, *selected;
> +    VIOsPAPRVTYDevice *dev;
>      BusChild *kid;
>  
>      /*
>       * To avoid the console bouncing around we want one VTY to be
> -     * the "default". We haven't really got anything to go on, so
> +     * the "default". If the user doesn't provide the information
> +     * we haven't really got anything to go on, so
>       * arbitrarily choose the one with the lowest reg value.
>       */
>  
> @@ -213,6 +217,13 @@ VIOsPAPRDevice *spapr_vty_get_default(VIOsPAPRBus *bus)
>  
>          sdev = VIO_SPAPR_DEVICE(iter);
>  
> +        /* The user can provide the default console to use */
> +
> +        dev = VIO_SPAPR_VTY_DEVICE(sdev);
> +        if (dev->is_default) {
> +            return sdev;
> +        }
> +
>          /* First VTY we've found, so it is selected for now */
>          if (!selected) {
>              selected = sdev;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH] spapr: select default vty

[Laurent Vivier]

On 27/09/2017 09:59, David Gibson wrote:
> On Tue, Sep 26, 2017 at 02:17:50PM +0200, Laurent Vivier wrote:
>> SLOF uses the "/chosen/linux,stdout-path" variable to
>> choose its console. This variable is provided by QEMU.
>> QEMU selects the spapr-vty using the "reg" property:
>> it takes the vty with the lowest reg number.
>> This patch allows the user to define "linux,stdout-path"
>> from the command line by adding a keyword 'default' to
>> the spapr-vty device.
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> 
> I'm a bit dubious about the worth of this.  With your SLOF fixes it
> should still be possible to redirect output correctly using -prom-env,
> yes?  Using a secondary vty is a pretty niche case, so I'm not
> convinced we need extra properties just to make it simpler.

I agree, but there is a difference between the SLOF fix and the QEMU change:
- with the QEMU fix, all logs go immediately to the selected console,
- with SLOF fix, logs start to go on the console with the lowest reg
value, and switch later to the selected console.

Moreover, Thomas has pointed out that the SLOF fix slows the characters
output (x 6).

Thanks,
Laurent