On Fri, May 09, 2025 at 10:08:26AM +0800, liu.xuem...@zte.com.cn wrote:
> From: Xuemei Liu <liu.xuem...@zte.com.cn>
>
> Address an error in migration by discarding 'riscv_aplic' and 'riscv_imsic'
> in vmstate_register_with_alias_id() when aia is configured as
> 'aplic-imsic' in riscv kvm vm.
>
> Previously, the fields in the vmsds of 'riscv_aplic' and 'riscv_imsic' can
> only be initialized under certain special conditions in commit 95a97b3fd2.
> However, the corresponding ses of these vmsds are inserted into the
> savevm_state.handlers unconditionally. This led to migration failure
> characterized by uninitialized fields when save vm state:
> qemu-system-riscv64: ../migration/vmstate.c:433: vmstate_save_state_v:
> Assertion 'first_elem || !n_elems || !size' failed.
>
> Fixes: 95a97b3fd2 ("target/riscv: update APLIC and IMSIC to support KVM AIA")
> Signed-off-by: Xuemei Liu <liu.xuem...@zte.com.cn>
> ---
> migration/savevm.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 0c12e373b4..db53ce6d55 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -71,6 +71,8 @@
> #include "yank_functions.h"
> #include "system/qtest.h"
> #include "options.h"
> +#include "hw/intc/riscv_aplic.h"
> +#include "system/kvm.h"
>
> const unsigned int postcopy_ram_discard_version;
>
> @@ -902,6 +904,21 @@ int vmstate_register_with_alias_id(VMStateIf *obj,
> uint32_t instance_id,
> {
> SaveStateEntry *se;
>
> + /* Drop "riscv_aplic" if emulated aplic is used*/
> + if (!g_strcmp0(vmsd->name, "riscv_aplic")) {
> + RISCVAPLICState *aplic = opaque;
> + if (!riscv_use_emulated_aplic(aplic->msimode)) {
> + return 0;
> + }
> + }
> +
> + /* Drop "riscv_imsic" if kvm_kernel_irqchip is true */
> + if (!g_strcmp0(vmsd->name, "riscv_imsic")) {
> + if (kvm_irqchip_in_kernel()) {
> + return 0;
> + }
> + }
I'm not yet sure what's the best for the issue you're working on, but this
doesn't look right to change migration core for a RISCV fixes..
Maybe you want one VMStateDescription.needed()?
> +
> /* If this triggers, alias support can be dropped for the vmsd. */
> assert(alias_id == -1 || required_for_version >= vmsd->minimum_version_id);
>
> --
> 2.27.0
>
--
Peter Xu
