linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: marc.zyngier@arm.com (Marc Zyngier)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: KVM: Allow host virt timer irq to be different from guest timer virt irq
Date: Sun, 28 Apr 2013 11:35:10 +0200	[thread overview]
Message-ID: <459ddcd84aa2644e21efbbca56ff6870@localhost> (raw)
In-Reply-To: <CAAhSdy101B=4cK9nnLtQa7rG15D0CT_DsWD6k3FZ1EYz2nAfEw@mail.gmail.com>

On Sat, 27 Apr 2013 17:49:31 +0530, Anup Patel <anup@brainfault.org>
wrote:
> On Sat, Apr 27, 2013 at 2:49 PM, Marc Zyngier <marc.zyngier@arm.com>
wrote:
>> On Sat, 27 Apr 2013 13:38:06 +0530, Anup Patel <anup.patel@linaro.org>
>> wrote:
>>> The arch_timer irq numbers (or PPI numbers) are implementation
dependent
>>> so, the host virtual timer irq number can be different from guest
>> virtual
>>> timer irq number.
>>>
>>> This patch ensures that host virtual timer irq number is read from DTB
>> and
>>> guest virtual timer irq is determined based on vcpu target type.

[...]

>>>
>>> +int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
>>> +                      const struct kvm_irq_level *irq)
>>> +{
>>> +     struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
>>> +
>>> +     /*
>>> +      * The vcpu timer irq number cannot be determined in
>>> +      * kvm_timer_vcpu_init() because it is called much before
>>> +      * kvm_vcpu_set_target(). To handle this, we determine
>>> +      * vcpu timer irq number when the vcpu is resetted.
>>> +      */
>>> +     timer->irq = irq;
>>> +
>>> +     /*
>>> +      * Make sure timer is disarmed.
>>> +      */
>>> +     timer_disarm(timer);
>>
>> What is that for? Timer has not been started yet, for the lack of an
IRQ.
>> Actually, the vcpu has never ran. Why would you need to disarm it?
> 
> This is in case a vcpu is resetted  at runtime from user space or psci.
> Also, since we have named it timer_vcpu_reset(), it made sense to
> atleast disarm the timer.

I would agree if the timer had a clearly defined reset state. The ARM ARM
says that the timer register reset values are UNKNOWN. So at CPU reset, the
timer is allowed to be in any possible state, including having a pending
interrupt.

This means the guest already has to deal with the situation, and I'd
rather not give additional guarantees. Please remove this call from your
next version of this patch.

Thanks,

        M.
-- 
Fast, cheap, reliable. Pick two.

  reply	other threads:[~2013-04-28  9:35 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-27  8:08 [PATCH] ARM: KVM: Allow host virt timer irq to be different from guest timer virt irq Anup Patel
2013-04-27  9:19 ` Marc Zyngier
2013-04-27 12:19   ` Anup Patel
2013-04-28  9:35     ` Marc Zyngier [this message]
2013-04-28 11:48       ` Anup Patel

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=459ddcd84aa2644e21efbbca56ff6870@localhost \
    --to=marc.zyngier@arm.com \
    --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).