| Message ID | 20241112103717.589952-12-james.clark@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | kvm/coresight: Support exclude guest and exclude host | expand |
On Tue, Nov 12, 2024 at 10:37:10AM +0000, James Clark wrote: > +void kvm_set_trfcr(u64 host_trfcr, u64 guest_trfcr) > +{ > + if (kvm_arm_skip_trace_state()) > + return; > + > + if (has_vhe()) > + write_sysreg_s(guest_trfcr, SYS_TRFCR_EL12); > + else > + if (host_trfcr != guest_trfcr) { > + *host_data_ptr(host_debug_state.trfcr_el1) = guest_trfcr; Huh? That's going into host_debug_state, which is the dumping grounds for *host* context when entering a guest. Not sure why we'd stick a *guest* value in there... > + host_data_set_flag(HOST_STATE_SWAP_TRFCR); > + } else > + host_data_clear_flag(HOST_STATE_SWAP_TRFCR); > +} > +EXPORT_SYMBOL_GPL(kvm_set_trfcr); I have a rather strong distaste for this interface, both with the coresight driver and internally with the hypervisor. It'd be better if the driver actually told KVM what the *intent* is rather than throwing a pile of bits over the fence and forcing KVM to interpret what that configuration means. > +static void __debug_swap_trace(void) > +{ > + u64 trfcr = read_sysreg_el1(SYS_TRFCR); > + > + write_sysreg_el1(*host_data_ptr(host_debug_state.trfcr_el1), SYS_TRFCR); > + *host_data_ptr(host_debug_state.trfcr_el1) = trfcr; > + host_data_set_flag(HOST_STATE_RESTORE_TRFCR); > +} > + What if trace is disabled in the guest or in the host? Do we need to synchronize when transitioning from an enabled -> disabled state like we do today? I took a stab at this, completely untested of course && punts on protected mode. But this is _generally_ how I'd like to see everything fit together. diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 8bc0ec151684..b4714cece5f0 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -611,7 +611,7 @@ struct cpu_sve_state { */ struct kvm_host_data { #define KVM_HOST_DATA_FLAG_HAS_SPE 0 -#define KVM_HOST_DATA_FLAG_HAS_TRBE 1 +#define KVM_HOST_DATA_FLAG_HOST_TRBE_ENABLED 1 #define KVM_HOST_DATA_FLAG_HOST_SVE_ENABLED 2 #define KVM_HOST_DATA_FLAG_HOST_SME_ENABLED 3 unsigned long flags; @@ -659,6 +659,9 @@ struct kvm_host_data { u64 mdcr_el2; } host_debug_state; + /* Guest trace filter value */ + u64 guest_trfcr_el1; + /* Number of programmable event counters (PMCR_EL0.N) for this CPU */ unsigned int nr_event_counters; @@ -1381,6 +1384,8 @@ static inline bool kvm_pmu_counter_deferred(struct perf_event_attr *attr) void kvm_set_pmu_events(u64 set, struct perf_event_attr *attr); void kvm_clr_pmu_events(u64 clr); bool kvm_set_pmuserenr(u64 val); +void kvm_enable_trbe(u64 guest_trfcr); +void kvm_disable_trbe(void); #else static inline void kvm_set_pmu_events(u64 set, struct perf_event_attr *attr) {} static inline void kvm_clr_pmu_events(u64 clr) {} @@ -1388,6 +1393,8 @@ static inline bool kvm_set_pmuserenr(u64 val) { return false; } +void kvm_enable_trbe(u64 guest_trfcr) {} +void kvm_disable_trbe(void) {} #endif void kvm_vcpu_load_vhe(struct kvm_vcpu *vcpu); diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c index 46dbeabd6833..6ef8d8f4b452 100644 --- a/arch/arm64/kvm/debug.c +++ b/arch/arm64/kvm/debug.c @@ -72,10 +72,6 @@ void kvm_init_host_debug_data(void) if (cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_EL1_PMSVer_SHIFT) && !(read_sysreg_s(SYS_PMBIDR_EL1) & PMBIDR_EL1_P)) host_data_set_flag(HAS_SPE); - - if (cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_EL1_TraceBuffer_SHIFT) && - !(read_sysreg_s(SYS_TRBIDR_EL1) & TRBIDR_EL1_P)) - host_data_set_flag(HAS_TRBE); } /* @@ -215,3 +211,27 @@ void kvm_debug_handle_oslar(struct kvm_vcpu *vcpu, u64 val) kvm_arch_vcpu_load(vcpu, smp_processor_id()); preempt_enable(); } + +void kvm_enable_trbe(u64 guest_trfcr) +{ + if (WARN_ON_ONCE(preemptible())) + return; + + if (has_vhe()) { + write_sysreg_s(guest_trfcr, SYS_TRFCR_EL12); + return; + } + + *host_data_ptr(guest_trfcr_el1) = guest_trfcr; + host_data_set_flag(HOST_TRBE_ENABLED); +} +EXPORT_SYMBOL_GPL(kvm_enable_trbe); + +void kvm_disable_trbe(void) +{ + if (has_vhe() || WARN_ON_ONCE(preemptible())) + return; + + host_data_clear_flag(HOST_TRBE_ENABLED); +} +EXPORT_SYMBOL_GPL(kvm_disable_trbe); diff --git a/arch/arm64/kvm/hyp/nvhe/debug-sr.c b/arch/arm64/kvm/hyp/nvhe/debug-sr.c index 858bb38e273f..d36cbce75bee 100644 --- a/arch/arm64/kvm/hyp/nvhe/debug-sr.c +++ b/arch/arm64/kvm/hyp/nvhe/debug-sr.c @@ -51,32 +51,33 @@ static void __debug_restore_spe(u64 pmscr_el1) write_sysreg_el1(pmscr_el1, SYS_PMSCR); } -static void __debug_save_trace(u64 *trfcr_el1) +static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr) { - *trfcr_el1 = 0; + *saved_trfcr = read_sysreg_el1(SYS_TRFCR); + write_sysreg_el1(new_trfcr, SYS_TRFCR); - /* Check if the TRBE is enabled */ - if (!(read_sysreg_s(SYS_TRBLIMITR_EL1) & TRBLIMITR_EL1_E)) + /* Nothing left to do if going to an enabled state */ + if (new_trfcr) return; + /* - * Prohibit trace generation while we are in guest. - * Since access to TRFCR_EL1 is trapped, the guest can't - * modify the filtering set by the host. + * Switching to a context with trace generation disabled. Drain the + * trace buffer to memory. */ - *trfcr_el1 = read_sysreg_el1(SYS_TRFCR); - write_sysreg_el1(0, SYS_TRFCR); isb(); - /* Drain the trace buffer to memory */ tsb_csync(); } -static void __debug_restore_trace(u64 trfcr_el1) +static void __trace_switch_to_guest(void) { - if (!trfcr_el1) - return; + __trace_do_switch(host_data_ptr(host_debug_state.trfcr_el1), + *host_data_ptr(guest_trfcr_el1)); +} - /* Restore trace filter controls */ - write_sysreg_el1(trfcr_el1, SYS_TRFCR); +static void __trace_switch_to_host(void) +{ + __trace_do_switch(host_data_ptr(guest_trfcr_el1), + *host_data_ptr(host_debug_state.trfcr_el1)); } void __debug_save_host_buffers_nvhe(struct kvm_vcpu *vcpu) @@ -84,9 +85,13 @@ void __debug_save_host_buffers_nvhe(struct kvm_vcpu *vcpu) /* Disable and flush SPE data generation */ if (host_data_test_flag(HAS_SPE)) __debug_save_spe(host_data_ptr(host_debug_state.pmscr_el1)); - /* Disable and flush Self-Hosted Trace generation */ - if (host_data_test_flag(HAS_TRBE)) - __debug_save_trace(host_data_ptr(host_debug_state.trfcr_el1)); + + /* + * Switch the trace filter, potentially disabling and flushing trace + * data generation + */ + if (host_data_test_flag(HOST_TRBE_ENABLED)) + __trace_switch_to_guest(); } void __debug_switch_to_guest(struct kvm_vcpu *vcpu) @@ -98,8 +103,8 @@ void __debug_restore_host_buffers_nvhe(struct kvm_vcpu *vcpu) { if (host_data_test_flag(HAS_SPE)) __debug_restore_spe(*host_data_ptr(host_debug_state.pmscr_el1)); - if (host_data_test_flag(HAS_TRBE)) - __debug_restore_trace(*host_data_ptr(host_debug_state.trfcr_el1)); + if (host_data_test_flag(HOST_TRBE_ENABLED)) + __trace_switch_to_host(); } void __debug_switch_to_host(struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index a8846689512b..9109d10c656e 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -949,6 +949,8 @@ struct kvm_vcpu_arch { #define HOST_STATE_TRBE_EN __kvm_single_flag(state, BIT(1)) /* Hyp modified TRFCR */ #define HOST_STATE_RESTORE_TRFCR __kvm_single_flag(state, BIT(2)) +/* Host wants a different trace filters for the guest */ +#define HOST_STATE_SWAP_TRFCR __kvm_single_flag(state, BIT(3)) /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */ #define vcpu_sve_pffr(vcpu) (kern_hyp_va((vcpu)->arch.sve_state) + \ @@ -1392,6 +1394,7 @@ void kvm_clr_pmu_events(u64 clr); bool kvm_set_pmuserenr(u64 val); void kvm_set_pmblimitr(u64 pmblimitr); void kvm_set_trblimitr(u64 trblimitr); +void kvm_set_trfcr(u64 host_trfcr, u64 guest_trfcr); #else static inline void kvm_set_pmu_events(u64 set, struct perf_event_attr *attr) {} static inline void kvm_clr_pmu_events(u64 clr) {} @@ -1401,6 +1404,7 @@ static inline bool kvm_set_pmuserenr(u64 val) } static inline void kvm_set_pmblimitr(u64 pmblimitr) {} static inline void kvm_set_trblimitr(u64 trblimitr) {} +static inline void kvm_set_trfcr(u64 host_trfcr, u64 guest_trfcr) {} #endif void kvm_vcpu_load_vhe(struct kvm_vcpu *vcpu); diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c index e99df2c3f62a..9acec1b67d5f 100644 --- a/arch/arm64/kvm/debug.c +++ b/arch/arm64/kvm/debug.c @@ -380,3 +380,19 @@ void kvm_set_trblimitr(u64 trblimitr) host_data_clear_flag(HOST_STATE_TRBE_EN); } EXPORT_SYMBOL_GPL(kvm_set_trblimitr); + +void kvm_set_trfcr(u64 host_trfcr, u64 guest_trfcr) +{ + if (kvm_arm_skip_trace_state()) + return; + + if (has_vhe()) + write_sysreg_s(guest_trfcr, SYS_TRFCR_EL12); + else + if (host_trfcr != guest_trfcr) { + *host_data_ptr(host_debug_state.trfcr_el1) = guest_trfcr; + host_data_set_flag(HOST_STATE_SWAP_TRFCR); + } else + host_data_clear_flag(HOST_STATE_SWAP_TRFCR); +} +EXPORT_SYMBOL_GPL(kvm_set_trfcr); diff --git a/arch/arm64/kvm/hyp/nvhe/debug-sr.c b/arch/arm64/kvm/hyp/nvhe/debug-sr.c index 17c23e52f5f4..47602c4d160a 100644 --- a/arch/arm64/kvm/hyp/nvhe/debug-sr.c +++ b/arch/arm64/kvm/hyp/nvhe/debug-sr.c @@ -100,6 +100,15 @@ static void __debug_save_trace(void) host_data_set_flag(HOST_STATE_RESTORE_TRFCR); } +static void __debug_swap_trace(void) +{ + u64 trfcr = read_sysreg_el1(SYS_TRFCR); + + write_sysreg_el1(*host_data_ptr(host_debug_state.trfcr_el1), SYS_TRFCR); + *host_data_ptr(host_debug_state.trfcr_el1) = trfcr; + host_data_set_flag(HOST_STATE_RESTORE_TRFCR); +} + static void __debug_restore_trace(void) { u64 trfcr_el1; @@ -124,10 +133,14 @@ void __debug_save_host_buffers_nvhe(void) if (!host_data_get_flag(HOST_FEAT_HAS_TRF)) return; - /* Disable and flush Self-Hosted Trace generation */ + /* + * Disable and flush Self-Hosted Trace generation for pKVM and TRBE, + * or swap if host requires different guest filters. + */ if (__debug_should_save_trace()) __debug_save_trace(); - + else if (host_data_get_flag(HOST_STATE_SWAP_TRFCR)) + __debug_swap_trace(); } void __debug_switch_to_guest(struct kvm_vcpu *vcpu)
This implements exclude/include guest rules of the active tracing session. Only do it if a different value is required for the guest, otherwise the filters remain untouched. In VHE we can just directly write the value. Signed-off-by: James Clark <james.clark@linaro.org> --- arch/arm64/include/asm/kvm_host.h | 4 ++++ arch/arm64/kvm/debug.c | 16 ++++++++++++++++ arch/arm64/kvm/hyp/nvhe/debug-sr.c | 17 +++++++++++++++-- 3 files changed, 35 insertions(+), 2 deletions(-)