[PATCH] arm64: KVM: unregister notifiers in hyp mode teardown path
Sudeep Holla
sudeep.holla at arm.com
Wed Apr 6 06:09:22 PDT 2016
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 at linaro.org>
>> Cc: Marc Zyngier <marc.zyngier at arm.com>
>> Signed-off-by: Sudeep Holla <sudeep.holla at arm.com>
>
> 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
More information about the linux-arm-kernel
mailing list