Re: [PATCH v7 2/7] xen/console: use memcpy() in console_init_ring()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 13/07/2026 7:16 pm, dmukhin@xxxxxxxx wrote:
> From: Denis Mukhin <dmukhin@xxxxxxxx> 
>
> Make console_init_ring() more efficient by using memcpy()'s, rather than
> copying the ring a byte at a time.
>
> No functional change intended.
>
> Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>
> Acked-by: Roger Pau Monne <roger.pau@xxxxxxxxxx>
> ---
> Changes since v6:
> - addressed Roger's feedback
> - added Roger's A-b
> ---
>  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 5ebbbf63c092..37ea84403180 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -463,7 +463,8 @@ static void cf_check conring_dump_keyhandler(unsigned 
> char key)
>  void __init console_init_ring(void)
>  {
>      char *ring;
> -    unsigned int i, order, memflags;
> +    XENCONS_RING_IDX done, size, n;
> +    unsigned int order, memflags;
>      unsigned long flags;
>  
>      if ( !opt_conring_size )
> @@ -479,8 +480,19 @@ void __init console_init_ring(void)
>      opt_conring_size = PAGE_SIZE << order;
>  
>      nrspin_lock_irqsave(&console_lock, flags);
> -    for ( i = conringc ; i != conringp; i++ )
> -        ring[i & (opt_conring_size - 1)] = conring[i & (conring_size - 1)];
> +
> +    size = conringp - conringc;
> +    for ( done = 0; done < size; done += n )
> +    {
> +        XENCONS_RING_IDX src = (conringc + done) & (conring_size - 1);
> +        XENCONS_RING_IDX dst = (conringc + done) & (opt_conring_size - 1);

I see this has been committed, but the use of XENCONS_RING_* is wrong
here.  This ring is not a XENCONS ring, and that header is almost
straight obfuscation.

The types obscure what's going on, including the fact that this logic is
unsafe with a 4G ring.  And while that might be a legitimate
implementation restriction, it needs to be obvious in the code.

These want to move back to being unsigned int.

~Andrew