Re: [Qemu-devel] [PATCH] i386/kvm: Do not sync nested state during runtime

[Jan Kiszka]

On 22.07.19 12:31, Liran Alon wrote:
>> On 22 Jul 2019, at 13:20, Jan Kiszka <address@hidden> wrote:
>>
>> On 22.07.19 11:44, Liran Alon wrote:
>>>
>>>
>>>> On 22 Jul 2019, at 7:00, Jan Kiszka <address@hidden> wrote:
>>>>
>>>> Writing the nested state e.g. after a vmport access can invalidate
>>>> important parts of the kernel-internal state, and it is not needed as
>>>> well. So leave this out from KVM_PUT_RUNTIME_STATE.
>>>>
>>>> Suggested-by: Paolo Bonzini <address@hidden>
>>>> Signed-off-by: Jan Kiszka <address@hidden>
>>>
>>> As QEMU never modifies vCPU nested-state in userspace besides in vmload and 
>>> vCPU creation,
>>> shouldn’t this be under KVM_PUT_FULL_STATE? Same as the call to 
>>> kvm_arch_set_tsc_khz().
>>
>> Reset is a relevant modification as well. If we do not write back a state 
>> that
>> is disabling virtualization, we break.
>>
>> Jan
> 
> Currently QEMU writes to userspace maintained nested-state only at 
> kvm_arch_init_vcpu() and
> when loading vmstate_nested_state vmstate subsection.
> kvm_arch_reset_vcpu() do not modify userspace maintained nested-state.

Hmm, then we probably achieve that effect by clearing the related bit in CR4. If
doing that implies an invalidation of the nested state by KVM, we are fine.
Otherwise, I would expect userspace to reset the state to VMCLEAR and purge any
traces of prior use.

> 
> I would expect KVM internal nested-state to be reset as part of KVM’s 
> vmx_vcpu_reset().

vmx_vcpu_reset is not issued on userspace initiated VM reset, only on INIT/SIPI
cycles. It's misleading, and I remember myself running into that when I hacked
on KVM back then.

Jan

> Are we missing a call to vmx_leave_nested() there? 
> 
> -Liran
> 

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux