All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Maxim Levitsky <mlevitsk@redhat.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Wanpeng Li <wanpengli@tencent.com>,
	Borislav Petkov <bp@alien8.de>, Joerg Roedel <joro@8bytes.org>,
	Jim Mattson <jmattson@google.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
	<x86@kernel.org>, Thomas Gleixner <tglx@linutronix.de>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Ingo Molnar <mingo@redhat.com>
Subject: Re: [PATCH 4/7] KVM: nVMX: move inject_page_fault tweak to .complete_mmu_init
Date: Wed, 17 Feb 2021 09:57:55 -0800	[thread overview]
Message-ID: <YC1ZI6DW49u0UP7m@google.com> (raw)
In-Reply-To: <5a8bea9b-deb1-673a-3dc8-f08b679de4c5@redhat.com>

On Wed, Feb 17, 2021, Paolo Bonzini wrote:
> On 17/02/21 18:29, Sean Christopherson wrote:
> > All that being said, I'm pretty we can eliminate setting
> > inject_page_fault dynamically. I think that would yield more
> > maintainable code. Following these flows is a nightmare. The change
> > itself will be scarier, but I'm pretty sure the end result will be a lot
> > cleaner.
> 
> I had a similar reaction, though my proposal was different.
> 
> The only thing we're changing in complete_mmu_init is the page fault
> callback for init_kvm_softmmu, so couldn't that be the callback directly
> (i.e. something like context->inject_page_fault =
> kvm_x86_ops.inject_softmmu_page_fault)?  And then adding is_guest_mode to
> the conditional that is already in vmx_inject_page_fault_nested and
> svm_inject_page_fault_nested.

Heh, that exact code crossed my mind as well.

> That said, I'm also rusty on _why_ this code is needed.  Why isn't it enough
> to inject the exception normally, and let nested_vmx_check_exception decide
> whether to inject a vmexit to L1 or an exception into L2?

Hmm, I suspect it was required at one point due to deficiencies elsewhere.
Handling this in the common fault handler logic does seem like the right
approach.

> Also, bonus question which should have been in the 5/7 changelog: are there
> kvm-unit-tests testcases that fail with npt=0, and if not could we write
> one?  [Answer: the mode_switch testcase fails, but I haven't checked why].
> 
> 
> Paolo
> 

  reply	other threads:[~2021-02-17 17:58 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-17 14:57 [PATCH 0/7] KVM: random nested fixes Maxim Levitsky
2021-02-17 14:57 ` [PATCH 1/7] KVM: VMX: read idt_vectoring_info a bit earlier Maxim Levitsky
2021-02-17 16:06   ` Paolo Bonzini
2021-02-17 16:18     ` Maxim Levitsky
2021-02-17 16:21       ` Sean Christopherson
2021-02-17 16:29         ` Paolo Bonzini
2021-02-17 14:57 ` [PATCH 2/7] KVM: nSVM: move nested vmrun tracepoint to enter_svm_guest_mode Maxim Levitsky
2021-02-17 14:57 ` [PATCH 3/7] KVM: x86: add .complete_mmu_init arch callback Maxim Levitsky
2021-02-17 14:57 ` [PATCH 4/7] KVM: nVMX: move inject_page_fault tweak to .complete_mmu_init Maxim Levitsky
2021-02-17 17:29   ` Sean Christopherson
2021-02-17 17:37     ` Paolo Bonzini
2021-02-17 17:57       ` Sean Christopherson [this message]
2021-02-17 18:00         ` Paolo Bonzini
2021-02-17 18:49       ` Maxim Levitsky
2021-02-17 18:43     ` Maxim Levitsky
2021-02-18  9:45       ` Paolo Bonzini
2021-02-17 14:57 ` [PATCH 5/7] KVM: nSVM: fix running nested guests when npt=0 Maxim Levitsky
2021-02-17 15:27   ` Maxim Levitsky
2021-02-17 14:57 ` [PATCH 6/7] KVM: nVMX: don't load PDPTRS right after nested state set Maxim Levitsky
2021-02-17 17:52   ` Sean Christopherson
2021-02-17 18:06     ` Paolo Bonzini
2021-02-17 14:57 ` [PATCH 7/7] KVM: nSVM: call nested_svm_load_cr3 on nested state load Maxim Levitsky

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=YC1ZI6DW49u0UP7m@google.com \
    --to=seanjc@google.com \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=mlevitsk@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    --cc=x86@kernel.org \
    /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.