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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 9 Jun 2017, Julien Grall wrote:
> Hi Stefano,
> 
> On 07/06/17 00:26, Stefano Stabellini wrote:
> > > diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
> > > index 00909ad4..a8efd5e 100644
> > > --- a/tools/libxc/xc_domain.c
> > > +++ b/tools/libxc/xc_domain.c
> > > @@ -343,6 +343,29 @@ int xc_domain_get_guest_width(xc_interface *xch,
> > > uint32_t domid,
> > >      return 0;
> > >  }
> > > 
> > > +int xc_dom_vpl011_init(xc_interface *xch,
> > > +                       uint32_t domid,
> > > +                       uint32_t console_domid,
> > > +                       xen_pfn_t gfn,
> > > +                       evtchn_port_t *evtchn)
> > > +{
> > > +    DECLARE_DOMCTL;
> > > +    int rc = 0;
> > > +
> > > +    domctl.cmd = XEN_DOMCTL_vuart_op;
> > > +    domctl.domain = (domid_t)domid;
> > > +    domctl.u.vuart_op.cmd = XEN_DOMCTL_VUART_OP_INIT_VPL011;
> > > +    domctl.u.vuart_op.console_domid = console_domid;
> > > +    domctl.u.vuart_op.gfn = gfn;
> > > +
> > > +    if ( (rc = do_domctl(xch, &domctl)) < 0 )
> > > +        return rc;
> > > +
> > > +    *evtchn = domctl.u.vuart_op.evtchn;
> > > +
> > > +    return rc;
> > > +}
> > 
> > It looks like this function should be in one of the arm specific files,
> > such as xc_dom_arm.c (otherwise it becomes available to x86 too).
> 
> AFAICT xc_dom_arm.c has a completely different purpose. Looking at other
> helpers, it seems the usage if too #ifdef helpers (see xc_vcpu_get_extstate or
> xc_domain_set_memory_map).

That's true. It's best to continue that pattern and use #ifdefs here.


> > > +int libxl__arch_domain_create_finish(libxl__gc *gc,
> > > +                                     libxl_domain_build_info *info,
> > > +                                     uint32_t domid,
> > > +                                     libxl__domain_build_state *state)
> > > +{
> > > +    return 0;
> > > +}
> > > +
> > >  int libxl__arch_extra_memory(libxl__gc *gc,
> > >                               const libxl_domain_build_info *info,
> > >                               uint64_t *out)
> > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> > > index 76310ed..9e150ba 100644
> > > --- a/xen/arch/arm/domain.c
> > > +++ b/xen/arch/arm/domain.c
> > > @@ -665,6 +665,8 @@ fail:
> > > 
> > >  void arch_domain_destroy(struct domain *d)
> > >  {
> > > +    domain_vpl011_deinit(d);
> > > +
> > >      /* IOMMU page table is shared with P2M, always call
> > >       * iommu_domain_destroy() before p2m_teardown().
> > >       */
> > 
> > I cannot find the definition of domain_vpl011_deinit
> 
> See patch #3.

All right, thanks. Initially I thought it was weird for this change to
be here, but now I think it makes sense because this patch introduces
the call to domain_vpl011_init. 

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