All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathias Krause <minipli@grsecurity.net>
To: Robert Hoo <robert.hoo.linux@gmail.com>, kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org,
	Sean Christopherson <seanjc@google.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH v4 2/6] KVM: x86: Do not unload MMU roots when only toggling CR0.WP with TDP enabled
Date: Mon, 8 May 2023 11:30:59 +0200	[thread overview]
Message-ID: <b0d4b430-afd2-979c-e7d2-b53e131412ad@grsecurity.net> (raw)
In-Reply-To: <e70af22a-f09f-aadb-8353-35b29d2def61@gmail.com>

On 07.05.23 09:32, Robert Hoo wrote:
> On 3/22/2023 9:37 AM, Mathias Krause wrote:
>> There is no need to unload the MMU roots with TDP enabled when only
>> CR0.WP has changed -- the paging structures are still valid, only the
>> permission bitmap needs to be updated.
>>
>> One heavy user of toggling CR0.WP is grsecurity's KERNEXEC feature to
>> implement kernel W^X.
>>
>> The optimization brings a huge performance gain for this case as the
>> following micro-benchmark running 'ssdd 10 50000' from rt-tests[1] on a
>> grsecurity L1 VM shows (runtime in seconds, lower is better):
>>
>>                         legacy     TDP    shadow
>> kvm-x86/next@d8708b     8.43s    9.45s    70.3s
>>               +patch     5.39s    5.63s    70.2s
>>
>> For legacy MMU this is ~36% faster, for TTP MMU even ~40% faster. 
> 
> TTP --> TDP

Thanks, Sean fixed it up in the final commit:
https://git.kernel.org/linus/01b31714bd90

> 
>>   void kvm_post_set_cr0(struct kvm_vcpu *vcpu, unsigned long old_cr0,
>> unsigned long cr0)
>>   {
>> +    /*
>> +     * CR0.WP is incorporated into the MMU role, but only for
>> non-nested,
>> +     * indirect shadow MMUs.  If TDP is enabled, the MMU's metadata
>> needs
>> +     * to be updated, e.g. so that emulating guest translations does the
>> +     * right thing, but there's no need to unload the root as CR0.WP
>> +     * doesn't affect SPTEs.
>> +     */
>> +    if (tdp_enabled && (cr0 ^ old_cr0) == X86_CR0_WP) {
> 
> Curiously, this patch only affects tdp_enabled, why does legacy MMU also
> see comparable performance gains?

Because 'tdp_enabled' just implies EPT / NPT and only 'tdp_mmu_enabled'
decides which MMU mode to use -- either legacy or TDP MMU (see
kvm_configure_mmu() and now gets invoked from vmx.c / svm.c).

Thanks,
Mathias

  reply	other threads:[~2023-05-08  9:31 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-22  1:37 [PATCH v4 0/6] KVM: MMU: performance tweaks for heavy CR0.WP users Mathias Krause
2023-03-22  1:37 ` [PATCH v4 1/6] KVM: x86/mmu: Avoid indirect call for get_cr3 Mathias Krause
2023-03-22  1:37 ` [PATCH v4 2/6] KVM: x86: Do not unload MMU roots when only toggling CR0.WP with TDP enabled Mathias Krause
2023-05-07  7:32   ` Robert Hoo
2023-05-08  9:30     ` Mathias Krause [this message]
2023-05-09  1:04       ` Robert Hoo
2023-03-22  1:37 ` [PATCH v4 3/6] KVM: x86: Ignore CR0.WP toggles in non-paging mode Mathias Krause
2023-03-22  1:37 ` [PATCH v4 4/6] KVM: x86: Make use of kvm_read_cr*_bits() when testing bits Mathias Krause
2023-03-22  1:37 ` [PATCH v4 5/6] KVM: x86/mmu: Fix comment typo Mathias Krause
2023-03-22  1:37 ` [PATCH v4 6/6] KVM: VMX: Make CR0.WP a guest owned bit Mathias Krause
2023-03-27  8:33   ` Xiaoyao Li
2023-03-27  8:37     ` Mathias Krause
2023-03-27 13:48       ` Xiaoyao Li
2023-03-30  8:45   ` Mathias Krause
2023-03-30 17:12     ` Sean Christopherson
2023-03-30 20:15       ` Mathias Krause
2023-03-30 20:30         ` Mathias Krause
2023-03-30 20:36           ` Sean Christopherson
2023-03-30 20:33       ` Sean Christopherson
2023-03-30 20:55         ` Mathias Krause
2023-03-31 14:18           ` Mathias Krause
2023-03-22  7:41 ` [PATCH v4 0/6] KVM: MMU: performance tweaks for heavy CR0.WP users Mathias Krause
2023-03-23 22:50 ` Sean Christopherson
2023-03-25 11:39   ` Mathias Krause
2023-03-25 12:25     ` Greg KH
2023-04-06  2:25       ` Sean Christopherson
2023-04-06 13:22         ` Mathias Krause
2023-04-14  9:29           ` Mathias Krause
2023-04-14 16:49             ` Sean Christopherson
2023-04-14 20:09               ` Jeremi Piotrowski
2023-04-14 20:17                 ` Sean Christopherson
2023-05-02 17:38                   ` Jeremi Piotrowski
2023-05-08  9:19               ` Mathias Krause
2023-05-08 15:57                 ` Mathias Krause

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b0d4b430-afd2-979c-e7d2-b53e131412ad@grsecurity.net \
    --to=minipli@grsecurity.net \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=robert.hoo.linux@gmail.com \
    --cc=seanjc@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.