[PATCH 0/5] QEMU: Fix RISC-V virt & spike machines' dtbs

[PATCH 0/5] QEMU: Fix RISC-V virt & spike machines' dtbs

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

The device trees produced automatically for the virt and spike machines
fail dt-validate on several grounds. Some of these need to be fixed in
the linux kernel's dt-bindings, but others are caused by bugs in QEMU.
Patch one of this series is lifted from an earlier submission by Palmer
that seems to have got lost by the wayside somewhere along the way,
hence the rather out of date email used for Palmer [0].

I mostly opted for what appeared to be the smallest change that would
fix the warnings, partly due to my inexperience with the QEMU codebase.
A "sister" patchset for the kernel will clear the remaining warnings.

Thanks to Rob for reporting these issues [1],
Conor.

To reproduce the errors:
./build/qemu-system-riscv64 -nographic -machine virt,dumpdtb=qemu.dtb
dt-validate -p /path/to/linux/kernel/Documentation/devicetree/bindings/processed-schema.json qemu.dtb
(The processed schema needs to be generated first)

0 - https://lore.kernel.org/qemu-devel/20190813225307.5792-1-palmer@sifive.com/
1 - https://lore.kernel.org/linux-riscv/20220803170552.GA2250266-robh@kernel.org/

Conor Dooley (4):
  hw/riscv: virt: fix uart node name
  hw/riscv: virt: Fix the plic's address cells
  hw/riscv: virt: fix syscon subnode paths
  hw/core: fix platform bus node name

Palmer Dabbelt (1):
  target/riscv: Ignore the S and U letters when formatting ISA strings

 hw/core/sysbus-fdt.c    |  2 +-
 hw/riscv/virt.c         | 10 +++++++---
 include/hw/riscv/virt.h |  1 +
 target/riscv/cpu.c      | 18 +++++++++++++++++-
 4 files changed, 26 insertions(+), 5 deletions(-)


base-commit: 2480f3bbd03814b0651a1f74959f5c6631ee5819
-- 
2.37.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 1/5] target/riscv: Ignore the S and U letters when formatting ISA strings

[Conor Dooley]

From: Palmer Dabbelt <palmer@sifive.com>

The ISA strings we're providing from QEMU aren't actually legal RISC-V
ISA strings, as both S and U cannot exist as single-letter extensions
and must instead be multi-letter strings.  We're still using the ISA
strings inside QEMU to track the availiable extensions, so just strip
out the S and U extensions when formatting ISA strings.

Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
[Conor: rebased on 7.1.0-rc1 & slightly tweaked the commit message]
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 target/riscv/cpu.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ac6f82ebd0..95fdc03b3d 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1122,7 +1122,23 @@ char *riscv_isa_string(RISCVCPU *cpu)
     char *p = isa_str + snprintf(isa_str, maxlen, "rv%d", TARGET_LONG_BITS);
     for (i = 0; i < sizeof(riscv_single_letter_exts) - 1; i++) {
         if (cpu->env.misa_ext & RV(riscv_single_letter_exts[i])) {
-            *p++ = qemu_tolower(riscv_single_letter_exts[i]);
+            char lower = qemu_tolower(riscv_single_letter_exts[i]);
+            switch (lower) {
+                case 's':
+                case 'u':
+                    /*
+                    * The 's' and 'u' letters shouldn't show up in ISA strings as
+                    * they're not extensions, but they should show up in MISA.
+                    * Since we use these letters interally as a pseudo ISA string
+                    * to set MISA it's easier to just strip them out when
+                    * formatting the ISA string.
+                    */
+                    break;
+
+                default:
+                    *p++ = lower;
+                    break;
+            }
         }
     }
     *p = '\0';
-- 
2.37.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/5] target/riscv: Ignore the S and U letters when formatting ISA strings

[Alistair Francis]

