Re: [PATCH v3 7/9] KVM: VMX: Add guest physical address check in EPT violation and misconfig
On Wed, Jan 20, 2021 at 1:16 PM Jim Mattson wrote: > > On Fri, Jan 15, 2021 at 11:35 AM Jim Mattson wrote: > > > > On Fri, Oct 23, 2020 at 10:43 AM Paolo Bonzini wrote: > > > > > > On 23/10/20 19:23, Jim Mattson wrote: > > > >> The information that we need is _not_ that provided by the advanced > > > >> VM-exit information (or by a page walk). If a page is neither writable > > > >> nor executable, the advanced information doesn't say if the injected > > > >> #PF > > > >> should be a W=1 or a F=1 fault. We need the information in bits 0..2 > > > >> of > > > >> the exit qualification for the final access, which however is not > > > >> available for the paging-structure access. > > > >> > > > > Are you planning to extend the emulator, then, to support all > > > > instructions? I'm not sure where you are going with this. > > > > > > I'm going to fix the bit 8=1 case, but for bit 8=0 there's not much that > > > you can do. In all likelihood the guest is buggy anyway. > > > > Did this drop off your radar? Are you still planning to fix the bit8=1 > > case to use advanced EPT exit qualification information? Or did I just > > miss it? > > Paolo, > If you're not working on this, do you mind if I ask Aaron to take a look at > it? Ugh. The advanced EPT exit qualification contains nothing useful here, AFAICT. It only contains x86 page protection information--nothing about the access itself.
Re: [PATCH v3 7/9] KVM: VMX: Add guest physical address check in EPT violation and misconfig
On Fri, Jan 15, 2021 at 11:35 AM Jim Mattson wrote: > > On Fri, Oct 23, 2020 at 10:43 AM Paolo Bonzini wrote: > > > > On 23/10/20 19:23, Jim Mattson wrote: > > >> The information that we need is _not_ that provided by the advanced > > >> VM-exit information (or by a page walk). If a page is neither writable > > >> nor executable, the advanced information doesn't say if the injected #PF > > >> should be a W=1 or a F=1 fault. We need the information in bits 0..2 of > > >> the exit qualification for the final access, which however is not > > >> available for the paging-structure access. > > >> > > > Are you planning to extend the emulator, then, to support all > > > instructions? I'm not sure where you are going with this. > > > > I'm going to fix the bit 8=1 case, but for bit 8=0 there's not much that > > you can do. In all likelihood the guest is buggy anyway. > > Did this drop off your radar? Are you still planning to fix the bit8=1 > case to use advanced EPT exit qualification information? Or did I just > miss it? Paolo, If you're not working on this, do you mind if I ask Aaron to take a look at it?
Re: [PATCH v3 7/9] KVM: VMX: Add guest physical address check in EPT violation and misconfig
On Fri, Oct 23, 2020 at 10:43 AM Paolo Bonzini wrote: > > On 23/10/20 19:23, Jim Mattson wrote: > >> The information that we need is _not_ that provided by the advanced > >> VM-exit information (or by a page walk). If a page is neither writable > >> nor executable, the advanced information doesn't say if the injected #PF > >> should be a W=1 or a F=1 fault. We need the information in bits 0..2 of > >> the exit qualification for the final access, which however is not > >> available for the paging-structure access. > >> > > Are you planning to extend the emulator, then, to support all > > instructions? I'm not sure where you are going with this. > > I'm going to fix the bit 8=1 case, but for bit 8=0 there's not much that > you can do. In all likelihood the guest is buggy anyway. Did this drop off your radar? Are you still planning to fix the bit8=1 case to use advanced EPT exit qualification information? Or did I just miss it? > It would be possible to only do the decode part of the emulator to get > the PFEC (matching the GVA from the vmexit to the memory operand, for > example, and retrying if the instruction is unexpected). Then one would > only need enough VEX/EVEX parsing to process the decoding. > > Paolo >
Re: [PATCH v3 7/9] KVM: VMX: Add guest physical address check in EPT violation and misconfig
On 23/10/20 19:23, Jim Mattson wrote: >> The information that we need is _not_ that provided by the advanced >> VM-exit information (or by a page walk). If a page is neither writable >> nor executable, the advanced information doesn't say if the injected #PF >> should be a W=1 or a F=1 fault. We need the information in bits 0..2 of >> the exit qualification for the final access, which however is not >> available for the paging-structure access. >> > Are you planning to extend the emulator, then, to support all > instructions? I'm not sure where you are going with this. I'm going to fix the bit 8=1 case, but for bit 8=0 there's not much that you can do. In all likelihood the guest is buggy anyway. It would be possible to only do the decode part of the emulator to get the PFEC (matching the GVA from the vmexit to the memory operand, for example, and retrying if the instruction is unexpected). Then one would only need enough VEX/EVEX parsing to process the decoding. Paolo
Re: [PATCH v3 7/9] KVM: VMX: Add guest physical address check in EPT violation and misconfig
On Fri, Oct 23, 2020 at 10:16 AM Paolo Bonzini wrote: > > On 23/10/20 18:59, Jim Mattson wrote: > >> The problem is that page fault error code bits cannot be reconstructed > >> from bits 0..2 of the EPT violation exit qualification, if bit 8 is > >> clear in the exit qualification (that is, if the access causing the EPT > >> violation is to a paging-structure entry). In that case bits 0..2 refer > >> to the paging-structure access rather than to the final access. In fact > >> advanced information is not available at all for paging-structure access > >> EPT violations. > > > > True, but the in-kernel emulator can only handle a very small subset > > of the available instructions. > > > > If bit 8 is set in the exit qualification, we should use the advanced > > VM-exit information. If it's clear, we should just do a software page > > walk of the guest's x86 page tables. > > The information that we need is _not_ that provided by the advanced > VM-exit information (or by a page walk). If a page is neither writable > nor executable, the advanced information doesn't say if the injected #PF > should be a W=1 or a F=1 fault. We need the information in bits 0..2 of > the exit qualification for the final access, which however is not > available for the paging-structure access. > Are you planning to extend the emulator, then, to support all instructions? I'm not sure where you are going with this.
Re: [PATCH v3 7/9] KVM: VMX: Add guest physical address check in EPT violation and misconfig
On 23/10/20 18:59, Jim Mattson wrote: >> The problem is that page fault error code bits cannot be reconstructed >> from bits 0..2 of the EPT violation exit qualification, if bit 8 is >> clear in the exit qualification (that is, if the access causing the EPT >> violation is to a paging-structure entry). In that case bits 0..2 refer >> to the paging-structure access rather than to the final access. In fact >> advanced information is not available at all for paging-structure access >> EPT violations. > > True, but the in-kernel emulator can only handle a very small subset > of the available instructions. > > If bit 8 is set in the exit qualification, we should use the advanced > VM-exit information. If it's clear, we should just do a software page > walk of the guest's x86 page tables. The information that we need is _not_ that provided by the advanced VM-exit information (or by a page walk). If a page is neither writable nor executable, the advanced information doesn't say if the injected #PF should be a W=1 or a F=1 fault. We need the information in bits 0..2 of the exit qualification for the final access, which however is not available for the paging-structure access. If bit 8 is set, however, we need not use the emulator indeed, as the W/F/U bits are all available from either the exit qualification or in the SS access rights. The access.flat test in kvm-unit-tests covers all this, so it will be easy to check the theory. Paolo > The in-kernel emulator should > only be used as a last resort on hardware that doesn't support the > advanced VM-exit information for EPT violations. >
Re: [PATCH v3 7/9] KVM: VMX: Add guest physical address check in EPT violation and misconfig
On Fri, Oct 23, 2020 at 2:22 AM Paolo Bonzini wrote: > > On 23/10/20 05:14, Sean Christopherson wrote: > + > + /* > +* Check that the GPA doesn't exceed physical memory limits, as > that is > +* a guest page fault. We have to emulate the instruction here, > because > +* if the illegal address is that of a paging structure, then > +* EPT_VIOLATION_ACC_WRITE bit is set. Alternatively, if > supported we > +* would also use advanced VM-exit information for EPT > violations to > +* reconstruct the page fault error code. > +*/ > + if (unlikely(kvm_mmu_is_illegal_gpa(vcpu, gpa))) > + return kvm_emulate_instruction(vcpu, 0); > + > >>> Is kvm's in-kernel emulator up to the task? What if the instruction in > >>> question is AVX-512, or one of the myriad instructions that the > >>> in-kernel emulator can't handle? Ice Lake must support the advanced > >>> VM-exit information for EPT violations, so that would seem like a > >>> better choice. > >>> > >> Anyone? > > > > Using "advanced info" if it's supported seems like the way to go. Outright > > requiring it is probably overkill; if userspace wants to risk having to > > kill a > > (likely broken) guest, so be it. > > Yeah, the instruction is expected to page-fault here. However the > comment is incorrect and advanced information does not help here. > > The problem is that page fault error code bits cannot be reconstructed > from bits 0..2 of the EPT violation exit qualification, if bit 8 is > clear in the exit qualification (that is, if the access causing the EPT > violation is to a paging-structure entry). In that case bits 0..2 refer > to the paging-structure access rather than to the final access. In fact > advanced information is not available at all for paging-structure access > EPT violations. True, but the in-kernel emulator can only handle a very small subset of the available instructions. If bit 8 is set in the exit qualification, we should use the advanced VM-exit information. If it's clear, we should just do a software page walk of the guest's x86 page tables. The in-kernel emulator should only be used as a last resort on hardware that doesn't support the advanced VM-exit information for EPT violations.
Re: [PATCH v3 7/9] KVM: VMX: Add guest physical address check in EPT violation and misconfig
On 23/10/20 05:14, Sean Christopherson wrote: + + /* +* Check that the GPA doesn't exceed physical memory limits, as that is +* a guest page fault. We have to emulate the instruction here, because +* if the illegal address is that of a paging structure, then +* EPT_VIOLATION_ACC_WRITE bit is set. Alternatively, if supported we +* would also use advanced VM-exit information for EPT violations to +* reconstruct the page fault error code. +*/ + if (unlikely(kvm_mmu_is_illegal_gpa(vcpu, gpa))) + return kvm_emulate_instruction(vcpu, 0); + >>> Is kvm's in-kernel emulator up to the task? What if the instruction in >>> question is AVX-512, or one of the myriad instructions that the >>> in-kernel emulator can't handle? Ice Lake must support the advanced >>> VM-exit information for EPT violations, so that would seem like a >>> better choice. >>> >> Anyone? > > Using "advanced info" if it's supported seems like the way to go. Outright > requiring it is probably overkill; if userspace wants to risk having to kill a > (likely broken) guest, so be it. Yeah, the instruction is expected to page-fault here. However the comment is incorrect and advanced information does not help here. The problem is that page fault error code bits cannot be reconstructed from bits 0..2 of the EPT violation exit qualification, if bit 8 is clear in the exit qualification (that is, if the access causing the EPT violation is to a paging-structure entry). In that case bits 0..2 refer to the paging-structure access rather than to the final access. In fact advanced information is not available at all for paging-structure access EPT violations. Thanks, Paolo
Re: [PATCH v3 7/9] KVM: VMX: Add guest physical address check in EPT violation and misconfig
On Wed, Oct 14, 2020 at 04:44:57PM -0700, Jim Mattson wrote: > On Fri, Oct 9, 2020 at 9:17 AM Jim Mattson wrote: > > > > On Fri, Jul 10, 2020 at 8:48 AM Mohammed Gamal wrote: > > > @@ -5308,6 +5314,18 @@ static int handle_ept_violation(struct kvm_vcpu > > > *vcpu) > > >PFERR_GUEST_FINAL_MASK : PFERR_GUEST_PAGE_MASK; > > > > > > vcpu->arch.exit_qualification = exit_qualification; > > > + > > > + /* > > > +* Check that the GPA doesn't exceed physical memory limits, as > > > that is > > > +* a guest page fault. We have to emulate the instruction here, > > > because > > > +* if the illegal address is that of a paging structure, then > > > +* EPT_VIOLATION_ACC_WRITE bit is set. Alternatively, if > > > supported we > > > +* would also use advanced VM-exit information for EPT violations > > > to > > > +* reconstruct the page fault error code. > > > +*/ > > > + if (unlikely(kvm_mmu_is_illegal_gpa(vcpu, gpa))) > > > + return kvm_emulate_instruction(vcpu, 0); > > > + > > > > Is kvm's in-kernel emulator up to the task? What if the instruction in > > question is AVX-512, or one of the myriad instructions that the > > in-kernel emulator can't handle? Ice Lake must support the advanced > > VM-exit information for EPT violations, so that would seem like a > > better choice. > > > Anyone? Using "advanced info" if it's supported seems like the way to go. Outright requiring it is probably overkill; if userspace wants to risk having to kill a (likely broken) guest, so be it.
Re: [PATCH v3 7/9] KVM: VMX: Add guest physical address check in EPT violation and misconfig
On Fri, Oct 9, 2020 at 9:17 AM Jim Mattson wrote:
>
> On Fri, Jul 10, 2020 at 8:48 AM Mohammed Gamal wrote:
> >
> > Check guest physical address against it's maximum physical memory. If
> > the guest's physical address exceeds the maximum (i.e. has reserved bits
> > set), inject a guest page fault with PFERR_RSVD_MASK set.
> >
> > This has to be done both in the EPT violation and page fault paths, as
> > there are complications in both cases with respect to the computation
> > of the correct error code.
> >
> > For EPT violations, unfortunately the only possibility is to emulate,
> > because the access type in the exit qualification might refer to an
> > access to a paging structure, rather than to the access performed by
> > the program.
> >
> > Trapping page faults instead is needed in order to correct the error code,
> > but the access type can be obtained from the original error code and
> > passed to gva_to_gpa. The corrections required in the error code are
> > subtle. For example, imagine that a PTE for a supervisor page has a reserved
> > bit set. On a supervisor-mode access, the EPT violation path would trigger.
> > However, on a user-mode access, the processor will not notice the reserved
> > bit and not include PFERR_RSVD_MASK in the error code.
> >
> > Co-developed-by: Mohammed Gamal
> > Signed-off-by: Paolo Bonzini
> > ---
> > arch/x86/kvm/vmx/vmx.c | 24 +---
> > arch/x86/kvm/vmx/vmx.h | 3 ++-
> > 2 files changed, 23 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index 770b090969fb..de3f436b2d32 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -4790,9 +4790,15 @@ static int handle_exception_nmi(struct kvm_vcpu
> > *vcpu)
> >
> > if (is_page_fault(intr_info)) {
> > cr2 = vmx_get_exit_qual(vcpu);
> > - /* EPT won't cause page fault directly */
> > - WARN_ON_ONCE(!vcpu->arch.apf.host_apf_flags && enable_ept);
> > - return kvm_handle_page_fault(vcpu, error_code, cr2, NULL,
> > 0);
> > + if (enable_ept && !vcpu->arch.apf.host_apf_flags) {
> > + /*
> > +* EPT will cause page fault only if we need to
> > +* detect illegal GPAs.
> > +*/
> > + kvm_fixup_and_inject_pf_error(vcpu, cr2,
> > error_code);
> > + return 1;
> > + } else
> > + return kvm_handle_page_fault(vcpu, error_code, cr2,
> > NULL, 0);
> > }
> >
> > ex_no = intr_info & INTR_INFO_VECTOR_MASK;
> > @@ -5308,6 +5314,18 @@ static int handle_ept_violation(struct kvm_vcpu
> > *vcpu)
> >PFERR_GUEST_FINAL_MASK : PFERR_GUEST_PAGE_MASK;
> >
> > vcpu->arch.exit_qualification = exit_qualification;
> > +
> > + /*
> > +* Check that the GPA doesn't exceed physical memory limits, as
> > that is
> > +* a guest page fault. We have to emulate the instruction here,
> > because
> > +* if the illegal address is that of a paging structure, then
> > +* EPT_VIOLATION_ACC_WRITE bit is set. Alternatively, if supported
> > we
> > +* would also use advanced VM-exit information for EPT violations to
> > +* reconstruct the page fault error code.
> > +*/
> > + if (unlikely(kvm_mmu_is_illegal_gpa(vcpu, gpa)))
> > + return kvm_emulate_instruction(vcpu, 0);
> > +
>
> Is kvm's in-kernel emulator up to the task? What if the instruction in
> question is AVX-512, or one of the myriad instructions that the
> in-kernel emulator can't handle? Ice Lake must support the advanced
> VM-exit information for EPT violations, so that would seem like a
> better choice.
>
Anyone?
Re: [PATCH v3 7/9] KVM: VMX: Add guest physical address check in EPT violation and misconfig
On Fri, Jul 10, 2020 at 8:48 AM Mohammed Gamal wrote:
>
> Check guest physical address against it's maximum physical memory. If
> the guest's physical address exceeds the maximum (i.e. has reserved bits
> set), inject a guest page fault with PFERR_RSVD_MASK set.
>
> This has to be done both in the EPT violation and page fault paths, as
> there are complications in both cases with respect to the computation
> of the correct error code.
>
> For EPT violations, unfortunately the only possibility is to emulate,
> because the access type in the exit qualification might refer to an
> access to a paging structure, rather than to the access performed by
> the program.
>
> Trapping page faults instead is needed in order to correct the error code,
> but the access type can be obtained from the original error code and
> passed to gva_to_gpa. The corrections required in the error code are
> subtle. For example, imagine that a PTE for a supervisor page has a reserved
> bit set. On a supervisor-mode access, the EPT violation path would trigger.
> However, on a user-mode access, the processor will not notice the reserved
> bit and not include PFERR_RSVD_MASK in the error code.
>
> Co-developed-by: Mohammed Gamal
> Signed-off-by: Paolo Bonzini
> ---
> arch/x86/kvm/vmx/vmx.c | 24 +---
> arch/x86/kvm/vmx/vmx.h | 3 ++-
> 2 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 770b090969fb..de3f436b2d32 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -4790,9 +4790,15 @@ static int handle_exception_nmi(struct kvm_vcpu *vcpu)
>
> if (is_page_fault(intr_info)) {
> cr2 = vmx_get_exit_qual(vcpu);
> - /* EPT won't cause page fault directly */
> - WARN_ON_ONCE(!vcpu->arch.apf.host_apf_flags && enable_ept);
> - return kvm_handle_page_fault(vcpu, error_code, cr2, NULL, 0);
> + if (enable_ept && !vcpu->arch.apf.host_apf_flags) {
> + /*
> +* EPT will cause page fault only if we need to
> +* detect illegal GPAs.
> +*/
> + kvm_fixup_and_inject_pf_error(vcpu, cr2, error_code);
> + return 1;
> + } else
> + return kvm_handle_page_fault(vcpu, error_code, cr2,
> NULL, 0);
> }
>
> ex_no = intr_info & INTR_INFO_VECTOR_MASK;
> @@ -5308,6 +5314,18 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu)
>PFERR_GUEST_FINAL_MASK : PFERR_GUEST_PAGE_MASK;
>
> vcpu->arch.exit_qualification = exit_qualification;
> +
> + /*
> +* Check that the GPA doesn't exceed physical memory limits, as that
> is
> +* a guest page fault. We have to emulate the instruction here,
> because
> +* if the illegal address is that of a paging structure, then
> +* EPT_VIOLATION_ACC_WRITE bit is set. Alternatively, if supported we
> +* would also use advanced VM-exit information for EPT violations to
> +* reconstruct the page fault error code.
> +*/
> + if (unlikely(kvm_mmu_is_illegal_gpa(vcpu, gpa)))
> + return kvm_emulate_instruction(vcpu, 0);
> +
Is kvm's in-kernel emulator up to the task? What if the instruction in
question is AVX-512, or one of the myriad instructions that the
in-kernel emulator can't handle? Ice Lake must support the advanced
VM-exit information for EPT violations, so that would seem like a
better choice.
> return kvm_mmu_page_fault(vcpu, gpa, error_code, NULL, 0);
> }
>
> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> index b0e5e210f1c1..0d06951e607c 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -11,6 +11,7 @@
> #include "kvm_cache_regs.h"
> #include "ops.h"
> #include "vmcs.h"
> +#include "cpuid.h"
>
> extern const u32 vmx_msr_index[];
>
> @@ -552,7 +553,7 @@ static inline bool vmx_has_waitpkg(struct vcpu_vmx *vmx)
>
> static inline bool vmx_need_pf_intercept(struct kvm_vcpu *vcpu)
> {
> - return !enable_ept;
> + return !enable_ept || cpuid_maxphyaddr(vcpu) <
> boot_cpu_data.x86_phys_bits;
> }
>
> void dump_vmcs(void);
> --
> 2.26.2
>
Re: [PATCH v3 7/9] KVM: VMX: Add guest physical address check in EPT violation and misconfig
On 17/08/20 19:22, Sean Christopherson wrote: >> This splats when running the PKU unit test, although the test still passed. >> I haven't yet spent the brain power to determine if this is a benign warning, >> i.e. simply unexpected, or if permission_fault() fault truly can't handle PK >> faults. It's more or less unexpected; the error is in the caller. This is not an error code but an access mask so only U/F/W bits are valid. Patch sent, thanks. Paolo >> WARNING: CPU: 25 PID: 5465 at arch/x86/kvm/mmu.h:197 >> paging64_walk_addr_generic+0x594/0x750 [kvm] >> Hardware name: Intel Corporation WilsonCity/WilsonCity, BIOS >> WLYDCRB1.SYS.0014.D62.2001092233 01/09/2020 >> RIP: 0010:paging64_walk_addr_generic+0x594/0x750 [kvm] >> Code: <0f> 0b e9 db fe ff ff 44 8b 43 04 4c 89 6c 24 30 8b 13 41 39 d0 89 >> RSP: 0018:ff53778fc623fb60 EFLAGS: 00010202 >> RAX: 0001 RBX: ff53778fc623fbf0 RCX: 0007 >> RDX: 0001 RSI: 0002 RDI: ff4501efba818000 >> RBP: 0020 R08: 0005 R09: 004000e7 >> R10: 0001 R11: R12: 0007 >> R13: ff4501efba818388 R14: 104000e7 R15: >> FS: 7f2dcf31a700() GS:ff4501f1c804() >> knlGS: >> CS: 0010 DS: ES: CR0: 80050033 >> CR2: CR3: 001dea475005 CR4: 00763ee0 >> DR0: DR1: DR2: >> DR3: DR6: fffe0ff0 DR7: 0400 >> PKRU: 5554 >> Call Trace: >>paging64_gva_to_gpa+0x3f/0xb0 [kvm] >>kvm_fixup_and_inject_pf_error+0x48/0xa0 [kvm] >>handle_exception_nmi+0x4fc/0x5b0 [kvm_intel] >>kvm_arch_vcpu_ioctl_run+0x911/0x1c10 [kvm] >>kvm_vcpu_ioctl+0x23e/0x5d0 [kvm] >>ksys_ioctl+0x92/0xb0 >>__x64_sys_ioctl+0x16/0x20 >>do_syscall_64+0x3e/0xb0 >>entry_SYSCALL_64_after_hwframe+0x44/0xa9 >> ---[ end trace d17eb998aee991da ]--- > > Looks like this series got pulled for 5.9, has anyone looked into this? >
Re: [PATCH v3 7/9] KVM: VMX: Add guest physical address check in EPT violation and misconfig
On Wed, Jul 15, 2020 at 04:00:08PM -0700, Sean Christopherson wrote:
> On Fri, Jul 10, 2020 at 05:48:09PM +0200, Mohammed Gamal wrote:
> > Check guest physical address against it's maximum physical memory. If
> > the guest's physical address exceeds the maximum (i.e. has reserved bits
> > set), inject a guest page fault with PFERR_RSVD_MASK set.
> >
> > This has to be done both in the EPT violation and page fault paths, as
> > there are complications in both cases with respect to the computation
> > of the correct error code.
> >
> > For EPT violations, unfortunately the only possibility is to emulate,
> > because the access type in the exit qualification might refer to an
> > access to a paging structure, rather than to the access performed by
> > the program.
> >
> > Trapping page faults instead is needed in order to correct the error code,
> > but the access type can be obtained from the original error code and
> > passed to gva_to_gpa. The corrections required in the error code are
> > subtle. For example, imagine that a PTE for a supervisor page has a reserved
> > bit set. On a supervisor-mode access, the EPT violation path would trigger.
> > However, on a user-mode access, the processor will not notice the reserved
> > bit and not include PFERR_RSVD_MASK in the error code.
> >
> > Co-developed-by: Mohammed Gamal
> > Signed-off-by: Paolo Bonzini
> > ---
> > arch/x86/kvm/vmx/vmx.c | 24 +---
> > arch/x86/kvm/vmx/vmx.h | 3 ++-
> > 2 files changed, 23 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index 770b090969fb..de3f436b2d32 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -4790,9 +4790,15 @@ static int handle_exception_nmi(struct kvm_vcpu
> > *vcpu)
> >
> > if (is_page_fault(intr_info)) {
> > cr2 = vmx_get_exit_qual(vcpu);
> > - /* EPT won't cause page fault directly */
> > - WARN_ON_ONCE(!vcpu->arch.apf.host_apf_flags && enable_ept);
> > - return kvm_handle_page_fault(vcpu, error_code, cr2, NULL, 0);
> > + if (enable_ept && !vcpu->arch.apf.host_apf_flags) {
> > + /*
> > +* EPT will cause page fault only if we need to
> > +* detect illegal GPAs.
> > +*/
> > + kvm_fixup_and_inject_pf_error(vcpu, cr2, error_code);
>
> This splats when running the PKU unit test, although the test still passed.
> I haven't yet spent the brain power to determine if this is a benign warning,
> i.e. simply unexpected, or if permission_fault() fault truly can't handle PK
> faults.
>
> WARNING: CPU: 25 PID: 5465 at arch/x86/kvm/mmu.h:197
> paging64_walk_addr_generic+0x594/0x750 [kvm]
> Hardware name: Intel Corporation WilsonCity/WilsonCity, BIOS
> WLYDCRB1.SYS.0014.D62.2001092233 01/09/2020
> RIP: 0010:paging64_walk_addr_generic+0x594/0x750 [kvm]
> Code: <0f> 0b e9 db fe ff ff 44 8b 43 04 4c 89 6c 24 30 8b 13 41 39 d0 89
> RSP: 0018:ff53778fc623fb60 EFLAGS: 00010202
> RAX: 0001 RBX: ff53778fc623fbf0 RCX: 0007
> RDX: 0001 RSI: 0002 RDI: ff4501efba818000
> RBP: 0020 R08: 0005 R09: 004000e7
> R10: 0001 R11: R12: 0007
> R13: ff4501efba818388 R14: 104000e7 R15:
> FS: 7f2dcf31a700() GS:ff4501f1c804() knlGS:
> CS: 0010 DS: ES: CR0: 80050033
> CR2: CR3: 001dea475005 CR4: 00763ee0
> DR0: DR1: DR2:
> DR3: DR6: fffe0ff0 DR7: 0400
> PKRU: 5554
> Call Trace:
>paging64_gva_to_gpa+0x3f/0xb0 [kvm]
>kvm_fixup_and_inject_pf_error+0x48/0xa0 [kvm]
>handle_exception_nmi+0x4fc/0x5b0 [kvm_intel]
>kvm_arch_vcpu_ioctl_run+0x911/0x1c10 [kvm]
>kvm_vcpu_ioctl+0x23e/0x5d0 [kvm]
>ksys_ioctl+0x92/0xb0
>__x64_sys_ioctl+0x16/0x20
>do_syscall_64+0x3e/0xb0
>entry_SYSCALL_64_after_hwframe+0x44/0xa9
> ---[ end trace d17eb998aee991da ]---
Looks like this series got pulled for 5.9, has anyone looked into this?
Re: [PATCH v3 7/9] KVM: VMX: Add guest physical address check in EPT violation and misconfig
On Fri, Jul 10, 2020 at 05:48:09PM +0200, Mohammed Gamal wrote:
> Check guest physical address against it's maximum physical memory. If
> the guest's physical address exceeds the maximum (i.e. has reserved bits
> set), inject a guest page fault with PFERR_RSVD_MASK set.
>
> This has to be done both in the EPT violation and page fault paths, as
> there are complications in both cases with respect to the computation
> of the correct error code.
>
> For EPT violations, unfortunately the only possibility is to emulate,
> because the access type in the exit qualification might refer to an
> access to a paging structure, rather than to the access performed by
> the program.
>
> Trapping page faults instead is needed in order to correct the error code,
> but the access type can be obtained from the original error code and
> passed to gva_to_gpa. The corrections required in the error code are
> subtle. For example, imagine that a PTE for a supervisor page has a reserved
> bit set. On a supervisor-mode access, the EPT violation path would trigger.
> However, on a user-mode access, the processor will not notice the reserved
> bit and not include PFERR_RSVD_MASK in the error code.
>
> Co-developed-by: Mohammed Gamal
> Signed-off-by: Paolo Bonzini
> ---
> arch/x86/kvm/vmx/vmx.c | 24 +---
> arch/x86/kvm/vmx/vmx.h | 3 ++-
> 2 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 770b090969fb..de3f436b2d32 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -4790,9 +4790,15 @@ static int handle_exception_nmi(struct kvm_vcpu *vcpu)
>
> if (is_page_fault(intr_info)) {
> cr2 = vmx_get_exit_qual(vcpu);
> - /* EPT won't cause page fault directly */
> - WARN_ON_ONCE(!vcpu->arch.apf.host_apf_flags && enable_ept);
> - return kvm_handle_page_fault(vcpu, error_code, cr2, NULL, 0);
> + if (enable_ept && !vcpu->arch.apf.host_apf_flags) {
> + /*
> + * EPT will cause page fault only if we need to
> + * detect illegal GPAs.
> + */
> + kvm_fixup_and_inject_pf_error(vcpu, cr2, error_code);
This splats when running the PKU unit test, although the test still passed.
I haven't yet spent the brain power to determine if this is a benign warning,
i.e. simply unexpected, or if permission_fault() fault truly can't handle PK
faults.
WARNING: CPU: 25 PID: 5465 at arch/x86/kvm/mmu.h:197
paging64_walk_addr_generic+0x594/0x750 [kvm]
Hardware name: Intel Corporation WilsonCity/WilsonCity, BIOS
WLYDCRB1.SYS.0014.D62.2001092233 01/09/2020
RIP: 0010:paging64_walk_addr_generic+0x594/0x750 [kvm]
Code: <0f> 0b e9 db fe ff ff 44 8b 43 04 4c 89 6c 24 30 8b 13 41 39 d0 89
RSP: 0018:ff53778fc623fb60 EFLAGS: 00010202
RAX: 0001 RBX: ff53778fc623fbf0 RCX: 0007
RDX: 0001 RSI: 0002 RDI: ff4501efba818000
RBP: 0020 R08: 0005 R09: 004000e7
R10: 0001 R11: R12: 0007
R13: ff4501efba818388 R14: 104000e7 R15:
FS: 7f2dcf31a700() GS:ff4501f1c804() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: CR3: 001dea475005 CR4: 00763ee0
DR0: DR1: DR2:
DR3: DR6: fffe0ff0 DR7: 0400
PKRU: 5554
Call Trace:
paging64_gva_to_gpa+0x3f/0xb0 [kvm]
kvm_fixup_and_inject_pf_error+0x48/0xa0 [kvm]
handle_exception_nmi+0x4fc/0x5b0 [kvm_intel]
kvm_arch_vcpu_ioctl_run+0x911/0x1c10 [kvm]
kvm_vcpu_ioctl+0x23e/0x5d0 [kvm]
ksys_ioctl+0x92/0xb0
__x64_sys_ioctl+0x16/0x20
do_syscall_64+0x3e/0xb0
entry_SYSCALL_64_after_hwframe+0x44/0xa9
---[ end trace d17eb998aee991da ]---
Re: [PATCH v3 7/9] KVM: VMX: Add guest physical address check in EPT violation and misconfig
On Fri, Jul 10, 2020 at 05:48:09PM +0200, Mohammed Gamal wrote:
> Check guest physical address against it's maximum physical memory. If
> the guest's physical address exceeds the maximum (i.e. has reserved bits
> set), inject a guest page fault with PFERR_RSVD_MASK set.
>
> This has to be done both in the EPT violation and page fault paths, as
> there are complications in both cases with respect to the computation
> of the correct error code.
>
> For EPT violations, unfortunately the only possibility is to emulate,
> because the access type in the exit qualification might refer to an
> access to a paging structure, rather than to the access performed by
> the program.
>
> Trapping page faults instead is needed in order to correct the error code,
> but the access type can be obtained from the original error code and
> passed to gva_to_gpa. The corrections required in the error code are
> subtle. For example, imagine that a PTE for a supervisor page has a reserved
> bit set. On a supervisor-mode access, the EPT violation path would trigger.
> However, on a user-mode access, the processor will not notice the reserved
> bit and not include PFERR_RSVD_MASK in the error code.
>
> Co-developed-by: Mohammed Gamal
> Signed-off-by: Paolo Bonzini
> ---
> arch/x86/kvm/vmx/vmx.c | 24 +---
> arch/x86/kvm/vmx/vmx.h | 3 ++-
> 2 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 770b090969fb..de3f436b2d32 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -4790,9 +4790,15 @@ static int handle_exception_nmi(struct kvm_vcpu *vcpu)
>
> if (is_page_fault(intr_info)) {
> cr2 = vmx_get_exit_qual(vcpu);
> - /* EPT won't cause page fault directly */
> - WARN_ON_ONCE(!vcpu->arch.apf.host_apf_flags && enable_ept);
> - return kvm_handle_page_fault(vcpu, error_code, cr2, NULL, 0);
> + if (enable_ept && !vcpu->arch.apf.host_apf_flags) {
> + /*
> + * EPT will cause page fault only if we need to
> + * detect illegal GPAs.
> + */
It'd be nice to retain a WARN_ON_ONCE() here, e.g.
WARN_ON_ONCE(!vmx_need_pf_intercept(vcpu));
This WARN has fired for me when I've botched the nested VM-Exit routing,
debugging a spurious L2 #PF without would be less than fun.
> + kvm_fixup_and_inject_pf_error(vcpu, cr2, error_code);
> + return 1;
> + } else
> + return kvm_handle_page_fault(vcpu, error_code, cr2,
> NULL, 0);
> }
>
> ex_no = intr_info & INTR_INFO_VECTOR_MASK;
> @@ -5308,6 +5314,18 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu)
> PFERR_GUEST_FINAL_MASK : PFERR_GUEST_PAGE_MASK;
>
> vcpu->arch.exit_qualification = exit_qualification;
> +
> + /*
> + * Check that the GPA doesn't exceed physical memory limits, as that is
> + * a guest page fault. We have to emulate the instruction here, because
> + * if the illegal address is that of a paging structure, then
> + * EPT_VIOLATION_ACC_WRITE bit is set. Alternatively, if supported we
> + * would also use advanced VM-exit information for EPT violations to
> + * reconstruct the page fault error code.
> + */
> + if (unlikely(kvm_mmu_is_illegal_gpa(vcpu, gpa)))
> + return kvm_emulate_instruction(vcpu, 0);
> +
> return kvm_mmu_page_fault(vcpu, gpa, error_code, NULL, 0);
> }
>
> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> index b0e5e210f1c1..0d06951e607c 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -11,6 +11,7 @@
> #include "kvm_cache_regs.h"
> #include "ops.h"
> #include "vmcs.h"
> +#include "cpuid.h"
>
> extern const u32 vmx_msr_index[];
>
> @@ -552,7 +553,7 @@ static inline bool vmx_has_waitpkg(struct vcpu_vmx *vmx)
>
> static inline bool vmx_need_pf_intercept(struct kvm_vcpu *vcpu)
> {
> - return !enable_ept;
> + return !enable_ept || cpuid_maxphyaddr(vcpu) <
> boot_cpu_data.x86_phys_bits;
> }
>
> void dump_vmcs(void);
> --
> 2.26.2
>
