On Wed, Sep 02, 2020 at 04:40:03PM +1000, Gavin Shan wrote:
> On 8/25/20 7:39 PM, Will Deacon wrote:
> > Introduce alloc() and free() functions to the generic page-table code
> > for guest stage-2 page-tables and plumb these into the existing KVM
> > page-table allocator. Subsequent patches will convert other operations
> > within the KVM allocator over to the generic code.
> >
> > Cc: Marc Zyngier <maz@xxxxxxxxxx>
> > Cc: Quentin Perret <qperret@xxxxxxxxxx>
> > Signed-off-by: Will Deacon <will@xxxxxxxxxx>
> > ---
> > arch/arm64/include/asm/kvm_host.h | 1 +
> > arch/arm64/include/asm/kvm_pgtable.h | 18 +++++++++
> > arch/arm64/kvm/hyp/pgtable.c | 51 ++++++++++++++++++++++++++
> > arch/arm64/kvm/mmu.c | 55 +++++++++++++++-------------
> > 4 files changed, 99 insertions(+), 26 deletions(-)
> >
>
> With the following one question resolved:
>
> Reviewed-by: Gavin Shan <gshan@xxxxxxxxxx>
Thanks!
> > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> > index fabd72b0c8a4..4607e9ca60a2 100644
> > --- a/arch/arm64/kvm/mmu.c
> > +++ b/arch/arm64/kvm/mmu.c
> > @@ -668,47 +668,49 @@ int create_hyp_exec_mappings(phys_addr_t phys_addr, size_t size,
> > * @kvm: The pointer to the KVM structure
> > * @mmu: The pointer to the s2 MMU structure
> > *
> > - * Allocates only the stage-2 HW PGD level table(s) of size defined by
> > - * stage2_pgd_size(mmu->kvm).
> > - *
> > + * Allocates only the stage-2 HW PGD level table(s).
> > * Note we don't need locking here as this is only called when the VM is
> > * created, which can only be done once.
> > */
> > int kvm_init_stage2_mmu(struct kvm *kvm, struct kvm_s2_mmu *mmu)
> > {
> > - phys_addr_t pgd_phys;
> > - pgd_t *pgd;
> > - int cpu;
> > + int cpu, err;
> > + struct kvm_pgtable *pgt;
> > - if (mmu->pgd != NULL) {
> > + if (mmu->pgt != NULL) {
> > kvm_err("kvm_arch already initialized?\n");
> > return -EINVAL;
> > }
> > - /* Allocate the HW PGD, making sure that each page gets its own refcount */
> > - pgd = alloc_pages_exact(stage2_pgd_size(kvm), GFP_KERNEL | __GFP_ZERO);
> > - if (!pgd)
> > + pgt = kzalloc(sizeof(*pgt), GFP_KERNEL);
> > + if (!pgt)
> > return -ENOMEM;
> > - pgd_phys = virt_to_phys(pgd);
> > - if (WARN_ON(pgd_phys & ~kvm_vttbr_baddr_mask(kvm)))
> > - return -EINVAL;
> > + err = kvm_pgtable_stage2_init(pgt, kvm);
> > + if (err)
> > + goto out_free_pgtable;
> > mmu->last_vcpu_ran = alloc_percpu(typeof(*mmu->last_vcpu_ran));
> > if (!mmu->last_vcpu_ran) {
> > - free_pages_exact(pgd, stage2_pgd_size(kvm));
> > - return -ENOMEM;
> > + err = -ENOMEM;
> > + goto out_destroy_pgtable;
> > }
> > for_each_possible_cpu(cpu)
> > *per_cpu_ptr(mmu->last_vcpu_ran, cpu) = -1;
> > mmu->kvm = kvm;
> > - mmu->pgd = pgd;
> > - mmu->pgd_phys = pgd_phys;
> > + mmu->pgt = pgt;
> > + mmu->pgd_phys = __pa(pgt->pgd);
> > + mmu->pgd = (void *)pgt->pgd;
> > mmu->vmid.vmid_gen = 0;
> > -
> > return 0;
> > +
> > +out_destroy_pgtable:
> > + kvm_pgtable_stage2_destroy(pgt);
> > +out_free_pgtable:
> > + kfree(pgt);
> > + return err;
> > }
> >
>
> kvm_pgtable_stage2_destroy() might not needed here because
> the stage2 page pgtable is empty so far. However, it should
> be rare to hit the case. If I'm correct, what we need to do
> is just freeing the PGDs.
Right, but kvm_pgtable_stage2_destroy() also frees the PGDs because it
knows how many pages there are and they were allocated by
kvm_pgtable_stage2_init().
Will
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
