Messages in this thread | | | Date | Mon, 23 Oct 2023 10:14:32 +0200 | Subject | Re: [PATCH v2 3/3] hvc/xen: fix console unplug | From | Juergen Gross <> |
| |
On 20.10.23 18:15, David Woodhouse wrote: > From: David Woodhouse <dwmw@amazon.co.uk> > > On unplug of a Xen console, xencons_disconnect_backend() unconditionally > calls free_irq() via unbind_from_irqhandler(), causing a warning of > freeing an already-free IRQ: > > (qemu) device_del con1 > [ 32.050919] ------------[ cut here ]------------ > [ 32.050942] Trying to free already-free IRQ 33 > [ 32.050990] WARNING: CPU: 0 PID: 51 at kernel/irq/manage.c:1895 __free_irq+0x1d4/0x330 > > It should be using evtchn_put() to tear down the event channel binding, > and let the Linux IRQ side of it be handled by notifier_del_irq() through > the HVC code. > > On which topic... xencons_disconnect_backend() should call hvc_remove() > *first*, rather than tearing down the event channel and grant mapping > while they are in use. And then the IRQ is guaranteed to be freed by > the time it's torn down by evtchn_put(). > > Since evtchn_put() also closes the actual event channel, avoid calling > xenbus_free_evtchn() except in the failure path where the IRQ was not > successfully set up. > > However, calling hvc_remove() at the start of xencons_disconnect_backend() > still isn't early enough. An unplug request is indicated by the backend > setting its state to XenbusStateClosing, which triggers a notification > to xencons_backend_changed(), which... does nothing except set its own > frontend state directly to XenbusStateClosed without *actually* tearing > down the HVC device or, you know, making sure it isn't actively in use. > > So the backend sees the guest frontend set its state to XenbusStateClosed > and stops servicing the interrupt... and the guest spins for ever in the > domU_write_console() function waiting for the ring to drain. > > Fix that one by calling hvc_remove() from xencons_backend_changed() before > signalling to the backend that it's OK to proceed with the removal. > > Tested with 'dd if=/dev/zero of=/dev/hvc1' while telling Qemu to remove > the console device. > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> > Cc: stable@vger.kernel.org
Reviewed-by: Juergen Gross <jgross@suse.com>
Juergen
[unhandled content-type:application/pgp-keys][unhandled content-type:application/pgp-signature] | |