From patchwork Tue Oct 17 23:46:52 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: David Woodhouse X-Patchwork-Id: 734958 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 635B8CDB474 for ; Tue, 17 Oct 2023 23:47:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234650AbjJQXrO (ORCPT ); Tue, 17 Oct 2023 19:47:14 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51770 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230219AbjJQXrN (ORCPT ); Tue, 17 Oct 2023 19:47:13 -0400 Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:8b0:10b:1236::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 368C898; Tue, 17 Oct 2023 16:47:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=MIME-Version:Content-Type:Date:Cc:To: From:Subject:Message-ID:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:In-Reply-To:References; bh=UyDAj6MCb5OdE6yv5OJjKtOf1V/fjbGh/bOznPBpmTU=; b=S2xRRGbvCIRDPTVLWsTxb6+4J0 rM0LeuRIgJXuilG8Ccms/UjP7yHbHHxrMqnETE9bL0CZcqXTSXG2ocAEu/bZ6ZTyCD8ME2bNF5Lcb r+kjjk3FaEh/aNifZIQvn/rvQmRekQ5rlbEC9lPHYmcYg9aLHQUn5svdmHJGdj7Zvo6ZnfmHy9JoT TCQ74hJ8BF6rqO9wmZSXlZ9HDJWicOMHu36/22lvtu+sKE94fIdQdAU8bhruK/uEfZ9JU6w5zbUF+ vLtHAnwvPyysMIgNpBIF2qbq4/OepGYDUXa+ZUD5jADp5HeQ5qEhWH/EGIyaBsWFS0v4fz0+C7IVc hpD6vRXQ==; Received: from [2001:8b0:10b:5:b9aa:f92b:5d4c:b38] (helo=u3832b3a9db3152.ant.amazon.com) by casper.infradead.org with esmtpsa (Exim 4.94.2 #2 (Red Hat Linux)) id 1qstm1-00FENl-21; Tue, 17 Oct 2023 23:46:53 +0000 Message-ID: <72609f999d9451c13240acb8e4a4e54f8c62f542.camel@infradead.org> Subject: [PATCH] hvc/xen: fix event channel handling for secondary consoles From: David Woodhouse To: Juergen Gross , xen-devel Cc: Greg Kroah-Hartman , Jiri Slaby , Roger Pau Monne , Stefano Stabellini , Dawei Li , linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org, Paul Durrant Date: Wed, 18 Oct 2023 00:46:52 +0100 User-Agent: Evolution 3.44.4-0ubuntu2 MIME-Version: 1.0 X-SRS-Rewrite: SMTP reverse-path rewritten from by casper.infradead.org. See http://www.infradead.org/rpr.html Precedence: bulk List-ID: X-Mailing-List: linux-serial@vger.kernel.org From: David Woodhouse The xencons_connect_backend() function allocates a local interdomain event channel with xenbus_alloc_evtchn(), then calls bind_interdomain_evtchn_to_irq_lateeoi() to bind to that port# on the *remote* domain. That doesn't work very well: (qemu) device_add xen-console,id=con1,chardev=pty0 [ 44.323872] xenconsole console-1: 2 xenbus_dev_probe on device/console/1 [ 44.323995] xenconsole: probe of console-1 failed with error -2 Fix it to use bind_evtchn_to_irq_lateeoi(), which does the right thing by just binding that *local* event channel to an irq. The backend will do the interdomain binding. This didn't affect the primary console because the setup for that is special — the toolstack allocates the guest event channel and the guest discovers it with HVMOP_get_param. Once that's fixed, there's also a warning on hot-unplug because xencons_disconnect_backend() unconditionally calls free_irq() via unbind_from_irqhandler(): (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 Fix that by calling notifier_del_irq() first, which only calls free_irq() if the irq was requested in the first place. Then use evtchn_put() to release the irq and event channel. Avoid calling xenbus_free_evtchn() in the normal case, as evtchn_put() will do that too. The only time xenbus_free_evtchn() needs to be called is for the cleanup when bind_evtchn_to_irq_lateeoi() fails. Finally, fix the error path in xen_hvc_init() when there's no primary console. It should still register the frontend driver, as there may be secondary consoles. (Qemu can always add secondary consoles, but only the toolstack can add the primary because it's special.) Fixes: fe415186b4 ("xen/console: harden hvc_xen against event channel storms") Signed-off-by: David Woodhouse Cc: stable@vger.kernel.org Reviewed-by: Juergen Gross --- Untested on actual Xen. drivers/tty/hvc/hvc_xen.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c index 98764e740c07..f0376612b267 100644 --- a/drivers/tty/hvc/hvc_xen.c +++ b/drivers/tty/hvc/hvc_xen.c @@ -377,9 +377,13 @@ void xen_console_resume(void) #ifdef CONFIG_HVC_XEN_FRONTEND static void xencons_disconnect_backend(struct xencons_info *info) { - if (info->irq > 0) - unbind_from_irqhandler(info->irq, NULL); - info->irq = 0; + if (info->irq > 0) { + notifier_del_irq(info->hvc, info->irq); + evtchn_put(info->evtchn); + info->irq = 0; + info->evtchn = 0; + } + /* evtchn_put() will also close it so this is only an error path */ if (info->evtchn > 0) xenbus_free_evtchn(info->xbdev, info->evtchn); info->evtchn = 0; @@ -433,7 +437,7 @@ static int xencons_connect_backend(struct xenbus_device *dev, if (ret) return ret; info->evtchn = evtchn; - irq = bind_interdomain_evtchn_to_irq_lateeoi(dev, evtchn); + irq = bind_evtchn_to_irq_lateeoi(evtchn); if (irq < 0) return irq; info->irq = irq; @@ -597,7 +601,7 @@ static int __init xen_hvc_init(void) else r = xen_pv_console_init(); if (r < 0) - return r; + goto register_fe; info = vtermno_to_xencons(HVC_COOKIE); info->irq = bind_evtchn_to_irq_lateeoi(info->evtchn); @@ -616,12 +620,13 @@ static int __init xen_hvc_init(void) list_del(&info->list); spin_unlock_irqrestore(&xencons_lock, flags); if (info->irq) - unbind_from_irqhandler(info->irq, NULL); + evtchn_put(info->evtchn); kfree(info); return r; } r = 0; + register_fe: #ifdef CONFIG_HVC_XEN_FRONTEND r = xenbus_register_frontend(&xencons_driver); #endif