From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH RFC 0/1] Introduce VCPUOP_reset_vcpu_info Date: Wed, 6 Aug 2014 10:49:12 -0400 Message-ID: <20140806144912.GE13079@laptop.dumpdata.com> References: <1407330502-18768-1-git-send-email-vkuznets@redhat.com> <53E253470200007800029C1D@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XF2Wq-0003e8-NS for xen-devel@lists.xenproject.org; Wed, 06 Aug 2014 14:49:25 +0000 Content-Disposition: inline In-Reply-To: <53E253470200007800029C1D@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel@lists.xenproject.org, Vitaly Kuznetsov , Stefano Stabellini , Andrew Jones , David Vrabel List-Id: xen-devel@lists.xenproject.org On Wed, Aug 06, 2014 at 03:09:43PM +0100, Jan Beulich wrote: > >>> On 06.08.14 at 15:08, wrote: > > --- a/arch/x86/xen/enlighten.c > > +++ b/arch/x86/xen/enlighten.c > > @@ -183,8 +183,6 @@ static void xen_vcpu_setup(int cpu) > > * This path is called twice on PVHVM - first during bootup via > > * smp_init -> xen_hvm_cpu_notify, and then if the VCPU is being > > * hotplugged: cpu_up -> xen_hvm_cpu_notify. > > - * As we can only do the VCPUOP_register_vcpu_info once lets > > - * not over-write its result. > > * > > * For PV it is called during restore (xen_vcpu_restore) and bootup > > * (xen_setup_vcpu_info_placement). The hotplug mechanism does not > > @@ -207,14 +205,23 @@ static void xen_vcpu_setup(int cpu) > > info.mfn = arbitrary_virt_to_mfn(vcpup); > > info.offset = offset_in_page(vcpup); > > > > + /* > > + * Call VCPUOP_reset_vcpu_info before VCPUOP_register_vcpu_info, this > > + * is required if we boot after kexec. > > + */ > > + > > + if (cpu != 0) { > > + err = HYPERVISOR_vcpu_op(VCPUOP_reset_vcpu_info, cpu, NULL); > > + if (err) > > + pr_warn("VCPUOP_reset_vcpu_info for CPU%d failed: %d\n", > > + cpu, err); > > + } > > Just for my understanding of why exactly you need the new operation: > Why is this being done here, when you already do the reset in the > cpu-die/shutdown paths? And why not for CPU 0? > > Furthermore, what is the state of vCPU-s beyond 31 going to be after > they got their vCPU info reset? They won't have any other area as > fallback. Yet I don't think you can now and forever guarantee that > native_cpu_die() won't do anything requiring that structure. native_cpu_die just spins around in an infinite halt sequence. The call could be replaced with the VCPU_down call instead, something like: if (xen_teardown_cpu(cpu) == -ENOSYS) native_cpu_die(); (void)HYPERVISOR_vcpu_op(VCPUP_down, cpu, NULL);