Fix walk_addr() page fault error codes
cover Fix walk_addr() page fault error codes
This patchset fixes bugs in page fault error code generation. With the patches applied, the access.flat unit test now passes with no errors on shadow, npt, and ept.
Avi Kivity (2):
KVM: MMU: Only indicate a fetch fault in page fault error code if nx
is enabled
KVM: MMU: Keep going on permission error
arch/x86/kvm/paging_tmpl.h | 52 +++++++++++++++++++++++++------------------ 1 files changed, 30 insertions(+), 22 deletions(-)
PATCH 1/2 KVM: MMU: Only indicate a fetch fault in page fault error code if nx is enabled
Bit 4 of the page fault error code is set only if EFER.NX is set.
--- arch/x86/kvm/paging_tmpl.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
arch/x86/kvm/paging_tmpl.h +1 -1
@@ -245,7 +245,7 @@ err: walker->error_code |=PFERR_WRITE_MASK ;if (user_fault) walker->error_code |=PFERR_USER_MASK ; -if (fetch_fault) +if (fetch_fault &&is_nx (vcpu)) walker->error_code |=PFERR_FETCH_MASK ;if (rsvd_fault) walker->error_code |=PFERR_RSVD_MASK ;
Signature · 1 line
1.7.1
PATCH 2/2 KVM: MMU: Keep going on permission error
In reply to Avi KivityReal hardware disregards permission errors when computing page fault error code bit 0 (page present). Do the same.
--- arch/x86/kvm/paging_tmpl.h | 50 +++++++++++++++++++++++++------------------ 1 files changed, 29 insertions(+), 21 deletions(-)
arch/x86/kvm/paging_tmpl.h +29 -21
@@ -121,19 +121,23 @@ static int FNAME(walk_addr)(struct guest_walker *walker,gfn_t table_gfn;unsigned index, pt_access, pte_access;gpa_t pte_gpa; -int rsvd_fault =0 ; +bool eperm, present, rsvd_fault;trace_kvm_mmu_pagetable_walk (addr, write_fault, user_fault, fetch_fault); walk: + present = true; + eperm = rsvd_fault = false; walker->level = vcpu->arch.mmu.root_level; pte = vcpu->arch.cr3;#if PTTYPE ==64 if (!is_long_mode (vcpu)) { pte =kvm_pdptr_read (vcpu, (addr >>30 ) &3 );trace_kvm_mmu_paging_element (pte, walker->level); -if (!is_present_gpte (pte)) - goto not_present; +if (!is_present_gpte (pte)) { + present = false; + goto error; + } --walker->level; }#endif
@@ -151,31 +155,36 @@ walk: walker->table_gfn[walker->level -1 ] = table_gfn; walker->pte_gpa[walker->level -1 ] = pte_gpa; -if (kvm_read_guest (vcpu->kvm, pte_gpa, &pte,sizeof (pte))) - goto not_present; +if (kvm_read_guest (vcpu->kvm, pte_gpa, &pte,sizeof (pte))) { + present = false; +break ; + }trace_kvm_mmu_paging_element (pte, walker->level); -if (!is_present_gpte (pte)) - goto not_present; +if (!is_present_gpte (pte)) { + present = false; +break ; + } - rsvd_fault =is_rsvd_bits_set (vcpu, pte, walker->level); -if (rsvd_fault) - goto access_error; +if (is_rsvd_bits_set (vcpu, pte, walker->level)) { + rsvd_fault = true; +break ; + }if (write_fault && !is_writable_pte (pte))if (user_fault ||is_write_protection (vcpu)) - goto access_error; + eperm = true;if (user_fault && !(pte &PT_USER_MASK )) - goto access_error; + eperm = true;#if PTTYPE ==64 if (fetch_fault && (pte &PT64_NX_MASK )) - goto access_error; + eperm = true;#endif -if (!(pte &PT_ACCESSED_MASK )) { +if (!eperm && !rsvd_fault && !(pte &PT_ACCESSED_MASK )) {trace_kvm_mmu_set_accessed_bit (table_gfn, index,sizeof (pte)); if (FNAME(cmpxchg_gpte)(vcpu->kvm, table_gfn,
@@ -214,6 +223,9 @@ walk: --walker->level; } +if (!present || eperm || rsvd_fault) + goto error; +if (write_fault && !is_dirty_gpte (pte)) {bool ret;
@@ -233,14 +245,10 @@ walk: __func__, (u64)pte, pte_access, pt_access);return 1 ; -not_present: +error: walker->error_code =0 ; - goto err; - -access_error: - walker->error_code =PFERR_PRESENT_MASK ; - -err: +if (present) + walker->error_code |=PFERR_PRESENT_MASK ;if (write_fault) walker->error_code |=PFERR_WRITE_MASK ;if (user_fault)
Signature · 1 line
1.7.1
Re: [PATCH 2/2] KVM: MMU: Keep going on permission error
Quoted 3 lines · Avi Kivity
Real hardware disregards permission errors when computing page fault error code bit 0 (page present). Do the same.
This generates (false positive) build warnings here:
CC [M] /data/kvm-kmod/x86/coalesced_mmio.o
/data/kvm-kmod/x86/paging_tmpl.h: In function ‘paging32_walk_addr’:
/data/kvm-kmod/x86/paging_tmpl.h:122: warning: ‘pte_access’ may be used uninitialized in this function /data/kvm-kmod/x86/paging_tmpl.h: In function ‘paging64_walk_addr’:
/data/kvm-kmod/x86/paging_tmpl.h:122: warning: ‘pte_access’ may be used uninitialized in this function
Quoted 44 lines
Signed-off-by Avi Kivity <[email protected]>--- arch/x86/kvm/paging_tmpl.h | 50 +++++++++++++++++++++++++------------------ 1 files changed, 29 insertions(+), 21 deletions(-)arch/x86/kvm/paging_tmpl.h +9
-5@@ -121,19 +121,23 @@ static int FNAME(walk_addr)(struct guest_walker *walker,gfn_t table_gfn;unsigned index, pt_access, pte_access;gpa_t pte_gpa; -int rsvd_fault =0 ; +bool eperm, present, rsvd_fault;trace_kvm_mmu_pagetable_walk (addr, write_fault, user_fault, fetch_fault); walk: + present = true; + eperm = rsvd_fault = false; walker->level = vcpu->arch.mmu.root_level; pte = vcpu->arch.cr3;#if PTTYPE ==64 if (!is_long_mode (vcpu)) { pte =kvm_pdptr_read (vcpu, (addr >>30 ) &3 );trace_kvm_mmu_paging_element (pte, walker->level); -if (!is_present_gpte (pte)) - goto not_present; +if (!is_present_gpte (pte)) { + present = false; + goto error; + } --walker->level; }#endif @@ -151,31 +155,36 @@ walk: walker->table_gfn[walker->level -1 ] = table_gfn; walker->pte_gpa[walker->level -1 ] = pte_gpa; -if (kvm_read_guest (vcpu->kvm, pte_gpa, &pte,sizeof (pte))) - goto not_present; +if (kvm_read_guest (vcpu->kvm, pte_gpa, &pte,sizeof (pte))) { + present = false;
arch/x86/kvm/paging_tmpl.h walk: · line 167
Quoted 10 lines
arch/x86/kvm/paging_tmpl.h +5
-2+break ; + }trace_kvm_mmu_paging_element (pte, walker->level); -if (!is_present_gpte (pte)) - goto not_present; +if (!is_present_gpte (pte)) { + present = false; +break ;
goto error?
arch/x86/kvm/paging_tmpl.h walk: · line 172
Quoted 8 lines
arch/x86/kvm/paging_tmpl.h +4
-3+ } - rsvd_fault =is_rsvd_bits_set (vcpu, pte, walker->level); -if (rsvd_fault) - goto access_error; +if (is_rsvd_bits_set (vcpu, pte, walker->level)) { + rsvd_fault = true; +break ;
goto error?
Quoted 9 lines
arch/x86/kvm/paging_tmpl.h +2
-2+ }if (write_fault && !is_writable_pte (pte))if (user_fault ||is_write_protection (vcpu)) - goto access_error; + eperm = true;if (user_fault && !(pte &PT_USER_MASK )) - goto access_error;
arch/x86/kvm/paging_tmpl.h walk: · line 187
Quoted 10 lines
arch/x86/kvm/paging_tmpl.h +3
-2+ eperm = true;#if PTTYPE ==64 if (fetch_fault && (pte &PT64_NX_MASK )) - goto access_error; + eperm = true;#endif -if (!(pte &PT_ACCESSED_MASK )) { +if (!eperm && !rsvd_fault && !(pte &PT_ACCESSED_MASK )) {
We should never get here with rsvd_fault == true - redundant check?
arch/x86/kvm/paging_tmpl.h walk: · line 228
Quoted 10 lines
arch/x86/kvm/paging_tmpl.h +3
-0trace_kvm_mmu_set_accessed_bit (table_gfn, index,sizeof (pte)); if (FNAME(cmpxchg_gpte)(vcpu->kvm, table_gfn,@@ -214,6 +223,9 @@ walk: --walker->level; } +if (!present || eperm || rsvd_fault) + goto error; +
We would only need to check for eperm here if we jumped above.
Jan
Quoted 21 lines
arch/x86/kvm/paging_tmpl.h +3
-7if (write_fault && !is_dirty_gpte (pte)) {bool ret;@@ -233,14 +245,10 @@ walk: __func__, (u64)pte, pte_access, pt_access);return 1 ; -not_present: +error: walker->error_code =0 ; - goto err; - -access_error: - walker->error_code =PFERR_PRESENT_MASK ; - -err: +if (present) + walker->error_code |=PFERR_PRESENT_MASK ;if (write_fault) walker->error_code |=PFERR_WRITE_MASK ;if (user_fault)
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [PATCH 2/2] KVM: MMU: Keep going on permission error
Quoted 8 lines · Jan Kiszka
Quoted 4 lines · Avi Kivity
Real hardware disregards permission errors when computing page fault error code bit 0 (page present). Do the same.
This generates (false positive) build warnings here:
Quoted 8 lines
CC [M] /data/kvm-kmod/x86/coalesced_mmio.o/data/kvm-kmod/x86/paging_tmpl.h: In function ‘paging32_walk_addr’:
/data/kvm-kmod/x86/paging_tmpl.h:122: warning: ‘pte_access’ may be used uninitialized in this function /data/kvm-kmod/x86/paging_tmpl.h: In function ‘paging64_walk_addr’:
/data/kvm-kmod/x86/paging_tmpl.h:122: warning: ‘pte_access’ may be used uninitialized in this function
Yes, I saw. I'll clear it up.
Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [PATCH 2/2] KVM: MMU: Keep going on permission error
Quoted 18 lines · Avi Kivity
Quoted 8 lines · Jan Kiszka
Quoted 4 lines · Avi Kivity
Real hardware disregards permission errors when computing page fault error code bit 0 (page present). Do the same.
This generates (false positive) build warnings here:
Quoted 8 lines
CC [M] /data/kvm-kmod/x86/coalesced_mmio.o/data/kvm-kmod/x86/paging_tmpl.h: In function ‘paging32_walk_addr’:
/data/kvm-kmod/x86/paging_tmpl.h:122: warning: ‘pte_access’ may be used uninitialized in this function /data/kvm-kmod/x86/paging_tmpl.h: In function ‘paging64_walk_addr’:
/data/kvm-kmod/x86/paging_tmpl.h:122: warning: ‘pte_access’ may be used uninitialized in this function
Quoted 2 lines
Yes, I saw. I'll clear it up.
You already pushed some update, don't you? Then I'm afraid I have to report that the problem persists.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [PATCH 2/2] KVM: MMU: Keep going on permission error
Quoted 7 lines · Jan Kiszka
Quoted 4 lines
Yes, I saw. I'll clear it up.
You already pushed some update, don't you? Then I'm afraid I have to report that the problem persists.
Yes, I did. I can't believe I didn't even compile test it.
Sorry, will fix for real now.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 9 lines · Avi Kivity
This patchset fixes bugs in page fault error code generation. With the patches applied, the access.flat unit test now passes with no errors on shadow, npt, and ept.
Avi Kivity (2):
KVM: MMU: Only indicate a fetch fault in page fault error code if nx
is enabled
KVM: MMU: Keep going on permission error
Quoted 11 lines · Avi Kivity
This patchset fixes bugs in page fault error code generation. With the patches applied, the access.flat unit test now passes with no errors on shadow, npt, and ept.
Avi Kivity (2):
KVM: MMU: Only indicate a fetch fault in page fault error code if nx
is enabled
KVM: MMU: Keep going on permission errorarch/x86/kvm/paging_tmpl.h | 52 +++++++++++++++++++++++++------------------ 1 files changed, 30 insertions(+), 22 deletions(-)
Looks good to me.