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 X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 397EACA90AF for ; Tue, 12 May 2020 17:50:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 07434206A5 for ; Tue, 12 May 2020 17:50:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728262AbgELRuU (ORCPT ); Tue, 12 May 2020 13:50:20 -0400 Received: from mga05.intel.com ([192.55.52.43]:45961 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726300AbgELRuU (ORCPT ); Tue, 12 May 2020 13:50:20 -0400 IronPort-SDR: 9bS7boKHOc+lFdRIYgVQl3EgKF3NdTblUDA97kdjUoqMMOqsvMbALB3X2AFpfdFDr0NeDIy5wS d9inPAu/YNYA== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 May 2020 10:50:18 -0700 IronPort-SDR: tEtccoh4JXKIVce1OuW2ZgJkYA+zs0VHCZR1tN1NZpXRTMNTggBxK2k9kAMX+RoeIrSKL/j2iR AaSuZDHX/mNg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,384,1583222400"; d="scan'208";a="265584793" Received: from sjchrist-coffee.jf.intel.com (HELO linux.intel.com) ([10.54.74.152]) by orsmga006.jf.intel.com with ESMTP; 12 May 2020 10:50:18 -0700 Date: Tue, 12 May 2020 10:50:17 -0700 From: Sean Christopherson To: Vivek Goyal Cc: Vitaly Kuznetsov , kvm@vger.kernel.org, x86@kernel.org, Paolo Bonzini , Andy Lutomirski , Thomas Gleixner , Borislav Petkov , "H. Peter Anvin" , Wanpeng Li , Jim Mattson , Gavin Shan , Peter Zijlstra , linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/8] KVM: x86: extend struct kvm_vcpu_pv_apf_data with token info Message-ID: <20200512175017.GC12100@linux.intel.com> References: <20200511164752.2158645-1-vkuznets@redhat.com> <20200511164752.2158645-3-vkuznets@redhat.com> <20200512152709.GB138129@redhat.com> <87o8qtmaat.fsf@vitty.brq.redhat.com> <20200512155339.GD138129@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200512155339.GD138129@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 12, 2020 at 11:53:39AM -0400, Vivek Goyal wrote: > On Tue, May 12, 2020 at 05:40:10PM +0200, Vitaly Kuznetsov wrote: > > Vivek Goyal writes: > > > > > On Mon, May 11, 2020 at 06:47:46PM +0200, Vitaly Kuznetsov wrote: > > >> Currently, APF mechanism relies on the #PF abuse where the token is being > > >> passed through CR2. If we switch to using interrupts to deliver page-ready > > >> notifications we need a different way to pass the data. Extent the existing > > >> 'struct kvm_vcpu_pv_apf_data' with token information for page-ready > > >> notifications. > > >> > > >> The newly introduced apf_put_user_ready() temporary puts both reason > > >> and token information, this will be changed to put token only when we > > >> switch to interrupt based notifications. > > >> > > >> Signed-off-by: Vitaly Kuznetsov > > >> --- > > >> arch/x86/include/uapi/asm/kvm_para.h | 3 ++- > > >> arch/x86/kvm/x86.c | 17 +++++++++++++---- > > >> 2 files changed, 15 insertions(+), 5 deletions(-) > > >> > > >> diff --git a/arch/x86/include/uapi/asm/kvm_para.h b/arch/x86/include/uapi/asm/kvm_para.h > > >> index 2a8e0b6b9805..e3602a1de136 100644 > > >> --- a/arch/x86/include/uapi/asm/kvm_para.h > > >> +++ b/arch/x86/include/uapi/asm/kvm_para.h > > >> @@ -113,7 +113,8 @@ struct kvm_mmu_op_release_pt { > > >> > > >> struct kvm_vcpu_pv_apf_data { > > >> __u32 reason; > > >> - __u8 pad[60]; > > >> + __u32 pageready_token; > > > > > > How about naming this just "token". That will allow me to deliver error > > > as well. pageready_token name seems to imply that this will always be > > > successful with page being ready. > > > > > > And reason will tell whether page could successfully be ready or > > > it was an error. And token will help us identify the task which > > > is waiting for the event. > > > > I added 'pageready_' prefix to make it clear this is not used for 'page > > not present' notifications where we pass token through CR2. (BTW > > 'reason' also becomes a misnomer because we can only see > > 'KVM_PV_REASON_PAGE_NOT_PRESENT' there.) > > Sure. I am just trying to keep names in such a way so that we could > deliver more events and not keep it too tightly coupled with only > two events (page not present, page ready). > > > > > I have no strong opinion, can definitely rename this to 'token' and add > > a line to the documentation to re-state that this is not used for type 1 > > events. > > I don't even know why are we calling "type 1" and "type 2" event. Calling > it KVM_PV_REASON_PAGE_NOT_PRESENT and KVM_PV_REASON_PAGE_READY event > is much more intuitive. If somebody is confused about how event will > be delivered, that could be part of documentation. And "type1" and "type2" > does not say anything about delivery method anyway. > > Also, type of event should not necessarily be tied to delivery method. > For example if we end up introducing say, "KVM_PV_REASON_PAGE_ERROR", then > I would think that event can be injected both using exception (#PF or #VE) > as well as interrupt (depending on state of system). Why bother preserving backwards compatibility? AIUI, both KVM and guest will support async #PF iff interrupt delivery is enabled. Why not make the interrupt delivery approach KVM_ASYNC_PF_V2 and completely redefine the ABI? E.g. to make it compatible with reflecting !PRESENT faults without a VM-Exit via Intel's EPT Violation #VE?