On 15/09/2018 22:48, Liran Alon wrote: > > >> 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). It's much less smarter than that. :) If env.nested_state_len is 12345, it reads 12345 bytes. There's no attempt at type checking. > Is there a simple way to do this in QEMU’s VMSTATE framework without implementing custom save/load callbacks? You can store the source code size and use VMSTATE_VALIDATE to test it against the KVM capability. >> 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? There is already a "hypervisor" (TCG) that implements AMD nested paging, so it makes sense to reuse the place that TCG uses for its hflags. Paolo
