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

[Jan Beulich <jbeulich@xxxxxxxx>]

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

Jan