[PATCH v2] hw/riscv/virt.c: re-insert and deprecate 'riscv,delegate'

Daniel Henrique Barboza posted 1 patch 5 months, 2 weeks ago
docs/about/deprecated.rst | 11 +++++++++++
hw/riscv/virt.c           |  9 +++++++++
2 files changed, 20 insertions(+)
[PATCH v2] hw/riscv/virt.c: re-insert and deprecate 'riscv,delegate'
Posted by Daniel Henrique Barboza 5 months, 2 weeks ago
Commit b1f1e9dcfa renamed 'riscv,delegate' to 'riscv,delegation' since
it is the correct name as per dt-bindings, and the absence of the
correct name will result in validation fails when dumping the dtb and
using dt-validate.

But this change has a side-effect: every other firmware available that
is AIA capable is using 'riscv,delegate', and it will fault/misbehave if
this property isn't present. The property was added back in QEMU 7.0,
meaning we have 2 years of firmware development using the wrong
property.

Re-introducing 'riscv,delegate' while keeping 'riscv,delegation' allows
older firmwares to keep booting with the 'virt' machine.
'riscv,delegate' is then marked for future deprecation with its use
being discouraged from now on.

Cc: Conor Dooley <conor@kernel.org>
Cc: Anup Patel <apatel@ventanamicro.com>
Fixes: b1f1e9dcfa ("hw/riscv/virt.c: aplic DT: rename prop to 'riscv, delegation'")
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---

v2:
 - changed commit msg to remove the 'dt-validate won't complain about
   it' bit since that was incorrect.

 docs/about/deprecated.rst | 11 +++++++++++
 hw/riscv/virt.c           |  9 +++++++++
 2 files changed, 20 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 20b7a17cf0..88f0f03786 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -479,6 +479,17 @@ versions, aliases will point to newer CPU model versions
 depending on the machine type, so management software must
 resolve CPU model aliases before starting a virtual machine.
 
+RISC-V "virt" board "riscv,delegate" DT property (since 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+The "riscv,delegate" DT property was added in QEMU 7.0 as part of
+the AIA APLIC support.  The property changed name during the review
+process in Linux and the correct name ended up being
+"riscv,delegation".  Changing the DT property name will break all
+available firmwares that are using the current (wrong) name.  The
+property is kept as is in 9.1, together with "riscv,delegation", to
+give more time for firmware developers to change their code.
+
 Migration
 ---------
 
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index bc0893e087..9981e0f6c9 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -651,6 +651,15 @@ static void create_fdt_one_aplic(RISCVVirtState *s, int socket,
         qemu_fdt_setprop_cells(ms->fdt, aplic_name, "riscv,delegation",
                                aplic_child_phandle, 0x1,
                                VIRT_IRQCHIP_NUM_SOURCES);
+        /*
+         * DEPRECATED_9.1: Compat property kept temporarily
+         * to allow old firmwares to work with AIA. Do *not*
+         * use 'riscv,delegate' in new code: use
+         * 'riscv,delegation' instead.
+         */
+        qemu_fdt_setprop_cells(ms->fdt, aplic_name, "riscv,delegate",
+                               aplic_child_phandle, 0x1,
+                               VIRT_IRQCHIP_NUM_SOURCES);
     }
 
     riscv_socket_fdt_write_id(ms, aplic_name, socket);
-- 
2.45.2
Re: [PATCH v2] hw/riscv/virt.c: re-insert and deprecate 'riscv, delegate'
Posted by Alistair Francis 5 months, 2 weeks ago
On Mon, Jul 15, 2024 at 7:05 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Commit b1f1e9dcfa renamed 'riscv,delegate' to 'riscv,delegation' since
> it is the correct name as per dt-bindings, and the absence of the
> correct name will result in validation fails when dumping the dtb and
> using dt-validate.
>
> But this change has a side-effect: every other firmware available that
> is AIA capable is using 'riscv,delegate', and it will fault/misbehave if
> this property isn't present. The property was added back in QEMU 7.0,
> meaning we have 2 years of firmware development using the wrong
> property.
>
> Re-introducing 'riscv,delegate' while keeping 'riscv,delegation' allows
> older firmwares to keep booting with the 'virt' machine.
> 'riscv,delegate' is then marked for future deprecation with its use
> being discouraged from now on.
>
> Cc: Conor Dooley <conor@kernel.org>
> Cc: Anup Patel <apatel@ventanamicro.com>
> Fixes: b1f1e9dcfa ("hw/riscv/virt.c: aplic DT: rename prop to 'riscv, delegation'")
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>
> v2:
>  - changed commit msg to remove the 'dt-validate won't complain about
>    it' bit since that was incorrect.
>
>  docs/about/deprecated.rst | 11 +++++++++++
>  hw/riscv/virt.c           |  9 +++++++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 20b7a17cf0..88f0f03786 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -479,6 +479,17 @@ versions, aliases will point to newer CPU model versions
>  depending on the machine type, so management software must
>  resolve CPU model aliases before starting a virtual machine.
>
> +RISC-V "virt" board "riscv,delegate" DT property (since 9.1)
> +''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> +The "riscv,delegate" DT property was added in QEMU 7.0 as part of
> +the AIA APLIC support.  The property changed name during the review
> +process in Linux and the correct name ended up being
> +"riscv,delegation".  Changing the DT property name will break all
> +available firmwares that are using the current (wrong) name.  The
> +property is kept as is in 9.1, together with "riscv,delegation", to
> +give more time for firmware developers to change their code.
> +
>  Migration
>  ---------
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index bc0893e087..9981e0f6c9 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -651,6 +651,15 @@ static void create_fdt_one_aplic(RISCVVirtState *s, int socket,
>          qemu_fdt_setprop_cells(ms->fdt, aplic_name, "riscv,delegation",
>                                 aplic_child_phandle, 0x1,
>                                 VIRT_IRQCHIP_NUM_SOURCES);
> +        /*
> +         * DEPRECATED_9.1: Compat property kept temporarily
> +         * to allow old firmwares to work with AIA. Do *not*
> +         * use 'riscv,delegate' in new code: use
> +         * 'riscv,delegation' instead.
> +         */
> +        qemu_fdt_setprop_cells(ms->fdt, aplic_name, "riscv,delegate",
> +                               aplic_child_phandle, 0x1,
> +                               VIRT_IRQCHIP_NUM_SOURCES);
>      }
>
>      riscv_socket_fdt_write_id(ms, aplic_name, socket);
> --
> 2.45.2
>
>