All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH dovetail 6.10] x86: dovetail: Allow multiple HV guest extensions to be enabled
@ 2024-08-23  9:18 Florian Bezdeka
  2024-08-23  9:18 ` [PATCH dovetail 6.6] " Florian Bezdeka
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Florian Bezdeka @ 2024-08-23  9:18 UTC (permalink / raw)
  To: xenomai; +Cc: Philippe Gerum, Jan Kiszka, Ionut Nechita, Florian Bezdeka

do_sysvec_inband() assumed that only one of CONFIG_ACRN_GUEST and
CONFIG_KVM_GUEST is active at the same time. If both were enabled we
run into a compile issue:

arch/x86/kernel/irq_pipeline.c: In function 'do_sysvec_inband':
arch/x86/kernel/irq_pipeline.c:160:9: error: duplicate case value
   160 |         case HYPERVISOR_CALLBACK_VECTOR:
       |         ^~~~
arch/x86/kernel/irq_pipeline.c:154:9: note: previously used here
   154 |         case HYPERVISOR_CALLBACK_VECTOR:
       |         ^~~~

As only one hypervisor (HV) can be active at the same time, but support
for more than one can be compiled in, we have to migrate from a build
time decision to a runtime based one.

We introduce sysvec_install() macro - like modern kernels already do -
and call a dovetail specific pipelined_sysvec_install() before
registering the IDT gate.

All HYPERVISOR_CALLBACK_VECTOR related callsites of alloc_intr_gate()
have been migrated to sysvec_install(). sysvec_install() is able to
distinguish between the low level entry point (asm_ variant) and the
higher entry point (__ variant).

The trampoline function (pipeline_hv_callback()) is needed to statisfy
type checks done by run_sysvec_on_irqstack_cond().

Reported-by: Ionut Nechita <ionut.nechita@intel.com>
Closes: https://lore.kernel.org/xenomai/PH0PR11MB59498BE915597AEF0B5BBB30F4B42@PH0PR11MB5949.namprd11.prod.outlook.com/
Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---

Hi all,

changes compared to the RFC:
- Changed title
- Some renamings, mainly taking care of the feedback provided by
  Philippe
- Added backports for all dovetail/Linux LTS versions

All patches are build tested and runtime tested for KVM guests. I have
no other HV / VMM at hand.

Ionut, it would be very nice if you could test at least one version for
ACRN. Thanks!

Best regards,
Florian

 arch/x86/include/asm/idtentry.h | 10 ++++++++++
 arch/x86/kernel/irq_pipeline.c  | 18 ++++++++----------
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/idtentry.h b/arch/x86/include/asm/idtentry.h
index 524cb93afc34..e7d434e2ace0 100644
--- a/arch/x86/include/asm/idtentry.h
+++ b/arch/x86/include/asm/idtentry.h
@@ -235,6 +235,13 @@ __visible __noreturn void __##func(struct pt_regs *regs)
 #define DEFINE_IDTENTRY_SYSVEC_SIMPLE_PIPELINED(vector, func)		\
 	DEFINE_IDTENTRY_SYSVEC_PIPELINED(vector, func)
 
+extern void (*pipeline_hv_callback_fn)(struct pt_regs *regs);
+static inline void pipeline_install_sysvec(int vector, const void *function)
+{
+	if (vector == HYPERVISOR_CALLBACK_VECTOR)
+		pipeline_hv_callback_fn = function;
+}
+
 #else  /* !CONFIG_IRQ_PIPELINE */
 
 #define DECLARE_IDTENTRY_SYSVEC_PIPELINED(vector, func)		DECLARE_IDTENTRY_SYSVEC(vector, func)
@@ -274,6 +281,8 @@ __visible noinstr void func(struct pt_regs *regs,			\
 									\
 static noinline void __##func(struct pt_regs *regs, u32 vector)
 
+static inline void pipeline_install_sysvec(vector, function) {}
+
 #endif	/* !CONFIG_IRQ_PIPELINE */
 
 /**
@@ -523,6 +532,7 @@ static inline void fred_install_sysvec(unsigned int vector, const idtentry_t fun
 #endif
 
 #define sysvec_install(vector, function) {				\
+	pipeline_install_sysvec(vector, __##function);			\
 	if (cpu_feature_enabled(X86_FEATURE_FRED))			\
 		fred_install_sysvec(vector, function);			\
 	else								\
diff --git a/arch/x86/kernel/irq_pipeline.c b/arch/x86/kernel/irq_pipeline.c
index 73e97fba55e7..7a64af5dab73 100644
--- a/arch/x86/kernel/irq_pipeline.c
+++ b/arch/x86/kernel/irq_pipeline.c
@@ -14,6 +14,8 @@
 #include <asm/mshyperv.h>
 #include <asm/idtentry.h>
 
+void (*pipeline_hv_callback_fn)(struct pt_regs *regs) = NULL;
+
 static struct irq_domain *sipic_domain;
 
 static void sipic_irq_noop(struct irq_data *data) { }
@@ -76,6 +78,11 @@ static void pipeline_exit_rcu(irqentry_state_t state)
 		ct_irq_exit();
 }
 
+static void pipeline_hv_callback(struct pt_regs *regs)
+{
+	pipeline_hv_callback_fn(regs);
+}
+
 static void do_sysvec_inband(struct irq_desc *desc, struct pt_regs *regs)
 {
 	unsigned int irq = irq_desc_get_irq(desc);
@@ -150,18 +157,9 @@ static void do_sysvec_inband(struct irq_desc *desc, struct pt_regs *regs)
 					regs);
 		break;
 #endif
-#ifdef CONFIG_ACRN_GUEST
 	case HYPERVISOR_CALLBACK_VECTOR:
-		run_sysvec_on_irqstack_cond(__sysvec_acrn_hv_callback,
-					regs);
+		run_sysvec_on_irqstack_cond(pipeline_hv_callback, regs);
 		break;
-#endif
-#ifdef CONFIG_KVM_GUEST
-	case HYPERVISOR_CALLBACK_VECTOR:
-		run_sysvec_on_irqstack_cond(__sysvec_kvm_asyncpf_interrupt,
-					    regs);
-		break;
-#endif
 	case LOCAL_TIMER_VECTOR:
 		run_sysvec_on_irqstack_cond(__sysvec_apic_timer_interrupt,
 					regs);
-- 
2.39.2


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

* [PATCH dovetail 6.6] x86: dovetail: Allow multiple HV guest extensions to be enabled
  2024-08-23  9:18 [PATCH dovetail 6.10] x86: dovetail: Allow multiple HV guest extensions to be enabled Florian Bezdeka
@ 2024-08-23  9:18 ` Florian Bezdeka
  2024-08-23  9:18 ` [PATCH dovetail 6.1] " Florian Bezdeka
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Florian Bezdeka @ 2024-08-23  9:18 UTC (permalink / raw)
  To: xenomai; +Cc: Philippe Gerum, Jan Kiszka, Ionut Nechita, Florian Bezdeka

do_sysvec_inband() assumed that only one of CONFIG_ACRN_GUEST and
CONFIG_KVM_GUEST is active at the same time. If both were enabled we
run into a compile issue:

arch/x86/kernel/irq_pipeline.c: In function 'do_sysvec_inband':
arch/x86/kernel/irq_pipeline.c:160:9: error: duplicate case value
   160 |         case HYPERVISOR_CALLBACK_VECTOR:
       |         ^~~~
arch/x86/kernel/irq_pipeline.c:154:9: note: previously used here
   154 |         case HYPERVISOR_CALLBACK_VECTOR:
       |         ^~~~

As only one hypervisor (HV) can be active at the same time, but support
for more than one can be compiled in, we have to migrate from a build
time decision to a runtime based one.

We introduce sysvec_install() macro - like modern kernels already do -
and call a dovetail specific pipelined_sysvec_install() before
registering the IDT gate.

All HYPERVISOR_CALLBACK_VECTOR related callsites of alloc_intr_gate()
have been migrated to sysvec_install(). sysvec_install() is able to
distinguish between the low level entry point (asm_ variant) and the
higher entry point (__ variant).

The trampoline function (pipeline_hv_callback()) is needed to statisfy
type checks done by run_sysvec_on_irqstack_cond().

Reported-by: Ionut Nechita <ionut.nechita@intel.com>
Closes: https://lore.kernel.org/xenomai/PH0PR11MB59498BE915597AEF0B5BBB30F4B42@PH0PR11MB5949.namprd11.prod.outlook.com/

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 arch/x86/include/asm/idtentry.h  | 15 +++++++++++++++
 arch/x86/kernel/cpu/acrn.c       |  2 +-
 arch/x86/kernel/cpu/mshyperv.c   |  2 +-
 arch/x86/kernel/irq_pipeline.c   | 18 ++++++++----------
 arch/x86/kernel/kvm.c            |  2 +-
 drivers/xen/events/events_base.c |  2 +-
 6 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/arch/x86/include/asm/idtentry.h b/arch/x86/include/asm/idtentry.h
index b747bbe64a33..347d13771f4d 100644
--- a/arch/x86/include/asm/idtentry.h
+++ b/arch/x86/include/asm/idtentry.h
@@ -220,6 +220,13 @@ __visible __noreturn void __##func(struct pt_regs *regs)
 #define DEFINE_IDTENTRY_SYSVEC_SIMPLE_PIPELINED(vector, func)		\
 	DEFINE_IDTENTRY_SYSVEC_PIPELINED(vector, func)
 
+extern void (*pipeline_hv_callback_fn)(struct pt_regs *regs);
+static inline void pipeline_install_sysvec(int vector, const void *function)
+{
+	if (vector == HYPERVISOR_CALLBACK_VECTOR)
+		pipeline_hv_callback_fn = function;
+}
+
 #else  /* !CONFIG_IRQ_PIPELINE */
 
 #define DECLARE_IDTENTRY_SYSVEC_PIPELINED(vector, func)		DECLARE_IDTENTRY_SYSVEC(vector, func)
@@ -259,6 +266,8 @@ __visible noinstr void func(struct pt_regs *regs,			\
 									\
 static noinline void __##func(struct pt_regs *regs, u32 vector)
 
+static inline void pipeline_install_sysvec(vector, function) {}
+
 #endif	/* !CONFIG_IRQ_PIPELINE */
 
 /**
@@ -476,6 +485,12 @@ __visible noinstr void func(struct pt_regs *regs,			\
 #define DEFINE_IDTENTRY_DEBUG_USER	DEFINE_IDTENTRY_NOIST
 #endif
 
+#define sysvec_install(vector, function)                       \
+	{                                                      \
+		pipeline_install_sysvec(vector, __##function); \
+		alloc_intr_gate(vector, asm_##function);       \
+	}
+
 #else /* !__ASSEMBLY__ */
 
 /*
diff --git a/arch/x86/kernel/cpu/acrn.c b/arch/x86/kernel/cpu/acrn.c
index fffcf2e48e93..54128c9972d5 100644
--- a/arch/x86/kernel/cpu/acrn.c
+++ b/arch/x86/kernel/cpu/acrn.c
@@ -27,7 +27,7 @@ static u32 __init acrn_detect(void)
 static void __init acrn_init_platform(void)
 {
 	/* Setup the IDT for ACRN hypervisor callback */
-	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_acrn_hv_callback);
+	sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_acrn_hv_callback);
 
 	x86_platform.calibrate_tsc = acrn_get_tsc_khz;
 	x86_platform.calibrate_cpu = acrn_get_tsc_khz;
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index c3dacff70d10..5b45fddd7ed9 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -539,7 +539,7 @@ static void __init ms_hyperv_init_platform(void)
 	x86_platform.apic_post_init = hyperv_init;
 	hyperv_setup_mmu_ops();
 	/* Setup the IDT for hypervisor callback */
-	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_hyperv_callback);
+	sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_hyperv_callback);
 
 	/* Setup the IDT for reenlightenment notifications */
 	if (ms_hyperv.features & HV_ACCESS_REENLIGHTENMENT) {
diff --git a/arch/x86/kernel/irq_pipeline.c b/arch/x86/kernel/irq_pipeline.c
index a6beb0040036..d001efd4b002 100644
--- a/arch/x86/kernel/irq_pipeline.c
+++ b/arch/x86/kernel/irq_pipeline.c
@@ -14,6 +14,8 @@
 #include <asm/mshyperv.h>
 #include <asm/idtentry.h>
 
+void (*pipeline_hv_callback_fn)(struct pt_regs *regs) = NULL;
+
 static struct irq_domain *sipic_domain;
 
 static void sipic_irq_noop(struct irq_data *data) { }
@@ -76,6 +78,11 @@ static void pipeline_exit_rcu(irqentry_state_t state)
 		ct_irq_exit();
 }
 
+static void pipeline_hv_callback(struct pt_regs *regs)
+{
+	pipeline_hv_callback_fn(regs);
+}
+
 static void do_sysvec_inband(struct irq_desc *desc, struct pt_regs *regs)
 {
 	unsigned int irq = irq_desc_get_irq(desc);
@@ -150,18 +157,9 @@ static void do_sysvec_inband(struct irq_desc *desc, struct pt_regs *regs)
 					regs);
 		break;
 #endif
-#ifdef CONFIG_ACRN_GUEST
 	case HYPERVISOR_CALLBACK_VECTOR:
-		run_sysvec_on_irqstack_cond(__sysvec_acrn_hv_callback,
-					regs);
+		run_sysvec_on_irqstack_cond(pipeline_hv_callback, regs);
 		break;
-#endif
-#ifdef CONFIG_KVM_GUEST
-	case HYPERVISOR_CALLBACK_VECTOR:
-		run_sysvec_on_irqstack_cond(__sysvec_kvm_asyncpf_interrupt,
-					    regs);
-		break;
-#endif
 	case LOCAL_TIMER_VECTOR:
 		run_sysvec_on_irqstack_cond(__sysvec_apic_timer_interrupt,
 					regs);
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index aa20662f01ac..16255ecb486f 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -839,7 +839,7 @@ static void __init kvm_guest_init(void)
 
 	if (kvm_para_has_feature(KVM_FEATURE_ASYNC_PF_INT) && kvmapf) {
 		static_branch_enable(&kvm_async_pf_enabled);
-		alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_kvm_asyncpf_interrupt);
+		sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_kvm_asyncpf_interrupt);
 	}
 
 #ifdef CONFIG_SMP
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 9e3b5d21d098..75733eefe77c 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -2235,7 +2235,7 @@ static __init void xen_alloc_callback_vector(void)
 		return;
 
 	pr_info("Xen HVM callback vector for event delivery is enabled\n");
-	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_xen_hvm_callback);
+	sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_xen_hvm_callback);
 }
 #else
 void xen_setup_callback_vector(void) {}
-- 
2.39.2


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

* [PATCH dovetail 6.1] x86: dovetail: Allow multiple HV guest extensions to be enabled
  2024-08-23  9:18 [PATCH dovetail 6.10] x86: dovetail: Allow multiple HV guest extensions to be enabled Florian Bezdeka
  2024-08-23  9:18 ` [PATCH dovetail 6.6] " Florian Bezdeka
