| Message ID | 20211122095803.28943-2-gankulkarni@os.amperecomputing.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: nv: Fix issue with Stage 2 MMU init for Nested case. | expand |
[+ Quentin] Hi Ganapatro, On Mon, 22 Nov 2021 09:58:02 +0000, Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> wrote: > > The kvm_pgtable_stage2_init/kvm_pgtable_stage2_init_flags function > assume arch->mmu is same across all stage 2 mmu and initializes > the pgt(page table) using arch->mmu. > Using armc->mmu is not appropriate when nested virtualization is enabled > since there are multiple stage 2 mmu tables are initialized to manage > Guest-Hypervisor as well as Nested VM for the same vCPU. > > Add a mmu argument to kvm_pgtable_stage2_init that can be used during > initialization. This patch is a preparatory patch for the > nested virtualization series and no functional changes. Thanks for having had a look, and for the analysis. This is obviously a result of a hasty conversion to the 'new' page table code, and a total oversight on my part. I'm however not particularly thrilled with the approach you have taken though, as carrying both the kvm->arch pointer *and* the mmu pointer seems totally redundant (the mmu structure already has a backpointer to kvm->arch or its pkvm equivalent). All we need is to rework the initialisation for this pointer to be correct at the point of where we follow it first. I've pushed out my own version of this[1]. Please have a look. Thanks, M. [1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/commit/?h=kvm-arm64/nv-5.16-WIP&id=21790a24d88c3ed37989533709dad3d40905f5c3
Hi Marc, On 25-11-2021 07:19 pm, Marc Zyngier wrote: > [+ Quentin] > > Hi Ganapatro, > > On Mon, 22 Nov 2021 09:58:02 +0000, > Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> wrote: >> >> The kvm_pgtable_stage2_init/kvm_pgtable_stage2_init_flags function >> assume arch->mmu is same across all stage 2 mmu and initializes >> the pgt(page table) using arch->mmu. >> Using armc->mmu is not appropriate when nested virtualization is enabled >> since there are multiple stage 2 mmu tables are initialized to manage >> Guest-Hypervisor as well as Nested VM for the same vCPU. >> >> Add a mmu argument to kvm_pgtable_stage2_init that can be used during >> initialization. This patch is a preparatory patch for the >> nested virtualization series and no functional changes. > > Thanks for having had a look, and for the analysis. This is obviously > a result of a hasty conversion to the 'new' page table code, and a > total oversight on my part. > > I'm however not particularly thrilled with the approach you have taken > though, as carrying both the kvm->arch pointer *and* the mmu pointer > seems totally redundant (the mmu structure already has a backpointer > to kvm->arch or its pkvm equivalent). All we need is to rework the > initialisation for this pointer to be correct at the point of where we > follow it first. > > I've pushed out my own version of this[1]. Please have a look. > > Thanks, > > M. > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/commit/?h=kvm-arm64/nv-5.16-WIP&id=21790a24d88c3ed37989533709dad3d40905f5c3 > Thanks for the rework and rebasing to 5.16. I went through the patch, the gist of the patch seems to me same. Please free feel to add, Reviewed-by: Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> Looks like kvm-arm64/nv-5.16-WIP branch is broken for NV. I tried booting Guest hypervisor using lkvm and the vcpu init from lkvm is failing(Fatal: Unable to initialise vcpu). Did not dig/debug more in to the issue yet. Thanks, Ganapat
Hi Ganapatro, On Fri, 26 Nov 2021 05:45:26 +0000, Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> wrote: > > Hi Marc, > > > On 25-11-2021 07:19 pm, Marc Zyngier wrote: > > [+ Quentin] > > > > Hi Ganapatro, > > > > On Mon, 22 Nov 2021 09:58:02 +0000, > > Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> wrote: > >> > >> The kvm_pgtable_stage2_init/kvm_pgtable_stage2_init_flags function > >> assume arch->mmu is same across all stage 2 mmu and initializes > >> the pgt(page table) using arch->mmu. > >> Using armc->mmu is not appropriate when nested virtualization is enabled > >> since there are multiple stage 2 mmu tables are initialized to manage > >> Guest-Hypervisor as well as Nested VM for the same vCPU. > >> > >> Add a mmu argument to kvm_pgtable_stage2_init that can be used during > >> initialization. This patch is a preparatory patch for the > >> nested virtualization series and no functional changes. > > > > Thanks for having had a look, and for the analysis. This is obviously > > a result of a hasty conversion to the 'new' page table code, and a > > total oversight on my part. > > > > I'm however not particularly thrilled with the approach you have taken > > though, as carrying both the kvm->arch pointer *and* the mmu pointer > > seems totally redundant (the mmu structure already has a backpointer > > to kvm->arch or its pkvm equivalent). All we need is to rework the > > initialisation for this pointer to be correct at the point of where we > > follow it first. > > > > I've pushed out my own version of this[1]. Please have a look. > > > > Thanks, > > > > M. > > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/commit/?h=kvm-arm64/nv-5.16-WIP&id=21790a24d88c3ed37989533709dad3d40905f5c3 > > > > Thanks for the rework and rebasing to 5.16. > > I went through the patch, the gist of the patch seems to me same. > Please free feel to add, > Reviewed-by: Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> Thanks! > Looks like kvm-arm64/nv-5.16-WIP branch is broken for NV. > I tried booting Guest hypervisor using lkvm and the vcpu init from > lkvm is failing(Fatal: Unable to initialise vcpu). Did not dig/debug > more in to the issue yet. I'm still trying to iron a few issues, but you should be able to boot a NV guest. However, the way it is enabled has changed: you need to pass 'kvm-arm.mode=nested' to the command line instead of the previous 'kvm-arm.nested=1' which I have got rid of. That could well be the issue. With the current state of the tree (I just pushed another fix), you should be able to boot a L1 guest hypervisor and a L2 guest. I'm getting a crash at the point where the L2 guest reaches userspace though, so something is broken in the PSTATE or ERET tracking, I'd expect. M.
On 26-11-2021 04:20 pm, Marc Zyngier wrote: > Hi Ganapatro, > > On Fri, 26 Nov 2021 05:45:26 +0000, > Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> wrote: >> >> Hi Marc, >> >> >> On 25-11-2021 07:19 pm, Marc Zyngier wrote: >>> [+ Quentin] >>> >>> Hi Ganapatro, >>> >>> On Mon, 22 Nov 2021 09:58:02 +0000, >>> Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> wrote: >>>> >>>> The kvm_pgtable_stage2_init/kvm_pgtable_stage2_init_flags function >>>> assume arch->mmu is same across all stage 2 mmu and initializes >>>> the pgt(page table) using arch->mmu. >>>> Using armc->mmu is not appropriate when nested virtualization is enabled >>>> since there are multiple stage 2 mmu tables are initialized to manage >>>> Guest-Hypervisor as well as Nested VM for the same vCPU. >>>> >>>> Add a mmu argument to kvm_pgtable_stage2_init that can be used during >>>> initialization. This patch is a preparatory patch for the >>>> nested virtualization series and no functional changes. >>> >>> Thanks for having had a look, and for the analysis. This is obviously >>> a result of a hasty conversion to the 'new' page table code, and a >>> total oversight on my part. >>> >>> I'm however not particularly thrilled with the approach you have taken >>> though, as carrying both the kvm->arch pointer *and* the mmu pointer >>> seems totally redundant (the mmu structure already has a backpointer >>> to kvm->arch or its pkvm equivalent). All we need is to rework the >>> initialisation for this pointer to be correct at the point of where we >>> follow it first. >>> >>> I've pushed out my own version of this[1]. Please have a look. >>> >>> Thanks, >>> >>> M. >>> >>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/commit/?h=kvm-arm64/nv-5.16-WIP&id=21790a24d88c3ed37989533709dad3d40905f5c3 >>> >> >> Thanks for the rework and rebasing to 5.16. >> >> I went through the patch, the gist of the patch seems to me same. >> Please free feel to add, >> Reviewed-by: Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> > > Thanks! > >> Looks like kvm-arm64/nv-5.16-WIP branch is broken for NV. >> I tried booting Guest hypervisor using lkvm and the vcpu init from >> lkvm is failing(Fatal: Unable to initialise vcpu). Did not dig/debug >> more in to the issue yet. > > I'm still trying to iron a few issues, but you should be able to boot > a NV guest. However, the way it is enabled has changed: you need to > pass 'kvm-arm.mode=nested' to the command line instead of the previous > 'kvm-arm.nested=1' which I have got rid of. That could well be the > issue. > > With the current state of the tree (I just pushed another fix), you > should be able to boot a L1 guest hypervisor and a L2 guest. I'm > getting a crash at the point where the L2 guest reaches userspace > though, so something is broken in the PSTATE or ERET tracking, I'd > expect. Thanks Marc. It is booting now, i could boot L1/Guest-Hypervisor and L2(NestedVM) as well. > > M. > Thanks, Ganapat
diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h index 4f432ea3094c..9c0c380f8e3b 100644 --- a/arch/arm64/include/asm/kvm_pgtable.h +++ b/arch/arm64/include/asm/kvm_pgtable.h @@ -223,16 +223,18 @@ u64 kvm_get_vtcr(u64 mmfr0, u64 mmfr1, u32 phys_shift); * @arch: Arch-specific KVM structure representing the guest virtual * machine. * @mm_ops: Memory management callbacks. + * @mmu: The pointer to the s2 MMU structure * @flags: Stage-2 configuration flags. * * Return: 0 on success, negative error code on failure. */ int kvm_pgtable_stage2_init_flags(struct kvm_pgtable *pgt, struct kvm_arch *arch, struct kvm_pgtable_mm_ops *mm_ops, + struct kvm_s2_mmu *mmu, enum kvm_pgtable_stage2_flags flags); -#define kvm_pgtable_stage2_init(pgt, arch, mm_ops) \ - kvm_pgtable_stage2_init_flags(pgt, arch, mm_ops, 0) +#define kvm_pgtable_stage2_init(pgt, arch, mm_ops, mmu) \ + kvm_pgtable_stage2_init_flags(pgt, arch, mm_ops, mmu, 0) /** * kvm_pgtable_stage2_destroy() - Destroy an unused guest stage-2 page-table. diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c index 4b60c0056c04..cf7e034a0453 100644 --- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c +++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c @@ -99,7 +99,7 @@ int kvm_host_prepare_stage2(void *mem_pgt_pool, void *dev_pgt_pool) return ret; ret = kvm_pgtable_stage2_init_flags(&host_kvm.pgt, &host_kvm.arch, - &host_kvm.mm_ops, KVM_HOST_S2_FLAGS); + &host_kvm.mm_ops, mmu, KVM_HOST_S2_FLAGS); if (ret) return ret; diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c index fa85da30c9b8..85acd9e19ed0 100644 --- a/arch/arm64/kvm/hyp/pgtable.c +++ b/arch/arm64/kvm/hyp/pgtable.c @@ -1018,6 +1018,7 @@ int kvm_pgtable_stage2_flush(struct kvm_pgtable *pgt, u64 addr, u64 size) int kvm_pgtable_stage2_init_flags(struct kvm_pgtable *pgt, struct kvm_arch *arch, struct kvm_pgtable_mm_ops *mm_ops, + struct kvm_s2_mmu *mmu, enum kvm_pgtable_stage2_flags flags) { size_t pgd_sz; @@ -1034,7 +1035,7 @@ int kvm_pgtable_stage2_init_flags(struct kvm_pgtable *pgt, struct kvm_arch *arch pgt->ia_bits = ia_bits; pgt->start_level = start_level; pgt->mm_ops = mm_ops; - pgt->mmu = &arch->mmu; + pgt->mmu = mmu; pgt->flags = flags; /* Ensure zeroed PGD pages are visible to the hardware walker */ diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c index 0cf6ab944adc..6cf86cafc65a 100644 --- a/arch/arm64/kvm/mmu.c +++ b/arch/arm64/kvm/mmu.c @@ -495,7 +495,7 @@ int kvm_init_stage2_mmu(struct kvm *kvm, struct kvm_s2_mmu *mmu) if (!pgt) return -ENOMEM; - err = kvm_pgtable_stage2_init(pgt, &kvm->arch, &kvm_s2_mm_ops); + err = kvm_pgtable_stage2_init(pgt, &kvm->arch, &kvm_s2_mm_ops, mmu); if (err) goto out_free_pgtable;
The kvm_pgtable_stage2_init/kvm_pgtable_stage2_init_flags function assume arch->mmu is same across all stage 2 mmu and initializes the pgt(page table) using arch->mmu. Using armc->mmu is not appropriate when nested virtualization is enabled since there are multiple stage 2 mmu tables are initialized to manage Guest-Hypervisor as well as Nested VM for the same vCPU. Add a mmu argument to kvm_pgtable_stage2_init that can be used during initialization. This patch is a preparatory patch for the nested virtualization series and no functional changes. Signed-off-by: Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> --- arch/arm64/include/asm/kvm_pgtable.h | 6 ++++-- arch/arm64/kvm/hyp/nvhe/mem_protect.c | 2 +- arch/arm64/kvm/hyp/pgtable.c | 3 ++- arch/arm64/kvm/mmu.c | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-)