From: Dexuan Cui <[email protected]>  Sent: Monday, July 8, 2019 10:29 PM
> 
> There is only one functional change: the unnecessary check
> "if (sctrl.enable != 1) return -EFAULT;" is removed, because when we're in
> hv_synic_cleanup(), we're absolutely sure sctrl.enable must be 1.
> 
> The new functions hv_synic_disable/enable_regs() will be used by a later patch
> to support hibernation.

Seems like this commit message doesn't really describe the main change.
How about:

Break out synic enable and disable operations into separate
hv_synic_disable_regs() and hv_synic_enable_regs() functions for use by a
later patch to support hibernation.

There is no functional change except the unnecessary check
"if (sctrl.enable != 1) return -EFAULT;" is removed, because when we're in
hv_synic_cleanup(), we're absolutely sure sctrl.enable must be 1.

Otherwise,

Reviewed-by:  Michael Kelley <[email protected]>

> 
> Signed-off-by: Dexuan Cui <[email protected]>
> ---
>  drivers/hv/hv.c           | 66 
> ++++++++++++++++++++++++++---------------------
>  drivers/hv/hyperv_vmbus.h |  2 ++
>  2 files changed, 39 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 6188fb7..fcc5279 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -154,7 +154,7 @@ void hv_synic_free(void)
>   * retrieve the initialized message and event pages.  Otherwise, we create 
> and
>   * initialize the message and event pages.
>   */
> -int hv_synic_init(unsigned int cpu)
> +void hv_synic_enable_regs(unsigned int cpu)
>  {
>       struct hv_per_cpu_context *hv_cpu
>               = per_cpu_ptr(hv_context.cpu_context, cpu);
> @@ -196,6 +196,11 @@ int hv_synic_init(unsigned int cpu)
>       sctrl.enable = 1;
> 
>       hv_set_synic_state(sctrl.as_uint64);
> +}
> +
> +int hv_synic_init(unsigned int cpu)
> +{
> +     hv_synic_enable_regs(cpu);
> 
>       hv_stimer_init(cpu);
> 
> @@ -205,20 +210,45 @@ int hv_synic_init(unsigned int cpu)
>  /*
>   * hv_synic_cleanup - Cleanup routine for hv_synic_init().
>   */
> -int hv_synic_cleanup(unsigned int cpu)
> +void hv_synic_disable_regs(unsigned int cpu)
>  {
>       union hv_synic_sint shared_sint;
>       union hv_synic_simp simp;
>       union hv_synic_siefp siefp;
>       union hv_synic_scontrol sctrl;
> +
> +     hv_get_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
> +
> +     shared_sint.masked = 1;
> +
> +     /* Need to correctly cleanup in the case of SMP!!! */
> +     /* Disable the interrupt */
> +     hv_set_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
> +
> +     hv_get_simp(simp.as_uint64);
> +     simp.simp_enabled = 0;
> +     simp.base_simp_gpa = 0;
> +
> +     hv_set_simp(simp.as_uint64);
> +
> +     hv_get_siefp(siefp.as_uint64);
> +     siefp.siefp_enabled = 0;
> +     siefp.base_siefp_gpa = 0;
> +
> +     hv_set_siefp(siefp.as_uint64);
> +
> +     /* Disable the global synic bit */
> +     hv_get_synic_state(sctrl.as_uint64);
> +     sctrl.enable = 0;
> +     hv_set_synic_state(sctrl.as_uint64);
> +}
> +
> +int hv_synic_cleanup(unsigned int cpu)
> +{
>       struct vmbus_channel *channel, *sc;
>       bool channel_found = false;
>       unsigned long flags;
> 
> -     hv_get_synic_state(sctrl.as_uint64);
> -     if (sctrl.enable != 1)
> -             return -EFAULT;
> -
>       /*
>        * Search for channels which are bound to the CPU we're about to
>        * cleanup. In case we find one and vmbus is still connected we need to
> @@ -249,29 +279,7 @@ int hv_synic_cleanup(unsigned int cpu)
> 
>       hv_stimer_cleanup(cpu);
> 
> -     hv_get_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
> -
> -     shared_sint.masked = 1;
> -
> -     /* Need to correctly cleanup in the case of SMP!!! */
> -     /* Disable the interrupt */
> -     hv_set_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
> -
> -     hv_get_simp(simp.as_uint64);
> -     simp.simp_enabled = 0;
> -     simp.base_simp_gpa = 0;
> -
> -     hv_set_simp(simp.as_uint64);
> -
> -     hv_get_siefp(siefp.as_uint64);
> -     siefp.siefp_enabled = 0;
> -     siefp.base_siefp_gpa = 0;
> -
> -     hv_set_siefp(siefp.as_uint64);
> -
> -     /* Disable the global synic bit */
> -     sctrl.enable = 0;
> -     hv_set_synic_state(sctrl.as_uint64);
> +     hv_synic_disable_regs(cpu);
> 
>       return 0;
>  }
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index 362e70e..26ea161 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -171,8 +171,10 @@ extern int hv_post_message(union hv_connection_id
> connection_id,
> 
>  extern void hv_synic_free(void);
> 
> +extern void hv_synic_enable_regs(unsigned int cpu);
>  extern int hv_synic_init(unsigned int cpu);
> 
> +extern void hv_synic_disable_regs(unsigned int cpu);
>  extern int hv_synic_cleanup(unsigned int cpu);
> 
>  /* Interface */
> --
> 1.8.3.1

Reply via email to