Re: [Xen-devel] [PATCH 2/2] arm: export platform_op XENPF_settime

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 5 Nov 2015, Julien Grall wrote:
> Hi Stefano,
> 
> You forgot to CC Daniel for the XSM part. Please use
> scripts/get_maintainers.pl to get the relevant maintainers.
> 
> On 05/11/15 16:57, Stefano Stabellini wrote:
> > Call update_domain_wallclock_time at domain initialization.
> 
> It's not really what you are doing in the code. You are calling
> update_domain_wallclock_time when the first vCPU is initialized.
> 
> Also some rationale to explain why this call should be done here would
> be good.
> 
> Finally, I'm a bit surprised that you only need to call
> update_domain_wallclock_time when the domain is created. x86 needs to
> call in various places.

It is also called automatically from do_settime. I don't think we need
any other arch-specific call sites.


> For instance we may want to call update_domain_wallclock_time in
> construct_dom0 before clearing the pause flags. This is because the
> wallclock may be out of sync as construction DOM0 takes some time.

That's not necessary: the wallclock in Xen is the number
of seconds since 1970 at the time the physical machine booted, plus the
domain specific offset, so it is not subject to quick incremental
changes, like a monotonic clock.


> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > ---
> >  xen/arch/arm/Makefile             |    1 +
> >  xen/arch/arm/domain.c             |    3 ++
> >  xen/arch/arm/platform_hypercall.c |   62 
> > +++++++++++++++++++++++++++++++++++++
> >  xen/arch/arm/traps.c              |    1 +
> >  xen/include/xsm/dummy.h           |   12 +++----
> >  xen/include/xsm/xsm.h             |   13 ++++----
> 
> You also have to fix xsm/flask/hooks.c.

Uhm.. OK


> >  6 files changed, 80 insertions(+), 12 deletions(-)
> >  create mode 100644 xen/arch/arm/platform_hypercall.c
> 
> [..]
> 
> > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> > index b2bfc7d..ac9b1b3 100644
> > --- a/xen/arch/arm/domain.c
> > +++ b/xen/arch/arm/domain.c
> > @@ -742,6 +742,9 @@ int arch_set_info_guest(
> >      v->arch.ttbr1 = ctxt->ttbr1;
> >      v->arch.ttbcr = ctxt->ttbcr;
> >  
> > +    if ( v->vcpu_id == 0 )
> > +        update_domain_wallclock_time(v->domain);
> > +
> >      v->is_initialised = 1;
> >  
> >      if ( ctxt->flags & VGCF_online )
> > diff --git a/xen/arch/arm/platform_hypercall.c 
> > b/xen/arch/arm/platform_hypercall.c
> > new file mode 100644
> > index 0000000..f60d7b3
> > --- /dev/null
> > +++ b/xen/arch/arm/platform_hypercall.c
> > @@ -0,0 +1,62 @@
> > +/******************************************************************************
> > + * platform_hypercall.c
> > + * 
> > + * Hardware platform operations. Intended for use by domain-0 kernel.
> > + * 
> > + * Copyright (c) 2015, Citrix
> > + */
> > +
> > +#include <xen/config.h>
> > +#include <xen/types.h>
> > +#include <xen/sched.h>
> > +#include <xen/guest_access.h>
> > +#include <xen/spinlock.h>
> > +#include <public/platform.h>
> > +#include <xsm/xsm.h>
> > +#include <asm/current.h>
> > +#include <asm/event.h>
> > +
> > +DEFINE_SPINLOCK(xenpf_lock);
> > +
> > +long do_platform_op(XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op)
> > +{
> 
> Would it make sense to introduce a common platform code which take care
> of common hypercall? See for instance do_domctl and arch_do_domctl.

In this case I don't think so. I don't see the other existing
platform_ops being used on arm.

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