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: 13426210 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 lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 523C7CDB482 for ; Tue, 17 Oct 2023 23:47:31 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.618369.961932 (Exim 4.92) (envelope-from ) id 1qstmF-0000nm-26; Tue, 17 Oct 2023 23:47:07 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 618369.961932; Tue, 17 Oct 2023 23:47:07 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1qstmE-0000nf-Vf; Tue, 17 Oct 2023 23:47:06 +0000 Received: by outflank-mailman (input) for mailman id 618369; Tue, 17 Oct 2023 23:47:05 +0000 Received: from se1-gles-flk1-in.inumbo.com ([94.247.172.50] helo=se1-gles-flk1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1qstmC-0000nX-4P for xen-devel@lists.xenproject.org; Tue, 17 Oct 2023 23:47:05 +0000 Received: from casper.infradead.org (casper.infradead.org [2001:8b0:10b:1236::1]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id 75963796-6d47-11ee-9b0e-b553b5be7939; Wed, 18 Oct 2023 01:46:58 +0200 (CEST) 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 X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 75963796-6d47-11ee-9b0e-b553b5be7939 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==; 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 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