On 23.06.2025 03:34, dm...@proton.me wrote:
> @@ -769,22 +770,23 @@ static long
> guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
> } while ( --kcount > 0 );
>
> *kout = '\0';
> - spin_lock(&cd->pbuf_lock);
> + spin_lock(&cons->lock);
> kcount = kin - kbuf;
> if ( c != '\n' &&
> - (cd->pbuf_idx + (kout - kbuf) < (DOMAIN_PBUF_SIZE - 1)) )
> + cons->idx + kout - kbuf < DOMAIN_CONSOLE_BUF_SIZE - 1 )
The dropping of the parentheses around the pointer subtraction is
problematic: You open up UB opportunities this way, as evaluation order
changes. We had UBSAN trip up on similar constructs already in the past.
> {
> /* buffer the output until a newline */
> - memcpy(cd->pbuf + cd->pbuf_idx, kbuf, kout - kbuf);
> - cd->pbuf_idx += (kout - kbuf);
> + memcpy(cons->buf + cons->idx, kbuf, kout - kbuf);
> + cons->idx += (kout - kbuf);
Here, otoh, the parentheses could be dropped while touching the line.
> }
> else
> {
> - cd->pbuf[cd->pbuf_idx] = '\0';
> - guest_printk(cd, XENLOG_G_DEBUG "%s%s\n", cd->pbuf, kbuf);
> - cd->pbuf_idx = 0;
> + cons->buf[cons->idx] = '\0';
> + guest_printk(cd,
> + XENLOG_G_DEBUG "%s%s\n", cons->buf, kbuf);
There's no need to wrap lines here, is there?
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -371,6 +371,20 @@ struct evtchn_port_ops;
>
> #define MAX_NR_IOREQ_SERVERS 8
>
> +/* Arbitrary value. */
> +#define DOMAIN_CONSOLE_BUF_SIZE 256
Nit: The value isn't entirely arbitrary.
> +/* Domain console settings. */
> +struct domain_console {
> + /* hvm_print_line() and guest_console_write() logging. */
> + char buf[DOMAIN_CONSOLE_BUF_SIZE];
This placement will negatively affect code generation on at least x86.
I did suggest putting the array at the end, for this very reason.
Jan
