kvm: x86: mmu: return zero if s > e in rsvd_bits()
RFC PATCH 1/2 kvm: x86: mmu: return zero if s > e in rsvd_bits()
In some real scenarios 'start' may not be less than 'end' like maxphyaddr = 52.
--- arch/x86/kvm/mmu.h | 2 ++ 1 file changed, 2 insertions(+)
arch/x86/kvm/mmu.h +2 -0
@@ -58,6 +58,8 @@static inline u64 rsvd_bits (int s,int e) { +if (unlikely (s > e)) +return 0 ;return ((1ULL << (e - s +1 )) -1 ) << s; }
Signature · 1 line
1.9.1
RFC PATCH 2/2 kvm: x86: mmio: fix setting the present bit of mmio spte
In PAE case maxphyaddr may be 52bit as well, we also need to disable mmio page fault. Here we can check MMIO_SPTE_GEN_HIGH_SHIFT directly to determine if we should set the present bit, and bring a little cleanup.
--- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/mmu.c | 23 +++++++++++++++++++++++ arch/x86/kvm/x86.c | 30 ------------------------------ 3 files changed, 24 insertions(+), 30 deletions(-)
arch/x86/include/asm/kvm_host.h +1 -0
@@ -809,6 +809,7 @@ void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,struct kvm_memory_slot *slot, gfn_t gfn_offset, unsigned long mask);void kvm_mmu_zap_all (struct kvm *kvm); +void kvm_set_mmio_spte_mask (void );void kvm_mmu_invalidate_mmio_sptes (struct kvm *kvm);unsigned int kvm_mmu_calculate_mmu_pages (struct kvm *kvm);void kvm_mmu_change_mmu_pages (struct kvm *kvm,unsigned int kvm_nr_mmu_pages);
arch/x86/kvm/mmu.c +23 -0
@@ -295,6 +295,29 @@ static bool check_mmio_spte(struct kvm *kvm, u64 spte)return likely (kvm_gen == spte_gen); } +/* +* Set the reserved bits and the present bit of an paging-structure +* entry to generate page fault with PFER.RSV = 1. +*/ +void kvm_set_mmio_spte_mask (void ) +{ +u64 mask; +int maxphyaddr = boot_cpu_data.x86_phys_bits; + +/* Mask the reserved physical address bits. */ + mask =rsvd_bits (maxphyaddr,MMIO_SPTE_GEN_HIGH_SHIFT -1 ); + +/* Magic bits are always reserved for 32bit host. */ + mask |=0x3ull <<62 ; + +/* Set the present bit to enable mmio page fault. */ +if (maxphyaddr <MMIO_SPTE_GEN_HIGH_SHIFT ) + mask =PT_PRESENT_MASK ; + +kvm_mmu_set_mmio_spte_mask (mask); +} +EXPORT_SYMBOL_GPL (kvm_set_mmio_spte_mask); +void kvm_mmu_set_mask_ptes (u64 user_mask,u64 accessed_mask,u64 dirty_mask,u64 nx_mask,u64 x_mask) {
arch/x86/kvm/x86.c +0 -30
@@ -5596,36 +5596,6 @@ void kvm_after_handle_nmi(struct kvm_vcpu *vcpu) }EXPORT_SYMBOL_GPL (kvm_after_handle_nmi); -static void kvm_set_mmio_spte_mask (void ) -{ -u64 mask; -int maxphyaddr = boot_cpu_data.x86_phys_bits; - -/* -* Set the reserved bits and the present bit of an paging-structure -* entry to generate page fault with PFER.RSV = 1. -*/ -/* Mask the reserved physical address bits. */ - mask =rsvd_bits (maxphyaddr,51 ); - -/* Bit 62 is always reserved for 32bit host. */ - mask |=0x3ull <<62 ; - -/* Set the present bit. */ - mask |=1ull ; - -#ifdef CONFIG_X86_64 -/* -* If reserved bit is not supported, clear the present bit to disable -* mmio page fault. -*/ -if (maxphyaddr ==52 ) - mask &= ~1ull ; -#endif - -kvm_mmu_set_mmio_spte_mask (mask); -} -#ifdef CONFIG_X86_64 static void pvclock_gtod_update_fn (struct work_struct *work) {
Signature · 1 line
1.9.1
Paolo Bonzini
maintainerRe: [RFC][PATCH 2/2] kvm: x86: mmio: fix setting the present bit of mmio spte
Quoted 36 lines · Tiejun Chen
In PAE case maxphyaddr may be 52bit as well, we also need to disable mmio page fault. Here we can check
MMIO_SPTE_GEN_HIGH_SHIFTdirectly to determine if we should set the present bit, and bring a little cleanup.Signed-off-by Tiejun Chen <[email protected]>--- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/mmu.c | 23 +++++++++++++++++++++++ arch/x86/kvm/x86.c | 30 ------------------------------ 3 files changed, 24 insertions(+), 30 deletions(-)arch/x86/include/asm/kvm_host.h +1
-0@@ -809,6 +809,7 @@ void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,struct kvm_memory_slot *slot, gfn_t gfn_offset, unsigned long mask);void kvm_mmu_zap_all (struct kvm *kvm); +void kvm_set_mmio_spte_mask (void );void kvm_mmu_invalidate_mmio_sptes (struct kvm *kvm);unsigned int kvm_mmu_calculate_mmu_pages (struct kvm *kvm);void kvm_mmu_change_mmu_pages (struct kvm *kvm,unsigned int kvm_nr_mmu_pages);arch/x86/kvm/mmu.c +4
-0@@ -295,6 +295,29 @@ static bool check_mmio_spte(struct kvm *kvm, u64 spte)return likely (kvm_gen == spte_gen); } +/* +* Set the reserved bits and the present bit of an paging-structure +* entry to generate page fault with PFER.RSV = 1. +*/
arch/x86/kvm/mmu.c static bool check_mmio_spte(struct kvm *kvm, u64 spte) · line 311
Quoted 10 lines
arch/x86/kvm/mmu.c +10
-0+void kvm_set_mmio_spte_mask (void ) +{ +u64 mask; +int maxphyaddr = boot_cpu_data.x86_phys_bits; + +/* Mask the reserved physical address bits. */ + mask =rsvd_bits (maxphyaddr,MMIO_SPTE_GEN_HIGH_SHIFT -1 ); + +/* Magic bits are always reserved for 32bit host. */ + mask |=0x3ull <<62 ;
This should be enough to trigger the page fault on PAE systems.
The problem is specific to non-EPT 64-bit hosts, where the PTEs have no reserved bits beyond 51:MAXPHYADDR. On EPT we use WX- permissions to trigger EPT misconfig, on 32-bit systems we have bit 62.
arch/x86/kvm/mmu.c static bool check_mmio_spte(struct kvm *kvm, u64 spte) · line 315
Quoted 3 lines
arch/x86/kvm/mmu.c +3
-0+/* Set the present bit to enable mmio page fault. */ +if (maxphyaddr <MMIO_SPTE_GEN_HIGH_SHIFT ) + mask =PT_PRESENT_MASK ;
Shouldn't this be "|=" anyway, instead of "="?
Paolo
Quoted 50 lines
arch/x86/kvm/mmu.c +5
-0+ +kvm_mmu_set_mmio_spte_mask (mask); +} +EXPORT_SYMBOL_GPL (kvm_set_mmio_spte_mask); +void kvm_mmu_set_mask_ptes (u64 user_mask,u64 accessed_mask,u64 dirty_mask,u64 nx_mask,u64 x_mask) {arch/x86/kvm/x86.c +0
-30@@ -5596,36 +5596,6 @@ void kvm_after_handle_nmi(struct kvm_vcpu *vcpu) }EXPORT_SYMBOL_GPL (kvm_after_handle_nmi); -static void kvm_set_mmio_spte_mask (void ) -{ -u64 mask; -int maxphyaddr = boot_cpu_data.x86_phys_bits; - -/* -* Set the reserved bits and the present bit of an paging-structure -* entry to generate page fault with PFER.RSV = 1. -*/ -/* Mask the reserved physical address bits. */ - mask =rsvd_bits (maxphyaddr,51 ); - -/* Bit 62 is always reserved for 32bit host. */ - mask |=0x3ull <<62 ; - -/* Set the present bit. */ - mask |=1ull ; - -#ifdef CONFIG_X86_64 -/* -* If reserved bit is not supported, clear the present bit to disable -* mmio page fault. -*/ -if (maxphyaddr ==52 ) - mask &= ~1ull ; -#endif - -kvm_mmu_set_mmio_spte_mask (mask); -} -#ifdef CONFIG_X86_64 static void pvclock_gtod_update_fn (struct work_struct *work) {
Re: [RFC][PATCH 2/2] kvm: x86: mmio: fix setting the present bit of mmio spte
Quoted 52 lines · Paolo Bonzini
Quoted 36 lines · Tiejun Chen
In PAE case maxphyaddr may be 52bit as well, we also need to disable mmio page fault. Here we can check
MMIO_SPTE_GEN_HIGH_SHIFTdirectly to determine if we should set the present bit, and bring a little cleanup.Signed-off-by Tiejun Chen <[email protected]>--- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/mmu.c | 23 +++++++++++++++++++++++ arch/x86/kvm/x86.c | 30 ------------------------------ 3 files changed, 24 insertions(+), 30 deletions(-)arch/x86/include/asm/kvm_host.h +1
-0@@ -809,6 +809,7 @@ void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,struct kvm_memory_slot *slot, gfn_t gfn_offset, unsigned long mask);void kvm_mmu_zap_all (struct kvm *kvm); +void kvm_set_mmio_spte_mask (void );void kvm_mmu_invalidate_mmio_sptes (struct kvm *kvm);unsigned int kvm_mmu_calculate_mmu_pages (struct kvm *kvm);void kvm_mmu_change_mmu_pages (struct kvm *kvm,unsigned int kvm_nr_mmu_pages);arch/x86/kvm/mmu.c +4
-0@@ -295,6 +295,29 @@ static bool check_mmio_spte(struct kvm *kvm, u64 spte)return likely (kvm_gen == spte_gen); } +/* +* Set the reserved bits and the present bit of an paging-structure +* entry to generate page fault with PFER.RSV = 1. +*/ arch/x86/kvm/mmu.c static bool check_mmio_spte(struct kvm *kvm, u64 spte) · line 311
Quoted 10 lines
arch/x86/kvm/mmu.c +10
-0+void kvm_set_mmio_spte_mask (void ) +{ +u64 mask; +int maxphyaddr = boot_cpu_data.x86_phys_bits; + +/* Mask the reserved physical address bits. */ + mask =rsvd_bits (maxphyaddr,MMIO_SPTE_GEN_HIGH_SHIFT -1 ); + +/* Magic bits are always reserved for 32bit host. */ + mask |=0x3ull <<62 ;This should be enough to trigger the page fault on PAE systems.
Quoted 3 lines
The problem is specific to non-EPT 64-bit hosts, where the PTEs have no reserved bits beyond 51:MAXPHYADDR. On EPT we use WX- permissions to trigger EPT misconfig, on 32-bit systems we have bit 62.
Thanks for your explanation.
Quoted 7 lines
arch/x86/kvm/mmu.c static bool check_mmio_spte(struct kvm *kvm, u64 spte) · line 315
Quoted 4 lines
arch/x86/kvm/mmu.c +3
-0+/* Set the present bit to enable mmio page fault. */ +if (maxphyaddr <MMIO_SPTE_GEN_HIGH_SHIFT ) + mask =PT_PRESENT_MASK ;Shouldn't this be "|=" anyway, instead of "="?
Yeah, just miss this. Thanks a lot, I will fix this in next revision.
Thanks
Tiejun
Paolo Bonzini
maintainerQuoted 14 lines · Tiejun Chen
In some real scenarios 'start' may not be less than 'end' like maxphyaddr = 52.
Signed-off-by Tiejun Chen <[email protected]>--- arch/x86/kvm/mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/x86/kvm/mmu.h +0
-0
arch/x86/kvm/mmu.h line 65
Quoted 8 lines
arch/x86/kvm/mmu.h +2
-0@@ -58,6 +58,8 @@static inline u64 rsvd_bits (int s,int e) { +if (unlikely (s > e)) +return 0 ;return ((1ULL << (e - s +1 )) -1 ) << s; }
s == e + 1 is supported:
(1ULL << (e - (e + 1) + 1)) - 1) << s ==
(1ULL << 0) << s ==
0
Is there any case where s is even bigger?
Paolo
Quoted 28 lines · Paolo Bonzini
Quoted 14 lines · Tiejun Chen
In some real scenarios 'start' may not be less than 'end' like maxphyaddr = 52.
Signed-off-by Tiejun Chen <[email protected]>--- arch/x86/kvm/mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/x86/kvm/mmu.h +0
-0arch/x86/kvm/mmu.h line 65
Quoted 8 lines
arch/x86/kvm/mmu.h +2
-0@@ -58,6 +58,8 @@static inline u64 rsvd_bits (int s,int e) { +if (unlikely (s > e)) +return 0 ;return ((1ULL << (e - s +1 )) -1 ) << s; }s == e + 1 is supported:
Quoted 1 line
(1ULL << (e - (e + 1) + 1)) - 1) << s ==
(1ULL << (e - (e + 1) + 1)) - 1) << s
= (1ULL << (e - e - 1) + 1)) - 1) << s = (1ULL << (-1) + 1)) - 1) << s = (1ULL << (0) - 1) << s = (1ULL << (- 1) << s
Am I missing something?
Thanks
Tiejun
Quoted 11 lines
(1ULL << 0) << s == 0Is there any case where s is even bigger?
Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Paolo Bonzini
maintainerQuoted 29 lines · Tiejun
Quoted 28 lines · Paolo Bonzini
Quoted 14 lines · Tiejun Chen
In some real scenarios 'start' may not be less than 'end' like maxphyaddr = 52.
Signed-off-by Tiejun Chen <[email protected]>--- arch/x86/kvm/mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/x86/kvm/mmu.h +0
-0arch/x86/kvm/mmu.h line 65
Quoted 8 lines
arch/x86/kvm/mmu.h +2
-0@@ -58,6 +58,8 @@static inline u64 rsvd_bits (int s,int e) { +if (unlikely (s > e)) +return 0 ;return ((1ULL << (e - s +1 )) -1 ) << s; }s == e + 1 is supported:
Quoted 5 lines
Quoted 1 line
(1ULL << (e - (e + 1) + 1)) - 1) << s ==(1ULL << (e - (e + 1) + 1)) - 1) << s
= (1ULL << (e - e - 1) + 1)) - 1) << s = (1ULL << (-1) + 1)) - 1) << s
no,
((1ULL << (-1 + 1)) - 1) << s
Quoted 1 line
= (1ULL << (0) - 1) << s
((1ULL << (0)) - 1) << s
Quoted 1 line
= (1ULL << (- 1) << s
(1 - 1) << s
0 << sPaolo
Quoted 22 lines
Am I missing something?
Thanks
TiejunQuoted 11 lines
(1ULL << 0) << s == 0Is there any case where s is even bigger?
Paolo
Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.htmlSignature · 3 lines
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 32 lines · Paolo Bonzini
Quoted 29 lines · Tiejun
Quoted 28 lines · Paolo Bonzini
Quoted 14 lines · Tiejun Chen
In some real scenarios 'start' may not be less than 'end' like maxphyaddr = 52.
Signed-off-by Tiejun Chen <[email protected]>--- arch/x86/kvm/mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/x86/kvm/mmu.h +0
-0arch/x86/kvm/mmu.h line 65
Quoted 8 lines
arch/x86/kvm/mmu.h +2
-0@@ -58,6 +58,8 @@static inline u64 rsvd_bits (int s,int e) { +if (unlikely (s > e)) +return 0 ;return ((1ULL << (e - s +1 )) -1 ) << s; }s == e + 1 is supported:
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
(1ULL << (e - (e + 1) + 1)) - 1) << s ==(1ULL << (e - (e + 1) + 1)) - 1) << s
= (1ULL << (e - e - 1) + 1)) - 1) << s = (1ULL << (-1) + 1)) - 1) << sno,
You're right since I'm seeing "()" wrongly.
Sorry to bother you.
Thanks
Tiejun