On Mon, Mar 13, 2023 at 10:21 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > On Mon, Mar 13, 2023, David Matlack wrote: > > On Mon, Mar 13, 2023 at 8:53 AM Sean Christopherson <seanjc@xxxxxxxxxx> 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; > + Gah, I just looked at the trimmed patch in the reply. Thanks for pointing that out.
