Re: [Xen-devel] [PATCH v4 16/23] xen/arm: generate vpl011 node on device tree for domU

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 15 Oct 2018, Julien Grall wrote:
> Hi,
> 
> On 05/10/2018 19:47, Stefano Stabellini wrote:
> > Introduce vpl011 support to guests started from Xen: it provides a
> > simple way to print output from a guest, as most guests come with a
> > pl011 driver. It is also able to provide a working console with
> > interrupt support.
> > 
> > The UART exposed to the guest is a SBSA compatible UART and not a PL011.
> > SBSA UART is a subset of PL011 r1p5. A full PL011 implementation in Xen
> > would just be too difficult, so guests may require some drivers changes.
> > 
> > Enable vpl011 conditionally if the user requested it.
> > 
> > Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
> > ---
> > Changes in v4:
> > - move rename set_interrupt_ppi and making set_interrupt_ppi generic to
> >    a separate patch
> > - properly name the vpl011 device node name
> > - code style
> > - use #define for addrcells and sizecells
> > 
> > Changes in v3:
> > - use bool
> > - retain BUG_ON(irq < 16)
> > - add vpl011 bool to kinfo
> > - return error of vpl011 is required but CONFIG_SBSA_VUART_CONSOLE is
> >    missing
> > 
> > Changes in v2:
> > - code style fixes
> > - make set_interrupt_ppi generic
> > - rename set_interrupt_ppi to set_interrupt
> > - only make the vpl011 node if the option was enabled
> > ---
> >   xen/arch/arm/domain_build.c | 61
> > +++++++++++++++++++++++++++++++++++++++++++++
> >   xen/arch/arm/kernel.h       |  3 +++
> >   2 files changed, 64 insertions(+)
> > 
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 760ebf8..049ab84 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -1605,6 +1605,54 @@ static int __init make_timer_domU_node(const struct
> > domain *d, void *fdt)
> >       return res;
> >   }
> >   +#ifdef CONFIG_SBSA_VUART_CONSOLE
> > +static int __init make_vpl011_uart_node(const struct domain *d, void *fdt)
> > +{
> > +    int res;
> > +    gic_interrupt_t intr;
> > +    __be32 reg[GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS];
> > +    __be32 *cells;
> > +
> > +    res = fdt_begin_node(fdt, "sbsa-uart@"__stringify(GUEST_PL011_BASE));
> > +    if ( res )
> > +        return res;
> > +
> > +    res = fdt_property_string(fdt, "compatible", "arm,sbsa-uart");
> > +    if ( res )
> > +        return res;
> > +
> > +    cells = &reg[0];
> > +    dt_child_set_range(&cells, GUEST_ROOT_ADDRESS_CELLS,
> > +                       GUEST_ROOT_SIZE_CELLS, GUEST_PL011_BASE,
> > +                       GUEST_PL011_SIZE);
> > +    if ( res )
> > +        return res;
> > +    res = fdt_property(fdt, "reg", reg, sizeof(reg));
> > +    if ( res )
> > +        return res;
> > +
> > +    set_interrupt(intr, GUEST_VPL011_SPI, 0xf, DT_IRQ_TYPE_LEVEL_HIGH);
> > +
> > +    res = fdt_property(fdt, "interrupts", intr, sizeof (intr));
> > +    if ( res )
> > +        return res;
> > +
> > +    res = fdt_property_cell(fdt, "interrupt-parent",
> > +                            GUEST_PHANDLE_GIC);
> > +    if ( res )
> > +        return res;
> > +
> > +    /* Use a default baud rate of 115200. */
> > +    fdt_property_u32(fdt, "current-speed", 115200);
> > +
> > +    res = fdt_end_node(fdt);
> > +    if ( res )
> > +        return res;
> > +
> > +    return 0;
> > +}
> > +#endif
> > +
> >   /*
> >    * The max size for DT is 2MB. However, the generated DT is small, 4KB
> >    * are enough for now, but we might have to increase it in the future.
> > @@ -1666,6 +1714,16 @@ static int __init prepare_dtb_domU(struct domain *d,
> > struct kernel_info *kinfo)
> >       if ( ret )
> >           goto err;
> >   +    if ( kinfo->vpl011 )
> > +    {
> > +        ret = -EINVAL;
> > +#ifdef CONFIG_SBSA_VUART_CONSOLE
> > +        ret = make_vpl011_uart_node(d, kinfo->fdt);
> > +#endif
> > +        if ( ret )
> > +            goto err;
> > +    }
> > +
> >       ret = fdt_end_node(kinfo->fdt);
> >       if ( ret < 0 )
> >           goto err;
> > @@ -2523,6 +2581,7 @@ static int __init construct_domU(struct domain *d,
> >       struct kernel_info kinfo = {};
> >       int rc;
> >       u64 mem;
> > +    u32 len;
> >         rc = dt_property_read_u64(node, "memory", &mem);
> >       if ( !rc )
> > @@ -2534,6 +2593,8 @@ static int __init construct_domU(struct domain *d,
> >         printk("*** LOADING DOMU cpus=%u memory=%luKB ***\n", d->max_vcpus,
> > mem);
> >   +    kinfo.vpl011 = dt_get_property(node, "vpl011", &len) != NULL;
> 
> You can use dt_property_read_bool here.

I'll do


> > +
> >       d->vcpu = xzalloc_array(struct vcpu *, d->max_vcpus);
> >       if ( !d->vcpu )
> >           return -ENOMEM;;
> > diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
> > index 4320f72..33f3e72 100644
> > --- a/xen/arch/arm/kernel.h
> > +++ b/xen/arch/arm/kernel.h
> > @@ -33,6 +33,9 @@ struct kernel_info {
> >       paddr_t dtb_paddr;
> >       paddr_t initrd_paddr;
> >   +    /* Enable pl011 emulation */
> > +    bool vpl011;
> > +
> >       /* loader to use for this kernel */
> >       void (*load)(struct kernel_info *info);
> >       /* loader specific state */
> > 
> 
> Cheers,
> 
> -- 
> Julien Grall
> 

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