Allow multiple dom0less domains to use the console_io hypercalls to
print to the console. Handle them in a similar way to vpl011: only the
domain which has focus can read from the console. All domains can write
to the console but the ones without focus have a prefix. In this case
the prefix is applied by using guest_printk instead of printk or
console_puts which is what the original code was already doing.

When switching focus using Ctrl-AAA, discard any unread data in the
input buffer. Input is read quickly and the user would be aware of it
being slow or stuck as they use Ctrl-AAA to switch focus domain.
In that situation, it is to be expected that the unread input is lost.

The domain writes are buffered when the domain is not in focus. Push out
the buffer after the domain enters focus on the first guest write.

Locking updates:

- Discard unread input under the lock when switching focus (including
  when returning to Xen) so that cross-domain reads can't see stale data

- Require is_focus_domain() callers to hold console_lock, and re-check
  focus after each chunk in the CONSOLEIO_read loop so a focus change
  simply stops further copies without duplicating or leaking input

- Hold cons->lock while flushing buffered writes in the focus path so
  the direct-write fast path does not race buffered guests or HVM debug
  output

Signed-off-by: Stefano Stabellini <[email protected]>
---
Changes in v10:
- patch split into two halves this is the second half
---
 xen/drivers/char/console.c | 52 ++++++++++++++++++++++++++++++++++----
 1 file changed, 47 insertions(+), 5 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index ad53073b99..d3ce925131 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -541,6 +541,12 @@ void console_put_domain(struct domain *d)
         rcu_unlock_domain(d);
 }
 
+static bool is_focus_domain(const struct domain *d)
+{
+    ASSERT(rspin_is_locked(&console_lock));
+    return d != NULL && d->domain_id == console_rx - 1;
+}
+
 static void console_switch_input(void)
 {
     unsigned int next_rx = ACCESS_ONCE(console_rx);
@@ -559,6 +565,7 @@ static void console_switch_input(void)
         {
             nrspin_lock_irqsave(&console_lock, flags);
             ACCESS_ONCE(console_rx) = 0;
+            serial_rx_cons = serial_rx_prod;
             nrspin_unlock_irqrestore(&console_lock, flags);
             printk("*** Serial input to Xen");
             break;
@@ -581,6 +588,8 @@ static void console_switch_input(void)
 
             nrspin_lock_irqsave(&console_lock, flags);
             ACCESS_ONCE(console_rx) = next_rx;
+            /* Don't let the next dom read the previous dom's unread data. */
+            serial_rx_cons = serial_rx_prod;
             nrspin_unlock_irqrestore(&console_lock, flags);
             printk("*** Serial input to DOM%u", domid);
             break;
@@ -610,7 +619,7 @@ static void __serial_rx(char c)
         unsigned long flags;
 
         /*
-         * Deliver input to the hardware domain buffer, unless it is
+         * Deliver input to the focus domain buffer, unless it is
          * already full.
          */
         nrspin_lock_irqsave(&console_lock, flags);
@@ -740,6 +749,7 @@ static long 
guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
     unsigned int flags = opt_console_to_ring
                          ? CONSOLE_ALL : CONSOLE_DEFAULT;
     struct domain *cd = current->domain;
+    struct domain_console *cons = cd->console;
 
     while ( count > 0 )
     {
@@ -752,17 +762,36 @@ static long 
guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
         if ( copy_from_guest(kbuf, buffer, kcount) )
             return -EFAULT;
 
-        if ( is_hardware_domain(cd) )
+        /*
+         * Take both cons->lock and console_lock:
+         * - cons->lock protects cons->buf and cons->idx
+         * - console_lock protects console_send() and is_focus_domain()
+         *   checks
+         *
+         * The order must be respected. guest_printk() takes the
+         * console_lock and it is called with cons->lock held. It is
+         * important that cons->lock is taken first.
+         */
+        spin_lock(&cons->lock);
+        nrspin_lock_irq(&console_lock);
+        if ( is_focus_domain(cd) )
         {
+            if ( cons->idx )
+            {
+                console_send(cons->buf, cons->idx, flags);
+                cons->idx = 0;
+            }
+            spin_unlock(&cons->lock);
+
             /* Use direct console output as it could be interactive */
-            nrspin_lock_irq(&console_lock);
             console_send(kbuf, kcount, flags);
             nrspin_unlock_irq(&console_lock);
         }
         else
         {
             char *kin = kbuf, *kout = kbuf, c;
-            struct domain_console *cons = cd->console;
+
+            nrspin_unlock_irq(&console_lock);
 
             /* Strip non-printable characters */
             do
@@ -775,7 +804,6 @@ static long 
guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
             } while ( --kcount > 0 );
 
             *kout = '\0';
-            spin_lock(&cons->lock);
             kcount = kin - kbuf;
             if ( c != '\n' &&
                  (cons->idx + (kout - kbuf) < (ARRAY_SIZE(cons->buf) - 1)) )
@@ -828,6 +856,8 @@ long do_console_io(
 
         rc = 0;
         nrspin_lock_irq(&console_lock);
+        if ( !is_focus_domain(current->domain) )
+            count = 0;
         while ( (serial_rx_cons != serial_rx_prod) && (rc < count) )
         {
             idx = SERIAL_RX_MASK(serial_rx_cons);
@@ -844,7 +874,19 @@ long do_console_io(
                return -EFAULT;
             rc += len;
 
+            /*
+             * First get console_lock again, then check is_focus_domain().
+             * It is possible that we switched focus domain during
+             * copy_to_guest_offset(). In that case, serial_rx_cons and
+             * serial_rx_prod have been reset so it would be wrong to
+             * update serial_rx_cons here. Get out of the loop instead.
+             *
+             * rc is updated regardless to provide the correct return
+             * value to the VM as the data has been copied.
+             */
             nrspin_lock_irq(&console_lock);
+            if ( !is_focus_domain(current->domain) )
+                break;
 
             serial_rx_cons += len;
         }
-- 
2.25.1


Reply via email to