Re: [Xen-devel] [PATCH 00/25 v7] SBSA UART emulation support in Xen

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Aug 10, 2017 at 05:11:52PM +0100, Julien Grall wrote:
> 
> 
> On 10/08/17 17:00, Wei Liu wrote:
> > On Thu, Aug 10, 2017 at 03:26:07PM +0100, Julien Grall wrote:
> > > 
> > > 
> > > On 09/08/17 11:58, Bhupinder Thakur wrote:
> > > > Hi Julien,
> > > 
> > > Hi Bhupinder,
> > > 
> > > > Thanks for the testing.
> > > > 
> > > > On 8 August 2017 at 21:29, Julien Grall <julien.grall@xxxxxxx> wrote:
> > > > > Hi Bhupinder,
> > > > > 
> > > > > I gave another and I have a couple of comments.
> > > > > 
> > > > > Booting Linux with earlycon enabled take quite a while. I can see the
> > > > > characters coming slower than on the minitel. It seems to be a bit 
> > > > > better
> > > > > after switching off the bootconsole. Overall Linux is taking ~20 
> > > > > times to
> > > > > boot with pl011 vs HVC console.
> > > > > 
> > > > > I do agree that pl011 is emulated and therefore you have to trap 
> > > > > after each
> > > > > character. But 20 times sounds far too much.
> > > > > 
> > > > I think this slowness could be due to ratelimiting of the pl011 events
> > > > in xenconosle. Currently, the rate limit is
> > > > set to 30 events per 200 msecs (see 
> > > > RATE_LIMIT_ALLOWANCE/RATE_LIMIT_PERIOD).
> > > > 
> > > > I increased the rate limit to 600 events (30 * 20) per 200 msecs. With
> > > > this change,
> > > > I see that the the find command is running faster and smoother.
> > > > Earlier the find output would be jerky.
> > > 
> > > I think there might be another solution avoiding increasing the rate 
> > > limit.
> > > 
> > > If you look at the earlycon code for pl011 in Linux:
> > > 
> > > static void pl011_putc(struct uart_port *port, int c)
> > > {
> > >   while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
> > >           cpu_relax();
> > >   if (port->iotype == UPIO_MEM32)
> > >           writel(c, port->membase + UART01x_DR);
> > >   else
> > >           writeb(c, port->membase + UART01x_DR);
> > >   while (readl(port->membase + UART01x_FR) & UART01x_FR_BUSY)
> > >           cpu_relax();
> > > }
> > > 
> > > Linux will wait the UART to be idle before sending a new character.
> > > 
> > > Now looking at vpl011 emulation, the busy bit set when a new character is
> > > queued (see vpl011_write_data). This bit will only be cleared when the
> > > console daemon will raise an event and the queue is empty (see
> > > vpl011_data_avail).
> > > 
> > > This means for earlycon, you will need a round trip Guest -> Xen -> Dom0 
> > > ->
> > > Xen -> Guest for each single character. This is a bit counterproductive 
> > > and
> > > combined with the limit it makes it worse.
> > > 
> > > I would take a different approach on the BUSY bit. We can consider the 
> > > queue
> > > between Xen and xenconsoled as outside of the UART. If the character is
> > > queued, then job done. I think this would improve quite a lot of the
> > > performance.
> > 
> > Yes. This.
> > 
> > The guest sees a register, which is essentially a synchronous interface
> > to the guest. The current code, as you already see, will issue one event
> > for every character. That's excessive.
> 
> I am actually not suggesting to modify that at the moment. I think you may
> have other trouble with the interaction between the user and th console by
> doing that. Imagine you want to print the prompt, it may lag a bit before
> getting it.
> 
> The only thing I suggest is to not set the BUSY bit in the UART everytime a
> character is queued.
> 

Did you come to that conclusion that this would work by looking at the
spec or Linux source code? I think it should conform to the spec, not a
specific guest. But you're the maintainer, you have the final say.

> > 
> > The interface between Xen and xenconsoled can be asynchronous, it can
> > opt to queue X characters before sending an event, also setup a oneshot
> > timer to avoid hanging.
> > 
> > This however has some other implications -- it might not be as reliable
> > as the original method because data is not guaranteed to hit backend. If
> > the guest crashes very early on, depending the actual implementation you
> > might not be able get the data.
> 
> Would it be possible to ask xenconsoled to dump everything on domain crash?
> Some kind of synchronization.
> 

No, not at the moment. If the data is still in Xen and destroyed,
xenconsoled can't do anything.

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