On Sat, Aug 6, 2022 at 2:08 AM Conor Dooley <mail@conchuod.ie> wrote:
>
> From: Palmer Dabbelt <palmer@sifive.com>
>
> The ISA strings we're providing from QEMU aren't actually legal RISC-V
> ISA strings, as both S and U cannot exist as single-letter extensions
> and must instead be multi-letter strings.  We're still using the ISA
> strings inside QEMU to track the availiable extensions, so just strip
> out the S and U extensions when formatting ISA strings.
>
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
> [Conor: rebased on 7.1.0-rc1 & slightly tweaked the commit message]
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  target/riscv/cpu.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ac6f82ebd0..95fdc03b3d 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1122,7 +1122,23 @@ char *riscv_isa_string(RISCVCPU *cpu)
>      char *p = isa_str + snprintf(isa_str, maxlen, "rv%d", TARGET_LONG_BITS);
>      for (i = 0; i < sizeof(riscv_single_letter_exts) - 1; i++) {
>          if (cpu->env.misa_ext & RV(riscv_single_letter_exts[i])) {
> -            *p++ = qemu_tolower(riscv_single_letter_exts[i]);

riscv_single_letter_exts doesn't contain S or U, is this patch still required?

Alistair

> +            char lower = qemu_tolower(riscv_single_letter_exts[i]);
> +            switch (lower) {
> +                case 's':
> +                case 'u':
> +                    /*
> +                    * The 's' and 'u' letters shouldn't show up in ISA strings as
> +                    * they're not extensions, but they should show up in MISA.
> +                    * Since we use these letters interally as a pseudo ISA string
> +                    * to set MISA it's easier to just strip them out when
> +                    * formatting the ISA string.
> +                    */
> +                    break;
> +
> +                default:
> +                    *p++ = lower;
> +                    break;
> +            }
>          }
>      }
>      *p = '\0';
> --
> 2.37.1
>
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/5] target/riscv: Ignore the S and U letters when formatting ISA strings

[Conor.Dooley]

On 07/08/2022 23:53, Alistair Francis wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Sat, Aug 6, 2022 at 2:08 AM Conor Dooley <mail@conchuod.ie> wrote:
>>
>> From: Palmer Dabbelt <palmer@sifive.com>
>>
>> The ISA strings we're providing from QEMU aren't actually legal RISC-V
>> ISA strings, as both S and U cannot exist as single-letter extensions
>> and must instead be multi-letter strings.  We're still using the ISA
>> strings inside QEMU to track the availiable extensions, so just strip
>> out the S and U extensions when formatting ISA strings.
>>
>> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
>> [Conor: rebased on 7.1.0-rc1 & slightly tweaked the commit message]
>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>> ---
>>   target/riscv/cpu.c | 18 +++++++++++++++++-
>>   1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index ac6f82ebd0..95fdc03b3d 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -1122,7 +1122,23 @@ char *riscv_isa_string(RISCVCPU *cpu)
>>       char *p = isa_str + snprintf(isa_str, maxlen, "rv%d", TARGET_LONG_BITS);
>>       for (i = 0; i < sizeof(riscv_single_letter_exts) - 1; i++) {
>>           if (cpu->env.misa_ext & RV(riscv_single_letter_exts[i])) {
>> -            *p++ = qemu_tolower(riscv_single_letter_exts[i]);
> 
> riscv_single_letter_exts doesn't contain S or U, is this patch still required?

Seemingly, yes. This is what ends up in the dtb:
/home/rob/riscv-virt.dtb: cpu@0: riscv,isa:0: 'rv64imafdcsuh' is not one of ['rv64imac', 'rv64imafdc']
         From schema: /home/rob/proj/git/linux-dt/Documentation/devicetree/bindings/riscv/cpus.yaml

With Palmer's patch applied, the dtb's isa string becomes
rv64imafdch_zicsr_zifencei_zba_zbb_zbc_zbs
while in n /proc/cpuinfo it is rv64imafdch

The short_isa_string flag (I think that's it's name) is not
set for the dtb creation.  meant to note this under the ---
for this patch but obviously I forgot.

Thanks,
Conor.

> 
> Alistair
> 
>> +            char lower = qemu_tolower(riscv_single_letter_exts[i]);
>> +            switch (lower) {
>> +                case 's':
>> +                case 'u':
>> +                    /*
>> +                    * The 's' and 'u' letters shouldn't show up in ISA strings as
>> +                    * they're not extensions, but they should show up in MISA.
>> +                    * Since we use these letters interally as a pseudo ISA string
>> +                    * to set MISA it's easier to just strip them out when
>> +                    * formatting the ISA string.
>> +                    */
>> +                    break;
>> +
>> +                default:
>> +                    *p++ = lower;
>> +                    break;
>> +            }
>>           }
>>       }
>>       *p = '\0';
>> --
>> 2.37.1
>>
>>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/5] target/riscv: Ignore the S and U letters when formatting ISA strings