@ 2024-08-23  9:18 ` Florian Bezdeka
  2024-08-23  9:18 ` [PATCH dovetail 5.15] " Florian Bezdeka
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Florian Bezdeka @ 2024-08-23  9:18 UTC (permalink / raw)
  To: xenomai; +Cc: Philippe Gerum, Jan Kiszka, Ionut Nechita, Florian Bezdeka

do_sysvec_inband() assumed that only one of CONFIG_ACRN_GUEST and
CONFIG_KVM_GUEST is active at the same time. If both were enabled we
run into a compile issue:

arch/x86/kernel/irq_pipeline.c: In function 'do_sysvec_inband':
arch/x86/kernel/irq_pipeline.c:160:9: error: duplicate case value
   160 |         case HYPERVISOR_CALLBACK_VECTOR:
       |         ^~~~
arch/x86/kernel/irq_pipeline.c:154:9: note: previously used here
   154 |         case HYPERVISOR_CALLBACK_VECTOR:
       |         ^~~~

As only one hypervisor (HV) can be active at the same time, but support
for more than one can be compiled in, we have to migrate from a build
time decision to a runtime based one.

We introduce sysvec_install() macro - like modern kernels already do -
and call a dovetail specific pipelined_sysvec_install() before
registering the IDT gate.

All HYPERVISOR_CALLBACK_VECTOR related callsites of alloc_intr_gate()
have been migrated to sysvec_install(). sysvec_install() is able to
distinguish between the low level entry point (asm_ variant) and the
higher entry point (__ variant).

