Hi Andrew, On 07/05/2020 16:13, Andrew Scull wrote: >> @@ -176,7 +177,7 @@ static void clear_stage2_pud_entry(struct kvm_s2_mmu *mmu, pud_t *pud, phys_addr >> pmd_t *pmd_table __maybe_unused = stage2_pmd_offset(kvm, pud, 0); >> VM_BUG_ON(stage2_pud_huge(kvm, *pud)); >> stage2_pud_clear(kvm, pud); >> - kvm_tlb_flush_vmid_ipa(mmu, addr); >> + kvm_tlb_flush_vmid_ipa(mmu, addr, S2_NO_LEVEL_HINT); >> stage2_pmd_free(kvm, pmd_table); >> put_page(virt_to_page(pud)); >> } >> @@ -186,7 +187,7 @@ static void clear_stage2_pmd_entry(struct kvm_s2_mmu *mmu, pmd_t *pmd, phys_addr >> pte_t *pte_table = pte_offset_kernel(pmd, 0); >> VM_BUG_ON(pmd_thp_or_huge(*pmd)); >> pmd_clear(pmd); >> - kvm_tlb_flush_vmid_ipa(mmu, addr); >> + kvm_tlb_flush_vmid_ipa(mmu, addr, S2_NO_LEVEL_HINT); >> free_page((unsigned long)pte_table); >> put_page(virt_to_page(pmd)); >> } > > Going by the names, is it possible to give a better level hint for these > cases? There is no leaf entry being invalidated here. After clearing the range, we found we'd emptied (and invalidated) a whole page of mappings: | if (stage2_pmd_table_empty(kvm, start_pmd)) | clear_stage2_pud_entry(mmu, pud, start_addr); Now we want to remove the link to the empty page so we can free it. We are changing the structure of the tables, not what gets mapped. I think this is why we need the un-hinted behaviour, to invalidate "any level of the translation table walk required to translate the specified IPA". Otherwise the hardware can look for a leaf at the indicated level, find none, and do nothing. This is sufficiently horrible, its possible I've got it completely wrong! (does it make sense?) Thanks, James
