From: Will Deacon <will@kernel.org>
To: Pingfan Liu <kernelfans@gmail.com>
Cc: linux-arm-kernel@lists.infradead.org,
Catalin Marinas <catalin.marinas@arm.com>,
Vincenzo Frascino <vincenzo.frascino@arm.com>,
Thomas Gleixner <tglx@linutronix.de>,
Mark Rutland <mark.rutland@arm.com>,
Andrei Vagin <avagin@gmail.com>, Marc Zyngier <maz@kernel.org>
Subject: Re: [PATCH 2/2] arm64/arch_timer: replace arch_counter_enforce_ordering() with isb
Date: Tue, 30 Mar 2021 12:05:53 +0100 [thread overview]
Message-ID: <20210330110552.GA5707@willie-the-truck> (raw)
In-Reply-To: <20210330105719.47760-3-kernelfans@gmail.com>
On Tue, Mar 30, 2021 at 06:57:19PM +0800, Pingfan Liu wrote:
> The description of getting counter value is not very clear. [1]
> 'mrs Xt, cntpct' may execute out of program order, either forward or
> backward.
>
> Now taking a look at this group of getting counter routines. All of them
> are called from sched_clock(). And there is an isb to protect forward
> speculation. But there is no isb for the backward speculation.
>
> The current code enforces read dependency instructions anchored on
> getting counter. But it is not enough to protect against other no
> dependency instructions, and even function call can not prevent the
> speculation between getting counter and them.
Which "no dependency instructions"?
>
> Replacing arch_counter_enforce_ordering() with isb to achieve the aim.
>
> [1]: AArch64 Programmer's Guides Generic Timer: 3.1. Count and frequency
I wouldn't trust that guide as far as I can throw it.
Please describe the problem you're trying to solve, and hopefully I can
help. ISB is an expensive instruction so we need a good justification to
add it here (i.e. an example of why the current scheme is not correct).
Thanks,
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-03-30 11:08 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-30 10:57 [PATCH 0/2] arm64/timer: trival fix for sync inside counter Pingfan Liu
2021-03-30 10:57 ` [PATCH 1/2] arm64/gettimeofday: correct the note about isb in __arch_get_hw_counter() Pingfan Liu
2021-03-30 11:08 ` Will Deacon
2021-03-30 10:57 ` [PATCH 2/2] arm64/arch_timer: replace arch_counter_enforce_ordering() with isb Pingfan Liu
2021-03-30 11:05 ` Will Deacon [this message]
2021-03-31 9:20 ` Pingfan Liu
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=20210330110552.GA5707@willie-the-truck \
--to=will@kernel.org \
--cc=avagin@gmail.com \
--cc=catalin.marinas@arm.com \
--cc=kernelfans@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=mark.rutland@arm.com \
--cc=maz@kernel.org \
--cc=tglx@linutronix.de \
--cc=vincenzo.frascino@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