[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 2/2] arm: export platform_op XENPF_settime
On 10/11/15 14:27, Stefano Stabellini wrote: > 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? Yes, it would make more sense. I'm also wondering if we can directly do the call in domain_vtimer_init? >>>>> 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. That's a call to forget changing the locking when it will be required and may lead to security issue. It really doesn't hurt us to do the spin_trylock solution today. > > I don't think there is any value in trying to share the do_platform_op > function now. But maybe in the future. Ok. -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |