Re: [PATCH v6 7/7] arm64: dts: mediatek: Initial mt8365-evk support

From: Marc Zyngier <maz@kernel.org>
To: "Bernhard Rosenkränzer" <bero@baylibre.com>
Cc: linux-mediatek@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	matthias.bgg@gmail.com, krzysztof.kozlowski+dt@linaro.org,
	robh+dt@kernel.org, gregkh@linuxfoundation.org,
	chunfeng.yun@mediatek.com, linus.walleij@linaro.org,
	lee@kernel.org, tglx@linutronix.de,
	angelogioacchino.delregno@collabora.com
Subject: Re: [PATCH v6 7/7] arm64: dts: mediatek: Initial mt8365-evk support
Date: Sun, 01 Jan 2023 22:44:39 +0000	[thread overview]
Message-ID: <87sfgteuy0.wl-maz@kernel.org> (raw)
In-Reply-To: <CAP2ifjMkKJbE_+B=XaxzXoALrc5+FUb7TKpPJQBLV3-3xqVh=g@mail.gmail.com>

On Sun, 01 Jan 2023 21:57:58 +0000,
Bernhard Rosenkränzer <bero@baylibre.com> wrote:
> 
> On Fri, Dec 30, 2022 at 11:41 PM Marc Zyngier <maz@kernel.org> wrote:
> > > +             gic: interrupt-controller@c000000 {
> > > +                     compatible = "arm,gic-v3";
> > > +                     #interrupt-cells = <4>;
> >
> > Why 4 cells? All the SPIs routed via sysirq are perfectly happy with 3
> > cells, and all the PPIs have 0 for the 4th cell (none of them use any
> > form of partitioning that'd require 4 cells). So where is this coming
> > from?
> 
> It's coming from the SoC vendor kernel (and went unnoticed because
> it happens to work).  Will send an updated version that does the
> right thing instead. I've been running it most of the day, so far
> looking good.
> 
> > > +                     interrupt-parent = <&gic>;
> > > +                     interrupt-controller;
> > > +                     reg = <0 0x0c000000 0 0x80000>, <0 0x0c080000 0 0x80000>;
> > > +
> >
> > The first region is obviously wrong (512kB for the distributor?
> > that's... most generous, but the architecture states that it is 64kB,
> > and that's wasteful enough).
> >
> > This is also missing the GICC/GICH/GICV regions that Cortex-A53
> > implements, and that must be provided as per the binding.
> 
> This was also taken from the vendor kernel; unfortunately neiter the
> datasheet for the SoC not the vendor kernel specifies the addresses
> for GICC/GICH/GICV.
> I've "guessed" based on what's in similar SoCs (MT8183, MT7986a) in
> v7; this seems to work (boots, kvm initializes hyp mode properly).

Please don't "guess", because this adds zero value, and we might as
well run with the vendor crap instead.

Read CBAR_EL1, and use this value to construct the memory map, as per
the A53 TRM. Just booting with KVM enabled means nothing, as this is
solely used at VM run time. You need run a full VM with GIC-2
emulation.

	M.

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