KVM: arm64: Pass level hint to TLBI during stage-2 permission fault
Will Deacon
maintainerPATCH 1/1 KVM: arm64: Pass level hint to TLBI during stage-2 permission fault
Alex pointed out that we don't pass a level hint to the TLBI instruction when handling a stage-2 permission fault, even though the walker does at some point have the level information in its hands.
Rework stage2_update_leaf_attrs() so that it can optionally return the level of the updated pte to its caller, which can in turn be used to provide the correct TLBI level hint.
Cc Marc Zyngier <[email protected]>
Reported-by Alexandru Elisei <[email protected]>
Signed-off-by Will Deacon <[email protected]>
--- arch/arm64/kvm/hyp/pgtable.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)
arch/arm64/kvm/hyp/pgtable.c +16 -7
@@ -694,6 +694,7 @@ struct stage2_attr_data {kvm_pte_t attr_set;kvm_pte_t attr_clr;kvm_pte_t pte; +u32 level; }; static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
@@ -706,6 +707,7 @@ static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,if (!kvm_pte_valid (pte))return 0 ; + data->level = level; data->pte = pte; pte &= ~data->attr_clr; pte |= data->attr_set;
@@ -723,7 +725,8 @@ static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,static int stage2_update_leaf_attrs (struct kvm_pgtable *pgt,u64 addr,u64 size,kvm_pte_t attr_set, -kvm_pte_t attr_clr,kvm_pte_t *orig_pte) +kvm_pte_t attr_clr,kvm_pte_t *orig_pte, +u32 *level) {int ret;kvm_pte_t attr_mask =KVM_PTE_LEAF_ATTR_LO |KVM_PTE_LEAF_ATTR_HI ;
@@ -743,20 +746,24 @@ static int stage2_update_leaf_attrs(struct kvm_pgtable *pgt, u64 addr,if (orig_pte) *orig_pte = data.pte; + +if (level) + *level = data.level;return 0 ; }int kvm_pgtable_stage2_wrprotect (struct kvm_pgtable *pgt,u64 addr,u64 size) {return stage2_update_leaf_attrs (pgt, addr, size,0 , -KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W ,NULL ); +KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W , +NULL ,NULL ); }kvm_pte_t kvm_pgtable_stage2_mkyoung (struct kvm_pgtable *pgt,u64 addr) {kvm_pte_t pte =0 ;stage2_update_leaf_attrs (pgt, addr,1 ,KVM_PTE_LEAF_ATTR_LO_S2_AF ,0 , - &pte); + &pte,NULL );dsb (ishst);return pte; }
@@ -765,7 +772,7 @@ kvm_pte_t kvm_pgtable_stage2_mkold(struct kvm_pgtable *pgt, u64 addr) {kvm_pte_t pte =0 ;stage2_update_leaf_attrs (pgt, addr,1 ,0 ,KVM_PTE_LEAF_ATTR_LO_S2_AF , - &pte); + &pte,NULL );/* * "But where's the TLBI?!", you scream. * "Over in the core code", I sigh.
@@ -778,7 +785,7 @@ kvm_pte_t kvm_pgtable_stage2_mkold(struct kvm_pgtable *pgt, u64 addr)bool kvm_pgtable_stage2_is_young (struct kvm_pgtable *pgt,u64 addr) {kvm_pte_t pte =0 ; -stage2_update_leaf_attrs (pgt, addr,1 ,0 ,0 , &pte); +stage2_update_leaf_attrs (pgt, addr,1 ,0 ,0 , &pte,NULL );return pte &KVM_PTE_LEAF_ATTR_LO_S2_AF ; }
@@ -786,6 +793,7 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr, enum kvm_pgtable_prot prot) {int ret; +u32 level;kvm_pte_t set =0 , clr =0 ;if (prot &KVM_PGTABLE_PROT_R )
@@ -797,8 +805,9 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr,if (prot &KVM_PGTABLE_PROT_X ) clr |=KVM_PTE_LEAF_ATTR_HI_S2_XN ; - ret =stage2_update_leaf_attrs (pgt, addr,1 , set, clr,NULL ); -kvm_call_hyp (__kvm_tlb_flush_vmid_ipa, pgt->mmu, addr,0 ); + ret =stage2_update_leaf_attrs (pgt, addr,1 , set, clr,NULL , &level); +if (!ret) +kvm_call_hyp (__kvm_tlb_flush_vmid_ipa, pgt->mmu, addr, level);return ret; }
Signature · 6 lines
2.28.0.709.gb0816b6eb0-goog _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Quoted 8 lines · Will Deacon
Alex pointed out that we don't pass a level hint to the TLBI instruction when handling a stage-2 permission fault, even though the walker does at some point have the level information in its hands.
Rework
stage2_update_leaf_attrs()so that it can optionally return the level of the updated pte to its caller, which can in turn be used to provide the correct TLBI level hint.
Quoted 8 lines
Cc Marc Zyngier <[email protected]>Reported-by Alexandru Elisei <[email protected]>Signed-off-by Will Deacon <[email protected]>--- arch/arm64/kvm/hyp/pgtable.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)
Reviewed-by Gavin Shan <[email protected]>
Quoted 86 lines
arch/arm64/kvm/hyp/pgtable.c +13
-5@@ -694,6 +694,7 @@ struct stage2_attr_data {kvm_pte_t attr_set;kvm_pte_t attr_clr;kvm_pte_t pte; +u32 level; }; static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,@@ -706,6 +707,7 @@ static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,if (!kvm_pte_valid (pte))return 0 ; + data->level = level; data->pte = pte; pte &= ~data->attr_clr; pte |= data->attr_set;@@ -723,7 +725,8 @@ static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,static int stage2_update_leaf_attrs (struct kvm_pgtable *pgt,u64 addr,u64 size,kvm_pte_t attr_set, -kvm_pte_t attr_clr,kvm_pte_t *orig_pte) +kvm_pte_t attr_clr,kvm_pte_t *orig_pte, +u32 *level) {int ret;kvm_pte_t attr_mask =KVM_PTE_LEAF_ATTR_LO |KVM_PTE_LEAF_ATTR_HI ;@@ -743,20 +746,24 @@ static int stage2_update_leaf_attrs(struct kvm_pgtable *pgt, u64 addr,if (orig_pte) *orig_pte = data.pte; + +if (level) + *level = data.level;return 0 ; }int kvm_pgtable_stage2_wrprotect (struct kvm_pgtable *pgt,u64 addr,u64 size) {return stage2_update_leaf_attrs (pgt, addr, size,0 , -KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W ,NULL ); +KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W , +NULL ,NULL ); }kvm_pte_t kvm_pgtable_stage2_mkyoung (struct kvm_pgtable *pgt,u64 addr) {kvm_pte_t pte =0 ;stage2_update_leaf_attrs (pgt, addr,1 ,KVM_PTE_LEAF_ATTR_LO_S2_AF ,0 , - &pte); + &pte,NULL );dsb (ishst);return pte; }@@ -765,7 +772,7 @@ kvm_pte_t kvm_pgtable_stage2_mkold(struct kvm_pgtable *pgt, u64 addr) {kvm_pte_t pte =0 ;stage2_update_leaf_attrs (pgt, addr,1 ,0 ,KVM_PTE_LEAF_ATTR_LO_S2_AF , - &pte); + &pte,NULL );/* * "But where's the TLBI?!", you scream. * "Over in the core code", I sigh. @@ -778,7 +785,7 @@ kvm_pte_t kvm_pgtable_stage2_mkold(struct kvm_pgtable *pgt, u64 addr)bool kvm_pgtable_stage2_is_young (struct kvm_pgtable *pgt,u64 addr) {kvm_pte_t pte =0 ; -stage2_update_leaf_attrs (pgt, addr,1 ,0 ,0 , &pte); +stage2_update_leaf_attrs (pgt, addr,1 ,0 ,0 , &pte,NULL );return pte &KVM_PTE_LEAF_ATTR_LO_S2_AF ; }@@ -786,6 +793,7 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr, enum kvm_pgtable_prot prot) {int ret; +u32 level;kvm_pte_t set =0 , clr =0 ;if (prot &KVM_PGTABLE_PROT_R )@@ -797,8 +805,9 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr,if (prot &KVM_PGTABLE_PROT_X )
arch/arm64/kvm/hyp/pgtable.c int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr, · line 813
Quoted 10 lines
arch/arm64/kvm/hyp/pgtable.c +3
-2clr |=KVM_PTE_LEAF_ATTR_HI_S2_XN ; - ret =stage2_update_leaf_attrs (pgt, addr,1 , set, clr,NULL ); -kvm_call_hyp (__kvm_tlb_flush_vmid_ipa, pgt->mmu, addr,0 ); + ret =stage2_update_leaf_attrs (pgt, addr,1 , set, clr,NULL , &level); +if (!ret) +kvm_call_hyp (__kvm_tlb_flush_vmid_ipa, pgt->mmu, addr, level);return ret; }
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Hi,
Quoted 7 lines · Will Deacon
Alex pointed out that we don't pass a level hint to the TLBI instruction when handling a stage-2 permission fault, even though the walker does at some point have the level information in its hands.
Rework
stage2_update_leaf_attrs()so that it can optionally return the level of the updated pte to its caller, which can in turn be used to provide the correct TLBI level hint.
Looks correct to me. I checked and all the functions that don't do a tlbi pass NULL as the level pointer, and the level parameter is always set by the walker algorithm (if level had an invalid value, no invalidation would have been performed):
Reviewed-by Alexandru Elisei <[email protected]>
Thanks,
Alex
Quoted 94 lines
Cc Marc Zyngier <[email protected]>Reported-by Alexandru Elisei <[email protected]>Signed-off-by Will Deacon <[email protected]>--- arch/arm64/kvm/hyp/pgtable.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)arch/arm64/kvm/hyp/pgtable.c +13
-5@@ -694,6 +694,7 @@ struct stage2_attr_data {kvm_pte_t attr_set;kvm_pte_t attr_clr;kvm_pte_t pte; +u32 level; }; static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,@@ -706,6 +707,7 @@ static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,if (!kvm_pte_valid (pte))return 0 ; + data->level = level; data->pte = pte; pte &= ~data->attr_clr; pte |= data->attr_set;@@ -723,7 +725,8 @@ static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,static int stage2_update_leaf_attrs (struct kvm_pgtable *pgt,u64 addr,u64 size,kvm_pte_t attr_set, -kvm_pte_t attr_clr,kvm_pte_t *orig_pte) +kvm_pte_t attr_clr,kvm_pte_t *orig_pte, +u32 *level) {int ret;kvm_pte_t attr_mask =KVM_PTE_LEAF_ATTR_LO |KVM_PTE_LEAF_ATTR_HI ;@@ -743,20 +746,24 @@ static int stage2_update_leaf_attrs(struct kvm_pgtable *pgt, u64 addr,if (orig_pte) *orig_pte = data.pte; + +if (level) + *level = data.level;return 0 ; }int kvm_pgtable_stage2_wrprotect (struct kvm_pgtable *pgt,u64 addr,u64 size) {return stage2_update_leaf_attrs (pgt, addr, size,0 , -KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W ,NULL ); +KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W , +NULL ,NULL ); }kvm_pte_t kvm_pgtable_stage2_mkyoung (struct kvm_pgtable *pgt,u64 addr) {kvm_pte_t pte =0 ;stage2_update_leaf_attrs (pgt, addr,1 ,KVM_PTE_LEAF_ATTR_LO_S2_AF ,0 , - &pte); + &pte,NULL );dsb (ishst);return pte; }@@ -765,7 +772,7 @@ kvm_pte_t kvm_pgtable_stage2_mkold(struct kvm_pgtable *pgt, u64 addr) {kvm_pte_t pte =0 ;stage2_update_leaf_attrs (pgt, addr,1 ,0 ,KVM_PTE_LEAF_ATTR_LO_S2_AF , - &pte); + &pte,NULL );/* * "But where's the TLBI?!", you scream. * "Over in the core code", I sigh. @@ -778,7 +785,7 @@ kvm_pte_t kvm_pgtable_stage2_mkold(struct kvm_pgtable *pgt, u64 addr)bool kvm_pgtable_stage2_is_young (struct kvm_pgtable *pgt,u64 addr) {kvm_pte_t pte =0 ; -stage2_update_leaf_attrs (pgt, addr,1 ,0 ,0 , &pte); +stage2_update_leaf_attrs (pgt, addr,1 ,0 ,0 , &pte,NULL );return pte &KVM_PTE_LEAF_ATTR_LO_S2_AF ; }@@ -786,6 +793,7 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr, enum kvm_pgtable_prot prot) {int ret; +u32 level;kvm_pte_t set =0 , clr =0 ;if (prot &KVM_PGTABLE_PROT_R )@@ -797,8 +805,9 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr,if (prot &KVM_PGTABLE_PROT_X )
arch/arm64/kvm/hyp/pgtable.c int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr, · line 813
Quoted 10 lines
arch/arm64/kvm/hyp/pgtable.c +3
-2clr |=KVM_PTE_LEAF_ATTR_HI_S2_XN ; - ret =stage2_update_leaf_attrs (pgt, addr,1 , set, clr,NULL ); -kvm_call_hyp (__kvm_tlb_flush_vmid_ipa, pgt->mmu, addr,0 ); + ret =stage2_update_leaf_attrs (pgt, addr,1 , set, clr,NULL , &level); +if (!ret) +kvm_call_hyp (__kvm_tlb_flush_vmid_ipa, pgt->mmu, addr, level);return ret; }
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Quoted 7 lines · Will Deacon
Alex pointed out that we don't pass a level hint to the TLBI instruction when handling a stage-2 permission fault, even though the walker does at some point have the level information in its hands.
Rework
stage2_update_leaf_attrs()so that it can optionally return the level of the updated pte to its caller, which can in turn be used to provide the correct TLBI level hint.
Applied to next, thanks!
[1/1] KVM: arm64: Pass level hint to TLBI during stage-2 permission fault
commit: b259d137e91d80bf92eac453ffab179eb7941ede
Cheers,
M.
Signature · 7 lines
Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm