linux-kernel - Re: [PATCH] of: property: Fix fw_devlink handling of interrupt-map

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK9=C2UvppYBUE8_u+M86KSdtULyMAMA=kPvmb4xS0S1_UJGhQ@mail.gmail.com>
Date: Wed, 29 May 2024 22:07:33 +0530
From: Anup Patel <apatel@...tanamicro.com>
To: Conor Dooley <conor@...nel.org>
Cc: Marc Zyngier <maz@...nel.org>, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-riscv@...ts.infradead.org, Saravana Kannan <saravanak@...gle.com>, 
	Rob Herring <robh@...nel.org>
Subject: Re: [PATCH] of: property: Fix fw_devlink handling of interrupt-map

On Wed, May 29, 2024 at 8:47 PM Conor Dooley <conor@...nel.org> wrote:
>
> 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=aplic,dumpdtb=$(qemu_dtb) -cpu max -m 1G -nographic
>
> qemu.dtb: aplic@...0000: $nodename:0: 'aplic@...0000' does not match '^interrupt-controller(@[0-9a-f,]+)*$'
>         from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
> qemu.dtb: aplic@...0000: compatible:0: 'riscv,aplic' is not one of ['qemu,aplic']
>         from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
> qemu.dtb: aplic@...0000: compatible: ['riscv,aplic'] is too short
>         from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
> qemu.dtb: aplic@...0000: Unevaluated properties are not allowed ('compatible' was unexpected)
>         from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
> qemu.dtb: aplic@...0000: $nodename:0: 'aplic@...0000' does not match '^interrupt-controller(@[0-9a-f,]+)*$'
>         from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
> qemu.dtb: aplic@...0000: compatible:0: 'riscv,aplic' is not one of ['qemu,aplic']
>         from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
> qemu.dtb: aplic@...0000: compatible: ['riscv,aplic'] is too short
>         from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
> qemu.dtb: aplic@...0000: Unevaluated properties are not allowed ('compatible', 'riscv,delegate' were unexpected)
>         from schema $id: http://devicetree.org/schemas/interrupt-controller/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