Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Vitaly Kuznetsov <vkuznets@redhat.com>
To:     Wanpeng Li <kernellwp@gmail.com>
Cc:     kvm <kvm@vger.kernel.org>, Paolo Bonzini <pbonzini@redhat.com>,
        Radim Krcmar <rkrcmar@redhat.com>,
        "the arch\/x86 maintainers" <x86@kernel.org>,
        Andy Lutomirski <luto@kernel.org>, ldv@altlinux.org,
        yamato@redhat.com, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] x86/kvm/vmx: don't read current->thread.{fs,gs}base of legacy tasks
References: <20180711173718.8850-1-vkuznets@redhat.com>
        <CANRm+CxKWuuzyu4Yi-phUR3RpVb0bx_=r3FoJvZMm1ii-8-wKw@mail.gmail.com>
Date:   Thu, 12 Jul 2018 11:31:41 +0200
In-Reply-To: <CANRm+CxKWuuzyu4Yi-phUR3RpVb0bx_=r3FoJvZMm1ii-8-wKw@mail.gmail.com>
        (Wanpeng Li's message of "Thu, 12 Jul 2018 09:39:09 +0800")
Message-ID: <877em0ztoi.fsf@vitty.brq.redhat.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Wanpeng Li <kernellwp@gmail.com> writes:

> On Thu, 12 Jul 2018 at 08:07, Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>>
>> When we switched from doing rdmsr() to reading FS/GS base values from
>> current->thread we completely forgot about legacy 32-bit userspaces which
>> we still support in KVM (why?). task->thread.{fsbase,gsbase} are only
>> synced for 64-bit processes, calling save_fsgs_for_kvm() and using
>> its result from current is illegal for legacy processes.
>>
>> There's no ARCH_SET_FS/GS prctls for legacy applications. Base MSRs are,
>> however, not always equal to zero. Intel's manual says (3.4.4 Segment
>> Loading Instructions in IA-32e Mode):
>>
>> "In order to set up compatibility mode for an application, segment-load
>> instructions (MOV to Sreg, POP Sreg) work normally in 64-bit mode. An
>> entry is read from the system descriptor table (GDT or LDT) and is loaded
>> in the hidden portion of the segment register.
>> ...
>> The hidden descriptor register fields for FS.base and GS.base are
>> physically mapped to MSRs in order to load all address bits supported by
>> a 64-bit implementation.
>> "
>>
>> The issue was found by strace test suite where 32-bit ioctl_kvm_run test
>> started segfaulting.
>
> Test suite: MSR switch
> PASS: VM entry MSR load
> PASS: VM exit MSR store
> PASS: VM exit MSR load
> FAIL: VM entry MSR load: try to load FS_BASE
> SUMMARY: 4 tests, 1 unexpected failures
>
> kvm-unit-tests fails w/ and w/o the patch, maybe it is another issue,
> i didn't dig further, you can have a look if you are interested in. :)

The patch only changes the behavior for legacy userspaces and I can
reproduce the failure on native x86_64, it is something different. I'm,
however, interested so stay tuned :-)

-- 
  Vitaly