Re: [Xen-devel] [PATCH 09/12 v3] xen/arm: vpl011: Add support for vuart in xenconsole

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 17/05/17 00:44, Stefano Stabellini wrote:
> On Wed, 10 May 2017, Bhupinder Thakur wrote:
> > Xenconsole supports only PV console currently. This patch adds support
> > for vuart console, which allows emulated pl011 UART to be accessed
> > as a console.
> >
> > Signed-off-by: Bhupinder Thakur <bhupinder.thakur@xxxxxxxxxx>
> > ---
> >
> > One review comment was to keep the vuart code under CONFIG_ARM64 && CONFIG_ACPI 
> > flags.
> > This code review could not be incorporated as I could not find out the 
> > appropriate flags
> > unders which this code can be kept. Are the CONFIG* flags exported to 
> > xenconsole?
> Not sure about ACPI, but CONFIG_ARM_64 definitely should be.
Why only CONFIG_ARM_64? I don't see any restriction preventing to be 
used for ARM32 bit also.
If we decide to limit to ARM_64 here, then we should do the same on the 
hypervisor side.
But I am wondering whether we should have a configure option for that as 
the hypervisor may not support pl011 (the user is allowed to disable it).
Cheers,

> This patch looks good, however I don't feel confident giving my acked-by
> until I can review properly the previous one.
>
>
> >  tools/console/daemon/io.c | 29 +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> >
> > diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
> > index 9bb14de..19a2f35 100644
> > --- a/tools/console/daemon/io.c
> > +++ b/tools/console/daemon/io.c
> > @@ -115,6 +115,7 @@ struct console_data {
> >  };
> >
> >  static int map_pvcon_ring_ref(struct console *, int );
> > +static int map_vuartcon_ring_ref(struct console *, int );
> >
> >  static struct console_data console_data[] = {
> >
> > @@ -124,6 +125,12 @@ static struct console_data console_data[] = {
> >                 .mapfunc = map_pvcon_ring_ref,
> >                 .mandatory = true
> >         },
> > +       {
> > +               .xsname = "/vuart/0",
> > +               .ttyname = "tty",
> > +               .mapfunc = map_vuartcon_ring_ref,
> > +               .mandatory = false
> > +       }
> >  };
> >
> >  #define MAX_CONSOLE (sizeof(console_data)/sizeof(struct console_data))
> > @@ -751,6 +758,28 @@ out:
> >         return err;
> >  }
> >
> > +static int map_vuartcon_ring_ref(struct console *con, int ring_ref)
> > +{
> > +       int err = 0;
> > +       struct domain *dom = con->d;
> > +
> > +       if (!con->interface) {
> > +               con->interface = xc_map_foreign_range(xc,
> > +                                                                                  
> >        dom->domid,
> > +                                                                               
> >           XC_PAGE_SIZE,
> > +                                                                               
> >           PROT_READ|PROT_WRITE,
> > +                                                                               
> >           (unsigned long)ring_ref);
> > +               if (con->interface == NULL) {
> > +                       err = EINVAL;
> > +                       goto out;
> > +               }
> > +               con->ring_ref = ring_ref;
> > +       }
> > +
> > +out:
> > +       return err;
> > +}
> > +
> >  static int console_create_ring(struct console *con)
> >  {
> >         int err, remote_port, ring_ref;
> > --
> > 2.7.4
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxx
> > https://lists.xen.org/xen-devel
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> https://lists.xen.org/xen-devel
--
Julien Grall

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