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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6DBB5C52D77 for ; Fri, 20 Jan 2023 23:28:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DDBB76B0078; Fri, 20 Jan 2023 18:28:57 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D8BCE6B007B; Fri, 20 Jan 2023 18:28:57 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C2C1F6B007D; Fri, 20 Jan 2023 18:28:57 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id B076D6B0078 for ; Fri, 20 Jan 2023 18:28:57 -0500 (EST) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id E280E140A95 for ; Fri, 20 Jan 2023 23:28:56 +0000 (UTC) X-FDA: 80376769872.16.A6D69A3 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf02.hostedemail.com (Postfix) with ESMTP id 12D0780007 for ; Fri, 20 Jan 2023 23:28:54 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=i8fMoEgf; spf=pass (imf02.hostedemail.com: domain of jarkko@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=jarkko@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1674257335; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Uap0K1kcpoj0gDyVcowfqqRnhWGkRUFqe4+AEboSEWY=; b=qdHqHQXL+sderIdwp3ZkwfVdx17CPhx3n0ienHEyWOsnfvanZWV9XGFtfNKPtEiDcsF1rE CVKpa/LAZMfDA1HC3QN1WE39hnmBAOz+6MyRiHzt6FxUb6yHpCU/HvY6QxfRXYW3GgRrZl Kq2EnC9fxffpK02ANU726Y//JJvF2Pk= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=i8fMoEgf; spf=pass (imf02.hostedemail.com: domain of jarkko@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=jarkko@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1674257335; a=rsa-sha256; cv=none; b=ML5dFmgoFmUr/GhTTuKQS6iBkNXQJjObBVNEEjhP58amIarQlYWmCse+81e+LhaeeQTtCV v8Pk8nNUiEXU7tj7E7ehXOhFe9om8gGO2o+Sv4njNX5Kr69JW/Z4nguTVyxH/qjCsfpgNC MLus1Qe0L3NuYMDhEoh70mUsTOrgxvA= Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id BD6DF620DF; Fri, 20 Jan 2023 23:28:53 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id A2A1FC433EF; Fri, 20 Jan 2023 23:28:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1674257333; bh=r2edRcFMISYKtEmPEu1cLKZR7c8iF9zsqqnoWv9hU3I=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=i8fMoEgfPYzBd+WPDibH/BLuqUwipoWJwcOyYpJ/Y0H3Rdml9thP589lRa96A/WAP uJE5kWD2JscX/Sdjpw0CyzXtfG/LxhSegqHv1fuSmC0iMrjiQf/55jY7QZ7uEHsAoF E8dMpHJh4CZNfldG+E9O6D3tHhc/7iue722H30CAwBzz6fKqrWs1mObgaZM6G6vx0B 6CxilOppA+Ky31fjHsYwZe3m5vUlfrsP8m1+9pPun5WaqAcNvIxLws8pDATrUwg9mj ui7nzJ6cNZIPoyAp1NyeZPWJcbL3gGUtyqacFP7P3DAAb/PThvmhPJwGbNBhCwd+HW B5OlWz8gR2uWg== Date: Fri, 20 Jan 2023 23:28:50 +0000 From: Jarkko Sakkinen To: Sean Christopherson Cc: Chao Peng , kvm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-arch@vger.kernel.org, linux-api@vger.kernel.org, linux-doc@vger.kernel.org, qemu-devel@nongnu.org, Paolo Bonzini , Jonathan Corbet , Vitaly Kuznetsov , Wanpeng Li , Jim Mattson , Joerg Roedel , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Arnd Bergmann , Naoya Horiguchi , Miaohe Lin , x86@kernel.org, "H . Peter Anvin" , Hugh Dickins , Jeff Layton , "J . Bruce Fields" , Andrew Morton , Shuah Khan , Mike Rapoport , Steven Price , "Maciej S . Szmigiero" , Vlastimil Babka , Vishal Annapurve , Yu Zhang , "Kirill A . Shutemov" , luto@kernel.org, jun.nakajima@intel.com, dave.hansen@intel.com, ak@linux.intel.com, david@redhat.com, aarcange@redhat.com, ddutile@redhat.com, dhildenb@redhat.com, Quentin Perret , tabba@google.com, Michael Roth , mhocko@suse.com, wei.w.wang@intel.com Subject: Re: [PATCH v10 3/9] KVM: Extend the memslot to support fd-based private memory Message-ID: References: <20221202061347.1070246-1-chao.p.peng@linux.intel.com> <20221202061347.1070246-4-chao.p.peng@linux.intel.com> <20230106094000.GA2297836@chaop.bj.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 12D0780007 X-Stat-Signature: 5431mbk9nfumrp6duid8fy7sswyjpfd7 X-HE-Tag: 1674257334-759233 X-HE-Meta: U2FsdGVkX1/3t2aYwPi3L0yXw/B5uTfIo7iIE+bY4PUWVMId10QUaynZmuO+xvQAE1IkAIOL+MDkRbzGS4Kz64Bfz3mSeVKUrj1CIGNLsHFltFGFDI3NsfqrkLhqcNNiAv1ih9jGWrXO70qRjHvMF3/W/954y0blC51HxX3UFc0Fcc54xJg8t8/rhj/DkHrdY26/RxOySS/FYDNzL3cA/C66HpxM7GZRfablLSApdRk6yU8qkw9iV7EehTQssCeZXnDKWcYtxlNgGUjcAdP4+oebY21vUStRfe/3udbcHMB82iWZllV323lYTE/ofTcn7wIqD3ee4VowrvCxhDZi2BlCiQukzLoaUH1r/sfbA6Fox5gtqlEnR5isjCPQsMG+1fKknPQzoAH9h1QBIgJO48RgDH2rAzWEHQkPzzzlqOYIb05uCmy9GymAJ0KwsyBgq+r2N28VBN9ImzpEPUEhxVRLo3XGCNvxhtY5png1v/QQu2IIZZFQG9Ote92Sttd0kHCgBaTgF/wriVvETLhieKxC/p4a8JHxCGEV7K6IO/Ffpz9foWQGvPWwxghA9AkvqVaGfA4GUZ/vcHEeiX0z8z5ga0ZUefCwVp0J4oHiEjdGim91YeovG9pqhfzthm99ypdKdMgl9FcbSAVfwVVtyZDRuR5I7BV5p1CLmFrz5ogg/nK2Dd152D4Yhuk/7J9751WsSikBL39exbiwUMtx+EHV/9Veibi2r+GvXr7axl9HrtomqLWPhQgALYvj44A0nN6qxPQWG1ob+t1O2dIVuuGgzQy1a9qAQbrXl5RK87tp0j9wRdFaBD4P01oURwrElgIO/CF1iOXgESEb+ILyjAFTpOzZrgvKNRVeRwW6muEJBQjDo9ajHiKsOfr3GskgkWvgvCl16KoF8RHXt88p6IQi/ZPxum4NyDZNiRaBQKFgq175E71fTDmT0lOUm6accV4b+e6pkscQtybYoRg y7qA7s0+ ShanceGj1NUDXCiLW6i3+Fg939HGXZCgX9Bp1J0sQgtjEr+UxlYVJu85Eijw/MG2GaY/wv5kBdJxO1lkg2hLI7+fYGvseqhKrnMZEIDdKpGUMKXu/gu66pgamx9jhStWQ9Th9JVJjjhVKX7Ttp20+N5d8OxgN4TKN4x33D1zofKENggL79S50YI7SF/+kdbXygTCbdeUI74RT5WdPNZ2GGq6Gb6DHJyCOU1I9ETlYnXkVhRV7EW6wixa/OJ1FCEhradY0rKgINE4eh+bvSgyoRaItbvNRkjb8izgi3EfqaZ3lfGFdCm+kReFxFHf7iPrnd9TvMuV69dsNuarhJXuWwjt6fPofYVmE+TMI/cLbWGmCUa8qDUQZfvm3VKO2ClIk4Sc2 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Jan 09, 2023 at 07:32:05PM +0000, Sean Christopherson wrote: > On Fri, Jan 06, 2023, Chao Peng wrote: > > On Thu, Jan 05, 2023 at 11:23:01AM +0000, Jarkko Sakkinen wrote: > > > On Fri, Dec 02, 2022 at 02:13:41PM +0800, Chao Peng wrote: > > > > To make future maintenance easy, internally use a binary compatible > > > > alias struct kvm_user_mem_region to handle both the normal and the > > > > '_ext' variants. > > > > > > Feels bit hacky IMHO, and more like a completely new feature than > > > an extension. > > > > > > Why not just add a new ioctl? The commit message does not address > > > the most essential design here. > > > > Yes, people can always choose to add a new ioctl for this kind of change > > and the balance point here is we want to also avoid 'too many ioctls' if > > the functionalities are similar. The '_ext' variant reuses all the > > existing fields in the 'normal' variant and most importantly KVM > > internally can reuse most of the code. I certainly can add some words in > > the commit message to explain this design choice. > > After seeing the userspace side of this, I agree with Jarkko; overloading > KVM_SET_USER_MEMORY_REGION is a hack. E.g. the size validation ends up being > bogus, and userspace ends up abusing unions or implementing kvm_user_mem_region > itself. > > It feels absolutely ridiculous, but I think the best option is to do: > > #define KVM_SET_USER_MEMORY_REGION2 _IOW(KVMIO, 0x49, \ > struct kvm_userspace_memory_region2) > > /* for KVM_SET_USER_MEMORY_REGION2 */ > struct kvm_user_mem_region2 { > __u32 slot; > __u32 flags; > __u64 guest_phys_addr; > __u64 memory_size; > __u64 userspace_addr; > __u64 restricted_offset; > __u32 restricted_fd; > __u32 pad1; > __u64 pad2[14]; > } > > And it's consistent with other KVM ioctls(), e.g. KVM_SET_CPUID2. > > Regarding the userspace side of things, please include Vishal's selftests in v11, > it's impossible to properly review the uAPI changes without seeing the userspace > side of things. I'm in the process of reviewing Vishal's v2[*], I'll try to > massage it into a set of patches that you can incorporate into your series. > > [*] https://lore.kernel.org/all/20221205232341.4131240-1-vannapurve@google.com +1 BR, Jarkko