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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 10 Nov 2015, Julien Grall wrote:
> Hi Stefano,
> 
> On 09/11/15 17:09, Stefano Stabellini wrote:
> > On Thu, 5 Nov 2015, Julien Grall wrote:
> >> 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.
> 
> Well, building dom0 takes more than one sec, even on big platform.
> 
> And if it's not subject to quick incremental, what's the point to call
> update_domain_wallclock_time in an odd way in arch_set_info_guest rather
> than in arch_domain_create?

Only to make sure that is called after domain_vtimer_init.
In fact I could move it to arch_domain_create right after it. That might
be better?


> >>> 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.
> 
> Are you sure? I can see some of the sub-hypercall implemented for ARM
> too such as XENPF_efi_runtime_call, XENPF_change_freq,
> XENPF_getidletime, XENPF_cpu_{online,offline}...
> 
> I'm not asking for implementing all of them now, but just preparing an
> infrastructure for later similar to the domctl version.

The spin_trylock call is not useful on ARM and many of the other ops are
not either. In addition the implementation of XENPF_settime64 is
slightly different too.

I don't think there is any value in trying to share the do_platform_op
function now. But maybe in the future.

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