From: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
To: Keno Fischer <keno@juliacomputing.com>, Andi Kleen <andi@firstfloor.org>
Cc: mtk.manpages@gmail.com, linux-man <linux-man@vger.kernel.org>,
Andy Lutomirski <luto@kernel.org>,
Dave Hansen <dave.hansen@intel.com>,
Peter Zijlstra <peterz@infradead.org>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>,
"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
<x86@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
Borislav Petkov <bp@alien8.de>,
Dave Hansen <dave.hansen@linux.intel.com>
Subject: Re: [PATCH] ptrace.2: Describe PTRACE_SET/GETREGSET on NT_X86_XSTATE
Date: Mon, 8 Jun 2020 17:46:39 +0200 [thread overview]
Message-ID: <8b8cb10d-6d53-d54e-1034-df2cca9a5442@gmail.com> (raw)
In-Reply-To: <CABV8kRy7XTSv=hJbVSyyKPbT6US7y9AgmG5r9g8AjBJCiVSntw@mail.gmail.com>
Hi Keno,
Any progress on a v2 patch?
Thanks,
Michael
On 5/20/20 5:30 AM, Keno Fischer wrote:
>>> +.SS The layout and operation of the NT_X86_XSTATE regset
>>
>> Should rather have a complete table of NT_* entries first. The others
>> can be dummies for now.
>
> Oh boy, I'm not sure my man-page-formatting-fu is up to the task of
> creating a nice looking table :).
> Michael, can you help?
>
>>> +Obtain the kernel xsave component bitmask from the software-reserved area of the
>>> +xstate buffer. The software-reserved area beings at offset 464 into the xsave
>>
>> It would be better to put some struct defining this into the kernel uapi
>> and then reference that instead of magic numbers.
>
> We have user_xstateregs in the kernel, but that's not in the uapi.
> I suppose we should move it, given that it is exposed here.
>
>>> +buffer and the first 64 bits of this area contain the kernel xsave component bitmask
>>> +.IP 2.
>>> +Compute the offset of each state component by adding the sizes of all prior state
>>> +components that are enabled in the kernel xsave component bitmask, aligning to 64 byte boundaries along the way. This
>>> +format matches that of a compacted xsave area with XCOMP_BV set to the
>>
>> The sizes of these areas should probably also be in the uapi include
>
> Yes, that seems like a good idea.
> What's the policy on helper functions in uapi includes?
> Can we have helper functions that given a buffer and the kernel xstate mask,
> does this computation for you?
>
>>> +kernel component bitmask. Further details on the layout of the compacted xsave
>>> +area may be found in the Intel architecture manual, but note that the xsave
>>> +buffer returned from ptrace will have its XCOMP_BV set to 0.
>>
>> The note seems disconnected. You'll have to explain it here.
>
> Ok, I'll elaborate. The point I wanted to make is that even though the buffer
> looks compressed, XCOMP_BV is 0, so it's not a valid compressed buffer
> that can be xrstor'ed.
>
>>> +Thus, to obtain an xsave area that may be set back to the tracee, all unused
>>> +state components must first be re-set to the correct initial state for the
>>> +corresponding state component, and the XSTATE_BV bitfield must subsequently
>>> +be adjusted to match the kernel xstate component bitmask (obtained as
>>> +described above).
>>
>> I wonder if we shouldn't just fix the kernel to do this properly on its
>> own. Presumably it won't break any existing user space.
>>
>> It seems more a bug than something that should be a documented ABI.
>
> I'd be happy to see this interface improved, since I do think it wasn't quite
> intended to work this way when originally conceived (i.e. originally, before
> the init optimization and before we had flags that turn off various xstate
> components resulting in a compressed buffer).
>
> As I said in the other email thread, I think it would be reasonable to change
> the offsets to always be non-compressed, which would at least make this
> a normal xsave buffer. No ptracer that I looked at knows that this buffer
> can be compressed, so changing the kernel behavior here would actually
> make it closer to what existing userspace expects ;).
>
> I'm not sure what to do about the getregset/setregset mismatch. On the one
> hand it's pretty bad, but on the other hand, I'm not really sure what to do
> about it, short of introducing a different NT_X86_* constant that behaves
> differently.
>
>>> +
>>> +The value of the kernel's state component bitmask is determined on boot and
>>> +need not be equivalent to the maximal set of state components supported by the
>>> +CPU (as enumerated through CPUID).
>>
>> Okay so how should someone get it? Looks like that's a hole in the
>> kernel API that we need to fix somehow.
>
> The cpuid enumerated value does still represent a maximum so that can be used
> to size the buffer and the actual value can then be read from the software saved
> area as described here. Is that what you were asking? Not sure I understood
> correctly.
>
>
>
> Keno
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
next prev parent reply other threads:[~2020-06-08 15:46 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-18 3:00 [PATCH] ptrace.2: Describe PTRACE_SET/GETREGSET on NT_X86_XSTATE Keno Fischer
2020-05-19 20:44 ` Michael Kerrisk (man-pages)
2020-05-19 21:29 ` Denys Vlasenko
2020-05-19 22:46 ` Keno Fischer
2020-05-20 10:03 ` Denys Vlasenko
2020-05-20 1:19 ` Andi Kleen
2020-05-20 3:30 ` Keno Fischer
2020-05-20 5:08 ` Michael Kerrisk (man-pages)
2020-05-20 13:56 ` Andi Kleen
2020-06-08 15:46 ` Michael Kerrisk (man-pages) [this message]
2020-06-08 22:07 ` Keno Fischer
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=8b8cb10d-6d53-d54e-1034-df2cca9a5442@gmail.com \
--to=mtk.manpages@gmail.com \
--cc=andi@firstfloor.org \
--cc=bp@alien8.de \
--cc=dave.hansen@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=keno@juliacomputing.com \
--cc=linux-man@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--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