From: Dexuan Cui <decui@xxxxxxxxxxxxx> 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 <mikelley@xxxxxxxxxxxxx> > > Signed-off-by: Dexuan Cui <decui@xxxxxxxxxxxxx> > --- > 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