Re: [Xen-devel] [PATCH v3 21/25] xen/arm: refactor vpl011_data_avail

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 01/08/18 00:28, Stefano Stabellini wrote:
> Move the code to calculate in_fifo_level and out_fifo_level out of
> vpl011_data_avail, to the caller.
> This change will make it possible to reuse vpl011_data_avail with
> different ring structures in a later patch.
>
> Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
Acked-by: Julien Grall <julien.grall@xxxxxxx>

Cheers,

> ---
> Changes in v3:
> - remove forward declaration of vpl011_data_avail
>
> Changes in v2:
> - new patch
> ---
>   xen/arch/arm/vpl011.c | 64 +++++++++++++++++++++++++++++----------------------
>   1 file changed, 36 insertions(+), 28 deletions(-)
>
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> index e70c5ec..725a203 100644
> --- a/xen/arch/arm/vpl011.c
> +++ b/xen/arch/arm/vpl011.c
> @@ -378,30 +378,13 @@ static const struct mmio_handler_ops vpl011_mmio_handler 
> = {
>       .write = vpl011_mmio_write,
>   };
>   
> -static void vpl011_data_avail(struct domain *d)
> +static void vpl011_data_avail(struct domain *d,
> +                              XENCONS_RING_IDX in_fifo_level,
> +                              XENCONS_RING_IDX in_size,
> +                              XENCONS_RING_IDX out_fifo_level,
> +                              XENCONS_RING_IDX out_size)
>   {
> -    unsigned long flags;
>       struct vpl011 *vpl011 = &d->arch.vpl011;
> -    struct xencons_interface *intf = vpl011->dom.ring_buf;
> -    XENCONS_RING_IDX in_cons, in_prod, out_cons, out_prod;
> -    XENCONS_RING_IDX in_fifo_level, out_fifo_level;
> -
> -    VPL011_LOCK(d, flags);
> -
> -    in_cons = intf->in_cons;
> -    in_prod = intf->in_prod;
> -    out_cons = intf->out_cons;
> -    out_prod = intf->out_prod;
> -
> -    smp_rmb();
> -
> -    in_fifo_level = xencons_queued(in_prod,
> -                                   in_cons,
> -                                   sizeof(intf->in));
> -
> -    out_fifo_level = xencons_queued(out_prod,
> -                                    out_cons,
> -                                    sizeof(intf->out));
>   
>       /**** Update the UART RX state ****/
>   
> @@ -410,11 +393,11 @@ static void vpl011_data_avail(struct domain *d)
>           vpl011->uartfr &= ~RXFE;
>   
>       /* Set the FIFO_FULL bit if the Xen buffer is full. */
> -    if ( in_fifo_level == sizeof(intf->in) )
> +    if ( in_fifo_level == in_size )
>           vpl011->uartfr |= RXFF;
>   
>       /* Assert the RX interrupt if the FIFO is more than half way filled. */
> -    if ( in_fifo_level >= sizeof(intf->in) - SBSA_UART_FIFO_LEVEL )
> +    if ( in_fifo_level >= in_size - SBSA_UART_FIFO_LEVEL )
>           vpl011->uartris |= RXI;
>   
>       /*
> @@ -427,7 +410,7 @@ static void vpl011_data_avail(struct domain *d)
>   
>       /**** Update the UART TX state ****/
>   
> -    if ( out_fifo_level != sizeof(intf->out) )
> +    if ( out_fifo_level != out_size )
>       {
>           vpl011->uartfr &= ~TXFF;
>   
> @@ -445,13 +428,38 @@ static void vpl011_data_avail(struct domain *d)
>   
>       if ( out_fifo_level == 0 )
>           vpl011->uartfr |= TXFE;
> -
> -    VPL011_UNLOCK(d, flags);
>   }
>   
>   static void vpl011_notification(struct vcpu *v, unsigned int port)
>   {
> -    vpl011_data_avail(v->domain);
> +    unsigned long flags;
> +    struct domain *d = v->domain;
> +    struct vpl011 *vpl011 = &d->arch.vpl011;
> +    struct xencons_interface *intf = vpl011->dom.ring_buf;
> +    XENCONS_RING_IDX in_cons, in_prod, out_cons, out_prod;
> +    XENCONS_RING_IDX in_fifo_level, out_fifo_level;
> +
> +    VPL011_LOCK(d, flags);
> +
> +    in_cons = intf->in_cons;
> +    in_prod = intf->in_prod;
> +    out_cons = intf->out_cons;
> +    out_prod = intf->out_prod;
> +
> +    smp_rmb();
> +
> +    in_fifo_level = xencons_queued(in_prod,
> +                                   in_cons,
> +                                   sizeof(intf->in));
> +
> +    out_fifo_level = xencons_queued(out_prod,
> +                                    out_cons,
> +                                    sizeof(intf->out));
> +
> +    vpl011_data_avail(v->domain, in_fifo_level, sizeof(intf->in),
> +                      out_fifo_level, sizeof(intf->out));
> +
> +    VPL011_UNLOCK(d, flags);
>   }
>   
>   int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
--
Julien Grall

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