public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] xen: HVM X2APIC support
@ 2010-12-06  7:49 Sheng Yang
  2010-12-06 18:07 ` Jeremy Fitzhardinge
  2010-12-06 20:40 ` Konrad Rzeszutek Wilk
  0 siblings, 2 replies; 6+ messages in thread
From: Sheng Yang @ 2010-12-06  7:49 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: Stefano Stabellini, linux-kernel, Sheng Yang

This patch is similiar to Gleb Natapov's patch for KVM, which enable the
hypervisor to emulate x2apic feature for the guest. By this way, the emulation
of lapic would be simpler with x2apic interface(MSR), and faster.

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 arch/x86/include/asm/xen/hypervisor.h |   35 +++++++++++++++++++++++++++++++++
 arch/x86/kernel/apic/apic.c           |    4 ++-
 arch/x86/xen/enlighten.c              |   30 ++++++++++-----------------
 3 files changed, 49 insertions(+), 20 deletions(-)

diff --git a/arch/x86/include/asm/xen/hypervisor.h b/arch/x86/include/asm/xen/hypervisor.h
index 396ff4c..bc5b804 100644
--- a/arch/x86/include/asm/xen/hypervisor.h
+++ b/arch/x86/include/asm/xen/hypervisor.h
@@ -37,4 +37,39 @@
 extern struct shared_info *HYPERVISOR_shared_info;
 extern struct start_info *xen_start_info;
 
+#include <asm/processor.h>
+
+static inline uint32_t xen_cpuid_base(void)
+{
+	uint32_t base, eax, ebx, ecx, edx;
+	char signature[13];
+
+	for (base = 0x40000000; base < 0x40010000; base += 0x100) {
+		cpuid(base, &eax, &ebx, &ecx, &edx);
+		*(uint32_t *)(signature + 0) = ebx;
+		*(uint32_t *)(signature + 4) = ecx;
+		*(uint32_t *)(signature + 8) = edx;
+		signature[12] = 0;
+
+		if (!strcmp("XenVMMXenVMM", signature) && ((eax - base) >= 2))
+			return base;
+	}
+
+	return 0;
+}
+
+#ifdef CONFIG_XEN
+extern bool xen_hvm_need_lapic(void);
+
+static inline bool xen_para_available(void)
+{
+	return xen_hvm_need_lapic();
+}
+#else
+static inline bool xen_para_available(void)
+{
+	return (xen_cpuid_base() != 0);
+}
+#endif
+
 #endif /* _ASM_X86_XEN_HYPERVISOR_H */
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 3f838d5..1b68221 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -52,6 +52,7 @@
 #include <asm/mce.h>
 #include <asm/kvm_para.h>
 #include <asm/tsc.h>
+#include <asm/xen/hypervisor.h>
 
 unsigned int num_processors;
 
@@ -1476,7 +1477,8 @@ void __init enable_IR_x2apic(void)
 		/* IR is required if there is APIC ID > 255 even when running
 		 * under KVM
 		 */
