For nVHE, switch the filter value in and out if the Coresight driver
asks for it. This will support filters for guests when sinks other than
TRBE are used.
For VHE, just write the filter directly to TRFCR_EL1 where trace can be
used even with TRBE sinks.
Signed-off-by: James Clark <james.clark@linaro.org>
---
arch/arm64/include/asm/kvm_host.h | 5 +++++
arch/arm64/kvm/debug.c | 28 ++++++++++++++++++++++++++++
arch/arm64/kvm/hyp/nvhe/debug-sr.c | 1 +
3 files changed, 34 insertions(+)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index ba251caa593b..cce07887551b 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -613,6 +613,7 @@ struct kvm_host_data {
#define KVM_HOST_DATA_FLAG_HAS_SPE 0
#define KVM_HOST_DATA_FLAG_HAS_TRF 1
#define KVM_HOST_DATA_FLAG_TRBE_ENABLED 2
+#define KVM_HOST_DATA_FLAG_GUEST_FILTER 3
unsigned long flags;
struct kvm_cpu_context host_ctxt;
@@ -1387,6 +1388,8 @@ void kvm_clr_pmu_events(u64 clr);
bool kvm_set_pmuserenr(u64 val);
void kvm_enable_trbe(void);
void kvm_disable_trbe(void);
+void kvm_set_trfcr(u64 guest_trfcr);
+void kvm_clear_trfcr(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) {}
@@ -1396,6 +1399,8 @@ static inline bool kvm_set_pmuserenr(u64 val)
}
static inline void kvm_enable_trbe(void) {}
static inline void kvm_disable_trbe(void) {}
+static inline void kvm_set_trfcr(u64 guest_trfcr) {}
+static inline void kvm_clear_trfcr(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 0c340ae7b5d1..9266f2776991 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -337,3 +337,31 @@ void kvm_disable_trbe(void)
host_data_clear_flag(TRBE_ENABLED);
}
EXPORT_SYMBOL_GPL(kvm_disable_trbe);
+
+void kvm_set_trfcr(u64 guest_trfcr)
+{
+ if (is_protected_kvm_enabled() || WARN_ON_ONCE(preemptible()))
+ return;
+
+ if (has_vhe())
+ write_sysreg_s(guest_trfcr, SYS_TRFCR_EL12);
+ else {
+ *host_data_ptr(guest_trfcr_el1) = guest_trfcr;
+ host_data_set_flag(GUEST_FILTER);
+ }
+}
+EXPORT_SYMBOL_GPL(kvm_set_trfcr);
+
+void kvm_clear_trfcr(void)
+{
+ if (is_protected_kvm_enabled() || WARN_ON_ONCE(preemptible()))
+ return;
+
+ if (has_vhe())
+ write_sysreg_s(0, SYS_TRFCR_EL12);
+ else {
+ *host_data_ptr(guest_trfcr_el1) = 0;
+ host_data_clear_flag(GUEST_FILTER);
+ }
+}
+EXPORT_SYMBOL_GPL(kvm_clear_trfcr);
diff --git a/arch/arm64/kvm/hyp/nvhe/debug-sr.c b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
index 9479bee41801..7edee7ace433 100644
--- a/arch/arm64/kvm/hyp/nvhe/debug-sr.c
+++ b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
@@ -67,6 +67,7 @@ static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr)
static bool __trace_needs_switch(void)
{
return host_data_test_flag(TRBE_ENABLED) ||
+ host_data_test_flag(GUEST_FILTER) ||
(is_protected_kvm_enabled() && host_data_test_flag(HAS_TRF));
}
--
2.34.1On Wed, 27 Nov 2024 10:01:24 +0000,
James Clark <james.clark@linaro.org> wrote:
>
> For nVHE, switch the filter value in and out if the Coresight driver
> asks for it. This will support filters for guests when sinks other than
> TRBE are used.
>
> For VHE, just write the filter directly to TRFCR_EL1 where trace can be
> used even with TRBE sinks.
>
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
> arch/arm64/include/asm/kvm_host.h | 5 +++++
> arch/arm64/kvm/debug.c | 28 ++++++++++++++++++++++++++++
> arch/arm64/kvm/hyp/nvhe/debug-sr.c | 1 +
> 3 files changed, 34 insertions(+)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index ba251caa593b..cce07887551b 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -613,6 +613,7 @@ struct kvm_host_data {
> #define KVM_HOST_DATA_FLAG_HAS_SPE 0
> #define KVM_HOST_DATA_FLAG_HAS_TRF 1
> #define KVM_HOST_DATA_FLAG_TRBE_ENABLED 2
> +#define KVM_HOST_DATA_FLAG_GUEST_FILTER 3
Guest filter what? This is meaningless.
> unsigned long flags;
>
> struct kvm_cpu_context host_ctxt;
> @@ -1387,6 +1388,8 @@ void kvm_clr_pmu_events(u64 clr);
> bool kvm_set_pmuserenr(u64 val);
> void kvm_enable_trbe(void);
> void kvm_disable_trbe(void);
> +void kvm_set_trfcr(u64 guest_trfcr);
> +void kvm_clear_trfcr(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) {}
> @@ -1396,6 +1399,8 @@ static inline bool kvm_set_pmuserenr(u64 val)
> }
> static inline void kvm_enable_trbe(void) {}
> static inline void kvm_disable_trbe(void) {}
> +static inline void kvm_set_trfcr(u64 guest_trfcr) {}
> +static inline void kvm_clear_trfcr(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 0c340ae7b5d1..9266f2776991 100644
> --- a/arch/arm64/kvm/debug.c
> +++ b/arch/arm64/kvm/debug.c
> @@ -337,3 +337,31 @@ void kvm_disable_trbe(void)
> host_data_clear_flag(TRBE_ENABLED);
> }
> EXPORT_SYMBOL_GPL(kvm_disable_trbe);
> +
> +void kvm_set_trfcr(u64 guest_trfcr)
Again. Is this the guest's view? or the host view while running the
guest? I asked the question on the previous patch, and you didn't
reply.
> +{
> + if (is_protected_kvm_enabled() || WARN_ON_ONCE(preemptible()))
> + return;
> +
> + if (has_vhe())
> + write_sysreg_s(guest_trfcr, SYS_TRFCR_EL12);
> + else {
> + *host_data_ptr(guest_trfcr_el1) = guest_trfcr;
> + host_data_set_flag(GUEST_FILTER);
> + }
Oh come on. This is basic coding style, see section 3 in
Documentation/process/coding-style.rst.
> +}
> +EXPORT_SYMBOL_GPL(kvm_set_trfcr);
> +
> +void kvm_clear_trfcr(void)
> +{
> + if (is_protected_kvm_enabled() || WARN_ON_ONCE(preemptible()))
> + return;
> +
> + if (has_vhe())
> + write_sysreg_s(0, SYS_TRFCR_EL12);
> + else {
> + *host_data_ptr(guest_trfcr_el1) = 0;
> + host_data_clear_flag(GUEST_FILTER);
> + }
> +}
> +EXPORT_SYMBOL_GPL(kvm_clear_trfcr);
Why do we have two helpers? Clearly, calling kvm_set_trfcr() with
E{1,0}TRE=={0,0} should result in *disabling* things. Except it
doesn't, and you should fix it. Once that is fixed, it becomes obvious
that kvm_clear_trfcr() serves no purpose.
To sum it up, KVM's API should reflect the architecture instead of
making things up.
> diff --git a/arch/arm64/kvm/hyp/nvhe/debug-sr.c b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
> index 9479bee41801..7edee7ace433 100644
> --- a/arch/arm64/kvm/hyp/nvhe/debug-sr.c
> +++ b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
> @@ -67,6 +67,7 @@ static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr)
> static bool __trace_needs_switch(void)
> {
> return host_data_test_flag(TRBE_ENABLED) ||
> + host_data_test_flag(GUEST_FILTER) ||
> (is_protected_kvm_enabled() && host_data_test_flag(HAS_TRF));
Wouldn't it make more sense to just force the "GUEST_FILTER" flag in
the pKVM case, and drop the 3rd term altogether?
M.
--
Without deviation from the norm, progress is not possible.
On 21/12/2024 12:34 pm, Marc Zyngier wrote:
> On Wed, 27 Nov 2024 10:01:24 +0000,
> James Clark <james.clark@linaro.org> wrote:
>>
>> For nVHE, switch the filter value in and out if the Coresight driver
>> asks for it. This will support filters for guests when sinks other than
>> TRBE are used.
>>
>> For VHE, just write the filter directly to TRFCR_EL1 where trace can be
>> used even with TRBE sinks.
>>
>> Signed-off-by: James Clark <james.clark@linaro.org>
>> ---
>> arch/arm64/include/asm/kvm_host.h | 5 +++++
>> arch/arm64/kvm/debug.c | 28 ++++++++++++++++++++++++++++
>> arch/arm64/kvm/hyp/nvhe/debug-sr.c | 1 +
>> 3 files changed, 34 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>> index ba251caa593b..cce07887551b 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -613,6 +613,7 @@ struct kvm_host_data {
>> #define KVM_HOST_DATA_FLAG_HAS_SPE 0
>> #define KVM_HOST_DATA_FLAG_HAS_TRF 1
>> #define KVM_HOST_DATA_FLAG_TRBE_ENABLED 2
>> +#define KVM_HOST_DATA_FLAG_GUEST_FILTER 3
>
> Guest filter what? This is meaningless.
>
KVM_HOST_DATA_FLAG_SWAP_TRFCR maybe?
>> unsigned long flags;
>>
>> struct kvm_cpu_context host_ctxt;
>> @@ -1387,6 +1388,8 @@ void kvm_clr_pmu_events(u64 clr);
>> bool kvm_set_pmuserenr(u64 val);
>> void kvm_enable_trbe(void);
>> void kvm_disable_trbe(void);
>> +void kvm_set_trfcr(u64 guest_trfcr);
>> +void kvm_clear_trfcr(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) {}
>> @@ -1396,6 +1399,8 @@ static inline bool kvm_set_pmuserenr(u64 val)
>> }
>> static inline void kvm_enable_trbe(void) {}
>> static inline void kvm_disable_trbe(void) {}
>> +static inline void kvm_set_trfcr(u64 guest_trfcr) {}
>> +static inline void kvm_clear_trfcr(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 0c340ae7b5d1..9266f2776991 100644
>> --- a/arch/arm64/kvm/debug.c
>> +++ b/arch/arm64/kvm/debug.c
>> @@ -337,3 +337,31 @@ void kvm_disable_trbe(void)
>> host_data_clear_flag(TRBE_ENABLED);
>> }
>> EXPORT_SYMBOL_GPL(kvm_disable_trbe);
>> +
>> +void kvm_set_trfcr(u64 guest_trfcr)
>
> Again. Is this the guest's view? or the host view while running the
> guest? I asked the question on the previous patch, and you didn't
> reply.
>
Ah sorry missed that one:
> Guest value? Or host state while running the guest? If the former,
> then this has nothing to do here. If the latter, this should be
> spelled out (trfcr_in_guest?), and the comment amended.
Yes, the latter, guest TRFCR reads are undef anyway. I can rename this
and the host_data variable to be trfcr_in_guest.
>> +{
>> + if (is_protected_kvm_enabled() || WARN_ON_ONCE(preemptible()))
>> + return;
>> +
>> + if (has_vhe())
>> + write_sysreg_s(guest_trfcr, SYS_TRFCR_EL12);
>> + else {
>> + *host_data_ptr(guest_trfcr_el1) = guest_trfcr;
>> + host_data_set_flag(GUEST_FILTER);
>> + }
>
> Oh come on. This is basic coding style, see section 3 in
> Documentation/process/coding-style.rst.
>
Oops, I'd have thought checkpatch could catch something like that. Will fix.
>> +}
>> +EXPORT_SYMBOL_GPL(kvm_set_trfcr);
>> +
>> +void kvm_clear_trfcr(void)
>> +{
>> + if (is_protected_kvm_enabled() || WARN_ON_ONCE(preemptible()))
>> + return;
>> +
>> + if (has_vhe())
>> + write_sysreg_s(0, SYS_TRFCR_EL12);
>> + else {
>> + *host_data_ptr(guest_trfcr_el1) = 0;
>> + host_data_clear_flag(GUEST_FILTER);
>> + }
>> +}
>> +EXPORT_SYMBOL_GPL(kvm_clear_trfcr);
>
> Why do we have two helpers? Clearly, calling kvm_set_trfcr() with
> E{1,0}TRE=={0,0} should result in *disabling* things. Except it
> doesn't, and you should fix it. Once that is fixed, it becomes
obvious> that kvm_clear_trfcr() serves no purpose.
>
With only one kvm_set_trfcr() there's no way to distinguish swapping in
a 0 value or stopping swapping altogether. I thought we wanted a single
flag that gated the register accesses so the hyp mostly does nothing?
With only kvm_set_trfcr() you first need to check FEAT_TRF then you need
to compare the real register with trfcr_in_guest to know whether to swap
or not every time.
Actually I think some of the previous versions had something like this
but it was a bit more complicated.
Maybe set/clear_trfcr() aren't great names. Perhaps
kvm_set_trfcr_in_guest() and kvm_disable_trfcr_in_guest()? With the
second one hinting that it stops the swapping regardless of what the
values are.
I don't think calling kvm_set_trfcr() with E{1,0}TRE=={0,0} is actually
broken in this version, it means that the Coresight driver wants that
value to be installed for guests. So it should actually _enable_
swapping in the value of 0, not disable anything.
> To sum it up, KVM's API should reflect the architecture instead of
> making things up.
>
We had kvm_set_trfcr(u64 host_trfcr, u64 guest_trfcr) on the last
version, which also serves the same purpose I mentioned above because
you can check if they're the same or not and disable swapping. I don't
know if that counts as reflecting the architecture better. But Oliver
mentioned he preferred it more "intent" based which is why I added the
clear_trfcr().
>> diff --git a/arch/arm64/kvm/hyp/nvhe/debug-sr.c b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
>> index 9479bee41801..7edee7ace433 100644
>> --- a/arch/arm64/kvm/hyp/nvhe/debug-sr.c
>> +++ b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
>> @@ -67,6 +67,7 @@ static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr)
>> static bool __trace_needs_switch(void)
>> {
>> return host_data_test_flag(TRBE_ENABLED) ||
>> + host_data_test_flag(GUEST_FILTER) ||
>> (is_protected_kvm_enabled() && host_data_test_flag(HAS_TRF));
>
> Wouldn't it make more sense to just force the "GUEST_FILTER" flag in
> the pKVM case, and drop the 3rd term altogether?
>
> M.
>
Yep we can set GUEST_FILTER once at startup and it gets dropped along
with HAS_TRF. That's a lot simpler.
Thanks
James
On Mon, 23 Dec 2024 11:28:06 +0000,
James Clark <james.clark@linaro.org> wrote:
>
>
>
> On 21/12/2024 12:34 pm, Marc Zyngier wrote:
> > On Wed, 27 Nov 2024 10:01:24 +0000,
> > James Clark <james.clark@linaro.org> wrote:
> >>
> >> For nVHE, switch the filter value in and out if the Coresight driver
> >> asks for it. This will support filters for guests when sinks other than
> >> TRBE are used.
> >>
> >> For VHE, just write the filter directly to TRFCR_EL1 where trace can be
> >> used even with TRBE sinks.
> >>
> >> Signed-off-by: James Clark <james.clark@linaro.org>
> >> ---
> >> arch/arm64/include/asm/kvm_host.h | 5 +++++
> >> arch/arm64/kvm/debug.c | 28 ++++++++++++++++++++++++++++
> >> arch/arm64/kvm/hyp/nvhe/debug-sr.c | 1 +
> >> 3 files changed, 34 insertions(+)
> >>
> >> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> >> index ba251caa593b..cce07887551b 100644
> >> --- a/arch/arm64/include/asm/kvm_host.h
> >> +++ b/arch/arm64/include/asm/kvm_host.h
> >> @@ -613,6 +613,7 @@ struct kvm_host_data {
> >> #define KVM_HOST_DATA_FLAG_HAS_SPE 0
> >> #define KVM_HOST_DATA_FLAG_HAS_TRF 1
> >> #define KVM_HOST_DATA_FLAG_TRBE_ENABLED 2
> >> +#define KVM_HOST_DATA_FLAG_GUEST_FILTER 3
> >
> > Guest filter what? This is meaningless.
> >
>
> KVM_HOST_DATA_FLAG_SWAP_TRFCR maybe?
A flag indicates a state, just like a level interrupt. So in cannot be
a verb that indicates an action, after which the flag should be
dropped (just like an edge interrupt). Maybe if you explained what
this is supposed to indicate, we could come up with a better name.
I would have thought that something like EL1_TRACING_ENABLED would be
adequate, but it is unusually hard to understand what this is supposed
to be doing.
>
> >> unsigned long flags;
> >> struct kvm_cpu_context host_ctxt;
> >> @@ -1387,6 +1388,8 @@ void kvm_clr_pmu_events(u64 clr);
> >> bool kvm_set_pmuserenr(u64 val);
> >> void kvm_enable_trbe(void);
> >> void kvm_disable_trbe(void);
> >> +void kvm_set_trfcr(u64 guest_trfcr);
> >> +void kvm_clear_trfcr(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) {}
> >> @@ -1396,6 +1399,8 @@ static inline bool kvm_set_pmuserenr(u64 val)
> >> }
> >> static inline void kvm_enable_trbe(void) {}
> >> static inline void kvm_disable_trbe(void) {}
> >> +static inline void kvm_set_trfcr(u64 guest_trfcr) {}
> >> +static inline void kvm_clear_trfcr(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 0c340ae7b5d1..9266f2776991 100644
> >> --- a/arch/arm64/kvm/debug.c
> >> +++ b/arch/arm64/kvm/debug.c
> >> @@ -337,3 +337,31 @@ void kvm_disable_trbe(void)
> >> host_data_clear_flag(TRBE_ENABLED);
> >> }
> >> EXPORT_SYMBOL_GPL(kvm_disable_trbe);
> >> +
> >> +void kvm_set_trfcr(u64 guest_trfcr)
> >
> > Again. Is this the guest's view? or the host view while running the
> > guest? I asked the question on the previous patch, and you didn't
> > reply.
> >
>
> Ah sorry missed that one:
>
> > Guest value? Or host state while running the guest? If the former,
> > then this has nothing to do here. If the latter, this should be
> > spelled out (trfcr_in_guest?), and the comment amended.
>
> Yes, the latter, guest TRFCR reads are undef anyway. I can rename this
> and the host_data variable to be trfcr_in_guest.
>
> >> +{
> >> + if (is_protected_kvm_enabled() || WARN_ON_ONCE(preemptible()))
> >> + return;
> >> +
> >> + if (has_vhe())
> >> + write_sysreg_s(guest_trfcr, SYS_TRFCR_EL12);
> >> + else {
> >> + *host_data_ptr(guest_trfcr_el1) = guest_trfcr;
> >> + host_data_set_flag(GUEST_FILTER);
> >> + }
> >
> > Oh come on. This is basic coding style, see section 3 in
> > Documentation/process/coding-style.rst.
> >
>
> Oops, I'd have thought checkpatch could catch something like that. Will fix.
Checkpatch serves little to no purpose, really, and relying on it is a
very bad idea.
>
> >> +}
> >> +EXPORT_SYMBOL_GPL(kvm_set_trfcr);
> >> +
> >> +void kvm_clear_trfcr(void)
> >> +{
> >> + if (is_protected_kvm_enabled() || WARN_ON_ONCE(preemptible()))
> >> + return;
> >> +
> >> + if (has_vhe())
> >> + write_sysreg_s(0, SYS_TRFCR_EL12);
> >> + else {
> >> + *host_data_ptr(guest_trfcr_el1) = 0;
> >> + host_data_clear_flag(GUEST_FILTER);
> >> + }
> >> +}
> >> +EXPORT_SYMBOL_GPL(kvm_clear_trfcr);
> >
> > Why do we have two helpers? Clearly, calling kvm_set_trfcr() with
> > E{1,0}TRE=={0,0} should result in *disabling* things. Except it
> > doesn't, and you should fix it. Once that is fixed, it becomes
> obvious> that kvm_clear_trfcr() serves no purpose.
> >
>
> With only one kvm_set_trfcr() there's no way to distinguish swapping
> in a 0 value or stopping swapping altogether. I thought we wanted a
> single flag that gated the register accesses so the hyp mostly does
> nothing? With only kvm_set_trfcr() you first need to check FEAT_TRF
> then you need to compare the real register with trfcr_in_guest to know
> whether to swap or not every time.
>
> Actually I think some of the previous versions had something like this
> but it was a bit more complicated.
>
> Maybe set/clear_trfcr() aren't great names. Perhaps
> kvm_set_trfcr_in_guest() and kvm_disable_trfcr_in_guest()? With the
> second one hinting that it stops the swapping regardless of what the
> values are.
I really think the way you name thing is getting in the way of simply
*understanding* what you are doing.
You don't disable or enable TRFCR. You enable or disable EL1 tracing
while in guest context. TRFCR is the tool by which you are doing that,
and the value you pass to these helpers is the tracing configuration.
>
> I don't think calling kvm_set_trfcr() with E{1,0}TRE=={0,0} is
> actually broken in this version, it means that the Coresight driver
> wants that value to be installed for guests. So it should actually
> _enable_ swapping in the value of 0, not disable anything.
But you can decide whether there is need for "swapping" if the
configuration is different than what's on the host, can't you?
>
> > To sum it up, KVM's API should reflect the architecture instead of
> > making things up.
> >
>
> We had kvm_set_trfcr(u64 host_trfcr, u64 guest_trfcr) on the last
> version, which also serves the same purpose I mentioned above because
> you can check if they're the same or not and disable swapping. I don't
> know if that counts as reflecting the architecture better. But Oliver
> mentioned he preferred it more "intent" based which is why I added the
> clear_trfcr().
My personal take on this would be something like:
void kvm_tracing_set_el1_configuration(u64 trfcr_while_in_guest)
{
[VHE stuff omitted]
*host_data_ptr(guest_trfcr_el1) = trfcr_while_in_guest;
if (read_sysreg_s(SYS_TRFCR_EL1) != trfcr_while_in_guest);
host_data_set_flag(EL1_TRACING_CONFIGURED);
else
host_data_clear_flag(EL1_TRACING_CONFIGURED);
}
and that'd about about it.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
© 2016 - 2026 Red Hat, Inc.