nvmx: get/set_nested_state ignores VM_EXIT_INSTRUCTION_LEN

nvmx: get/set_nested_state ignores VM_EXIT_INSTRUCTION_LEN

[Jan Kiszka]

Hi all,

made some progress understanding why vmport from L2 breaks since QEMU gets/sets
the nested state around it: We do not preserve VM_EXIT_INSTRUCTION_LEN, and that
breaks skip_emulated_instruction when completing the PIO access on next run. The
field is suddenly 0, and so we loop infinitely over the IO instruction. Unless
some other magic prevents migration while an IO instruction is in flight, vmport
may not be the only victim here.

Now the question is how to preserve that information: Can we restore the value
into vmcs02 on set_nested_state, despite this field being read-only? Or do we
need to cache its content and use that instead in skip_emulated_instruction?

Looking at this pattern, I wonder if there is more. What other fields are used
across PIO or MMIO when the handling is done by userland?

Jan

Re: nvmx: get/set_nested_state ignores VM_EXIT_INSTRUCTION_LEN

[Paolo Bonzini]

On 21/07/19 19:40, Jan Kiszka wrote:
> Hi all,
> 
> made some progress understanding why vmport from L2 breaks since QEMU gets/sets
> the nested state around it: We do not preserve VM_EXIT_INSTRUCTION_LEN, and that
> breaks skip_emulated_instruction when completing the PIO access on next run. The
> field is suddenly 0, and so we loop infinitely over the IO instruction. Unless
> some other magic prevents migration while an IO instruction is in flight, vmport
> may not be the only victim here.
> 
> Now the question is how to preserve that information: Can we restore the value
> into vmcs02 on set_nested_state, despite this field being read-only? Or do we
> need to cache its content and use that instead in skip_emulated_instruction?

Hmm I think technically this is invalid, since you're not supposed to
modify state at all while MMIO is pending.  Of course that's kinda moot
if it's the only way to emulate vmport, but perhaps we can (or even
should!) fix it in QEMU.  Is KVM_SET_NESTED_STATE needed for level <
KVM_PUT_RESET_STATE?  Even if it is, we should first complete I/O and
then do kvm_arch_put_registers.

> Looking at this pattern, I wonder if there is more. What other fields are used
> across PIO or MMIO when the handling is done by userland?

Re: nvmx: get/set_nested_state ignores VM_EXIT_INSTRUCTION_LEN

[Jan Kiszka]

On 21.07.19 21:14, Paolo Bonzini wrote:
> On 21/07/19 19:40, Jan Kiszka wrote:
>> Hi all,
>>
>> made some progress understanding why vmport from L2 breaks since QEMU gets/sets
>> the nested state around it: We do not preserve VM_EXIT_INSTRUCTION_LEN, and that
>> breaks skip_emulated_instruction when completing the PIO access on next run. The
>> field is suddenly 0, and so we loop infinitely over the IO instruction. Unless
>> some other magic prevents migration while an IO instruction is in flight, vmport
>> may not be the only victim here.
>>
>> Now the question is how to preserve that information: Can we restore the value
>> into vmcs02 on set_nested_state, despite this field being read-only? Or do we
>> need to cache its content and use that instead in skip_emulated_instruction?
>
> Hmm I think technically this is invalid, since you're not supposed to
> modify state at all while MMIO is pending.  Of course that's kinda moot
> if it's the only way to emulate vmport, but perhaps we can (or even
> should!) fix it in QEMU.  Is KVM_SET_NESTED_STATE needed for level <
> KVM_PUT_RESET_STATE?  Even if it is, we should first complete I/O and
> then do kvm_arch_put_registers.

Are we sure that vmport is the only case? What prevents a migration from
starting in the middle of an IO exit?

And I suspect we need to enhance the API specification then.

Jan

Re: nvmx: get/set_nested_state ignores VM_EXIT_INSTRUCTION_LEN

[Paolo Bonzini]

On 21/07/19 21:31, Jan Kiszka wrote:
> On 21.07.19 21:14, Paolo Bonzini wrote:
>> On 21/07/19 19:40, Jan Kiszka wrote:
>>> Hi all,
>>>
>>> made some progress understanding why vmport from L2 breaks since QEMU gets/sets
>>> the nested state around it: We do not preserve VM_EXIT_INSTRUCTION_LEN, and that
>>> breaks skip_emulated_instruction when completing the PIO access on next run. The
>>> field is suddenly 0, and so we loop infinitely over the IO instruction. Unless
>>> some other magic prevents migration while an IO instruction is in flight, vmport
>>> may not be the only victim here.
>>>
>>> Now the question is how to preserve that information: Can we restore the value
>>> into vmcs02 on set_nested_state, despite this field being read-only? Or do we
>>> need to cache its content and use that instead in skip_emulated_instruction?
>>
>> Hmm I think technically this is invalid, since you're not supposed to
>> modify state at all while MMIO is pending.  Of course that's kinda moot
>> if it's the only way to emulate vmport, but perhaps we can (or even
>> should!) fix it in QEMU.  Is KVM_SET_NESTED_STATE needed for level <
>> KVM_PUT_RESET_STATE?  Even if it is, we should first complete I/O and
>> then do kvm_arch_put_registers.
> 
> Are we sure that vmport is the only case? What prevents a migration from
> starting in the middle of an IO exit?

Migration syncs with the CPU thread via pause_all_vcpus(), after which
the CPU thread must be in qemu_wait_io_event.  KVM_SET_NESTED_STATE
should definitely not be part of KVM_PUT_RUNTIME_STATE.

Paolo