Re: [PATCH 1/4] clocksource/hyper-v: irq_pipeline: Enable pipelined clock events

[Philippe Gerum <rpm@xenomai.org>]

Florian Bezdeka <florian.bezdeka@siemens.com> writes:

> Fixes a boot failure on hyper-v.
>

A bit of context would help here. Failure doing what, enabling the proxy
tick on STIMER0?

> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> ---
>  drivers/clocksource/hyperv_timer.c | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
> index 2edc13ca184e0..0850b0c385a8e 100644
> --- a/drivers/clocksource/hyperv_timer.c
> +++ b/drivers/clocksource/hyperv_timer.c
> @@ -26,6 +26,7 @@
>  #include <clocksource/hyperv_timer.h>
>  #include <hyperv/hvhdk.h>
>  #include <asm/mshyperv.h>
> +#include <asm/trace/irq_vectors.h>
>  
>  static struct clock_event_device __percpu *hv_clock_event;
>  /* Note: offset can hold negative values after hibernation. */
> @@ -53,6 +54,30 @@ static int stimer0_irq = -1;
>  static int stimer0_message_sint;
>  static __maybe_unused DEFINE_PER_CPU(long, stimer0_evt);
>  
> +#ifdef CONFIG_IRQ_PIPELINE
> +
> +#define HV_STIMER_IRQ	apicm_vector_irq(HYPERV_STIMER0_VECTOR)
> +

Does this mean that we won't support Hyper-V on architectures with
per-CPU irqs (according to the comment heading hv_setup_stimer0_irq()),
or is this ok to always bypass the ACPI in order to get the STIMER0
interrupt in the pipeline case?

> +static irqreturn_t hv_stimer_oob_handler(int irq, void *dev_id)
> +{
> +	struct clock_event_device *evt = this_cpu_ptr(hv_clock_event);
> +
> +	trace_local_timer_entry(HYPERV_STIMER0_VECTOR);
> +	clockevents_handle_event(evt);
> +	trace_local_timer_exit(HYPERV_STIMER0_VECTOR);
> +
> +	return IRQ_HANDLED;
> +}
> +

Could not we have hv_stimer0_isr() call clockevents_handle_event()
instead, so that hv_setup_stimer0_irq() -> hv_stimer0_percpu_isr() could
be reused, only fixed up for picking the IRQ number from the apic
mapping if interrupts are pipelined?

> +static struct irqaction hv_stimer_oob_action = {
> +	.handler = hv_stimer_oob_handler,
> +	.name = "Out-of-band STIMER0 timer interrupt",
> +	.flags = IRQF_TIMER | IRQF_PERCPU,
> +};
> +#else
> +#define HV_STIMER_IRQ	-1
> +#endif
> +
>  /*
>   * Common code for stimer0 interrupts coming via Direct Mode or
>   * as a VMbus message.
> @@ -137,8 +162,9 @@ static int hv_stimer_init(unsigned int cpu)
>  
>  	ce = per_cpu_ptr(hv_clock_event, cpu);
>  	ce->name = "Hyper-V clockevent";
> -	ce->features = CLOCK_EVT_FEAT_ONESHOT;
> +	ce->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PIPELINE;
>  	ce->cpumask = cpumask_of(cpu);
> +	ce->irq = HV_STIMER_IRQ;
>  
>  	/*
>  	 * Lower the rating of the Hyper-V timer in a TDX VM without paravisor,
> @@ -305,6 +331,12 @@ int hv_stimer_alloc(bool have_percpu_irqs)
>  		hv_remove_stimer0_irq();
>  		goto free_clock_event;
>  	}
> +
> +#ifdef CONFIG_IRQ_PIPELINE
> +	ret = setup_percpu_irq(HV_STIMER_IRQ, &hv_stimer_oob_action);
> +	if (ret)
> +		goto free_clock_event;
> +#endif
>  	return ret;
>  
>  free_clock_event:

-- 
Philippe.