Re: [PATCH 1/2] KVM: arm64: Use appropriate mmu pointer in stage2 page table init.

From: Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com>
To: Marc Zyngier <maz@kernel.org>
Cc: D Scott Phillips <scott@os.amperecomputing.com>,
	kvm@vger.kernel.org, catalin.marinas@arm.com,
	darren@os.amperecomputing.com, andre.przywara@arm.com,
	will@kernel.org, kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/2] KVM: arm64: Use appropriate mmu pointer in stage2 page table init.
Date: Fri, 26 Nov 2021 11:15:26 +0530	[thread overview]
Message-ID: <84b7602f-93c1-74e3-bebf-23ed9e795b9b@os.amperecomputing.com> (raw)
In-Reply-To: <87bl28cpko.wl-maz@kernel.org>

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
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm