From patchwork Mon Sep 5 09:42:22 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vitaly Kuznetsov X-Patchwork-Id: 9313303 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 528E9607D3 for ; Mon, 5 Sep 2016 09:44:42 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3B7AA28A27 for ; Mon, 5 Sep 2016 09:44:42 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3007A28A29; Mon, 5 Sep 2016 09:44:42 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 81C5128A27 for ; Mon, 5 Sep 2016 09:44:41 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bgqQF-0007Qp-EN; Mon, 05 Sep 2016 09:42:35 +0000 Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bgqQD-0007Qj-VP for xen-devel@lists.xenproject.org; Mon, 05 Sep 2016 09:42:34 +0000 Received: from [85.158.143.35] by server-10.bemta-6.messagelabs.com id 4E/F1-27438-90E3DC75; Mon, 05 Sep 2016 09:42:33 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrAIsWRWlGSWpSXmKPExsVysWW7jC6H3dl wgwdX9S2+b5nM5MDocfjDFZYAxijWzLyk/IoE1oylVzazFzw3rjjUuZ25gfGhVhcjF4eQwG4m ie8r/7JCODOZJDo3z2brYuTkYBPQkfj+9BQziC0ioC7x58IERpAiZoHzLBL72/YygiSEBVwkm qevYIHoXsMosXXJYrBuFgFVieutm4FsDg5OgUqJd+djQMK8AoYS51ceACsRFbCReL5uFStEXF Di5MwnLCA2s4CExMEXL8AWSwhoSzTMmswCYfcxSlx+mTaBkX8WkpZZSFoWMDKtYtQoTi0qSy3 SNTTXSyrKTM8oyU3MzNE1NDDTy00tLk5MT81JTCrWS87P3cQIDDkGINjBeHtjwCFGSQ4mJVHe AtWz4UJ8SfkplRmJxRnxRaU5qcWHGGU4OJQkeOfaAOUEi1LTUyvSMnOAwQ+TluDgURLhdQZJ8 xYXJOYWZ6ZDpE4xKkqJ86aDJARAEhmleXBtsIi7xCgrJczLCHSIEE9BalFuZgmq/CtGcQ5GJW FeGVugKTyZeSVw018BLWYCWrxu92mQxSWJCCmpBkb/+ypebXN3+Nhne2hxNS57ska7jU/SO/V /+TcfBWmOQJ2SKXlXdcJjS/ulGtZlez1cPjH0h8wpHUX7720GkeVf3ukLmLxUvJR9Y8bHQ8/b 9Tx18n9q3nxivI8jeMoegRP6CW2Lf8ydkx0caXz93vb6sMDMHtnbiTNLTM9c2/gw3HUuh+ZzG yWW4oxEQy3mouJEAEcYdT2zAgAA X-Env-Sender: vkuznets@redhat.com X-Msg-Ref: server-8.tower-21.messagelabs.com!1473068551!31548454!1 X-Originating-IP: [209.132.183.28] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogMjA5LjEzMi4xODMuMjggPT4gNTQwNjQ=\n X-StarScan-Received: X-StarScan-Version: 8.84; banners=-,-,- X-VirusChecked: Checked Received: (qmail 875 invoked from network); 5 Sep 2016 09:42:32 -0000 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by server-8.tower-21.messagelabs.com with DHE-RSA-AES256-GCM-SHA384 encrypted SMTP; 5 Sep 2016 09:42:32 -0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9C022821C3; Mon, 5 Sep 2016 09:42:27 +0000 (UTC) Received: from vitty.brq.redhat.com.redhat.com (vitty.brq.redhat.com [10.34.26.3]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u859gNvk030972 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Mon, 5 Sep 2016 05:42:24 -0400 From: Vitaly Kuznetsov To: Julien Grall References: <1469536228-29932-1-git-send-email-vkuznets@redhat.com> <1469536228-29932-4-git-send-email-vkuznets@redhat.com> <258e8e2e-29c4-1a88-2271-581f2ea59ec2@arm.com> Date: Mon, 05 Sep 2016 11:42:22 +0200 In-Reply-To: <258e8e2e-29c4-1a88-2271-581f2ea59ec2@arm.com> (Julien Grall's message of "Fri, 2 Sep 2016 16:29:58 +0100") Message-ID: <87d1kiwum9.fsf@vitty.brq.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.0.95 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Mon, 05 Sep 2016 09:42:27 +0000 (UTC) Cc: Juergen Gross , Stefano Stabellini , Wei Chen , Steve Capper , Andrew Cooper , x86@kernel.org, linux-kernel@vger.kernel.org, Kaly Xin , Ingo Molnar , David Vrabel , Jan Beulich , "H. Peter Anvin" , xen-devel@lists.xenproject.org, Boris Ostrovsky , Thomas Gleixner Subject: Re: [Xen-devel] [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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: (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 + 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? Nothing, actually, but xen_hvm_guest_init() happens early and we always get these notifiers in the right order (and, reading Boris' reply, we're gonna make it explicit in future). > >> 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, 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,