Re: [PATCH v2 01/11] xen/manage: keep track of the on-going suspend mode
From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Date: 2020-07-21 21:50:03
Also in:
linux-mm, linux-pm, lkml, xen-devel
quoted
quoted
quoted
quoted
quoted
+static int xen_setup_pm_notifier(void) +{ + if (!xen_hvm_domain()) + return -ENODEV; I forgot --- what did we decide about non-x86 (i.e. ARM)?It would be great to support that however, its out of scope for this patch set. I’ll be happy to discuss it separately.I wasn't implying that this *should* work on ARM but rather whether this will break ARM somehow (because xen_hvm_domain() is true there).Ok makes sense. TBH, I haven't tested this part of code on ARM and the series was only support x86 guests hibernation. Moreover, this notifier is there to distinguish between 2 PM events PM SUSPEND and PM hibernation. Now since we only care about PM HIBERNATION I may just remove this code and rely on "SHUTDOWN_SUSPEND" state. However, I may have to fix other patches in the series where this check may appear and cater it only for x86 right?I don't know what would happen if ARM guest tries to handle hibernation callbacks. The only ones that you are introducing are in block and net fronts and that's arch-independent. You do add a bunch of x86-specific code though (syscore ops), would something similar be needed for ARM?I don't expect this to work out of the box on ARM. To start with something similar will be needed for ARM too. We may still want to keep the driver code as-is. I understand the concern here wrt ARM, however, currently the support is only proposed for x86 guests here and similar work could be carried out for ARM. Also, if regular hibernation works correctly on arm, then all is needed is to fix Xen side of things. I am not sure what could be done to achieve any assurances on arm side as far as this series is concerned.
If you are not sure what the effects are (or sure that it won't work) on ARM then I'd add IS_ENABLED(CONFIG_X86) check, i.e. if (!IS_ENABLED(CONFIG_X86) || !xen_hvm_domain()) return -ENODEV; (plus '|| xen_initial_domain()' for PVH dom0 as Roger suggested.) -boris