Initialization of cr.dcr

Initialization of cr.dcr

[Christian Hildner]

Hi all,

playing around with speculation I found that on initialization dcr.dm is 
not set, while the comment says "Initialize default control register to 
defer all speculative faults". To be conform to the comment (and also to 
the expected behavior) the value IA64_DCR_DM should be added in 
arch/ia64/kernel/setup.c.

Christian

RE: Initialization of cr.dcr

[Chen, Kenneth W]

Christian Hildner wrote on Friday, June 03, 2005 2:02 AM
> playing around with speculation I found that on initialization dcr.dm is 
> not set, while the comment says "Initialize default control register to 
> defer all speculative faults". To be conform to the comment (and also to 
> the expected behavior) the value IA64_DCR_DM should be added in 
> arch/ia64/kernel/setup.c.

It should be the other way around: update the comments to reflect what
the code does.  Turning off dcr.dm is a big win for speculative load
where you do want the tlb miss to be serviced up front.

- Ken

Re: Initialization of cr.dcr

[Russ Anderson]

Ken Chen wrote:
> 
> Christian Hildner wrote on Friday, June 03, 2005 2:02 AM
> > playing around with speculation I found that on initialization dcr.dm is 
> > not set, while the comment says "Initialize default control register to 
> > defer all speculative faults". To be conform to the comment (and also to 
> > the expected behavior) the value IA64_DCR_DM should be added in 
> > arch/ia64/kernel/setup.c.
> 
> It should be the other way around: update the comments to reflect what
> the code does.  Turning off dcr.dm is a big win for speculative load
> where you do want the tlb miss to be serviced up front.

  Virtual Memory in the IA-64 Linux Kernel
    * By Stephane Eranian, David Mosberger.

  A related problem arises from speculative loads in the kernel. If 
  TLB misses are not deferred (dcr.dm is 0), a speculative load inside
  the kernel may cause a TLB miss to an arbitrary address. If that
  address happens to fall inside region 6 or 7, the speculative load
  would trigger an alternate TLB fault. This again poses the risk of
  inserting a translation with conflicting memory attributes. To
  prevent this, the alternate DTLB miss handler also checks whether the
  faulting access was caused by a speculative load and, if so, turns on
  the exception deferral bit (ed in psr) instead of installing a
  translation. The net effect of this method is that all speculative
  loads to region 6 and 7 produce a NaT value, unless the translation
  for the page being accessed happens to be in the TLB already. This
  solution may sometimes produce a NaT unnecessarily, but apart from a
  small performance impact, does not affect the correct operation of
  the kernel. This solution also has the advantage that speculative
  loads cannot pollute the TLB with unnecessary translations.

http://www.informit.com/articles/article.asp?p)961&seqNum=5&rl=1

-- 
Russ Anderson, OS RAS/Partitioning Project Lead  
SGI - Silicon Graphics Inc          rja@sgi.com

RE: Initialization of cr.dcr

[Chen, Kenneth W]

Russ Anderson wrote on Friday, June 03, 2005 11:28 AM
> Ken Chen wrote:
> > 
> > Christian Hildner wrote on Friday, June 03, 2005 2:02 AM
> > > playing around with speculation I found that on initialization dcr.dm is 
> > > not set, while the comment says "Initialize default control register to 
> > > defer all speculative faults". To be conform to the comment (and also to 
> > > the expected behavior) the value IA64_DCR_DM should be added in 
> > > arch/ia64/kernel/setup.c.
> > 
> > It should be the other way around: update the comments to reflect what
> > the code does.  Turning off dcr.dm is a big win for speculative load
> > where you do want the tlb miss to be serviced up front.
> 
>   Virtual Memory in the IA-64 Linux Kernel
>     * By Stephane Eranian, David Mosberger.
> 
>   A related problem arises from speculative loads in the kernel. If 
>   TLB misses are not deferred (dcr.dm is 0), a speculative load inside
>   the kernel may cause a TLB miss to an arbitrary address. If that
>   address happens to fall inside region 6 or 7, the speculative load
>   would trigger an alternate TLB fault. This again poses the risk of
>   inserting a translation with conflicting memory attributes. To
>   prevent this, the alternate DTLB miss handler also checks whether the
>   faulting access was caused by a speculative load and, if so, turns on
>   the exception deferral bit (ed in psr) instead of installing a
>   translation. The net effect of this method is that all speculative
>   loads to region 6 and 7 produce a NaT value, unless the translation
>   for the page being accessed happens to be in the TLB already. This
>   solution may sometimes produce a NaT unnecessarily, but apart from a
>   small performance impact, does not affect the correct operation of
>   the kernel. This solution also has the advantage that speculative
>   loads cannot pollute the TLB with unnecessary translations.

What this paragraph described is already implemented in the kernel. I'm
not sure what is the concern here.

Re: Initialization of cr.dcr

[David Mosberger]

