Re: [PATCH 2/2] arm: export platform_op XENPF_settime

[Julien Grall <julien.grall@citrix.com>]

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?

>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>>> ---
>>>  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.

Regards,

-- 
Julien Grall