Re: [PATCH 1/2] xenbus: Use .freeze/.thaw to handle xenbus devices

["Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>]

[-- Attachment #1: Type: text/plain, Size: 3099 bytes --]

On Wed, Nov 19, 2025 at 05:47:29PM -0500, Jason Andryuk wrote:
> The goal is to fix s2idle and S3 for Xen PV devices.  

Can you give a little more context of this? We do have working S3 in
qubes with no need for such change. We trigger it via the toolstack (libxl_domain_suspend_only()).
Are you talking about guest-initiated suspend here?

We also have kinda working (host) s2idle. You may want to take a look at this
work (some/most of it was posted upstream, but not all got
committed/reviewed):
https://github.com/QubesOS/qubes-issues/issues/6411#issuecomment-1538089344
https://github.com/QubesOS/qubes-linux-kernel/pull/910 (some patches
changed since that PR, see the current main too).

> A domain resuming
> from s3 or s2idle disconnects its PV devices during resume.  The
> backends are not expecting this and do not reconnect.
> 
> b3e96c0c7562 ("xen: use freeze/restore/thaw PM events for suspend/
> resume/chkpt") changed xen_suspend()/do_suspend() from
> PMSG_SUSPEND/PMSG_RESUME to PMSG_FREEZE/PMSG_THAW/PMSG_RESTORE, but the
> suspend/resume callbacks remained.
> 
> .freeze/restore are used with hiberation where Linux restarts in a new
> place in the future.  .suspend/resume are useful for runtime power
> management for the duration of a boot.
> 
> The current behavior of the callbacks works for an xl save/restore or
> live migration where the domain is restored/migrated to a new location
> and connecting to a not-already-connected backend.
> 
> Change xenbus_pm_ops to use .freeze/thaw/restore and drop the
> .suspend/resume hook.  This matches the use in drivers/xen/manage.c for
> save/restore and live migration.  With .suspend/resume empty, PV devices
> are left connected during s2idle and s3, so PV devices are not changed
> and work after resume.

Is that intended? While it might work for suspend by a chance(*), I'm
pretty sure not disconnecting + re-reconnecting PV devices across
save/restore/live migration will break them.

(*) and even that I'm not sure - with driver domains, depending on
suspend order this feels like might result in a deadlock...

> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> ---
>  drivers/xen/xenbus/xenbus_probe_frontend.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c
> index 6d1819269cbe..199917b6f77c 100644
> --- a/drivers/xen/xenbus/xenbus_probe_frontend.c
> +++ b/drivers/xen/xenbus/xenbus_probe_frontend.c
> @@ -148,11 +148,9 @@ static void xenbus_frontend_dev_shutdown(struct device *_dev)
>  }
>  
>  static const struct dev_pm_ops xenbus_pm_ops = {
> -	.suspend	= xenbus_dev_suspend,
> -	.resume		= xenbus_frontend_dev_resume,
>  	.freeze		= xenbus_dev_suspend,
>  	.thaw		= xenbus_dev_cancel,
> -	.restore	= xenbus_dev_resume,
> +	.restore	= xenbus_frontend_dev_resume,
>  };
>  
>  static struct xen_bus_type xenbus_frontend = {
> -- 
> 2.34.1
> 
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]