[PATCH v3 0/2] wallclock time on arm

[PATCH v3 0/2] wallclock time on arm

[Stefano Stabellini]

Hi all,

this small series enables the wallclock time on arm and it consists
mostly in code movement from x86 to common.

Changes in v3:
- remove stray blank lines
- move update_domain_wallclock_time() call to arch_domain_create
- move setting time_offset_seconds to domain_vtimer_init
- use spin_trylock(&xenpf_lock) loop in do_platform_op

Changes in v2:
- remove stray blank lines
- remove include <xen/config.h>
- move version_update_* to include/xen/time.h
- introduce ifdef to fix build issue in common/time.c
- define wc_sec and sec as uint64_t
- pass u64 to do_settime
- drop XENPF_settime32
- set time_offset_seconds
- modify xen/xsm/flask/hooks.c


Stefano Stabellini (2):
      xen: move wallclock functions from x86 to common
      arm: export platform_op XENPF_settime64

 xen/arch/arm/Makefile             |    1 +
 xen/arch/arm/domain.c             |    2 +
 xen/arch/arm/platform_hypercall.c |   70 +++++++++++++++++++++++++++
 xen/arch/arm/time.c               |    5 --
 xen/arch/arm/traps.c              |    1 +
 xen/arch/arm/vtimer.c             |    4 +-
 xen/arch/x86/time.c               |   96 +------------------------------------
 xen/common/time.c                 |   95 +++++++++++++++++++++++++++++++++++-
 xen/include/asm-arm/time.h        |    2 +
 xen/include/xen/time.h            |    5 +-
 xen/include/xsm/dummy.h           |   12 ++---
 xen/include/xsm/xsm.h             |   13 ++---
 xen/xsm/flask/hooks.c             |    2 +-
 13 files changed, 192 insertions(+), 116 deletions(-)
 create mode 100644 xen/arch/arm/platform_hypercall.c


Cheers,

Stefano

[PATCH v3 1/2] xen: move wallclock functions from x86 to common

[Stefano Stabellini]

Remove dummy arm implementation of wallclock_time.
Use shared_info() in common code rather than x86-ism to access it, when
possible.

Define the static variable wc_sec, and the local variale sec in
update_domain_wallclock_time, as uint64_t instead of unsigned long, to
avoid size issue on arm.
Take a uint64_t sec paramter in do_settime for the same reason.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
CC: JBeulich@suse.com
CC: andrew.cooper3@citrix.com

---

Changes in v3:
- remove stray blank lines

Changes in v2:
- remove stray blank lines
- remove include <xen/config.h>
- move version_update_* to include/xen/time.h
- introduce ifdef to fix build issue in common/time.c
- define wc_sec and sec as uint64_t
- pass u64 to do_settime
---
 xen/arch/arm/time.c    |    5 ---
 xen/arch/x86/time.c    |   96 +-----------------------------------------------
 xen/common/time.c      |   95 ++++++++++++++++++++++++++++++++++++++++++++++-
 xen/include/xen/time.h |    5 ++-
 4 files changed, 99 insertions(+), 102 deletions(-)

diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index 5ded30c..6207615 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -280,11 +280,6 @@ void domain_set_time_offset(struct domain *d, int64_t time_offset_seconds)
     /* XXX update guest visible wallclock time */
 }
 
-struct tm wallclock_time(uint64_t *ns)
-{
-    return (struct tm) { 0 };
-}
-
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index bbb7e6c..0f16db5 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -47,9 +47,6 @@ string_param("clocksource", opt_clocksource);
 unsigned long __read_mostly cpu_khz;  /* CPU clock frequency in kHz. */
 DEFINE_SPINLOCK(rtc_lock);
 unsigned long pit0_ticks;
-static unsigned long wc_sec; /* UTC time at last 'time update'. */
-static unsigned int wc_nsec;
-static DEFINE_SPINLOCK(wc_lock);
 
 struct cpu_time {
     u64 local_tsc_stamp;
@@ -783,10 +780,6 @@ uint64_t tsc_ticks2ns(uint64_t ticks)
     return scale_delta(ticks, &t->tsc_scale);
 }
 
