Re: [Xen-devel] [PATCH v3 22/25] xen/arm: Allow vpl011 to be used by DomU

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 13 Aug 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 01/08/18 00:28, Stefano Stabellini wrote:
> > Make vpl011 being able to be used without a userspace component in Dom0.
> > In that case, output is printed to the Xen serial and input is received
> > from the Xen serial one character at a time.
> > 
> > Call domain_vpl011_init during construct_domU if vpl011 is enabled.
> > 
> > Introduce a new ring struct with only the ring array to avoid a waste of
> > memory. Introduce separate read_data and write_data functions for
> > initial domains: vpl011_write_data_xen is very simple and just writes
> > to the console, while vpl011_read_data_xen is a duplicate of
> > vpl011_read_data. Although textually almost identical, we are forced to
> > duplicate the functions because the struct layout is different.
> > 
> > Output characters are printed one by one, potentially leading to
> > intermixed output of different domains on the console. A follow-up patch
> > will solve the issue by introducing buffering.
> > 
> > Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
> > ---
> > Changes in v3:
> > - add in-code comments
> > - improve existing comments
> > - remove ifdef around domain_vpl011_init in construct_domU
> > - add ASSERT
> > - use SBSA_UART_FIFO_SIZE for in buffer size
> > - rename ring_enable to backend_in_domain
> > - rename struct xencons_in to struct vpl011_xen_backend
> > - rename inring field to xen
> > - rename helper functions accordingly
> > - remove unnecessary stub implementation of vpl011_rx_char
> > - move vpl011_rx_char_xen within the file to avoid the need of a forward
> >    declaration of vpl011_data_avail
> > - fix small bug in vpl011_rx_char_xen: increment in_prod before using it
> >    to check xencons_queued.
> > 
> > Changes in v2:
> > - only init if vpl011
> > - rename vpl011_read_char to vpl011_rx_char
> > - remove spurious change
> > - fix coding style
> > - use different ring struct
> > - move the write_data changes to their own function
> >    (vpl011_write_data_noring)
> > - duplicate vpl011_read_data
> > ---
> >   xen/arch/arm/domain_build.c  |   9 +-
> >   xen/arch/arm/vpl011.c        | 198
> > ++++++++++++++++++++++++++++++++++++++-----
> >   xen/include/asm-arm/vpl011.h |   8 ++
> >   3 files changed, 192 insertions(+), 23 deletions(-)
> > 
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index f9fa484..0888a76 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -2638,7 +2638,14 @@ static int __init construct_domU(struct domain *d,
> > struct dt_device_node *node)
> >       if ( rc < 0 )
> >           return rc;
> >   -    return __construct_domain(d, &kinfo);
> > +    rc = __construct_domain(d, &kinfo);
> > +    if ( rc < 0 )
> > +        return rc;
> > +
> > +    if ( kinfo.vpl011 )
> > +        rc = domain_vpl011_init(d, NULL);
> > +
> > +    return rc;
> >   }
> >     void __init create_domUs(void)
> > diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> > index 725a203..f206c61 100644
> > --- a/xen/arch/arm/vpl011.c
> > +++ b/xen/arch/arm/vpl011.c
> > @@ -77,6 +77,91 @@ static void vpl011_update_interrupt_status(struct domain
> > *d)
> >   #endif
> >   }
> >   +/*
> > + * vpl011_write_data_xen writes chars from the vpl011 out buffer to the
> > + * console. Only to be used when the backend is Xen.
> > + */
> > +static void vpl011_write_data_xen(struct domain *d, uint8_t data)
> > +{
> > +    unsigned long flags;
> > +    struct vpl011 *vpl011 = &d->arch.vpl011;
> > +
> > +    VPL011_LOCK(d, flags);
> > +
> > +    printk("%c", data);
> > +    if (data == '\n')
> > +        printk("DOM%u: ", d->domain_id);
> 
> There are a problem in this code. The first line of a domain will always
> printed without "DOM%u: " in front. This means you don't really know where it
> is coming from until you get the second line.

This problem is solved by the follow-up patch that introduces characters
buffering. I'll mention it in the commit message.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel