From nobody Tue Feb 10 01:15:32 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=linaro.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1551352824723686.5347513619282; Thu, 28 Feb 2019 03:20:24 -0800 (PST) Received: from localhost ([127.0.0.1]:36294 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gzJjh-0001Do-HQ for importer@patchew.org; Thu, 28 Feb 2019 06:20:21 -0500 Received: from eggs.gnu.org ([209.51.188.92]:36443) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gzJZ0-0001QO-CW for qemu-devel@nongnu.org; Thu, 28 Feb 2019 06:09:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gzJYz-00040R-5Q for qemu-devel@nongnu.org; Thu, 28 Feb 2019 06:09:18 -0500 Received: from mail-wm1-x344.google.com ([2a00:1450:4864:20::344]:54085) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gzJYy-0003zL-Ud for qemu-devel@nongnu.org; Thu, 28 Feb 2019 06:09:17 -0500 Received: by mail-wm1-x344.google.com with SMTP id e74so8740026wmg.3 for ; Thu, 28 Feb 2019 03:09:16 -0800 (PST) Received: from orth.archaic.org.uk (orth.archaic.org.uk. [81.2.115.148]) by smtp.gmail.com with ESMTPSA id c17sm13153241wrs.17.2019.02.28.03.09.11 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 28 Feb 2019 03:09:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:subject:date:message-id:in-reply-to:references:mime-version :content-transfer-encoding; bh=mf7V9uU9ymuIqkyXpzwqWM2jTKYUDZpzMgmpHzsl8MQ=; b=xfaAzeHSz/YRXFEwyymlRt80ogHo07YeTp3rNIxEEg+gJbbQQEfjCp8nOMDmlIUvNc ED8yD4y6409YxpqlBDjoeQ3LZV8hYk41QggoooTqQyFN9PXF2Q65C8iFmVFqns9JAZ6U UeCHTR4oiUevKY2AFjEFUZ4KT2qm2HAzvW+KGjj2OC7dev6jiyFd2TlJzEOU7iW/exor FOl3Fl8rbil3ai8NDPJXdrf6weE03+xcjgZ77DEBp+kmf/72eSM/rNR4ejeXNt/xhAqz LI55/n7Bq9udtwpkAXNTtS4xzMKaIl9+NThycfet54P8tI4OBAGOJveqyEGcg/Zfh+H3 wNBg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=mf7V9uU9ymuIqkyXpzwqWM2jTKYUDZpzMgmpHzsl8MQ=; b=n6guZ/v/1twhDDBu4LAZlF9OSYrE2KPVb66gTIj6D2/j+e5Vq1HzpROlKEvGKhBSIk rWhD58h+f1/I4cWTVEJzUuxFMJM/iZqEoC8jwqUuKO6KRoZ8egpaLYvzt3SX3wYmW2/J UMPZ8wcqxHVB1k2ROt/UtFsxi39kX/NN4b82gTSMBBmnSa1cs+nHmieDFz6amX7bPFFP 74kV+Ut3EhzDMTKoxat31dbwcPLL+oCSgxBbZO8ms4wBlxR9AdpTQHonY1brw306W9EG XbIkUs7xPVSJj4NTUrtawuhBgJPrJjRYpjINJ0i/TqaUNnQpYLX70/8Q54l/NbTYBmaR bCvw== X-Gm-Message-State: AHQUAuYTTB6ldmqpTjRIzvCdZk2HsmOBBp3to8tDJ4BERecyk00CjVNg SiCb7FgtL/5h8VPV/TRzkqjzeg7Ny7o= X-Google-Smtp-Source: APXvYqwYjLlUmUkG1jCY6HTejq+KwQYHM8YMWoY0RsSFCvRmTrI9EvT6RzWK4tvtfytTF1JoV7C4mQ== X-Received: by 2002:a1c:cf82:: with SMTP id f124mr2595447wmg.95.1551352155667; Thu, 28 Feb 2019 03:09:15 -0800 (PST) From: Peter Maydell To: qemu-devel@nongnu.org Date: Thu, 28 Feb 2019 11:08:30 +0000 Message-Id: <20190228110835.16159-12-peter.maydell@linaro.org> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190228110835.16159-1-peter.maydell@linaro.org> References: <20190228110835.16159-1-peter.maydell@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a00:1450:4864:20::344 Subject: [Qemu-devel] [PULL 11/16] Revert "arm: Allow system registers for KVM guests to be changed by QEMU code" X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) Content-Type: text/plain; charset="utf-8" This reverts commit 823e1b3818f9b10b824ddcd756983b6e2fa68730, which introduces a regression running EDK2 guest firmware under KVM: error: kvm run failed Function not implemented PC=3D000000013f5a6208 X00=3D00000000404003c4 X01=3D000000000000003a X02=3D0000000000000000 X03=3D00000000404003c4 X04=3D0000000000000000 X05=3D0000000096000046 X06=3D000000013d2ef270 X07=3D000000013e3d1710 X08=3D09010755ffaf8ba8 X09=3Dffaf8b9cfeeb5468 X10=3Dfeeb546409010756 X11=3D09010757ffaf8b90 X12=3Dfeeb50680903068b X13=3D090306a1ffaf8bc0 X14=3D0000000000000000 X15=3D0000000000000000 X16=3D000000013f872da0 X17=3D00000000ffffa6ab X18=3D0000000000000000 X19=3D000000013f5a92d0 X20=3D000000013f5a7a78 X21=3D000000000000003a X22=3D000000013f5a7ab2 X23=3D000000013f5a92e8 X24=3D000000013f631090 X25=3D0000000000000010 X26=3D0000000000000100 X27=3D000000013f89501b X28=3D000000013e3d14e0 X29=3D000000013e3d12a0 X30=3D000000013f5a2518 SP=3D000000013b7be0b0 PSTATE=3D404003c4 -Z-- EL1t with [ 3507.926571] kvm [35042]: load/store instruction decoding not implemented in the host dmesg. Revert the change for the moment until we can investigate the cause of the regression. Reported-by: Eric Auger Signed-off-by: Peter Maydell --- target/arm/cpu.h | 9 +-------- target/arm/helper.c | 27 ++------------------------- target/arm/kvm32.c | 20 ++++++++++++++++++-- target/arm/kvm64.c | 2 -- target/arm/machine.c | 2 +- 5 files changed, 22 insertions(+), 38 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index ff9ba387b42..3e545a2b146 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -2558,25 +2558,18 @@ bool write_list_to_cpustate(ARMCPU *cpu); /** * write_cpustate_to_list: * @cpu: ARMCPU - * @kvm_sync: true if this is for syncing back to KVM * * For each register listed in the ARMCPU cpreg_indexes list, write * its value from the ARMCPUState structure into the cpreg_values list. * This is used to copy info from TCG's working data structures into * KVM or for outbound migration. * - * @kvm_sync is true if we are doing this in order to sync the - * register state back to KVM. In this case we will only update - * values in the list if the previous list->cpustate sync actually - * successfully wrote the CPU state. Otherwise we will keep the value - * that is in the list. - * * Returns: true if all register values were read correctly, * false if some register was unknown or could not be read. * Note that we do not stop early on failure -- we will attempt * reading all registers in the list. */ -bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync); +bool write_cpustate_to_list(ARMCPU *cpu); =20 #define ARM_CPUID_TI915T 0x54029152 #define ARM_CPUID_TI925T 0x54029252 diff --git a/target/arm/helper.c b/target/arm/helper.c index fbaa801ceaa..1fa282a7fc1 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -265,7 +265,7 @@ static bool raw_accessors_invalid(const ARMCPRegInfo *r= i) return true; } =20 -bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync) +bool write_cpustate_to_list(ARMCPU *cpu) { /* Write the coprocessor state from cpu->env to the (index,value) list= . */ int i; @@ -274,7 +274,6 @@ bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync) for (i =3D 0; i < cpu->cpreg_array_len; i++) { uint32_t regidx =3D kvm_to_cpreg_id(cpu->cpreg_indexes[i]); const ARMCPRegInfo *ri; - uint64_t newval; =20 ri =3D get_arm_cp_reginfo(cpu->cp_regs, regidx); if (!ri) { @@ -284,29 +283,7 @@ bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync) if (ri->type & ARM_CP_NO_RAW) { continue; } - - newval =3D read_raw_cp_reg(&cpu->env, ri); - if (kvm_sync) { - /* - * Only sync if the previous list->cpustate sync succeeded. - * Rather than tracking the success/failure state for every - * item in the list, we just recheck "does the raw write we mu= st - * have made in write_list_to_cpustate() read back OK" here. - */ - uint64_t oldval =3D cpu->cpreg_values[i]; - - if (oldval =3D=3D newval) { - continue; - } - - write_raw_cp_reg(&cpu->env, ri, oldval); - if (read_raw_cp_reg(&cpu->env, ri) !=3D oldval) { - continue; - } - - write_raw_cp_reg(&cpu->env, ri, newval); - } - cpu->cpreg_values[i] =3D newval; + cpu->cpreg_values[i] =3D read_raw_cp_reg(&cpu->env, ri); } return ok; } diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c index 327375f6252..50327989dcc 100644 --- a/target/arm/kvm32.c +++ b/target/arm/kvm32.c @@ -384,8 +384,24 @@ int kvm_arch_put_registers(CPUState *cs, int level) return ret; } =20 - write_cpustate_to_list(cpu, true); - + /* Note that we do not call write_cpustate_to_list() + * here, so we are only writing the tuple list back to + * KVM. This is safe because nothing can change the + * CPUARMState cp15 fields (in particular gdb accesses cannot) + * and so there are no changes to sync. In fact syncing would + * be wrong at this point: for a constant register where TCG and + * KVM disagree about its value, the preceding write_list_to_cpustate() + * would not have had any effect on the CPUARMState value (since the + * register is read-only), and a write_cpustate_to_list() here would + * then try to write the TCG value back into KVM -- this would either + * fail or incorrectly change the value the guest sees. + * + * If we ever want to allow the user to modify cp15 registers via + * the gdb stub, we would need to be more clever here (for instance + * tracking the set of registers kvm_arch_get_registers() successfully + * managed to update the CPUARMState with, and only allowing those + * to be written back up into the kernel). + */ if (!write_list_to_kvmstate(cpu, level)) { return EINVAL; } diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index e3ba1492482..089af9c5f02 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -838,8 +838,6 @@ int kvm_arch_put_registers(CPUState *cs, int level) return ret; } =20 - write_cpustate_to_list(cpu, true); - if (!write_list_to_kvmstate(cpu, level)) { return EINVAL; } diff --git a/target/arm/machine.c b/target/arm/machine.c index 124192bfc26..b2925496148 100644 --- a/target/arm/machine.c +++ b/target/arm/machine.c @@ -630,7 +630,7 @@ static int cpu_pre_save(void *opaque) abort(); } } else { - if (!write_cpustate_to_list(cpu, false)) { + if (!write_cpustate_to_list(cpu)) { /* This should never fail. */ abort(); } --=20 2.20.1