Daniel De Graaf
2013-Aug-16 19:01 UTC
[PATCH v2] xen/console: buffer and show origin of guest PV writes
Guests other than domain 0 using the console output have previously been
controlled by the VERBOSE define, but with no designation of which
guest''s output was on the console. This patch converts the HVM output
buffering to be used by all domains, line buffering their output and
prefixing it with the domain ID. This is especially useful for debugging
stub domains.
Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
Changes since v1 (RFC):
- Use prefix of (d%d) in place of (XEN) for both PV and HVM output
- Guests other than dom0 have non-printable characters stripped
- Use unsigned type for pbuf_idx
- Formatting fixes
xen/arch/x86/hvm/hvm.c | 27 +++++-------
xen/common/domain.c | 8 ++++
xen/drivers/char/console.c | 94 ++++++++++++++++++++++++++++++++--------
xen/include/asm-x86/hvm/domain.h | 6 ---
xen/include/xen/lib.h | 2 +
xen/include/xen/sched.h | 6 +++
6 files changed, 103 insertions(+), 40 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 1fcaed0..4ff76cc 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -485,8 +485,7 @@ static int hvm_set_ioreq_page(
static int hvm_print_line(
int dir, uint32_t port, uint32_t bytes, uint32_t *val)
{
- struct vcpu *curr = current;
- struct hvm_domain *hd = &curr->domain->arch.hvm_domain;
+ struct domain *cd = current->domain;
char c = *val;
BUG_ON(bytes != 1);
@@ -495,17 +494,16 @@ static int hvm_print_line(
if ( !isprint(c) && (c != ''\n'') && (c !=
''\t'') )
return X86EMUL_OKAY;
- spin_lock(&hd->pbuf_lock);
- hd->pbuf[hd->pbuf_idx++] = c;
- if ( (hd->pbuf_idx == (HVM_PBUF_SIZE - 2)) || (c ==
''\n'') )
+ spin_lock(&cd->pbuf_lock);
+ if ( c != ''\n'' )
+ cd->pbuf[cd->pbuf_idx++] = c;
+ if ( (cd->pbuf_idx == (DOMAIN_PBUF_SIZE - 1)) || (c ==
''\n'') )
{
- if ( c != ''\n'' )
- hd->pbuf[hd->pbuf_idx++] = ''\n'';
- hd->pbuf[hd->pbuf_idx] = ''\0'';
- printk(XENLOG_G_DEBUG "HVM%u: %s",
curr->domain->domain_id, hd->pbuf);
- hd->pbuf_idx = 0;
+ cd->pbuf[cd->pbuf_idx] = ''\0'';
+ guest_printk(cd, XENLOG_G_DEBUG "%s\n", cd->pbuf);
+ cd->pbuf_idx = 0;
}
- spin_unlock(&hd->pbuf_lock);
+ spin_unlock(&cd->pbuf_lock);
return X86EMUL_OKAY;
}
@@ -521,19 +519,16 @@ int hvm_domain_initialise(struct domain *d)
return -EINVAL;
}
- spin_lock_init(&d->arch.hvm_domain.pbuf_lock);
spin_lock_init(&d->arch.hvm_domain.irq_lock);
spin_lock_init(&d->arch.hvm_domain.uc_lock);
INIT_LIST_HEAD(&d->arch.hvm_domain.msixtbl_list);
spin_lock_init(&d->arch.hvm_domain.msixtbl_list_lock);
- d->arch.hvm_domain.pbuf = xzalloc_array(char, HVM_PBUF_SIZE);
d->arch.hvm_domain.params = xzalloc_array(uint64_t, HVM_NR_PARAMS);
d->arch.hvm_domain.io_handler = xmalloc(struct hvm_io_handler);
rc = -ENOMEM;
- if ( !d->arch.hvm_domain.pbuf || !d->arch.hvm_domain.params ||
- !d->arch.hvm_domain.io_handler )
+ if ( !d->arch.hvm_domain.params || !d->arch.hvm_domain.io_handler )
goto fail0;
d->arch.hvm_domain.io_handler->num_slot = 0;
@@ -578,7 +573,6 @@ int hvm_domain_initialise(struct domain *d)
fail0:
xfree(d->arch.hvm_domain.io_handler);
xfree(d->arch.hvm_domain.params);
- xfree(d->arch.hvm_domain.pbuf);
return rc;
}
@@ -603,7 +597,6 @@ void hvm_domain_relinquish_resources(struct domain *d)
xfree(d->arch.hvm_domain.io_handler);
xfree(d->arch.hvm_domain.params);
- xfree(d->arch.hvm_domain.pbuf);
}
void hvm_domain_destroy(struct domain *d)
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 6c264a5..daac2c9 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -231,6 +231,8 @@ struct domain *domain_create(
spin_lock_init(&d->shutdown_lock);
d->shutdown_code = -1;
+ spin_lock_init(&d->pbuf_lock);
+
err = -ENOMEM;
if ( !zalloc_cpumask_var(&d->domain_dirty_cpumask) )
goto fail;
@@ -286,6 +288,10 @@ struct domain *domain_create(
d->mem_event = xzalloc(struct mem_event_per_domain);
if ( !d->mem_event )
goto fail;
+
+ d->pbuf = xzalloc_array(char, DOMAIN_PBUF_SIZE);
+ if ( !d->pbuf )
+ goto fail;
}
if ( (err = arch_domain_create(d, domcr_flags)) != 0 )
@@ -318,6 +324,7 @@ struct domain *domain_create(
d->is_dying = DOMDYING_dead;
atomic_set(&d->refcnt, DOMAIN_DESTROYED);
xfree(d->mem_event);
+ xfree(d->pbuf);
if ( init_status & INIT_arch )
arch_domain_destroy(d);
if ( init_status & INIT_gnttab )
@@ -730,6 +737,7 @@ static void complete_domain_destroy(struct rcu_head *head)
#endif
xfree(d->mem_event);
+ xfree(d->pbuf);
for ( i = d->max_vcpus - 1; i >= 0; i-- )
if ( (v = d->vcpu[i]) != NULL )
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 8ac32e4..b8d9a9f 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -24,6 +24,7 @@
#include <xen/shutdown.h>
#include <xen/video.h>
#include <xen/kexec.h>
+#include <xen/ctype.h>
#include <asm/debugger.h>
#include <asm/div64.h>
#include <xen/hypercall.h> /* for do_console_io */
@@ -375,6 +376,7 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char)
buffer, int count)
{
char kbuf[128];
int kcount;
+ struct domain *cd = current->domain;
while ( count > 0 )
{
@@ -388,19 +390,60 @@ static long
guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count)
return -EFAULT;
kbuf[kcount] = ''\0'';
- spin_lock_irq(&console_lock);
+ if ( is_hardware_domain(cd) )
+ {
+ /* Use direct console output as it could be interactive */
+ spin_lock_irq(&console_lock);
+
+ sercon_puts(kbuf);
+ video_puts(kbuf);
- sercon_puts(kbuf);
- video_puts(kbuf);
+ if ( opt_console_to_ring )
+ {
+ conring_puts(kbuf);
+ tasklet_schedule(¬ify_dom0_con_ring_tasklet);
+ }
- if ( opt_console_to_ring )
+ spin_unlock_irq(&console_lock);
+ }
+ else
{
- conring_puts(kbuf);
- tasklet_schedule(¬ify_dom0_con_ring_tasklet);
+ char *kin = kbuf;
+ char *kout = kbuf;
+ char c;
+ /* Strip non-printable characters */
+ for ( ; ; )
+ {
+ c = *kin++;
+ if ( c == ''\0'' || c == ''\n''
)
+ break;
+ if ( isprint(c) || c == ''\t'' )
+ *kout++ = c;
+ }
+ *kout = ''\0'';
+ spin_lock(&cd->pbuf_lock);
+ if ( c == ''\n'' )
+ {
+ kcount = kin - kbuf;
+ cd->pbuf[cd->pbuf_idx] = ''\0'';
+ guest_printk(cd, XENLOG_G_DEBUG "%s%s\n",
cd->pbuf, kbuf);
+ cd->pbuf_idx = 0;
+ }
+ else if ( cd->pbuf_idx + kcount < (DOMAIN_PBUF_SIZE - 1) )
+ {
+ /* buffer the output until a newline */
+ memcpy(cd->pbuf + cd->pbuf_idx, kbuf, kcount);
+ cd->pbuf_idx += kcount;
+ }
+ else
+ {
+ cd->pbuf[cd->pbuf_idx] = ''\0'';
+ guest_printk(cd, XENLOG_G_DEBUG "%s%s\n",
cd->pbuf, kbuf);
+ cd->pbuf_idx = 0;
+ }
+ spin_unlock(&cd->pbuf_lock);
}
- spin_unlock_irq(&console_lock);
-
guest_handle_add_offset(buffer, kcount);
count -= kcount;
}
@@ -504,12 +547,12 @@ static int printk_prefix_check(char *p, char **pp)
((loglvl < upper_thresh) && printk_ratelimit()));
}
-static void printk_start_of_line(void)
+static void printk_start_of_line(const char *prefix)
{
struct tm tm;
char tstr[32];
- __putstr("(XEN) ");
+ __putstr(prefix);
if ( !opt_console_timestamps )
return;
@@ -524,12 +567,11 @@ static void printk_start_of_line(void)
__putstr(tstr);
}
-void printk(const char *fmt, ...)
+static void vprintk_common(const char *prefix, const char *fmt, va_list args)
{
static char buf[1024];
static int start_of_line = 1, do_print;
- va_list args;
char *p, *q;
unsigned long flags;
@@ -537,9 +579,7 @@ void printk(const char *fmt, ...)
local_irq_save(flags);
spin_lock_recursive(&console_lock);
- va_start(args, fmt);
(void)vsnprintf(buf, sizeof(buf), fmt, args);
- va_end(args);
p = buf;
@@ -551,7 +591,7 @@ void printk(const char *fmt, ...)
if ( do_print )
{
if ( start_of_line )
- printk_start_of_line();
+ printk_start_of_line(prefix);
__putstr(p);
__putstr("\n");
}
@@ -566,7 +606,7 @@ void printk(const char *fmt, ...)
if ( do_print )
{
if ( start_of_line )
- printk_start_of_line();
+ printk_start_of_line(prefix);
__putstr(p);
}
start_of_line = 0;
@@ -576,6 +616,26 @@ void printk(const char *fmt, ...)
local_irq_restore(flags);
}
+void printk(const char *fmt, ...)
+{
+ va_list args;
+ va_start(args, fmt);
+ vprintk_common("(XEN) ", fmt, args);
+ va_end(args);
+}
+
+void guest_printk(struct domain *d, const char *fmt, ...)
+{
+ va_list args;
+ char prefix[16];
+
+ snprintf(prefix, sizeof(prefix), "(d%d) ", d->domain_id);
+
+ va_start(args, fmt);
+ vprintk_common(prefix, fmt, args);
+ va_end(args);
+}
+
void __init console_init_preirq(void)
{
char *p;
@@ -791,7 +851,7 @@ int __printk_ratelimit(int ratelimit_ms, int
ratelimit_burst)
snprintf(lost_str, sizeof(lost_str), "%d", lost);
/* console_lock may already be acquired by printk(). */
spin_lock_recursive(&console_lock);
- printk_start_of_line();
+ printk_start_of_line("(XEN) ");
__putstr("printk: ");
__putstr(lost_str);
__putstr(" messages suppressed.\n");
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index 27b3de5..b1e3187 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -62,12 +62,6 @@ struct hvm_domain {
/* emulated irq to pirq */
struct radix_tree_root emuirq_pirq;
- /* hvm_print_line() logging. */
-#define HVM_PBUF_SIZE 80
- char *pbuf;
- int pbuf_idx;
- spinlock_t pbuf_lock;
-
uint64_t *params;
/* Memory ranges with pinned cache attributes. */
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 74b34eb..40afe12 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -83,6 +83,8 @@ extern void debugtrace_printk(const char *fmt, ...);
#define _p(_x) ((void *)(unsigned long)(_x))
extern void printk(const char *format, ...)
__attribute__ ((format (printf, 1, 2)));
+extern void guest_printk(struct domain *d, const char *format, ...)
+ __attribute__ ((format (printf, 2, 3)));
extern void panic(const char *format, ...)
__attribute__ ((format (printf, 1, 2)));
extern long vm_assist(struct domain *, unsigned int, unsigned int);
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index ae6a3b8..0013a8d 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -341,6 +341,12 @@ struct domain
/* Control-plane tools handle for this domain. */
xen_domain_handle_t handle;
+ /* hvm_print_line() and guest_console_write() logging. */
+#define DOMAIN_PBUF_SIZE 80
+ char *pbuf;
+ unsigned pbuf_idx;
+ spinlock_t pbuf_lock;
+
/* OProfile support. */
struct xenoprof *xenoprof;
int32_t time_offset_seconds;
--
1.8.1.4
Keir Fraser
2013-Sep-09 11:13 UTC
Re: [PATCH v2] xen/console: buffer and show origin of guest PV writes
On 16/08/2013 20:01, "Daniel De Graaf" <dgdegra@tycho.nsa.gov> wrote:> Guests other than domain 0 using the console output have previously been > controlled by the VERBOSE define, but with no designation of which > guest''s output was on the console. This patch converts the HVM output > buffering to be used by all domains, line buffering their output and > prefixing it with the domain ID. This is especially useful for debugging > stub domains. > > Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>This seems good, but, if we process and buffer dom0''s output, we lose the possibility of running a terminal session in dom0 over the Xen console. Personally I do that quite a bit -- serial access only, get Xen''s debugging there, but also can log in to dom0. Does noone else?? -- Keir> --- > > Changes since v1 (RFC): > - Use prefix of (d%d) in place of (XEN) for both PV and HVM output > - Guests other than dom0 have non-printable characters stripped > - Use unsigned type for pbuf_idx > - Formatting fixes > > xen/arch/x86/hvm/hvm.c | 27 +++++------- > xen/common/domain.c | 8 ++++ > xen/drivers/char/console.c | 94 > ++++++++++++++++++++++++++++++++-------- > xen/include/asm-x86/hvm/domain.h | 6 --- > xen/include/xen/lib.h | 2 + > xen/include/xen/sched.h | 6 +++ > 6 files changed, 103 insertions(+), 40 deletions(-) > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index 1fcaed0..4ff76cc 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -485,8 +485,7 @@ static int hvm_set_ioreq_page( > static int hvm_print_line( > int dir, uint32_t port, uint32_t bytes, uint32_t *val) > { > - struct vcpu *curr = current; > - struct hvm_domain *hd = &curr->domain->arch.hvm_domain; > + struct domain *cd = current->domain; > char c = *val; > > BUG_ON(bytes != 1); > @@ -495,17 +494,16 @@ static int hvm_print_line( > if ( !isprint(c) && (c != ''\n'') && (c != ''\t'') ) > return X86EMUL_OKAY; > > - spin_lock(&hd->pbuf_lock); > - hd->pbuf[hd->pbuf_idx++] = c; > - if ( (hd->pbuf_idx == (HVM_PBUF_SIZE - 2)) || (c == ''\n'') ) > + spin_lock(&cd->pbuf_lock); > + if ( c != ''\n'' ) > + cd->pbuf[cd->pbuf_idx++] = c; > + if ( (cd->pbuf_idx == (DOMAIN_PBUF_SIZE - 1)) || (c == ''\n'') ) > { > - if ( c != ''\n'' ) > - hd->pbuf[hd->pbuf_idx++] = ''\n''; > - hd->pbuf[hd->pbuf_idx] = ''\0''; > - printk(XENLOG_G_DEBUG "HVM%u: %s", curr->domain->domain_id, > hd->pbuf); > - hd->pbuf_idx = 0; > + cd->pbuf[cd->pbuf_idx] = ''\0''; > + guest_printk(cd, XENLOG_G_DEBUG "%s\n", cd->pbuf); > + cd->pbuf_idx = 0; > } > - spin_unlock(&hd->pbuf_lock); > + spin_unlock(&cd->pbuf_lock); > > return X86EMUL_OKAY; > } > @@ -521,19 +519,16 @@ int hvm_domain_initialise(struct domain *d) > return -EINVAL; > } > > - spin_lock_init(&d->arch.hvm_domain.pbuf_lock); > spin_lock_init(&d->arch.hvm_domain.irq_lock); > spin_lock_init(&d->arch.hvm_domain.uc_lock); > > INIT_LIST_HEAD(&d->arch.hvm_domain.msixtbl_list); > spin_lock_init(&d->arch.hvm_domain.msixtbl_list_lock); > > - d->arch.hvm_domain.pbuf = xzalloc_array(char, HVM_PBUF_SIZE); > d->arch.hvm_domain.params = xzalloc_array(uint64_t, HVM_NR_PARAMS); > d->arch.hvm_domain.io_handler = xmalloc(struct hvm_io_handler); > rc = -ENOMEM; > - if ( !d->arch.hvm_domain.pbuf || !d->arch.hvm_domain.params || > - !d->arch.hvm_domain.io_handler ) > + if ( !d->arch.hvm_domain.params || !d->arch.hvm_domain.io_handler ) > goto fail0; > d->arch.hvm_domain.io_handler->num_slot = 0; > > @@ -578,7 +573,6 @@ int hvm_domain_initialise(struct domain *d) > fail0: > xfree(d->arch.hvm_domain.io_handler); > xfree(d->arch.hvm_domain.params); > - xfree(d->arch.hvm_domain.pbuf); > return rc; > } > > @@ -603,7 +597,6 @@ void hvm_domain_relinquish_resources(struct domain *d) > > xfree(d->arch.hvm_domain.io_handler); > xfree(d->arch.hvm_domain.params); > - xfree(d->arch.hvm_domain.pbuf); > } > > void hvm_domain_destroy(struct domain *d) > diff --git a/xen/common/domain.c b/xen/common/domain.c > index 6c264a5..daac2c9 100644 > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -231,6 +231,8 @@ struct domain *domain_create( > spin_lock_init(&d->shutdown_lock); > d->shutdown_code = -1; > > + spin_lock_init(&d->pbuf_lock); > + > err = -ENOMEM; > if ( !zalloc_cpumask_var(&d->domain_dirty_cpumask) ) > goto fail; > @@ -286,6 +288,10 @@ struct domain *domain_create( > d->mem_event = xzalloc(struct mem_event_per_domain); > if ( !d->mem_event ) > goto fail; > + > + d->pbuf = xzalloc_array(char, DOMAIN_PBUF_SIZE); > + if ( !d->pbuf ) > + goto fail; > } > > if ( (err = arch_domain_create(d, domcr_flags)) != 0 ) > @@ -318,6 +324,7 @@ struct domain *domain_create( > d->is_dying = DOMDYING_dead; > atomic_set(&d->refcnt, DOMAIN_DESTROYED); > xfree(d->mem_event); > + xfree(d->pbuf); > if ( init_status & INIT_arch ) > arch_domain_destroy(d); > if ( init_status & INIT_gnttab ) > @@ -730,6 +737,7 @@ static void complete_domain_destroy(struct rcu_head *head) > #endif > > xfree(d->mem_event); > + xfree(d->pbuf); > > for ( i = d->max_vcpus - 1; i >= 0; i-- ) > if ( (v = d->vcpu[i]) != NULL ) > diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c > index 8ac32e4..b8d9a9f 100644 > --- a/xen/drivers/char/console.c > +++ b/xen/drivers/char/console.c > @@ -24,6 +24,7 @@ > #include <xen/shutdown.h> > #include <xen/video.h> > #include <xen/kexec.h> > +#include <xen/ctype.h> > #include <asm/debugger.h> > #include <asm/div64.h> > #include <xen/hypercall.h> /* for do_console_io */ > @@ -375,6 +376,7 @@ static long > guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count) > { > char kbuf[128]; > int kcount; > + struct domain *cd = current->domain; > > while ( count > 0 ) > { > @@ -388,19 +390,60 @@ static long > guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count) > return -EFAULT; > kbuf[kcount] = ''\0''; > > - spin_lock_irq(&console_lock); > + if ( is_hardware_domain(cd) ) > + { > + /* Use direct console output as it could be interactive */ > + spin_lock_irq(&console_lock); > + > + sercon_puts(kbuf); > + video_puts(kbuf); > > - sercon_puts(kbuf); > - video_puts(kbuf); > + if ( opt_console_to_ring ) > + { > + conring_puts(kbuf); > + tasklet_schedule(¬ify_dom0_con_ring_tasklet); > + } > > - if ( opt_console_to_ring ) > + spin_unlock_irq(&console_lock); > + } > + else > { > - conring_puts(kbuf); > - tasklet_schedule(¬ify_dom0_con_ring_tasklet); > + char *kin = kbuf; > + char *kout = kbuf; > + char c; > + /* Strip non-printable characters */ > + for ( ; ; ) > + { > + c = *kin++; > + if ( c == ''\0'' || c == ''\n'' ) > + break; > + if ( isprint(c) || c == ''\t'' ) > + *kout++ = c; > + } > + *kout = ''\0''; > + spin_lock(&cd->pbuf_lock); > + if ( c == ''\n'' ) > + { > + kcount = kin - kbuf; > + cd->pbuf[cd->pbuf_idx] = ''\0''; > + guest_printk(cd, XENLOG_G_DEBUG "%s%s\n", cd->pbuf, kbuf); > + cd->pbuf_idx = 0; > + } > + else if ( cd->pbuf_idx + kcount < (DOMAIN_PBUF_SIZE - 1) ) > + { > + /* buffer the output until a newline */ > + memcpy(cd->pbuf + cd->pbuf_idx, kbuf, kcount); > + cd->pbuf_idx += kcount; > + } > + else > + { > + cd->pbuf[cd->pbuf_idx] = ''\0''; > + guest_printk(cd, XENLOG_G_DEBUG "%s%s\n", cd->pbuf, kbuf); > + cd->pbuf_idx = 0; > + } > + spin_unlock(&cd->pbuf_lock); > } > > - spin_unlock_irq(&console_lock); > - > guest_handle_add_offset(buffer, kcount); > count -= kcount; > } > @@ -504,12 +547,12 @@ static int printk_prefix_check(char *p, char **pp) > ((loglvl < upper_thresh) && printk_ratelimit())); > } > > -static void printk_start_of_line(void) > +static void printk_start_of_line(const char *prefix) > { > struct tm tm; > char tstr[32]; > > - __putstr("(XEN) "); > + __putstr(prefix); > > if ( !opt_console_timestamps ) > return; > @@ -524,12 +567,11 @@ static void printk_start_of_line(void) > __putstr(tstr); > } > > -void printk(const char *fmt, ...) > +static void vprintk_common(const char *prefix, const char *fmt, va_list args) > { > static char buf[1024]; > static int start_of_line = 1, do_print; > > - va_list args; > char *p, *q; > unsigned long flags; > > @@ -537,9 +579,7 @@ void printk(const char *fmt, ...) > local_irq_save(flags); > spin_lock_recursive(&console_lock); > > - va_start(args, fmt); > (void)vsnprintf(buf, sizeof(buf), fmt, args); > - va_end(args); > > p = buf; > > @@ -551,7 +591,7 @@ void printk(const char *fmt, ...) > if ( do_print ) > { > if ( start_of_line ) > - printk_start_of_line(); > + printk_start_of_line(prefix); > __putstr(p); > __putstr("\n"); > } > @@ -566,7 +606,7 @@ void printk(const char *fmt, ...) > if ( do_print ) > { > if ( start_of_line ) > - printk_start_of_line(); > + printk_start_of_line(prefix); > __putstr(p); > } > start_of_line = 0; > @@ -576,6 +616,26 @@ void printk(const char *fmt, ...) > local_irq_restore(flags); > } > > +void printk(const char *fmt, ...) > +{ > + va_list args; > + va_start(args, fmt); > + vprintk_common("(XEN) ", fmt, args); > + va_end(args); > +} > + > +void guest_printk(struct domain *d, const char *fmt, ...) > +{ > + va_list args; > + char prefix[16]; > + > + snprintf(prefix, sizeof(prefix), "(d%d) ", d->domain_id); > + > + va_start(args, fmt); > + vprintk_common(prefix, fmt, args); > + va_end(args); > +} > + > void __init console_init_preirq(void) > { > char *p; > @@ -791,7 +851,7 @@ int __printk_ratelimit(int ratelimit_ms, int > ratelimit_burst) > snprintf(lost_str, sizeof(lost_str), "%d", lost); > /* console_lock may already be acquired by printk(). */ > spin_lock_recursive(&console_lock); > - printk_start_of_line(); > + printk_start_of_line("(XEN) "); > __putstr("printk: "); > __putstr(lost_str); > __putstr(" messages suppressed.\n"); > diff --git a/xen/include/asm-x86/hvm/domain.h > b/xen/include/asm-x86/hvm/domain.h > index 27b3de5..b1e3187 100644 > --- a/xen/include/asm-x86/hvm/domain.h > +++ b/xen/include/asm-x86/hvm/domain.h > @@ -62,12 +62,6 @@ struct hvm_domain { > /* emulated irq to pirq */ > struct radix_tree_root emuirq_pirq; > > - /* hvm_print_line() logging. */ > -#define HVM_PBUF_SIZE 80 > - char *pbuf; > - int pbuf_idx; > - spinlock_t pbuf_lock; > - > uint64_t *params; > > /* Memory ranges with pinned cache attributes. */ > diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h > index 74b34eb..40afe12 100644 > --- a/xen/include/xen/lib.h > +++ b/xen/include/xen/lib.h > @@ -83,6 +83,8 @@ extern void debugtrace_printk(const char *fmt, ...); > #define _p(_x) ((void *)(unsigned long)(_x)) > extern void printk(const char *format, ...) > __attribute__ ((format (printf, 1, 2))); > +extern void guest_printk(struct domain *d, const char *format, ...) > + __attribute__ ((format (printf, 2, 3))); > extern void panic(const char *format, ...) > __attribute__ ((format (printf, 1, 2))); > extern long vm_assist(struct domain *, unsigned int, unsigned int); > diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h > index ae6a3b8..0013a8d 100644 > --- a/xen/include/xen/sched.h > +++ b/xen/include/xen/sched.h > @@ -341,6 +341,12 @@ struct domain > /* Control-plane tools handle for this domain. */ > xen_domain_handle_t handle; > > + /* hvm_print_line() and guest_console_write() logging. */ > +#define DOMAIN_PBUF_SIZE 80 > + char *pbuf; > + unsigned pbuf_idx; > + spinlock_t pbuf_lock; > + > /* OProfile support. */ > struct xenoprof *xenoprof; > int32_t time_offset_seconds;
Ian Campbell
2013-Sep-09 12:13 UTC
Re: [PATCH v2] xen/console: buffer and show origin of guest PV writes
On Mon, 2013-09-09 at 04:13 -0700, Keir Fraser wrote:> On 16/08/2013 20:01, "Daniel De Graaf" <dgdegra@tycho.nsa.gov> wrote: > > > Guests other than domain 0 using the console output have previously been > > controlled by the VERBOSE define, but with no designation of which > > guest''s output was on the console. This patch converts the HVM output > > buffering to be used by all domains, line buffering their output and > > prefixing it with the domain ID. This is especially useful for debugging > > stub domains. > > > > Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > > This seems good, but, if we process and buffer dom0''s output, we lose the > possibility of running a terminal session in dom0 over the Xen console. > Personally I do that quite a bit -- serial access only, get Xen''s debugging > there, but also can log in to dom0. Does noone else??I think I do too, I''m more likely to on ARM as well since there is often only a single debug serial port and no graphics. Oh, doesn''t code like:> + if ( is_hardware_domain(cd) ) > > + { > > + /* Use direct console output as it could be interactive */ > > + spin_lock_irq(&console_lock); > > + > > + sercon_puts(kbuf); > > + video_puts(kbuf);...mean that Daniel has already done the right thing for dom0 here?> > -- Keir > > > --- > > > > Changes since v1 (RFC): > > - Use prefix of (d%d) in place of (XEN) for both PV and HVM output > > - Guests other than dom0 have non-printable characters stripped > > - Use unsigned type for pbuf_idx > > - Formatting fixes > > > > xen/arch/x86/hvm/hvm.c | 27 +++++------- > > xen/common/domain.c | 8 ++++ > > xen/drivers/char/console.c | 94 > > ++++++++++++++++++++++++++++++++-------- > > xen/include/asm-x86/hvm/domain.h | 6 --- > > xen/include/xen/lib.h | 2 + > > xen/include/xen/sched.h | 6 +++ > > 6 files changed, 103 insertions(+), 40 deletions(-) > > > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > > index 1fcaed0..4ff76cc 100644 > > --- a/xen/arch/x86/hvm/hvm.c > > +++ b/xen/arch/x86/hvm/hvm.c > > @@ -485,8 +485,7 @@ static int hvm_set_ioreq_page( > > static int hvm_print_line( > > int dir, uint32_t port, uint32_t bytes, uint32_t *val) > > { > > - struct vcpu *curr = current; > > - struct hvm_domain *hd = &curr->domain->arch.hvm_domain; > > + struct domain *cd = current->domain; > > char c = *val; > > > > BUG_ON(bytes != 1); > > @@ -495,17 +494,16 @@ static int hvm_print_line( > > if ( !isprint(c) && (c != ''\n'') && (c != ''\t'') ) > > return X86EMUL_OKAY; > > > > - spin_lock(&hd->pbuf_lock); > > - hd->pbuf[hd->pbuf_idx++] = c; > > - if ( (hd->pbuf_idx == (HVM_PBUF_SIZE - 2)) || (c == ''\n'') ) > > + spin_lock(&cd->pbuf_lock); > > + if ( c != ''\n'' ) > > + cd->pbuf[cd->pbuf_idx++] = c; > > + if ( (cd->pbuf_idx == (DOMAIN_PBUF_SIZE - 1)) || (c == ''\n'') ) > > { > > - if ( c != ''\n'' ) > > - hd->pbuf[hd->pbuf_idx++] = ''\n''; > > - hd->pbuf[hd->pbuf_idx] = ''\0''; > > - printk(XENLOG_G_DEBUG "HVM%u: %s", curr->domain->domain_id, > > hd->pbuf); > > - hd->pbuf_idx = 0; > > + cd->pbuf[cd->pbuf_idx] = ''\0''; > > + guest_printk(cd, XENLOG_G_DEBUG "%s\n", cd->pbuf); > > + cd->pbuf_idx = 0; > > } > > - spin_unlock(&hd->pbuf_lock); > > + spin_unlock(&cd->pbuf_lock); > > > > return X86EMUL_OKAY; > > } > > @@ -521,19 +519,16 @@ int hvm_domain_initialise(struct domain *d) > > return -EINVAL; > > } > > > > - spin_lock_init(&d->arch.hvm_domain.pbuf_lock); > > spin_lock_init(&d->arch.hvm_domain.irq_lock); > > spin_lock_init(&d->arch.hvm_domain.uc_lock); > > > > INIT_LIST_HEAD(&d->arch.hvm_domain.msixtbl_list); > > spin_lock_init(&d->arch.hvm_domain.msixtbl_list_lock); > > > > - d->arch.hvm_domain.pbuf = xzalloc_array(char, HVM_PBUF_SIZE); > > d->arch.hvm_domain.params = xzalloc_array(uint64_t, HVM_NR_PARAMS); > > d->arch.hvm_domain.io_handler = xmalloc(struct hvm_io_handler); > > rc = -ENOMEM; > > - if ( !d->arch.hvm_domain.pbuf || !d->arch.hvm_domain.params || > > - !d->arch.hvm_domain.io_handler ) > > + if ( !d->arch.hvm_domain.params || !d->arch.hvm_domain.io_handler ) > > goto fail0; > > d->arch.hvm_domain.io_handler->num_slot = 0; > > > > @@ -578,7 +573,6 @@ int hvm_domain_initialise(struct domain *d) > > fail0: > > xfree(d->arch.hvm_domain.io_handler); > > xfree(d->arch.hvm_domain.params); > > - xfree(d->arch.hvm_domain.pbuf); > > return rc; > > } > > > > @@ -603,7 +597,6 @@ void hvm_domain_relinquish_resources(struct domain *d) > > > > xfree(d->arch.hvm_domain.io_handler); > > xfree(d->arch.hvm_domain.params); > > - xfree(d->arch.hvm_domain.pbuf); > > } > > > > void hvm_domain_destroy(struct domain *d) > > diff --git a/xen/common/domain.c b/xen/common/domain.c > > index 6c264a5..daac2c9 100644 > > --- a/xen/common/domain.c > > +++ b/xen/common/domain.c > > @@ -231,6 +231,8 @@ struct domain *domain_create( > > spin_lock_init(&d->shutdown_lock); > > d->shutdown_code = -1; > > > > + spin_lock_init(&d->pbuf_lock); > > + > > err = -ENOMEM; > > if ( !zalloc_cpumask_var(&d->domain_dirty_cpumask) ) > > goto fail; > > @@ -286,6 +288,10 @@ struct domain *domain_create( > > d->mem_event = xzalloc(struct mem_event_per_domain); > > if ( !d->mem_event ) > > goto fail; > > + > > + d->pbuf = xzalloc_array(char, DOMAIN_PBUF_SIZE); > > + if ( !d->pbuf ) > > + goto fail; > > } > > > > if ( (err = arch_domain_create(d, domcr_flags)) != 0 ) > > @@ -318,6 +324,7 @@ struct domain *domain_create( > > d->is_dying = DOMDYING_dead; > > atomic_set(&d->refcnt, DOMAIN_DESTROYED); > > xfree(d->mem_event); > > + xfree(d->pbuf); > > if ( init_status & INIT_arch ) > > arch_domain_destroy(d); > > if ( init_status & INIT_gnttab ) > > @@ -730,6 +737,7 @@ static void complete_domain_destroy(struct rcu_head *head) > > #endif > > > > xfree(d->mem_event); > > + xfree(d->pbuf); > > > > for ( i = d->max_vcpus - 1; i >= 0; i-- ) > > if ( (v = d->vcpu[i]) != NULL ) > > diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c > > index 8ac32e4..b8d9a9f 100644 > > --- a/xen/drivers/char/console.c > > +++ b/xen/drivers/char/console.c > > @@ -24,6 +24,7 @@ > > #include <xen/shutdown.h> > > #include <xen/video.h> > > #include <xen/kexec.h> > > +#include <xen/ctype.h> > > #include <asm/debugger.h> > > #include <asm/div64.h> > > #include <xen/hypercall.h> /* for do_console_io */ > > @@ -375,6 +376,7 @@ static long > > guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count) > > { > > char kbuf[128]; > > int kcount; > > + struct domain *cd = current->domain; > > > > while ( count > 0 ) > > { > > @@ -388,19 +390,60 @@ static long > > guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count) > > return -EFAULT; > > kbuf[kcount] = ''\0''; > > > > - spin_lock_irq(&console_lock); > > + if ( is_hardware_domain(cd) ) > > + { > > + /* Use direct console output as it could be interactive */ > > + spin_lock_irq(&console_lock); > > + > > + sercon_puts(kbuf); > > + video_puts(kbuf); > > > > - sercon_puts(kbuf); > > - video_puts(kbuf); > > + if ( opt_console_to_ring ) > > + { > > + conring_puts(kbuf); > > + tasklet_schedule(¬ify_dom0_con_ring_tasklet); > > + } > > > > - if ( opt_console_to_ring ) > > + spin_unlock_irq(&console_lock); > > + } > > + else > > { > > - conring_puts(kbuf); > > - tasklet_schedule(¬ify_dom0_con_ring_tasklet); > > + char *kin = kbuf; > > + char *kout = kbuf; > > + char c; > > + /* Strip non-printable characters */ > > + for ( ; ; ) > > + { > > + c = *kin++; > > + if ( c == ''\0'' || c == ''\n'' ) > > + break; > > + if ( isprint(c) || c == ''\t'' ) > > + *kout++ = c; > > + } > > + *kout = ''\0''; > > + spin_lock(&cd->pbuf_lock); > > + if ( c == ''\n'' ) > > + { > > + kcount = kin - kbuf; > > + cd->pbuf[cd->pbuf_idx] = ''\0''; > > + guest_printk(cd, XENLOG_G_DEBUG "%s%s\n", cd->pbuf, kbuf); > > + cd->pbuf_idx = 0; > > + } > > + else if ( cd->pbuf_idx + kcount < (DOMAIN_PBUF_SIZE - 1) ) > > + { > > + /* buffer the output until a newline */ > > + memcpy(cd->pbuf + cd->pbuf_idx, kbuf, kcount); > > + cd->pbuf_idx += kcount; > > + } > > + else > > + { > > + cd->pbuf[cd->pbuf_idx] = ''\0''; > > + guest_printk(cd, XENLOG_G_DEBUG "%s%s\n", cd->pbuf, kbuf); > > + cd->pbuf_idx = 0; > > + } > > + spin_unlock(&cd->pbuf_lock); > > } > > > > - spin_unlock_irq(&console_lock); > > - > > guest_handle_add_offset(buffer, kcount); > > count -= kcount; > > } > > @@ -504,12 +547,12 @@ static int printk_prefix_check(char *p, char **pp) > > ((loglvl < upper_thresh) && printk_ratelimit())); > > } > > > > -static void printk_start_of_line(void) > > +static void printk_start_of_line(const char *prefix) > > { > > struct tm tm; > > char tstr[32]; > > > > - __putstr("(XEN) "); > > + __putstr(prefix); > > > > if ( !opt_console_timestamps ) > > return; > > @@ -524,12 +567,11 @@ static void printk_start_of_line(void) > > __putstr(tstr); > > } > > > > -void printk(const char *fmt, ...) > > +static void vprintk_common(const char *prefix, const char *fmt, va_list args) > > { > > static char buf[1024]; > > static int start_of_line = 1, do_print; > > > > - va_list args; > > char *p, *q; > > unsigned long flags; > > > > @@ -537,9 +579,7 @@ void printk(const char *fmt, ...) > > local_irq_save(flags); > > spin_lock_recursive(&console_lock); > > > > - va_start(args, fmt); > > (void)vsnprintf(buf, sizeof(buf), fmt, args); > > - va_end(args); > > > > p = buf; > > > > @@ -551,7 +591,7 @@ void printk(const char *fmt, ...) > > if ( do_print ) > > { > > if ( start_of_line ) > > - printk_start_of_line(); > > + printk_start_of_line(prefix); > > __putstr(p); > > __putstr("\n"); > > } > > @@ -566,7 +606,7 @@ void printk(const char *fmt, ...) > > if ( do_print ) > > { > > if ( start_of_line ) > > - printk_start_of_line(); > > + printk_start_of_line(prefix); > > __putstr(p); > > } > > start_of_line = 0; > > @@ -576,6 +616,26 @@ void printk(const char *fmt, ...) > > local_irq_restore(flags); > > } > > > > +void printk(const char *fmt, ...) > > +{ > > + va_list args; > > + va_start(args, fmt); > > + vprintk_common("(XEN) ", fmt, args); > > + va_end(args); > > +} > > + > > +void guest_printk(struct domain *d, const char *fmt, ...) > > +{ > > + va_list args; > > + char prefix[16]; > > + > > + snprintf(prefix, sizeof(prefix), "(d%d) ", d->domain_id); > > + > > + va_start(args, fmt); > > + vprintk_common(prefix, fmt, args); > > + va_end(args); > > +} > > + > > void __init console_init_preirq(void) > > { > > char *p; > > @@ -791,7 +851,7 @@ int __printk_ratelimit(int ratelimit_ms, int > > ratelimit_burst) > > snprintf(lost_str, sizeof(lost_str), "%d", lost); > > /* console_lock may already be acquired by printk(). */ > > spin_lock_recursive(&console_lock); > > - printk_start_of_line(); > > + printk_start_of_line("(XEN) "); > > __putstr("printk: "); > > __putstr(lost_str); > > __putstr(" messages suppressed.\n"); > > diff --git a/xen/include/asm-x86/hvm/domain.h > > b/xen/include/asm-x86/hvm/domain.h > > index 27b3de5..b1e3187 100644 > > --- a/xen/include/asm-x86/hvm/domain.h > > +++ b/xen/include/asm-x86/hvm/domain.h > > @@ -62,12 +62,6 @@ struct hvm_domain { > > /* emulated irq to pirq */ > > struct radix_tree_root emuirq_pirq; > > > > - /* hvm_print_line() logging. */ > > -#define HVM_PBUF_SIZE 80 > > - char *pbuf; > > - int pbuf_idx; > > - spinlock_t pbuf_lock; > > - > > uint64_t *params; > > > > /* Memory ranges with pinned cache attributes. */ > > diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h > > index 74b34eb..40afe12 100644 > > --- a/xen/include/xen/lib.h > > +++ b/xen/include/xen/lib.h > > @@ -83,6 +83,8 @@ extern void debugtrace_printk(const char *fmt, ...); > > #define _p(_x) ((void *)(unsigned long)(_x)) > > extern void printk(const char *format, ...) > > __attribute__ ((format (printf, 1, 2))); > > +extern void guest_printk(struct domain *d, const char *format, ...) > > + __attribute__ ((format (printf, 2, 3))); > > extern void panic(const char *format, ...) > > __attribute__ ((format (printf, 1, 2))); > > extern long vm_assist(struct domain *, unsigned int, unsigned int); > > diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h > > index ae6a3b8..0013a8d 100644 > > --- a/xen/include/xen/sched.h > > +++ b/xen/include/xen/sched.h > > @@ -341,6 +341,12 @@ struct domain > > /* Control-plane tools handle for this domain. */ > > xen_domain_handle_t handle; > > > > + /* hvm_print_line() and guest_console_write() logging. */ > > +#define DOMAIN_PBUF_SIZE 80 > > + char *pbuf; > > + unsigned pbuf_idx; > > + spinlock_t pbuf_lock; > > + > > /* OProfile support. */ > > struct xenoprof *xenoprof; > > int32_t time_offset_seconds; > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Jan Beulich
2013-Sep-09 12:48 UTC
Re: [PATCH v2] xen/console: buffer and show origin of guest PV writes
>>> On 09.09.13 at 13:13, Keir Fraser <keir.xen@gmail.com> wrote: > On 16/08/2013 20:01, "Daniel De Graaf" <dgdegra@tycho.nsa.gov> wrote: > >> Guests other than domain 0 using the console output have previously been >> controlled by the VERBOSE define, but with no designation of which >> guest''s output was on the console. This patch converts the HVM output >> buffering to be used by all domains, line buffering their output and >> prefixing it with the domain ID. This is especially useful for debugging >> stub domains. >> >> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > > This seems good, but, if we process and buffer dom0''s output, we lose the > possibility of running a terminal session in dom0 over the Xen console. > Personally I do that quite a bit -- serial access only, get Xen''s debugging > there, but also can log in to dom0. Does noone else??I use this only rarely; where possible I prefer using the normal (VGA) console, and in most other cases ssh access to a remote machine is available. But yes, I view this as an important feature too, and simply failed to realize it would get broken by the change here. Jan
Keir Fraser
2013-Sep-09 13:43 UTC
Re: [PATCH v2] xen/console: buffer and show origin of guest PV writes
On 09/09/2013 05:13, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:> On Mon, 2013-09-09 at 04:13 -0700, Keir Fraser wrote: >> On 16/08/2013 20:01, "Daniel De Graaf" <dgdegra@tycho.nsa.gov> wrote: >> >>> Guests other than domain 0 using the console output have previously been >>> controlled by the VERBOSE define, but with no designation of which >>> guest''s output was on the console. This patch converts the HVM output >>> buffering to be used by all domains, line buffering their output and >>> prefixing it with the domain ID. This is especially useful for debugging >>> stub domains. >>> >>> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> >> >> This seems good, but, if we process and buffer dom0''s output, we lose the >> possibility of running a terminal session in dom0 over the Xen console. >> Personally I do that quite a bit -- serial access only, get Xen''s debugging >> there, but also can log in to dom0. Does noone else?? > > I think I do too, I''m more likely to on ARM as well since there is often > only a single debug serial port and no graphics. > > Oh, doesn''t code like: >> + if ( is_hardware_domain(cd) ) >>> + { >>> + /* Use direct console output as it could be interactive */ >>> + spin_lock_irq(&console_lock); >>> + >>> + sercon_puts(kbuf); >>> + video_puts(kbuf); > > ...mean that Daniel has already done the right thing for dom0 here?Ah, you could be right. Well if he will confirm that then I will give my Ack. -- Keir>> >> -- Keir >> >>> --- >>> >>> Changes since v1 (RFC): >>> - Use prefix of (d%d) in place of (XEN) for both PV and HVM output >>> - Guests other than dom0 have non-printable characters stripped >>> - Use unsigned type for pbuf_idx >>> - Formatting fixes >>> >>> xen/arch/x86/hvm/hvm.c | 27 +++++------- >>> xen/common/domain.c | 8 ++++ >>> xen/drivers/char/console.c | 94 >>> ++++++++++++++++++++++++++++++++-------- >>> xen/include/asm-x86/hvm/domain.h | 6 --- >>> xen/include/xen/lib.h | 2 + >>> xen/include/xen/sched.h | 6 +++ >>> 6 files changed, 103 insertions(+), 40 deletions(-) >>> >>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c >>> index 1fcaed0..4ff76cc 100644 >>> --- a/xen/arch/x86/hvm/hvm.c >>> +++ b/xen/arch/x86/hvm/hvm.c >>> @@ -485,8 +485,7 @@ static int hvm_set_ioreq_page( >>> static int hvm_print_line( >>> int dir, uint32_t port, uint32_t bytes, uint32_t *val) >>> { >>> - struct vcpu *curr = current; >>> - struct hvm_domain *hd = &curr->domain->arch.hvm_domain; >>> + struct domain *cd = current->domain; >>> char c = *val; >>> >>> BUG_ON(bytes != 1); >>> @@ -495,17 +494,16 @@ static int hvm_print_line( >>> if ( !isprint(c) && (c != ''\n'') && (c != ''\t'') ) >>> return X86EMUL_OKAY; >>> >>> - spin_lock(&hd->pbuf_lock); >>> - hd->pbuf[hd->pbuf_idx++] = c; >>> - if ( (hd->pbuf_idx == (HVM_PBUF_SIZE - 2)) || (c == ''\n'') ) >>> + spin_lock(&cd->pbuf_lock); >>> + if ( c != ''\n'' ) >>> + cd->pbuf[cd->pbuf_idx++] = c; >>> + if ( (cd->pbuf_idx == (DOMAIN_PBUF_SIZE - 1)) || (c == ''\n'') ) >>> { >>> - if ( c != ''\n'' ) >>> - hd->pbuf[hd->pbuf_idx++] = ''\n''; >>> - hd->pbuf[hd->pbuf_idx] = ''\0''; >>> - printk(XENLOG_G_DEBUG "HVM%u: %s", curr->domain->domain_id, >>> hd->pbuf); >>> - hd->pbuf_idx = 0; >>> + cd->pbuf[cd->pbuf_idx] = ''\0''; >>> + guest_printk(cd, XENLOG_G_DEBUG "%s\n", cd->pbuf); >>> + cd->pbuf_idx = 0; >>> } >>> - spin_unlock(&hd->pbuf_lock); >>> + spin_unlock(&cd->pbuf_lock); >>> >>> return X86EMUL_OKAY; >>> } >>> @@ -521,19 +519,16 @@ int hvm_domain_initialise(struct domain *d) >>> return -EINVAL; >>> } >>> >>> - spin_lock_init(&d->arch.hvm_domain.pbuf_lock); >>> spin_lock_init(&d->arch.hvm_domain.irq_lock); >>> spin_lock_init(&d->arch.hvm_domain.uc_lock); >>> >>> INIT_LIST_HEAD(&d->arch.hvm_domain.msixtbl_list); >>> spin_lock_init(&d->arch.hvm_domain.msixtbl_list_lock); >>> >>> - d->arch.hvm_domain.pbuf = xzalloc_array(char, HVM_PBUF_SIZE); >>> d->arch.hvm_domain.params = xzalloc_array(uint64_t, HVM_NR_PARAMS); >>> d->arch.hvm_domain.io_handler = xmalloc(struct hvm_io_handler); >>> rc = -ENOMEM; >>> - if ( !d->arch.hvm_domain.pbuf || !d->arch.hvm_domain.params || >>> - !d->arch.hvm_domain.io_handler ) >>> + if ( !d->arch.hvm_domain.params || !d->arch.hvm_domain.io_handler ) >>> goto fail0; >>> d->arch.hvm_domain.io_handler->num_slot = 0; >>> >>> @@ -578,7 +573,6 @@ int hvm_domain_initialise(struct domain *d) >>> fail0: >>> xfree(d->arch.hvm_domain.io_handler); >>> xfree(d->arch.hvm_domain.params); >>> - xfree(d->arch.hvm_domain.pbuf); >>> return rc; >>> } >>> >>> @@ -603,7 +597,6 @@ void hvm_domain_relinquish_resources(struct domain *d) >>> >>> xfree(d->arch.hvm_domain.io_handler); >>> xfree(d->arch.hvm_domain.params); >>> - xfree(d->arch.hvm_domain.pbuf); >>> } >>> >>> void hvm_domain_destroy(struct domain *d) >>> diff --git a/xen/common/domain.c b/xen/common/domain.c >>> index 6c264a5..daac2c9 100644 >>> --- a/xen/common/domain.c >>> +++ b/xen/common/domain.c >>> @@ -231,6 +231,8 @@ struct domain *domain_create( >>> spin_lock_init(&d->shutdown_lock); >>> d->shutdown_code = -1; >>> >>> + spin_lock_init(&d->pbuf_lock); >>> + >>> err = -ENOMEM; >>> if ( !zalloc_cpumask_var(&d->domain_dirty_cpumask) ) >>> goto fail; >>> @@ -286,6 +288,10 @@ struct domain *domain_create( >>> d->mem_event = xzalloc(struct mem_event_per_domain); >>> if ( !d->mem_event ) >>> goto fail; >>> + >>> + d->pbuf = xzalloc_array(char, DOMAIN_PBUF_SIZE); >>> + if ( !d->pbuf ) >>> + goto fail; >>> } >>> >>> if ( (err = arch_domain_create(d, domcr_flags)) != 0 ) >>> @@ -318,6 +324,7 @@ struct domain *domain_create( >>> d->is_dying = DOMDYING_dead; >>> atomic_set(&d->refcnt, DOMAIN_DESTROYED); >>> xfree(d->mem_event); >>> + xfree(d->pbuf); >>> if ( init_status & INIT_arch ) >>> arch_domain_destroy(d); >>> if ( init_status & INIT_gnttab ) >>> @@ -730,6 +737,7 @@ static void complete_domain_destroy(struct rcu_head >>> *head) >>> #endif >>> >>> xfree(d->mem_event); >>> + xfree(d->pbuf); >>> >>> for ( i = d->max_vcpus - 1; i >= 0; i-- ) >>> if ( (v = d->vcpu[i]) != NULL ) >>> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c >>> index 8ac32e4..b8d9a9f 100644 >>> --- a/xen/drivers/char/console.c >>> +++ b/xen/drivers/char/console.c >>> @@ -24,6 +24,7 @@ >>> #include <xen/shutdown.h> >>> #include <xen/video.h> >>> #include <xen/kexec.h> >>> +#include <xen/ctype.h> >>> #include <asm/debugger.h> >>> #include <asm/div64.h> >>> #include <xen/hypercall.h> /* for do_console_io */ >>> @@ -375,6 +376,7 @@ static long >>> guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count) >>> { >>> char kbuf[128]; >>> int kcount; >>> + struct domain *cd = current->domain; >>> >>> while ( count > 0 ) >>> { >>> @@ -388,19 +390,60 @@ static long >>> guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count) >>> return -EFAULT; >>> kbuf[kcount] = ''\0''; >>> >>> - spin_lock_irq(&console_lock); >>> + if ( is_hardware_domain(cd) ) >>> + { >>> + /* Use direct console output as it could be interactive */ >>> + spin_lock_irq(&console_lock); >>> + >>> + sercon_puts(kbuf); >>> + video_puts(kbuf); >>> >>> - sercon_puts(kbuf); >>> - video_puts(kbuf); >>> + if ( opt_console_to_ring ) >>> + { >>> + conring_puts(kbuf); >>> + tasklet_schedule(¬ify_dom0_con_ring_tasklet); >>> + } >>> >>> - if ( opt_console_to_ring ) >>> + spin_unlock_irq(&console_lock); >>> + } >>> + else >>> { >>> - conring_puts(kbuf); >>> - tasklet_schedule(¬ify_dom0_con_ring_tasklet); >>> + char *kin = kbuf; >>> + char *kout = kbuf; >>> + char c; >>> + /* Strip non-printable characters */ >>> + for ( ; ; ) >>> + { >>> + c = *kin++; >>> + if ( c == ''\0'' || c == ''\n'' ) >>> + break; >>> + if ( isprint(c) || c == ''\t'' ) >>> + *kout++ = c; >>> + } >>> + *kout = ''\0''; >>> + spin_lock(&cd->pbuf_lock); >>> + if ( c == ''\n'' ) >>> + { >>> + kcount = kin - kbuf; >>> + cd->pbuf[cd->pbuf_idx] = ''\0''; >>> + guest_printk(cd, XENLOG_G_DEBUG "%s%s\n", cd->pbuf, kbuf); >>> + cd->pbuf_idx = 0; >>> + } >>> + else if ( cd->pbuf_idx + kcount < (DOMAIN_PBUF_SIZE - 1) ) >>> + { >>> + /* buffer the output until a newline */ >>> + memcpy(cd->pbuf + cd->pbuf_idx, kbuf, kcount); >>> + cd->pbuf_idx += kcount; >>> + } >>> + else >>> + { >>> + cd->pbuf[cd->pbuf_idx] = ''\0''; >>> + guest_printk(cd, XENLOG_G_DEBUG "%s%s\n", cd->pbuf, kbuf); >>> + cd->pbuf_idx = 0; >>> + } >>> + spin_unlock(&cd->pbuf_lock); >>> } >>> >>> - spin_unlock_irq(&console_lock); >>> - >>> guest_handle_add_offset(buffer, kcount); >>> count -= kcount; >>> } >>> @@ -504,12 +547,12 @@ static int printk_prefix_check(char *p, char **pp) >>> ((loglvl < upper_thresh) && printk_ratelimit())); >>> } >>> >>> -static void printk_start_of_line(void) >>> +static void printk_start_of_line(const char *prefix) >>> { >>> struct tm tm; >>> char tstr[32]; >>> >>> - __putstr("(XEN) "); >>> + __putstr(prefix); >>> >>> if ( !opt_console_timestamps ) >>> return; >>> @@ -524,12 +567,11 @@ static void printk_start_of_line(void) >>> __putstr(tstr); >>> } >>> >>> -void printk(const char *fmt, ...) >>> +static void vprintk_common(const char *prefix, const char *fmt, va_list >>> args) >>> { >>> static char buf[1024]; >>> static int start_of_line = 1, do_print; >>> >>> - va_list args; >>> char *p, *q; >>> unsigned long flags; >>> >>> @@ -537,9 +579,7 @@ void printk(const char *fmt, ...) >>> local_irq_save(flags); >>> spin_lock_recursive(&console_lock); >>> >>> - va_start(args, fmt); >>> (void)vsnprintf(buf, sizeof(buf), fmt, args); >>> - va_end(args); >>> >>> p = buf; >>> >>> @@ -551,7 +591,7 @@ void printk(const char *fmt, ...) >>> if ( do_print ) >>> { >>> if ( start_of_line ) >>> - printk_start_of_line(); >>> + printk_start_of_line(prefix); >>> __putstr(p); >>> __putstr("\n"); >>> } >>> @@ -566,7 +606,7 @@ void printk(const char *fmt, ...) >>> if ( do_print ) >>> { >>> if ( start_of_line ) >>> - printk_start_of_line(); >>> + printk_start_of_line(prefix); >>> __putstr(p); >>> } >>> start_of_line = 0; >>> @@ -576,6 +616,26 @@ void printk(const char *fmt, ...) >>> local_irq_restore(flags); >>> } >>> >>> +void printk(const char *fmt, ...) >>> +{ >>> + va_list args; >>> + va_start(args, fmt); >>> + vprintk_common("(XEN) ", fmt, args); >>> + va_end(args); >>> +} >>> + >>> +void guest_printk(struct domain *d, const char *fmt, ...) >>> +{ >>> + va_list args; >>> + char prefix[16]; >>> + >>> + snprintf(prefix, sizeof(prefix), "(d%d) ", d->domain_id); >>> + >>> + va_start(args, fmt); >>> + vprintk_common(prefix, fmt, args); >>> + va_end(args); >>> +} >>> + >>> void __init console_init_preirq(void) >>> { >>> char *p; >>> @@ -791,7 +851,7 @@ int __printk_ratelimit(int ratelimit_ms, int >>> ratelimit_burst) >>> snprintf(lost_str, sizeof(lost_str), "%d", lost); >>> /* console_lock may already be acquired by printk(). */ >>> spin_lock_recursive(&console_lock); >>> - printk_start_of_line(); >>> + printk_start_of_line("(XEN) "); >>> __putstr("printk: "); >>> __putstr(lost_str); >>> __putstr(" messages suppressed.\n"); >>> diff --git a/xen/include/asm-x86/hvm/domain.h >>> b/xen/include/asm-x86/hvm/domain.h >>> index 27b3de5..b1e3187 100644 >>> --- a/xen/include/asm-x86/hvm/domain.h >>> +++ b/xen/include/asm-x86/hvm/domain.h >>> @@ -62,12 +62,6 @@ struct hvm_domain { >>> /* emulated irq to pirq */ >>> struct radix_tree_root emuirq_pirq; >>> >>> - /* hvm_print_line() logging. */ >>> -#define HVM_PBUF_SIZE 80 >>> - char *pbuf; >>> - int pbuf_idx; >>> - spinlock_t pbuf_lock; >>> - >>> uint64_t *params; >>> >>> /* Memory ranges with pinned cache attributes. */ >>> diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h >>> index 74b34eb..40afe12 100644 >>> --- a/xen/include/xen/lib.h >>> +++ b/xen/include/xen/lib.h >>> @@ -83,6 +83,8 @@ extern void debugtrace_printk(const char *fmt, ...); >>> #define _p(_x) ((void *)(unsigned long)(_x)) >>> extern void printk(const char *format, ...) >>> __attribute__ ((format (printf, 1, 2))); >>> +extern void guest_printk(struct domain *d, const char *format, ...) >>> + __attribute__ ((format (printf, 2, 3))); >>> extern void panic(const char *format, ...) >>> __attribute__ ((format (printf, 1, 2))); >>> extern long vm_assist(struct domain *, unsigned int, unsigned int); >>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h >>> index ae6a3b8..0013a8d 100644 >>> --- a/xen/include/xen/sched.h >>> +++ b/xen/include/xen/sched.h >>> @@ -341,6 +341,12 @@ struct domain >>> /* Control-plane tools handle for this domain. */ >>> xen_domain_handle_t handle; >>> >>> + /* hvm_print_line() and guest_console_write() logging. */ >>> +#define DOMAIN_PBUF_SIZE 80 >>> + char *pbuf; >>> + unsigned pbuf_idx; >>> + spinlock_t pbuf_lock; >>> + >>> /* OProfile support. */ >>> struct xenoprof *xenoprof; >>> int32_t time_offset_seconds; >> >> >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xen.org >> http://lists.xen.org/xen-devel > >
Daniel De Graaf
2013-Sep-09 13:46 UTC
Re: [PATCH v2] xen/console: buffer and show origin of guest PV writes
On 09/09/2013 07:13 AM, Keir Fraser wrote:> On 16/08/2013 20:01, "Daniel De Graaf" <dgdegra@tycho.nsa.gov> wrote: > >> Guests other than domain 0 using the console output have previously been >> controlled by the VERBOSE define, but with no designation of which >> guest''s output was on the console. This patch converts the HVM output >> buffering to be used by all domains, line buffering their output and >> prefixing it with the domain ID. This is especially useful for debugging >> stub domains. >> >> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > > This seems good, but, if we process and buffer dom0''s output, we lose the > possibility of running a terminal session in dom0 over the Xen console. > Personally I do that quite a bit -- serial access only, get Xen''s debugging > there, but also can log in to dom0. Does noone else??I do care about this use case (I also use it rather often), and it is preserved - this patch explicitly does not buffer or insert characters in dom0''s output. This means that we waste the 80-byte buffer for dom0, but I didn''t think it was worth special-casing dom0 there too (also, doing that might break a PVH dom0 that uses the HVM output - if that method is available, which I did not check).> > -- Keir > >> --- >> >> Changes since v1 (RFC): >> - Use prefix of (d%d) in place of (XEN) for both PV and HVM output >> - Guests other than dom0 have non-printable characters stripped >> - Use unsigned type for pbuf_idx >> - Formatting fixes >> >> xen/arch/x86/hvm/hvm.c | 27 +++++------- >> xen/common/domain.c | 8 ++++ >> xen/drivers/char/console.c | 94 >> ++++++++++++++++++++++++++++++++-------- >> xen/include/asm-x86/hvm/domain.h | 6 --- >> xen/include/xen/lib.h | 2 + >> xen/include/xen/sched.h | 6 +++ >> 6 files changed, 103 insertions(+), 40 deletions(-) >> >> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c >> index 1fcaed0..4ff76cc 100644 >> --- a/xen/arch/x86/hvm/hvm.c >> +++ b/xen/arch/x86/hvm/hvm.c >> @@ -485,8 +485,7 @@ static int hvm_set_ioreq_page( >> static int hvm_print_line( >> int dir, uint32_t port, uint32_t bytes, uint32_t *val) >> { >> - struct vcpu *curr = current; >> - struct hvm_domain *hd = &curr->domain->arch.hvm_domain; >> + struct domain *cd = current->domain; >> char c = *val; >> >> BUG_ON(bytes != 1); >> @@ -495,17 +494,16 @@ static int hvm_print_line( >> if ( !isprint(c) && (c != ''\n'') && (c != ''\t'') ) >> return X86EMUL_OKAY; >> >> - spin_lock(&hd->pbuf_lock); >> - hd->pbuf[hd->pbuf_idx++] = c; >> - if ( (hd->pbuf_idx == (HVM_PBUF_SIZE - 2)) || (c == ''\n'') ) >> + spin_lock(&cd->pbuf_lock); >> + if ( c != ''\n'' ) >> + cd->pbuf[cd->pbuf_idx++] = c; >> + if ( (cd->pbuf_idx == (DOMAIN_PBUF_SIZE - 1)) || (c == ''\n'') ) >> { >> - if ( c != ''\n'' ) >> - hd->pbuf[hd->pbuf_idx++] = ''\n''; >> - hd->pbuf[hd->pbuf_idx] = ''\0''; >> - printk(XENLOG_G_DEBUG "HVM%u: %s", curr->domain->domain_id, >> hd->pbuf); >> - hd->pbuf_idx = 0; >> + cd->pbuf[cd->pbuf_idx] = ''\0''; >> + guest_printk(cd, XENLOG_G_DEBUG "%s\n", cd->pbuf); >> + cd->pbuf_idx = 0; >> } >> - spin_unlock(&hd->pbuf_lock); >> + spin_unlock(&cd->pbuf_lock); >> >> return X86EMUL_OKAY; >> } >> @@ -521,19 +519,16 @@ int hvm_domain_initialise(struct domain *d) >> return -EINVAL; >> } >> >> - spin_lock_init(&d->arch.hvm_domain.pbuf_lock); >> spin_lock_init(&d->arch.hvm_domain.irq_lock); >> spin_lock_init(&d->arch.hvm_domain.uc_lock); >> >> INIT_LIST_HEAD(&d->arch.hvm_domain.msixtbl_list); >> spin_lock_init(&d->arch.hvm_domain.msixtbl_list_lock); >> >> - d->arch.hvm_domain.pbuf = xzalloc_array(char, HVM_PBUF_SIZE); >> d->arch.hvm_domain.params = xzalloc_array(uint64_t, HVM_NR_PARAMS); >> d->arch.hvm_domain.io_handler = xmalloc(struct hvm_io_handler); >> rc = -ENOMEM; >> - if ( !d->arch.hvm_domain.pbuf || !d->arch.hvm_domain.params || >> - !d->arch.hvm_domain.io_handler ) >> + if ( !d->arch.hvm_domain.params || !d->arch.hvm_domain.io_handler ) >> goto fail0; >> d->arch.hvm_domain.io_handler->num_slot = 0; >> >> @@ -578,7 +573,6 @@ int hvm_domain_initialise(struct domain *d) >> fail0: >> xfree(d->arch.hvm_domain.io_handler); >> xfree(d->arch.hvm_domain.params); >> - xfree(d->arch.hvm_domain.pbuf); >> return rc; >> } >> >> @@ -603,7 +597,6 @@ void hvm_domain_relinquish_resources(struct domain *d) >> >> xfree(d->arch.hvm_domain.io_handler); >> xfree(d->arch.hvm_domain.params); >> - xfree(d->arch.hvm_domain.pbuf); >> } >> >> void hvm_domain_destroy(struct domain *d) >> diff --git a/xen/common/domain.c b/xen/common/domain.c >> index 6c264a5..daac2c9 100644 >> --- a/xen/common/domain.c >> +++ b/xen/common/domain.c >> @@ -231,6 +231,8 @@ struct domain *domain_create( >> spin_lock_init(&d->shutdown_lock); >> d->shutdown_code = -1; >> >> + spin_lock_init(&d->pbuf_lock); >> + >> err = -ENOMEM; >> if ( !zalloc_cpumask_var(&d->domain_dirty_cpumask) ) >> goto fail; >> @@ -286,6 +288,10 @@ struct domain *domain_create( >> d->mem_event = xzalloc(struct mem_event_per_domain); >> if ( !d->mem_event ) >> goto fail; >> + >> + d->pbuf = xzalloc_array(char, DOMAIN_PBUF_SIZE); >> + if ( !d->pbuf ) >> + goto fail; >> } >> >> if ( (err = arch_domain_create(d, domcr_flags)) != 0 ) >> @@ -318,6 +324,7 @@ struct domain *domain_create( >> d->is_dying = DOMDYING_dead; >> atomic_set(&d->refcnt, DOMAIN_DESTROYED); >> xfree(d->mem_event); >> + xfree(d->pbuf); >> if ( init_status & INIT_arch ) >> arch_domain_destroy(d); >> if ( init_status & INIT_gnttab ) >> @@ -730,6 +737,7 @@ static void complete_domain_destroy(struct rcu_head *head) >> #endif >> >> xfree(d->mem_event); >> + xfree(d->pbuf); >> >> for ( i = d->max_vcpus - 1; i >= 0; i-- ) >> if ( (v = d->vcpu[i]) != NULL ) >> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c >> index 8ac32e4..b8d9a9f 100644 >> --- a/xen/drivers/char/console.c >> +++ b/xen/drivers/char/console.c >> @@ -24,6 +24,7 @@ >> #include <xen/shutdown.h> >> #include <xen/video.h> >> #include <xen/kexec.h> >> +#include <xen/ctype.h> >> #include <asm/debugger.h> >> #include <asm/div64.h> >> #include <xen/hypercall.h> /* for do_console_io */ >> @@ -375,6 +376,7 @@ static long >> guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count) >> { >> char kbuf[128]; >> int kcount; >> + struct domain *cd = current->domain; >> >> while ( count > 0 ) >> { >> @@ -388,19 +390,60 @@ static long >> guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count) >> return -EFAULT; >> kbuf[kcount] = ''\0''; >> >> - spin_lock_irq(&console_lock); >> + if ( is_hardware_domain(cd) ) >> + { >> + /* Use direct console output as it could be interactive */ >> + spin_lock_irq(&console_lock); >> + >> + sercon_puts(kbuf); >> + video_puts(kbuf); >> >> - sercon_puts(kbuf); >> - video_puts(kbuf); >> + if ( opt_console_to_ring ) >> + { >> + conring_puts(kbuf); >> + tasklet_schedule(¬ify_dom0_con_ring_tasklet); >> + } >> >> - if ( opt_console_to_ring ) >> + spin_unlock_irq(&console_lock); >> + } >> + else >> { >> - conring_puts(kbuf); >> - tasklet_schedule(¬ify_dom0_con_ring_tasklet); >> + char *kin = kbuf; >> + char *kout = kbuf; >> + char c; >> + /* Strip non-printable characters */ >> + for ( ; ; ) >> + { >> + c = *kin++; >> + if ( c == ''\0'' || c == ''\n'' ) >> + break; >> + if ( isprint(c) || c == ''\t'' ) >> + *kout++ = c; >> + } >> + *kout = ''\0''; >> + spin_lock(&cd->pbuf_lock); >> + if ( c == ''\n'' ) >> + { >> + kcount = kin - kbuf; >> + cd->pbuf[cd->pbuf_idx] = ''\0''; >> + guest_printk(cd, XENLOG_G_DEBUG "%s%s\n", cd->pbuf, kbuf); >> + cd->pbuf_idx = 0; >> + } >> + else if ( cd->pbuf_idx + kcount < (DOMAIN_PBUF_SIZE - 1) ) >> + { >> + /* buffer the output until a newline */ >> + memcpy(cd->pbuf + cd->pbuf_idx, kbuf, kcount); >> + cd->pbuf_idx += kcount; >> + } >> + else >> + { >> + cd->pbuf[cd->pbuf_idx] = ''\0''; >> + guest_printk(cd, XENLOG_G_DEBUG "%s%s\n", cd->pbuf, kbuf); >> + cd->pbuf_idx = 0; >> + } >> + spin_unlock(&cd->pbuf_lock); >> } >> >> - spin_unlock_irq(&console_lock); >> - >> guest_handle_add_offset(buffer, kcount); >> count -= kcount; >> } >> @@ -504,12 +547,12 @@ static int printk_prefix_check(char *p, char **pp) >> ((loglvl < upper_thresh) && printk_ratelimit())); >> } >> >> -static void printk_start_of_line(void) >> +static void printk_start_of_line(const char *prefix) >> { >> struct tm tm; >> char tstr[32]; >> >> - __putstr("(XEN) "); >> + __putstr(prefix); >> >> if ( !opt_console_timestamps ) >> return; >> @@ -524,12 +567,11 @@ static void printk_start_of_line(void) >> __putstr(tstr); >> } >> >> -void printk(const char *fmt, ...) >> +static void vprintk_common(const char *prefix, const char *fmt, va_list args) >> { >> static char buf[1024]; >> static int start_of_line = 1, do_print; >> >> - va_list args; >> char *p, *q; >> unsigned long flags; >> >> @@ -537,9 +579,7 @@ void printk(const char *fmt, ...) >> local_irq_save(flags); >> spin_lock_recursive(&console_lock); >> >> - va_start(args, fmt); >> (void)vsnprintf(buf, sizeof(buf), fmt, args); >> - va_end(args); >> >> p = buf; >> >> @@ -551,7 +591,7 @@ void printk(const char *fmt, ...) >> if ( do_print ) >> { >> if ( start_of_line ) >> - printk_start_of_line(); >> + printk_start_of_line(prefix); >> __putstr(p); >> __putstr("\n"); >> } >> @@ -566,7 +606,7 @@ void printk(const char *fmt, ...) >> if ( do_print ) >> { >> if ( start_of_line ) >> - printk_start_of_line(); >> + printk_start_of_line(prefix); >> __putstr(p); >> } >> start_of_line = 0; >> @@ -576,6 +616,26 @@ void printk(const char *fmt, ...) >> local_irq_restore(flags); >> } >> >> +void printk(const char *fmt, ...) >> +{ >> + va_list args; >> + va_start(args, fmt); >> + vprintk_common("(XEN) ", fmt, args); >> + va_end(args); >> +} >> + >> +void guest_printk(struct domain *d, const char *fmt, ...) >> +{ >> + va_list args; >> + char prefix[16]; >> + >> + snprintf(prefix, sizeof(prefix), "(d%d) ", d->domain_id); >> + >> + va_start(args, fmt); >> + vprintk_common(prefix, fmt, args); >> + va_end(args); >> +} >> + >> void __init console_init_preirq(void) >> { >> char *p; >> @@ -791,7 +851,7 @@ int __printk_ratelimit(int ratelimit_ms, int >> ratelimit_burst) >> snprintf(lost_str, sizeof(lost_str), "%d", lost); >> /* console_lock may already be acquired by printk(). */ >> spin_lock_recursive(&console_lock); >> - printk_start_of_line(); >> + printk_start_of_line("(XEN) "); >> __putstr("printk: "); >> __putstr(lost_str); >> __putstr(" messages suppressed.\n"); >> diff --git a/xen/include/asm-x86/hvm/domain.h >> b/xen/include/asm-x86/hvm/domain.h >> index 27b3de5..b1e3187 100644 >> --- a/xen/include/asm-x86/hvm/domain.h >> +++ b/xen/include/asm-x86/hvm/domain.h >> @@ -62,12 +62,6 @@ struct hvm_domain { >> /* emulated irq to pirq */ >> struct radix_tree_root emuirq_pirq; >> >> - /* hvm_print_line() logging. */ >> -#define HVM_PBUF_SIZE 80 >> - char *pbuf; >> - int pbuf_idx; >> - spinlock_t pbuf_lock; >> - >> uint64_t *params; >> >> /* Memory ranges with pinned cache attributes. */ >> diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h >> index 74b34eb..40afe12 100644 >> --- a/xen/include/xen/lib.h >> +++ b/xen/include/xen/lib.h >> @@ -83,6 +83,8 @@ extern void debugtrace_printk(const char *fmt, ...); >> #define _p(_x) ((void *)(unsigned long)(_x)) >> extern void printk(const char *format, ...) >> __attribute__ ((format (printf, 1, 2))); >> +extern void guest_printk(struct domain *d, const char *format, ...) >> + __attribute__ ((format (printf, 2, 3))); >> extern void panic(const char *format, ...) >> __attribute__ ((format (printf, 1, 2))); >> extern long vm_assist(struct domain *, unsigned int, unsigned int); >> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h >> index ae6a3b8..0013a8d 100644 >> --- a/xen/include/xen/sched.h >> +++ b/xen/include/xen/sched.h >> @@ -341,6 +341,12 @@ struct domain >> /* Control-plane tools handle for this domain. */ >> xen_domain_handle_t handle; >> >> + /* hvm_print_line() and guest_console_write() logging. */ >> +#define DOMAIN_PBUF_SIZE 80 >> + char *pbuf; >> + unsigned pbuf_idx; >> + spinlock_t pbuf_lock; >> + >> /* OProfile support. */ >> struct xenoprof *xenoprof; >> int32_t time_offset_seconds; > >-- Daniel De Graaf National Security Agency
Keir Fraser
2013-Sep-09 13:46 UTC
Re: [PATCH v2] xen/console: buffer and show origin of guest PV writes
On 09/09/2013 06:43, "Keir Fraser" <keir.xen@gmail.com> wrote:>> ...mean that Daniel has already done the right thing for dom0 here? > > Ah, you could be right. Well if he will confirm that then I will give my > Ack. > > -- KeirAlso, the unchanged behaviour for dom0 should be stated in the patch header. As it stands it actually explicitly says that the buffering/cooking is applied to all domains, with dom0 excluded only from stripping of non-printable chars. So either the patch header is wrong/misleading, or the code needs changing. -- Keir
Keir Fraser
2013-Sep-09 14:14 UTC
Re: [PATCH v2] xen/console: buffer and show origin of guest PV writes
On 09/09/2013 06:46, "Daniel De Graaf" <dgdegra@tycho.nsa.gov> wrote:> On 09/09/2013 07:13 AM, Keir Fraser wrote: >> On 16/08/2013 20:01, "Daniel De Graaf" <dgdegra@tycho.nsa.gov> wrote: >> >>> Guests other than domain 0 using the console output have previously been >>> controlled by the VERBOSE define, but with no designation of which >>> guest''s output was on the console. This patch converts the HVM output >>> buffering to be used by all domains, line buffering their output and >>> prefixing it with the domain ID. This is especially useful for debugging >>> stub domains. >>> >>> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> >> >> This seems good, but, if we process and buffer dom0''s output, we lose the >> possibility of running a terminal session in dom0 over the Xen console. >> Personally I do that quite a bit -- serial access only, get Xen''s debugging >> there, but also can log in to dom0. Does noone else?? > > I do care about this use case (I also use it rather often), and it is > preserved > - this patch explicitly does not buffer or insert characters in dom0''s output. > This means that we waste the 80-byte buffer for dom0, but I didn''t think it > was > worth special-casing dom0 there too (also, doing that might break a PVH dom0 > that uses the HVM output - if that method is available, which I did not > check).That''s great, but then the patch header is wrong as the output buffering is not used by *all* domains. Make it clear that dom0 is unaffected and then: Acked-by: Keir Fraser <keir@xen.org> -- Keir