-/* Explicitly OR with 1 just in case version number gets out of sync. */
-#define version_update_begin(v) (((v)+1)|1)
-#define version_update_end(v)   ((v)+1)
-
 static void __update_vcpu_system_time(struct vcpu *v, int force)
 {
     struct cpu_time       *t;
@@ -900,37 +893,6 @@ void force_update_vcpu_system_time(struct vcpu *v)
     __update_vcpu_system_time(v, 1);
 }
 
-void update_domain_wallclock_time(struct domain *d)
-{
-    uint32_t *wc_version;
-    unsigned long sec;
-
-    spin_lock(&wc_lock);
-
-    wc_version = &shared_info(d, wc_version);
-    *wc_version = version_update_begin(*wc_version);
-    wmb();
-
-    sec = wc_sec + d->time_offset_seconds;
-    if ( likely(!has_32bit_shinfo(d)) )
-    {
-        d->shared_info->native.wc_sec    = sec;
-        d->shared_info->native.wc_nsec   = wc_nsec;
-        d->shared_info->native.wc_sec_hi = sec >> 32;
-    }
-    else
-    {
-        d->shared_info->compat.wc_sec         = sec;
-        d->shared_info->compat.wc_nsec        = wc_nsec;
-        d->shared_info->compat.arch.wc_sec_hi = sec >> 32;
-    }
-
-    wmb();
-    *wc_version = version_update_end(*wc_version);
-
-    spin_unlock(&wc_lock);
-}
-
 static void update_domain_rtc(void)
 {
     struct domain *d;
@@ -988,27 +950,6 @@ int cpu_frequency_change(u64 freq)
     return 0;
 }
 
-/* Set clock to <secs,usecs> after 00:00:00 UTC, 1 January, 1970. */
-void do_settime(unsigned long secs, unsigned int nsecs, u64 system_time_base)
-{
-    u64 x;
-    u32 y;
-    struct domain *d;
-
-    x = SECONDS(secs) + nsecs - system_time_base;
-    y = do_div(x, 1000000000);
-
-    spin_lock(&wc_lock);
-    wc_sec  = x;
-    wc_nsec = y;
-    spin_unlock(&wc_lock);
-
-    rcu_read_lock(&domlist_read_lock);
-    for_each_domain ( d )
-        update_domain_wallclock_time(d);
-    rcu_read_unlock(&domlist_read_lock);
-}
-
 /* Per-CPU communication between rendezvous IRQ and softirq handler. */
 struct cpu_calibration {
     u64 local_tsc_stamp;
@@ -1608,25 +1549,6 @@ void send_timer_event(struct vcpu *v)
     send_guest_vcpu_virq(v, VIRQ_TIMER);
 }
 
-/* Return secs after 00:00:00 localtime, 1 January, 1970. */
-unsigned long get_localtime(struct domain *d)
-{
-    return wc_sec + (wc_nsec + NOW()) / 1000000000ULL 
-        + d->time_offset_seconds;
-}
-
-/* Return microsecs after 00:00:00 localtime, 1 January, 1970. */
-uint64_t get_localtime_us(struct domain *d)
-{
-    return (SECONDS(wc_sec + d->time_offset_seconds) + wc_nsec + NOW())
-           / 1000UL;
-}
-
-unsigned long get_sec(void)
-{
-    return wc_sec + (wc_nsec + NOW()) / 1000000000ULL;
-}
-
 /* "cmos_utc_offset" is the difference between UTC time and CMOS time. */
 static long cmos_utc_offset; /* in seconds */
 
@@ -1635,7 +1557,7 @@ int time_suspend(void)
     if ( smp_processor_id() == 0 )
     {
         cmos_utc_offset = -get_cmos_time();
-        cmos_utc_offset += (wc_sec + (wc_nsec + NOW()) / 1000000000ULL);
+        cmos_utc_offset += get_sec();
         kill_timer(&calibration_timer);
 
         /* Sync platform timer stamps. */
@@ -1715,22 +1637,6 @@ int hwdom_pit_access(struct ioreq *ioreq)
     return 0;
 }
 
