linux-kernel - Re: [PATCH RFC 2/7] KVM: nVMX: modify vmcs12 fields to match Hyper-V enlightened VMCS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALMp9eTVV7EunsGFgypVXwncvQo_tDCV2NgrOOprzCbs=U-p0Q@mail.gmail.com>
Date:   Tue, 19 Dec 2017 09:40:44 -0800
From:   Jim Mattson <jmattson@...gle.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     Vitaly Kuznetsov <vkuznets@...hat.com>,
        kvm list <kvm@...r.kernel.org>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Radim Krčmář <rkrcmar@...hat.com>,
        "K. Y. Srinivasan" <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "Michael Kelley (EOSG)" <Michael.H.Kelley@...rosoft.com>,
        Mohammed Gamal <mmorsy@...hat.com>,
        Cathy Avery <cavery@...hat.com>, Bandan Das <bsd@...hat.com>,
        Roman Kagan <rkagan@...tuozzo.com>,
        LKML <linux-kernel@...r.kernel.org>, devel@...uxdriverproject.org
Subject: Re: [PATCH RFC 2/7] KVM: nVMX: modify vmcs12 fields to match Hyper-V
 enlightened VMCS

On Tue, Dec 19, 2017 at 4:37 AM, Paolo Bonzini <pbonzini@...hat.com> wrote:
> On 19/12/2017 13:25, Vitaly Kuznetsov wrote:
>>
>>> At this point in time, I don't think you can just blithely change the
>>> virtual VMCS layout and revision number. Existing VMs using the old
>>> layout and revision number must continue to work on versions of kvm
>>> past this point. You could tie the layout and revision number changes
>>> to KVM_CAP_HYPERV_ENLIGHTENED_VMCS if you like, but kvm must be able
>>> to continue to service VMs using the previous layout and revision
>>> number in perpetuity.
>>>
>> I see what you mean. In case we need to keep migration of nested
>> workloads working between KVMs of different versions we can't (ever)
>> touch vmcs12.
>
> Actually we can, for two reasons.
>
> First, the active VMCS is stored in host RAM (not in guest RAM).  This
> means there are clear points where to do the translation, namely vmptrld
> and the (not yet upstream) ioctl to set VMX state.
>
> Therefore you only need to keep an (offset, type) map from old to new
> layout map; at those two points if you detect an old VMCS12_REVISION you
> copy the fields one by one instead of doing a memcpy.  The next vmclear
> or vmptrld or get-VMX-state ioctl will automatically update to the new
> VMCS12_REVISION.  Of course, this is a one-way street unless you also
> add support for writing old VMCS12_REVISIONs.

I'm not sure that's really the right way to go, since any guest that
has already read the IA32_VMX_BASIC MSR has a right to expect the VMCS
revision to remain unchanged. Also, this breaks migration back to
older versions of kvm, even for VMs that are not making use of the
enlightened VMCS. It would be nice to be able to maintain the ability
for an older VM to run on a newer kvm version without picking up any
taint that would make it impossible to migrate it back to the older
kvm version where it started.

> But, second, VMX state migration is not upstream yet, so nested
> hypervisors are currently not migratable: the active VMCS12 state will
> not be migrated at all!  So in upstream KVM we wouldn't even need to
> upgrade the VMCS12_REVISION to make changes to vmcs12.

Mea culpa. Perhaps this is the motivation I needed to prioritize
incorporating the community feedback on that change set.

> That said...
>
>> The way to go in this case, I think, is to create a completely separate
>> enlightened_vmcs12 struct and use it when appropriate. We can't possibly
>> support migrating workloads which use enlightened VMCS to an old KVM
>> which doesn't support it.
>
> ... this is probably a good idea as well.
>
> Paolo