Re: [PATCH 03/10 v2] xen/arm: vpl011: Enable pl011 emulation for a guest domain in Xen

[Wei Liu <wei.liu2@citrix.com>]

On Fri, May 05, 2017 at 02:43:40PM +0100, Julien Grall wrote:
> On 05/05/17 08:10, Bhupinder Thakur wrote:
> > Hi Julien,
> 
> Hi Bhupinder,
> 
> > > > > Hi Jan,
> > > > > 
> > > > > > > @@ -631,6 +632,9 @@ int arch_domain_create(struct domain *d,
> > > > > > > unsigned int domcr_flags,
> > > > > > >      if ( (rc = domain_vtimer_init(d, config)) != 0 )
> > > > > > >          goto fail;
> > > > > > > 
> > > > > > > +    if ( domcr_flags & DOMCRF_vuart )
> > > > > > > +        if ( (rc = domain_vpl011_init(d, config)) != 0 )
> > > > > > > +            goto fail;
> > > > > > >      update_domain_wallclock_time(d);
> > > > > > 
> > > > > > 
> > > > > I am planning to remove the usage of domain creation flag to check
> > > > > whether vuart is enabled/disabled. Please see my next comment. With
> > > > > that change, domain_vpl011_init() will be called always. The
> > > > > domain_vpl011_init() will check whether vuart is enabled or disabled
> > > > > in the config structure passed. If vuart is enabled then it will go
> > > > > ahead with vpl011 initialization else it will return without
> > > > > initializing vpl011.
> > > > 
> > > > 
> > > > Please don't do that. The arch code decides whether domain_vpl011_init
> > > > not the invert.
> > > 
> > > 
> > > I was wondering whether it would be better to defer the PL011 creation to a
> > > domctl. This could be called after the domain is created with all the
> > > information required (MMIO region, Console PFN...).
> > > 
> > > This would also make the migration support more trivial as the we will not
> > > need to know in advance whether a UART is been used.
> > > 
> > > Any opinions?
> > 
> > Would there be race condition where the guest tries to access the
> > pl011 mmio region (as the domain has been created) but pl011 is not
> > initialized yet as domctl is not called? What could be an appropriate
> > place to call this domctl? It should be before xenstore is populated
> > with vuart ring-ref/port information.
> 
> There are no race condition. The domain will only be started when everything
> has been created by calling XEN_DOMCTL_unpausedomain.
> 
> The DOMCTL createdomain only initialize the basic structure for the domain,
> after the hypercall the domain is not in state to be run because, for
> instance, the vCPUs were not allocated (see XEN_DOMCTL_max_vcpus) and the
> guest RAM were not populated.
> 
> I am not very familiar with the libxl code, but I think
> libxl__arch_domain_create should be a good candidate. I will let Ian and Wei
> confirm that.
> 

That sounds reasonable.

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