Re: [PATCH v1] xen/console: group pbuf under console field

[dmkhn@xxxxxxxxx]

On Tue, Jun 10, 2025 at 12:24:57PM +0100, Andrew Cooper wrote:
> On 10/06/2025 9:10 am, Jan Beulich wrote:
> > On 06.06.2025 21:49, dmkhn@xxxxxxxxx wrote:
> >> From: Denis Mukhin <dmukhin@xxxxxxxx>
> >>
> >> Group all pbuf-related data structures under domain's console field.
> > Fine with me in principle, as I was indeed wondering about the lack of
> > grouping when the sub-struct was introduced, but ...
> >
> >> @@ -654,6 +648,12 @@ struct domain
> >>
> >>      /* Console settings. */
> >>      struct {
> >> +        /* hvm_print_line() and guest_console_write() logging. */
> >> +#define DOMAIN_PBUF_SIZE 200
> >> +        char *pbuf;
> >> +        unsigned int pbuf_idx;
> >> +        spinlock_t pbuf_lock;
> >> +
> >>          /* Permission to take ownership of the physical console input. */
> >>          bool input_allowed;
> >>      } console;
> > ... since all uses of the fields need touching anyway, can we perhaps
> > think of giving the fields better names? I never understood what the
> > 'p' in "pbuf" actually stands for, for example.
> 
> I always assumed it was Hungarian notation, so pointer.
> 
> As it's namespaced within .console, plain .buf, .idx and .lock names
> work fine.

Ack.

> 
> Separately, 200 is a silly and arbitrary number.  Furthermore the
> allocation is unconditional, despite the fact that in !VERSBOSE builds,
> domUs can't use this facility.  Also, where's the input buffer?

Thanks!

I will try to address those under individual changes.

re: arbitrary number: Will bumping the buffer size to the next power of 2 ==
256 work?

re: input buffer: Looks like there's only global serial_rx_ring buffer in
current design. My guess - because the input buffer is shared between domains
and Xen itself which does not have domain struct associated with it.

> 
> ~Andrew