Re: [PATCH v8 5/7] xen/console: use memcpy() in conring_puts()

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 27 Jul 2026, dmukhin@xxxxxxxx wrote:
> From: Denis Mukhin <dmukhin@xxxxxxxx> 
> 
> Make conring_puts() more efficient by using memcpy()'s, rather than
> copying the ring a byte at a time.
> 
> No functional change intended.
> 
> Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>
> ---
> Changes since v7:
> - hardended len check in conring_puts()
> ---
>  xen/drivers/char/console.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 09282a7a4f8e..a1b8e5f5b507 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -361,12 +361,24 @@ static DECLARE_SOFTIRQ_TASKLET(conring_tasklet, 
> conring_notify, NULL);
>  /* NB: Do not send conring VIRQs during panic. */
>  static bool conring_no_notify;
>  
> -static void conring_puts(const char *str, size_t len)
> +static void conring_puts(const char *str, unsigned int len)
>  {
> +    unsigned int src = len;
> +
> +    /* There are no callers with strings longer than PAGE_SIZE. */
> +    BUG_ON(len > PAGE_SIZE);

Should be an ASSERT


>      ASSERT(rspin_is_locked(&console_lock));
>  
> -    while ( len-- )
> -        conring[CONRING_IDX_MASK(conringp++)] = *str++;
> +    while ( src < len )

src is initialized to len, so this is a problem?


> +    {
> +        unsigned int dst = CONRING_IDX_MASK(conringp + src);
> +        unsigned int n = min(conring_size - dst, len - src);
> +
> +        memcpy(&conring[dst], &str[src], n);
> +        src += n;
> +    }
> +
> +    conringp += len;
>  
>      if ( conringp - conringc > conring_size )
>          conringc = conringp - conring_size;
> -- 
> 2.54.0
>