From: Marc Zyngier <maz@kernel.org>
To: kvmarm@lists.linux.dev, kvm@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Cc: James Morse <james.morse@arm.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
Oliver Upton <oliver.upton@linux.dev>,
Zenghui Yu <yuzenghui@huawei.com>,
Ricardo Koller <ricarkol@google.com>,
Simon Veith <sveith@amazon.de>,
Reiji Watanabe <reijiw@google.com>,
Colton Lewis <coltonlewis@google.com>,
Joey Gouly <joey.gouly@arm.com>,
dwmw2@infradead.org
Subject: Re: [PATCH v2 06/19] KVM: arm64: timers: Allow physical offset without CNTPOFF_EL2
Date: Mon, 13 Mar 2023 16:43:14 +0000 [thread overview]
Message-ID: <86mt4gy53x.wl-maz@kernel.org> (raw)
In-Reply-To: <20230313124837.2264882-7-maz@kernel.org>
On Mon, 13 Mar 2023 12:48:24 +0000,
Marc Zyngier <maz@kernel.org> wrote:
>
> CNTPOFF_EL2 is awesome, but it is mostly vapourware, and no publicly
> available implementation has it. So for the common mortals, let's
> implement the emulated version of this thing.
>
> It means trapping accesses to the physical counter and timer, and
> emulate some of it as necessary.
>
> As for CNTPOFF_EL2, nobody sets the offset yet.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> arch/arm64/include/asm/sysreg.h | 1 +
> arch/arm64/kvm/arch_timer.c | 98 +++++++++++++++++++++++-------
> arch/arm64/kvm/hyp/nvhe/timer-sr.c | 18 ++++--
> arch/arm64/kvm/sys_regs.c | 7 +++
> 4 files changed, 95 insertions(+), 29 deletions(-)
>
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 9e3ecba3c4e6..a22219c51891 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -388,6 +388,7 @@
>
> #define SYS_CNTFRQ_EL0 sys_reg(3, 3, 14, 0, 0)
>
> +#define SYS_CNTPCT_EL0 sys_reg(3, 3, 14, 0, 1)
> #define SYS_CNTPCTSS_EL0 sys_reg(3, 3, 14, 0, 5)
> #define SYS_CNTVCTSS_EL0 sys_reg(3, 3, 14, 0, 6)
>
> diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
> index 3118ea0a1b41..bb64a71ae193 100644
> --- a/arch/arm64/kvm/arch_timer.c
> +++ b/arch/arm64/kvm/arch_timer.c
> @@ -458,6 +458,8 @@ static void timer_save_state(struct arch_timer_context *ctx)
> goto out;
>
> switch (index) {
> + u64 cval;
> +
> case TIMER_VTIMER:
> timer_set_ctl(ctx, read_sysreg_el0(SYS_CNTV_CTL));
> timer_set_cval(ctx, read_sysreg_el0(SYS_CNTV_CVAL));
> @@ -485,7 +487,12 @@ static void timer_save_state(struct arch_timer_context *ctx)
> break;
> case TIMER_PTIMER:
> timer_set_ctl(ctx, read_sysreg_el0(SYS_CNTP_CTL));
> - timer_set_cval(ctx, read_sysreg_el0(SYS_CNTP_CVAL));
> + cval = read_sysreg_el0(SYS_CNTP_CVAL);
> +
> + if (!has_cntpoff())
> + cval -= timer_get_offset(ctx);
> +
> + timer_set_cval(ctx, cval);
>
> /* Disable the timer */
> write_sysreg_el0(0, SYS_CNTP_CTL);
> @@ -555,6 +562,8 @@ static void timer_restore_state(struct arch_timer_context *ctx)
> goto out;
>
> switch (index) {
> + u64 cval, offset;
> +
> case TIMER_VTIMER:
> set_cntvoff(timer_get_offset(ctx));
> write_sysreg_el0(timer_get_cval(ctx), SYS_CNTV_CVAL);
> @@ -562,8 +571,12 @@ static void timer_restore_state(struct arch_timer_context *ctx)
> write_sysreg_el0(timer_get_ctl(ctx), SYS_CNTV_CTL);
> break;
> case TIMER_PTIMER:
> - set_cntpoff(timer_get_offset(ctx));
> - write_sysreg_el0(timer_get_cval(ctx), SYS_CNTP_CVAL);
> + cval = timer_get_cval(ctx);
> + offset = timer_get_offset(ctx);
> + set_cntpoff(offset);
> + if (!has_cntpoff())
> + cval += offset;
> + write_sysreg_el0(cval, SYS_CNTP_CVAL);
> isb();
> write_sysreg_el0(timer_get_ctl(ctx), SYS_CNTP_CTL);
> break;
> @@ -634,6 +647,61 @@ static void kvm_timer_vcpu_load_nogic(struct kvm_vcpu *vcpu)
> enable_percpu_irq(host_vtimer_irq, host_vtimer_irq_flags);
> }
>
> +/* If _pred is true, set bit in _set, otherwise set it in _clr */
> +#define assign_clear_set_bit(_pred, _bit, _clr, _set) \
> + do { \
> + if (_pred) \
> + (_set) |= (_bit); \
> + else \
> + (_clr) |= (_bit); \
> + } while (0)
> +
> +static void timer_set_traps(struct kvm_vcpu *vcpu, struct timer_map *map)
> +{
> + bool tpt, tpc;
> + u64 clr, set;
> +
> + /*
> + * No trapping gets configured here with nVHE. See
> + * __timer_enable_traps(), which is where the stuff happens.
> + */
> + if (!has_vhe())
> + return;
> +
> + /*
> + * Our default policy is not to trap anything. As we progress
> + * within this function, reality kicks in and we start adding
> + * traps based on emulation requirements.
> + */
> + tpt = tpc = false;
> +
> + /*
> + * We have two possibility to deal with a physical offset:
> + *
> + * - Either we have CNTPOFF (yay!) or the offset is 0:
> + * we let the guest freely access the HW
> + *
> + * - or neither of these condition apply:
> + * we trap accesses to the HW, but still use it
> + * after correcting the physical offset
> + */
> + if (!has_cntpoff() && timer_get_offset(map->direct_ptimer))
> + tpt = tpc = true;
> +
> + /*
> + * Now that we have collected our requirements, compute the
> + * trap and enable bits.
> + */
> + set = 0;
> + clr = 0;
> +
> + assign_clear_set_bit(tpt, CNTHCTL_EL1PCEN << 10, set, clr);
> + assign_clear_set_bit(tpc, CNTHCTL_EL1PCTEN << 10, set, clr);
> +
> + /* This only happens on VHE, so use the CNTKCTL_EL1 accessor */
> + sysreg_clear_set(cntkctl_el1, clr, set);
> +}
> +
> void kvm_timer_vcpu_load(struct kvm_vcpu *vcpu)
> {
> struct arch_timer_cpu *timer = vcpu_timer(vcpu);
> @@ -657,9 +725,10 @@ void kvm_timer_vcpu_load(struct kvm_vcpu *vcpu)
> timer_restore_state(map.direct_vtimer);
> if (map.direct_ptimer)
> timer_restore_state(map.direct_ptimer);
> -
> if (map.emul_ptimer)
> timer_emulate(map.emul_ptimer);
> +
> + timer_set_traps(vcpu, &map);
> }
>
> bool kvm_timer_should_notify_user(struct kvm_vcpu *vcpu)
> @@ -1292,28 +1361,11 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
> return 0;
> }
>
> -/*
> - * On VHE system, we only need to configure the EL2 timer trap register once,
> - * not for every world switch.
> - * The host kernel runs at EL2 with HCR_EL2.TGE == 1,
> - * and this makes those bits have no effect for the host kernel execution.
> - */
> +/* If we have CNTPOFF, permanently set ECV to enable it */
> void kvm_timer_init_vhe(void)
> {
> - /* When HCR_EL2.E2H ==1, EL1PCEN and EL1PCTEN are shifted by 10 */
> - u32 cnthctl_shift = 10;
> - u64 val;
> -
> - /*
> - * VHE systems allow the guest direct access to the EL1 physical
> - * timer/counter.
> - */
> - val = read_sysreg(cnthctl_el2);
> - val |= (CNTHCTL_EL1PCEN << cnthctl_shift);
> - val |= (CNTHCTL_EL1PCTEN << cnthctl_shift);
> if (cpus_have_final_cap(ARM64_HAS_ECV_CNTPOFF))
> - val |= CNTHCTL_ECV;
> - write_sysreg(val, cnthctl_el2);
> + sysreg_clear_set(cntkctl_el1, 0, CNTHCTL_ECV);
> }
>
> static void set_timer_irqs(struct kvm *kvm, int vtimer_irq, int ptimer_irq)
> diff --git a/arch/arm64/kvm/hyp/nvhe/timer-sr.c b/arch/arm64/kvm/hyp/nvhe/timer-sr.c
> index 9072e71693ba..b185ac0dbd47 100644
> --- a/arch/arm64/kvm/hyp/nvhe/timer-sr.c
> +++ b/arch/arm64/kvm/hyp/nvhe/timer-sr.c
> @@ -9,6 +9,7 @@
> #include <linux/kvm_host.h>
>
> #include <asm/kvm_hyp.h>
> +#include <asm/kvm_mmu.h>
>
> void __kvm_timer_set_cntvoff(u64 cntvoff)
> {
> @@ -35,14 +36,19 @@ void __timer_disable_traps(struct kvm_vcpu *vcpu)
> */
> void __timer_enable_traps(struct kvm_vcpu *vcpu)
> {
> - u64 val;
> + u64 clr = 0, set = 0;
>
> /*
> * Disallow physical timer access for the guest
> - * Physical counter access is allowed
> + * Physical counter access is allowed if no offset is enforced
> + * or running protected (we don't offset anything in this case).
> */
> - val = read_sysreg(cnthctl_el2);
> - val &= ~CNTHCTL_EL1PCEN;
> - val |= CNTHCTL_EL1PCTEN;
> - write_sysreg(val, cnthctl_el2);
> + clr = CNTHCTL_EL1PCEN;
> + if (is_protected_kvm_enabled() ||
> + !kern_hyp_va(vcpu->kvm)->arch.timer_data.poffset)
> + set |= CNTHCTL_EL1PCTEN;
> + else
> + clr |= CNTHCTL_EL1PCTEN;
> +
> + sysreg_clear_set(cnthctl_el2, clr, set);
> }
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 53749d3a0996..78aeddd4c4f5 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1139,6 +1139,11 @@ static bool access_arch_timer(struct kvm_vcpu *vcpu,
> tmr = TIMER_PTIMER;
> treg = TIMER_REG_CVAL;
> break;
> + case SYS_CNTPCT_EL0:
> + case SYS_CNTPCTSS_EL0:
> + tmr = TIMER_PTIMER;
> + treg = TIMER_REG_CNT;
> + break;
> default:
> print_sys_reg_msg(p, "%s", "Unhandled trapped timer register");
> kvm_inject_undefined(vcpu);
> @@ -2075,6 +2080,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> AMU_AMEVTYPER1_EL0(14),
> AMU_AMEVTYPER1_EL0(15),
>
> + { SYS_DESC(SYS_CNTPCT_EL0), access_arch_timer },
> + { SYS_DESC(SYS_CNTPCTSS_EL0), access_arch_timer },
> { SYS_DESC(SYS_CNTP_TVAL_EL0), access_arch_timer },
> { SYS_DESC(SYS_CNTP_CTL_EL0), access_arch_timer },
> { SYS_DESC(SYS_CNTP_CVAL_EL0), access_arch_timer },
And of course I forgot to test 32bit, which means it is broken for a
non-zero physical offset. I'm squashing this in, which fixes it.
M.
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index a22219c51891..f8da9e1b0c11 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -401,6 +401,7 @@
#define SYS_AARCH32_CNTP_TVAL sys_reg(0, 0, 14, 2, 0)
#define SYS_AARCH32_CNTP_CTL sys_reg(0, 0, 14, 2, 1)
+#define SYS_AARCH32_CNTPCT sys_reg(0, 0, 0, 14, 0)
#define SYS_AARCH32_CNTP_CVAL sys_reg(0, 2, 0, 14, 0)
#define __PMEV_op2(n) ((n) & 0x7)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 78aeddd4c4f5..be7c2598e563 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1141,6 +1141,7 @@ static bool access_arch_timer(struct kvm_vcpu *vcpu,
break;
case SYS_CNTPCT_EL0:
case SYS_CNTPCTSS_EL0:
+ case SYS_AARCH32_CNTPCT:
tmr = TIMER_PTIMER;
treg = TIMER_REG_CNT;
break;
@@ -2532,6 +2533,7 @@ static const struct sys_reg_desc cp15_64_regs[] = {
{ Op1( 0), CRn( 0), CRm( 2), Op2( 0), access_vm_reg, NULL, TTBR0_EL1 },
{ CP15_PMU_SYS_REG(DIRECT, 0, 0, 9, 0), .access = access_pmu_evcntr },
{ Op1( 0), CRn( 0), CRm(12), Op2( 0), access_gic_sgi }, /* ICC_SGI1R */
+ { SYS_DESC(SYS_AARCH32_CNTPCT), access_arch_timer },
{ Op1( 1), CRn( 0), CRm( 2), Op2( 0), access_vm_reg, NULL, TTBR1_EL1 },
{ Op1( 1), CRn( 0), CRm(12), Op2( 0), access_gic_sgi }, /* ICC_ASGI1R */
{ Op1( 2), CRn( 0), CRm(12), Op2( 0), access_gic_sgi }, /* ICC_SGI0R */
--
Without deviation from the norm, progress is not possible.
next prev parent reply other threads:[~2023-03-13 16:43 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-13 12:48 [PATCH v2 00/19] KVM: arm64: Rework timer offsetting for fun and profit Marc Zyngier
2023-03-13 12:48 ` [PATCH v2 01/19] KVM: arm64: timers: Convert per-vcpu virtual offset to a global value Marc Zyngier
2023-03-13 12:48 ` [PATCH v2 02/19] KVM: arm64: timers: Use a per-vcpu, per-timer accumulator for fractional ns Marc Zyngier
2023-03-13 12:48 ` [PATCH v2 03/19] arm64: Add CNTPOFF_EL2 register definition Marc Zyngier
2023-03-13 12:48 ` [PATCH v2 04/19] arm64: Add HAS_ECV_CNTPOFF capability Marc Zyngier
2023-03-13 12:48 ` [PATCH v2 05/19] KVM: arm64: timers: Use CNTPOFF_EL2 to offset the physical timer Marc Zyngier
2023-03-13 12:48 ` [PATCH v2 06/19] KVM: arm64: timers: Allow physical offset without CNTPOFF_EL2 Marc Zyngier
2023-03-13 16:43 ` Marc Zyngier [this message]
2023-03-13 12:48 ` [PATCH v2 07/19] KVM: arm64: Expose {un,}lock_all_vcpus() to the reset of KVM Marc Zyngier
2023-03-13 12:48 ` [PATCH v2 08/19] KVM: arm64: timers: Allow userspace to set the global counter offset Marc Zyngier
2023-03-13 12:48 ` [PATCH v2 09/19] KVM: arm64: timers: Allow save/restoring of the physical timer Marc Zyngier
2023-03-13 12:48 ` [PATCH v2 10/19] KVM: arm64: timers: Rationalise per-vcpu timer init Marc Zyngier
2023-03-13 12:48 ` [PATCH v2 11/19] KVM: arm64: timers: Abstract per-timer IRQ access Marc Zyngier
2023-03-13 12:48 ` [PATCH v2 12/19] KVM: arm64: timers: Move the timer IRQs into arch_timer_vm_data Marc Zyngier
2023-03-13 12:48 ` [PATCH v2 13/19] KVM: arm64: Abstract the number of valid timers per vcpu Marc Zyngier
2023-03-13 12:48 ` [PATCH v2 14/19] KVM: arm64: Document KVM_ARM_SET_CNT_OFFSETS and co Marc Zyngier
2023-03-13 12:48 ` [PATCH v2 15/19] KVM: arm64: nv: timers: Add a per-timer, per-vcpu offset Marc Zyngier
2023-03-13 12:48 ` [PATCH v2 16/19] KVM: arm64: nv: timers: Support hyp timer emulation Marc Zyngier
2023-03-13 12:48 ` [PATCH v2 17/19] KVM: arm64: selftests: Add physical timer registers to the sysreg list Marc Zyngier
2023-03-13 12:48 ` [PATCH v2 18/19] KVM: arm64: selftests: Augment existing timer test to handle variable offset Marc Zyngier
2023-03-13 12:48 ` [PATCH v2 19/19] KVM: arm64: selftests: Deal with spurious timer interrupts Marc Zyngier
2023-03-23 22:19 ` [PATCH v2 00/19] KVM: arm64: Rework timer offsetting for fun and profit Colton Lewis
2023-03-23 22:54 ` Marc Zyngier
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=86mt4gy53x.wl-maz@kernel.org \
--to=maz@kernel.org \
--cc=coltonlewis@google.com \
--cc=dwmw2@infradead.org \
--cc=james.morse@arm.com \
--cc=joey.gouly@arm.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.linux.dev \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=oliver.upton@linux.dev \
--cc=reijiw@google.com \
--cc=ricarkol@google.com \
--cc=suzuki.poulose@arm.com \
--cc=sveith@amazon.de \
--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).