[Tsukasa OI]

On 2022/08/06 0:54, Conor Dooley wrote:
> From: Palmer Dabbelt <palmer@sifive.com>
> 
> The ISA strings we're providing from QEMU aren't actually legal RISC-V
> ISA strings, as both S and U cannot exist as single-letter extensions
> and must instead be multi-letter strings.  We're still using the ISA
> strings inside QEMU to track the availiable extensions, so just strip
> out the S and U extensions when formatting ISA strings.
> 
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
> [Conor: rebased on 7.1.0-rc1 & slightly tweaked the commit message]
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  target/riscv/cpu.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ac6f82ebd0..95fdc03b3d 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1122,7 +1122,23 @@ char *riscv_isa_string(RISCVCPU *cpu)
>      char *p = isa_str + snprintf(isa_str, maxlen, "rv%d", TARGET_LONG_BITS);
>      for (i = 0; i < sizeof(riscv_single_letter_exts) - 1; i++) {
>          if (cpu->env.misa_ext & RV(riscv_single_letter_exts[i])) {
> -            *p++ = qemu_tolower(riscv_single_letter_exts[i]);
> +            char lower = qemu_tolower(riscv_single_letter_exts[i]);
> +            switch (lower) {
> +                case 's':
> +                case 'u':
> +                    /*
> +                    * The 's' and 'u' letters shouldn't show up in ISA strings as
> +                    * they're not extensions, but they should show up in MISA.
> +                    * Since we use these letters interally as a pseudo ISA string
> +                    * to set MISA it's easier to just strip them out when
> +                    * formatting the ISA string.
> +                    */
> +                    break;
> +
> +                default:
> +                    *p++ = lower;
> +                    break;
> +            }
>          }
>      }
>      *p = '\0';

I agree with Alistair.  **I** removed 'S' and 'U' from the ISA string
and it should be working in the latest development branch (I tested).

I tested it on master and QEMU 7.1-rc1 (tag: v7.1-rc1).

Example:
/opt/riscv/bin/qemu-system-riscv64
         -machine virt
         -nographic
         -cpu rv64
         -smp 1
         -kernel images/linux.bin
         -initrd images/busybox.cpio.gz
         -append 'console=hvc0 earlycon=sbi'
         -bios images/opensbi-fw_jump.elf
         -gdb tcp::9000

Replacing -machine virt with -machine virt,dumpdtb=sample.dtb dumps the
binary DeviceTree as sample.dtb and generated CPU-related parts like...

                cpu@0 {
                        phandle = <0x01>;
                        device_type = "cpu";
                        reg = <0x00>;
                        status = "okay";
                        compatible = "riscv";
                        riscv,isa =
"rv64imafdch_zicsr_zifencei_zba_zbb_zbc_zbs";
                        mmu-type = "riscv,sv48";

                        interrupt-controller {
                                #interrupt-cells = <0x01>;
                                interrupt-controller;
                                compatible = "riscv,cpu-intc";
                                phandle = <0x02>;
                        };
                };

Besides, this function alone generates the ISA string for DTB and
there's no way such ISA strings with invalid 'S' and 'U' can be
generated.  It's definitely a behavior of QEMU 7.0 or before.

Please. Please make sure that you are testing the right version of QEMU.

Thanks,
Tsukasa

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/5] target/riscv: Ignore the S and U letters when formatting ISA strings

[Conor.Dooley]

