On Wed, May 29, 2024 at 8:47 PM Conor Dooley <conor@xxxxxxxxxx> 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@d000000: $nodename:0: 'aplic@d000000' does not match '^interrupt-controller(@[0-9a-f,]+)*$' > from schema $id: http://devicetree.org/schemas/interrupt-controller/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-controller/riscv,aplic.yaml# > qemu.dtb: aplic@d000000: compatible: ['riscv,aplic'] is too short > from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml# > qemu.dtb: aplic@d000000: Unevaluated properties are not allowed ('compatible' was unexpected) > from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml# > qemu.dtb: aplic@c000000: $nodename:0: 'aplic@c000000' does not match '^interrupt-controller(@[0-9a-f,]+)*$' > from schema $id: http://devicetree.org/schemas/interrupt-controller/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-controller/riscv,aplic.yaml# > qemu.dtb: aplic@c000000: compatible: ['riscv,aplic'] is too short > from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml# > qemu.dtb: aplic@c000000: 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
