[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v4 08/16] xen/mm: Drop the parameter mfn from populate_pt_range



Hi Wei,

On 22/02/18 16:51, Wei Liu wrote:
On Thu, Feb 22, 2018 at 04:40:04PM +0000, Julien Grall wrote:
Hi Wei,

On 22/02/18 16:35, Wei Liu wrote:
On Wed, Feb 21, 2018 at 02:02:51PM +0000, Julien Grall wrote:
The function populate_pt_range is used to populate in advance the
page-table but it will not do the actual mapping. So passing the MFN in
parameter is pointless. Note that the only caller pass 0...

At the same time replace 0 by INVALID_MFN to make clear the MFN is
invalid.


The mfn parameter is the first mfn of a consecutive nr MFNs passed to
map_pages_to_xen. Putting INVALID_MFN isn't helping -- the value written
to page table(s) will wrap around to 0.

And I think starting from 0 to avoid overflow is probably a better
behaviour. If you really want to make sure all entries are filled with
INVALID_MFN you should call map_pages_to_xen for nr times with each
page.

I am not sure to understand this. From its name, populate_pt_range should
only create the intermediate tables. The leaf entry will stay invalid. So
how the value of mfn matters? Is it because the code is written in a such
way that passing INVALID_MFN will result to undefined behavior?

Right, that's what I meant. It doesn't matter whether you use 0 or
INVALID_MFN.

Unsigned integer overflow is not UB in C, so passing INVALID_MFN is
safe.

But your intention seemed to be filling all entries with INVALID_MFN to
aid debugging, so the function doesn't do what I think you wanted it to
do. It could be I misunderstood your intention.

That was not my intention. I replaced 0 by INVALID_MFN because from the name you know the MFN is invalid. 0 could potentially be valid (at least on Arm) and make the code confusing to understand.

I can make it clearer in the commit message.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.