From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.1 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2CAFEC43461 for ; Mon, 7 Sep 2020 07:01:49 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D111C206D4 for ; Mon, 7 Sep 2020 07:01:48 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D111C206D4 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1kFB9d-0007cG-7P; Mon, 07 Sep 2020 07:01:29 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1kFB9c-0007cB-8p for xen-devel@lists.xenproject.org; Mon, 07 Sep 2020 07:01:28 +0000 X-Inumbo-ID: 54d63d87-7c63-4293-af02-a8d580fa75cd Received: from mx2.suse.de (unknown [195.135.220.15]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 54d63d87-7c63-4293-af02-a8d580fa75cd; Mon, 07 Sep 2020 07:01:26 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id C01BBADD5; Mon, 7 Sep 2020 07:01:25 +0000 (UTC) Subject: Re: [PATCH v7 7/9] common/domain: add a domain context record for shared_info... To: paul@xen.org Cc: xen-devel@lists.xenproject.org, 'Paul Durrant' , 'Ian Jackson' , 'Wei Liu' , 'Andrew Cooper' , 'George Dunlap' , 'Julien Grall' , 'Stefano Stabellini' References: <20200818103032.3050-1-paul@xen.org> <20200818103032.3050-8-paul@xen.org> <003501d682e0$f4e8a1e0$deb9e5a0$@xen.org> From: Jan Beulich Message-ID: <174bb33f-9acb-dc52-76d1-e1ad7a350dc1@suse.com> Date: Mon, 7 Sep 2020 09:01:15 +0200 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: <003501d682e0$f4e8a1e0$deb9e5a0$@xen.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" On 04.09.2020 19:29, Paul Durrant wrote: >> From: Jan Beulich >> Sent: 26 August 2020 14:58 >> >> On 18.08.2020 12:30, Paul Durrant wrote: >>> v7: >>> - Only restore vcpu_info and arch sub-structures for PV domains, to match >>> processing of SHARED_INFO in xc_sr_restore_x86_pv.c >> >> Since you point out this original logic, ... >> >>> +static int load_shared_info(struct domain *d, struct domain_context *c) >>> +{ >>> + struct domain_shared_info_context ctxt; >>> + size_t hdr_size = offsetof(typeof(ctxt), buffer); >>> + unsigned int i; >>> + int rc; >>> + >>> + rc = DOMAIN_LOAD_BEGIN(SHARED_INFO, c, &i); >>> + if ( rc ) >>> + return rc; >>> + >>> + if ( i ) /* expect only a single instance */ >>> + return -ENXIO; >>> + >>> + rc = domain_load_data(c, &ctxt, hdr_size); >>> + if ( rc ) >>> + return rc; >>> + >>> + if ( ctxt.buffer_size > sizeof(shared_info_t) || >>> + (ctxt.flags & ~DOMAIN_SAVE_32BIT_SHINFO) ) >>> + return -EINVAL; >>> + >>> + if ( ctxt.flags & DOMAIN_SAVE_32BIT_SHINFO ) >>> + { >>> +#ifdef CONFIG_COMPAT >>> + has_32bit_shinfo(d) = true; >>> +#else >>> + return -EINVAL; >>> +#endif >>> + } >>> + >>> + if ( is_pv_domain(d) ) >>> + { >>> + shared_info_t *shinfo = xmalloc(shared_info_t); >>> + >>> + rc = domain_load_data(c, shinfo, sizeof(*shinfo)); >>> + if ( rc ) >>> + { >>> + xfree(shinfo); >>> + return rc; >>> + } >>> + >>> +#ifdef CONFIG_COMPAT >>> + if ( has_32bit_shinfo(d) ) >>> + { >>> + memcpy(&d->shared_info->compat.vcpu_info, >>> + &shinfo->compat.vcpu_info, >>> + sizeof(d->shared_info->compat.vcpu_info)); >>> + memcpy(&d->shared_info->compat.arch, >>> + &shinfo->compat.arch, >>> + sizeof(d->shared_info->compat.vcpu_info)); >>> + } >>> + else >>> + { >>> + memcpy(&d->shared_info->native.vcpu_info, >>> + &shinfo->native.vcpu_info, >>> + sizeof(d->shared_info->native.vcpu_info)); >>> + memcpy(&d->shared_info->native.arch, >>> + &shinfo->native.arch, >>> + sizeof(d->shared_info->native.arch)); >>> + } >>> +#else >>> + memcpy(&d->shared_info->vcpu_info, >>> + &shinfo->vcpu_info, >>> + sizeof(d->shared_info->vcpu_info)); >>> + memcpy(&d->shared_info->arch, >>> + &shinfo->arch, >>> + sizeof(d->shared_info->shared)); >>> +#endif >> >> ... where does the rest of that logic (resetting of >> arch.pfn_to_mfn_frame_list_list, evtchn_pending, evtchn_mask, and >> evtchn_pending_sel) get done? Or why is it not needed anymore? > > The resetting logic is still in xc_sr_restore_x86_pv.c (see patch #6). > It's going to need to stay there anyway to deal with older streams so > I made it common to both cases; it seems slightly separate from > restoring the shared info. I guess I at least don't fully agree: The resetting is part of restoring, as it effectively determines which parts are restored and which parts are simply set (not truly reset, but I agree the perception may change depending on whose position you take). Hence at the very least this aspect wants clearly spelling out in the description, I think. But really I'd prefer if for old streams libxc took care of (all of) it, and if for new streams all logic lived in the hypervisor. Jan