| Message ID | 20230728082952.959212-14-maz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: NV trap forwarding infrastructure | expand |
Hi Marc, On 7/28/23 10:29, Marc Zyngier wrote: > As we're about to majorly extend the handling of FGT registers, > restructure the code to actually save/restore the registers > as required. This is made easy thanks to the previous addition > of the EL2 registers, allowing us to use the host context for > this purpose. > > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > arch/arm64/include/asm/kvm_arm.h | 21 ++++++++++ > arch/arm64/kvm/hyp/include/hyp/switch.h | 56 +++++++++++++------------ > 2 files changed, 50 insertions(+), 27 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h > index 028049b147df..85908aa18908 100644 > --- a/arch/arm64/include/asm/kvm_arm.h > +++ b/arch/arm64/include/asm/kvm_arm.h > @@ -333,6 +333,27 @@ > BIT(18) | \ > GENMASK(16, 15)) > > +/* > + * FGT register definitions > + * > + * RES0 and polarity masks as of DDI0487J.a, to be updated as needed. > + * We're not using the generated masks as they are usually ahead of > + * the published ARM ARM, which we use as a reference. > + * > + * Once we get to a point where the two describe the same thing, we'll > + * merge the definitions. One day. > + */ > +#define __HFGRTR_EL2_RES0 (GENMASK(63, 56) | GENMASK(53, 51)) > +#define __HFGRTR_EL2_MASK GENMASK(49, 0) > +#define __HFGRTR_EL2_nMASK (GENMASK(55, 54) | BIT(50)) > + > +#define __HFGWTR_EL2_RES0 (GENMASK(63, 56) | GENMASK(53, 51) | \ > + BIT(46) | BIT(42) | BIT(40) | BIT(28) | \ > + GENMASK(26, 25) | BIT(21) | BIT(18) | \ > + GENMASK(15, 14) | GENMASK(10, 9) | BIT(2)) > +#define __HFGWTR_EL2_MASK GENMASK(49, 0) > +#define __HFGWTR_EL2_nMASK (GENMASK(55, 54) | BIT(50)) > + > /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */ > #define HPFAR_MASK (~UL(0xf)) > /* > diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h > index 4bddb8541bec..966295178aee 100644 > --- a/arch/arm64/kvm/hyp/include/hyp/switch.h > +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h > @@ -70,20 +70,19 @@ static inline void __activate_traps_fpsimd32(struct kvm_vcpu *vcpu) > } > } > > -static inline bool __hfgxtr_traps_required(void) > -{ > - if (cpus_have_final_cap(ARM64_SME)) > - return true; > - > - if (cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_38)) > - return true; > > - return false; > -} > > -static inline void __activate_traps_hfgxtr(void) > +static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu) > { > + struct kvm_cpu_context *hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt; > u64 r_clr = 0, w_clr = 0, r_set = 0, w_set = 0, tmp; > + u64 r_val, w_val; > + > + if (!cpus_have_final_cap(ARM64_HAS_FGT)) > + return; > + > + ctxt_sys_reg(hctxt, HFGRTR_EL2) = read_sysreg_s(SYS_HFGRTR_EL2); > + ctxt_sys_reg(hctxt, HFGWTR_EL2) = read_sysreg_s(SYS_HFGWTR_EL2); > > if (cpus_have_final_cap(ARM64_SME)) { > tmp = HFGxTR_EL2_nSMPRI_EL1_MASK | HFGxTR_EL2_nTPIDR2_EL0_MASK; > @@ -98,26 +97,31 @@ static inline void __activate_traps_hfgxtr(void) > if (cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_38)) > w_set |= HFGxTR_EL2_TCR_EL1_MASK; > > - sysreg_clear_set_s(SYS_HFGRTR_EL2, r_clr, r_set); > - sysreg_clear_set_s(SYS_HFGWTR_EL2, w_clr, w_set); > + > + /* The default is not to trap amything but ACCDATA_EL1 */ > + r_val = __HFGRTR_EL2_nMASK & ~HFGxTR_EL2_nACCDATA_EL1; > + r_val |= r_set; > + r_val &= ~r_clr; > + > + w_val = __HFGWTR_EL2_nMASK & ~HFGxTR_EL2_nACCDATA_EL1; > + w_val |= w_set; > + w_val &= ~w_clr; > + > + write_sysreg_s(r_val, SYS_HFGRTR_EL2); > + write_sysreg_s(w_val, SYS_HFGWTR_EL2); > } > > -static inline void __deactivate_traps_hfgxtr(void) > +static inline void __deactivate_traps_hfgxtr(struct kvm_vcpu *vcpu) > { > - u64 r_clr = 0, w_clr = 0, r_set = 0, w_set = 0, tmp; > + struct kvm_cpu_context *hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt; > > - if (cpus_have_final_cap(ARM64_SME)) { > - tmp = HFGxTR_EL2_nSMPRI_EL1_MASK | HFGxTR_EL2_nTPIDR2_EL0_MASK; > + if (!cpus_have_final_cap(ARM64_HAS_FGT)) > + return; > > - r_set |= tmp; > - w_set |= tmp; > - } > + write_sysreg_s(ctxt_sys_reg(hctxt, HFGRTR_EL2), SYS_HFGRTR_EL2); > + write_sysreg_s(ctxt_sys_reg(hctxt, HFGWTR_EL2), SYS_HFGWTR_EL2); > > - if (cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_38)) > - w_clr |= HFGxTR_EL2_TCR_EL1_MASK; > > - sysreg_clear_set_s(SYS_HFGRTR_EL2, r_clr, r_set); > - sysreg_clear_set_s(SYS_HFGWTR_EL2, w_clr, w_set); > } > > static inline void __activate_traps_common(struct kvm_vcpu *vcpu) > @@ -145,8 +149,7 @@ static inline void __activate_traps_common(struct kvm_vcpu *vcpu) > vcpu->arch.mdcr_el2_host = read_sysreg(mdcr_el2); > write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2); > > - if (__hfgxtr_traps_required()) > - __activate_traps_hfgxtr(); > + __activate_traps_hfgxtr(vcpu); > } > > static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu) > @@ -162,8 +165,7 @@ static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu) > vcpu_clear_flag(vcpu, PMUSERENR_ON_CPU); > } > > - if (__hfgxtr_traps_required()) > - __deactivate_traps_hfgxtr(); > + __deactivate_traps_hfgxtr(vcpu); > } > > static inline void ___activate_traps(struct kvm_vcpu *vcpu) Reviewed-by: Eric Auger <eric.auger@redhat.com> Thanks Eric
Hey Marc, Looks good, just a typo to fix. On Fri, Jul 28, 2023 at 09:29:39AM +0100, Marc Zyngier wrote: > As we're about to majorly extend the handling of FGT registers, > restructure the code to actually save/restore the registers > as required. This is made easy thanks to the previous addition > of the EL2 registers, allowing us to use the host context for > this purpose. > > Signed-off-by: Marc Zyngier <maz@kernel.org> Reviewed-by: Oliver Upton <oliver.upton@linux.dev> > -static inline void __activate_traps_hfgxtr(void) > +static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu) > { > + struct kvm_cpu_context *hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt; > u64 r_clr = 0, w_clr = 0, r_set = 0, w_set = 0, tmp; > + u64 r_val, w_val; > + > + if (!cpus_have_final_cap(ARM64_HAS_FGT)) > + return; > + > + ctxt_sys_reg(hctxt, HFGRTR_EL2) = read_sysreg_s(SYS_HFGRTR_EL2); > + ctxt_sys_reg(hctxt, HFGWTR_EL2) = read_sysreg_s(SYS_HFGWTR_EL2); > > if (cpus_have_final_cap(ARM64_SME)) { > tmp = HFGxTR_EL2_nSMPRI_EL1_MASK | HFGxTR_EL2_nTPIDR2_EL0_MASK; > @@ -98,26 +97,31 @@ static inline void __activate_traps_hfgxtr(void) > if (cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_38)) > w_set |= HFGxTR_EL2_TCR_EL1_MASK; > > - sysreg_clear_set_s(SYS_HFGRTR_EL2, r_clr, r_set); > - sysreg_clear_set_s(SYS_HFGWTR_EL2, w_clr, w_set); > + > + /* The default is not to trap amything but ACCDATA_EL1 */ typo: anything
Hi Marc, > On 28 Jul 2023, at 08:29, Marc Zyngier <maz@kernel.org> wrote: > > As we're about to majorly extend the handling of FGT registers, > restructure the code to actually save/restore the registers > as required. This is made easy thanks to the previous addition > of the EL2 registers, allowing us to use the host context for > this purpose. > > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > arch/arm64/include/asm/kvm_arm.h | 21 ++++++++++ > arch/arm64/kvm/hyp/include/hyp/switch.h | 56 +++++++++++++------------ > 2 files changed, 50 insertions(+), 27 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h > index 028049b147df..85908aa18908 100644 > --- a/arch/arm64/include/asm/kvm_arm.h > +++ b/arch/arm64/include/asm/kvm_arm.h > @@ -333,6 +333,27 @@ > BIT(18) | \ > GENMASK(16, 15)) > > +/* > + * FGT register definitions > + * > + * RES0 and polarity masks as of DDI0487J.a, to be updated as needed. > + * We're not using the generated masks as they are usually ahead of > + * the published ARM ARM, which we use as a reference. > + * > + * Once we get to a point where the two describe the same thing, we'll > + * merge the definitions. One day. > + */ > +#define __HFGRTR_EL2_RES0 (GENMASK(63, 56) | GENMASK(53, 51)) > +#define __HFGRTR_EL2_MASK GENMASK(49, 0) > +#define __HFGRTR_EL2_nMASK (GENMASK(55, 54) | BIT(50)) > + > +#define __HFGWTR_EL2_RES0 (GENMASK(63, 56) | GENMASK(53, 51) | \ > + BIT(46) | BIT(42) | BIT(40) | BIT(28) | \ > + GENMASK(26, 25) | BIT(21) | BIT(18) | \ > + GENMASK(15, 14) | GENMASK(10, 9) | BIT(2)) > +#define __HFGWTR_EL2_MASK GENMASK(49, 0) > +#define __HFGWTR_EL2_nMASK (GENMASK(55, 54) | BIT(50)) > + > /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */ > #define HPFAR_MASK (~UL(0xf)) > /* > diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h > index 4bddb8541bec..966295178aee 100644 > --- a/arch/arm64/kvm/hyp/include/hyp/switch.h > +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h > @@ -70,20 +70,19 @@ static inline void __activate_traps_fpsimd32(struct kvm_vcpu *vcpu) > } > } > > -static inline bool __hfgxtr_traps_required(void) > -{ > - if (cpus_have_final_cap(ARM64_SME)) > - return true; > - > - if (cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_38)) > - return true; > > - return false; > -} > > -static inline void __activate_traps_hfgxtr(void) > +static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu) > { > + struct kvm_cpu_context *hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt; > u64 r_clr = 0, w_clr = 0, r_set = 0, w_set = 0, tmp; > + u64 r_val, w_val; > + > + if (!cpus_have_final_cap(ARM64_HAS_FGT)) > + return; > + > + ctxt_sys_reg(hctxt, HFGRTR_EL2) = read_sysreg_s(SYS_HFGRTR_EL2); > + ctxt_sys_reg(hctxt, HFGWTR_EL2) = read_sysreg_s(SYS_HFGWTR_EL2); > > if (cpus_have_final_cap(ARM64_SME)) { > tmp = HFGxTR_EL2_nSMPRI_EL1_MASK | HFGxTR_EL2_nTPIDR2_EL0_MASK; > @@ -98,26 +97,31 @@ static inline void __activate_traps_hfgxtr(void) > if (cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_38)) > w_set |= HFGxTR_EL2_TCR_EL1_MASK; > > - sysreg_clear_set_s(SYS_HFGRTR_EL2, r_clr, r_set); > - sysreg_clear_set_s(SYS_HFGWTR_EL2, w_clr, w_set); > + > + /* The default is not to trap amything but ACCDATA_EL1 */ > + r_val = __HFGRTR_EL2_nMASK & ~HFGxTR_EL2_nACCDATA_EL1; > + r_val |= r_set; > + r_val &= ~r_clr; > + > + w_val = __HFGWTR_EL2_nMASK & ~HFGxTR_EL2_nACCDATA_EL1; > + w_val |= w_set; > + w_val &= ~w_clr; > + > + write_sysreg_s(r_val, SYS_HFGRTR_EL2); > + write_sysreg_s(w_val, SYS_HFGWTR_EL2); > } > > -static inline void __deactivate_traps_hfgxtr(void) > +static inline void __deactivate_traps_hfgxtr(struct kvm_vcpu *vcpu) > { > - u64 r_clr = 0, w_clr = 0, r_set = 0, w_set = 0, tmp; > + struct kvm_cpu_context *hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt; > > - if (cpus_have_final_cap(ARM64_SME)) { > - tmp = HFGxTR_EL2_nSMPRI_EL1_MASK | HFGxTR_EL2_nTPIDR2_EL0_MASK; > + if (!cpus_have_final_cap(ARM64_HAS_FGT)) > + return; > > - r_set |= tmp; > - w_set |= tmp; > - } > + write_sysreg_s(ctxt_sys_reg(hctxt, HFGRTR_EL2), SYS_HFGRTR_EL2); > + write_sysreg_s(ctxt_sys_reg(hctxt, HFGWTR_EL2), SYS_HFGWTR_EL2); > > - if (cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_38)) > - w_clr |= HFGxTR_EL2_TCR_EL1_MASK; > > - sysreg_clear_set_s(SYS_HFGRTR_EL2, r_clr, r_set); > - sysreg_clear_set_s(SYS_HFGWTR_EL2, w_clr, w_set); > } > > static inline void __activate_traps_common(struct kvm_vcpu *vcpu) > @@ -145,8 +149,7 @@ static inline void __activate_traps_common(struct kvm_vcpu *vcpu) > vcpu->arch.mdcr_el2_host = read_sysreg(mdcr_el2); > write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2); > > - if (__hfgxtr_traps_required()) > - __activate_traps_hfgxtr(); > + __activate_traps_hfgxtr(vcpu); > } > > static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu) > @@ -162,8 +165,7 @@ static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu) > vcpu_clear_flag(vcpu, PMUSERENR_ON_CPU); > } > > - if (__hfgxtr_traps_required()) > - __deactivate_traps_hfgxtr(); > + __deactivate_traps_hfgxtr(vcpu); Reviewed-by: Miguel Luis <miguel.luis@oracle.com> Thanks Miguel > } > > static inline void ___activate_traps(struct kvm_vcpu *vcpu) > -- > 2.34.1 >
diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h index 028049b147df..85908aa18908 100644 --- a/arch/arm64/include/asm/kvm_arm.h +++ b/arch/arm64/include/asm/kvm_arm.h @@ -333,6 +333,27 @@ BIT(18) | \ GENMASK(16, 15)) +/* + * FGT register definitions + * + * RES0 and polarity masks as of DDI0487J.a, to be updated as needed. + * We're not using the generated masks as they are usually ahead of + * the published ARM ARM, which we use as a reference. + * + * Once we get to a point where the two describe the same thing, we'll + * merge the definitions. One day. + */ +#define __HFGRTR_EL2_RES0 (GENMASK(63, 56) | GENMASK(53, 51)) +#define __HFGRTR_EL2_MASK GENMASK(49, 0) +#define __HFGRTR_EL2_nMASK (GENMASK(55, 54) | BIT(50)) + +#define __HFGWTR_EL2_RES0 (GENMASK(63, 56) | GENMASK(53, 51) | \ + BIT(46) | BIT(42) | BIT(40) | BIT(28) | \ + GENMASK(26, 25) | BIT(21) | BIT(18) | \ + GENMASK(15, 14) | GENMASK(10, 9) | BIT(2)) +#define __HFGWTR_EL2_MASK GENMASK(49, 0) +#define __HFGWTR_EL2_nMASK (GENMASK(55, 54) | BIT(50)) + /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */ #define HPFAR_MASK (~UL(0xf)) /* diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h index 4bddb8541bec..966295178aee 100644 --- a/arch/arm64/kvm/hyp/include/hyp/switch.h +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h @@ -70,20 +70,19 @@ static inline void __activate_traps_fpsimd32(struct kvm_vcpu *vcpu) } } -static inline bool __hfgxtr_traps_required(void) -{ - if (cpus_have_final_cap(ARM64_SME)) - return true; - - if (cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_38)) - return true; - return false; -} -static inline void __activate_traps_hfgxtr(void) +static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu) { + struct kvm_cpu_context *hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt; u64 r_clr = 0, w_clr = 0, r_set = 0, w_set = 0, tmp; + u64 r_val, w_val; + + if (!cpus_have_final_cap(ARM64_HAS_FGT)) + return; + + ctxt_sys_reg(hctxt, HFGRTR_EL2) = read_sysreg_s(SYS_HFGRTR_EL2); + ctxt_sys_reg(hctxt, HFGWTR_EL2) = read_sysreg_s(SYS_HFGWTR_EL2); if (cpus_have_final_cap(ARM64_SME)) { tmp = HFGxTR_EL2_nSMPRI_EL1_MASK | HFGxTR_EL2_nTPIDR2_EL0_MASK; @@ -98,26 +97,31 @@ static inline void __activate_traps_hfgxtr(void) if (cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_38)) w_set |= HFGxTR_EL2_TCR_EL1_MASK; - sysreg_clear_set_s(SYS_HFGRTR_EL2, r_clr, r_set); - sysreg_clear_set_s(SYS_HFGWTR_EL2, w_clr, w_set); + + /* The default is not to trap amything but ACCDATA_EL1 */ + r_val = __HFGRTR_EL2_nMASK & ~HFGxTR_EL2_nACCDATA_EL1; + r_val |= r_set; + r_val &= ~r_clr; + + w_val = __HFGWTR_EL2_nMASK & ~HFGxTR_EL2_nACCDATA_EL1; + w_val |= w_set; + w_val &= ~w_clr; + + write_sysreg_s(r_val, SYS_HFGRTR_EL2); + write_sysreg_s(w_val, SYS_HFGWTR_EL2); } -static inline void __deactivate_traps_hfgxtr(void) +static inline void __deactivate_traps_hfgxtr(struct kvm_vcpu *vcpu) { - u64 r_clr = 0, w_clr = 0, r_set = 0, w_set = 0, tmp; + struct kvm_cpu_context *hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt; - if (cpus_have_final_cap(ARM64_SME)) { - tmp = HFGxTR_EL2_nSMPRI_EL1_MASK | HFGxTR_EL2_nTPIDR2_EL0_MASK; + if (!cpus_have_final_cap(ARM64_HAS_FGT)) + return; - r_set |= tmp; - w_set |= tmp; - } + write_sysreg_s(ctxt_sys_reg(hctxt, HFGRTR_EL2), SYS_HFGRTR_EL2); + write_sysreg_s(ctxt_sys_reg(hctxt, HFGWTR_EL2), SYS_HFGWTR_EL2); - if (cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_38)) - w_clr |= HFGxTR_EL2_TCR_EL1_MASK; - sysreg_clear_set_s(SYS_HFGRTR_EL2, r_clr, r_set); - sysreg_clear_set_s(SYS_HFGWTR_EL2, w_clr, w_set); } static inline void __activate_traps_common(struct kvm_vcpu *vcpu) @@ -145,8 +149,7 @@ static inline void __activate_traps_common(struct kvm_vcpu *vcpu) vcpu->arch.mdcr_el2_host = read_sysreg(mdcr_el2); write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2); - if (__hfgxtr_traps_required()) - __activate_traps_hfgxtr(); + __activate_traps_hfgxtr(vcpu); } static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu) @@ -162,8 +165,7 @@ static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu) vcpu_clear_flag(vcpu, PMUSERENR_ON_CPU); } - if (__hfgxtr_traps_required()) - __deactivate_traps_hfgxtr(); + __deactivate_traps_hfgxtr(vcpu); } static inline void ___activate_traps(struct kvm_vcpu *vcpu)
As we're about to majorly extend the handling of FGT registers, restructure the code to actually save/restore the registers as required. This is made easy thanks to the previous addition of the EL2 registers, allowing us to use the host context for this purpose. Signed-off-by: Marc Zyngier <maz@kernel.org> --- arch/arm64/include/asm/kvm_arm.h | 21 ++++++++++ arch/arm64/kvm/hyp/include/hyp/switch.h | 56 +++++++++++++------------ 2 files changed, 50 insertions(+), 27 deletions(-)