From: Christoffer Dall <christoffer.dall@linaro.org>
To: Jintack Lim <jintack@cs.columbia.edu>
Cc: kvmarm@lists.cs.columbia.edu, marc.zyngier@arm.com,
pbonzini@redhat.com, rkrcmar@redhat.com, linux@armlinux.org.uk,
catalin.marinas@arm.com, will.deacon@arm.com,
andre.przywara@arm.com, linux-arm-kernel@lists.infradead.org,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC 4/8] KVM: arm/arm64: Initialize the emulated EL1 physical timer
Date: Mon, 9 Jan 2017 13:02:35 +0100 [thread overview]
Message-ID: <20170109120235.GD4348@cbox> (raw)
In-Reply-To: <1482772326-29110-5-git-send-email-jintack@cs.columbia.edu>
On Mon, Dec 26, 2016 at 12:12:02PM -0500, Jintack Lim wrote:
> Initialize the emulated EL1 physical timer with the default irq number.
>
> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
> ---
> arch/arm/kvm/reset.c | 9 ++++++++-
> arch/arm64/kvm/reset.c | 9 ++++++++-
> include/kvm/arm_arch_timer.h | 3 ++-
> virt/kvm/arm/arch_timer.c | 12 ++++++++++--
> 4 files changed, 28 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/kvm/reset.c b/arch/arm/kvm/reset.c
> index 4b5e802..1da8b2d 100644
> --- a/arch/arm/kvm/reset.c
> +++ b/arch/arm/kvm/reset.c
> @@ -37,6 +37,11 @@
> .usr_regs.ARM_cpsr = SVC_MODE | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT,
> };
>
> +static const struct kvm_irq_level cortexa_ptimer_irq = {
> + { .irq = 30 },
> + .level = 1,
> +};
> +
> static const struct kvm_irq_level cortexa_vtimer_irq = {
> { .irq = 27 },
> .level = 1,
> @@ -58,6 +63,7 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> {
> struct kvm_regs *reset_regs;
> const struct kvm_irq_level *cpu_vtimer_irq;
> + const struct kvm_irq_level *cpu_ptimer_irq;
>
> switch (vcpu->arch.target) {
> case KVM_ARM_TARGET_CORTEX_A7:
> @@ -65,6 +71,7 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> reset_regs = &cortexa_regs_reset;
> vcpu->arch.midr = read_cpuid_id();
> cpu_vtimer_irq = &cortexa_vtimer_irq;
> + cpu_ptimer_irq = &cortexa_ptimer_irq;
> break;
> default:
> return -ENODEV;
> @@ -77,5 +84,5 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> kvm_reset_coprocs(vcpu);
>
> /* Reset arch_timer context */
> - return kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq);
> + return kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq, cpu_ptimer_irq);
> }
> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index 5bc4608..74322c2 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -46,6 +46,11 @@
> COMPAT_PSR_I_BIT | COMPAT_PSR_F_BIT),
> };
>
> +static const struct kvm_irq_level default_ptimer_irq = {
> + .irq = 30,
> + .level = 1,
> +};
> +
> static const struct kvm_irq_level default_vtimer_irq = {
> .irq = 27,
> .level = 1,
> @@ -110,6 +115,7 @@ int kvm_arch_dev_ioctl_check_extension(struct kvm *kvm, long ext)
> int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> {
> const struct kvm_irq_level *cpu_vtimer_irq;
> + const struct kvm_irq_level *cpu_ptimer_irq;
> const struct kvm_regs *cpu_reset;
>
> switch (vcpu->arch.target) {
> @@ -123,6 +129,7 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> }
>
> cpu_vtimer_irq = &default_vtimer_irq;
> + cpu_ptimer_irq = &default_ptimer_irq;
> break;
> }
>
> @@ -136,5 +143,5 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> kvm_pmu_vcpu_reset(vcpu);
>
> /* Reset timer */
> - return kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq);
> + return kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq, cpu_ptimer_irq);
> }
> diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
> index d21652a..04ed9c1 100644
> --- a/include/kvm/arm_arch_timer.h
> +++ b/include/kvm/arm_arch_timer.h
> @@ -61,7 +61,8 @@ struct arch_timer_cpu {
> int kvm_timer_enable(struct kvm_vcpu *vcpu);
> void kvm_timer_init(struct kvm *kvm);
> int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
> - const struct kvm_irq_level *irq);
> + const struct kvm_irq_level *virt_irq,
> + const struct kvm_irq_level *phys_irq);
> void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu);
> void kvm_timer_flush_hwstate(struct kvm_vcpu *vcpu);
> void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu);
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index 3bd6063..ed80864 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -339,9 +339,11 @@ void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu)
> }
>
> int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
> - const struct kvm_irq_level *irq)
> + const struct kvm_irq_level *virt_irq,
> + const struct kvm_irq_level *phys_irq)
> {
> struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
> + struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
>
> /*
> * The vcpu timer irq number cannot be determined in
> @@ -349,7 +351,8 @@ int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
> * kvm_vcpu_set_target(). To handle this, we determine
> * vcpu timer irq number when the vcpu is reset.
> */
> - vtimer->irq.irq = irq->irq;
> + vtimer->irq.irq = virt_irq->irq;
> + ptimer->irq.irq = phys_irq->irq;
>
> /*
> * The bits in CNTV_CTL are architecturally reset to UNKNOWN for ARMv8
> @@ -358,6 +361,7 @@ int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
> * the ARMv7 architecture.
> */
> vtimer->cnt_ctl = 0;
> + ptimer->cnt_ctl = 0;
> kvm_timer_update_state(vcpu);
>
> return 0;
> @@ -477,11 +481,15 @@ void kvm_timer_vcpu_terminate(struct kvm_vcpu *vcpu)
> int kvm_timer_enable(struct kvm_vcpu *vcpu)
> {
> struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
> + struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
> struct irq_desc *desc;
> struct irq_data *data;
> int phys_irq;
> int ret;
>
> + /* Always enable emulated the EL1 physical timer */
Dubious comment the way it stands.
Does the rest of the code really support one timer being enabled while
another one not so?
In any case, the comment should explain the rationale as opposed to
reiterate what the following code line does, or just not be there.
> + ptimer->enabled = 1;
> +
> if (vtimer->enabled)
> return 0;
>
> --
> 1.9.1
>
>
Thanks,
-Christoffer
next prev parent reply other threads:[~2017-01-09 12:03 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-26 17:11 [RFC 0/8] Provide the EL1 physical timer to the VM Jintack Lim
2016-12-26 17:11 ` [RFC 1/8] KVM: arm/arm64: Abstract virtual timer context into separate structure Jintack Lim
2016-12-26 17:12 ` [RFC 2/8] KVM: arm/arm64: Decouple kvm timer functions from virtual timer Jintack Lim
2017-01-09 11:59 ` Christoffer Dall
2016-12-26 17:12 ` [RFC 3/8] KVM: arm/arm64: Add the EL1 physical timer context Jintack Lim
2016-12-26 17:12 ` [RFC 4/8] KVM: arm/arm64: Initialize the emulated EL1 physical timer Jintack Lim
2017-01-09 12:02 ` Christoffer Dall [this message]
2017-01-10 17:03 ` Jintack Lim
2017-01-10 19:34 ` Christoffer Dall
2017-01-10 20:22 ` Jintack Lim
2016-12-26 17:12 ` [RFC 5/8] KVM: arm64: Add the EL1 physical timer access handler Jintack Lim
2016-12-26 17:12 ` [RFC 6/8] KVM: arm/arm64: Update the physical timer interrupt level Jintack Lim
2017-01-09 12:14 ` Christoffer Dall
2017-01-10 17:27 ` Jintack Lim
2016-12-26 17:12 ` [RFC 7/8] KVM: arm/arm64: Set up a background timer for the physical timer emulation Jintack Lim
2017-01-09 12:13 ` Christoffer Dall
2017-01-10 18:47 ` Jintack Lim
2017-01-10 19:39 ` Christoffer Dall
2016-12-26 17:12 ` [RFC 8/8] KVM: arm/arm64: Emulate the EL1 phys timer register access Jintack Lim
2017-01-09 12:16 ` Christoffer Dall
2017-01-10 17:36 ` Jintack Lim
2017-01-10 19:40 ` Christoffer Dall
2017-01-10 20:10 ` Jintack Lim
2017-01-17 17:09 ` [RFC 0/8] Provide the EL1 physical timer to the VM Marc Zyngier
2017-01-17 17:15 ` Jintack Lim
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=20170109120235.GD4348@cbox \
--to=christoffer.dall@linaro.org \
--cc=andre.przywara@arm.com \
--cc=catalin.marinas@arm.com \
--cc=jintack@cs.columbia.edu \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=marc.zyngier@arm.com \
--cc=pbonzini@redhat.com \
--cc=rkrcmar@redhat.com \
--cc=will.deacon@arm.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).