Re: [PATCH v2 17/35] xen/console: introduce consoled_is_enabled()

[Denis Mukhin <dmkhn@xxxxxxxxx>]

On Tuesday, December 10th, 2024 at 6:31 AM, Jan Beulich <jbeulich@xxxxxxxx> 
wrote:

>
>
> On 06.12.2024 05:41, Denis Mukhin via B4 Relay wrote:
>
> > --- a/xen/drivers/char/consoled.c
> > +++ b/xen/drivers/char/consoled.c
> > @@ -43,13 +43,13 @@ struct xencons_interface *consoled_get_ring_addr(void)
> > static char buf[BUF_SZ + 1];
> >
> > /* Receives characters from a domain's PV console */
> > -void consoled_guest_rx(void)
> > +int consoled_guest_rx(void)
> > {
> > size_t idx = 0;
> > XENCONS_RING_IDX cons, prod;
> >
> > if ( !cons_ring )
> > - return;
> > + return 0;
> >
> > spin_lock(&rx_lock);
> >
> > @@ -91,15 +91,17 @@ void consoled_guest_rx(void)
> >
> > out:
> > spin_unlock(&rx_lock);
> > +
> > + return 0;
> > }
> >
> > /* Sends a character into a domain's PV console */
> > -void consoled_guest_tx(char c)
> > +int consoled_guest_tx(char c)
> > {
> > XENCONS_RING_IDX cons, prod;
> >
> > if ( !cons_ring )
> > - return;
> > + return 0;
> >
> > cons = ACCESS_ONCE(cons_ring->in_cons);
> > prod = cons_ring->in_prod;
> > @@ -118,6 +120,7 @@ void consoled_guest_tx(char c)
> >
> > cons_ring->in[MASK_XENCONS_IDX(prod++, cons_ring->in)] = c;
> >
> > +
> > /* Write to the ring before updating the pointer */
>
>
> No excess blank lines please.

Fixed.

>
> > @@ -125,6 +128,13 @@ void consoled_guest_tx(char c)
> > notify:
> > /* Always notify the guest: prevents receive path from getting stuck. */
> > pv_shim_inject_evtchn(pv_console_evtchn());
> > +
> > + return 0;
> > +}
>
>
> For both of the functions - what use is it to make the functions return
> a value, when all they'd ever return is zero (and callers don't care)?

Fixed.

> I'm also having a hard time seeing how this adjustment is related to ...
>
> > +bool consoled_is_enabled(void)
> > +{
> > + return pv_shim && pv_console;
> > }
>
>
> ... the introduction of this function (which by itself is probably fine).

That will be a cleanup in console driver on the code path I touched wrt console
focus switch.

>
> Jan