* [PATCH v2] arm/kvm: report registers we failed to set
@ 2025-09-02 9:27 Cornelia Huck
2025-09-02 15:49 ` Sebastian Ott
2025-09-11 13:03 ` Peter Maydell
0 siblings, 2 replies; 4+ messages in thread
From: Cornelia Huck @ 2025-09-02 9:27 UTC (permalink / raw)
To: qemu-arm, qemu-devel
Cc: Peter Maydell, Eric Auger, Sebastian Ott, Cornelia Huck
If we fail migration because of a mismatch of some registers between
source and destination, the error message is not very informative:
qemu-system-aarch64: error while loading state for instance 0x0 ofdevice 'cpu'
qemu-system-aarch64: Failed to put registers after init: Invalid argument
At least try to give the user a hint which registers had a problem,
even if they cannot really do anything about it right now.
Sample output:
Could not set register op0:3 op1:0 crn:0 crm:0 op2:0 to c00fac31 (is 413fd0c1)
We could be even more helpful once we support writable ID registers,
at which point the user might actually be able to configure something
that is migratable.
Suggested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
Changes RFC->v2:
* cover different register types
* less macro magic
* less memory leaks
---
target/arm/kvm.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 86 insertions(+)
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 667234485547..0423d4df7c06 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -900,6 +900,58 @@ bool write_kvmstate_to_list(ARMCPU *cpu)
return ok;
}
+/* pretty-print a KVM register */
+#define CP_REG_ARM64_SYSREG_OP(_reg, _op) \
+ ((uint8_t)((_reg & CP_REG_ARM64_SYSREG_ ## _op ## _MASK) >> \
+ CP_REG_ARM64_SYSREG_ ## _op ## _SHIFT))
+
+static gchar *kvm_print_sve_register_name(uint64_t regidx)
+{
+ uint16_t sve_reg = regidx & 0x000000000000ffff;
+
+ if (regidx == KVM_REG_ARM64_SVE_VLS) {
+ return g_strdup_printf("SVE VLS");
+ }
+ /* zreg, preg, ffr */
+ switch (sve_reg & 0xfc00) {
+ case 0:
+ return g_strdup_printf("SVE zreg n:%d slice:%d",
+ (sve_reg & 0x03e0) >> 5, sve_reg & 0x001f);
+ case 0x04:
+ return g_strdup_printf("SVE preg n:%d slice:%d",
+ (sve_reg & 0x01e0) >> 5, sve_reg & 0x001f);
+ case 0x06:
+ return g_strdup_printf("SVE ffr slice:%d", sve_reg & 0x001f);
+ default:
+ return g_strdup_printf("SVE ???");
+ }
+}
+
+static gchar *kvm_print_register_name(uint64_t regidx)
+{
+ switch ((regidx & KVM_REG_ARM_COPROC_MASK)) {
+ case KVM_REG_ARM_CORE:
+ return g_strdup_printf("core reg %lx", regidx);
+ case KVM_REG_ARM_DEMUX:
+ return g_strdup_printf("demuxed reg %lx", regidx);
+ case KVM_REG_ARM64_SYSREG:
+ return g_strdup_printf("op0:%d op1:%d crn:%d crm:%d op2:%d",
+ CP_REG_ARM64_SYSREG_OP(regidx, OP0),
+ CP_REG_ARM64_SYSREG_OP(regidx, OP1),
+ CP_REG_ARM64_SYSREG_OP(regidx, CRN),
+ CP_REG_ARM64_SYSREG_OP(regidx, CRM),
+ CP_REG_ARM64_SYSREG_OP(regidx, OP2));
+ case KVM_REG_ARM_FW:
+ return g_strdup_printf("fw reg %d", (int)(regidx & 0xffff));
+ case KVM_REG_ARM64_SVE:
+ return kvm_print_sve_register_name(regidx);
+ case KVM_REG_ARM_FW_FEAT_BMAP:
+ return g_strdup_printf("fw feat reg %d", (int)(regidx & 0xffff));
+ default:
+ return g_strdup_printf("%lx", regidx);
+ }
+}
+
bool write_list_to_kvmstate(ARMCPU *cpu, int level)
{
CPUState *cs = CPU(cpu);
@@ -927,11 +979,45 @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level)
g_assert_not_reached();
}
if (ret) {
+ gchar *reg_str = kvm_print_register_name(regidx);
+
/* We might fail for "unknown register" and also for
* "you tried to set a register which is constant with
* a different value from what it actually contains".
*/
ok = false;
+ switch (ret) {
+ case -ENOENT:
+ error_report("Could not set register %s: unknown to KVM",
+ reg_str);
+ break;
+ case -EINVAL:
+ if ((regidx & KVM_REG_SIZE_MASK) == KVM_REG_SIZE_U32) {
+ if (!kvm_get_one_reg(cs, regidx, &v32)) {
+ error_report("Could not set register %s to %x (is %x)",
+ reg_str, (uint32_t)cpu->cpreg_values[i],
+ v32);
+ } else {
+ error_report("Could not set register %s to %x",
+ reg_str, (uint32_t)cpu->cpreg_values[i]);
+ }
+ } else /* U64 */ {
+ uint64_t v64;
+
+ if (!kvm_get_one_reg(cs, regidx, &v64)) {
+ error_report("Could not set register %s to %lx (is %lx)",
+ reg_str, cpu->cpreg_values[i], v64);
+ } else {
+ error_report("Could not set register %s to %lx",
+ reg_str, cpu->cpreg_values[i]);
+ }
+ }
+ break;
+ default:
+ error_report("Could not set register %s: %s",
+ reg_str, strerror(-ret));
+ }
+ g_free(reg_str);
}
}
return ok;
--
2.50.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] arm/kvm: report registers we failed to set
2025-09-02 9:27 [PATCH v2] arm/kvm: report registers we failed to set Cornelia Huck
@ 2025-09-02 15:49 ` Sebastian Ott
2025-09-11 13:03 ` Peter Maydell
1 sibling, 0 replies; 4+ messages in thread
From: Sebastian Ott @ 2025-09-02 15:49 UTC (permalink / raw)
To: Cornelia Huck; +Cc: qemu-arm, qemu-devel, Peter Maydell, Eric Auger
On Tue, 2 Sep 2025, Cornelia Huck wrote:
> If we fail migration because of a mismatch of some registers between
> source and destination, the error message is not very informative:
>
> qemu-system-aarch64: error while loading state for instance 0x0 ofdevice 'cpu'
> qemu-system-aarch64: Failed to put registers after init: Invalid argument
>
> At least try to give the user a hint which registers had a problem,
> even if they cannot really do anything about it right now.
>
> Sample output:
>
> Could not set register op0:3 op1:0 crn:0 crm:0 op2:0 to c00fac31 (is 413fd0c1)
>
> We could be even more helpful once we support writable ID registers,
> at which point the user might actually be able to configure something
> that is migratable.
>
> Suggested-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
I did some basic tests - this one looks good!
Reviewed-by: Sebastian Ott <sebott@redhat.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] arm/kvm: report registers we failed to set
2025-09-02 9:27 [PATCH v2] arm/kvm: report registers we failed to set Cornelia Huck
2025-09-02 15:49 ` Sebastian Ott
@ 2025-09-11 13:03 ` Peter Maydell
2025-09-11 15:20 ` Cornelia Huck
1 sibling, 1 reply; 4+ messages in thread
From: Peter Maydell @ 2025-09-11 13:03 UTC (permalink / raw)
To: Cornelia Huck; +Cc: qemu-arm, qemu-devel, Eric Auger, Sebastian Ott
On Tue, 2 Sept 2025 at 10:27, Cornelia Huck <cohuck@redhat.com> wrote:
>
> If we fail migration because of a mismatch of some registers between
> source and destination, the error message is not very informative:
>
> qemu-system-aarch64: error while loading state for instance 0x0 ofdevice 'cpu'
> qemu-system-aarch64: Failed to put registers after init: Invalid argument
>
> At least try to give the user a hint which registers had a problem,
> even if they cannot really do anything about it right now.
>
> Sample output:
>
> Could not set register op0:3 op1:0 crn:0 crm:0 op2:0 to c00fac31 (is 413fd0c1)
>
> We could be even more helpful once we support writable ID registers,
> at which point the user might actually be able to configure something
> that is migratable.
>
> Suggested-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
> Changes RFC->v2:
> * cover different register types
> * less macro magic
> * less memory leaks
> ---
> target/arm/kvm.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 86 insertions(+)
>
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 667234485547..0423d4df7c06 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -900,6 +900,58 @@ bool write_kvmstate_to_list(ARMCPU *cpu)
> return ok;
> }
>
> +/* pretty-print a KVM register */
> +#define CP_REG_ARM64_SYSREG_OP(_reg, _op) \
> + ((uint8_t)((_reg & CP_REG_ARM64_SYSREG_ ## _op ## _MASK) >> \
> + CP_REG_ARM64_SYSREG_ ## _op ## _SHIFT))
> +
> +static gchar *kvm_print_sve_register_name(uint64_t regidx)
> +{
> + uint16_t sve_reg = regidx & 0x000000000000ffff;
> +
> + if (regidx == KVM_REG_ARM64_SVE_VLS) {
> + return g_strdup_printf("SVE VLS");
> + }
> + /* zreg, preg, ffr */
> + switch (sve_reg & 0xfc00) {
> + case 0:
> + return g_strdup_printf("SVE zreg n:%d slice:%d",
> + (sve_reg & 0x03e0) >> 5, sve_reg & 0x001f);
> + case 0x04:
> + return g_strdup_printf("SVE preg n:%d slice:%d",
> + (sve_reg & 0x01e0) >> 5, sve_reg & 0x001f);
> + case 0x06:
> + return g_strdup_printf("SVE ffr slice:%d", sve_reg & 0x001f);
> + default:
> + return g_strdup_printf("SVE ???");
> + }
> +}
> +
> +static gchar *kvm_print_register_name(uint64_t regidx)
> +{
> + switch ((regidx & KVM_REG_ARM_COPROC_MASK)) {
> + case KVM_REG_ARM_CORE:
> + return g_strdup_printf("core reg %lx", regidx);
> + case KVM_REG_ARM_DEMUX:
> + return g_strdup_printf("demuxed reg %lx", regidx);
You can't print a uint64_t with %lx. I suppose this code is
only going to get compiled on a 64-bit host but we might
someday want to move it so we can pretty-print registers
elsewhere (e.g. in cpu_post_load() where we fail for
"incoming migration stream has sysreg X but we don't know it").
> + case KVM_REG_ARM64_SYSREG:
> + return g_strdup_printf("op0:%d op1:%d crn:%d crm:%d op2:%d",
> + CP_REG_ARM64_SYSREG_OP(regidx, OP0),
> + CP_REG_ARM64_SYSREG_OP(regidx, OP1),
> + CP_REG_ARM64_SYSREG_OP(regidx, CRN),
> + CP_REG_ARM64_SYSREG_OP(regidx, CRM),
> + CP_REG_ARM64_SYSREG_OP(regidx, OP2));
> + case KVM_REG_ARM_FW:
> + return g_strdup_printf("fw reg %d", (int)(regidx & 0xffff));
> + case KVM_REG_ARM64_SVE:
> + return kvm_print_sve_register_name(regidx);
> + case KVM_REG_ARM_FW_FEAT_BMAP:
> + return g_strdup_printf("fw feat reg %d", (int)(regidx & 0xffff));
> + default:
> + return g_strdup_printf("%lx", regidx);
> + }
> +}
> +
> bool write_list_to_kvmstate(ARMCPU *cpu, int level)
> {
> CPUState *cs = CPU(cpu);
> @@ -927,11 +979,45 @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level)
> g_assert_not_reached();
> }
> if (ret) {
> + gchar *reg_str = kvm_print_register_name(regidx);
> +
> /* We might fail for "unknown register" and also for
> * "you tried to set a register which is constant with
> * a different value from what it actually contains".
> */
> ok = false;
> + switch (ret) {
> + case -ENOENT:
> + error_report("Could not set register %s: unknown to KVM",
> + reg_str);
> + break;
> + case -EINVAL:
> + if ((regidx & KVM_REG_SIZE_MASK) == KVM_REG_SIZE_U32) {
> + if (!kvm_get_one_reg(cs, regidx, &v32)) {
> + error_report("Could not set register %s to %x (is %x)",
> + reg_str, (uint32_t)cpu->cpreg_values[i],
> + v32);
> + } else {
> + error_report("Could not set register %s to %x",
> + reg_str, (uint32_t)cpu->cpreg_values[i]);
> + }
> + } else /* U64 */ {
> + uint64_t v64;
> +
> + if (!kvm_get_one_reg(cs, regidx, &v64)) {
> + error_report("Could not set register %s to %lx (is %lx)",
> + reg_str, cpu->cpreg_values[i], v64);
> + } else {
> + error_report("Could not set register %s to %lx",
> + reg_str, cpu->cpreg_values[i]);
Similarly we should be using PRIx64 here.
> + }
> + }
> + break;
> + default:
> + error_report("Could not set register %s: %s",
> + reg_str, strerror(-ret));
> + }
> + g_free(reg_str);
> }
> }
> return ok;
thanks
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] arm/kvm: report registers we failed to set
2025-09-11 13:03 ` Peter Maydell
@ 2025-09-11 15:20 ` Cornelia Huck
0 siblings, 0 replies; 4+ messages in thread
From: Cornelia Huck @ 2025-09-11 15:20 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, qemu-devel, Eric Auger, Sebastian Ott
On Thu, Sep 11 2025, Peter Maydell <peter.maydell@linaro.org> wrote:
> On Tue, 2 Sept 2025 at 10:27, Cornelia Huck <cohuck@redhat.com> wrote:
>> +static gchar *kvm_print_register_name(uint64_t regidx)
>> +{
>> + switch ((regidx & KVM_REG_ARM_COPROC_MASK)) {
>> + case KVM_REG_ARM_CORE:
>> + return g_strdup_printf("core reg %lx", regidx);
>> + case KVM_REG_ARM_DEMUX:
>> + return g_strdup_printf("demuxed reg %lx", regidx);
>
> You can't print a uint64_t with %lx. I suppose this code is
> only going to get compiled on a 64-bit host but we might
> someday want to move it so we can pretty-print registers
> elsewhere (e.g. in cpu_post_load() where we fail for
> "incoming migration stream has sysreg X but we don't know it").
OK, if we want to have this as some kind of generic function eventually,
it needs to become PRIx64.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-09-11 15:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-02 9:27 [PATCH v2] arm/kvm: report registers we failed to set Cornelia Huck
2025-09-02 15:49 ` Sebastian Ott
2025-09-11 13:03 ` Peter Maydell
2025-09-11 15:20 ` Cornelia Huck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).