diff mbox series

[PULL,22/30] hw/arm: Move virt's PSCI DT fixup code to arm/boot.c

Message ID 20180209110314.11766-23-peter.maydell@linaro.org
State Accepted
Commit 4cbca7d9b4de5ecf6a8447b2eba89d591717f6c4
Headers show
Series target-arm queue | expand

Commit Message

Peter Maydell Feb. 9, 2018, 11:03 a.m. UTC 
From: Andrey Smirnov <andrew.smirnov@gmail.com>


Move virt's PSCI DT fixup code to arm/boot.c and set this fixup to
happen automatically for every board that doesn't mark "psci-conduit"
as disabled. This way emulated boards other than "virt" that rely on
PSIC for SMP could benefit from that code.

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
Cc: Marcel Apfelbaum <marcel.apfelbaum@zoho.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: qemu-devel@nongnu.org
Cc: qemu-arm@nongnu.org
Cc: yurovsky@gmail.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---
 hw/arm/boot.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/arm/virt.c | 61 -------------------------------------------------------
 2 files changed, 65 insertions(+), 61 deletions(-)

-- 
2.16.1

Comments

Marc Zyngier March 27, 2018, 2:22 p.m. UTC | #1 
On Fri, 09 Feb 2018 11:03:06 +0000,
Peter Maydell wrote:

Hi Peter, Andrey,

> 

> From: Andrey Smirnov <andrew.smirnov@gmail.com>

> 

> Move virt's PSCI DT fixup code to arm/boot.c and set this fixup to

> happen automatically for every board that doesn't mark "psci-conduit"

> as disabled. This way emulated boards other than "virt" that rely on

> PSIC for SMP could benefit from that code.

> 

> Cc: Peter Maydell <peter.maydell@linaro.org>

> Cc: Jason Wang <jasowang@redhat.com>

> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>

> Cc: Marcel Apfelbaum <marcel.apfelbaum@zoho.com>

> Cc: Michael S. Tsirkin <mst@redhat.com>

> Cc: qemu-devel@nongnu.org

> Cc: qemu-arm@nongnu.org

> Cc: yurovsky@gmail.com

> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> ---

>  hw/arm/boot.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

>  hw/arm/virt.c | 61 -------------------------------------------------------

>  2 files changed, 65 insertions(+), 61 deletions(-)

> 

> diff --git a/hw/arm/boot.c b/hw/arm/boot.c

> index bb244ec359..9b174b982c 100644

> --- a/hw/arm/boot.c

> +++ b/hw/arm/boot.c

> @@ -385,6 +385,69 @@ static void set_kernel_args_old(const struct arm_boot_info *info)

>      }

>  }

>  

> +static void fdt_add_psci_node(void *fdt)

> +{

> +    uint32_t cpu_suspend_fn;

> +    uint32_t cpu_off_fn;

> +    uint32_t cpu_on_fn;

> +    uint32_t migrate_fn;

> +    ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(0));

> +    const char *psci_method;

> +    int64_t psci_conduit;

> +

> +    psci_conduit = object_property_get_int(OBJECT(armcpu),

> +                                           "psci-conduit",

> +                                           &error_abort);

> +    switch (psci_conduit) {

> +    case QEMU_PSCI_CONDUIT_DISABLED:

> +        return;

> +    case QEMU_PSCI_CONDUIT_HVC:

> +        psci_method = "hvc";

> +        break;

> +    case QEMU_PSCI_CONDUIT_SMC:

> +        psci_method = "smc";

> +        break;

> +    default:

> +        g_assert_not_reached();

> +    }

> +

> +    qemu_fdt_add_subnode(fdt, "/psci");

> +    if (armcpu->psci_version == 2) {

> +        const char comp[] = "arm,psci-0.2\0arm,psci";

> +        qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp));

> +

> +        cpu_off_fn = QEMU_PSCI_0_2_FN_CPU_OFF;

> +        if (arm_feature(&armcpu->env, ARM_FEATURE_AARCH64)) {

> +            cpu_suspend_fn = QEMU_PSCI_0_2_FN64_CPU_SUSPEND;

> +            cpu_on_fn = QEMU_PSCI_0_2_FN64_CPU_ON;

> +            migrate_fn = QEMU_PSCI_0_2_FN64_MIGRATE;

> +        } else {

> +            cpu_suspend_fn = QEMU_PSCI_0_2_FN_CPU_SUSPEND;

> +            cpu_on_fn = QEMU_PSCI_0_2_FN_CPU_ON;

> +            migrate_fn = QEMU_PSCI_0_2_FN_MIGRATE;

> +        }

