All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: Ingo Molnar <mingo@redhat.com>,
	the arch/x86 maintainers <x86@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Xen-devel <xen-devel@lists.xensource.com>,
	Stable Kernel <stable@kernel.org>, Tejun Heo <tj@kernel.org>
Subject: Re: [GIT PULL] Xen bugfixes
Date: Thu, 10 Sep 2009 07:16:48 +0200	[thread overview]
Message-ID: <20090910051648.GA1335@elte.hu> (raw)
In-Reply-To: <4AA83F99.5040401@goop.org>


* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> Hi,
> 
> Here's 3 patches which fix two Xen PV spinlock bugs, and makes 
> CONFIG_CC_STACKPROTECTOR work on both 32- and 64-bit.
> 
> The spinlock bugs are both rare races:
>  - lock can briefly hold the lock while interrupts are enabled, allowing an ISR to deadlock
>  - unlock doesn't enforce CPU memory ordering between the actual unlock write and the check
>    to see if there are any pending waiters, so it can end up deciding there's nobody to
>    kick on unlock, leaving another CPU hanging.  It needs a full mb() to guarantee the correct
>    ordering.
> 
> The stack-protector fix bites the bullet and does a full GDT setup 
> early so that we can load %gs for the stack-protector canary 
> segment on i386.  This also removes the assumption that the 
> initial percpu %fs segment has a zero base.
> 
> x86-64 still just needs the GS_BASE MSR written, but that now 
> happens using the same code as i386 rather than being special 
> cased.
> 
> Thanks,
> 	J
> 
> The following changes since commit e07cccf4046978df10f2e13fe2b99b2f9b3a65db:
>   Linus Torvalds (1):
>         Linux 2.6.31-rc9
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git bugfix
> 
> Jeremy Fitzhardinge (2):
>       xen: make -fstack-protector work under Xen
>       xen: only enable interrupts while actually blocking for spinlock
> 
> Yang Xiaowei (1):
>       xen: use stronger barrier after unlocking lock
> 
>  arch/x86/mm/Makefile     |    4 ++
>  arch/x86/xen/Makefile    |    2 +
>  arch/x86/xen/enlighten.c |  131 +++++++++++++++++++++++++++++++++++++++-------
>  arch/x86/xen/smp.c       |    1 +
>  arch/x86/xen/spinlock.c  |   28 ++++++----
>  drivers/xen/Makefile     |    3 +
>  6 files changed, 140 insertions(+), 29 deletions(-)

Pulled, thanks a lot Jeremy!

A few comments:

> +# Make sure __phys_addr has no stackprotector
> +nostackp := $(call cc-option, -fno-stack-protector)
> +CFLAGS_ioremap.o		:= $(nostackp)
> +

Sure we could move __phys_addr into its own file and thus avoid 
turning off stackprotector for the rest of ioremap.c?

> --- a/arch/x86/xen/Makefile
> +++ b/arch/x86/xen/Makefile
> @@ -8,6 +8,7 @@ endif
>  # Make sure early boot has no stackprotector
>  nostackp := $(call cc-option, -fno-stack-protector)
>  CFLAGS_enlighten.o		:= $(nostackp)
> +CFLAGS_mmu.o			:= $(nostackp)

A similar argument could be made here - what proportion of mmu.c is 
affected?

Also, once the commits have hit upstream feel free bounce them to 
stable@kernel.org - they dont have Cc: <stable@kernel.org> tags for 
automatic back-merging requests. The fixes narrowly missed v2.6.31.

	Ingo

WARNING: multiple messages have this Message-ID (diff)
From: Ingo Molnar <mingo@elte.hu>
To: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: Xen-devel <xen-devel@lists.xensource.com>,
	the arch/x86 maintainers <x86@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@redhat.com>, Tejun Heo <tj@kernel.org>,
	Stable Kernel <stable@kernel.org>
Subject: Re: [GIT PULL] Xen bugfixes
Date: Thu, 10 Sep 2009 07:16:48 +0200	[thread overview]
Message-ID: <20090910051648.GA1335@elte.hu> (raw)
In-Reply-To: <4AA83F99.5040401@goop.org>


* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> Hi,
> 
> Here's 3 patches which fix two Xen PV spinlock bugs, and makes 
> CONFIG_CC_STACKPROTECTOR work on both 32- and 64-bit.
> 
> The spinlock bugs are both rare races:
>  - lock can briefly hold the lock while interrupts are enabled, allowing an ISR to deadlock
>  - unlock doesn't enforce CPU memory ordering between the actual unlock write and the check
>    to see if there are any pending waiters, so it can end up deciding there's nobody to
>    kick on unlock, leaving another CPU hanging.  It needs a full mb() to guarantee the correct
>    ordering.
> 
> The stack-protector fix bites the bullet and does a full GDT setup 
> early so that we can load %gs for the stack-protector canary 
> segment on i386.  This also removes the assumption that the 
> initial percpu %fs segment has a zero base.
> 
> x86-64 still just needs the GS_BASE MSR written, but that now 
> happens using the same code as i386 rather than being special 
> cased.
> 
> Thanks,
> 	J
> 
> The following changes since commit e07cccf4046978df10f2e13fe2b99b2f9b3a65db:
>   Linus Torvalds (1):
>         Linux 2.6.31-rc9
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git bugfix
> 
> Jeremy Fitzhardinge (2):
>       xen: make -fstack-protector work under Xen
>       xen: only enable interrupts while actually blocking for spinlock
> 
> Yang Xiaowei (1):
>       xen: use stronger barrier after unlocking lock
> 
>  arch/x86/mm/Makefile     |    4 ++
>  arch/x86/xen/Makefile    |    2 +
>  arch/x86/xen/enlighten.c |  131 +++++++++++++++++++++++++++++++++++++++-------
>  arch/x86/xen/smp.c       |    1 +
>  arch/x86/xen/spinlock.c  |   28 ++++++----
>  drivers/xen/Makefile     |    3 +
>  6 files changed, 140 insertions(+), 29 deletions(-)

Pulled, thanks a lot Jeremy!

A few comments:

> +# Make sure __phys_addr has no stackprotector
> +nostackp := $(call cc-option, -fno-stack-protector)
> +CFLAGS_ioremap.o		:= $(nostackp)
> +

Sure we could move __phys_addr into its own file and thus avoid 
turning off stackprotector for the rest of ioremap.c?

> --- a/arch/x86/xen/Makefile
> +++ b/arch/x86/xen/Makefile
> @@ -8,6 +8,7 @@ endif
>  # Make sure early boot has no stackprotector
>  nostackp := $(call cc-option, -fno-stack-protector)
>  CFLAGS_enlighten.o		:= $(nostackp)
> +CFLAGS_mmu.o			:= $(nostackp)

A similar argument could be made here - what proportion of mmu.c is 
affected?

Also, once the commits have hit upstream feel free bounce them to 
stable@kernel.org - they dont have Cc: <stable@kernel.org> tags for 
automatic back-merging requests. The fixes narrowly missed v2.6.31.

	Ingo

  reply	other threads:[~2009-09-10  5:17 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-09 23:51 [GIT PULL] Xen bugfixes Jeremy Fitzhardinge
2009-09-09 23:51 ` Jeremy Fitzhardinge
2009-09-10  5:16 ` Ingo Molnar [this message]
2009-09-10  5:16   ` Ingo Molnar
2009-09-10 17:16   ` Jeremy Fitzhardinge
2009-09-10 17:16     ` Jeremy Fitzhardinge
2009-09-10 17:26     ` Ingo Molnar
2009-09-10 17:26       ` Ingo Molnar
2009-09-10 19:02       ` Jeremy Fitzhardinge
2009-09-10 19:02         ` Jeremy Fitzhardinge
  -- strict thread matches above, loose matches on Subject: below --
2009-12-03 23:46 Jeremy Fitzhardinge

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090910051648.GA1335@elte.hu \
    --to=mingo@elte.hu \
    --cc=jeremy@goop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=stable@kernel.org \
    --cc=tj@kernel.org \
    --cc=x86@kernel.org \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.