-struct tm wallclock_time(uint64_t *ns)
-{
-    uint64_t seconds, nsec;
-
-    if ( !wc_sec )
-        return (struct tm) { 0 };
-
-    seconds = NOW() + SECONDS(wc_sec) + wc_nsec;
-    nsec = do_div(seconds, 1000000000);
-
-    if ( ns )
-        *ns = nsec;
-
-    return gmtime(seconds);
-}
-
 /*
  * PV SoftTSC Emulation.
  */
diff --git a/xen/common/time.c b/xen/common/time.c
index 29fdf52..721ada8 100644
--- a/xen/common/time.c
+++ b/xen/common/time.c
@@ -15,8 +15,12 @@
  * along with this program; If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <xen/config.h>
+#include <xen/sched.h>
+#include <xen/shared.h>
+#include <xen/spinlock.h>
 #include <xen/time.h>
+#include <asm/div64.h>
+#include <asm/domain.h>
 
 /* Nonzero if YEAR is a leap year (every 4 years,
    except every 100th isn't, and every 400th is).  */
@@ -34,6 +38,10 @@ const unsigned short int __mon_lengths[2][12] = {
 #define SECS_PER_HOUR (60 * 60)
 #define SECS_PER_DAY  (SECS_PER_HOUR * 24)
 
+static uint64_t wc_sec; /* UTC time at last 'time update'. */
+static unsigned int wc_nsec;
+static DEFINE_SPINLOCK(wc_lock);
+
 struct tm gmtime(unsigned long t)
 {
     struct tm tbuf;
@@ -85,3 +93,88 @@ struct tm gmtime(unsigned long t)
 
     return tbuf;
 }
+
+void update_domain_wallclock_time(struct domain *d)
+{
+    uint32_t *wc_version;
+    uint64_t sec;
+
+    spin_lock(&wc_lock);
+
+    wc_version = &shared_info(d, wc_version);
+    *wc_version = version_update_begin(*wc_version);
+    wmb();
+
+    sec = wc_sec + d->time_offset_seconds;
+    shared_info(d, wc_sec)    = sec;
+    shared_info(d, wc_nsec)   = wc_nsec;
+#ifdef CONFIG_X86
+    if ( likely(!has_32bit_shinfo(d)) )
+        d->shared_info->native.wc_sec_hi = sec >> 32;
+    else
+        d->shared_info->compat.arch.wc_sec_hi = sec >> 32;
+#else
+    shared_info(d, wc_sec_hi) = sec >> 32;
+#endif
+
+    wmb();
+    *wc_version = version_update_end(*wc_version);
+
+    spin_unlock(&wc_lock);
+}
+
+/* Set clock to <secs,usecs> after 00:00:00 UTC, 1 January, 1970. */
+void do_settime(u64 secs, unsigned int nsecs, u64 system_time_base)
+{
+    u64 x;
+    u32 y;
+    struct domain *d;
+
+    x = SECONDS(secs) + nsecs - system_time_base;
+    y = do_div(x, 1000000000);
+
+    spin_lock(&wc_lock);
+    wc_sec  = x;
+    wc_nsec = y;
+    spin_unlock(&wc_lock);
+
+    rcu_read_lock(&domlist_read_lock);
+    for_each_domain ( d )
+        update_domain_wallclock_time(d);
+    rcu_read_unlock(&domlist_read_lock);
+}
+
+/* Return secs after 00:00:00 localtime, 1 January, 1970. */
+unsigned long get_localtime(struct domain *d)
+{
+    return wc_sec + (wc_nsec + NOW()) / 1000000000ULL
+        + d->time_offset_seconds;
+}
+
+/* Return microsecs after 00:00:00 localtime, 1 January, 1970. */
+uint64_t get_localtime_us(struct domain *d)
+{
+    return (SECONDS(wc_sec + d->time_offset_seconds) + wc_nsec + NOW())
+           / 1000UL;
+}
+
+unsigned long get_sec(void)
+{
+    return wc_sec + (wc_nsec + NOW()) / 1000000000ULL;
+}
+
+struct tm wallclock_time(uint64_t *ns)
+{
+    uint64_t seconds, nsec;
+
+    if ( !wc_sec )
+        return (struct tm) { 0 };
+
+    seconds = NOW() + SECONDS(wc_sec) + wc_nsec;
+    nsec = do_div(seconds, 1000000000);
+
+    if ( ns )
+        *ns = nsec;
+
+    return gmtime(seconds);
+}
diff --git a/xen/include/xen/time.h b/xen/include/xen/time.h
index da4e8d7..b742746 100644
--- a/xen/include/xen/time.h
+++ b/xen/include/xen/time.h
@@ -60,11 +60,14 @@ struct tm wallclock_time(uint64_t *ns);
 /* Chosen so (NOW() + delta) wont overflow without an uptime of 200 years */
 #define STIME_DELTA_MAX ((s_time_t)((uint64_t)~0ull>>2))
 
+/* Explicitly OR with 1 just in case version number gets out of sync. */
+#define version_update_begin(v) (((v) + 1) | 1)
+#define version_update_end(v)   ((v) + 1)
 extern void update_vcpu_system_time(struct vcpu *v);
 extern void update_domain_wallclock_time(struct domain *d);
 
 extern void do_settime(
-    unsigned long secs, unsigned int nsecs, u64 system_time_base);
+    u64 secs, unsigned int nsecs, u64 system_time_base);
 
 extern void send_timer_event(struct vcpu *v);
 
