| Message ID | 20210510165920.1913477-1-maz@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | KVM: arm64: ARMv8.3/8.4 Nested Virtualization support | expand |
Hi Marc, On Mon, May 10, 2021 at 05:58:14PM +0100, Marc Zyngier wrote: > Here the bi-annual drop of the KVM/arm64 NV support code. > > Not a lot has changed since [1], except for a discovery mechanism for > the EL2 support, some tidying up in the idreg emulation, dropping RMR > support, and a rebase on top of 5.13-rc1. > > As usual, blame me for any bug, and nobody else. > > It is still massively painful to run on the FVP, but if you have a > Neoverse V1 or N2 system that is collecting dust, I have the right > stuff to keep it busy! I've been testing this series on FVP and get a crash when returning from __kvm_vcpu_run_vhe because the autiasp is failing. The problem is when the L1 boots and during EL2 setup sets hcr_el2 to HCR_HOST_NVHE_FLAGS and so enables HCR_APK|HCR_API. Then the guest enter+exit logic in L0 starts performing the key save restore, but as we didn't go through __hyp_handle_ptrauth, we haven't saved the host keys and invoked vcpu_ptrauth_enable() so restore the host keys back to 0. I wonder if the pointer auth keys should be saved+restored unconditionally for a guest when running nested rather than the lazy faulting that we have today? Alternatively we would need to duplicate the lazy logic for hcr_el2 writes. A quick hack of saving the host keys in __kvm_vcpu_run_vhe before sysreg_save_host_state_vhe is enough to allow me to boot an L1 with --nested and then an L2. Do we also need to filter out HCR_APK|HCR_API for hcr_el2 writes when pointer authentication hasn't been exposed to the guest? I haven't yet tried making ptrauth visible to the L1. Thanks, Jamie
Hi Jamie, Funny, your email has a "Mail-Followup-To:" field that contains everyone but you... Not ideal! ;-) On Thu, 03 Jun 2021 08:07:22 +0100, Jamie Iles <jamie@nuviainc.com> wrote: > > Hi Marc, > > On Mon, May 10, 2021 at 05:58:14PM +0100, Marc Zyngier wrote: > > Here the bi-annual drop of the KVM/arm64 NV support code. > > > > Not a lot has changed since [1], except for a discovery mechanism for > > the EL2 support, some tidying up in the idreg emulation, dropping RMR > > support, and a rebase on top of 5.13-rc1. > > > > As usual, blame me for any bug, and nobody else. > > > > It is still massively painful to run on the FVP, but if you have a > > Neoverse V1 or N2 system that is collecting dust, I have the right > > stuff to keep it busy! > > I've been testing this series on FVP and get a crash when returning from > __kvm_vcpu_run_vhe because the autiasp is failing. Ah, the joy of testing with older guests. I guess i should upgrade by test rig and play with some newer guests at L1. > > The problem is when the L1 boots and during EL2 setup sets hcr_el2 to > HCR_HOST_NVHE_FLAGS and so enables HCR_APK|HCR_API. Then the guest > enter+exit logic in L0 starts performing the key save restore, but as we > didn't go through __hyp_handle_ptrauth, we haven't saved the host keys > and invoked vcpu_ptrauth_enable() so restore the host keys back to 0. > > I wonder if the pointer auth keys should be saved+restored > unconditionally for a guest when running nested rather than the lazy > faulting that we have today? I'd like to try and avoid that in order to keep the basic logic as simple as possible for the time being, and as close to the tried and trusted flow we have today. > Alternatively we would need to duplicate > the lazy logic for hcr_el2 writes. A quick hack of saving the host keys > in __kvm_vcpu_run_vhe before sysreg_save_host_state_vhe is enough to > allow me to boot an L1 with --nested and then an L2. > > Do we also need to filter out HCR_APK|HCR_API for hcr_el2 writes when > pointer authentication hasn't been exposed to the guest? I haven't yet > tried making ptrauth visible to the L1. I think this is the real thing. We should never propagate trap bits for features we don't want to support in guests. The L1 kernel sets these bits unconditionally, despite PtrAuth never being advertised, which trips the host code. Could you try the untested hack below? Thanks, M. diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h index ce682bcce56f..54301d5ce58c 100644 --- a/arch/arm64/include/asm/kvm_arm.h +++ b/arch/arm64/include/asm/kvm_arm.h @@ -82,6 +82,7 @@ HCR_BSU_IS | HCR_FB | HCR_TAC | \ HCR_AMO | HCR_SWIO | HCR_TIDCP | HCR_RW | HCR_TLOR | \ HCR_FMO | HCR_IMO | HCR_PTW ) +#define HCR_GUEST_NV_FILTER_FLAGS (HCR_ATA | HCR_API | HCR_APK | HCR_RW) #define HCR_VIRT_EXCP_MASK (HCR_VSE | HCR_VI | HCR_VF) #define HCR_HOST_NVHE_FLAGS (HCR_RW | HCR_API | HCR_APK | HCR_ATA) #define HCR_HOST_NVHE_PROTECTED_FLAGS (HCR_HOST_NVHE_FLAGS | HCR_TSC) diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c index 67f8b7d89db6..bf39bf4ef63c 100644 --- a/arch/arm64/kvm/hyp/vhe/switch.c +++ b/arch/arm64/kvm/hyp/vhe/switch.c @@ -64,6 +64,8 @@ static void __activate_traps(struct kvm_vcpu *vcpu) */ u64 vhcr_el2 = __vcpu_sys_reg(vcpu, HCR_EL2); + vhcr_el2 &= ~HCR_GUEST_NV_FILTER_FLAGS; + /* * We already set TVM to handle set/way cache maint * ops traps, this somewhat collides with the nested @@ -91,7 +93,10 @@ static void __activate_traps(struct kvm_vcpu *vcpu) SYS_VNCR_EL2); } } else if (nested_virt_in_use(vcpu)) { - hcr |= __vcpu_sys_reg(vcpu, HCR_EL2); + u64 vhcr_el2 = __vcpu_sys_reg(vcpu, HCR_EL2); + + vhcr_el2 &= ~HCR_GUEST_NV_FILTER_FLAGS; + hcr |= vhcr_el2; } ___activate_traps(vcpu, hcr);
On Thu, 03 Jun 2021 09:39:09 +0100, Marc Zyngier <maz@kernel.org> wrote: > > Hi Jamie, > > Funny, your email has a "Mail-Followup-To:" field that contains > everyone but you... Not ideal! ;-) > > On Thu, 03 Jun 2021 08:07:22 +0100, > Jamie Iles <jamie@nuviainc.com> wrote: > > > > Hi Marc, > > > > On Mon, May 10, 2021 at 05:58:14PM +0100, Marc Zyngier wrote: > > > Here the bi-annual drop of the KVM/arm64 NV support code. > > > > > > Not a lot has changed since [1], except for a discovery mechanism for > > > the EL2 support, some tidying up in the idreg emulation, dropping RMR > > > support, and a rebase on top of 5.13-rc1. > > > > > > As usual, blame me for any bug, and nobody else. > > > > > > It is still massively painful to run on the FVP, but if you have a > > > Neoverse V1 or N2 system that is collecting dust, I have the right > > > stuff to keep it busy! > > > > I've been testing this series on FVP and get a crash when returning from > > __kvm_vcpu_run_vhe because the autiasp is failing. > > Ah, the joy of testing with older guests. I guess i should upgrade by > test rig and play with some newer guests at L1. > > > > > The problem is when the L1 boots and during EL2 setup sets hcr_el2 to > > HCR_HOST_NVHE_FLAGS and so enables HCR_APK|HCR_API. Then the guest > > enter+exit logic in L0 starts performing the key save restore, but as we > > didn't go through __hyp_handle_ptrauth, we haven't saved the host keys > > and invoked vcpu_ptrauth_enable() so restore the host keys back to 0. > > > > I wonder if the pointer auth keys should be saved+restored > > unconditionally for a guest when running nested rather than the lazy > > faulting that we have today? > > I'd like to try and avoid that in order to keep the basic logic as > simple as possible for the time being, and as close to the tried and > trusted flow we have today. > > > Alternatively we would need to duplicate > > the lazy logic for hcr_el2 writes. A quick hack of saving the host keys > > in __kvm_vcpu_run_vhe before sysreg_save_host_state_vhe is enough to > > allow me to boot an L1 with --nested and then an L2. > > > > Do we also need to filter out HCR_APK|HCR_API for hcr_el2 writes when > > pointer authentication hasn't been exposed to the guest? I haven't yet > > tried making ptrauth visible to the L1. > > I think this is the real thing. We should never propagate trap bits > for features we don't want to support in guests. The L1 kernel sets > these bits unconditionally, despite PtrAuth never being advertised, > which trips the host code. > > Could you try the untested hack below? > > Thanks, > > M. > > diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h > index ce682bcce56f..54301d5ce58c 100644 > --- a/arch/arm64/include/asm/kvm_arm.h > +++ b/arch/arm64/include/asm/kvm_arm.h > @@ -82,6 +82,7 @@ > HCR_BSU_IS | HCR_FB | HCR_TAC | \ > HCR_AMO | HCR_SWIO | HCR_TIDCP | HCR_RW | HCR_TLOR | \ > HCR_FMO | HCR_IMO | HCR_PTW ) > +#define HCR_GUEST_NV_FILTER_FLAGS (HCR_ATA | HCR_API | HCR_APK | HCR_RW) > #define HCR_VIRT_EXCP_MASK (HCR_VSE | HCR_VI | HCR_VF) > #define HCR_HOST_NVHE_FLAGS (HCR_RW | HCR_API | HCR_APK | HCR_ATA) > #define HCR_HOST_NVHE_PROTECTED_FLAGS (HCR_HOST_NVHE_FLAGS | HCR_TSC) > diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c > index 67f8b7d89db6..bf39bf4ef63c 100644 > --- a/arch/arm64/kvm/hyp/vhe/switch.c > +++ b/arch/arm64/kvm/hyp/vhe/switch.c > @@ -64,6 +64,8 @@ static void __activate_traps(struct kvm_vcpu *vcpu) > */ > u64 vhcr_el2 = __vcpu_sys_reg(vcpu, HCR_EL2); > > + vhcr_el2 &= ~HCR_GUEST_NV_FILTER_FLAGS; > + > /* > * We already set TVM to handle set/way cache maint > * ops traps, this somewhat collides with the nested > @@ -91,7 +93,10 @@ static void __activate_traps(struct kvm_vcpu *vcpu) > SYS_VNCR_EL2); > } > } else if (nested_virt_in_use(vcpu)) { > - hcr |= __vcpu_sys_reg(vcpu, HCR_EL2); > + u64 vhcr_el2 = __vcpu_sys_reg(vcpu, HCR_EL2); > + > + vhcr_el2 &= ~HCR_GUEST_NV_FILTER_FLAGS; > + hcr |= vhcr_el2; > } > > ___activate_traps(vcpu, hcr); > FWIW, I can boot a 5.13-rc4 kernel as a L1 guest without any visible issue with this patch (and another guest as L2). I've folded this into the series and pushed the result out (with a rebase on -rc4 for a good measure). Thanks again, M.
On Thu, Jun 03, 2021 at 09:39:09AM +0100, Marc Zyngier wrote: > Hi Jamie, > > Funny, your email has a "Mail-Followup-To:" field that contains > everyone but you... Not ideal! ;-) Oops, new mutt config, thanks. > On Thu, 03 Jun 2021 08:07:22 +0100, > Jamie Iles <jamie@nuviainc.com> wrote: > > > > Hi Marc, > > > > On Mon, May 10, 2021 at 05:58:14PM +0100, Marc Zyngier wrote: > > > Here the bi-annual drop of the KVM/arm64 NV support code. > > > > > > Not a lot has changed since [1], except for a discovery mechanism for > > > the EL2 support, some tidying up in the idreg emulation, dropping RMR > > > support, and a rebase on top of 5.13-rc1. > > > > > > As usual, blame me for any bug, and nobody else. > > > > > > It is still massively painful to run on the FVP, but if you have a > > > Neoverse V1 or N2 system that is collecting dust, I have the right > > > stuff to keep it busy! > > > > I've been testing this series on FVP and get a crash when returning from > > __kvm_vcpu_run_vhe because the autiasp is failing. > > Ah, the joy of testing with older guests. I guess i should upgrade by > test rig and play with some newer guests at L1. > > > > > The problem is when the L1 boots and during EL2 setup sets hcr_el2 to > > HCR_HOST_NVHE_FLAGS and so enables HCR_APK|HCR_API. Then the guest > > enter+exit logic in L0 starts performing the key save restore, but as we > > didn't go through __hyp_handle_ptrauth, we haven't saved the host keys > > and invoked vcpu_ptrauth_enable() so restore the host keys back to 0. > > > > I wonder if the pointer auth keys should be saved+restored > > unconditionally for a guest when running nested rather than the lazy > > faulting that we have today? > > I'd like to try and avoid that in order to keep the basic logic as > simple as possible for the time being, and as close to the tried and > trusted flow we have today. > > > Alternatively we would need to duplicate > > the lazy logic for hcr_el2 writes. A quick hack of saving the host keys > > in __kvm_vcpu_run_vhe before sysreg_save_host_state_vhe is enough to > > allow me to boot an L1 with --nested and then an L2. > > > > Do we also need to filter out HCR_APK|HCR_API for hcr_el2 writes when > > pointer authentication hasn't been exposed to the guest? I haven't yet > > tried making ptrauth visible to the L1. > > I think this is the real thing. We should never propagate trap bits > for features we don't want to support in guests. The L1 kernel sets > these bits unconditionally, despite PtrAuth never being advertised, > which trips the host code. > > Could you try the untested hack below? That fixes the issue that I was seeing, lgtm. Thanks Marc! Jamie
Here the bi-annual drop of the KVM/arm64 NV support code. Not a lot has changed since [1], except for a discovery mechanism for the EL2 support, some tidying up in the idreg emulation, dropping RMR support, and a rebase on top of 5.13-rc1. As usual, blame me for any bug, and nobody else. It is still massively painful to run on the FVP, but if you have a Neoverse V1 or N2 system that is collecting dust, I have the right stuff to keep it busy! M. [1] https://lore.kernel.org/r/20201210160002.1407373-1-maz@kernel.org Andre Przywara (1): KVM: arm64: nv: vgic: Allow userland to set VGIC maintenance IRQ Christoffer Dall (15): KVM: arm64: nv: Introduce nested virtualization VCPU feature KVM: arm64: nv: Reset VCPU to EL2 registers if VCPU nested virt is set KVM: arm64: nv: Allow userspace to set PSR_MODE_EL2x KVM: arm64: nv: Add nested virt VCPU primitives for vEL2 VCPU state KVM: arm64: nv: Reset VMPIDR_EL2 and VPIDR_EL2 to sane values KVM: arm64: nv: Handle trapped ERET from virtual EL2 KVM: arm64: nv: Emulate PSTATE.M for a guest hypervisor KVM: arm64: nv: Trap EL1 VM register accesses in virtual EL2 KVM: arm64: nv: Only toggle cache for virtual EL2 when SCTLR_EL2 changes KVM: arm64: nv: Implement nested Stage-2 page table walk logic KVM: arm64: nv: Unmap/flush shadow stage 2 page tables KVM: arm64: nv: arch_timer: Support hyp timer emulation KVM: arm64: nv: vgic: Emulate the HW bit in software KVM: arm64: nv: Add nested GICv3 tracepoints KVM: arm64: nv: Sync nested timer state with ARMv8.4 Jintack Lim (18): arm64: Add ARM64_HAS_NESTED_VIRT cpufeature KVM: arm64: nv: Handle HCR_EL2.NV system register traps KVM: arm64: nv: Support virtual EL2 exceptions KVM: arm64: nv: Inject HVC exceptions to the virtual EL2 KVM: arm64: nv: Trap SPSR_EL1, ELR_EL1 and VBAR_EL1 from virtual EL2 KVM: arm64: nv: Trap CPACR_EL1 access in virtual EL2 KVM: arm64: nv: Handle PSCI call via smc from the guest KVM: arm64: nv: Respect virtual HCR_EL2.TWX setting KVM: arm64: nv: Respect virtual CPTR_EL2.{TFP,FPEN} settings KVM: arm64: nv: Respect the virtual HCR_EL2.NV bit setting KVM: arm64: nv: Respect virtual HCR_EL2.TVM and TRVM settings KVM: arm64: nv: Respect the virtual HCR_EL2.NV1 bit setting KVM: arm64: nv: Emulate EL12 register accesses from the virtual EL2 KVM: arm64: nv: Configure HCR_EL2 for nested virtualization KVM: arm64: nv: Introduce sys_reg_desc.forward_trap KVM: arm64: nv: Set a handler for the system instruction traps KVM: arm64: nv: Trap and emulate AT instructions from virtual EL2 KVM: arm64: nv: Nested GICv3 Support Marc Zyngier (32): KVM: arm64: nv: Add EL2 system registers to vcpu context KVM: arm64: nv: Add non-VHE-EL2->EL1 translation helpers KVM: arm64: nv: Handle virtual EL2 registers in vcpu_read/write_sys_reg() KVM: arm64: nv: Handle SPSR_EL2 specially KVM: arm64: nv: Handle HCR_EL2.E2H specially KVM: arm64: nv: Save/Restore vEL2 sysregs KVM: arm64: nv: Forward debug traps to the nested guest KVM: arm64: nv: Filter out unsupported features from ID regs KVM: arm64: nv: Hide RAS from nested guests KVM: arm64: nv: Support multiple nested Stage-2 mmu structures KVM: arm64: nv: Handle shadow stage 2 page faults KVM: arm64: nv: Restrict S2 RD/WR permissions to match the guest's KVM: arm64: nv: Trap and emulate TLBI instructions from virtual EL2 KVM: arm64: nv: Fold guest's HCR_EL2 configuration into the host's KVM: arm64: nv: Add handling of EL2-specific timer registers KVM: arm64: nv: Load timer before the GIC KVM: arm64: nv: Don't load the GICv4 context on entering a nested guest KVM: arm64: nv: Implement maintenance interrupt forwarding KVM: arm64: nv: Allow userspace to request KVM_ARM_VCPU_NESTED_VIRT KVM: arm64: nv: Add handling of ARMv8.4-TTL TLB invalidation KVM: arm64: nv: Invalidate TLBs based on shadow S2 TTL-like information KVM: arm64: Allow populating S2 SW bits KVM: arm64: nv: Tag shadow S2 entries with nested level KVM: arm64: nv: Add include containing the VNCR_EL2 offsets KVM: arm64: Map VNCR-capable registers to a separate page KVM: arm64: nv: Move nested vgic state into the sysreg file KVM: arm64: Add ARMv8.4 Enhanced Nested Virt cpufeature KVM: arm64: nv: Synchronize PSTATE early on exit KVM: arm64: nv: Allocate VNCR page when required KVM: arm64: nv: Enable ARMv8.4-NV support KVM: arm64: nv: Fast-track 'InHost' exception returns KVM: arm64: nv: Fast-track EL1 TLBIs for VHE guests .../admin-guide/kernel-parameters.txt | 4 + .../virt/kvm/devices/arm-vgic-v3.rst | 12 +- arch/arm64/include/asm/cpucaps.h | 2 + arch/arm64/include/asm/esr.h | 6 + arch/arm64/include/asm/kvm_arm.h | 28 +- arch/arm64/include/asm/kvm_asm.h | 4 + arch/arm64/include/asm/kvm_emulate.h | 145 +- arch/arm64/include/asm/kvm_host.h | 174 ++- arch/arm64/include/asm/kvm_hyp.h | 2 + arch/arm64/include/asm/kvm_mmu.h | 18 +- arch/arm64/include/asm/kvm_nested.h | 152 +++ arch/arm64/include/asm/kvm_pgtable.h | 10 + arch/arm64/include/asm/sysreg.h | 105 +- arch/arm64/include/asm/vncr_mapping.h | 73 + arch/arm64/include/uapi/asm/kvm.h | 2 + arch/arm64/kernel/cpufeature.c | 35 + arch/arm64/kvm/Makefile | 4 +- arch/arm64/kvm/arch_timer.c | 189 ++- arch/arm64/kvm/arm.c | 37 +- arch/arm64/kvm/at.c | 231 ++++ arch/arm64/kvm/emulate-nested.c | 186 +++ arch/arm64/kvm/guest.c | 6 + arch/arm64/kvm/handle_exit.c | 81 +- arch/arm64/kvm/hyp/exception.c | 45 +- arch/arm64/kvm/hyp/include/hyp/switch.h | 28 +- arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 28 +- arch/arm64/kvm/hyp/nvhe/switch.c | 10 +- arch/arm64/kvm/hyp/nvhe/sysreg-sr.c | 2 +- arch/arm64/kvm/hyp/pgtable.c | 6 + arch/arm64/kvm/hyp/vgic-v3-sr.c | 2 +- arch/arm64/kvm/hyp/vhe/switch.c | 207 ++- arch/arm64/kvm/hyp/vhe/sysreg-sr.c | 125 +- arch/arm64/kvm/hyp/vhe/tlb.c | 83 ++ arch/arm64/kvm/inject_fault.c | 63 +- arch/arm64/kvm/mmu.c | 191 ++- arch/arm64/kvm/nested.c | 910 ++++++++++++ arch/arm64/kvm/reset.c | 14 +- arch/arm64/kvm/sys_regs.c | 1215 ++++++++++++++++- arch/arm64/kvm/sys_regs.h | 8 + arch/arm64/kvm/trace_arm.h | 65 +- arch/arm64/kvm/vgic/vgic-init.c | 30 + arch/arm64/kvm/vgic/vgic-kvm-device.c | 22 + arch/arm64/kvm/vgic/vgic-nested-trace.h | 137 ++ arch/arm64/kvm/vgic/vgic-v3-nested.c | 240 ++++ arch/arm64/kvm/vgic/vgic-v3.c | 39 +- arch/arm64/kvm/vgic/vgic.c | 44 + arch/arm64/kvm/vgic/vgic.h | 10 + include/kvm/arm_arch_timer.h | 7 + include/kvm/arm_vgic.h | 16 + include/uapi/linux/kvm.h | 1 + tools/arch/arm/include/uapi/asm/kvm.h | 1 + 51 files changed, 4893 insertions(+), 162 deletions(-) create mode 100644 arch/arm64/include/asm/kvm_nested.h create mode 100644 arch/arm64/include/asm/vncr_mapping.h create mode 100644 arch/arm64/kvm/at.c create mode 100644 arch/arm64/kvm/emulate-nested.c create mode 100644 arch/arm64/kvm/nested.c create mode 100644 arch/arm64/kvm/vgic/vgic-nested-trace.h create mode 100644 arch/arm64/kvm/vgic/vgic-v3-nested.c