[Linux-ia64] itc sync & clock_*

[Linux-ia64] itc sync & clock_*

[Jesse Barnes]

David, you mentinoned awhile back that you don't expect the itc counters
to be synchronized in future machines and that apps should use
gettimeofday and the posix clock_* routines to get timestamps.  However,
the clock_* routines in recent versions of glibc for ia64 look at ar.itc
as the source of clock tick information.  Does that need to be
fixed?  This problem is more immediate on our hardware, as the clocks are
not synchronized, so I'd like to come up with a solution.  I'm not sure I
understand all the issues yet, but I'd like to solicit people for ideas
and/or explaination about this issue.

Thanks,
Jesse

Re: [Linux-ia64] itc sync & clock_*

[David Mosberger]

>>>>> On Fri, 26 Oct 2001 10:09:47 -0700, Jesse Barnes <jbarnes@sgi.com> said:

  Jesse> David, you mentinoned awhile back that you don't expect the
  Jesse> itc counters to be synchronized in future machines and that
  Jesse> apps should use gettimeofday and the posix clock_* routines
  Jesse> to get timestamps.  However, the clock_* routines in recent
  Jesse> versions of glibc for ia64 look at ar.itc as the source of
  Jesse> clock tick information.  Does that need to be fixed?

Yes.  Uli knows about this.  It's just that we couldn't do anything
about it because there is no way of detecting this case.

  Jesse> This problem is more immediate on our hardware, as the clocks
  Jesse> are not synchronized, so I'd like to come up with a solution.
  Jesse> I'm not sure I understand all the issues yet, but I'd like to
  Jesse> solicit people for ideas and/or explaination about this
  Jesse> issue.

The first thing to do is to get this into the firmware.  Linux needs a
way to detect when the ITCs are not driven off the same clock (i.e.,
when there is a chance for the ITCs to drift).  There also needs to be
a way for detecting when the clock frequencies are different, but I
believe the info provided by PAL/SAL is sufficient (my only worry are
rounding errors...).

	--david

Re: [Linux-ia64] itc sync & clock_*

[Jesse Barnes]

On Mon, 29 Oct 2001, David Mosberger wrote:

> The first thing to do is to get this into the firmware.  Linux needs a
> way to detect when the ITCs are not driven off the same clock (i.e.,
> when there is a chance for the ITCs to drift).  There also needs to be
> a way for detecting when the clock frequencies are different, but I
> believe the info provided by PAL/SAL is sufficient (my only worry are
> rounding errors...).

Ok, we'll work on getting this pushed back, so at least the kernel can
figure out what to do.  Regarding glibc though: we really don't want to
have a different version just for our systems, so I'd like to come up with
a generic solution.  Maybe an ia64 specific clock driver that glibc could
map in to read synchronized clock info?  Is there a platform independent
way of doing this?  I'll poke around glibc some more to see if any of the
other arches deal with tis problem, but I'm sure there are people that are
already familiar with these sorts of issues that might be able to help
out.  Anyone?

Thanks a lot,
Jesse

Re: [Linux-ia64] itc sync & clock_*

[David Mosberger]

>>>>> On Mon, 29 Oct 2001 10:10:44 -0800, Jesse Barnes <jbarnes@sgi.com> said:

  Jesse> Regarding glibc though: we really don't want to have a
  Jesse> different version just for our systems, so I'd like to come
  Jesse> up with a generic solution.

Oh, yes, I guess I wasn't quite clear in my response: the "ITC are
synchronous" flag needs to be exported to user level (probably through
a /proc file).  glibc then reads that flag and if the ITCs are *not*
synchronous, it has two choices:

	- if there is a driver for a fine-grained machine timer, it can
	  use it

	- if no such driver exists, it will not be able to provide
	  fine grained time stamps

