mhkelley58@xxxxxxxxx writes:
> From: Michael Kelley <mikelley@xxxxxxxxxxxxx>
>
> The synic_initialized flag is part of the global hv_context
> structure. But the Hyper-V synthetic interrupt controller is
> fundamentally a per-cpu device, and other synic related
> fields are in hv_per_cpu_context. In a multi-CPU system,
> synic_initialized gets set multiple times, making the test in
> hv_synic_cleanup() invalid. Fix this by moving the flag to
> hv_per_cpu_context and adjusting the references.
>
> Signed-off-by: Michael Kelley <mikelley@xxxxxxxxxxxxx>
> ---
> drivers/hv/hv.c | 16 +++++++---------
> drivers/hv/hyperv_vmbus.h | 4 ++--
> 2 files changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 312fe5e..8d4fe0e 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -33,9 +33,7 @@
> #include "hyperv_vmbus.h"
>
> /* The one and only */
> -struct hv_context hv_context = {
> - .synic_initialized = false,
> -};
> +struct hv_context hv_context;
>
> /*
> * If false, we're using the old mechanism for stimer0 interrupts
> @@ -315,7 +313,7 @@ int hv_synic_init(unsigned int cpu)
>
> hv_set_synic_state(sctrl.as_uint64);
>
> - hv_context.synic_initialized = true;
> + hv_cpu->synic_initialized = true;
>
> /*
> * Register the per-cpu clockevent source.
> @@ -354,6 +352,8 @@ void hv_synic_clockevents_cleanup(void)
> */
> int hv_synic_cleanup(unsigned int cpu)
> {
> + struct hv_per_cpu_context *hv_cpu
> + = per_cpu_ptr(hv_context.cpu_context, cpu);
> union hv_synic_sint shared_sint;
> union hv_synic_simp simp;
> union hv_synic_siefp siefp;
> @@ -362,7 +362,7 @@ int hv_synic_cleanup(unsigned int cpu)
> bool channel_found = false;
> unsigned long flags;
>
> - if (!hv_context.synic_initialized)
> + if (!hv_cpu->synic_initialized)
> return -EFAULT;
>
> /*
> @@ -395,12 +395,8 @@ int hv_synic_cleanup(unsigned int cpu)
>
> /* Turn off clockevent device */
> if (ms_hyperv.features & HV_MSR_SYNTIMER_AVAILABLE) {
> - struct hv_per_cpu_context *hv_cpu
> - = this_cpu_ptr(hv_context.cpu_context);
> -
> clockevents_unbind_device(hv_cpu->clk_evt, cpu);
> hv_ce_shutdown(hv_cpu->clk_evt);
> - put_cpu_ptr(hv_cpu);
> }
>
> hv_get_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
> @@ -428,5 +424,7 @@ int hv_synic_cleanup(unsigned int cpu)
> sctrl.enable = 0;
> hv_set_synic_state(sctrl.as_uint64);
>
> + hv_cpu->synic_initialized = false;
> +
> return 0;
> }
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index 72eaba3..eadd3df 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -202,6 +202,8 @@ enum {
> struct hv_per_cpu_context {
> void *synic_message_page;
> void *synic_event_page;
> + bool synic_initialized;
> +
> /*
> * buffer to post messages to the host.
> */
> @@ -230,8 +232,6 @@ struct hv_context {
>
> void *tsc_page;
>
> - bool synic_initialized;
> -
> struct hv_per_cpu_context __percpu *cpu_context;
>
> /*
Reviewed-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
Alternatively, we can get rid of synic_initialized flag altogether:
hv_synic_init() never fails in the first place but we can always
implement something like:
int hv_synic_is_initialized(void) {
union hv_synic_scontrol sctrl;
hv_get_synic_state(sctrl.as_uint64);
return sctrl.enable;
}
as it doesn't seem that we need to check synic state on _other_ CPUs.
--
Vitaly
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
