Re: [PATCH 03/12 v3] xen/arm: vpl011: Add pl011 uart emulation in Xen

From: Julien Grall <julien.grall@arm.com>
To: Bhupinder Thakur <bhupinder.thakur@linaro.org>
Cc: xen-devel@lists.xenproject.org,
	Stefano Stabellini <sstabellini@kernel.org>
Subject: Re: [PATCH 03/12 v3] xen/arm: vpl011: Add pl011 uart emulation in Xen
Date: Thu, 1 Jun 2017 13:42:45 +0100	[thread overview]
Message-ID: <11dd1ae4-400a-499b-208f-38efcc2834f1@arm.com> (raw)
In-Reply-To: <CACtJ1JRazKdv=jf4+HPG1DVC1dQ68zdLbyrbXNr_BMSJFNJu2Q@mail.gmail.com>

On 01/06/17 11:33, Bhupinder Thakur wrote:
> Hi Julien,

Hi Bhupinder,

> On 22 May 2017 at 19:54, Julien Grall <julien.grall@arm.com> wrote:
>>> +static const struct mmio_handler_ops vpl011_mmio_handler = {
>>> +    .read = vpl011_mmio_read,
>>> +    .write = vpl011_mmio_write,
>>> +};
>>> +
>>> +int vpl011_map_guest_page(struct domain *d, xen_pfn_t gfn)
>>
>>
>> This function should either have the prototype defined in an header if used
>> outside or static.
>>
>> Also, I have asked to use gfn_t and not xen_pfn_t. The former is a typesafe
>> avoiding mix between MFN and GFN.
>
> The gfn is passed on from the domctl API to this function. Is the
> gfn_t definition exposed to domctl interface? Currently, gfn_t is
> defined in xen/include/xen/mm.h. So I would have to include this
> header file in xen/include/public/domctl.h where I have defined the
> vuart_ops structure containing gfn. Is it ok to include a xen header
> file in the public header file?

No. The only thing I asked is to replace xen_pfn_t by gfn_t in the 
parameter and the caller should use _gfn(...) to do the conversion.

Have a look on how we do in populate_physmap (xen/common/memory.c).

Cheers,

-- 
Julien Grall

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