> +    } else {

> +        qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci");

> +

> +        cpu_suspend_fn = QEMU_PSCI_0_1_FN_CPU_SUSPEND;

> +        cpu_off_fn = QEMU_PSCI_0_1_FN_CPU_OFF;

> +        cpu_on_fn = QEMU_PSCI_0_1_FN_CPU_ON;

> +        migrate_fn = QEMU_PSCI_0_1_FN_MIGRATE;

> +    }

> +

> +    /* We adopt the PSCI spec's nomenclature, and use 'conduit' to refer

> +     * to the instruction that should be used to invoke PSCI functions.

> +     * However, the device tree binding uses 'method' instead, so that is

> +     * what we should use here.

> +     */

> +    qemu_fdt_setprop_string(fdt, "/psci", "method", psci_method);

> +

> +    qemu_fdt_setprop_cell(fdt, "/psci", "cpu_suspend", cpu_suspend_fn);

> +    qemu_fdt_setprop_cell(fdt, "/psci", "cpu_off", cpu_off_fn);

> +    qemu_fdt_setprop_cell(fdt, "/psci", "cpu_on", cpu_on_fn);

> +    qemu_fdt_setprop_cell(fdt, "/psci", "migrate", migrate_fn);

> +}

> +

>  /**

>   * load_dtb() - load a device tree binary image into memory

>   * @addr:       the address to load the image at

> @@ -541,6 +604,8 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo,

>          }

>      }

>  

> +    fdt_add_psci_node(fdt);

> +


This particular change broke one of my favourite use cases, which is
to feed the guest with a hand-crafted DTB, and expect QEMU to just
take it as granted, and not touch it.

I now get an error:

qemu-system-aarch64: FDT: Failed to create subnode /psci: FDT_ERR_EXISTS

The obvious workaround would be to test the presence of the /psci node
before generating it (such as done for the /chosen node, for example).

Thanks,

	M.

-- 
Jazz is not dead, it just smell funny.
diff mbox series

Patch

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index bb244ec359..9b174b982c 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -385,6 +385,69 @@  static void set_kernel_args_old(const struct arm_boot_info *info)
     }
 }
 
+static void fdt_add_psci_node(void *fdt)
+{
+    uint32_t cpu_suspend_fn;
+    uint32_t cpu_off_fn;
+    uint32_t cpu_on_fn;
+    uint32_t migrate_fn;
+    ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(0));
+    const char *psci_method;
+    int64_t psci_conduit;
+
+    psci_conduit = object_property_get_int(OBJECT(armcpu),
+                                           "psci-conduit",
+                                           &error_abort);
+    switch (psci_conduit) {
+    case QEMU_PSCI_CONDUIT_DISABLED:
+        return;
+    case QEMU_PSCI_CONDUIT_HVC:
+        psci_method = "hvc";
+        break;
+    case QEMU_PSCI_CONDUIT_SMC:
+        psci_method = "smc";
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    qemu_fdt_add_subnode(fdt, "/psci");
+    if (armcpu->psci_version == 2) {
+        const char comp[] = "arm,psci-0.2\0arm,psci";
+        qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp));
+
+        cpu_off_fn = QEMU_PSCI_0_2_FN_CPU_OFF;
+        if (arm_feature(&armcpu->env, ARM_FEATURE_AARCH64)) {
+            cpu_suspend_fn = QEMU_PSCI_0_2_FN64_CPU_SUSPEND;
+            cpu_on_fn = QEMU_PSCI_0_2_FN64_CPU_ON;
+            migrate_fn = QEMU_PSCI_0_2_FN64_MIGRATE;
+        } else {
+            cpu_suspend_fn = QEMU_PSCI_0_2_FN_CPU_SUSPEND;
+            cpu_on_fn = QEMU_PSCI_0_2_FN_CPU_ON;
+            migrate_fn = QEMU_PSCI_0_2_FN_MIGRATE;
+        }
+    } else {
+        qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci");
+
+        cpu_suspend_fn = QEMU_PSCI_0_1_FN_CPU_SUSPEND;
+        cpu_off_fn = QEMU_PSCI_0_1_FN_CPU_OFF;
+        cpu_on_fn = QEMU_PSCI_0_1_FN_CPU_ON;
+        migrate_fn = QEMU_PSCI_0_1_FN_MIGRATE;
+    }
+
+    /* We adopt the PSCI spec's nomenclature, and use 'conduit' to refer
+     * to the instruction that should be used to invoke PSCI functions.
+     * However, the device tree binding uses 'method' instead, so that is
+     * what we should use here.
+     */
+    qemu_fdt_setprop_string(fdt, "/psci", "method", psci_method);
+
+    qemu_fdt_setprop_cell(fdt, "/psci", "cpu_suspend", cpu_suspend_fn);
+    qemu_fdt_setprop_cell(fdt, "/psci", "cpu_off", cpu_off_fn);
+    qemu_fdt_setprop_cell(fdt, "/psci", "cpu_on", cpu_on_fn);
+    qemu_fdt_setprop_cell(fdt, "/psci", "migrate", migrate_fn);
+}
+
 /**
  * load_dtb() - load a device tree binary image into memory
  * @addr:       the address to load the image at
@@ -541,6 +604,8 @@  static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
         }
     }
 
+    fdt_add_psci_node(fdt);
+
     if (binfo->modify_dtb) {
         binfo->modify_dtb(binfo, fdt);
     }
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b334c82eda..dbb3c8036a 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -244,66 +244,6 @@  static void create_fdt(VirtMachineState *vms)
     }
 }
 
-static void fdt_add_psci_node(const VirtMachineState *vms)
-{
-    uint32_t cpu_suspend_fn;
-    uint32_t cpu_off_fn;
-    uint32_t cpu_on_fn;
-    uint32_t migrate_fn;
-    void *fdt = vms->fdt;
-    ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(0));
-    const char *psci_method;
-
-    switch (vms->psci_conduit) {
-    case QEMU_PSCI_CONDUIT_DISABLED:
-        return;
-    case QEMU_PSCI_CONDUIT_HVC:
-        psci_method = "hvc";
-        break;
-    case QEMU_PSCI_CONDUIT_SMC:
-        psci_method = "smc";
-        break;
-    default:
-        g_assert_not_reached();
-    }
-
-    qemu_fdt_add_subnode(fdt, "/psci");
-    if (armcpu->psci_version == 2) {
-        const char comp[] = "arm,psci-0.2\0arm,psci";
-        qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp));
-
-        cpu_off_fn = QEMU_PSCI_0_2_FN_CPU_OFF;
-        if (arm_feature(&armcpu->env, ARM_FEATURE_AARCH64)) {
-            cpu_suspend_fn = QEMU_PSCI_0_2_FN64_CPU_SUSPEND;
-            cpu_on_fn = QEMU_PSCI_0_2_FN64_CPU_ON;
-            migrate_fn = QEMU_PSCI_0_2_FN64_MIGRATE;
-        } else {
-            cpu_suspend_fn = QEMU_PSCI_0_2_FN_CPU_SUSPEND;
-            cpu_on_fn = QEMU_PSCI_0_2_FN_CPU_ON;
-            migrate_fn = QEMU_PSCI_0_2_FN_MIGRATE;
-        }
-    } else {
-        qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci");
-
-        cpu_suspend_fn = QEMU_PSCI_0_1_FN_CPU_SUSPEND;
-        cpu_off_fn = QEMU_PSCI_0_1_FN_CPU_OFF;
-        cpu_on_fn = QEMU_PSCI_0_1_FN_CPU_ON;
-        migrate_fn = QEMU_PSCI_0_1_FN_MIGRATE;
-    }
-
-    /* We adopt the PSCI spec's nomenclature, and use 'conduit' to refer
-     * to the instruction that should be used to invoke PSCI functions.
-     * However, the device tree binding uses 'method' instead, so that is
-     * what we should use here.
-     */
-    qemu_fdt_setprop_string(fdt, "/psci", "method", psci_method);
-
-    qemu_fdt_setprop_cell(fdt, "/psci", "cpu_suspend", cpu_suspend_fn);
-    qemu_fdt_setprop_cell(fdt, "/psci", "cpu_off", cpu_off_fn);
-    qemu_fdt_setprop_cell(fdt, "/psci", "cpu_on", cpu_on_fn);
-    qemu_fdt_setprop_cell(fdt, "/psci", "migrate", migrate_fn);
-}
-
 static void fdt_add_timer_nodes(const VirtMachineState *vms)
 {
     /* On real hardware these interrupts are level-triggered.
@@ -1409,7 +1349,6 @@  static void machvirt_init(MachineState *machine)
     }
     fdt_add_timer_nodes(vms);
     fdt_add_cpu_nodes(vms);
-    fdt_add_psci_node(vms);
 
     memory_region_allocate_system_memory(ram, NULL, "mach-virt.ram",
                                          machine->ram_size);