[PATCH 0/3] make KVM PV use apic through APIs

[PATCH 0/3] make KVM PV use apic through APIs

[Michael S. Tsirkin]

KVM PV EOI optimization overrides eoi_write apic op with its own
version. at Ingo's suggestion, add an API for this and switch kvm to use
it, to avoid meddling with core x86 apic driver data structures
directly.

Ingo, could you please review and let us know whether this
matches what you had in mind?

This patchset is on top of kvm/next since it touches some
of the code modified in that tree (and patch 3 reverts a commit
in that tree). Could x86 some maintainers pls ACK merging this patchset
through the kvm tree?

Michael S. Tsirkin (3):
  apic: add apic_set_eoi_write for PV use
  kvm: switch to apic_set_eoi_write, apic_write
  Revert "apic: fix kvm build on UP without IOAPIC"

 arch/x86/include/asm/apic.h |  8 +++-----
 arch/x86/kernel/apic/apic.c | 17 +++++++++++++++++
 arch/x86/kernel/kvm.c       | 13 +++----------
 3 files changed, 23 insertions(+), 15 deletions(-)

-- 
MST

[PATCH 1/3] apic: add apic_set_eoi_write for PV use

[Michael S. Tsirkin]

KVM PV EOI optimization overrides eoi_write apic op with its own
version. Add an API for this to avoid meddling with core x86 apic driver
data structures directly.

For KVM use, we don't need any guarantees about when the switch to the
new op will take place, so it could in theory use this API after SMP init,
but it currently doesn't, and restricting callers to early init makes it
clear that it's safe as it won't race with actual APIC driver use.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/x86/include/asm/apic.h |  3 +++
 arch/x86/kernel/apic/apic.c | 17 +++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index aa5b2ee..ff8dff6 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -469,6 +469,8 @@ static inline u32 safe_apic_wait_icr_idle(void)
 	return apic->safe_wait_icr_idle();
 }
 
+extern void __init apic_set_eoi_write(void (*eoi_write)(u32 reg, u32 v));
+
 #else /* CONFIG_X86_LOCAL_APIC */
 
 static inline u32 apic_read(u32 reg) { return 0; }
@@ -478,6 +480,7 @@ static inline u64 apic_icr_read(void) { return 0; }
 static inline void apic_icr_write(u32 low, u32 high) { }
 static inline void apic_wait_icr_idle(void) { }
 static inline u32 safe_apic_wait_icr_idle(void) { return 0; }
+static inline void apic_set_eoi_write(void (*eoi_write)(u32 reg, u32 v)) {}
 
 #endif /* CONFIG_X86_LOCAL_APIC */
 
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 39a222e..c7520b6 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2124,6 +2124,23 @@ void default_init_apic_ldr(void)
 }
 
 /*
+ * Override the generic EOI implementation with an optimized version.
+ * Only called during early boot when only one CPU is active and with
+ * interrupts disabled, so we know this does not race with actual APIC driver
+ * use.
+ */
+void __init apic_set_eoi_write(void (*eoi_write)(u32 reg, u32 v))
+{
+	struct apic **drv;
+
+	for (drv = __apicdrivers; drv < __apicdrivers_end; drv++) {
+		/* Should happen once for each apic */
+		WARN_ON((*drv)->eoi_write == eoi_write);
+		(*drv)->eoi_write = eoi_write;
+	}
+}
+
+/*
  * Power management
  */
 #ifdef CONFIG_PM
-- 
MST

[PATCH 2/3] kvm: switch to apic_set_eoi_write, apic_write

[Michael S. Tsirkin]

Use apic_set_eoi_write, apic_write to avoid meedling in core apic
driver data structures directly.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/x86/kernel/kvm.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 75ab94c..60108ce 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -298,7 +298,7 @@ static void kvm_guest_apic_eoi_write(u32 reg, u32 val)
 	 */
 	if (__test_and_clear_bit(KVM_PV_EOI_BIT, &__get_cpu_var(kvm_apic_eoi)))
 		return;
-	apic->write(APIC_EOI, APIC_EOI_ACK);
+	apic_write(APIC_EOI, APIC_EOI_ACK);
 }
 
 void __cpuinit kvm_guest_cpu_init(void)
@@ -465,15 +465,8 @@ void __init kvm_guest_init(void)
 		pv_time_ops.steal_clock = kvm_steal_clock;
 	}
 
-	if (kvm_para_has_feature(KVM_FEATURE_PV_EOI)) {
-		struct apic **drv;
-
-		for (drv = __apicdrivers; drv < __apicdrivers_end; drv++) {
-			/* Should happen once for each apic */
-			WARN_ON((*drv)->eoi_write == kvm_guest_apic_eoi_write);
-			(*drv)->eoi_write = kvm_guest_apic_eoi_write;
-		}
-	}
+	if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
+		apic_set_eoi_write(kvm_guest_apic_eoi_write);
 
 #ifdef CONFIG_SMP
 	smp_ops.smp_prepare_boot_cpu = kvm_smp_prepare_boot_cpu;
