Re: [PATCH v2] xen: HVM X2APIC support

[Jeremy Fitzhardinge]

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@xxxxxxxxxxxxxxx>
> ---
>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/