On 04/30/2018 01:07 PM, Andrew Cooper wrote:
> On 30/04/18 12:37, Jan Beulich wrote:
>> While the main problem to be addressed here is the issue of what so far
>> was named "vmcb_in_sync" starting out with the wrong value (should have
>> been true instead of false, to prevent performing a VMSAVE without ever
>> having VMLOADed the vCPU's state), go a step further and make the
>> sync-ed state a tristate: CPU and memory may be in sync or an update
>> may be required in either direction. Rename the field and introduce an
>> enum. Callers of svm_sync_vmcb() now indicate the intended new state
>> (with a slight "anomaly" when requesting VMLOAD: we could store
>> vmcb_needs_vmsave in those cases as the callers request, but the VMCB
>> really is in sync at that point, and hence there's no need to VMSAVE in
>> case we don't make it out to guest context), and all syncing goes
>> through that function.
>>
>> With that, there's no need to VMLOAD the state perhaps multiple times;
>> all that's needed is loading it once before VM entry.
>>
>> Signed-off-by: Jan Beulich <[email protected]>
>> ---
>> v2: Also handle VMLOAD in svm_sync_vmcb(). Add comment to enum
>> vmcb_sync_state.
> -1 from me. This is even more confusing to use than v1.
>
> It is not obvious at all that using svm_sync_vmcb(v, vmcb_needs_vmsave);
> means "vmload", and its actively wrong that the state doesn't remain
> in-sync.
It does become in-sync:
+ if ( new_state == vmcb_needs_vmsave )
+ {
+ ASSERT(arch_svm->vmcb_sync_state == vmcb_needs_vmload);
+ svm_vmload(arch_svm->vmcb);
+ arch_svm->vmcb_sync_state = vmcb_in_sync;
+ }
+ else
(although Jan is questioning whether to drop that change in the comments to
patch 2, if I understood him correctly)
-boris
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
