| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20a6d0d5-2733-4959-a9a1-bc6c2e64111f@os.amperecomputing.com>
Date: Tue, 10 Dec 2024 18:18:04 +0530
From: Ganapatrao Kulkarni <gankulkarni@...amperecomputing.com>
To: Marc Zyngier <maz@...nel.org>
Cc: kvmarm <kvmarm@...ts.linux.dev>, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, oliver.upton@...ux.dev,
christoffer.dall@....com, suzuki.poulose@....com, will@...nel.org,
catalin.marinas@....com, coltonlewis@...gle.com, joey.gouly@....com,
yuzenghui@...wei.com, darren@...amperecomputing.com,
vishnu@...amperecomputing.com
Subject: Re: [PATCH] KVM: arm64: nv: Set ISTATUS for emulated timers, If timer
expired
> Please give the fixup below a go.
>
> M.
>
> [1] https://lore.kernel.org/all/20241202172134.384923-6-maz@kernel.org/
>
> From 2bbd6f9b41a20ad573376c20c158ff3c12db5009 Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <maz@...nel.org>
> Date: Mon, 9 Dec 2024 10:58:08 +0000
> Subject: [PATCH] fixup! KVM: arm64: nv: Publish emulated timer interrupt state
> in the in-memory state
>
> ---
> arch/arm64/kvm/arch_timer.c | 32 +++++++++++++-------------------
> 1 file changed, 13 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
> index 895f09658ef83..91bda986c344b 100644
> --- a/arch/arm64/kvm/arch_timer.c
> +++ b/arch/arm64/kvm/arch_timer.c
> @@ -432,25 +432,6 @@ static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level,
> {
> int ret;
>
> - /*
> - * Paper over NV2 brokenness by publishing the interrupt status
> - * bit. This still results in a poor quality of emulation (guest
> - * writes will have no effect until the next exit).
> - *
> - * But hey, it's fast, right?
> - */
> - if (is_hyp_ctxt(vcpu) &&
> - (timer_ctx == vcpu_vtimer(vcpu) || timer_ctx == vcpu_ptimer(vcpu))) {
> - u32 ctl = timer_get_ctl(timer_ctx);
> -
> - if (new_level)
> - ctl |= ARCH_TIMER_CTRL_IT_STAT;
> - else
> - ctl &= ~ARCH_TIMER_CTRL_IT_STAT;
> -
> - timer_set_ctl(timer_ctx, ctl);
> - }
> -
> timer_ctx->irq.level = new_level;
> trace_kvm_timer_update_irq(vcpu->vcpu_id, timer_irq(timer_ctx),
> timer_ctx->irq.level);
> @@ -471,6 +452,19 @@ static void timer_emulate(struct arch_timer_context *ctx)
>
> trace_kvm_timer_emulate(ctx, should_fire);
>
> + /*
> + * Paper over NV2 brokenness by publishing the interrupt status
> + * bit. This still results in a poor quality of emulation (guest
> + * writes will have no effect until the next exit).
> + *
> + * But hey, it's fast, right?
> + */
> + if (is_hyp_ctxt(ctx->vcpu)) {
> + unsigned long val = timer_get_ctl(ctx);
> + __assign_bit(__ffs(ARCH_TIMER_CTRL_IT_STAT), &val, should_fire);
> + timer_set_ctl(ctx, val);
> + }
> +
> if (should_fire != ctx->irq.level) {
> kvm_timer_update_irq(ctx->vcpu, should_fire, ctx);
> return;
I tried this patch and it did not work, looks like there are some
exit/entry paths which updates timer interrupt without going though
timer_emulate (like kvm_hrtimer_expire). If I undo the deleted lines in
above diff in the function kvm_timer_update_irq, then it is working.
below is the diff which is working.
diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
index dd038d62e99b..a54bdb6dc566 100644
--- a/arch/arm64/kvm/arch_timer.c
+++ b/arch/arm64/kvm/arch_timer.c
@@ -432,23 +432,11 @@ static void kvm_timer_update_irq(struct kvm_vcpu
*vcpu, bool new_level,
{
int ret;
- /*
- * Paper over NV2 brokenness by publishing the interrupt status
- * bit. This still results in a poor quality of emulation (guest
- * writes will have no effect until the next exit).
- *
- * But hey, it's fast, right?
- */
if (is_hyp_ctxt(vcpu) &&
(timer_ctx == vcpu_vtimer(vcpu) || timer_ctx ==
vcpu_ptimer(vcpu))) {
- u32 ctl = timer_get_ctl(timer_ctx);
-
- if (new_level)
- ctl |= ARCH_TIMER_CTRL_IT_STAT;
- else
- ctl &= ~ARCH_TIMER_CTRL_IT_STAT;
-
- timer_set_ctl(timer_ctx, ctl);
+ unsigned long val = timer_get_ctl(timer_ctx);
+ __assign_bit(__ffs(ARCH_TIMER_CTRL_IT_STAT), &val,
new_level);
+ timer_set_ctl(timer_ctx, val);
}
timer_ctx->irq.level = new_level;
@@ -471,6 +459,19 @@ static void timer_emulate(struct arch_timer_context
*ctx)
trace_kvm_timer_emulate(ctx, should_fire);
+ /*
+ * Paper over NV2 brokenness by publishing the interrupt status
+ * bit. This still results in a poor quality of emulation (guest
+ * writes will have no effect until the next exit).
+ *
+ * But hey, it's fast, right?
+ */
+ if (is_hyp_ctxt(ctx->vcpu)) {
+ unsigned long val = timer_get_ctl(ctx);
+ __assign_bit(__ffs(ARCH_TIMER_CTRL_IT_STAT), &val,
should_fire);
+ timer_set_ctl(ctx, val);
+ }
+
if (should_fire != ctx->irq.level) {
kvm_timer_update_irq(ctx->vcpu, should_fire, ctx);
return;
@@ -948,9 +949,6 @@ void kvm_timer_sync_nested(struct kvm_vcpu *vcpu)
* which allows trapping of the timer registers even with NV2.
* Still, this is still worse than FEAT_NV on its own. Meh.
*/
- if (cpus_have_final_cap(ARM64_HAS_ECV) || !is_hyp_ctxt(vcpu))
- return;
-
if (!vcpu_el2_e2h_is_set(vcpu)) {
/*
* A non-VHE guest hypervisor doesn't have any direct
access
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 2c35e1f8193e..52a263e7dcca 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1238,7 +1238,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
if (static_branch_unlikely(&userspace_irqchip_in_use))
kvm_timer_sync_user(vcpu);
- if (vcpu_has_nv(vcpu))
+ if (is_hyp_ctxt(vcpu))
kvm_timer_sync_nested(vcpu);
kvm_arch_vcpu_ctxsync_fp(vcpu);
--
Thanks,
Ganapat/GK
Powered by blists - more mailing lists