diff mbox series

[RFC,Hyperlaunch,v2] xenconsole: Add connected flag

Message ID 20241219223222.2211-1-jason.andryuk@amd.com (mailing list archive)
State New
Headers show
Series [RFC,Hyperlaunch,v2] xenconsole: Add connected flag | expand

Commit Message

Jason Andryuk Dec. 19, 2024, 10:32 p.m. UTC
Sending again with an expanded description.  RFC to have a discussion
about the approach.

With hyperlaunch, a domU can start before its console ring is connected
by xenconsoled.  With nothing emptying the ring, it can quickly fill
during boot.  In domU_write_console(), __write_console returns 0 when
the ring is full.  This loops spins until xenconsoled starts emptying
the ring:

        while (len) {
                ssize_t sent = __write_console(cons, data, len);

                if (sent < 0)
                        return sent;

                data += sent;
                len -= sent;

                if (unlikely(len))
                        HYPERVISOR_sched_op(SCHEDOP_yield, NULL);
        }

The goal of this patch is to add a way for the frontend to know when a
console is connected.  This patch adds a new flag to the end of the
console ring structure.  It is used for the backend to indicate that it
has connected and started servicing the page.  The initial value is
XENCONSOLE_DISCONNECTED (0), which matches a zero-initialized page
value.  XENCONSOLE_DISCONNECTED indicates to the guest that ring is
disconnected, so it will not be serviced.

Once the backend (xenconsoled) maps and starts servicing the
console, the flag will be set to XENCONSOLE_CONNECTED (1) to indicate
the backend state to the frontend.

A domU can use console hypercalls and only transition to the ring when
it is connected and won't fill and block.

With the flag change, the backend sends an event channel notification so
the frontend can check the state.  Though this is not strictly
necessary.

Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
This patch is similar to the xenstore late init in that it indicates
whether the backend is usable.  This patch is not changing existing
meaning of flags.

This does assume that existing frontends are not using the flag space
for some other use.  Also, it assumes zero-initialized memory for the
page used by xencons_interface structure.  Or at least the 32bit flag
value being not XENCONSOLE_CONNECTED.  However, this only matters for a
hyperlaunched guest - xenconsoled will normally readily connect the
console and set the value.

Disconnecting and reconnecting would not be supported.  I don't think
that is supported today, but I could be wrong.  Does soft_reset
disconnect and reconnect, or does xenconsoled maintain the connection?
The backend could set the flag to DISCONNECTED before unmapping on
garceful disconnect.  A crash would not be handled.

The idea of the event channel notification is to let the domU receive an
indication that xenconsoled is connected.  For xenstore, the xenstore
driver owns the event channel/irq and can rebind it.  For hvc_xen, the
hvc subsystem owns the irq, so it isn't readily available for rebinding.

I had the idea for the kernel to use a static key and switch writing
from the hypercall to the PV ring once connected.  It didn't actually
work in my short attempt - I think changing the static key from within
an interupt was wrong.  I fell back to just checking the flag directly
without an optimization.  That would work and would make the event
channel notification unnecessary.
---
 tools/console/daemon/io.c       |  6 ++++++
 xen/include/public/io/console.h | 13 +++++++++++++
 2 files changed, 19 insertions(+)

Comments

Jan Beulich Dec. 20, 2024, 1 p.m. UTC | #1
On 19.12.2024 23:32, Jason Andryuk wrote:
> --- a/xen/include/public/io/console.h
> +++ b/xen/include/public/io/console.h
> @@ -19,6 +19,19 @@ struct xencons_interface {
>      char out[2048];
>      XENCONS_RING_IDX in_cons, in_prod;
>      XENCONS_RING_IDX out_cons, out_prod;
> +/*
> + * Flag values signaling from backend to frontend whether the console is
> + * connected.  i.e. Whether it will be serviced and emptied.
> + *
> + * The flag starts as disconnected.
> + */
> +#define XENCONSOLE_DISCONNECTED 0
> +/*
> + * The flag is set to connected when the backend connects and the console
> + * will be serviced.
> + */
> +#define XENCONSOLE_CONNECTED    1
> +    uint32_t flag;
>  };

Given no other information, how would a guest know that it can actually
expect the flag to (eventually) become set?

Jan
Jason Andryuk Dec. 20, 2024, 3:38 p.m. UTC | #2
On 2024-12-20 08:00, Jan Beulich wrote:
> On 19.12.2024 23:32, Jason Andryuk wrote:
>> --- a/xen/include/public/io/console.h
>> +++ b/xen/include/public/io/console.h
>> @@ -19,6 +19,19 @@ struct xencons_interface {
>>       char out[2048];
>>       XENCONS_RING_IDX in_cons, in_prod;
>>       XENCONS_RING_IDX out_cons, out_prod;
>> +/*
>> + * Flag values signaling from backend to frontend whether the console is
>> + * connected.  i.e. Whether it will be serviced and emptied.
>> + *
>> + * The flag starts as disconnected.
>> + */
>> +#define XENCONSOLE_DISCONNECTED 0
>> +/*
>> + * The flag is set to connected when the backend connects and the console
>> + * will be serviced.
>> + */
>> +#define XENCONSOLE_CONNECTED    1
>> +    uint32_t flag;
>>   };
> 
> Given no other information, how would a guest know that it can actually
> expect the flag to (eventually) become set?

You mean that without some other flag to indicate the presence of this 
flag, the guest wouldn't know whether this field should change?  Yes, 
that is a good point.

Non-hyperlaunch, a new xenconsoled would set the flag ~before the guest 
starts.  AIUI, there is no explicit synchronization between the 
toolstack and xenconsoled that the console is connected before the guest 
is unpaused.

Old kernels without checking the flag will just use the console.

A new kernel, with an old xenconsoled, would never see the flag set and 
would never use the console.

Thanks,
Jason
diff mbox series

Patch

diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index bb739bdb8c..2a7ecc6369 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -731,6 +731,9 @@  static int console_create_ring(struct console *con)
 		con->ring_ref = ring_ref;
 	}
 
+	/* Mark the console as connected. */
+	con->interface->flag = XENCONSOLE_CONNECTED;
+
 	/* Go no further if port has not changed and we are still bound. */
 	if (remote_port == con->remote_port) {
 		xc_evtchn_status_t status = {
@@ -780,6 +783,9 @@  static int console_create_ring(struct console *con)
 	if (log_guest && (con->log_fd == -1))
 		con->log_fd = create_console_log(con);
 
+	/* Notify to check flags field. */
+	xenevtchn_notify(con->xce_handle, con->local_port);
+
  out:
 	return err;
 }
diff --git a/xen/include/public/io/console.h b/xen/include/public/io/console.h
index 4509b4b689..d847aa080f 100644
--- a/xen/include/public/io/console.h
+++ b/xen/include/public/io/console.h
@@ -19,6 +19,19 @@  struct xencons_interface {
     char out[2048];
     XENCONS_RING_IDX in_cons, in_prod;
     XENCONS_RING_IDX out_cons, out_prod;
+/*
+ * Flag values signaling from backend to frontend whether the console is
+ * connected.  i.e. Whether it will be serviced and emptied.
+ *
+ * The flag starts as disconnected.
+ */
+#define XENCONSOLE_DISCONNECTED 0
+/*
+ * The flag is set to connected when the backend connects and the console
+ * will be serviced.
+ */
+#define XENCONSOLE_CONNECTED    1
+    uint32_t flag;
 };
 
 #ifdef XEN_WANT_FLEX_CONSOLE_RING