All of lore.kernel.org
 help / color / mirror / Atom feed
From: oss@buserror.net (Scott Wood)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v4 3/4] arm64: arch_timer: Work around QorIQ Erratum A-008585
Date: Thu, 08 Sep 2016 20:08:56 -0500	[thread overview]
Message-ID: <1473383336.30217.96.camel@buserror.net> (raw)
In-Reply-To: <20160826134004.7e86e798@arm.com>

On Fri, 2016-08-26 at 13:40 +0100, Marc Zyngier wrote:
> On Thu, 7 Jul 2016 02:46:11 -0500
> Scott Wood <oss@buserror.net> wrote:
> 
> (+Mark)
> 
> > 
> > ?static __always_inline
> > ?u32 arch_timer_reg_read_cp15(int access, enum arch_timer_reg reg)
> > ?{
> > @@ -66,19 +102,19 @@ u32 arch_timer_reg_read_cp15(int access, enum
> > arch_timer_reg reg)
> > ?	if (access == ARCH_TIMER_PHYS_ACCESS) {
> > ?		switch (reg) {
> > ?		case ARCH_TIMER_REG_CTRL:
> > -			asm volatile("mrs %0,??cntp_ctl_el0" : "=r"
> > (val));
> > +			asm volatile("mrs %0, cntp_ctl_el0" : "=r"
> > (val));
> Spurious change?
> 
> > 
> > ?			break;
> > ?		case ARCH_TIMER_REG_TVAL:
> > -			asm volatile("mrs %0, cntp_tval_el0" : "=r"
> > (val));
> > +			val = _arch_timer_get_ptval();
> > ?			break;
> > ?		}
> > ?	} else if (access == ARCH_TIMER_VIRT_ACCESS) {
> > ?		switch (reg) {
> > ?		case ARCH_TIMER_REG_CTRL:
> > -			asm volatile("mrs %0,??cntv_ctl_el0" : "=r"
> > (val));
> > +			asm volatile("mrs %0, cntv_ctl_el0" : "=r"
> > (val));
> Here too?

No, it's not spurious.

I answered this in?http://lists.infradead.org/pipermail/linux-arm-kernel/2016-
June/438310.html

The extra spacing seemed to be an attempt to get things to line up between the
CTRL and TVAL asm statements.??When the TVAL case was converted to a function
call, there was nothing for the above to line up with, so I moved it back to
normal spacing.

> I'm still worried that this series doesn't address Xen or KVM guests
> that need to be made aware of the broken timers.
> 
> At the very least, I'd like a kernel command line option that'd let the
> user reliably run its VMs. You can do something along the lines of
> 46fd5c6b, and have a command line argument like
> "clocksource.arm_arch_timer.fsl-a008585=1", which would enable the
> workaround.

OK, I'll respin with a command line argument to use for now. ?Mike Caraman has
said he plans to do a better solution for KVM -- Mike, have you had a chance
to look at this?

-Scott

WARNING: multiple messages have this Message-ID (diff)
From: Scott Wood <oss-fOR+EgIDQEHk1uMJSBkQmQ@public.gmane.org>
To: Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>
Cc: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>,
	Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	stuart.yoder-3arQi8VN3Tc@public.gmane.org,
	Russell King <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>,
	Mark Rutland <Mark.Rutland-5wv7dgnIgG8@public.gmane.org>,
	mike.caraman-3arQi8VN3Tc@public.gmane.org
Subject: Re: [PATCH v4 3/4] arm64: arch_timer: Work around QorIQ Erratum A-008585
Date: Thu, 08 Sep 2016 20:08:56 -0500	[thread overview]
Message-ID: <1473383336.30217.96.camel@buserror.net> (raw)
In-Reply-To: <20160826134004.7e86e798-5wv7dgnIgG8@public.gmane.org>

On Fri, 2016-08-26 at 13:40 +0100, Marc Zyngier wrote:
> On Thu, 7 Jul 2016 02:46:11 -0500
> Scott Wood <oss-fOR+EgIDQEHk1uMJSBkQmQ@public.gmane.org> wrote:
> 
> (+Mark)
> 
> > 
> >  static __always_inline
> >  u32 arch_timer_reg_read_cp15(int access, enum arch_timer_reg reg)
> >  {
> > @@ -66,19 +102,19 @@ u32 arch_timer_reg_read_cp15(int access, enum
> > arch_timer_reg reg)
> >  	if (access == ARCH_TIMER_PHYS_ACCESS) {
> >  		switch (reg) {
> >  		case ARCH_TIMER_REG_CTRL:
> > -			asm volatile("mrs %0,  cntp_ctl_el0" : "=r"
> > (val));
> > +			asm volatile("mrs %0, cntp_ctl_el0" : "=r"
> > (val));
> Spurious change?
> 
> > 
> >  			break;
> >  		case ARCH_TIMER_REG_TVAL:
> > -			asm volatile("mrs %0, cntp_tval_el0" : "=r"
> > (val));
> > +			val = _arch_timer_get_ptval();
> >  			break;
> >  		}
> >  	} else if (access == ARCH_TIMER_VIRT_ACCESS) {
> >  		switch (reg) {
> >  		case ARCH_TIMER_REG_CTRL:
> > -			asm volatile("mrs %0,  cntv_ctl_el0" : "=r"
> > (val));
> > +			asm volatile("mrs %0, cntv_ctl_el0" : "=r"
> > (val));
> Here too?

No, it's not spurious.

I answered this in http://lists.infradead.org/pipermail/linux-arm-kernel/2016-
June/438310.html

The extra spacing seemed to be an attempt to get things to line up between the
CTRL and TVAL asm statements.  When the TVAL case was converted to a function
call, there was nothing for the above to line up with, so I moved it back to
normal spacing.

> I'm still worried that this series doesn't address Xen or KVM guests
> that need to be made aware of the broken timers.
> 
> At the very least, I'd like a kernel command line option that'd let the
> user reliably run its VMs. You can do something along the lines of
> 46fd5c6b, and have a command line argument like
> "clocksource.arm_arch_timer.fsl-a008585=1", which would enable the
> workaround.

OK, I'll respin with a command line argument to use for now.  Mike Caraman has
said he plans to do a better solution for KVM -- Mike, have you had a chance
to look at this?

-Scott

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2016-09-09  1:08 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-07  7:46 [PATCH v4 1/4] arm64: arch_timer: Add device tree binding for A-008585 erratum Scott Wood
2016-07-07  7:46 ` Scott Wood
2016-07-07  7:46 ` [PATCH v4 2/4] arm64: dts: Add timer erratum property for LS2080A and LS1043A Scott Wood
2016-07-07  7:46   ` Scott Wood
2016-07-07  7:46 ` [PATCH v4 3/4] arm64: arch_timer: Work around QorIQ Erratum A-008585 Scott Wood
2016-07-07  7:46   ` Scott Wood
2016-08-26 12:40   ` Marc Zyngier
2016-08-26 12:40     ` Marc Zyngier
2016-09-09  1:08     ` Scott Wood [this message]
2016-09-09  1:08       ` Scott Wood
2016-09-09  6:53       ` Marc Zyngier
2016-09-09  6:53         ` Marc Zyngier
2016-09-09  5:20     ` Ding Tianhong
2016-09-09  5:20       ` Ding Tianhong
2016-09-09  7:06       ` Marc Zyngier
2016-09-09  7:06         ` Marc Zyngier
2016-07-07  7:46 ` [PATCH v4 4/4] arm/arm64: arch_timer: Use archdata to indicate vdso suitability Scott Wood
2016-07-07  7:46   ` Scott Wood
2016-07-07 15:18   ` Will Deacon
2016-07-07 15:18     ` Will Deacon
2016-08-25 11:04 ` [PATCH v4 1/4] arm64: arch_timer: Add device tree binding for A-008585 erratum Matthias Brugger
2016-08-25 11:04   ` Matthias Brugger
2016-09-08 11:46   ` Ding Tianhong
2016-09-08 11:46     ` Ding Tianhong
2016-09-08 12:07     ` Marc Zyngier
2016-09-08 12:07       ` Marc Zyngier
2016-09-08 12:31       ` Ding Tianhong
2016-09-08 12:31         ` Ding Tianhong

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=1473383336.30217.96.camel@buserror.net \
    --to=oss@buserror.net \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.