| Message ID | 20241217222424.7632-1-jason.andryuk@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC,Hyperlaunch] xenconsole: Add connected flag | expand |
On 17/12/2024 10:24 pm, Jason Andryuk wrote: > This patch is similar to the xenstore late init (though the flag is > reversed). xenstore late init is fundamentally broken. Do not copy it. c/s fc2b57c9af462d67df871b080c0897156a616b7d broke the the ABI of that bit by trying to repurpose it for an incompatible use. Despite the claims, it is not possible for a 3rd to be involved in any way in the xenstore page. This lead to some nasty state corruption issues in the ring, which were simply papered over by repositioning the middle hunk, and not fixed. It causes cxenstored to have an incompatible ABI divergence of an oxenstored feature. The only reason I haven't reverted it is because it is my understanding that Hyperlauch is going to need to revert it anyway. The very first thing you need is a precise description of the semantics of this bit, including how it is used by the client and the server. I'll be dammed if I let a second spectacular breakage of a public interface occur due to inadequate planning; one is bad enough. ~Andrew
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); + /* spurious 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..2b408f92a3 100644 --- a/xen/include/public/io/console.h +++ b/xen/include/public/io/console.h @@ -19,6 +19,8 @@ struct xencons_interface { char out[2048]; XENCONS_RING_IDX in_cons, in_prod; XENCONS_RING_IDX out_cons, out_prod; + #define XENCONSOLE_CONNECTED 1 + uint32_t flag; }; #ifdef XEN_WANT_FLEX_CONSOLE_RING
Linux will spin in hvc_xen.c:domU_write_console() when it fills its console ring. This blocks hyperlaunch parallel boot since the domU won't progress until dom0 sets up xenconsoled and starts draining the ring. Add a flag to the console ring page. zero-initialized, have xenconsoled set to 1 when it connects. Also send an event channel notification. The linux domU side can keep using the console hypercalls and only transition to the ring when it is connected and won't fill and block. Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> --- 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); } This patch is similar to the xenstore late init (though the flag is reversed). 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 | 2 ++ 2 files changed, 8 insertions(+)