linux-kernel - Re: [PATCH v2 6/6] KVM: nVMX: optimize prepare_vmcs02{,

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: <87in53pjgv.fsf@vitty.brq.redhat.com>
Date:   Wed, 25 Jul 2018 14:50:56 +0200
From:   Vitaly Kuznetsov <vkuznets@...hat.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     kvm@...r.kernel.org,
        Radim Krčmář <rkrcmar@...hat.com>,
        Roman Kagan <rkagan@...tuozzo.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>, linux-kernel@...r.kernel.org,
        Jim Mattson <jmattson@...gle.com>,
        Liran Alon <liran.alon@...cle.com>
Subject: Re: [PATCH v2 6/6] KVM: nVMX: optimize prepare_vmcs02{,_full} for Enlightened VMCS case

Paolo Bonzini <pbonzini@...hat.com> writes:

> On 25/07/2018 10:37, Vitaly Kuznetsov wrote:
>>> Why is this needed?  If it weren't for it, you could pass hv_evmcs
>>> directly to evmcs_needs_write, which would simplify the code a bit in
>>> the caller.
>> This is an equivalent of prepare_vmcs02()/prepare_vmcs02_full() split
>> for eVMCS case: when we switch from L2 guest A to L2 guest B we need to
>> write the whole VMCS so evmcs_needs_write() needs to return true.
>
> Right, I missed the dirty_vmcs12 assignment in patch 5.
>
> But is L0 allowed to write to hv_clean_fields?  

It is kinda expected to: currently I reset it in vmx_vcpu_run() and (if
I remember correctly) L1 Hyper-V only clears bits in this mask when it
touches certain fields so if we don't set it to 'all clean' it stays
zeroed forever. So nothing stops us from doing 

       if (hv_evmcs && vmx->nested.dirty_vmcs12)
               hv_evmcs->hv_clean_fields &=
                       ~HV_VMX_ENLIGHTENED_CLEAN_FIELD_ALL;

in prepare_vmcs02() I guess.

> One possibility is to
> add a dirty_evmcs field to struct nested_vmx, and "OR" ~hv_clean_fields
> into it at the beginning of prepare_vmcs02.
>
> Something like
>
> 	if (vmx->nested.hv_evmcs) {
> 		vmx->nested.dirty_evmcs |=
> 			~vmx->nested.hv_evmcs->hv_clean_fields;
> 		prepare_vmcs02_full(vcpu, vmcs12,
> 				    vmx->nested.dirty_evmcs);
> 	} else if (vmx->nested.dirty_vmcs12) {
> 		prepare_vmcs02_full(vcpu, vmcs12, ~0);
> 	}
>
> 	...
> 	vmx->nested.dirty_evmcs = 0;
> 	vmx->nested.dirty_vmcs12 = false;
>
> ?
>

I think we can even get away with a local variable in prepare_vmcs02()
and pass it to prepare_vmcs02_full(), no need to have it in struct
nested_vmx. But I would slightly prefer to just reset
hv_evmcs->hv_clean_fields when vmcs12 is dirty.

Thanks,

-- 
  Vitaly