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

[Julien Grall <julien.grall@arm.com>]

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.

After that I tried to stress the emulation a bit with "find ." to get a 
lot of output. And I noticed a lot of message similar to the one below 
on xen console:

d6v0 vpl011: Unexpected OUT ring buffer full

Associated to that the character have been eaten resulting to non-sense log.

A bit above the printk printing this message, there are a comment saying:

     /*
      * It is expected that the ring is not full when this function is 
called
      * as the guest is expected to write to the data register only when the
      * TXFF flag is not set.
      * In case the guest does write even when the TXFF flag is set then the
      * data will be silently dropped.
      */

I am quite surprised that Linux is not looking at the TXFF flags. So 
this needs some investigation.

Cheers,

On 07/08/17 09:52, Bhupinder Thakur wrote:
> SBSA UART emulation for guests in Xen
> ======================================
> Linaro has published VM System specification for ARM Processors, which
> provides a set of guidelines for both guest OS and hypervisor implementations,
> such that building OS images according to these guidelines guarantees
> that those images can also run on hypervisors compliant with this specification.
>
> One of the spec requirements is that the hypervisor must provide an
> emulated SBSA UART as a serial console which meets the minimum requirements in
> SBSA UART as defined in appendix B of the following
> ARM Server Base Architecture Document:
>
> https://static.docs.arm.com/den0029/a/Server_Base_System_Architecture_v3_1_ARM_DEN_0029A.pdf.
>
> This feature allows the Xen guests to use SBSA compliant UART as
> as a console.
>
> Note that SBSA UART is a subset of full featured ARM pl011 UART and
> supports only a subset of registers as mentioned below. It does not support
> rx/tx DMA.
>
> Currently, Xen supports paravirtualized (aka PV console) and an emulated serial
> consoles. This feature will expose an emulated SBSA UART console to the
> guest, which a user can access using xenconsole.
>
> The device tree passed to the guest VM will contain the SBSA UART MMIO address
> range and an irq for receiving rx/tx interrupts. The device tree format
> is specified in Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt.
>
> The Xen hypervisor will expose two types of interfaces to the backend and domU.
>
> The interface exposed to domU will be an emulated SBSA UART by emulating the
> access to the following registers by the guest.
>
> - Data register (DR)            - RW
> - Raw interrupt status register (RIS)   - RO
> - Masked interrupt status register (MIS)- RO
> - Interrupt Mask (IMSC)         - RW
> - Interrupt Clear (ICR)         - WO
>
> It will also inject the interrupts to the guest in the following
> conditions:
>
> - incoming data in the rx buffer for the guest
> - there is space in the tx buffer for the guest to write more data
>
> The interface exposed to the backend will be the same PV console interface,
> which minimizes the changes required in xenconsole to support a new SBSA UART
> console.
>
> This interface has rx and tx ring buffers and an event channel for
> sending/receiving events from the backend.
>
> So essentially Xen handles the data on behalf of domU and the backend. Any data
> written by domU is captured by Xen and written to the TX (OUT) ring buffer
> and an event is raised to the backend to read the TX ring buffer.
>
> Similarly on reciving an event from xenconsole, Xen injects an interrupt to guest to
> indicate there is data available in the RX (IN) ring buffer.
>
> The SBSA UART state is completely captured in the set of registers
> mentioned above and this state is updated everytime there is an event from
> the backend or there is register read/write access from domU.
>
> For example, if domU has masked the rx interrupt in the IMSC register, then Xen
> will not inject an interrupt to guest and will just update the RIS register.
> Once the interrupt is unmasked by guest, the interrupt will be delivered to the
> guest.
>
> Changes summary:
>
> Xen Hypervisor
> ===============
>
> 1. Add emulation code to emulate read/write access to SBSA UART registers and
>    interrupts:
>     - It emulates DR read/write by reading and writing from/to the IN and
>       OUT ring buffers and raising an event to dom0 when there is data in
>       the OUT ring buffer and injecting an interrupt to the guest when there
>       is data in the IN ring buffer.
>     - Other registers are related to interrupt management and essentially
>       control when interrupts are delivered to the guest.
>
> 2. Add a new domctl API to initialize SBSA UART emulation in Xen.
>
> 3. Enable SBSA UART emulation for a domain based on a libxl option passed during
>    domain creation.
>
> Toolstack
> ==========
>
> 1. Add a new option "vuart" in the domU configuration file to enable/disable vuart.
>
> 2. Create a SBSA UART DT node in the guest device tree. It uses a fixed
>    SPI IRQ number and MMIO address range for SBSA UART.
>
> 3. Call vuart init DOMCTL API to enable SBSA UART emulation.
>
> 5. Add a new vuart xenstore node, which contains:
>     - ring-ref
>     - event channel
>     - buffer limit
>     - type
>
> Xenconsoled
> ============
>
> 1. Split the domain structure to support multiple consoles.
>
> 2. Modify different APIs such as buffer_append() etc. to operate on the
>    console structure.
>
> 3. Add support for handling multiple consoles.
>
> 4. Add support for vuart console:
>
> The vpl011 changes available at the following repo:
>
> url: https://git@git.linaro.org:/people/bhupinder.thakur/xen.git
> branch: vpl011_v6
>
> Kindly wait for one day to checkout the code from the above URL.
>
> There are some TBD items which need to be looked at in the future:
>
> 1. Currently UEFI firmware logs the output to hvc console only. How can
>    UEFI firmware be made aware of pl011 console and how it can use it
>    as a console instead of hvc.
>
>    There was a discussion on this and it was decided that SBSA UART should
>    be used as a debug port by the UEFI firmware so that all debug output
>    is redirected to this port.
>
> 2. Linux seems to have hvc console as the default console i.e. if no
>    console is specified then it uses hvc as the console. How can an
>    option be provided in Linux to select either hvc or pl011 as the
>    default console.
>
>    It was suggeted to use the SPCR in ACPI and the stdout-path option in the
>    device tree to specify the default console. However, currently hvc console
>    is not describable in the ACPI/device tree. This support will have to be
>    added to allow the user to specify the default console.
>
> 3. ACPI support for pl011 device.
>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: George Dunlap <George.Dunlap@eu.citrix.com>
> CC: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Tim Deegan <tim@xen.org>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Julien Grall <julien.grall@arm.com>
> CC: Andre Przywara <andre.przywara@arm.com>
>
> Bhupinder Thakur (25):
>   xen/arm: vpl011: Define common ring buffer helper functions in
>     console.h
>   xen/arm: vpl011: Add SBSA UART emulation in Xen
>   xen/arm: vpl011: Allocate a new GFN in the toolstack for vuart
>   xen/arm: vpl011: Add support for vuart in libxl
>   xen/arm: vpl011: Rearrange xen header includes in alphabetical order
>     in domctl.c
>   xen/arm: vpl011: Add a new domctl API to initialize vpl011
>   xen/arm: vpl011: Add a new vuart node in the xenstore
>   xen/arm: vpl011: Modify xenconsole to define and use a new console
>     structure
>   xen/arm: vpl011: Rename the console structure field conspath to xspath
>   xen/arm: vpl011: Modify xenconsole functions to take console structure
>     as input
>   xen/arm: vpl011: Add a new console_init function in xenconsole
>   xen/arm: vpl011: Add a new buffer_available function in xenconsole
>   xen/arm: vpl011: Add a new maybe_add_console_evtchn_fd function in
>     xenconsole
>   xen/arm: vpl011: Add a new maybe_add_console_tty_fd function in
>     xenconsole
>   xen/arm: vpl011: Add a new console_evtchn_unmask function in
>     xenconsole
>   xen/arm: vpl011: Add a new handle_console_ring function in xenconsole
>   xen/arm: vpl011: Add a new handle_console_tty function in xenconsole
>   xen/arm: vpl011: Add a new console_cleanup function in xenconsole
>   xen/arm: vpl011: Add a new console_open_log function in xenconsole
>   xen/arm: vpl011: Add a new console_close_evtchn function in xenconsole
>   xen/arm: vpl011: Add support for multiple consoles in xenconsole
>   xen/arm: vpl011: Add support for vuart console in xenconsole
>   xen/arm: vpl011: Add a new vuart console type to xenconsole client
>   xen/arm: vpl011: Add a pl011 uart DT node in the guest device tree
>   xen/arm: vpl011: Update documentation for vuart console support
>
>  config/arm32.mk                      |   1 +
>  config/arm64.mk                      |   1 +
>  docs/man/xl.cfg.pod.5.in             |  12 +
>  docs/misc/console.txt                |  44 ++-
>  tools/console/Makefile               |   3 +-
>  tools/console/client/main.c          |  13 +-
>  tools/console/daemon/io.c            | 659 +++++++++++++++++++++++------------
>  tools/libxc/include/xc_dom.h         |   2 +
>  tools/libxc/include/xenctrl.h        |  20 ++
>  tools/libxc/xc_dom_arm.c             |   5 +-
>  tools/libxc/xc_dom_boot.c            |   2 +
>  tools/libxc/xc_domain.c              |  25 ++
>  tools/libxl/libxl.h                  |   5 +
>  tools/libxl/libxl_arch.h             |   7 +
>  tools/libxl/libxl_arm.c              |  74 ++++
>  tools/libxl/libxl_console.c          |  47 +++
>  tools/libxl/libxl_create.c           |   9 +-
>  tools/libxl/libxl_device.c           |   9 +-
>  tools/libxl/libxl_dom.c              |   5 +
>  tools/libxl/libxl_internal.h         |   6 +
>  tools/libxl/libxl_types.idl          |   7 +
>  tools/libxl/libxl_types_internal.idl |   1 +
>  tools/libxl/libxl_x86.c              |   8 +
>  tools/xl/xl_cmdtable.c               |   2 +-
>  tools/xl/xl_console.c                |   5 +-
>  tools/xl/xl_parse.c                  |   8 +
>  xen/arch/arm/Kconfig                 |   7 +
>  xen/arch/arm/Makefile                |   1 +
>  xen/arch/arm/domain.c                |   6 +
>  xen/arch/arm/domctl.c                |  48 ++-
>  xen/arch/arm/vpl011.c                | 454 ++++++++++++++++++++++++
>  xen/include/asm-arm/domain.h         |   6 +
>  xen/include/asm-arm/pl011-uart.h     |   2 +
>  xen/include/asm-arm/vpl011.h         |  72 ++++
>  xen/include/public/arch-arm.h        |   6 +
>  xen/include/public/domctl.h          |  21 ++
>  xen/include/public/io/console.h      |   4 +
>  37 files changed, 1366 insertions(+), 241 deletions(-)
>  create mode 100644 xen/arch/arm/vpl011.c
>  create mode 100644 xen/include/asm-arm/vpl011.h
>

-- 
Julien Grall

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