[PATCH] xen: HVM X2APIC support

[PATCH] xen: HVM X2APIC support

[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 |   33 +++++++++++++++++++++++++++++++++
 arch/x86/kernel/apic/apic.c           |    4 +++-
 arch/x86/xen/enlighten.c              |   19 -------------------
 3 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/arch/x86/include/asm/xen/hypervisor.h b/arch/x86/include/asm/xen/hypervisor.h
index 396ff4c..e862874 100644
--- a/arch/x86/include/asm/xen/hypervisor.h
+++ b/arch/x86/include/asm/xen/hypervisor.h
@@ -37,4 +37,37 @@
 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
+static inline bool xen_para_available(void)
+{
+	return 0;
+}
+#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..819c365 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;
-- 
1.7.0.1

Re: [PATCH] xen: HVM X2APIC support

[Jeremy Fitzhardinge]

On 12/01/2010 07:03 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.

We have a set of patches to directly use event channels from within hvm
domains, completely bypassing the apic altogether.  Do we need this as well?

> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
>  arch/x86/include/asm/xen/hypervisor.h |   33 +++++++++++++++++++++++++++++++++
>  arch/x86/kernel/apic/apic.c           |    4 +++-
>  arch/x86/xen/enlighten.c              |   19 -------------------
>  3 files changed, 36 insertions(+), 20 deletions(-)
>
> diff --git a/arch/x86/include/asm/xen/hypervisor.h b/arch/x86/include/asm/xen/hypervisor.h
> index 396ff4c..e862874 100644
> --- a/arch/x86/include/asm/xen/hypervisor.h
> +++ b/arch/x86/include/asm/xen/hypervisor.h
> @@ -37,4 +37,37 @@
>  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
> +static inline bool xen_para_available(void)
> +{
> +	return 0;
> +}
> +#else
> +static inline bool xen_para_available(void)
> +{
> +	return (xen_cpuid_base() != 0);
> +}
> +#endif

So this returns true if you're running a kernel without CONFIG_XEN under
Xen?  Does that assume that all versions of Xen implement x2apic
emulation?  Why wouldn't we also want this for CONFIG_XEN kernels?

    J


> +
>  #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..819c365 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;

Re: [PATCH] xen: HVM X2APIC support

[Jeremy Fitzhardinge]

On 12/01/2010 07:03 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.

We have a set of patches to directly use event channels from within hvm
domains, completely bypassing the apic altogether.  Do we need this as well?

> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
>  arch/x86/include/asm/xen/hypervisor.h |   33 +++++++++++++++++++++++++++++++++
>  arch/x86/kernel/apic/apic.c           |    4 +++-
>  arch/x86/xen/enlighten.c              |   19 -------------------
>  3 files changed, 36 insertions(+), 20 deletions(-)
>
> diff --git a/arch/x86/include/asm/xen/hypervisor.h b/arch/x86/include/asm/xen/hypervisor.h
> index 396ff4c..e862874 100644
> --- a/arch/x86/include/asm/xen/hypervisor.h
> +++ b/arch/x86/include/asm/xen/hypervisor.h
> @@ -37,4 +37,37 @@
>  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
> +static inline bool xen_para_available(void)
> +{
> +	return 0;
> +}
> +#else
> +static inline bool xen_para_available(void)
> +{
> +	return (xen_cpuid_base() != 0);
> +}
> +#endif

So this returns true if you're running a kernel without CONFIG_XEN under
Xen?  Does that assume that all versions of Xen implement x2apic
emulation?  Why wouldn't we also want this for CONFIG_XEN kernels?

    J


> +
>  #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..819c365 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;

Re: [PATCH] xen: HVM X2APIC support

[Sheng Yang]

On Thursday 02 December 2010 14:28:16 Jeremy Fitzhardinge wrote:
> On 12/01/2010 07:03 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.
> 
> We have a set of patches to directly use event channels from within hvm
> domains, completely bypassing the apic altogether.  Do we need this as
> well?

This is for other HVMs. And the pvhvm still have limitation like it can't use 
MSI/MSI-X assigned device.

> 
> > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > ---
> > 
> >  arch/x86/include/asm/xen/hypervisor.h |   33
> >  +++++++++++++++++++++++++++++++++ arch/x86/kernel/apic/apic.c          
> >  |    4 +++-
> >  arch/x86/xen/enlighten.c              |   19 -------------------
> >  3 files changed, 36 insertions(+), 20 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/xen/hypervisor.h
> > b/arch/x86/include/asm/xen/hypervisor.h index 396ff4c..e862874 100644
> > --- a/arch/x86/include/asm/xen/hypervisor.h
> > +++ b/arch/x86/include/asm/xen/hypervisor.h
> > @@ -37,4 +37,37 @@
> > 
> >  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
> > +static inline bool xen_para_available(void)
> > +{
> > +	return 0;
> > +}
> > +#else
> > +static inline bool xen_para_available(void)
> > +{
> > +	return (xen_cpuid_base() != 0);
> > +}
> > +#endif
> 
> So this returns true if you're running a kernel without CONFIG_XEN under
> Xen?  Does that assume that all versions of Xen implement x2apic
> emulation?  Why wouldn't we also want this for CONFIG_XEN kernels?

Because only the ones that implement the feature would expose x2apic CPUID.

For CONFIG_XEN(pv or pvhvm), they both use evtchn, so no need for x2apic.

--
regards
Yang, Sheng

> 
>     J
> 
> > +
> > 
> >  #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..819c365 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;

Re: [PATCH] xen: HVM X2APIC support

[Sheng Yang]

On Thursday 02 December 2010 14:28:16 Jeremy Fitzhardinge wrote:
> On 12/01/2010 07:03 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.
> 
> We have a set of patches to directly use event channels from within hvm
> domains, completely bypassing the apic altogether.  Do we need this as
> well?

This is for other HVMs. And the pvhvm still have limitation like it can't use 
MSI/MSI-X assigned device.

> 
> > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > ---
> > 
> >  arch/x86/include/asm/xen/hypervisor.h |   33
> >  +++++++++++++++++++++++++++++++++ arch/x86/kernel/apic/apic.c          
> >  |    4 +++-
> >  arch/x86/xen/enlighten.c              |   19 -------------------
> >  3 files changed, 36 insertions(+), 20 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/xen/hypervisor.h
> > b/arch/x86/include/asm/xen/hypervisor.h index 396ff4c..e862874 100644
> > --- a/arch/x86/include/asm/xen/hypervisor.h
> > +++ b/arch/x86/include/asm/xen/hypervisor.h
> > @@ -37,4 +37,37 @@
> > 
> >  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
> > +static inline bool xen_para_available(void)
> > +{
> > +	return 0;
> > +}
> > +#else
> > +static inline bool xen_para_available(void)
> > +{
> > +	return (xen_cpuid_base() != 0);
> > +}
> > +#endif
> 
> So this returns true if you're running a kernel without CONFIG_XEN under
> Xen?  Does that assume that all versions of Xen implement x2apic
> emulation?  Why wouldn't we also want this for CONFIG_XEN kernels?

Because only the ones that implement the feature would expose x2apic CPUID.

For CONFIG_XEN(pv or pvhvm), they both use evtchn, so no need for x2apic.

--
regards
Yang, Sheng

> 
>     J
> 
> > +
> > 
> >  #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..819c365 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;

Re: [PATCH] xen: HVM X2APIC support

[Stefano Stabellini]

On Thu, 2 Dec 2010, Sheng Yang wrote:
> On Thursday 02 December 2010 14:28:16 Jeremy Fitzhardinge wrote:
> > On 12/01/2010 07:03 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.
> > 
> > We have a set of patches to directly use event channels from within hvm
> > domains, completely bypassing the apic altogether.  Do we need this as
> > well?
> 
> This is for other HVMs. And the pvhvm still have limitation like it can't use 
> MSI/MSI-X assigned device.

That is not true: upstream Linux kernels can remap MSI/MSI-X into pirqs,
if it doesn't work is a bug :)
If you are interested give a look at arch/x86/pci/xen.c:xen_hvm_setup_msi_irqs.

> 
> > 
> > > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > > ---
> > > 
> > >  arch/x86/include/asm/xen/hypervisor.h |   33
> > >  +++++++++++++++++++++++++++++++++ arch/x86/kernel/apic/apic.c          
> > >  |    4 +++-
> > >  arch/x86/xen/enlighten.c              |   19 -------------------
> > >  3 files changed, 36 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/arch/x86/include/asm/xen/hypervisor.h
> > > b/arch/x86/include/asm/xen/hypervisor.h index 396ff4c..e862874 100644
> > > --- a/arch/x86/include/asm/xen/hypervisor.h
> > > +++ b/arch/x86/include/asm/xen/hypervisor.h
> > > @@ -37,4 +37,37 @@
> > > 
> > >  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
> > > +static inline bool xen_para_available(void)
> > > +{
> > > +	return 0;
> > > +}
> > > +#else
> > > +static inline bool xen_para_available(void)
> > > +{
> > > +	return (xen_cpuid_base() != 0);
> > > +}
> > > +#endif
> > 
> > So this returns true if you're running a kernel without CONFIG_XEN under
> > Xen?  Does that assume that all versions of Xen implement x2apic
> > emulation?  Why wouldn't we also want this for CONFIG_XEN kernels?
> 
> Because only the ones that implement the feature would expose x2apic CPUID.
> 
> For CONFIG_XEN(pv or pvhvm), they both use evtchn, so no need for x2apic.

In that case you need to check for CONFIG_XEN_PVHVM and the presence of
xen_feature(XENFEAT_hvm_pirqs) because only in this case a PV on HVM
guests are able to remap both GSIs and MSIs into evtchns.
So I would do something like this:


#ifdef CONFIG_XEN_PVHVM
static inline bool xen_para_available(void)
{
    if (xen_cpuid_base() != 0 && xen_feature(XENFEAT_hvm_pirqs))
    	return 0;
    else
        return 1;
}
#else
static inline bool xen_para_available(void)
{
	return (xen_cpuid_base() != 0);
}
#endif


This is assuming that enabling x2apic doesn't prevent Linux from
receiving evtchns either using the callback vector mechanism or the
legacy platform-pci interrupt.
Finally when running as dom0 would this feature create problems in the
presence of a real x2apic?

Re: [PATCH] xen: HVM X2APIC support

[Sheng Yang]

On Thursday 02 December 2010 21:54:55 Stefano Stabellini wrote:
> On Thu, 2 Dec 2010, Sheng Yang wrote:
> > On Thursday 02 December 2010 14:28:16 Jeremy Fitzhardinge wrote:
> > > On 12/01/2010 07:03 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.
> > > 
> > > We have a set of patches to directly use event channels from within hvm
> > > domains, completely bypassing the apic altogether.  Do we need this as
> > > well?
> > 
> > This is for other HVMs. And the pvhvm still have limitation like it can't
> > use MSI/MSI-X assigned device.
> 
> That is not true: upstream Linux kernels can remap MSI/MSI-X into pirqs,
> if it doesn't work is a bug :)
> If you are interested give a look at
> arch/x86/pci/xen.c:xen_hvm_setup_msi_irqs.

That's great!
> 
> > > > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > > > ---
> > > > 
> > > >  arch/x86/include/asm/xen/hypervisor.h |   33
> > > >  +++++++++++++++++++++++++++++++++ arch/x86/kernel/apic/apic.c
> > > >  
> > > >  |    4 +++-
> > > >  
> > > >  arch/x86/xen/enlighten.c              |   19 -------------------
> > > >  3 files changed, 36 insertions(+), 20 deletions(-)
> > > > 
> > > > diff --git a/arch/x86/include/asm/xen/hypervisor.h
> > > > b/arch/x86/include/asm/xen/hypervisor.h index 396ff4c..e862874 100644
> > > > --- a/arch/x86/include/asm/xen/hypervisor.h
> > > > +++ b/arch/x86/include/asm/xen/hypervisor.h
> > > > @@ -37,4 +37,37 @@
> > > > 
> > > >  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
> > > > +static inline bool xen_para_available(void)
> > > > +{
> > > > +	return 0;
> > > > +}
> > > > +#else
> > > > +static inline bool xen_para_available(void)
> > > > +{
> > > > +	return (xen_cpuid_base() != 0);
> > > > +}
> > > > +#endif
> > > 
> > > So this returns true if you're running a kernel without CONFIG_XEN
> > > under Xen?  Does that assume that all versions of Xen implement x2apic
> > > emulation?  Why wouldn't we also want this for CONFIG_XEN kernels?
> > 
> > Because only the ones that implement the feature would expose x2apic
> > CPUID.
> > 
> > For CONFIG_XEN(pv or pvhvm), they both use evtchn, so no need for x2apic.
> 
> In that case you need to check for CONFIG_XEN_PVHVM and the presence of
> xen_feature(XENFEAT_hvm_pirqs) because only in this case a PV on HVM
> guests are able to remap both GSIs and MSIs into evtchns.
> So I would do something like this:
> 
> 
> #ifdef CONFIG_XEN_PVHVM
> static inline bool xen_para_available(void)
> {
>     if (xen_cpuid_base() != 0 && xen_feature(XENFEAT_hvm_pirqs))
>     	return 0;
>     else
>         return 1;

I suppose only HVM guest without XENFEAT_hvm_pirqs may need this. But does this 
code covered PV guest as well? We don't need cover them.

> }
> #else
> static inline bool xen_para_available(void)
> {
> 	return (xen_cpuid_base() != 0);
> }
> #endif
> 
> 
> This is assuming that enabling x2apic doesn't prevent Linux from
> receiving evtchns either using the callback vector mechanism or the
> legacy platform-pci interrupt.

I suppose only legacy platform-pci would need this, because it would use lapic. 
Callback vector method would use evtchn so this won't be enabled.

> Finally when running as dom0 would this feature create problems in the
> presence of a real x2apic?

I don't think this can be enabled on dom0.

This one target on HVM domain, maybe also PVHVM domain without XENFEAT_hvm_pirqs, 
but not the domains using evtchn.

--
regards
Yang, Sheng

Re: [PATCH] xen: HVM X2APIC support

[Stefano Stabellini]

On Fri, 3 Dec 2010, Sheng Yang wrote:
> I suppose only HVM guest without XENFEAT_hvm_pirqs may need this. But does this 
> code covered PV guest as well? We don't need cover them.
> 

We can add a check on xen_pv_domain() to rule PV guests out.

> > }
> > #else
> > static inline bool xen_para_available(void)
> > {
> > 	return (xen_cpuid_base() != 0);
> > }
> > #endif
> > 
> > 
> > This is assuming that enabling x2apic doesn't prevent Linux from
> > receiving evtchns either using the callback vector mechanism or the
> > legacy platform-pci interrupt.
> 
> I suppose only legacy platform-pci would need this, because it would use lapic. 
> Callback vector method would use evtchn so this won't be enabled.
> 

Right, should add a check on xen_have_vector_callback too.


> > Finally when running as dom0 would this feature create problems in the
> > presence of a real x2apic?
> 
> I don't think this can be enabled on dom0.
> 
> This one target on HVM domain, maybe also PVHVM domain without XENFEAT_hvm_pirqs, 
> but not the domains using evtchn.
> 

Ok, a check on xen_pv_domain() will disable x2apic for both pv guests
and dom0:


#ifdef CONFIG_XEN_PVHVM
static inline bool xen_para_available(void)
{
    if (xen_pv_domain())
        return 0;
    if (xen_cpuid_base() != 0 &&
            xen_feature(XENFEAT_hvm_pirqs) &&
            xen_have_vector_callback)
        return 0;
    else
        return 1;
}