From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vitaly Kuznetsov Subject: Re: [PATCH RFC 0/1] Introduce VCPUOP_reset_vcpu_info Date: Wed, 06 Aug 2014 17:03:25 +0200 Message-ID: <87a97h64oy.fsf@vitty.brq.redhat.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.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XF2kc-0005d8-Ky for xen-devel@lists.xenproject.org; Wed, 06 Aug 2014 15:03:38 +0000 In-Reply-To: <53E253470200007800029C1D@mail.emea.novell.com> (Jan Beulich's message of "Wed, 06 Aug 2014 15:09:43 +0100") 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, Andrew Jones , David Vrabel , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org "Jan Beulich" writes: >>>> 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? We can avoid doing it here if we put VCPUOP_reset_vcpu_info to kdump handler in addition to kexec path. > And why not for CPU 0? > Because the suggested op will fail for already running CPU0.. > 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. In xen their vcpu_info will point to dummy_vcpu_info (that's what we have there before VCPUOP_register_vcpu_info I guess). I tested kexec with 64 vcpus case, no issues noticed. > Yet I don't think you can now and forever guarantee that > native_cpu_die() won't do anything requiring that structure. I agree with Konrad, we can exclude native_cpu_die() as it does almost nothing (or call VCPUOP_reset_vcpu_info after it). Thank you for your comments (especially for your other email), I'll try addressing them. -- Vitaly