[PATCH 1/7] ICH Force HPET: Make generic time capable of switching broadcast timer

[PATCH 1/7] ICH Force HPET: Make generic time capable of switching broadcast timer

[Venki Pallipadi]

Auto-detect the presence of HPET on ICH5 or newer platforms and enable
HPET for broadcast timer. This gives a bigger upperlimit for tickless time
tick and improves the power consumption in comparison to PIT as broadcast timer.

This patch:

Change the broadcast timer, if a timer with higher rating becomes available.

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>

---

Applies over linux-2.6.22-rc4-mm2 +
tglx's  patch-2.6.22-rc4-mm2-hrt4 patch

The patchset had been baking for a while along with patch-2.6.22-rc*-hrt* for
a while without breaking anything and redusing the number of
timer interrupts with tickless on various platforms.

 kernel/time/tick-broadcast.c |   13 ++++++-------
 kernel/time/tick-common.c    |    4 ++--
 2 files changed, 8 insertions(+), 9 deletions(-)

Index: linux-2.6.22-rc5/kernel/time/tick-common.c
===================================================================
--- linux-2.6.22-rc5.orig/kernel/time/tick-common.c	2007-06-17 08:52:07.000000000 +0200
+++ linux-2.6.22-rc5/kernel/time/tick-common.c	2007-06-17 08:52:10.000000000 +0200
@@ -200,7 +200,7 @@ static int tick_check_new_device(struct 
 
 	cpu = smp_processor_id();
 	if (!cpu_isset(cpu, newdev->cpumask))
-		goto out;
+		goto out_bc;
 
 	td = &per_cpu(tick_cpu_device, cpu);
 	curdev = td->evtdev;
@@ -265,7 +265,7 @@ out_bc:
 	 */
 	if (tick_check_broadcast_device(newdev))
 		ret = NOTIFY_STOP;
-out:
+
 	spin_unlock_irqrestore(&tick_device_lock, flags);
 
 	return ret;
Index: linux-2.6.22-rc5/kernel/time/tick-broadcast.c
===================================================================
--- linux-2.6.22-rc5.orig/kernel/time/tick-broadcast.c	2007-06-17 08:52:07.000000000 +0200
+++ linux-2.6.22-rc5/kernel/time/tick-broadcast.c	2007-06-17 08:52:10.000000000 +0200
@@ -64,8 +64,9 @@ static void tick_broadcast_start_periodi
  */
 int tick_check_broadcast_device(struct clock_event_device *dev)
 {
-	if (tick_broadcast_device.evtdev ||
-	    (dev->features & CLOCK_EVT_FEAT_C3STOP))
+	if ((tick_broadcast_device.evtdev &&
+	     tick_broadcast_device.evtdev->rating >= dev->rating) ||
+	     (dev->features & CLOCK_EVT_FEAT_C3STOP))
 		return 0;
 
 	clockevents_exchange_device(NULL, dev);
@@ -519,11 +520,9 @@ static void tick_broadcast_clear_oneshot
  */
 void tick_broadcast_setup_oneshot(struct clock_event_device *bc)
 {
-	if (bc->mode != CLOCK_EVT_MODE_ONESHOT) {
-		bc->event_handler = tick_handle_oneshot_broadcast;
-		clockevents_set_mode(bc, CLOCK_EVT_MODE_ONESHOT);
-		bc->next_event.tv64 = KTIME_MAX;
-	}
+	bc->event_handler = tick_handle_oneshot_broadcast;
+	clockevents_set_mode(bc, CLOCK_EVT_MODE_ONESHOT);
+	bc->next_event.tv64 = KTIME_MAX;
 }
 
 /*

Re: [PATCH 1/7] ICH Force HPET: Make generic time capable of switching broadcast timer

[Andrew Morton]

> On Fri, 22 Jun 2007 13:38:22 -0700 Venki Pallipadi <venkatesh.pallipadi@intel.com> wrote:
> Auto-detect the presence of HPET on ICH5 or newer platforms and enable
> HPET for broadcast timer. This gives a bigger upperlimit for tickless time
> tick and improves the power consumption in comparison to PIT as broadcast timer.
> 
> This patch:
> 
> Change the broadcast timer, if a timer with higher rating becomes available.
> 

so...  is this the mysterious patchset which will get NOHZ working
on my little Vaio?

I _think_ what's going on here is that your code will go and poke the
hardware to enable the hpet even f the BIOS decided to hide its presence. 
Is that correct?  If so, perhaps the changelog should mention this
explicitly.

> Applies over linux-2.6.22-rc4-mm2 +
> tglx's  patch-2.6.22-rc4-mm2-hrt4 patch

Oh.  Well that tears that then.

Thomas, can I assume that you'll send all this stuff back at me?

Re: [PATCH 1/7] ICH Force HPET: Make generic time capable of switching broadcast timer

[Thomas Gleixner]

On Sat, 2007-06-23 at 09:52 -0700, Andrew Morton wrote:
> I _think_ what's going on here is that your code will go and poke the
> hardware to enable the hpet even f the BIOS decided to hide its presence. 
> Is that correct?  If so, perhaps the changelog should mention this
> explicitly.
> 
> > Applies over linux-2.6.22-rc4-mm2 +
> > tglx's  patch-2.6.22-rc4-mm2-hrt4 patch
> 
> Oh.  Well that tears that then.
> 
> Thomas, can I assume that you'll send all this stuff back at me?

I sent out a V3 queue today, but the aggregate of my queue and Venki's
HPET stuff + HPET force enable for non Intel chip sets is available as a
full queue here:

http://www.tglx.de/projects/hrtimers/2.6.22-rc4-mm2/patch-2.6.22-rc4-mm2-hrt6.patches.tar.bz2

I can resend the 40 patches if you want, but they are out on LKML
already (-hrt and the hpet ones), so there is little value to spam
everyones inbox with the same stuff again.

	tglx

Re: [PATCH 1/7] ICH Force HPET: Make generic time capable of switching broadcast timer

[Andrew Morton]

On Fri, 22 Jun 2007 13:38:22 -0700 Venki Pallipadi <venkatesh.pallipadi@intel.com> wrote:

> Auto-detect the presence of HPET on ICH5 or newer platforms and enable
> HPET for broadcast timer.

Hey.  This works.  The Vaio-of-doom is running tickless for the first time.

I can't start the X server any more, but that's probably unrelated ;)

<goes back to yet another bisection search.  sigh.  you guys have
me hopelessly outnumbered>

Re: [PATCH 1/7] ICH Force HPET: Make generic time capable of switching broadcast timer

[Andrew Morton]

On Thu, 28 Jun 2007 00:12:21 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:

> I can't start the X server any more, but that's probably unrelated ;)
> 
> <goes back to yet another bisection search.  sigh.  you guys have
> me hopelessly outnumbered>

And the winner of the u-o-akpm-1-hour-of-his-life-back award (and the bonus
u-delayed-next-mm-by-a-day award!) is ....  git-selinux.patch!

(II) Loading /usr/lib/xorg/modules/libvgahw.so
(II) Module vgahw: vendor="X.Org Foundation"
        compiled for 7.0.0, module version = 0.1.0
        ABI class: X.Org Video Driver, version 0.8
(**) I810(0): Depth 24, (--) framebuffer bpp 32
(==) I810(0): RGB weight 888
(==) I810(0): Default visual is TrueColor
(II) Loading sub module "int10"
(II) LoadModule: "int10"
(II) Reloading /usr/lib/xorg/modules/libint10.so
(II) I810(0): initializing int10
(EE) I810(0): Cannot shmat() low memory
(EE) I810(0): shmat(low_mem) error: Invalid argument
(EE) I810(0): VBE initialization failed.

Due to

commit fe276418917d00e6f7ecab2efa8331dd86db2f3f
Author: Eric Paris <eparis@redhat.com>
Date:   Wed Jun 6 02:30:33 2007 -0400

    security: Protection for exploiting null dereference using mmap
    
    Add a new security check on mmap operations to see if the user is attempting
    to mmap to low area of the address space.  The amount of space protected is
    indicated by the new proc tunable /proc/sys/kernel/mmap_min_addr and
    defaults to 0, preserving existing behavior.
    
    This patch uses a new SELinux security class "memprotect."  Policy already
    contains a number of allow rules like a_t self:process * (unconfined_t being
    one of them) which mean that putting this check in the process class (its
    best current fit) would make it useless as all user processes, which we also
    want to protect against, would be allowed. By taking the memprotect name of
    the new class it will also make it possible for us to move some of the other
    memory protect permissions out of 'process' and into the new class next time
    we bump the policy version number (which I also think is a good future idea)
    
    Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
    Signed-off-by: Eric Paris <eparis@redhat.com>
    Signed-off-by: James Morris <jmorris@namei.org>

and yes, setting /proc/sys/kernel/mmap_min_addr to zero does fix it. 
(Shouldn't this be in /proc/sys/vm?)

My fix:

--- a/security/selinux/hooks.c~git-selinux-disable-mmap_min_addr-by-default
+++ a/security/selinux/hooks.c
@@ -4917,15 +4917,6 @@ static __init int selinux_init(void)
 					    sizeof(struct inode_security_struct),
 					    0, SLAB_PANIC, NULL, NULL);
 
-	/*
-	 * Tasks cannot mmap below this without the mmap_zero permission.
-	 * If not enabled already, do so by setting it to 64KB.
-	 */
-	if (mmap_min_addr == 0) {
-		enabled_mmap_min_addr = 1;
-		mmap_min_addr = 65536;
-	}
-
 	avc_init();
 
 	original_ops = secondary_ops = security_ops;
_

although perhaps we should permit suitably-privileged apps (ie: X) to
override this instead.

Re: [PATCH 1/7] ICH Force HPET: Make generic time capable of switching broadcast timer

[James Morris]

On Thu, 28 Jun 2007, Andrew Morton wrote:

Sorry, we should have discovered this before you.


> (Shouldn't this be in /proc/sys/vm?)

Probably.

> My fix:

This should work.

(We have some code to allow things like this to be added without needing 
new policy, but it's too late for this case).




-- 
James Morris
<jmorris@namei.org>

Re: [PATCH 1/7] ICH Force HPET: Make generic time capable of switching broadcast timer

[Eric Paris]

On Thu, 2007-06-28 at 01:27 -0700, Andrew Morton wrote:
> On Thu, 28 Jun 2007 00:12:21 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > I can't start the X server any more, but that's probably unrelated ;)
> > 
> > <goes back to yet another bisection search.  sigh.  you guys have
> > me hopelessly outnumbered>
> 
> And the winner of the u-o-akpm-1-hour-of-his-life-back award (and the bonus
> u-delayed-next-mm-by-a-day award!) is ....  git-selinux.patch!
> 
> (II) Loading /usr/lib/xorg/modules/libvgahw.so
> (II) Module vgahw: vendor="X.Org Foundation"
>         compiled for 7.0.0, module version = 0.1.0
>         ABI class: X.Org Video Driver, version 0.8
> (**) I810(0): Depth 24, (--) framebuffer bpp 32
> (==) I810(0): RGB weight 888
> (==) I810(0): Default visual is TrueColor
> (II) Loading sub module "int10"
> (II) LoadModule: "int10"
> (II) Reloading /usr/lib/xorg/modules/libint10.so
> (II) I810(0): initializing int10
> (EE) I810(0): Cannot shmat() low memory
> (EE) I810(0): shmat(low_mem) error: Invalid argument
> (EE) I810(0): VBE initialization failed.

It has been found and the policy people already know about it.  I'm not
sure what the upstream status of the fixed policy is should appear soon.
(I also talked with the X people and apparently this is unfixable on
i686.  ajax posted about this on the original thread)

There is another bug however which I have a patch but haven't tested
much, I'll try to get it out today.  If you use a hint > 0 but <
mmap_min_addr without map fixed you will likely be denied.  Simple fix,
change hints up to mmap_min_addr if they are too low.

-Eric

[PATCH 1/2] selinux: don't enable minimum mmap checking by default

[James Morris]

Don't enable minimum mmap checking by default in SELinux, as it may break
existing applications which do not have updated policy.

We will be able to enable it by default later, once we have code to handle
new permissions which are not present in the user's policy.

Signed-off-by: James Morris <jmorris@namei.org>
---

This is an updated version of the patch, essentially a reverting an 
earlier patch which I'll drop once this are settles down.


 security/selinux/hooks.c |   16 ----------------
 1 files changed, 0 insertions(+), 16 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 4b0ecfd..8575555 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -112,9 +112,6 @@ int selinux_enabled = 1;
 /* Original (dummy) security module. */
 static struct security_operations *original_ops = NULL;
 
-/* Did we enable minimum mmap address checking? */
-static int enabled_mmap_min_addr;
-
 /* Minimal support for a secondary security module,
    just to allow the use of the dummy or capability modules.
    The owlsm module can alternatively be used as a secondary
@@ -4917,15 +4914,6 @@ static __init int selinux_init(void)
 					    sizeof(struct inode_security_struct),
 					    0, SLAB_PANIC, NULL, NULL);
 
-	/*
-	 * Tasks cannot mmap below this without the mmap_zero permission.
-	 * If not enabled already, do so by setting it to 64KB.
-	 */
-	if (mmap_min_addr == 0) {
-		enabled_mmap_min_addr = 1;
-		mmap_min_addr = 65536;
-	}
-
 	avc_init();
 
 	original_ops = secondary_ops = security_ops;
@@ -5076,10 +5064,6 @@ int selinux_disable(void)
 	selinux_disabled = 1;
 	selinux_enabled = 0;
 
-	/* Disable minimum mmap address check only if we enabled it */
-	if (enabled_mmap_min_addr)
-		mmap_min_addr = 0;
-
 	/* Reset security_ops to the secondary module, dummy or capability. */
 	security_ops = secondary_ops;
 
-- 
1.5.0.6

[PATCH 2/2] security: move mmap_min_addr sysctl to /proc/sys/vm

[James Morris]

Move mmap_min_addr sysctl to /proc/sys/vm, as suggested by Andrew Morton.

Signed-off-by: James Morris <jmorris@namei.org>
---
 Documentation/sysctl/kernel.txt |   14 --------------
 Documentation/sysctl/vm.txt     |   15 +++++++++++++++
 kernel/sysctl.c                 |   20 ++++++++++----------
 3 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index 1a9dc8a..111fd28 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -29,7 +29,6 @@ show up in /proc/sys/kernel:
 - java-interpreter            [ binfmt_java, obsolete ]
 - kstack_depth_to_print       [ X86 only ]
 - l2cr                        [ PPC only ]
-- mmap_min_addr
 - modprobe                    ==> Documentation/kmod.txt
 - msgmax
 - msgmnb
@@ -179,19 +178,6 @@ kernel stack.
 
 ==============================================================
 
-mmap_min_addr
-
-This file indicates the amount of address space  which a user process will be
-restricted from mmaping.  Since kernel null dereference bugs could
-accidentally operate based on the information in the first couple of pages of
-memory userspace processes should not be allowed to write to them.  By default
-this value is set to 0 and no protections will be enforced by the security
-module.  Setting this value to something like 64k will allow the vast majority
-of applications to work correctly and provide defense in depth against future
-potential kernel bugs.
-
-==============================================================
-
 osrelease, ostype & version:
 
 # cat osrelease
diff --git a/Documentation/sysctl/vm.txt b/Documentation/sysctl/vm.txt
index 1d19256..8cfca17 100644
--- a/Documentation/sysctl/vm.txt
+++ b/Documentation/sysctl/vm.txt
@@ -31,6 +31,7 @@ Currently, these files are in /proc/sys/vm:
 - min_unmapped_ratio
 - min_slab_ratio
 - panic_on_oom
+- mmap_min_address
 
 ==============================================================
 
@@ -216,3 +217,17 @@ above-mentioned.
 The default value is 0.
 1 and 2 are for failover of clustering. Please select either
 according to your policy of failover.
+
+==============================================================
+
+mmap_min_addr
+
+This file indicates the amount of address space  which a user process will
+be restricted from mmaping.  Since kernel null dereference bugs could
+accidentally operate based on the information in the first couple of pages
+of memory userspace processes should not be allowed to write to them.  By
+default this value is set to 0 and no protections will be enforced by the
+security module.  Setting this value to something like 64k will allow the
+vast majority of applications to work correctly and provide defense in depth
+against future potential kernel bugs.
+
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 8f9a737..44f0a50 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -615,16 +615,6 @@ static ctl_table kern_table[] = {
 		.proc_handler   = &proc_dointvec,
 	},
 #endif
-#ifdef CONFIG_SECURITY
-	{
-		.ctl_name	= CTL_UNNUMBERED,
-		.procname	= "mmap_min_addr",
-		.data		= &mmap_min_addr,
-		.maxlen         = sizeof(unsigned long),
-		.mode		= 0644,
-		.proc_handler	= &proc_doulongvec_minmax,
-	},
-#endif
 	{ .ctl_name = 0 }
 };
 
@@ -878,6 +868,16 @@ static ctl_table vm_table[] = {
 		.strategy	= &sysctl_jiffies,
 	},
 #endif
+#ifdef CONFIG_SECURITY
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "mmap_min_addr",
+		.data		= &mmap_min_addr,
+		.maxlen         = sizeof(unsigned long),
+		.mode		= 0644,
+		.proc_handler	= &proc_doulongvec_minmax,
+	},
+#endif
 #if defined(CONFIG_X86_32) || \
    (defined(CONFIG_SUPERH) && defined(CONFIG_VSYSCALL))
 	{
-- 
1.5.0.6

Re: [PATCH 1/2] selinux: don't enable minimum mmap checking by default

[Eric Paris]

On Thu, 2007-06-28 at 11:40 -0400, James Morris wrote:
> Don't enable minimum mmap checking by default in SELinux, as it may break
> existing applications which do not have updated policy.
> 
> We will be able to enable it by default later, once we have code to handle
> new permissions which are not present in the user's policy.
> 
> Signed-off-by: James Morris <jmorris@namei.org>
Acked-by: Eric Paris <eparis@redhat.com>

Re: [PATCH 1/7] ICH Force HPET: Make generic time capable of switching broadcast timer

[Andrew Morton]

On Thu, 28 Jun 2007 08:12:48 -0400 (EDT) James Morris <jmorris@namei.org> wrote:

> > My fix:
> 
> This should work.
> 

uh, OK, surprised.

Completer version:

--- a/security/selinux/hooks.c~git-selinux-disable-mmap_min_addr-by-default
+++ a/security/selinux/hooks.c
@@ -112,9 +112,6 @@ int selinux_enabled = 1;
 /* Original (dummy) security module. */
 static struct security_operations *original_ops = NULL;
 
-/* Did we enable minimum mmap address checking? */
-static int enabled_mmap_min_addr;
-
 /* Minimal support for a secondary security module,
    just to allow the use of the dummy or capability modules.
    The owlsm module can alternatively be used as a secondary
@@ -4917,15 +4914,6 @@ static __init int selinux_init(void)
 					    sizeof(struct inode_security_struct),
 					    0, SLAB_PANIC, NULL, NULL);
 
-	/*
-	 * Tasks cannot mmap below this without the mmap_zero permission.
-	 * If not enabled already, do so by setting it to 64KB.
-	 */
-	if (mmap_min_addr == 0) {
-		enabled_mmap_min_addr = 1;
-		mmap_min_addr = 65536;
-	}
-
 	avc_init();
 
 	original_ops = secondary_ops = security_ops;
@@ -5076,10 +5064,6 @@ int selinux_disable(void)
 	selinux_disabled = 1;
 	selinux_enabled = 0;
 
-	/* Disable minimum mmap address check only if we enabled it */
-	if (enabled_mmap_min_addr)
-		mmap_min_addr = 0;
-
 	/* Reset security_ops to the secondary module, dummy or capability. */
 	security_ops = secondary_ops;
 
_

Re: [PATCH 1/7] ICH Force HPET: Make generic time capable of switching broadcast timer

[Andrew Morton]

On Thu, 28 Jun 2007 08:45:48 -0400 Eric Paris <eparis@redhat.com> wrote:

> On Thu, 2007-06-28 at 01:27 -0700, Andrew Morton wrote:
> > On Thu, 28 Jun 2007 00:12:21 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> > 
> > > I can't start the X server any more, but that's probably unrelated ;)
> > > 
> > > <goes back to yet another bisection search.  sigh.  you guys have
> > > me hopelessly outnumbered>
> > 
> > And the winner of the u-o-akpm-1-hour-of-his-life-back award (and the bonus
> > u-delayed-next-mm-by-a-day award!) is ....  git-selinux.patch!
> > 
> > (II) Loading /usr/lib/xorg/modules/libvgahw.so
> > (II) Module vgahw: vendor="X.Org Foundation"
> >         compiled for 7.0.0, module version = 0.1.0
> >         ABI class: X.Org Video Driver, version 0.8
> > (**) I810(0): Depth 24, (--) framebuffer bpp 32
> > (==) I810(0): RGB weight 888
> > (==) I810(0): Default visual is TrueColor
> > (II) Loading sub module "int10"
> > (II) LoadModule: "int10"
> > (II) Reloading /usr/lib/xorg/modules/libint10.so
> > (II) I810(0): initializing int10
> > (EE) I810(0): Cannot shmat() low memory
> > (EE) I810(0): shmat(low_mem) error: Invalid argument
> > (EE) I810(0): VBE initialization failed.
> 
> It has been found and the policy people already know about it.

So I spent slightly over an hour hunting down an already-known-about bug.

>  I'm not
> sure what the upstream status of the fixed policy is should appear soon.

We cannot merge a kernel change which requires a userspace upgrade to get
the X server working again.

You can do it the other way: make the X server work by default (my patch)
then, in your upgraded userspace, set /proc/sys/kernel/mmap_min_addr to
65536.  

Repeat: I think you should have used /proc/sys/vm/ for that tunable.

Re: [PATCH 1/7] ICH Force HPET: Make generic time capable of switching broadcast timer

[James Morris]

On Thu, 28 Jun 2007, Andrew Morton wrote:

> We cannot merge a kernel change which requires a userspace upgrade to get
> the X server working again.

Agreed.

> 
> You can do it the other way: make the X server work by default (my patch)
> then, in your upgraded userspace, set /proc/sys/kernel/mmap_min_addr to
> 65536.  
> 
> Repeat: I think you should have used /proc/sys/vm/ for that tunable.

Andrew, I sent patches for these earlier.  Possibly lost somewhere?

http://marc.info/?l=linux-kernel&m=118304565827673&w=2
http://marc.info/?l=linux-kernel&m=118304566015586&w=2


-- 
James Morris
<jmorris@namei.org>

Re: [PATCH 2/2] security: move mmap_min_addr sysctl to /proc/sys/vm

[Alexey Dobriyan]

On Thu, Jun 28, 2007 at 11:41:38AM -0400, James Morris wrote:
> Move mmap_min_addr sysctl to /proc/sys/vm, as suggested by Andrew Morton.

Probably too late, since it's userspace visible. Everyone using
/proc/sys/kernel/mmap_min_addr in scrips will be surprised.

Re: [PATCH 2/2] security: move mmap_min_addr sysctl to /proc/sys/vm

[James Morris]

On Thu, 28 Jun 2007, Alexey Dobriyan wrote:

> On Thu, Jun 28, 2007 at 11:41:38AM -0400, James Morris wrote:
> > Move mmap_min_addr sysctl to /proc/sys/vm, as suggested by Andrew Morton.
> 
> Probably too late, since it's userspace visible. Everyone using
> /proc/sys/kernel/mmap_min_addr in scrips will be surprised.

It's only in -mm so far.


-- 
James Morris
<jmorris@namei.org>