Disable the BRBE before we enter the guest, saving the status and enable it
back once we get out of the guest. This avoids capturing branch records in
the guest kernel or userspace, which would be confusing the host samples.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Oliver Upton <oliver.upton@linux.dev>
Cc: James Morse <james.morse@arm.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: kvmarm@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
Changes in V16:
- Dropped BRBCR_EL1 and BRBFCR_EL1 from enum vcpu_sysreg
- Reverted back the KVM NVHE patch - used host_debug_state based 'brbcr_el1'
element, and dropped the previous dependency on Jame's coresight series
arch/arm64/include/asm/kvm_host.h | 5 ++++-
arch/arm64/kvm/debug.c | 5 +++++
arch/arm64/kvm/hyp/nvhe/debug-sr.c | 33 ++++++++++++++++++++++++++++++
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 21c57b812569..bce8792092af 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -569,7 +569,7 @@ struct kvm_vcpu_arch {
u8 cflags;
/* Input flags to the hypervisor code, potentially cleared after use */
- u8 iflags;
+ u16 iflags;
/* State flags for kernel bookkeeping, unused by the hypervisor code */
u8 sflags;
@@ -610,6 +610,7 @@ struct kvm_vcpu_arch {
u64 pmscr_el1;
/* Self-hosted trace */
u64 trfcr_el1;
+ u64 brbcr_el1;
} host_debug_state;
/* VGIC state */
@@ -779,6 +780,8 @@ struct kvm_vcpu_arch {
#define DEBUG_STATE_SAVE_TRBE __vcpu_single_flag(iflags, BIT(6))
/* vcpu running in HYP context */
#define VCPU_HYP_CONTEXT __vcpu_single_flag(iflags, BIT(7))
+/* Save BRBE context if active */
+#define DEBUG_STATE_SAVE_BRBE __vcpu_single_flag(iflags, BIT(8))
/* SVE enabled for host EL0 */
#define HOST_SVE_ENABLED __vcpu_single_flag(sflags, BIT(0))
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index 8725291cb00a..99f85d8acbf3 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -335,10 +335,15 @@ void kvm_arch_vcpu_load_debug_state_flags(struct kvm_vcpu *vcpu)
if (cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_EL1_TraceBuffer_SHIFT) &&
!(read_sysreg_s(SYS_TRBIDR_EL1) & TRBIDR_EL1_P))
vcpu_set_flag(vcpu, DEBUG_STATE_SAVE_TRBE);
+
+ /* Check if we have BRBE implemented and available at the host */
+ if (cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_EL1_BRBE_SHIFT))
+ vcpu_set_flag(vcpu, DEBUG_STATE_SAVE_BRBE);
}
void kvm_arch_vcpu_put_debug_state_flags(struct kvm_vcpu *vcpu)
{
vcpu_clear_flag(vcpu, DEBUG_STATE_SAVE_SPE);
vcpu_clear_flag(vcpu, DEBUG_STATE_SAVE_TRBE);
+ vcpu_clear_flag(vcpu, DEBUG_STATE_SAVE_BRBE);
}
diff --git a/arch/arm64/kvm/hyp/nvhe/debug-sr.c b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
index 4558c02eb352..79bcf0fb1326 100644
--- a/arch/arm64/kvm/hyp/nvhe/debug-sr.c
+++ b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
@@ -79,6 +79,34 @@ static void __debug_restore_trace(u64 trfcr_el1)
write_sysreg_s(trfcr_el1, SYS_TRFCR_EL1);
}
+static void __debug_save_brbe(u64 *brbcr_el1)
+{
+ *brbcr_el1 = 0;
+
+ /* Check if the BRBE is enabled */
+ if (!(read_sysreg_s(SYS_BRBCR_EL1) & (BRBCR_ELx_E0BRE | BRBCR_ELx_ExBRE)))
+ return;
+
+ /*
+ * Prohibit branch record generation while we are in guest.
+ * Since access to BRBCR_EL1 is trapped, the guest can't
+ * modify the filtering set by the host.
+ */
+ *brbcr_el1 = read_sysreg_s(SYS_BRBCR_EL1);
+ write_sysreg_s(0, SYS_BRBCR_EL1);
+ isb();
+}
+
+static void __debug_restore_brbe(u64 brbcr_el1)
+{
+ if (!brbcr_el1)
+ return;
+
+ /* Restore BRBE controls */
+ write_sysreg_s(brbcr_el1, SYS_BRBCR_EL1);
+ isb();
+}
+
void __debug_save_host_buffers_nvhe(struct kvm_vcpu *vcpu)
{
/* Disable and flush SPE data generation */
@@ -87,6 +115,9 @@ void __debug_save_host_buffers_nvhe(struct kvm_vcpu *vcpu)
/* Disable and flush Self-Hosted Trace generation */
if (vcpu_get_flag(vcpu, DEBUG_STATE_SAVE_TRBE))
__debug_save_trace(&vcpu->arch.host_debug_state.trfcr_el1);
+ /* Disable BRBE branch records */
+ if (vcpu_get_flag(vcpu, DEBUG_STATE_SAVE_BRBE))
+ __debug_save_brbe(&vcpu->arch.host_debug_state.brbcr_el1);
}
void __debug_switch_to_guest(struct kvm_vcpu *vcpu)
@@ -100,6 +131,8 @@ void __debug_restore_host_buffers_nvhe(struct kvm_vcpu *vcpu)
__debug_restore_spe(vcpu->arch.host_debug_state.pmscr_el1);
if (vcpu_get_flag(vcpu, DEBUG_STATE_SAVE_TRBE))
__debug_restore_trace(vcpu->arch.host_debug_state.trfcr_el1);
+ if (vcpu_get_flag(vcpu, DEBUG_STATE_SAVE_BRBE))
+ __debug_restore_brbe(vcpu->arch.host_debug_state.brbcr_el1);
}
void __debug_switch_to_host(struct kvm_vcpu *vcpu)
--
2.25.1On Thu, 25 Jan 2024 09:41:16 +0000,
Anshuman Khandual <anshuman.khandual@arm.com> wrote:
>
> Disable the BRBE before we enter the guest, saving the status and enable it
> back once we get out of the guest. This avoids capturing branch records in
> the guest kernel or userspace, which would be confusing the host samples.
>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Oliver Upton <oliver.upton@linux.dev>
> Cc: James Morse <james.morse@arm.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: kvmarm@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> CC: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> Changes in V16:
>
> - Dropped BRBCR_EL1 and BRBFCR_EL1 from enum vcpu_sysreg
> - Reverted back the KVM NVHE patch - used host_debug_state based 'brbcr_el1'
> element, and dropped the previous dependency on Jame's coresight series
>
> arch/arm64/include/asm/kvm_host.h | 5 ++++-
> arch/arm64/kvm/debug.c | 5 +++++
> arch/arm64/kvm/hyp/nvhe/debug-sr.c | 33 ++++++++++++++++++++++++++++++
> 3 files changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 21c57b812569..bce8792092af 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -569,7 +569,7 @@ struct kvm_vcpu_arch {
> u8 cflags;
>
> /* Input flags to the hypervisor code, potentially cleared after use */
> - u8 iflags;
> + u16 iflags;
>
> /* State flags for kernel bookkeeping, unused by the hypervisor code */
> u8 sflags;
> @@ -610,6 +610,7 @@ struct kvm_vcpu_arch {
> u64 pmscr_el1;
> /* Self-hosted trace */
> u64 trfcr_el1;
> + u64 brbcr_el1;
> } host_debug_state;
>
> /* VGIC state */
> @@ -779,6 +780,8 @@ struct kvm_vcpu_arch {
> #define DEBUG_STATE_SAVE_TRBE __vcpu_single_flag(iflags, BIT(6))
> /* vcpu running in HYP context */
> #define VCPU_HYP_CONTEXT __vcpu_single_flag(iflags, BIT(7))
> +/* Save BRBE context if active */
> +#define DEBUG_STATE_SAVE_BRBE __vcpu_single_flag(iflags, BIT(8))
>
> /* SVE enabled for host EL0 */
> #define HOST_SVE_ENABLED __vcpu_single_flag(sflags, BIT(0))
> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> index 8725291cb00a..99f85d8acbf3 100644
> --- a/arch/arm64/kvm/debug.c
> +++ b/arch/arm64/kvm/debug.c
> @@ -335,10 +335,15 @@ void kvm_arch_vcpu_load_debug_state_flags(struct kvm_vcpu *vcpu)
> if (cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_EL1_TraceBuffer_SHIFT) &&
> !(read_sysreg_s(SYS_TRBIDR_EL1) & TRBIDR_EL1_P))
> vcpu_set_flag(vcpu, DEBUG_STATE_SAVE_TRBE);
> +
> + /* Check if we have BRBE implemented and available at the host */
> + if (cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_EL1_BRBE_SHIFT))
> + vcpu_set_flag(vcpu, DEBUG_STATE_SAVE_BRBE);
> }
>
> void kvm_arch_vcpu_put_debug_state_flags(struct kvm_vcpu *vcpu)
> {
> vcpu_clear_flag(vcpu, DEBUG_STATE_SAVE_SPE);
> vcpu_clear_flag(vcpu, DEBUG_STATE_SAVE_TRBE);
> + vcpu_clear_flag(vcpu, DEBUG_STATE_SAVE_BRBE);
> }
> diff --git a/arch/arm64/kvm/hyp/nvhe/debug-sr.c b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
> index 4558c02eb352..79bcf0fb1326 100644
> --- a/arch/arm64/kvm/hyp/nvhe/debug-sr.c
> +++ b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
> @@ -79,6 +79,34 @@ static void __debug_restore_trace(u64 trfcr_el1)
> write_sysreg_s(trfcr_el1, SYS_TRFCR_EL1);
> }
>
> +static void __debug_save_brbe(u64 *brbcr_el1)
> +{
> + *brbcr_el1 = 0;
> +
> + /* Check if the BRBE is enabled */
> + if (!(read_sysreg_s(SYS_BRBCR_EL1) & (BRBCR_ELx_E0BRE | BRBCR_ELx_ExBRE)))
> + return;
> +
> + /*
> + * Prohibit branch record generation while we are in guest.
> + * Since access to BRBCR_EL1 is trapped, the guest can't
> + * modify the filtering set by the host.
> + */
> + *brbcr_el1 = read_sysreg_s(SYS_BRBCR_EL1);
> + write_sysreg_s(0, SYS_BRBCR_EL1);
As for TRFCR and PMSCR, this is broken on hVHE.
Please see [1]
M.
[1] https://lore.kernel.org/r/20240229145417.3606279-1-maz@kernel.org
--
Without deviation from the norm, progress is not possible.
On 3/1/24 00:10, Marc Zyngier wrote:
> On Thu, 25 Jan 2024 09:41:16 +0000,
> Anshuman Khandual <anshuman.khandual@arm.com> wrote:
>>
>> Disable the BRBE before we enter the guest, saving the status and enable it
>> back once we get out of the guest. This avoids capturing branch records in
>> the guest kernel or userspace, which would be confusing the host samples.
>>
>> Cc: Marc Zyngier <maz@kernel.org>
>> Cc: Oliver Upton <oliver.upton@linux.dev>
>> Cc: James Morse <james.morse@arm.com>
>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: kvmarm@lists.linux.dev
>> Cc: linux-arm-kernel@lists.infradead.org
>> CC: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>> Changes in V16:
>>
>> - Dropped BRBCR_EL1 and BRBFCR_EL1 from enum vcpu_sysreg
>> - Reverted back the KVM NVHE patch - used host_debug_state based 'brbcr_el1'
>> element, and dropped the previous dependency on Jame's coresight series
>>
>> arch/arm64/include/asm/kvm_host.h | 5 ++++-
>> arch/arm64/kvm/debug.c | 5 +++++
>> arch/arm64/kvm/hyp/nvhe/debug-sr.c | 33 ++++++++++++++++++++++++++++++
>> 3 files changed, 42 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>> index 21c57b812569..bce8792092af 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -569,7 +569,7 @@ struct kvm_vcpu_arch {
>> u8 cflags;
>>
>> /* Input flags to the hypervisor code, potentially cleared after use */
>> - u8 iflags;
>> + u16 iflags;
>>
>> /* State flags for kernel bookkeeping, unused by the hypervisor code */
>> u8 sflags;
>> @@ -610,6 +610,7 @@ struct kvm_vcpu_arch {
>> u64 pmscr_el1;
>> /* Self-hosted trace */
>> u64 trfcr_el1;
>> + u64 brbcr_el1;
>> } host_debug_state;
>>
>> /* VGIC state */
>> @@ -779,6 +780,8 @@ struct kvm_vcpu_arch {
>> #define DEBUG_STATE_SAVE_TRBE __vcpu_single_flag(iflags, BIT(6))
>> /* vcpu running in HYP context */
>> #define VCPU_HYP_CONTEXT __vcpu_single_flag(iflags, BIT(7))
>> +/* Save BRBE context if active */
>> +#define DEBUG_STATE_SAVE_BRBE __vcpu_single_flag(iflags, BIT(8))
>>
>> /* SVE enabled for host EL0 */
>> #define HOST_SVE_ENABLED __vcpu_single_flag(sflags, BIT(0))
>> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
>> index 8725291cb00a..99f85d8acbf3 100644
>> --- a/arch/arm64/kvm/debug.c
>> +++ b/arch/arm64/kvm/debug.c
>> @@ -335,10 +335,15 @@ void kvm_arch_vcpu_load_debug_state_flags(struct kvm_vcpu *vcpu)
>> if (cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_EL1_TraceBuffer_SHIFT) &&
>> !(read_sysreg_s(SYS_TRBIDR_EL1) & TRBIDR_EL1_P))
>> vcpu_set_flag(vcpu, DEBUG_STATE_SAVE_TRBE);
>> +
>> + /* Check if we have BRBE implemented and available at the host */
>> + if (cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_EL1_BRBE_SHIFT))
>> + vcpu_set_flag(vcpu, DEBUG_STATE_SAVE_BRBE);
>> }
>>
>> void kvm_arch_vcpu_put_debug_state_flags(struct kvm_vcpu *vcpu)
>> {
>> vcpu_clear_flag(vcpu, DEBUG_STATE_SAVE_SPE);
>> vcpu_clear_flag(vcpu, DEBUG_STATE_SAVE_TRBE);
>> + vcpu_clear_flag(vcpu, DEBUG_STATE_SAVE_BRBE);
>> }
>> diff --git a/arch/arm64/kvm/hyp/nvhe/debug-sr.c b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
>> index 4558c02eb352..79bcf0fb1326 100644
>> --- a/arch/arm64/kvm/hyp/nvhe/debug-sr.c
>> +++ b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
>> @@ -79,6 +79,34 @@ static void __debug_restore_trace(u64 trfcr_el1)
>> write_sysreg_s(trfcr_el1, SYS_TRFCR_EL1);
>> }
>>
>> +static void __debug_save_brbe(u64 *brbcr_el1)
>> +{
>> + *brbcr_el1 = 0;
>> +
>> + /* Check if the BRBE is enabled */
>> + if (!(read_sysreg_s(SYS_BRBCR_EL1) & (BRBCR_ELx_E0BRE | BRBCR_ELx_ExBRE)))
>> + return;
>> +
>> + /*
>> + * Prohibit branch record generation while we are in guest.
>> + * Since access to BRBCR_EL1 is trapped, the guest can't
>> + * modify the filtering set by the host.
>> + */
>> + *brbcr_el1 = read_sysreg_s(SYS_BRBCR_EL1);
>> + write_sysreg_s(0, SYS_BRBCR_EL1);
>
> As for TRFCR and PMSCR, this is broken on hVHE.
>
> Please see [1]
>
> M.
>
> [1] https://lore.kernel.org/r/20240229145417.3606279-1-maz@kernel.org
>
Ahh I see, so the unified accessors read_sysreg_el1()/write_sysreg_el1()
need to be used here - which will choose between BRBCR_EL1 & BRBCR_EL12
as required. Will do the changes, thanks for pointing out.
On 25/01/2024 09:41, Anshuman Khandual wrote:
> Disable the BRBE before we enter the guest, saving the status and enable it
> back once we get out of the guest. This avoids capturing branch records in
> the guest kernel or userspace, which would be confusing the host samples.
>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Oliver Upton <oliver.upton@linux.dev>
> Cc: James Morse <james.morse@arm.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: kvmarm@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> CC: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> Changes in V16:
>
> - Dropped BRBCR_EL1 and BRBFCR_EL1 from enum vcpu_sysreg
> - Reverted back the KVM NVHE patch - used host_debug_state based 'brbcr_el1'
> element, and dropped the previous dependency on Jame's coresight series
>
> arch/arm64/include/asm/kvm_host.h | 5 ++++-
> arch/arm64/kvm/debug.c | 5 +++++
> arch/arm64/kvm/hyp/nvhe/debug-sr.c | 33 ++++++++++++++++++++++++++++++
> 3 files changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 21c57b812569..bce8792092af 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -569,7 +569,7 @@ struct kvm_vcpu_arch {
> u8 cflags;
>
> /* Input flags to the hypervisor code, potentially cleared after use */
> - u8 iflags;
> + u16 iflags;
>
> /* State flags for kernel bookkeeping, unused by the hypervisor code */
> u8 sflags;
> @@ -610,6 +610,7 @@ struct kvm_vcpu_arch {
> u64 pmscr_el1;
> /* Self-hosted trace */
> u64 trfcr_el1;
> + u64 brbcr_el1;
> } host_debug_state;
>
> /* VGIC state */
> @@ -779,6 +780,8 @@ struct kvm_vcpu_arch {
> #define DEBUG_STATE_SAVE_TRBE __vcpu_single_flag(iflags, BIT(6))
> /* vcpu running in HYP context */
> #define VCPU_HYP_CONTEXT __vcpu_single_flag(iflags, BIT(7))
> +/* Save BRBE context if active */
> +#define DEBUG_STATE_SAVE_BRBE __vcpu_single_flag(iflags, BIT(8))
>
> /* SVE enabled for host EL0 */
> #define HOST_SVE_ENABLED __vcpu_single_flag(sflags, BIT(0))
> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> index 8725291cb00a..99f85d8acbf3 100644
> --- a/arch/arm64/kvm/debug.c
> +++ b/arch/arm64/kvm/debug.c
> @@ -335,10 +335,15 @@ void kvm_arch_vcpu_load_debug_state_flags(struct kvm_vcpu *vcpu)
> if (cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_EL1_TraceBuffer_SHIFT) &&
> !(read_sysreg_s(SYS_TRBIDR_EL1) & TRBIDR_EL1_P))
> vcpu_set_flag(vcpu, DEBUG_STATE_SAVE_TRBE);
> +
> + /* Check if we have BRBE implemented and available at the host */
> + if (cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_EL1_BRBE_SHIFT))
> + vcpu_set_flag(vcpu, DEBUG_STATE_SAVE_BRBE);
> }
>
> void kvm_arch_vcpu_put_debug_state_flags(struct kvm_vcpu *vcpu)
> {
> vcpu_clear_flag(vcpu, DEBUG_STATE_SAVE_SPE);
> vcpu_clear_flag(vcpu, DEBUG_STATE_SAVE_TRBE);
> + vcpu_clear_flag(vcpu, DEBUG_STATE_SAVE_BRBE);
> }
> diff --git a/arch/arm64/kvm/hyp/nvhe/debug-sr.c b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
> index 4558c02eb352..79bcf0fb1326 100644
> --- a/arch/arm64/kvm/hyp/nvhe/debug-sr.c
> +++ b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
> @@ -79,6 +79,34 @@ static void __debug_restore_trace(u64 trfcr_el1)
> write_sysreg_s(trfcr_el1, SYS_TRFCR_EL1);
> }
>
> +static void __debug_save_brbe(u64 *brbcr_el1)
> +{
> + *brbcr_el1 = 0;
> +
> + /* Check if the BRBE is enabled */
> + if (!(read_sysreg_s(SYS_BRBCR_EL1) & (BRBCR_ELx_E0BRE | BRBCR_ELx_ExBRE)))
> + return;
> +
> + /*
> + * Prohibit branch record generation while we are in guest.
> + * Since access to BRBCR_EL1 is trapped, the guest can't
> + * modify the filtering set by the host.
> + */
> + *brbcr_el1 = read_sysreg_s(SYS_BRBCR_EL1);
> + write_sysreg_s(0, SYS_BRBCR_EL1);
> + isb();
Is this isb() required here ? This can be synchronised with the Guest
entry ?
> +}
> +
> +static void __debug_restore_brbe(u64 brbcr_el1)
> +{
> + if (!brbcr_el1)
> + return;
> +
> + /* Restore BRBE controls */
> + write_sysreg_s(brbcr_el1, SYS_BRBCR_EL1);
> + isb();
Similarly here, exit back to EL1 host can synchronise the setting ?
Suzuki
> +}
> +
> void __debug_save_host_buffers_nvhe(struct kvm_vcpu *vcpu)
> {
> /* Disable and flush SPE data generation */
> @@ -87,6 +115,9 @@ void __debug_save_host_buffers_nvhe(struct kvm_vcpu *vcpu)
> /* Disable and flush Self-Hosted Trace generation */
> if (vcpu_get_flag(vcpu, DEBUG_STATE_SAVE_TRBE))
> __debug_save_trace(&vcpu->arch.host_debug_state.trfcr_el1);
> + /* Disable BRBE branch records */
> + if (vcpu_get_flag(vcpu, DEBUG_STATE_SAVE_BRBE))
> + __debug_save_brbe(&vcpu->arch.host_debug_state.brbcr_el1);
> }
>
> void __debug_switch_to_guest(struct kvm_vcpu *vcpu)
> @@ -100,6 +131,8 @@ void __debug_restore_host_buffers_nvhe(struct kvm_vcpu *vcpu)
> __debug_restore_spe(vcpu->arch.host_debug_state.pmscr_el1);
> if (vcpu_get_flag(vcpu, DEBUG_STATE_SAVE_TRBE))
> __debug_restore_trace(vcpu->arch.host_debug_state.trfcr_el1);
> + if (vcpu_get_flag(vcpu, DEBUG_STATE_SAVE_BRBE))
> + __debug_restore_brbe(vcpu->arch.host_debug_state.brbcr_el1);
> }
>
> void __debug_switch_to_host(struct kvm_vcpu *vcpu)
On 1/29/24 17:50, Suzuki K Poulose wrote:
> On 25/01/2024 09:41, Anshuman Khandual wrote:
>> Disable the BRBE before we enter the guest, saving the status and enable it
>> back once we get out of the guest. This avoids capturing branch records in
>> the guest kernel or userspace, which would be confusing the host samples.
>>
>> Cc: Marc Zyngier <maz@kernel.org>
>> Cc: Oliver Upton <oliver.upton@linux.dev>
>> Cc: James Morse <james.morse@arm.com>
>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: kvmarm@lists.linux.dev
>> Cc: linux-arm-kernel@lists.infradead.org
>> CC: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>> Changes in V16:
>>
>> - Dropped BRBCR_EL1 and BRBFCR_EL1 from enum vcpu_sysreg
>> - Reverted back the KVM NVHE patch - used host_debug_state based 'brbcr_el1'
>> element, and dropped the previous dependency on Jame's coresight series
>>
>> arch/arm64/include/asm/kvm_host.h | 5 ++++-
>> arch/arm64/kvm/debug.c | 5 +++++
>> arch/arm64/kvm/hyp/nvhe/debug-sr.c | 33 ++++++++++++++++++++++++++++++
>> 3 files changed, 42 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>> index 21c57b812569..bce8792092af 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -569,7 +569,7 @@ struct kvm_vcpu_arch {
>> u8 cflags;
>> /* Input flags to the hypervisor code, potentially cleared after use */
>> - u8 iflags;
>> + u16 iflags;
>> /* State flags for kernel bookkeeping, unused by the hypervisor code */
>> u8 sflags;
>> @@ -610,6 +610,7 @@ struct kvm_vcpu_arch {
>> u64 pmscr_el1;
>> /* Self-hosted trace */
>> u64 trfcr_el1;
>> + u64 brbcr_el1;
>> } host_debug_state;
>> /* VGIC state */
>> @@ -779,6 +780,8 @@ struct kvm_vcpu_arch {
>> #define DEBUG_STATE_SAVE_TRBE __vcpu_single_flag(iflags, BIT(6))
>> /* vcpu running in HYP context */
>> #define VCPU_HYP_CONTEXT __vcpu_single_flag(iflags, BIT(7))
>> +/* Save BRBE context if active */
>> +#define DEBUG_STATE_SAVE_BRBE __vcpu_single_flag(iflags, BIT(8))
>> /* SVE enabled for host EL0 */
>> #define HOST_SVE_ENABLED __vcpu_single_flag(sflags, BIT(0))
>> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
>> index 8725291cb00a..99f85d8acbf3 100644
>> --- a/arch/arm64/kvm/debug.c
>> +++ b/arch/arm64/kvm/debug.c
>> @@ -335,10 +335,15 @@ void kvm_arch_vcpu_load_debug_state_flags(struct kvm_vcpu *vcpu)
>> if (cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_EL1_TraceBuffer_SHIFT) &&
>> !(read_sysreg_s(SYS_TRBIDR_EL1) & TRBIDR_EL1_P))
>> vcpu_set_flag(vcpu, DEBUG_STATE_SAVE_TRBE);
>> +
>> + /* Check if we have BRBE implemented and available at the host */
>> + if (cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_EL1_BRBE_SHIFT))
>> + vcpu_set_flag(vcpu, DEBUG_STATE_SAVE_BRBE);
>> }
>> void kvm_arch_vcpu_put_debug_state_flags(struct kvm_vcpu *vcpu)
>> {
>> vcpu_clear_flag(vcpu, DEBUG_STATE_SAVE_SPE);
>> vcpu_clear_flag(vcpu, DEBUG_STATE_SAVE_TRBE);
>> + vcpu_clear_flag(vcpu, DEBUG_STATE_SAVE_BRBE);
>> }
>> diff --git a/arch/arm64/kvm/hyp/nvhe/debug-sr.c b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
>> index 4558c02eb352..79bcf0fb1326 100644
>> --- a/arch/arm64/kvm/hyp/nvhe/debug-sr.c
>> +++ b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
>> @@ -79,6 +79,34 @@ static void __debug_restore_trace(u64 trfcr_el1)
>> write_sysreg_s(trfcr_el1, SYS_TRFCR_EL1);
>> }
>> +static void __debug_save_brbe(u64 *brbcr_el1)
>> +{
>> + *brbcr_el1 = 0;
>> +
>> + /* Check if the BRBE is enabled */
>> + if (!(read_sysreg_s(SYS_BRBCR_EL1) & (BRBCR_ELx_E0BRE | BRBCR_ELx_ExBRE)))
>> + return;
>> +
>> + /*
>> + * Prohibit branch record generation while we are in guest.
>> + * Since access to BRBCR_EL1 is trapped, the guest can't
>> + * modify the filtering set by the host.
>> + */
>> + *brbcr_el1 = read_sysreg_s(SYS_BRBCR_EL1);
>> + write_sysreg_s(0, SYS_BRBCR_EL1);
>> + isb();
>
> Is this isb() required here ? This can be synchronised with the Guest entry ?
>
>> +}
>> +
>> +static void __debug_restore_brbe(u64 brbcr_el1)
>> +{
>> + if (!brbcr_el1)
>> + return;
>> +
>> + /* Restore BRBE controls */
>> + write_sysreg_s(brbcr_el1, SYS_BRBCR_EL1);
>> + isb();
>
> Similarly here, exit back to EL1 host can synchronise the setting ?
Sure, will drop both the isb() here.
© 2016 - 2025 Red Hat, Inc.