| Message ID | 20240510112645.3625702-4-ptosi@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Add support for hypervisor kCFI | expand |
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@google.com> > --- > 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
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! */ diff --git a/arch/arm64/kvm/hyp/nvhe/setup.c b/arch/arm64/kvm/hyp/nvhe/setup.c index bcaeb0fafd2d..45b83f3ed012 100644 --- a/arch/arm64/kvm/hyp/nvhe/setup.c +++ b/arch/arm64/kvm/hyp/nvhe/setup.c @@ -314,7 +314,6 @@ void __noreturn __pkvm_init_finalise(void) int __pkvm_init(phys_addr_t phys, unsigned long size, unsigned long nr_cpus, unsigned long *per_cpu_base, u32 hyp_va_bits) { - struct kvm_nvhe_init_params *params; void *virt = hyp_phys_to_virt(phys); typeof(__pkvm_init_switch_pgd) *fn; int ret; @@ -338,9 +337,8 @@ int __pkvm_init(phys_addr_t phys, unsigned long size, unsigned long nr_cpus, update_nvhe_init_params(); /* Jump in the idmap page to switch to the new page-tables */ - params = this_cpu_ptr(&kvm_init_params); fn = (typeof(fn))__hyp_pa(__pkvm_init_switch_pgd); - fn(__hyp_pa(params), __pkvm_init_finalise); + fn(this_cpu_ptr(&kvm_init_params), __pkvm_init_finalise); unreachable(); }
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@google.com> --- 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(-)