So, you'll have to define and implement a driver interface to your
machine timer (on SN1 its in the chipset, right?).  If you want to go
ahead with this, you might want to take a look at the Intel multimedia
timer spec as well.  I have my issues with that spec (it doesn't
provide sufficient alignment guarantees, in my opiniono), but
considering it should help in designing an interface that will be
useful not just for SGI machines, but for all "big" ia64 machines.

  Jesse> Maybe an ia64 specific clock driver that glibc could map in
  Jesse> to read synchronized clock info?

Yup.  The driver interface needs to allow direct user-level access to
the fine grained timer.  Though I'd say the interface should be able
to accommodate hardware that can't map the timer into user space.

	--david

Re: [Linux-ia64] itc sync & clock_*

[Jack Steiner]

I sent mail to Asit last week about this. Copy is attached:


>> Asit -
>> 
>> The SGI platforms do not have a synchronized FSB clock across all nodes.
>> The result of this is ar.itc on different cpus may drift. There is code in the kernel
>> that assumes that all nodes stay synchronized. This obviously doesn't work on
>> our platform. (I expect other platforms will have similar problems).
>> 
>> We send David mods to handle this but he felt (& I agree) that SAL
>> should pass a flag that indicates whether the clocks are synchronized.
>> Then, the kernel can enable/disable the code that assumes clocks are
>> synchronized based on this flag - not a platform #ifdef.
>> 
>> Do you agree with this approach??
>> 
>> What do we need to do to get this flag added to the SAL spec.
>> 
>> My suggestion is (include/asm-ia64/sal.h):
>> 
>>        #define IA64_SAL_PLATFORM_FEATURE_BUS_LOCK              (1 << 0)
>>        #define IA64_SAL_PLATFORM_FEATURE_IRQ_REDIR_HINT        (1 << 1)
>>        #define IA64_SAL_PLATFORM_FEATURE_IPI_REDIR_HINT        (1 << 2)
>>        #define IA64_SAL_PLATFORM_FEATURE_ITC_UNSYNCHRONIZED    (1 << 3)        <<<< NEW
>>
>>        typedef struct ia64_sal_desc_platform_feature {
>>                u8 type;
>>                u8 feature_mask;
>>                u8 reserved1[14];
>>        } ia64_sal_desc_platform_feature_t;
>>

> 
> On Mon, 29 Oct 2001, David Mosberger wrote:
> 
> > The first thing to do is to get this into the firmware.  Linux needs a
> > way to detect when the ITCs are not driven off the same clock (i.e.,
> > when there is a chance for the ITCs to drift).  There also needs to be
> > a way for detecting when the clock frequencies are different, but I
> > believe the info provided by PAL/SAL is sufficient (my only worry are
> > rounding errors...).
> 
> Ok, we'll work on getting this pushed back, so at least the kernel can
> figure out what to do.  Regarding glibc though: we really don't want to
> have a different version just for our systems, so I'd like to come up with
> a generic solution.  Maybe an ia64 specific clock driver that glibc could
> map in to read synchronized clock info?  Is there a platform independent
> way of doing this?  I'll poke around glibc some more to see if any of the
> other arches deal with tis problem, but I'm sure there are people that are
> already familiar with these sorts of issues that might be able to help
> out.  Anyone?
> 
> Thanks a lot,
> Jesse
> 
> 
> _______________________________________________
> Linux-IA64 mailing list
> Linux-IA64@linuxia64.org
> http://lists.linuxia64.org/lists/listinfo/linux-ia64
> 


-- 
Thanks

Jack Steiner    (651-683-5302)   (vnet 233-5302)      steiner@sgi.com

Re: [Linux-ia64] itc sync & clock_*

[David Mosberger]

>>>>> On Mon, 29 Oct 2001 12:33:37 -0600 (CST), Jack Steiner <steiner@sgi.com> said:

  Jack> I sent mail to Asit last week about this.

Yes, I know of these discussions.  However, I don't think anything has
been settled yet.  Some people prefer to put this info in ACPI for
example.  Personally, I like the SAL approach, but the point is that
the firmware folks need to agree on something before we can start
using it.

