On Fri, May 10, 2024 at 12:26:32PM +0100, Pierre-Clément Tosi wrote: > Make the function take a VA pointer, instead of a phys_addr_t, to fully > take advantage of the high-level C language and its type checker. > > Perform all accesses to the kvm_nvhe_init_params before disabling the > MMU, removing the need to access it using physical addresses, which was > the reason for taking a phys_addr_t. > > Signed-off-by: Pierre-Clément Tosi <ptosi@xxxxxxxxxx> > --- > arch/arm64/include/asm/kvm_hyp.h | 3 ++- > arch/arm64/kvm/hyp/nvhe/hyp-init.S | 12 +++++++++--- > arch/arm64/kvm/hyp/nvhe/setup.c | 4 +--- > 3 files changed, 12 insertions(+), 7 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h > index 96daf7cf6802..c195e71d0746 100644 > --- a/arch/arm64/include/asm/kvm_hyp.h > +++ b/arch/arm64/include/asm/kvm_hyp.h > @@ -123,7 +123,8 @@ void __noreturn __hyp_do_panic(struct kvm_cpu_context *host_ctxt, u64 spsr, > #endif > > #ifdef __KVM_NVHE_HYPERVISOR__ > -void __pkvm_init_switch_pgd(phys_addr_t params, void (*finalize_fn)(void)); > +void __pkvm_init_switch_pgd(struct kvm_nvhe_init_params *params, > + void (*finalize_fn)(void)); > int __pkvm_init(phys_addr_t phys, unsigned long size, unsigned long nr_cpus, > unsigned long *per_cpu_base, u32 hyp_va_bits); > void __noreturn __host_enter(struct kvm_cpu_context *host_ctxt); > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S > index 2994878d68ea..5a15737b4233 100644 > --- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S > @@ -265,7 +265,15 @@ alternative_else_nop_endif > > SYM_CODE_END(__kvm_handle_stub_hvc) > > +/* > + * void __pkvm_init_switch_pgd(struct kvm_nvhe_init_params *params, > + * void (*finalize_fn)(void)); > + */ > SYM_FUNC_START(__pkvm_init_switch_pgd) > + /* Load the inputs from the VA pointer before turning the MMU off */ > + ldr x5, [x0, #NVHE_INIT_PGD_PA] > + ldr x0, [x0, #NVHE_INIT_STACK_HYP_VA] > + > /* Turn the MMU off */ > pre_disable_mmu_workaround > mrs x2, sctlr_el2 > @@ -276,15 +284,13 @@ SYM_FUNC_START(__pkvm_init_switch_pgd) > tlbi alle2 > > /* Install the new pgtables */ > - ldr x3, [x0, #NVHE_INIT_PGD_PA] > - phys_to_ttbr x4, x3 > + phys_to_ttbr x4, x5 > alternative_if ARM64_HAS_CNP > orr x4, x4, #TTBR_CNP_BIT > alternative_else_nop_endif > msr ttbr0_el2, x4 > > /* Set the new stack pointer */ > - ldr x0, [x0, #NVHE_INIT_STACK_HYP_VA] > mov sp, x0 > > /* And turn the MMU back on! */ Hmm, if we can hoist the memory accesses all the way like this, then couldn't we just move them into the caller's C code? Maybe that's what we planned to do in the first place, which would explain why the prototype of __pkvm_init_switch_pgd() is out-of-sync. In other words, drop the previous patch and pass in the pgd and SP as arguments to the asm. Will
