[PATCH] hw/intc/xics: Add missing call to register vmstate_icp_server

Fabian Vogt posted 1 patch 5 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/6187781.lOV4Wx5bFT@fvogt-thinkpad
Maintainers: Nicholas Piggin <npiggin@gmail.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>
hw/intc/xics.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH] hw/intc/xics: Add missing call to register vmstate_icp_server
Posted by Fabian Vogt 5 months, 3 weeks ago
An obsolete wrapper function with a workaround was removed entirely,
without restoring the call it wrapped.

Without this, the guest is stuck after savevm/loadvm.

Fixes: 24ee9229fe31 ("ppc/spapr: remove deprecated machine pseries-2.9")
Signed-off-by: Fabian Vogt <fvogt@suse.de>
---
 hw/intc/xics.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index d9a199e883..200710eb6c 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -335,6 +335,8 @@ static void icp_realize(DeviceState *dev, Error **errp)
             return;
         }
     }
+
+    vmstate_register(NULL, icp->cs->cpu_index, &vmstate_icp_server, icp);
 }
 
 static void icp_unrealize(DeviceState *dev)
-- 
2.50.1
Re: [PATCH] hw/intc/xics: Add missing call to register vmstate_icp_server
Posted by Fabiano Rosas 5 months, 3 weeks ago
Fabian Vogt <fvogt@suse.de> writes:

+CC Peter

> An obsolete wrapper function with a workaround was removed entirely,
> without restoring the call it wrapped.
>
> Without this, the guest is stuck after savevm/loadvm.
>

I'd note that this only affects guests that have actually negotiated
XICS, so guests using ic-mode=xive are safe.

With this patch, backward migration to QEMU versions >= 9.2 is
broken. Which I can only assume is ok, since the hard break (vmstate
gets rejected) is better than weird post-migration hangs.

If the PPC folks can spot a scenario where migration would provide
reasonable results even without the emulated interrupt controller state
being migrated, then we'll need a compat property for this.

Also, a 9.2 guest cannot move to a more recent QEMU version without a
reboot. If this is an important scenario, then the new QEMU needs to
provide some compatibility knob.

Otherwise:

Acked-by: Fabiano Rosas <farosas@suse.de>
Cc: qemu-stable@nongnu.org
Re: [PATCH] hw/intc/xics: Add missing call to register vmstate_icp_server
Posted by Harsh Prateek Bora 5 months, 3 weeks ago
+ Gautam, Glenn

Hi Fabian,


On 8/19/25 08:25, Fabiano Rosas wrote:
> Fabian Vogt <fvogt@suse.de> writes:
> 
> +CC Peter
> 
>> An obsolete wrapper function with a workaround was removed entirely,
>> without restoring the call it wrapped.
>>
>> Without this, the guest is stuck after savevm/loadvm.
>>
> 
> I'd note that this only affects guests that have actually negotiated
> XICS, so guests using ic-mode=xive are safe.

Since this affected only guests using XICS, would it be a better idea to 
mention this explicitly in commit log as well (?

> 
> With this patch, backward migration to QEMU versions >= 9.2 is
> broken. Which I can only assume is ok, since the hard break (vmstate
> gets rejected) is better than weird post-migration hangs.
> 
> If the PPC folks can spot a scenario where migration would provide
> reasonable results even without the emulated interrupt controller state
> being migrated, then we'll need a compat property for this.
> 
> Also, a 9.2 guest cannot move to a more recent QEMU version without a
> reboot. If this is an important scenario, then the new QEMU needs to
> provide some compatibility knob.

Gautam, Glenn - thoughts on above comments?

Thanks for the patch.

regards,
Harsh

> 
> Otherwise:
> 
> Acked-by: Fabiano Rosas <farosas@suse.de>
> Cc: qemu-stable@nongnu.org
Re: [PATCH] hw/intc/xics: Add missing call to register vmstate_icp_server
Posted by Miles Glenn 5 months, 3 weeks ago
On Tue, 2025-08-19 at 12:59 +0530, Harsh Prateek Bora wrote:
> + Gautam, Glenn
> 
> Hi Fabian,
> 
> 
> On 8/19/25 08:25, Fabiano Rosas wrote:
> > Fabian Vogt <fvogt@suse.de> writes:
> > 
> > +CC Peter
> > 
> > > An obsolete wrapper function with a workaround was removed entirely,
> > > without restoring the call it wrapped.
> > > 
> > > Without this, the guest is stuck after savevm/loadvm.
> > > 
> > 
> > I'd note that this only affects guests that have actually negotiated
> > XICS, so guests using ic-mode=xive are safe.
> 
> Since this affected only guests using XICS, would it be a better idea to 
> mention this explicitly in commit log as well (?
> 
> > With this patch, backward migration to QEMU versions >= 9.2 is
> > broken. Which I can only assume is ok, since the hard break (vmstate
> > gets rejected) is better than weird post-migration hangs.
> > 
> > If the PPC folks can spot a scenario where migration would provide
> > reasonable results even without the emulated interrupt controller state
> > being migrated, then we'll need a compat property for this.
> > 

I don't know much about the XICS controller, but migration without
restoring the interrupt controller state seems pretty risky to me.  I
wouldn't recommend it.

Glenn

> > Also, a 9.2 guest cannot move to a more recent QEMU version without a
> > reboot. If this is an important scenario, then the new QEMU needs to
> > provide some compatibility knob.
> 
> Gautam, Glenn - thoughts on above comments?
> 
> Thanks for the patch.
> 
> regards,
> Harsh
> 
> > Otherwise:
> > 
> > Acked-by: Fabiano Rosas <farosas@suse.de>
> > Cc: qemu-stable@nongnu.org
Re: [PATCH-for-10.1] hw/intc/xics: Add missing call to register vmstate_icp_server
Posted by Philippe Mathieu-Daudé 5 months, 3 weeks ago
On 14/8/25 13:35, Fabian Vogt wrote:
> An obsolete wrapper function with a workaround was removed entirely,
> without restoring the call it wrapped.
> 
> Without this, the guest is stuck after savevm/loadvm.
> 
> Fixes: 24ee9229fe31 ("ppc/spapr: remove deprecated machine pseries-2.9")
> Signed-off-by: Fabian Vogt <fvogt@suse.de>
> ---
>   hw/intc/xics.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index d9a199e883..200710eb6c 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -335,6 +335,8 @@ static void icp_realize(DeviceState *dev, Error **errp)
>               return;
>           }
>       }
> +
> +    vmstate_register(NULL, icp->cs->cpu_index, &vmstate_icp_server, icp);
>   }
>   
>   static void icp_unrealize(DeviceState *dev)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>