The trampoline function (pipeline_hv_callback()) is needed to statisfy
type checks done by run_sysvec_on_irqstack_cond().

Reported-by: Ionut Nechita <ionut.nechita@intel.com>
Closes: https://lore.kernel.org/xenomai/PH0PR11MB59498BE915597AEF0B5BBB30F4B42@PH0PR11MB5949.namprd11.prod.outlook.com/

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 arch/x86/include/asm/idtentry.h  | 15 +++++++++++++++
 arch/x86/kernel/cpu/acrn.c       |  2 +-
 arch/x86/kernel/cpu/mshyperv.c   |  2 +-
 arch/x86/kernel/irq_pipeline.c   | 18 ++++++++----------
 arch/x86/kernel/kvm.c            |  2 +-
 drivers/xen/events/events_base.c |  2 +-
 6 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/arch/x86/include/asm/idtentry.h b/arch/x86/include/asm/idtentry.h
index 61984b960a9f..c176e541fd1d 100644
--- a/arch/x86/include/asm/idtentry.h
+++ b/arch/x86/include/asm/idtentry.h
@@ -220,6 +220,13 @@ __visible __noreturn void __##func(struct pt_regs *regs)
 #define DEFINE_IDTENTRY_SYSVEC_SIMPLE_PIPELINED(vector, func)		\
 	DEFINE_IDTENTRY_SYSVEC_PIPELINED(vector, func)
 
+extern void (*pipeline_hv_callback_fn)(struct pt_regs *regs);
+static inline void pipeline_install_sysvec(int vector, const void *function)
+{
+	if (vector == HYPERVISOR_CALLBACK_VECTOR)
+		pipeline_hv_callback_fn = function;
+}
+
 #else  /* !CONFIG_IRQ_PIPELINE */
 
 #define DECLARE_IDTENTRY_SYSVEC_PIPELINED(vector, func)		DECLARE_IDTENTRY_SYSVEC(vector, func)
@@ -259,6 +266,8 @@ __visible noinstr void func(struct pt_regs *regs,			\
 									\
 static noinline void __##func(struct pt_regs *regs, u32 vector)
 
+static inline void pipeline_install_sysvec(vector, function) {}
+
 #endif	/* !CONFIG_IRQ_PIPELINE */
 
 /**
@@ -476,6 +485,12 @@ __visible noinstr void func(struct pt_regs *regs,			\
 #define DEFINE_IDTENTRY_DEBUG_USER	DEFINE_IDTENTRY_NOIST
 #endif
 
+#define sysvec_install(vector, function)                       \
+	{                                                      \
+		pipeline_install_sysvec(vector, __##function); \
+		alloc_intr_gate(vector, asm_##function);       \
+	}
+
 #else /* !__ASSEMBLY__ */
 
 /*
diff --git a/arch/x86/kernel/cpu/acrn.c b/arch/x86/kernel/cpu/acrn.c
index 8fa6c605b579..8fb5f4945325 100644
--- a/arch/x86/kernel/cpu/acrn.c
+++ b/arch/x86/kernel/cpu/acrn.c
@@ -27,7 +27,7 @@ static u32 __init acrn_detect(void)
 static void __init acrn_init_platform(void)
 {
 	/* Setup the IDT for ACRN hypervisor callback */
-	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_acrn_hv_callback);
+	sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_acrn_hv_callback);
 
 	x86_platform.calibrate_tsc = acrn_get_tsc_khz;
 	x86_platform.calibrate_cpu = acrn_get_tsc_khz;
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 54d8436ee56a..db92a583c126 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -422,7 +422,7 @@ static void __init ms_hyperv_init_platform(void)
 	x86_platform.apic_post_init = hyperv_init;
 	hyperv_setup_mmu_ops();
 	/* Setup the IDT for hypervisor callback */
-	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_hyperv_callback);
+	sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_hyperv_callback);
 
 	/* Setup the IDT for reenlightenment notifications */
 	if (ms_hyperv.features & HV_ACCESS_REENLIGHTENMENT) {
diff --git a/arch/x86/kernel/irq_pipeline.c b/arch/x86/kernel/irq_pipeline.c
index 8a71a61327d5..d48440081f83 100644
--- a/arch/x86/kernel/irq_pipeline.c
+++ b/arch/x86/kernel/irq_pipeline.c
@@ -14,6 +14,8 @@
 #include <asm/mshyperv.h>
 #include <asm/idtentry.h>
 
+void (*pipeline_hv_callback_fn)(struct pt_regs *regs) = NULL;
+
 static struct irq_domain *sipic_domain;
 
 static void sipic_irq_noop(struct irq_data *data) { }
@@ -76,6 +78,11 @@ static void pipeline_exit_rcu(irqentry_state_t state)
 		ct_irq_exit();
 }
 
+static void pipeline_hv_callback(struct pt_regs *regs)
+{
+	pipeline_hv_callback_fn(regs);
+}
+
 static void do_sysvec_inband(struct irq_desc *desc, struct pt_regs *regs)
 {
 	unsigned int irq = irq_desc_get_irq(desc);
@@ -150,18 +157,9 @@ static void do_sysvec_inband(struct irq_desc *desc, struct pt_regs *regs)
 					regs);
 		break;
 #endif
-#ifdef CONFIG_ACRN_GUEST
 	case HYPERVISOR_CALLBACK_VECTOR:
-		run_sysvec_on_irqstack_cond(__sysvec_acrn_hv_callback,
-					regs);
+		run_sysvec_on_irqstack_cond(pipeline_hv_callback, regs);
 		break;
-#endif
-#ifdef CONFIG_KVM_GUEST
-	case HYPERVISOR_CALLBACK_VECTOR:
-		run_sysvec_on_irqstack_cond(__sysvec_kvm_asyncpf_interrupt,
-					    regs);
-		break;
-#endif
 	case LOCAL_TIMER_VECTOR:
 		run_sysvec_on_irqstack_cond(__sysvec_apic_timer_interrupt,
 					regs);
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 9dde33d59efe..6af49d30816c 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -845,7 +845,7 @@ static void __init kvm_guest_init(void)
 
 	if (kvm_para_has_feature(KVM_FEATURE_ASYNC_PF_INT) && kvmapf) {
 		static_branch_enable(&kvm_async_pf_enabled);
-		alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_kvm_asyncpf_interrupt);
+		sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_kvm_asyncpf_interrupt);
 	}
 
 #ifdef CONFIG_SMP
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 96b96516c980..867ead24d93a 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -2238,7 +2238,7 @@ static __init void xen_alloc_callback_vector(void)
 		return;
 
 	pr_info("Xen HVM callback vector for event delivery is enabled\n");
-	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_xen_hvm_callback);
+	sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_xen_hvm_callback);
 }
 #else
 void xen_setup_callback_vector(void) {}
-- 
2.39.2


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

* [PATCH dovetail 5.15] x86: dovetail: Allow multiple HV guest extensions to be enabled
  2024-08-23  9:18 [PATCH dovetail 6.10] x86: dovetail: Allow multiple HV guest extensions to be enabled Florian Bezdeka
  2024-08-23  9:18 ` [PATCH dovetail 6.6] " Florian Bezdeka
  2024-08-23  9:18 ` [PATCH dovetail 6.1] " Florian Bezdeka
@ 2024-08-23  9:18 ` Florian Bezdeka
  2024-09-06 11:11   ` Jan Kiszka
  2024-08-23  9:18 ` [PATCH dovetail 5.10] " Florian Bezdeka
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Florian Bezdeka @ 2024-08-23  9:18 UTC (permalink / raw)
  To: xenomai; +Cc: Philippe Gerum, Jan Kiszka, Ionut Nechita, Florian Bezdeka

do_sysvec_inband() assumed that only one of CONFIG_ACRN_GUEST and
CONFIG_KVM_GUEST is active at the same time. If both were enabled we
run into a compile issue:

arch/x86/kernel/irq_pipeline.c: In function 'do_sysvec_inband':
arch/x86/kernel/irq_pipeline.c:160:9: error: duplicate case value
   160 |         case HYPERVISOR_CALLBACK_VECTOR:
       |         ^~~~
arch/x86/kernel/irq_pipeline.c:154:9: note: previously used here
   154 |         case HYPERVISOR_CALLBACK_VECTOR:
       |         ^~~~

As only one hypervisor (HV) can be active at the same time, but support
for more than one can be compiled in, we have to migrate from a build
time decision to a runtime based one.

We introduce sysvec_install() macro - like modern kernels already do -
and call a dovetail specific pipelined_sysvec_install() before
registering the IDT gate.

All HYPERVISOR_CALLBACK_VECTOR related callsites of alloc_intr_gate()
have been migrated to sysvec_install(). sysvec_install() is able to
distinguish between the low level entry point (asm_ variant) and the
higher entry point (__ variant).

The trampoline function (pipeline_hv_callback()) is needed to statisfy
type checks done by run_sysvec_on_irqstack_cond().

Reported-by: Ionut Nechita <ionut.nechita@intel.com>
Closes: https://lore.kernel.org/xenomai/PH0PR11MB59498BE915597AEF0B5BBB30F4B42@PH0PR11MB5949.namprd11.prod.outlook.com/

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 arch/x86/include/asm/idtentry.h  | 15 +++++++++++++++
 arch/x86/kernel/cpu/acrn.c       |  2 +-
 arch/x86/kernel/cpu/mshyperv.c   |  2 +-
 arch/x86/kernel/irq_pipeline.c   | 18 ++++++++----------
 arch/x86/kernel/kvm.c            |  2 +-
 drivers/xen/events/events_base.c |  2 +-
 6 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/arch/x86/include/asm/idtentry.h b/arch/x86/include/asm/idtentry.h
index 1f277e852c4a..5d1140bb5acc 100644
--- a/arch/x86/include/asm/idtentry.h
+++ b/arch/x86/include/asm/idtentry.h
@@ -218,6 +218,13 @@ __visible __noreturn void __##func(struct pt_regs *regs)
 #define DEFINE_IDTENTRY_SYSVEC_SIMPLE_PIPELINED(vector, func)		\
 	DEFINE_IDTENTRY_SYSVEC_PIPELINED(vector, func)
 
+extern void (*pipeline_hv_callback_fn)(struct pt_regs *regs);
+static inline void pipeline_install_sysvec(int vector, const void *function)
+{
+	if (vector == HYPERVISOR_CALLBACK_VECTOR)
+		pipeline_hv_callback_fn = function;
+}
+
 #else  /* !CONFIG_IRQ_PIPELINE */
 
 #define DECLARE_IDTENTRY_SYSVEC_PIPELINED(vector, func)		DECLARE_IDTENTRY_SYSVEC(vector, func)
@@ -257,6 +264,8 @@ __visible noinstr void func(struct pt_regs *regs,			\
 									\
 static noinline void __##func(struct pt_regs *regs, u32 vector)
 
+static inline void pipeline_install_sysvec(vector, function) {}
+
 #endif	/* !CONFIG_IRQ_PIPELINE */
 
 /**
@@ -474,6 +483,12 @@ __visible noinstr void func(struct pt_regs *regs,			\
 #define DEFINE_IDTENTRY_DEBUG_USER	DEFINE_IDTENTRY_NOIST
 #endif
 
+#define sysvec_install(vector, function)                       \
+	{                                                      \
+		pipeline_install_sysvec(vector, __##function); \
+		alloc_intr_gate(vector, asm_##function);       \
+	}
+
 #else /* !__ASSEMBLY__ */
 
 /*
diff --git a/arch/x86/kernel/cpu/acrn.c b/arch/x86/kernel/cpu/acrn.c
index 0c9d6953f89a..3b02a4fbec97 100644
--- a/arch/x86/kernel/cpu/acrn.c
+++ b/arch/x86/kernel/cpu/acrn.c
@@ -27,7 +27,7 @@ static u32 __init acrn_detect(void)
 static void __init acrn_init_platform(void)
 {
 	/* Setup the IDT for ACRN hypervisor callback */
-	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_acrn_hv_callback);
+	sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_acrn_hv_callback);
 }
 
 static bool acrn_x2apic_available(void)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 53ded40f3a48..01242d5c204a 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -406,7 +406,7 @@ static void __init ms_hyperv_init_platform(void)
 	x86_platform.apic_post_init = hyperv_init;
 	hyperv_setup_mmu_ops();
 	/* Setup the IDT for hypervisor callback */
-	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_hyperv_callback);
+	sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_hyperv_callback);
 
 	/* Setup the IDT for reenlightenment notifications */
 	if (ms_hyperv.features & HV_ACCESS_REENLIGHTENMENT) {
diff --git a/arch/x86/kernel/irq_pipeline.c b/arch/x86/kernel/irq_pipeline.c
index 1332f328a45b..921898cbc7b3 100644
--- a/arch/x86/kernel/irq_pipeline.c
+++ b/arch/x86/kernel/irq_pipeline.c
@@ -14,6 +14,8 @@
 #include <asm/mshyperv.h>
 #include <asm/idtentry.h>
 
+void (*pipeline_hv_callback_fn)(struct pt_regs *regs) = NULL;
+
 static struct irq_domain *sipic_domain;
 
 static void sipic_irq_noop(struct irq_data *data) { }
@@ -76,6 +78,11 @@ static void pipeline_exit_rcu(irqentry_state_t state)
 		rcu_irq_exit();
 }
 
+static void pipeline_hv_callback(struct pt_regs *regs)
+{
+	pipeline_hv_callback_fn(regs);
+}
+
 static void do_sysvec_inband(struct irq_desc *desc, struct pt_regs *regs)
 {
 	unsigned int irq = irq_desc_get_irq(desc);
@@ -150,18 +157,9 @@ static void do_sysvec_inband(struct irq_desc *desc, struct pt_regs *regs)
 					regs);
 		break;
 #endif
-#ifdef CONFIG_ACRN_GUEST
 	case HYPERVISOR_CALLBACK_VECTOR:
-		run_sysvec_on_irqstack_cond(__sysvec_acrn_hv_callback,
-					regs);
+		run_sysvec_on_irqstack_cond(pipeline_hv_callback, regs);
 		break;
-#endif
-#ifdef CONFIG_KVM_GUEST
-	case HYPERVISOR_CALLBACK_VECTOR:
-		run_sysvec_on_irqstack_cond(__sysvec_kvm_asyncpf_interrupt,
-					    regs);
-		break;
-#endif
 	case LOCAL_TIMER_VECTOR:
 		run_sysvec_on_irqstack_cond(__sysvec_apic_timer_interrupt,
 					regs);
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 9beb81d32d98..107190bd2309 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -751,7 +751,7 @@ static void __init kvm_guest_init(void)
 
 	if (kvm_para_has_feature(KVM_FEATURE_ASYNC_PF_INT) && kvmapf) {
 		static_branch_enable(&kvm_async_pf_enabled);
-		alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_kvm_asyncpf_interrupt);
+		sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_kvm_asyncpf_interrupt);
 	}
 
 #ifdef CONFIG_SMP
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 04ff194fecf4..7d0c56f6bf90 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -2213,7 +2213,7 @@ static __init void xen_alloc_callback_vector(void)
 		return;
 
 	pr_info("Xen HVM callback vector for event delivery is enabled\n");
-	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_xen_hvm_callback);
+	sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_xen_hvm_callback);
 }
 #else
 void xen_setup_callback_vector(void) {}
-- 
2.39.2


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

* [PATCH dovetail 5.10] x86: dovetail: Allow multiple HV guest extensions to be enabled
  2024-08-23  9:18 [PATCH dovetail 6.10] x86: dovetail: Allow multiple HV guest extensions to be enabled Florian Bezdeka
                   ` (2 preceding siblings ...)
  2024-08-23  9:18 ` [PATCH dovetail 5.15] " Florian Bezdeka
@ 2024-08-23  9:18 ` Florian Bezdeka
  2024-08-23  9:46 ` [PATCH dovetail 6.10] " Florian Bezdeka
  2024-09-05 16:06 ` Philippe Gerum
  5 siblings, 0 replies; 10+ messages in thread
From: Florian Bezdeka @ 2024-08-23  9:18 UTC (permalink / raw)
  To: xenomai; +Cc: Philippe Gerum, Jan Kiszka, Ionut Nechita, Florian Bezdeka

do_sysvec_inband() assumed that only one of CONFIG_ACRN_GUEST and
CONFIG_KVM_GUEST is active at the same time. If both were enabled we
run into a compile issue:

arch/x86/kernel/irq_pipeline.c: In function 'do_sysvec_inband':
arch/x86/kernel/irq_pipeline.c:160:9: error: duplicate case value
   160 |         case HYPERVISOR_CALLBACK_VECTOR:
       |         ^~~~
arch/x86/kernel/irq_pipeline.c:154:9: note: previously used here
   154 |         case HYPERVISOR_CALLBACK_VECTOR:
       |         ^~~~

As only one hypervisor (HV) can be active at the same time, but support
for more than one can be compiled in, we have to migrate from a build
time decision to a runtime based one.

We introduce sysvec_install() macro - like modern kernels already do -
and call a dovetail specific pipelined_sysvec_install() before
registering the IDT gate.

All HYPERVISOR_CALLBACK_VECTOR related callsites of alloc_intr_gate()
have been migrated to sysvec_install(). sysvec_install() is able to
distinguish between the low level entry point (asm_ variant) and the
higher entry point (__ variant).

Reported-by: Ionut Nechita <ionut.nechita@intel.com>
Closes: https://lore.kernel.org/xenomai/PH0PR11MB59498BE915597AEF0B5BBB30F4B42@PH0PR11MB5949.namprd11.prod.outlook.com/

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 arch/x86/include/asm/idtentry.h  | 15 +++++++++++++++
 arch/x86/kernel/cpu/acrn.c       |  2 +-
 arch/x86/kernel/cpu/mshyperv.c   |  2 +-
 arch/x86/kernel/irq_pipeline.c   | 19 +++----------------
 arch/x86/kernel/kvm.c            |  2 +-
 drivers/xen/events/events_base.c |  2 +-
 6 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/arch/x86/include/asm/idtentry.h b/arch/x86/include/asm/idtentry.h
index e60cc4c79541..e636b0b1859e 100644
--- a/arch/x86/include/asm/idtentry.h
+++ b/arch/x86/include/asm/idtentry.h
@@ -224,6 +224,13 @@ __visible __noreturn void __##func(struct pt_regs *regs)
 #define DEFINE_IDTENTRY_SYSVEC_SIMPLE_PIPELINED(vector, func)		\
 	DEFINE_IDTENTRY_SYSVEC_PIPELINED(vector, func)
 
+extern void (*pipeline_hv_callback_fn)(struct pt_regs *regs);
+static inline void pipeline_install_sysvec(int vector, const void *function)
+{
+	if (vector == HYPERVISOR_CALLBACK_VECTOR)
+		pipeline_hv_callback_fn = function;
+}
+
 #else  /* !CONFIG_IRQ_PIPELINE */
 
 #define DECLARE_IDTENTRY_SYSVEC_PIPELINED(vector, func)		DECLARE_IDTENTRY_SYSVEC(vector, func)
@@ -264,6 +271,8 @@ __visible noinstr void func(struct pt_regs *regs,			\
 									\
 static __always_inline void __##func(struct pt_regs *regs, u8 vector)
 
+static inline void pipeline_install_sysvec(vector, function) {}
+
 #endif	/* !CONFIG_IRQ_PIPELINE */
 
 /**
@@ -483,6 +492,12 @@ __visible noinstr void func(struct pt_regs *regs,			\
 #define DEFINE_IDTENTRY_DEBUG_USER	DEFINE_IDTENTRY_NOIST
 #endif
 
+#define sysvec_install(vector, function)                       \
+	{                                                      \
+		pipeline_install_sysvec(vector, __##function); \
+		alloc_intr_gate(vector, asm_##function);       \
+	}
+
 #else /* !__ASSEMBLY__ */
 
 /*
diff --git a/arch/x86/kernel/cpu/acrn.c b/arch/x86/kernel/cpu/acrn.c
index 7f0694b812e6..c9eca7840143 100644
--- a/arch/x86/kernel/cpu/acrn.c
+++ b/arch/x86/kernel/cpu/acrn.c
@@ -25,7 +25,7 @@ static u32 __init acrn_detect(void)
 static void __init acrn_init_platform(void)
 {
 	/* Setup the IDT for ACRN hypervisor callback */
-	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_acrn_hv_callback);
+	sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_acrn_hv_callback);
 }
 
 static bool acrn_x2apic_available(void)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index ffa7ff0bd10d..e9f5fb02c27c 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -350,7 +350,7 @@ static void __init ms_hyperv_init_platform(void)
 	x86_platform.apic_post_init = hyperv_init;
 	hyperv_setup_mmu_ops();
 	/* Setup the IDT for hypervisor callback */