-		if (max_physical_apicid > 255 || !kvm_para_available())
+		if (max_physical_apicid > 255 ||
+		    (!kvm_para_available() && !xen_para_available()))
 			goto nox2apic;
 		/*
 		 * without IR all CPUs can be addressed by IOAPIC/MSI
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 235c0f4..7ef1645 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1245,25 +1245,6 @@ asmlinkage void __init xen_start_kernel(void)
 #endif
 }
 
-static uint32_t xen_cpuid_base(void)
-{
-	uint32_t base, eax, ebx, ecx, edx;
-	char signature[13];
-
-	for (base = 0x40000000; base < 0x40010000; base += 0x100) {
-		cpuid(base, &eax, &ebx, &ecx, &edx);
-		*(uint32_t *)(signature + 0) = ebx;
-		*(uint32_t *)(signature + 4) = ecx;
-		*(uint32_t *)(signature + 8) = edx;
-		signature[12] = 0;
-
-		if (!strcmp("XenVMMXenVMM", signature) && ((eax - base) >= 2))
-			return base;
-	}
-
-	return 0;
-}
-
 static int init_hvm_pv_info(int *major, int *minor)
 {
 	uint32_t eax, ebx, ecx, edx, pages, msr, base;
@@ -1373,6 +1354,17 @@ static bool __init xen_hvm_platform(void)
 	return true;
 }
 
+bool xen_hvm_need_lapic(void)
+{
+	if (xen_pv_domain())
+		return false;
+	if (xen_hvm_domain() &&
+	   (xen_feature(XENFEAT_hvm_pirqs) || xen_have_vector_callback))
+		return false;
+	return (xen_cpuid_base() != 0);
+}
+EXPORT_SYMBOL_GPL(xen_hvm_need_lapic);
+
 const __refconst struct hypervisor_x86 x86_hyper_xen_hvm = {
 	.name			= "Xen HVM",
 	.detect			= xen_hvm_platform,
-- 
1.7.0.1


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

* Re: [PATCH v2] xen: HVM X2APIC support
  2010-12-06  7:49 [PATCH v2] xen: HVM X2APIC support Sheng Yang
@ 2010-12-06 18:07 ` Jeremy Fitzhardinge
  2010-12-07  2:01   ` Sheng Yang
  2010-12-06 20:40 ` Konrad Rzeszutek Wilk
  1 sibling, 1 reply; 6+ messages in thread
From: Jeremy Fitzhardinge @ 2010-12-06 18:07 UTC (permalink / raw)
  To: Sheng Yang; +Cc: Stefano Stabellini, linux-kernel

On 12/05/2010 11:49 PM, Sheng Yang wrote:
> This patch is similiar to Gleb Natapov's patch for KVM, which enable the
> hypervisor to emulate x2apic feature for the guest. By this way, the emulation
> of lapic would be simpler with x2apic interface(MSR), and faster.
>
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
>  arch/x86/include/asm/xen/hypervisor.h |   35 +++++++++++++++++++++++++++++++++
>  arch/x86/kernel/apic/apic.c           |    4 ++-
>  arch/x86/xen/enlighten.c              |   30 ++++++++++-----------------
>  3 files changed, 49 insertions(+), 20 deletions(-)
>
> diff --git a/arch/x86/include/asm/xen/hypervisor.h b/arch/x86/include/asm/xen/hypervisor.h
> index 396ff4c..bc5b804 100644
> --- a/arch/x86/include/asm/xen/hypervisor.h
> +++ b/arch/x86/include/asm/xen/hypervisor.h
> @@ -37,4 +37,39 @@
>  extern struct shared_info *HYPERVISOR_shared_info;
>  extern struct start_info *xen_start_info;
>  
> +#include <asm/processor.h>
> +
> +static inline uint32_t xen_cpuid_base(void)
> +{
> +	uint32_t base, eax, ebx, ecx, edx;
> +	char signature[13];
> +
> +	for (base = 0x40000000; base < 0x40010000; base += 0x100) {
> +		cpuid(base, &eax, &ebx, &ecx, &edx);
> +		*(uint32_t *)(signature + 0) = ebx;
> +		*(uint32_t *)(signature + 4) = ecx;
> +		*(uint32_t *)(signature + 8) = edx;
> +		signature[12] = 0;
> +
> +		if (!strcmp("XenVMMXenVMM", signature) && ((eax - base) >= 2))
> +			return base;
> +	}
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_XEN
> +extern bool xen_hvm_need_lapic(void);
> +
> +static inline bool xen_para_available(void)
> +{
> +	return xen_hvm_need_lapic();
> +}
> +#else
> +static inline bool xen_para_available(void)
> +{
> +	return (xen_cpuid_base() != 0);
> +}
> +#endif

Surely this should be using the asm/hypervisor.h interface?

> +
>  #endif /* _ASM_X86_XEN_HYPERVISOR_H */
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index 3f838d5..1b68221 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -52,6 +52,7 @@
>  #include <asm/mce.h>
>  #include <asm/kvm_para.h>
>  #include <asm/tsc.h>
> +#include <asm/xen/hypervisor.h>
>  
>  unsigned int num_processors;
>  
> @@ -1476,7 +1477,8 @@ void __init enable_IR_x2apic(void)
>  		/* IR is required if there is APIC ID > 255 even when running
>  		 * under KVM
>  		 */
> -		if (max_physical_apicid > 255 || !kvm_para_available())
> +		if (max_physical_apicid > 255 ||
> +		    (!kvm_para_available() && !xen_para_available()))
>  			goto nox2apic;