-- 
1.7.10.4

[PATCH v3 2/2] arm: export platform_op XENPF_settime64

[Stefano Stabellini]

Call update_domain_wallclock_time at domain initialization.
Set time_offset_seconds to the number of seconds between physical boot
and domain initialization: it is going to be used to get/set the
wallclock time.
Add time_offset_seconds to system_time when before calling do_settime,
so that system_time actually accounts for all the time in nsec between
machine boot and when the wallclock was set.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: dgdegra@tycho.nsa.gov
CC: dgdegra@tycho.nsa.gov

---

Changes in v3:
- move update_domain_wallclock_time() call to arch_domain_create
- move setting time_offset_seconds to domain_vtimer_init
- use spin_trylock(&xenpf_lock) loop in do_platform_op

Changes in v2:
- drop XENPF_settime32
- set time_offset_seconds
- modify xen/xsm/flask/hooks.c
---
 xen/arch/arm/Makefile             |    1 +
 xen/arch/arm/domain.c             |    2 ++
 xen/arch/arm/platform_hypercall.c |   70 +++++++++++++++++++++++++++++++++++++
 xen/arch/arm/traps.c              |    1 +
 xen/arch/arm/vtimer.c             |    4 ++-
 xen/include/asm-arm/time.h        |    2 ++
 xen/include/xsm/dummy.h           |   12 +++----
 xen/include/xsm/xsm.h             |   13 +++----
 xen/xsm/flask/hooks.c             |    2 +-
 9 files changed, 93 insertions(+), 14 deletions(-)
 create mode 100644 xen/arch/arm/platform_hypercall.c

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 1ef39f7..240aa29 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -23,6 +23,7 @@ obj-y += percpu.o
 obj-y += guestcopy.o
 obj-y += physdev.o
 obj-y += platform.o
+obj-y += platform_hypercall.o
 obj-y += setup.o
 obj-y += bootfdt.o
 obj-y += time.o
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index b2bfc7d..b17f6cf 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -598,6 +598,8 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
     if ( (rc = domain_vtimer_init(d, config)) != 0 )
         goto fail;
 
