Re: [PATCH v2 3/7] KVM: nVMX: Ignore 'hv_clean_fields' data when eVMCS data is copied in vmx_get_nested_state()

[Maxim Levitsky]

On Mon, 2021-05-24 at 15:58 +0200, Paolo Bonzini wrote:
> On 24/05/21 15:01, Vitaly Kuznetsov wrote:
> > With 'need_vmcs12_to_shadow_sync', we treat eVMCS as shadow VMCS which
> > happens to shadow all fields and while it may not be the most optimal
> > solution, it is at least easy to comprehend. We can try drafting
> > something up instead, maybe it will also be good but honestly I'm afraid
> > of incompatible changes in KVM_GET_NESTED_STATE/KVM_SET_NESTED_STATE, we
> > can ask Paolo's opinion on that.
> 
> Yes, it's much easier to understand it if the eVMCS is essentially a 
> memory-backed shadow VMCS, than if it's really the vmcs12 format.  I 
> understand that it's bound to be a little slower, but at least the two 
> formats are not all over the place.
> 
> Paolo
> 

Hi!

Please see my other reply to this in patch 1.
 
I understand this concern, but what bugs me is that we sort of 
shouldn't read evmcs while L1 is running.
(e.g its clean bits might be not up to date and
such).
 
Actually instead of thinking of evmcs as a shadow, I am thinking of it
more as a vmcb12 (the SVM one), 
which we load when we do a nested entry and which is then
updated when we do a nested vmexit, and other than that, while
L1 is running, we don't touch it.
 
Yes there is that vm instruction error field in evmcs which I suppose we should
write when we fail a VMX instruction (invept only practically I think) 
while we just run L1, and even that we might just avoid doing, 
which will allow us to avoid even keeping
the evmcs mapped while L1 is running.

Just my 0.2 cents.

Best regards,
	Maxim Levitsky