KVM: MMU: drop read-only large sptes when creating lower level sptes
Read-only large sptes can be created due to read-only faults as follows:
- QEMU pagetable entry that maps guest memory is read-only due to COW.
- Guest read faults such memory, COW is not broken, because it is a read-only fault.
- Enable dirty logging, large spte not nuked because it is read-only.
- Write-fault on such memory causes guest to loop endlessly (which must go down to level 1 because dirty logging is enabled).
Fix by dropping large spte when necessary.
arch/x86/kvm/mmu.c +1 -0
@@ -2672,6 +2672,7 @@ static int __direct_map(struct kvm_vcpu *vcpu, gpa_t v, int write,break ; } +drop_large_spte (vcpu, iterator.sptep);if (!is_shadow_present_pte (*iterator.sptep)) {u64 base_addr = iterator.addr;
Quoted 11 lines · Marcelo Tosatti
Read-only large sptes can be created due to read-only faults as follows:
- QEMU pagetable entry that maps guest memory is read-only due to COW.
- Guest read faults such memory, COW is not broken, because it is a read-only fault.
- Enable dirty logging, large spte not nuked because it is read-only.
- Write-fault on such memory causes guest to loop endlessly (which must go down to level 1 because dirty logging is enabled).
Hi Marcelo,
It surprised me that the large-readonly mapping was not dropped by mmu-notifer as this is write fault on readonly mapping in Qemu. Hmm... i missed something?
Quoted 13 lines · Xiao Guangrong
Quoted 11 lines · Marcelo Tosatti
Read-only large sptes can be created due to read-only faults as follows:
- QEMU pagetable entry that maps guest memory is read-only due to COW.
- Guest read faults such memory, COW is not broken, because it is a read-only fault.
- Enable dirty logging, large spte not nuked because it is read-only.
- Write-fault on such memory causes guest to loop endlessly (which must go down to level 1 because dirty logging is enabled).
Quoted 5 lines
Hi Marcelo,
It surprised me that the large-readonly mapping was not dropped by mmu-notifer as this is write fault on readonly mapping in Qemu. Hmm... i missed something?
You mean COW was not broken by gup? (that is the problem, so a read-only large spte is created).
Don't see why mmu-notifier should invalidate the spte?
Quoted 14 lines · Marcelo Tosatti
Quoted 13 lines · Xiao Guangrong
Quoted 11 lines · Marcelo Tosatti
Read-only large sptes can be created due to read-only faults as follows:
- QEMU pagetable entry that maps guest memory is read-only due to COW.
- Guest read faults such memory, COW is not broken, because it is a read-only fault.
- Enable dirty logging, large spte not nuked because it is read-only.
- Write-fault on such memory causes guest to loop endlessly (which must go down to level 1 because dirty logging is enabled).
Quoted 9 lines
Quoted 5 lines
Hi Marcelo,
It surprised me that the large-readonly mapping was not dropped by mmu-notifer as this is write fault on readonly mapping in Qemu. Hmm... i missed something?
You mean COW was not broken by gup? (that is the problem, so a read-only large spte is created).
I mean the final step that “Write-fault on such memory causes guest” should break the readonly QEMU pagetable entry and change it to writable, at that time, mmu-notifier should be called since the page’s permission has been changed.
After read the code more carefully, i figure the reason out that mmu-notifier is not called when the page can be reused. I am thinking whether it is reasonable:
- anyway, the permission bits on the pre are changed, why not call mmu-notifier->change_pte()?
- kvm and other users of mum-notifier need to check the permission changing very carefully, that is really subtle, like the case fixed here.
- if we do proper prefetch when it happen, the addition page-fault can be avoided, that is good for the performance.
Quoted 27 lines · Xiao Guangrong
Quoted 14 lines · Marcelo Tosatti
Quoted 13 lines · Xiao Guangrong
Quoted 11 lines · Marcelo Tosatti
Read-only large sptes can be created due to read-only faults as follows:
- QEMU pagetable entry that maps guest memory is read-only due to COW.
- Guest read faults such memory, COW is not broken, because it is a read-only fault.
- Enable dirty logging, large spte not nuked because it is read-only.
- Write-fault on such memory causes guest to loop endlessly (which must go down to level 1 because dirty logging is enabled).Quoted 9 lines
Quoted 5 lines
Hi Marcelo,
It surprised me that the large-readonly mapping was not dropped by mmu-notifer as this is write fault on readonly mapping in Qemu. Hmm... i missed something?
You mean COW was not broken by gup? (that is the problem, so a read-only large spte is created).
Quoted 7 lines
I mean the final step that “Write-fault on such memory causes guest” should break the readonly QEMU pagetable entry and change it to writable, at that time, mmu-notifier should be called since the page’s permission has been changed.
After read the code more carefully, i figure the reason out that mmu-notifier is not called when the page can be reused.
Well a transition from read-only host-pte to read-write host-pte does not require the read-only spte to be upgraded immediatelly, right? Permission is upgraded lazily.
Quoted 4 lines
I am thinking whether it is
reasonable:
- anyway, the permission bits on the pre are changed, why not call mmu-notifier->change_pte()?
It could, but lazy updates are better i suppose.
Quoted 2 lines
- kvm and other users of mum-notifier need to check the permission changing very carefully, that is really subtle, like the case fixed here.
Yes.
Quoted 2 lines
- if we do proper prefetch when it happen, the addition page-fault can be avoided, that is good for the performance.
Agree.
Paolo Bonzini
maintainerIl 24/02/2014 17:59, Marcelo Tosatti ha scritto:
Quoted 23 lines
Read-only large sptes can be created due to read-only faults as follows:
- QEMU pagetable entry that maps guest memory is read-only due to COW.
- Guest read faults such memory, COW is not broken, because it is a read-only fault.
- Enable dirty logging, large spte not nuked because it is read-only.
- Write-fault on such memory causes guest to loop endlessly (which must go down to level 1 because dirty logging is enabled).Fix by dropping large spte when necessary.
Signed-off-by Marcelo Tosatti <[email protected]>arch/x86/kvm/mmu.c +0
-0
arch/x86/kvm/mmu.c static int __direct_map(struct kvm_vcpu *vcpu, gpa_t v, int write, · line 2678
Quoted 7 lines
arch/x86/kvm/mmu.c +1
-0@@ -2672,6 +2672,7 @@ static int __direct_map(struct kvm_vcpu *vcpu, gpa_t v, int write,break ; } +drop_large_spte (vcpu, iterator.sptep);if (!is_shadow_present_pte (*iterator.sptep)) {u64 base_addr = iterator.addr;
Queued for kvm/master, will push as soon as I test it.
Thanks,
Paolo