| Message ID | 20230802234255.466782-9-oliver.upton@linux.dev (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | aarch64: Handle PSCI calls in userspace | expand |
On Wed, Aug 02, 2023 at 11:42:46PM +0000, Oliver Upton wrote: > Pausing the VM from a vCPU thread is perilous with the current helpers, > as it waits indefinitely for a signal that never comes when invoked from > a vCPU thread. Instead, add a helper for pausing the VM from a vCPU, > working around the issue by explicitly marking the caller as paused > before proceeding. > > Signed-off-by: Oliver Upton <oliver.upton@linux.dev> > --- > include/kvm/kvm-cpu.h | 3 +++ > kvm-cpu.c | 16 ++++++++++++++++ > 2 files changed, 19 insertions(+) > > diff --git a/include/kvm/kvm-cpu.h b/include/kvm/kvm-cpu.h > index 0f16f8d6e872..9a4901bf94ca 100644 > --- a/include/kvm/kvm-cpu.h > +++ b/include/kvm/kvm-cpu.h > @@ -29,4 +29,7 @@ void kvm_cpu__show_page_tables(struct kvm_cpu *vcpu); > void kvm_cpu__arch_nmi(struct kvm_cpu *cpu); > void kvm_cpu__run_on_all_cpus(struct kvm *kvm, struct kvm_cpu_task *task); > > +void kvm_cpu__pause_vm(struct kvm_cpu *vcpu); > +void kvm_cpu__continue_vm(struct kvm_cpu *vcpu); > + > #endif /* KVM__KVM_CPU_H */ > diff --git a/kvm-cpu.c b/kvm-cpu.c > index 1c566b3f21d6..9adc9d4f7841 100644 > --- a/kvm-cpu.c > +++ b/kvm-cpu.c > @@ -141,6 +141,22 @@ void kvm_cpu__run_on_all_cpus(struct kvm *kvm, struct kvm_cpu_task *task) > mutex_unlock(&task_lock); > } > > +void kvm_cpu__pause_vm(struct kvm_cpu *vcpu) > +{ > + /* > + * Mark the calling vCPU as paused to avoid waiting indefinitely for a > + * signal exit. > + */ > + vcpu->paused = true; > + kvm__pause(vcpu->kvm); > +} > + > +void kvm_cpu__continue_vm(struct kvm_cpu *vcpu) > +{ > + vcpu->paused = false; > + kvm__continue(vcpu->kvm); > +} Why is it safe to manipulate 'vcpu->paused' here without the pause_lock held? Relatedly, how does this interact with the 'pause' and 'resume' lkvm commands? Will
Hey Will, On Mon, Sep 18, 2023 at 11:40:28AM +0100, Will Deacon wrote: > On Wed, Aug 02, 2023 at 11:42:46PM +0000, Oliver Upton wrote: > > +void kvm_cpu__pause_vm(struct kvm_cpu *vcpu) > > +{ > > + /* > > + * Mark the calling vCPU as paused to avoid waiting indefinitely for a > > + * signal exit. > > + */ > > + vcpu->paused = true; > > + kvm__pause(vcpu->kvm); > > +} > > + > > +void kvm_cpu__continue_vm(struct kvm_cpu *vcpu) > > +{ > > + vcpu->paused = false; > > + kvm__continue(vcpu->kvm); > > +} > > Why is it safe to manipulate 'vcpu->paused' here without the pause_lock > held? Heh, I hacked this up to get _something_ working and never re-evaluated the locking that I completely sidestepped. > Relatedly, how does this interact with the 'pause' and 'resume' > lkvm commands? Poorly, if I had to guess. I hadn't actually tested with them. I'll take another crack at this to safely quiesce when handling calls. Thanks for having a look.
diff --git a/include/kvm/kvm-cpu.h b/include/kvm/kvm-cpu.h index 0f16f8d6e872..9a4901bf94ca 100644 --- a/include/kvm/kvm-cpu.h +++ b/include/kvm/kvm-cpu.h @@ -29,4 +29,7 @@ void kvm_cpu__show_page_tables(struct kvm_cpu *vcpu); void kvm_cpu__arch_nmi(struct kvm_cpu *cpu); void kvm_cpu__run_on_all_cpus(struct kvm *kvm, struct kvm_cpu_task *task); +void kvm_cpu__pause_vm(struct kvm_cpu *vcpu); +void kvm_cpu__continue_vm(struct kvm_cpu *vcpu); + #endif /* KVM__KVM_CPU_H */ diff --git a/kvm-cpu.c b/kvm-cpu.c index 1c566b3f21d6..9adc9d4f7841 100644 --- a/kvm-cpu.c +++ b/kvm-cpu.c @@ -141,6 +141,22 @@ void kvm_cpu__run_on_all_cpus(struct kvm *kvm, struct kvm_cpu_task *task) mutex_unlock(&task_lock); } +void kvm_cpu__pause_vm(struct kvm_cpu *vcpu) +{ + /* + * Mark the calling vCPU as paused to avoid waiting indefinitely for a + * signal exit. + */ + vcpu->paused = true; + kvm__pause(vcpu->kvm); +} + +void kvm_cpu__continue_vm(struct kvm_cpu *vcpu) +{ + vcpu->paused = false; + kvm__continue(vcpu->kvm); +} + int kvm_cpu__start(struct kvm_cpu *cpu) { sigset_t sigset;
Pausing the VM from a vCPU thread is perilous with the current helpers, as it waits indefinitely for a signal that never comes when invoked from a vCPU thread. Instead, add a helper for pausing the VM from a vCPU, working around the issue by explicitly marking the caller as paused before proceeding. Signed-off-by: Oliver Upton <oliver.upton@linux.dev> --- include/kvm/kvm-cpu.h | 3 +++ kvm-cpu.c | 16 ++++++++++++++++ 2 files changed, 19 insertions(+)