kvm,x86: Exit to user space in case of page fault error
RFC PATCH v2 1/1 kvm,x86: Exit to user space in case of page fault error
Page fault error handling behavior in kvm seems little inconsistent when page fault reports error. If we are doing fault synchronously
then we capture error (-EFAULT) returned by __gfn_to_pfn_memslot() and exit to user space and qemu reports error, "error: kvm run failed Bad address".
But if we are doing async page fault, then async_pf_execute() will simply ignore the error reported by get_user_pages_remote() or
by kvm_mmu_do_page_fault(). It is assumed that page fault was successful and either a page ready event is injected in guest or guest is brought out of artificial halt state and run again. In both the cases when guest retries the instruction, it takes exit again as page fault was not successful in previous attempt. And then this infinite loop continues forever.
Trying fault in a loop will make sense if error is temporary and will be resolved on retry. But I don't see any intention in the code to determine if error is temporary or not. Whether to do fault synchronously or asynchronously, depends on so many variables but none of the varibales is whether error is temporary or not. (kvm_can_do_async_pf()).
And that makes it very inconsistent or unpredictable to figure out whether kvm will exit to qemu with error or it will just retry and go into an infinite loop.
This patch tries to make this behavior consistent. That is instead of getting into infinite loop of retrying page fault, exit to user space and stop VM if page fault error happens.
In future this can be improved by injecting errors into guest. As of now we don't have any race free method to inject errors in guest.
When page fault error happens in async path save that pfn and when next time guest retries, do a sync fault instead of async fault. So that if error is encountered, we exit to qemu and avoid infinite loop.
We maintain a cache of error gfns and force sync fault if a gfn is found in cache of error gfn. There is a small possibility that we miss an error gfn (as it got overwritten by a new error gfn). But its just a hint and sooner or later some error pfn will match and we will force sync fault and exit to user space.
Change from v1:
- Maintain a cache of error gfns, instead of single gfn. (Vitaly)
--- arch/x86/include/asm/kvm_host.h | 2 ++ arch/x86/kvm/mmu.h | 2 +- arch/x86/kvm/mmu/mmu.c | 2 +- arch/x86/kvm/x86.c | 61 +++++++++++++++++++++++++++++++-- 4 files changed, 62 insertions(+), 5 deletions(-)
arch/x86/include/asm/kvm_host.h +2 -0
@@ -137,6 +137,7 @@ static inline gfn_t gfn_to_index(gfn_t gfn, gfn_t base_gfn, int level)#define KVM_NR_VAR_MTRR 8#define ASYNC_PF_PER_VCPU 64 +#define ERROR_GFN_PER_VCPU 64enum kvm_reg {VCPU_REGS_RAX = __VCPU_REGS_RAX,
@@ -778,6 +779,7 @@ struct kvm_vcpu_arch {unsigned long nested_apf_token;bool delivery_as_pf_vmexit;bool pageready_pending; +gfn_t error_gfns[ERROR_GFN_PER_VCPU ]; } apf;/* OSVW MSRs (AMD only) */
arch/x86/kvm/mmu.h +1 -1
@@ -60,7 +60,7 @@ void kvm_init_mmu(struct kvm_vcpu *vcpu, bool reset_roots);void kvm_init_shadow_mmu (struct kvm_vcpu *vcpu,u32 cr0,u32 cr4,u32 efer);void kvm_init_shadow_ept_mmu (struct kvm_vcpu *vcpu,bool execonly,bool accessed_dirty,gpa_t new_eptp); -bool kvm_can_do_async_pf (struct kvm_vcpu *vcpu); +bool kvm_can_do_async_pf (struct kvm_vcpu *vcpu,gfn_t gfn);int kvm_handle_page_fault (struct kvm_vcpu *vcpu,u64 error_code,u64 fault_address,char *insn,int insn_len);
arch/x86/kvm/mmu/mmu.c +1 -1
@@ -4078,7 +4078,7 @@ static bool try_async_pf(struct kvm_vcpu *vcpu, bool prefault, gfn_t gfn,if (!async)return false;/* *pfn has correct page already */ -if (!prefault &&kvm_can_do_async_pf (vcpu)) { +if (!prefault &&kvm_can_do_async_pf (vcpu, cr2_or_gpa >>PAGE_SHIFT )) {trace_kvm_try_async_get_page (cr2_or_gpa, gfn);if (kvm_find_async_pf_gfn (vcpu, gfn)) {trace_kvm_async_pf_doublefault (cr2_or_gpa, gfn);
arch/x86/kvm/x86.c +58 -3
103 lines not on this page · raw message
Signature · 1 line
2.25.4
kernel test robot
build botRe: [RFC PATCH v2] kvm, x86: Exit to user space in case of page fault error
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch]
Hi Vivek,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on kvm/linux-next]
[also build test WARNING on linux/master linus/master v5.8-rc4 next-20200707] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Vivek-Goyal/kvm-x86-Exit-to-user-space-in-case-of-page-fault-error/20200708-041749 base: https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next config: x86_64-rhel-7.6 (attached as .config) compiler: gcc-9 (Debian 9.3.0-14) 9.3.0 reproduce (this is a W=1 build): # save the attached .config to linux build tree make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
All warnings (new ones prefixed by >>):
>> arch/x86/kvm/x86.c:10403:6: warning: no previous prototype for 'kvm_find_error_gfn' [-Wmissing-prototypes]
10403 | bool kvm_find_error_gfn(struct kvm_vcpu *vcpu, gfn_t gfn)
| ^~~~~~~~~~~~~~~~~~vim +/kvm_find_error_gfn +10403 arch/x86/kvm/x86.c
10402
10403 bool kvm_find_error_gfn(struct kvm_vcpu *vcpu, gfn_t gfn)
10404 {
10405 u32 key = kvm_error_gfn_hash_fn(gfn);
10406
10407 return vcpu->arch.apf.error_gfns[key] == gfn;
10408 }
10409
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
kernel test robot
build botRe: [RFC PATCH v2] kvm, x86: Exit to user space in case of page fault error
In reply to Vivek Goyal[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch]
Hi Vivek,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on kvm/linux-next]
[also build test WARNING on linux/master linus/master v5.8-rc4 next-20200707] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Vivek-Goyal/kvm-x86-Exit-to-user-space-in-case-of-page-fault-error/20200708-041749 base: https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next config: x86_64-allyesconfig (attached as .config) compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 02946de3802d3bc65bc9f2eb9b8d4969b5a7add8)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
All warnings (new ones prefixed by >>):
>> arch/x86/kvm/x86.c:10403:6: warning: no previous prototype for function 'kvm_find_error_gfn' [-Wmissing-prototypes]
bool kvm_find_error_gfn(struct kvm_vcpu *vcpu, gfn_t gfn)
^ arch/x86/kvm/x86.c:10403:1: note: declare 'static' if the function is not intended to be used outside of this translation unit bool kvm_find_error_gfn(struct kvm_vcpu *vcpu, gfn_t gfn)
^
static
1 warning generated.
vim +/kvm_find_error_gfn +10403 arch/x86/kvm/x86.c
10402
10403 bool kvm_find_error_gfn(struct kvm_vcpu *vcpu, gfn_t gfn)
10404 {
10405 u32 key = kvm_error_gfn_hash_fn(gfn);
10406
10407 return vcpu->arch.apf.error_gfns[key] == gfn;
10408 }
10409
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org