> On 16 Jan 2020, at 2:16, Jim Mattson <jmattson@xxxxxxxxxx> wrote:
>
> Beginning with commit 0b9f6c4615c99 ("x86/kvm: Support the vCPU
> preemption check"), the KVM_VCPU_PREEMPTED flag is set in the guest
> copy of the kvm_steal_time struct on every call to vcpu_put(). As a
> result, guest memory is dirtied on every call to vcpu_put(), even when
> the VM is quiescent.
>
> To avoid dirtying guest memory unnecessarily, don't bother setting the
> flag in the guest copy of the struct if it is already set in the
> kernel copy of the struct.
I suggest adding this comment to code as-well.
>
> If a different vCPU thread clears the guest copy of the flag, it will
> no longer get reset on the next call to vcpu_put, but it's not clear
> that resetting the flag in this case was intentional to begin with.
I agree… I find it hard to believe that guest vCPU is allowed to clear the flag
and expect host to set it again on the next vcpu_put() call. Doesn’t really make sense.
>
> Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
> Tested-by: Kevin Mcgaire <kevinmcgaire@xxxxxxxxxx>
> Reviewed-by: Ben Gardon <bgardon@xxxxxxxxxx>
> Reviewed-by: Oliver Upton <oupton@xxxxxxxxxx>
Good catch.
Reviewed-by: Liran Alon <liran.alon@xxxxxxxxxx>
-Liran
>
> ---
> arch/x86/kvm/x86.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index cf917139de6b..3dc17b173f88 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3504,6 +3504,9 @@ static void kvm_steal_time_set_preempted(struct kvm_vcpu *vcpu)
> if (!(vcpu->arch.st.msr_val & KVM_MSR_ENABLED))
> return;
>
> + if (vcpu->arch.st.steal.preempted & KVM_VCPU_PREEMPTED)
> + return;
> +
> vcpu->arch.st.steal.preempted = KVM_VCPU_PREEMPTED;
>
> kvm_write_guest_offset_cached(vcpu->kvm, &vcpu->arch.st.stime,
> --
> 2.25.0.rc1.283.g88dfdc4193-goog
>
