Re: [Xen-devel] [PATCH 22/25 v6] xen/arm: vpl011: Add support for vuart console in xenconsole

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 21 Jul 2017, Julien Grall wrote:
> Hi,
> 
> On 18/07/17 21:07, Stefano Stabellini wrote:
> > On Mon, 17 Jul 2017, Bhupinder Thakur wrote:
> > > This patch finally adds the support for vuart console. It adds
> > > two new fields in the console initialization:
> > > 
> > > - optional
> > > - prefer_gnttab
> > > 
> > > optional flag tells whether the console is optional.
> > > 
> > > prefer_gnttab tells whether the ring buffer should be allocated using
> > > grant table.
> > > 
> > > Signed-off-by: Bhupinder Thakur <bhupinder.thakur@xxxxxxxxxx>
> > > ---
> > > CC: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > > CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > > CC: Julien Grall <julien.grall@xxxxxxx>
> > > 
> > > Changes since v4:
> > > - Renamed VUART_CFLAGS- to CFLAGS_vuart- in the Makefile as per the
> > > convention.
> > > 
> > >  config/arm32.mk           |  1 +
> > >  config/arm64.mk           |  1 +
> > >  tools/console/Makefile    |  3 ++-
> > >  tools/console/daemon/io.c | 29 ++++++++++++++++++++++++++++-
> > >  4 files changed, 32 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/config/arm32.mk b/config/arm32.mk
> > > index f95228e..b9f23fe 100644
> > > --- a/config/arm32.mk
> > > +++ b/config/arm32.mk
> > > @@ -1,5 +1,6 @@
> > >  CONFIG_ARM := y
> > >  CONFIG_ARM_32 := y
> > > +CONFIG_VUART_CONSOLE := y
> > >  CONFIG_ARM_$(XEN_OS) := y
> > > 
> > >  CONFIG_XEN_INSTALL_SUFFIX :=
> > 
> > What about leaving this off for ARM32 by default?
> 
> Why? This will only disable xenconsole changes and not the hypervisor. The
> changes are quite tiny, so I would even be in favor of enabling for all
> architectures.
> 
> Or are you suggesting to disable the VPL011 emulation in the hypervisor? But I
> don't see the emulation AArch64 specific, and a user could disable it if he
> doesn't want it...

I was thinking that the virtual pl011 is mostly useful for SBSA
compliance, which doesn't really apply to ARM32 (there are no ARM32 SBSA
compliant platforms as far as I am aware).

Given that we don't need vpl011 on ARM32, I thought we might as well
disable it. Less code the better. I wouldn't go as far as introducing
more #ifdefs to disable it, but I would make use of the existing config
options to turn it off by default on ARM32. Does it make sense?

That said, you are right that there is no point in disabling only
CONFIG_VUART_CONSOLE, which affects the tools only. We should really
disable SBSA_VUART_CONSOLE by default on ARM32. In fact, ideally
CONFIG_VUART_CONSOLE would be set dependning on the value of
SBSA_VUART_CONSOLE. What do you think?

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel