Received-SPF: pass (google.com: domain of linux-kernel+bounces-194484-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240528164132.2451685-1-maz@kernel.org> <CAK9=C2XNPJP0X=pg5TSrQbsuouDD3jP-gy2Sm4BXNJp0ZiWp+A@mail.gmail.com>
 <86bk4pm8j1.wl-maz@kernel.org> <CAK9=C2XRx==OTPoGW_AHmjq4Th0bv4okwcq6-3L5JYwHwQp97A@mail.gmail.com>
 <86a5k8nbh1.wl-maz@kernel.org> <CAK9=C2Ugq=0y8M86CD75mQccBo=TBLBomb4rqC4i1naKy2LyWQ@mail.gmail.com>
 <868qzsn7zs.wl-maz@kernel.org> <20240529-rust-tile-a05517a6260f@spud>
In-Reply-To: <20240529-rust-tile-a05517a6260f@spud>
From: Anup Patel <apatel@ventanamicro.com>
Date: Wed, 29 May 2024 22:07:33 +0530
Message-ID: <CAK9=C2UvppYBUE8_u+M86KSdtULyMAMA=kPvmb4xS0S1_UJGhQ@mail.gmail.com>
Subject: Re: [PATCH] of: property: Fix fw_devlink handling of interrupt-map
To: Conor Dooley <conor@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>, devicetree@vger.kernel.org, 
	linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, 
	linux-riscv@lists.infradead.org, Saravana Kannan <saravanak@google.com>, 
	Rob Herring <robh@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, May 29, 2024 at 8:47=E2=80=AFPM Conor Dooley <conor@kernel.org> wro=
te:
>
> On Wed, May 29, 2024 at 01:00:07PM +0100, Marc Zyngier wrote:
> > > > > In the RISC-V world, there have been quite a few QEMU releases
> > > > > where the generated DT node of the interrupt controller does not
> > > > > have the "#address-cells" property. This patch breaks the kernel
> > > > > for all such QEMU releases.
> > > >
> > > > Congratulations, you've forked DT. News at 11.
> > >
> > > Can you elaborate how ?
> >
> > You've stated it yourself. You are relying on a behaviour that
> > deviates from the standard by having DTs with missing properties
> >
> > And since we can't travel back it time to fix this, the only solution
> > I can see is to support both behaviours by quirking it.
>
> I'm not convinced that there is any actual production hardware that
> would get broken by your patch, just QEMU, so I think it should get
> fixed to output devicetrees that are spec compliant rather than add some
> riscv-specific hacks that we can't even gate on the "qemu,aplic"
> compatible because QEMU doesn't use the compatible created for it...

I also did further digging and it turns out the "#address-cells"
is missing only for APLIC DT nodes and this issue only impacts
APLIC DT node creation in QEMU RISC-V virt machine. We
should just go ahead and fix QEMU.

>
> Spec violations aside, the QEMU aplic nodes in the DT contain a bunch
> of other issues, including using properties that changed in the
> upstreaming process. Here's the issues with Alistair's current riscv
> tree for QEMU w/ -smp 4 -M virt,aia=3Daplic,dumpdtb=3D$(qemu_dtb) -cpu ma=
x -m 1G -nographic
>
> qemu.dtb: aplic@d000000: $nodename:0: 'aplic@d000000' does not match '^in=
terrupt-controller(@[0-9a-f,]+)*$'
>         from schema $id: http://devicetree.org/schemas/interrupt-controll=
er/riscv,aplic.yaml#
> qemu.dtb: aplic@d000000: compatible:0: 'riscv,aplic' is not one of ['qemu=
,aplic']
>         from schema $id: http://devicetree.org/schemas/interrupt-controll=
er/riscv,aplic.yaml#
> qemu.dtb: aplic@d000000: compatible: ['riscv,aplic'] is too short
>         from schema $id: http://devicetree.org/schemas/interrupt-controll=
er/riscv,aplic.yaml#
> qemu.dtb: aplic@d000000: Unevaluated properties are not allowed ('compati=
ble' was unexpected)
>         from schema $id: http://devicetree.org/schemas/interrupt-controll=
er/riscv,aplic.yaml#
> qemu.dtb: aplic@c000000: $nodename:0: 'aplic@c000000' does not match '^in=
terrupt-controller(@[0-9a-f,]+)*$'
>         from schema $id: http://devicetree.org/schemas/interrupt-controll=
er/riscv,aplic.yaml#
> qemu.dtb: aplic@c000000: compatible:0: 'riscv,aplic' is not one of ['qemu=
,aplic']
>         from schema $id: http://devicetree.org/schemas/interrupt-controll=
er/riscv,aplic.yaml#
> qemu.dtb: aplic@c000000: compatible: ['riscv,aplic'] is too short
>         from schema $id: http://devicetree.org/schemas/interrupt-controll=
er/riscv,aplic.yaml#
> qemu.dtb: aplic@c000000: Unevaluated properties are not allowed ('compati=
ble', 'riscv,delegate' were unexpected)
>         from schema $id: http://devicetree.org/schemas/interrupt-controll=
er/riscv,aplic.yaml#
>
> I guess noone updated QEMU to comply with the bindings that actually got
> upstreamed for the aplic?

Yes, we never bothered to update the QEMU DT generation after
AIA DT bindings were accepted. Thanks for catching.

Regards,
Anup