Hi,
Changes since v5 [1]:
- Rebase on 5.15-rc2
- Include Marc's early exception handlers in the series
- Refactoring and fixes (Drew, Marc)
This patch series adds support for restricting CPU features for protected VMs
in KVM (pKVM). For more background, please refer to the previous series [1].
This series is based on 5.15-rc2. You can find the applied series here [2].
Cheers,
/fuad
[1] https://lore.kernel.org/kvmarm/20210827101609.2808181-1-tabba@google.com/
[2] https://android-kvm.googlesource.com/linux/+/refs/heads/tabba/el2_fixed_feature_v6
Fuad Tabba (9):
KVM: arm64: Add missing FORCE prerequisite in Makefile
KVM: arm64: Pass struct kvm to per-EC handlers
KVM: arm64: Add missing field descriptor for MDCR_EL2
KVM: arm64: Simplify masking out MTE in feature id reg
KVM: arm64: Add handlers for protected VM System Registers
KVM: arm64: Initialize trap registers for protected VMs
KVM: arm64: Move sanitized copies of CPU features
KVM: arm64: Trap access to pVM restricted features
KVM: arm64: Handle protected guests at 32 bits
Marc Zyngier (3):
KVM: arm64: Move __get_fault_info() and co into their own include file
KVM: arm64: Don't include switch.h into nvhe/kvm-main.c
KVM: arm64: Move early handlers to per-EC handlers
arch/arm64/include/asm/kvm_arm.h | 1 +
arch/arm64/include/asm/kvm_asm.h | 1 +
arch/arm64/include/asm/kvm_fixed_config.h | 195 ++++++++
arch/arm64/include/asm/kvm_host.h | 2 +
arch/arm64/include/asm/kvm_hyp.h | 5 +
arch/arm64/kvm/arm.c | 13 +
arch/arm64/kvm/hyp/include/hyp/fault.h | 75 ++++
arch/arm64/kvm/hyp/include/hyp/switch.h | 221 ++++-----
arch/arm64/kvm/hyp/include/nvhe/pkvm.h | 14 +
arch/arm64/kvm/hyp/include/nvhe/sys_regs.h | 28 ++
arch/arm64/kvm/hyp/nvhe/Makefile | 4 +-
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 12 +-
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 8 +-
arch/arm64/kvm/hyp/nvhe/pkvm.c | 186 ++++++++
arch/arm64/kvm/hyp/nvhe/switch.c | 117 +++++
arch/arm64/kvm/hyp/nvhe/sys_regs.c | 494 +++++++++++++++++++++
arch/arm64/kvm/hyp/vhe/switch.c | 17 +
arch/arm64/kvm/sys_regs.c | 10 +-
18 files changed, 1257 insertions(+), 146 deletions(-)
create mode 100644 arch/arm64/include/asm/kvm_fixed_config.h
create mode 100644 arch/arm64/kvm/hyp/include/hyp/fault.h
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/pkvm.h
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/sys_regs.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/pkvm.c
create mode 100644 arch/arm64/kvm/hyp/nvhe/sys_regs.c
base-commit: e4e737bb5c170df6135a127739a9e6148ee3da82
--
2.33.0.464.g1972c5931b-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marc Zyngier <maz@kernel.org>
In order to avoid including the whole of the switching helpers
in unrelated files, move the __get_fault_info() and related helpers
into their own include file.
Signed-off-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/kvm/hyp/include/hyp/fault.h | 75 +++++++++++++++++++++++++
arch/arm64/kvm/hyp/include/hyp/switch.h | 61 +-------------------
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 2 +-
3 files changed, 77 insertions(+), 61 deletions(-)
create mode 100644 arch/arm64/kvm/hyp/include/hyp/fault.h
@@ -0,0 +1,75 @@+// SPDX-License-Identifier: GPL-2.0-only+/*+*Copyright(C)2015-ARMLtd+*Author:MarcZyngier<marc.zyngier@arm.com>+*/++#ifndef __ARM64_KVM_HYP_FAULT_H__+#define __ARM64_KVM_HYP_FAULT_H__++#include<asm/kvm_asm.h>+#include<asm/kvm_emulate.h>+#include<asm/kvm_hyp.h>+#include<asm/kvm_mmu.h>++staticinlinebool__translate_far_to_hpfar(u64far,u64*hpfar)+{+u64par,tmp;++/*+*ResolvetheIPAthehardwayusingtheguestVA.+*+*Stage-1translationalreadyvalidatedthememoryaccess+*rights.Assuch,wecanusetheEL1translationregime,and+*don'thavetodistinguishbetweenEL0andEL1access.+*+*Wedoneedtosave/restorePAR_EL1though,aswehaven't+*savedtheguestcontextyet,andwemayreturnearly...+*/+par=read_sysreg_par();+if(!__kvm_at("s1e1r",far))+tmp=read_sysreg_par();+else+tmp=SYS_PAR_EL1_F;/* back to the guest */+write_sysreg(par,par_el1);++if(unlikely(tmp&SYS_PAR_EL1_F))+returnfalse;/* Translation failed, back to guest */++/* Convert PAR to HPFAR format */+*hpfar=PAR_TO_HPFAR(tmp);+returntrue;+}++staticinlinebool__get_fault_info(u64esr,structkvm_vcpu_fault_info*fault)+{+u64hpfar,far;++far=read_sysreg_el2(SYS_FAR);++/*+*TheHPFARcanbeinvalidifthestage2faultdidnot+*happenduringastage1pagetablewalk(theESR_EL2.S1PTW+*bitisclear)andoneofthetwofollowingcasesaretrue:+*1.Thefaultwasduetoapermissionfault+*2.Theprocessorcarrieserrata834220+*+*Therefore,forallnonS1PTWfaultswhereweeitherhavea+*permissionfaultortheerrataworkaroundisenabled,we+*resolvetheIPAusingtheATinstruction.+*/+if(!(esr&ESR_ELx_S1PTW)&&+(cpus_have_final_cap(ARM64_WORKAROUND_834220)||+(esr&ESR_ELx_FSC_TYPE)==FSC_PERM)){+if(!__translate_far_to_hpfar(far,&hpfar))+returnfalse;+}else{+hpfar=read_sysreg(hpfar_el2);+}++fault->far_el2=far;+fault->hpfar_el2=hpfar;+returntrue;+}++#endif
@@ -133,66 +134,6 @@ static inline void ___deactivate_traps(struct kvm_vcpu *vcpu)}}-staticinlinebool__translate_far_to_hpfar(u64far,u64*hpfar)-{-u64par,tmp;--/*-*ResolvetheIPAthehardwayusingtheguestVA.-*-*Stage-1translationalreadyvalidatedthememoryaccess-*rights.Assuch,wecanusetheEL1translationregime,and-*don'thavetodistinguishbetweenEL0andEL1access.-*-*Wedoneedtosave/restorePAR_EL1though,aswehaven't-*savedtheguestcontextyet,andwemayreturnearly...-*/-par=read_sysreg_par();-if(!__kvm_at("s1e1r",far))-tmp=read_sysreg_par();-else-tmp=SYS_PAR_EL1_F;/* back to the guest */-write_sysreg(par,par_el1);--if(unlikely(tmp&SYS_PAR_EL1_F))-returnfalse;/* Translation failed, back to guest */--/* Convert PAR to HPFAR format */-*hpfar=PAR_TO_HPFAR(tmp);-returntrue;-}--staticinlinebool__get_fault_info(u64esr,structkvm_vcpu_fault_info*fault)-{-u64hpfar,far;--far=read_sysreg_el2(SYS_FAR);--/*-*TheHPFARcanbeinvalidifthestage2faultdidnot-*happenduringastage1pagetablewalk(theESR_EL2.S1PTW-*bitisclear)andoneofthetwofollowingcasesaretrue:-*1.Thefaultwasduetoapermissionfault-*2.Theprocessorcarrieserrata834220-*-*Therefore,forallnonS1PTWfaultswhereweeitherhavea-*permissionfaultortheerrataworkaroundisenabled,we-*resolvetheIPAusingtheATinstruction.-*/-if(!(esr&ESR_ELx_S1PTW)&&-(cpus_have_final_cap(ARM64_WORKAROUND_834220)||-(esr&ESR_ELx_FSC_TYPE)==FSC_PERM)){-if(!__translate_far_to_hpfar(far,&hpfar))-returnfalse;-}else{-hpfar=read_sysreg(hpfar_el2);-}--fault->far_el2=far;-fault->hpfar_el2=hpfar;-returntrue;-}-staticinlinebool__populate_fault_info(structkvm_vcpu*vcpu){u8ec;
From: Marc Zyngier <maz@kernel.org>
Simplify the early exception handling by slicing the gigantic decoding
tree into a more manageable set of functions, similar to what we have
in handle_exit.c.
This will also make the structure reusable for pKVM's own early exit
handling.
Signed-off-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/kvm/hyp/include/hyp/switch.h | 160 ++++++++++++++----------
arch/arm64/kvm/hyp/nvhe/switch.c | 17 +++
arch/arm64/kvm/hyp/vhe/switch.c | 17 +++
3 files changed, 126 insertions(+), 68 deletions(-)
@@ -166,8 +157,13 @@ static inline void __hyp_sve_restore_guest(struct kvm_vcpu *vcpu)write_sysreg_el1(__vcpu_sys_reg(vcpu,ZCR_EL1),SYS_ZCR);}-/* Check for an FPSIMD/SVE trap and handle as appropriate */-staticinlinebool__hyp_handle_fpsimd(structkvm_vcpu*vcpu)+/*+*WetrapthefirstaccesstotheFP/SIMDtosavethehostcontextand+*restoretheguestcontextlazily.+*IfFP/SIMDisnotimplemented,handlethetrapandinjectanundefined+*instructionexceptiontotheguest.SimilarlyfortrappedSVEaccesses.+*/+staticboolkvm_hyp_handle_fpsimd(structkvm_vcpu*vcpu,u64*exit_code){boolsve_guest,sve_host;u8esr_ec;
@@ -185,9 +181,6 @@ static inline bool __hyp_handle_fpsimd(struct kvm_vcpu *vcpu)}esr_ec=kvm_vcpu_trap_get_class(vcpu);-if(esr_ec!=ESR_ELx_EC_FP_ASIMD&&-esr_ec!=ESR_ELx_EC_SVE)-returnfalse;/* Don't handle SVE traps for non-SVE vcpus here: */if(!sve_guest&&esr_ec!=ESR_ELx_EC_FP_ASIMD)
@@ -350,6 +343,87 @@ static inline bool __hyp_handle_ptrauth(struct kvm_vcpu *vcpu)returntrue;}+staticboolkvm_hyp_handle_sysreg(structkvm_vcpu*vcpu,u64*exit_code)+{+if(cpus_have_final_cap(ARM64_WORKAROUND_CAVIUM_TX2_219_TVM)&&+handle_tx2_tvm(vcpu))+returntrue;++if(static_branch_unlikely(&vgic_v3_cpuif_trap)&&+__vgic_v3_perform_cpuif_access(vcpu)==1)+returntrue;++returnfalse;+}++staticboolkvm_hyp_handle_cp15(structkvm_vcpu*vcpu,u64*exit_code)+{+if(static_branch_unlikely(&vgic_v3_cpuif_trap)&&+__vgic_v3_perform_cpuif_access(vcpu)==1)+returntrue;++returnfalse;+}++staticboolkvm_hyp_handle_iabt_low(structkvm_vcpu*vcpu,u64*exit_code)+{+if(!__populate_fault_info(vcpu))+returntrue;++returnfalse;+}++staticboolkvm_hyp_handle_dabt_low(structkvm_vcpu*vcpu,u64*exit_code)+{+if(!__populate_fault_info(vcpu))+returntrue;++if(static_branch_unlikely(&vgic_v2_cpuif_trap)){+boolvalid;++valid=kvm_vcpu_trap_get_fault_type(vcpu)==FSC_FAULT&&+kvm_vcpu_dabt_isvalid(vcpu)&&+!kvm_vcpu_abt_issea(vcpu)&&+!kvm_vcpu_abt_iss1tw(vcpu);++if(valid){+intret=__vgic_v2_perform_cpuif_access(vcpu);++if(ret==1)+returntrue;++/* Promote an illegal access to an SError.*/+if(ret==-1)+*exit_code=ARM_EXCEPTION_EL1_SERROR;+}+}++returnfalse;+}++typedefbool(*exit_handler_fn)(structkvm_vcpu*,u64*);++staticconstexit_handler_fn*kvm_get_exit_handler_array(void);++/*+*Allowthehypervisortohandletheexitwithanexithandlerifithasone.+*+*Returnstrueifthehypervisorhandledtheexit,andcontrolshouldgoback+*totheguest,orfalseifithasn't.+*/+staticinlineboolkvm_hyp_handle_exit(structkvm_vcpu*vcpu,u64*exit_code)+{+constexit_handler_fn*handlers=kvm_get_exit_handler_array();+exit_handler_fnfn;++fn=handlers[kvm_vcpu_trap_get_class(vcpu)];++if(fn)+returnfn(vcpu,exit_code);++returnfalse;+}+/**Returntruewhenwewereabletofixuptheguestexitandshouldreturnto*theguest,falsewhenweshouldrestorethehoststateandreturntothe
@@ -384,59 +458,9 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)if(*exit_code!=ARM_EXCEPTION_TRAP)gotoexit;-if(cpus_have_final_cap(ARM64_WORKAROUND_CAVIUM_TX2_219_TVM)&&-kvm_vcpu_trap_get_class(vcpu)==ESR_ELx_EC_SYS64&&-handle_tx2_tvm(vcpu))+/* Check if there's an exit handler and allow it to handle the exit. */+if(kvm_hyp_handle_exit(vcpu,exit_code))gotoguest;--/*-*WetrapthefirstaccesstotheFP/SIMDtosavethehostcontext-*andrestoretheguestcontextlazily.-*IfFP/SIMDisnotimplemented,handlethetrapandinjectan-*undefinedinstructionexceptiontotheguest.-*SimilarlyfortrappedSVEaccesses.-*/-if(__hyp_handle_fpsimd(vcpu))-gotoguest;--if(__hyp_handle_ptrauth(vcpu))-gotoguest;--if(!__populate_fault_info(vcpu))-gotoguest;--if(static_branch_unlikely(&vgic_v2_cpuif_trap)){-boolvalid;--valid=kvm_vcpu_trap_get_class(vcpu)==ESR_ELx_EC_DABT_LOW&&-kvm_vcpu_trap_get_fault_type(vcpu)==FSC_FAULT&&-kvm_vcpu_dabt_isvalid(vcpu)&&-!kvm_vcpu_abt_issea(vcpu)&&-!kvm_vcpu_abt_iss1tw(vcpu);--if(valid){-intret=__vgic_v2_perform_cpuif_access(vcpu);--if(ret==1)-gotoguest;--/* Promote an illegal access to an SError.*/-if(ret==-1)-*exit_code=ARM_EXCEPTION_EL1_SERROR;--gotoexit;-}-}--if(static_branch_unlikely(&vgic_v3_cpuif_trap)&&-(kvm_vcpu_trap_get_class(vcpu)==ESR_ELx_EC_SYS64||-kvm_vcpu_trap_get_class(vcpu)==ESR_ELx_EC_CP15_32)){-intret=__vgic_v3_perform_cpuif_access(vcpu);--if(ret==1)-gotoguest;-}-exit:/* Return to the host kernel and handle the exit */returnfalse;
@@ -158,6 +158,23 @@ static void __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)write_sysreg(pmu->events_host,pmcntenset_el0);}+staticconstexit_handler_fnhyp_exit_handlers[]={+[0...ESR_ELx_EC_MAX]=NULL,+[ESR_ELx_EC_CP15_32]=kvm_hyp_handle_cp15,+[ESR_ELx_EC_CP15_64]=kvm_hyp_handle_cp15,+[ESR_ELx_EC_SYS64]=kvm_hyp_handle_sysreg,+[ESR_ELx_EC_SVE]=kvm_hyp_handle_fpsimd,+[ESR_ELx_EC_FP_ASIMD]=kvm_hyp_handle_fpsimd,+[ESR_ELx_EC_IABT_LOW]=kvm_hyp_handle_iabt_low,+[ESR_ELx_EC_DABT_LOW]=kvm_hyp_handle_dabt_low,+[ESR_ELx_EC_PAC]=kvm_hyp_handle_ptrauth,+};++staticconstexit_handler_fn*kvm_get_exit_handler_array(void)+{+returnhyp_exit_handlers;+}+/* Switch to the guest for legacy non-VHE systems */int__kvm_vcpu_run(structkvm_vcpu*vcpu){
@@ -96,6 +96,23 @@ void deactivate_traps_vhe_put(struct kvm_vcpu *vcpu)__deactivate_traps_common(vcpu);}+staticconstexit_handler_fnhyp_exit_handlers[]={+[0...ESR_ELx_EC_MAX]=NULL,+[ESR_ELx_EC_CP15_32]=kvm_hyp_handle_cp15,+[ESR_ELx_EC_CP15_64]=kvm_hyp_handle_cp15,+[ESR_ELx_EC_SYS64]=kvm_hyp_handle_sysreg,+[ESR_ELx_EC_SVE]=kvm_hyp_handle_fpsimd,+[ESR_ELx_EC_FP_ASIMD]=kvm_hyp_handle_fpsimd,+[ESR_ELx_EC_IABT_LOW]=kvm_hyp_handle_iabt_low,+[ESR_ELx_EC_DABT_LOW]=kvm_hyp_handle_dabt_low,+[ESR_ELx_EC_PAC]=kvm_hyp_handle_ptrauth,+};++staticconstexit_handler_fn*kvm_get_exit_handler_array(void)+{+returnhyp_exit_handlers;+}+/* Switch to the guest for VHE systems running in EL2 */staticint__kvm_vcpu_run_vhe(structkvm_vcpu*vcpu){
--
2.33.0.464.g1972c5931b-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
@@ -54,7 +54,7 @@ $(obj)/kvm_nvhe.tmp.o: $(obj)/hyp.lds $(addprefix $(obj)/,$(hyp-obj)) FORCE# runtime. Because the hypervisor is part of the kernel binary, relocations# produce a kernel VA. We enumerate relocations targeting hyp at build time# and convert the kernel VAs at those positions to hyp VAs.-$(obj)/hyp-reloc.S:$(obj)/kvm_nvhe.tmp.o$(obj)/gen-hyprel+$(obj)/hyp-reloc.S:$(obj)/kvm_nvhe.tmp.o$(obj)/gen-hyprelFORCE$(callif_changed,hyprel)# 5) Compile hyp-reloc.S and link it into the existing partially linked object.
--
2.33.0.464.g1972c5931b-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marc Zyngier <maz@kernel.org>
hyp-main.c includes switch.h while it only requires adjust-pc.h.
Fix it to remove an unnecessary dependency.
Signed-off-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
We need struct kvm to check for protected VMs to be able to pick
the right handlers for them in subsequent patches.
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/kvm/hyp/include/hyp/switch.h | 4 ++--
arch/arm64/kvm/hyp/nvhe/switch.c | 2 +-
arch/arm64/kvm/hyp/vhe/switch.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
It's not currently used. Added for completeness.
No functional change intended.
Suggested-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Fuad Tabba <redacted>
Reviewed-by: Andrew Jones <redacted>
---
arch/arm64/include/asm/kvm_arm.h | 1 +
1 file changed, 1 insertion(+)
Simplify code for hiding MTE support in feature id register when
MTE is not enabled/supported by KVM.
No functional change intended.
Signed-off-by: Fuad Tabba <redacted>
Reviewed-by: Andrew Jones <redacted>
---
arch/arm64/kvm/sys_regs.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
Add system register handlers for protected VMs. These cover Sys64
registers (including feature id registers), and debug.
No functional change intended as these are not hooked in yet to
the guest exit handlers introduced earlier. So when trapping is
triggered, the exit handlers let the host handle it, as before.
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/include/asm/kvm_fixed_config.h | 195 ++++++++
arch/arm64/include/asm/kvm_hyp.h | 5 +
arch/arm64/kvm/arm.c | 5 +
arch/arm64/kvm/hyp/include/nvhe/sys_regs.h | 28 ++
arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
arch/arm64/kvm/hyp/nvhe/sys_regs.c | 492 +++++++++++++++++++++
6 files changed, 726 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/include/asm/kvm_fixed_config.h
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/sys_regs.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/sys_regs.c
@@ -0,0 +1,492 @@+// SPDX-License-Identifier: GPL-2.0-only+/*+*Copyright(C)2021GoogleLLC+*Author:FuadTabba<tabba@google.com>+*/++#include<asm/kvm_asm.h>+#include<asm/kvm_fixed_config.h>+#include<asm/kvm_mmu.h>++#include<hyp/adjust_pc.h>++#include"../../sys_regs.h"++/*+*Copiesofthehost'sCPUfeaturesregistersholdingsanitizedvaluesathyp.+*/+u64id_aa64pfr0_el1_sys_val;+u64id_aa64pfr1_el1_sys_val;+u64id_aa64isar0_el1_sys_val;+u64id_aa64isar1_el1_sys_val;+u64id_aa64mmfr2_el1_sys_val;++staticinlinevoidinject_undef64(structkvm_vcpu*vcpu)+{+u32esr=(ESR_ELx_EC_UNKNOWN<<ESR_ELx_EC_SHIFT);++vcpu->arch.flags|=(KVM_ARM64_EXCEPT_AA64_EL1|+KVM_ARM64_EXCEPT_AA64_ELx_SYNC|+KVM_ARM64_PENDING_EXCEPTION);++__kvm_adjust_pc(vcpu);++write_sysreg_el1(esr,SYS_ESR);+write_sysreg_el1(read_sysreg_el2(SYS_ELR),SYS_ELR);+}++/*+*Injectanunknown/undefinedexceptiontoanAArch64guestwhilemostofits+*sysregsarelive.+*/+void__inject_undef64(structkvm_vcpu*vcpu)+{+*vcpu_pc(vcpu)=read_sysreg_el2(SYS_ELR);+*vcpu_cpsr(vcpu)=read_sysreg_el2(SYS_SPSR);++inject_undef64(vcpu);++write_sysreg_el2(*vcpu_pc(vcpu),SYS_ELR);+write_sysreg_el2(*vcpu_cpsr(vcpu),SYS_SPSR);+}++/*+*Accessorforundefinedaccesses.+*/+staticboolundef_access(structkvm_vcpu*vcpu,+structsys_reg_params*p,+conststructsys_reg_desc*r)+{+__inject_undef64(vcpu);+returnfalse;+}++/*+*Returnstherestrictedfeaturesvaluesofthefeatureregisterbasedonthe+*limitationsinrestrict_fields.+*Afeatureidfieldvalueof0b0000doesnotimposeanyrestrictions.+*Note:Useonlyforunsignedfeaturefieldvalues.+*/+staticu64get_restricted_features_unsigned(u64sys_reg_val,+u64restrict_fields)+{+u64value=0UL;+u64mask=GENMASK_ULL(ARM64_FEATURE_FIELD_BITS-1,0);++/*+*AccordingtotheArmArchitectureReferenceManual,featurefields+*useincreasingvaluestoindicateincreasesinfunctionality.+*Iterateovertherestrictedfeaturefieldsandcalculatetheminimum+*unsignedvaluebetweentheonesupportedbythesystem,andwhatthe+*valueisbeingrestrictedto.+*/+while(sys_reg_val&&restrict_fields){+value|=min(sys_reg_val&mask,restrict_fields&mask);+sys_reg_val&=~mask;+restrict_fields&=~mask;+mask<<=ARM64_FEATURE_FIELD_BITS;+}++returnvalue;+}++/*+*FunctionsthatreturnthevalueoffeatureidregistersforprotectedVMs+*basedonallowedfeatures,systemfeatures,andKVMsupport.+*/++u64get_pvm_id_aa64pfr0(conststructkvm_vcpu*vcpu)+{+conststructkvm*kvm=(conststructkvm*)kern_hyp_va(vcpu->kvm);+u64set_mask=0;+u64allow_mask=PVM_ID_AA64PFR0_ALLOW;++if(!vcpu_has_sve(vcpu))+allow_mask&=~ARM64_FEATURE_MASK(ID_AA64PFR0_SVE);++set_mask|=get_restricted_features_unsigned(id_aa64pfr0_el1_sys_val,+PVM_ID_AA64PFR0_RESTRICT_UNSIGNED);++/* Spectre and Meltdown mitigation in KVM */+set_mask|=FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2),+(u64)kvm->arch.pfr0_csv2);+set_mask|=FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3),+(u64)kvm->arch.pfr0_csv3);++return(id_aa64pfr0_el1_sys_val&allow_mask)|set_mask;+}++u64get_pvm_id_aa64pfr1(conststructkvm_vcpu*vcpu)+{+conststructkvm*kvm=(conststructkvm*)kern_hyp_va(vcpu->kvm);+u64allow_mask=PVM_ID_AA64PFR1_ALLOW;++if(!kvm_has_mte(kvm))+allow_mask&=~ARM64_FEATURE_MASK(ID_AA64PFR1_MTE);++returnid_aa64pfr1_el1_sys_val&allow_mask;+}++u64get_pvm_id_aa64zfr0(conststructkvm_vcpu*vcpu)+{+/*+*NosupportforScalableVectors,therefore,hyphasnosanitized+*copyofthefeatureidregister.+*/+BUILD_BUG_ON(PVM_ID_AA64ZFR0_ALLOW!=0ULL);+return0;+}++u64get_pvm_id_aa64dfr0(conststructkvm_vcpu*vcpu)+{+/*+*Nosupportfordebug,includingbreakpoints,andwatchpoints,+*therefore,pKVMhasnosanitizedcopyofthefeatureidregister.+*/+BUILD_BUG_ON(PVM_ID_AA64DFR0_ALLOW!=0ULL);+return0;+}++u64get_pvm_id_aa64dfr1(conststructkvm_vcpu*vcpu)+{+/*+*Nosupportfordebug,therefore,hyphasnosanitizedcopyofthe+*featureidregister.+*/+BUILD_BUG_ON(PVM_ID_AA64DFR1_ALLOW!=0ULL);+return0;+}++u64get_pvm_id_aa64afr0(conststructkvm_vcpu*vcpu)+{+/*+*Nosupportforimplementationdefinedfeatures,therefore,hyphasno+*sanitizedcopyofthefeatureidregister.+*/+BUILD_BUG_ON(PVM_ID_AA64AFR0_ALLOW!=0ULL);+return0;+}++u64get_pvm_id_aa64afr1(conststructkvm_vcpu*vcpu)+{+/*+*Nosupportforimplementationdefinedfeatures,therefore,hyphasno+*sanitizedcopyofthefeatureidregister.+*/+BUILD_BUG_ON(PVM_ID_AA64AFR1_ALLOW!=0ULL);+return0;+}++u64get_pvm_id_aa64isar0(conststructkvm_vcpu*vcpu)+{+returnid_aa64isar0_el1_sys_val&PVM_ID_AA64ISAR0_ALLOW;+}++u64get_pvm_id_aa64isar1(conststructkvm_vcpu*vcpu)+{+u64allow_mask=PVM_ID_AA64ISAR1_ALLOW;++if(!vcpu_has_ptrauth(vcpu))+allow_mask&=~(ARM64_FEATURE_MASK(ID_AA64ISAR1_APA)|+ARM64_FEATURE_MASK(ID_AA64ISAR1_API)|+ARM64_FEATURE_MASK(ID_AA64ISAR1_GPA)|+ARM64_FEATURE_MASK(ID_AA64ISAR1_GPI));++returnid_aa64isar1_el1_sys_val&allow_mask;+}++u64get_pvm_id_aa64mmfr0(conststructkvm_vcpu*vcpu)+{+u64set_mask;++set_mask=get_restricted_features_unsigned(id_aa64mmfr0_el1_sys_val,+PVM_ID_AA64MMFR0_RESTRICT_UNSIGNED);++return(id_aa64mmfr0_el1_sys_val&PVM_ID_AA64MMFR0_ALLOW)|set_mask;+}++u64get_pvm_id_aa64mmfr1(conststructkvm_vcpu*vcpu)+{+returnid_aa64mmfr1_el1_sys_val&PVM_ID_AA64MMFR1_ALLOW;+}++u64get_pvm_id_aa64mmfr2(conststructkvm_vcpu*vcpu)+{+returnid_aa64mmfr2_el1_sys_val&PVM_ID_AA64MMFR2_ALLOW;+}++/* Read a sanitized cpufeature ID register by its sys_reg_desc. */+staticu64read_id_reg(conststructkvm_vcpu*vcpu,+structsys_reg_descconst*r)+{+u32id=reg_to_encoding(r);++switch(id){+caseSYS_ID_AA64PFR0_EL1:+returnget_pvm_id_aa64pfr0(vcpu);+caseSYS_ID_AA64PFR1_EL1:+returnget_pvm_id_aa64pfr1(vcpu);+caseSYS_ID_AA64ZFR0_EL1:+returnget_pvm_id_aa64zfr0(vcpu);+caseSYS_ID_AA64DFR0_EL1:+returnget_pvm_id_aa64dfr0(vcpu);+caseSYS_ID_AA64DFR1_EL1:+returnget_pvm_id_aa64dfr1(vcpu);+caseSYS_ID_AA64AFR0_EL1:+returnget_pvm_id_aa64afr0(vcpu);+caseSYS_ID_AA64AFR1_EL1:+returnget_pvm_id_aa64afr1(vcpu);+caseSYS_ID_AA64ISAR0_EL1:+returnget_pvm_id_aa64isar0(vcpu);+caseSYS_ID_AA64ISAR1_EL1:+returnget_pvm_id_aa64isar1(vcpu);+caseSYS_ID_AA64MMFR0_EL1:+returnget_pvm_id_aa64mmfr0(vcpu);+caseSYS_ID_AA64MMFR1_EL1:+returnget_pvm_id_aa64mmfr1(vcpu);+caseSYS_ID_AA64MMFR2_EL1:+returnget_pvm_id_aa64mmfr2(vcpu);+default:+/*+*Shouldneverhappenbecauseallcasesarecoveredin+*pvm_sys_reg_descs[]below.+*/+WARN_ON(1);+break;+}++return0;+}++/*+*AccessorforAArch32featureidregisters.+*+*Thevalueoftheseregistersis"unknown"accordingtothespecifAArch32+*isn'tsupported.+*/+staticboolpvm_access_id_aarch32(structkvm_vcpu*vcpu,+structsys_reg_params*p,+conststructsys_reg_desc*r)+{+if(p->is_write)+returnundef_access(vcpu,p,r);++/*+*NosupportforAArch32guests,therefore,pKVMhasnosanitizedcopy+*ofAArch32featureidregisters.+*/+BUILD_BUG_ON(FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1),+PVM_ID_AA64PFR0_RESTRICT_UNSIGNED)>ID_AA64PFR0_ELx_64BIT_ONLY);++/* Use 0 for architecturally "unknown" values. */+p->regval=0;+returntrue;+}++/*+*AccessorforAArch64featureidregisters.+*+*Ifaccessisallowed,settheregvaltotheprotectedVM'sviewofthe+*registerandreturntrue.+*Otherwise,injectanundefinedexceptionandreturnfalse.+*/+staticboolpvm_access_id_aarch64(structkvm_vcpu*vcpu,+structsys_reg_params*p,+conststructsys_reg_desc*r)+{+if(p->is_write)+returnundef_access(vcpu,p,r);++p->regval=read_id_reg(vcpu,r);+returntrue;+}++/* Mark the specified system register as an AArch32 feature id register. */+#define AARCH32(REG) { SYS_DESC(REG), .access = pvm_access_id_aarch32 }++/* Mark the specified system register as an AArch64 feature id register. */+#define AARCH64(REG) { SYS_DESC(REG), .access = pvm_access_id_aarch64 }++/* Mark the specified system register as not being handled in hyp. */+#define HOST_HANDLED(REG) { SYS_DESC(REG), .access = NULL }++/*+*Architectedsystemregisters.+*Important:MustbesortedascendingbyOp0,Op1,CRn,CRm,Op2+*+*NOTE:Anythingnotexplicitlylistedhereis*restrictedbydefault*,i.e.,+*itwillleadtoinjectinganexceptionintotheguest.+*/+staticconststructsys_reg_descpvm_sys_reg_descs[]={+/* Cache maintenance by set/way operations are restricted. */++/* Debug and Trace Registers are restricted. */++/* AArch64 mappings of the AArch32 ID registers */+/* CRm=1 */+AARCH32(SYS_ID_PFR0_EL1),+AARCH32(SYS_ID_PFR1_EL1),+AARCH32(SYS_ID_DFR0_EL1),+AARCH32(SYS_ID_AFR0_EL1),+AARCH32(SYS_ID_MMFR0_EL1),+AARCH32(SYS_ID_MMFR1_EL1),+AARCH32(SYS_ID_MMFR2_EL1),+AARCH32(SYS_ID_MMFR3_EL1),++/* CRm=2 */+AARCH32(SYS_ID_ISAR0_EL1),+AARCH32(SYS_ID_ISAR1_EL1),+AARCH32(SYS_ID_ISAR2_EL1),+AARCH32(SYS_ID_ISAR3_EL1),+AARCH32(SYS_ID_ISAR4_EL1),+AARCH32(SYS_ID_ISAR5_EL1),+AARCH32(SYS_ID_MMFR4_EL1),+AARCH32(SYS_ID_ISAR6_EL1),++/* CRm=3 */+AARCH32(SYS_MVFR0_EL1),+AARCH32(SYS_MVFR1_EL1),+AARCH32(SYS_MVFR2_EL1),+AARCH32(SYS_ID_PFR2_EL1),+AARCH32(SYS_ID_DFR1_EL1),+AARCH32(SYS_ID_MMFR5_EL1),++/* AArch64 ID registers */+/* CRm=4 */+AARCH64(SYS_ID_AA64PFR0_EL1),+AARCH64(SYS_ID_AA64PFR1_EL1),+AARCH64(SYS_ID_AA64ZFR0_EL1),+AARCH64(SYS_ID_AA64DFR0_EL1),+AARCH64(SYS_ID_AA64DFR1_EL1),+AARCH64(SYS_ID_AA64AFR0_EL1),+AARCH64(SYS_ID_AA64AFR1_EL1),+AARCH64(SYS_ID_AA64ISAR0_EL1),+AARCH64(SYS_ID_AA64ISAR1_EL1),+AARCH64(SYS_ID_AA64MMFR0_EL1),+AARCH64(SYS_ID_AA64MMFR1_EL1),+AARCH64(SYS_ID_AA64MMFR2_EL1),++HOST_HANDLED(SYS_SCTLR_EL1),+HOST_HANDLED(SYS_ACTLR_EL1),+HOST_HANDLED(SYS_CPACR_EL1),++HOST_HANDLED(SYS_RGSR_EL1),+HOST_HANDLED(SYS_GCR_EL1),++/* Scalable Vector Registers are restricted. */++HOST_HANDLED(SYS_TTBR0_EL1),+HOST_HANDLED(SYS_TTBR1_EL1),+HOST_HANDLED(SYS_TCR_EL1),++HOST_HANDLED(SYS_APIAKEYLO_EL1),+HOST_HANDLED(SYS_APIAKEYHI_EL1),+HOST_HANDLED(SYS_APIBKEYLO_EL1),+HOST_HANDLED(SYS_APIBKEYHI_EL1),+HOST_HANDLED(SYS_APDAKEYLO_EL1),+HOST_HANDLED(SYS_APDAKEYHI_EL1),+HOST_HANDLED(SYS_APDBKEYLO_EL1),+HOST_HANDLED(SYS_APDBKEYHI_EL1),+HOST_HANDLED(SYS_APGAKEYLO_EL1),+HOST_HANDLED(SYS_APGAKEYHI_EL1),++HOST_HANDLED(SYS_AFSR0_EL1),+HOST_HANDLED(SYS_AFSR1_EL1),+HOST_HANDLED(SYS_ESR_EL1),++HOST_HANDLED(SYS_ERRIDR_EL1),+HOST_HANDLED(SYS_ERRSELR_EL1),+HOST_HANDLED(SYS_ERXFR_EL1),+HOST_HANDLED(SYS_ERXCTLR_EL1),+HOST_HANDLED(SYS_ERXSTATUS_EL1),+HOST_HANDLED(SYS_ERXADDR_EL1),+HOST_HANDLED(SYS_ERXMISC0_EL1),+HOST_HANDLED(SYS_ERXMISC1_EL1),++HOST_HANDLED(SYS_TFSR_EL1),+HOST_HANDLED(SYS_TFSRE0_EL1),++HOST_HANDLED(SYS_FAR_EL1),+HOST_HANDLED(SYS_PAR_EL1),++/* Performance Monitoring Registers are restricted. */++HOST_HANDLED(SYS_MAIR_EL1),+HOST_HANDLED(SYS_AMAIR_EL1),++/* Limited Ordering Regions Registers are restricted. */++HOST_HANDLED(SYS_VBAR_EL1),+HOST_HANDLED(SYS_DISR_EL1),++/* GIC CPU Interface registers are restricted. */++HOST_HANDLED(SYS_CONTEXTIDR_EL1),+HOST_HANDLED(SYS_TPIDR_EL1),++HOST_HANDLED(SYS_SCXTNUM_EL1),++HOST_HANDLED(SYS_CNTKCTL_EL1),++HOST_HANDLED(SYS_CCSIDR_EL1),+HOST_HANDLED(SYS_CLIDR_EL1),+HOST_HANDLED(SYS_CSSELR_EL1),+HOST_HANDLED(SYS_CTR_EL0),++/* Performance Monitoring Registers are restricted. */++HOST_HANDLED(SYS_TPIDR_EL0),+HOST_HANDLED(SYS_TPIDRRO_EL0),++HOST_HANDLED(SYS_SCXTNUM_EL0),++/* Activity Monitoring Registers are restricted. */++HOST_HANDLED(SYS_CNTP_TVAL_EL0),+HOST_HANDLED(SYS_CNTP_CTL_EL0),+HOST_HANDLED(SYS_CNTP_CVAL_EL0),++/* Performance Monitoring Registers are restricted. */++HOST_HANDLED(SYS_DACR32_EL2),+HOST_HANDLED(SYS_IFSR32_EL2),+HOST_HANDLED(SYS_FPEXC32_EL2),+};++/*+*HandlerforprotectedVMMSR,MRSorSysteminstructionexecution.+*+*Returnstrueifthehypervisorhashandledtheexit,andcontrolshouldgo+*backtotheguest,orfalseifithasn't,tobehandledbythehost.+*/+boolkvm_handle_pvm_sysreg(structkvm_vcpu*vcpu,u64*exit_code)+{+conststructsys_reg_desc*r;+structsys_reg_paramsparams;+unsignedlongesr=kvm_vcpu_get_esr(vcpu);+intRt=kvm_vcpu_sys_get_rt(vcpu);++params=esr_sys64_to_params(esr);+params.regval=vcpu_get_reg(vcpu,Rt);++r=find_reg(¶ms,pvm_sys_reg_descs,ARRAY_SIZE(pvm_sys_reg_descs));++/* Undefined access (RESTRICTED). */+if(r==NULL){+__inject_undef64(vcpu);+returntrue;+}++/* Handled by the host (HOST_HANDLED) */+if(r->access==NULL)+returnfalse;++/* Handled by hyp: skip instruction if instructed to do so. */+if(r->access(vcpu,¶ms,r))+__kvm_skip_instr(vcpu);++if(!params.is_write)+vcpu_set_reg(vcpu,Rt,params.regval);++returntrue;+}
--
2.33.0.464.g1972c5931b-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Protected VMs have more restricted features that need to be
trapped. Moreover, the host should not be trusted to set the
appropriate trapping registers and their values.
Initialize the trapping registers, i.e., hcr_el2, mdcr_el2, and
cptr_el2 at EL2 for protected guests, based on the values of the
guest's feature id registers.
No functional change intended as trap handlers introduced in the
previous patch are still not hooked in to the guest exit
handlers.
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/include/asm/kvm_asm.h | 1 +
arch/arm64/include/asm/kvm_host.h | 2 +
arch/arm64/kvm/arm.c | 8 ++
arch/arm64/kvm/hyp/include/nvhe/pkvm.h | 14 ++
arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 10 ++
arch/arm64/kvm/hyp/nvhe/pkvm.c | 186 +++++++++++++++++++++++++
7 files changed, 222 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/pkvm.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/pkvm.c
Move the sanitized copies of the CPU feature registers to the
recently created sys_regs.c. This consolidates all copies in a
more relevant file.
No functional change intended.
Acked-by: Will Deacon <will@kernel.org>
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 6 ------
arch/arm64/kvm/hyp/nvhe/sys_regs.c | 2 ++
2 files changed, 2 insertions(+), 6 deletions(-)
Trap accesses to restricted features for VMs running in protected
mode.
Access to feature registers are emulated, and only supported
features are exposed to protected VMs.
Accesses to restricted registers as well as restricted
instructions are trapped, and an undefined exception is injected
into the protected guests, i.e., with EC = 0x0 (unknown reason).
This EC is the one used, according to the Arm Architecture
Reference Manual, for unallocated or undefined system registers
or instructions.
Only affects the functionality of protected VMs. Otherwise,
should not affect non-protected VMs when KVM is running in
protected mode.
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/kvm/hyp/nvhe/switch.c | 60 ++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
@@ -28,6 +29,7 @@#include<asm/thread_info.h>#include<nvhe/mem_protect.h>+#include<nvhe/sys_regs.h>/* Non-VHE specific context */DEFINE_PER_CPU(structkvm_host_data,kvm_host_data);
@@ -158,6 +160,49 @@ static void __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)write_sysreg(pmu->events_host,pmcntenset_el0);}+/**+*HandlerforprotectedVMrestrictedexceptions.+*+*Injectanundefinedexceptionintotheguestandreturntruetoindicatethat+*thehypervisorhashandledtheexit,andcontrolshouldgobacktotheguest.+*/+staticboolkvm_handle_pvm_restricted(structkvm_vcpu*vcpu,u64*exit_code)+{+__inject_undef64(vcpu);+returntrue;+}++/**+*HandlerforprotectedVMMSR,MRSorSysteminstructionexecutioninAArch64.+*+*Returnstrueifthehypervisorhashandledtheexit,andcontrolshouldgo+*backtotheguest,orfalseifithasn't.+*/+staticboolkvm_handle_pvm_sys64(structkvm_vcpu*vcpu,u64*exit_code)+{+if(kvm_handle_pvm_sysreg(vcpu,exit_code))+returntrue;+else+returnkvm_hyp_handle_sysreg(vcpu,exit_code);+}++/**+*Handlerforprotectedfloating-pointandAdvancedSIMDaccesses.+*+*Returnstrueifthehypervisorhashandledtheexit,andcontrolshouldgo+*backtotheguest,orfalseifithasn't.+*/+staticboolkvm_handle_pvm_fpsimd(structkvm_vcpu*vcpu,u64*exit_code)+{+/* Linux guests assume support for floating-point and Advanced SIMD. */+BUILD_BUG_ON(!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_FP),+PVM_ID_AA64PFR0_ALLOW));+BUILD_BUG_ON(!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_ASIMD),+PVM_ID_AA64PFR0_ALLOW));++returnkvm_hyp_handle_fpsimd(vcpu,exit_code);+}+staticconstexit_handler_fnhyp_exit_handlers[]={[0...ESR_ELx_EC_MAX]=NULL,[ESR_ELx_EC_CP15_32]=kvm_hyp_handle_cp15,
Protected KVM does not support protected AArch32 guests. However,
it is possible for the guest to force run AArch32, potentially
causing problems. Add an extra check so that if the hypervisor
catches the guest doing that, it can prevent the guest from
running again by resetting vcpu->arch.target and returning
ARM_EXCEPTION_IL.
If this were to happen, The VMM can try and fix it by re-
initializing the vcpu with KVM_ARM_VCPU_INIT, however, this is
likely not possible for protected VMs.
Adapted from commit 22f553842b14 ("KVM: arm64: Handle Asymmetric
AArch32 systems")
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/kvm/hyp/nvhe/switch.c | 40 ++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
@@ -235,6 +235,43 @@ static const exit_handler_fn *kvm_get_exit_handler_array(struct kvm *kvm)returnhyp_exit_handlers;}+/*+*Someguests(e.g.,protectedVMs)mightnotbeallowedtoruninAArch32.+*TheARMv8architecturedoesnotgivethehypervisoramechanismtopreventa+*guestfromdroppingtoAArch32EL0ifimplementedbytheCPU.Ifthe+*hypervisorspotsaguestinsuchastateensureitishandled,anddon't+*trustthehosttospotorfixit.Thecheckbelowisbasedontheonein+*kvm_arch_vcpu_ioctl_run().+*+*ReturnsfalseiftheguestraninAArch32whenitshouldn'thave,and+*thusshouldexittothehost,ortrueifatheguestrunloopcancontinue.+*/+staticboolhandle_aarch32_guest(structkvm_vcpu*vcpu,u64*exit_code)+{+structkvm*kvm=(structkvm*)kern_hyp_va(vcpu->kvm);+boolis_aarch32_allowed=+FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL0),+get_pvm_id_aa64pfr0(vcpu))>=+ID_AA64PFR0_ELx_32BIT_64BIT;++if(kvm_vm_is_protected(kvm)&&+vcpu_mode_is_32bit(vcpu)&&+!is_aarch32_allowed){+/*+*Aswehavecaughttheguestred-handed,decidethatitisn't+*fitforpurposeanymorebymakingthevcpuinvalid.TheVMM+*cantryandfixitbyre-initializingthevcpuwith+*KVM_ARM_VCPU_INIT,however,thisislikelynotpossiblefor+*protectedVMs.+*/+vcpu->arch.target=-1;+*exit_code=ARM_EXCEPTION_IL;+returnfalse;+}++returntrue;+}+/* Switch to the guest for legacy non-VHE systems */int__kvm_vcpu_run(structkvm_vcpu*vcpu){
@@ -297,6 +334,9 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)/* Jump in the fire! */exit_code=__guest_enter(vcpu);+if(unlikely(!handle_aarch32_guest(vcpu,&exit_code)))+break;+/* And we're baaack! */}while(fixup_guest_exit(vcpu,&exit_code));
--
2.33.0.464.g1972c5931b-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
@@ -54,7 +54,7 @@ $(obj)/kvm_nvhe.tmp.o: $(obj)/hyp.lds $(addprefix $(obj)/,$(hyp-obj)) FORCE# runtime. Because the hypervisor is part of the kernel binary, relocations# produce a kernel VA. We enumerate relocations targeting hyp at build time# and convert the kernel VAs at those positions to hyp VAs.-$(obj)/hyp-reloc.S:$(obj)/kvm_nvhe.tmp.o$(obj)/gen-hyprel+$(obj)/hyp-reloc.S:$(obj)/kvm_nvhe.tmp.o$(obj)/gen-hyprelFORCE$(callif_changed,hyprel)# 5) Compile hyp-reloc.S and link it into the existing partially linked object.
From: Will Deacon <will@kernel.org> Date: 2021-09-30 13:08:41
On Wed, Sep 22, 2021 at 01:46:53PM +0100, Fuad Tabba wrote:
quoted hunk
From: Marc Zyngier <maz@kernel.org>
In order to avoid including the whole of the switching helpers
in unrelated files, move the __get_fault_info() and related helpers
into their own include file.
Signed-off-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/kvm/hyp/include/hyp/fault.h | 75 +++++++++++++++++++++++++
arch/arm64/kvm/hyp/include/hyp/switch.h | 61 +-------------------
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 2 +-
3 files changed, 77 insertions(+), 61 deletions(-)
create mode 100644 arch/arm64/kvm/hyp/include/hyp/fault.h
Strictly speaking, I think you're probably missing a bunch of includes here
(e.g. asm/sysreg.h, asm/kvm_arm.h, asm/cpufeature.h, ...)
Nits aside:
Acked-by: Will Deacon <will@kernel.org>
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-09-30 13:11:21
On Wed, Sep 22, 2021 at 01:46:54PM +0100, Fuad Tabba wrote:
quoted hunk
From: Marc Zyngier <maz@kernel.org>
hyp-main.c includes switch.h while it only requires adjust-pc.h.
Fix it to remove an unnecessary dependency.
Signed-off-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Will Deacon <will@kernel.org> Date: 2021-09-30 13:38:34
On Wed, Sep 22, 2021 at 01:46:55PM +0100, Fuad Tabba wrote:
quoted hunk
From: Marc Zyngier <maz@kernel.org>
Simplify the early exception handling by slicing the gigantic decoding
tree into a more manageable set of functions, similar to what we have
in handle_exit.c.
This will also make the structure reusable for pKVM's own early exit
handling.
Signed-off-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/kvm/hyp/include/hyp/switch.h | 160 ++++++++++++++----------
arch/arm64/kvm/hyp/nvhe/switch.c | 17 +++
arch/arm64/kvm/hyp/vhe/switch.c | 17 +++
3 files changed, 126 insertions(+), 68 deletions(-)
@@ -166,8 +157,13 @@ static inline void __hyp_sve_restore_guest(struct kvm_vcpu *vcpu)write_sysreg_el1(__vcpu_sys_reg(vcpu,ZCR_EL1),SYS_ZCR);}-/* Check for an FPSIMD/SVE trap and handle as appropriate */-staticinlinebool__hyp_handle_fpsimd(structkvm_vcpu*vcpu)+/*+*WetrapthefirstaccesstotheFP/SIMDtosavethehostcontextand+*restoretheguestcontextlazily.+*IfFP/SIMDisnotimplemented,handlethetrapandinjectanundefined+*instructionexceptiontotheguest.SimilarlyfortrappedSVEaccesses.+*/+staticboolkvm_hyp_handle_fpsimd(structkvm_vcpu*vcpu,u64*exit_code){boolsve_guest,sve_host;u8esr_ec;
@@ -185,9 +181,6 @@ static inline bool __hyp_handle_fpsimd(struct kvm_vcpu *vcpu)}esr_ec=kvm_vcpu_trap_get_class(vcpu);-if(esr_ec!=ESR_ELx_EC_FP_ASIMD&&-esr_ec!=ESR_ELx_EC_SVE)-returnfalse;/* Don't handle SVE traps for non-SVE vcpus here: */if(!sve_guest&&esr_ec!=ESR_ELx_EC_FP_ASIMD)
I think you're now calling this for the 64-bit CP15 access path, which I
don't think is correct. Maybe have separate handlers for 32-bit v4 64-bit
accesses?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
I think you're now calling this for the 64-bit CP15 access path, which I
don't think is correct. Maybe have separate handlers for 32-bit v4 64-bit
accesses?
Good point. The saving grace is that there is no 32bit-capable CPU that
requires GICv3 trapping, nor any 64bit cp15 register in the GICv3
architecture apart form the SGI registers, which are always handled at EL1.
So this code is largely academic!
Not providing a handler is the way to go for CP15-64.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
I think you're now calling this for the 64-bit CP15 access path, which I
don't think is correct. Maybe have separate handlers for 32-bit v4 64-bit
accesses?
FWIW, here's what I'm queuing as a fix.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marc Zyngier <maz@kernel.org> Date: 2021-10-04 17:29:35
Hi Fuad,
On Wed, 22 Sep 2021 13:47:03 +0100,
Fuad Tabba [off-list ref] wrote:
quoted hunk
Trap accesses to restricted features for VMs running in protected
mode.
Access to feature registers are emulated, and only supported
features are exposed to protected VMs.
Accesses to restricted registers as well as restricted
instructions are trapped, and an undefined exception is injected
into the protected guests, i.e., with EC = 0x0 (unknown reason).
This EC is the one used, according to the Arm Architecture
Reference Manual, for unallocated or undefined system registers
or instructions.
Only affects the functionality of protected VMs. Otherwise,
should not affect non-protected VMs when KVM is running in
protected mode.
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/kvm/hyp/nvhe/switch.c | 60 ++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
nit: drop the else.
I wonder though: what if there is an overlap between between the pVM
handling and the normal KVM stuff? Are we guaranteed that there is
none?
For example, ESR_ELx_EC_SYS64 is used when working around some bugs
(see the TX2 TVM handling). What happens if you return early and don't
let it happen? This has a huge potential for some bad breakage.
quoted hunk
+}++/**+ * Handler for protected floating-point and Advanced SIMD accesses.+ *+ * Returns true if the hypervisor has handled the exit, and control should go+ * back to the guest, or false if it hasn't.+ */+static bool kvm_handle_pvm_fpsimd(struct kvm_vcpu *vcpu, u64 *exit_code)+{+ /* Linux guests assume support for floating-point and Advanced SIMD. */+ BUILD_BUG_ON(!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_FP),+ PVM_ID_AA64PFR0_ALLOW));+ BUILD_BUG_ON(!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_ASIMD),+ PVM_ID_AA64PFR0_ALLOW));++ return kvm_hyp_handle_fpsimd(vcpu, exit_code);+}+ static const exit_handler_fn hyp_exit_handlers[] = { [0 ... ESR_ELx_EC_MAX] = NULL, [ESR_ELx_EC_CP15_32] = kvm_hyp_handle_cp15,
Heads up, this one was bogus, and I removed it in my patches[1].
But it really begs the question: given that you really don't want to
handle any AArch32 for protected VMs, why handling anything at all the
first place? You really should let the exit happen and let the outer
run loop deal with it.
Hi Marc,
On Mon, Oct 4, 2021 at 6:27 PM Marc Zyngier [off-list ref] wrote:
Hi Fuad,
On Wed, 22 Sep 2021 13:47:03 +0100,
Fuad Tabba [off-list ref] wrote:
quoted
Trap accesses to restricted features for VMs running in protected
mode.
Access to feature registers are emulated, and only supported
features are exposed to protected VMs.
Accesses to restricted registers as well as restricted
instructions are trapped, and an undefined exception is injected
into the protected guests, i.e., with EC = 0x0 (unknown reason).
This EC is the one used, according to the Arm Architecture
Reference Manual, for unallocated or undefined system registers
or instructions.
Only affects the functionality of protected VMs. Otherwise,
should not affect non-protected VMs when KVM is running in
protected mode.
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/kvm/hyp/nvhe/switch.c | 60 ++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
I wonder though: what if there is an overlap between between the pVM
handling and the normal KVM stuff? Are we guaranteed that there is
none?
For example, ESR_ELx_EC_SYS64 is used when working around some bugs
(see the TX2 TVM handling). What happens if you return early and don't
let it happen? This has a huge potential for some bad breakage.
This is a tough one. Especially because it's dealing with bugs, there
is no guarantee really. I think that for the TVM handling there is no
overlap and the TVM handling code in kvm_hyp_handle_sysreg() will be
invoked. However, workarounds could always be added, and if that
happens, we need to make sure that they're on all paths. One solution
is to make sure that such code is in a common function called by both
paths. Not sure how we could enforce that other than by documenting
it.
What do you think?
quoted
+}++/**+ * Handler for protected floating-point and Advanced SIMD accesses.+ *+ * Returns true if the hypervisor has handled the exit, and control should go+ * back to the guest, or false if it hasn't.+ */+static bool kvm_handle_pvm_fpsimd(struct kvm_vcpu *vcpu, u64 *exit_code)+{+ /* Linux guests assume support for floating-point and Advanced SIMD. */+ BUILD_BUG_ON(!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_FP),+ PVM_ID_AA64PFR0_ALLOW));+ BUILD_BUG_ON(!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_ASIMD),+ PVM_ID_AA64PFR0_ALLOW));++ return kvm_hyp_handle_fpsimd(vcpu, exit_code);+}+ static const exit_handler_fn hyp_exit_handlers[] = { [0 ... ESR_ELx_EC_MAX] = NULL, [ESR_ELx_EC_CP15_32] = kvm_hyp_handle_cp15,
Heads up, this one was bogus, and I removed it in my patches[1].
But it really begs the question: given that you really don't want to
handle any AArch32 for protected VMs, why handling anything at all the
first place? You really should let the exit happen and let the outer
run loop deal with it.
Protected KVM does not support protected AArch32 guests. However,
it is possible for the guest to force run AArch32, potentially
causing problems. Add an extra check so that if the hypervisor
catches the guest doing that, it can prevent the guest from
running again by resetting vcpu->arch.target and returning
ARM_EXCEPTION_IL.
If this were to happen, The VMM can try and fix it by re-
initializing the vcpu with KVM_ARM_VCPU_INIT, however, this is
likely not possible for protected VMs.
Adapted from commit 22f553842b14 ("KVM: arm64: Handle Asymmetric
AArch32 systems")
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/kvm/hyp/nvhe/switch.c | 40 ++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
Do we really need to go through this is_aarch32_allowed check?
Protected VMs don't have AArch32, and we don't have the infrastructure
to handle 32bit at all. For non-protected VMs, we already have what we
need at EL1. So the extra check only adds complexity.
quoted hunk
+ /*+ * As we have caught the guest red-handed, decide that it isn't+ * fit for purpose anymore by making the vcpu invalid. The VMM+ * can try and fix it by re-initializing the vcpu with+ * KVM_ARM_VCPU_INIT, however, this is likely not possible for+ * protected VMs.+ */+ vcpu->arch.target = -1;+ *exit_code = ARM_EXCEPTION_IL;+ return false;+ }++ return true;+}+ /* Switch to the guest for legacy non-VHE systems */ int __kvm_vcpu_run(struct kvm_vcpu *vcpu) {
@@ -297,6 +334,9 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu) /* Jump in the fire! */ exit_code = __guest_enter(vcpu);+ if (unlikely(!handle_aarch32_guest(vcpu, &exit_code)))+ break;+ /* And we're baaack! */ } while (fixup_guest_exit(vcpu, &exit_code));
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Andrew Jones <hidden> Date: 2021-10-05 08:55:06
On Wed, Sep 22, 2021 at 01:47:00PM +0100, Fuad Tabba wrote:
quoted hunk
Add system register handlers for protected VMs. These cover Sys64
registers (including feature id registers), and debug.
No functional change intended as these are not hooked in yet to
the guest exit handlers introduced earlier. So when trapping is
triggered, the exit handlers let the host handle it, as before.
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/include/asm/kvm_fixed_config.h | 195 ++++++++
arch/arm64/include/asm/kvm_hyp.h | 5 +
arch/arm64/kvm/arm.c | 5 +
arch/arm64/kvm/hyp/include/nvhe/sys_regs.h | 28 ++
arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
arch/arm64/kvm/hyp/nvhe/sys_regs.c | 492 +++++++++++++++++++++
6 files changed, 726 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/include/asm/kvm_fixed_config.h
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/sys_regs.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/sys_regs.c
@@ -0,0 +1,492 @@+// SPDX-License-Identifier: GPL-2.0-only+/*+*Copyright(C)2021GoogleLLC+*Author:FuadTabba<tabba@google.com>+*/++#include<asm/kvm_asm.h>+#include<asm/kvm_fixed_config.h>+#include<asm/kvm_mmu.h>++#include<hyp/adjust_pc.h>++#include"../../sys_regs.h"++/*+*Copiesofthehost'sCPUfeaturesregistersholdingsanitizedvaluesathyp.+*/+u64id_aa64pfr0_el1_sys_val;+u64id_aa64pfr1_el1_sys_val;+u64id_aa64isar0_el1_sys_val;+u64id_aa64isar1_el1_sys_val;+u64id_aa64mmfr2_el1_sys_val;++staticinlinevoidinject_undef64(structkvm_vcpu*vcpu)+{+u32esr=(ESR_ELx_EC_UNKNOWN<<ESR_ELx_EC_SHIFT);++vcpu->arch.flags|=(KVM_ARM64_EXCEPT_AA64_EL1|+KVM_ARM64_EXCEPT_AA64_ELx_SYNC|+KVM_ARM64_PENDING_EXCEPTION);++__kvm_adjust_pc(vcpu);++write_sysreg_el1(esr,SYS_ESR);+write_sysreg_el1(read_sysreg_el2(SYS_ELR),SYS_ELR);+}++/*+*Injectanunknown/undefinedexceptiontoanAArch64guestwhilemostofits+*sysregsarelive.+*/+void__inject_undef64(structkvm_vcpu*vcpu)+{+*vcpu_pc(vcpu)=read_sysreg_el2(SYS_ELR);+*vcpu_cpsr(vcpu)=read_sysreg_el2(SYS_SPSR);++inject_undef64(vcpu);++write_sysreg_el2(*vcpu_pc(vcpu),SYS_ELR);+write_sysreg_el2(*vcpu_cpsr(vcpu),SYS_SPSR);+}++/*+*Accessorforundefinedaccesses.+*/+staticboolundef_access(structkvm_vcpu*vcpu,+structsys_reg_params*p,+conststructsys_reg_desc*r)+{+__inject_undef64(vcpu);+returnfalse;+}++/*+*Returnstherestrictedfeaturesvaluesofthefeatureregisterbasedonthe+*limitationsinrestrict_fields.+*Afeatureidfieldvalueof0b0000doesnotimposeanyrestrictions.+*Note:Useonlyforunsignedfeaturefieldvalues.+*/+staticu64get_restricted_features_unsigned(u64sys_reg_val,+u64restrict_fields)+{+u64value=0UL;+u64mask=GENMASK_ULL(ARM64_FEATURE_FIELD_BITS-1,0);++/*+*AccordingtotheArmArchitectureReferenceManual,featurefields+*useincreasingvaluestoindicateincreasesinfunctionality.+*Iterateovertherestrictedfeaturefieldsandcalculatetheminimum+*unsignedvaluebetweentheonesupportedbythesystem,andwhatthe+*valueisbeingrestrictedto.+*/+while(sys_reg_val&&restrict_fields){+value|=min(sys_reg_val&mask,restrict_fields&mask);+sys_reg_val&=~mask;+restrict_fields&=~mask;+mask<<=ARM64_FEATURE_FIELD_BITS;+}++returnvalue;+}++/*+*FunctionsthatreturnthevalueoffeatureidregistersforprotectedVMs+*basedonallowedfeatures,systemfeatures,andKVMsupport.+*/++u64get_pvm_id_aa64pfr0(conststructkvm_vcpu*vcpu)+{+conststructkvm*kvm=(conststructkvm*)kern_hyp_va(vcpu->kvm);+u64set_mask=0;+u64allow_mask=PVM_ID_AA64PFR0_ALLOW;++if(!vcpu_has_sve(vcpu))+allow_mask&=~ARM64_FEATURE_MASK(ID_AA64PFR0_SVE);++set_mask|=get_restricted_features_unsigned(id_aa64pfr0_el1_sys_val,+PVM_ID_AA64PFR0_RESTRICT_UNSIGNED);++/* Spectre and Meltdown mitigation in KVM */+set_mask|=FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2),+(u64)kvm->arch.pfr0_csv2);+set_mask|=FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3),+(u64)kvm->arch.pfr0_csv3);++return(id_aa64pfr0_el1_sys_val&allow_mask)|set_mask;+}++u64get_pvm_id_aa64pfr1(conststructkvm_vcpu*vcpu)+{+conststructkvm*kvm=(conststructkvm*)kern_hyp_va(vcpu->kvm);+u64allow_mask=PVM_ID_AA64PFR1_ALLOW;++if(!kvm_has_mte(kvm))+allow_mask&=~ARM64_FEATURE_MASK(ID_AA64PFR1_MTE);++returnid_aa64pfr1_el1_sys_val&allow_mask;+}++u64get_pvm_id_aa64zfr0(conststructkvm_vcpu*vcpu)+{+/*+*NosupportforScalableVectors,therefore,hyphasnosanitized+*copyofthefeatureidregister.+*/+BUILD_BUG_ON(PVM_ID_AA64ZFR0_ALLOW!=0ULL);+return0;+}++u64get_pvm_id_aa64dfr0(conststructkvm_vcpu*vcpu)+{+/*+*Nosupportfordebug,includingbreakpoints,andwatchpoints,+*therefore,pKVMhasnosanitizedcopyofthefeatureidregister.+*/+BUILD_BUG_ON(PVM_ID_AA64DFR0_ALLOW!=0ULL);+return0;+}++u64get_pvm_id_aa64dfr1(conststructkvm_vcpu*vcpu)+{+/*+*Nosupportfordebug,therefore,hyphasnosanitizedcopyofthe+*featureidregister.+*/+BUILD_BUG_ON(PVM_ID_AA64DFR1_ALLOW!=0ULL);+return0;+}++u64get_pvm_id_aa64afr0(conststructkvm_vcpu*vcpu)+{+/*+*Nosupportforimplementationdefinedfeatures,therefore,hyphasno+*sanitizedcopyofthefeatureidregister.+*/+BUILD_BUG_ON(PVM_ID_AA64AFR0_ALLOW!=0ULL);+return0;+}++u64get_pvm_id_aa64afr1(conststructkvm_vcpu*vcpu)+{+/*+*Nosupportforimplementationdefinedfeatures,therefore,hyphasno+*sanitizedcopyofthefeatureidregister.+*/+BUILD_BUG_ON(PVM_ID_AA64AFR1_ALLOW!=0ULL);+return0;+}
Reading the same function five times make me wonder if a generator macro
wouldn't be better for these.
quoted hunk
++u64 get_pvm_id_aa64isar0(const struct kvm_vcpu *vcpu)+{+ return id_aa64isar0_el1_sys_val & PVM_ID_AA64ISAR0_ALLOW;+}++u64 get_pvm_id_aa64isar1(const struct kvm_vcpu *vcpu)+{+ u64 allow_mask = PVM_ID_AA64ISAR1_ALLOW;++ if (!vcpu_has_ptrauth(vcpu))+ allow_mask &= ~(ARM64_FEATURE_MASK(ID_AA64ISAR1_APA) |+ ARM64_FEATURE_MASK(ID_AA64ISAR1_API) |+ ARM64_FEATURE_MASK(ID_AA64ISAR1_GPA) |+ ARM64_FEATURE_MASK(ID_AA64ISAR1_GPI));++ return id_aa64isar1_el1_sys_val & allow_mask;+}++u64 get_pvm_id_aa64mmfr0(const struct kvm_vcpu *vcpu)+{+ u64 set_mask;++ set_mask = get_restricted_features_unsigned(id_aa64mmfr0_el1_sys_val,+ PVM_ID_AA64MMFR0_RESTRICT_UNSIGNED);++ return (id_aa64mmfr0_el1_sys_val & PVM_ID_AA64MMFR0_ALLOW) | set_mask;+}++u64 get_pvm_id_aa64mmfr1(const struct kvm_vcpu *vcpu)+{+ return id_aa64mmfr1_el1_sys_val & PVM_ID_AA64MMFR1_ALLOW;+}++u64 get_pvm_id_aa64mmfr2(const struct kvm_vcpu *vcpu)+{+ return id_aa64mmfr2_el1_sys_val & PVM_ID_AA64MMFR2_ALLOW;+}++/* Read a sanitized cpufeature ID register by its sys_reg_desc. */+static u64 read_id_reg(const struct kvm_vcpu *vcpu,+ struct sys_reg_desc const *r)+{+ u32 id = reg_to_encoding(r);++ switch (id) {+ case SYS_ID_AA64PFR0_EL1:+ return get_pvm_id_aa64pfr0(vcpu);+ case SYS_ID_AA64PFR1_EL1:+ return get_pvm_id_aa64pfr1(vcpu);+ case SYS_ID_AA64ZFR0_EL1:+ return get_pvm_id_aa64zfr0(vcpu);+ case SYS_ID_AA64DFR0_EL1:+ return get_pvm_id_aa64dfr0(vcpu);+ case SYS_ID_AA64DFR1_EL1:+ return get_pvm_id_aa64dfr1(vcpu);+ case SYS_ID_AA64AFR0_EL1:+ return get_pvm_id_aa64afr0(vcpu);+ case SYS_ID_AA64AFR1_EL1:+ return get_pvm_id_aa64afr1(vcpu);+ case SYS_ID_AA64ISAR0_EL1:+ return get_pvm_id_aa64isar0(vcpu);+ case SYS_ID_AA64ISAR1_EL1:+ return get_pvm_id_aa64isar1(vcpu);+ case SYS_ID_AA64MMFR0_EL1:+ return get_pvm_id_aa64mmfr0(vcpu);+ case SYS_ID_AA64MMFR1_EL1:+ return get_pvm_id_aa64mmfr1(vcpu);+ case SYS_ID_AA64MMFR2_EL1:+ return get_pvm_id_aa64mmfr2(vcpu);+ default:+ /*+ * Should never happen because all cases are covered in+ * pvm_sys_reg_descs[] below.
I'd drop the 'below' word. It's not overly helpful and since code gets
moved it can go out of date.
+ */
+ WARN_ON(1);
The above cases could also be generated by a macro. And I wonder if we can
come up with something that makes sure these separate lists stay
consistent with macros and build bugs in order to better avoid these
"should never happen" situations.
+ break;
+ }
+
+ return 0;
+}
+
+/*
+ * Accessor for AArch32 feature id registers.
+ *
+ * The value of these registers is "unknown" according to the spec if AArch32
+ * isn't supported.
+ */
+static bool pvm_access_id_aarch32(struct kvm_vcpu *vcpu,
+ struct sys_reg_params *p,
+ const struct sys_reg_desc *r)
+{
+ if (p->is_write)
+ return undef_access(vcpu, p, r);
+
+ /*
+ * No support for AArch32 guests, therefore, pKVM has no sanitized copy
+ * of AArch32 feature id registers.
+ */
+ BUILD_BUG_ON(FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1),
+ PVM_ID_AA64PFR0_RESTRICT_UNSIGNED) > ID_AA64PFR0_ELx_64BIT_ONLY);
+
+ /* Use 0 for architecturally "unknown" values. */
+ p->regval = 0;
+ return true;
+}
+
+/*
+ * Accessor for AArch64 feature id registers.
+ *
+ * If access is allowed, set the regval to the protected VM's view of the
+ * register and return true.
+ * Otherwise, inject an undefined exception and return false.
+ */
+static bool pvm_access_id_aarch64(struct kvm_vcpu *vcpu,
+ struct sys_reg_params *p,
+ const struct sys_reg_desc *r)
+{
+ if (p->is_write)
+ return undef_access(vcpu, p, r);
+
+ p->regval = read_id_reg(vcpu, r);
+ return true;
+}
+
+/* Mark the specified system register as an AArch32 feature id register. */
+#define AARCH32(REG) { SYS_DESC(REG), .access = pvm_access_id_aarch32 }
+
+/* Mark the specified system register as an AArch64 feature id register. */
+#define AARCH64(REG) { SYS_DESC(REG), .access = pvm_access_id_aarch64 }
+
+/* Mark the specified system register as not being handled in hyp. */
+#define HOST_HANDLED(REG) { SYS_DESC(REG), .access = NULL }
+
+/*
+ * Architected system registers.
+ * Important: Must be sorted ascending by Op0, Op1, CRn, CRm, Op2
+ *
+ * NOTE: Anything not explicitly listed here is *restricted by default*, i.e.,
+ * it will lead to injecting an exception into the guest.
+ */
+static const struct sys_reg_desc pvm_sys_reg_descs[] = {
+ /* Cache maintenance by set/way operations are restricted. */
+
+ /* Debug and Trace Registers are restricted. */
+
+ /* AArch64 mappings of the AArch32 ID registers */
+ /* CRm=1 */
+ AARCH32(SYS_ID_PFR0_EL1),
+ AARCH32(SYS_ID_PFR1_EL1),
+ AARCH32(SYS_ID_DFR0_EL1),
+ AARCH32(SYS_ID_AFR0_EL1),
+ AARCH32(SYS_ID_MMFR0_EL1),
+ AARCH32(SYS_ID_MMFR1_EL1),
+ AARCH32(SYS_ID_MMFR2_EL1),
+ AARCH32(SYS_ID_MMFR3_EL1),
+
+ /* CRm=2 */
+ AARCH32(SYS_ID_ISAR0_EL1),
+ AARCH32(SYS_ID_ISAR1_EL1),
+ AARCH32(SYS_ID_ISAR2_EL1),
+ AARCH32(SYS_ID_ISAR3_EL1),
+ AARCH32(SYS_ID_ISAR4_EL1),
+ AARCH32(SYS_ID_ISAR5_EL1),
+ AARCH32(SYS_ID_MMFR4_EL1),
+ AARCH32(SYS_ID_ISAR6_EL1),
+
+ /* CRm=3 */
+ AARCH32(SYS_MVFR0_EL1),
+ AARCH32(SYS_MVFR1_EL1),
+ AARCH32(SYS_MVFR2_EL1),
+ AARCH32(SYS_ID_PFR2_EL1),
+ AARCH32(SYS_ID_DFR1_EL1),
+ AARCH32(SYS_ID_MMFR5_EL1),
+
+ /* AArch64 ID registers */
+ /* CRm=4 */
+ AARCH64(SYS_ID_AA64PFR0_EL1),
+ AARCH64(SYS_ID_AA64PFR1_EL1),
+ AARCH64(SYS_ID_AA64ZFR0_EL1),
+ AARCH64(SYS_ID_AA64DFR0_EL1),
+ AARCH64(SYS_ID_AA64DFR1_EL1),
+ AARCH64(SYS_ID_AA64AFR0_EL1),
+ AARCH64(SYS_ID_AA64AFR1_EL1),
+ AARCH64(SYS_ID_AA64ISAR0_EL1),
+ AARCH64(SYS_ID_AA64ISAR1_EL1),
+ AARCH64(SYS_ID_AA64MMFR0_EL1),
+ AARCH64(SYS_ID_AA64MMFR1_EL1),
+ AARCH64(SYS_ID_AA64MMFR2_EL1),
+
+ HOST_HANDLED(SYS_SCTLR_EL1),
+ HOST_HANDLED(SYS_ACTLR_EL1),
+ HOST_HANDLED(SYS_CPACR_EL1),
+
+ HOST_HANDLED(SYS_RGSR_EL1),
+ HOST_HANDLED(SYS_GCR_EL1),
+
+ /* Scalable Vector Registers are restricted. */
+
+ HOST_HANDLED(SYS_TTBR0_EL1),
+ HOST_HANDLED(SYS_TTBR1_EL1),
+ HOST_HANDLED(SYS_TCR_EL1),
+
+ HOST_HANDLED(SYS_APIAKEYLO_EL1),
+ HOST_HANDLED(SYS_APIAKEYHI_EL1),
+ HOST_HANDLED(SYS_APIBKEYLO_EL1),
+ HOST_HANDLED(SYS_APIBKEYHI_EL1),
+ HOST_HANDLED(SYS_APDAKEYLO_EL1),
+ HOST_HANDLED(SYS_APDAKEYHI_EL1),
+ HOST_HANDLED(SYS_APDBKEYLO_EL1),
+ HOST_HANDLED(SYS_APDBKEYHI_EL1),
+ HOST_HANDLED(SYS_APGAKEYLO_EL1),
+ HOST_HANDLED(SYS_APGAKEYHI_EL1),
+
+ HOST_HANDLED(SYS_AFSR0_EL1),
+ HOST_HANDLED(SYS_AFSR1_EL1),
+ HOST_HANDLED(SYS_ESR_EL1),
+
+ HOST_HANDLED(SYS_ERRIDR_EL1),
+ HOST_HANDLED(SYS_ERRSELR_EL1),
+ HOST_HANDLED(SYS_ERXFR_EL1),
+ HOST_HANDLED(SYS_ERXCTLR_EL1),
+ HOST_HANDLED(SYS_ERXSTATUS_EL1),
+ HOST_HANDLED(SYS_ERXADDR_EL1),
+ HOST_HANDLED(SYS_ERXMISC0_EL1),
+ HOST_HANDLED(SYS_ERXMISC1_EL1),
+
+ HOST_HANDLED(SYS_TFSR_EL1),
+ HOST_HANDLED(SYS_TFSRE0_EL1),
+
+ HOST_HANDLED(SYS_FAR_EL1),
+ HOST_HANDLED(SYS_PAR_EL1),
+
+ /* Performance Monitoring Registers are restricted. */
+
+ HOST_HANDLED(SYS_MAIR_EL1),
+ HOST_HANDLED(SYS_AMAIR_EL1),
+
+ /* Limited Ordering Regions Registers are restricted. */
+
+ HOST_HANDLED(SYS_VBAR_EL1),
+ HOST_HANDLED(SYS_DISR_EL1),
+
+ /* GIC CPU Interface registers are restricted. */
+
+ HOST_HANDLED(SYS_CONTEXTIDR_EL1),
+ HOST_HANDLED(SYS_TPIDR_EL1),
+
+ HOST_HANDLED(SYS_SCXTNUM_EL1),
+
+ HOST_HANDLED(SYS_CNTKCTL_EL1),
+
+ HOST_HANDLED(SYS_CCSIDR_EL1),
+ HOST_HANDLED(SYS_CLIDR_EL1),
+ HOST_HANDLED(SYS_CSSELR_EL1),
+ HOST_HANDLED(SYS_CTR_EL0),
+
+ /* Performance Monitoring Registers are restricted. */
+
+ HOST_HANDLED(SYS_TPIDR_EL0),
+ HOST_HANDLED(SYS_TPIDRRO_EL0),
+
+ HOST_HANDLED(SYS_SCXTNUM_EL0),
+
+ /* Activity Monitoring Registers are restricted. */
+
+ HOST_HANDLED(SYS_CNTP_TVAL_EL0),
+ HOST_HANDLED(SYS_CNTP_CTL_EL0),
+ HOST_HANDLED(SYS_CNTP_CVAL_EL0),
+
+ /* Performance Monitoring Registers are restricted. */
+
+ HOST_HANDLED(SYS_DACR32_EL2),
+ HOST_HANDLED(SYS_IFSR32_EL2),
+ HOST_HANDLED(SYS_FPEXC32_EL2),
+};
+
+/*
+ * Handler for protected VM MSR, MRS or System instruction execution.
+ *
+ * Returns true if the hypervisor has handled the exit, and control should go
+ * back to the guest, or false if it hasn't, to be handled by the host.
+ */
+bool kvm_handle_pvm_sysreg(struct kvm_vcpu *vcpu, u64 *exit_code)
+{
+ const struct sys_reg_desc *r;
+ struct sys_reg_params params;
+ unsigned long esr = kvm_vcpu_get_esr(vcpu);
+ int Rt = kvm_vcpu_sys_get_rt(vcpu);
+
+ params = esr_sys64_to_params(esr);
+ params.regval = vcpu_get_reg(vcpu, Rt);
+
+ r = find_reg(¶ms, pvm_sys_reg_descs, ARRAY_SIZE(pvm_sys_reg_descs));
+
+ /* Undefined access (RESTRICTED). */
+ if (r == NULL) {
+ __inject_undef64(vcpu);
+ return true;
+ }
+
+ /* Handled by the host (HOST_HANDLED) */
+ if (r->access == NULL)
+ return false;
+
+ /* Handled by hyp: skip instruction if instructed to do so. */
+ if (r->access(vcpu, ¶ms, r))
+ __kvm_skip_instr(vcpu);
+
+ if (!params.is_write)
+ vcpu_set_reg(vcpu, Rt, params.regval);
+
+ return true;
+}
--
2.33.0.464.g1972c5931b-goog
Other than the nits and suggestion to try and build in some register list
consistency checks, this looks good to me. I don't know what pKVM
should / should not expose, but I like the approach this takes, so,
FWIW,
Reviewed-by: Andrew Jones <redacted>
Thanks,
drew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Protected KVM does not support protected AArch32 guests. However,
it is possible for the guest to force run AArch32, potentially
causing problems. Add an extra check so that if the hypervisor
catches the guest doing that, it can prevent the guest from
running again by resetting vcpu->arch.target and returning
ARM_EXCEPTION_IL.
If this were to happen, The VMM can try and fix it by re-
initializing the vcpu with KVM_ARM_VCPU_INIT, however, this is
likely not possible for protected VMs.
Adapted from commit 22f553842b14 ("KVM: arm64: Handle Asymmetric
AArch32 systems")
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/kvm/hyp/nvhe/switch.c | 40 ++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
Do we really need to go through this is_aarch32_allowed check?
Protected VMs don't have AArch32, and we don't have the infrastructure
to handle 32bit at all. For non-protected VMs, we already have what we
need at EL1. So the extra check only adds complexity.
No. I could change it to a build-time assertion just to make sure that
AArch32 is not allowed instead.
Thanks,
/fuad
quoted
+ /*+ * As we have caught the guest red-handed, decide that it isn't+ * fit for purpose anymore by making the vcpu invalid. The VMM+ * can try and fix it by re-initializing the vcpu with+ * KVM_ARM_VCPU_INIT, however, this is likely not possible for+ * protected VMs.+ */+ vcpu->arch.target = -1;+ *exit_code = ARM_EXCEPTION_IL;+ return false;+ }++ return true;+}+ /* Switch to the guest for legacy non-VHE systems */ int __kvm_vcpu_run(struct kvm_vcpu *vcpu) {
@@ -297,6 +334,9 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu) /* Jump in the fire! */ exit_code = __guest_enter(vcpu);+ if (unlikely(!handle_aarch32_guest(vcpu, &exit_code)))+ break;+ /* And we're baaack! */ } while (fixup_guest_exit(vcpu, &exit_code));
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
Protected VMs have more restricted features that need to be
trapped. Moreover, the host should not be trusted to set the
appropriate trapping registers and their values.
Initialize the trapping registers, i.e., hcr_el2, mdcr_el2, and
cptr_el2 at EL2 for protected guests, based on the values of the
guest's feature id registers.
No functional change intended as trap handlers introduced in the
previous patch are still not hooked in to the guest exit
handlers.
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/include/asm/kvm_asm.h | 1 +
arch/arm64/include/asm/kvm_host.h | 2 +
arch/arm64/kvm/arm.c | 8 ++
arch/arm64/kvm/hyp/include/nvhe/pkvm.h | 14 ++
arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 10 ++
arch/arm64/kvm/hyp/nvhe/pkvm.c | 186 +++++++++++++++++++++++++
7 files changed, 222 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/pkvm.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/pkvm.c
We need to stop adding these small files with only two lines in
them. Please merge this with nvhe/trap_handler.h, for example, and
rename the whole thing to pkvm.h if you want.
We have defined that pVMs don't have AArch32 at all. So RW should
always be set. And if RW is set, the TID0 serves no purpose as EL1 is
AArch64, as it only traps AArch32 EL1 accesses.
I like the fact that this is all driven from the feature set, but it
is also a bit unreadable. So I'd drop it in favour of:
u64 hcr_set = HCR_RW;
at the top of the function.
quoted hunk
++ /* Trap RAS unless all current versions are supported */+ if (FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_RAS), feature_ids) <+ ID_AA64PFR0_RAS_V1P1) {+ hcr_set |= HCR_TERR | HCR_TEA;+ hcr_clear |= HCR_FIEN;+ }++ /* Trap AMU */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_AMU), feature_ids)) {+ hcr_clear |= HCR_AMVOFFEN;+ cptr_set |= CPTR_EL2_TAM;+ }++ /*+ * Linux guests assume support for floating-point and Advanced SIMD. Do+ * not change the trapping behavior for these from the KVM default.+ */+ BUILD_BUG_ON(!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_FP),+ PVM_ID_AA64PFR0_ALLOW));+ BUILD_BUG_ON(!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_ASIMD),+ PVM_ID_AA64PFR0_ALLOW));++ /* Trap SVE */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_SVE), feature_ids))+ cptr_set |= CPTR_EL2_TZ;++ vcpu->arch.hcr_el2 |= hcr_set;+ vcpu->arch.hcr_el2 &= ~hcr_clear;+ vcpu->arch.cptr_el2 |= cptr_set;+}++/*+ * Set trap register values based on features in ID_AA64PFR1.+ */+static void pvm_init_traps_aa64pfr1(struct kvm_vcpu *vcpu)+{+ const u64 feature_ids = get_pvm_id_aa64pfr1(vcpu);+ u64 hcr_set = 0;+ u64 hcr_clear = 0;++ /* Memory Tagging: Trap and Treat as Untagged if not supported. */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR1_MTE), feature_ids)) {+ hcr_set |= HCR_TID5;+ hcr_clear |= HCR_DCT | HCR_ATA;+ }++ vcpu->arch.hcr_el2 |= hcr_set;+ vcpu->arch.hcr_el2 &= ~hcr_clear;+}++/*+ * Set trap register values based on features in ID_AA64DFR0.+ */+static void pvm_init_traps_aa64dfr0(struct kvm_vcpu *vcpu)+{+ const u64 feature_ids = get_pvm_id_aa64dfr0(vcpu);+ u64 mdcr_set = 0;+ u64 mdcr_clear = 0;+ u64 cptr_set = 0;++ /* Trap/constrain PMU */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_PMUVER), feature_ids)) {+ mdcr_set |= MDCR_EL2_TPM | MDCR_EL2_TPMCR;+ mdcr_clear |= MDCR_EL2_HPME | MDCR_EL2_MTPME |+ MDCR_EL2_HPMN_MASK;+ }++ /* Trap Debug */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER), feature_ids))+ mdcr_set |= MDCR_EL2_TDRA | MDCR_EL2_TDA | MDCR_EL2_TDE;++ /* Trap OS Double Lock */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_DOUBLELOCK), feature_ids))+ mdcr_set |= MDCR_EL2_TDOSA;++ /* Trap SPE */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_PMSVER), feature_ids)) {+ mdcr_set |= MDCR_EL2_TPMS;+ mdcr_clear |= MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT;+ }++ /* Trap Trace Filter */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_TRACE_FILT), feature_ids))+ mdcr_set |= MDCR_EL2_TTRF;++ /* Trap Trace */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_TRACEVER), feature_ids))+ cptr_set |= CPTR_EL2_TTA;++ vcpu->arch.mdcr_el2 |= mdcr_set;+ vcpu->arch.mdcr_el2 &= ~mdcr_clear;+ vcpu->arch.cptr_el2 |= cptr_set;+}++/*+ * Set trap register values based on features in ID_AA64MMFR0.+ */+static void pvm_init_traps_aa64mmfr0(struct kvm_vcpu *vcpu)+{+ const u64 feature_ids = get_pvm_id_aa64mmfr0(vcpu);+ u64 mdcr_set = 0;++ /* Trap Debug Communications Channel registers */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64MMFR0_FGT), feature_ids))+ mdcr_set |= MDCR_EL2_TDCC;++ vcpu->arch.mdcr_el2 |= mdcr_set;+}++/*+ * Set trap register values based on features in ID_AA64MMFR1.+ */+static void pvm_init_traps_aa64mmfr1(struct kvm_vcpu *vcpu)+{+ const u64 feature_ids = get_pvm_id_aa64mmfr1(vcpu);+ u64 hcr_set = 0;++ /* Trap LOR */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64MMFR1_LOR), feature_ids))+ hcr_set |= HCR_TLOR;++ vcpu->arch.hcr_el2 |= hcr_set;+}++/*+ * Set baseline trap register values.+ */+static void pvm_init_trap_regs(struct kvm_vcpu *vcpu)+{+ const u64 hcr_trap_feat_regs = HCR_TID3;+ const u64 hcr_trap_impdef = HCR_TACR | HCR_TIDCP | HCR_TID1;++ /*+ * Always trap:+ * - Feature id registers: to control features exposed to guests+ * - Implementation-defined features+ */+ vcpu->arch.hcr_el2 |= hcr_trap_feat_regs | hcr_trap_impdef;++ /* Clear res0 and set res1 bits to trap potential new features. */+ vcpu->arch.hcr_el2 &= ~(HCR_RES0);+ vcpu->arch.mdcr_el2 &= ~(MDCR_EL2_RES0);+ vcpu->arch.cptr_el2 |= CPTR_NVHE_EL2_RES1;+ vcpu->arch.cptr_el2 &= ~(CPTR_NVHE_EL2_RES0);+}++/*+ * Initialize trap register values for protected VMs.+ */+void __pkvm_vcpu_init_traps(struct kvm_vcpu *vcpu)+{+ pvm_init_trap_regs(vcpu);+ pvm_init_traps_aa64pfr0(vcpu);+ pvm_init_traps_aa64pfr1(vcpu);+ pvm_init_traps_aa64dfr0(vcpu);+ pvm_init_traps_aa64mmfr0(vcpu);+ pvm_init_traps_aa64mmfr1(vcpu);+}
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Protected VMs have more restricted features that need to be
trapped. Moreover, the host should not be trusted to set the
appropriate trapping registers and their values.
Initialize the trapping registers, i.e., hcr_el2, mdcr_el2, and
cptr_el2 at EL2 for protected guests, based on the values of the
guest's feature id registers.
No functional change intended as trap handlers introduced in the
previous patch are still not hooked in to the guest exit
handlers.
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/include/asm/kvm_asm.h | 1 +
arch/arm64/include/asm/kvm_host.h | 2 +
arch/arm64/kvm/arm.c | 8 ++
arch/arm64/kvm/hyp/include/nvhe/pkvm.h | 14 ++
arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 10 ++
arch/arm64/kvm/hyp/nvhe/pkvm.c | 186 +++++++++++++++++++++++++
7 files changed, 222 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/pkvm.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/pkvm.c
We need to stop adding these small files with only two lines in
them. Please merge this with nvhe/trap_handler.h, for example, and
rename the whole thing to pkvm.h if you want.
We have defined that pVMs don't have AArch32 at all. So RW should
always be set. And if RW is set, the TID0 serves no purpose as EL1 is
AArch64, as it only traps AArch32 EL1 accesses.
I like the fact that this is all driven from the feature set, but it
is also a bit unreadable. So I'd drop it in favour of:
u64 hcr_set = HCR_RW;
at the top of the function.
Sure. What I could do, which I mentioned in a reply to your comments
on patch 12/12, is to have a build time assertion that checks that
AArch32 is not supported for pvms.
Cheers,
/fuad
quoted
++ /* Trap RAS unless all current versions are supported */+ if (FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_RAS), feature_ids) <+ ID_AA64PFR0_RAS_V1P1) {+ hcr_set |= HCR_TERR | HCR_TEA;+ hcr_clear |= HCR_FIEN;+ }++ /* Trap AMU */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_AMU), feature_ids)) {+ hcr_clear |= HCR_AMVOFFEN;+ cptr_set |= CPTR_EL2_TAM;+ }++ /*+ * Linux guests assume support for floating-point and Advanced SIMD. Do+ * not change the trapping behavior for these from the KVM default.+ */+ BUILD_BUG_ON(!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_FP),+ PVM_ID_AA64PFR0_ALLOW));+ BUILD_BUG_ON(!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_ASIMD),+ PVM_ID_AA64PFR0_ALLOW));++ /* Trap SVE */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_SVE), feature_ids))+ cptr_set |= CPTR_EL2_TZ;++ vcpu->arch.hcr_el2 |= hcr_set;+ vcpu->arch.hcr_el2 &= ~hcr_clear;+ vcpu->arch.cptr_el2 |= cptr_set;+}++/*+ * Set trap register values based on features in ID_AA64PFR1.+ */+static void pvm_init_traps_aa64pfr1(struct kvm_vcpu *vcpu)+{+ const u64 feature_ids = get_pvm_id_aa64pfr1(vcpu);+ u64 hcr_set = 0;+ u64 hcr_clear = 0;++ /* Memory Tagging: Trap and Treat as Untagged if not supported. */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR1_MTE), feature_ids)) {+ hcr_set |= HCR_TID5;+ hcr_clear |= HCR_DCT | HCR_ATA;+ }++ vcpu->arch.hcr_el2 |= hcr_set;+ vcpu->arch.hcr_el2 &= ~hcr_clear;+}++/*+ * Set trap register values based on features in ID_AA64DFR0.+ */+static void pvm_init_traps_aa64dfr0(struct kvm_vcpu *vcpu)+{+ const u64 feature_ids = get_pvm_id_aa64dfr0(vcpu);+ u64 mdcr_set = 0;+ u64 mdcr_clear = 0;+ u64 cptr_set = 0;++ /* Trap/constrain PMU */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_PMUVER), feature_ids)) {+ mdcr_set |= MDCR_EL2_TPM | MDCR_EL2_TPMCR;+ mdcr_clear |= MDCR_EL2_HPME | MDCR_EL2_MTPME |+ MDCR_EL2_HPMN_MASK;+ }++ /* Trap Debug */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER), feature_ids))+ mdcr_set |= MDCR_EL2_TDRA | MDCR_EL2_TDA | MDCR_EL2_TDE;++ /* Trap OS Double Lock */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_DOUBLELOCK), feature_ids))+ mdcr_set |= MDCR_EL2_TDOSA;++ /* Trap SPE */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_PMSVER), feature_ids)) {+ mdcr_set |= MDCR_EL2_TPMS;+ mdcr_clear |= MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT;+ }++ /* Trap Trace Filter */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_TRACE_FILT), feature_ids))+ mdcr_set |= MDCR_EL2_TTRF;++ /* Trap Trace */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_TRACEVER), feature_ids))+ cptr_set |= CPTR_EL2_TTA;++ vcpu->arch.mdcr_el2 |= mdcr_set;+ vcpu->arch.mdcr_el2 &= ~mdcr_clear;+ vcpu->arch.cptr_el2 |= cptr_set;+}++/*+ * Set trap register values based on features in ID_AA64MMFR0.+ */+static void pvm_init_traps_aa64mmfr0(struct kvm_vcpu *vcpu)+{+ const u64 feature_ids = get_pvm_id_aa64mmfr0(vcpu);+ u64 mdcr_set = 0;++ /* Trap Debug Communications Channel registers */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64MMFR0_FGT), feature_ids))+ mdcr_set |= MDCR_EL2_TDCC;++ vcpu->arch.mdcr_el2 |= mdcr_set;+}++/*+ * Set trap register values based on features in ID_AA64MMFR1.+ */+static void pvm_init_traps_aa64mmfr1(struct kvm_vcpu *vcpu)+{+ const u64 feature_ids = get_pvm_id_aa64mmfr1(vcpu);+ u64 hcr_set = 0;++ /* Trap LOR */+ if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64MMFR1_LOR), feature_ids))+ hcr_set |= HCR_TLOR;++ vcpu->arch.hcr_el2 |= hcr_set;+}++/*+ * Set baseline trap register values.+ */+static void pvm_init_trap_regs(struct kvm_vcpu *vcpu)+{+ const u64 hcr_trap_feat_regs = HCR_TID3;+ const u64 hcr_trap_impdef = HCR_TACR | HCR_TIDCP | HCR_TID1;++ /*+ * Always trap:+ * - Feature id registers: to control features exposed to guests+ * - Implementation-defined features+ */+ vcpu->arch.hcr_el2 |= hcr_trap_feat_regs | hcr_trap_impdef;++ /* Clear res0 and set res1 bits to trap potential new features. */+ vcpu->arch.hcr_el2 &= ~(HCR_RES0);+ vcpu->arch.mdcr_el2 &= ~(MDCR_EL2_RES0);+ vcpu->arch.cptr_el2 |= CPTR_NVHE_EL2_RES1;+ vcpu->arch.cptr_el2 &= ~(CPTR_NVHE_EL2_RES0);+}++/*+ * Initialize trap register values for protected VMs.+ */+void __pkvm_vcpu_init_traps(struct kvm_vcpu *vcpu)+{+ pvm_init_trap_regs(vcpu);+ pvm_init_traps_aa64pfr0(vcpu);+ pvm_init_traps_aa64pfr1(vcpu);+ pvm_init_traps_aa64dfr0(vcpu);+ pvm_init_traps_aa64mmfr0(vcpu);+ pvm_init_traps_aa64mmfr1(vcpu);+}
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
Add system register handlers for protected VMs. These cover Sys64
registers (including feature id registers), and debug.
No functional change intended as these are not hooked in yet to
the guest exit handlers introduced earlier. So when trapping is
triggered, the exit handlers let the host handle it, as before.
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/include/asm/kvm_fixed_config.h | 195 ++++++++
arch/arm64/include/asm/kvm_hyp.h | 5 +
arch/arm64/kvm/arm.c | 5 +
arch/arm64/kvm/hyp/include/nvhe/sys_regs.h | 28 ++
arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
arch/arm64/kvm/hyp/nvhe/sys_regs.c | 492 +++++++++++++++++++++
6 files changed, 726 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/include/asm/kvm_fixed_config.h
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/sys_regs.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/sys_regs.c
An access exception is the result of a memory access. undef_access
makes my head spin because you are conflating two unrelated terms.
I suggest you merge all three functions in a single inject_undef64().
quoted hunk
+}++/*+ * Returns the restricted features values of the feature register based on the+ * limitations in restrict_fields.+ * A feature id field value of 0b0000 does not impose any restrictions.+ * Note: Use only for unsigned feature field values.+ */+static u64 get_restricted_features_unsigned(u64 sys_reg_val,+ u64 restrict_fields)+{+ u64 value = 0UL;+ u64 mask = GENMASK_ULL(ARM64_FEATURE_FIELD_BITS - 1, 0);++ /*+ * According to the Arm Architecture Reference Manual, feature fields+ * use increasing values to indicate increases in functionality.+ * Iterate over the restricted feature fields and calculate the minimum+ * unsigned value between the one supported by the system, and what the+ * value is being restricted to.+ */+ while (sys_reg_val && restrict_fields) {+ value |= min(sys_reg_val & mask, restrict_fields & mask);+ sys_reg_val &= ~mask;+ restrict_fields &= ~mask;+ mask <<= ARM64_FEATURE_FIELD_BITS;+ }++ return value;+}++/*+ * Functions that return the value of feature id registers for protected VMs+ * based on allowed features, system features, and KVM support.+ */++u64 get_pvm_id_aa64pfr0(const struct kvm_vcpu *vcpu)+{+ const struct kvm *kvm = (const struct kvm *)kern_hyp_va(vcpu->kvm);+ u64 set_mask = 0;+ u64 allow_mask = PVM_ID_AA64PFR0_ALLOW;++ if (!vcpu_has_sve(vcpu))+ allow_mask &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_SVE);++ set_mask |= get_restricted_features_unsigned(id_aa64pfr0_el1_sys_val,+ PVM_ID_AA64PFR0_RESTRICT_UNSIGNED);++ /* Spectre and Meltdown mitigation in KVM */+ set_mask |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2),+ (u64)kvm->arch.pfr0_csv2);+ set_mask |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3),+ (u64)kvm->arch.pfr0_csv3);++ return (id_aa64pfr0_el1_sys_val & allow_mask) | set_mask;+}++u64 get_pvm_id_aa64pfr1(const struct kvm_vcpu *vcpu)+{+ const struct kvm *kvm = (const struct kvm *)kern_hyp_va(vcpu->kvm);+ u64 allow_mask = PVM_ID_AA64PFR1_ALLOW;++ if (!kvm_has_mte(kvm))+ allow_mask &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_MTE);++ return id_aa64pfr1_el1_sys_val & allow_mask;+}++u64 get_pvm_id_aa64zfr0(const struct kvm_vcpu *vcpu)+{+ /*+ * No support for Scalable Vectors, therefore, hyp has no sanitized+ * copy of the feature id register.+ */+ BUILD_BUG_ON(PVM_ID_AA64ZFR0_ALLOW != 0ULL);+ return 0;+}++u64 get_pvm_id_aa64dfr0(const struct kvm_vcpu *vcpu)+{+ /*+ * No support for debug, including breakpoints, and watchpoints,+ * therefore, pKVM has no sanitized copy of the feature id register.+ */+ BUILD_BUG_ON(PVM_ID_AA64DFR0_ALLOW != 0ULL);+ return 0;+}++u64 get_pvm_id_aa64dfr1(const struct kvm_vcpu *vcpu)+{+ /*+ * No support for debug, therefore, hyp has no sanitized copy of the+ * feature id register.+ */+ BUILD_BUG_ON(PVM_ID_AA64DFR1_ALLOW != 0ULL);+ return 0;+}++u64 get_pvm_id_aa64afr0(const struct kvm_vcpu *vcpu)+{+ /*+ * No support for implementation defined features, therefore, hyp has no+ * sanitized copy of the feature id register.+ */+ BUILD_BUG_ON(PVM_ID_AA64AFR0_ALLOW != 0ULL);+ return 0;+}++u64 get_pvm_id_aa64afr1(const struct kvm_vcpu *vcpu)+{+ /*+ * No support for implementation defined features, therefore, hyp has no+ * sanitized copy of the feature id register.+ */+ BUILD_BUG_ON(PVM_ID_AA64AFR1_ALLOW != 0ULL);+ return 0;+}++u64 get_pvm_id_aa64isar0(const struct kvm_vcpu *vcpu)+{+ return id_aa64isar0_el1_sys_val & PVM_ID_AA64ISAR0_ALLOW;+}++u64 get_pvm_id_aa64isar1(const struct kvm_vcpu *vcpu)+{+ u64 allow_mask = PVM_ID_AA64ISAR1_ALLOW;++ if (!vcpu_has_ptrauth(vcpu))+ allow_mask &= ~(ARM64_FEATURE_MASK(ID_AA64ISAR1_APA) |+ ARM64_FEATURE_MASK(ID_AA64ISAR1_API) |+ ARM64_FEATURE_MASK(ID_AA64ISAR1_GPA) |+ ARM64_FEATURE_MASK(ID_AA64ISAR1_GPI));++ return id_aa64isar1_el1_sys_val & allow_mask;+}++u64 get_pvm_id_aa64mmfr0(const struct kvm_vcpu *vcpu)+{+ u64 set_mask;++ set_mask = get_restricted_features_unsigned(id_aa64mmfr0_el1_sys_val,+ PVM_ID_AA64MMFR0_RESTRICT_UNSIGNED);++ return (id_aa64mmfr0_el1_sys_val & PVM_ID_AA64MMFR0_ALLOW) | set_mask;+}++u64 get_pvm_id_aa64mmfr1(const struct kvm_vcpu *vcpu)+{+ return id_aa64mmfr1_el1_sys_val & PVM_ID_AA64MMFR1_ALLOW;+}++u64 get_pvm_id_aa64mmfr2(const struct kvm_vcpu *vcpu)+{+ return id_aa64mmfr2_el1_sys_val & PVM_ID_AA64MMFR2_ALLOW;+}++/* Read a sanitized cpufeature ID register by its sys_reg_desc. */+static u64 read_id_reg(const struct kvm_vcpu *vcpu,+ struct sys_reg_desc const *r)+{+ u32 id = reg_to_encoding(r);++ switch (id) {+ case SYS_ID_AA64PFR0_EL1:+ return get_pvm_id_aa64pfr0(vcpu);+ case SYS_ID_AA64PFR1_EL1:+ return get_pvm_id_aa64pfr1(vcpu);+ case SYS_ID_AA64ZFR0_EL1:+ return get_pvm_id_aa64zfr0(vcpu);+ case SYS_ID_AA64DFR0_EL1:+ return get_pvm_id_aa64dfr0(vcpu);+ case SYS_ID_AA64DFR1_EL1:+ return get_pvm_id_aa64dfr1(vcpu);+ case SYS_ID_AA64AFR0_EL1:+ return get_pvm_id_aa64afr0(vcpu);+ case SYS_ID_AA64AFR1_EL1:+ return get_pvm_id_aa64afr1(vcpu);+ case SYS_ID_AA64ISAR0_EL1:+ return get_pvm_id_aa64isar0(vcpu);+ case SYS_ID_AA64ISAR1_EL1:+ return get_pvm_id_aa64isar1(vcpu);+ case SYS_ID_AA64MMFR0_EL1:+ return get_pvm_id_aa64mmfr0(vcpu);+ case SYS_ID_AA64MMFR1_EL1:+ return get_pvm_id_aa64mmfr1(vcpu);+ case SYS_ID_AA64MMFR2_EL1:+ return get_pvm_id_aa64mmfr2(vcpu);+ default:+ /*+ * Should never happen because all cases are covered in+ * pvm_sys_reg_descs[] below.+ */+ WARN_ON(1);+ break;+ }++ return 0;+}++/*+ * Accessor for AArch32 feature id registers.+ *+ * The value of these registers is "unknown" according to the spec if AArch32+ * isn't supported.+ */+static bool pvm_access_id_aarch32(struct kvm_vcpu *vcpu,+ struct sys_reg_params *p,+ const struct sys_reg_desc *r)+{+ if (p->is_write)+ return undef_access(vcpu, p, r);++ /*+ * No support for AArch32 guests, therefore, pKVM has no sanitized copy+ * of AArch32 feature id registers.+ */+ BUILD_BUG_ON(FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1),+ PVM_ID_AA64PFR0_RESTRICT_UNSIGNED) > ID_AA64PFR0_ELx_64BIT_ONLY);++ /* Use 0 for architecturally "unknown" values. */+ p->regval = 0;+ return true;+}++/*+ * Accessor for AArch64 feature id registers.+ *+ * If access is allowed, set the regval to the protected VM's view of the+ * register and return true.+ * Otherwise, inject an undefined exception and return false.+ */+static bool pvm_access_id_aarch64(struct kvm_vcpu *vcpu,+ struct sys_reg_params *p,+ const struct sys_reg_desc *r)+{+ if (p->is_write)+ return undef_access(vcpu, p, r);++ p->regval = read_id_reg(vcpu, r);+ return true;+}++/* Mark the specified system register as an AArch32 feature id register. */+#define AARCH32(REG) { SYS_DESC(REG), .access = pvm_access_id_aarch32 }++/* Mark the specified system register as an AArch64 feature id register. */+#define AARCH64(REG) { SYS_DESC(REG), .access = pvm_access_id_aarch64 }++/* Mark the specified system register as not being handled in hyp. */+#define HOST_HANDLED(REG) { SYS_DESC(REG), .access = NULL }++/*+ * Architected system registers.+ * Important: Must be sorted ascending by Op0, Op1, CRn, CRm, Op2+ *+ * NOTE: Anything not explicitly listed here is *restricted by default*, i.e.,+ * it will lead to injecting an exception into the guest.+ */+static const struct sys_reg_desc pvm_sys_reg_descs[] = {+ /* Cache maintenance by set/way operations are restricted. */++ /* Debug and Trace Registers are restricted. */++ /* AArch64 mappings of the AArch32 ID registers */+ /* CRm=1 */+ AARCH32(SYS_ID_PFR0_EL1),+ AARCH32(SYS_ID_PFR1_EL1),+ AARCH32(SYS_ID_DFR0_EL1),+ AARCH32(SYS_ID_AFR0_EL1),+ AARCH32(SYS_ID_MMFR0_EL1),+ AARCH32(SYS_ID_MMFR1_EL1),+ AARCH32(SYS_ID_MMFR2_EL1),+ AARCH32(SYS_ID_MMFR3_EL1),++ /* CRm=2 */+ AARCH32(SYS_ID_ISAR0_EL1),+ AARCH32(SYS_ID_ISAR1_EL1),+ AARCH32(SYS_ID_ISAR2_EL1),+ AARCH32(SYS_ID_ISAR3_EL1),+ AARCH32(SYS_ID_ISAR4_EL1),+ AARCH32(SYS_ID_ISAR5_EL1),+ AARCH32(SYS_ID_MMFR4_EL1),+ AARCH32(SYS_ID_ISAR6_EL1),++ /* CRm=3 */+ AARCH32(SYS_MVFR0_EL1),+ AARCH32(SYS_MVFR1_EL1),+ AARCH32(SYS_MVFR2_EL1),+ AARCH32(SYS_ID_PFR2_EL1),+ AARCH32(SYS_ID_DFR1_EL1),+ AARCH32(SYS_ID_MMFR5_EL1),++ /* AArch64 ID registers */+ /* CRm=4 */+ AARCH64(SYS_ID_AA64PFR0_EL1),+ AARCH64(SYS_ID_AA64PFR1_EL1),+ AARCH64(SYS_ID_AA64ZFR0_EL1),+ AARCH64(SYS_ID_AA64DFR0_EL1),+ AARCH64(SYS_ID_AA64DFR1_EL1),+ AARCH64(SYS_ID_AA64AFR0_EL1),+ AARCH64(SYS_ID_AA64AFR1_EL1),+ AARCH64(SYS_ID_AA64ISAR0_EL1),+ AARCH64(SYS_ID_AA64ISAR1_EL1),+ AARCH64(SYS_ID_AA64MMFR0_EL1),+ AARCH64(SYS_ID_AA64MMFR1_EL1),+ AARCH64(SYS_ID_AA64MMFR2_EL1),++ HOST_HANDLED(SYS_SCTLR_EL1),+ HOST_HANDLED(SYS_ACTLR_EL1),+ HOST_HANDLED(SYS_CPACR_EL1),++ HOST_HANDLED(SYS_RGSR_EL1),+ HOST_HANDLED(SYS_GCR_EL1),++ /* Scalable Vector Registers are restricted. */++ HOST_HANDLED(SYS_TTBR0_EL1),+ HOST_HANDLED(SYS_TTBR1_EL1),+ HOST_HANDLED(SYS_TCR_EL1),++ HOST_HANDLED(SYS_APIAKEYLO_EL1),+ HOST_HANDLED(SYS_APIAKEYHI_EL1),+ HOST_HANDLED(SYS_APIBKEYLO_EL1),+ HOST_HANDLED(SYS_APIBKEYHI_EL1),+ HOST_HANDLED(SYS_APDAKEYLO_EL1),+ HOST_HANDLED(SYS_APDAKEYHI_EL1),+ HOST_HANDLED(SYS_APDBKEYLO_EL1),+ HOST_HANDLED(SYS_APDBKEYHI_EL1),+ HOST_HANDLED(SYS_APGAKEYLO_EL1),+ HOST_HANDLED(SYS_APGAKEYHI_EL1),++ HOST_HANDLED(SYS_AFSR0_EL1),+ HOST_HANDLED(SYS_AFSR1_EL1),+ HOST_HANDLED(SYS_ESR_EL1),++ HOST_HANDLED(SYS_ERRIDR_EL1),+ HOST_HANDLED(SYS_ERRSELR_EL1),+ HOST_HANDLED(SYS_ERXFR_EL1),+ HOST_HANDLED(SYS_ERXCTLR_EL1),+ HOST_HANDLED(SYS_ERXSTATUS_EL1),+ HOST_HANDLED(SYS_ERXADDR_EL1),+ HOST_HANDLED(SYS_ERXMISC0_EL1),+ HOST_HANDLED(SYS_ERXMISC1_EL1),++ HOST_HANDLED(SYS_TFSR_EL1),+ HOST_HANDLED(SYS_TFSRE0_EL1),++ HOST_HANDLED(SYS_FAR_EL1),+ HOST_HANDLED(SYS_PAR_EL1),++ /* Performance Monitoring Registers are restricted. */++ HOST_HANDLED(SYS_MAIR_EL1),+ HOST_HANDLED(SYS_AMAIR_EL1),++ /* Limited Ordering Regions Registers are restricted. */++ HOST_HANDLED(SYS_VBAR_EL1),+ HOST_HANDLED(SYS_DISR_EL1),++ /* GIC CPU Interface registers are restricted. */++ HOST_HANDLED(SYS_CONTEXTIDR_EL1),+ HOST_HANDLED(SYS_TPIDR_EL1),++ HOST_HANDLED(SYS_SCXTNUM_EL1),++ HOST_HANDLED(SYS_CNTKCTL_EL1),++ HOST_HANDLED(SYS_CCSIDR_EL1),+ HOST_HANDLED(SYS_CLIDR_EL1),+ HOST_HANDLED(SYS_CSSELR_EL1),+ HOST_HANDLED(SYS_CTR_EL0),++ /* Performance Monitoring Registers are restricted. */++ HOST_HANDLED(SYS_TPIDR_EL0),+ HOST_HANDLED(SYS_TPIDRRO_EL0),++ HOST_HANDLED(SYS_SCXTNUM_EL0),++ /* Activity Monitoring Registers are restricted. */++ HOST_HANDLED(SYS_CNTP_TVAL_EL0),+ HOST_HANDLED(SYS_CNTP_CTL_EL0),+ HOST_HANDLED(SYS_CNTP_CVAL_EL0),++ /* Performance Monitoring Registers are restricted. */++ HOST_HANDLED(SYS_DACR32_EL2),+ HOST_HANDLED(SYS_IFSR32_EL2),+ HOST_HANDLED(SYS_FPEXC32_EL2),+};
It would be good if you had something that checks the ordering of this
array at boot time. It is incredibly easy to screw up the ordering,
and then everything goes subtly wrong.
quoted hunk
++/*+ * Handler for protected VM MSR, MRS or System instruction execution.+ *+ * Returns true if the hypervisor has handled the exit, and control should go+ * back to the guest, or false if it hasn't, to be handled by the host.+ */+bool kvm_handle_pvm_sysreg(struct kvm_vcpu *vcpu, u64 *exit_code)+{+ const struct sys_reg_desc *r;+ struct sys_reg_params params;+ unsigned long esr = kvm_vcpu_get_esr(vcpu);+ int Rt = kvm_vcpu_sys_get_rt(vcpu);++ params = esr_sys64_to_params(esr);+ params.regval = vcpu_get_reg(vcpu, Rt);++ r = find_reg(¶ms, pvm_sys_reg_descs, ARRAY_SIZE(pvm_sys_reg_descs));++ /* Undefined access (RESTRICTED). */+ if (r == NULL) {+ __inject_undef64(vcpu);+ return true;+ }++ /* Handled by the host (HOST_HANDLED) */+ if (r->access == NULL)+ return false;++ /* Handled by hyp: skip instruction if instructed to do so. */+ if (r->access(vcpu, ¶ms, r))+ __kvm_skip_instr(vcpu);++ if (!params.is_write)+ vcpu_set_reg(vcpu, Rt, params.regval);++ return true;+}
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Drew,
On Tue, Oct 5, 2021 at 9:53 AM Andrew Jones [off-list ref] wrote:
On Wed, Sep 22, 2021 at 01:47:00PM +0100, Fuad Tabba wrote:
quoted
Add system register handlers for protected VMs. These cover Sys64
registers (including feature id registers), and debug.
No functional change intended as these are not hooked in yet to
the guest exit handlers introduced earlier. So when trapping is
triggered, the exit handlers let the host handle it, as before.
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/include/asm/kvm_fixed_config.h | 195 ++++++++
arch/arm64/include/asm/kvm_hyp.h | 5 +
arch/arm64/kvm/arm.c | 5 +
arch/arm64/kvm/hyp/include/nvhe/sys_regs.h | 28 ++
arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
arch/arm64/kvm/hyp/nvhe/sys_regs.c | 492 +++++++++++++++++++++
6 files changed, 726 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/include/asm/kvm_fixed_config.h
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/sys_regs.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/sys_regs.c
@@ -0,0 +1,492 @@+// SPDX-License-Identifier: GPL-2.0-only+/*+*Copyright(C)2021GoogleLLC+*Author:FuadTabba<tabba@google.com>+*/++#include<asm/kvm_asm.h>+#include<asm/kvm_fixed_config.h>+#include<asm/kvm_mmu.h>++#include<hyp/adjust_pc.h>++#include"../../sys_regs.h"++/*+*Copiesofthehost'sCPUfeaturesregistersholdingsanitizedvaluesathyp.+*/+u64id_aa64pfr0_el1_sys_val;+u64id_aa64pfr1_el1_sys_val;+u64id_aa64isar0_el1_sys_val;+u64id_aa64isar1_el1_sys_val;+u64id_aa64mmfr2_el1_sys_val;++staticinlinevoidinject_undef64(structkvm_vcpu*vcpu)+{+u32esr=(ESR_ELx_EC_UNKNOWN<<ESR_ELx_EC_SHIFT);++vcpu->arch.flags|=(KVM_ARM64_EXCEPT_AA64_EL1|+KVM_ARM64_EXCEPT_AA64_ELx_SYNC|+KVM_ARM64_PENDING_EXCEPTION);++__kvm_adjust_pc(vcpu);++write_sysreg_el1(esr,SYS_ESR);+write_sysreg_el1(read_sysreg_el2(SYS_ELR),SYS_ELR);+}++/*+*Injectanunknown/undefinedexceptiontoanAArch64guestwhilemostofits+*sysregsarelive.+*/+void__inject_undef64(structkvm_vcpu*vcpu)+{+*vcpu_pc(vcpu)=read_sysreg_el2(SYS_ELR);+*vcpu_cpsr(vcpu)=read_sysreg_el2(SYS_SPSR);++inject_undef64(vcpu);++write_sysreg_el2(*vcpu_pc(vcpu),SYS_ELR);+write_sysreg_el2(*vcpu_cpsr(vcpu),SYS_SPSR);+}++/*+*Accessorforundefinedaccesses.+*/+staticboolundef_access(structkvm_vcpu*vcpu,+structsys_reg_params*p,+conststructsys_reg_desc*r)+{+__inject_undef64(vcpu);+returnfalse;+}++/*+*Returnstherestrictedfeaturesvaluesofthefeatureregisterbasedonthe+*limitationsinrestrict_fields.+*Afeatureidfieldvalueof0b0000doesnotimposeanyrestrictions.+*Note:Useonlyforunsignedfeaturefieldvalues.+*/+staticu64get_restricted_features_unsigned(u64sys_reg_val,+u64restrict_fields)+{+u64value=0UL;+u64mask=GENMASK_ULL(ARM64_FEATURE_FIELD_BITS-1,0);++/*+*AccordingtotheArmArchitectureReferenceManual,featurefields+*useincreasingvaluestoindicateincreasesinfunctionality.+*Iterateovertherestrictedfeaturefieldsandcalculatetheminimum+*unsignedvaluebetweentheonesupportedbythesystem,andwhatthe+*valueisbeingrestrictedto.+*/+while(sys_reg_val&&restrict_fields){+value|=min(sys_reg_val&mask,restrict_fields&mask);+sys_reg_val&=~mask;+restrict_fields&=~mask;+mask<<=ARM64_FEATURE_FIELD_BITS;+}++returnvalue;+}++/*+*FunctionsthatreturnthevalueoffeatureidregistersforprotectedVMs+*basedonallowedfeatures,systemfeatures,andKVMsupport.+*/++u64get_pvm_id_aa64pfr0(conststructkvm_vcpu*vcpu)+{+conststructkvm*kvm=(conststructkvm*)kern_hyp_va(vcpu->kvm);+u64set_mask=0;+u64allow_mask=PVM_ID_AA64PFR0_ALLOW;++if(!vcpu_has_sve(vcpu))+allow_mask&=~ARM64_FEATURE_MASK(ID_AA64PFR0_SVE);++set_mask|=get_restricted_features_unsigned(id_aa64pfr0_el1_sys_val,+PVM_ID_AA64PFR0_RESTRICT_UNSIGNED);++/* Spectre and Meltdown mitigation in KVM */+set_mask|=FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2),+(u64)kvm->arch.pfr0_csv2);+set_mask|=FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3),+(u64)kvm->arch.pfr0_csv3);++return(id_aa64pfr0_el1_sys_val&allow_mask)|set_mask;+}++u64get_pvm_id_aa64pfr1(conststructkvm_vcpu*vcpu)+{+conststructkvm*kvm=(conststructkvm*)kern_hyp_va(vcpu->kvm);+u64allow_mask=PVM_ID_AA64PFR1_ALLOW;++if(!kvm_has_mte(kvm))+allow_mask&=~ARM64_FEATURE_MASK(ID_AA64PFR1_MTE);++returnid_aa64pfr1_el1_sys_val&allow_mask;+}++u64get_pvm_id_aa64zfr0(conststructkvm_vcpu*vcpu)+{+/*+*NosupportforScalableVectors,therefore,hyphasnosanitized+*copyofthefeatureidregister.+*/+BUILD_BUG_ON(PVM_ID_AA64ZFR0_ALLOW!=0ULL);+return0;+}++u64get_pvm_id_aa64dfr0(conststructkvm_vcpu*vcpu)+{+/*+*Nosupportfordebug,includingbreakpoints,andwatchpoints,+*therefore,pKVMhasnosanitizedcopyofthefeatureidregister.+*/+BUILD_BUG_ON(PVM_ID_AA64DFR0_ALLOW!=0ULL);+return0;+}++u64get_pvm_id_aa64dfr1(conststructkvm_vcpu*vcpu)+{+/*+*Nosupportfordebug,therefore,hyphasnosanitizedcopyofthe+*featureidregister.+*/+BUILD_BUG_ON(PVM_ID_AA64DFR1_ALLOW!=0ULL);+return0;+}++u64get_pvm_id_aa64afr0(conststructkvm_vcpu*vcpu)+{+/*+*Nosupportforimplementationdefinedfeatures,therefore,hyphasno+*sanitizedcopyofthefeatureidregister.+*/+BUILD_BUG_ON(PVM_ID_AA64AFR0_ALLOW!=0ULL);+return0;+}++u64get_pvm_id_aa64afr1(conststructkvm_vcpu*vcpu)+{+/*+*Nosupportforimplementationdefinedfeatures,therefore,hyphasno+*sanitizedcopyofthefeatureidregister.+*/+BUILD_BUG_ON(PVM_ID_AA64AFR1_ALLOW!=0ULL);+return0;+}
Reading the same function five times make me wonder if a generator macro
wouldn't be better for these.
I think so too. I'll do that.
quoted
++u64 get_pvm_id_aa64isar0(const struct kvm_vcpu *vcpu)+{+ return id_aa64isar0_el1_sys_val & PVM_ID_AA64ISAR0_ALLOW;+}++u64 get_pvm_id_aa64isar1(const struct kvm_vcpu *vcpu)+{+ u64 allow_mask = PVM_ID_AA64ISAR1_ALLOW;++ if (!vcpu_has_ptrauth(vcpu))+ allow_mask &= ~(ARM64_FEATURE_MASK(ID_AA64ISAR1_APA) |+ ARM64_FEATURE_MASK(ID_AA64ISAR1_API) |+ ARM64_FEATURE_MASK(ID_AA64ISAR1_GPA) |+ ARM64_FEATURE_MASK(ID_AA64ISAR1_GPI));++ return id_aa64isar1_el1_sys_val & allow_mask;+}++u64 get_pvm_id_aa64mmfr0(const struct kvm_vcpu *vcpu)+{+ u64 set_mask;++ set_mask = get_restricted_features_unsigned(id_aa64mmfr0_el1_sys_val,+ PVM_ID_AA64MMFR0_RESTRICT_UNSIGNED);++ return (id_aa64mmfr0_el1_sys_val & PVM_ID_AA64MMFR0_ALLOW) | set_mask;+}++u64 get_pvm_id_aa64mmfr1(const struct kvm_vcpu *vcpu)+{+ return id_aa64mmfr1_el1_sys_val & PVM_ID_AA64MMFR1_ALLOW;+}++u64 get_pvm_id_aa64mmfr2(const struct kvm_vcpu *vcpu)+{+ return id_aa64mmfr2_el1_sys_val & PVM_ID_AA64MMFR2_ALLOW;+}++/* Read a sanitized cpufeature ID register by its sys_reg_desc. */+static u64 read_id_reg(const struct kvm_vcpu *vcpu,+ struct sys_reg_desc const *r)+{+ u32 id = reg_to_encoding(r);++ switch (id) {+ case SYS_ID_AA64PFR0_EL1:+ return get_pvm_id_aa64pfr0(vcpu);+ case SYS_ID_AA64PFR1_EL1:+ return get_pvm_id_aa64pfr1(vcpu);+ case SYS_ID_AA64ZFR0_EL1:+ return get_pvm_id_aa64zfr0(vcpu);+ case SYS_ID_AA64DFR0_EL1:+ return get_pvm_id_aa64dfr0(vcpu);+ case SYS_ID_AA64DFR1_EL1:+ return get_pvm_id_aa64dfr1(vcpu);+ case SYS_ID_AA64AFR0_EL1:+ return get_pvm_id_aa64afr0(vcpu);+ case SYS_ID_AA64AFR1_EL1:+ return get_pvm_id_aa64afr1(vcpu);+ case SYS_ID_AA64ISAR0_EL1:+ return get_pvm_id_aa64isar0(vcpu);+ case SYS_ID_AA64ISAR1_EL1:+ return get_pvm_id_aa64isar1(vcpu);+ case SYS_ID_AA64MMFR0_EL1:+ return get_pvm_id_aa64mmfr0(vcpu);+ case SYS_ID_AA64MMFR1_EL1:+ return get_pvm_id_aa64mmfr1(vcpu);+ case SYS_ID_AA64MMFR2_EL1:+ return get_pvm_id_aa64mmfr2(vcpu);+ default:+ /*+ * Should never happen because all cases are covered in+ * pvm_sys_reg_descs[] below.
I'd drop the 'below' word. It's not overly helpful and since code gets
moved it can go out of date.
Will fix.
quoted
+ */
+ WARN_ON(1);
The above cases could also be generated by a macro. And I wonder if we can
come up with something that makes sure these separate lists stay
consistent with macros and build bugs in order to better avoid these
"should never happen" situations.
Which ties in to Marc's comment for this patch. I'll handle this.
quoted
+ break;
+ }
+
+ return 0;
+}
+
+/*
+ * Accessor for AArch32 feature id registers.
+ *
+ * The value of these registers is "unknown" according to the spec if AArch32
+ * isn't supported.
+ */
+static bool pvm_access_id_aarch32(struct kvm_vcpu *vcpu,
+ struct sys_reg_params *p,
+ const struct sys_reg_desc *r)
+{
+ if (p->is_write)
+ return undef_access(vcpu, p, r);
+
+ /*
+ * No support for AArch32 guests, therefore, pKVM has no sanitized copy
+ * of AArch32 feature id registers.
+ */
+ BUILD_BUG_ON(FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1),
+ PVM_ID_AA64PFR0_RESTRICT_UNSIGNED) > ID_AA64PFR0_ELx_64BIT_ONLY);
+
+ /* Use 0 for architecturally "unknown" values. */
+ p->regval = 0;
+ return true;
+}
+
+/*
+ * Accessor for AArch64 feature id registers.
+ *
+ * If access is allowed, set the regval to the protected VM's view of the
+ * register and return true.
+ * Otherwise, inject an undefined exception and return false.
+ */
+static bool pvm_access_id_aarch64(struct kvm_vcpu *vcpu,
+ struct sys_reg_params *p,
+ const struct sys_reg_desc *r)
+{
+ if (p->is_write)
+ return undef_access(vcpu, p, r);
+
+ p->regval = read_id_reg(vcpu, r);
+ return true;
+}
+
+/* Mark the specified system register as an AArch32 feature id register. */
+#define AARCH32(REG) { SYS_DESC(REG), .access = pvm_access_id_aarch32 }
+
+/* Mark the specified system register as an AArch64 feature id register. */
+#define AARCH64(REG) { SYS_DESC(REG), .access = pvm_access_id_aarch64 }
+
+/* Mark the specified system register as not being handled in hyp. */
+#define HOST_HANDLED(REG) { SYS_DESC(REG), .access = NULL }
+
+/*
+ * Architected system registers.
+ * Important: Must be sorted ascending by Op0, Op1, CRn, CRm, Op2
+ *
+ * NOTE: Anything not explicitly listed here is *restricted by default*, i.e.,
+ * it will lead to injecting an exception into the guest.
+ */
+static const struct sys_reg_desc pvm_sys_reg_descs[] = {
+ /* Cache maintenance by set/way operations are restricted. */
+
+ /* Debug and Trace Registers are restricted. */
+
+ /* AArch64 mappings of the AArch32 ID registers */
+ /* CRm=1 */
+ AARCH32(SYS_ID_PFR0_EL1),
+ AARCH32(SYS_ID_PFR1_EL1),
+ AARCH32(SYS_ID_DFR0_EL1),
+ AARCH32(SYS_ID_AFR0_EL1),
+ AARCH32(SYS_ID_MMFR0_EL1),
+ AARCH32(SYS_ID_MMFR1_EL1),
+ AARCH32(SYS_ID_MMFR2_EL1),
+ AARCH32(SYS_ID_MMFR3_EL1),
+
+ /* CRm=2 */
+ AARCH32(SYS_ID_ISAR0_EL1),
+ AARCH32(SYS_ID_ISAR1_EL1),
+ AARCH32(SYS_ID_ISAR2_EL1),
+ AARCH32(SYS_ID_ISAR3_EL1),
+ AARCH32(SYS_ID_ISAR4_EL1),
+ AARCH32(SYS_ID_ISAR5_EL1),
+ AARCH32(SYS_ID_MMFR4_EL1),
+ AARCH32(SYS_ID_ISAR6_EL1),
+
+ /* CRm=3 */
+ AARCH32(SYS_MVFR0_EL1),
+ AARCH32(SYS_MVFR1_EL1),
+ AARCH32(SYS_MVFR2_EL1),
+ AARCH32(SYS_ID_PFR2_EL1),
+ AARCH32(SYS_ID_DFR1_EL1),
+ AARCH32(SYS_ID_MMFR5_EL1),
+
+ /* AArch64 ID registers */
+ /* CRm=4 */
+ AARCH64(SYS_ID_AA64PFR0_EL1),
+ AARCH64(SYS_ID_AA64PFR1_EL1),
+ AARCH64(SYS_ID_AA64ZFR0_EL1),
+ AARCH64(SYS_ID_AA64DFR0_EL1),
+ AARCH64(SYS_ID_AA64DFR1_EL1),
+ AARCH64(SYS_ID_AA64AFR0_EL1),
+ AARCH64(SYS_ID_AA64AFR1_EL1),
+ AARCH64(SYS_ID_AA64ISAR0_EL1),
+ AARCH64(SYS_ID_AA64ISAR1_EL1),
+ AARCH64(SYS_ID_AA64MMFR0_EL1),
+ AARCH64(SYS_ID_AA64MMFR1_EL1),
+ AARCH64(SYS_ID_AA64MMFR2_EL1),
+
+ HOST_HANDLED(SYS_SCTLR_EL1),
+ HOST_HANDLED(SYS_ACTLR_EL1),
+ HOST_HANDLED(SYS_CPACR_EL1),
+
+ HOST_HANDLED(SYS_RGSR_EL1),
+ HOST_HANDLED(SYS_GCR_EL1),
+
+ /* Scalable Vector Registers are restricted. */
+
+ HOST_HANDLED(SYS_TTBR0_EL1),
+ HOST_HANDLED(SYS_TTBR1_EL1),
+ HOST_HANDLED(SYS_TCR_EL1),
+
+ HOST_HANDLED(SYS_APIAKEYLO_EL1),
+ HOST_HANDLED(SYS_APIAKEYHI_EL1),
+ HOST_HANDLED(SYS_APIBKEYLO_EL1),
+ HOST_HANDLED(SYS_APIBKEYHI_EL1),
+ HOST_HANDLED(SYS_APDAKEYLO_EL1),
+ HOST_HANDLED(SYS_APDAKEYHI_EL1),
+ HOST_HANDLED(SYS_APDBKEYLO_EL1),
+ HOST_HANDLED(SYS_APDBKEYHI_EL1),
+ HOST_HANDLED(SYS_APGAKEYLO_EL1),
+ HOST_HANDLED(SYS_APGAKEYHI_EL1),
+
+ HOST_HANDLED(SYS_AFSR0_EL1),
+ HOST_HANDLED(SYS_AFSR1_EL1),
+ HOST_HANDLED(SYS_ESR_EL1),
+
+ HOST_HANDLED(SYS_ERRIDR_EL1),
+ HOST_HANDLED(SYS_ERRSELR_EL1),
+ HOST_HANDLED(SYS_ERXFR_EL1),
+ HOST_HANDLED(SYS_ERXCTLR_EL1),
+ HOST_HANDLED(SYS_ERXSTATUS_EL1),
+ HOST_HANDLED(SYS_ERXADDR_EL1),
+ HOST_HANDLED(SYS_ERXMISC0_EL1),
+ HOST_HANDLED(SYS_ERXMISC1_EL1),
+
+ HOST_HANDLED(SYS_TFSR_EL1),
+ HOST_HANDLED(SYS_TFSRE0_EL1),
+
+ HOST_HANDLED(SYS_FAR_EL1),
+ HOST_HANDLED(SYS_PAR_EL1),
+
+ /* Performance Monitoring Registers are restricted. */
+
+ HOST_HANDLED(SYS_MAIR_EL1),
+ HOST_HANDLED(SYS_AMAIR_EL1),
+
+ /* Limited Ordering Regions Registers are restricted. */
+
+ HOST_HANDLED(SYS_VBAR_EL1),
+ HOST_HANDLED(SYS_DISR_EL1),
+
+ /* GIC CPU Interface registers are restricted. */
+
+ HOST_HANDLED(SYS_CONTEXTIDR_EL1),
+ HOST_HANDLED(SYS_TPIDR_EL1),
+
+ HOST_HANDLED(SYS_SCXTNUM_EL1),
+
+ HOST_HANDLED(SYS_CNTKCTL_EL1),
+
+ HOST_HANDLED(SYS_CCSIDR_EL1),
+ HOST_HANDLED(SYS_CLIDR_EL1),
+ HOST_HANDLED(SYS_CSSELR_EL1),
+ HOST_HANDLED(SYS_CTR_EL0),
+
+ /* Performance Monitoring Registers are restricted. */
+
+ HOST_HANDLED(SYS_TPIDR_EL0),
+ HOST_HANDLED(SYS_TPIDRRO_EL0),
+
+ HOST_HANDLED(SYS_SCXTNUM_EL0),
+
+ /* Activity Monitoring Registers are restricted. */
+
+ HOST_HANDLED(SYS_CNTP_TVAL_EL0),
+ HOST_HANDLED(SYS_CNTP_CTL_EL0),
+ HOST_HANDLED(SYS_CNTP_CVAL_EL0),
+
+ /* Performance Monitoring Registers are restricted. */
+
+ HOST_HANDLED(SYS_DACR32_EL2),
+ HOST_HANDLED(SYS_IFSR32_EL2),
+ HOST_HANDLED(SYS_FPEXC32_EL2),
+};
+
+/*
+ * Handler for protected VM MSR, MRS or System instruction execution.
+ *
+ * Returns true if the hypervisor has handled the exit, and control should go
+ * back to the guest, or false if it hasn't, to be handled by the host.
+ */
+bool kvm_handle_pvm_sysreg(struct kvm_vcpu *vcpu, u64 *exit_code)
+{
+ const struct sys_reg_desc *r;
+ struct sys_reg_params params;
+ unsigned long esr = kvm_vcpu_get_esr(vcpu);
+ int Rt = kvm_vcpu_sys_get_rt(vcpu);
+
+ params = esr_sys64_to_params(esr);
+ params.regval = vcpu_get_reg(vcpu, Rt);
+
+ r = find_reg(¶ms, pvm_sys_reg_descs, ARRAY_SIZE(pvm_sys_reg_descs));
+
+ /* Undefined access (RESTRICTED). */
+ if (r == NULL) {
+ __inject_undef64(vcpu);
+ return true;
+ }
+
+ /* Handled by the host (HOST_HANDLED) */
+ if (r->access == NULL)
+ return false;
+
+ /* Handled by hyp: skip instruction if instructed to do so. */
+ if (r->access(vcpu, ¶ms, r))
+ __kvm_skip_instr(vcpu);
+
+ if (!params.is_write)
+ vcpu_set_reg(vcpu, Rt, params.regval);
+
+ return true;
+}
--
2.33.0.464.g1972c5931b-goog
Other than the nits and suggestion to try and build in some register list
consistency checks, this looks good to me. I don't know what pKVM
should / should not expose, but I like the approach this takes, so,
FWIW,
Reviewed-by: Andrew Jones <redacted>
Thank you,
/fuad
Thanks,
drew
--
To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
Add system register handlers for protected VMs. These cover Sys64
registers (including feature id registers), and debug.
No functional change intended as these are not hooked in yet to
the guest exit handlers introduced earlier. So when trapping is
triggered, the exit handlers let the host handle it, as before.
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/include/asm/kvm_fixed_config.h | 195 ++++++++
arch/arm64/include/asm/kvm_hyp.h | 5 +
arch/arm64/kvm/arm.c | 5 +
arch/arm64/kvm/hyp/include/nvhe/sys_regs.h | 28 ++
arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
arch/arm64/kvm/hyp/nvhe/sys_regs.c | 492 +++++++++++++++++++++
6 files changed, 726 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/include/asm/kvm_fixed_config.h
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/sys_regs.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/sys_regs.c
An access exception is the result of a memory access. undef_access
makes my head spin because you are conflating two unrelated terms.
I suggest you merge all three functions in a single inject_undef64().
Sure.
quoted
+}++/*+ * Returns the restricted features values of the feature register based on the+ * limitations in restrict_fields.+ * A feature id field value of 0b0000 does not impose any restrictions.+ * Note: Use only for unsigned feature field values.+ */+static u64 get_restricted_features_unsigned(u64 sys_reg_val,+ u64 restrict_fields)+{+ u64 value = 0UL;+ u64 mask = GENMASK_ULL(ARM64_FEATURE_FIELD_BITS - 1, 0);++ /*+ * According to the Arm Architecture Reference Manual, feature fields+ * use increasing values to indicate increases in functionality.+ * Iterate over the restricted feature fields and calculate the minimum+ * unsigned value between the one supported by the system, and what the+ * value is being restricted to.+ */+ while (sys_reg_val && restrict_fields) {+ value |= min(sys_reg_val & mask, restrict_fields & mask);+ sys_reg_val &= ~mask;+ restrict_fields &= ~mask;+ mask <<= ARM64_FEATURE_FIELD_BITS;+ }++ return value;+}++/*+ * Functions that return the value of feature id registers for protected VMs+ * based on allowed features, system features, and KVM support.+ */++u64 get_pvm_id_aa64pfr0(const struct kvm_vcpu *vcpu)+{+ const struct kvm *kvm = (const struct kvm *)kern_hyp_va(vcpu->kvm);+ u64 set_mask = 0;+ u64 allow_mask = PVM_ID_AA64PFR0_ALLOW;++ if (!vcpu_has_sve(vcpu))+ allow_mask &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_SVE);++ set_mask |= get_restricted_features_unsigned(id_aa64pfr0_el1_sys_val,+ PVM_ID_AA64PFR0_RESTRICT_UNSIGNED);++ /* Spectre and Meltdown mitigation in KVM */+ set_mask |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2),+ (u64)kvm->arch.pfr0_csv2);+ set_mask |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3),+ (u64)kvm->arch.pfr0_csv3);++ return (id_aa64pfr0_el1_sys_val & allow_mask) | set_mask;+}++u64 get_pvm_id_aa64pfr1(const struct kvm_vcpu *vcpu)+{+ const struct kvm *kvm = (const struct kvm *)kern_hyp_va(vcpu->kvm);+ u64 allow_mask = PVM_ID_AA64PFR1_ALLOW;++ if (!kvm_has_mte(kvm))+ allow_mask &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_MTE);++ return id_aa64pfr1_el1_sys_val & allow_mask;+}++u64 get_pvm_id_aa64zfr0(const struct kvm_vcpu *vcpu)+{+ /*+ * No support for Scalable Vectors, therefore, hyp has no sanitized+ * copy of the feature id register.+ */+ BUILD_BUG_ON(PVM_ID_AA64ZFR0_ALLOW != 0ULL);+ return 0;+}++u64 get_pvm_id_aa64dfr0(const struct kvm_vcpu *vcpu)+{+ /*+ * No support for debug, including breakpoints, and watchpoints,+ * therefore, pKVM has no sanitized copy of the feature id register.+ */+ BUILD_BUG_ON(PVM_ID_AA64DFR0_ALLOW != 0ULL);+ return 0;+}++u64 get_pvm_id_aa64dfr1(const struct kvm_vcpu *vcpu)+{+ /*+ * No support for debug, therefore, hyp has no sanitized copy of the+ * feature id register.+ */+ BUILD_BUG_ON(PVM_ID_AA64DFR1_ALLOW != 0ULL);+ return 0;+}++u64 get_pvm_id_aa64afr0(const struct kvm_vcpu *vcpu)+{+ /*+ * No support for implementation defined features, therefore, hyp has no+ * sanitized copy of the feature id register.+ */+ BUILD_BUG_ON(PVM_ID_AA64AFR0_ALLOW != 0ULL);+ return 0;+}++u64 get_pvm_id_aa64afr1(const struct kvm_vcpu *vcpu)+{+ /*+ * No support for implementation defined features, therefore, hyp has no+ * sanitized copy of the feature id register.+ */+ BUILD_BUG_ON(PVM_ID_AA64AFR1_ALLOW != 0ULL);+ return 0;+}++u64 get_pvm_id_aa64isar0(const struct kvm_vcpu *vcpu)+{+ return id_aa64isar0_el1_sys_val & PVM_ID_AA64ISAR0_ALLOW;+}++u64 get_pvm_id_aa64isar1(const struct kvm_vcpu *vcpu)+{+ u64 allow_mask = PVM_ID_AA64ISAR1_ALLOW;++ if (!vcpu_has_ptrauth(vcpu))+ allow_mask &= ~(ARM64_FEATURE_MASK(ID_AA64ISAR1_APA) |+ ARM64_FEATURE_MASK(ID_AA64ISAR1_API) |+ ARM64_FEATURE_MASK(ID_AA64ISAR1_GPA) |+ ARM64_FEATURE_MASK(ID_AA64ISAR1_GPI));++ return id_aa64isar1_el1_sys_val & allow_mask;+}++u64 get_pvm_id_aa64mmfr0(const struct kvm_vcpu *vcpu)+{+ u64 set_mask;++ set_mask = get_restricted_features_unsigned(id_aa64mmfr0_el1_sys_val,+ PVM_ID_AA64MMFR0_RESTRICT_UNSIGNED);++ return (id_aa64mmfr0_el1_sys_val & PVM_ID_AA64MMFR0_ALLOW) | set_mask;+}++u64 get_pvm_id_aa64mmfr1(const struct kvm_vcpu *vcpu)+{+ return id_aa64mmfr1_el1_sys_val & PVM_ID_AA64MMFR1_ALLOW;+}++u64 get_pvm_id_aa64mmfr2(const struct kvm_vcpu *vcpu)+{+ return id_aa64mmfr2_el1_sys_val & PVM_ID_AA64MMFR2_ALLOW;+}++/* Read a sanitized cpufeature ID register by its sys_reg_desc. */+static u64 read_id_reg(const struct kvm_vcpu *vcpu,+ struct sys_reg_desc const *r)+{+ u32 id = reg_to_encoding(r);++ switch (id) {+ case SYS_ID_AA64PFR0_EL1:+ return get_pvm_id_aa64pfr0(vcpu);+ case SYS_ID_AA64PFR1_EL1:+ return get_pvm_id_aa64pfr1(vcpu);+ case SYS_ID_AA64ZFR0_EL1:+ return get_pvm_id_aa64zfr0(vcpu);+ case SYS_ID_AA64DFR0_EL1:+ return get_pvm_id_aa64dfr0(vcpu);+ case SYS_ID_AA64DFR1_EL1:+ return get_pvm_id_aa64dfr1(vcpu);+ case SYS_ID_AA64AFR0_EL1:+ return get_pvm_id_aa64afr0(vcpu);+ case SYS_ID_AA64AFR1_EL1:+ return get_pvm_id_aa64afr1(vcpu);+ case SYS_ID_AA64ISAR0_EL1:+ return get_pvm_id_aa64isar0(vcpu);+ case SYS_ID_AA64ISAR1_EL1:+ return get_pvm_id_aa64isar1(vcpu);+ case SYS_ID_AA64MMFR0_EL1:+ return get_pvm_id_aa64mmfr0(vcpu);+ case SYS_ID_AA64MMFR1_EL1:+ return get_pvm_id_aa64mmfr1(vcpu);+ case SYS_ID_AA64MMFR2_EL1:+ return get_pvm_id_aa64mmfr2(vcpu);+ default:+ /*+ * Should never happen because all cases are covered in+ * pvm_sys_reg_descs[] below.+ */+ WARN_ON(1);+ break;+ }++ return 0;+}++/*+ * Accessor for AArch32 feature id registers.+ *+ * The value of these registers is "unknown" according to the spec if AArch32+ * isn't supported.+ */+static bool pvm_access_id_aarch32(struct kvm_vcpu *vcpu,+ struct sys_reg_params *p,+ const struct sys_reg_desc *r)+{+ if (p->is_write)+ return undef_access(vcpu, p, r);++ /*+ * No support for AArch32 guests, therefore, pKVM has no sanitized copy+ * of AArch32 feature id registers.+ */+ BUILD_BUG_ON(FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1),+ PVM_ID_AA64PFR0_RESTRICT_UNSIGNED) > ID_AA64PFR0_ELx_64BIT_ONLY);++ /* Use 0 for architecturally "unknown" values. */+ p->regval = 0;+ return true;+}++/*+ * Accessor for AArch64 feature id registers.+ *+ * If access is allowed, set the regval to the protected VM's view of the+ * register and return true.+ * Otherwise, inject an undefined exception and return false.+ */+static bool pvm_access_id_aarch64(struct kvm_vcpu *vcpu,+ struct sys_reg_params *p,+ const struct sys_reg_desc *r)+{+ if (p->is_write)+ return undef_access(vcpu, p, r);++ p->regval = read_id_reg(vcpu, r);+ return true;+}++/* Mark the specified system register as an AArch32 feature id register. */+#define AARCH32(REG) { SYS_DESC(REG), .access = pvm_access_id_aarch32 }++/* Mark the specified system register as an AArch64 feature id register. */+#define AARCH64(REG) { SYS_DESC(REG), .access = pvm_access_id_aarch64 }++/* Mark the specified system register as not being handled in hyp. */+#define HOST_HANDLED(REG) { SYS_DESC(REG), .access = NULL }++/*+ * Architected system registers.+ * Important: Must be sorted ascending by Op0, Op1, CRn, CRm, Op2+ *+ * NOTE: Anything not explicitly listed here is *restricted by default*, i.e.,+ * it will lead to injecting an exception into the guest.+ */+static const struct sys_reg_desc pvm_sys_reg_descs[] = {+ /* Cache maintenance by set/way operations are restricted. */++ /* Debug and Trace Registers are restricted. */++ /* AArch64 mappings of the AArch32 ID registers */+ /* CRm=1 */+ AARCH32(SYS_ID_PFR0_EL1),+ AARCH32(SYS_ID_PFR1_EL1),+ AARCH32(SYS_ID_DFR0_EL1),+ AARCH32(SYS_ID_AFR0_EL1),+ AARCH32(SYS_ID_MMFR0_EL1),+ AARCH32(SYS_ID_MMFR1_EL1),+ AARCH32(SYS_ID_MMFR2_EL1),+ AARCH32(SYS_ID_MMFR3_EL1),++ /* CRm=2 */+ AARCH32(SYS_ID_ISAR0_EL1),+ AARCH32(SYS_ID_ISAR1_EL1),+ AARCH32(SYS_ID_ISAR2_EL1),+ AARCH32(SYS_ID_ISAR3_EL1),+ AARCH32(SYS_ID_ISAR4_EL1),+ AARCH32(SYS_ID_ISAR5_EL1),+ AARCH32(SYS_ID_MMFR4_EL1),+ AARCH32(SYS_ID_ISAR6_EL1),++ /* CRm=3 */+ AARCH32(SYS_MVFR0_EL1),+ AARCH32(SYS_MVFR1_EL1),+ AARCH32(SYS_MVFR2_EL1),+ AARCH32(SYS_ID_PFR2_EL1),+ AARCH32(SYS_ID_DFR1_EL1),+ AARCH32(SYS_ID_MMFR5_EL1),++ /* AArch64 ID registers */+ /* CRm=4 */+ AARCH64(SYS_ID_AA64PFR0_EL1),+ AARCH64(SYS_ID_AA64PFR1_EL1),+ AARCH64(SYS_ID_AA64ZFR0_EL1),+ AARCH64(SYS_ID_AA64DFR0_EL1),+ AARCH64(SYS_ID_AA64DFR1_EL1),+ AARCH64(SYS_ID_AA64AFR0_EL1),+ AARCH64(SYS_ID_AA64AFR1_EL1),+ AARCH64(SYS_ID_AA64ISAR0_EL1),+ AARCH64(SYS_ID_AA64ISAR1_EL1),+ AARCH64(SYS_ID_AA64MMFR0_EL1),+ AARCH64(SYS_ID_AA64MMFR1_EL1),+ AARCH64(SYS_ID_AA64MMFR2_EL1),++ HOST_HANDLED(SYS_SCTLR_EL1),+ HOST_HANDLED(SYS_ACTLR_EL1),+ HOST_HANDLED(SYS_CPACR_EL1),++ HOST_HANDLED(SYS_RGSR_EL1),+ HOST_HANDLED(SYS_GCR_EL1),++ /* Scalable Vector Registers are restricted. */++ HOST_HANDLED(SYS_TTBR0_EL1),+ HOST_HANDLED(SYS_TTBR1_EL1),+ HOST_HANDLED(SYS_TCR_EL1),++ HOST_HANDLED(SYS_APIAKEYLO_EL1),+ HOST_HANDLED(SYS_APIAKEYHI_EL1),+ HOST_HANDLED(SYS_APIBKEYLO_EL1),+ HOST_HANDLED(SYS_APIBKEYHI_EL1),+ HOST_HANDLED(SYS_APDAKEYLO_EL1),+ HOST_HANDLED(SYS_APDAKEYHI_EL1),+ HOST_HANDLED(SYS_APDBKEYLO_EL1),+ HOST_HANDLED(SYS_APDBKEYHI_EL1),+ HOST_HANDLED(SYS_APGAKEYLO_EL1),+ HOST_HANDLED(SYS_APGAKEYHI_EL1),++ HOST_HANDLED(SYS_AFSR0_EL1),+ HOST_HANDLED(SYS_AFSR1_EL1),+ HOST_HANDLED(SYS_ESR_EL1),++ HOST_HANDLED(SYS_ERRIDR_EL1),+ HOST_HANDLED(SYS_ERRSELR_EL1),+ HOST_HANDLED(SYS_ERXFR_EL1),+ HOST_HANDLED(SYS_ERXCTLR_EL1),+ HOST_HANDLED(SYS_ERXSTATUS_EL1),+ HOST_HANDLED(SYS_ERXADDR_EL1),+ HOST_HANDLED(SYS_ERXMISC0_EL1),+ HOST_HANDLED(SYS_ERXMISC1_EL1),++ HOST_HANDLED(SYS_TFSR_EL1),+ HOST_HANDLED(SYS_TFSRE0_EL1),++ HOST_HANDLED(SYS_FAR_EL1),+ HOST_HANDLED(SYS_PAR_EL1),++ /* Performance Monitoring Registers are restricted. */++ HOST_HANDLED(SYS_MAIR_EL1),+ HOST_HANDLED(SYS_AMAIR_EL1),++ /* Limited Ordering Regions Registers are restricted. */++ HOST_HANDLED(SYS_VBAR_EL1),+ HOST_HANDLED(SYS_DISR_EL1),++ /* GIC CPU Interface registers are restricted. */++ HOST_HANDLED(SYS_CONTEXTIDR_EL1),+ HOST_HANDLED(SYS_TPIDR_EL1),++ HOST_HANDLED(SYS_SCXTNUM_EL1),++ HOST_HANDLED(SYS_CNTKCTL_EL1),++ HOST_HANDLED(SYS_CCSIDR_EL1),+ HOST_HANDLED(SYS_CLIDR_EL1),+ HOST_HANDLED(SYS_CSSELR_EL1),+ HOST_HANDLED(SYS_CTR_EL0),++ /* Performance Monitoring Registers are restricted. */++ HOST_HANDLED(SYS_TPIDR_EL0),+ HOST_HANDLED(SYS_TPIDRRO_EL0),++ HOST_HANDLED(SYS_SCXTNUM_EL0),++ /* Activity Monitoring Registers are restricted. */++ HOST_HANDLED(SYS_CNTP_TVAL_EL0),+ HOST_HANDLED(SYS_CNTP_CTL_EL0),+ HOST_HANDLED(SYS_CNTP_CVAL_EL0),++ /* Performance Monitoring Registers are restricted. */++ HOST_HANDLED(SYS_DACR32_EL2),+ HOST_HANDLED(SYS_IFSR32_EL2),+ HOST_HANDLED(SYS_FPEXC32_EL2),+};
It would be good if you had something that checks the ordering of this
array at boot time. It is incredibly easy to screw up the ordering,
and then everything goes subtly wrong.
Yes. I'll do something like check_sysreg_table().
Thanks,
/fuad
quoted
++/*+ * Handler for protected VM MSR, MRS or System instruction execution.+ *+ * Returns true if the hypervisor has handled the exit, and control should go+ * back to the guest, or false if it hasn't, to be handled by the host.+ */+bool kvm_handle_pvm_sysreg(struct kvm_vcpu *vcpu, u64 *exit_code)+{+ const struct sys_reg_desc *r;+ struct sys_reg_params params;+ unsigned long esr = kvm_vcpu_get_esr(vcpu);+ int Rt = kvm_vcpu_sys_get_rt(vcpu);++ params = esr_sys64_to_params(esr);+ params.regval = vcpu_get_reg(vcpu, Rt);++ r = find_reg(¶ms, pvm_sys_reg_descs, ARRAY_SIZE(pvm_sys_reg_descs));++ /* Undefined access (RESTRICTED). */+ if (r == NULL) {+ __inject_undef64(vcpu);+ return true;+ }++ /* Handled by the host (HOST_HANDLED) */+ if (r->access == NULL)+ return false;++ /* Handled by hyp: skip instruction if instructed to do so. */+ if (r->access(vcpu, ¶ms, r))+ __kvm_skip_instr(vcpu);++ if (!params.is_write)+ vcpu_set_reg(vcpu, Rt, params.regval);++ return true;+}
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
From: Andrew Jones <hidden> Date: 2021-10-06 06:58:40
On Wed, Sep 22, 2021 at 01:47:01PM +0100, Fuad Tabba wrote:
Protected VMs have more restricted features that need to be
trapped. Moreover, the host should not be trusted to set the
appropriate trapping registers and their values.
Initialize the trapping registers, i.e., hcr_el2, mdcr_el2, and
cptr_el2 at EL2 for protected guests, based on the values of the
guest's feature id registers.
No functional change intended as trap handlers introduced in the
previous patch are still not hooked in to the guest exit
handlers.
Signed-off-by: Fuad Tabba <redacted>
---
arch/arm64/include/asm/kvm_asm.h | 1 +
arch/arm64/include/asm/kvm_host.h | 2 +
arch/arm64/kvm/arm.c | 8 ++
arch/arm64/kvm/hyp/include/nvhe/pkvm.h | 14 ++
arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 10 ++
arch/arm64/kvm/hyp/nvhe/pkvm.c | 186 +++++++++++++++++++++++++
7 files changed, 222 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/pkvm.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/pkvm.c
Regarding the approach, LGTM
Reviewed-by: Andrew Jones <redacted>
Thanks,
drew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel