> On 14 Sep 2018, at 18:08, Paolo Bonzini <pbonzini@xxxxxxxxxx> 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
