linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Dave Hansen <dave.hansen@intel.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Qiaowei Ren <qiaowei.ren@intel.com>,
	"H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@redhat.com>,
	x86@kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v8 08/10] x86, mpx: add prctl commands PR_MPX_REGISTER, PR_MPX_UNREGISTER
Date: Fri, 12 Sep 2014 13:33:02 -0700	[thread overview]
Message-ID: <5413587E.4000303@intel.com> (raw)
In-Reply-To: <alpine.DEB.2.10.1409121934520.4178@nanos>

On 09/12/2014 10:42 AM, Thomas Gleixner wrote:
> On Fri, 12 Sep 2014, Dave Hansen wrote:
>> The prctl(register) is meant to be a signal from userspace to the kernel
>> to say, "I would like your help in managing these bounds tables".
>> prctl(unregister) is the opposite, meaning "I don't want your help any
>> more".
> 
> Fine, but that's a totally different story. I can see the usefulness
> of this, but then it's a complete misnomer. It should be:
> 
>    prctl(EN/DISABLE_MPX_BT_MANAGEMENT)

Agreed.  Those are much better names.

> So this wants to be a boolean value and not some random user space
> address collected at some random point and then ignored until you do
> the magic cleanup. See the other reply.

I know at this point you think the kernel can not or should not keep a
copy of the bounds directory location around.  I understand that.  Bear
with me for a moment, and please just assume for a moment that we need it.

It's far from a random userspace address.  When you make a syscall, we
put the arguments in registers.  The register we're putting it in here
just happens to be used by the hardware.

Right now, we do (ignoring the actual xsave/xrstr):

	bndcfgu = bnd_dir_ptr | ENABLE_BIT;
	prctl(ENABLE_MPX_BT_MANAGEMENT); // kernel grabs from xsave buf

We could pass it explicitly in %rdi as a syscall argument and not have
the prctl() code fetch it from the xsave buffer.  I'm just not sure what
this buys us:

	bndcfgu = bnd_dir_ptr | ENABLE_BIT;
	prctl(ENABLE_MPX_BT_MANAGEMENT, bndcfgu);

Also, the "random cleanup" just happens to correspond with memory
deallocation, which is something we want to go fast.  I'd _prefer_ to
keep xsaves out of the unmap path if possible.  It's not a strict
requirement, but it does seem prudent as an xsave eats a dozen or so
cachelines.

It's also not "sampled".  I can't imagine a situation where the register
will change values during the execution of any sane program.  It really
is essentially fixed.  It's probably one of the reasons it is so
expensive to access: there's *no* reason to do it frequently.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2014-09-12 20:33 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-11  8:46 [PATCH v8 00/10] Intel MPX support Qiaowei Ren
2014-09-11  8:46 ` [PATCH v8 01/10] x86, mpx: introduce VM_MPX to indicate that a VMA is MPX specific Qiaowei Ren
2014-09-11  8:46 ` [PATCH v8 02/10] x86, mpx: add MPX specific mmap interface Qiaowei Ren
2014-09-11  8:46 ` [PATCH v8 03/10] x86, mpx: add macro cpu_has_mpx Qiaowei Ren
2014-09-11  8:46 ` [PATCH v8 04/10] x86, mpx: hook #BR exception handler to allocate bound tables Qiaowei Ren
2014-09-12 22:58   ` Dave Hansen
2014-09-13  7:24     ` Ren, Qiaowei
2014-09-24 14:40   ` Dave Hansen
2014-09-11  8:46 ` [PATCH v8 05/10] x86, mpx: extend siginfo structure to include bound violation information Qiaowei Ren
2014-09-11  8:46 ` [PATCH v8 06/10] mips: sync struct siginfo with general version Qiaowei Ren
2014-09-11 22:13   ` Thomas Gleixner
2014-09-12  2:54     ` Ren, Qiaowei
2014-09-12  8:17       ` Thomas Gleixner
2014-09-13  7:13         ` Ren, Qiaowei
2014-09-11  8:46 ` [PATCH v8 07/10] x86, mpx: decode MPX instruction to get bound violation information Qiaowei Ren
2014-09-11 22:18   ` Thomas Gleixner
2014-09-11 22:32     ` Dave Hansen
2014-09-11 22:35       ` H. Peter Anvin
2014-09-11 23:37         ` Thomas Gleixner
2014-09-12  4:44           ` H. Peter Anvin
2014-09-12 13:10             ` Thomas Gleixner
2014-09-12 13:39               ` H. Peter Anvin
2014-09-12 17:48                 ` Thomas Gleixner
2014-09-12 17:52         ` Thomas Gleixner
2014-09-12 19:07           ` H. Peter Anvin
2014-09-11  8:46 ` [PATCH v8 08/10] x86, mpx: add prctl commands PR_MPX_REGISTER, PR_MPX_UNREGISTER Qiaowei Ren
2014-09-11 15:03   ` Dave Hansen
2014-09-12  3:10     ` Ren, Qiaowei
2014-09-11 23:28   ` Thomas Gleixner
2014-09-12  0:10     ` Dave Hansen
2014-09-12  8:11       ` Thomas Gleixner
2014-09-12  9:24         ` Thomas Gleixner
2014-09-12 14:36           ` Dave Hansen
2014-09-12 17:34             ` Thomas Gleixner
2014-09-12 18:42               ` Thomas Gleixner
2014-09-12 20:35                 ` Dave Hansen
2014-09-12 20:18               ` Dave Hansen
2014-09-13  9:01                 ` Thomas Gleixner
2014-09-12 15:22         ` Dave Hansen
2014-09-12 17:42           ` Thomas Gleixner
2014-09-12 20:33             ` Dave Hansen [this message]
2014-09-15  0:00   ` One Thousand Gnomes
2014-09-16  3:20     ` Ren, Qiaowei
2014-09-16  4:17       ` Dave Hansen
2014-09-16  7:50   ` Kevin Easton
2014-09-18  0:40     ` Ren, Qiaowei
2014-09-18  3:23       ` Kevin Easton
2014-09-18  2:37         ` Ren, Qiaowei
2014-09-18  4:43         ` Dave Hansen
2014-09-18  7:17           ` Kevin Easton
2014-09-18  6:20             ` Dave Hansen
2014-09-11  8:46 ` [PATCH v8 09/10] x86, mpx: cleanup unused bound tables Qiaowei Ren
2014-09-11 14:59   ` Dave Hansen
2014-09-12  3:02     ` Ren, Qiaowei
2014-09-12  4:59       ` Dave Hansen
2014-09-15 20:53   ` Dave Hansen
2014-09-16  8:06     ` Ren, Qiaowei
2014-09-11  8:46 ` [PATCH v8 10/10] x86, mpx: add documentation on Intel MPX Qiaowei Ren
2014-09-12  0:51 ` [PATCH v8 00/10] Intel MPX support Dave Hansen
2014-09-12 19:21   ` Thomas Gleixner
2014-09-12 21:23     ` Dave Hansen
2014-09-13  9:25       ` Thomas Gleixner
2014-09-12 21:31     ` Dave Hansen
2014-09-12 22:08     ` Dave Hansen
2014-09-13  9:39       ` Thomas Gleixner

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=5413587E.4000303@intel.com \
    --to=dave.hansen@intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mingo@redhat.com \
    --cc=qiaowei.ren@intel.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).