Re: [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Paul Durrant <Paul.Durrant@citrix.com>,
	Julien Grall <julien.grall@arm.com>,
	"sstabellini@kernel.org" <sstabellini@kernel.org>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: Kevin Tian <kevin.tian@intel.com>, Wei Liu <wei.liu2@citrix.com>,
	Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Jun Nakajima <jun.nakajima@intel.com>,
	"Tim (Xen.org)" <tim@xen.org>,
	George Dunlap <George.Dunlap@citrix.com>,
	Julien Grall <julie.grall@arm.com>,
	Jan Beulich <jbeulich@suse.com>,
	Ian Jackson <Ian.Jackson@citrix.com>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Brian Woods <brian.woods@amd.com>
Subject: Re: [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn
Date: Wed, 7 Nov 2018 13:08:43 +0000	[thread overview]
Message-ID: <a6103e7f-218b-f30a-fe05-57348eb35447@citrix.com> (raw)
In-Reply-To: <edf7fafc10944dbab6379200ee952104@AMSPEX02CL03.citrite.net>

On 07/11/18 08:57, Paul Durrant wrote:
>>
>>>  static inline struct page_info *get_page_from_gfn(
>>> -    struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q)
>>> +    struct domain *d, gfn_t gfn, p2m_type_t *t, p2m_query_t q)
>>>  {
>>>      struct page_info *page;
>>> +    mfn_t mfn;
>>>
>>>      if ( paging_mode_translate(d) )
>>> -        return p2m_get_page_from_gfn(p2m_get_hostp2m(d), _gfn(gfn), t,
>> NULL, q);
>>> +        return p2m_get_page_from_gfn(p2m_get_hostp2m(d), gfn, t, NULL,
>> q);
>>>      /* Non-translated guests see 1-1 RAM / MMIO mappings everywhere */
>>>      if ( t )
>>>          *t = likely(d != dom_io) ? p2m_ram_rw : p2m_mmio_direct;
>>> -    page = mfn_to_page(_mfn(gfn));
>>> -    return mfn_valid(_mfn(gfn)) && get_page(page, d) ? page : NULL;
>>> +
>>> +    mfn = _mfn(gfn_x(gfn));
>>> +    page = mfn_to_page(mfn);
>>> +    return mfn_valid(mfn) && get_page(page, d) ? page : NULL;
>> This looks like it would be cleaner by not splitting mfn out into a
>> separate variable.
>>
>> page = mfn_to_page(_mfn(gfn_x(gfn)));
>>
>> return mfn_valid(mfn) && get_page(page, d) ? page : NULL;
> ^^ er... how's that mfn_valid() going to work? You'd need mfn_valid(page_to_mfn(pahge)), or somesuch.

Oops - I'm blind.  Sorry for the noise.

~Andrew

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