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

[Denis Mukhin <dmkhn@xxxxxxxxx>]

On Wednesday, February 19th, 2025 at 5:52 AM, Jan Beulich <jbeulich@xxxxxxxx> 
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)

Thanks a lot for help and review!

> 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).
> 
> Jan