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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3C958C46467 for ; Mon, 16 Jan 2023 16:20:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232876AbjAPQUj (ORCPT ); Mon, 16 Jan 2023 11:20:39 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38214 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232818AbjAPQUJ (ORCPT ); Mon, 16 Jan 2023 11:20:09 -0500 Received: from outgoing2021.csail.mit.edu (outgoing2021.csail.mit.edu [128.30.2.78]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CA3332FCCC for ; Mon, 16 Jan 2023 08:10:47 -0800 (PST) Received: from c-24-17-218-140.hsd1.wa.comcast.net ([24.17.218.140] helo=srivatsab3MD6R.vmware.com) by outgoing2021.csail.mit.edu with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.95) (envelope-from ) id 1pHS49-00Ec49-Pd; Mon, 16 Jan 2023 11:10:33 -0500 Subject: Re: [PATCH] x86/paravirt: merge activate_mm and dup_mmap callbacks To: Juergen Gross , linux-kernel@vger.kernel.org, x86@kernel.org, virtualization@lists.linux-foundation.org Cc: Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , "H. Peter Anvin" , Alexey Makhalov , VMware PV-Drivers Reviewers , Boris Ostrovsky , xen-devel@lists.xenproject.org References: <20230112152132.4399-1-jgross@suse.com> <3fcb5078-852e-0886-c084-7fb0cfa5b757@csail.mit.edu> <27d08d32-1a17-0959-203f-39e769f555d1@suse.com> From: "Srivatsa S. Bhat" Message-ID: <6a8e25eb-758d-8ad6-23c1-5fea7dab3b09@csail.mit.edu> Date: Mon, 16 Jan 2023 08:10:30 -0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: <27d08d32-1a17-0959-203f-39e769f555d1@suse.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 1/15/23 10:43 PM, Juergen Gross wrote: > On 16.01.23 05:27, Srivatsa S. Bhat wrote: >> >> Hi Juergen, >> >> On 1/12/23 7:21 AM, Juergen Gross wrote: >>> The two paravirt callbacks .mmu.activate_mm and .mmu.dup_mmap are >>> sharing the same implementations in all cases: for Xen PV guests they >>> are pinning the PGD of the new mm_struct, and for all other cases >>> they are a NOP. >>> >> >> I was expecting that the duplicated functions xen_activate_mm() and >> xen_dup_mmap() would be merged into a common one, and that both >> .mmu.activate_mm and .mmu.dup_mmap callbacks would be mapped to that >> common implementation for Xen PV. >> >>> So merge them to a common callback .mmu.enter_mmap (in contrast to the >>> corresponding already existing .mmu.exit_mmap). >>> >> >> Instead, this patch seems to be merging the callbacks themselves... >> >> I see that's not an issue right now since there is no other actual >> user for these callbacks. But are we sure that merging the callbacks >> just because the current user (Xen PV) has the same implementation for >> both is a good idea? The callbacks are invoked at distinct points from >> fork/exec, so wouldn't it be valuable to retain that distinction in >> semantics in the callbacks as well? >> >> However, if you believe that two separate callbacks are not really >> required here (because there is no significant difference in what they >> mean, rather than because their callback implementations happen to be >> the same right now), then could you please expand on this and call it >> out in the commit message, please? > > Would you be fine with: > >   In the end both callbacks are meant to register an address space with the >   underlying hypervisor, so there needs to be only a single callback for that >   purpose. > Sure, that looks good. Thank you! Regards, Srivatsa VMware Photon OS