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=-5.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no 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 F3DA3C2D0E4 for ; Tue, 17 Nov 2020 11:03:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9524F20773 for ; Tue, 17 Nov 2020 11:03:21 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="dtoD41TI" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728269AbgKQLDU (ORCPT ); Tue, 17 Nov 2020 06:03:20 -0500 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:37248 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727740AbgKQLDT (ORCPT ); Tue, 17 Nov 2020 06:03:19 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1605610997; h=from:from: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:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=h5lmX7LExkzM+QM3a2cocE9/WZPLYRxF3to83Xkns2o=; b=dtoD41TI1HKT2P8ADcaX/MlftH/jTBmq1Ghd+ctT+fg0ES4fefV+CL+Oj5HvIAZvrc03Zc 6wLvarHAhKpnPiO9chyCaxdTOPQv6cPSFvY71qIcHjBQbnBBnHC8xaINtBTQ89AdkWfyvj qWBPJoTFgWfVZSRRdq0mqezkY6z4MyU= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-545-_8LL4duEMRy0b54WlA5mQA-1; Tue, 17 Nov 2020 06:03:15 -0500 X-MC-Unique: _8LL4duEMRy0b54WlA5mQA-1 Received: by mail-wr1-f71.google.com with SMTP id u1so12924227wri.6 for ; Tue, 17 Nov 2020 03:03:15 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=h5lmX7LExkzM+QM3a2cocE9/WZPLYRxF3to83Xkns2o=; b=KT16YZ7xTNIXP+KV8d1ga9VDztD238hE3XGNbgMdO+GLx/cb22G4xGfsOYBXvqa6yP C7e+JO0Fs+OGdyXlsaXPTgjm7Y8PKVSJo08v0TTdnXRIFWAMDrYcBid88j7WstBd92FP tKYY0akw+o1esYNdxEm2bVg/lS1QUXHgWpDXBzHi96nSYDQTRNDiDbZPhjjn+GP7mnxb WcpywaCIbT/SZw4O37xO4Wlx7axvEe2N8aJaL0AMvXsHB1LHi8nvRVcmSJur/4zYRCJ/ rE2Rp+lCDeywADMiH16kPwQ6GmFHoyyGc52doZGIdx8ROuPqJvJK6pVVx7vBuLO5YjAb 6xmw== X-Gm-Message-State: AOAM533cDUIusDy8qBjW2ZlRDwlqf5mtuGydYUDG/vn3WxKwJJKWE45G ZO7Sal7Iw5wGKeSZGtolHB96MqyQXXCmhmIpAppU/+OmUXuKio6JhaW+Ej+vPbTO59xJjt5t/wV h11boGfkonTyh6uSf0E14uy3w X-Received: by 2002:adf:a2c2:: with SMTP id t2mr25295042wra.54.1605610994149; Tue, 17 Nov 2020 03:03:14 -0800 (PST) X-Google-Smtp-Source: ABdhPJxhzCHrbfSX6RQjbbtqNxHHoDd2RnJkMXxmQCe8E3wqAYWOCJAnCPyC2ZlHe6lR3BY1URbqVg== X-Received: by 2002:adf:a2c2:: with SMTP id t2mr25295004wra.54.1605610993917; Tue, 17 Nov 2020 03:03:13 -0800 (PST) Received: from ?IPv6:2001:b07:6468:f312:c8dd:75d4:99ab:290a? ([2001:b07:6468:f312:c8dd:75d4:99ab:290a]) by smtp.gmail.com with ESMTPSA id h128sm3216045wme.38.2020.11.17.03.03.12 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 17 Nov 2020 03:03:13 -0800 (PST) Subject: Re: [PATCH v2 0/2] KVM: SVM: Create separate vmcbs for L1 and L2 To: Cathy Avery , linux-kernel@vger.kernel.org, kvm@vger.kernel.org Cc: vkuznets@redhat.com, wei.huang2@amd.com, mlevitsk@redhat.com, Sean Christopherson References: <20201011184818.3609-1-cavery@redhat.com> From: Paolo Bonzini Message-ID: <1a450000-dd0c-6c38-191b-8ad869c21807@redhat.com> Date: Tue, 17 Nov 2020 12:03:12 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0 MIME-Version: 1.0 In-Reply-To: <20201011184818.3609-1-cavery@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Cathy, I found an issue with the second patch: the svm->asid_generation and svm->vcpu.cpu fields must become per-VMCB. Once again we are rediscovering what VMX already does (for different reasons) with its struct loaded_vmcs. The good thing is that it can be worked around easily: for the ASID generation, it simply be cleared after changing svm->vmcb. For the CPU field, it's not an issue yet because the VMCB is marked all-dirty after each switch. With these workarounds, everything works nicely. However, removing the calls to vmcb_mark_all_dirty is the main optimization that is enabled by the VMCB01/VMCB02 split, so it should be fixed too. And also, the code duplication every time svm->vmcb is assigned starts to be ugly, proving Sean to be right. :) My suggestion is that you do something like this: 1) create a struct for all per-VMCB data: struct kvm_vmcb_info { void *ptr; u64 pa; } and use it in structs vcpu_svm and svm_nested_state: struct vcpu_svm { ... struct kvm_vmcb_info vmcb01; struct kvm_vmcb_info *current_vmcb; void *vmcb; u64 vmcb_pa; ... } struct svm_nested_state { struct kvm_vmcb_info vmcb02; ... } The struct can be passed to a vmcb_switch function like this one: void vmcb_switch(struct vcpu_svm *svm, struct kvm_vmcb_info *target_vmcb) { svm->current_vmcb = target_vmcb; svm->vmcb = target_vmcb->ptr; svm->vmcb_pa = target_vmcb->pa; /* * Workaround: we don't yet track the ASID generation * that was active the last time target_vmcb was run. */ svm->asid_generation = 0; /* * Workaround: we don't yet track the physical CPU that * target_vmcb has run on. */ vmcb_mark_all_dirty(svm->vmcb); } You can use this function every time the current code is assigning to svm->vmcb. Once the struct and function is in place, we can proceed to removing the last two (inefficient) lines of vmcb_switch by augmenting struct kvm_vmcb_info. 2) First, add an "int cpu" field. Move the vcpu->cpu check from svm_vcpu_load to pre_svm_run, using svm->current_vmcb->cpu instead of vcpu->cpu, and you will be able to remove the vmcb_mark_all_dirty call from vmcb_switch. 3) Then do the same with asid_generation. All uses of svm->asid_generation become uses of svm->current_vmcb->asid_generation, and you can remove the clearing of svm->asid_generation. These can be three separate patches on top of the changes you have sent (or rather the rebased version, see below). Writing good commit messages for them will be a good exercise too. :) I have pushed the current nested SVM queue to kvm.git on a "nested-svm" branch. You can discard the top commits and work on top of commit a33b86f151a0 from that branch ("KVM: SVM: Use a separate vmcb for the nested L2 guest", 2020-11-17). Once this is done, we can start reaping the advantages of the VMCB01/VMCB02 split. Some patches for that are already in the nested-svm branch, but there's more fun to be had. First of all, Maxim's ill-fated attempt at using VMCB12 clean bits will now work. Second, we can try doing VMLOAD/VMSAVE always from VMCB01 (while VMRUN switches between VMCB01 and VMCB02) and therefore remove the nested_svm_vmloadsave calls from nested_vmrun and nested_vmexit. But, one thing at a time. Thanks, Paolo