Message-ID: <CAMzpN2g63_CxLOR2Z0BqXNwYypguSoUq292VCpue9wdqPLWxvg@mail.gmail.com>
Date:	Wed, 29 Jul 2015 13:14:54 -0400
From:	Brian Gerst <brgerst@...il.com>
To:	Andy Lutomirski <luto@...capital.net>
Cc:	X86 ML <x86@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...nel.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH 4/8] x86/vm86: Use the normal pt_regs area for vm86

On Wed, Jul 29, 2015 at 11:50 AM, Andy Lutomirski <luto@...capital.net> wrote:
> On Tue, Jul 28, 2015 at 10:41 PM, Brian Gerst <brgerst@...il.com> wrote:
>> Change to use the normal pt_regs area to enter and exit vm86 mode.  This is
>> done by increasing the padding at the top of the stack to make room for the
>> extra vm86 segment slots in the IRET frame.  It then saves the 32-bit regs
>> in the off-stack vm86 data, and copies in the vm86 regs.  Exiting back to
>> 32-bit mode does the reverse.  This allows removing the hacks to jump directly
>> into the exit asm code due to having to change the stack pointer.  Returning
>> normally from the vm86 syscall and the exception handlers allows things like
>> ptrace and auditing to work properly.
>
> With caveats below:
>
> Acked-by: Andy Lutomirski <luto@...nel.org>
>
>> -       movb    PT_CS(%esp), %bl
>> -       andb    $SEGMENT_RPL_MASK, %bl
>> -       cmpb    $USER_RPL, %bl
>> -       jb      resume_kernel
>
> I agree with this hunk, and I have the same hunk in my tree.  However,
> it has nothing to do with vm86 and, on the off chance that this hunk
> is wrong, I think it should be bisect-friendly.

I think it was causing signal handling to fail, but I can't remember
exactly.  If I kept the code it would have needed to add a test for
the VM flag.  But I came to the same conclusion you did, that it was
unneeded.

> If you want to preserve my bit of archeology, you could include:
>
> https://git.kernel.org/cgit/linux/kernel/git/luto/linux.git/commit/?h=x86/entry&id=bb2a2fc1dc6423e87e7f1c3c9c4567a47f727b6e
>
> in this series and stick your patch on top.  (Or, Ingo, if you want to
> apply just that one patch from my pile and merge this on top, you'd
> get exactly the same result.)

I'd be OK with that.

>> --- a/arch/x86/include/asm/thread_info.h
>> +++ b/arch/x86/include/asm/thread_info.h
>> @@ -27,14 +27,17 @@
>>   * Without this offset, that can result in a page fault.  (We are
>>   * careful that, in this case, the value we read doesn't matter.)
>>   *
>> - * In vm86 mode, the hardware frame is much longer still, but we neither
>> - * access the extra members from NMI context, nor do we write such a
>> - * frame at sp0 at all.
>> + * In vm86 mode, the hardware frame is much longer still, so add 16
>> + * bytes to make room for the real-mode segments.
>
> I've now read this comment a few times, and I think it could be
> better.  How about:
>
> In vm86 mode, the hardware frame is extended by 4 bytes for each of
> ds, es, fs and gs.  To allow room for those extra slots while still
> keeping pt_regs in its normal location on entries from vm86 mode, we
> need at least 16 bytes of padding.  In the vm86 case, we don't need to
> worry about the SYSENTER + NMI issue: we disallow SYSENTER from vm86
> mode, so we don't need to worry about an NMI trying to read above the
> top of the vm86 hardware frame.

That looks good.

--
Brian Gerst
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists