All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Dufour <ldufour@linux.vnet.ibm.com>
To: Michael Ellerman <mpe@ellerman.id.au>,
	Simon Guo <wei.guo.simon@gmail.com>
Cc: Kees Cook <keescook@chromium.org>,
	Rashmica Gupta <rashmicy@gmail.com>,
	linux-kernel@vger.kernel.org, Paul Mackerras <paulus@samba.org>,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [v4] powerpc: Export thread_struct.used_vr/used_vsr to user space
Date: Thu, 7 Jul 2016 15:12:20 +0200	[thread overview]
Message-ID: <577E5534.70300@linux.vnet.ibm.com> (raw)
In-Reply-To: <3rlZmP39HNz9sXR@ozlabs.org>

On 07/07/2016 13:15, Michael Ellerman wrote:
> On Thu, 2016-07-07 at 07:49:36 UTC, Simon Guo wrote:
>> From: Simon Guo <wei.guo.simon@gmail.com>
>>
>> These 2 fields track whether user process has used Altivec/VSX
>> registers or not. They are used by kernel to setup signal frame
>> on user stack correctly regarding vector part.
> 
> I still dislike this. It's just exporting internal kernel state, which I know is
> the point.
> 
> And I'd still like to know why we're the only arch that needs to do this.
> 
> I'm not saying I won't merge it, but I'd like to understand it better first.
> 
>> CRIU(Checkpoint and Restore In User space) builds signal frame
>> for restored process. It will need this export information to
>> setup signal frame correctly. And CRIU will need to restore these
>> 2 fields for the restored process.
> 
> I don't really know how CRIU works, but ..
> 
> Does the kernel write a sigframe for the process that's being checkpointed? If
> so can't you infer the state of the bits based on what was written?

Hi Michael,

Basically, CRIU checkpoints the process register's state through the
ptrace API, and it restores it through a signal frame at restart time.
This is quite odd but that the way it works on all the CRIU's supported
architectures.

Obviously everything is done from/in user space, so the sigframe
building too.
Since we can't know from user space if the thread has used or not the
Altivec/VSX registers, since we can't rely on the MSR bits, we always
dump these registers.

> Alternately, when restoring, can you setup the sigframe with the Altivec/VSX
> fields populated, and the kernel will then load them, regardless of whether
> they were actually used or not prior to the checkpoint?

In the case of Altivec/VSX fields, we currently force the kernel to
retrieve them from the signal frame by setting MSR_VEC/MSR_VSX so
restore_sigcontext() will copy them to the kernel thread's state.
However this doesn't touch to used_vsr and used_vr which may remain at 0.

Most of the time this is fine, but in the case a thread which has really
used those registers is catching a signal just after the restore and
before it has touched to these registers again (and so set used_vsr/vr),
these registers will not be pushed in the newly built signal frame since
setup_sigcontext() check for used_vsr/vr before pushing the registers on
the stack.
This may be an issue in the case the thread wants to changed those
registers (don't ask me why :)) in the stacked signal frame from the
signal handler since they will not be there...

Being able to get and set the used_vr and used_vsr thread's variables,
fixes this issue.

Cheers,
Laurent.

  reply	other threads:[~2016-07-07 13:12 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-07  7:49 [PATCH v4] powerpc: Export thread_struct.used_vr/used_vsr to user space wei.guo.simon
2016-07-07 11:15 ` [v4] " Michael Ellerman
2016-07-07 13:12   ` Laurent Dufour [this message]
2016-07-07 13:21     ` Benjamin Herrenschmidt
2016-07-07 13:29       ` Benjamin Herrenschmidt
2016-07-08 10:02         ` Michael Ellerman
     [not found]         ` <577f7a45.568f6b0a.bd0eb.4aa8SMTPIN_ADDED_BROKEN@mx.google.com>
2016-07-11 17:39           ` Simon Guo
2016-07-15  7:15           ` Simon Guo
2016-07-21 10:57             ` Michael Ellerman
     [not found]             ` <5790aa9c.05296b0a.adf58.5901SMTPIN_ADDED_BROKEN@mx.google.com>
2016-07-25  8:52               ` Simon Guo
2016-07-07 13:40       ` Laurent Dufour
2016-07-08  8:26         ` Michael Ellerman
2016-07-08  5:39       ` Simon Guo

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=577E5534.70300@linux.vnet.ibm.com \
    --to=ldufour@linux.vnet.ibm.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=paulus@samba.org \
    --cc=rashmicy@gmail.com \
    --cc=wei.guo.simon@gmail.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.