Re: [PATCH v2 01/17] KVM: arm64: Factor out stage 2 page table data from struct kvm

Alexandru Elisei
2020-06-25T12:48:24Z

Hi,

On 6/15/20 2:27 PM, Marc Zyngier wrote:
> From: Christoffer Dall <christoffer.dall@arm.com>
>
> As we are about to reuse our stage 2 page table manipulation code for
> shadow stage 2 page tables in the context of nested virtualization, we
> are going to manage multiple stage 2 page tables for a single VM.
>
> This requires some pretty invasive changes to our data structures,
> which moves the vmid and pgd pointers into a separate structure and
> change pretty much all of our mmu code to operate on this structure
> instead.
>
> The new structure is called struct kvm_s2_mmu.
>
> There is no intended functional change by this patch alone.
>
> Reviewed-by: James Morse <james.morse@arm.com>
> [Designed data structure layout in collaboration]
> Signed-off-by: Christoffer Dall <christoffer.dall@arm.com>
> Co-developed-by: Marc Zyngier <maz@kernel.org>
> [maz: Moved the last_vcpu_ran down to the S2 MMU structure as well]
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> arch/arm64/include/asm/kvm_asm.h | 7 +-
> arch/arm64/include/asm/kvm_host.h | 32 +++-
> arch/arm64/include/asm/kvm_mmu.h | 16 +-
> arch/arm64/kvm/arm.c | 36 ++--
> arch/arm64/kvm/hyp/switch.c | 8 +-
> arch/arm64/kvm/hyp/tlb.c | 52 +++---
> arch/arm64/kvm/mmu.c | 278 +++++++++++++++++-------------
> 7 files changed, 233 insertions(+), 196 deletions(-)
>
> [..]
>
> @@ -96,31 +96,33 @@ static bool kvm_is_device_pfn(unsigned long pfn)
> *
> * Function clears a PMD entry, flushes addr 1st and 2nd stage TLBs.
> */
> -static void stage2_dissolve_pmd(struct kvm *kvm, phys_addr_t addr, pmd_t *pmd)
> +static void stage2_dissolve_pmd(struct kvm_s2_mmu *mmu, phys_addr_t addr, pmd_t *pmd)

The comment for the function hasn't been updated, it still mentions kvm instead of
mmu.

I applied your fix to __kvm_tlb_flush_local_vmid, and I was able to boot a virtual
machine and run perf in it. The remaining comments from me are minor, so for what
it's worth:

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,
Alex