-	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_hyperv_callback);
+	sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_hyperv_callback);
 
 	/* Setup the IDT for reenlightenment notifications */
 	if (ms_hyperv.features & HV_ACCESS_REENLIGHTENMENT) {
diff --git a/arch/x86/kernel/irq_pipeline.c b/arch/x86/kernel/irq_pipeline.c
index 687a9375d374..2a5909384c09 100644
--- a/arch/x86/kernel/irq_pipeline.c
+++ b/arch/x86/kernel/irq_pipeline.c
@@ -14,6 +14,8 @@
 #include <asm/mshyperv.h>
 #include <asm/idtentry.h>
 
+void (*pipeline_hv_callback_fn)(struct pt_regs *regs) = NULL;
+
 static struct irq_domain *sipic_domain;
 
 static void sipic_irq_noop(struct irq_data *data) { }
@@ -117,9 +119,6 @@ static void do_sysvec_inband(struct irq_desc *desc)
 		break;
 #endif
 #ifdef CONFIG_HYPERV
-	case HYPERVISOR_CALLBACK_VECTOR:
-		__sysvec_hyperv_callback(regs);
-		break;
 	case HYPERV_REENLIGHTENMENT_VECTOR:
 		__sysvec_hyperv_reenlightenment(regs);
 		break;
@@ -127,21 +126,9 @@ static void do_sysvec_inband(struct irq_desc *desc)
 		__sysvec_hyperv_stimer0(regs);
 		break;
 #endif
-#ifdef CONFIG_ACRN_GUEST
-	case HYPERVISOR_CALLBACK_VECTOR:
-		__sysvec_acrn_hv_callback(regs);
-		break;
-#endif
-#ifdef CONFIG_XEN_PVHVM
 	case HYPERVISOR_CALLBACK_VECTOR:
-		__sysvec_xen_hvm_callback(regs);
+		pipeline_hv_callback_fn(regs);
 		break;
-#endif
-#ifdef CONFIG_KVM_GUEST
-	case HYPERVISOR_CALLBACK_VECTOR:
-		__sysvec_kvm_asyncpf_interrupt(regs);
-		break;
-#endif
 	case LOCAL_TIMER_VECTOR:
 		__sysvec_apic_timer_interrupt(regs);
 		break;
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 82e929abb7bb..8745e48966e0 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -740,7 +740,7 @@ static void __init kvm_guest_init(void)
 
 	if (kvm_para_has_feature(KVM_FEATURE_ASYNC_PF_INT) && kvmapf) {
 		static_branch_enable(&kvm_async_pf_enabled);
-		alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_kvm_asyncpf_interrupt);
+		sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_kvm_asyncpf_interrupt);
 	}
 
 #ifdef CONFIG_SMP
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index f608663bdfd5..ef80482fcfe4 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -2132,7 +2132,7 @@ static __init void xen_alloc_callback_vector(void)
 		return;
 
 	pr_info("Xen HVM callback vector for event delivery is enabled\n");
-	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_xen_hvm_callback);
+	sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_xen_hvm_callback);
 }
 #else
 void xen_setup_callback_vector(void) {}
-- 
2.39.2


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

* Re: [PATCH dovetail 6.10] x86: dovetail: Allow multiple HV guest extensions to be enabled
  2024-08-23  9:18 [PATCH dovetail 6.10] x86: dovetail: Allow multiple HV guest extensions to be enabled Florian Bezdeka
                   ` (3 preceding siblings ...)
  2024-08-23  9:18 ` [PATCH dovetail 5.10] " Florian Bezdeka
@ 2024-08-23  9:46 ` Florian Bezdeka
  2024-09-05 16:06 ` Philippe Gerum
  5 siblings, 0 replies; 10+ messages in thread
From: Florian Bezdeka @ 2024-08-23  9:46 UTC (permalink / raw)
  To: Philippe Gerum; +Cc: xenomai

On Fri, 2024-08-23 at 11:18 +0200, Florian Bezdeka wrote:
> do_sysvec_inband() assumed that only one of CONFIG_ACRN_GUEST and
> CONFIG_KVM_GUEST is active at the same time. If both were enabled we
> run into a compile issue:
> 
> arch/x86/kernel/irq_pipeline.c: In function 'do_sysvec_inband':
> arch/x86/kernel/irq_pipeline.c:160:9: error: duplicate case value
>    160 |         case HYPERVISOR_CALLBACK_VECTOR:
>        |         ^~~~
> arch/x86/kernel/irq_pipeline.c:154:9: note: previously used here
>    154 |         case HYPERVISOR_CALLBACK_VECTOR:
>        |         ^~~~
> 
> As only one hypervisor (HV) can be active at the same time, but support
> for more than one can be compiled in, we have to migrate from a build
> time decision to a runtime based one.
> 
> We introduce sysvec_install() macro - like modern kernels already do -
> and call a dovetail specific pipelined_sysvec_install() before
> registering the IDT gate.

@Philippe: Feel free to drop this ^^ paragraph for the 6.10 version. It
only applies to the backport versions (6.6 and below). Sorry for
that...

> 
> All HYPERVISOR_CALLBACK_VECTOR related callsites of alloc_intr_gate()
> have been migrated to sysvec_install(). sysvec_install() is able to
> distinguish between the low level entry point (asm_ variant) and the
> higher entry point (__ variant).
> 
> The trampoline function (pipeline_hv_callback()) is needed to statisfy
> type checks done by run_sysvec_on_irqstack_cond().
> 
> Reported-by: Ionut Nechita <ionut.nechita@intel.com>
> Closes: https://lore.kernel.org/xenomai/PH0PR11MB59498BE915597AEF0B5BBB30F4B42@PH0PR11MB5949.namprd11.prod.outlook.com/
> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> ---


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

* Re: [PATCH dovetail 6.10] x86: dovetail: Allow multiple HV guest extensions to be enabled
  2024-08-23  9:18 [PATCH dovetail 6.10] x86: dovetail: Allow multiple HV guest extensions to be enabled Florian Bezdeka
                   ` (4 preceding siblings ...)
  2024-08-23  9:46 ` [PATCH dovetail 6.10] " Florian Bezdeka
@ 2024-09-05 16:06 ` Philippe Gerum
  2024-09-09 14:51   ` Florian Bezdeka
  5 siblings, 1 reply; 10+ messages in thread
From: Philippe Gerum @ 2024-09-05 16:06 UTC (permalink / raw)
  To: Florian Bezdeka; +Cc: xenomai, Jan Kiszka, Ionut Nechita

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

