Message ID | 20170926121750.9015-1-lvivier@redhat.com |
---|---|
State | New |
Headers | show |
Series | spapr: select default vty | expand |
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;
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
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;
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(-)