Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 26 Aug 2018 20:55:06 +0000
From:      Colin Percival <cperciva@tarsnap.com>
To:        =?UTF-8?Q?Roger_Pau_Monn=c3=a9?= <royger@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r335668 - head/sys/x86/xen
Message-ID:  <010001657804d39a-54227e19-0fac-430f-9574-f83e225afb47-000000@email.amazonses.com>
In-Reply-To: <201806261500.w5QF0s1I071232@repo.freebsd.org>
References:  <201806261500.w5QF0s1I071232@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
This breaks the kernel boot in EC2 Xen-based instances; it now hangs
in the taskqgroup_adjust_if_io_tqg SYSINIT.  I've attached a verbose
dmesg from r335666 in case that helps you figure out what's going on.

My guess is that EC2 is numbering virtual CPUs oddly -- but regardless
of the cause, this definitely needs to be fixed.

Colin Percival

On 6/26/18 8:00 AM, Roger Pau Monné wrote:
> Author: royger
> Date: Tue Jun 26 15:00:54 2018
> New Revision: 335668
> URL: https://svnweb.freebsd.org/changeset/base/335668
> 
> Log:
>   xen: obtain vCPU ID from CPUID
>   
>   The Xen vCPU ID can be fetched from the cpuid instead of inferring it
>   from the ACPI ID.
>   
>   Sponsored by: Citrix Systems R&D
> 
> Modified:
>   head/sys/x86/xen/hvm.c
> 
> Modified: head/sys/x86/xen/hvm.c
> ==============================================================================
> --- head/sys/x86/xen/hvm.c	Tue Jun 26 14:48:23 2018	(r335667)
> +++ head/sys/x86/xen/hvm.c	Tue Jun 26 15:00:54 2018	(r335668)
> @@ -56,6 +56,7 @@ __FBSDID("$FreeBSD$");
>  #include <xen/hvm.h>
>  #include <xen/xen_intr.h>
>  
> +#include <xen/interface/arch-x86/cpuid.h>
>  #include <xen/interface/hvm/params.h>
>  #include <xen/interface/vcpu.h>
>  
> @@ -103,6 +104,9 @@ TUNABLE_INT("hw.xen.disable_pv_disks", &xen_disable_pv
>  TUNABLE_INT("hw.xen.disable_pv_nics", &xen_disable_pv_nics);
>  
>  /*---------------------- XEN Hypervisor Probe and Setup ----------------------*/
> +
> +static uint32_t cpuid_base;
> +
>  static uint32_t
>  xen_hvm_cpuid_base(void)
>  {
> @@ -123,21 +127,21 @@ xen_hvm_cpuid_base(void)
>  static int
>  xen_hvm_init_hypercall_stubs(enum xen_hvm_init_type init_type)
>  {
> -	uint32_t base, regs[4];
> +	uint32_t regs[4];
>  
>  	if (xen_pv_domain()) {
>  		/* hypercall page is already set in the PV case */
>  		return (0);
>  	}
>  
> -	base = xen_hvm_cpuid_base();
> -	if (base == 0)
> +	cpuid_base = xen_hvm_cpuid_base();
> +	if (cpuid_base == 0)
>  		return (ENXIO);
>  
>  	if (init_type == XEN_HVM_INIT_COLD) {
>  		int major, minor;
>  
> -		do_cpuid(base + 1, regs);
> +		do_cpuid(cpuid_base + 1, regs);
>  
>  		major = regs[0] >> 16;
>  		minor = regs[0] & 0xffff;
> @@ -165,7 +169,7 @@ xen_hvm_init_hypercall_stubs(enum xen_hvm_init_type in
>  	/*
>  	 * Find the hypercall pages.
>  	 */
> -	do_cpuid(base + 2, regs);
> +	do_cpuid(cpuid_base + 2, regs);
>  	if (regs[0] != 1)
>  		return (EINVAL);
>  
> @@ -371,31 +375,14 @@ xen_hvm_sysinit(void *arg __unused)
>  {
>  	xen_hvm_init(XEN_HVM_INIT_COLD);
>  }
> +SYSINIT(xen_hvm_init, SI_SUB_HYPERVISOR, SI_ORDER_FIRST, xen_hvm_sysinit, NULL);
>  
>  static void
> -xen_set_vcpu_id(void)
> -{
> -	struct pcpu *pc;
> -	int i;
> -
> -	if (!xen_hvm_domain())
> -		return;
> -
> -	/* Set vcpu_id to acpi_id */
> -	CPU_FOREACH(i) {
> -		pc = pcpu_find(i);
> -		pc->pc_vcpu_id = pc->pc_acpi_id;
> -		if (bootverbose)
> -			printf("XEN: CPU %u has VCPU ID %u\n",
> -			       i, pc->pc_vcpu_id);
> -	}
> -}
> -
> -static void
>  xen_hvm_cpu_init(void)
>  {
>  	struct vcpu_register_vcpu_info info;
>  	struct vcpu_info *vcpu_info;
> +	uint32_t regs[4];
>  	int cpu, rc;
>  
>  	if (!xen_domain())
> @@ -410,6 +397,22 @@ xen_hvm_cpu_init(void)
>  		return;
>  	}
>  
> +	/*
> +	 * Set vCPU ID. If available fetch the ID from CPUID, if not just use
> +	 * the ACPI ID.
> +	 */
> +	KASSERT(cpuid_base != 0, ("Invalid base Xen CPUID leaf"));
> +	cpuid_count(cpuid_base + 4, 0, regs);
> +	PCPU_SET(vcpu_id, (regs[0] & XEN_HVM_CPUID_VCPU_ID_PRESENT) ?
> +	    regs[1] : PCPU_GET(acpi_id));
> +
> +	/*
> +	 * Set the vCPU info.
> +	 *
> +	 * NB: the vCPU info for vCPUs < 32 can be fetched from the shared info
> +	 * page, but in order to make sure the mapping code is correct always
> +	 * attempt to map the vCPU info at a custom place.
> +	 */
>  	vcpu_info = DPCPU_PTR(vcpu_local_info);
>  	cpu = PCPU_GET(vcpu_id);
>  	info.mfn = vtophys(vcpu_info) >> PAGE_SHIFT;
> @@ -421,7 +424,4 @@ xen_hvm_cpu_init(void)
>  	else
>  		DPCPU_SET(vcpu_info, vcpu_info);
>  }
> -
> -SYSINIT(xen_hvm_init, SI_SUB_HYPERVISOR, SI_ORDER_FIRST, xen_hvm_sysinit, NULL);
>  SYSINIT(xen_hvm_cpu_init, SI_SUB_INTR, SI_ORDER_FIRST, xen_hvm_cpu_init, NULL);
> -SYSINIT(xen_set_vcpu_id, SI_SUB_CPU, SI_ORDER_ANY, xen_set_vcpu_id, NULL);
> 
> 
> 

-- 
Colin Percival
Security Officer Emeritus, FreeBSD | The power to serve
Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoid



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?010001657804d39a-54227e19-0fac-430f-9574-f83e225afb47-000000>