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

Laurent Vivier posted 1 patch 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170926121750.9015-1-lvivier@redhat.com
Test checkpatch passed
Test docker passed
Test s390x passed
hw/char/spapr_vty.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH] spapr: select default vty
Posted by Laurent Vivier 6 years, 6 months ago
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
Posted by David Gibson 6 years, 6 months ago
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
Re: [Qemu-devel] [PATCH] spapr: select default vty
Posted by Laurent Vivier 6 years, 6 months ago
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