>>>>> On Fri, 3 Jun 2005 13:27:54 -0500 (CDT), Russ Anderson <rja@sgi.com> said:

  Russ> Ken Chen wrote:
  >>  Christian Hildner wrote on Friday, June 03, 2005 2:02 AM >
  >> playing around with speculation I found that on initialization
  >> dcr.dm is > not set, while the comment says "Initialize default
  >> control register to > defer all speculative faults". To be
  >> conform to the comment (and also to > the expected behavior) the
  >> value IA64_DCR_DM should be added in > arch/ia64/kernel/setup.c.

  >> It should be the other way around: update the comments to reflect
  >> what the code does.  Turning off dcr.dm is a big win for
  >> speculative load where you do want the tlb miss to be serviced up
  >> front.

That's correct.  We changed this a long time ago and apparently forgot
to update the comment.

  Russ>   Virtual Memory in the IA-64 Linux Kernel * By Stephane
  Russ> Eranian, David Mosberger.

  Russ>   A related problem arises from speculative loads in the
  Russ> kernel. If TLB misses are not deferred (dcr.dm is 0), a
  Russ> speculative load inside the kernel may cause a TLB miss to an
  Russ> arbitrary address. If that address happens to fall inside
  Russ> region 6 or 7, the speculative load would trigger an alternate
  Russ> TLB fault. This again poses the risk of inserting a
  Russ> translation with conflicting memory attributes. To prevent
  Russ> this, the alternate DTLB miss handler also checks whether the
  Russ> faulting access was caused by a speculative load and, if so,
  Russ> turns on the exception deferral bit (ed in psr) instead of
  Russ> installing a translation. The net effect of this method is
  Russ> that all speculative loads to region 6 and 7 produce a NaT
  Russ> value, unless the translation for the page being accessed
  Russ> happens to be in the TLB already. This solution may sometimes
  Russ> produce a NaT unnecessarily, but apart from a small
  Russ> performance impact, does not affect the correct operation of
  Russ> the kernel. This solution also has the advantage that
  Russ> speculative loads cannot pollute the TLB with unnecessary
  Russ> translations.

  Russ> http://www.informit.com/articles/article.asp?p)961&seqNum=5&rl=1

And your point is?

	--david

RE: Initialization of cr.dcr

[Chen, Kenneth W]

David Mosberger wrote on Friday, June 03, 2005 12:11 PM
> >>> On Fri, 3 Jun 2005 13:27:54 -0500 (CDT), Russ Anderson <rja@sgi.com> said:
> 
> Russ> Ken Chen wrote:
> >>  Christian Hildner wrote on Friday, June 03, 2005 2:02 AM >
> >> playing around with speculation I found that on initialization
> >> dcr.dm is > not set, while the comment says "Initialize default
> >> control register to > defer all speculative faults". To be
> >> conform to the comment (and also to > the expected behavior) the
> >> value IA64_DCR_DM should be added in > arch/ia64/kernel/setup.c.
> 
> >> It should be the other way around: update the comments to reflect
> >> what the code does.  Turning off dcr.dm is a big win for
> >> speculative load where you do want the tlb miss to be serviced up
> >> front.
> 
> That's correct.  We changed this a long time ago and apparently forgot
> to update the comment.

Patch to update the comments to reflect the actual code in cpu_init().

Signed-off-by: Ken Chen <kenneth.w.chen@intel.com>
Acked-by: David Mosberger-Tang <davidm@hpl.hp.com>

--- linux-2.6.11/arch/ia64/kernel/setup.c.orig	2005-06-03 12:21:00.891940166 -0700
+++ linux-2.6.11/arch/ia64/kernel/setup.c	2005-06-03 12:22:36.036470251 -0700
@@ -720,7 +720,7 @@ cpu_init (void)
 	ia64_set_kr(IA64_KR_PT_BASE, __pa(ia64_imva(empty_zero_page)));
 
 	/*
-	 * Initialize default control register to defer all speculative faults.  The
+	 * Initialize default control register not to defer speculative faults.  The
 	 * kernel MUST NOT depend on a particular setting of these bits (in other words,
 	 * the kernel must have recovery code for all speculative accesses).  Turn on
 	 * dcr.lc as per recommendation by the architecture team.  Most IA-32 apps

Re: Initialization of cr.dcr

[Christian Hildner]

Chen, Kenneth W schrieb:

>Patch to update the comments to reflect the actual code in cpu_init().
>
However the actual code distincts between TLB miss and other faults. The 
other faults are deferred.

Christian

>
>Signed-off-by: Ken Chen <kenneth.w.chen@intel.com>
>Acked-by: David Mosberger-Tang <davidm@hpl.hp.com>
>
>--- linux-2.6.11/arch/ia64/kernel/setup.c.orig	2005-06-03 12:21:00.891940166 -0700
>+++ linux-2.6.11/arch/ia64/kernel/setup.c	2005-06-03 12:22:36.036470251 -0700
>@@ -720,7 +720,7 @@ cpu_init (void)
> 	ia64_set_kr(IA64_KR_PT_BASE, __pa(ia64_imva(empty_zero_page)));
> 
> 	/*
>-	 * Initialize default control register to defer all speculative faults.  The
>+	 * Initialize default control register not to defer speculative faults.  The
> 	 * kernel MUST NOT depend on a particular setting of these bits (in other words,
> 	 * the kernel must have recovery code for all speculative accesses).  Turn on
> 	 * dcr.lc as per recommendation by the architecture team.  Most IA-32 apps
>
>  
>