From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] tools/mfn-dump: Fixes to 'dump-p2m' Date: Tue, 29 Apr 2014 17:39:17 +0100 Message-ID: <535FD5B5.30208@citrix.com> References: <1398373587-26948-1-git-send-email-andrew.cooper3@citrix.com> <1398785123.16933.16.camel@Solace> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8050686957765628698==" Return-path: In-Reply-To: <1398785123.16933.16.camel@Solace> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Dario Faggioli Cc: Ian Jackson , Ian Campbell , Xen-devel List-Id: xen-devel@lists.xenproject.org --===============8050686957765628698== Content-Type: multipart/alternative; boundary="------------070902050303000901030505" --------------070902050303000901030505 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit On 29/04/14 16:25, Dario Faggioli wrote: > On gio, 2014-04-24 at 22:06 +0100, Andrew Cooper wrote: >> * Don't walk off the end of p2m_table under the mistaken impression that it >> contains toolstack unsigned longs. Despite its array type it contains guest >> unsigned longs so unconditionally needs casting to the guest width to use >> correctly. Furthermore, a 64bit toolstack must be extra careful when it >> finds a 32bit guest's INVALID_MFN. >> >> * Drop 'mapped' and 'pinned' descriptions. This are both bogus, including all >> uses of the is_mapped() macro. >> > Just a question, what do you mean by 'bogus' here? About pinned, I think > I see it, and I like the way you put it in the patch. About 'mapped' and > is_mapped()? Do you mean to say it's not useful information here? > > Again, just curious. The top bit of the type has nothing to do with mappings, or certainly nothing that I am aware of, having successfully rewritten PV migration from scratch. The guest can still have mappings to its pinned pagetables, which would have the top bit of the type set. In xc_domain_save.c, this macro is used 4 times. 3 are used on mfns and 1 is used on a type (for a bit of debugging code which appears dead anyway). I expect noone has every tried migrating a PV domain whose pages are located in mfns with the 44th bit set. So by bogus, I mean the macro itself, and all uses of it. My best guess is that is some vestigial code left over from a previous way of doing things, although I didn't encounter anything related to this in code archaeology I performed when trying to work out why the legacy migration did certain things the way they did. ~Andrew --------------070902050303000901030505 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: 8bit On 29/04/14 16:25, Dario Faggioli wrote:
> On gio, 2014-04-24 at 22:06 +0100, Andrew Cooper wrote:
>> * Don't walk off the end of p2m_table under the mistaken impression that it
>>   contains toolstack unsigned longs.  Despite its array type it contains guest
>>   unsigned longs so unconditionally needs casting to the guest width to use
>>   correctly.  Furthermore, a 64bit toolstack must be extra careful when it
>>   finds a 32bit guest's INVALID_MFN.
>>
>> * Drop 'mapped' and 'pinned' descriptions.  This are both bogus, including all
>>   uses of the is_mapped() macro.
>>
> Just a question, what do you mean by 'bogus' here? About pinned, I think
> I see it, and I like the way you put it in the patch. About 'mapped' and
> is_mapped()? Do you mean to say it's not useful information here?
>
> Again, just curious.

The top bit of the type has nothing to do with mappings, or certainly nothing that I am aware of, having successfully rewritten PV migration from scratch.

The guest can still have mappings to its pinned pagetables, which would have the top bit of the type set.

In xc_domain_save.c, this macro is used 4 times.  3 are used on mfns and 1 is used on a type (for a bit of debugging code which appears dead anyway).  I expect noone has every tried migrating a PV domain whose pages are located in mfns with the 44th bit set.

So by bogus, I mean the macro itself, and all uses of it.

My best guess is that is some vestigial code left over from a previous way of doing things, although I didn't encounter anything related to this in code archaeology I performed when trying to work out why the legacy migration did certain things the way they did.

~Andrew

--------------070902050303000901030505-- --===============8050686957765628698== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============8050686957765628698==--