From: Alexandru Elisei <alexandru.elisei@arm.com>
To: Marc Zyngier <maz@kernel.org>, Paolo Bonzini <pbonzini@redhat.com>
Cc: Andrew Jones <drjones@redhat.com>,
Andrew Murray <andrew.murray@arm.com>,
Beata Michalska <beata.michalska@linaro.org>,
Christoffer Dall <christoffer.dall@arm.com>,
Eric Auger <eric.auger@redhat.com>, Gavin Shan <gshan@redhat.com>,
Haibin Wang <wanghaibin.wang@huawei.com>,
James Morse <james.morse@arm.com>,
Mark Brown <broonie@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Russell King <rmk+kernel@armlinux.org.uk>,
Shannon Zhao <shannon.zhao@linux.alibaba.com>,
Steven Price <steven.price@arm.com>,
Will Deacon <will@kernel.org>, YueHaibing <yuehaibing@huawei.com>,
Zenghui Yu <yuzenghui@huawei.com>,
Julien Thierry <julien.thierry.kdev@gmail.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
linux-arm-kernel@lists.infradead.org,
kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org
Subject: Re: [PATCH 23/23] KVM: arm64: Treat emulated TVAL TimerValue as a signed 32-bit integer
Date: Thu, 30 Jan 2020 14:11:47 +0000 [thread overview]
Message-ID: <aec2c75e-24d0-8157-14bd-72883a9df6bf@arm.com> (raw)
In-Reply-To: <20200130132558.10201-24-maz@kernel.org>
Hi,
On 1/30/20 1:25 PM, Marc Zyngier wrote:
> From: Alexandru Elisei <alexandru.elisei@arm.com>
>
> According to the ARM ARM, registers CNT{P,V}_TVAL_EL0 have bits [63:32]
> RES0 [1]. When reading the register, the value is truncated to the least
> significant 32 bits [2], and on writes, TimerValue is treated as a signed
> 32-bit integer [1, 2].
>
> When the guest behaves correctly and writes 32-bit values, treating TVAL
> as an unsigned 64 bit register works as expected. However, things start
> to break down when the guest writes larger values, because
> (u64)0x1_ffff_ffff = 8589934591. but (s32)0x1_ffff_ffff = -1, and the
> former will cause the timer interrupt to be asserted in the future, but
> the latter will cause it to be asserted now. Let's treat TVAL as a
> signed 32-bit register on writes, to match the behaviour described in
> the architecture, and the behaviour experimentally exhibited by the
> virtual timer on a non-vhe host.
>
> [1] Arm DDI 0487E.a, section D13.8.18
> [2] Arm DDI 0487E.a, section D11.2.4
>
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> [maz: replaced the read-side mask with lower_32_bits]
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Fixes: 8fa761624871 ("KVM: arm/arm64: arch_timer: Fix CNTP_TVAL calculation")
> Link: https://lore.kernel.org/r/20200127103652.2326-1-alexandru.elisei@arm.com
> ---
> virt/kvm/arm/arch_timer.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index f182b2380345..c6c2a9dde00c 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -805,6 +805,7 @@ static u64 kvm_arm_timer_read(struct kvm_vcpu *vcpu,
> switch (treg) {
> case TIMER_REG_TVAL:
> val = timer->cnt_cval - kvm_phys_timer_read() + timer->cntvoff;
> + val &= lower_32_bits(val);
This is correct, but how about making it val = lower_32_bits(val) for more
clarity? Apologies for not spotting it earlier :(
Thanks,
Alex
> break;
>
> case TIMER_REG_CTL:
> @@ -850,7 +851,7 @@ static void kvm_arm_timer_write(struct kvm_vcpu *vcpu,
> {
> switch (treg) {
> case TIMER_REG_TVAL:
> - timer->cnt_cval = kvm_phys_timer_read() - timer->cntvoff + val;
> + timer->cnt_cval = kvm_phys_timer_read() - timer->cntvoff + (s32)val;
> break;
>
> case TIMER_REG_CTL:
next prev parent reply other threads:[~2020-01-30 14:12 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-30 13:25 [GIT PULL 00/23] KVM/arm updates for 5.6 Marc Zyngier
2020-01-30 13:25 ` [PATCH 01/23] KVM: arm64: Only sign-extend MMIO up to register width Marc Zyngier
2020-01-30 13:25 ` [PATCH 02/23] KVM: arm/arm64: vgic-its: Fix restoration of unmapped collections Marc Zyngier
2020-01-30 13:25 ` [PATCH 03/23] KVM: arm/arm64: vgic: Handle GICR_PENDBASER.PTZ filed as RAZ Marc Zyngier
2020-01-30 13:25 ` [PATCH 04/23] arm64: kvm: Fix IDMAP overlap with HYP VA Marc Zyngier
2020-01-30 13:25 ` [PATCH 05/23] KVM: ARM: Call hyp_cpu_pm_exit at the right place Marc Zyngier
2020-01-30 13:25 ` [PATCH 06/23] KVM: arm: Remove duplicate include Marc Zyngier
2020-01-30 13:25 ` [PATCH 07/23] KVM: arm/arm64: Re-check VMA on detecting a poisoned page Marc Zyngier
2020-01-30 13:25 ` [PATCH 08/23] KVM: arm64: Correct PSTATE on exception entry Marc Zyngier
2020-01-30 13:25 ` [PATCH 09/23] KVM: arm/arm64: Correct CPSR " Marc Zyngier
2020-01-30 13:25 ` [PATCH 10/23] KVM: arm/arm64: Correct AArch32 SPSR " Marc Zyngier
2020-01-30 13:25 ` [PATCH 11/23] KVM: arm/arm64: vgic-its: Properly check the unmapped coll in DISCARD handler Marc Zyngier
2020-01-30 13:25 ` [PATCH 12/23] KVM: arm/arm64: vgic: Drop the kvm_vgic_register_mmio_region() Marc Zyngier
2020-01-30 13:25 ` [PATCH 13/23] KVM: arm/arm64: Cleanup MMIO handling Marc Zyngier
2020-01-30 13:25 ` [PATCH 14/23] arm64: KVM: Add UAPI notes for swapped registers Marc Zyngier
2020-01-30 13:25 ` [PATCH 15/23] arm64: KVM: Annotate guest entry/exit as a single function Marc Zyngier
2020-01-30 13:25 ` [PATCH 16/23] KVM: arm/arm64: Fix young bit from mmu notifier Marc Zyngier
2020-01-30 13:25 ` [PATCH 17/23] KVM: arm: Fix DFSR setting for non-LPAE aarch32 guests Marc Zyngier
2020-01-30 13:25 ` [PATCH 18/23] KVM: arm: Make inject_abt32() inject an external abort instead Marc Zyngier
2020-01-30 13:25 ` [PATCH 19/23] KVM: arm64: pmu: Don't increment SW_INCR if PMCR.E is unset Marc Zyngier
2020-01-30 13:25 ` [PATCH 20/23] KVM: arm64: pmu: Don't mark a counter as chained if the odd one is disabled Marc Zyngier
2020-01-30 13:25 ` [PATCH 21/23] KVM: arm64: pmu: Fix chained SW_INCR counters Marc Zyngier
2020-01-30 13:25 ` [PATCH 22/23] KVM: arm64: pmu: Only handle supported event counters Marc Zyngier
2020-01-30 13:25 ` [PATCH 23/23] KVM: arm64: Treat emulated TVAL TimerValue as a signed 32-bit integer Marc Zyngier
2020-01-30 14:11 ` Alexandru Elisei [this message]
2020-01-30 14:15 ` Marc Zyngier
2020-01-30 17:13 ` [GIT PULL 00/23] KVM/arm updates for 5.6 Paolo Bonzini
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=aec2c75e-24d0-8157-14bd-72883a9df6bf@arm.com \
--to=alexandru.elisei@arm.com \
--cc=andrew.murray@arm.com \
--cc=beata.michalska@linaro.org \
--cc=broonie@kernel.org \
--cc=christoffer.dall@arm.com \
--cc=drjones@redhat.com \
--cc=eric.auger@redhat.com \
--cc=gshan@redhat.com \
--cc=james.morse@arm.com \
--cc=julien.thierry.kdev@gmail.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=mark.rutland@arm.com \
--cc=maz@kernel.org \
--cc=pbonzini@redhat.com \
--cc=rmk+kernel@armlinux.org.uk \
--cc=shannon.zhao@linux.alibaba.com \
--cc=steven.price@arm.com \
--cc=suzuki.poulose@arm.com \
--cc=wanghaibin.wang@huawei.com \
--cc=will@kernel.org \
--cc=yuehaibing@huawei.com \
--cc=yuzenghui@huawei.com \
/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;
as well as URLs for NNTP newsgroup(s).