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

[Julien Grall <julien.grall@xxxxxxx>]

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@xxxxxxxxxx> 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.
> So the issue is what is the right place to call domain_vpl011_init()?
>
> I hope it clarifies the issue.
Cheers,

--
Julien Grall

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