diff mbox series

[07/17] serial: serial_xen: Add Xen PV serial driver

Message ID 20200701162959.9814-8-vicooodin@gmail.com
State New
Headers show
Series Add new board: Xen guest for ARM64 | expand

Commit Message

Nastya Vicodin July 1, 2020, 4:29 p.m. UTC
From: Peng Fan <peng.fan at nxp.com>

Add support for Xen para-virtualized serial driver. This
driver fully supports serial console for the virtual machine.

Please note that as the driver is initialized late, so no banner
nor memory size is visible.

Signed-off-by: Peng Fan <peng.fan at nxp.com>
Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko at epam.com>
Signed-off-by: Anastasiia Lukianenko <anastasiia_lukianenko at epam.com>
---
 arch/arm/Kconfig                          |   1 +
 board/xen/xenguest_arm64/xenguest_arm64.c |  31 +++-
 configs/xenguest_arm64_defconfig          |   4 +-
 drivers/serial/Kconfig                    |   7 +
 drivers/serial/Makefile                   |   1 +
 drivers/serial/serial_xen.c               | 175 ++++++++++++++++++++++
 drivers/xen/events.c                      |   4 +
 7 files changed, 214 insertions(+), 9 deletions(-)
 create mode 100644 drivers/serial/serial_xen.c

Comments

Simon Glass July 3, 2020, 3:50 a.m. UTC | #1
Hi Anastasiia,

On Wed, 1 Jul 2020 at 10:30, Anastasiia Lukianenko <vicooodin at gmail.com> wrote:
>
> From: Peng Fan <peng.fan at nxp.com>
>
> Add support for Xen para-virtualized serial driver. This
> driver fully supports serial console for the virtual machine.
>
> Please note that as the driver is initialized late, so no banner
> nor memory size is visible.
>
> Signed-off-by: Peng Fan <peng.fan at nxp.com>
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko at epam.com>
> Signed-off-by: Anastasiia Lukianenko <anastasiia_lukianenko at epam.com>
> ---
>  arch/arm/Kconfig                          |   1 +
>  board/xen/xenguest_arm64/xenguest_arm64.c |  31 +++-
>  configs/xenguest_arm64_defconfig          |   4 +-
>  drivers/serial/Kconfig                    |   7 +
>  drivers/serial/Makefile                   |   1 +
>  drivers/serial/serial_xen.c               | 175 ++++++++++++++++++++++
>  drivers/xen/events.c                      |   4 +
>  7 files changed, 214 insertions(+), 9 deletions(-)
>  create mode 100644 drivers/serial/serial_xen.c

Reviewed-by: Simon Glass <sjg at chromium.org>

nits below

