linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: christoffer.dall@linaro.org (Christoffer Dall)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] KVM: arm/arm64: don't set vtimer->cnt_ctl in kvm_arch_timer_handler
Date: Fri, 15 Dec 2017 11:10:53 +0100	[thread overview]
Message-ID: <20171215101053.GZ910@cbox> (raw)
In-Reply-To: <519f0e33-4419-68be-32b4-11bb5e19cf17@arm.com>

On Fri, Dec 15, 2017 at 09:09:05AM +0000, Marc Zyngier wrote:
> On 15/12/17 02:27, Jia He wrote:
> > 
> > 
> 
> [...]
> 
> >> @@ -367,6 +368,7 @@ static void vtimer_save_state(struct kvm_vcpu *vcpu)
> >>   
> >>   	/* Disable the virtual timer */
> >>   	write_sysreg_el0(0, cntv_ctl);
> >> +	isb();
> > My only concern is whether this isb() is required here?
> > Sorryif this is a stupid question.I understand little about arm arch 
> > memory barrier. But seems isb will flush all the instruction prefetch.Do 
> > you think if an timer interrupt irq arrives, arm will use the previous 
> > instruction prefetch?
> 
> 
> This barrier has little to do with prefetch. It just guarantees that the
> code after the isb() is now running with a disabled virtual timer.
> Otherwise, a CPU can freely reorder the write_sysreg() until the next
> context synchronization event.
> 
> An interrupt coming between the write and the barrier will also act as a
> context synchronization event. For more details, see the ARMv8 ARM (the
> glossary has a section on the concept).
> 

So since an ISB doesn't guarantee that the timer will actually be
disabled, is it a waste of energy to have it, or should we keep it as a
best effort kind of thing?

Thanks,
-Christoffer

  reply	other threads:[~2017-12-15 10:10 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-13  7:00 [PATCH] KVM: arm/arm64: don't set vtimer->cnt_ctl in kvm_arch_timer_handler Jia He
2017-12-13  8:56 ` Marc Zyngier
2017-12-13  9:08   ` Auger Eric
2017-12-13  9:27     ` Marc Zyngier
2017-12-13  9:34       ` Christoffer Dall
2017-12-13  9:20   ` Christoffer Dall
2017-12-13  9:18 ` Christoffer Dall
2017-12-14  4:57   ` Jia He
2017-12-14  5:35     ` Jia He
2017-12-14 13:09     ` Christoffer Dall
2017-12-14 15:28       ` Jia He
2017-12-14 15:45         ` Christoffer Dall
2017-12-15  2:27           ` Jia He
2017-12-15  9:09             ` Marc Zyngier
2017-12-15 10:10               ` Christoffer Dall [this message]
2017-12-15 10:33                 ` Marc Zyngier
2017-12-15 11:15                   ` Christoffer Dall
2017-12-15 10:04             ` Christoffer Dall
2017-12-21  9:16               ` Jia He
2017-12-21 11:35                 ` Christoffer Dall

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=20171215101053.GZ910@cbox \
    --to=christoffer.dall@linaro.org \
    --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).