On 08/08/2022 16:03, Tsukasa OI wrote:
> I agree with Alistair.  **I** removed 'S' and 'U' from the ISA string
> and it should be working in the latest development branch (I tested).

Yeah, I saw what you as I looked at the commit log while trying to
understand why there were invalid strings appearing when the code
looked like it was impossible. Certainly I found it very very odd.
I didn't just revive a 2 year old patch without taking a look at
the code.


> Besides, this function alone generates the ISA string for DTB and
> there's no way such ISA strings with invalid 'S' and 'U' can be
> generated.  It's definitely a behavior of QEMU 7.0 or before.

Hmm, it would seem that you're right - I have retested on a fresh
clone. I did checkout v7.1.0-rc1 before running the first build that
I saw the invalid string on as I'd been on some hacked up & fossilised
version prior to that. Perhaps some build artifacts were not correctly
removed, consider me quite confused! I do recall the configure script
saying something about my build directory when I kicked it off, so
it is likely down to that.

Unfortunately my bash history is out of order so I will not be able to
replicate the conditions, having many terminals open does have it's
downsides.

> Please. Please make sure that you are testing the right version of QEMU.

Heh. Please, please give me some allowance for reasonably believing I
was in fact on the latest qemu/master after checking it out and building!

I guess this patch can then be safely ignored :)
Glad to have cleared this up as I was rather confused by what I saw.
Thanks Tsukasa/Alistair.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/5] target/riscv: Ignore the S and U letters when formatting ISA strings

[Alistair Francis]

On Tue, Aug 9, 2022 at 2:14 AM <Conor.Dooley@microchip.com> wrote:
>
> On 08/08/2022 16:03, Tsukasa OI wrote:
> > I agree with Alistair.  **I** removed 'S' and 'U' from the ISA string
> > and it should be working in the latest development branch (I tested).
>
> Yeah, I saw what you as I looked at the commit log while trying to
> understand why there were invalid strings appearing when the code
> looked like it was impossible. Certainly I found it very very odd.
> I didn't just revive a 2 year old patch without taking a look at
> the code.
>
>
> > Besides, this function alone generates the ISA string for DTB and
> > there's no way such ISA strings with invalid 'S' and 'U' can be
> > generated.  It's definitely a behavior of QEMU 7.0 or before.
>
> Hmm, it would seem that you're right - I have retested on a fresh
> clone. I did checkout v7.1.0-rc1 before running the first build that
> I saw the invalid string on as I'd been on some hacked up & fossilised
> version prior to that. Perhaps some build artifacts were not correctly
> removed, consider me quite confused! I do recall the configure script
> saying something about my build directory when I kicked it off, so
> it is likely down to that.
>
> Unfortunately my bash history is out of order so I will not be able to
> replicate the conditions, having many terminals open does have it's
> downsides.
>
> > Please. Please make sure that you are testing the right version of QEMU.
>
> Heh. Please, please give me some allowance for reasonably believing I
> was in fact on the latest qemu/master after checking it out and building!

No worries! Glad we cleared that up

>
> I guess this patch can then be safely ignored :)
> Glad to have cleared this up as I was rather confused by what I saw.

Great! Do you mind resending the series then with this patch dropped?
It just makes it easier for me to track and manage

Alistair

> Thanks Tsukasa/Alistair.
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/5] target/riscv: Ignore the S and U letters when formatting ISA strings

[Conor.Dooley]

On 08/08/2022 21:51, Alistair Francis wrote:
> On Tue, Aug 9, 2022 at 2:14 AM <Conor.Dooley@microchip.com> wrote:
>> I guess this patch can then be safely ignored :)
>> Glad to have cleared this up as I was rather confused by what I saw.
> 
> Great! Do you mind resending the series then with this patch dropped?
> It just makes it easier for me to track and manage

Aye, willdo.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 2/5] hw/riscv: virt: fix uart node name

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

