[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2] xen/console: buffer and show origin of guest PV writes
On 09/09/2013 05:13, "Ian Campbell" <Ian.Campbell@xxxxxxxxxx> wrote: > On Mon, 2013-09-09 at 04:13 -0700, Keir Fraser wrote: >> On 16/08/2013 20:01, "Daniel De Graaf" <dgdegra@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> >> >> 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@xxxxxxxxxxxxx >> http://lists.xen.org/xen-devel > > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |