linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Vivek Goyal <vgoyal@redhat.com>
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	virtio-fs-list <virtio-fs@redhat.com>,
	vkuznets@redhat.com, pbonzini@redhat.com
Subject: Re: [PATCH v4] kvm,x86: Exit to user space in case page fault error
Date: Fri, 2 Oct 2020 11:30:37 -0700	[thread overview]
Message-ID: <20201002183036.GB24460@linux.intel.com> (raw)
In-Reply-To: <20201002153854.GC3119@redhat.com>

On Fri, Oct 02, 2020 at 11:38:54AM -0400, Vivek Goyal wrote:
> On Thu, Oct 01, 2020 at 03:33:20PM -0700, Sean Christopherson wrote:
> > Alternatively, what about adding a new KVM request type to handle this?
> > E.g. when the APF comes back with -EFAULT, snapshot the GFN and make a
> > request.  The vCPU then gets kicked and exits to userspace.  Before exiting
> > to userspace, the request handler resets vcpu->arch.apf.error_gfn.  Bad GFNs
> > simply get if error_gfn is "valid", i.e. there's a pending request.
> 
> Sorry, I did not understand the above proposal. Can you please elaborate
> a bit more. Part of it is that I don't know much about KVM requests.
> Looking at the code it looks like that main loop is parsing if some
> kvm request is pending and executing that action.
> 
> Don't we want to make sure that we exit to user space when guest retries
> error gfn access again.

> In this case once we get -EFAULT, we will still inject page_ready into
> guest. And then either same process or a different process might run. 
> 
> So when exactly code raises a kvm request. If I raise it right when
> I get -EFAULT, then kvm will exit to user space upon next entry
> time. But there is no guarantee guest vcpu is running the process which
> actually accessed the error gfn. And that probably means that register
> state of cpu does not mean much and one can not easily figure out
> which task tried to access the bad memory and when.
> 
> That's why we prepare a list of error gfn and only exit to user space
> when error_gfn access is retried so that guest vcpu context is correct.
> 
> What am I missing?

I don't think it's necessary to provide userspace with the register state of
the guest task that hit the bad page.  Other than debugging, I don't see how
userspace can do anything useful which such information.

Even if you want to inject an event of some form into the guest, having the
correct context for the event itself is not required.  IMO it's perfectly
reasonable for such an event to be asynchronous.

IIUC, your end goal is to be able to gracefully handle DAX file truncation.
Simply killing the guest task that hit the bad page isn't sufficient, as
nothing prevents a future task from accessing the same bad page.  To fully
handle the situation, the guest needs to remove the bad page from its memory
pool.  Once the page is offlined, the guest kernel's error handling will
kick in when a task accesses the bad page (or nothing ever touches the bad
page again and everyone is happy).

Note, I'm not necessarily suggesting that QEMU piggyback its #MC injection
to handle this, but I suspect the resulting behavior will look quite similar,
e.g. notify the virtiofs driver in the guest, which does some magic to take
the offending region offline, and then guest tasks get SIGBUS or whatever.

I also don't think it's KVM's responsibility to _directly_ handle such a
scenario.  As I said in an earlier version, KVM can't possibly know _why_ a
page fault came back with -EFAULT, only userspace can connect the dots of
GPA -> HVA -> vm_area_struct -> file -> inject event.  KVM definitely should
exit to userspace on the -EFAULT instead of hanging the guest, but that can
be done via a new request, as suggested.

  reply	other threads:[~2020-10-02 18:30 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-20 21:13 [PATCH v4] kvm,x86: Exit to user space in case page fault error Vivek Goyal
2020-07-27 13:56 ` Vivek Goyal
2020-07-27 16:09   ` Vitaly Kuznetsov
2020-07-27 18:40     ` Vivek Goyal
2020-07-30  5:01 ` Pankaj Gupta
2020-08-07 17:51 ` Vivek Goyal
2020-09-29  4:37 ` Sean Christopherson
2020-10-01 21:55   ` Vivek Goyal
2020-10-01 22:33     ` Sean Christopherson
2020-10-02 15:38       ` Vivek Goyal
2020-10-02 18:30         ` Sean Christopherson [this message]
2020-10-02 19:27           ` Vivek Goyal
2020-10-02 19:45             ` Sean Christopherson
2020-10-02 20:02               ` Vivek Goyal
2020-10-02 21:13                 ` Sean Christopherson
2020-10-05 15:33                   ` Vivek Goyal
2020-10-05 16:16                     ` Sean Christopherson
2020-10-06 13:46                       ` Vivek Goyal
2020-10-06 14:05                         ` Vitaly Kuznetsov
2020-10-06 14:15                           ` Vivek Goyal
2020-10-06 14:50                             ` Vitaly Kuznetsov
2020-10-06 15:08                               ` Vivek Goyal
2020-10-06 15:24                                 ` Vitaly Kuznetsov
2020-10-06 16:12                                   ` Sean Christopherson
2020-10-06 16:24                                     ` Vivek Goyal
2020-10-06 16:39                                     ` Vitaly Kuznetsov
2020-10-06 17:17                                       ` Sean Christopherson
2020-10-06 17:21                                         ` [Virtio-fs] [PATCH v4] kvm, x86: " Dr. David Alan Gilbert
2020-10-06 17:28                                           ` Vivek Goyal
2020-10-06 17:35                                         ` [PATCH v4] kvm,x86: " Vivek Goyal
2020-10-07  0:04                                           ` Sean Christopherson

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=20201002183036.GB24460@linux.intel.com \
    --to=sean.j.christopherson@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=vgoyal@redhat.com \
    --cc=virtio-fs@redhat.com \
    --cc=vkuznets@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).