Re: [Xen-devel] [PATCH v2 18/21] xen/arm: Allow vpl011 to be used by DomU

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 27/07/18 01:10, Stefano Stabellini wrote:
> On Tue, 24 Jul 2018, Julien Grall wrote:
> > On 07/07/18 00:12, Stefano Stabellini wrote:
> > > +    VPL011_UNLOCK(d, flags);
> > > +}
> > > +
> > > +static void vpl011_write_data_noring(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);
> > You want to buffer the characters here and only print on newline or when the
> > buffer is full. Otherwise characters will get mangled with the Xen console
> > output or other domains output.
> I did the implementation, but then when I type characters at the domain
> prompt, I don't see them anymore one by one. Only after I press
> "enter". That makes the domain console not very usable. Should we keep
> it as?
I haven't thought about that case. However, if you don't implement the 
buffer solution, you will get all the domain consoles mangled during 
boot. This is not really nice for debugging. A potential solution is to 
buffer for all the domains but the one that is reading characters.
> > > +
> > > +    vpl011->uartris |= TXI;
> > > +    vpl011->uartfr &= ~TXFE;
> > > +    vpl011_update_interrupt_status(d);
> > > +
> > > +    VPL011_UNLOCK(d, flags);
> > > +}
> > > +
> > > +static uint8_t vpl011_read_data_inring(struct domain *d)
> > > +{
> > I think you can avoid the duplication here by using a macro.
> I prefer to avoid MACROS for things like this. I would rather reuse the
> existing function for both cases like in v1. Would you be OK to go back
> to that?
I would rather keep the duplication over going back to v1.

I may have another way to keep the code common, but let's look at that 
in a latter patch. For now, I will deal with the duplication.
Cheers,

--
Julien Grall

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