From: jintack@cs.columbia.edu (Jintack Lim)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC 4/8] KVM: arm/arm64: Initialize the emulated EL1 physical timer
Date: Tue, 10 Jan 2017 12:03:00 -0500 [thread overview]
Message-ID: <CAHyh4xgq_me42Jv+=6P=NhcyxpjJ4R6zgWGY2HibEc6e09Af2g@mail.gmail.com> (raw)
In-Reply-To: <20170109120235.GD4348@cbox>
On Mon, Jan 9, 2017 at 7:02 AM, Christoffer Dall
<christoffer.dall@linaro.org> wrote:
> 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?
No. The code never check if the physical timer is enabled. I think
it's not necessary to set enable bit for this emulated physical timer.
We, however, may need to set/check this enable bit later if we decide
to give the EL1 physical timer to the guest instead of emulating it.
>
> 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-10 17: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
2017-01-10 17:03 ` Jintack Lim [this message]
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='CAHyh4xgq_me42Jv+=6P=NhcyxpjJ4R6zgWGY2HibEc6e09Af2g@mail.gmail.com' \
--to=jintack@cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
/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).