Re: [Xen-devel] [PATCH v5 22/25] xen/arm: Allow vpl011 to be used by DomU
On Mon, Oct 29, 2018 at 10:03 PM Stefano Stabellini
wrote:
>
> On Wed, 24 Oct 2018, Oleksandr Tyshchenko wrote:
> > Hi, Stefano
> >
> > On Tue, Oct 23, 2018 at 5:04 AM Stefano Stabellini
> > wrote:
> > >
> > > Make vpl011 being able to be used without a userspace component in Dom0.
> > > In that case, output is printed to the Xen serial and input is received
> > > from the Xen serial one character at a time.
> > >
> > > Call domain_vpl011_init during construct_domU if vpl011 is enabled.
> > >
> > > Introduce a new ring struct with only the ring array to avoid a waste of
> > > memory. Introduce separate read_data and write_data functions for
> > > initial domains: vpl011_write_data_xen is very simple and just writes
> > > to the console, while vpl011_read_data_xen is a duplicate of
> > > vpl011_read_data. Although textually almost identical, we are forced to
> > > duplicate the functions because the struct layout is different.
> > >
> > > Output characters are printed one by one, potentially leading to
> > > intermixed output of different domains on the console. A follow-up patch
> > > will solve the issue by introducing buffering.
> > >
> > > Signed-off-by: Stefano Stabellini
> > > Acked-by: Julien Grall
> > > ---
> > > Changes in v5:
> > > - renable call to vpl011_rx_char_xen from console.c
> > >
> > > Changes in v4:
> > > - code style
> > >
> > > Changes in v3:
> > > - add in-code comments
> > > - improve existing comments
> > > - remove ifdef around domain_vpl011_init in construct_domU
> > > - add ASSERT
> > > - use SBSA_UART_FIFO_SIZE for in buffer size
> > > - rename ring_enable to backend_in_domain
> > > - rename struct xencons_in to struct vpl011_xen_backend
> > > - rename inring field to xen
> > > - rename helper functions accordingly
> > > - remove unnecessary stub implementation of vpl011_rx_char
> > > - move vpl011_rx_char_xen within the file to avoid the need of a forward
> > > declaration of vpl011_data_avail
> > > - fix small bug in vpl011_rx_char_xen: increment in_prod before using it
> > > to check xencons_queued.
> > >
> > > Changes in v2:
> > > - only init if vpl011
> > > - rename vpl011_read_char to vpl011_rx_char
> > > - remove spurious change
> > > - fix coding style
> > > - use different ring struct
> > > - move the write_data changes to their own function
> > > (vpl011_write_data_noring)
> > > - duplicate vpl011_read_data
> > > ---
> > > xen/arch/arm/domain_build.c | 9 +-
> > > xen/arch/arm/vpl011.c| 200
> > > +--
> > > xen/drivers/char/console.c | 2 +-
> > > xen/include/asm-arm/vpl011.h | 8 ++
> > > 4 files changed, 193 insertions(+), 26 deletions(-)
> > >
> > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > > index 6026b77..9ffd919 100644
> > > --- a/xen/arch/arm/domain_build.c
> > > +++ b/xen/arch/arm/domain_build.c
> > > @@ -2629,7 +2629,14 @@ static int __init construct_domU(struct domain *d,
> > > if ( rc < 0 )
> > > return rc;
> > >
> > > -return construct_domain(d, &kinfo);
> > > +rc = construct_domain(d, &kinfo);
> > > +if ( rc < 0 )
> > > +return rc;
> > > +
> > > +if ( kinfo.vpl011 )
> > > +rc = domain_vpl011_init(d, NULL);
> > > +
> > > +return rc;
> > > }
> > >
> > > void __init create_domUs(void)
> > > diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> > > index 68452a8..131507e 100644
> > > --- a/xen/arch/arm/vpl011.c
> > > +++ b/xen/arch/arm/vpl011.c
> > > @@ -77,6 +77,91 @@ static void vpl011_update_interrupt_status(struct
> > > domain *d)
> > > #endif
> > > }
> > >
> > > +/*
> > > + * vpl011_write_data_xen writes chars from the vpl011 out buffer to the
> > > + * console. Only to be used when the backend is Xen.
> > > + */
> > > +static void vpl011_write_data_xen(struct domain *d, uint8_t data)
> > > +{
> > > +unsigned long flags;
> > > +struct vpl011 *vpl011 = &d->arch.vpl011;
> > > +
> > > +VPL011_LOCK(d, flags);
> > > +
> > > +printk("%c", data);
> > > +if (data == '\n')
> > > +printk("DOM%u: ", d->domain_id);
> > > +
> > > +vpl011->uartris |= TXI;
> > > +vpl011->uartfr &= ~TXFE;
> > > +vpl011_update_interrupt_status(d);
> > > +
> > > +VPL011_UNLOCK(d, flags);
> > > +}
> > > +
> > > +/*
> > > + * vpl011_read_data_xen reads data when the backend is xen. Characters
> > > + * are added to the vpl011 receive buffer by vpl011_rx_char_xen.
> > > + */
> > > +static uint8_t vpl011_read_data_xen(struct domain *d)
> > > +{
> > > +unsigned long flags;
> > > +uint8_t data = 0;
> > > +struct vpl011 *vpl011 = &d->arch.vpl011;
> > > +struct vpl011_xen_backend *intf = vpl011->backend.xen;
> > > +XENCONS_RING_IDX in_cons, in_prod;
> > > +
> > > +VPL011_LOCK(d, flags);
> > > +
> > > +in_cons = intf->in_cons;
> > > +in_prod = intf->in_prod;
> > > +
> > > +smp_rmb();
> > > +
> > > +/*
> > > + * It is expected that there will be data i
Re: [Xen-devel] [PATCH v5 22/25] xen/arm: Allow vpl011 to be used by DomU
On Wed, 24 Oct 2018, Oleksandr Tyshchenko wrote:
> Hi, Stefano
>
> On Tue, Oct 23, 2018 at 5:04 AM Stefano Stabellini
> wrote:
> >
> > Make vpl011 being able to be used without a userspace component in Dom0.
> > In that case, output is printed to the Xen serial and input is received
> > from the Xen serial one character at a time.
> >
> > Call domain_vpl011_init during construct_domU if vpl011 is enabled.
> >
> > Introduce a new ring struct with only the ring array to avoid a waste of
> > memory. Introduce separate read_data and write_data functions for
> > initial domains: vpl011_write_data_xen is very simple and just writes
> > to the console, while vpl011_read_data_xen is a duplicate of
> > vpl011_read_data. Although textually almost identical, we are forced to
> > duplicate the functions because the struct layout is different.
> >
> > Output characters are printed one by one, potentially leading to
> > intermixed output of different domains on the console. A follow-up patch
> > will solve the issue by introducing buffering.
> >
> > Signed-off-by: Stefano Stabellini
> > Acked-by: Julien Grall
> > ---
> > Changes in v5:
> > - renable call to vpl011_rx_char_xen from console.c
> >
> > Changes in v4:
> > - code style
> >
> > Changes in v3:
> > - add in-code comments
> > - improve existing comments
> > - remove ifdef around domain_vpl011_init in construct_domU
> > - add ASSERT
> > - use SBSA_UART_FIFO_SIZE for in buffer size
> > - rename ring_enable to backend_in_domain
> > - rename struct xencons_in to struct vpl011_xen_backend
> > - rename inring field to xen
> > - rename helper functions accordingly
> > - remove unnecessary stub implementation of vpl011_rx_char
> > - move vpl011_rx_char_xen within the file to avoid the need of a forward
> > declaration of vpl011_data_avail
> > - fix small bug in vpl011_rx_char_xen: increment in_prod before using it
> > to check xencons_queued.
> >
> > Changes in v2:
> > - only init if vpl011
> > - rename vpl011_read_char to vpl011_rx_char
> > - remove spurious change
> > - fix coding style
> > - use different ring struct
> > - move the write_data changes to their own function
> > (vpl011_write_data_noring)
> > - duplicate vpl011_read_data
> > ---
> > xen/arch/arm/domain_build.c | 9 +-
> > xen/arch/arm/vpl011.c| 200
> > +--
> > xen/drivers/char/console.c | 2 +-
> > xen/include/asm-arm/vpl011.h | 8 ++
> > 4 files changed, 193 insertions(+), 26 deletions(-)
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 6026b77..9ffd919 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -2629,7 +2629,14 @@ static int __init construct_domU(struct domain *d,
> > if ( rc < 0 )
> > return rc;
> >
> > -return construct_domain(d, &kinfo);
> > +rc = construct_domain(d, &kinfo);
> > +if ( rc < 0 )
> > +return rc;
> > +
> > +if ( kinfo.vpl011 )
> > +rc = domain_vpl011_init(d, NULL);
> > +
> > +return rc;
> > }
> >
> > void __init create_domUs(void)
> > diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> > index 68452a8..131507e 100644
> > --- a/xen/arch/arm/vpl011.c
> > +++ b/xen/arch/arm/vpl011.c
> > @@ -77,6 +77,91 @@ static void vpl011_update_interrupt_status(struct domain
> > *d)
> > #endif
> > }
> >
> > +/*
> > + * vpl011_write_data_xen writes chars from the vpl011 out buffer to the
> > + * console. Only to be used when the backend is Xen.
> > + */
> > +static void vpl011_write_data_xen(struct domain *d, uint8_t data)
> > +{
> > +unsigned long flags;
> > +struct vpl011 *vpl011 = &d->arch.vpl011;
> > +
> > +VPL011_LOCK(d, flags);
> > +
> > +printk("%c", data);
> > +if (data == '\n')
> > +printk("DOM%u: ", d->domain_id);
> > +
> > +vpl011->uartris |= TXI;
> > +vpl011->uartfr &= ~TXFE;
> > +vpl011_update_interrupt_status(d);
> > +
> > +VPL011_UNLOCK(d, flags);
> > +}
> > +
> > +/*
> > + * vpl011_read_data_xen reads data when the backend is xen. Characters
> > + * are added to the vpl011 receive buffer by vpl011_rx_char_xen.
> > + */
> > +static uint8_t vpl011_read_data_xen(struct domain *d)
> > +{
> > +unsigned long flags;
> > +uint8_t data = 0;
> > +struct vpl011 *vpl011 = &d->arch.vpl011;
> > +struct vpl011_xen_backend *intf = vpl011->backend.xen;
> > +XENCONS_RING_IDX in_cons, in_prod;
> > +
> > +VPL011_LOCK(d, flags);
> > +
> > +in_cons = intf->in_cons;
> > +in_prod = intf->in_prod;
> > +
> > +smp_rmb();
> > +
> > +/*
> > + * It is expected that there will be data in the ring buffer when this
> > + * function is called since the guest is expected to read the data
> > register
> > + * only if the TXFE flag is not set.
> > + * If the guest still does read when TXFE bit is set then 0 will be
> > returned.
> > + */
> > +if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) >
Re: [Xen-devel] [PATCH v5 22/25] xen/arm: Allow vpl011 to be used by DomU
Hi, Stefano
On Tue, Oct 23, 2018 at 5:04 AM Stefano Stabellini
wrote:
>
> Make vpl011 being able to be used without a userspace component in Dom0.
> In that case, output is printed to the Xen serial and input is received
> from the Xen serial one character at a time.
>
> Call domain_vpl011_init during construct_domU if vpl011 is enabled.
>
> Introduce a new ring struct with only the ring array to avoid a waste of
> memory. Introduce separate read_data and write_data functions for
> initial domains: vpl011_write_data_xen is very simple and just writes
> to the console, while vpl011_read_data_xen is a duplicate of
> vpl011_read_data. Although textually almost identical, we are forced to
> duplicate the functions because the struct layout is different.
>
> Output characters are printed one by one, potentially leading to
> intermixed output of different domains on the console. A follow-up patch
> will solve the issue by introducing buffering.
>
> Signed-off-by: Stefano Stabellini
> Acked-by: Julien Grall
> ---
> Changes in v5:
> - renable call to vpl011_rx_char_xen from console.c
>
> Changes in v4:
> - code style
>
> Changes in v3:
> - add in-code comments
> - improve existing comments
> - remove ifdef around domain_vpl011_init in construct_domU
> - add ASSERT
> - use SBSA_UART_FIFO_SIZE for in buffer size
> - rename ring_enable to backend_in_domain
> - rename struct xencons_in to struct vpl011_xen_backend
> - rename inring field to xen
> - rename helper functions accordingly
> - remove unnecessary stub implementation of vpl011_rx_char
> - move vpl011_rx_char_xen within the file to avoid the need of a forward
> declaration of vpl011_data_avail
> - fix small bug in vpl011_rx_char_xen: increment in_prod before using it
> to check xencons_queued.
>
> Changes in v2:
> - only init if vpl011
> - rename vpl011_read_char to vpl011_rx_char
> - remove spurious change
> - fix coding style
> - use different ring struct
> - move the write_data changes to their own function
> (vpl011_write_data_noring)
> - duplicate vpl011_read_data
> ---
> xen/arch/arm/domain_build.c | 9 +-
> xen/arch/arm/vpl011.c| 200
> +--
> xen/drivers/char/console.c | 2 +-
> xen/include/asm-arm/vpl011.h | 8 ++
> 4 files changed, 193 insertions(+), 26 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 6026b77..9ffd919 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2629,7 +2629,14 @@ static int __init construct_domU(struct domain *d,
> if ( rc < 0 )
> return rc;
>
> -return construct_domain(d, &kinfo);
> +rc = construct_domain(d, &kinfo);
> +if ( rc < 0 )
> +return rc;
> +
> +if ( kinfo.vpl011 )
> +rc = domain_vpl011_init(d, NULL);
> +
> +return rc;
> }
>
> void __init create_domUs(void)
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> index 68452a8..131507e 100644
> --- a/xen/arch/arm/vpl011.c
> +++ b/xen/arch/arm/vpl011.c
> @@ -77,6 +77,91 @@ static void vpl011_update_interrupt_status(struct domain
> *d)
> #endif
> }
>
> +/*
> + * vpl011_write_data_xen writes chars from the vpl011 out buffer to the
> + * console. Only to be used when the backend is Xen.
> + */
> +static void vpl011_write_data_xen(struct domain *d, uint8_t data)
> +{
> +unsigned long flags;
> +struct vpl011 *vpl011 = &d->arch.vpl011;
> +
> +VPL011_LOCK(d, flags);
> +
> +printk("%c", data);
> +if (data == '\n')
> +printk("DOM%u: ", d->domain_id);
> +
> +vpl011->uartris |= TXI;
> +vpl011->uartfr &= ~TXFE;
> +vpl011_update_interrupt_status(d);
> +
> +VPL011_UNLOCK(d, flags);
> +}
> +
> +/*
> + * vpl011_read_data_xen reads data when the backend is xen. Characters
> + * are added to the vpl011 receive buffer by vpl011_rx_char_xen.
> + */
> +static uint8_t vpl011_read_data_xen(struct domain *d)
> +{
> +unsigned long flags;
> +uint8_t data = 0;
> +struct vpl011 *vpl011 = &d->arch.vpl011;
> +struct vpl011_xen_backend *intf = vpl011->backend.xen;
> +XENCONS_RING_IDX in_cons, in_prod;
> +
> +VPL011_LOCK(d, flags);
> +
> +in_cons = intf->in_cons;
> +in_prod = intf->in_prod;
> +
> +smp_rmb();
> +
> +/*
> + * It is expected that there will be data in the ring buffer when this
> + * function is called since the guest is expected to read the data
> register
> + * only if the TXFE flag is not set.
> + * If the guest still does read when TXFE bit is set then 0 will be
> returned.
> + */
> +if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 )
> +{
> +unsigned int fifo_level;
> +
> +data = intf->in[xencons_mask(in_cons, sizeof(intf->in))];
> +in_cons += 1;
> +smp_mb();
> +intf->in_cons = in_cons;
> +
> +fifo_level = xencons_queued(in_prod, in_cons, sizeof(intf->in));
> +
> +/* If the FIFO is now empty, we cle
[Xen-devel] [PATCH v5 22/25] xen/arm: Allow vpl011 to be used by DomU
Make vpl011 being able to be used without a userspace component in Dom0.
In that case, output is printed to the Xen serial and input is received
from the Xen serial one character at a time.
Call domain_vpl011_init during construct_domU if vpl011 is enabled.
Introduce a new ring struct with only the ring array to avoid a waste of
memory. Introduce separate read_data and write_data functions for
initial domains: vpl011_write_data_xen is very simple and just writes
to the console, while vpl011_read_data_xen is a duplicate of
vpl011_read_data. Although textually almost identical, we are forced to
duplicate the functions because the struct layout is different.
Output characters are printed one by one, potentially leading to
intermixed output of different domains on the console. A follow-up patch
will solve the issue by introducing buffering.
Signed-off-by: Stefano Stabellini
Acked-by: Julien Grall
---
Changes in v5:
- renable call to vpl011_rx_char_xen from console.c
Changes in v4:
- code style
Changes in v3:
- add in-code comments
- improve existing comments
- remove ifdef around domain_vpl011_init in construct_domU
- add ASSERT
- use SBSA_UART_FIFO_SIZE for in buffer size
- rename ring_enable to backend_in_domain
- rename struct xencons_in to struct vpl011_xen_backend
- rename inring field to xen
- rename helper functions accordingly
- remove unnecessary stub implementation of vpl011_rx_char
- move vpl011_rx_char_xen within the file to avoid the need of a forward
declaration of vpl011_data_avail
- fix small bug in vpl011_rx_char_xen: increment in_prod before using it
to check xencons_queued.
Changes in v2:
- only init if vpl011
- rename vpl011_read_char to vpl011_rx_char
- remove spurious change
- fix coding style
- use different ring struct
- move the write_data changes to their own function
(vpl011_write_data_noring)
- duplicate vpl011_read_data
---
xen/arch/arm/domain_build.c | 9 +-
xen/arch/arm/vpl011.c| 200 +--
xen/drivers/char/console.c | 2 +-
xen/include/asm-arm/vpl011.h | 8 ++
4 files changed, 193 insertions(+), 26 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 6026b77..9ffd919 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2629,7 +2629,14 @@ static int __init construct_domU(struct domain *d,
if ( rc < 0 )
return rc;
-return construct_domain(d, &kinfo);
+rc = construct_domain(d, &kinfo);
+if ( rc < 0 )
+return rc;
+
+if ( kinfo.vpl011 )
+rc = domain_vpl011_init(d, NULL);
+
+return rc;
}
void __init create_domUs(void)
diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 68452a8..131507e 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -77,6 +77,91 @@ static void vpl011_update_interrupt_status(struct domain *d)
#endif
}
+/*
+ * vpl011_write_data_xen writes chars from the vpl011 out buffer to the
+ * console. Only to be used when the backend is Xen.
+ */
+static void vpl011_write_data_xen(struct domain *d, uint8_t data)
+{
+unsigned long flags;
+struct vpl011 *vpl011 = &d->arch.vpl011;
+
+VPL011_LOCK(d, flags);
+
+printk("%c", data);
+if (data == '\n')
+printk("DOM%u: ", d->domain_id);
+
+vpl011->uartris |= TXI;
+vpl011->uartfr &= ~TXFE;
+vpl011_update_interrupt_status(d);
+
+VPL011_UNLOCK(d, flags);
+}
+
+/*
+ * vpl011_read_data_xen reads data when the backend is xen. Characters
+ * are added to the vpl011 receive buffer by vpl011_rx_char_xen.
+ */
+static uint8_t vpl011_read_data_xen(struct domain *d)
+{
+unsigned long flags;
+uint8_t data = 0;
+struct vpl011 *vpl011 = &d->arch.vpl011;
+struct vpl011_xen_backend *intf = vpl011->backend.xen;
+XENCONS_RING_IDX in_cons, in_prod;
+
+VPL011_LOCK(d, flags);
+
+in_cons = intf->in_cons;
+in_prod = intf->in_prod;
+
+smp_rmb();
+
+/*
+ * It is expected that there will be data in the ring buffer when this
+ * function is called since the guest is expected to read the data register
+ * only if the TXFE flag is not set.
+ * If the guest still does read when TXFE bit is set then 0 will be
returned.
+ */
+if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 )
+{
+unsigned int fifo_level;
+
+data = intf->in[xencons_mask(in_cons, sizeof(intf->in))];
+in_cons += 1;
+smp_mb();
+intf->in_cons = in_cons;
+
+fifo_level = xencons_queued(in_prod, in_cons, sizeof(intf->in));
+
+/* If the FIFO is now empty, we clear the receive timeout interrupt. */
+if ( fifo_level == 0 )
+{
+vpl011->uartfr |= RXFE;
+vpl011->uartris &= ~RTI;
+}
+
+/* If the FIFO is more than half empty, we clear the RX interrupt. */
+if ( fifo_level < sizeof(intf->in) - SBSA_UART_FIFO_LEVEL )
+vpl011->uartris &= ~RXI;
+
+
