KVM: arm64: Restrict host hypercalls when pKVM is enabled
Will Deacon
maintainercover KVM: arm64: Restrict host hypercalls when pKVM is enabled
Hi folks,
This series restricts the hypercalls available to the KVM host on arm64 when pKVM is enabled so that it is not possible for the host to use them to replace the EL2 component with something else.
This occurs in two stages: when switching to the pKVM vectors, the stub hypercalls are removed and then later when pKVM is finalised, the pKVM init hypercalls are removed.
There are still a few dubious calls remaining in terms of protecting the guest (e.g. __kvm_adjust_pc) but these will be dealt with later when we have more VM state at EL2 to play with.
Patches based on -rc2. Feedback welcome.
Cheers,
Will
--->8
Will Deacon (5):
arm64: Prevent kexec and hibernation if is_protected_kvm_enabled()
KVM: arm64: Reject stub hypercalls after pKVM has been initialised
KVM: arm64: Propagate errors from __pkvm_prot_finalize hypercall
KVM: arm64: Prevent re-finalisation of pKVM for a given CPU
KVM: arm64: Disable privileged hypercalls after pKVM finalisation
arch/arm64/include/asm/kvm_asm.h | 43 ++++++++++--------- arch/arm64/kernel/smp.c | 3 +- arch/arm64/kvm/arm.c | 61 ++++++++++++++++++--------- arch/arm64/kvm/hyp/nvhe/host.S | 26 ++++++++---- arch/arm64/kvm/hyp/nvhe/hyp-main.c | 26 +++++++----- arch/arm64/kvm/hyp/nvhe/mem_protect.c | 3 ++ 6 files changed, 103 insertions(+), 59 deletions(-)
Signature · 6 lines
2.33.0.464.g1972c5931b-goog _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Will Deacon
maintainerPATCH 1/5 arm64: Prevent kexec and hibernation if is_protected_kvm_enabled()
When pKVM is enabled, the hypervisor code at EL2 and its data structures are inaccessible to the host kernel and cannot be torn down or replaced as this would defeat the integrity properies which pKVM aims to provide. Furthermore, the ABI between the host and EL2 is flexible and private to whatever the current implementation of KVM requires and so booting a new kernel with an old EL2 component is very likely to end in disaster.
In preparation for uninstalling the hyp stub calls which are relied upon to reset EL2, disable kexec and hibernation in the host when protected KVM is enabled.
--- arch/arm64/kernel/smp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
arch/arm64/kernel/smp.c +2 -1
@@ -1128,5 +1128,6 @@ bool cpus_are_stuck_in_kernel(void) {bool smp_spin_tables = (num_possible_cpus () >1 && !have_cpu_die ()); -return !!cpus_stuck_in_kernel || smp_spin_tables; +return !!cpus_stuck_in_kernel || smp_spin_tables || +is_protected_kvm_enabled (); }
Signature · 6 lines
2.33.0.464.g1972c5931b-goog _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Mark Rutland
reviewerRe: [PATCH 1/5] arm64: Prevent kexec and hibernation if is_protected_kvm_enabled()
Quoted 23 lines · Will Deacon
When pKVM is enabled, the hypervisor code at EL2 and its data structures are inaccessible to the host kernel and cannot be torn down or replaced as this would defeat the integrity properies which pKVM aims to provide. Furthermore, the ABI between the host and EL2 is flexible and private to whatever the current implementation of KVM requires and so booting a new kernel with an old EL2 component is very likely to end in disaster.
In preparation for uninstalling the hyp stub calls which are relied upon to reset EL2, disable kexec and hibernation in the host when protected KVM is enabled.
Cc Marc Zyngier <[email protected]>Cc Quentin Perret <[email protected]>Signed-off-by Will Deacon <[email protected]>--- arch/arm64/kernel/smp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)arch/arm64/kernel/smp.c +0
-0
arch/arm64/kernel/smp.c bool cpus_are_stuck_in_kernel(void) · line 1133
Quoted 7 lines
arch/arm64/kernel/smp.c +2
-1@@ -1128,5 +1128,6 @@ bool cpus_are_stuck_in_kernel(void) {bool smp_spin_tables = (num_possible_cpus () >1 && !have_cpu_die ()); -return !!cpus_stuck_in_kernel || smp_spin_tables; +return !!cpus_stuck_in_kernel || smp_spin_tables || +is_protected_kvm_enabled (); }
IIUC you'll also need to do something to prevent kdump, since even with CPUs stuck in the kernel that will try to do a kexec on the crashed CPU and __cpu_soft_restart() won't be able to return to EL2.
You could fiddle with the BUG_ON() in machine_kexec() to die in this case too.
Thanks,
Mark.
Quoted 7 lines
Signature · 6 lines
2.33.0.464.g1972c5931b-goog _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Will Deacon
maintainerRe: [PATCH 1/5] arm64: Prevent kexec and hibernation if is_protected_kvm_enabled()
Quoted 36 lines · Mark Rutland
Quoted 23 lines · Will Deacon
When pKVM is enabled, the hypervisor code at EL2 and its data structures are inaccessible to the host kernel and cannot be torn down or replaced as this would defeat the integrity properies which pKVM aims to provide. Furthermore, the ABI between the host and EL2 is flexible and private to whatever the current implementation of KVM requires and so booting a new kernel with an old EL2 component is very likely to end in disaster.
In preparation for uninstalling the hyp stub calls which are relied upon to reset EL2, disable kexec and hibernation in the host when protected KVM is enabled.
Cc Marc Zyngier <[email protected]>Cc Quentin Perret <[email protected]>Signed-off-by Will Deacon <[email protected]>--- arch/arm64/kernel/smp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)arch/arm64/kernel/smp.c +0
-0arch/arm64/kernel/smp.c bool cpus_are_stuck_in_kernel(void) · line 1133
Quoted 7 lines
arch/arm64/kernel/smp.c +2
-1@@ -1128,5 +1128,6 @@ bool cpus_are_stuck_in_kernel(void) {bool smp_spin_tables = (num_possible_cpus () >1 && !have_cpu_die ()); -return !!cpus_stuck_in_kernel || smp_spin_tables; +return !!cpus_stuck_in_kernel || smp_spin_tables || +is_protected_kvm_enabled (); }IIUC you'll also need to do something to prevent kdump, since even with CPUs stuck in the kernel that will try to do a kexec on the crashed CPU and
__cpu_soft_restart()won't be able to return to EL2.
Quoted 2 lines
You could fiddle with the
BUG_ON()inmachine_kexec()to die in this case too.
I wondered about that, and I'm happy to do it if you reckon it's better, but if the host is crashing _anyway_ then I wasn't convinced it was worth the effort. With the approach here, we'll WARN and then enter the kdump kernel at EL1 which maybe might work sometimes possibly? I suppose if the kdump kernel is careful about the memory it accesses, then it has a fighting chance of doing something useful.
Will
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Will Deacon
maintainerPATCH 2/5 KVM: arm64: Reject stub hypercalls after pKVM has been initialised
In reply to Will DeaconThe stub hypercalls provide mechanisms to reset and replace the EL2 code, so uninstall them once pKVM has been initialised in order to ensure the integrity of the hypervisor code.
To ensure pKVM initialisation remains functional, split cpu_hyp_reinit() into two helper functions to separate usage of the stub from usage of pkvm hypercalls either side of __pkvm_init on the boot CPU.
--- arch/arm64/kvm/arm.c | 31 +++++++++++++++++++++++-------- arch/arm64/kvm/hyp/nvhe/host.S | 26 +++++++++++++++++--------- 2 files changed, 40 insertions(+), 17 deletions(-)
arch/arm64/kvm/arm.c +23 -8
@@ -1579,25 +1579,33 @@ static void cpu_set_hyp_vector(void)kvm_call_hyp_nvhe (__pkvm_cpu_set_vector, data->slot); } -static void cpu_hyp_reinit (void ) +static void cpu_hyp_init_context (void ) {kvm_init_host_cpu_context (&this_cpu_ptr_hyp_sym (kvm_host_data)->host_ctxt); -cpu_hyp_reset (); - -if (is_kernel_in_hyp_mode ()) -kvm_timer_init_vhe (); -else +if (!is_kernel_in_hyp_mode ())cpu_init_hyp_mode (); +} +static void cpu_hyp_init_features (void ) +{cpu_set_hyp_vector (); -kvm_arm_init_debug (); +if (is_kernel_in_hyp_mode ()) +kvm_timer_init_vhe (); +if (vgic_present)kvm_vgic_init_cpu_hardware (); } +static void cpu_hyp_reinit (void ) +{ +cpu_hyp_reset (); +cpu_hyp_init_context (); +cpu_hyp_init_features (); +} +static void _kvm_arch_hardware_enable (void *discard) {if (!__this_cpu_read (kvm_arm_hardware_enabled)) {
@@ -1788,10 +1796,17 @@ static int do_pkvm_init(u32 hyp_va_bits)int ret;preempt_disable (); -hyp_install_host_vector (); +cpu_hyp_init_context (); ret =kvm_call_hyp_nvhe (__pkvm_init, hyp_mem_base, hyp_mem_size,num_possible_cpus (),kern_hyp_va (per_cpu_base), hyp_va_bits); +cpu_hyp_init_features (); + +/* +* The stub hypercalls are now disabled, so set our local flag to +* prevent a later re-init attempt in kvm_arch_hardware_enable(). +*/ +__this_cpu_write (kvm_arm_hardware_enabled,1 );preempt_enable ();return ret;
arch/arm64/kvm/hyp/nvhe/host.S +17 -9
@@ -110,17 +110,14 @@ SYM_FUNC_START(__hyp_do_panic) b __host_enter_for_panic SYM_FUNC_END(__hyp_do_panic) -.macro host_el1_sync_vect - .align 7 -.L__vect_start\@: - stp x0, x1, [sp, #-16]! - mrs x0, esr_el2 - lsr x0, x0, #ESR_ELx_EC_SHIFT - cmp x0, #ESR_ELx_EC_HVC64 - b.ne __host_exit - +SYM_FUNC_START(__host_hvc) ldp x0, x1, [sp] // Don't fixup the stack yet + /* No stub for you, sonny Jim */ +alternative_if ARM64_KVM_PROTECTED_MODE + b __host_exit +alternative_else_nop_endif + /* Check for a stub HVC call */ cmp x0, #HVC_STUB_HCALL_NR b.hs __host_exit
@@ -137,6 +134,17 @@ SYM_FUNC_END(__hyp_do_panic) ldr x5, =__kvm_handle_stub_hvc hyp_pa x5, x6 br x5 +SYM_FUNC_END(__host_hvc) + +.macro host_el1_sync_vect + .align 7 +.L__vect_start\@: + stp x0, x1, [sp, #-16]! + mrs x0, esr_el2 + lsr x0, x0, #ESR_ELx_EC_SHIFT + cmp x0, #ESR_ELx_EC_HVC64 + b.eq __host_hvc + b __host_exit .L__vect_end\@: .if ((.L__vect_end\@ - .L__vect_start\@) > 0x80) .error "host_el1_sync_vect larger than vector entry"
Signature · 6 lines
2.33.0.464.g1972c5931b-goog _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 2/5] KVM: arm64: Reject stub hypercalls after pKVM has been initialised
Quoted 11 lines · Will Deacon
The stub hypercalls provide mechanisms to reset and replace the EL2 code, so uninstall them once pKVM has been initialised in order to ensure the integrity of the hypervisor code.
To ensure pKVM initialisation remains functional, split
cpu_hyp_reinit()into two helper functions to separate usage of the stub from usage of pkvm hypercalls either side of__pkvm_initon the boot CPU.Cc Marc Zyngier <[email protected]>Cc Quentin Perret <[email protected]>Signed-off-by Will Deacon <[email protected]>
Thanks,
Quentin
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Will Deacon
maintainerPATCH 3/5 KVM: arm64: Propagate errors from __pkvm_prot_finalize hypercall
In reply to Will DeaconIf the __pkvm_prot_finalize hypercall returns an error, we WARN but fail to propagate the failure code back to kvm_arch_init().
Pass a pointer to a zero-initialised return variable so that failure to finalise the pKVM protections on a host CPU can be reported back to KVM.
--- arch/arm64/kvm/arm.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-)
arch/arm64/kvm/arm.c +19 -11
@@ -1986,9 +1986,25 @@ static int init_hyp_mode(void)return err; } -static void _kvm_host_prot_finalize (void *discard) +static void _kvm_host_prot_finalize (void *arg) { -WARN_ON (kvm_call_hyp_nvhe (__pkvm_prot_finalize)); +int *err = arg; + +if (WARN_ON (kvm_call_hyp_nvhe (__pkvm_prot_finalize))) +WRITE_ONCE (*err, -EINVAL ); +} + +static int pkvm_drop_host_privileges (void ) +{ +int ret =0 ; + +/* +* Flip the static key upfront as that may no longer be possible +* once the host stage 2 is installed. +*/ +static_branch_enable (&kvm_protected_mode_initialized); +on_each_cpu (_kvm_host_prot_finalize, &ret,1 ); +return ret; }static int finalize_hyp_mode (void )
@@ -2002,15 +2018,7 @@ static int finalize_hyp_mode(void)* None of other sections should ever be introspected. */ kmemleak_free_part (__hyp_bss_start, __hyp_bss_end - __hyp_bss_start); - -/* -* Flip the static key upfront as that may no longer be possible -* once the host stage 2 is installed. -*/ -static_branch_enable (&kvm_protected_mode_initialized); -on_each_cpu (_kvm_host_prot_finalize,NULL ,1 ); - -return 0 ; +return pkvm_drop_host_privileges (); }struct kvm_vcpu *kvm_mpidr_to_vcpu (struct kvm *kvm,unsigned long mpidr)
Signature · 6 lines
2.33.0.464.g1972c5931b-goog _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 3/5] KVM: arm64: Propagate errors from __pkvm_prot_finalize hypercall
Quoted 21 lines · Will Deacon
If the
__pkvm_prot_finalizehypercall returns an error, we WARN but fail to propagate the failure code back tokvm_arch_init().Pass a pointer to a zero-initialised return variable so that failure to finalise the pKVM protections on a host CPU can be reported back to KVM.
Cc Marc Zyngier <[email protected]>Cc Quentin Perret <[email protected]>Signed-off-by Will Deacon <[email protected]>--- arch/arm64/kvm/arm.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-)arch/arm64/kvm/arm.c +0
-0@@ -1986,9 +1986,25 @@ static int init_hyp_mode(void)return err; }
arch/arm64/kvm/arm.c static int init_hyp_mode(void) · line 1995
Quoted 10 lines
arch/arm64/kvm/arm.c +6
-2-static void _kvm_host_prot_finalize (void *discard) +static void _kvm_host_prot_finalize (void *arg) { -WARN_ON (kvm_call_hyp_nvhe (__pkvm_prot_finalize)); +int *err = arg; + +if (WARN_ON (kvm_call_hyp_nvhe (__pkvm_prot_finalize))) +WRITE_ONCE (*err, -EINVAL ); +}
I was going to suggest to propagate the hypercall's error code directly, but this becomes very racy so n/m...
But this got me thinking about what we should do when the hyp init fails while the protected mode has been explicitly enabled on the kernel cmdline. That is, if we continue and boot the kernel w/o KVM support, then I don't know how e.g. EL3 can know that it shouldn't give keys to VMs because the kernel (and EL2) can't be trusted. It feels like it is the kernel's responsibility to do something while it _is_ still trustworthy.
I guess we could make any error code fatal in kvm_arch_init() when is_protected_kvm_enabled() is on, or something along those lines? Maybe dependent on CONFIG_NVHE_EL2_DEBUG=n?
It's probably a bit theoretical because there really shouldn't be any reason to fail hyp init in production when using a signed kernel image etc etc, but then if that is the case the additional check I'm suggesting shouldn't hurt and will give us some peace of mind. Thoughts?
Thanks,
Quentin
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Will Deacon
maintainerRe: [PATCH 3/5] KVM: arm64: Propagate errors from __pkvm_prot_finalize hypercall
Hey Quentin,
Quoted 44 lines · Quentin Perret
Quoted 21 lines · Will Deacon
If the
__pkvm_prot_finalizehypercall returns an error, we WARN but fail to propagate the failure code back tokvm_arch_init().Pass a pointer to a zero-initialised return variable so that failure to finalise the pKVM protections on a host CPU can be reported back to KVM.
Cc Marc Zyngier <[email protected]>Cc Quentin Perret <[email protected]>Signed-off-by Will Deacon <[email protected]>--- arch/arm64/kvm/arm.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-)arch/arm64/kvm/arm.c +0
-0@@ -1986,9 +1986,25 @@ static int init_hyp_mode(void)return err; }arch/arm64/kvm/arm.c static int init_hyp_mode(void) · line 1995
Quoted 10 lines
arch/arm64/kvm/arm.c +6
-2-static void _kvm_host_prot_finalize (void *discard) +static void _kvm_host_prot_finalize (void *arg) { -WARN_ON (kvm_call_hyp_nvhe (__pkvm_prot_finalize)); +int *err = arg; + +if (WARN_ON (kvm_call_hyp_nvhe (__pkvm_prot_finalize))) +WRITE_ONCE (*err, -EINVAL ); +}I was going to suggest to propagate the hypercall's error code directly, but this becomes very racy so n/m...
But this got me thinking about what we should do when the hyp init fails while the protected mode has been explicitly enabled on the kernel cmdline. That is, if we continue and boot the kernel w/o KVM support, then I don't know how e.g. EL3 can know that it shouldn't give keys to VMs because the kernel (and EL2) can't be trusted. It feels like it is the kernel's responsibility to do something while it _is_ still trustworthy.
Quoted 8 lines
I guess we could make any error code fatal in
kvm_arch_init()whenis_protected_kvm_enabled()is on, or something along those lines? Maybe dependent onCONFIG_NVHE_EL2_DEBUG=n?It's probably a bit theoretical because there really shouldn't be any reason to fail hyp init in production when using a signed kernel image etc etc, but then if that is the case the additional check I'm suggesting shouldn't hurt and will give us some peace of mind. Thoughts?
It's an interesting one.
I'm not hugely keen on crashing the system if we fail to deprivilege the host (which I think is effectively what is happening in the case you describe), but you're right that we need to disable pKVM somehow in this case. I think the best thing would be to wipe the pvmfw memory; that would mean that the host can do whatever it likes at EL2, as the keys will no longer be available.
I'll make a note about this, since I've parked the pvmfw patches until we've got more of the pKVM infrastructure up and running.
Will
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Will Deacon
maintainerPATCH 4/5 KVM: arm64: Prevent re-finalisation of pKVM for a given CPU
In reply to Will Deacon__pkvm_prot_finalize() completes the deprivilege of the host when pKVM is in use by installing a stage-2 translation table for the calling CPU.
Issuing the hypercall multiple times for a given CPU makes little sense, but in such a case just return early with -EPERM rather than go through the whole page-table dance again.
--- arch/arm64/kvm/hyp/nvhe/mem_protect.c | 3 +++ 1 file changed, 3 insertions(+)
arch/arm64/kvm/hyp/nvhe/mem_protect.c +3 -0
@@ -123,6 +123,9 @@ int __pkvm_prot_finalize(void)struct kvm_s2_mmu *mmu = &host_kvm.arch.mmu;struct kvm_nvhe_init_params *params =this_cpu_ptr (&kvm_init_params); +if (params->hcr_el2 &HCR_VM ) +return -EPERM ; + params->vttbr =kvm_get_vttbr (mmu); params->vtcr = host_kvm.arch.vtcr; params->hcr_el2 |=HCR_VM ;
Signature · 6 lines
2.33.0.464.g1972c5931b-goog _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 4/5] KVM: arm64: Prevent re-finalisation of pKVM for a given CPU
Quoted 19 lines · Will Deacon
__pkvm_prot_finalize()completes the deprivilege of the host when pKVM is in use by installing a stage-2 translation table for the calling CPU.Issuing the hypercall multiple times for a given CPU makes little sense, but in such a case just return early with
-EPERMrather than go through the whole page-table dance again.Cc Marc Zyngier <[email protected]>Cc Quentin Perret <[email protected]>Signed-off-by Will Deacon <[email protected]>--- arch/arm64/kvm/hyp/nvhe/mem_protect.c | 3 +++ 1 file changed, 3 insertions(+)arch/arm64/kvm/hyp/nvhe/mem_protect.c +0
-0
arch/arm64/kvm/hyp/nvhe/mem_protect.c int __pkvm_prot_finalize(void) · line 127
Quoted 5 lines
arch/arm64/kvm/hyp/nvhe/mem_protect.c +2
-0@@ -123,6 +123,9 @@ int __pkvm_prot_finalize(void)struct kvm_s2_mmu *mmu = &host_kvm.arch.mmu;struct kvm_nvhe_init_params *params =this_cpu_ptr (&kvm_init_params); +if (params->hcr_el2 &HCR_VM ) +return -EPERM ;
And you check this rather than the static key because we flip it upfront I guess. Makes sense to me, but maybe a little comment would be useful :) In any case:
Thanks,
Quentin
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Will Deacon
maintainerPATCH 5/5 KVM: arm64: Disable privileged hypercalls after pKVM finalisation
In reply to Will DeaconAfter pKVM has been 'finalised' using the __pkvm_prot_finalize hypercall, the calling CPU will have a Stage-2 translation enabled to prevent access to memory pages owned by EL2.
Although this forms a significant part of the process to deprivilege the host kernel, we also need to ensure that the hypercall interface is reduced so that the EL2 code cannot, for example, be re-initialised using a new set of vectors.
Re-order the hypercalls so that only a suffix remains available after finalisation of pKVM.
--- arch/arm64/include/asm/kvm_asm.h | 43 ++++++++++++++++-------------- arch/arm64/kvm/hyp/nvhe/hyp-main.c | 26 +++++++++++------- 2 files changed, 39 insertions(+), 30 deletions(-)
arch/arm64/include/asm/kvm_asm.h +23 -20
@@ -43,27 +43,30 @@#define KVM_HOST_SMCCC_FUNC (name) KVM_HOST_SMCCC_ID(__KVM_HOST_SMCCC_FUNC_##name) +/* Hypercalls available only prior to pKVM finalisation */ #define __KVM_HOST_SMCCC_FUNC___kvm_hyp_init 0 -#define __KVM_HOST_SMCCC_FUNC___kvm_vcpu_run 1 -#define __KVM_HOST_SMCCC_FUNC___kvm_flush_vm_context 2 -#define __KVM_HOST_SMCCC_FUNC___kvm_tlb_flush_vmid_ipa 3 -#define __KVM_HOST_SMCCC_FUNC___kvm_tlb_flush_vmid 4 -#define __KVM_HOST_SMCCC_FUNC___kvm_flush_cpu_context 5 -#define __KVM_HOST_SMCCC_FUNC___kvm_timer_set_cntvoff 6 -#define __KVM_HOST_SMCCC_FUNC___kvm_enable_ssbs 7 -#define __KVM_HOST_SMCCC_FUNC___vgic_v3_get_gic_config 8 -#define __KVM_HOST_SMCCC_FUNC___vgic_v3_read_vmcr 9 -#define __KVM_HOST_SMCCC_FUNC___vgic_v3_write_vmcr 10 -#define __KVM_HOST_SMCCC_FUNC___vgic_v3_init_lrs 11 -#define __KVM_HOST_SMCCC_FUNC___kvm_get_mdcr_el2 12 -#define __KVM_HOST_SMCCC_FUNC___vgic_v3_save_aprs 13 -#define __KVM_HOST_SMCCC_FUNC___vgic_v3_restore_aprs 14 -#define __KVM_HOST_SMCCC_FUNC___pkvm_init 15 -#define __KVM_HOST_SMCCC_FUNC___pkvm_host_share_hyp 16 -#define __KVM_HOST_SMCCC_FUNC___pkvm_create_private_mapping 17 -#define __KVM_HOST_SMCCC_FUNC___pkvm_cpu_set_vector 18 -#define __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize 19 -#define __KVM_HOST_SMCCC_FUNC___kvm_adjust_pc 20 +#define __KVM_HOST_SMCCC_FUNC___kvm_get_mdcr_el2 1 +#define __KVM_HOST_SMCCC_FUNC___pkvm_init 2 +#define __KVM_HOST_SMCCC_FUNC___pkvm_create_private_mapping 3 +#define __KVM_HOST_SMCCC_FUNC___pkvm_cpu_set_vector 4 +#define __KVM_HOST_SMCCC_FUNC___kvm_enable_ssbs 5 +#define __KVM_HOST_SMCCC_FUNC___vgic_v3_init_lrs 6 +#define __KVM_HOST_SMCCC_FUNC___vgic_v3_get_gic_config 7 +#define __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize 8 + +/* Hypercalls available after pKVM finalisation */ +#define __KVM_HOST_SMCCC_FUNC___pkvm_host_share_hyp 9 +#define __KVM_HOST_SMCCC_FUNC___kvm_adjust_pc 10 +#define __KVM_HOST_SMCCC_FUNC___kvm_vcpu_run 11 +#define __KVM_HOST_SMCCC_FUNC___kvm_flush_vm_context 12 +#define __KVM_HOST_SMCCC_FUNC___kvm_tlb_flush_vmid_ipa 13 +#define __KVM_HOST_SMCCC_FUNC___kvm_tlb_flush_vmid 14 +#define __KVM_HOST_SMCCC_FUNC___kvm_flush_cpu_context 15 +#define __KVM_HOST_SMCCC_FUNC___kvm_timer_set_cntvoff 16 +#define __KVM_HOST_SMCCC_FUNC___vgic_v3_read_vmcr 17 +#define __KVM_HOST_SMCCC_FUNC___vgic_v3_write_vmcr 18 +#define __KVM_HOST_SMCCC_FUNC___vgic_v3_save_aprs 19 +#define __KVM_HOST_SMCCC_FUNC___vgic_v3_restore_aprs 20#ifndef __ASSEMBLY__
arch/arm64/kvm/hyp/nvhe/hyp-main.c +16 -10
@@ -165,36 +165,42 @@ typedef void (*hcall_t)(struct kvm_cpu_context *);#define HANDLE_FUNC (x) [__KVM_HOST_SMCCC_FUNC_##x] = (hcall_t)handle_##xstatic const hcall_t host_hcall[] = { -HANDLE_FUNC (__kvm_vcpu_run), +/* ___kvm_hyp_init */ +HANDLE_FUNC (__kvm_get_mdcr_el2), +HANDLE_FUNC (__pkvm_init), +HANDLE_FUNC (__pkvm_create_private_mapping), +HANDLE_FUNC (__pkvm_cpu_set_vector), +HANDLE_FUNC (__kvm_enable_ssbs), +HANDLE_FUNC (__vgic_v3_init_lrs), +HANDLE_FUNC (__pkvm_prot_finalize), + +HANDLE_FUNC (__pkvm_host_share_hyp),HANDLE_FUNC (__kvm_adjust_pc), +HANDLE_FUNC (__kvm_vcpu_run),HANDLE_FUNC (__kvm_flush_vm_context),HANDLE_FUNC (__kvm_tlb_flush_vmid_ipa),HANDLE_FUNC (__kvm_tlb_flush_vmid),HANDLE_FUNC (__kvm_flush_cpu_context),HANDLE_FUNC (__kvm_timer_set_cntvoff), -HANDLE_FUNC (__kvm_enable_ssbs),HANDLE_FUNC (__vgic_v3_get_gic_config),HANDLE_FUNC (__vgic_v3_read_vmcr),HANDLE_FUNC (__vgic_v3_write_vmcr), -HANDLE_FUNC (__vgic_v3_init_lrs), -HANDLE_FUNC (__kvm_get_mdcr_el2),HANDLE_FUNC (__vgic_v3_save_aprs),HANDLE_FUNC (__vgic_v3_restore_aprs), -HANDLE_FUNC (__pkvm_init), -HANDLE_FUNC (__pkvm_cpu_set_vector), -HANDLE_FUNC (__pkvm_host_share_hyp), -HANDLE_FUNC (__pkvm_create_private_mapping), -HANDLE_FUNC (__pkvm_prot_finalize), };static void handle_host_hcall (struct kvm_cpu_context *host_ctxt) { DECLARE_REG(unsigned long, id, host_ctxt, 0); +unsigned long hcall_min =0 ;hcall_t hfn; +if (static_branch_unlikely (&kvm_protected_mode_initialized)) + hcall_min = __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize; + id -=KVM_HOST_SMCCC_ID (0 ); -if (unlikely (id >=ARRAY_SIZE (host_hcall))) +if (unlikely (id < hcall_min || id >=ARRAY_SIZE (host_hcall))) goto inval; hfn = host_hcall[id];
Signature · 6 lines
2.33.0.464.g1972c5931b-goog _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Marc Zyngier
maintainerRe: [PATCH 5/5] KVM: arm64: Disable privileged hypercalls after pKVM finalisation
Quoted 120 lines · Will Deacon
After pKVM has been 'finalised' using the
__pkvm_prot_finalizehypercall, the calling CPU will have a Stage-2 translation enabled to prevent access to memory pages owned by EL2.Although this forms a significant part of the process to deprivilege the host kernel, we also need to ensure that the hypercall interface is reduced so that the EL2 code cannot, for example, be re-initialised using a new set of vectors.
Re-order the hypercalls so that only a suffix remains available after finalisation of pKVM.
Cc Marc Zyngier <[email protected]>Cc Quentin Perret <[email protected]>Signed-off-by Will Deacon <[email protected]>--- arch/arm64/include/asm/kvm_asm.h | 43 ++++++++++++++++-------------- arch/arm64/kvm/hyp/nvhe/hyp-main.c | 26 +++++++++++------- 2 files changed, 39 insertions(+), 30 deletions(-)arch/arm64/include/asm/kvm_asm.h +23
-20@@ -43,27 +43,30 @@#define KVM_HOST_SMCCC_FUNC (name) KVM_HOST_SMCCC_ID(__KVM_HOST_SMCCC_FUNC_##name) +/* Hypercalls available only prior to pKVM finalisation */ #define __KVM_HOST_SMCCC_FUNC___kvm_hyp_init 0 -#define __KVM_HOST_SMCCC_FUNC___kvm_vcpu_run 1 -#define __KVM_HOST_SMCCC_FUNC___kvm_flush_vm_context 2 -#define __KVM_HOST_SMCCC_FUNC___kvm_tlb_flush_vmid_ipa 3 -#define __KVM_HOST_SMCCC_FUNC___kvm_tlb_flush_vmid 4 -#define __KVM_HOST_SMCCC_FUNC___kvm_flush_cpu_context 5 -#define __KVM_HOST_SMCCC_FUNC___kvm_timer_set_cntvoff 6 -#define __KVM_HOST_SMCCC_FUNC___kvm_enable_ssbs 7 -#define __KVM_HOST_SMCCC_FUNC___vgic_v3_get_gic_config 8 -#define __KVM_HOST_SMCCC_FUNC___vgic_v3_read_vmcr 9 -#define __KVM_HOST_SMCCC_FUNC___vgic_v3_write_vmcr 10 -#define __KVM_HOST_SMCCC_FUNC___vgic_v3_init_lrs 11 -#define __KVM_HOST_SMCCC_FUNC___kvm_get_mdcr_el2 12 -#define __KVM_HOST_SMCCC_FUNC___vgic_v3_save_aprs 13 -#define __KVM_HOST_SMCCC_FUNC___vgic_v3_restore_aprs 14 -#define __KVM_HOST_SMCCC_FUNC___pkvm_init 15 -#define __KVM_HOST_SMCCC_FUNC___pkvm_host_share_hyp 16 -#define __KVM_HOST_SMCCC_FUNC___pkvm_create_private_mapping 17 -#define __KVM_HOST_SMCCC_FUNC___pkvm_cpu_set_vector 18 -#define __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize 19 -#define __KVM_HOST_SMCCC_FUNC___kvm_adjust_pc 20 +#define __KVM_HOST_SMCCC_FUNC___kvm_get_mdcr_el2 1 +#define __KVM_HOST_SMCCC_FUNC___pkvm_init 2 +#define __KVM_HOST_SMCCC_FUNC___pkvm_create_private_mapping 3 +#define __KVM_HOST_SMCCC_FUNC___pkvm_cpu_set_vector 4 +#define __KVM_HOST_SMCCC_FUNC___kvm_enable_ssbs 5 +#define __KVM_HOST_SMCCC_FUNC___vgic_v3_init_lrs 6 +#define __KVM_HOST_SMCCC_FUNC___vgic_v3_get_gic_config 7 +#define __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize 8 + +/* Hypercalls available after pKVM finalisation */ +#define __KVM_HOST_SMCCC_FUNC___pkvm_host_share_hyp 9 +#define __KVM_HOST_SMCCC_FUNC___kvm_adjust_pc 10 +#define __KVM_HOST_SMCCC_FUNC___kvm_vcpu_run 11 +#define __KVM_HOST_SMCCC_FUNC___kvm_flush_vm_context 12 +#define __KVM_HOST_SMCCC_FUNC___kvm_tlb_flush_vmid_ipa 13 +#define __KVM_HOST_SMCCC_FUNC___kvm_tlb_flush_vmid 14 +#define __KVM_HOST_SMCCC_FUNC___kvm_flush_cpu_context 15 +#define __KVM_HOST_SMCCC_FUNC___kvm_timer_set_cntvoff 16 +#define __KVM_HOST_SMCCC_FUNC___vgic_v3_read_vmcr 17 +#define __KVM_HOST_SMCCC_FUNC___vgic_v3_write_vmcr 18 +#define __KVM_HOST_SMCCC_FUNC___vgic_v3_save_aprs 19 +#define __KVM_HOST_SMCCC_FUNC___vgic_v3_restore_aprs 20#ifndef __ASSEMBLY__arch/arm64/kvm/hyp/nvhe/hyp-main.c +11
-9@@ -165,36 +165,42 @@ typedef void (*hcall_t)(struct kvm_cpu_context *);#define HANDLE_FUNC (x) [__KVM_HOST_SMCCC_FUNC_##x] = (hcall_t)handle_##xstatic const hcall_t host_hcall[] = { -HANDLE_FUNC (__kvm_vcpu_run), +/* ___kvm_hyp_init */ +HANDLE_FUNC (__kvm_get_mdcr_el2), +HANDLE_FUNC (__pkvm_init), +HANDLE_FUNC (__pkvm_create_private_mapping), +HANDLE_FUNC (__pkvm_cpu_set_vector), +HANDLE_FUNC (__kvm_enable_ssbs), +HANDLE_FUNC (__vgic_v3_init_lrs), +HANDLE_FUNC (__pkvm_prot_finalize), + +HANDLE_FUNC (__pkvm_host_share_hyp),HANDLE_FUNC (__kvm_adjust_pc), +HANDLE_FUNC (__kvm_vcpu_run),HANDLE_FUNC (__kvm_flush_vm_context),HANDLE_FUNC (__kvm_tlb_flush_vmid_ipa),HANDLE_FUNC (__kvm_tlb_flush_vmid),HANDLE_FUNC (__kvm_flush_cpu_context),HANDLE_FUNC (__kvm_timer_set_cntvoff), -HANDLE_FUNC (__kvm_enable_ssbs),HANDLE_FUNC (__vgic_v3_get_gic_config),HANDLE_FUNC (__vgic_v3_read_vmcr),HANDLE_FUNC (__vgic_v3_write_vmcr), -HANDLE_FUNC (__vgic_v3_init_lrs), -HANDLE_FUNC (__kvm_get_mdcr_el2),HANDLE_FUNC (__vgic_v3_save_aprs),HANDLE_FUNC (__vgic_v3_restore_aprs), -HANDLE_FUNC (__pkvm_init), -HANDLE_FUNC (__pkvm_cpu_set_vector), -HANDLE_FUNC (__pkvm_host_share_hyp), -HANDLE_FUNC (__pkvm_create_private_mapping), -HANDLE_FUNC (__pkvm_prot_finalize), };static void handle_host_hcall (struct kvm_cpu_context *host_ctxt) { DECLARE_REG(unsigned long, id, host_ctxt, 0);
arch/arm64/kvm/hyp/nvhe/hyp-main.c typedef void (*hcall_t)(struct kvm_cpu_context *); · line 203
Quoted 10 lines
arch/arm64/kvm/hyp/nvhe/hyp-main.c +5
-1+unsigned long hcall_min =0 ;hcall_t hfn; +if (static_branch_unlikely (&kvm_protected_mode_initialized)) + hcall_min = __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize; + id -=KVM_HOST_SMCCC_ID (0 ); -if (unlikely (id >=ARRAY_SIZE (host_hcall))) +if (unlikely (id < hcall_min || id >=ARRAY_SIZE (host_hcall)))
So I can still issue a pkvm_prot_finalize after finalisation? Seems odd. As hcall_min has to be inclusive, you probably want it to be set to __KVM_HOST_SMCCC_FUNC___pkvm_host_share_hyp once protected.
Thanks,
M.
Signature · 5 lines
Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Will Deacon
maintainerRe: [PATCH 5/5] KVM: arm64: Disable privileged hypercalls after pKVM finalisation
Quoted 130 lines · Marc Zyngier
Quoted 128 lines · Will Deacon
After pKVM has been 'finalised' using the
__pkvm_prot_finalizehypercall, the calling CPU will have a Stage-2 translation enabled to prevent access to memory pages owned by EL2.Although this forms a significant part of the process to deprivilege the host kernel, we also need to ensure that the hypercall interface is reduced so that the EL2 code cannot, for example, be re-initialised using a new set of vectors.
Re-order the hypercalls so that only a suffix remains available after finalisation of pKVM.
Cc Marc Zyngier <[email protected]>Cc Quentin Perret <[email protected]>Signed-off-by Will Deacon <[email protected]>--- arch/arm64/include/asm/kvm_asm.h | 43 ++++++++++++++++-------------- arch/arm64/kvm/hyp/nvhe/hyp-main.c | 26 +++++++++++------- 2 files changed, 39 insertions(+), 30 deletions(-)arch/arm64/include/asm/kvm_asm.h +23
-20@@ -43,27 +43,30 @@#define KVM_HOST_SMCCC_FUNC (name) KVM_HOST_SMCCC_ID(__KVM_HOST_SMCCC_FUNC_##name) +/* Hypercalls available only prior to pKVM finalisation */ #define __KVM_HOST_SMCCC_FUNC___kvm_hyp_init 0 -#define __KVM_HOST_SMCCC_FUNC___kvm_vcpu_run 1 -#define __KVM_HOST_SMCCC_FUNC___kvm_flush_vm_context 2 -#define __KVM_HOST_SMCCC_FUNC___kvm_tlb_flush_vmid_ipa 3 -#define __KVM_HOST_SMCCC_FUNC___kvm_tlb_flush_vmid 4 -#define __KVM_HOST_SMCCC_FUNC___kvm_flush_cpu_context 5 -#define __KVM_HOST_SMCCC_FUNC___kvm_timer_set_cntvoff 6 -#define __KVM_HOST_SMCCC_FUNC___kvm_enable_ssbs 7 -#define __KVM_HOST_SMCCC_FUNC___vgic_v3_get_gic_config 8 -#define __KVM_HOST_SMCCC_FUNC___vgic_v3_read_vmcr 9 -#define __KVM_HOST_SMCCC_FUNC___vgic_v3_write_vmcr 10 -#define __KVM_HOST_SMCCC_FUNC___vgic_v3_init_lrs 11 -#define __KVM_HOST_SMCCC_FUNC___kvm_get_mdcr_el2 12 -#define __KVM_HOST_SMCCC_FUNC___vgic_v3_save_aprs 13 -#define __KVM_HOST_SMCCC_FUNC___vgic_v3_restore_aprs 14 -#define __KVM_HOST_SMCCC_FUNC___pkvm_init 15 -#define __KVM_HOST_SMCCC_FUNC___pkvm_host_share_hyp 16 -#define __KVM_HOST_SMCCC_FUNC___pkvm_create_private_mapping 17 -#define __KVM_HOST_SMCCC_FUNC___pkvm_cpu_set_vector 18 -#define __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize 19 -#define __KVM_HOST_SMCCC_FUNC___kvm_adjust_pc 20 +#define __KVM_HOST_SMCCC_FUNC___kvm_get_mdcr_el2 1 +#define __KVM_HOST_SMCCC_FUNC___pkvm_init 2 +#define __KVM_HOST_SMCCC_FUNC___pkvm_create_private_mapping 3 +#define __KVM_HOST_SMCCC_FUNC___pkvm_cpu_set_vector 4 +#define __KVM_HOST_SMCCC_FUNC___kvm_enable_ssbs 5 +#define __KVM_HOST_SMCCC_FUNC___vgic_v3_init_lrs 6 +#define __KVM_HOST_SMCCC_FUNC___vgic_v3_get_gic_config 7 +#define __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize 8 + +/* Hypercalls available after pKVM finalisation */ +#define __KVM_HOST_SMCCC_FUNC___pkvm_host_share_hyp 9 +#define __KVM_HOST_SMCCC_FUNC___kvm_adjust_pc 10 +#define __KVM_HOST_SMCCC_FUNC___kvm_vcpu_run 11 +#define __KVM_HOST_SMCCC_FUNC___kvm_flush_vm_context 12 +#define __KVM_HOST_SMCCC_FUNC___kvm_tlb_flush_vmid_ipa 13 +#define __KVM_HOST_SMCCC_FUNC___kvm_tlb_flush_vmid 14 +#define __KVM_HOST_SMCCC_FUNC___kvm_flush_cpu_context 15 +#define __KVM_HOST_SMCCC_FUNC___kvm_timer_set_cntvoff 16 +#define __KVM_HOST_SMCCC_FUNC___vgic_v3_read_vmcr 17 +#define __KVM_HOST_SMCCC_FUNC___vgic_v3_write_vmcr 18 +#define __KVM_HOST_SMCCC_FUNC___vgic_v3_save_aprs 19 +#define __KVM_HOST_SMCCC_FUNC___vgic_v3_restore_aprs 20#ifndef __ASSEMBLY__arch/arm64/kvm/hyp/nvhe/hyp-main.c +15
-9@@ -165,36 +165,42 @@ typedef void (*hcall_t)(struct kvm_cpu_context *);#define HANDLE_FUNC (x) [__KVM_HOST_SMCCC_FUNC_##x] = (hcall_t)handle_##xstatic const hcall_t host_hcall[] = { -HANDLE_FUNC (__kvm_vcpu_run), +/* ___kvm_hyp_init */ +HANDLE_FUNC (__kvm_get_mdcr_el2), +HANDLE_FUNC (__pkvm_init), +HANDLE_FUNC (__pkvm_create_private_mapping), +HANDLE_FUNC (__pkvm_cpu_set_vector), +HANDLE_FUNC (__kvm_enable_ssbs), +HANDLE_FUNC (__vgic_v3_init_lrs), +HANDLE_FUNC (__pkvm_prot_finalize), + +HANDLE_FUNC (__pkvm_host_share_hyp),HANDLE_FUNC (__kvm_adjust_pc), +HANDLE_FUNC (__kvm_vcpu_run),HANDLE_FUNC (__kvm_flush_vm_context),HANDLE_FUNC (__kvm_tlb_flush_vmid_ipa),HANDLE_FUNC (__kvm_tlb_flush_vmid),HANDLE_FUNC (__kvm_flush_cpu_context),HANDLE_FUNC (__kvm_timer_set_cntvoff), -HANDLE_FUNC (__kvm_enable_ssbs),HANDLE_FUNC (__vgic_v3_get_gic_config),HANDLE_FUNC (__vgic_v3_read_vmcr),HANDLE_FUNC (__vgic_v3_write_vmcr), -HANDLE_FUNC (__vgic_v3_init_lrs), -HANDLE_FUNC (__kvm_get_mdcr_el2),HANDLE_FUNC (__vgic_v3_save_aprs),HANDLE_FUNC (__vgic_v3_restore_aprs), -HANDLE_FUNC (__pkvm_init), -HANDLE_FUNC (__pkvm_cpu_set_vector), -HANDLE_FUNC (__pkvm_host_share_hyp), -HANDLE_FUNC (__pkvm_create_private_mapping), -HANDLE_FUNC (__pkvm_prot_finalize), };static void handle_host_hcall (struct kvm_cpu_context *host_ctxt) { DECLARE_REG(unsigned long, id, host_ctxt, 0); +unsigned long hcall_min =0 ;hcall_t hfn; +if (static_branch_unlikely (&kvm_protected_mode_initialized)) + hcall_min = __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize; + id -=KVM_HOST_SMCCC_ID (0 );
Quoted 6 lines
arch/arm64/kvm/hyp/nvhe/hyp-main.c typedef void (*hcall_t)(struct kvm_cpu_context *); · line 203
Quoted 2 lines
arch/arm64/kvm/hyp/nvhe/hyp-main.c +1
-1-if (unlikely (id >=ARRAY_SIZE (host_hcall))) +if (unlikely (id < hcall_min || id >=ARRAY_SIZE (host_hcall)))So I can still issue a
pkvm_prot_finalizeafter finalisation? Seems odd. Ashcall_minhas to be inclusive, you probably want it to be set to__KVM_HOST_SMCCC_FUNC___pkvm_host_share_hyponce protected.
Yeah, I ended up addresing that one in the previous patch. The problem is that we need to allow pkvm_prot_finalize to be called on each CPU, so I think we'd end up having an extra "really finalize damnit!" call to be issued _once_ after each CPU is done with the finalisation if we want to lock it down.
The approach I took instead is to make pkvm_prot_finalize return -EBUSY if it's called on a CPU where it's already been called.
Will
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Marc Zyngier
maintainerRe: [PATCH 5/5] KVM: arm64: Disable privileged hypercalls after pKVM finalisation
Quoted 4 lines · Will Deacon
Quoted 2 lines · Marc Zyngier
On Thu, 23 Sep 2021 12:22:56 +0100,
Will Deacon <[email protected]> wrote:
[...]
Quoted 18 lines
Quoted 11 lines
Quoted 11 lines
static void
handle_host_hcall(struct kvm_cpu_context *host_ctxt){arch/arm64/kvm/hyp/nvhe/hyp-main.c +4
-0DECLARE_REG(unsigned long, id, host_ctxt, 0); +unsigned long hcall_min =0 ;hcall_t hfn; +if (static_branch_unlikely (&kvm_protected_mode_initialized)) + hcall_min = __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize; + id -=KVM_HOST_SMCCC_ID (0 );Quoted 6 lines
arch/arm64/kvm/hyp/nvhe/hyp-main.c typedef void (*hcall_t)(struct kvm_cpu_context *); · line 203
Quoted 2 lines
arch/arm64/kvm/hyp/nvhe/hyp-main.c +1
-1-if (unlikely (id >=ARRAY_SIZE (host_hcall))) +if (unlikely (id < hcall_min || id >=ARRAY_SIZE (host_hcall)))So I can still issue a
pkvm_prot_finalizeafter finalisation? Seems odd. Ashcall_minhas to be inclusive, you probably want it to be set to__KVM_HOST_SMCCC_FUNC___pkvm_host_share_hyponce protected.
Quoted 8 lines
Yeah, I ended up addresing that one in the previous patch. The problem is that we need to allow
pkvm_prot_finalizeto be called on each CPU, so I think we'd end up having an extra "really finalize damnit!" call to be issued _once_ after each CPU is done with the finalisation if we want to lock it down.The approach I took instead is to make
pkvm_prot_finalizereturn-EBUSYif it's called on a CPU where it's already been called.
Ah, I see. Serves me right for reading patches out of order. Finalise is of course per-CPU, and the static key global. Epic fail.
Probably deserves a comment, because I'm surely going to jump at that again in three months.
Thanks,
M.
Signature · 5 lines
Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Will Deacon
maintainerRe: [PATCH 5/5] KVM: arm64: Disable privileged hypercalls after pKVM finalisation
In reply to Will DeaconQuoted 19 lines · Will Deacon
After pKVM has been 'finalised' using the
__pkvm_prot_finalizehypercall, the calling CPU will have a Stage-2 translation enabled to prevent access to memory pages owned by EL2.Although this forms a significant part of the process to deprivilege the host kernel, we also need to ensure that the hypercall interface is reduced so that the EL2 code cannot, for example, be re-initialised using a new set of vectors.
Re-order the hypercalls so that only a suffix remains available after finalisation of pKVM.
Cc Marc Zyngier <[email protected]>Cc Quentin Perret <[email protected]>Signed-off-by Will Deacon <[email protected]>--- arch/arm64/include/asm/kvm_asm.h | 43 ++++++++++++++++-------------- arch/arm64/kvm/hyp/nvhe/hyp-main.c | 26 +++++++++++------- 2 files changed, 39 insertions(+), 30 deletions(-)
[...]
Quoted 29 lines
arch/arm64/kvm/hyp/nvhe/hyp-main.c +11
-2@@ -165,36 +165,42 @@ typedef void (*hcall_t)(struct kvm_cpu_context *);#define HANDLE_FUNC (x) [__KVM_HOST_SMCCC_FUNC_##x] = (hcall_t)handle_##xstatic const hcall_t host_hcall[] = { -HANDLE_FUNC (__kvm_vcpu_run), +/* ___kvm_hyp_init */ +HANDLE_FUNC (__kvm_get_mdcr_el2), +HANDLE_FUNC (__pkvm_init), +HANDLE_FUNC (__pkvm_create_private_mapping), +HANDLE_FUNC (__pkvm_cpu_set_vector), +HANDLE_FUNC (__kvm_enable_ssbs), +HANDLE_FUNC (__vgic_v3_init_lrs), +HANDLE_FUNC (__pkvm_prot_finalize), + +HANDLE_FUNC (__pkvm_host_share_hyp),HANDLE_FUNC (__kvm_adjust_pc), +HANDLE_FUNC (__kvm_vcpu_run),HANDLE_FUNC (__kvm_flush_vm_context),HANDLE_FUNC (__kvm_tlb_flush_vmid_ipa),HANDLE_FUNC (__kvm_tlb_flush_vmid),HANDLE_FUNC (__kvm_flush_cpu_context),HANDLE_FUNC (__kvm_timer_set_cntvoff), -HANDLE_FUNC (__kvm_enable_ssbs),HANDLE_FUNC (__vgic_v3_get_gic_config),HANDLE_FUNC (__vgic_v3_read_vmcr),
arch/arm64/kvm/hyp/nvhe/hyp-main.c typedef void (*hcall_t)(struct kvm_cpu_context *); · line 187
Quoted 10 lines
arch/arm64/kvm/hyp/nvhe/hyp-main.c +0
-7HANDLE_FUNC (__vgic_v3_write_vmcr), -HANDLE_FUNC (__vgic_v3_init_lrs), -HANDLE_FUNC (__kvm_get_mdcr_el2),HANDLE_FUNC (__vgic_v3_save_aprs), HANDLE_FUNC(__vgic_v3_restore_aprs), -HANDLE_FUNC (__pkvm_init), -HANDLE_FUNC (__pkvm_cpu_set_vector), -HANDLE_FUNC (__pkvm_host_share_hyp), -HANDLE_FUNC (__pkvm_create_private_mapping), - HANDLE_FUNC(__pkvm_prot_finalize),
Not that it makes any functional difference, but I was trying to keep this in numerical order and evidently didn't manage it after renumbering __vgic_v3_get_gic_config. Will fix for v2.
Will
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Marc Zyngier
maintainerQuoted 7 lines · Will Deacon
Hi folks,
This series restricts the hypercalls available to the KVM host on arm64 when pKVM is enabled so that it is not possible for the host to use them to replace the EL2 component with something else.
Quoted 7 lines
This occurs in two stages: when switching to the pKVM vectors, the stub hypercalls are removed and then later when pKVM is finalised, the pKVM init hypercalls are removed.
There are still a few dubious calls remaining in terms of protecting the guest (e.g.
__kvm_adjust_pc) but these will be dealt with later when we have more VM state at EL2 to play with.
Yup. This particular one should have an equivalent at EL2 and pending exceptions committed to the state before exiting to EL1.
M.
Signature · 5 lines
Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm