[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 07:13 AM, 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 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(&notify_dom0_con_ring_tasklet);
+            }

-        if ( opt_console_to_ring )
+            spin_unlock_irq(&console_lock);
+        }
+        else
          {
-            conring_puts(kbuf);
-            tasklet_schedule(&notify_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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.