Re: [Xen-devel] [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping
Hi Vitaly, On 2016/9/5 17:42, Vitaly Kuznetsov wrote: > Julien Grall writes: > >> Hi Vitaly, >> >> On 26/07/16 13:30, Vitaly Kuznetsov wrote: >>> It may happen that Xen's and Linux's ideas of vCPU id diverge. In >>> particular, when we crash on a secondary vCPU we may want to do kdump >>> and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting >>> on the vCPU which crashed. This doesn't work very well for PVHVM guests as >>> we have a number of hypercalls where we pass vCPU id as a parameter. These >>> hypercalls either fail or do something unexpected. To solve the issue >>> introduce percpu xen_vcpu_id mapping. ARM and PV guests get direct mapping >>> for now. Boot CPU for PVHVM guest gets its id from CPUID. With secondary >>> CPUs it is a bit more trickier. Currently, we initialize IPI vectors >>> before these CPUs boot so we can't use CPUID. Use ACPI ids from MADT >>> instead. >>> >>> Signed-off-by: Vitaly Kuznetsov >>> --- >>> Changes since v2: >>> - Use uint32_t for xen_vcpu_id mapping [Julien Grall] >>> >>> Changes since v1: >>> - Introduce xen_vcpu_nr() helper [David Vrabel] >>> - Use ACPI ids instead of vLAPIC ids /2 [Andrew Cooper, Jan Beulich] >>> --- >>> arch/arm/xen/enlighten.c | 10 ++ >>> arch/x86/xen/enlighten.c | 23 ++- >>> include/xen/xen-ops.h| 6 ++ >>> 3 files changed, 38 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c >>> index 75cd734..fe32267 100644 >>> --- a/arch/arm/xen/enlighten.c >>> +++ b/arch/arm/xen/enlighten.c >>> @@ -46,6 +46,10 @@ struct shared_info *HYPERVISOR_shared_info = (void >>> *)&xen_dummy_shared_info; >>> DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu); >>> static struct vcpu_info __percpu *xen_vcpu_info; >>> >>> +/* Linux <-> Xen vCPU id mapping */ >>> +DEFINE_PER_CPU(uint32_t, xen_vcpu_id) = U32_MAX; >>> +EXPORT_PER_CPU_SYMBOL(xen_vcpu_id); >>> + >>> /* These are unused until we support booting "pre-ballooned" */ >>> unsigned long xen_released_pages; >>> struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] >>> __initdata; >>> @@ -179,6 +183,9 @@ static void xen_percpu_init(void) >>> pr_info("Xen: initializing cpu%d\n", cpu); >>> vcpup = per_cpu_ptr(xen_vcpu_info, cpu); >>> >>> +/* Direct vCPU id mapping for ARM guests. */ >>> +per_cpu(xen_vcpu_id, cpu) = cpu; >>> + >> >> We did some internal testing on ARM64 with the latest Linux kernel >> (4.8-rc4) and noticed that this patch is breaking SMP support. Sorry >> for noticing the issue that late. > > Sorry for the breakage :-( > >> >> This function is called on the running CPU whilst some code (e.g >> init_control_block in drivers/xen/events/events_fifo.c) is executed >> whilst preparing the CPU on the boot CPU. >> >> So xen_vcpu_nr(cpu) will always return 0 in this case and >> init_control_block will fail to execute. >> > > I see, > > CPU_UP_PREPARE event happens before xen_starting_cpu() is called. > > >> I am not sure how to fix. I guess we could setup per_cpu(xen_vcpu_id, >> *) in xen_guest_init. Any opinions? > > As we're not doing kexec on ARM we can fix the immediate issue. I don't > know much about ARM and unfortunatelly I don't have a setup to test but > it seems there is no early_per_cpu* infrastructure for ARM so we may fix > it with the following: > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > index 3d2cef6..f193414 100644 > --- a/arch/arm/xen/enlighten.c > +++ b/arch/arm/xen/enlighten.c > @@ -170,9 +170,6 @@ static int xen_starting_cpu(unsigned int cpu) > pr_info("Xen: initializing cpu%d\n", cpu); > vcpup = per_cpu_ptr(xen_vcpu_info, cpu); > > - /* Direct vCPU id mapping for ARM guests. */ > - per_cpu(xen_vcpu_id, cpu) = cpu; > - > info.mfn = virt_to_gfn(vcpup); > info.offset = xen_offset_in_page(vcpup); > > @@ -330,6 +327,7 @@ static int __init xen_guest_init(void) > { > struct xen_add_to_physmap xatp; > struct shared_info *shared_info_page = NULL; > + int cpu; > > if (!xen_domain()) > return 0; > @@ -380,7 +378,8 @@ static int __init xen_guest_init(void) > return -ENOMEM; > > /* Direct vCPU id mapping for ARM guests. */ > - per_cpu(xen_vcpu_id, 0) = 0; > + for_each_possible_cpu(cpu) > + per_cpu(xen_vcpu_id, cpu) = cpu; > > xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames(); > if (xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn, > > (not tested, if we can't use for_each_possible_cpu() that early we'll > have to check against NR_CPUS instead). > I have tested this patch just now, it can work with the latest Linux kernel and latest Xen hypervisor on my ARM platform: [0.299927] xen:events: Using FIFO-based ABI [0.304259] Xen: initializing cpu0 [0.336402] EFI services will not be available. [0.388985] Detected PIPT I-cache on CPU1 [0.389024] Xen: initia
Re: [Xen-devel] [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping
Hi Vitaly, On 07/09/2016 12:23, Vitaly Kuznetsov wrote: BTW, were you able to try the patch I suggested? In my opinion it would be preferable to fix the immediate SMP issue now and play with MPIDR info later. Not yet sorry. I will see if I can try to today or tomorrow. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping
Julien Grall writes: > Hi Vitaly, > > On 07/09/2016 10:07, Vitaly Kuznetsov wrote: >> Stefano Stabellini writes: >>> I don't know that much about cpuid, but the virtual MPIDR is constructed >>> from the vcpu id right now: >>> >>> v->arch.vmpidr = MPIDR_SMP | vcpuid_to_vaffinity(v->vcpu_id); >>> >>> [...] >>> >>> static inline register_t vcpuid_to_vaffinity(unsigned int vcpuid) >>> { >>> register_t vaff; >>> >>> vaff = (vcpuid & 0x0f) << MPIDR_LEVEL_SHIFT(0); >>> vaff |= ((vcpuid >> 4) & MPIDR_LEVEL_MASK) << MPIDR_LEVEL_SHIFT(1); >>> >>> return vaff; >>> } >> >> This could work but only in case there is a way to get MPIDR for _other_ >> cpu (e.g. CPU0 needs to get MPIDR of CPU1 when CPU1 is not yet runnning) >> or we'll have to change the machinery of how we bring up secondary CPUs >> - e.g. CPUn starts, writes its id somewhere and 'hangs' waiting for CPU0 >> to set up event channels. > > You can get the MPIDR from both the device tree and ACPI. The firmware > table is parsed at boot time and the value is stored in > cpu_logical_map(). Good, in this case we can easily do the same trick we did for x86 and we don't need to change the way secondary CPUs are strarted. BTW, were you able to try the patch I suggested? In my opinion it would be preferable to fix the immediate SMP issue now and play with MPIDR info later. > > Regards, -- Vitaly ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping
Hi Vitaly, On 07/09/2016 10:07, Vitaly Kuznetsov wrote: Stefano Stabellini writes: I don't know that much about cpuid, but the virtual MPIDR is constructed from the vcpu id right now: v->arch.vmpidr = MPIDR_SMP | vcpuid_to_vaffinity(v->vcpu_id); [...] static inline register_t vcpuid_to_vaffinity(unsigned int vcpuid) { register_t vaff; vaff = (vcpuid & 0x0f) << MPIDR_LEVEL_SHIFT(0); vaff |= ((vcpuid >> 4) & MPIDR_LEVEL_MASK) << MPIDR_LEVEL_SHIFT(1); return vaff; } This could work but only in case there is a way to get MPIDR for _other_ cpu (e.g. CPU0 needs to get MPIDR of CPU1 when CPU1 is not yet runnning) or we'll have to change the machinery of how we bring up secondary CPUs - e.g. CPUn starts, writes its id somewhere and 'hangs' waiting for CPU0 to set up event channels. You can get the MPIDR from both the device tree and ACPI. The firmware table is parsed at boot time and the value is stored in cpu_logical_map(). Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping
On 07/09/16 10:07, Vitaly Kuznetsov wrote: > Stefano Stabellini writes: > >> On Tue, 6 Sep 2016, Vitaly Kuznetsov wrote: >>> Stefano Stabellini writes: >>> On Mon, 5 Sep 2016, Vitaly Kuznetsov wrote: > Julien Grall writes: > >> Hi Vitaly, >> >> On 26/07/16 13:30, Vitaly Kuznetsov wrote: >>> It may happen that Xen's and Linux's ideas of vCPU id diverge. In >>> particular, when we crash on a secondary vCPU we may want to do kdump >>> and unlike plain kexec where we do migrate_to_reboot_cpu() we try >>> booting >>> on the vCPU which crashed. This doesn't work very well for PVHVM guests >>> as >>> we have a number of hypercalls where we pass vCPU id as a parameter. >>> These >>> hypercalls either fail or do something unexpected. To solve the issue >>> introduce percpu xen_vcpu_id mapping. ARM and PV guests get direct >>> mapping >>> for now. Boot CPU for PVHVM guest gets its id from CPUID. With secondary >>> CPUs it is a bit more trickier. Currently, we initialize IPI vectors >>> before these CPUs boot so we can't use CPUID. Use ACPI ids from MADT >>> instead. >>> >>> Signed-off-by: Vitaly Kuznetsov >>> --- >>> Changes since v2: >>> - Use uint32_t for xen_vcpu_id mapping [Julien Grall] >>> >>> Changes since v1: >>> - Introduce xen_vcpu_nr() helper [David Vrabel] >>> - Use ACPI ids instead of vLAPIC ids /2 [Andrew Cooper, Jan Beulich] >>> --- >>> arch/arm/xen/enlighten.c | 10 ++ >>> arch/x86/xen/enlighten.c | 23 ++- >>> include/xen/xen-ops.h| 6 ++ >>> 3 files changed, 38 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c >>> index 75cd734..fe32267 100644 >>> --- a/arch/arm/xen/enlighten.c >>> +++ b/arch/arm/xen/enlighten.c >>> @@ -46,6 +46,10 @@ struct shared_info *HYPERVISOR_shared_info = (void >>> *)&xen_dummy_shared_info; >>> DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu); >>> static struct vcpu_info __percpu *xen_vcpu_info; >>> >>> +/* Linux <-> Xen vCPU id mapping */ >>> +DEFINE_PER_CPU(uint32_t, xen_vcpu_id) = U32_MAX; >>> +EXPORT_PER_CPU_SYMBOL(xen_vcpu_id); >>> + >>> /* These are unused until we support booting "pre-ballooned" */ >>> unsigned long xen_released_pages; >>> struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] >>> __initdata; >>> @@ -179,6 +183,9 @@ static void xen_percpu_init(void) >>> pr_info("Xen: initializing cpu%d\n", cpu); >>> vcpup = per_cpu_ptr(xen_vcpu_info, cpu); >>> >>> + /* Direct vCPU id mapping for ARM guests. */ >>> + per_cpu(xen_vcpu_id, cpu) = cpu; >>> + >> >> We did some internal testing on ARM64 with the latest Linux kernel >> (4.8-rc4) and noticed that this patch is breaking SMP support. Sorry >> for noticing the issue that late. > > Sorry for the breakage :-( > >> >> This function is called on the running CPU whilst some code (e.g >> init_control_block in drivers/xen/events/events_fifo.c) is executed >> whilst preparing the CPU on the boot CPU. >> >> So xen_vcpu_nr(cpu) will always return 0 in this case and >> init_control_block will fail to execute. >> > > I see, > > CPU_UP_PREPARE event happens before xen_starting_cpu() is called. > > >> I am not sure how to fix. I guess we could setup per_cpu(xen_vcpu_id, >> *) in xen_guest_init. Any opinions? > > As we're not doing kexec on ARM we can fix the immediate issue. I don't > know much about ARM and unfortunatelly I don't have a setup to test but > it seems there is no early_per_cpu* infrastructure for ARM so we may fix > it with the following: > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > index 3d2cef6..f193414 100644 > --- a/arch/arm/xen/enlighten.c > +++ b/arch/arm/xen/enlighten.c > @@ -170,9 +170,6 @@ static int xen_starting_cpu(unsigned int cpu) > pr_info("Xen: initializing cpu%d\n", cpu); > vcpup = per_cpu_ptr(xen_vcpu_info, cpu); > > - /* Direct vCPU id mapping for ARM guests. */ > - per_cpu(xen_vcpu_id, cpu) = cpu; > - > info.mfn = virt_to_gfn(vcpup); > info.offset = xen_offset_in_page(vcpup); > > @@ -330,6 +327,7 @@ static int __init xen_guest_init(void) > { > struct xen_add_to_physmap xatp; > struct shared_info *shared_info_page = NULL; > + int cpu; > > if (!xen_domain()) > return 0; > @@ -380,7 +378,8 @@ static int __init xen_guest_init(void) > return -ENOMEM; > > /* Direct vCPU id mapping for ARM guests. */ > - per_cpu(xen_vcpu_id, 0) = 0; > + for_ea
Re: [Xen-devel] [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping
Stefano Stabellini writes: > On Tue, 6 Sep 2016, Vitaly Kuznetsov wrote: >> Stefano Stabellini writes: >> >> > On Mon, 5 Sep 2016, Vitaly Kuznetsov wrote: >> >> Julien Grall writes: >> >> >> >> > Hi Vitaly, >> >> > >> >> > On 26/07/16 13:30, Vitaly Kuznetsov wrote: >> >> >> It may happen that Xen's and Linux's ideas of vCPU id diverge. In >> >> >> particular, when we crash on a secondary vCPU we may want to do kdump >> >> >> and unlike plain kexec where we do migrate_to_reboot_cpu() we try >> >> >> booting >> >> >> on the vCPU which crashed. This doesn't work very well for PVHVM >> >> >> guests as >> >> >> we have a number of hypercalls where we pass vCPU id as a parameter. >> >> >> These >> >> >> hypercalls either fail or do something unexpected. To solve the issue >> >> >> introduce percpu xen_vcpu_id mapping. ARM and PV guests get direct >> >> >> mapping >> >> >> for now. Boot CPU for PVHVM guest gets its id from CPUID. With >> >> >> secondary >> >> >> CPUs it is a bit more trickier. Currently, we initialize IPI vectors >> >> >> before these CPUs boot so we can't use CPUID. Use ACPI ids from MADT >> >> >> instead. >> >> >> >> >> >> Signed-off-by: Vitaly Kuznetsov >> >> >> --- >> >> >> Changes since v2: >> >> >> - Use uint32_t for xen_vcpu_id mapping [Julien Grall] >> >> >> >> >> >> Changes since v1: >> >> >> - Introduce xen_vcpu_nr() helper [David Vrabel] >> >> >> - Use ACPI ids instead of vLAPIC ids /2 [Andrew Cooper, Jan Beulich] >> >> >> --- >> >> >> arch/arm/xen/enlighten.c | 10 ++ >> >> >> arch/x86/xen/enlighten.c | 23 ++- >> >> >> include/xen/xen-ops.h| 6 ++ >> >> >> 3 files changed, 38 insertions(+), 1 deletion(-) >> >> >> >> >> >> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c >> >> >> index 75cd734..fe32267 100644 >> >> >> --- a/arch/arm/xen/enlighten.c >> >> >> +++ b/arch/arm/xen/enlighten.c >> >> >> @@ -46,6 +46,10 @@ struct shared_info *HYPERVISOR_shared_info = (void >> >> >> *)&xen_dummy_shared_info; >> >> >> DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu); >> >> >> static struct vcpu_info __percpu *xen_vcpu_info; >> >> >> >> >> >> +/* Linux <-> Xen vCPU id mapping */ >> >> >> +DEFINE_PER_CPU(uint32_t, xen_vcpu_id) = U32_MAX; >> >> >> +EXPORT_PER_CPU_SYMBOL(xen_vcpu_id); >> >> >> + >> >> >> /* These are unused until we support booting "pre-ballooned" */ >> >> >> unsigned long xen_released_pages; >> >> >> struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] >> >> >> __initdata; >> >> >> @@ -179,6 +183,9 @@ static void xen_percpu_init(void) >> >> >>pr_info("Xen: initializing cpu%d\n", cpu); >> >> >>vcpup = per_cpu_ptr(xen_vcpu_info, cpu); >> >> >> >> >> >> + /* Direct vCPU id mapping for ARM guests. */ >> >> >> + per_cpu(xen_vcpu_id, cpu) = cpu; >> >> >> + >> >> > >> >> > We did some internal testing on ARM64 with the latest Linux kernel >> >> > (4.8-rc4) and noticed that this patch is breaking SMP support. Sorry >> >> > for noticing the issue that late. >> >> >> >> Sorry for the breakage :-( >> >> >> >> > >> >> > This function is called on the running CPU whilst some code (e.g >> >> > init_control_block in drivers/xen/events/events_fifo.c) is executed >> >> > whilst preparing the CPU on the boot CPU. >> >> > >> >> > So xen_vcpu_nr(cpu) will always return 0 in this case and >> >> > init_control_block will fail to execute. >> >> > >> >> >> >> I see, >> >> >> >> CPU_UP_PREPARE event happens before xen_starting_cpu() is called. >> >> >> >> >> >> > I am not sure how to fix. I guess we could setup per_cpu(xen_vcpu_id, >> >> > *) in xen_guest_init. Any opinions? >> >> >> >> As we're not doing kexec on ARM we can fix the immediate issue. I don't >> >> know much about ARM and unfortunatelly I don't have a setup to test but >> >> it seems there is no early_per_cpu* infrastructure for ARM so we may fix >> >> it with the following: >> >> >> >> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c >> >> index 3d2cef6..f193414 100644 >> >> --- a/arch/arm/xen/enlighten.c >> >> +++ b/arch/arm/xen/enlighten.c >> >> @@ -170,9 +170,6 @@ static int xen_starting_cpu(unsigned int cpu) >> >> pr_info("Xen: initializing cpu%d\n", cpu); >> >> vcpup = per_cpu_ptr(xen_vcpu_info, cpu); >> >> >> >> - /* Direct vCPU id mapping for ARM guests. */ >> >> - per_cpu(xen_vcpu_id, cpu) = cpu; >> >> - >> >> info.mfn = virt_to_gfn(vcpup); >> >> info.offset = xen_offset_in_page(vcpup); >> >> >> >> @@ -330,6 +327,7 @@ static int __init xen_guest_init(void) >> >> { >> >> struct xen_add_to_physmap xatp; >> >> struct shared_info *shared_info_page = NULL; >> >> + int cpu; >> >> >> >> if (!xen_domain()) >> >> return 0; >> >> @@ -380,7 +378,8 @@ static int __init xen_guest_init(void) >> >> return -ENOMEM; >> >> >> >> /* Direct vCPU id mapping for ARM guests. */ >> >> - per_cpu(xen_
Re: [Xen-devel] [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping
On Tue, 6 Sep 2016, Vitaly Kuznetsov wrote: > Stefano Stabellini writes: > > > On Mon, 5 Sep 2016, Vitaly Kuznetsov wrote: > >> Julien Grall writes: > >> > >> > Hi Vitaly, > >> > > >> > On 26/07/16 13:30, Vitaly Kuznetsov wrote: > >> >> It may happen that Xen's and Linux's ideas of vCPU id diverge. In > >> >> particular, when we crash on a secondary vCPU we may want to do kdump > >> >> and unlike plain kexec where we do migrate_to_reboot_cpu() we try > >> >> booting > >> >> on the vCPU which crashed. This doesn't work very well for PVHVM guests > >> >> as > >> >> we have a number of hypercalls where we pass vCPU id as a parameter. > >> >> These > >> >> hypercalls either fail or do something unexpected. To solve the issue > >> >> introduce percpu xen_vcpu_id mapping. ARM and PV guests get direct > >> >> mapping > >> >> for now. Boot CPU for PVHVM guest gets its id from CPUID. With secondary > >> >> CPUs it is a bit more trickier. Currently, we initialize IPI vectors > >> >> before these CPUs boot so we can't use CPUID. Use ACPI ids from MADT > >> >> instead. > >> >> > >> >> Signed-off-by: Vitaly Kuznetsov > >> >> --- > >> >> Changes since v2: > >> >> - Use uint32_t for xen_vcpu_id mapping [Julien Grall] > >> >> > >> >> Changes since v1: > >> >> - Introduce xen_vcpu_nr() helper [David Vrabel] > >> >> - Use ACPI ids instead of vLAPIC ids /2 [Andrew Cooper, Jan Beulich] > >> >> --- > >> >> arch/arm/xen/enlighten.c | 10 ++ > >> >> arch/x86/xen/enlighten.c | 23 ++- > >> >> include/xen/xen-ops.h| 6 ++ > >> >> 3 files changed, 38 insertions(+), 1 deletion(-) > >> >> > >> >> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > >> >> index 75cd734..fe32267 100644 > >> >> --- a/arch/arm/xen/enlighten.c > >> >> +++ b/arch/arm/xen/enlighten.c > >> >> @@ -46,6 +46,10 @@ struct shared_info *HYPERVISOR_shared_info = (void > >> >> *)&xen_dummy_shared_info; > >> >> DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu); > >> >> static struct vcpu_info __percpu *xen_vcpu_info; > >> >> > >> >> +/* Linux <-> Xen vCPU id mapping */ > >> >> +DEFINE_PER_CPU(uint32_t, xen_vcpu_id) = U32_MAX; > >> >> +EXPORT_PER_CPU_SYMBOL(xen_vcpu_id); > >> >> + > >> >> /* These are unused until we support booting "pre-ballooned" */ > >> >> unsigned long xen_released_pages; > >> >> struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] > >> >> __initdata; > >> >> @@ -179,6 +183,9 @@ static void xen_percpu_init(void) > >> >> pr_info("Xen: initializing cpu%d\n", cpu); > >> >> vcpup = per_cpu_ptr(xen_vcpu_info, cpu); > >> >> > >> >> + /* Direct vCPU id mapping for ARM guests. */ > >> >> + per_cpu(xen_vcpu_id, cpu) = cpu; > >> >> + > >> > > >> > We did some internal testing on ARM64 with the latest Linux kernel > >> > (4.8-rc4) and noticed that this patch is breaking SMP support. Sorry > >> > for noticing the issue that late. > >> > >> Sorry for the breakage :-( > >> > >> > > >> > This function is called on the running CPU whilst some code (e.g > >> > init_control_block in drivers/xen/events/events_fifo.c) is executed > >> > whilst preparing the CPU on the boot CPU. > >> > > >> > So xen_vcpu_nr(cpu) will always return 0 in this case and > >> > init_control_block will fail to execute. > >> > > >> > >> I see, > >> > >> CPU_UP_PREPARE event happens before xen_starting_cpu() is called. > >> > >> > >> > I am not sure how to fix. I guess we could setup per_cpu(xen_vcpu_id, > >> > *) in xen_guest_init. Any opinions? > >> > >> As we're not doing kexec on ARM we can fix the immediate issue. I don't > >> know much about ARM and unfortunatelly I don't have a setup to test but > >> it seems there is no early_per_cpu* infrastructure for ARM so we may fix > >> it with the following: > >> > >> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > >> index 3d2cef6..f193414 100644 > >> --- a/arch/arm/xen/enlighten.c > >> +++ b/arch/arm/xen/enlighten.c > >> @@ -170,9 +170,6 @@ static int xen_starting_cpu(unsigned int cpu) > >> pr_info("Xen: initializing cpu%d\n", cpu); > >> vcpup = per_cpu_ptr(xen_vcpu_info, cpu); > >> > >> - /* Direct vCPU id mapping for ARM guests. */ > >> - per_cpu(xen_vcpu_id, cpu) = cpu; > >> - > >> info.mfn = virt_to_gfn(vcpup); > >> info.offset = xen_offset_in_page(vcpup); > >> > >> @@ -330,6 +327,7 @@ static int __init xen_guest_init(void) > >> { > >> struct xen_add_to_physmap xatp; > >> struct shared_info *shared_info_page = NULL; > >> + int cpu; > >> > >> if (!xen_domain()) > >> return 0; > >> @@ -380,7 +378,8 @@ static int __init xen_guest_init(void) > >> return -ENOMEM; > >> > >> /* Direct vCPU id mapping for ARM guests. */ > >> - per_cpu(xen_vcpu_id, 0) = 0; > >> + for_each_possible_cpu(cpu) > >> + per_cpu(xen_vcpu_id, cpu) = cpu; > >> > >> xen_auto_xlat_grant_fram
Re: [Xen-devel] [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping
Stefano Stabellini writes: > On Mon, 5 Sep 2016, Vitaly Kuznetsov wrote: >> Julien Grall writes: >> >> > Hi Vitaly, >> > >> > On 26/07/16 13:30, Vitaly Kuznetsov wrote: >> >> It may happen that Xen's and Linux's ideas of vCPU id diverge. In >> >> particular, when we crash on a secondary vCPU we may want to do kdump >> >> and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting >> >> on the vCPU which crashed. This doesn't work very well for PVHVM guests as >> >> we have a number of hypercalls where we pass vCPU id as a parameter. These >> >> hypercalls either fail or do something unexpected. To solve the issue >> >> introduce percpu xen_vcpu_id mapping. ARM and PV guests get direct mapping >> >> for now. Boot CPU for PVHVM guest gets its id from CPUID. With secondary >> >> CPUs it is a bit more trickier. Currently, we initialize IPI vectors >> >> before these CPUs boot so we can't use CPUID. Use ACPI ids from MADT >> >> instead. >> >> >> >> Signed-off-by: Vitaly Kuznetsov >> >> --- >> >> Changes since v2: >> >> - Use uint32_t for xen_vcpu_id mapping [Julien Grall] >> >> >> >> Changes since v1: >> >> - Introduce xen_vcpu_nr() helper [David Vrabel] >> >> - Use ACPI ids instead of vLAPIC ids /2 [Andrew Cooper, Jan Beulich] >> >> --- >> >> arch/arm/xen/enlighten.c | 10 ++ >> >> arch/x86/xen/enlighten.c | 23 ++- >> >> include/xen/xen-ops.h| 6 ++ >> >> 3 files changed, 38 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c >> >> index 75cd734..fe32267 100644 >> >> --- a/arch/arm/xen/enlighten.c >> >> +++ b/arch/arm/xen/enlighten.c >> >> @@ -46,6 +46,10 @@ struct shared_info *HYPERVISOR_shared_info = (void >> >> *)&xen_dummy_shared_info; >> >> DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu); >> >> static struct vcpu_info __percpu *xen_vcpu_info; >> >> >> >> +/* Linux <-> Xen vCPU id mapping */ >> >> +DEFINE_PER_CPU(uint32_t, xen_vcpu_id) = U32_MAX; >> >> +EXPORT_PER_CPU_SYMBOL(xen_vcpu_id); >> >> + >> >> /* These are unused until we support booting "pre-ballooned" */ >> >> unsigned long xen_released_pages; >> >> struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] >> >> __initdata; >> >> @@ -179,6 +183,9 @@ static void xen_percpu_init(void) >> >> pr_info("Xen: initializing cpu%d\n", cpu); >> >> vcpup = per_cpu_ptr(xen_vcpu_info, cpu); >> >> >> >> + /* Direct vCPU id mapping for ARM guests. */ >> >> + per_cpu(xen_vcpu_id, cpu) = cpu; >> >> + >> > >> > We did some internal testing on ARM64 with the latest Linux kernel >> > (4.8-rc4) and noticed that this patch is breaking SMP support. Sorry >> > for noticing the issue that late. >> >> Sorry for the breakage :-( >> >> > >> > This function is called on the running CPU whilst some code (e.g >> > init_control_block in drivers/xen/events/events_fifo.c) is executed >> > whilst preparing the CPU on the boot CPU. >> > >> > So xen_vcpu_nr(cpu) will always return 0 in this case and >> > init_control_block will fail to execute. >> > >> >> I see, >> >> CPU_UP_PREPARE event happens before xen_starting_cpu() is called. >> >> >> > I am not sure how to fix. I guess we could setup per_cpu(xen_vcpu_id, >> > *) in xen_guest_init. Any opinions? >> >> As we're not doing kexec on ARM we can fix the immediate issue. I don't >> know much about ARM and unfortunatelly I don't have a setup to test but >> it seems there is no early_per_cpu* infrastructure for ARM so we may fix >> it with the following: >> >> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c >> index 3d2cef6..f193414 100644 >> --- a/arch/arm/xen/enlighten.c >> +++ b/arch/arm/xen/enlighten.c >> @@ -170,9 +170,6 @@ static int xen_starting_cpu(unsigned int cpu) >> pr_info("Xen: initializing cpu%d\n", cpu); >> vcpup = per_cpu_ptr(xen_vcpu_info, cpu); >> >> - /* Direct vCPU id mapping for ARM guests. */ >> - per_cpu(xen_vcpu_id, cpu) = cpu; >> - >> info.mfn = virt_to_gfn(vcpup); >> info.offset = xen_offset_in_page(vcpup); >> >> @@ -330,6 +327,7 @@ static int __init xen_guest_init(void) >> { >> struct xen_add_to_physmap xatp; >> struct shared_info *shared_info_page = NULL; >> + int cpu; >> >> if (!xen_domain()) >> return 0; >> @@ -380,7 +378,8 @@ static int __init xen_guest_init(void) >> return -ENOMEM; >> >> /* Direct vCPU id mapping for ARM guests. */ >> - per_cpu(xen_vcpu_id, 0) = 0; >> + for_each_possible_cpu(cpu) >> + per_cpu(xen_vcpu_id, cpu) = cpu; >> >> xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames(); >> if (xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn, >> >> (not tested, if we can't use for_each_possible_cpu() that early we'll >> have to check against NR_CPUS instead). > > Kind of defeat the purpose of xen_vcpu_id, but I guess it should work. > >> But unfortunatelly we'll
Re: [Xen-devel] [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping
On Mon, 5 Sep 2016, Vitaly Kuznetsov wrote: > Julien Grall writes: > > > Hi Vitaly, > > > > On 26/07/16 13:30, Vitaly Kuznetsov wrote: > >> It may happen that Xen's and Linux's ideas of vCPU id diverge. In > >> particular, when we crash on a secondary vCPU we may want to do kdump > >> and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting > >> on the vCPU which crashed. This doesn't work very well for PVHVM guests as > >> we have a number of hypercalls where we pass vCPU id as a parameter. These > >> hypercalls either fail or do something unexpected. To solve the issue > >> introduce percpu xen_vcpu_id mapping. ARM and PV guests get direct mapping > >> for now. Boot CPU for PVHVM guest gets its id from CPUID. With secondary > >> CPUs it is a bit more trickier. Currently, we initialize IPI vectors > >> before these CPUs boot so we can't use CPUID. Use ACPI ids from MADT > >> instead. > >> > >> Signed-off-by: Vitaly Kuznetsov > >> --- > >> Changes since v2: > >> - Use uint32_t for xen_vcpu_id mapping [Julien Grall] > >> > >> Changes since v1: > >> - Introduce xen_vcpu_nr() helper [David Vrabel] > >> - Use ACPI ids instead of vLAPIC ids /2 [Andrew Cooper, Jan Beulich] > >> --- > >> arch/arm/xen/enlighten.c | 10 ++ > >> arch/x86/xen/enlighten.c | 23 ++- > >> include/xen/xen-ops.h| 6 ++ > >> 3 files changed, 38 insertions(+), 1 deletion(-) > >> > >> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > >> index 75cd734..fe32267 100644 > >> --- a/arch/arm/xen/enlighten.c > >> +++ b/arch/arm/xen/enlighten.c > >> @@ -46,6 +46,10 @@ struct shared_info *HYPERVISOR_shared_info = (void > >> *)&xen_dummy_shared_info; > >> DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu); > >> static struct vcpu_info __percpu *xen_vcpu_info; > >> > >> +/* Linux <-> Xen vCPU id mapping */ > >> +DEFINE_PER_CPU(uint32_t, xen_vcpu_id) = U32_MAX; > >> +EXPORT_PER_CPU_SYMBOL(xen_vcpu_id); > >> + > >> /* These are unused until we support booting "pre-ballooned" */ > >> unsigned long xen_released_pages; > >> struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] > >> __initdata; > >> @@ -179,6 +183,9 @@ static void xen_percpu_init(void) > >>pr_info("Xen: initializing cpu%d\n", cpu); > >>vcpup = per_cpu_ptr(xen_vcpu_info, cpu); > >> > >> + /* Direct vCPU id mapping for ARM guests. */ > >> + per_cpu(xen_vcpu_id, cpu) = cpu; > >> + > > > > We did some internal testing on ARM64 with the latest Linux kernel > > (4.8-rc4) and noticed that this patch is breaking SMP support. Sorry > > for noticing the issue that late. > > Sorry for the breakage :-( > > > > > This function is called on the running CPU whilst some code (e.g > > init_control_block in drivers/xen/events/events_fifo.c) is executed > > whilst preparing the CPU on the boot CPU. > > > > So xen_vcpu_nr(cpu) will always return 0 in this case and > > init_control_block will fail to execute. > > > > I see, > > CPU_UP_PREPARE event happens before xen_starting_cpu() is called. > > > > I am not sure how to fix. I guess we could setup per_cpu(xen_vcpu_id, > > *) in xen_guest_init. Any opinions? > > As we're not doing kexec on ARM we can fix the immediate issue. I don't > know much about ARM and unfortunatelly I don't have a setup to test but > it seems there is no early_per_cpu* infrastructure for ARM so we may fix > it with the following: > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > index 3d2cef6..f193414 100644 > --- a/arch/arm/xen/enlighten.c > +++ b/arch/arm/xen/enlighten.c > @@ -170,9 +170,6 @@ static int xen_starting_cpu(unsigned int cpu) > pr_info("Xen: initializing cpu%d\n", cpu); > vcpup = per_cpu_ptr(xen_vcpu_info, cpu); > > - /* Direct vCPU id mapping for ARM guests. */ > - per_cpu(xen_vcpu_id, cpu) = cpu; > - > info.mfn = virt_to_gfn(vcpup); > info.offset = xen_offset_in_page(vcpup); > > @@ -330,6 +327,7 @@ static int __init xen_guest_init(void) > { > struct xen_add_to_physmap xatp; > struct shared_info *shared_info_page = NULL; > + int cpu; > > if (!xen_domain()) > return 0; > @@ -380,7 +378,8 @@ static int __init xen_guest_init(void) > return -ENOMEM; > > /* Direct vCPU id mapping for ARM guests. */ > - per_cpu(xen_vcpu_id, 0) = 0; > + for_each_possible_cpu(cpu) > + per_cpu(xen_vcpu_id, cpu) = cpu; > > xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames(); > if (xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn, > > (not tested, if we can't use for_each_possible_cpu() that early we'll > have to check against NR_CPUS instead). Kind of defeat the purpose of xen_vcpu_id, but I guess it should work. > But unfortunatelly we'll have to get back to this in future. Turns out > we need to know Xen's idea of vCPU id _before_ this vCPU starts > executing code. Why? > On x86 we
Re: [Xen-devel] [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping
Julien Grall writes: > Hi Vitaly, > > On 26/07/16 13:30, Vitaly Kuznetsov wrote: >> It may happen that Xen's and Linux's ideas of vCPU id diverge. In >> particular, when we crash on a secondary vCPU we may want to do kdump >> and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting >> on the vCPU which crashed. This doesn't work very well for PVHVM guests as >> we have a number of hypercalls where we pass vCPU id as a parameter. These >> hypercalls either fail or do something unexpected. To solve the issue >> introduce percpu xen_vcpu_id mapping. ARM and PV guests get direct mapping >> for now. Boot CPU for PVHVM guest gets its id from CPUID. With secondary >> CPUs it is a bit more trickier. Currently, we initialize IPI vectors >> before these CPUs boot so we can't use CPUID. Use ACPI ids from MADT >> instead. >> >> Signed-off-by: Vitaly Kuznetsov >> --- >> Changes since v2: >> - Use uint32_t for xen_vcpu_id mapping [Julien Grall] >> >> Changes since v1: >> - Introduce xen_vcpu_nr() helper [David Vrabel] >> - Use ACPI ids instead of vLAPIC ids /2 [Andrew Cooper, Jan Beulich] >> --- >> arch/arm/xen/enlighten.c | 10 ++ >> arch/x86/xen/enlighten.c | 23 ++- >> include/xen/xen-ops.h| 6 ++ >> 3 files changed, 38 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c >> index 75cd734..fe32267 100644 >> --- a/arch/arm/xen/enlighten.c >> +++ b/arch/arm/xen/enlighten.c >> @@ -46,6 +46,10 @@ struct shared_info *HYPERVISOR_shared_info = (void >> *)&xen_dummy_shared_info; >> DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu); >> static struct vcpu_info __percpu *xen_vcpu_info; >> >> +/* Linux <-> Xen vCPU id mapping */ >> +DEFINE_PER_CPU(uint32_t, xen_vcpu_id) = U32_MAX; >> +EXPORT_PER_CPU_SYMBOL(xen_vcpu_id); >> + >> /* These are unused until we support booting "pre-ballooned" */ >> unsigned long xen_released_pages; >> struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] >> __initdata; >> @@ -179,6 +183,9 @@ static void xen_percpu_init(void) >> pr_info("Xen: initializing cpu%d\n", cpu); >> vcpup = per_cpu_ptr(xen_vcpu_info, cpu); >> >> +/* Direct vCPU id mapping for ARM guests. */ >> +per_cpu(xen_vcpu_id, cpu) = cpu; >> + > > We did some internal testing on ARM64 with the latest Linux kernel > (4.8-rc4) and noticed that this patch is breaking SMP support. Sorry > for noticing the issue that late. Sorry for the breakage :-( > > This function is called on the running CPU whilst some code (e.g > init_control_block in drivers/xen/events/events_fifo.c) is executed > whilst preparing the CPU on the boot CPU. > > So xen_vcpu_nr(cpu) will always return 0 in this case and > init_control_block will fail to execute. > I see, CPU_UP_PREPARE event happens before xen_starting_cpu() is called. > I am not sure how to fix. I guess we could setup per_cpu(xen_vcpu_id, > *) in xen_guest_init. Any opinions? As we're not doing kexec on ARM we can fix the immediate issue. I don't know much about ARM and unfortunatelly I don't have a setup to test but it seems there is no early_per_cpu* infrastructure for ARM so we may fix it with the following: diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c index 3d2cef6..f193414 100644 --- a/arch/arm/xen/enlighten.c +++ b/arch/arm/xen/enlighten.c @@ -170,9 +170,6 @@ static int xen_starting_cpu(unsigned int cpu) pr_info("Xen: initializing cpu%d\n", cpu); vcpup = per_cpu_ptr(xen_vcpu_info, cpu); - /* Direct vCPU id mapping for ARM guests. */ - per_cpu(xen_vcpu_id, cpu) = cpu; - info.mfn = virt_to_gfn(vcpup); info.offset = xen_offset_in_page(vcpup); @@ -330,6 +327,7 @@ static int __init xen_guest_init(void) { struct xen_add_to_physmap xatp; struct shared_info *shared_info_page = NULL; + int cpu; if (!xen_domain()) return 0; @@ -380,7 +378,8 @@ static int __init xen_guest_init(void) return -ENOMEM; /* Direct vCPU id mapping for ARM guests. */ - per_cpu(xen_vcpu_id, 0) = 0; + for_each_possible_cpu(cpu) + per_cpu(xen_vcpu_id, cpu) = cpu; xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames(); if (xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn, (not tested, if we can't use for_each_possible_cpu() that early we'll have to check against NR_CPUS instead). But unfortunatelly we'll have to get back to this in future. Turns out we need to know Xen's idea of vCPU id _before_ this vCPU starts executing code. On x86 we used ACPI_ID from MADT. Is there anything like it on ARM? > > [...] > >> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c >> index 0f87db2..c833912 100644 >> --- a/arch/x86/xen/enlighten.c >> +++ b/arch/x86/xen/enlighten.c >> @@ -1795,6 +1806,12 @@ static void __init init_hvm_pv_info(void) >> >> xen_setup_features(); >> >> +cpuid(base
Re: [Xen-devel] [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping
On 09/02/2016 11:29 AM, Julien Grall wrote: > >> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c >> index 0f87db2..c833912 100644 >> --- a/arch/x86/xen/enlighten.c >> +++ b/arch/x86/xen/enlighten.c >> @@ -1795,6 +1806,12 @@ static void __init init_hvm_pv_info(void) >> >> xen_setup_features(); >> >> +cpuid(base + 4, &eax, &ebx, &ecx, &edx); >> +if (eax & XEN_HVM_CPUID_VCPU_ID_PRESENT) >> +this_cpu_write(xen_vcpu_id, ebx); >> +else >> +this_cpu_write(xen_vcpu_id, smp_processor_id()); >> + >> pv_info.name = "Xen HVM"; >> >> xen_domain_type = XEN_HVM_DOMAIN; >> @@ -1806,6 +1823,10 @@ static int xen_hvm_cpu_notify(struct >> notifier_block *self, unsigned long action, >> int cpu = (long)hcpu; >> switch (action) { >> case CPU_UP_PREPARE: >> +if (cpu_acpi_id(cpu) != U32_MAX) >> +per_cpu(xen_vcpu_id, cpu) = cpu_acpi_id(cpu); >> +else >> +per_cpu(xen_vcpu_id, cpu) = cpu; > > I have not tried myself. But looking at the code, the notifiers > xen_hvm_cpu_notifier and evtchn_fifo_cpu_notifier have the same > priority. So what does prevent the code above to be executed after the > event channel callback? We will be converting to new hotplug state machine where this order will be guaranteed: https://lists.xenproject.org/archives/html/xen-devel/2016-08/msg01914.html -boris ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping
Hi Vitaly, On 26/07/16 13:30, Vitaly Kuznetsov wrote: It may happen that Xen's and Linux's ideas of vCPU id diverge. In particular, when we crash on a secondary vCPU we may want to do kdump and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting on the vCPU which crashed. This doesn't work very well for PVHVM guests as we have a number of hypercalls where we pass vCPU id as a parameter. These hypercalls either fail or do something unexpected. To solve the issue introduce percpu xen_vcpu_id mapping. ARM and PV guests get direct mapping for now. Boot CPU for PVHVM guest gets its id from CPUID. With secondary CPUs it is a bit more trickier. Currently, we initialize IPI vectors before these CPUs boot so we can't use CPUID. Use ACPI ids from MADT instead. Signed-off-by: Vitaly Kuznetsov --- Changes since v2: - Use uint32_t for xen_vcpu_id mapping [Julien Grall] Changes since v1: - Introduce xen_vcpu_nr() helper [David Vrabel] - Use ACPI ids instead of vLAPIC ids /2 [Andrew Cooper, Jan Beulich] --- arch/arm/xen/enlighten.c | 10 ++ arch/x86/xen/enlighten.c | 23 ++- include/xen/xen-ops.h| 6 ++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c index 75cd734..fe32267 100644 --- a/arch/arm/xen/enlighten.c +++ b/arch/arm/xen/enlighten.c @@ -46,6 +46,10 @@ struct shared_info *HYPERVISOR_shared_info = (void *)&xen_dummy_shared_info; DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu); static struct vcpu_info __percpu *xen_vcpu_info; +/* Linux <-> Xen vCPU id mapping */ +DEFINE_PER_CPU(uint32_t, xen_vcpu_id) = U32_MAX; +EXPORT_PER_CPU_SYMBOL(xen_vcpu_id); + /* These are unused until we support booting "pre-ballooned" */ unsigned long xen_released_pages; struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] __initdata; @@ -179,6 +183,9 @@ static void xen_percpu_init(void) pr_info("Xen: initializing cpu%d\n", cpu); vcpup = per_cpu_ptr(xen_vcpu_info, cpu); + /* Direct vCPU id mapping for ARM guests. */ + per_cpu(xen_vcpu_id, cpu) = cpu; + We did some internal testing on ARM64 with the latest Linux kernel (4.8-rc4) and noticed that this patch is breaking SMP support. Sorry for noticing the issue that late. This function is called on the running CPU whilst some code (e.g init_control_block in drivers/xen/events/events_fifo.c) is executed whilst preparing the CPU on the boot CPU. So xen_vcpu_nr(cpu) will always return 0 in this case and init_control_block will fail to execute. I am not sure how to fix. I guess we could setup per_cpu(xen_vcpu_id, *) in xen_guest_init. Any opinions? [...] diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c index 0f87db2..c833912 100644 --- a/arch/x86/xen/enlighten.c +++ b/arch/x86/xen/enlighten.c @@ -1795,6 +1806,12 @@ static void __init init_hvm_pv_info(void) xen_setup_features(); + cpuid(base + 4, &eax, &ebx, &ecx, &edx); + if (eax & XEN_HVM_CPUID_VCPU_ID_PRESENT) + this_cpu_write(xen_vcpu_id, ebx); + else + this_cpu_write(xen_vcpu_id, smp_processor_id()); + pv_info.name = "Xen HVM"; xen_domain_type = XEN_HVM_DOMAIN; @@ -1806,6 +1823,10 @@ static int xen_hvm_cpu_notify(struct notifier_block *self, unsigned long action, int cpu = (long)hcpu; switch (action) { case CPU_UP_PREPARE: + if (cpu_acpi_id(cpu) != U32_MAX) + per_cpu(xen_vcpu_id, cpu) = cpu_acpi_id(cpu); + else + per_cpu(xen_vcpu_id, cpu) = cpu; I have not tried myself. But looking at the code, the notifiers xen_hvm_cpu_notifier and evtchn_fifo_cpu_notifier have the same priority. So what does prevent the code above to be executed after the event channel callback? xen_vcpu_setup(cpu); if (xen_have_vector_callback) { if (xen_feature(XENFEAT_hvm_safe_pvclock)) diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h index 86abe07..648ce814 100644 --- a/include/xen/xen-ops.h +++ b/include/xen/xen-ops.h @@ -9,6 +9,12 @@ DECLARE_PER_CPU(struct vcpu_info *, xen_vcpu); +DECLARE_PER_CPU(uint32_t, xen_vcpu_id); +static inline int xen_vcpu_nr(int cpu) +{ + return per_cpu(xen_vcpu_id, cpu); +} + void xen_arch_pre_suspend(void); void xen_arch_post_suspend(int suspend_cancelled); Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel