linux-kernel - Re: [PATCH v3 6/6] arm64: dts: cix: add initial CIX P1(SKY1) dts support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Z8b6na8RjtF5FXi7@nchen-desktop>
Date: Tue, 4 Mar 2025 21:05:33 +0800
From: Peter Chen <peter.chen@...tech.com>
To: Marc Zyngier <maz@...nel.org>
Cc: robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
	catalin.marinas@....com, will@...nel.org, arnd@...db.de,
	linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, cix-kernel-upstream@...tech.com,
	marcin@...zkiewicz.com.pl, Fugang Duan <fugang.duan@...tech.com>
Subject: Re: [PATCH v3 6/6] arm64: dts: cix: add initial CIX P1(SKY1) dts
 support

On 25-03-03 18:49:58, Marc Zyngier wrote:
> > > > +
> > > > +     pmu-a520 {
> > > > +             compatible = "arm,cortex-a520-pmu";
> > > > +             interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>;
> > > > +     };
> > > > +
> > > > +     pmu-a720 {
> > > > +             compatible = "arm,cortex-a720-pmu";
> > > > +             interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>;
> > > > +     };
> > >
> > > This is wrong. The default configuration for PPIs is to expose the
> > > *same* device on all CPUs. You must use PPI affinities for your PMUs.
> > > Please see the GICv3 binding for the details.
> >
> > We have discussed internally, we have not seen the benefits routing
> > different PPI interrupt to dedicated CPUs. Any use cases?
> 
> This isn't about changing the PPI. It is about matching CPUs with
> their PMU. Here, you are saying "both PMU types are connected to all
> the CPUs using PPI7".
> 
> That's obviously not the case.
> 
> > I prefer changing pmu nodes as one generic Armv8 PMU node. Is it accepted?
> 
> No, that's not acceptable.
> 
> > Or must I keep both pmu for A520 and A720, and add PPI affinities to
> > describe hardware well?
> 
> This is an established practice on all big-little systems: each PMU
> node has an affinity that indicates which CPUs they are connected
> to. For GICv3+, this is carried by the interrupt specifier.
> 
> Please look at existing SoCs supported, such as rk3399, for example.

I see. I will add ppi-partitions for gic-v3 node.

> > >
> > > This will need to be bumped up to 4, and all the interrupt specifiers adjusted.
> >
> > Depends on if PPI affinities is must.
> 
> Definitely a must, unless you want to completely remove all traces of
> the PMU, which is of course silly, but a valid alternative.

I will change #interrupt-cells to 4, and applies to all interrupt
specifiers.

> 
> > > > +             arm,no-tick-in-suspend;
> > >
> > > Why do you need this? Is the HW so broken that you have implemented
> > > the global counter in a power domain that isn't always on?
> > >
> >
> > Not hardware broken, just arch timer will be powered off at cpu idle
> > and system suspend due to power consumption reason.
> 
> This is not about the timer. This is about the global counter. If your
> counter stops ticking when you're in idle or suspended, your system is
> broken and you need this property. If the timer (or more precisely the
> comparator) is turned off because the CPU is off, then that's the
> expected behaviour and you don't need this property.
> 

I will delete this property.

-- 

Best regards,
Peter