>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index c469863967..d4de1139aa 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1723,6 +1723,7 @@ config TARGET_XENGUEST_ARM64
>         select XEN
>         select OF_CONTROL
>         select LINUX_KERNEL_IMAGE_HEADER
> +       select XEN_SERIAL
>  endchoice
>
>  config ARCH_SUPPORT_TFABOOT
> diff --git a/board/xen/xenguest_arm64/xenguest_arm64.c b/board/xen/xenguest_arm64/xenguest_arm64.c
> index 9e099f388f..fd10a002e9 100644
> --- a/board/xen/xenguest_arm64/xenguest_arm64.c
> +++ b/board/xen/xenguest_arm64/xenguest_arm64.c
> @@ -18,9 +18,12 @@
>  #include <asm/armv8/mmu.h>
>  #include <asm/xen.h>
>  #include <asm/xen/hypercall.h>
> +#include <asm/xen/system.h>
>
>  #include <linux/compiler.h>
>
> +#include <xen/hvm.h>
> +
>  DECLARE_GLOBAL_DATA_PTR;
>
>  int board_init(void)
> @@ -57,9 +60,28 @@ static int get_next_memory_node(const void *blob, int mem)
>
>  static int setup_mem_map(void)
>  {
> -       int i, ret, mem, reg = 0;
> +       int i = 0, ret, mem, reg = 0;
>         struct fdt_resource res;
>         const void *blob = gd->fdt_blob;
> +       u64 gfn;
> +
> +       /*
> +        * Add "magic" region which is used by Xen to provide some essentials
> +        * for the guest: we need console.
> +        */
> +       ret = hvm_get_parameter_maintain_dcache(HVM_PARAM_CONSOLE_PFN, &gfn);
> +       if (ret < 0) {
> +               printf("%s: Can't get HVM_PARAM_CONSOLE_PFN, ret %d\n",
> +                      __func__, ret);
> +               return -EINVAL;
> +       }
> +
> +       xen_mem_map[i].virt = PFN_PHYS(gfn);
> +       xen_mem_map[i].phys = PFN_PHYS(gfn);
> +       xen_mem_map[i].size = PAGE_SIZE;
> +       xen_mem_map[i].attrs = (PTE_BLOCK_MEMTYPE(MT_NORMAL) |
> +                               PTE_BLOCK_INNER_SHARE);
> +       i++;
>
>         mem = get_next_memory_node(blob, -1);
>         if (mem < 0) {
> @@ -67,7 +89,7 @@ static int setup_mem_map(void)
>                 return -EINVAL;
>         }
>
> -       for (i = 0; i < MAX_MEM_MAP_REGIONS; i++) {
> +       for (; i < MAX_MEM_MAP_REGIONS; i++) {
>                 ret = fdt_get_resource(blob, mem, "reg", reg++, &res);
>                 if (ret == -FDT_ERR_NOTFOUND) {
>                         reg = 0;
> @@ -146,8 +168,3 @@ int print_cpuinfo(void)
>         return 0;
>  }
>
> -__weak struct serial_device *default_serial_console(void)
> -{
> -       return NULL;
> -}
> -
> diff --git a/configs/xenguest_arm64_defconfig b/configs/xenguest_arm64_defconfig
> index 2a8caf8647..45559a161b 100644
> --- a/configs/xenguest_arm64_defconfig
> +++ b/configs/xenguest_arm64_defconfig
> @@ -47,9 +47,9 @@ CONFIG_CMD_UMS=n
>  #CONFIG_EFI_PARTITION=y
>  # CONFIG_EFI_LOADER is not set
>
> -# CONFIG_DM is not set
> +CONFIG_DM=y
>  # CONFIG_MMC is not set
> -# CONFIG_DM_SERIAL is not set
> +CONFIG_DM_SERIAL=y
>  # CONFIG_REQUIRE_SERIAL_CONSOLE is not set
>
>  CONFIG_OF_BOARD=y
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index 17d0e73623..33c989a66d 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -821,6 +821,13 @@ config MPC8XX_CONS
>         depends on MPC8xx
>         default y
>
> +config XEN_SERIAL
> +       bool "XEN serial support"
> +       depends on XEN
> +       help
> +         If built without DM support, then requires Xen
> +         to be built with CONFIG_VERBOSE_DEBUG.

Yes but what does it do? Also should probably not support non-DM at this point.

> +
>  choice
>         prompt "Console port"
>         default 8xx_CONS_SMC1
> diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
> index e4a92bbbb7..25f7f8d342 100644
> --- a/drivers/serial/Makefile
> +++ b/drivers/serial/Makefile
> @@ -70,6 +70,7 @@ obj-$(CONFIG_OWL_SERIAL) += serial_owl.o
>  obj-$(CONFIG_OMAP_SERIAL) += serial_omap.o
>  obj-$(CONFIG_MTK_SERIAL) += serial_mtk.o
>  obj-$(CONFIG_SIFIVE_SERIAL) += serial_sifive.o
> +obj-$(CONFIG_XEN_SERIAL) += serial_xen.o
>
>  ifndef CONFIG_SPL_BUILD
>  obj-$(CONFIG_USB_TTY) += usbtty.o
> diff --git a/drivers/serial/serial_xen.c b/drivers/serial/serial_xen.c
> new file mode 100644
> index 0000000000..dcd4b2df79
> --- /dev/null
> +++ b/drivers/serial/serial_xen.c
> @@ -0,0 +1,175 @@
> +/*
> + * SPDX-License-Identifier:    GPL-2.0+
> + *
> + * (C) 2018 NXP
> + * (C) 2020 EPAM Systems Inc.
> + */
> +#include <common.h>
> +#include <cpu_func.h>
> +#include <dm.h>
> +#include <serial.h>
> +#include <watchdog.h>
> +
> +#include <linux/bug.h>
> +
> +#include <xen/hvm.h>
> +#include <xen/events.h>
> +
> +#include <xen/interface/sched.h>
> +#include <xen/interface/hvm/hvm_op.h>
> +#include <xen/interface/hvm/params.h>
> +#include <xen/interface/io/console.h>
> +#include <xen/interface/io/ring.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +u32 console_evtchn;
> +
> +struct xen_uart_priv {
> +       struct xencons_interface *intf;
> +       u32 evtchn;
> +       int vtermno;
> +       struct hvc_struct *hvc;

comment for struct

> +};
> +
> +int xen_serial_setbrg(struct udevice *dev, int baudrate)
> +{
> +       return 0;
> +}
> +
> +static int xen_serial_probe(struct udevice *dev)
> +{
> +       struct xen_uart_priv *priv = dev_get_priv(dev);
> +       u64 v = 0;
> +       unsigned long gfn;
> +       int r;
> +
> +       r = hvm_get_parameter(HVM_PARAM_CONSOLE_EVTCHN, &v);

Can you use ret and val instead of single-char var names? It is OK for
loops, but not here.

> +       if (r < 0 || v == 0)
> +               return r;
> +
> +       priv->evtchn = v;
> +       console_evtchn = v;
> +
> +       r = hvm_get_parameter(HVM_PARAM_CONSOLE_PFN, &v);
> +       if (r < 0 || v == 0)
> +               return -ENODEV;

return r if non-zero

return -EINVAL perhaps or -ENXIO if !v

-ENODEV means there is no device and is reserved for driver model.
Clearly in this case there is a device.

> +
> +       gfn = v;
> +
> +       priv->intf = (struct xencons_interface *)(gfn << XEN_PAGE_SHIFT);
> +       if (!priv->intf)

Don't you already check for !v above?

> +               return -EINVAL;

Blank line

> +       return 0;
> +}
> +
> +static int xen_serial_pending(struct udevice *dev, bool input)
> +{
> +       struct xen_uart_priv *priv = dev_get_priv(dev);
> +       struct xencons_interface *intf = priv->intf;
> +
> +       if (!input || intf->in_cons == intf->in_prod)
> +               return 0;

blank line before final return. Please fix globally

> +       return 1;
> +}
> +
> +static int xen_serial_getc(struct udevice *dev)
> +{
> +       struct xen_uart_priv *priv = dev_get_priv(dev);
> +       struct xencons_interface *intf = priv->intf;
> +       XENCONS_RING_IDX cons;
> +       char c;
> +
> +       while (intf->in_cons == intf->in_prod) {
> +               mb(); /* wait */
> +       }

Drop {}. Has this been through patman?

> +
> +       cons = intf->in_cons;
> +       mb();                   /* get pointers before reading ring */
> +
> +       c = intf->in[MASK_XENCONS_IDX(cons++, intf->in)];
> +
> +       mb();                   /* read ring before consuming */
> +       intf->in_cons = cons;
> +
> +       notify_remote_via_evtchn(priv->evtchn);
> +       return c;
> +}
> +
> +static int __write_console(struct udevice *dev, const char *data, int len)
> +{
> +       struct xen_uart_priv *priv = dev_get_priv(dev);
> +       struct xencons_interface *intf = priv->intf;
> +       XENCONS_RING_IDX cons, prod;
> +       int sent = 0;
> +
> +       cons = intf->out_cons;
> +       prod = intf->out_prod;
> +       mb(); /* Update pointer */
> +
> +       WARN_ON((prod - cons) > sizeof(intf->out));
> +
> +       while ((sent < len) && ((prod - cons) < sizeof(intf->out)))
> +               intf->out[MASK_XENCONS_IDX(prod++, intf->out)] = data[sent++];
> +
> +       mb(); /* Update data before pointer */
> +       intf->out_prod = prod;
> +
> +       if (sent)
> +               notify_remote_via_evtchn(priv->evtchn);
> +       return sent;
> +}
> +
> +static int write_console(struct udevice *dev, const char *data, int len)
> +{
> +       /*
> +        * Make sure the whole buffer is emitted, polling if
> +        * necessary.  We don't ever want to rely on the hvc daemon
> +        * because the most interesting console output is when the
> +        * kernel is crippled.
> +        */
> +       while (len) {
> +               int sent = __write_console(dev, data, len);
> +
> +               data += sent;
> +               len -= sent;
> +
> +               if (unlikely(len))
> +                       HYPERVISOR_sched_op(SCHEDOP_yield, NULL);
> +       }
> +       return 0;
> +}
> +
> +static int xen_serial_putc(struct udevice *dev, const char ch)
> +{
> +       write_console(dev, &ch, 1);
> +       return 0;
> +}
> +
> +static const struct dm_serial_ops xen_serial_ops = {
> +       .putc = xen_serial_putc,
> +       .getc = xen_serial_getc,
> +       .pending = xen_serial_pending,
> +};
> +
> +#if CONFIG_IS_ENABLED(OF_CONTROL)
> +static const struct udevice_id xen_serial_ids[] = {
> +       { .compatible = "xen,xen" },
> +       { }
> +};
> +#endif
> +
> +U_BOOT_DRIVER(serial_xen) = {
> +       .name                   = "serial_xen",
> +       .id                     = UCLASS_SERIAL,
> +#if CONFIG_IS_ENABLED(OF_CONTROL)

of_patch_ptr() - but I think you can drop this

> +       .of_match               = xen_serial_ids,
> +#endif
> +       .priv_auto_alloc_size   = sizeof(struct xen_uart_priv),
> +       .probe                  = xen_serial_probe,
> +       .ops                    = &xen_serial_ops,
> +#if !CONFIG_IS_ENABLED(OF_CONTROL)

and this?

> +       .flags                  = DM_FLAG_PRE_RELOC,
> +#endif
> +};
> +
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index eddc6b6e29..a1b36a2196 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -25,6 +25,8 @@
>  #include <xen/events.h>
>  #include <xen/hvm.h>
>
> +extern u32 console_evtchn;
> +
>  #define NR_EVS 1024
>
>  /* this represents a event handler. Chaining or sharing is not allowed */
> @@ -47,6 +49,8 @@ void unbind_all_ports(void)
>         struct vcpu_info *vcpu_info = &s->vcpu_info[cpu];
>
>         for (i = 0; i < NR_EVS; i++) {
> +               if (i == console_evtchn)
> +                       continue;
>                 if (test_and_clear_bit(i, bound_ports)) {
>                         printf("port %d still bound!\n", i);
>                         unbind_evtchn(i);
> --
> 2.17.1
>

Regards,
Simon
Anastasiia Lukianenko July 3, 2020, 12:59 p.m. UTC | #2
Hello Simon,

On Thu, 2020-07-02 at 21:50 -0600, Simon Glass wrote:
> Hi Anastasiia,
> 
> On Wed, 1 Jul 2020 at 10:30, Anastasiia Lukianenko <
> vicooodin at gmail.com> wrote:
> > 
> > From: Peng Fan <peng.fan at nxp.com>
> > 
> > Add support for Xen para-virtualized serial driver. This
> > driver fully supports serial console for the virtual machine.
> > 
> > Please note that as the driver is initialized late, so no banner
> > nor memory size is visible.
> > 
> > Signed-off-by: Peng Fan <peng.fan at nxp.com>
> > Signed-off-by: Oleksandr Andrushchenko <
> > oleksandr_andrushchenko at epam.com>
> > Signed-off-by: Anastasiia Lukianenko <
> > anastasiia_lukianenko at epam.com>
> > ---
> >  arch/arm/Kconfig                          |   1 +
> >  board/xen/xenguest_arm64/xenguest_arm64.c |  31 +++-
> >  configs/xenguest_arm64_defconfig          |   4 +-
> >  drivers/serial/Kconfig                    |   7 +
> >  drivers/serial/Makefile                   |   1 +
> >  drivers/serial/serial_xen.c               | 175
> > ++++++++++++++++++++++
> >  drivers/xen/events.c                      |   4 +
> >  7 files changed, 214 insertions(+), 9 deletions(-)
> >  create mode 100644 drivers/serial/serial_xen.c
> 
> Reviewed-by: Simon Glass <sjg at chromium.org>
> 
> nits below
> 
> > 
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index c469863967..d4de1139aa 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -1723,6 +1723,7 @@ config TARGET_XENGUEST_ARM64
> >         select XEN
> >         select OF_CONTROL
> >         select LINUX_KERNEL_IMAGE_HEADER
> > +       select XEN_SERIAL
> >  endchoice
> > 
> >  config ARCH_SUPPORT_TFABOOT
> > diff --git a/board/xen/xenguest_arm64/xenguest_arm64.c
> > b/board/xen/xenguest_arm64/xenguest_arm64.c
> > index 9e099f388f..fd10a002e9 100644
> > --- a/board/xen/xenguest_arm64/xenguest_arm64.c
> > +++ b/board/xen/xenguest_arm64/xenguest_arm64.c
> > @@ -18,9 +18,12 @@
> >  #include <asm/armv8/mmu.h>
> >  #include <asm/xen.h>
> >  #include <asm/xen/hypercall.h>
> > +#include <asm/xen/system.h>
> > 
> >  #include <linux/compiler.h>
> > 
> > +#include <xen/hvm.h>
> > +
> >  DECLARE_GLOBAL_DATA_PTR;
> > 
> >  int board_init(void)
> > @@ -57,9 +60,28 @@ static int get_next_memory_node(const void
> > *blob, int mem)
> > 
> >  static int setup_mem_map(void)
> >  {
> > -       int i, ret, mem, reg = 0;
> > +       int i = 0, ret, mem, reg = 0;
> >         struct fdt_resource res;
> >         const void *blob = gd->fdt_blob;
> > +       u64 gfn;
> > +
> > +       /*
> > +        * Add "magic" region which is used by Xen to provide some
> > essentials
> > +        * for the guest: we need console.
> > +        */
> > +       ret =
> > hvm_get_parameter_maintain_dcache(HVM_PARAM_CONSOLE_PFN, &gfn);
> > +       if (ret < 0) {
> > +               printf("%s: Can't get HVM_PARAM_CONSOLE_PFN, ret
> > %d\n",
> > +                      __func__, ret);
> > +               return -EINVAL;
> > +       }
> > +
> > +       xen_mem_map[i].virt = PFN_PHYS(gfn);
> > +       xen_mem_map[i].phys = PFN_PHYS(gfn);
> > +       xen_mem_map[i].size = PAGE_SIZE;
> > +       xen_mem_map[i].attrs = (PTE_BLOCK_MEMTYPE(MT_NORMAL) |
> > +                               PTE_BLOCK_INNER_SHARE);
> > +       i++;
> > 
> >         mem = get_next_memory_node(blob, -1);
> >         if (mem < 0) {
> > @@ -67,7 +89,7 @@ static int setup_mem_map(void)
> >                 return -EINVAL;
> >         }
> > 
> > -       for (i = 0; i < MAX_MEM_MAP_REGIONS; i++) {
> > +       for (; i < MAX_MEM_MAP_REGIONS; i++) {
> >                 ret = fdt_get_resource(blob, mem, "reg", reg++,
> > &res);
> >                 if (ret == -FDT_ERR_NOTFOUND) {
> >                         reg = 0;
> > @@ -146,8 +168,3 @@ int print_cpuinfo(void)
> >         return 0;
> >  }
> > 
> > -__weak struct serial_device *default_serial_console(void)
> > -{
> > -       return NULL;
> > -}
> > -
> > diff --git a/configs/xenguest_arm64_defconfig
> > b/configs/xenguest_arm64_defconfig
> > index 2a8caf8647..45559a161b 100644
> > --- a/configs/xenguest_arm64_defconfig
> > +++ b/configs/xenguest_arm64_defconfig
> > @@ -47,9 +47,9 @@ CONFIG_CMD_UMS=n
> >  #CONFIG_EFI_PARTITION=y
> >  # CONFIG_EFI_LOADER is not set
> > 
> > -# CONFIG_DM is not set
> > +CONFIG_DM=y
> >  # CONFIG_MMC is not set
> > -# CONFIG_DM_SERIAL is not set
> > +CONFIG_DM_SERIAL=y
> >  # CONFIG_REQUIRE_SERIAL_CONSOLE is not set
> > 
> >  CONFIG_OF_BOARD=y
> > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> > index 17d0e73623..33c989a66d 100644
> > --- a/drivers/serial/Kconfig
> > +++ b/drivers/serial/Kconfig
> > @@ -821,6 +821,13 @@ config MPC8XX_CONS
> >         depends on MPC8xx
> >         default y
> > 
> > +config XEN_SERIAL
> > +       bool "XEN serial support"
> > +       depends on XEN
> > +       help
> > +         If built without DM support, then requires Xen
> > +         to be built with CONFIG_VERBOSE_DEBUG.
> 
> Yes but what does it do? Also should probably not support non-DM at
> this point.
> 

This will be removed at all as only DM based drivers are added.

> > +
> >  choice
> >         prompt "Console port"
> >         default 8xx_CONS_SMC1
> > diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
> > index e4a92bbbb7..25f7f8d342 100644
> > --- a/drivers/serial/Makefile
> > +++ b/drivers/serial/Makefile
> > @@ -70,6 +70,7 @@ obj-$(CONFIG_OWL_SERIAL) += serial_owl.o
> >  obj-$(CONFIG_OMAP_SERIAL) += serial_omap.o
> >  obj-$(CONFIG_MTK_SERIAL) += serial_mtk.o
> >  obj-$(CONFIG_SIFIVE_SERIAL) += serial_sifive.o
> > +obj-$(CONFIG_XEN_SERIAL) += serial_xen.o
> > 
> >  ifndef CONFIG_SPL_BUILD
> >  obj-$(CONFIG_USB_TTY) += usbtty.o
> > diff --git a/drivers/serial/serial_xen.c
> > b/drivers/serial/serial_xen.c
> > new file mode 100644
> > index 0000000000..dcd4b2df79
> > --- /dev/null
> > +++ b/drivers/serial/serial_xen.c
> > @@ -0,0 +1,175 @@
> > +/*
> > + * SPDX-License-Identifier:    GPL-2.0+
> > + *
> > + * (C) 2018 NXP
> > + * (C) 2020 EPAM Systems Inc.
> > + */
> > +#include <common.h>
> > +#include <cpu_func.h>
> > +#include <dm.h>
> > +#include <serial.h>
> > +#include <watchdog.h>
> > +
> > +#include <linux/bug.h>
> > +
> > +#include <xen/hvm.h>
> > +#include <xen/events.h>
> > +
> > +#include <xen/interface/sched.h>
> > +#include <xen/interface/hvm/hvm_op.h>
> > +#include <xen/interface/hvm/params.h>
> > +#include <xen/interface/io/console.h>
> > +#include <xen/interface/io/ring.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +u32 console_evtchn;
> > +
> > +struct xen_uart_priv {
> > +       struct xencons_interface *intf;
> > +       u32 evtchn;
> > +       int vtermno;
> > +       struct hvc_struct *hvc;
> 
> comment for struct

Ok, will add.

> 
> > +};
> > +
> > +int xen_serial_setbrg(struct udevice *dev, int baudrate)
> > +{
> > +       return 0;
> > +}
> > +
> > +static int xen_serial_probe(struct udevice *dev)
> > +{
> > +       struct xen_uart_priv *priv = dev_get_priv(dev);
> > +       u64 v = 0;
> > +       unsigned long gfn;
> > +       int r;
> > +
> > +       r = hvm_get_parameter(HVM_PARAM_CONSOLE_EVTCHN, &v);
> 
> Can you use ret and val instead of single-char var names? It is OK
> for
> loops, but not here.

Sure.

> 
> > +       if (r < 0 || v == 0)
> > +               return r;
> > +
> > +       priv->evtchn = v;
> > +       console_evtchn = v;
> > +
> > +       r = hvm_get_parameter(HVM_PARAM_CONSOLE_PFN, &v);
> > +       if (r < 0 || v == 0)
> > +               return -ENODEV;
> 
> return r if non-zero
> 
> return -EINVAL perhaps or -ENXIO if !v
> 
> -ENODEV means there is no device and is reserved for driver model.
> Clearly in this case there is a device.

Ok, makes sense.

> 
> > +
> > +       gfn = v;
> > +
> > +       priv->intf = (struct xencons_interface *)(gfn <<
> > XEN_PAGE_SHIFT);
> > +       if (!priv->intf)
> 
> Don't you already check for !v above?
> 
> > +               return -EINVAL;
> 
> Blank line

Ok, will fix.

> 
> > +       return 0;
> > +}
> > +
> > +static int xen_serial_pending(struct udevice *dev, bool input)
> > +{
> > +       struct xen_uart_priv *priv = dev_get_priv(dev);
> > +       struct xencons_interface *intf = priv->intf;
> > +
> > +       if (!input || intf->in_cons == intf->in_prod)
> > +               return 0;
> 
> blank line before final return. Please fix globally

Ok, will fix in the next version.

> 
> > +       return 1;
> > +}
> > +
> > +static int xen_serial_getc(struct udevice *dev)
> > +{
> > +       struct xen_uart_priv *priv = dev_get_priv(dev);
> > +       struct xencons_interface *intf = priv->intf;
> > +       XENCONS_RING_IDX cons;
> > +       char c;
> > +
> > +       while (intf->in_cons == intf->in_prod) {
> > +               mb(); /* wait */
> > +       }
> 
> Drop {}. Has this been through patman?

We used checkpatch before sending.

> 
> > +
> > +       cons = intf->in_cons;
> > +       mb();                   /* get pointers before reading ring
> > */
> > +
> > +       c = intf->in[MASK_XENCONS_IDX(cons++, intf->in)];
> > +
> > +       mb();                   /* read ring before consuming */
> > +       intf->in_cons = cons;
> > +
> > +       notify_remote_via_evtchn(priv->evtchn);
> > +       return c;
> > +}
> > +
> > +static int __write_console(struct udevice *dev, const char *data,
> > int len)
> > +{
> > +       struct xen_uart_priv *priv = dev_get_priv(dev);
> > +       struct xencons_interface *intf = priv->intf;
> > +       XENCONS_RING_IDX cons, prod;
> > +       int sent = 0;
> > +
> > +       cons = intf->out_cons;
> > +       prod = intf->out_prod;
> > +       mb(); /* Update pointer */
> > +
> > +       WARN_ON((prod - cons) > sizeof(intf->out));
> > +
> > +       while ((sent < len) && ((prod - cons) < sizeof(intf->out)))
> > +               intf->out[MASK_XENCONS_IDX(prod++, intf->out)] =
> > data[sent++];
> > +
> > +       mb(); /* Update data before pointer */
> > +       intf->out_prod = prod;
> > +
> > +       if (sent)
> > +               notify_remote_via_evtchn(priv->evtchn);
> > +       return sent;
> > +}
> > +
> > +static int write_console(struct udevice *dev, const char *data,
> > int len)
> > +{
> > +       /*
> > +        * Make sure the whole buffer is emitted, polling if
> > +        * necessary.  We don't ever want to rely on the hvc daemon
> > +        * because the most interesting console output is when the
> > +        * kernel is crippled.
> > +        */
> > +       while (len) {
> > +               int sent = __write_console(dev, data, len);
> > +
> > +               data += sent;
> > +               len -= sent;
> > +
> > +               if (unlikely(len))
> > +                       HYPERVISOR_sched_op(SCHEDOP_yield, NULL);
> > +       }
> > +       return 0;
> > +}
> > +
> > +static int xen_serial_putc(struct udevice *dev, const char ch)
> > +{
> > +       write_console(dev, &ch, 1);
> > +       return 0;
> > +}
> > +
> > +static const struct dm_serial_ops xen_serial_ops = {
> > +       .putc = xen_serial_putc,
> > +       .getc = xen_serial_getc,
> > +       .pending = xen_serial_pending,
> > +};
> > +
> > +#if CONFIG_IS_ENABLED(OF_CONTROL)
> > +static const struct udevice_id xen_serial_ids[] = {
> > +       { .compatible = "xen,xen" },
> > +       { }
> > +};
> > +#endif
> > +
> > +U_BOOT_DRIVER(serial_xen) = {
> > +       .name                   = "serial_xen",
> > +       .id                     = UCLASS_SERIAL,
> > +#if CONFIG_IS_ENABLED(OF_CONTROL)
> 
> of_patch_ptr() - but I think you can drop this
> 
> > +       .of_match               = xen_serial_ids,
> > +#endif
> > +       .priv_auto_alloc_size   = sizeof(struct xen_uart_priv),
> > +       .probe                  = xen_serial_probe,
> > +       .ops                    = &xen_serial_ops,
> > +#if !CONFIG_IS_ENABLED(OF_CONTROL)
> 
> and this?
> 
> > +       .flags                  = DM_FLAG_PRE_RELOC,
> > +#endif
> > +};
> > +
> > diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> > index eddc6b6e29..a1b36a2196 100644
> > --- a/drivers/xen/events.c
> > +++ b/drivers/xen/events.c
> > @@ -25,6 +25,8 @@
> >  #include <xen/events.h>
> >  #include <xen/hvm.h>
> > 
> > +extern u32 console_evtchn;
> > +
> >  #define NR_EVS 1024
> > 
> >  /* this represents a event handler. Chaining or sharing is not
> > allowed */
> > @@ -47,6 +49,8 @@ void unbind_all_ports(void)
> >         struct vcpu_info *vcpu_info = &s->vcpu_info[cpu];
> > 
> >         for (i = 0; i < NR_EVS; i++) {
> > +               if (i == console_evtchn)
> > +                       continue;
> >                 if (test_and_clear_bit(i, bound_ports)) {
> >                         printf("port %d still bound!\n", i);
> >                         unbind_evtchn(i);
> > --
> > 2.17.1
> > 
> 
> Regards,
> Simon

Regards,
Anastasiia
diff mbox series

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index c469863967..d4de1139aa 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1723,6 +1723,7 @@  config TARGET_XENGUEST_ARM64
 	select XEN
 	select OF_CONTROL
 	select LINUX_KERNEL_IMAGE_HEADER
+	select XEN_SERIAL
 endchoice
 
 config ARCH_SUPPORT_TFABOOT
diff --git a/board/xen/xenguest_arm64/xenguest_arm64.c b/board/xen/xenguest_arm64/xenguest_arm64.c
index 9e099f388f..fd10a002e9 100644
--- a/board/xen/xenguest_arm64/xenguest_arm64.c
+++ b/board/xen/xenguest_arm64/xenguest_arm64.c
@@ -18,9 +18,12 @@ 
 #include <asm/armv8/mmu.h>
 #include <asm/xen.h>
 #include <asm/xen/hypercall.h>
+#include <asm/xen/system.h>
 
 #include <linux/compiler.h>
 
+#include <xen/hvm.h>
+
 DECLARE_GLOBAL_DATA_PTR;
 
 int board_init(void)
@@ -57,9 +60,28 @@  static int get_next_memory_node(const void *blob, int mem)
 
 static int setup_mem_map(void)
 {
-	int i, ret, mem, reg = 0;
+	int i = 0, ret, mem, reg = 0;
 	struct fdt_resource res;
 	const void *blob = gd->fdt_blob;
+	u64 gfn;
+
+	/*
+	 * Add "magic" region which is used by Xen to provide some essentials
+	 * for the guest: we need console.
+	 */
+	ret = hvm_get_parameter_maintain_dcache(HVM_PARAM_CONSOLE_PFN, &gfn);
+	if (ret < 0) {
+		printf("%s: Can't get HVM_PARAM_CONSOLE_PFN, ret %d\n",
+		       __func__, ret);
+		return -EINVAL;
+	}
+
+	xen_mem_map[i].virt = PFN_PHYS(gfn);
+	xen_mem_map[i].phys = PFN_PHYS(gfn);
+	xen_mem_map[i].size = PAGE_SIZE;
+	xen_mem_map[i].attrs = (PTE_BLOCK_MEMTYPE(MT_NORMAL) |
+				PTE_BLOCK_INNER_SHARE);
+	i++;
 
 	mem = get_next_memory_node(blob, -1);
 	if (mem < 0) {
@@ -67,7 +89,7 @@  static int setup_mem_map(void)
 		return -EINVAL;
 	}
 
-	for (i = 0; i < MAX_MEM_MAP_REGIONS; i++) {
+	for (; i < MAX_MEM_MAP_REGIONS; i++) {
 		ret = fdt_get_resource(blob, mem, "reg", reg++, &res);
 		if (ret == -FDT_ERR_NOTFOUND) {
 			reg = 0;
@@ -146,8 +168,3 @@  int print_cpuinfo(void)
 	return 0;
 }
 
-__weak struct serial_device *default_serial_console(void)
-{
-	return NULL;
-}
-
diff --git a/configs/xenguest_arm64_defconfig b/configs/xenguest_arm64_defconfig
index 2a8caf8647..45559a161b 100644
--- a/configs/xenguest_arm64_defconfig
+++ b/configs/xenguest_arm64_defconfig
@@ -47,9 +47,9 @@  CONFIG_CMD_UMS=n
 #CONFIG_EFI_PARTITION=y
 # CONFIG_EFI_LOADER is not set
 
-# CONFIG_DM is not set
+CONFIG_DM=y
 # CONFIG_MMC is not set
-# CONFIG_DM_SERIAL is not set
+CONFIG_DM_SERIAL=y
 # CONFIG_REQUIRE_SERIAL_CONSOLE is not set
 
 CONFIG_OF_BOARD=y
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 17d0e73623..33c989a66d 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -821,6 +821,13 @@  config MPC8XX_CONS
 	depends on MPC8xx
 	default y
 
+config XEN_SERIAL
+	bool "XEN serial support"
+	depends on XEN
+	help
+	  If built without DM support, then requires Xen
+	  to be built with CONFIG_VERBOSE_DEBUG.
+
 choice
 	prompt "Console port"
 	default 8xx_CONS_SMC1
diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
index e4a92bbbb7..25f7f8d342 100644
--- a/drivers/serial/Makefile
+++ b/drivers/serial/Makefile
@@ -70,6 +70,7 @@  obj-$(CONFIG_OWL_SERIAL) += serial_owl.o
 obj-$(CONFIG_OMAP_SERIAL) += serial_omap.o
 obj-$(CONFIG_MTK_SERIAL) += serial_mtk.o
 obj-$(CONFIG_SIFIVE_SERIAL) += serial_sifive.o
+obj-$(CONFIG_XEN_SERIAL) += serial_xen.o
 
 ifndef CONFIG_SPL_BUILD
 obj-$(CONFIG_USB_TTY) += usbtty.o
diff --git a/drivers/serial/serial_xen.c b/drivers/serial/serial_xen.c
new file mode 100644
index 0000000000..dcd4b2df79
--- /dev/null
+++ b/drivers/serial/serial_xen.c
@@ -0,0 +1,175 @@ 
+/*
+ * SPDX-License-Identifier:	GPL-2.0+
+ *
+ * (C) 2018 NXP
+ * (C) 2020 EPAM Systems Inc.
+ */
+#include <common.h>
+#include <cpu_func.h>
+#include <dm.h>
+#include <serial.h>
+#include <watchdog.h>
+
+#include <linux/bug.h>
+
+#include <xen/hvm.h>
+#include <xen/events.h>
+
+#include <xen/interface/sched.h>
+#include <xen/interface/hvm/hvm_op.h>
+#include <xen/interface/hvm/params.h>
+#include <xen/interface/io/console.h>
+#include <xen/interface/io/ring.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+u32 console_evtchn;
+
+struct xen_uart_priv {
+	struct xencons_interface *intf;
+	u32 evtchn;
+	int vtermno;
+	struct hvc_struct *hvc;
+};
+
+int xen_serial_setbrg(struct udevice *dev, int baudrate)
+{
+	return 0;
+}
+
+static int xen_serial_probe(struct udevice *dev)
+{
+	struct xen_uart_priv *priv = dev_get_priv(dev);
+	u64 v = 0;
+	unsigned long gfn;
+	int r;
+
+	r = hvm_get_parameter(HVM_PARAM_CONSOLE_EVTCHN, &v);
+	if (r < 0 || v == 0)
+		return r;
+
+	priv->evtchn = v;
+	console_evtchn = v;
+
+	r = hvm_get_parameter(HVM_PARAM_CONSOLE_PFN, &v);
+	if (r < 0 || v == 0)
+		return -ENODEV;
+
+	gfn = v;
+
+	priv->intf = (struct xencons_interface *)(gfn << XEN_PAGE_SHIFT);
+	if (!priv->intf)
+		return -EINVAL;
+	return 0;
+}
+
+static int xen_serial_pending(struct udevice *dev, bool input)
+{
+	struct xen_uart_priv *priv = dev_get_priv(dev);
+	struct xencons_interface *intf = priv->intf;
+
+	if (!input || intf->in_cons == intf->in_prod)
+		return 0;
+	return 1;
+}
+
+static int xen_serial_getc(struct udevice *dev)
+{
+	struct xen_uart_priv *priv = dev_get_priv(dev);
+	struct xencons_interface *intf = priv->intf;
+	XENCONS_RING_IDX cons;
+	char c;
+
+	while (intf->in_cons == intf->in_prod) {
+		mb(); /* wait */
+	}
+
+	cons = intf->in_cons;
+	mb();			/* get pointers before reading ring */
+
+	c = intf->in[MASK_XENCONS_IDX(cons++, intf->in)];
+
+	mb();			/* read ring before consuming */
+	intf->in_cons = cons;
+
+	notify_remote_via_evtchn(priv->evtchn);
+	return c;
+}
+
+static int __write_console(struct udevice *dev, const char *data, int len)
+{
+	struct xen_uart_priv *priv = dev_get_priv(dev);
+	struct xencons_interface *intf = priv->intf;
+	XENCONS_RING_IDX cons, prod;
+	int sent = 0;
+
+	cons = intf->out_cons;
+	prod = intf->out_prod;
+	mb(); /* Update pointer */
+
+	WARN_ON((prod - cons) > sizeof(intf->out));
+
+	while ((sent < len) && ((prod - cons) < sizeof(intf->out)))
+		intf->out[MASK_XENCONS_IDX(prod++, intf->out)] = data[sent++];
+
+	mb(); /* Update data before pointer */
+	intf->out_prod = prod;
+
+	if (sent)
+		notify_remote_via_evtchn(priv->evtchn);
+	return sent;
+}
+
+static int write_console(struct udevice *dev, const char *data, int len)
+{
+	/*
+	 * Make sure the whole buffer is emitted, polling if
+	 * necessary.  We don't ever want to rely on the hvc daemon
+	 * because the most interesting console output is when the
+	 * kernel is crippled.
+	 */
+	while (len) {
+		int sent = __write_console(dev, data, len);
+
+		data += sent;
+		len -= sent;
+
+		if (unlikely(len))
+			HYPERVISOR_sched_op(SCHEDOP_yield, NULL);
+	}
+	return 0;
+}
+
+static int xen_serial_putc(struct udevice *dev, const char ch)
+{
+	write_console(dev, &ch, 1);
+	return 0;
+}
+
+static const struct dm_serial_ops xen_serial_ops = {
+	.putc = xen_serial_putc,
+	.getc = xen_serial_getc,
+	.pending = xen_serial_pending,
+};
+
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+static const struct udevice_id xen_serial_ids[] = {
+	{ .compatible = "xen,xen" },
+	{ }
+};
+#endif
+
+U_BOOT_DRIVER(serial_xen) = {
+	.name			= "serial_xen",
+	.id			= UCLASS_SERIAL,
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+	.of_match		= xen_serial_ids,
+#endif
+	.priv_auto_alloc_size	= sizeof(struct xen_uart_priv),
+	.probe			= xen_serial_probe,
+	.ops			= &xen_serial_ops,
+#if !CONFIG_IS_ENABLED(OF_CONTROL)
+	.flags			= DM_FLAG_PRE_RELOC,
+#endif
+};
+
diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index eddc6b6e29..a1b36a2196 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -25,6 +25,8 @@ 
 #include <xen/events.h>
 #include <xen/hvm.h>
 
+extern u32 console_evtchn;
+
 #define NR_EVS 1024
 
 /* this represents a event handler. Chaining or sharing is not allowed */
@@ -47,6 +49,8 @@  void unbind_all_ports(void)
 	struct vcpu_info *vcpu_info = &s->vcpu_info[cpu];
 
 	for (i = 0; i < NR_EVS; i++) {
+		if (i == console_evtchn)
+			continue;
 		if (test_and_clear_bit(i, bound_ports)) {
 			printf("port %d still bound!\n", i);
 			unbind_evtchn(i);