Re: [PATCH 06/14 v4] xen/arm: vpl011: Add a new domctl API to initialize vpl011

From: Bhupinder Thakur <bhupinder.thakur@linaro.org>
To: Julien Grall <julien.grall@arm.com>
Cc: xen-devel@lists.xenproject.org,
	Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>
Subject: Re: [PATCH 06/14 v4] xen/arm: vpl011: Add a new domctl API to initialize vpl011
Date: Wed, 21 Jun 2017 15:48:11 +0530	[thread overview]
Message-ID: <CACtJ1JS0UwrDn70iT0GkfGKviu4rtt=pWQ_bB-Pbr6fosD82WA@mail.gmail.com> (raw)
In-Reply-To: <0e446e52-84e8-f684-3e6e-8d7d55cb7cd8@arm.com>

Hi Julien,

On 20 June 2017 at 16:46, Julien Grall <julien.grall@arm.com> wrote:
> On 06/19/2017 02:11 PM, Bhupinder Thakur wrote:
>>
>> Hi Wei,
>
>
> Hi Bhupinder,
>
>
>> On 19 June 2017 at 17:17, Wei Liu <wei.liu2@citrix.com> wrote:
>>>
>>> On Mon, Jun 19, 2017 at 12:01:32PM +0100, Julien Grall wrote:
>>>>
>>>>
>>>>
>>>> On 19/06/17 11:59, Bhupinder Thakur wrote:
>>>>>
>>>>> Hi Julien,
>>>>>
>>>>> I was mistaken in my earlier mail about vpl011 init working if it is
>>>>> moved to libxl__arch_domain_create(). It is failing because as you
>>>>> have mentioned vuart_pfn is allocated later in xc_dom_build_image().
>>>>>
>>>>> Can we delay mapping of this page in Xen until the ring buffer is
>>>>> actually required by the emulation code for reading/writing data. By
>>>>> that time, the page would have been physically mapped.
>>>>
>>>>
>>>> You would not be able to report an error if you fail to map it. But this
>>>> looks like to me a workaround for a tool problem.
>>>>
>>>> Anyway, as I said, I'd like feedback from the tools maintainers to see
>>>> how
>>>> we can proceed.
>>>>
>>>
>>> Is there a summary of the problem, is there a particular email in this
>>> thread I should look at? Sorry I'm swamped by emails and patches at the
>>> moment.
>>
>>
>> I will summarize the problem.
>>
>> It was decided to call domain_vpl011_init() from inside
>> libxl__arch_domain_create() to initialize vpl011. However,
>> domain_vpl011_init() fails to map the the vuart GFN because it has not
>> been physically mapped yet by the toolstack.
>>
>> The following call flows highlights the issue.
>>
>> libxl__domain_build() ---> libxl__build_pv ---> libxl__build_dom()
>> ----> xc_dom_build_image() ---> alloc_magic_pages() ----> vuart GFN
>> allocated/mapped here
>>
>> libxl__domain_build() ----> libxl__build_pre()  ---->
>> libxl__arch_domain_create() ----> domain_vpl011_init() ---> this call
>> fails as the vuart GFN has not been physically mapped yet as shown in
>> the first call flow.
>>
>> However, libxl__build_pv() is called after libxl__build_pre(). It
>> means that the domain_vpl011_init() is called before
>> alloc_magic_pages() is called and hence the initialization fails.
>>
>> For that reason, I had introduced a new function
>> libxl__arch_domain_create_finish() which will be called from
>> libxl__build_post(). I moved the domain_vpl011_init() there. However,
>> Julien pointed out that vuart should be initialized early in
>> libxl__arch_domain_create() function.
>
>
> libxl__arch_domain_create could be a place or even
> libxl__arch_domain_finalise_hw_descriptions.
>
> My point is it looks a bit odd to create the vpl011 UART very late in the
> process as from the code you would expect all the hardware to be setup after
> libxl__arch_domain_finialise_hw_descriptions is called.
>

libxl__arch_domain_finalise_hw_descriptions() is called just before
xc_dom_build_image() and therefore the vuart gfn is still not
allocated. Maybe I can introduce a new arch specific
libxl__arch_domain_init_vpl011() function and call it from inside
libxl__build_dom() after call to xc_dom_build_image() so that the
vuart gfn is allocated.

Regards,
Bhupinder

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