Message ID | 20231019154020.99080-17-dwmw2@infradead.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Get Xen PV shim running in Qemu, add net & console | expand |
On 19/10/2023 16:40, David Woodhouse wrote: > From: David Woodhouse <dwmw@amazon.co.uk> > > On soft reset, the prinary console event channel needs to be rebound to > the backend port (in the xen-console driver). We could put that into the > xen-console driver itself, but it's slightly less ugly to keep it within > the KVM/Xen code, by stashing the backend port# on event channel reset > and then rebinding in the primary console reset when it has to recreate > the guest port anyway. Does Xen re-bind the primary console on EVTCHNOP_reset? That's news to me. I go check. Paul > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> > --- > hw/i386/kvm/xen_evtchn.c | 9 +++++++++ > hw/i386/kvm/xen_primary_console.c | 29 ++++++++++++++++++++++++++++- > hw/i386/kvm/xen_primary_console.h | 1 + > 3 files changed, 38 insertions(+), 1 deletion(-) > > diff --git a/hw/i386/kvm/xen_evtchn.c b/hw/i386/kvm/xen_evtchn.c > index d72dca6591..ce4da6d37a 100644 > --- a/hw/i386/kvm/xen_evtchn.c > +++ b/hw/i386/kvm/xen_evtchn.c > @@ -40,6 +40,7 @@ > #include "xen_evtchn.h" > #include "xen_overlay.h" > #include "xen_xenstore.h" > +#include "xen_primary_console.h" > > #include "sysemu/kvm.h" > #include "sysemu/kvm_xen.h" > @@ -1098,6 +1099,7 @@ int xen_evtchn_soft_reset(void) > { > XenEvtchnState *s = xen_evtchn_singleton; > bool flush_kvm_routes; > + uint16_t con_port = xen_primary_console_get_port(); > int i; > > if (!s) { > @@ -1108,6 +1110,13 @@ int xen_evtchn_soft_reset(void) > > qemu_mutex_lock(&s->port_lock); > > + if (con_port) { > + XenEvtchnPort *p = &s->port_table[con_port]; > + if (p->type == EVTCHNSTAT_interdomain) { > + xen_primary_console_set_be_port(p->u.interdomain.port); > + } > + } > + > for (i = 0; i < s->nr_ports; i++) { > close_port(s, i, &flush_kvm_routes); > } > diff --git a/hw/i386/kvm/xen_primary_console.c b/hw/i386/kvm/xen_primary_console.c > index 0aa1c16ad6..5e6e085ac7 100644 > --- a/hw/i386/kvm/xen_primary_console.c > +++ b/hw/i386/kvm/xen_primary_console.c > @@ -112,6 +112,15 @@ uint16_t xen_primary_console_get_port(void) > return s->guest_port; > } > > +void xen_primary_console_set_be_port(uint16_t port) > +{ > + XenPrimaryConsoleState *s = xen_primary_console_singleton; > + if (s) { > + printf("be port set to %d\n", port); > + s->be_port = port; > + } > +} > + > uint64_t xen_primary_console_get_pfn(void) > { > XenPrimaryConsoleState *s = xen_primary_console_singleton; > @@ -142,6 +151,20 @@ static void alloc_guest_port(XenPrimaryConsoleState *s) > } > } > > +static void rebind_guest_port(XenPrimaryConsoleState *s) > +{ > + struct evtchn_bind_interdomain inter = { > + .remote_dom = DOMID_QEMU, > + .remote_port = s->be_port, > + }; > + > + if (!xen_evtchn_bind_interdomain_op(&inter)) { > + s->guest_port = inter.local_port; > + } > + > + s->be_port = 0; > +} > + > int xen_primary_console_reset(void) > { > XenPrimaryConsoleState *s = xen_primary_console_singleton; > @@ -154,7 +177,11 @@ int xen_primary_console_reset(void) > xen_overlay_do_map_page(&s->console_page, gpa); > } > > - alloc_guest_port(s); > + if (s->be_port) { > + rebind_guest_port(s); > + } else { > + alloc_guest_port(s); > + } > > trace_xen_primary_console_reset(s->guest_port); > > diff --git a/hw/i386/kvm/xen_primary_console.h b/hw/i386/kvm/xen_primary_console.h > index dd4922f3f4..7e2989ea0d 100644 > --- a/hw/i386/kvm/xen_primary_console.h > +++ b/hw/i386/kvm/xen_primary_console.h > @@ -16,6 +16,7 @@ void xen_primary_console_create(void); > int xen_primary_console_reset(void); > > uint16_t xen_primary_console_get_port(void); > +void xen_primary_console_set_be_port(uint16_t port); > uint64_t xen_primary_console_get_pfn(void); > void *xen_primary_console_get_map(void); >
On Tue, 2023-10-24 at 16:44 +0100, Paul Durrant wrote: > On 19/10/2023 16:40, David Woodhouse wrote: > > From: David Woodhouse <dwmw@amazon.co.uk> > > > > On soft reset, the prinary console event channel needs to be rebound to > > the backend port (in the xen-console driver). We could put that into the > > xen-console driver itself, but it's slightly less ugly to keep it within > > the KVM/Xen code, by stashing the backend port# on event channel reset > > and then rebinding in the primary console reset when it has to recreate > > the guest port anyway. > > Does Xen re-bind the primary console on EVTCHNOP_reset? That's news to > me. I go check. I spent an unhapp hour trying to work out how Xen actually does any of this :) In the short term I'm more interested in having soft reset work, than an explicit EVTCHNOP_reset. And I can't work out *how*, but we do seem to have console again after a kexec in real Xen.
On 24/10/2023 16:48, David Woodhouse wrote: > On Tue, 2023-10-24 at 16:44 +0100, Paul Durrant wrote: >> On 19/10/2023 16:40, David Woodhouse wrote: >>> From: David Woodhouse <dwmw@amazon.co.uk> >>> >>> On soft reset, the prinary console event channel needs to be rebound to >>> the backend port (in the xen-console driver). We could put that into the >>> xen-console driver itself, but it's slightly less ugly to keep it within >>> the KVM/Xen code, by stashing the backend port# on event channel reset >>> and then rebinding in the primary console reset when it has to recreate >>> the guest port anyway. >> >> Does Xen re-bind the primary console on EVTCHNOP_reset? That's news to >> me. I go check. > > I spent an unhapp hour trying to work out how Xen actually does any of > this :) > > In the short term I'm more interested in having soft reset work, than > an explicit EVTCHNOP_reset. And I can't work out *how*, but we do seem > to have console again after a kexec in real Xen. *Soft* reset may do it, but not the EVTCHNOP_reset hypercall itself, because there's a bunch of impenetrable toolstack magic involved the former. Perhaps you could just push the re-bind code up a layer into kvm_xen_soft_reset(). Paul
On Tue, 2023-10-24 at 17:22 +0100, Paul Durrant wrote: > On 24/10/2023 16:48, David Woodhouse wrote: > > On Tue, 2023-10-24 at 16:44 +0100, Paul Durrant wrote: > > > On 19/10/2023 16:40, David Woodhouse wrote: > > > > From: David Woodhouse <dwmw@amazon.co.uk> > > > > > > > > On soft reset, the prinary console event channel needs to be rebound to > > > > the backend port (in the xen-console driver). We could put that into the > > > > xen-console driver itself, but it's slightly less ugly to keep it within > > > > the KVM/Xen code, by stashing the backend port# on event channel reset > > > > and then rebinding in the primary console reset when it has to recreate > > > > the guest port anyway. > > > > > > Does Xen re-bind the primary console on EVTCHNOP_reset? That's news to > > > me. I go check. > > > > I spent an unhapp hour trying to work out how Xen actually does any of > > this :) > > > > In the short term I'm more interested in having soft reset work, than > > an explicit EVTCHNOP_reset. And I can't work out *how*, but we do seem > > to have console again after a kexec in real Xen. > > *Soft* reset may do it, but not the EVTCHNOP_reset hypercall itself, > because there's a bunch of impenetrable toolstack magic involved the > former. Perhaps you could just push the re-bind code up a layer into > kvm_xen_soft_reset(). Actually, all we're doing here is *storing* the be_port so that the *console* reset code can later connect to it. So the actual reconnect is already called separately from a layer up in kvm_xen_soft_reset(). If the guest just calls EVTCHNOP_reset then it'll just get the event channels reset and the console *won't* reconnect. Actually.. if the guest just calls EVTCHNOP_reset I think they'll just hit the assert(qemu_mutex_iothread_locked()) at line 1109. We need a QEMU_IOTHREAD_LOCK_GUARD() in the penultimate line of xen_evtchn_reset_op(). I'll fix that separately.
diff --git a/hw/i386/kvm/xen_evtchn.c b/hw/i386/kvm/xen_evtchn.c index d72dca6591..ce4da6d37a 100644 --- a/hw/i386/kvm/xen_evtchn.c +++ b/hw/i386/kvm/xen_evtchn.c @@ -40,6 +40,7 @@ #include "xen_evtchn.h" #include "xen_overlay.h" #include "xen_xenstore.h" +#include "xen_primary_console.h" #include "sysemu/kvm.h" #include "sysemu/kvm_xen.h" @@ -1098,6 +1099,7 @@ int xen_evtchn_soft_reset(void) { XenEvtchnState *s = xen_evtchn_singleton; bool flush_kvm_routes; + uint16_t con_port = xen_primary_console_get_port(); int i; if (!s) { @@ -1108,6 +1110,13 @@ int xen_evtchn_soft_reset(void) qemu_mutex_lock(&s->port_lock); + if (con_port) { + XenEvtchnPort *p = &s->port_table[con_port]; + if (p->type == EVTCHNSTAT_interdomain) { + xen_primary_console_set_be_port(p->u.interdomain.port); + } + } + for (i = 0; i < s->nr_ports; i++) { close_port(s, i, &flush_kvm_routes); } diff --git a/hw/i386/kvm/xen_primary_console.c b/hw/i386/kvm/xen_primary_console.c index 0aa1c16ad6..5e6e085ac7 100644 --- a/hw/i386/kvm/xen_primary_console.c +++ b/hw/i386/kvm/xen_primary_console.c @@ -112,6 +112,15 @@ uint16_t xen_primary_console_get_port(void) return s->guest_port; } +void xen_primary_console_set_be_port(uint16_t port) +{ + XenPrimaryConsoleState *s = xen_primary_console_singleton; + if (s) { + printf("be port set to %d\n", port); + s->be_port = port; + } +} + uint64_t xen_primary_console_get_pfn(void) { XenPrimaryConsoleState *s = xen_primary_console_singleton; @@ -142,6 +151,20 @@ static void alloc_guest_port(XenPrimaryConsoleState *s) } } +static void rebind_guest_port(XenPrimaryConsoleState *s) +{ + struct evtchn_bind_interdomain inter = { + .remote_dom = DOMID_QEMU, + .remote_port = s->be_port, + }; + + if (!xen_evtchn_bind_interdomain_op(&inter)) { + s->guest_port = inter.local_port; + } + + s->be_port = 0; +} + int xen_primary_console_reset(void) { XenPrimaryConsoleState *s = xen_primary_console_singleton; @@ -154,7 +177,11 @@ int xen_primary_console_reset(void) xen_overlay_do_map_page(&s->console_page, gpa); } - alloc_guest_port(s); + if (s->be_port) { + rebind_guest_port(s); + } else { + alloc_guest_port(s); + } trace_xen_primary_console_reset(s->guest_port); diff --git a/hw/i386/kvm/xen_primary_console.h b/hw/i386/kvm/xen_primary_console.h index dd4922f3f4..7e2989ea0d 100644 --- a/hw/i386/kvm/xen_primary_console.h +++ b/hw/i386/kvm/xen_primary_console.h @@ -16,6 +16,7 @@ void xen_primary_console_create(void); int xen_primary_console_reset(void); uint16_t xen_primary_console_get_port(void); +void xen_primary_console_set_be_port(uint16_t port); uint64_t xen_primary_console_get_pfn(void); void *xen_primary_console_get_map(void);