+    update_domain_wallclock_time(d);
+
     /*
      * The hardware domain will get a PPI later in
      * arch/arm/domain_build.c  depending on the
diff --git a/xen/arch/arm/platform_hypercall.c b/xen/arch/arm/platform_hypercall.c
new file mode 100644
index 0000000..b708711
--- /dev/null
+++ b/xen/arch/arm/platform_hypercall.c
@@ -0,0 +1,70 @@
+/******************************************************************************
+ * 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)
+{
+    long ret;
+    struct xen_platform_op curop, *op = &curop;
+    struct domain *d;
+
+    if ( copy_from_guest(op, u_xenpf_op, 1) )
+        return -EFAULT;
+
+    if ( op->interface_version != XENPF_INTERFACE_VERSION )
+        return -EACCES;
+
+    d = rcu_lock_current_domain();
+    if ( d == NULL )
+        return -ESRCH;
+
+    ret = xsm_platform_op(XSM_PRIV, op->cmd);
+    if ( ret )
+        return ret;
+
+    /*
+     * Trylock here avoids deadlock with an existing platform critical section
+     * which might (for some current or future reason) want to synchronise
+     * with this vcpu.
+     */
+    while ( !spin_trylock(&xenpf_lock) )
+        if ( hypercall_preempt_check() )
+            return hypercall_create_continuation(
+                __HYPERVISOR_platform_op, "h", u_xenpf_op);
+
+    switch ( op->cmd )
+    {
+    case XENPF_settime64:
+        if ( likely(!op->u.settime64.mbz) )
+            do_settime(op->u.settime64.secs,
+                       op->u.settime64.nsecs,
+                       op->u.settime64.system_time + SECONDS(d->time_offset_seconds));
+        else
+            ret = -EINVAL;
+        break;
+
+    default:
+        ret = -ENOSYS;
+        break;
+    }
+
+    spin_unlock(&xenpf_lock);
+    rcu_unlock_domain(d);
+    return ret;
+}
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 9d2bd6a..c49bd3f 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1233,6 +1233,7 @@ static arm_hypercall_t arm_hypercall_table[] = {
     HYPERCALL(hvm_op, 2),
     HYPERCALL(grant_table_op, 3),
     HYPERCALL(multicall, 2),
+    HYPERCALL(platform_op, 1),
     HYPERCALL_ARM(vcpu_op, 3),
 };
 
diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
index 1418092..aa0fde6 100644
--- a/xen/arch/arm/vtimer.c
+++ b/xen/arch/arm/vtimer.c
@@ -22,13 +22,13 @@
 #include <xen/timer.h>
 #include <xen/sched.h>
 #include <xen/perfc.h>
+#include <asm/div64.h>
 #include <asm/irq.h>
 #include <asm/time.h>
 #include <asm/gic.h>
 #include <asm/vgic.h>
 #include <asm/regs.h>
 
-extern s_time_t ticks_to_ns(uint64_t ticks);
 extern uint64_t ns_to_ticks(s_time_t ns);
 
 /*
@@ -64,6 +64,8 @@ int domain_vtimer_init(struct domain *d, struct xen_arch_domainconfig *config)
 {
     d->arch.phys_timer_base.offset = NOW();
     d->arch.virt_timer_base.offset = READ_SYSREG64(CNTPCT_EL0);
+    d->time_offset_seconds = ticks_to_ns(d->arch.virt_timer_base.offset - boot_count);
+    do_div(d->time_offset_seconds, 1000000000);
 
     config->clock_frequency = timer_dt_clock_frequency;
 
diff --git a/xen/include/asm-arm/time.h b/xen/include/asm-arm/time.h
index d755f36..23e5a28 100644
--- a/xen/include/asm-arm/time.h
+++ b/xen/include/asm-arm/time.h
@@ -37,6 +37,8 @@ extern void __cpuinit init_timer_interrupt(void);
 /* Counter value at boot time */
 extern uint64_t boot_count;
 
+extern s_time_t ticks_to_ns(uint64_t ticks);
+
 void preinit_xen_time(void);
 
 #endif /* __ARM_TIME_H__ */
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 9fe372c..e43f2a1 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -584,6 +584,12 @@ static XSM_INLINE int xsm_mem_sharing(XSM_DEFAULT_ARG struct domain *d)
 }
 #endif
 
+static XSM_INLINE int xsm_platform_op(XSM_DEFAULT_ARG uint32_t op)
+{
+    XSM_ASSERT_ACTION(XSM_PRIV);
+    return xsm_default_action(action, current->domain, NULL);
+}
+
 #ifdef CONFIG_X86
 static XSM_INLINE int xsm_do_mca(XSM_DEFAULT_VOID)
 {
@@ -639,12 +645,6 @@ static XSM_INLINE int xsm_apic(XSM_DEFAULT_ARG struct domain *d, int cmd)
     return xsm_default_action(action, d, NULL);
 }
 