BTW: I do not like the name "ITC_UNSYNCHRONIZED".  The issue is not
synchronization (we take care of that when booting the kernel).  The
issue is *drift* (and, to a secondary degree, ITC frequency).

	--david

RE: [Linux-ia64] itc sync & clock_*

[David Mosberger]

>>>>> On Mon, 29 Oct 2001 15:35:02 -0500, "WEI,DONG (HP-Roseville,ex1)" <dong_wei@am.exch.hp.com> said:

  Dong> Right now I need a description of the proposed flag to be
  Dong> included in the firmware architectural definition. The
  Dong> description should also include where (hardware register, or
  Dong> platform knowledge, etc.) firmware would get such information
  Dong> as to determine whether to set or clear the flag.

As far as I'm concerned, there definitely needs to be a flag that
indicates whether the ITCs are guaranteed to operate in lockstep
(i.e., with zero drift among them).  The flag would *not* indicate
that the ITC are necessarily synchronized when the OS is started, just
that if the OS synchronizes them, they'll be guaranteed to stay in
lockstep.

A secondary question is whether the flag should indicate whether the
ITCs all run at the same frequency.  At the moment, I don't believe
this is a good idea, because frequency and drift are separate issues.
Plus I *think* we should be able to handle the frequency question with
PAL_FREQ_RATIOS and SAL_FREQ_BASE.  But we need to verify this (the
issue is whether measurement problems might cause PAL report slightly
different frequencies for different ITCs, even when they're driven off
the same clock and are therefore really the same).

