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

Re: [Xen-devel] [PATCH v2 3/3] Convert map_domain_page() to use the new mfn_t type



>>> On 09.07.15 at 12:29, <Ben.Catterall@xxxxxxxxxx> wrote:

> 
> On 07/07/15 11:10, Jan Beulich wrote:
>>>>> On 02.07.15 at 14:04, <Ben.Catterall@xxxxxxxxxx> wrote:
>>> Reworked the internals and declaration, applying (un)boxing
>>> where needed. Converted calls to map_domain_page() to
>>> provide mfn_t types, boxing where needed.
>>>
>>> Signed-off-by: Ben Catterall <Ben.Catterall@xxxxxxxxxx>
>>> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>>
>>> ---
>>> Changed since v1:
>>>     * Created paddr_to_mfn() and mfn_to_paddr() for both x86 and ARM
>>>     * Converted code to use the new paddr_to_mfn() rather than e.g.
>>>       paddr>>PAGE_SHIFT
>> This was a bogus change - why can't you use paddr_to_pfn() and
>> pfn_to_paddr()? And if you needed new macros, they should be
>> named consistently, i.e. maddr_to_mfn() and mfn_to_maddr().
>> And perhaps they should then produce/take mfn_t?
>>
> In [PATCH 3/3] Andrew said that I should use _mfn(paddr_to_mfn(ma)) rather 
> than ma >> PAGE_SIZE
> so I made the change based on that. Can you clarify if I should proceed and 
> use paddr_to_pfn() instead.

Yes, I'd prefer not to add new macros when we have existing ones
that fit our needs. And in no case would I accept ones with
inconsistent names.

> Re. the rename: can you clarify what the difference between maddr (machine 
> addr?) and
> paddr (physical addr?) are please?

From only the hypervisor perspective there's none. When dealing with
PV guests, physical and machine addresses are different. For the
frame number <-> address conversion, though, the precise kind of
address doesn't matter at all, since the page size universal.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

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