-static XSM_INLINE int xsm_platform_op(XSM_DEFAULT_ARG uint32_t op)
-{
-    XSM_ASSERT_ACTION(XSM_PRIV);
-    return xsm_default_action(action, current->domain, NULL);
-}
-
 static XSM_INLINE int xsm_machine_memory_map(XSM_DEFAULT_VOID)
 {
     XSM_ASSERT_ACTION(XSM_PRIV);
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index ba3caed..f48cf60 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -164,6 +164,8 @@ struct xsm_operations {
     int (*mem_sharing) (struct domain *d);
 #endif
 
+    int (*platform_op) (uint32_t cmd);
+
 #ifdef CONFIG_X86
     int (*do_mca) (void);
     int (*shadow_control) (struct domain *d, uint32_t op);
@@ -175,7 +177,6 @@ struct xsm_operations {
     int (*mem_sharing_op) (struct domain *d, struct domain *cd, int op);
     int (*apic) (struct domain *d, int cmd);
     int (*memtype) (uint32_t access);
-    int (*platform_op) (uint32_t cmd);
     int (*machine_memory_map) (void);
     int (*domain_memory_map) (struct domain *d);
 #define XSM_MMU_UPDATE_READ      1
@@ -624,6 +625,11 @@ static inline int xsm_mem_sharing (xsm_default_t def, struct domain *d)
 }
 #endif
 
+static inline int xsm_platform_op (xsm_default_t def, uint32_t op)
+{
+    return xsm_ops->platform_op(op);
+}
+
 #ifdef CONFIG_X86
 static inline int xsm_do_mca(xsm_default_t def)
 {
@@ -675,11 +681,6 @@ static inline int xsm_memtype (xsm_default_t def, uint32_t access)
     return xsm_ops->memtype(access);
 }
 
-static inline int xsm_platform_op (xsm_default_t def, uint32_t op)
-{
-    return xsm_ops->platform_op(op);
-}
-
 static inline int xsm_machine_memory_map(xsm_default_t def)
 {
     return xsm_ops->machine_memory_map();
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index fafb1a4..680485e 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1735,6 +1735,7 @@ static struct xsm_operations flask_ops = {
     .deassign_dtdevice = flask_deassign_dtdevice,
 #endif
 
+    .platform_op = flask_platform_op,
 #ifdef CONFIG_X86
     .do_mca = flask_do_mca,
     .shadow_control = flask_shadow_control,
@@ -1745,7 +1746,6 @@ static struct xsm_operations flask_ops = {
     .hvm_ioreq_server = flask_hvm_ioreq_server,
     .mem_sharing_op = flask_mem_sharing_op,
     .apic = flask_apic,
-    .platform_op = flask_platform_op,
     .machine_memory_map = flask_machine_memory_map,
     .domain_memory_map = flask_domain_memory_map,
     .mmu_update = flask_mmu_update,
-- 
1.7.10.4

Re: [PATCH v3 2/2] arm: export platform_op XENPF_settime64

[Julien Grall]

Hi Stefano,

On 11/11/15 17:19, Stefano Stabellini wrote:
> Call update_domain_wallclock_time at domain initialization.
> Set time_offset_seconds to the number of seconds between physical boot
> and domain initialization: it is going to be used to get/set the
> wallclock time.
> Add time_offset_seconds to system_time when before calling do_settime,
> so that system_time actually accounts for all the time in nsec between
> machine boot and when the wallclock was set.

+ Expose the xsm_platform_op to ARM

+ Add the prototype of ticks_to_ns in the headers

> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Acked-by: dgdegra@tycho.nsa.gov
> CC: dgdegra@tycho.nsa.gov
> 
> ---

[...]

> diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
> index 1418092..aa0fde6 100644
> --- a/xen/arch/arm/vtimer.c
> +++ b/xen/arch/arm/vtimer.c
> @@ -22,13 +22,13 @@
>  #include <xen/timer.h>
>  #include <xen/sched.h>
>  #include <xen/perfc.h>
> +#include <asm/div64.h>
>  #include <asm/irq.h>
>  #include <asm/time.h>
>  #include <asm/gic.h>
>  #include <asm/vgic.h>
>  #include <asm/regs.h>
>  
> -extern s_time_t ticks_to_ns(uint64_t ticks);

If it's a valid change you should mention it in the commit message
because it's not related to this patch.

Otherwise it looks spurious...

[...]

> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> index fafb1a4..680485e 100644
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -1735,6 +1735,7 @@ static struct xsm_operations flask_ops = {
>      .deassign_dtdevice = flask_deassign_dtdevice,
>  #endif
>  
> +    .platform_op = flask_platform_op,

Please build test Xen with XSM enabled, I'm nearly sure you that you
need to move the function flask_platform_op out of the #ifdef CONFIG_X86.

Regards,

-- 
Julien Grall

Re: [PATCH v3 2/2] arm: export platform_op XENPF_settime64

[Stefano Stabellini]

On Wed, 11 Nov 2015, Julien Grall wrote:
> Hi Stefano,
> 
> On 11/11/15 17:19, Stefano Stabellini wrote:
> > Call update_domain_wallclock_time at domain initialization.
> > Set time_offset_seconds to the number of seconds between physical boot
> > and domain initialization: it is going to be used to get/set the
> > wallclock time.
> > Add time_offset_seconds to system_time when before calling do_settime,
> > so that system_time actually accounts for all the time in nsec between
> > machine boot and when the wallclock was set.
> 
> + Expose the xsm_platform_op to ARM
> 
> + Add the prototype of ticks_to_ns in the headers
> 
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > Acked-by: dgdegra@tycho.nsa.gov
> > CC: dgdegra@tycho.nsa.gov
> > 
> > ---
> 
> [...]
> 
> > diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
> > index 1418092..aa0fde6 100644
> > --- a/xen/arch/arm/vtimer.c
> > +++ b/xen/arch/arm/vtimer.c
> > @@ -22,13 +22,13 @@
> >  #include <xen/timer.h>
> >  #include <xen/sched.h>
> >  #include <xen/perfc.h>
> > +#include <asm/div64.h>
> >  #include <asm/irq.h>
> >  #include <asm/time.h>
> >  #include <asm/gic.h>
> >  #include <asm/vgic.h>
> >  #include <asm/regs.h>
> >  
> > -extern s_time_t ticks_to_ns(uint64_t ticks);
> 
> If it's a valid change you should mention it in the commit message
> because it's not related to this patch.
> 
> Otherwise it looks spurious...

Sorry, that's not a valid change anymore, I meant to remove it. Thanks
for checking.


> > diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> > index fafb1a4..680485e 100644
> > --- a/xen/xsm/flask/hooks.c
> > +++ b/xen/xsm/flask/hooks.c
> > @@ -1735,6 +1735,7 @@ static struct xsm_operations flask_ops = {
> >      .deassign_dtdevice = flask_deassign_dtdevice,
> >  #endif
> >  
> > +    .platform_op = flask_platform_op,
> 
> Please build test Xen with XSM enabled, I'm nearly sure you that you
> need to move the function flask_platform_op out of the #ifdef CONFIG_X86.
 
Yes, you are right. Besides flask_platform_op, I found another issue
that way.

Re: [PATCH v3 2/2] arm: export platform_op XENPF_settime64

[Julien Grall]

On 11/11/15 18:14, Stefano Stabellini wrote:
> On Wed, 11 Nov 2015, Julien Grall wrote:
>>> diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
>>> index 1418092..aa0fde6 100644
>>> --- a/xen/arch/arm/vtimer.c
>>> +++ b/xen/arch/arm/vtimer.c
>>> @@ -22,13 +22,13 @@
>>>  #include <xen/timer.h>
>>>  #include <xen/sched.h>
>>>  #include <xen/perfc.h>
>>> +#include <asm/div64.h>
>>>  #include <asm/irq.h>
>>>  #include <asm/time.h>
>>>  #include <asm/gic.h>
>>>  #include <asm/vgic.h>
>>>  #include <asm/regs.h>
>>>  
>>> -extern s_time_t ticks_to_ns(uint64_t ticks);
>>
>> If it's a valid change you should mention it in the commit message
>> because it's not related to this patch.
>>
>> Otherwise it looks spurious...
> 
> Sorry, that's not a valid change anymore, I meant to remove it. Thanks
> for checking.

If it wasn't clear, I think this change is valid. Defining the prototype
in the C file is a call to have the prototype not matching the declaration.

Any prototypes should be declared in the header and not that way.

I'm fine with this, but it should be mentioned in the commit message.

Regards,

-- 
Julien Grall

Re: [PATCH v3 2/2] arm: export platform_op XENPF_settime64

[Julien Grall]

On 12/11/15 11:32, Stefano Stabellini wrote:
> On Wed, 11 Nov 2015, Julien Grall wrote:
>> On 11/11/15 18:14, Stefano Stabellini wrote:
>>> On Wed, 11 Nov 2015, Julien Grall wrote:
>>>>> diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
>>>>> index 1418092..aa0fde6 100644
>>>>> --- a/xen/arch/arm/vtimer.c
>>>>> +++ b/xen/arch/arm/vtimer.c
>>>>> @@ -22,13 +22,13 @@
>>>>>  #include <xen/timer.h>
>>>>>  #include <xen/sched.h>
>>>>>  #include <xen/perfc.h>
>>>>> +#include <asm/div64.h>
>>>>>  #include <asm/irq.h>
>>>>>  #include <asm/time.h>
>>>>>  #include <asm/gic.h>
>>>>>  #include <asm/vgic.h>
>>>>>  #include <asm/regs.h>
>>>>>  
>>>>> -extern s_time_t ticks_to_ns(uint64_t ticks);
>>>>
>>>> If it's a valid change you should mention it in the commit message
>>>> because it's not related to this patch.
>>>>
>>>> Otherwise it looks spurious...
>>>
>>> Sorry, that's not a valid change anymore, I meant to remove it. Thanks
>>> for checking.
>>
>> If it wasn't clear, I think this change is valid. Defining the prototype
>> in the C file is a call to have the prototype not matching the declaration.
>>
>> Any prototypes should be declared in the header and not that way.
>>
>> I'm fine with this, but it should be mentioned in the commit message.
> 
> It might be a decent change to have, but it is not needed by this patch
> anymore, so if anything it should go in on its own.

I'm fine with that.

Regards,

-- 
Julien Grall

Re: [PATCH v3 2/2] arm: export platform_op XENPF_settime64

[Stefano Stabellini]

On Wed, 11 Nov 2015, Julien Grall wrote:
> On 11/11/15 18:14, Stefano Stabellini wrote:
> > On Wed, 11 Nov 2015, Julien Grall wrote:
> >>> diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
> >>> index 1418092..aa0fde6 100644
> >>> --- a/xen/arch/arm/vtimer.c
> >>> +++ b/xen/arch/arm/vtimer.c
> >>> @@ -22,13 +22,13 @@
> >>>  #include <xen/timer.h>
> >>>  #include <xen/sched.h>
> >>>  #include <xen/perfc.h>
> >>> +#include <asm/div64.h>
> >>>  #include <asm/irq.h>
> >>>  #include <asm/time.h>
> >>>  #include <asm/gic.h>
> >>>  #include <asm/vgic.h>
> >>>  #include <asm/regs.h>
> >>>  
> >>> -extern s_time_t ticks_to_ns(uint64_t ticks);
> >>
> >> If it's a valid change you should mention it in the commit message
> >> because it's not related to this patch.
> >>
> >> Otherwise it looks spurious...
> > 
> > Sorry, that's not a valid change anymore, I meant to remove it. Thanks
> > for checking.
> 
> If it wasn't clear, I think this change is valid. Defining the prototype
> in the C file is a call to have the prototype not matching the declaration.
> 
> Any prototypes should be declared in the header and not that way.
> 
> I'm fine with this, but it should be mentioned in the commit message.

It might be a decent change to have, but it is not needed by this patch
anymore, so if anything it should go in on its own.