-- 
MST

[PATCH 3/3] Revert "apic: fix kvm build on UP without IOAPIC"

[Michael S. Tsirkin]

This reverts commit f9808b7fd422b965cea52e05ba470e0a473c53d3.
After commit 'kvm: switch to apic_set_eoi_write, apic_write'
the stubs are no longer needed as kvm does not look at apicdrivers anymore.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/x86/include/asm/apic.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index ff8dff6..839b8f5 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -417,12 +417,7 @@ extern struct apic *apic;
 	__aligned(sizeof(struct apic *))				\
 	__section(.apicdrivers) = { &sym1, &sym2 }
 
-#ifdef CONFIG_X86_LOCAL_APIC
 extern struct apic *__apicdrivers[], *__apicdrivers_end[];
-#else
-#define __apicdrivers ((struct apic **)NULL)
-#define __apicdrivers_end ((struct apic **)NULL)
-#endif
 
 /*
  * APIC functionality to boot other CPUs - only used on SMP:
-- 
MST

Re: [PATCH 0/3] make KVM PV use apic through APIs

[Avi Kivity]

On 07/15/2012 03:56 PM, Michael S. Tsirkin wrote:
> KVM PV EOI optimization overrides eoi_write apic op with its own
> version. at Ingo's suggestion, add an API for this and switch kvm to use
> it, to avoid meddling with core x86 apic driver data structures
> directly.
> 
> Ingo, could you please review and let us know whether this
> matches what you had in mind?
> 
> This patchset is on top of kvm/next since it touches some
> of the code modified in that tree (and patch 3 reverts a commit
> in that tree). Could x86 some maintainers pls ACK merging this patchset
> through the kvm tree?
> 

Looks okay to me.


-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 1/3] apic: add apic_set_eoi_write for PV use

[Ingo Molnar]

* Michael S. Tsirkin <mst@redhat.com> wrote:

> KVM PV EOI optimization overrides eoi_write apic op with its own
> version. Add an API for this to avoid meddling with core x86 apic driver
> data structures directly.
> 
> For KVM use, we don't need any guarantees about when the switch to the
> new op will take place, so it could in theory use this API after SMP init,
> but it currently doesn't, and restricting callers to early init makes it
> clear that it's safe as it won't race with actual APIC driver use.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  arch/x86/include/asm/apic.h |  3 +++
>  arch/x86/kernel/apic/apic.c | 17 +++++++++++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
> index aa5b2ee..ff8dff6 100644
> --- a/arch/x86/include/asm/apic.h
> +++ b/arch/x86/include/asm/apic.h
> @@ -469,6 +469,8 @@ static inline u32 safe_apic_wait_icr_idle(void)
>  	return apic->safe_wait_icr_idle();
>  }
>  
> +extern void __init apic_set_eoi_write(void (*eoi_write)(u32 reg, u32 v));
> +
>  #else /* CONFIG_X86_LOCAL_APIC */
>  
>  static inline u32 apic_read(u32 reg) { return 0; }
> @@ -478,6 +480,7 @@ static inline u64 apic_icr_read(void) { return 0; }
>  static inline void apic_icr_write(u32 low, u32 high) { }
>  static inline void apic_wait_icr_idle(void) { }
>  static inline u32 safe_apic_wait_icr_idle(void) { return 0; }
> +static inline void apic_set_eoi_write(void (*eoi_write)(u32 reg, u32 v)) {}
>  
>  #endif /* CONFIG_X86_LOCAL_APIC */
>  
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index 39a222e..c7520b6 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -2124,6 +2124,23 @@ void default_init_apic_ldr(void)
>  }
>  
>  /*
> + * Override the generic EOI implementation with an optimized version.
> + * Only called during early boot when only one CPU is active and with
> + * interrupts disabled, so we know this does not race with actual APIC driver
> + * use.
> + */
> +void __init apic_set_eoi_write(void (*eoi_write)(u32 reg, u32 v))
> +{
> +	struct apic **drv;
> +
> +	for (drv = __apicdrivers; drv < __apicdrivers_end; drv++) {
> +		/* Should happen once for each apic */
> +		WARN_ON((*drv)->eoi_write == eoi_write);
> +		(*drv)->eoi_write = eoi_write;
> +	}
> +}
> +

ok, it's better this way.

Acked-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

Re: [PATCH 0/3] make KVM PV use apic through APIs

[Avi Kivity]

On 07/15/2012 03:56 PM, Michael S. Tsirkin wrote:
> KVM PV EOI optimization overrides eoi_write apic op with its own
> version. at Ingo's suggestion, add an API for this and switch kvm to use
> it, to avoid meddling with core x86 apic driver data structures
> directly.

Applied to next, thanks.

-- 
error compiling committee.c: too many arguments to function