All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86: let 32bit use apic_ops too - fix
  2008-07-12  1:41   ` [PATCH] x86: let 32bit use apic_ops too Yinghai Lu
@ 2008-07-14  5:19     ` Yinghai Lu
  2008-07-14  7:12       ` Ingo Molnar
  2008-07-15 17:33       ` Suresh Siddha
  0 siblings, 2 replies; 20+ messages in thread
From: Yinghai Lu @ 2008-07-14  5:19 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Suresh Siddha; +Cc: LKML


fix for pv.

Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>

---
 arch/x86/kernel/paravirt.c |    5 ----
 arch/x86/kernel/vmi_32.c   |   51 ++++++++++++++++++++++++++++++++++++++++++---
 arch/x86/xen/enlighten.c   |   19 +++++++---------
 include/asm-x86/paravirt.h |   29 -------------------------
 4 files changed, 57 insertions(+), 47 deletions(-)

Index: linux-2.6/arch/x86/kernel/paravirt.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/paravirt.c
+++ linux-2.6/arch/x86/kernel/paravirt.c
@@ -373,11 +373,6 @@ struct pv_cpu_ops pv_cpu_ops = {
 
 struct pv_apic_ops pv_apic_ops = {
 #ifdef CONFIG_X86_LOCAL_APIC
-#ifndef CONFIG_X86_64
-	.apic_write = native_apic_mem_write,
-	.apic_write_atomic = native_apic_mem_write_atomic,
-	.apic_read = native_apic_mem_read,
-#endif
 	.setup_boot_clock = setup_boot_APIC_clock,
 	.setup_secondary_clock = setup_secondary_APIC_clock,
 	.startup_ipi_hook = paravirt_nop,
Index: linux-2.6/arch/x86/kernel/vmi_32.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/vmi_32.c
+++ linux-2.6/arch/x86/kernel/vmi_32.c
@@ -676,6 +676,50 @@ static inline int __init probe_vmi_rom(v
 	return 0;
 }
 
+#ifdef CONFIG_X86_LOCAL_APIC
+static u32 vmi_apic_read(u32 reg)
+{
+	return 0;
+}
+
+static void vmi_apic_write(u32 reg, u32 val)
+{
+	/* Warn to see if there's any stray references */
+	WARN_ON(1);
+}
+
+static u64 vmi_apic_icr_read(void)
+{
+	return 0;
+}
+
+static void vmi_apic_icr_write(u32 low, u32 id)
+{
+	/* Warn to see if there's any stray references */
+	WARN_ON(1);
+}
+
+static void vmi_apic_wait_icr_idle(void)
+{
+	return;
+}
+
+static u32 vmi_safe_apic_wait_icr_idle(void)
+{
+	return 0;
+}
+
+static struct apic_ops vmi_basic_apic_ops = {
+        .read = vmi_apic_read,
+        .write = vmi_apic_write,
+        .write_atomic = vmi_apic_write,
+        .icr_read = vmi_apic_icr_read,
+        .icr_write = vmi_apic_icr_write,
+        .wait_icr_idle = vmi_apic_wait_icr_idle,
+        .safe_wait_icr_idle = vmi_safe_apic_wait_icr_idle,
+};
+#endif
+
 /*
  * VMI setup common to all processors
  */
@@ -904,9 +948,10 @@ static inline int __init activate_vmi(vo
 #endif
 
 #ifdef CONFIG_X86_LOCAL_APIC
-	para_fill(pv_apic_ops.apic_read, APICRead);
-	para_fill(pv_apic_ops.apic_write, APICWrite);
-	para_fill(pv_apic_ops.apic_write_atomic, APICWrite);
+	para_fill(vmi_basic_apic_ops.read, APICRead);
+	para_fill(vmi_basic_apic_ops.write, APICWrite);
+	para_fill(vmi_basic_apic_ops.write_atomic, APICWrite);
+	apic_ops = &vmi_basic_apic_ops;
 #endif
 
 	/*
Index: linux-2.6/arch/x86/xen/enlighten.c
===================================================================
--- linux-2.6.orig/arch/x86/xen/enlighten.c
+++ linux-2.6/arch/x86/xen/enlighten.c
@@ -587,7 +587,6 @@ static void xen_apic_write(u32 reg, u32
 	WARN_ON(1);
 }
 
-#ifdef CONFIG_X86_64
 static u64 xen_apic_icr_read(void)
 {
 	return 0;
@@ -604,6 +603,11 @@ static void xen_apic_wait_icr_idle(void)
         return;
 }
 
+static u32 xen_safe_apic_wait_icr_idle(void)
+{
+        return 0;
+}
+
 static struct apic_ops xen_basic_apic_ops = {
 	.read = xen_apic_read,
 	.write = xen_apic_write,
@@ -611,9 +615,8 @@ static struct apic_ops xen_basic_apic_op
 	.icr_read = xen_apic_icr_read,
 	.icr_write = xen_apic_icr_write,
 	.wait_icr_idle = xen_apic_wait_icr_idle,
-	.safe_wait_icr_idle = xen_apic_wait_icr_idle,
+	.safe_wait_icr_idle = xen_safe_apic_wait_icr_idle,
 };
-#endif
 
 #endif
 
@@ -1298,11 +1301,6 @@ static const struct pv_irq_ops xen_irq_o
 
 static const struct pv_apic_ops xen_apic_ops __initdata = {
 #ifdef CONFIG_X86_LOCAL_APIC
-#ifndef CONFIG_X86_64
-	.apic_write = xen_apic_write,
-	.apic_write_atomic = xen_apic_write,
-	.apic_read = xen_apic_read,
-#endif
 	.setup_boot_clock = paravirt_nop,
 	.setup_secondary_clock = paravirt_nop,
 	.startup_ipi_hook = paravirt_nop,
@@ -1704,9 +1702,10 @@ asmlinkage void __init xen_start_kernel(
 	pv_irq_ops = xen_irq_ops;
 	pv_apic_ops = xen_apic_ops;
 	pv_mmu_ops = xen_mmu_ops;
-#ifdef CONFIG_X86_64
+
+#ifdef CONFIG_X86_LOCAL_APIC
 	/*
-	 * for 64bit, set up the basic apic ops aswell.
+	 * set up the basic apic ops.
 	 */
 	apic_ops = &xen_basic_apic_ops;
 #endif
Index: linux-2.6/include/asm-x86/paravirt.h
===================================================================
--- linux-2.6.orig/include/asm-x86/paravirt.h
+++ linux-2.6/include/asm-x86/paravirt.h
@@ -200,15 +200,6 @@ struct pv_irq_ops {
 
 struct pv_apic_ops {
 #ifdef CONFIG_X86_LOCAL_APIC
-#ifndef CONFIG_X86_64
-	/*
-	 * Direct APIC operations, principally for VMI.  Ideally
-	 * these shouldn't be in this interface.
-	 */
-	void (*apic_write)(u32 reg, u32 v);
-	void (*apic_write_atomic)(u32 reg, u32 v);
-	u32 (*apic_read)(u32 reg);
-#endif
 	void (*setup_boot_clock)(void);
 	void (*setup_secondary_clock)(void);
 
@@ -901,26 +892,6 @@ static inline void slow_down_io(void)
 }
 
 #ifdef CONFIG_X86_LOCAL_APIC
-/*
- * Basic functions accessing APICs.
- */
-#ifndef CONFIG_X86_64
-static inline void apic_write(u32 reg, u32 v)
-{
-	PVOP_VCALL2(pv_apic_ops.apic_write, reg, v);
-}
-
-static inline void apic_write_atomic(u32 reg, u32 v)
-{
-	PVOP_VCALL2(pv_apic_ops.apic_write_atomic, reg, v);
-}
-
-static inline u32 apic_read(u32 reg)
-{
-	return PVOP_CALL1(unsigned long, pv_apic_ops.apic_read, reg);
-}
-#endif
-
 static inline void setup_boot_clock(void)
 {
 	PVOP_VCALL0(pv_apic_ops.setup_boot_clock);

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

* Re: [PATCH] x86: let 32bit use apic_ops too - fix
  2008-07-14  5:19     ` [PATCH] x86: let 32bit use apic_ops too - fix Yinghai Lu
@ 2008-07-14  7:12       ` Ingo Molnar
  2008-07-14 16:49         ` Suresh Siddha
  2008-07-15 17:33       ` Suresh Siddha
  1 sibling, 1 reply; 20+ messages in thread
From: Ingo Molnar @ 2008-07-14  7:12 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Thomas Gleixner, H. Peter Anvin, Suresh Siddha, LKML


* Yinghai Lu <yhlu.kernel@gmail.com> wrote:

> fix for pv.
> 
> Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>

applied to tip/x86/x2apic - thanks Yinghai.

	Ingo

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

* Re: [PATCH] x86: let 32bit use apic_ops too - fix
  2008-07-14  7:12       ` Ingo Molnar
@ 2008-07-14 16:49         ` Suresh Siddha
  2008-07-14 17:00           ` Yinghai Lu
  2008-07-18 17:06           ` Ingo Molnar
  0 siblings, 2 replies; 20+ messages in thread
From: Suresh Siddha @ 2008-07-14 16:49 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Yinghai Lu, Thomas Gleixner, H. Peter Anvin, Siddha, Suresh B,
	LKML

On Mon, Jul 14, 2008 at 12:12:07AM -0700, Ingo Molnar wrote:
> 
> * Yinghai Lu <yhlu.kernel@gmail.com> wrote:
> 
> > fix for pv.
> >
> > Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
> 
> applied to tip/x86/x2apic - thanks Yinghai.

Ingo, before you try for the third attempt ;) we need one more lguest apic_ops
fix. Patch appended. Thanks.

---
[patch] x86: apic_ops for lguest

apic_ops for lguest.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Yinghai Lu <yhlu.kernel@gmail.com>
---

Index: x86.git/arch/x86/lguest/boot.c
===================================================================
--- x86.git.orig/arch/x86/lguest/boot.c	2008-07-14 09:44:31.000000000 -0700
+++ x86.git/arch/x86/lguest/boot.c	2008-07-14 09:45:12.000000000 -0700
@@ -791,6 +791,37 @@
 {
 	return 0;
 }
+
+static u64 lguest_apic_icr_read(void)
+{
+	return 0;
+}
+
+static void lguest_apic_icr_write(u32 low, u32 id)
+{
+	/* Warn to see if there's any stray references */
+	WARN_ON(1);
+}
+
+static void lguest_apic_wait_icr_idle(void)
+{
+	return;
+}
+
+static u32 lguest_apic_safe_wait_icr_idle(void)
+{
+	return 0;
+}
+
+static struct apic_ops lguest_basic_apic_ops = {
+	.read = lguest_apic_read,
+	.write = lguest_apic_write,
+	.write_atomic = lguest_apic_write,
+	.icr_read = lguest_apic_icr_read,
+	.icr_write = lguest_apic_icr_write,
+	.wait_icr_idle = lguest_apic_wait_icr_idle,
+	.safe_wait_icr_idle = lguest_apic_safe_wait_icr_idle,
+};
 #endif
 
 /* STOP!  Until an interrupt comes in. */
@@ -990,9 +1021,7 @@
 
 #ifdef CONFIG_X86_LOCAL_APIC
 	/* apic read/write intercepts */
-	pv_apic_ops.apic_write = lguest_apic_write;
-	pv_apic_ops.apic_write_atomic = lguest_apic_write;
-	pv_apic_ops.apic_read = lguest_apic_read;
+	apic_ops = &lguest_basic_apic_ops;
 #endif
 
 	/* time operations */

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

* Re: [PATCH] x86: let 32bit use apic_ops too - fix
  2008-07-14 16:49         ` Suresh Siddha
@ 2008-07-14 17:00           ` Yinghai Lu
  2008-07-14 18:03             ` Suresh Siddha
  2008-07-18 17:06           ` Ingo Molnar
  1 sibling, 1 reply; 20+ messages in thread
From: Yinghai Lu @ 2008-07-14 17:00 UTC (permalink / raw)
  To: Suresh Siddha; +Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, LKML

On Mon, Jul 14, 2008 at 9:49 AM, Suresh Siddha
<suresh.b.siddha@intel.com> wrote:
> On Mon, Jul 14, 2008 at 12:12:07AM -0700, Ingo Molnar wrote:
>>
>> * Yinghai Lu <yhlu.kernel@gmail.com> wrote:
>>
>> > fix for pv.
>> >
>> > Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
>>
>> applied to tip/x86/x2apic - thanks Yinghai.
>
> Ingo, before you try for the third attempt ;) we need one more lguest apic_ops
> fix. Patch appended. Thanks.
>
> ---
> [patch] x86: apic_ops for lguest
>
> apic_ops for lguest.
>
> Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
> Cc: Yinghai Lu <yhlu.kernel@gmail.com>
> ---
>
> Index: x86.git/arch/x86/lguest/boot.c
> ===================================================================
> --- x86.git.orig/arch/x86/lguest/boot.c 2008-07-14 09:44:31.000000000 -0700
> +++ x86.git/arch/x86/lguest/boot.c      2008-07-14 09:45:12.000000000 -0700
> @@ -791,6 +791,37 @@
>  {
>        return 0;
>  }
> +
> +static u64 lguest_apic_icr_read(void)
> +{
> +       return 0;
> +}
> +
> +static void lguest_apic_icr_write(u32 low, u32 id)
> +{
> +       /* Warn to see if there's any stray references */
> +       WARN_ON(1);
> +}
> +
> +static void lguest_apic_wait_icr_idle(void)
> +{
> +       return;
> +}
> +
> +static u32 lguest_apic_safe_wait_icr_idle(void)
> +{
> +       return 0;
> +}
> +
> +static struct apic_ops lguest_basic_apic_ops = {
> +       .read = lguest_apic_read,
> +       .write = lguest_apic_write,
> +       .write_atomic = lguest_apic_write,
> +       .icr_read = lguest_apic_icr_read,
> +       .icr_write = lguest_apic_icr_write,
> +       .wait_icr_idle = lguest_apic_wait_icr_idle,
> +       .safe_wait_icr_idle = lguest_apic_safe_wait_icr_idle,
> +};
>  #endif
>
>  /* STOP!  Until an interrupt comes in. */
> @@ -990,9 +1021,7 @@
>
>  #ifdef CONFIG_X86_LOCAL_APIC
>        /* apic read/write intercepts */
> -       pv_apic_ops.apic_write = lguest_apic_write;
> -       pv_apic_ops.apic_write_atomic = lguest_apic_write;
> -       pv_apic_ops.apic_read = lguest_apic_read;
> +       apic_ops = &lguest_basic_apic_ops;
>  #endif
>
>        /* time operations */

do we need one for KVM pv?

YH

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

* Re: [PATCH] x86: let 32bit use apic_ops too - fix
  2008-07-14 17:00           ` Yinghai Lu
@ 2008-07-14 18:03             ` Suresh Siddha
  0 siblings, 0 replies; 20+ messages in thread
From: Suresh Siddha @ 2008-07-14 18:03 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Siddha, Suresh B, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
	LKML

On Mon, Jul 14, 2008 at 10:00:28AM -0700, Yinghai Lu wrote:
> On Mon, Jul 14, 2008 at 9:49 AM, Suresh Siddha
> <suresh.b.siddha@intel.com> wrote:
> > [patch] x86: apic_ops for lguest
> >
> > apic_ops for lguest.
> >
> > Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
> > Cc: Yinghai Lu <yhlu.kernel@gmail.com>
> 
> do we need one for KVM pv?

No. They use different op's for complete apic virt.

thanks,
suresh

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

* Re: [PATCH] x86: let 32bit use apic_ops too - fix
  2008-07-14  5:19     ` [PATCH] x86: let 32bit use apic_ops too - fix Yinghai Lu
  2008-07-14  7:12       ` Ingo Molnar
@ 2008-07-15 17:33       ` Suresh Siddha
  2008-07-15 18:10         ` Yinghai Lu
  2008-07-18 17:07         ` Ingo Molnar
  1 sibling, 2 replies; 20+ messages in thread
From: Suresh Siddha @ 2008-07-15 17:33 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Siddha, Suresh B,
	LKML

On Sun, Jul 13, 2008 at 10:19:35PM -0700, Yinghai Lu wrote:
> 
> fix for pv.
> 
> Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
> 
> ---
>  arch/x86/kernel/paravirt.c |    5 ----
>  arch/x86/kernel/vmi_32.c   |   51 ++++++++++++++++++++++++++++++++++++++++++---
>  arch/x86/xen/enlighten.c   |   19 +++++++---------
>  include/asm-x86/paravirt.h |   29 -------------------------
>  4 files changed, 57 insertions(+), 47 deletions(-)
> 
> Index: linux-2.6/arch/x86/kernel/paravirt.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/paravirt.c
> +++ linux-2.6/arch/x86/kernel/paravirt.c
> @@ -373,11 +373,6 @@ struct pv_cpu_ops pv_cpu_ops = {
> 
>  struct pv_apic_ops pv_apic_ops = {
>  #ifdef CONFIG_X86_LOCAL_APIC
> -#ifndef CONFIG_X86_64
> -       .apic_write = native_apic_mem_write,
> -       .apic_write_atomic = native_apic_mem_write_atomic,
> -       .apic_read = native_apic_mem_read,
> -#endif
>         .setup_boot_clock = setup_boot_APIC_clock,
>         .setup_secondary_clock = setup_secondary_APIC_clock,
>         .startup_ipi_hook = paravirt_nop,
> Index: linux-2.6/arch/x86/kernel/vmi_32.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/vmi_32.c
> +++ linux-2.6/arch/x86/kernel/vmi_32.c
> @@ -676,6 +676,50 @@ static inline int __init probe_vmi_rom(v
>         return 0;
>  }
> 
> +#ifdef CONFIG_X86_LOCAL_APIC
> +static u32 vmi_apic_read(u32 reg)
> +{
> +       return 0;
> +}
> +
> +static void vmi_apic_write(u32 reg, u32 val)
> +{
> +       /* Warn to see if there's any stray references */
> +       WARN_ON(1);
> +}
> +
> +static u64 vmi_apic_icr_read(void)
> +{
> +       return 0;
> +}
> +
> +static void vmi_apic_icr_write(u32 low, u32 id)
> +{
> +       /* Warn to see if there's any stray references */
> +       WARN_ON(1);
> +}
> +
> +static void vmi_apic_wait_icr_idle(void)
> +{
> +       return;
> +}
> +
> +static u32 vmi_safe_apic_wait_icr_idle(void)
> +{
> +       return 0;
> +}
> +
> +static struct apic_ops vmi_basic_apic_ops = {
> +        .read = vmi_apic_read,
> +        .write = vmi_apic_write,
> +        .write_atomic = vmi_apic_write,
> +        .icr_read = vmi_apic_icr_read,
> +        .icr_write = vmi_apic_icr_write,
> +        .wait_icr_idle = vmi_apic_wait_icr_idle,
> +        .safe_wait_icr_idle = vmi_safe_apic_wait_icr_idle,
> +};
> +#endif
> +
>  /*
>   * VMI setup common to all processors
>   */
> @@ -904,9 +948,10 @@ static inline int __init activate_vmi(vo
>  #endif
> 
>  #ifdef CONFIG_X86_LOCAL_APIC
> -       para_fill(pv_apic_ops.apic_read, APICRead);
> -       para_fill(pv_apic_ops.apic_write, APICWrite);
> -       para_fill(pv_apic_ops.apic_write_atomic, APICWrite);
> +       para_fill(vmi_basic_apic_ops.read, APICRead);
> +       para_fill(vmi_basic_apic_ops.write, APICWrite);
> +       para_fill(vmi_basic_apic_ops.write_atomic, APICWrite);
> +       apic_ops = &vmi_basic_apic_ops;

Yinghai, Looking more closely at this, based on my understanding this might be
wrong for VMI. Correct patch should be as follows. Any comments?

thanks,
suresh
---
Fix VMI apic_ops.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
---

diff --git a/arch/x86/kernel/apic_32.c b/arch/x86/kernel/apic_32.c
index b1375fa..3410196 100644
--- a/arch/x86/kernel/apic_32.c
+++ b/arch/x86/kernel/apic_32.c
@@ -145,6 +145,11 @@ static int modern_apic(void)
 	return lapic_get_version() >= 0x14;
 }
 
+/*
+ * Paravirt kernels also might be using these below ops. So we still
+ * use generic apic_read()/apic_write(), which might be pointing to different
+ * ops in PARAVIRT case.
+ */
 void xapic_wait_icr_idle(void)
 {
 	while (apic_read(APIC_ICR) & APIC_ICR_BUSY)
diff --git a/arch/x86/kernel/vmi_32.c b/arch/x86/kernel/vmi_32.c
index cf30743..d6897e4 100644
--- a/arch/x86/kernel/vmi_32.c
+++ b/arch/x86/kernel/vmi_32.c
@@ -676,50 +676,6 @@ static inline int __init probe_vmi_rom(void)
 	return 0;
 }
 
-#ifdef CONFIG_X86_LOCAL_APIC
-static u32 vmi_apic_read(u32 reg)
-{
-	return 0;
-}
-
-static void vmi_apic_write(u32 reg, u32 val)
-{
-	/* Warn to see if there's any stray references */
-	WARN_ON(1);
-}
-
-static u64 vmi_apic_icr_read(void)
-{
-	return 0;
-}
-
-static void vmi_apic_icr_write(u32 low, u32 id)
-{
-	/* Warn to see if there's any stray references */
-	WARN_ON(1);
-}
-
-static void vmi_apic_wait_icr_idle(void)
-{
-	return;
-}
-
-static u32 vmi_safe_apic_wait_icr_idle(void)
-{
-	return 0;
-}
-
-static struct apic_ops vmi_basic_apic_ops = {
-        .read = vmi_apic_read,
-        .write = vmi_apic_write,
-        .write_atomic = vmi_apic_write,
-        .icr_read = vmi_apic_icr_read,
-        .icr_write = vmi_apic_icr_write,
-        .wait_icr_idle = vmi_apic_wait_icr_idle,
-        .safe_wait_icr_idle = vmi_safe_apic_wait_icr_idle,
-};
-#endif
-
 /*
  * VMI setup common to all processors
  */
@@ -948,10 +904,9 @@ static inline int __init activate_vmi(void)
 #endif
 
 #ifdef CONFIG_X86_LOCAL_APIC
-	para_fill(vmi_basic_apic_ops.read, APICRead);
-	para_fill(vmi_basic_apic_ops.write, APICWrite);
-	para_fill(vmi_basic_apic_ops.write_atomic, APICWrite);
-	apic_ops = &vmi_basic_apic_ops;
+	para_fill(apic_ops->read, APICRead);
+	para_fill(apic_ops->write, APICWrite);
+	para_fill(apic_ops->write_atomic, APICWrite);
 #endif
 
 	/*

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

* Re: [PATCH] x86: let 32bit use apic_ops too - fix
  2008-07-15 17:33       ` Suresh Siddha
@ 2008-07-15 18:10         ` Yinghai Lu
  2008-07-15 18:27           ` Suresh Siddha
  2008-07-18 17:07         ` Ingo Molnar
  1 sibling, 1 reply; 20+ messages in thread
From: Yinghai Lu @ 2008-07-15 18:10 UTC (permalink / raw)
  To: Suresh Siddha; +Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, LKML

On Tue, Jul 15, 2008 at 10:33 AM, Suresh Siddha
<suresh.b.siddha@intel.com> wrote:
> On Sun, Jul 13, 2008 at 10:19:35PM -0700, Yinghai Lu wrote:
>>
>> fix for pv.
>>
>> Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
>>
>> ---
>>  arch/x86/kernel/paravirt.c |    5 ----
>>  arch/x86/kernel/vmi_32.c   |   51 ++++++++++++++++++++++++++++++++++++++++++---
>>  arch/x86/xen/enlighten.c   |   19 +++++++---------
>>  include/asm-x86/paravirt.h |   29 -------------------------
>>  4 files changed, 57 insertions(+), 47 deletions(-)
>>
>> Index: linux-2.6/arch/x86/kernel/paravirt.c
>> ===================================================================
>> --- linux-2.6.orig/arch/x86/kernel/paravirt.c
>> +++ linux-2.6/arch/x86/kernel/paravirt.c
>> @@ -373,11 +373,6 @@ struct pv_cpu_ops pv_cpu_ops = {
>>
>>  struct pv_apic_ops pv_apic_ops = {
>>  #ifdef CONFIG_X86_LOCAL_APIC
>> -#ifndef CONFIG_X86_64
>> -       .apic_write = native_apic_mem_write,
>> -       .apic_write_atomic = native_apic_mem_write_atomic,
>> -       .apic_read = native_apic_mem_read,
>> -#endif
>>         .setup_boot_clock = setup_boot_APIC_clock,
>>         .setup_secondary_clock = setup_secondary_APIC_clock,
>>         .startup_ipi_hook = paravirt_nop,
>> Index: linux-2.6/arch/x86/kernel/vmi_32.c
>> ===================================================================
>> --- linux-2.6.orig/arch/x86/kernel/vmi_32.c
>> +++ linux-2.6/arch/x86/kernel/vmi_32.c
>> @@ -676,6 +676,50 @@ static inline int __init probe_vmi_rom(v
>>         return 0;
>>  }
>>
>> +#ifdef CONFIG_X86_LOCAL_APIC
>> +static u32 vmi_apic_read(u32 reg)
>> +{
>> +       return 0;
>> +}
>> +
>> +static void vmi_apic_write(u32 reg, u32 val)
>> +{
>> +       /* Warn to see if there's any stray references */
>> +       WARN_ON(1);
>> +}
>> +
>> +static u64 vmi_apic_icr_read(void)
>> +{
>> +       return 0;
>> +}
>> +
>> +static void vmi_apic_icr_write(u32 low, u32 id)
>> +{
>> +       /* Warn to see if there's any stray references */
>> +       WARN_ON(1);
>> +}
>> +
>> +static void vmi_apic_wait_icr_idle(void)
>> +{
>> +       return;
>> +}
>> +
>> +static u32 vmi_safe_apic_wait_icr_idle(void)
>> +{
>> +       return 0;
>> +}
>> +
>> +static struct apic_ops vmi_basic_apic_ops = {
>> +        .read = vmi_apic_read,
>> +        .write = vmi_apic_write,
>> +        .write_atomic = vmi_apic_write,
>> +        .icr_read = vmi_apic_icr_read,
>> +        .icr_write = vmi_apic_icr_write,
>> +        .wait_icr_idle = vmi_apic_wait_icr_idle,
>> +        .safe_wait_icr_idle = vmi_safe_apic_wait_icr_idle,
>> +};
>> +#endif
>> +
>>  /*
>>   * VMI setup common to all processors
>>   */
>> @@ -904,9 +948,10 @@ static inline int __init activate_vmi(vo
>>  #endif
>>
>>  #ifdef CONFIG_X86_LOCAL_APIC
>> -       para_fill(pv_apic_ops.apic_read, APICRead);
>> -       para_fill(pv_apic_ops.apic_write, APICWrite);
>> -       para_fill(pv_apic_ops.apic_write_atomic, APICWrite);
>> +       para_fill(vmi_basic_apic_ops.read, APICRead);
>> +       para_fill(vmi_basic_apic_ops.write, APICWrite);
>> +       para_fill(vmi_basic_apic_ops.write_atomic, APICWrite);
>> +       apic_ops = &vmi_basic_apic_ops;
>
> Yinghai, Looking more closely at this, based on my understanding this might be
> wrong for VMI. Correct patch should be as follows. Any comments?

so you mean icr related will still use default native member?

YH

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

* Re: [PATCH] x86: let 32bit use apic_ops too - fix
  2008-07-15 18:10         ` Yinghai Lu
@ 2008-07-15 18:27           ` Suresh Siddha
  0 siblings, 0 replies; 20+ messages in thread
From: Suresh Siddha @ 2008-07-15 18:27 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Siddha, Suresh B, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
	LKML

On Tue, Jul 15, 2008 at 11:10:37AM -0700, Yinghai Lu wrote:
> On Tue, Jul 15, 2008 at 10:33 AM, Suresh Siddha
> <suresh.b.siddha@intel.com> wrote:
> > Yinghai, Looking more closely at this, based on my understanding this might be
> > wrong for VMI. Correct patch should be as follows. Any comments?
> 
> so you mean icr related will still use default native member?

Yes. This is similar to pre apic_ops.

I think VMI uses apic operations in the paravirt case. For example,
please refer to vmi_time_bsp_init/vmi_time_ap_init.

thanks,
suresh

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

* Patch from LKML
@ 2008-07-15 18:38 ` Zachary Amsden
  0 siblings, 0 replies; 20+ messages in thread
From: Zachary Amsden @ 2008-07-15 18:38 UTC (permalink / raw)
  To: suresh.b.siddha, yhlu.kernel, Chris Wright, Jeremy Fitzhardinge,
	Rusty Russell


> On Tue, Jul 15, 2008 at 10:33 AM, Suresh Siddha
> <suresh.b.siddha@intel.com> wrote:
> > On Sun, Jul 13, 2008 at 10:19:35PM -0700, Yinghai Lu wrote:
> >>
> >> fix for pv.
> >>
> >> Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
> >>
> >> ---
> >>  arch/x86/kernel/paravirt.c |    5 ----
> >>  arch/x86/kernel/vmi_32.c   |   51 ++++++++++++++++++++++++++++++++++++++++++---
> >>  arch/x86/xen/enlighten.c   |   19 +++++++---------
> >>  include/asm-x86/paravirt.h |   29 -------------------------
> >>  4 files changed, 57 insertions(+), 47 deletions(-)
> >>
> >> Index: linux-2.6/arch/x86/kernel/paravirt.c
> >> ===================================================================
> >> --- linux-2.6.orig/arch/x86/kernel/paravirt.c
> >> +++ linux-2.6/arch/x86/kernel/paravirt.c
> >> @@ -373,11 +373,6 @@ struct pv_cpu_ops pv_cpu_ops = {
> >>
> >>  struct pv_apic_ops pv_apic_ops = {
> >>  #ifdef CONFIG_X86_LOCAL_APIC
> >> -#ifndef CONFIG_X86_64
> >> -       .apic_write = native_apic_mem_write,
> >> -       .apic_write_atomic = native_apic_mem_write_atomic,
> >> -       .apic_read = native_apic_mem_read,
> >> -#endif
> >>         .setup_boot_clock = setup_boot_APIC_clock,
> >>         .setup_secondary_clock = setup_secondary_APIC_clock,
> >>         .startup_ipi_hook = paravirt_nop,
> >> Index: linux-2.6/arch/x86/kernel/vmi_32.c
> >> ===================================================================
> >> --- linux-2.6.orig/arch/x86/kernel/vmi_32.c
> >> +++ linux-2.6/arch/x86/kernel/vmi_32.c
> >> @@ -676,6 +676,50 @@ static inline int __init probe_vmi_rom(v
> >>         return 0;
> >>  }
> >>
> >> +#ifdef CONFIG_X86_LOCAL_APIC
> >> +static u32 vmi_apic_read(u32 reg)
> >> +{
> >> +       return 0;
> >> +}
> >> +
> >> +static void vmi_apic_write(u32 reg, u32 val)
> >> +{
> >> +       /* Warn to see if there's any stray references */
> >> +       WARN_ON(1);
> >> +}
> >> +
> >> +static u64 vmi_apic_icr_read(void)
> >> +{
> >> +       return 0;
> >> +}
> >> +
> >> +static void vmi_apic_icr_write(u32 low, u32 id)
> >> +{
> >> +       /* Warn to see if there's any stray references */
> >> +       WARN_ON(1);
> >> +}
> >> +
> >> +static void vmi_apic_wait_icr_idle(void)
> >> +{
> >> +       return;
> >> +}
> >> +
> >> +static u32 vmi_safe_apic_wait_icr_idle(void)
> >> +{
> >> +       return 0;
> >> +}
> >> +
> >> +static struct apic_ops vmi_basic_apic_ops = {
> >> +        .read = vmi_apic_read,
> >> +        .write = vmi_apic_write,
> >> +        .write_atomic = vmi_apic_write,
> >> +        .icr_read = vmi_apic_icr_read,
> >> +        .icr_write = vmi_apic_icr_write,
> >> +        .wait_icr_idle = vmi_apic_wait_icr_idle,
> >> +        .safe_wait_icr_idle = vmi_safe_apic_wait_icr_idle,
> >> +};
> >> +#endif
> >> +
> >>  /*
> >>   * VMI setup common to all processors
> >>   */
> >> @@ -904,9 +948,10 @@ static inline int __init activate_vmi(vo
> >>  #endif
> >>
> >>  #ifdef CONFIG_X86_LOCAL_APIC
> >> -       para_fill(pv_apic_ops.apic_read, APICRead);
> >> -       para_fill(pv_apic_ops.apic_write, APICWrite);
> >> -       para_fill(pv_apic_ops.apic_write_atomic, APICWrite);
> >> +       para_fill(vmi_basic_apic_ops.read, APICRead);
> >> +       para_fill(vmi_basic_apic_ops.write, APICWrite);
> >> +       para_fill(vmi_basic_apic_ops.write_atomic, APICWrite);
> >> +       apic_ops = &vmi_basic_apic_ops;
> >
> > Yinghai, Looking more closely at this, based on my understanding this might be
> > wrong for VMI. Correct patch should be as follows. Any comments?
> 
> so you mean icr related will still use default native member?
> 
> YH

Nacked-by: Zachary Amsden <zach@vmware.com>

What are you doing here and why aren't you cc-ing the maintainers?

Thanks,

Zach

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

* Patch from LKML
@ 2008-07-15 18:38 ` Zachary Amsden
  0 siblings, 0 replies; 20+ messages in thread
From: Zachary Amsden @ 2008-07-15 18:38 UTC (permalink / raw)
  To: suresh.b.siddha, yhlu.kernel, Chris Wright, Jeremy Fitzhardinge,
	Rusty Russell, virtualization, Linux Kernel Mailing List


> On Tue, Jul 15, 2008 at 10:33 AM, Suresh Siddha
> <suresh.b.siddha@intel.com> wrote:
> > On Sun, Jul 13, 2008 at 10:19:35PM -0700, Yinghai Lu wrote:
> >>
> >> fix for pv.
> >>
> >> Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
> >>
> >> ---
> >>  arch/x86/kernel/paravirt.c |    5 ----
> >>  arch/x86/kernel/vmi_32.c   |   51 ++++++++++++++++++++++++++++++++++++++++++---
> >>  arch/x86/xen/enlighten.c   |   19 +++++++---------
> >>  include/asm-x86/paravirt.h |   29 -------------------------
> >>  4 files changed, 57 insertions(+), 47 deletions(-)
> >>
> >> Index: linux-2.6/arch/x86/kernel/paravirt.c
> >> ===================================================================
> >> --- linux-2.6.orig/arch/x86/kernel/paravirt.c
> >> +++ linux-2.6/arch/x86/kernel/paravirt.c
> >> @@ -373,11 +373,6 @@ struct pv_cpu_ops pv_cpu_ops = {
> >>
> >>  struct pv_apic_ops pv_apic_ops = {
> >>  #ifdef CONFIG_X86_LOCAL_APIC
> >> -#ifndef CONFIG_X86_64
> >> -       .apic_write = native_apic_mem_write,
> >> -       .apic_write_atomic = native_apic_mem_write_atomic,
> >> -       .apic_read = native_apic_mem_read,
> >> -#endif
> >>         .setup_boot_clock = setup_boot_APIC_clock,
> >>         .setup_secondary_clock = setup_secondary_APIC_clock,
> >>         .startup_ipi_hook = paravirt_nop,
> >> Index: linux-2.6/arch/x86/kernel/vmi_32.c
> >> ===================================================================
> >> --- linux-2.6.orig/arch/x86/kernel/vmi_32.c
> >> +++ linux-2.6/arch/x86/kernel/vmi_32.c
> >> @@ -676,6 +676,50 @@ static inline int __init probe_vmi_rom(v
> >>         return 0;
> >>  }
> >>
> >> +#ifdef CONFIG_X86_LOCAL_APIC
> >> +static u32 vmi_apic_read(u32 reg)
> >> +{
> >> +       return 0;
> >> +}
> >> +
> >> +static void vmi_apic_write(u32 reg, u32 val)
> >> +{
> >> +       /* Warn to see if there's any stray references */
> >> +       WARN_ON(1);
> >> +}
> >> +
> >> +static u64 vmi_apic_icr_read(void)
> >> +{
> >> +       return 0;
> >> +}
> >> +
> >> +static void vmi_apic_icr_write(u32 low, u32 id)
> >> +{
> >> +       /* Warn to see if there's any stray references */
> >> +       WARN_ON(1);
> >> +}
> >> +
> >> +static void vmi_apic_wait_icr_idle(void)
> >> +{
> >> +       return;
> >> +}
> >> +
> >> +static u32 vmi_safe_apic_wait_icr_idle(void)
> >> +{
> >> +       return 0;
> >> +}
> >> +
> >> +static struct apic_ops vmi_basic_apic_ops = {
> >> +        .read = vmi_apic_read,
> >> +        .write = vmi_apic_write,
> >> +        .write_atomic = vmi_apic_write,
> >> +        .icr_read = vmi_apic_icr_read,
> >> +        .icr_write = vmi_apic_icr_write,
> >> +        .wait_icr_idle = vmi_apic_wait_icr_idle,
> >> +        .safe_wait_icr_idle = vmi_safe_apic_wait_icr_idle,
> >> +};
> >> +#endif
> >> +
> >>  /*
> >>   * VMI setup common to all processors
> >>   */
> >> @@ -904,9 +948,10 @@ static inline int __init activate_vmi(vo
> >>  #endif
> >>
> >>  #ifdef CONFIG_X86_LOCAL_APIC
> >> -       para_fill(pv_apic_ops.apic_read, APICRead);
> >> -       para_fill(pv_apic_ops.apic_write, APICWrite);
> >> -       para_fill(pv_apic_ops.apic_write_atomic, APICWrite);
> >> +       para_fill(vmi_basic_apic_ops.read, APICRead);
> >> +       para_fill(vmi_basic_apic_ops.write, APICWrite);
> >> +       para_fill(vmi_basic_apic_ops.write_atomic, APICWrite);
> >> +       apic_ops = &vmi_basic_apic_ops;
> >
> > Yinghai, Looking more closely at this, based on my understanding this might be
> > wrong for VMI. Correct patch should be as follows. Any comments?
> 
> so you mean icr related will still use default native member?
> 
> YH

Nacked-by: Zachary Amsden <zach@vmware.com>

What are you doing here and why aren't you cc-ing the maintainers?

Thanks,

Zach


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

* Re: [PATCH] x86: let 32bit use apic_ops too - fix
  2008-07-15 18:38 ` Zachary Amsden
  (?)
@ 2008-07-15 18:51 ` Suresh Siddha
  2008-07-15 19:04   ` Zachary Amsden
  2008-07-15 19:22   ` Yinghai Lu
  -1 siblings, 2 replies; 20+ messages in thread
From: Suresh Siddha @ 2008-07-15 18:51 UTC (permalink / raw)
  To: Zachary Amsden
  Cc: Siddha, Suresh B, yhlu.kernel@gmail.com, Chris Wright,
	Jeremy Fitzhardinge, Rusty Russell, virtualization,
	Linux Kernel Mailing List

On Tue, Jul 15, 2008 at 11:38:50AM -0700, Zachary Amsden wrote:
> 
> Nacked-by: Zachary Amsden <zach@vmware.com>
> 
> What are you doing here and why aren't you cc-ing the maintainers?

Sorry. I was about to bring you into the loop.

Yinghai posted 32bit native apic_ops(similar to my 64bit apic ops patch, which
is different from pv_apic_ops) which is in tip/x86/x2apic and proposed a fix 
for VMI case aswell.

Based on my understanding, tip/x86/x2apic git commit
94a8c3c2437c8946f1b6c8e0b2c560a7db8ed3c6 is wrong and it should be fixed with
something like
http://marc.info/?l=linux-kernel&m=121614328831237&w=2

Can you please check and Ack/Nack this fix?

thanks,
suresh

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

* Re: Patch from LKML
  2008-07-15 18:38 ` Zachary Amsden
  (?)
  (?)
@ 2008-07-15 18:52 ` Yinghai Lu
  2008-07-15 18:57   ` Zachary Amsden
  -1 siblings, 1 reply; 20+ messages in thread
From: Yinghai Lu @ 2008-07-15 18:52 UTC (permalink / raw)
  To: Zachary Amsden
  Cc: suresh.b.siddha, Chris Wright, Jeremy Fitzhardinge, Rusty Russell,
	virtualization, Linux Kernel Mailing List

On Tue, Jul 15, 2008 at 11:38 AM, Zachary Amsden <zach@vmware.com> wrote:
>
>> On Tue, Jul 15, 2008 at 10:33 AM, Suresh Siddha
>> <suresh.b.siddha@intel.com> wrote:
>> > On Sun, Jul 13, 2008 at 10:19:35PM -0700, Yinghai Lu wrote:
>> >>
>> >> fix for pv.
>> >>
>> >> Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
>> >>
>> >> ---
>> >>  arch/x86/kernel/paravirt.c |    5 ----
>> >>  arch/x86/kernel/vmi_32.c   |   51 ++++++++++++++++++++++++++++++++++++++++++---
>> >>  arch/x86/xen/enlighten.c   |   19 +++++++---------
>> >>  include/asm-x86/paravirt.h |   29 -------------------------
>> >>  4 files changed, 57 insertions(+), 47 deletions(-)
>> >>
>> >> Index: linux-2.6/arch/x86/kernel/paravirt.c
>> >> ===================================================================
>> >> --- linux-2.6.orig/arch/x86/kernel/paravirt.c
>> >> +++ linux-2.6/arch/x86/kernel/paravirt.c
>> >> @@ -373,11 +373,6 @@ struct pv_cpu_ops pv_cpu_ops = {
>> >>
>> >>  struct pv_apic_ops pv_apic_ops = {
>> >>  #ifdef CONFIG_X86_LOCAL_APIC
>> >> -#ifndef CONFIG_X86_64
>> >> -       .apic_write = native_apic_mem_write,
>> >> -       .apic_write_atomic = native_apic_mem_write_atomic,
>> >> -       .apic_read = native_apic_mem_read,
>> >> -#endif
>> >>         .setup_boot_clock = setup_boot_APIC_clock,
>> >>         .setup_secondary_clock = setup_secondary_APIC_clock,
>> >>         .startup_ipi_hook = paravirt_nop,
>> >> Index: linux-2.6/arch/x86/kernel/vmi_32.c
>> >> ===================================================================
>> >> --- linux-2.6.orig/arch/x86/kernel/vmi_32.c
>> >> +++ linux-2.6/arch/x86/kernel/vmi_32.c
>> >> @@ -676,6 +676,50 @@ static inline int __init probe_vmi_rom(v
>> >>         return 0;
>> >>  }
>> >>
>> >> +#ifdef CONFIG_X86_LOCAL_APIC
>> >> +static u32 vmi_apic_read(u32 reg)
>> >> +{
>> >> +       return 0;
>> >> +}
>> >> +
>> >> +static void vmi_apic_write(u32 reg, u32 val)
>> >> +{
>> >> +       /* Warn to see if there's any stray references */
>> >> +       WARN_ON(1);
>> >> +}
>> >> +
>> >> +static u64 vmi_apic_icr_read(void)
>> >> +{
>> >> +       return 0;
>> >> +}
>> >> +
>> >> +static void vmi_apic_icr_write(u32 low, u32 id)
>> >> +{
>> >> +       /* Warn to see if there's any stray references */
>> >> +       WARN_ON(1);
>> >> +}
>> >> +
>> >> +static void vmi_apic_wait_icr_idle(void)
>> >> +{
>> >> +       return;
>> >> +}
>> >> +
>> >> +static u32 vmi_safe_apic_wait_icr_idle(void)
>> >> +{
>> >> +       return 0;
>> >> +}
>> >> +
>> >> +static struct apic_ops vmi_basic_apic_ops = {
>> >> +        .read = vmi_apic_read,
>> >> +        .write = vmi_apic_write,
>> >> +        .write_atomic = vmi_apic_write,
>> >> +        .icr_read = vmi_apic_icr_read,
>> >> +        .icr_write = vmi_apic_icr_write,
>> >> +        .wait_icr_idle = vmi_apic_wait_icr_idle,
>> >> +        .safe_wait_icr_idle = vmi_safe_apic_wait_icr_idle,
>> >> +};
>> >> +#endif
>> >> +
>> >>  /*
>> >>   * VMI setup common to all processors
>> >>   */
>> >> @@ -904,9 +948,10 @@ static inline int __init activate_vmi(vo
>> >>  #endif
>> >>
>> >>  #ifdef CONFIG_X86_LOCAL_APIC
>> >> -       para_fill(pv_apic_ops.apic_read, APICRead);
>> >> -       para_fill(pv_apic_ops.apic_write, APICWrite);
>> >> -       para_fill(pv_apic_ops.apic_write_atomic, APICWrite);
>> >> +       para_fill(vmi_basic_apic_ops.read, APICRead);
>> >> +       para_fill(vmi_basic_apic_ops.write, APICWrite);
>> >> +       para_fill(vmi_basic_apic_ops.write_atomic, APICWrite);
>> >> +       apic_ops = &vmi_basic_apic_ops;
>> >
>> > Yinghai, Looking more closely at this, based on my understanding this might be
>> > wrong for VMI. Correct patch should be as follows. Any comments?
>>
>> so you mean icr related will still use default native member?
>>
>> YH
>
> Nacked-by: Zachary Amsden <zach@vmware.com>

because of not ccing you?

>
> What are you doing here and why aren't you cc-ing the maintainers?

did you checking tip tree for x86 changing?

YH

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

* Re: Patch from LKML
  2008-07-15 18:52 ` Patch from LKML Yinghai Lu
@ 2008-07-15 18:57   ` Zachary Amsden
  0 siblings, 0 replies; 20+ messages in thread
From: Zachary Amsden @ 2008-07-15 18:57 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: suresh.b.siddha@intel.com, Chris Wright, Jeremy Fitzhardinge,
	Rusty Russell, virtualization, Linux Kernel Mailing List

On Tue, 2008-07-15 at 11:52 -0700, Yinghai Lu wrote:

> > Nacked-by: Zachary Amsden <zach@vmware.com>
> 
> because of not ccing you?

Because it's wrong.

> >
> > What are you doing here and why aren't you cc-ing the maintainers?
> 
> did you checking tip tree for x86 changing?

No, this was brought to my attention by someone who spotted it.

Zach

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

* Re: [PATCH] x86: let 32bit use apic_ops too - fix
  2008-07-15 18:51 ` [PATCH] x86: let 32bit use apic_ops too - fix Suresh Siddha
@ 2008-07-15 19:04   ` Zachary Amsden
  2008-07-15 19:10     ` Yinghai Lu
  2008-07-15 19:22   ` Yinghai Lu
  1 sibling, 1 reply; 20+ messages in thread
From: Zachary Amsden @ 2008-07-15 19:04 UTC (permalink / raw)
  To: Suresh Siddha
  Cc: yhlu.kernel@gmail.com, Chris Wright, Jeremy Fitzhardinge,
	Rusty Russell, virtualization, Linux Kernel Mailing List

On Tue, 2008-07-15 at 11:51 -0700, Suresh Siddha wrote:
> On Tue, Jul 15, 2008 at 11:38:50AM -0700, Zachary Amsden wrote:
> >
> > Nacked-by: Zachary Amsden <zach@vmware.com>
> >
> > What are you doing here and why aren't you cc-ing the maintainers?
> 
> Sorry. I was about to bring you into the loop.
> 
> Yinghai posted 32bit native apic_ops(similar to my 64bit apic ops patch, which
> is different from pv_apic_ops) which is in tip/x86/x2apic and proposed a fix
> for VMI case aswell.
> 
> Based on my understanding, tip/x86/x2apic git commit
> 94a8c3c2437c8946f1b6c8e0b2c560a7db8ed3c6 is wrong and it should be fixed with
> something like
> http://marc.info/?l=linux-kernel&m=121614328831237&w=2

Looks better, but I need to read more context to find out where the
apic_ops variable comes from; I'll read the list for patches.

You are correct in that we will want to use the same wait_icr_idle
routine as native hardware; it's not clear from just this patch how that
happens.

Also, the VMI operations are sensitive to parameter order because they
interface with an ABI at the other end.  I need to check the parameter
order for apic read / write is still consistent with the ABI.

Zach

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

* Re: [PATCH] x86: let 32bit use apic_ops too - fix
  2008-07-15 19:04   ` Zachary Amsden
@ 2008-07-15 19:10     ` Yinghai Lu
  2008-07-15 19:19       ` Zachary Amsden
  0 siblings, 1 reply; 20+ messages in thread
From: Yinghai Lu @ 2008-07-15 19:10 UTC (permalink / raw)
  To: Zachary Amsden
  Cc: Suresh Siddha, Chris Wright, Jeremy Fitzhardinge, Rusty Russell,
	virtualization, Linux Kernel Mailing List

On Tue, Jul 15, 2008 at 12:04 PM, Zachary Amsden <zach@vmware.com> wrote:
> On Tue, 2008-07-15 at 11:51 -0700, Suresh Siddha wrote:
>> On Tue, Jul 15, 2008 at 11:38:50AM -0700, Zachary Amsden wrote:
>> >
>> > Nacked-by: Zachary Amsden <zach@vmware.com>
>> >
>> > What are you doing here and why aren't you cc-ing the maintainers?
>>
>> Sorry. I was about to bring you into the loop.
>>
>> Yinghai posted 32bit native apic_ops(similar to my 64bit apic ops patch, which
>> is different from pv_apic_ops) which is in tip/x86/x2apic and proposed a fix
>> for VMI case aswell.
>>
>> Based on my understanding, tip/x86/x2apic git commit
>> 94a8c3c2437c8946f1b6c8e0b2c560a7db8ed3c6 is wrong and it should be fixed with
>> something like
>> http://marc.info/?l=linux-kernel&m=121614328831237&w=2
>
> Looks better, but I need to read more context to find out where the
> apic_ops variable comes from; I'll read the list for patches.
>
> You are correct in that we will want to use the same wait_icr_idle
> routine as native hardware; it's not clear from just this patch how that
> happens.
>
> Also, the VMI operations are sensitive to parameter order because they
> interface with an ABI at the other end.  I need to check the parameter
> order for apic read / write is still consistent with the ABI.

http://people.redhat.com/mingo/tip.git/readme.txt

mkdir linux-2.6
cd linux-2.6

git init

git remote add linus
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git

git remote add tip
git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git

git remote update

git checkout -b tip-latest tip/master

git merge tip/x86/x2apic

YH

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

* Re: [PATCH] x86: let 32bit use apic_ops too - fix
  2008-07-15 19:10     ` Yinghai Lu
@ 2008-07-15 19:19       ` Zachary Amsden
  0 siblings, 0 replies; 20+ messages in thread
From: Zachary Amsden @ 2008-07-15 19:19 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Suresh Siddha, Chris Wright, Jeremy Fitzhardinge, Rusty Russell,
	virtualization, Linux Kernel Mailing List

On Tue, 2008-07-15 at 12:10 -0700, Yinghai Lu wrote:
> On Tue, Jul 15, 2008 at 12:04 PM, Zachary Amsden <zach@vmware.com> wrote:
> > On Tue, 2008-07-15 at 11:51 -0700, Suresh Siddha wrote:
> >> On Tue, Jul 15, 2008 at 11:38:50AM -0700, Zachary Amsden wrote:
> >> >
> >> > Nacked-by: Zachary Amsden <zach@vmware.com>
> >> >
> >> > What are you doing here and why aren't you cc-ing the maintainers?
> >>
> >> Sorry. I was about to bring you into the loop.
> >>
> >> Yinghai posted 32bit native apic_ops(similar to my 64bit apic ops patch, which
> >> is different from pv_apic_ops) which is in tip/x86/x2apic and proposed a fix
> >> for VMI case aswell.
> >>
> >> Based on my understanding, tip/x86/x2apic git commit
> >> 94a8c3c2437c8946f1b6c8e0b2c560a7db8ed3c6 is wrong and it should be fixed with
> >> something like
> >> http://marc.info/?l=linux-kernel&m=121614328831237&w=2


Suresh's patch looks good.  Thanks Suresh!

Acked-by: Zachary Amsden <zach@vmware.com>

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

* Re: [PATCH] x86: let 32bit use apic_ops too - fix
  2008-07-15 18:51 ` [PATCH] x86: let 32bit use apic_ops too - fix Suresh Siddha
  2008-07-15 19:04   ` Zachary Amsden
@ 2008-07-15 19:22   ` Yinghai Lu
  2008-07-15 19:24     ` Zachary Amsden
  1 sibling, 1 reply; 20+ messages in thread
From: Yinghai Lu @ 2008-07-15 19:22 UTC (permalink / raw)
  To: Suresh Siddha, Jeremy Fitzhardinge
  Cc: Zachary Amsden, Chris Wright, Rusty Russell, virtualization,
	Linux Kernel Mailing List

On Tue, Jul 15, 2008 at 11:51 AM, Suresh Siddha
<suresh.b.siddha@intel.com> wrote:
> On Tue, Jul 15, 2008 at 11:38:50AM -0700, Zachary Amsden wrote:
>>
>> Nacked-by: Zachary Amsden <zach@vmware.com>
>>
>> What are you doing here and why aren't you cc-ing the maintainers?
>
> Sorry. I was about to bring you into the loop.
>
> Yinghai posted 32bit native apic_ops(similar to my 64bit apic ops patch, which
> is different from pv_apic_ops) which is in tip/x86/x2apic and proposed a fix
> for VMI case aswell.
>
> Based on my understanding, tip/x86/x2apic git commit
> 94a8c3c2437c8946f1b6c8e0b2c560a7db8ed3c6 is wrong and it should be fixed with
> something like
> http://marc.info/?l=linux-kernel&m=121614328831237&w=2
>
> Can you please check and Ack/Nack this fix?

how about xen pv with icr related?

YH

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

* Re: [PATCH] x86: let 32bit use apic_ops too - fix
  2008-07-15 19:22   ` Yinghai Lu
@ 2008-07-15 19:24     ` Zachary Amsden
  0 siblings, 0 replies; 20+ messages in thread
From: Zachary Amsden @ 2008-07-15 19:24 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Suresh Siddha, Jeremy Fitzhardinge, Chris Wright, Rusty Russell,
	virtualization, Linux Kernel Mailing List

On Tue, 2008-07-15 at 12:22 -0700, Yinghai Lu wrote:
> On Tue, Jul 15, 2008 at 11:51 AM, Suresh Siddha
> <suresh.b.siddha@intel.com> wrote:
> > On Tue, Jul 15, 2008 at 11:38:50AM -0700, Zachary Amsden wrote:
> >>
> >> Nacked-by: Zachary Amsden <zach@vmware.com>
> >>
> >> What are you doing here and why aren't you cc-ing the maintainers?
> >
> > Sorry. I was about to bring you into the loop.
> >
> > Yinghai posted 32bit native apic_ops(similar to my 64bit apic ops patch, which
> > is different from pv_apic_ops) which is in tip/x86/x2apic and proposed a fix
> > for VMI case aswell.
> >
> > Based on my understanding, tip/x86/x2apic git commit
> > 94a8c3c2437c8946f1b6c8e0b2c560a7db8ed3c6 is wrong and it should be fixed with
> > something like
> > http://marc.info/?l=linux-kernel&m=121614328831237&w=2
> >
> > Can you please check and Ack/Nack this fix?
> 
> how about xen pv with icr related?

Xen doesn't have an APIC; they should be fine as is I think.

Zach

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

* Re: [PATCH] x86: let 32bit use apic_ops too - fix
  2008-07-14 16:49         ` Suresh Siddha
  2008-07-14 17:00           ` Yinghai Lu
@ 2008-07-18 17:06           ` Ingo Molnar
  1 sibling, 0 replies; 20+ messages in thread
From: Ingo Molnar @ 2008-07-18 17:06 UTC (permalink / raw)
  To: Suresh Siddha; +Cc: Yinghai Lu, Thomas Gleixner, H. Peter Anvin, LKML


* Suresh Siddha <suresh.b.siddha@intel.com> wrote:

> Ingo, before you try for the third attempt ;) we need one more lguest 
> apic_ops fix. Patch appended. Thanks.
> 
> ---
> [patch] x86: apic_ops for lguest
> 
> apic_ops for lguest.

applied to tip/x86/x2apic, thanks Suresh.

	Ingo

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

* Re: [PATCH] x86: let 32bit use apic_ops too - fix
  2008-07-15 17:33       ` Suresh Siddha
  2008-07-15 18:10         ` Yinghai Lu
@ 2008-07-18 17:07         ` Ingo Molnar
  1 sibling, 0 replies; 20+ messages in thread
From: Ingo Molnar @ 2008-07-18 17:07 UTC (permalink / raw)
  To: Suresh Siddha; +Cc: Yinghai Lu, Thomas Gleixner, H. Peter Anvin, LKML


* Suresh Siddha <suresh.b.siddha@intel.com> wrote:

> Yinghai, Looking more closely at this, based on my understanding this 
> might be wrong for VMI. Correct patch should be as follows. Any 
> comments?

applied to tip/x86/x2apic (with some fixups), thanks Suresh.

	Ingo

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

end of thread, other threads:[~2008-07-18 17:08 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-07-15 18:38 Patch from LKML Zachary Amsden
2008-07-15 18:38 ` Zachary Amsden
2008-07-15 18:51 ` [PATCH] x86: let 32bit use apic_ops too - fix Suresh Siddha
2008-07-15 19:04   ` Zachary Amsden
2008-07-15 19:10     ` Yinghai Lu
2008-07-15 19:19       ` Zachary Amsden
2008-07-15 19:22   ` Yinghai Lu
2008-07-15 19:24     ` Zachary Amsden
2008-07-15 18:52 ` Patch from LKML Yinghai Lu
2008-07-15 18:57   ` Zachary Amsden
  -- strict thread matches above, loose matches on Subject: below --
2008-07-08  8:41 [PATCH] x86: introduce page_size_mask for 64bit Yinghai Lu
2008-07-11  3:38 ` [PATCH] x86: introduce max_low_pfn_mapped " Yinghai Lu
2008-07-12  1:41   ` [PATCH] x86: let 32bit use apic_ops too Yinghai Lu
2008-07-14  5:19     ` [PATCH] x86: let 32bit use apic_ops too - fix Yinghai Lu
2008-07-14  7:12       ` Ingo Molnar
2008-07-14 16:49         ` Suresh Siddha
2008-07-14 17:00           ` Yinghai Lu
2008-07-14 18:03             ` Suresh Siddha
2008-07-18 17:06           ` Ingo Molnar
2008-07-15 17:33       ` Suresh Siddha
2008-07-15 18:10         ` Yinghai Lu
2008-07-15 18:27           ` Suresh Siddha
2008-07-18 17:07         ` Ingo Molnar

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.