All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@kernel.org>
To: "Benoît Monin" <benoit.monin@bootlin.com>,
	"Thomas Bogendoerfer" <tsbogend@alpha.franken.de>,
	"Daniel Lezcano" <daniel.lezcano@kernel.org>,
	"Dragan Mladjenovic" <dragan.mladjenovic@syrmia.com>,
	"Chao-ying Fu" <cfu@wavecomp.com>,
	"Aleksandar Rikalo" <arikalo@gmail.com>,
	"Paul Burton" <paulburton@kernel.org>
Cc: "Vladimir Kondratiev" <vladimir.kondratiev@mobileye.com>,
	"Tawfik Bayouk" <tawfik.bayouk@mobileye.com>,
	"Gregory CLEMENT" <gregory.clement@bootlin.com>,
	"Théo Lebrun" <theo.lebrun@bootlin.com>,
	"Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
	linux-mips@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 4/5] clocksource: mips-gic-timer: Set next GIC event on the correct VP
Date: Tue, 01 Sep 2026 18:42:41 +0200	[thread overview]
Message-ID: <87ik4oc48u.ffs@fw13> (raw)
In-Reply-To: <-6r_yDA2RwW_Yn6kas-7SQ@bootlin.com>

On Tue, Sep 01 2026 at 17:53, Benoît Monin wrote:
> On Thursday, 20 August 2026 at 10:41:29 CEST, Thomas Gleixner wrote:
>> > On a multi-cluster system each cluster has its own GIC. The VO window of
>> > the local GIC is what write_gic_vl_other() redirects, so when the target
>> > CPU belongs to another cluster, write_gic_vo_compare() lands in the
>> > wrong GIC, overwriting the COMPARE register of a VP in the local cluster.
>> 
>> The MIPS GIC clockevent device is strictly registered per CPU with the
>> cpumask of that CPU. See gic_clockevent_cpu_init().
>> 
>> The set_next_event() callback of a per CPU clockevent device is _always_
>> invoked on that CPU and cannot be invoked on a different CPU.
>> 
>> So I completely fail to understand what this cluster and non-current CPU
>> voodoo is about.
>> 
> Going through the log, here is what I found:
>
> The commit 414408d0eedd ("MIPS: allow GIC clockevent device config
> from other CPUs") was merged in v3.16, its intent was to make the GIC
> clockevent usable as a tick broadcast device.

Ah. Nice detective work.

> Also in v3.16, the commit 0467811e9bdb ("MIPS: mark GIC clockevent device
> with CLOCK_EVT_FEAT_C3STOP") sets the C3STOP flag. With this flag, as
> I understand it, the GIC clockevent cannot be used as a tick broadcast
> device. So the set_next_event() callback is always invoked on the CPU
> of the event.

Correct. A broadcast device cannot stop. The whole point of a broadcast
decive is to act as a global backup for per CPU devices which are
affected by C3STOP. The PPC people added a hrtimer broadcast device to
handle the case where no other devices are available

> The `if/else` was added later in v4.15 by commit f16ff2bdb135
> ("clocksource/drivers/mips-gic-timer: Add fastpath for local timer
> updates"). But the `else` path was never called.

So my assumption about voodoo was not too far off :)

> I am going to drop the `else` path, should I also add the
> CLOCK_EVT_FEAT_PERCPU flag, or is C3STOP and the cpumask enough?

You can, though the device i already excluded from being qualified for
broadcast by C3STOP. But for clarity sake, yes.

Thanks,

        tglx

  reply	other threads:[~2026-09-01 16:42 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-10 14:25 [PATCH v2 0/5] MIPS: GIC clocksource/irqchip improvements and fixes for multi-cluster systems Benoît Monin
2026-08-10 14:25 ` [PATCH v2 1/5] irqchip/mips-gic: Fix unbalanced cm_core_lock in for_each_online_cpu_gic() Benoît Monin
2026-08-10 14:25 ` [PATCH v2 2/5] irqchip/mips-gic: Fix recursive acquisition of gic_lock in gic_set_affinity() Benoît Monin
2026-08-10 14:25 ` [PATCH v2 3/5] irqchip/mips-gic: Enable interrupt when moving affinity across clusters Benoît Monin
2026-08-10 14:25 ` [PATCH v2 4/5] clocksource: mips-gic-timer: Set next GIC event on the correct VP Benoît Monin
2026-08-20  8:41   ` Thomas Gleixner
2026-09-01 15:53     ` Benoît Monin
2026-09-01 16:42       ` Thomas Gleixner [this message]
2026-08-10 14:25 ` [PATCH v2 5/5] clocksource: mips-gic-timer: Use local counter on synced multi-cluster systems Benoît Monin

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=87ik4oc48u.ffs@fw13 \
    --to=tglx@kernel.org \
    --cc=arikalo@gmail.com \
    --cc=benoit.monin@bootlin.com \
    --cc=cfu@wavecomp.com \
    --cc=daniel.lezcano@kernel.org \
    --cc=dragan.mladjenovic@syrmia.com \
    --cc=gregory.clement@bootlin.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=paulburton@kernel.org \
    --cc=tawfik.bayouk@mobileye.com \
    --cc=theo.lebrun@bootlin.com \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=tsbogend@alpha.franken.de \
    --cc=vladimir.kondratiev@mobileye.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 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.