On Mon, Mar 13, 2023, David Matlack wrote:
On Mon, Mar 13, 2023 at 8:53 AM Sean Christopherson seanjc@google.com wrote:
+David
On Mon, Mar 13, 2023, Marc Zyngier wrote:
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c index 7113587222ff..d7b8b25942df 100644 --- a/arch/arm64/kvm/mmu.c +++ b/arch/arm64/kvm/mmu.c @@ -666,14 +666,23 @@ static int get_user_mapping_size(struct kvm *kvm, u64 addr) CONFIG_PGTABLE_LEVELS), .mm_ops = &kvm_user_mm_ops, };
unsigned long flags; kvm_pte_t pte = 0; /* Keep GCC quiet... */ u32 level = ~0; int ret;/** Disable IRQs so that we hazard against a concurrent* teardown of the userspace page tables (which relies on* IPI-ing threads).*/local_irq_save(flags); ret = kvm_pgtable_get_leaf(&pgt, addr, &pte, &level);
VM_BUG_ON(ret);VM_BUG_ON(level >= KVM_PGTABLE_MAX_LEVELS);VM_BUG_ON(!(pte & PTE_VALID));
local_irq_restore(flags);/* Oops, the userspace PTs are gone... */if (ret || level >= KVM_PGTABLE_MAX_LEVELS || !(pte & PTE_VALID))return -EFAULT;I don't think this should return -EFAULT all the way out to userspace. Unless arm64 differs from x86 in terms of how the userspace page tables are managed, not having a valid translation _right now_ doesn't mean that one can't be created in the future, e.g. by way of a subsequent hva_to_pfn().
FWIW, the approach x86 takes is to install a 4KiB (smallest granuale) translation,
If I'm reading the ARM code correctly, returning -EFAULT here will have that effect. get_user_mapping_size() is only called by transparent_hugepage_adjust() which returns PAGE_SIZE if get_user_mapping_size() returns anything less than PMD_SIZE.
No, this patch adds
+ int sz = get_user_mapping_size(kvm, hva); + + if (sz < 0) + return sz; + + if (sz < PMD_SIZE) + return PAGE_SIZE; +
and
vma_pagesize = transparent_hugepage_adjust(kvm, memslot, hva, &pfn, &fault_ipa); + + if (vma_pagesize < 0) { + ret = vma_pagesize; + goto out_unlock; + }