What are the downsides of just using x2apic unconditionally?

Assuming there is some downside to using it all the time, what about
adding a pseudo-cpu feature flag meaning "use x2apic unconditionally"
rather than adding a bunch of ad-hoc tests here?

>  		/*
>  		 * without IR all CPUs can be addressed by IOAPIC/MSI
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 235c0f4..7ef1645 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1245,25 +1245,6 @@ asmlinkage void __init xen_start_kernel(void)
>  #endif
>  }
>  
> -static uint32_t xen_cpuid_base(void)
> -{
> -	uint32_t base, eax, ebx, ecx, edx;
> -	char signature[13];
> -
> -	for (base = 0x40000000; base < 0x40010000; base += 0x100) {
> -		cpuid(base, &eax, &ebx, &ecx, &edx);
> -		*(uint32_t *)(signature + 0) = ebx;
> -		*(uint32_t *)(signature + 4) = ecx;
> -		*(uint32_t *)(signature + 8) = edx;
> -		signature[12] = 0;
> -
> -		if (!strcmp("XenVMMXenVMM", signature) && ((eax - base) >= 2))
> -			return base;
> -	}
> -
> -	return 0;
> -}
> -
>  static int init_hvm_pv_info(int *major, int *minor)
>  {
>  	uint32_t eax, ebx, ecx, edx, pages, msr, base;
> @@ -1373,6 +1354,17 @@ static bool __init xen_hvm_platform(void)
>  	return true;
>  }
>  
> +bool xen_hvm_need_lapic(void)

Where does this get used?

> +{
> +	if (xen_pv_domain())
> +		return false;
> +	if (xen_hvm_domain() &&
> +	   (xen_feature(XENFEAT_hvm_pirqs) || xen_have_vector_callback))
> +		return false;
> +	return (xen_cpuid_base() != 0);
> +}
> +EXPORT_SYMBOL_GPL(xen_hvm_need_lapic);
> +
>  const __refconst struct hypervisor_x86 x86_hyper_xen_hvm = {
>  	.name			= "Xen HVM",
>  	.detect			= xen_hvm_platform,

    J

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

* Re: [PATCH v2] xen: HVM X2APIC support
  2010-12-06  7:49 [PATCH v2] xen: HVM X2APIC support Sheng Yang
  2010-12-06 18:07 ` Jeremy Fitzhardinge
@ 2010-12-06 20:40 ` Konrad Rzeszutek Wilk
  2010-12-07  2:04   ` Sheng Yang
  1 sibling, 1 reply; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2010-12-06 20:40 UTC (permalink / raw)
  To: Sheng Yang; +Cc: Jeremy Fitzhardinge, Stefano Stabellini, linux-kernel

> +static inline uint32_t xen_cpuid_base(void)
> +{
> +	uint32_t base, eax, ebx, ecx, edx;
> +	char signature[13];
> +
> +	for (base = 0x40000000; base < 0x40010000; base += 0x100) {
> +		cpuid(base, &eax, &ebx, &ecx, &edx);
> +		*(uint32_t *)(signature + 0) = ebx;
> +		*(uint32_t *)(signature + 4) = ecx;
> +		*(uint32_t *)(signature + 8) = edx;
> +		signature[12] = 0;
> +
> +		if (!strcmp("XenVMMXenVMM", signature) && ((eax - base) >= 2))
> +			return base;
> +	}
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_XEN
> +extern bool xen_hvm_need_lapic(void);
> +
> +static inline bool xen_para_available(void)
> +{
> +	return xen_hvm_need_lapic();
> +}
> +#else
> +static inline bool xen_para_available(void)
> +{
> +	return (xen_cpuid_base() != 0);

Would it make sense to collapse the kvm_para_available and
the xen_cpuid_base together (and maybe even the HyperV detection code)
together in one and just return "x2_apic_para_capable" ?


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

* Re: [PATCH v2] xen: HVM X2APIC support
  2010-12-06 18:07 ` Jeremy Fitzhardinge
@ 2010-12-07  2:01   ` Sheng Yang
  2010-12-07  2:03     ` [Xen-devel] " Sheng Yang
  0 siblings, 1 reply; 6+ messages in thread
From: Sheng Yang @ 2010-12-07  2:01 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: Stefano Stabellini, linux-kernel, xen-devel@lists.xensource.com

On Tuesday 07 December 2010 02:07:23 Jeremy Fitzhardinge wrote:
> On 12/05/2010 11:49 PM, Sheng Yang wrote:
> > This patch is similiar to Gleb Natapov's patch for KVM, which enable the
> > hypervisor to emulate x2apic feature for the guest. By this way, the
> > emulation of lapic would be simpler with x2apic interface(MSR), and
> > faster.
> > 
> > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > ---
> > 
> >  arch/x86/include/asm/xen/hypervisor.h |   35
> >  +++++++++++++++++++++++++++++++++ arch/x86/kernel/apic/apic.c          
> >  |    4 ++-
> >  arch/x86/xen/enlighten.c              |   30 ++++++++++-----------------
> >  3 files changed, 49 insertions(+), 20 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/xen/hypervisor.h
> > b/arch/x86/include/asm/xen/hypervisor.h index 396ff4c..bc5b804 100644
> > --- a/arch/x86/include/asm/xen/hypervisor.h
> > +++ b/arch/x86/include/asm/xen/hypervisor.h
> > @@ -37,4 +37,39 @@
> > 
> >  extern struct shared_info *HYPERVISOR_shared_info;
> >  extern struct start_info *xen_start_info;
> > 
> > +#include <asm/processor.h>
> > +
> > +static inline uint32_t xen_cpuid_base(void)
> > +{
> > +	uint32_t base, eax, ebx, ecx, edx;
> > +	char signature[13];
> > +
> > +	for (base = 0x40000000; base < 0x40010000; base += 0x100) {
> > +		cpuid(base, &eax, &ebx, &ecx, &edx);
> > +		*(uint32_t *)(signature + 0) = ebx;
> > +		*(uint32_t *)(signature + 4) = ecx;
> > +		*(uint32_t *)(signature + 8) = edx;
> > +		signature[12] = 0;
> > +
> > +		if (!strcmp("XenVMMXenVMM", signature) && ((eax - base) >= 2))
> > +			return base;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +#ifdef CONFIG_XEN
> > +extern bool xen_hvm_need_lapic(void);
> > +
> > +static inline bool xen_para_available(void)
> > +{
> > +	return xen_hvm_need_lapic();
> > +}
> > +#else
> > +static inline bool xen_para_available(void)
> > +{
> > +	return (xen_cpuid_base() != 0);
> > +}
> > +#endif
> 
> Surely this should be using the asm/hypervisor.h interface?

I think it's the proper place. Or any suggestion?
> 
> > +
> > 
> >  #endif /* _ASM_X86_XEN_HYPERVISOR_H */
> > 
> > diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> > index 3f838d5..1b68221 100644
> > --- a/arch/x86/kernel/apic/apic.c
> > +++ b/arch/x86/kernel/apic/apic.c
> > @@ -52,6 +52,7 @@
> > 
> >  #include <asm/mce.h>
> >  #include <asm/kvm_para.h>
> >  #include <asm/tsc.h>
> > 
> > +#include <asm/xen/hypervisor.h>
> > 
> >  unsigned int num_processors;
> > 
> > @@ -1476,7 +1477,8 @@ void __init enable_IR_x2apic(void)
> > 
> >  		/* IR is required if there is APIC ID > 255 even when running
> >  		
> >  		 * under KVM
> >  		 */
> > 
> > -		if (max_physical_apicid > 255 || !kvm_para_available())
> > +		if (max_physical_apicid > 255 ||
> > +		    (!kvm_para_available() && !xen_para_available()))
> > 
> >  			goto nox2apic;
> 
> What are the downsides of just using x2apic unconditionally?
> 
> Assuming there is some downside to using it all the time, what about
> adding a pseudo-cpu feature flag meaning "use x2apic unconditionally"
> rather than adding a bunch of ad-hoc tests here?

