From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 3A667C433EF for ; Mon, 7 Feb 2022 14:34:30 +0000 (UTC) Received: from localhost ([::1]:32898 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nH565-0007TZ-Bd for qemu-devel@archiver.kernel.org; Mon, 07 Feb 2022 09:34:29 -0500 Received: from eggs.gnu.org ([209.51.188.92]:57416) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nH4tY-00016r-5F; Mon, 07 Feb 2022 09:21:32 -0500 Received: from mail.csgraf.de ([85.25.223.15]:33676 helo=zulu616.server4you.de) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nH4tT-0005Tm-AV; Mon, 07 Feb 2022 09:21:29 -0500 Received: from [0.0.0.0] (ec2-3-122-114-9.eu-central-1.compute.amazonaws.com [3.122.114.9]) by csgraf.de (Postfix) with ESMTPSA id 5EB826080AD5; Mon, 7 Feb 2022 15:21:17 +0100 (CET) Message-ID: <2c67bde5-65b2-0af0-afde-7353a4fe2a1b@csgraf.de> Date: Mon, 7 Feb 2022 15:21:16 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.5.1 Subject: Re: [PATCH 06/16] hw/arm/xlnx-zcu102: Don't enable PSCI conduit when booting guest in EL3 To: Peter Maydell , qemu-arm@nongnu.org, qemu-devel@nongnu.org References: <20220127154639.2090164-1-peter.maydell@linaro.org> <20220127154639.2090164-7-peter.maydell@linaro.org> From: Alexander Graf In-Reply-To: <20220127154639.2090164-7-peter.maydell@linaro.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Received-SPF: pass client-ip=85.25.223.15; envelope-from=agraf@csgraf.de; helo=zulu616.server4you.de X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, NICE_REPLY_A=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Eduardo Habkost , Beniamino Galvani , Alistair Francis , Rob Herring , Andrew Jeffery , Andre Przywara , Tyrone Ting , =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , Jean-Christophe Dubois , Yanan Wang , Igor Mitsyanko , Niek Linnenbank , sstabellini@kernel.org, =?UTF-8?Q?C=c3=a9dric_Le_Goater?= , "Edgar E. Iglesias" , Havard Skinnemoen , Andrey Smirnov , michal.simek@xilinx.com, Joel Stanley Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On 27.01.22 16:46, Peter Maydell wrote: > Change the Xilinx ZynqMP-based board xlnx-zcu102 to use the new > boot.c functionality to allow us to enable psci-conduit only if > the guest is being booted in EL1 or EL2, so that if the user runs > guest EL3 firmware code our PSCI emulation doesn't get in its > way. > > To do this we stop setting the psci-conduit property on the CPU > objects in the SoC code, and instead set the psci_conduit field in > the arm_boot_info struct to tell the common boot loader code that > we'd like PSCI if the guest is starting at an EL that it makes > sense with. > > Note that this means that EL3 guest code will have no way > to power on secondary cores, because we don't model any > kind of power controller that does that on this SoC. > > Signed-off-by: Peter Maydell It's been a while since I worked with ZynqMP, but typically your ATF in EL3 will want to talk to a microblaze firmware blob on the PMU. I only see a stand alone PMU machine for microblaze and a PMU IRQ handling I/O block in QEMU, but nothing that would listen to the events. So I'm fairly sure it will be broken after this patch - and really only worked by accident before. I've added Michal Simek and Stefano Stabellini (both Xilinx) to CC to clarify and determine the best path forward here - either disallow EL3 in the model or build proper PMU emulation in QEMU which then handles those PSCI triggered IPI events. Alex [1] https://github.com/Xilinx/arm-trusted-firmware/blob/master/plat/xilinx/zynqmp/plat_psci.c > --- > Again, if anybody knows the real-hardware EL3 behaviour for > CPUs that would be great. > --- > hw/arm/xlnx-zcu102.c | 1 + > hw/arm/xlnx-zynqmp.c | 13 ++++++++----- > 2 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c > index 45eb19ab3b7..4c84bb932aa 100644 > --- a/hw/arm/xlnx-zcu102.c > +++ b/hw/arm/xlnx-zcu102.c > @@ -236,6 +236,7 @@ static void xlnx_zcu102_init(MachineState *machine) > s->binfo.ram_size = ram_size; > s->binfo.loader_start = 0; > s->binfo.modify_dtb = zcu102_modify_dtb; > + s->binfo.psci_conduit = QEMU_PSCI_CONDUIT_SMC; > arm_load_kernel(s->soc.boot_cpu_ptr, machine, &s->binfo); > } > > diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c > index 1c52a575aad..17305fe7b76 100644 > --- a/hw/arm/xlnx-zynqmp.c > +++ b/hw/arm/xlnx-zynqmp.c > @@ -215,7 +215,10 @@ static void xlnx_zynqmp_create_rpu(MachineState *ms, XlnxZynqMPState *s, > > name = object_get_canonical_path_component(OBJECT(&s->rpu_cpu[i])); > if (strcmp(name, boot_cpu)) { > - /* Secondary CPUs start in PSCI powered-down state */ > + /* > + * Secondary CPUs start in powered-down state. > + * TODO: check this is what EL3 firmware expects. > + */ > object_property_set_bool(OBJECT(&s->rpu_cpu[i]), > "start-powered-off", true, &error_abort); > } else { > @@ -435,12 +438,12 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) > for (i = 0; i < num_apus; i++) { > const char *name; > > - object_property_set_int(OBJECT(&s->apu_cpu[i]), "psci-conduit", > - QEMU_PSCI_CONDUIT_SMC, &error_abort); > - > name = object_get_canonical_path_component(OBJECT(&s->apu_cpu[i])); > if (strcmp(name, boot_cpu)) { > - /* Secondary CPUs start in PSCI powered-down state */ > + /* > + * Secondary CPUs start in powered-down state. > + * TODO: check this is what EL3 firmware expects. > + */ > object_property_set_bool(OBJECT(&s->apu_cpu[i]), > "start-powered-off", true, &error_abort); > } else {