From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47342) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f0pVP-0002rU-8p for qemu-devel@nongnu.org; Tue, 27 Mar 2018 10:23:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1f0pVJ-0001r8-5g for qemu-devel@nongnu.org; Tue, 27 Mar 2018 10:23:19 -0400 Date: Tue, 27 Mar 2018 15:22:46 +0100 Message-ID: <86605hsimh.wl-marc.zyngier@arm.com> From: Marc Zyngier In-Reply-To: <20180209110314.11766-23-peter.maydell@linaro.org> References: <20180209110314.11766-23-peter.maydell@linaro.org> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PULL, 22/30] hw/arm: Move virt's PSCI DT fixup code to arm/boot.c List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell , Andrey Smirnov Cc: Jason Wang , Philippe =?ISO-8859-1?Q?Mathieu-Daud=E9?= , Marcel Apfelbaum , "Michael S. Tsirkin" , qemu-devel@nongnu.org, qemu-arm@nongnu.org, yurovsky@gmail.com On Fri, 09 Feb 2018 11:03:06 +0000, Peter Maydell wrote: Hi Peter, Andrey, >=20 > From: Andrey Smirnov >=20 > 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. >=20 > Cc: Peter Maydell > Cc: Jason Wang > Cc: Philippe Mathieu-Daud=E9 > Cc: Marcel Apfelbaum > Cc: Michael S. Tsirkin > Cc: qemu-devel@nongnu.org > Cc: qemu-arm@nongnu.org > Cc: yurovsky@gmail.com > Reviewed-by: Peter Maydell > Signed-off-by: Andrey Smirnov > Reviewed-by: Philippe Mathieu-Daud=E9 > Signed-off-by: Peter Maydell > --- > hw/arm/boot.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++= ++++++ > hw/arm/virt.c | 61 -----------------------------------------------------= -- > 2 files changed, 65 insertions(+), 61 deletions(-) >=20 > 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_boo= t_info *info) > } > } > =20 > +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 =3D ARM_CPU(qemu_get_cpu(0)); > + const char *psci_method; > + int64_t psci_conduit; > + > + psci_conduit =3D 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 =3D "hvc"; > + break; > + case QEMU_PSCI_CONDUIT_SMC: > + psci_method =3D "smc"; > + break; > + default: > + g_assert_not_reached(); > + } > + > + qemu_fdt_add_subnode(fdt, "/psci"); > + if (armcpu->psci_version =3D=3D 2) { > + const char comp[] =3D "arm,psci-0.2\0arm,psci"; > + qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp)); > + > + cpu_off_fn =3D QEMU_PSCI_0_2_FN_CPU_OFF; > + if (arm_feature(&armcpu->env, ARM_FEATURE_AARCH64)) { > + cpu_suspend_fn =3D QEMU_PSCI_0_2_FN64_CPU_SUSPEND; > + cpu_on_fn =3D QEMU_PSCI_0_2_FN64_CPU_ON; > + migrate_fn =3D QEMU_PSCI_0_2_FN64_MIGRATE; > + } else { > + cpu_suspend_fn =3D QEMU_PSCI_0_2_FN_CPU_SUSPEND; > + cpu_on_fn =3D QEMU_PSCI_0_2_FN_CPU_ON; > + migrate_fn =3D QEMU_PSCI_0_2_FN_MIGRATE; > + } > + } else { > + qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci"); > + > + cpu_suspend_fn =3D QEMU_PSCI_0_1_FN_CPU_SUSPEND; > + cpu_off_fn =3D QEMU_PSCI_0_1_FN_CPU_OFF; > + cpu_on_fn =3D QEMU_PSCI_0_1_FN_CPU_ON; > + migrate_fn =3D 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_boo= t_info *binfo, > } > } > =20 > + 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. --=20 Jazz is not dead, it just smell funny.