From: Denis Mukhin <dmukhin@ford.com>
If virtual UART from domain X prints on the physical console, the behavior is
updated to (see [1]):
- console focus in domain X: do not prefix messages;
- no console focus in domain X: prefix all messages with "(dX)".
Use guest_printk() in all current in-hypervisor UART emulators. That aligns
behavior with debug I/O port 0xe9 handler in x86 port and slightly improves the
logging since guest_printk() already prints the domain ID. guest_printk() was
modified to account for console focus ownership.
Modify guest_console_write() for hardware domain case by adding domain ID
to the message when hwdom does not have console focus.
[1] https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2412121655360.463523@ubuntu-linux-20-04-desktop/
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
xen/arch/arm/vpl011.c | 6 +++---
xen/arch/arm/vuart.c | 2 +-
xen/arch/x86/hvm/hvm.c | 2 +-
xen/drivers/char/console.c | 25 ++++++++++++++++++++++---
4 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 66047bf33c..6298e377b3 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -87,7 +87,7 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t data)
{
if ( intf->out_prod == 1 )
{
- printk("%c", data);
+ guest_printk(d, "%c", data);
intf->out_prod = 0;
}
else
@@ -95,7 +95,7 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t data)
if ( data != '\n' )
intf->out[intf->out_prod++] = '\n';
intf->out[intf->out_prod++] = '\0';
- printk("%s", intf->out);
+ guest_printk(d, "%s", intf->out);
intf->out_prod = 0;
}
}
@@ -107,7 +107,7 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t data)
if ( data != '\n' )
intf->out[intf->out_prod++] = '\n';
intf->out[intf->out_prod++] = '\0';
- printk("DOM%u: %s", d->domain_id, intf->out);
+ guest_printk(d, "%s", intf->out);
intf->out_prod = 0;
}
}
diff --git a/xen/arch/arm/vuart.c b/xen/arch/arm/vuart.c
index bd2f425214..8c9f9e2182 100644
--- a/xen/arch/arm/vuart.c
+++ b/xen/arch/arm/vuart.c
@@ -89,7 +89,7 @@ static void vuart_print_char(struct vcpu *v, char c)
if ( c != '\n' )
uart->buf[uart->idx++] = '\n';
uart->buf[uart->idx] = '\0';
- printk(XENLOG_G_DEBUG "DOM%u: %s", d->domain_id, uart->buf);
+ guest_printk(d, "%s", uart->buf);
uart->idx = 0;
}
spin_unlock(&uart->lock);
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 4cb2e13046..8cc94bee81 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -577,7 +577,7 @@ static int cf_check hvm_print_line(
if ( (cd->pbuf_idx == (DOMAIN_PBUF_SIZE - 1)) || (c == '\n') )
{
cd->pbuf[cd->pbuf_idx] = '\0';
- guest_printk(cd, XENLOG_G_DEBUG "%s\n", cd->pbuf);
+ guest_printk(cd, "%s\n", cd->pbuf);
cd->pbuf_idx = 0;
}
spin_unlock(&cd->pbuf_lock);
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 27e3f8d8c6..16d6003a0f 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -724,7 +724,17 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
if ( is_hardware_domain(cd) )
{
/* Use direct console output as it could be interactive */
+ char prefix[16] = "";
+ struct domain *consd;
+
+ consd = console_get_domain();
+ if ( consd != cd )
+ snprintf(prefix, sizeof(prefix), "(d%d) ", cd->domain_id);
+ console_put_domain(consd);
+
nrspin_lock_irq(&console_lock);
+ if ( prefix[0] != '\0' )
+ console_send(prefix, strlen(prefix), flags);
console_send(kbuf, kcount, flags);
nrspin_unlock_irq(&console_lock);
}
@@ -755,7 +765,7 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
else
{
cd->pbuf[cd->pbuf_idx] = '\0';
- guest_printk(cd, XENLOG_G_DEBUG "%s%s\n", cd->pbuf, kbuf);
+ guest_printk(cd, "%s%s\n", cd->pbuf, kbuf);
cd->pbuf_idx = 0;
}
spin_unlock(&cd->pbuf_lock);
@@ -1013,12 +1023,21 @@ void printk(const char *fmt, ...)
va_end(args);
}
+/*
+ * Print message from the guest on the diagnostic console.
+ * Prefixes all messages w/ "(dX)" if domain X does not own physical console
+ * focus.
+ */
void guest_printk(const struct domain *d, const char *fmt, ...)
{
va_list args;
- char prefix[16];
+ char prefix[16] = "";
+ struct domain *consd;
- snprintf(prefix, sizeof(prefix), "(d%d) ", d->domain_id);
+ consd = console_get_domain();
+ if ( consd != d )
+ snprintf(prefix, sizeof(prefix), "(d%d) ", d->domain_id);
+ console_put_domain(consd);
va_start(args, fmt);
vprintk_common(fmt, args, prefix);
--
2.34.1On 31.05.2025 02:04, dmkhn@proton.me wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -577,7 +577,7 @@ static int cf_check hvm_print_line(
> if ( (cd->pbuf_idx == (DOMAIN_PBUF_SIZE - 1)) || (c == '\n') )
> {
> cd->pbuf[cd->pbuf_idx] = '\0';
> - guest_printk(cd, XENLOG_G_DEBUG "%s\n", cd->pbuf);
> + guest_printk(cd, "%s\n", cd->pbuf);
> cd->pbuf_idx = 0;
> }
Why this and ...
> @@ -755,7 +765,7 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
> else
> {
> cd->pbuf[cd->pbuf_idx] = '\0';
> - guest_printk(cd, XENLOG_G_DEBUG "%s%s\n", cd->pbuf, kbuf);
> + guest_printk(cd, "%s%s\n", cd->pbuf, kbuf);
> cd->pbuf_idx = 0;
> }
... this change? There's no compensation for it ...
> @@ -1013,12 +1023,21 @@ void printk(const char *fmt, ...)
> va_end(args);
> }
>
> +/*
> + * Print message from the guest on the diagnostic console.
> + * Prefixes all messages w/ "(dX)" if domain X does not own physical console
> + * focus.
> + */
> void guest_printk(const struct domain *d, const char *fmt, ...)
> {
> va_list args;
> - char prefix[16];
> + char prefix[16] = "";
> + struct domain *consd;
>
> - snprintf(prefix, sizeof(prefix), "(d%d) ", d->domain_id);
> + consd = console_get_domain();
> + if ( consd != d )
> + snprintf(prefix, sizeof(prefix), "(d%d) ", d->domain_id);
> + console_put_domain(consd);
>
> va_start(args, fmt);
> vprintk_common(fmt, args, prefix);
... here afaics, so it looks like you're undermining rate-limiting of
those messages.
Jan
On Wed, Jun 04, 2025 at 12:48:05PM +0200, Jan Beulich wrote:
> On 31.05.2025 02:04, dmkhn@proton.me wrote:
> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -577,7 +577,7 @@ static int cf_check hvm_print_line(
> > if ( (cd->pbuf_idx == (DOMAIN_PBUF_SIZE - 1)) || (c == '\n') )
> > {
> > cd->pbuf[cd->pbuf_idx] = '\0';
> > - guest_printk(cd, XENLOG_G_DEBUG "%s\n", cd->pbuf);
> > + guest_printk(cd, "%s\n", cd->pbuf);
> > cd->pbuf_idx = 0;
> > }
>
> Why this and ...
>
> > @@ -755,7 +765,7 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
> > else
> > {
> > cd->pbuf[cd->pbuf_idx] = '\0';
> > - guest_printk(cd, XENLOG_G_DEBUG "%s%s\n", cd->pbuf, kbuf);
> > + guest_printk(cd, "%s%s\n", cd->pbuf, kbuf);
> > cd->pbuf_idx = 0;
> > }
>
> ... this change? There's no compensation for it ...
>
> > @@ -1013,12 +1023,21 @@ void printk(const char *fmt, ...)
> > va_end(args);
> > }
> >
> > +/*
> > + * Print message from the guest on the diagnostic console.
> > + * Prefixes all messages w/ "(dX)" if domain X does not own physical console
> > + * focus.
> > + */
> > void guest_printk(const struct domain *d, const char *fmt, ...)
> > {
> > va_list args;
> > - char prefix[16];
> > + char prefix[16] = "";
> > + struct domain *consd;
> >
> > - snprintf(prefix, sizeof(prefix), "(d%d) ", d->domain_id);
> > + consd = console_get_domain();
> > + if ( consd != d )
> > + snprintf(prefix, sizeof(prefix), "(d%d) ", d->domain_id);
> > + console_put_domain(consd);
> >
> > va_start(args, fmt);
> > vprintk_common(fmt, args, prefix);
>
> ... here afaics, so it looks like you're undermining rate-limiting of
> those messages.
I droppped behavior change for I/O debug port on x86 and HYPERVISOR_console_io
hypercall.
But my understanding is that all guest debugging facilities, if enabled, should
not be rate-limited.
>
> Jan
On 05.06.2025 02:57, dmkhn@proton.me wrote:
> On Wed, Jun 04, 2025 at 12:48:05PM +0200, Jan Beulich wrote:
>> On 31.05.2025 02:04, dmkhn@proton.me wrote:
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -577,7 +577,7 @@ static int cf_check hvm_print_line(
>>> if ( (cd->pbuf_idx == (DOMAIN_PBUF_SIZE - 1)) || (c == '\n') )
>>> {
>>> cd->pbuf[cd->pbuf_idx] = '\0';
>>> - guest_printk(cd, XENLOG_G_DEBUG "%s\n", cd->pbuf);
>>> + guest_printk(cd, "%s\n", cd->pbuf);
>>> cd->pbuf_idx = 0;
>>> }
>>
>> Why this and ...
>>
>>> @@ -755,7 +765,7 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
>>> else
>>> {
>>> cd->pbuf[cd->pbuf_idx] = '\0';
>>> - guest_printk(cd, XENLOG_G_DEBUG "%s%s\n", cd->pbuf, kbuf);
>>> + guest_printk(cd, "%s%s\n", cd->pbuf, kbuf);
>>> cd->pbuf_idx = 0;
>>> }
>>
>> ... this change? There's no compensation for it ...
>>
>>> @@ -1013,12 +1023,21 @@ void printk(const char *fmt, ...)
>>> va_end(args);
>>> }
>>>
>>> +/*
>>> + * Print message from the guest on the diagnostic console.
>>> + * Prefixes all messages w/ "(dX)" if domain X does not own physical console
>>> + * focus.
>>> + */
>>> void guest_printk(const struct domain *d, const char *fmt, ...)
>>> {
>>> va_list args;
>>> - char prefix[16];
>>> + char prefix[16] = "";
>>> + struct domain *consd;
>>>
>>> - snprintf(prefix, sizeof(prefix), "(d%d) ", d->domain_id);
>>> + consd = console_get_domain();
>>> + if ( consd != d )
>>> + snprintf(prefix, sizeof(prefix), "(d%d) ", d->domain_id);
>>> + console_put_domain(consd);
>>>
>>> va_start(args, fmt);
>>> vprintk_common(fmt, args, prefix);
>>
>> ... here afaics, so it looks like you're undermining rate-limiting of
>> those messages.
>
> I droppped behavior change for I/O debug port on x86 and HYPERVISOR_console_io
> hypercall.
>
> But my understanding is that all guest debugging facilities, if enabled, should
> not be rate-limited.
I certainly disagree there. How much rate limiting to apply to guest output is a
matter of the guest_loglvl= command line option. Its default settings are the way
they are for a reason.
Jan
On Thu, Jun 05, 2025 at 08:15:15AM +0200, Jan Beulich wrote:
> On 05.06.2025 02:57, dmkhn@proton.me wrote:
> > On Wed, Jun 04, 2025 at 12:48:05PM +0200, Jan Beulich wrote:
> >> On 31.05.2025 02:04, dmkhn@proton.me wrote:
> >>> --- a/xen/arch/x86/hvm/hvm.c
> >>> +++ b/xen/arch/x86/hvm/hvm.c
> >>> @@ -577,7 +577,7 @@ static int cf_check hvm_print_line(
> >>> if ( (cd->pbuf_idx == (DOMAIN_PBUF_SIZE - 1)) || (c == '\n') )
> >>> {
> >>> cd->pbuf[cd->pbuf_idx] = '\0';
> >>> - guest_printk(cd, XENLOG_G_DEBUG "%s\n", cd->pbuf);
> >>> + guest_printk(cd, "%s\n", cd->pbuf);
> >>> cd->pbuf_idx = 0;
> >>> }
> >>
> >> Why this and ...
> >>
> >>> @@ -755,7 +765,7 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
> >>> else
> >>> {
> >>> cd->pbuf[cd->pbuf_idx] = '\0';
> >>> - guest_printk(cd, XENLOG_G_DEBUG "%s%s\n", cd->pbuf, kbuf);
> >>> + guest_printk(cd, "%s%s\n", cd->pbuf, kbuf);
> >>> cd->pbuf_idx = 0;
> >>> }
> >>
> >> ... this change? There's no compensation for it ...
> >>
> >>> @@ -1013,12 +1023,21 @@ void printk(const char *fmt, ...)
> >>> va_end(args);
> >>> }
> >>>
> >>> +/*
> >>> + * Print message from the guest on the diagnostic console.
> >>> + * Prefixes all messages w/ "(dX)" if domain X does not own physical console
> >>> + * focus.
> >>> + */
> >>> void guest_printk(const struct domain *d, const char *fmt, ...)
> >>> {
> >>> va_list args;
> >>> - char prefix[16];
> >>> + char prefix[16] = "";
> >>> + struct domain *consd;
> >>>
> >>> - snprintf(prefix, sizeof(prefix), "(d%d) ", d->domain_id);
> >>> + consd = console_get_domain();
> >>> + if ( consd != d )
> >>> + snprintf(prefix, sizeof(prefix), "(d%d) ", d->domain_id);
> >>> + console_put_domain(consd);
> >>>
> >>> va_start(args, fmt);
> >>> vprintk_common(fmt, args, prefix);
> >>
> >> ... here afaics, so it looks like you're undermining rate-limiting of
> >> those messages.
> >
> > I droppped behavior change for I/O debug port on x86 and HYPERVISOR_console_io
> > hypercall.
> >
> > But my understanding is that all guest debugging facilities, if enabled, should
> > not be rate-limited.
>
> I certainly disagree there. How much rate limiting to apply to guest output is a
> matter of the guest_loglvl= command line option. Its default settings are the way
> they are for a reason.
Oh, I see!
Thanks for clarification!
>
> Jan
>
© 2016 - 2025 Red Hat, Inc.