Re: [RFC patch 14/15] workpending: Provide infrastructure for work before entering a guest

From: Paolo Bonzini
Date: Thu Sep 19 2019 - 11:45:45 EST


Quick API review before I dive into the implementation.

On 19/09/19 17:03, Thomas Gleixner wrote:
> + /*
> + * Before returning to guest mode handle all pending work
> + */
> + if (ti_work & _TIF_SIGPENDING) {
> + vcpu->run->exit_reason = KVM_EXIT_INTR;
> + vcpu->stat.signal_exits++;
> + return -EINTR;
> + }
> +
> + if (ti_work & _TIF_NEED_RESCHED) {
> + srcu_read_unlock(&kvm->srcu, vcpu->srcu_idx);
> + schedule();
> + vcpu->srcu_idx = srcu_read_lock(&kvm->srcu);
> + }
> +
> + if (ti_work & _TIF_PATCH_PENDING) {
> + srcu_read_unlock(&kvm->srcu, vcpu->srcu_idx);
> + klp_update_patch_state(current);
> + vcpu->srcu_idx = srcu_read_lock(&kvm->srcu);
> + }
> +
> + if (ti_work & _TIF_NOTIFY_RESUME) {
> + srcu_read_unlock(&kvm->srcu, vcpu->srcu_idx);
> + clear_thread_flag(TIF_NOTIFY_RESUME);
> + tracehook_notify_resume(NULL);
> + vcpu->srcu_idx = srcu_read_lock(&kvm->srcu);
> + }
> +
> + /* Any extra architecture specific work */
> + return arch_exit_to_guestmode_work(kvm, vcpu, ti_work);
> +}

Perhaps, in virt/kvm/kvm_main.c:

int kvm_exit_to_guestmode_work(struct kvm *kvm, struct kvm_vcpu *vcpu,
unsigned long ti_work)
{
int r;

/*
* Before returning to guest mode handle all pending work
*/
if (ti_work & _TIF_SIGPENDING) {
vcpu->run->exit_reason = KVM_EXIT_INTR;
vcpu->stat.signal_exits++;
return -EINTR;
}

srcu_read_unlock(&kvm->srcu, vcpu->srcu_idx);
core_exit_to_guestmode_work(ti_work);
vcpu->srcu_idx = srcu_read_lock(&kvm->srcu);

return r;
}

and in kernel/entry/common.c:

int core_exit_to_guestmode_work(unsigned long ti_work)
{
/*
* Before returning to guest mode handle all pending work
*/
if (ti_work & _TIF_NEED_RESCHED)
schedule();

if (ti_work & _TIF_PATCH_PENDING)
klp_update_patch_state(current);

if (ti_work & _TIF_NOTIFY_RESUME) {
clear_thread_flag(TIF_NOTIFY_RESUME);
tracehook_notify_resume(NULL);
}
return arch_exit_to_guestmode_work(ti_work);
}

so that kernel/entry/ is not polluted with KVM structs and APIs.

Perhaps even extract the body of core_exit_to_usermode_work's while loop
to a separate function, and call it as

core_exit_to_usermode_work_once(NULL,
ti_work & EXIT_TO_GUESTMODE_WORK);

from core_exit_to_guestmode_work.

In general I don't mind having these exit_to_guestmode functions in
kvm_host.h, and only having entry-common.h export EXIT_TO_GUESTMODE_WORK
and ARCH_EXIT_TO_GUESTMODE_WORK. Unless you had good reasons to do the
opposite...

Paolo