Hi Christoffer, On 06/04/16 12:52, Christoffer Dall wrote:
Hi Sudeep, On Mon, Apr 04, 2016 at 02:46:51PM +0100, Sudeep Holla wrote:Commit 1e947bad0b63 ("arm64: KVM: Skip HYP setup when already running in HYP") re-organized the hyp init code and ended up leaving the CPU hotplug and PM notifier even if hyp mode initialization fails. Since KVM is not yet supported with ACPI, the above mentioned commit breaks CPU hotplug in ACPI boot. This patch fixes teardown_hyp_mode to properly unregister both CPU hotplug and PM notifiers in the teardown path. Fixes: 1e947bad0b63 ("arm64: KVM: Skip HYP setup when already running in HYP") Cc: Christoffer Dall <christoffer.dall@xxxxxxxxxx> Cc: Marc Zyngier <marc.zyngier@xxxxxxx> Signed-off-by: Sudeep Holla <sudeep.holla@xxxxxxx>I fixed up your patch to apply after James' patch: 5f5560b (arm64: KVM: Register CPU notifiers when the kernel runs at HYP, 2016-03-30)
Thanks for that, sorry I didn't realize it would conflict with that change.
My only concern with this approach is that we're not checking the return values from the cpu_pm_register_notifier calls, and we're potentially calling unregister_cpu_notifier even if the original registration failed.
I agree with your concern and I had the same when I first wrote the patch. But considering the return values makes it unnecessarily ugly, so I dropped it and kept it simple.
I know this can't happen given current implementations, but if any of these functions ever start returning error values, then we're silently ignoring them. What is our policy on these things?
I am fine to handle that, but as you mentioned it's not really needed. May be we can add some error message if that's really required.
Let me know if the following revised version of your patch looks ok to you (against kvmarm/master):
Looks fine and tested kvmarm/master with this patch on top. -- Regards, Sudeep _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