It would be good if someone else could take the lead on this because
otherwise (and I'm sorry if I sound like a broken record) it will have
to wait until the book is done (which will be another couple of weeks,
really... ;-).

	--david

Re: [Linux-ia64] itc sync & clock_*

[Jesse Barnes]

On Mon, 29 Oct 2001, David Mosberger wrote:

> 	- if there is a driver for a fine-grained machine timer, it can
> 	  use it

Right, and this should be some sort of generic interface that glibc can
count on, right?  I've been looking at the multimedia timer spec you
mentioned for ideas about how to implement it.
 
> Yup.  The driver interface needs to allow direct user-level access to
> the fine grained timer.  Though I'd say the interface should be able
> to accommodate hardware that can't map the timer into user space.

Ok, I'll keep that in mind.

Here's a patch against 2.4.13+ia64 to export the sal platform features to
a directory in /proc.  Please let me know if it's ok; I'll start working
on the clock driver if it is.

Thanks,
Jesse

diff -Naur --exclude=*~ --exclude=TAGS linux-2.4.13-ia64/arch/ia64/kernel/Makefile linux-2.4.13-ia64-salinfo/arch/ia64/kernel/Makefile
--- linux-2.4.13-ia64/arch/ia64/kernel/Makefile	Wed Oct 31 11:25:04 2001
+++ linux-2.4.13-ia64-salinfo/arch/ia64/kernel/Makefile	Wed Oct 31 11:27:31 2001
@@ -15,7 +15,7 @@
 
 obj-y := acpi.o entry.o gate.o efi.o efi_stub.o ia64_ksyms.o irq.o irq_ia64.o irq_lsapic.o ivt.o \
 	 machvec.o pal.o process.o perfmon.o ptrace.o sal.o semaphore.o setup.o	\
-	 signal.o sys_ia64.o traps.o time.o unaligned.o unwind.o
+	 signal.o sys_ia64.o traps.o time.o unaligned.o unwind.o salinfo.o
 obj-$(CONFIG_IA64_GENERIC) += iosapic.o
 obj-$(CONFIG_IA64_DIG) += iosapic.o
 obj-$(CONFIG_IA64_PALINFO) += palinfo.o
diff -Naur --exclude=*~ --exclude=TAGS linux-2.4.13-ia64/arch/ia64/kernel/sal.c linux-2.4.13-ia64-salinfo/arch/ia64/kernel/sal.c
--- linux-2.4.13-ia64/arch/ia64/kernel/sal.c	Wed Oct 31 11:25:04 2001
+++ linux-2.4.13-ia64-salinfo/arch/ia64/kernel/sal.c	Wed Oct 31 11:29:00 2001
@@ -19,6 +19,7 @@
 #include <asm/pal.h>
 
 spinlock_t sal_lock = SPIN_LOCK_UNLOCKED;
+unsigned long sal_platform_features = 0;
 
 static struct {
 	void *addr;	/* function entry point */
@@ -154,11 +155,13 @@
 			      struct ia64_sal_desc_platform_feature *pf = (void *) p;
 			      printk("SAL: Platform features ");
 
-			      if (pf->feature_mask & (1 << 0))
+			      if (pf->feature_mask & IA64_SAL_PLATFORM_FEATURE_BUS_LOCK) {
 				      printk("BusLock ");
-
-			      if (pf->feature_mask & (1 << 1)) {
+      				      sal_platform_features |= IA64_SAL_PLATFORM_FEATURE_BUS_LOCK;
+			      }
+			      if (pf->feature_mask & IA64_SAL_PLATFORM_FEATURE_IRQ_REDIR_HINT) {
 				      printk("IRQ_Redirection ");
+				      sal_platform_features |= IA64_SAL_PLATFORM_FEATURE_IRQ_REDIR_HINT;
 #ifdef CONFIG_SMP
 				      if (no_int_routing)
 					      smp_int_redirect &= ~SMP_IRQ_REDIRECTION;
@@ -166,14 +169,19 @@
 					      smp_int_redirect |= SMP_IRQ_REDIRECTION;
 #endif
 			      }
-			      if (pf->feature_mask & (1 << 2)) {
+			      if (pf->feature_mask & IA64_SAL_PLATFORM_FEATURE_IPI_REDIR_HINT) {
 				      printk("IPI_Redirection ");
+				      sal_platform_features |= IA64_SAL_PLATFORM_FEATURE_IPI_REDIR_HINT;
 #ifdef CONFIG_SMP
 				      if (no_int_routing) 
 					      smp_int_redirect &= ~SMP_IPI_REDIRECTION;
 				      else
 					      smp_int_redirect |= SMP_IPI_REDIRECTION;
 #endif
+			      }
+			      if (pf->feature_mask & IA64_SAL_PLATFORM_FEATURE_ITC_MAY_DRIFT) {
+				      printk("ITC_may_drift ");
+				      sal_platform_features |= IA64_SAL_PLATFORM_FEATURE_ITC_MAY_DRIFT;
 			      }
 			      printk("\n");
 			      break;
diff -Naur --exclude=*~ --exclude=TAGS linux-2.4.13-ia64/arch/ia64/kernel/salinfo.c linux-2.4.13-ia64-salinfo/arch/ia64/kernel/salinfo.c
--- linux-2.4.13-ia64/arch/ia64/kernel/salinfo.c	Wed Dec 31 16:00:00 1969
+++ linux-2.4.13-ia64-salinfo/arch/ia64/kernel/salinfo.c	Wed Oct 31 11:36:35 2001
@@ -0,0 +1,104 @@
+/*
+ * salinfo.c
+ *
+ * Creates entries in /proc/sal for various system features.
+ *
+ * Copyright (c) 2001 Silicon Graphics, Inc.  All rights reserved.
+ *
+ * 10/30/2001	jbarnes@sgi.com		copied much of Stephane's palinfo
+ *					code to create this file
+ */
+
+#include <linux/types.h>
+#include <linux/proc_fs.h>
+#include <linux/module.h>
+
+#include <asm/sal.h>
+
+MODULE_AUTHOR("Jesse Barnes <jbarnes@sgi.com>");
+MODULE_DESCRIPTION("/proc interface to IA-64 SAL features");
+
+int salinfo_read(char *page, char **start, off_t off, int count, int *eof, void *data);
+
+typedef struct {
+	const char		*name;		/* name of the proc entry */
+	unsigned long           feature;        /* feature bit */
+	struct proc_dir_entry	*entry;		/* registered entry (removal) */
+} salinfo_entry_t;
+
+/*
+ * List {name,feature} pairs for every entry in /proc/sal/<feature>
+ * that this module exports
+ */
+static salinfo_entry_t salinfo_entries[]={
+	{ "bus_lock",           IA64_SAL_PLATFORM_FEATURE_BUS_LOCK, },
+	{ "irq_redirection",	IA64_SAL_PLATFORM_FEATURE_IRQ_REDIR_HINT, },
+	{ "ipi_redirection",	IA64_SAL_PLATFORM_FEATURE_IPI_REDIR_HINT, },
+	{ "itc_may_drift",	IA64_SAL_PLATFORM_FEATURE_ITC_MAY_DRIFT, },
+};
+
+#define NR_SALINFO_ENTRIES (sizeof(salinfo_entries)/sizeof(salinfo_entry_t))
+
+/*
+ * One for each feature and one more for the directory entry...
+ */
+static struct proc_dir_entry *salinfo_proc_entries[NR_SALINFO_ENTRIES + 1];
+
+static int __init
+salinfo_init(void)
+{
+	struct proc_dir_entry *salinfo_dir; /* /proc/sal dir entry */
+	struct proc_dir_entry **sdir = salinfo_proc_entries; /* keeps track of every entry */
+	int i;
+
+	salinfo_dir = proc_mkdir("sal", NULL);
+	
+	for (i=0; i < NR_SALINFO_ENTRIES; i++) {
+		/* pass the feature bit in question as misc data */
+		*sdir++ = create_proc_read_entry (salinfo_entries[i].name, 0, salinfo_dir,
+						  salinfo_read, (void *)salinfo_entries[i].feature);
+	}
+	*sdir++ = salinfo_dir;
+
+	return 0;
+}
+
+static void __exit
+salinfo_exit(void)
+{
+	int i = 0;
+
+	for (i = 0; i < NR_SALINFO_ENTRIES ; i++) {
+		if (salinfo_proc_entries[i])
+			remove_proc_entry (salinfo_proc_entries[i]->name, NULL);
+	}
+}
+
+/*
+ * 'data' contains an integer that corresponds to the feature we're
+ * testing
+ */
+int
+salinfo_read(char *page, char **start, off_t off, int count, int *eof, void *data)
+{
+	int len = 0;
+
+	MOD_INC_USE_COUNT;
+
+	len = sprintf(page, (sal_platform_features & (unsigned long)data) ? "1" : "0");
+
+	if (len <= off+count) *eof = 1;
+
+	*start = page + off;
+	len   -= off;
+
+	if (len>count) len = count;
+	if (len<0) len = 0;
+
+	MOD_DEC_USE_COUNT;
+
+	return len;
+}
+
+module_init(salinfo_init);
+module_exit(salinfo_exit);
diff -Naur --exclude=*~ --exclude=TAGS linux-2.4.13-ia64/include/asm-ia64/sal.h linux-2.4.13-ia64-salinfo/include/asm-ia64/sal.h
--- linux-2.4.13-ia64/include/asm-ia64/sal.h	Wed Oct 31 11:25:05 2001
+++ linux-2.4.13-ia64-salinfo/include/asm-ia64/sal.h	Wed Oct 31 11:32:32 2001
@@ -149,6 +149,7 @@
 #define IA64_SAL_PLATFORM_FEATURE_BUS_LOCK		(1 << 0)
 #define IA64_SAL_PLATFORM_FEATURE_IRQ_REDIR_HINT	(1 << 1)
 #define IA64_SAL_PLATFORM_FEATURE_IPI_REDIR_HINT	(1 << 2)
+#define IA64_SAL_PLATFORM_FEATURE_ITC_MAY_DRIFT 	(1 << 3)
 
 typedef struct ia64_sal_desc_platform_feature {
 	u8 type;
@@ -775,5 +776,7 @@
 		*scratch_buf_size_needed = isrv.v1;
 	return isrv.status;
 }
+
+extern unsigned long sal_platform_features;
 
 #endif /* _ASM_IA64_PAL_H */

Re: [Linux-ia64] itc sync & clock_*

[David Mosberger]

>>>>> On Wed, 31 Oct 2001 11:41:58 -0800, Jesse Barnes <jbarnes@sgi.com> said:

  Jesse> Here's a patch against 2.4.13+ia64 to export the sal platform
  Jesse> features to a directory in /proc.  Please let me know if it's
  Jesse> ok; I'll start working on the clock driver if it is.

Looks great to me!  I'll hold off with applying the patch until the
firmware interface has been finalized, if that's all right with you.

Thanks,

	--david

Re: [Linux-ia64] itc sync & clock_*

[Jesse Barnes]

Yeah, sounds good.  I'm not sure about the name too, hopefully the
firmware folks will come up with something better.  ITC_MAY_DRIFT seems a
little clunky...

Jesse

On Mon, 5 Nov 2001, David Mosberger wrote:

> >>>>> On Wed, 31 Oct 2001 11:41:58 -0800, Jesse Barnes <jbarnes@sgi.com> said:
> 
>   Jesse> Here's a patch against 2.4.13+ia64 to export the sal platform
>   Jesse> features to a directory in /proc.  Please let me know if it's
>   Jesse> ok; I'll start working on the clock driver if it is.
> 
> Looks great to me!  I'll hold off with applying the patch until the
> firmware interface has been finalized, if that's all right with you.
> 
> Thanks,
> 
> 	--david
> 

RE: [Linux-ia64] itc sync & clock_*

[Mallick, Asit K]

David/Jesse,

We have got an agreement with the Intel SAL team on defining the bit 3 of
the SAL platform features as the following:

	0 - no ITC drift
	1 - ITC may drift

We will get this definition reviewed by other SAL users/members and do not
expect any problem with the review. The definition should not affect most of
the FWs since this bit is already set to 0 and ITCs do not drift. The
specification will be updated and currently it is planned for Q1 next year.

Thanks,
Asit


> -----Original Message-----
> From: Jesse Barnes [mailto:jbarnes@sgi.com]
> Sent: Monday, November 05, 2001 5:03 PM
> To: David Mosberger
> Cc: linux-ia64@linuxia64.org
> Subject: Re: [Linux-ia64] itc sync & clock_*
> 
> 
> Yeah, sounds good.  I'm not sure about the name too, hopefully the
> firmware folks will come up with something better.  
> ITC_MAY_DRIFT seems a
> little clunky...
> 
> Jesse
> 
> On Mon, 5 Nov 2001, David Mosberger wrote:
> 
> > >>>>> On Wed, 31 Oct 2001 11:41:58 -0800, Jesse Barnes 
> <jbarnes@sgi.com> said:
> > 
> >   Jesse> Here's a patch against 2.4.13+ia64 to export the 
> sal platform
> >   Jesse> features to a directory in /proc.  Please let me 
> know if it's
> >   Jesse> ok; I'll start working on the clock driver if it is.
> > 
> > Looks great to me!  I'll hold off with applying the patch until the
> > firmware interface has been finalized, if that's all right with you.
> > 
> > Thanks,
> > 
> > 	--david
> > 
> 
> 
> _______________________________________________
> Linux-IA64 mailing list
> Linux-IA64@linuxia64.org
> http://lists.linuxia64.org/lists/listinfo/linux-ia64
> 

RE: [Linux-ia64] itc sync & clock_*

[David Mosberger]

>>>>> On Thu, 8 Nov 2001 17:31:47 -0800, "Mallick, Asit K" <asit.k.mallick@intel.com> said:

  Asit> David/Jesse, We have got an agreement with the Intel SAL team
  Asit> on defining the bit 3 of the SAL platform features as the
  Asit> following:

  Asit> 	0 - no ITC drift 1 - ITC may drift

Very good.  The timing is excellent.

Thanks,

	--david