| Message ID | 20241025182354.3364124-9-oliver.upton@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: arm64: nv: Support for EL2 PMU controls | expand |
On Fri, 25 Oct 2024 19:23:43 +0100, Oliver Upton <oliver.upton@linux.dev> wrote: > > Wire up the other end of traps that affect host EL0 by actually > injecting them into the guest hypervisor. Skip over FGT entirely, as a > cursory glance suggests no FGT is effective in host EL0. Yes, and this (thankfully) is by design! :-) > > Note that kvm_inject_nested() is already equipped for handling > exceptions while the VM is already in a host context. > > Signed-off-by: Oliver Upton <oliver.upton@linux.dev> > --- > arch/arm64/include/asm/kvm_emulate.h | 5 +++++ > arch/arm64/kvm/emulate-nested.c | 29 ++++++++++++++++++++++++---- > 2 files changed, 30 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h > index a601a9305b10..bf0c48403f59 100644 > --- a/arch/arm64/include/asm/kvm_emulate.h > +++ b/arch/arm64/include/asm/kvm_emulate.h > @@ -225,6 +225,11 @@ static inline bool is_hyp_ctxt(const struct kvm_vcpu *vcpu) > return vcpu_has_nv(vcpu) && __is_hyp_ctxt(&vcpu->arch.ctxt); > } > > +static inline bool vcpu_is_host_el0(const struct kvm_vcpu *vcpu) > +{ > + return is_hyp_ctxt(vcpu) && !vcpu_is_el2(vcpu); > +} > + > /* > * The layout of SPSR for an AArch32 state is different when observed from an > * AArch64 SPSR_ELx or an AArch32 SPSR_*. This function generates the AArch32 > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c > index e1a30d1bcd06..db3149379a4d 100644 > --- a/arch/arm64/kvm/emulate-nested.c > +++ b/arch/arm64/kvm/emulate-nested.c > @@ -20,6 +20,9 @@ enum trap_behaviour { > BEHAVE_FORWARD_READ = BIT(0), > BEHAVE_FORWARD_WRITE = BIT(1), > BEHAVE_FORWARD_RW = BEHAVE_FORWARD_READ | BEHAVE_FORWARD_WRITE, > + > + /* Traps that take effect in Host EL0, this is rare! */ > + BEHAVE_IN_HOST_EL0 = BIT(2), nit: BEHAVE_IN_HOST_EL0 lacks an action verb (forward?). Thanks, M.
Hey, On Sat, Oct 26, 2024 at 09:13:17AM +0100, Marc Zyngier wrote: > On Fri, 25 Oct 2024 19:23:43 +0100, > Oliver Upton <oliver.upton@linux.dev> wrote: > > > > Wire up the other end of traps that affect host EL0 by actually > > injecting them into the guest hypervisor. Skip over FGT entirely, as a > > cursory glance suggests no FGT is effective in host EL0. > > Yes, and this (thankfully) is by design! :-) > > > > > Note that kvm_inject_nested() is already equipped for handling > > exceptions while the VM is already in a host context. > > > > Signed-off-by: Oliver Upton <oliver.upton@linux.dev> > > --- > > arch/arm64/include/asm/kvm_emulate.h | 5 +++++ > > arch/arm64/kvm/emulate-nested.c | 29 ++++++++++++++++++++++++---- > > 2 files changed, 30 insertions(+), 4 deletions(-) > > > > diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h > > index a601a9305b10..bf0c48403f59 100644 > > --- a/arch/arm64/include/asm/kvm_emulate.h > > +++ b/arch/arm64/include/asm/kvm_emulate.h > > @@ -225,6 +225,11 @@ static inline bool is_hyp_ctxt(const struct kvm_vcpu *vcpu) > > return vcpu_has_nv(vcpu) && __is_hyp_ctxt(&vcpu->arch.ctxt); > > } > > > > +static inline bool vcpu_is_host_el0(const struct kvm_vcpu *vcpu) > > +{ > > + return is_hyp_ctxt(vcpu) && !vcpu_is_el2(vcpu); > > +} > > + > > /* > > * The layout of SPSR for an AArch32 state is different when observed from an > > * AArch64 SPSR_ELx or an AArch32 SPSR_*. This function generates the AArch32 > > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c > > index e1a30d1bcd06..db3149379a4d 100644 > > --- a/arch/arm64/kvm/emulate-nested.c > > +++ b/arch/arm64/kvm/emulate-nested.c > > @@ -20,6 +20,9 @@ enum trap_behaviour { > > BEHAVE_FORWARD_READ = BIT(0), > > BEHAVE_FORWARD_WRITE = BIT(1), > > BEHAVE_FORWARD_RW = BEHAVE_FORWARD_READ | BEHAVE_FORWARD_WRITE, > > + > > + /* Traps that take effect in Host EL0, this is rare! */ > > + BEHAVE_IN_HOST_EL0 = BIT(2), > > nit: BEHAVE_IN_HOST_EL0 lacks an action verb (forward?). Thinking I'll squash this in (plus renaming in later patches): diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c index db3149379a4d..b072098ee44e 100644 --- a/arch/arm64/kvm/emulate-nested.c +++ b/arch/arm64/kvm/emulate-nested.c @@ -22,7 +22,7 @@ enum trap_behaviour { BEHAVE_FORWARD_RW = BEHAVE_FORWARD_READ | BEHAVE_FORWARD_WRITE, /* Traps that take effect in Host EL0, this is rare! */ - BEHAVE_IN_HOST_EL0 = BIT(2), + BEHAVE_FORWARD_IN_HOST_EL0 = BIT(2), }; struct trap_bits { @@ -2279,7 +2279,7 @@ bool triage_sysreg_trap(struct kvm_vcpu *vcpu, int *sr_index) b = compute_trap_behaviour(vcpu, tc); - if (!(b & BEHAVE_IN_HOST_EL0) && vcpu_is_host_el0(vcpu)) + if (!(b & BEHAVE_FORWARD_IN_HOST_EL0) && vcpu_is_host_el0(vcpu)) goto local; if (((b & BEHAVE_FORWARD_READ) && is_read) ||
On 10/26/24 20:05, Oliver Upton wrote: > Hey, > > On Sat, Oct 26, 2024 at 09:13:17AM +0100, Marc Zyngier wrote: >> On Fri, 25 Oct 2024 19:23:43 +0100, >> Oliver Upton <oliver.upton@linux.dev> wrote: >>> >>> Wire up the other end of traps that affect host EL0 by actually >>> injecting them into the guest hypervisor. Skip over FGT entirely, as a >>> cursory glance suggests no FGT is effective in host EL0. >> >> Yes, and this (thankfully) is by design! :-) >> >>> >>> Note that kvm_inject_nested() is already equipped for handling >>> exceptions while the VM is already in a host context. >>> >>> Signed-off-by: Oliver Upton <oliver.upton@linux.dev> >>> --- >>> arch/arm64/include/asm/kvm_emulate.h | 5 +++++ >>> arch/arm64/kvm/emulate-nested.c | 29 ++++++++++++++++++++++++---- >>> 2 files changed, 30 insertions(+), 4 deletions(-) >>> >>> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h >>> index a601a9305b10..bf0c48403f59 100644 >>> --- a/arch/arm64/include/asm/kvm_emulate.h >>> +++ b/arch/arm64/include/asm/kvm_emulate.h >>> @@ -225,6 +225,11 @@ static inline bool is_hyp_ctxt(const struct kvm_vcpu *vcpu) >>> return vcpu_has_nv(vcpu) && __is_hyp_ctxt(&vcpu->arch.ctxt); >>> } >>> >>> +static inline bool vcpu_is_host_el0(const struct kvm_vcpu *vcpu) >>> +{ >>> + return is_hyp_ctxt(vcpu) && !vcpu_is_el2(vcpu); >>> +} >>> + >>> /* >>> * The layout of SPSR for an AArch32 state is different when observed from an >>> * AArch64 SPSR_ELx or an AArch32 SPSR_*. This function generates the AArch32 >>> diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c >>> index e1a30d1bcd06..db3149379a4d 100644 >>> --- a/arch/arm64/kvm/emulate-nested.c >>> +++ b/arch/arm64/kvm/emulate-nested.c >>> @@ -20,6 +20,9 @@ enum trap_behaviour { >>> BEHAVE_FORWARD_READ = BIT(0), >>> BEHAVE_FORWARD_WRITE = BIT(1), >>> BEHAVE_FORWARD_RW = BEHAVE_FORWARD_READ | BEHAVE_FORWARD_WRITE, >>> + >>> + /* Traps that take effect in Host EL0, this is rare! */ >>> + BEHAVE_IN_HOST_EL0 = BIT(2), >> >> nit: BEHAVE_IN_HOST_EL0 lacks an action verb (forward?). > > Thinking I'll squash this in (plus renaming in later patches): Right, the following additional replacements are required for build. @@ -307,14 +307,14 @@ static const struct trap_bits coarse_trap_bits[] = { .value = MDCR_EL2_TPMCR, .mask = MDCR_EL2_TPMCR, .behaviour = BEHAVE_FORWARD_RW | - BEHAVE_IN_HOST_EL0, + BEHAVE_FORWARD_IN_HOST_EL0, }, [CGT_MDCR_TPM] = { .index = MDCR_EL2, .value = MDCR_EL2_TPM, .mask = MDCR_EL2_TPM, .behaviour = BEHAVE_FORWARD_RW | - BEHAVE_IN_HOST_EL0, + BEHAVE_FORWARD_IN_HOST_EL0, }, [CGT_MDCR_TDE] = { .index = MDCR_EL2, @@ -530,7 +530,7 @@ static enum trap_behaviour check_mdcr_hpmn(struct kvm_vcpu *vcpu) } if (kvm_pmu_counter_is_hyp(vcpu, idx)) - return BEHAVE_FORWARD_RW | BEHAVE_IN_HOST_EL0; + return BEHAVE_FORWARD_RW | BEHAVE_FORWARD_IN_HOST_EL0; return BEHAVE_HANDLE_LOCALLY; } > > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c > index db3149379a4d..b072098ee44e 100644 > --- a/arch/arm64/kvm/emulate-nested.c > +++ b/arch/arm64/kvm/emulate-nested.c > @@ -22,7 +22,7 @@ enum trap_behaviour { > BEHAVE_FORWARD_RW = BEHAVE_FORWARD_READ | BEHAVE_FORWARD_WRITE, > > /* Traps that take effect in Host EL0, this is rare! */ > - BEHAVE_IN_HOST_EL0 = BIT(2), > + BEHAVE_FORWARD_IN_HOST_EL0 = BIT(2), > }; > > struct trap_bits { > @@ -2279,7 +2279,7 @@ bool triage_sysreg_trap(struct kvm_vcpu *vcpu, int *sr_index) > > b = compute_trap_behaviour(vcpu, tc); > > - if (!(b & BEHAVE_IN_HOST_EL0) && vcpu_is_host_el0(vcpu)) > + if (!(b & BEHAVE_FORWARD_IN_HOST_EL0) && vcpu_is_host_el0(vcpu)) > goto local; > > if (((b & BEHAVE_FORWARD_READ) && is_read) || >
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h index a601a9305b10..bf0c48403f59 100644 --- a/arch/arm64/include/asm/kvm_emulate.h +++ b/arch/arm64/include/asm/kvm_emulate.h @@ -225,6 +225,11 @@ static inline bool is_hyp_ctxt(const struct kvm_vcpu *vcpu) return vcpu_has_nv(vcpu) && __is_hyp_ctxt(&vcpu->arch.ctxt); } +static inline bool vcpu_is_host_el0(const struct kvm_vcpu *vcpu) +{ + return is_hyp_ctxt(vcpu) && !vcpu_is_el2(vcpu); +} + /* * The layout of SPSR for an AArch32 state is different when observed from an * AArch64 SPSR_ELx or an AArch32 SPSR_*. This function generates the AArch32 diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c index e1a30d1bcd06..db3149379a4d 100644 --- a/arch/arm64/kvm/emulate-nested.c +++ b/arch/arm64/kvm/emulate-nested.c @@ -20,6 +20,9 @@ enum trap_behaviour { BEHAVE_FORWARD_READ = BIT(0), BEHAVE_FORWARD_WRITE = BIT(1), BEHAVE_FORWARD_RW = BEHAVE_FORWARD_READ | BEHAVE_FORWARD_WRITE, + + /* Traps that take effect in Host EL0, this is rare! */ + BEHAVE_IN_HOST_EL0 = BIT(2), }; struct trap_bits { @@ -2128,11 +2131,19 @@ static u64 kvm_get_sysreg_res0(struct kvm *kvm, enum vcpu_sysreg sr) return masks->mask[sr - __VNCR_START__].res0; } -static bool check_fgt_bit(struct kvm *kvm, bool is_read, +static bool check_fgt_bit(struct kvm_vcpu *vcpu, bool is_read, u64 val, const union trap_config tc) { + struct kvm *kvm = vcpu->kvm; enum vcpu_sysreg sr; + /* + * KVM doesn't know about any FGTs that apply to the host, and hopefully + * that'll remain the case. + */ + if (is_hyp_ctxt(vcpu)) + return false; + if (tc.pol) return (val & BIT(tc.bit)); @@ -2209,7 +2220,15 @@ bool triage_sysreg_trap(struct kvm_vcpu *vcpu, int *sr_index) * If we're not nesting, immediately return to the caller, with the * sysreg index, should we have it. */ - if (!vcpu_has_nv(vcpu) || is_hyp_ctxt(vcpu)) + if (!vcpu_has_nv(vcpu)) + goto local; + + /* + * There are a few traps that take effect InHost, but are constrained + * to EL0. Don't bother with computing the trap behaviour if the vCPU + * isn't in EL0. + */ + if (is_hyp_ctxt(vcpu) && !vcpu_is_host_el0(vcpu)) goto local; switch ((enum fgt_group_id)tc.fgt) { @@ -2255,12 +2274,14 @@ bool triage_sysreg_trap(struct kvm_vcpu *vcpu, int *sr_index) goto local; } - if (tc.fgt != __NO_FGT_GROUP__ && check_fgt_bit(vcpu->kvm, is_read, - val, tc)) + if (tc.fgt != __NO_FGT_GROUP__ && check_fgt_bit(vcpu, is_read, val, tc)) goto inject; b = compute_trap_behaviour(vcpu, tc); + if (!(b & BEHAVE_IN_HOST_EL0) && vcpu_is_host_el0(vcpu)) + goto local; + if (((b & BEHAVE_FORWARD_READ) && is_read) || ((b & BEHAVE_FORWARD_WRITE) && !is_read)) goto inject;
Wire up the other end of traps that affect host EL0 by actually injecting them into the guest hypervisor. Skip over FGT entirely, as a cursory glance suggests no FGT is effective in host EL0. Note that kvm_inject_nested() is already equipped for handling exceptions while the VM is already in a host context. Signed-off-by: Oliver Upton <oliver.upton@linux.dev> --- arch/arm64/include/asm/kvm_emulate.h | 5 +++++ arch/arm64/kvm/emulate-nested.c | 29 ++++++++++++++++++++++++---- 2 files changed, 30 insertions(+), 4 deletions(-)