From: Suzuki K Poulose <suzuki.poulose@arm.com>
To: Marc Zyngier <maz@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, coresight@lists.linaro.org,
mathieu.poirier@linaro.org, mike.leach@linaro.org,
leo.yan@linaro.org, anshuman.khandual@arm.com,
catalin.marinas@arm.com, Will Deacon <will@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexandru Elisei <Alexandru.Elisei@arm.com>
Subject: Re: [PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host
Date: Tue, 30 Mar 2021 12:12:49 +0100 [thread overview]
Message-ID: <c87d9dd9-72b2-6730-e3d4-9aaa0370a639@arm.com> (raw)
In-Reply-To: <87r1jxq7ax.wl-maz@kernel.org>
Hi Marc
On 30/03/2021 11:12, Marc Zyngier wrote:
> Hi Suzuki,
>
> [+ Alex]
>
> On Tue, 23 Mar 2021 12:06:35 +0000,
> Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>>
>> For a nvhe host, the EL2 must allow the EL1&0 translation
>> regime for TraceBuffer (MDCR_EL2.E2TB == 0b11). This must
>> be saved/restored over a trip to the guest. Also, before
>> entering the guest, we must flush any trace data if the
>> TRBE was enabled. And we must prohibit the generation
>> of trace while we are in EL1 by clearing the TRFCR_EL1.
>>
>> For vhe, the EL2 must prevent the EL1 access to the Trace
>> Buffer.
>>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Marc Zyngier <maz@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
>> Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>> arch/arm64/include/asm/el2_setup.h | 13 +++++++++
>> arch/arm64/include/asm/kvm_arm.h | 2 ++
>> arch/arm64/include/asm/kvm_host.h | 2 ++
>> arch/arm64/kernel/hyp-stub.S | 3 ++-
>> arch/arm64/kvm/debug.c | 6 ++---
>> arch/arm64/kvm/hyp/nvhe/debug-sr.c | 42 ++++++++++++++++++++++++++++++
>> arch/arm64/kvm/hyp/nvhe/switch.c | 1 +
>> 7 files changed, 65 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
>> index d77d358f9395..bda918948471 100644
>> --- a/arch/arm64/include/asm/el2_setup.h
>> +++ b/arch/arm64/include/asm/el2_setup.h
>> @@ -65,6 +65,19 @@
>> // use EL1&0 translation.
>>
>> .Lskip_spe_\@:
>> + /* Trace buffer */
>> + ubfx x0, x1, #ID_AA64DFR0_TRBE_SHIFT, #4
>> + cbz x0, .Lskip_trace_\@ // Skip if TraceBuffer is not present
>> +
>> + mrs_s x0, SYS_TRBIDR_EL1
>> + and x0, x0, TRBIDR_PROG
>> + cbnz x0, .Lskip_trace_\@ // If TRBE is available at EL2
>> +
>> + mov x0, #(MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT)
>> + orr x2, x2, x0 // allow the EL1&0 translation
>> + // to own it.
>> +
>> +.Lskip_trace_\@:
>> msr mdcr_el2, x2 // Configure debug traps
>> .endm
>>
>> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
>> index 94d4025acc0b..692c9049befa 100644
>> --- a/arch/arm64/include/asm/kvm_arm.h
>> +++ b/arch/arm64/include/asm/kvm_arm.h
>> @@ -278,6 +278,8 @@
>> #define CPTR_EL2_DEFAULT CPTR_EL2_RES1
>>
>> /* Hyp Debug Configuration Register bits */
>> +#define MDCR_EL2_E2TB_MASK (UL(0x3))
>> +#define MDCR_EL2_E2TB_SHIFT (UL(24))
>
> Where are these bits defined? DDI0487G_a has them as RES0.
They are part of the Future architecture technology and a register
definition XML is available here :
https://developer.arm.com/documentation/ddi0601/2020-12/AArch64-Registers/MDCR-EL2--Monitor-Debug-Configuration-Register--EL2-?lang=en#fieldset_0-25_24-1
>
>> #define MDCR_EL2_TTRF (1 << 19)
>> #define MDCR_EL2_TPMS (1 << 14)
>> #define MDCR_EL2_E2PB_MASK (UL(0x3))
>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>> index 3d10e6527f7d..80d0a1a82a4c 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -315,6 +315,8 @@ struct kvm_vcpu_arch {
>> struct kvm_guest_debug_arch regs;
>> /* Statistical profiling extension */
>> u64 pmscr_el1;
>> + /* Self-hosted trace */
>> + u64 trfcr_el1;
>> } host_debug_state;
>>
>> /* VGIC state */
>> diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
>> index 5eccbd62fec8..05d25e645b46 100644
>> --- a/arch/arm64/kernel/hyp-stub.S
>> +++ b/arch/arm64/kernel/hyp-stub.S
>> @@ -115,9 +115,10 @@ SYM_CODE_START_LOCAL(mutate_to_vhe)
>> mrs_s x0, SYS_VBAR_EL12
>> msr vbar_el1, x0
>>
>> - // Use EL2 translations for SPE and disable access from EL1
>> + // Use EL2 translations for SPE & TRBE and disable access from EL1
>> mrs x0, mdcr_el2
>> bic x0, x0, #(MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT)
>> + bic x0, x0, #(MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT)
>> msr mdcr_el2, x0
>>
>> // Transfer the MM state from EL1 to EL2
>> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
>> index dbc890511631..7b16f42d39f4 100644
>> --- a/arch/arm64/kvm/debug.c
>> +++ b/arch/arm64/kvm/debug.c
>> @@ -89,7 +89,7 @@ void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu)
>> * - Debug ROM Address (MDCR_EL2_TDRA)
>> * - OS related registers (MDCR_EL2_TDOSA)
>> * - Statistical profiler (MDCR_EL2_TPMS/MDCR_EL2_E2PB)
>> - * - Self-hosted Trace Filter controls (MDCR_EL2_TTRF)
>> + * - Self-hosted Trace (MDCR_EL2_TTRF/MDCR_EL2_E2TB)
>
> For the record, this is likely to conflict with [1], although that
> patch still has some issues.
Thanks for the heads up. I think that patch will also conflict with my
fixes that is queued in kvmarm/fixes.
>
>> *
>> * Additionally, KVM only traps guest accesses to the debug registers if
>> * the guest is not actively using them (see the KVM_ARM64_DEBUG_DIRTY
>> @@ -107,8 +107,8 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
>> trace_kvm_arm_setup_debug(vcpu, vcpu->guest_debug);
>>
>> /*
>> - * This also clears MDCR_EL2_E2PB_MASK to disable guest access
>> - * to the profiling buffer.
>> + * This also clears MDCR_EL2_E2PB_MASK and MDCR_EL2_E2TB_MASK
>> + * to disable guest access to the profiling and trace buffers
>> */
>> vcpu->arch.mdcr_el2 = __this_cpu_read(mdcr_el2) & MDCR_EL2_HPMN_MASK;
>> vcpu->arch.mdcr_el2 |= (MDCR_EL2_TPM |
>> diff --git a/arch/arm64/kvm/hyp/nvhe/debug-sr.c b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
>> index f401724f12ef..9499e18dd28f 100644
>> --- a/arch/arm64/kvm/hyp/nvhe/debug-sr.c
>> +++ b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
>> @@ -58,10 +58,51 @@ static void __debug_restore_spe(u64 pmscr_el1)
>> write_sysreg_s(pmscr_el1, SYS_PMSCR_EL1);
>> }
>>
>> +static void __debug_save_trace(u64 *trfcr_el1)
>> +{
>> +
>
> Spurious blank line?
>
Sure, will fix it
>> + *trfcr_el1 = 0;
>> +
>> + /* Check if we have TRBE */
>> + if (!cpuid_feature_extract_unsigned_field(read_sysreg(id_aa64dfr0_el1),
>> + ID_AA64DFR0_TRBE_SHIFT))
>> + return;
>
> Do we have a way to track this that doesn't involve reading an ID
> register? This is on the hot path, and is going to really suck badly
> with NV (which traps all ID regs for obvious reasons). I would have
> hoped that one way or another, we'd have a static key for this.
TRBE, like SPE can be optionally enabled on a subset of the CPUs. We
could have a per-CPU static key in the worst case. I guess this would
apply to SPE as well.
May be we could do this check at kvm_arch_vcpu_load()/put() ?
>
>> +
>> + /* Check we can access the TRBE */
>> + if ((read_sysreg_s(SYS_TRBIDR_EL1) & TRBIDR_PROG))
>> + return;
>> +
>> + /* Check if the TRBE is enabled */
>> + if (!(read_sysreg_s(SYS_TRBLIMITR_EL1) & TRBLIMITR_ENABLE))
>> + 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.
>
> If TRFCR_EL1 is trapped, where is the trap handling? This series
> doesn't touch sys_regs.c, so I assume you rely on the "inject an UNDEF
> for anything unknown" default behaviour?
Yes.
>
> If that's the case, I'd rather you add an explicit handler.
>
I could add one.
Cheers
Suzuki
>> + */
>> + *trfcr_el1 = read_sysreg_s(SYS_TRFCR_EL1);
>> + write_sysreg_s(0, SYS_TRFCR_EL1);
>> + isb();
>> + /* Drain the trace buffer to memory */
>> + tsb_csync();
>> + dsb(nsh);
>> +}
>> +
>> +static void __debug_restore_trace(u64 trfcr_el1)
>> +{
>> + if (!trfcr_el1)
>> + return;
>> +
>> + /* Restore trace filter controls */
>> + write_sysreg_s(trfcr_el1, SYS_TRFCR_EL1);
>> +}
>> +
>> void __debug_save_host_buffers_nvhe(struct kvm_vcpu *vcpu)
>> {
>> /* Disable and flush SPE data generation */
>> __debug_save_spe(&vcpu->arch.host_debug_state.pmscr_el1);
>> + /* Disable and flush Self-Hosted Trace generation */
>> + __debug_save_trace(&vcpu->arch.host_debug_state.trfcr_el1);
>> }
>>
>> void __debug_switch_to_guest(struct kvm_vcpu *vcpu)
>> @@ -72,6 +113,7 @@ void __debug_switch_to_guest(struct kvm_vcpu *vcpu)
>> void __debug_restore_host_buffers_nvhe(struct kvm_vcpu *vcpu)
>> {
>> __debug_restore_spe(vcpu->arch.host_debug_state.pmscr_el1);
>> + __debug_restore_trace(vcpu->arch.host_debug_state.trfcr_el1);
>> }
>>
>> void __debug_switch_to_host(struct kvm_vcpu *vcpu)
>> diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
>> index 68ab6b4d5141..736805232521 100644
>> --- a/arch/arm64/kvm/hyp/nvhe/switch.c
>> +++ b/arch/arm64/kvm/hyp/nvhe/switch.c
>> @@ -95,6 +95,7 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu)
>>
>> mdcr_el2 &= MDCR_EL2_HPMN_MASK;
>> mdcr_el2 |= MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT;
>> + mdcr_el2 |= MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT;
>>
>> write_sysreg(mdcr_el2, mdcr_el2);
>> if (is_protected_kvm_enabled())
>> --
>> 2.24.1
>>
>>
>
> Thanks,
>
> M.
>
> [1] https://lore.kernel.org/r/20210323180057.263356-1-alexandru.elisei@arm.com
>
next prev parent reply other threads:[~2021-03-30 11:13 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-23 12:06 [PATCH v5 00/19] coresight: Add support for ETE and TRBE Suzuki K Poulose
2021-03-23 12:06 ` [PATCH v5 01/19] [Queued] kvm: arm64: Hide system instruction access to Trace registers Suzuki K Poulose
2021-03-23 12:06 ` [PATCH v5 02/19] [Queued] kvm: arm64: Disable guest access to trace filter controls Suzuki K Poulose
2021-03-23 12:06 ` [PATCH v5 03/19] perf: aux: Add flags for the buffer format Suzuki K Poulose
2021-03-23 12:06 ` [PATCH v5 04/19] perf: aux: Add CoreSight PMU buffer formats Suzuki K Poulose
2021-03-29 16:56 ` Mathieu Poirier
2021-04-19 7:46 ` Peter Zijlstra
2021-03-23 12:06 ` [PATCH v5 05/19] arm64: Add support for trace synchronization barrier Suzuki K Poulose
2021-03-23 18:21 ` Catalin Marinas
2021-03-24 9:39 ` Suzuki K Poulose
2021-03-24 13:49 ` Marc Zyngier
2021-03-24 15:51 ` Suzuki K Poulose
2021-03-24 16:16 ` Marc Zyngier
2021-03-24 16:25 ` Suzuki K Poulose
2021-03-24 16:30 ` Marc Zyngier
2021-03-24 17:06 ` Suzuki K Poulose
2021-03-24 17:19 ` Catalin Marinas
2021-03-24 17:40 ` Marc Zyngier
2021-03-26 16:31 ` Mathieu Poirier
2021-03-23 12:06 ` [PATCH v5 06/19] arm64: Add TRBE definitions Suzuki K Poulose
2021-03-23 12:06 ` [PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host Suzuki K Poulose
2021-03-26 16:55 ` Mathieu Poirier
2021-03-30 10:16 ` Marc Zyngier
2021-03-30 10:38 ` Suzuki K Poulose
2021-03-30 15:23 ` Mathieu Poirier
2021-03-30 15:34 ` Marc Zyngier
2021-03-30 15:35 ` Greg KH
2021-03-30 16:33 ` Mathieu Poirier
2021-03-30 16:47 ` Greg KH
2021-03-30 16:51 ` Mathieu Poirier
2021-03-30 10:12 ` Marc Zyngier
2021-03-30 11:12 ` Suzuki K Poulose [this message]
2021-03-30 12:15 ` Marc Zyngier
2021-03-30 13:34 ` Suzuki K Poulose
2021-03-30 14:00 ` Marc Zyngier
2021-03-31 15:28 ` Alexandru Elisei
2021-03-31 15:37 ` Marc Zyngier
2021-03-23 12:06 ` [PATCH v5 08/19] coresight: etm4x: Move ETM to prohibited region for disable Suzuki K Poulose
2021-03-23 12:06 ` [PATCH v5 09/19] coresight: etm-perf: Allow an event to use different sinks Suzuki K Poulose
2021-03-23 12:06 ` [PATCH v5 10/19] coresight: Do not scan for graph if none is present Suzuki K Poulose
2021-03-23 12:06 ` [PATCH v5 11/19] coresight: etm4x: Add support for PE OS lock Suzuki K Poulose
2021-03-23 12:06 ` [PATCH v5 12/19] coresight: ete: Add support for ETE tracing Suzuki K Poulose
2021-03-23 12:06 ` [PATCH v5 13/19] dts: bindings: Document device tree bindings for ETE Suzuki K Poulose
2021-03-23 22:46 ` Rob Herring
2021-03-23 12:06 ` [PATCH v5 14/19] coresight: etm-perf: Handle stale output handles Suzuki K Poulose
2021-03-23 12:06 ` [PATCH v5 15/19] coresight: core: Add support for dedicated percpu sinks Suzuki K Poulose
2021-03-23 12:06 ` [PATCH v5 16/19] coresight: sink: Add TRBE driver Suzuki K Poulose
2021-03-23 12:06 ` [PATCH v5 17/19] Documentation: coresight: trbe: Sysfs ABI description Suzuki K Poulose
2021-03-23 12:06 ` [PATCH v5 18/19] Documentation: trace: Add documentation for TRBE Suzuki K Poulose
2021-03-23 12:06 ` [PATCH v5 19/19] dts: bindings: Document device tree bindings for Arm TRBE Suzuki K Poulose
2021-03-23 16:33 ` (subset) [PATCH v5 00/19] coresight: Add support for ETE and TRBE Marc Zyngier
2021-03-23 16:34 ` Marc Zyngier
2021-03-23 17:05 ` Suzuki K Poulose
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c87d9dd9-72b2-6730-e3d4-9aaa0370a639@arm.com \
--to=suzuki.poulose@arm.com \
--cc=Alexandru.Elisei@arm.com \
--cc=anshuman.khandual@arm.com \
--cc=catalin.marinas@arm.com \
--cc=coresight@lists.linaro.org \
--cc=leo.yan@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mathieu.poirier@linaro.org \
--cc=maz@kernel.org \
--cc=mike.leach@linaro.org \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®