> do_sysvec_inband() assumed that only one of CONFIG_ACRN_GUEST and
> CONFIG_KVM_GUEST is active at the same time. If both were enabled we
> run into a compile issue:
>
> arch/x86/kernel/irq_pipeline.c: In function 'do_sysvec_inband':
> arch/x86/kernel/irq_pipeline.c:160:9: error: duplicate case value
>    160 |         case HYPERVISOR_CALLBACK_VECTOR:
>        |         ^~~~
> arch/x86/kernel/irq_pipeline.c:154:9: note: previously used here
>    154 |         case HYPERVISOR_CALLBACK_VECTOR:
>        |         ^~~~
>
> As only one hypervisor (HV) can be active at the same time, but support
> for more than one can be compiled in, we have to migrate from a build
> time decision to a runtime based one.
>
> We introduce sysvec_install() macro - like modern kernels already do -
> and call a dovetail specific pipelined_sysvec_install() before
> registering the IDT gate.
>
> All HYPERVISOR_CALLBACK_VECTOR related callsites of alloc_intr_gate()
> have been migrated to sysvec_install(). sysvec_install() is able to
> distinguish between the low level entry point (asm_ variant) and the
> higher entry point (__ variant).
>
> The trampoline function (pipeline_hv_callback()) is needed to statisfy
> type checks done by run_sysvec_on_irqstack_cond().
>
> Reported-by: Ionut Nechita <ionut.nechita@intel.com>
> Closes: https://lore.kernel.org/xenomai/PH0PR11MB59498BE915597AEF0B5BBB30F4B42@PH0PR11MB5949.namprd11.prod.outlook.com/
> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> ---
>
> Hi all,
>
> changes compared to the RFC:
> - Changed title
> - Some renamings, mainly taking care of the feedback provided by
>   Philippe
> - Added backports for all dovetail/Linux LTS versions
>
> All patches are build tested and runtime tested for KVM guests. I have
> no other HV / VMM at hand.
>
> Ionut, it would be very nice if you could test at least one version for
> ACRN. Thanks!
>
> Best regards,
> Florian
>
>  arch/x86/include/asm/idtentry.h | 10 ++++++++++
>  arch/x86/kernel/irq_pipeline.c  | 18 ++++++++----------
>  2 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/include/asm/idtentry.h b/arch/x86/include/asm/idtentry.h
> index 524cb93afc34..e7d434e2ace0 100644
> --- a/arch/x86/include/asm/idtentry.h
> +++ b/arch/x86/include/asm/idtentry.h
> @@ -235,6 +235,13 @@ __visible __noreturn void __##func(struct pt_regs *regs)
>  #define DEFINE_IDTENTRY_SYSVEC_SIMPLE_PIPELINED(vector, func)		\
>  	DEFINE_IDTENTRY_SYSVEC_PIPELINED(vector, func)
>  
> +extern void (*pipeline_hv_callback_fn)(struct pt_regs *regs);
> +static inline void pipeline_install_sysvec(int vector, const void *function)
> +{
> +	if (vector == HYPERVISOR_CALLBACK_VECTOR)
> +		pipeline_hv_callback_fn = function;
> +}
> +
>  #else  /* !CONFIG_IRQ_PIPELINE */
>  
>  #define DECLARE_IDTENTRY_SYSVEC_PIPELINED(vector, func)		DECLARE_IDTENTRY_SYSVEC(vector, func)
> @@ -274,6 +281,8 @@ __visible noinstr void func(struct pt_regs *regs,			\
>  									\
>  static noinline void __##func(struct pt_regs *regs, u32 vector)
>  
> +static inline void pipeline_install_sysvec(vector, function) {}
> +
>  #endif	/* !CONFIG_IRQ_PIPELINE */
>  
>  /**
> @@ -523,6 +532,7 @@ static inline void fred_install_sysvec(unsigned int vector, const idtentry_t fun
>  #endif
>  
>  #define sysvec_install(vector, function) {				\
> +	pipeline_install_sysvec(vector, __##function);			\
>  	if (cpu_feature_enabled(X86_FEATURE_FRED))			\
>  		fred_install_sysvec(vector, function);			\
>  	else								\
> diff --git a/arch/x86/kernel/irq_pipeline.c b/arch/x86/kernel/irq_pipeline.c
> index 73e97fba55e7..7a64af5dab73 100644
> --- a/arch/x86/kernel/irq_pipeline.c
> +++ b/arch/x86/kernel/irq_pipeline.c
> @@ -14,6 +14,8 @@
>  #include <asm/mshyperv.h>
>  #include <asm/idtentry.h>
>  
> +void (*pipeline_hv_callback_fn)(struct pt_regs *regs) = NULL;
> +
>  static struct irq_domain *sipic_domain;
>  
>  static void sipic_irq_noop(struct irq_data *data) { }
> @@ -76,6 +78,11 @@ static void pipeline_exit_rcu(irqentry_state_t state)
>  		ct_irq_exit();
>  }
>  
> +static void pipeline_hv_callback(struct pt_regs *regs)
> +{
> +	pipeline_hv_callback_fn(regs);
> +}
> +
>  static void do_sysvec_inband(struct irq_desc *desc, struct pt_regs *regs)
>  {
>  	unsigned int irq = irq_desc_get_irq(desc);
> @@ -150,18 +157,9 @@ static void do_sysvec_inband(struct irq_desc *desc, struct pt_regs *regs)
>  					regs);
>  		break;
>  #endif
> -#ifdef CONFIG_ACRN_GUEST
>  	case HYPERVISOR_CALLBACK_VECTOR:
> -		run_sysvec_on_irqstack_cond(__sysvec_acrn_hv_callback,
> -					regs);
> +		run_sysvec_on_irqstack_cond(pipeline_hv_callback, regs);
>  		break;
> -#endif
> -#ifdef CONFIG_KVM_GUEST
> -	case HYPERVISOR_CALLBACK_VECTOR:
> -		run_sysvec_on_irqstack_cond(__sysvec_kvm_asyncpf_interrupt,
> -					    regs);
> -		break;
> -#endif
>  	case LOCAL_TIMER_VECTOR:
>  		run_sysvec_on_irqstack_cond(__sysvec_apic_timer_interrupt,
>  					regs);

6.1.y, 6.6.y and 6.10 series merged, thanks.

-- 
Philippe.

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

* Re: [PATCH dovetail 5.15] x86: dovetail: Allow multiple HV guest extensions to be enabled
  2024-08-23  9:18 ` [PATCH dovetail 5.15] " Florian Bezdeka
@ 2024-09-06 11:11   ` Jan Kiszka
  0 siblings, 0 replies; 10+ messages in thread
From: Jan Kiszka @ 2024-09-06 11:11 UTC (permalink / raw)
  To: Florian Bezdeka, xenomai; +Cc: Philippe Gerum, Ionut Nechita

On 23.08.24 11:18, Florian Bezdeka wrote:
> do_sysvec_inband() assumed that only one of CONFIG_ACRN_GUEST and
> CONFIG_KVM_GUEST is active at the same time. If both were enabled we
> run into a compile issue:
> 
> arch/x86/kernel/irq_pipeline.c: In function 'do_sysvec_inband':
> arch/x86/kernel/irq_pipeline.c:160:9: error: duplicate case value
>    160 |         case HYPERVISOR_CALLBACK_VECTOR:
>        |         ^~~~
> arch/x86/kernel/irq_pipeline.c:154:9: note: previously used here
>    154 |         case HYPERVISOR_CALLBACK_VECTOR:
>        |         ^~~~
> 
> As only one hypervisor (HV) can be active at the same time, but support
> for more than one can be compiled in, we have to migrate from a build
> time decision to a runtime based one.
> 
> We introduce sysvec_install() macro - like modern kernels already do -
> and call a dovetail specific pipelined_sysvec_install() before
> registering the IDT gate.
> 
> All HYPERVISOR_CALLBACK_VECTOR related callsites of alloc_intr_gate()
> have been migrated to sysvec_install(). sysvec_install() is able to
> distinguish between the low level entry point (asm_ variant) and the
> higher entry point (__ variant).
> 
> The trampoline function (pipeline_hv_callback()) is needed to statisfy
> type checks done by run_sysvec_on_irqstack_cond().
> 
> Reported-by: Ionut Nechita <ionut.nechita@intel.com>
> Closes: https://lore.kernel.org/xenomai/PH0PR11MB59498BE915597AEF0B5BBB30F4B42@PH0PR11MB5949.namprd11.prod.outlook.com/
> 
> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> ---
>  arch/x86/include/asm/idtentry.h  | 15 +++++++++++++++
>  arch/x86/kernel/cpu/acrn.c       |  2 +-
>  arch/x86/kernel/cpu/mshyperv.c   |  2 +-
>  arch/x86/kernel/irq_pipeline.c   | 18 ++++++++----------
>  arch/x86/kernel/kvm.c            |  2 +-
>  drivers/xen/events/events_base.c |  2 +-
>  6 files changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/x86/include/asm/idtentry.h b/arch/x86/include/asm/idtentry.h
> index 1f277e852c4a..5d1140bb5acc 100644
> --- a/arch/x86/include/asm/idtentry.h
> +++ b/arch/x86/include/asm/idtentry.h
> @@ -218,6 +218,13 @@ __visible __noreturn void __##func(struct pt_regs *regs)
>  #define DEFINE_IDTENTRY_SYSVEC_SIMPLE_PIPELINED(vector, func)		\
>  	DEFINE_IDTENTRY_SYSVEC_PIPELINED(vector, func)
>  
> +extern void (*pipeline_hv_callback_fn)(struct pt_regs *regs);
> +static inline void pipeline_install_sysvec(int vector, const void *function)
> +{
> +	if (vector == HYPERVISOR_CALLBACK_VECTOR)
> +		pipeline_hv_callback_fn = function;
> +}
> +
>  #else  /* !CONFIG_IRQ_PIPELINE */
>  
>  #define DECLARE_IDTENTRY_SYSVEC_PIPELINED(vector, func)		DECLARE_IDTENTRY_SYSVEC(vector, func)
> @@ -257,6 +264,8 @@ __visible noinstr void func(struct pt_regs *regs,			\
>  									\
>  static noinline void __##func(struct pt_regs *regs, u32 vector)
>  
> +static inline void pipeline_install_sysvec(vector, function) {}
> +
>  #endif	/* !CONFIG_IRQ_PIPELINE */
>  
>  /**
> @@ -474,6 +483,12 @@ __visible noinstr void func(struct pt_regs *regs,			\
>  #define DEFINE_IDTENTRY_DEBUG_USER	DEFINE_IDTENTRY_NOIST
>  #endif
>  
> +#define sysvec_install(vector, function)                       \
> +	{                                                      \
> +		pipeline_install_sysvec(vector, __##function); \
> +		alloc_intr_gate(vector, asm_##function);       \
> +	}
> +
>  #else /* !__ASSEMBLY__ */
>  
>  /*
> diff --git a/arch/x86/kernel/cpu/acrn.c b/arch/x86/kernel/cpu/acrn.c
> index 0c9d6953f89a..3b02a4fbec97 100644
> --- a/arch/x86/kernel/cpu/acrn.c
> +++ b/arch/x86/kernel/cpu/acrn.c
> @@ -27,7 +27,7 @@ static u32 __init acrn_detect(void)
>  static void __init acrn_init_platform(void)
>  {
>  	/* Setup the IDT for ACRN hypervisor callback */
> -	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_acrn_hv_callback);
> +	sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_acrn_hv_callback);
>  }
>  
>  static bool acrn_x2apic_available(void)
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 53ded40f3a48..01242d5c204a 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -406,7 +406,7 @@ static void __init ms_hyperv_init_platform(void)
>  	x86_platform.apic_post_init = hyperv_init;
>  	hyperv_setup_mmu_ops();
>  	/* Setup the IDT for hypervisor callback */
> -	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_hyperv_callback);
> +	sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_hyperv_callback);
>  
>  	/* Setup the IDT for reenlightenment notifications */
>  	if (ms_hyperv.features & HV_ACCESS_REENLIGHTENMENT) {
> diff --git a/arch/x86/kernel/irq_pipeline.c b/arch/x86/kernel/irq_pipeline.c
> index 1332f328a45b..921898cbc7b3 100644
> --- a/arch/x86/kernel/irq_pipeline.c
> +++ b/arch/x86/kernel/irq_pipeline.c
> @@ -14,6 +14,8 @@
>  #include <asm/mshyperv.h>
>  #include <asm/idtentry.h>
>  
> +void (*pipeline_hv_callback_fn)(struct pt_regs *regs) = NULL;
> +
>  static struct irq_domain *sipic_domain;
>  
>  static void sipic_irq_noop(struct irq_data *data) { }
> @@ -76,6 +78,11 @@ static void pipeline_exit_rcu(irqentry_state_t state)
>  		rcu_irq_exit();
>  }
>  
> +static void pipeline_hv_callback(struct pt_regs *regs)
> +{
> +	pipeline_hv_callback_fn(regs);
> +}
> +
>  static void do_sysvec_inband(struct irq_desc *desc, struct pt_regs *regs)
>  {
>  	unsigned int irq = irq_desc_get_irq(desc);
> @@ -150,18 +157,9 @@ static void do_sysvec_inband(struct irq_desc *desc, struct pt_regs *regs)
>  					regs);
>  		break;
>  #endif
> -#ifdef CONFIG_ACRN_GUEST
>  	case HYPERVISOR_CALLBACK_VECTOR:
> -		run_sysvec_on_irqstack_cond(__sysvec_acrn_hv_callback,
> -					regs);
> +		run_sysvec_on_irqstack_cond(pipeline_hv_callback, regs);
>  		break;
> -#endif
> -#ifdef CONFIG_KVM_GUEST
> -	case HYPERVISOR_CALLBACK_VECTOR:
> -		run_sysvec_on_irqstack_cond(__sysvec_kvm_asyncpf_interrupt,
> -					    regs);
> -		break;
> -#endif
>  	case LOCAL_TIMER_VECTOR:
>  		run_sysvec_on_irqstack_cond(__sysvec_apic_timer_interrupt,
>  					regs);
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 9beb81d32d98..107190bd2309 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -751,7 +751,7 @@ static void __init kvm_guest_init(void)
>  
>  	if (kvm_para_has_feature(KVM_FEATURE_ASYNC_PF_INT) && kvmapf) {
>  		static_branch_enable(&kvm_async_pf_enabled);
> -		alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_kvm_asyncpf_interrupt);
> +		sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_kvm_asyncpf_interrupt);
>  	}
>  
>  #ifdef CONFIG_SMP
> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
> index 04ff194fecf4..7d0c56f6bf90 100644
> --- a/drivers/xen/events/events_base.c
> +++ b/drivers/xen/events/events_base.c
> @@ -2213,7 +2213,7 @@ static __init void xen_alloc_callback_vector(void)
>  		return;
>  
>  	pr_info("Xen HVM callback vector for event delivery is enabled\n");
> -	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_xen_hvm_callback);
> +	sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_xen_hvm_callback);
>  }
>  #else
>  void xen_setup_callback_vector(void) {}