Because for native, x2apic should be used with interrupt-remapping, otherwise it 
didn't make much sense for native(the main reason to x2apic for bare-metal is 
support big machine with more than 255 APIC-ID, and without interrupt remapping it 
can't do so). But for hypervisor, we want to emulate the accessing faster through 
MSR, so we want them.
> 
> >  		/*
> >  		
> >  		 * without IR all CPUs can be addressed by IOAPIC/MSI
> > 
> > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> > index 235c0f4..7ef1645 100644
> > --- a/arch/x86/xen/enlighten.c
> > +++ b/arch/x86/xen/enlighten.c
> > @@ -1245,25 +1245,6 @@ asmlinkage void __init xen_start_kernel(void)
> > 
> >  #endif
> >  }
> > 
> > -static uint32_t xen_cpuid_base(void)
> > -{
> > -	uint32_t base, eax, ebx, ecx, edx;
> > -	char signature[13];
> > -
> > -	for (base = 0x40000000; base < 0x40010000; base += 0x100) {
> > -		cpuid(base, &eax, &ebx, &ecx, &edx);
> > -		*(uint32_t *)(signature + 0) = ebx;
> > -		*(uint32_t *)(signature + 4) = ecx;
> > -		*(uint32_t *)(signature + 8) = edx;
> > -		signature[12] = 0;
> > -
> > -		if (!strcmp("XenVMMXenVMM", signature) && ((eax - base) >= 2))
> > -			return base;
> > -	}
> > -
> > -	return 0;
> > -}
> > -
> > 
> >  static int init_hvm_pv_info(int *major, int *minor)
> >  {
> >  
> >  	uint32_t eax, ebx, ecx, edx, pages, msr, base;
> > 
> > @@ -1373,6 +1354,17 @@ static bool __init xen_hvm_platform(void)
> > 
> >  	return true;
> >  
> >  }
> > 
> > +bool xen_hvm_need_lapic(void)
> 
> Where does this get used?

?... xen_para_available() above...

--
regards
Yang, Sheng

> 
> > +{
> > +	if (xen_pv_domain())
> > +		return false;
> > +	if (xen_hvm_domain() &&
> > +	   (xen_feature(XENFEAT_hvm_pirqs) || xen_have_vector_callback))
> > +		return false;
> > +	return (xen_cpuid_base() != 0);
> > +}
> > +EXPORT_SYMBOL_GPL(xen_hvm_need_lapic);
> > +
> > 
> >  const __refconst struct hypervisor_x86 x86_hyper_xen_hvm = {
> >  
> >  	.name			= "Xen HVM",
> >  	.detect			= xen_hvm_platform,
> 
>     J

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

* Re: [Xen-devel] Re: [PATCH v2] xen: HVM X2APIC support
  2010-12-07  2:01   ` Sheng Yang
@ 2010-12-07  2:03     ` Sheng Yang
  0 siblings, 0 replies; 6+ messages in thread
From: Sheng Yang @ 2010-12-07  2:03 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: xen-devel, linux-kernel, Stefano Stabellini

On Tuesday 07 December 2010 10:01:48 Sheng Yang wrote:
> On Tuesday 07 December 2010 02:07:23 Jeremy Fitzhardinge wrote:
> > On 12/05/2010 11:49 PM, Sheng Yang wrote:
> > > This patch is similiar to Gleb Natapov's patch for KVM, which enable
> > > the hypervisor to emulate x2apic feature for the guest. By this way,
> > > the emulation of lapic would be simpler with x2apic interface(MSR),
> > > and faster.
> > > 
> > > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > > ---
> > > 
> > >  arch/x86/include/asm/xen/hypervisor.h |   35
> > >  +++++++++++++++++++++++++++++++++ arch/x86/kernel/apic/apic.c
> > >  
> > >  |    4 ++-
> > >  
> > >  arch/x86/xen/enlighten.c              |   30
> > >  ++++++++++----------------- 3 files changed, 49 insertions(+), 20
> > >  deletions(-)
> > > 
> > > diff --git a/arch/x86/include/asm/xen/hypervisor.h
> > > b/arch/x86/include/asm/xen/hypervisor.h index 396ff4c..bc5b804 100644
> > > --- a/arch/x86/include/asm/xen/hypervisor.h
> > > +++ b/arch/x86/include/asm/xen/hypervisor.h
> > > @@ -37,4 +37,39 @@
> > > 
> > >  extern struct shared_info *HYPERVISOR_shared_info;
> > >  extern struct start_info *xen_start_info;
> > > 
> > > +#include <asm/processor.h>
> > > +
> > > +static inline uint32_t xen_cpuid_base(void)
> > > +{
> > > +	uint32_t base, eax, ebx, ecx, edx;
> > > +	char signature[13];
> > > +
> > > +	for (base = 0x40000000; base < 0x40010000; base += 0x100) {
> > > +		cpuid(base, &eax, &ebx, &ecx, &edx);
> > > +		*(uint32_t *)(signature + 0) = ebx;
> > > +		*(uint32_t *)(signature + 4) = ecx;
> > > +		*(uint32_t *)(signature + 8) = edx;
> > > +		signature[12] = 0;
> > > +
> > > +		if (!strcmp("XenVMMXenVMM", signature) && ((eax - base) >= 2))
> > > +			return base;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +#ifdef CONFIG_XEN
> > > +extern bool xen_hvm_need_lapic(void);
> > > +
> > > +static inline bool xen_para_available(void)
> > > +{
> > > +	return xen_hvm_need_lapic();
> > > +}
> > > +#else
> > > +static inline bool xen_para_available(void)
> > > +{
> > > +	return (xen_cpuid_base() != 0);
> > > +}
> > > +#endif
> > 
> > Surely this should be using the asm/hypervisor.h interface?
> 
> I think it's the proper place. Or any suggestion?

Oh, misread the comment. Seems put all detection there is a good idea.

--
regards
Yang, Sheng

> 
> > > +
> > > 
> > >  #endif /* _ASM_X86_XEN_HYPERVISOR_H */
> > > 
> > > diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> > > index 3f838d5..1b68221 100644
> > > --- a/arch/x86/kernel/apic/apic.c
> > > +++ b/arch/x86/kernel/apic/apic.c
> > > @@ -52,6 +52,7 @@
> > > 
> > >  #include <asm/mce.h>
> > >  #include <asm/kvm_para.h>
> > >  #include <asm/tsc.h>
> > > 
> > > +#include <asm/xen/hypervisor.h>
> > > 
> > >  unsigned int num_processors;
> > > 
> > > @@ -1476,7 +1477,8 @@ void __init enable_IR_x2apic(void)
> > > 
> > >  		/* IR is required if there is APIC ID > 255 even when running
> > >  		
> > >  		 * under KVM
> > >  		 */
> > > 
> > > -		if (max_physical_apicid > 255 || !kvm_para_available())
> > > +		if (max_physical_apicid > 255 ||
> > > +		    (!kvm_para_available() && !xen_para_available()))
> > > 
> > >  			goto nox2apic;
> > 
> > What are the downsides of just using x2apic unconditionally?
> > 
> > Assuming there is some downside to using it all the time, what about
> > adding a pseudo-cpu feature flag meaning "use x2apic unconditionally"
> > rather than adding a bunch of ad-hoc tests here?
> 
> Because for native, x2apic should be used with interrupt-remapping,
> otherwise it didn't make much sense for native(the main reason to x2apic
> for bare-metal is support big machine with more than 255 APIC-ID, and
> without interrupt remapping it can't do so). But for hypervisor, we want
> to emulate the accessing faster through MSR, so we want them.
> 
> > >  		/*
> > >  		
> > >  		 * without IR all CPUs can be addressed by IOAPIC/MSI
> > > 
> > > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> > > index 235c0f4..7ef1645 100644
> > > --- a/arch/x86/xen/enlighten.c
> > > +++ b/arch/x86/xen/enlighten.c
> > > @@ -1245,25 +1245,6 @@ asmlinkage void __init xen_start_kernel(void)
> > > 
> > >  #endif
> > >  }
> > > 
> > > -static uint32_t xen_cpuid_base(void)
> > > -{
> > > -	uint32_t base, eax, ebx, ecx, edx;
> > > -	char signature[13];
> > > -
> > > -	for (base = 0x40000000; base < 0x40010000; base += 0x100) {
> > > -		cpuid(base, &eax, &ebx, &ecx, &edx);
> > > -		*(uint32_t *)(signature + 0) = ebx;
> > > -		*(uint32_t *)(signature + 4) = ecx;
> > > -		*(uint32_t *)(signature + 8) = edx;
> > > -		signature[12] = 0;
> > > -
> > > -		if (!strcmp("XenVMMXenVMM", signature) && ((eax - base) >= 2))
> > > -			return base;
> > > -	}
> > > -
> > > -	return 0;
> > > -}
> > > -
> > > 
> > >  static int init_hvm_pv_info(int *major, int *minor)
> > >  {
> > >  
> > >  	uint32_t eax, ebx, ecx, edx, pages, msr, base;
> > > 
> > > @@ -1373,6 +1354,17 @@ static bool __init xen_hvm_platform(void)
> > > 
> > >  	return true;
> > >  
> > >  }
> > > 
> > > +bool xen_hvm_need_lapic(void)
> > 
> > Where does this get used?
> 
> ?... xen_para_available() above...
> 
> --
> regards
> Yang, Sheng
> 
> > > +{
> > > +	if (xen_pv_domain())
> > > +		return false;
> > > +	if (xen_hvm_domain() &&
> > > +	   (xen_feature(XENFEAT_hvm_pirqs) || xen_have_vector_callback))
> > > +		return false;
> > > +	return (xen_cpuid_base() != 0);
> > > +}
> > > +EXPORT_SYMBOL_GPL(xen_hvm_need_lapic);
> > > +
> > > 
> > >  const __refconst struct hypervisor_x86 x86_hyper_xen_hvm = {
> > >  
> > >  	.name			= "Xen HVM",
> > >  	.detect			= xen_hvm_platform,
> > >  	
> >     J
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

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

* Re: [PATCH v2] xen: HVM X2APIC support
  2010-12-06 20:40 ` Konrad Rzeszutek Wilk
@ 2010-12-07  2:04   ` Sheng Yang
  0 siblings, 0 replies; 6+ messages in thread
From: Sheng Yang @ 2010-12-07  2:04 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: Jeremy Fitzhardinge, Stefano Stabellini, linux-kernel

On Tuesday 07 December 2010 04:40:54 Konrad Rzeszutek Wilk wrote:
> > +static inline uint32_t xen_cpuid_base(void)
> > +{
> > +	uint32_t base, eax, ebx, ecx, edx;
> > +	char signature[13];
> > +
> > +	for (base = 0x40000000; base < 0x40010000; base += 0x100) {
> > +		cpuid(base, &eax, &ebx, &ecx, &edx);
> > +		*(uint32_t *)(signature + 0) = ebx;
> > +		*(uint32_t *)(signature + 4) = ecx;
> > +		*(uint32_t *)(signature + 8) = edx;
> > +		signature[12] = 0;
> > +
> > +		if (!strcmp("XenVMMXenVMM", signature) && ((eax - base) >= 2))
> > +			return base;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +#ifdef CONFIG_XEN
> > +extern bool xen_hvm_need_lapic(void);
> > +
> > +static inline bool xen_para_available(void)
> > +{
> > +	return xen_hvm_need_lapic();
> > +}
> > +#else
> > +static inline bool xen_para_available(void)
> > +{
> > +	return (xen_cpuid_base() != 0);
> 
> Would it make sense to collapse the kvm_para_available and
> the xen_cpuid_base together (and maybe even the HyperV detection code)
> together in one and just return "x2_apic_para_capable" ?

Well, no hyper-v now. But put them together is a good idea.

--
regards
Yang, Sheng

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

end of thread, other threads:[~2010-12-07  2:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-06  7:49 [PATCH v2] xen: HVM X2APIC support Sheng Yang
2010-12-06 18:07 ` Jeremy Fitzhardinge
2010-12-07  2:01   ` Sheng Yang
2010-12-07  2:03     ` [Xen-devel] " Sheng Yang
2010-12-06 20:40 ` Konrad Rzeszutek Wilk
2010-12-07  2:04   ` Sheng Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox