> On 14 Sep 2018, at 18:08, Paolo Bonzini <pbonz...@redhat.com> wrote:
>
> On 14/09/2018 16:31, Liran Alon wrote:
>>> There is still a problem, however, in that the same input stream would
>>> be parsed differently depending on the kernel version. In particular,
>>> if in the future the maximum nested state size grows, you break all
>>> existing save files.
>>
>> I’m not sure I agree with this.
>> 1) Newer kernels should change struct only by utilizing unused fields in
>> current struct
>> or enlarging it with extra fields. It should not change the meaning of
>> existing fields.
>
> Newer kernels will return a larger size, which is stored in
> env->nested_state_len, and the file format depends on it:
>
>> + VMSTATE_VBUFFER_UINT32(env.nested_state, X86CPU,
>> + 0, NULL,
>> + env.nested_state_len),
>
Oh. I thought that QEMU will just receive to dest buffer only what was sent
from source buffer.
I didn’t know that it also enforces that the sizes of the source and dest
buffer are equal.
(I thought that dest_buffer_size only needed to be >= src_buffer_size).
Anyway, my intention here was that QEMU will only enforce (dest_buffer_size >=
source_buffer_size)
and if so, receive source buffer into destination buffer.
Is there a simple way to do this in QEMU’s VMSTATE framework without
implementing custom save/load callbacks?
>>>
>>> By defining more HF flags. I'd rather avoid having multiple ways to
>>> store the same thing, in case for example in the future HVF implements
>>> nested virt.
>>
>> I agree it is possible to define more hflags and to translate
>> kvm_nested_flags->flags to flags in hflags and vice-versa.
>> However, I am not sure I understand the extra value provided by doing so.
>> I think it makes more sense to rename struct kvm_nested_state to struct
>> nested_state and
>> use this as a generic interface to get/set nested_state for all hypervisors.
>> If a given hypervisor, such as HVF, needs to store different blob than KVM
>> VMX, it can just add another struct field to
>> the union-part of struct kvm_nested_state.
>> This way, the code that handles the save/restore of nested_state can remain
>> generic for all hypervisors.
>
> I agree that the value is small, but it's there. For example, the
> debugging commands support AMD nested paging, and sharing the flags
> means that it works for KVM too, not just TCG. So I'd prefer to do it
> the same way for Intel too.
Can you explain this example? I’m not sure I follow.
In the solution I proposed above, the nested_state fixed-size header is also
shared between hypervisors.
Thus, flags here are shared exactly the same as hflags are shared.
Only the blob union-part is not necessarely shared between hypervisors (Which
makes sense as it may differ).
Am I missing something trivial?
-Liran
>
> Paolo
