Re: [PATCH ARM v6 11/14] mini-os: arm: time

[Julien Grall <julien.grall@linaro.org>]

Hi Thomas,

On 07/16/2014 12:07 PM, Thomas Leonard wrote:
> +/* These are peridically updated in shared_info, and then copied here. */
> +struct shadow_time_info {
> +    uint64_t tsc_timestamp;     /* TSC at last update of time vals.  */
> +    uint64_t system_timestamp;  /* Time, in nanosecs, since boot.    */
> +    uint32_t tsc_to_nsec_mul;
> +    uint32_t tsc_to_usec_mul;
> +    int tsc_shift;
> +    uint32_t version;
> +};
> +static struct timespec shadow_ts;
> +static uint32_t shadow_ts_version;
> +
> +static struct shadow_time_info shadow;
> +
> +static void get_time_values_from_xen(void)
> +{
> +    struct vcpu_time_info    *src = &HYPERVISOR_shared_info->vcpu_info[0].time;
> +
> +     do {
> +        shadow.version = src->version;
> +        rmb();
> +        shadow.tsc_timestamp     = src->tsc_timestamp;
> +        shadow.system_timestamp  = src->system_time;
> +        shadow.tsc_to_nsec_mul   = src->tsc_to_system_mul;
> +        shadow.tsc_shift         = src->tsc_shift;
> +        rmb();
> +    }
> +    while ((src->version & 1) | (shadow.version ^ src->version));
> +
> +    shadow.tsc_to_usec_mul = shadow.tsc_to_nsec_mul / 1000;
> +}

It looks the code above is the same as x86. Did you think about sharing
the code which used Xen time?

Looking closer to this patch, nothing is using the field of this shared
structure, which is CPU time consuming and adding line for no
improvement. So I think you can drop all the code related the PV timer
for now.

Regards,

-- 
Julien Grall