Thanks, merged to 5.10 and 5.15 branches.

Jan

-- 
Siemens AG, Technology
Linux Expert Center


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

* Re: [PATCH dovetail 6.10] x86: dovetail: Allow multiple HV guest extensions to be enabled
  2024-09-05 16:06 ` Philippe Gerum
@ 2024-09-09 14:51   ` Florian Bezdeka
  2024-09-10  9:06     ` Philippe Gerum
  0 siblings, 1 reply; 10+ messages in thread
From: Florian Bezdeka @ 2024-09-09 14:51 UTC (permalink / raw)
  To: Philippe Gerum; +Cc: xenomai, Jan Kiszka


> 
> 6.1.y, 6.6.y and 6.10 series merged, thanks.

Thanks. Might I ask why it was not applied to 6.11? Any problems
detected or just forgotten? 6.11 did not (yet) exist at the time of the
series but the patch for 6.10 version should apply cleanly.

6.11 currently still has the problem that only one HV guest code can be
active at build time.

Florian

> 
> -- 
> Philippe.


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

* Re: [PATCH dovetail 6.10] x86: dovetail: Allow multiple HV guest extensions to be enabled
  2024-09-09 14:51   ` Florian Bezdeka
@ 2024-09-10  9:06     ` Philippe Gerum
  0 siblings, 0 replies; 10+ messages in thread
From: Philippe Gerum @ 2024-09-10  9:06 UTC (permalink / raw)
  To: Florian Bezdeka; +Cc: xenomai, Jan Kiszka

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

>> 
>> 6.1.y, 6.6.y and 6.10 series merged, thanks.
>
> Thanks. Might I ask why it was not applied to 6.11? Any problems
> detected or just forgotten? 6.11 did not (yet) exist at the time of the
> series but the patch for 6.10 version should apply cleanly.
>
> 6.11 currently still has the problem that only one HV guest code can be
> active at build time.
>

This patch is already queued locally for 6.11. It will appear when this
tree is rebased on -rc7 (soon).

-- 
Philippe.

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

end of thread, other threads:[~2024-09-10  9:07 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-23  9:18 [PATCH dovetail 6.10] x86: dovetail: Allow multiple HV guest extensions to be enabled Florian Bezdeka
2024-08-23  9:18 ` [PATCH dovetail 6.6] " Florian Bezdeka
2024-08-23  9:18 ` [PATCH dovetail 6.1] " Florian Bezdeka
2024-08-23  9:18 ` [PATCH dovetail 5.15] " Florian Bezdeka
2024-09-06 11:11   ` Jan Kiszka
2024-08-23  9:18 ` [PATCH dovetail 5.10] " Florian Bezdeka
2024-08-23  9:46 ` [PATCH dovetail 6.10] " Florian Bezdeka
2024-09-05 16:06 ` Philippe Gerum
2024-09-09 14:51   ` Florian Bezdeka
2024-09-10  9:06     ` Philippe Gerum

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.