Re: [Xen-devel] [PATCH 06/11] xen/arm: vpl011: Add a new pl011 uart node in the guest DT in the toolstack

[Julien Grall <julien.grall@xxxxxxx>]

Hi Bhupinder,

CC toolstack maintainers.

On 02/21/2017 11:26 AM, Bhupinder Thakur wrote:
> Add a new pl011 uart node
>     - Get the pl011 spi virq from Xen using a hvm call
See my comment on previous patches.

>     - Add a new device tree node in the guest DT for SBSA pl011 uart containing 
> the IRQ
>       (read above) and the MMIO address range to be used by the guest
>
> The format for the node is specified in 
> Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt.
The ordering of the patches in this series is a bit weird. You are 
exposing the pl011 to the guest before all the code to handle it is 
there. This patch should likely be at the end of this series.
Also, you want a libxl option to allow the user enabling/disable pl011 
emulation. We likely want this emulation disable by default.
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@xxxxxxxxxx>
> ---
>  tools/libxl/libxl_arm.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index d842d88..34c7e39 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -130,9 +130,10 @@ static struct arch_info {
>      const char *guest_type;
>      const char *timer_compat;
>      const char *cpu_compat;
> +    const char *uart_compat;
>  } arch_info[] = {
> -    {"xen-3.0-armv7l",  "arm,armv7-timer", "arm,cortex-a15" },
> -    {"xen-3.0-aarch64", "arm,armv8-timer", "arm,armv8" },
> +    {"xen-3.0-armv7l",  "arm,armv7-timer", "arm,cortex-a15", "arm,sbsa-uart" },
> +    {"xen-3.0-aarch64", "arm,armv8-timer", "arm,armv8", "arm,sbsa-uart" },
>  };
>
>  /*
> @@ -590,6 +591,38 @@ static int make_hypervisor_node(libxl__gc *gc, void *fdt,
>      return 0;
>  }
>
> +static int make_vpl011_uart_node(libxl__gc *gc, void *fdt,
> +                           const struct arch_info *ainfo,
> +                           struct xc_dom_image *dom, uint64_t irq)
Why uint64_t for irq?

> +{
> +    int res;
> +    gic_interrupt intr;
> +
> +    res = fdt_begin_node(fdt, "sbsa-pl011");
> +    if (res) return res;
> +
> +    res = fdt_property_compat(gc, fdt, 1, ainfo->uart_compat);
> +    if (res) return res;
> +
> +    res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS,
> +                            1,
> +                            GUEST_PL011_BASE, GUEST_PL011_SIZE);
> +    if (res)
> +        return res;
> +
> +    set_interrupt(intr, irq, 0xf, DT_IRQ_TYPE_LEVEL_HIGH);
> +
> +    res = fdt_property_interrupts(gc, fdt, &intr, 1);
> +    if (res) return res;
> +
> +    fdt_property_u32(fdt, "current-speed", 115200);
> +
> +    res = fdt_end_node(fdt);
> +    if (res) return res;
> +
> +    return 0;
> +}
> +
>  static const struct arch_info *get_arch_info(libxl__gc *gc,
>                                               const struct xc_dom_image *dom)
>  {
> @@ -790,6 +823,7 @@ static int libxl__prepare_dtb(libxl__gc *gc, 
> libxl_domain_build_info *info,
>      int rc, res;
>      size_t fdt_size = 0;
>      int pfdt_size = 0;
> +    uint64_t vpl011_irq=0;
>
>      const libxl_version_info *vers;
>      const struct arch_info *ainfo;
> @@ -889,6 +923,13 @@ next_resize:
>          FDT( make_timer_node(gc, fdt, ainfo, xc_config->clock_frequency) );
>          FDT( make_hypervisor_node(gc, fdt, vers) );
>
> +        /*
> +         * get the vpl011 VIRQ and use it for creating a vpl011 node entry
> +         */
> +        if ( !xc_hvm_param_get(dom->xch, dom->guest_domid, 
> HVM_PARAM_VPL011_VIRQ,
> +                                                               &vpl011_irq) )
> +            FDT( make_vpl011_uart_node(gc, fdt, ainfo, dom, vpl011_irq) );
> +
>          if (pfdt)
>              FDT( copy_partial_fdt(gc, fdt, pfdt) );
>
> @@ -933,9 +974,11 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc,
>      val |= GUEST_EVTCHN_PPI;
>      rc = xc_hvm_param_set(dom->xch, dom->guest_domid, HVM_PARAM_CALLBACK_IRQ,
>                            val);
> +
>      if (rc)
>          return rc;
>
> +
>      rc = libxl__prepare_dtb(gc, info, state, dom);
>      if (rc) goto out;
Cheers,

--
Julien Grall

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