Re: + stupid-hack-to-make-mainline-build.patch added to -mm tree

Dan Hecht
2007-03-07T23:57:08Z

On 03/07/2007 03:33 PM, Jeremy Fitzhardinge wrote:
> I know the vmi time code has coloured your view here, but I surely hope
> it can be got into a better state before posting. I'm biased of course,
> but I would rather hope that all these drivers we're talking about will
> be as stylistically clean as the Xen time code (which has room for
> improvement, of course).
>

Could you send us comments on where you feel the style needs some fixing
up?

VMI encapsulates all the implementation details away from the kernel,
whereas the Xen time code puts it all out there in the kernel (see
snippet below). What happens when Xen wants to change the way it
implements "system time"? It looses compatibility with all existing
kernels....

In VMI terms, the code to read "system time" from the hypervisor is this
one-liner (it can be written in any "style" you want; the fact is, it's
just an interface call to the VMI-layer):

vmi_timer_ops.get_cycle_counter(VMI_CYCLES_REAL);

In Xen terms, the same code to accomplish that is:

/*
* Scale a 64-bit delta by scaling and multiplying by a 32-bit fraction,
* yielding a 64-bit result.
*/
static inline u64 scale_delta(u64 delta, u32 mul_frac, int shift)
{
u64 product;
#ifdef __i386__
u32 tmp1, tmp2;
#endif

if (shift < 0)
delta >>= -shift;
else
delta <<= shift;

#ifdef __i386__
__asm__ (
"mul %5 ; "
"mov %4,%%eax ; "
"mov %%edx,%4 ; "
"mul %5 ; "
"xor %5,%5 ; "
"add %4,%%eax ; "
"adc %5,%%edx ; "
: "=A" (product), "=r" (tmp1), "=r" (tmp2)
: "a" ((u32)delta), "1" ((u32)(delta >> 32)), "2" (mul_frac) );
#elif __x86_64__
__asm__ (
"mul %%rdx ; shrd $32,%%rdx,%%rax"
: "=a" (product) : "0" (delta), "d" ((u64)mul_frac) );
#else
#error implement me!
#endif

return product;
}

static u64 get_nsec_offset(struct shadow_time_info *shadow)
{
u64 now, delta;
rdtscll(now);
delta = now - shadow->tsc_timestamp;
return scale_delta(delta, shadow->tsc_to_nsec_mul, shadow->tsc_shift);
}

static cycle_t xen_clocksource_read(void)
{
struct shadow_time_info *shadow = &get_cpu_var(shadow_time);
cycle_t ret;

get_time_values_from_xen();

ret = shadow->system_timestamp + get_nsec_offset(shadow);

put_cpu_var(shadow_time);

return ret;
}