Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 10 Aug 2021 09:27:32 +0100
Message-ID: <87fsvhbt6z.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Oliver Upton <oupton@google.com>
Cc:     linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        Mark Rutland <mark.rutland@arm.com>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Peter Shier <pshier@google.com>,
        Raghavendra Rao Ananta <rananta@google.com>,
        Ricardo Koller <ricarkol@google.com>,
        Will Deacon <will@kernel.org>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        kernel-team@android.com
Subject: Re: [PATCH 05/13] clocksource/arm_arch_timer: Fix MMIO base address vs callback ordering issue
In-Reply-To: <CAOQ_QsjATdRRFdiG5kR+Ni7X-_kMaF+V_XQx3uuxivk0t4xt_w@mail.gmail.com>
References: <20210809152651.2297337-1-maz@kernel.org>
        <20210809152651.2297337-6-maz@kernel.org>
        <CAOQ_QsjATdRRFdiG5kR+Ni7X-_kMaF+V_XQx3uuxivk0t4xt_w@mail.gmail.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1
 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Mon, 09 Aug 2021 17:52:00 +0100,
Oliver Upton <oupton@google.com> wrote:
> 
> On Mon, Aug 9, 2021 at 8:27 AM Marc Zyngier <maz@kernel.org> wrote:
> >
> > The MMIO timer base address gets published after we have registered
> > the callbacks and the interrupt handler, which is... a bit dangerous.
> >
> > Fix this by moving the base address publication to the point where
> > we register the timer, and expose a pointer to the timer structure
> > itself rather than a naked value.
> >
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> 
> Is this patch stable-worthy? I take it there haven't been any reports
> of issues, though this seems rather perilous.

It *could* deserve a Cc stable, although I suspect it doesn't easily
fall over with the current code:

- When programming a timer, the driver uses the base contained in
  struct arch_timer, and derived from the clock_event_device.
- As long as you don't need to read the counter, you are good (the
  whole point of using TVAL is that you avoid reading the counter).

It is only if someone called into the standalone counter accessor that
there would be a firework, and that's rather unlikely.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.