From mboxrd@z Thu Jan  1 00:00:00 1970
From: Juergen Gross <jgross@suse.com>
Subject: Re: [PATCH 2/4] libxc: support of linear p2m list for
 migration of pv-domains
Date: Fri, 11 Dec 2015 17:00:31 +0100
Message-ID: <566AF31F.3000208@suse.com>
References: <1449833485-10163-1-git-send-email-jgross@suse.com>
	<1449833485-10163-3-git-send-email-jgross@suse.com>
	<566AE2D8.8050601@citrix.com> <566AE7E9.8010704@suse.com>
	<566AEACA.4070105@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <566AEACA.4070105@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Andrew Cooper <andrew.cooper3@citrix.com>, xen-devel@lists.xen.org, Ian.Campbell@citrix.com, ian.jackson@eu.citrix.com, stefano.stabellini@eu.citrix.com, wei.liu2@citrix.com
List-Id: xen-devel@lists.xenproject.org

On 11/12/15 16:24, Andrew Cooper wrote:
> 
>>>> + */
>>>> +static int map_p2m_list(struct xc_sr_context *ctx, uint64_t p2m_cr3)
>>>> +{
>>>> +    xc_interface *xch = ctx->xch;
>>>> +    xen_vaddr_t p2m_vaddr, p2m_end, mask, off;
>>>> +    xen_pfn_t p2m_mfn, mfn, saved_mfn, max_pfn;
>>>> +    uint64_t *ptes;
>>>> +    xen_pfn_t *mfns;
>>>> +    unsigned fpp, n_pages, level, shift, idx_start, idx_end, idx, saved_idx;
>>>> +    int rc = -1;
>>>> +
>>>> +    p2m_mfn = cr3_to_mfn(ctx, p2m_cr3);
>>>> +    if ( p2m_mfn == 0 || p2m_mfn > ctx->x86_pv.max_mfn )
>>> mfn 0 isn't invalid to use here.  It could, in principle, be available
>>> for PV guest use.
>> No, the value 0 indicates that the linear p2m info isn't valid. See
>> comments in xen/include/public/arch-x86/xen.h
> 
> Technically speaking, that is p2m_cr3, rather than p2m_mfn but I suppose
> there is a linear mapping between the two.
> 
> As this function only gets called with a non-zero p2m_cr3, an
> alternative would be assert(p2m_cr3 > 0).

Hmm, yes.

> The mfn == 0 comment also applies for reading the ptes in the loop below.

Sure? Is the hypervisor really giving mfn 0 to a guest? I don't mind
dropping the test, but I'd be surprised if mfn 0 would be valid.

> 
>>
>>> I believe this allows you drop 'mask' in its entirety.
>> Hmm, no. I'd still have to mask possible top 16 '1' bits away.
> 
> So you would.  My mistake.


Juergen