From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anastasiia Lukianenko Date: Fri, 3 Jul 2020 12:59:35 +0000 Subject: [PATCH 07/17] serial: serial_xen: Add Xen PV serial driver In-Reply-To: References: <20200701162959.9814-1-vicooodin@gmail.com> <20200701162959.9814-8-vicooodin@gmail.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > > > > 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 > > 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 > > 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 > > #include > > #include > > +#include > > > > #include > > > > +#include > > + > > 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 > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > + > > +#include > > +#include > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +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 > > #include > > > > +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