Re: [PATCH] xen/console: introduce console_{get,put}_domain()

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 19 Feb 2025, Jan Beulich wrote:
> On 18.02.2025 09:31, dmkhn@xxxxxxxxx wrote:
> > @@ -546,31 +555,23 @@ static void __serial_rx(char c)
> >           * getting stuck.
> >           */
> >          send_global_virq(VIRQ_CONSOLE);
> > -        break;
> > -
> > +    }
> >  #ifdef CONFIG_SBSA_VUART_CONSOLE
> > -    default:
> > -    {
> > -        struct domain *d = rcu_lock_domain_by_id(console_rx - 1);
> > -
> > -        if ( d )
> > -        {
> > -            int rc = vpl011_rx_char_xen(d, c);
> > -            if ( rc )
> > -                guest_printk(d, XENLOG_G_WARNING
> > -                                "failed to process console input: %d\n", 
> > rc);
> > -            rcu_unlock_domain(d);
> > -        }
> > -
> > -        break;
> > -    }
> > +    else
> > +        /* Deliver input to the emulated UART. */
> > +        rc = vpl011_rx_char_xen(d, c);
> >  #endif
> > -    }
> >  
> >  #ifdef CONFIG_X86
> >      if ( pv_shim && pv_console )
> >          consoled_guest_tx(c);
> >  #endif
> > +
> > +    if ( rc )
> > +        guest_printk(d, XENLOG_G_WARNING
> > +                        "failed to process console input: %d\n", rc);
> 
> Wouldn't this better live ahead of the four shim related lines?
> 
> Also please correct the log level specifier here. I realize you only move
> what was there before, but I consider i bad practice to move buggy code.
> gprintk() already prepends XENLOG_GUEST, so instead of XENLOG_G_WARNING
> is should just be XENLOG_WARNING. (Line wrapping is also odd here, at
> least according to my taste. But since that's not written down anywhere,
> I wouldn't insist on adjusting that as well.)
> 
> With both adjustments (provided you agree, of course)
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> Given they're reasonably simple to make, I could once again offer to
> adjust while committing (provided an Arm ack also arrives).

Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>