From mboxrd@z Thu Jan 1 00:00:00 1970 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 359D6FF8877 for ; Wed, 29 Apr 2026 09:56:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=rLlTGpnl82QX+tRpG0gd+HQaTomjLOyXMXGgqvgkCBk=; b=xnHHDG4SnZlOmd A21rFOYKQnJyBaji860Mjc+Vb84r6OdiRvavn8hOR1KfMvM6rN9ljeyF0E7R6v9PMgnbiNMcFeZx6 D2/zHXN/B7uTftIFE61o+9ORygiFsaWEYTp8+wAVRcOn1ZprcI4IwxG+7I6ZAQ9Zck0sEitZknYLh g6ysRYvCm+ke4HdfdISxdioiIiYS8lIjTTmSjhati9G0TOqYQf3GjxrgDaCGmE3Hbtbh1CsmzpP9R ZsISlyhtUhJ8180MAqZxsd4N1kXKVBKe2m2t5eWXJGhTeO3ZqqUyz3zNw62IuDIt0p0P88E/438ft cl929Fu0qzCY6pSjHyaw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1wI1eO-00000003Og9-3BrW; Wed, 29 Apr 2026 09:56:12 +0000 Received: from linux.microsoft.com ([13.77.154.182]) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1wI1eM-00000003OfF-127w; Wed, 29 Apr 2026 09:56:11 +0000 Received: from [10.95.65.160] (unknown [167.220.238.0]) by linux.microsoft.com (Postfix) with ESMTPSA id 64EEC20B716D; Wed, 29 Apr 2026 02:55:55 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 64EEC20B716D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1777456562; bh=fPeBJsFqpoo+2GdY4mHjPpIMHuwqDUdZaSruCXTJrak=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=T8QQBSnuhbdga7x4qEdwC5GZPzFgwlYf2WqARJb11uBsZQpYGJmwXoDwqDXFtzto5 fPLHAG0knbJHiSxvTqL44F14Cc2BFS+PyBGByoqAywuQ8/vZQellbl02B5e1OOdsJ6 DjHGtk8P6ExJYnkJrl2siSeOafxOqtujr+xxRSFE= Message-ID: Date: Wed, 29 Apr 2026 15:25:51 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 03/15] Drivers: hv: Move vmbus_handler to common code To: Michael Kelley , "K . Y . Srinivasan" , Haiyang Zhang , Wei Liu , Dexuan Cui , Long Li , Catalin Marinas , Will Deacon , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , "x86@kernel.org" , "H . Peter Anvin" , Arnd Bergmann , Paul Walmsley , Palmer Dabbelt , Albert Ou , Alexandre Ghiti Cc: Marc Zyngier , Timothy Hayes , Lorenzo Pieralisi , Sascha Bischoff , mrigendrachaubey , "linux-hyperv@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "linux-arch@vger.kernel.org" , "linux-riscv@lists.infradead.org" , "vdso@mailbox.org" , "ssengar@linux.microsoft.com" References: <20260423124206.2410879-1-namjain@linux.microsoft.com> <20260423124206.2410879-4-namjain@linux.microsoft.com> Content-Language: en-US From: Naman Jain In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20260429_025610_315258_AE8B2994 X-CRM114-Status: GOOD ( 28.99 ) X-BeenThere: linux-riscv@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-riscv" Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org On 4/27/2026 11:08 AM, Michael Kelley wrote: > From: Naman Jain Sent: Thursday, April 23, 2026 5:42 AM >> >> Move the vmbus_handler global variable and hv_setup_vmbus_handler()/ >> hv_remove_vmbus_handler() from arch/x86 to drivers/hv/hv_common.c. >> >> hv_setup_vmbus_handler() is called unconditionally in vmbus_bus_init() >> and works for both x86 (sysvec handler) and arm64 (vmbus_percpu_isr). >> >> This eliminates the need for separate percpu vmbus handler setup >> functions and __weak stubs, that are needed for adding ARM64 support >> in MSHV_VTL driver where we need to set a custom per-cpu vmbus handler. >> >> Signed-off-by: Naman Jain >> --- >> arch/x86/kernel/cpu/mshyperv.c | 12 ------------ >> drivers/hv/hv_common.c | 9 +++++++-- >> drivers/hv/vmbus_drv.c | 17 +++++++++-------- >> include/asm-generic/mshyperv.h | 1 + >> 4 files changed, 17 insertions(+), 22 deletions(-) >> >> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c >> index 89a2eb8a0722..68706ff5880e 100644 >> --- a/arch/x86/kernel/cpu/mshyperv.c >> +++ b/arch/x86/kernel/cpu/mshyperv.c >> @@ -145,7 +145,6 @@ void hv_set_msr(unsigned int reg, u64 value) >> EXPORT_SYMBOL_GPL(hv_set_msr); >> >> static void (*mshv_handler)(void); >> -static void (*vmbus_handler)(void); >> static void (*hv_stimer0_handler)(void); >> static void (*hv_kexec_handler)(void); >> static void (*hv_crash_handler)(struct pt_regs *regs); >> @@ -172,17 +171,6 @@ void hv_setup_mshv_handler(void (*handler)(void)) >> mshv_handler = handler; >> } >> >> -void hv_setup_vmbus_handler(void (*handler)(void)) >> -{ >> - vmbus_handler = handler; >> -} >> - >> -void hv_remove_vmbus_handler(void) >> -{ >> - /* We have no way to deallocate the interrupt gate */ >> - vmbus_handler = NULL; >> -} >> - >> /* >> * Routines to do per-architecture handling of stimer0 >> * interrupts when in Direct Mode >> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c >> index e8633bc51d56..eb7b0028b45d 100644 >> --- a/drivers/hv/hv_common.c >> +++ b/drivers/hv/hv_common.c >> @@ -758,13 +758,18 @@ bool __weak hv_isolation_type_tdx(void) >> } >> EXPORT_SYMBOL_GPL(hv_isolation_type_tdx); >> >> -void __weak hv_setup_vmbus_handler(void (*handler)(void)) >> +void (*vmbus_handler)(void); >> +EXPORT_SYMBOL_GPL(vmbus_handler); >> + >> +void hv_setup_vmbus_handler(void (*handler)(void)) >> { >> + vmbus_handler = handler; >> } >> EXPORT_SYMBOL_GPL(hv_setup_vmbus_handler); >> >> -void __weak hv_remove_vmbus_handler(void) >> +void hv_remove_vmbus_handler(void) >> { >> + vmbus_handler = NULL; >> } >> EXPORT_SYMBOL_GPL(hv_remove_vmbus_handler); > > I'd suggest moving hv_setup_vmbus_handler() and > hv_remove_vmbus_handler() above or below the group > of __weak stubs in this source code file. There's a comment > describing the purpose of these __weak functions, and > intermixing these two functions that are no longer __weak > produces something of a jumble. > Acked. >> >> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c >> index bc4fc1951ae1..052ca8b11cee 100644 >> --- a/drivers/hv/vmbus_drv.c >> +++ b/drivers/hv/vmbus_drv.c >> @@ -1415,7 +1415,8 @@ EXPORT_SYMBOL_FOR_MODULES(vmbus_isr, "mshv_vtl"); >> >> static irqreturn_t vmbus_percpu_isr(int irq, void *dev_id) >> { >> - vmbus_isr(); >> + if (vmbus_handler) >> + vmbus_handler(); > > Is it necessary to test vmbus_handler first? From what I can > see, it is always set before the per-cpu interrupt is setup. After the shuffle of hv_remove_vmbus_handler() and freeing the irq, it can be safely removed. When I was setting the vmbus_handler to NULL first, before freeing the IRQ, this was required. > >> return IRQ_HANDLED; >> } >> >> @@ -1517,8 +1518,10 @@ static int vmbus_bus_init(void) >> vmbus_irq_initialized = true; >> } >> >> + hv_setup_vmbus_handler(vmbus_isr); >> + >> if (vmbus_irq == -1) { >> - hv_setup_vmbus_handler(vmbus_isr); >> + /* x86: sysvec handler uses vmbus_handler directly */ >> } else { >> ret = request_percpu_irq(vmbus_irq, vmbus_percpu_isr, >> "Hyper-V VMbus", &vmbus_evt); >> @@ -1553,9 +1556,8 @@ static int vmbus_bus_init(void) >> return 0; >> >> err_connect: >> - if (vmbus_irq == -1) >> - hv_remove_vmbus_handler(); >> - else >> + hv_remove_vmbus_handler(); >> + if (vmbus_irq != -1) >> free_percpu_irq(vmbus_irq, &vmbus_evt); > > These operations should be reordered so they are the inverse > of how they are setup. I.e., free_percpu_irq() first, then remove > the VMBus handler. That's just good standard practice unless > there's a specific reason to do the cleanup ordering differently. In > fact, hv_remove_vmbus_handler() needs to be moved down > to the err_setup label so it's done if request_percpu_irq() > fails. Acked. I will do the same for other hv_remove_vmbus_handler() as well. > >> err_setup: >> if (IS_ENABLED(CONFIG_PREEMPT_RT) && vmbus_irq_initialized) { >> @@ -3026,9 +3028,8 @@ static void __exit vmbus_exit(void) >> vmbus_connection.conn_state = DISCONNECTED; >> hv_stimer_global_cleanup(); >> vmbus_disconnect(); >> - if (vmbus_irq == -1) >> - hv_remove_vmbus_handler(); >> - else >> + hv_remove_vmbus_handler(); >> + if (vmbus_irq != -1) >> free_percpu_irq(vmbus_irq, &vmbus_evt); > > Ordering should be changed here as well so it is the inverse > of how things are set up. > >> if (IS_ENABLED(CONFIG_PREEMPT_RT) && vmbus_irq_initialized) { >> smpboot_unregister_percpu_thread(&vmbus_irq_threads); >> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h >> index 2810aa05dc73..db183c8cfb95 100644 >> --- a/include/asm-generic/mshyperv.h >> +++ b/include/asm-generic/mshyperv.h >> @@ -179,6 +179,7 @@ static inline u64 hv_generate_guest_id(u64 kernel_version) >> >> int hv_get_hypervisor_version(union hv_hypervisor_version_info *info); >> >> +extern void (*vmbus_handler)(void); >> void hv_setup_vmbus_handler(void (*handler)(void)); >> void hv_remove_vmbus_handler(void); >> void hv_setup_stimer0_handler(void (*handler)(void)); >> -- >> 2.43.0 >> Regards, Naman _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv