All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] dovetail: Add support for hyper-v as hypervisor
@ 2025-09-18 13:37 Florian Bezdeka
  2025-09-18 13:37 ` [PATCH 1/4] clocksource/hyper-v: irq_pipeline: Enable pipelined clock events Florian Bezdeka
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Florian Bezdeka @ 2025-09-18 13:37 UTC (permalink / raw)
  To: xenomai; +Cc: Florian Bezdeka

Hi all,

with this series applied dovetail is successfully booting up on a
Windows host using the Hyper-V hypervisor as virtualization platform.

You won't get any RT guarantees - that's clear - but it helps to test
non-RT stuff using a dovetail enabled kernel on such a platform.

The following config knobs were turned on during testing:
  - CONFIG_HYPERV
  - CONFIG_HYPERV_NET
  - CONFIG_HYPERV_KEYBOARD
  - CONFIG_HYPERV_TIMER
  - CONFIG_HYPERV_UTILS
  - CONFIG_HYPERV_BALLOON
  - CONFIG_HYPERV_IOMMU
  - CONFIG_HYPERV_STORAGE

Best regards,
Florian

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
Florian Bezdeka (4):
      clocksource/hyper-v: irq_pipeline: Enable pipelined clock events
      x86: irq_pipeline: Implement inband handler for hyper-v specific vectors
      hyper-v: x86: dovetail: Close race window in PV spinlocks
      x86: irq_pipeline: Allow CONFIG_HYPERV in combination with CONFIG_DOVETAIL

 arch/x86/hyperv/hv_spinlock.c      |  4 ++--
 arch/x86/kernel/irq_pipeline.c     |  9 +++++++++
 drivers/clocksource/hyperv_timer.c | 34 +++++++++++++++++++++++++++++++++-
 drivers/hv/Kconfig                 |  2 +-
 4 files changed, 45 insertions(+), 4 deletions(-)
---
base-commit: 138f1183deed300fd1f03abb6511635a916b98d0
change-id: 20250602-flo-add-hyperv-support-for-6-15-37378bd63fe1

Best regards,
-- 
Florian Bezdeka <florian.bezdeka@siemens.com>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/4] clocksource/hyper-v: irq_pipeline: Enable pipelined clock events
  2025-09-18 13:37 [PATCH 0/4] dovetail: Add support for hyper-v as hypervisor Florian Bezdeka
@ 2025-09-18 13:37 ` Florian Bezdeka
  2025-09-23 18:41   ` Philippe Gerum
  2025-09-18 13:37 ` [PATCH 2/4] x86: irq_pipeline: Implement inband handler for hyper-v specific vectors Florian Bezdeka
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Florian Bezdeka @ 2025-09-18 13:37 UTC (permalink / raw)
  To: xenomai; +Cc: Florian Bezdeka

Fixes a boot failure on hyper-v.

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)
+
+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;
+}
+
+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:

-- 
2.39.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/4] x86: irq_pipeline: Implement inband handler for hyper-v specific vectors
  2025-09-18 13:37 [PATCH 0/4] dovetail: Add support for hyper-v as hypervisor Florian Bezdeka
  2025-09-18 13:37 ` [PATCH 1/4] clocksource/hyper-v: irq_pipeline: Enable pipelined clock events Florian Bezdeka
@ 2025-09-18 13:37 ` Florian Bezdeka
  2025-09-18 13:37 ` [PATCH 3/4] hyper-v: x86: dovetail: Close race window in PV spinlocks Florian Bezdeka
  2025-09-18 13:38 ` [PATCH 4/4] x86: irq_pipeline: Allow CONFIG_HYPERV in combination with CONFIG_DOVETAIL Florian Bezdeka
  3 siblings, 0 replies; 8+ messages in thread
From: Florian Bezdeka @ 2025-09-18 13:37 UTC (permalink / raw)
  To: xenomai; +Cc: Florian Bezdeka

Both vectors were already routed to the inband stage but were not
implemented / handled there. We ended up ignoring those IRQs. That
resulted in a boot hang on hyper-v.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 arch/x86/kernel/irq_pipeline.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/x86/kernel/irq_pipeline.c b/arch/x86/kernel/irq_pipeline.c
index 1d6dfc6e95122..d6162310929c6 100644
--- a/arch/x86/kernel/irq_pipeline.c
+++ b/arch/x86/kernel/irq_pipeline.c
@@ -160,6 +160,15 @@ static void do_sysvec_inband(struct irq_desc *desc, struct pt_regs *regs)
 	case POSTED_INTR_NESTED_VECTOR:
 		run_sysvec_on_irqstack_cond(__sysvec_kvm_posted_intr_nested_ipi, regs);
 		break;
+#endif
+#if IS_ENABLED(CONFIG_HYPERV)
+	case HYPERV_REENLIGHTENMENT_VECTOR:
+		run_sysvec_on_irqstack_cond(__sysvec_hyperv_reenlightenment,
+					regs);
+		break;
+	case HYPERV_STIMER0_VECTOR:
+		run_sysvec_on_irqstack_cond(__sysvec_hyperv_stimer0, regs);
+		break;
 #endif
 	case HYPERVISOR_CALLBACK_VECTOR:
 		run_sysvec_on_irqstack_cond(pipeline_hv_callback, regs);

-- 
2.39.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/4] hyper-v: x86: dovetail: Close race window in PV spinlocks
  2025-09-18 13:37 [PATCH 0/4] dovetail: Add support for hyper-v as hypervisor Florian Bezdeka
  2025-09-18 13:37 ` [PATCH 1/4] clocksource/hyper-v: irq_pipeline: Enable pipelined clock events Florian Bezdeka
  2025-09-18 13:37 ` [PATCH 2/4] x86: irq_pipeline: Implement inband handler for hyper-v specific vectors Florian Bezdeka
@ 2025-09-18 13:37 ` Florian Bezdeka
  2025-09-18 13:38 ` [PATCH 4/4] x86: irq_pipeline: Allow CONFIG_HYPERV in combination with CONFIG_DOVETAIL Florian Bezdeka
  3 siblings, 0 replies; 8+ messages in thread
From: Florian Bezdeka @ 2025-09-18 13:37 UTC (permalink / raw)
  To: xenomai; +Cc: Florian Bezdeka

Stalling the in-band stage is not enough, we have to disable hard IRQs
to avoid additional instructions between the comparison of the value
and the rdmsrl() - as explained in the comment above.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 arch/x86/hyperv/hv_spinlock.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/hyperv/hv_spinlock.c b/arch/x86/hyperv/hv_spinlock.c
index 81b006601370c..282a0c69c9110 100644
--- a/arch/x86/hyperv/hv_spinlock.c
+++ b/arch/x86/hyperv/hv_spinlock.c
@@ -44,7 +44,7 @@ static void hv_qlock_wait(u8 *byte, u8 val)
 	 * into 'idle' state by the hypervisor and kept in that state for
 	 * an unspecified amount of time.
 	 */
-	local_irq_save(flags);
+	flags = hard_local_irq_save();
 	/*
 	 * Only issue the rdmsrq() when the lock state has not changed.
 	 */
@@ -55,7 +55,7 @@ static void hv_qlock_wait(u8 *byte, u8 val)
 
 		(void)msr_val;
 	}
-	local_irq_restore(flags);
+	hard_local_irq_restore(flags);
 }
 
 /*

-- 
2.39.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/4] x86: irq_pipeline: Allow CONFIG_HYPERV in combination with CONFIG_DOVETAIL
  2025-09-18 13:37 [PATCH 0/4] dovetail: Add support for hyper-v as hypervisor Florian Bezdeka
                   ` (2 preceding siblings ...)
  2025-09-18 13:37 ` [PATCH 3/4] hyper-v: x86: dovetail: Close race window in PV spinlocks Florian Bezdeka
@ 2025-09-18 13:38 ` Florian Bezdeka
  3 siblings, 0 replies; 8+ messages in thread
From: Florian Bezdeka @ 2025-09-18 13:38 UTC (permalink / raw)
  To: xenomai; +Cc: Florian Bezdeka

No known issues anymore, so we can allow that combination now.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 drivers/hv/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
index cf9cf77ac2187..6c7e76fd1b01e 100644
--- a/drivers/hv/Kconfig
+++ b/drivers/hv/Kconfig
@@ -5,7 +5,7 @@ menu "Microsoft Hyper-V guest support"
 config HYPERV
 	tristate "Microsoft Hyper-V client drivers"
 	depends on ((X86 && X86_LOCAL_APIC && HYPERVISOR_GUEST) \
-		|| (ARM64 && !CPU_BIG_ENDIAN)) && !IRQ_PIPELINE
+		|| (ARM64 && !CPU_BIG_ENDIAN))
 	select PARAVIRT
 	select X86_HV_CALLBACK_VECTOR if X86
 	select OF_EARLY_FLATTREE if OF

-- 
2.39.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/4] clocksource/hyper-v: irq_pipeline: Enable pipelined clock events
  2025-09-18 13:37 ` [PATCH 1/4] clocksource/hyper-v: irq_pipeline: Enable pipelined clock events Florian Bezdeka
@ 2025-09-23 18:41   ` Philippe Gerum
  2025-09-24  7:51     ` Florian Bezdeka
  0 siblings, 1 reply; 8+ messages in thread
From: Philippe Gerum @ 2025-09-23 18:41 UTC (permalink / raw)
  To: Florian Bezdeka; +Cc: xenomai

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/4] clocksource/hyper-v: irq_pipeline: Enable pipelined clock events
  2025-09-23 18:41   ` Philippe Gerum
@ 2025-09-24  7:51     ` Florian Bezdeka
  2025-09-28  7:24       ` Philippe Gerum
  0 siblings, 1 reply; 8+ messages in thread
From: Florian Bezdeka @ 2025-09-24  7:51 UTC (permalink / raw)
  To: Philippe Gerum; +Cc: xenomai

On Tue, 2025-09-23 at 20:41 +0200, Philippe Gerum wrote:
> 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?

ACK. That goes back to the early phase of enabling hyper-v where I
simply did not care about story telling yet.

Proposal:

Make the clock event around the STIMER0 pipeline safe. The
implementation is following the same pattern that we already have in
place for the x86 LAPIC interrupt / clock event.

To get the proxy tick working we need to link the hyperv clock event
device with the STIMER0 vector. This part is also following the pattern
of the x86 lapic interrupt. Both have a fixed arch specific vector
assigned.

> 
> > 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?

I came along the same question during implementation. The most
confusing part is, that this per-cpu IRQ implementation has no in-tree
user - or I could not find it.

It all starts in hv_stimer_setup_percpu_clockev() (x86 specific) where
we have the only user of hv_stimer_alloc() with have_percpu_irqs set to
false.

With that I decided to follow the same pattern as we have in place for
the APIC timer IRQ on x86.

> 
> > +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?

I definitely tried updating hv_stimer0_isr() to call
clockevents_handle_event(). It has been a while so I'm not sure if
memories are still correct: It might happen that the STIMER0 IRQ fires
before the proxy tick infrastructure is in place, so it ended up in
"ignoring a timer tick".

In any case there was/is a reason why the LAPIC is using
local_apic_timer_interrupt() for a short period in time and later moves
on to the oob action. The same reason applied to STIMER0.

> 
> > +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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/4] clocksource/hyper-v: irq_pipeline: Enable pipelined clock events
  2025-09-24  7:51     ` Florian Bezdeka
@ 2025-09-28  7:24       ` Philippe Gerum
  0 siblings, 0 replies; 8+ messages in thread
From: Philippe Gerum @ 2025-09-28  7:24 UTC (permalink / raw)
  To: Florian Bezdeka; +Cc: xenomai

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

> On Tue, 2025-09-23 at 20:41 +0200, Philippe Gerum wrote:
>> 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?
>
> ACK. That goes back to the early phase of enabling hyper-v where I
> simply did not care about story telling yet.
>
> Proposal:
>
> Make the clock event around the STIMER0 pipeline safe. The
> implementation is following the same pattern that we already have in
> place for the x86 LAPIC interrupt / clock event.
>
> To get the proxy tick working we need to link the hyperv clock event
> device with the STIMER0 vector. This part is also following the pattern
> of the x86 lapic interrupt. Both have a fixed arch specific vector
> assigned.
>

LGTM.

>> 
>> > 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?
>
> I came along the same question during implementation. The most
> confusing part is, that this per-cpu IRQ implementation has no in-tree
> user - or I could not find it.
>
> It all starts in hv_stimer_setup_percpu_clockev() (x86 specific) where
> we have the only user of hv_stimer_alloc() with have_percpu_irqs set to
> false.
>
> With that I decided to follow the same pattern as we have in place for
> the APIC timer IRQ on x86.
>

Makes sense to me too.

>> 
>> > +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?
>
> I definitely tried updating hv_stimer0_isr() to call
> clockevents_handle_event(). It has been a while so I'm not sure if
> memories are still correct: It might happen that the STIMER0 IRQ fires
> before the proxy tick infrastructure is in place, so it ended up in
> "ignoring a timer tick".
>
> In any case there was/is a reason why the LAPIC is using
> local_apic_timer_interrupt() for a short period in time and later moves
> on to the oob action. The same reason applied to STIMER0.
>

Ok. Another way would be to make the handler bullet-proof to the
inband->oob transition of the clock device if this is indeed the issue
at stake, following the same pattern than armv7 is implementing, i.e.:

static irqreturn_t twd_handler(int irq, void *dev_id)
{
	struct clock_event_device *evt = dev_id;

	if ((running_inband() && clockevent_is_oob(evt)) || twd_timer_ack()) {
		clockevents_handle_event(evt);
		return IRQ_HANDLED;
	}

	return IRQ_NONE;
}

This said, Hyper-v is hardly going to be a hot spot for pipeline-related
changes, so I'm ok with the fully decoupled approach you suggested.

-- 
Philippe.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-09-28  7:24 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-18 13:37 [PATCH 0/4] dovetail: Add support for hyper-v as hypervisor Florian Bezdeka
2025-09-18 13:37 ` [PATCH 1/4] clocksource/hyper-v: irq_pipeline: Enable pipelined clock events Florian Bezdeka
2025-09-23 18:41   ` Philippe Gerum
2025-09-24  7:51     ` Florian Bezdeka
2025-09-28  7:24       ` Philippe Gerum
2025-09-18 13:37 ` [PATCH 2/4] x86: irq_pipeline: Implement inband handler for hyper-v specific vectors Florian Bezdeka
2025-09-18 13:37 ` [PATCH 3/4] hyper-v: x86: dovetail: Close race window in PV spinlocks Florian Bezdeka
2025-09-18 13:38 ` [PATCH 4/4] x86: irq_pipeline: Allow CONFIG_HYPERV in combination with CONFIG_DOVETAIL Florian Bezdeka

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.