"uart" is not a node name that complies with the dt-schema.
Change the node name to "serial" to ix warnings seen during
dt-validate on a dtbdump of the virt machine such as:
/stuff/qemu/qemu.dtb: uart@10000000: $nodename:0: 'uart@10000000' does not match '^serial(@.*)?$'
        From schema: /stuff/linux/Documentation/devicetree/bindings/serial/8250.yaml

Reported-by: Rob Herring <robh@kernel.org>
Link: https://lore.kernel.org/linux-riscv/20220803170552.GA2250266-robh@kernel.org/
Fixes: 04331d0b56 ("RISC-V VirtIO Machine")
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 hw/riscv/virt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index bc424dd2f5..6c61a406c4 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -917,7 +917,7 @@ static void create_fdt_uart(RISCVVirtState *s, const MemMapEntry *memmap,
     char *name;
     MachineState *mc = MACHINE(s);
 
-    name = g_strdup_printf("/soc/uart@%lx", (long)memmap[VIRT_UART0].base);
+    name = g_strdup_printf("/soc/serial@%lx", (long)memmap[VIRT_UART0].base);
     qemu_fdt_add_subnode(mc->fdt, name);
     qemu_fdt_setprop_string(mc->fdt, name, "compatible", "ns16550a");
     qemu_fdt_setprop_cells(mc->fdt, name, "reg",
-- 
2.37.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/5] hw/riscv: virt: fix uart node name

[Alistair Francis]

On Sat, Aug 6, 2022 at 2:01 AM Conor Dooley <mail@conchuod.ie> wrote:
>
> From: Conor Dooley <conor.dooley@microchip.com>
>
> "uart" is not a node name that complies with the dt-schema.
> Change the node name to "serial" to ix warnings seen during
> dt-validate on a dtbdump of the virt machine such as:
> /stuff/qemu/qemu.dtb: uart@10000000: $nodename:0: 'uart@10000000' does not match '^serial(@.*)?$'
>         From schema: /stuff/linux/Documentation/devicetree/bindings/serial/8250.yaml
>
> Reported-by: Rob Herring <robh@kernel.org>
> Link: https://lore.kernel.org/linux-riscv/20220803170552.GA2250266-robh@kernel.org/
> Fixes: 04331d0b56 ("RISC-V VirtIO Machine")
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/riscv/virt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index bc424dd2f5..6c61a406c4 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -917,7 +917,7 @@ static void create_fdt_uart(RISCVVirtState *s, const MemMapEntry *memmap,
>      char *name;
>      MachineState *mc = MACHINE(s);
>
> -    name = g_strdup_printf("/soc/uart@%lx", (long)memmap[VIRT_UART0].base);
> +    name = g_strdup_printf("/soc/serial@%lx", (long)memmap[VIRT_UART0].base);
>      qemu_fdt_add_subnode(mc->fdt, name);
>      qemu_fdt_setprop_string(mc->fdt, name, "compatible", "ns16550a");
>      qemu_fdt_setprop_cells(mc->fdt, name, "reg",
> --
> 2.37.1
>
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 3/5] hw/riscv: virt: Fix the plic's address cells

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

When optional AIA PLIC support was added the to the virt machine, the
address cells property was removed leading the issues with dt-validate
on a dump from the virt machine:
/stuff/qemu/qemu.dtb: plic@c000000: '#address-cells' is a required property
        From schema: /stuff/linux/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
Add back the property to suppress the warning.

Reported-by: Rob Herring <robh@kernel.org>
Link: https://lore.kernel.org/linux-riscv/20220803170552.GA2250266-robh@kernel.org/
Fixes: e6faee6585 ("hw/riscv: virt: Add optional AIA APLIC support to virt machine")
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 hw/riscv/virt.c         | 2 ++
 include/hw/riscv/virt.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 6c61a406c4..8b2978076e 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -465,6 +465,8 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
     qemu_fdt_add_subnode(mc->fdt, plic_name);
     qemu_fdt_setprop_cell(mc->fdt, plic_name,
         "#interrupt-cells", FDT_PLIC_INT_CELLS);
+    qemu_fdt_setprop_cell(mc->fdt, plic_name,
+        "#address-cells", FDT_PLIC_ADDR_CELLS);
     qemu_fdt_setprop_string_array(mc->fdt, plic_name, "compatible",
                                   (char **)&plic_compat,
                                   ARRAY_SIZE(plic_compat));
diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index 984e55c77f..be4ab8fe7f 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -111,6 +111,7 @@ enum {
 
 #define FDT_PCI_ADDR_CELLS    3
 #define FDT_PCI_INT_CELLS     1
+#define FDT_PLIC_ADDR_CELLS   0
 #define FDT_PLIC_INT_CELLS    1
 #define FDT_APLIC_INT_CELLS   2
 #define FDT_IMSIC_INT_CELLS   0
-- 
2.37.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 3/5] hw/riscv: virt: Fix the plic's address cells

[Alistair Francis]

On Sat, Aug 6, 2022 at 2:11 AM Conor Dooley <mail@conchuod.ie> wrote:
>
> From: Conor Dooley <conor.dooley@microchip.com>
>
> When optional AIA PLIC support was added the to the virt machine, the
> address cells property was removed leading the issues with dt-validate
> on a dump from the virt machine:
> /stuff/qemu/qemu.dtb: plic@c000000: '#address-cells' is a required property
>         From schema: /stuff/linux/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> Add back the property to suppress the warning.
>
> Reported-by: Rob Herring <robh@kernel.org>
> Link: https://lore.kernel.org/linux-riscv/20220803170552.GA2250266-robh@kernel.org/
> Fixes: e6faee6585 ("hw/riscv: virt: Add optional AIA APLIC support to virt machine")
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/riscv/virt.c         | 2 ++
>  include/hw/riscv/virt.h | 1 +
>  2 files changed, 3 insertions(+)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 6c61a406c4..8b2978076e 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -465,6 +465,8 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
>      qemu_fdt_add_subnode(mc->fdt, plic_name);
>      qemu_fdt_setprop_cell(mc->fdt, plic_name,
>          "#interrupt-cells", FDT_PLIC_INT_CELLS);
> +    qemu_fdt_setprop_cell(mc->fdt, plic_name,
> +        "#address-cells", FDT_PLIC_ADDR_CELLS);
>      qemu_fdt_setprop_string_array(mc->fdt, plic_name, "compatible",
>                                    (char **)&plic_compat,
>                                    ARRAY_SIZE(plic_compat));
> diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
> index 984e55c77f..be4ab8fe7f 100644
> --- a/include/hw/riscv/virt.h
> +++ b/include/hw/riscv/virt.h
> @@ -111,6 +111,7 @@ enum {
>
>  #define FDT_PCI_ADDR_CELLS    3
>  #define FDT_PCI_INT_CELLS     1
> +#define FDT_PLIC_ADDR_CELLS   0
>  #define FDT_PLIC_INT_CELLS    1
>  #define FDT_APLIC_INT_CELLS   2
>  #define FDT_IMSIC_INT_CELLS   0
> --
> 2.37.1
>
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 4/5] hw/riscv: virt: fix syscon subnode paths

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

The subnodes of the syscon have been added to the incorrect paths.
Rather than add them as subnodes, they were originally added to "/foo"
and a later patch moved them to "/soc/foo". Both are incorrect & they
should have been added as "/soc/test@###/foo" as "/soc/test" is the
syscon node. Fix both the reboot and poweroff subnodes to avoid errors
such as:

/stuff/qemu/qemu.dtb: soc: poweroff: {'value': [[21845]], 'offset': [[0]], 'regmap': [[4]], 'compatible': ['syscon-poweroff']} should not be valid under {'type': 'object'}
        From schema: /home/conor/.local/lib/python3.9/site-packages/dtschema/schemas/simple-bus.yaml
/stuff/qemu/qemu.dtb: soc: reboot: {'value': [[30583]], 'offset': [[0]], 'regmap': [[4]], 'compatible': ['syscon-reboot']} should not be valid under {'type': 'object'}
        From schema: /home/conor/.local/lib/python3.9/site-packages/dtschema/schemas/simple-bus.yaml

Reported-by: Rob Herring <robh@kernel.org>
Link: https://lore.kernel.org/linux-riscv/20220803170552.GA2250266-robh@kernel.org/
Fixes: 18df0b4695 ("hw/riscv: virt: Allow creating multiple NUMA sockets")
Fixes: 0e404da007 ("riscv/virt: Add syscon reboot and poweroff DT nodes")
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 hw/riscv/virt.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 8b2978076e..a98b054545 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -896,7 +896,8 @@ static void create_fdt_reset(RISCVVirtState *s, const MemMapEntry *memmap,
     test_phandle = qemu_fdt_get_phandle(mc->fdt, name);
     g_free(name);
 
-    name = g_strdup_printf("/soc/reboot");
+    name = g_strdup_printf("/soc/test@%lx/reboot",
+        (long)memmap[VIRT_TEST].base);
     qemu_fdt_add_subnode(mc->fdt, name);
     qemu_fdt_setprop_string(mc->fdt, name, "compatible", "syscon-reboot");
     qemu_fdt_setprop_cell(mc->fdt, name, "regmap", test_phandle);
@@ -904,7 +905,8 @@ static void create_fdt_reset(RISCVVirtState *s, const MemMapEntry *memmap,
     qemu_fdt_setprop_cell(mc->fdt, name, "value", FINISHER_RESET);
     g_free(name);
 
-    name = g_strdup_printf("/soc/poweroff");
+    name = g_strdup_printf("/soc/test@%lx/poweroff",
+        (long)memmap[VIRT_TEST].base);
     qemu_fdt_add_subnode(mc->fdt, name);
     qemu_fdt_setprop_string(mc->fdt, name, "compatible", "syscon-poweroff");
     qemu_fdt_setprop_cell(mc->fdt, name, "regmap", test_phandle);
-- 
2.37.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 4/5] hw/riscv: virt: fix syscon subnode paths

[Alistair Francis]

On Sat, Aug 6, 2022 at 2:10 AM Conor Dooley <mail@conchuod.ie> wrote:
>
> From: Conor Dooley <conor.dooley@microchip.com>
>
> The subnodes of the syscon have been added to the incorrect paths.
> Rather than add them as subnodes, they were originally added to "/foo"
> and a later patch moved them to "/soc/foo". Both are incorrect & they
> should have been added as "/soc/test@###/foo" as "/soc/test" is the
> syscon node. Fix both the reboot and poweroff subnodes to avoid errors
> such as:
>
> /stuff/qemu/qemu.dtb: soc: poweroff: {'value': [[21845]], 'offset': [[0]], 'regmap': [[4]], 'compatible': ['syscon-poweroff']} should not be valid under {'type': 'object'}
>         From schema: /home/conor/.local/lib/python3.9/site-packages/dtschema/schemas/simple-bus.yaml
> /stuff/qemu/qemu.dtb: soc: reboot: {'value': [[30583]], 'offset': [[0]], 'regmap': [[4]], 'compatible': ['syscon-reboot']} should not be valid under {'type': 'object'}
>         From schema: /home/conor/.local/lib/python3.9/site-packages/dtschema/schemas/simple-bus.yaml
>
> Reported-by: Rob Herring <robh@kernel.org>
> Link: https://lore.kernel.org/linux-riscv/20220803170552.GA2250266-robh@kernel.org/
> Fixes: 18df0b4695 ("hw/riscv: virt: Allow creating multiple NUMA sockets")
> Fixes: 0e404da007 ("riscv/virt: Add syscon reboot and poweroff DT nodes")
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/riscv/virt.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 8b2978076e..a98b054545 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -896,7 +896,8 @@ static void create_fdt_reset(RISCVVirtState *s, const MemMapEntry *memmap,
>      test_phandle = qemu_fdt_get_phandle(mc->fdt, name);
>      g_free(name);
>
> -    name = g_strdup_printf("/soc/reboot");
> +    name = g_strdup_printf("/soc/test@%lx/reboot",
> +        (long)memmap[VIRT_TEST].base);
>      qemu_fdt_add_subnode(mc->fdt, name);
>      qemu_fdt_setprop_string(mc->fdt, name, "compatible", "syscon-reboot");
>      qemu_fdt_setprop_cell(mc->fdt, name, "regmap", test_phandle);
> @@ -904,7 +905,8 @@ static void create_fdt_reset(RISCVVirtState *s, const MemMapEntry *memmap,
>      qemu_fdt_setprop_cell(mc->fdt, name, "value", FINISHER_RESET);
>      g_free(name);
>
> -    name = g_strdup_printf("/soc/poweroff");
> +    name = g_strdup_printf("/soc/test@%lx/poweroff",
> +        (long)memmap[VIRT_TEST].base);
>      qemu_fdt_add_subnode(mc->fdt, name);
>      qemu_fdt_setprop_string(mc->fdt, name, "compatible", "syscon-poweroff");
>      qemu_fdt_setprop_cell(mc->fdt, name, "regmap", test_phandle);
> --
> 2.37.1
>
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 5/5] hw/core: fix platform bus node name

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

"platform" is not a valid name for a bus node in dt-schema, so warnings
can be see in dt-validate on a dump of the riscv virt dtb:

/stuff/qemu/qemu.dtb: platform@4000000: $nodename:0: 'platform@4000000' does not match '^([a-z][a-z0-9\\-]+-bus|bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$'
        From schema: /home/conor/.local/lib/python3.9/site-packages/dtschema/schemas/simple-bus.yaml
"platform-bus" is a valid name, so use that instead.

CC: Rob Herring <robh@kernel.org>
Fixes: 11d306b9df ("hw/arm/sysbus-fdt: helpers for platform bus nodes addition")
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 hw/core/sysbus-fdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/core/sysbus-fdt.c b/hw/core/sysbus-fdt.c
index 19d22cbe73..edb0c49b19 100644
--- a/hw/core/sysbus-fdt.c
+++ b/hw/core/sysbus-fdt.c
@@ -539,7 +539,7 @@ void platform_bus_add_all_fdt_nodes(void *fdt, const char *intc, hwaddr addr,
 
     assert(fdt);
 
-    node = g_strdup_printf("/platform@%"PRIx64, addr);
+    node = g_strdup_printf("/platform-bus@%"PRIx64, addr);
 
     /* Create a /platform node that we can put all devices into */
     qemu_fdt_add_subnode(fdt, node);
-- 
2.37.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 5/5] hw/core: fix platform bus node name

[Alistair Francis]

On Sat, Aug 6, 2022 at 2:02 AM Conor Dooley <mail@conchuod.ie> wrote:
>
> From: Conor Dooley <conor.dooley@microchip.com>
>
> "platform" is not a valid name for a bus node in dt-schema, so warnings
> can be see in dt-validate on a dump of the riscv virt dtb:
>
> /stuff/qemu/qemu.dtb: platform@4000000: $nodename:0: 'platform@4000000' does not match '^([a-z][a-z0-9\\-]+-bus|bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$'
>         From schema: /home/conor/.local/lib/python3.9/site-packages/dtschema/schemas/simple-bus.yaml
> "platform-bus" is a valid name, so use that instead.
>
> CC: Rob Herring <robh@kernel.org>
> Fixes: 11d306b9df ("hw/arm/sysbus-fdt: helpers for platform bus nodes addition")
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/core/sysbus-fdt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/core/sysbus-fdt.c b/hw/core/sysbus-fdt.c
> index 19d22cbe73..edb0c49b19 100644
> --- a/hw/core/sysbus-fdt.c
> +++ b/hw/core/sysbus-fdt.c
> @@ -539,7 +539,7 @@ void platform_bus_add_all_fdt_nodes(void *fdt, const char *intc, hwaddr addr,
>
>      assert(fdt);
>
> -    node = g_strdup_printf("/platform@%"PRIx64, addr);
> +    node = g_strdup_printf("/platform-bus@%"PRIx64, addr);
>
>      /* Create a /platform node that we can put all devices into */
>      qemu_fdt_add_subnode(fdt, node);
> --
> 2.37.1
>
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv