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

On Fri, Jul 17, 2026 at 11:01:27PM +0100, Andrew Cooper wrote:
> On 13/07/2026 7:16 pm, [email protected] wrote:
> > From: Denis Mukhin <[email protected]> 
> >
> > 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 <[email protected]>
> > Signed-off-by: Denis Mukhin <[email protected]>
> > Acked-by: Roger Pau Monne <[email protected]>
> > ---
> > 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.

Will update in the next iteration of the series.