On 9/14/20 2:00 AM, Zelin Deng wrote:
> pvclock data pointers of vCPUs >= HVC_BOOT_ARRAY_SIZE (64) are stored in
> hvclock_mem wihch is initialized in kvmclock_init_mem().
> Here're 3 scenarios in current implementation:
> - no-kvmclock is set in cmdline. kvm pv clock driver is disabled,
> no impact.
> - no-kvmclock-vsyscall is set in cmdline. kvmclock_init_mem() won't
> be called. No memory for storing pvclock data of vCPUs >= 64, vCPUs
> >= 64 can not be online or hotpluged.
> - tsc unstable. kvmclock_init_mem() won't be called. vCPUs >= 64 can
> not be online or hotpluged.
> It's not reasonable that vCPUs hotplug have been impacted by last 2
> scenarios. Hence move kvmclock_init_mem() to front, in case hvclock_mem
> can not be initialized unexpectedly.
>
> Fixes: 6a1cac56f41f9 (x86/kvm: Use __bss_decrypted attribute in shared variables)
> Cc: <stable@xxxxxxxxxxxxxxx>
> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Cc: Brijesh Singh <brijesh.singh@xxxxxxx>
> Cc: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> Signed-off-by: Zelin Deng <zelin.deng@xxxxxxxxxxxxxxxxx>
> ---
> arch/x86/kernel/kvmclock.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> index 34b18f6eeb2c..1abbda25e037 100644
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
> @@ -271,7 +271,14 @@ static int __init kvm_setup_vsyscall_timeinfo(void)
> {
> #ifdef CONFIG_X86_64
> u8 flags;
> +#endif
> +
> + if (!kvmclock)
> + return 0;
Overall, I agree with the fix to move the kvmclock_init_mem() in the
beginning of the function so that memory hvclock_mem is allocated. But
curious, why do we need this check? The if (kvmclock) did not exist in
original function and I don't think kvmclock_init_mem() has any
dependency with it, am I missing something ?
> +
> + kvmclock_init_mem();
>
> +#ifdef CONFIG_X86_64
> if (!per_cpu(hv_clock_per_cpu, 0) || !kvmclock_vsyscall)
> return 0;
>
> @@ -282,8 +289,6 @@ static int __init kvm_setup_vsyscall_timeinfo(void)
> kvm_clock.vdso_clock_mode = VDSO_CLOCKMODE_PVCLOCK;
> #endif
>
> - kvmclock_init_mem();
> -
> return 0;
> }
> early_initcall(kvm_setup_vsyscall_timeinfo);
