| Message ID | 20250211073852.571625-2-aneesh.kumar@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [kvmtool,1/2] cpu: vmexit: Handle KVM_EXIT_UNKNOWN exit reason correctly | expand |
On Tue, Feb 11, 2025 at 01:08:52PM +0530, Aneesh Kumar K.V (Arm) wrote: > Linux kernel documentation states: > > "Note! KVM_EXIT_MEMORY_FAULT is unique among all KVM exit reasons in > that it accompanies a return code of '-1', not '0'! errno will always be > set to EFAULT or EHWPOISON when KVM exits with KVM_EXIT_MEMORY_FAULT, > userspace should assume kvm_run.exit_reason is stale/undefined for all > other error numbers." " > > Update the KVM_RUN ioctl error handling to correctly handle > KVM_EXIT_MEMORY_FAULT. > > Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org> > --- > kvm-cpu.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/kvm-cpu.c b/kvm-cpu.c > index 66e30ba54e26..40e4efc33a1d 100644 > --- a/kvm-cpu.c > +++ b/kvm-cpu.c > @@ -41,8 +41,15 @@ void kvm_cpu__run(struct kvm_cpu *vcpu) > return; > > err = ioctl(vcpu->vcpu_fd, KVM_RUN, 0); > - if (err < 0 && (errno != EINTR && errno != EAGAIN)) > - die_perror("KVM_RUN failed"); > + if (err < 0) { > + if (errno == EINTR || errno == EAGAIN) > + return; > + else if (errno == EFAULT && > + vcpu->kvm_run->exit_reason == KVM_EXIT_MEMORY_FAULT) > + return; > + else > + die_perror("KVM_RUN failed"); > + } Probably cleaner to switch on errno? Will
Hi, On Tue, Feb 11, 2025 at 01:08:52PM +0530, Aneesh Kumar K.V (Arm) wrote: > Linux kernel documentation states: > > "Note! KVM_EXIT_MEMORY_FAULT is unique among all KVM exit reasons in > that it accompanies a return code of '-1', not '0'! errno will always be > set to EFAULT or EHWPOISON when KVM exits with KVM_EXIT_MEMORY_FAULT, > userspace should assume kvm_run.exit_reason is stale/undefined for all > other error numbers." " > > Update the KVM_RUN ioctl error handling to correctly handle > KVM_EXIT_MEMORY_FAULT. I've tried to follow how kvmtool handles KVM_EXIT_MEMORY_FAULT before and after this patch. Before: calls die_perror(). After: prints more information about the error, in kvm_cpu_thread(). Is that what you want? Because "correctly handle KVM_EXIT_MEMORY_FAULT" can be interpreted as kvmtool resolving the memory fault, which is something that kvmtool does not do. Also, can you update kvm_exit_reasons with KVM_EXIT_MEMORY_FAULT, because otherwise kvm_cpu_thread() will segfault when it tries to access kvm_exit_reasons[KVM_EXIT_MEMORY_FAULT]. Thanks, Alex > > Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org> > --- > kvm-cpu.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/kvm-cpu.c b/kvm-cpu.c > index 66e30ba54e26..40e4efc33a1d 100644 > --- a/kvm-cpu.c > +++ b/kvm-cpu.c > @@ -41,8 +41,15 @@ void kvm_cpu__run(struct kvm_cpu *vcpu) > return; > > err = ioctl(vcpu->vcpu_fd, KVM_RUN, 0); > - if (err < 0 && (errno != EINTR && errno != EAGAIN)) > - die_perror("KVM_RUN failed"); > + if (err < 0) { > + if (errno == EINTR || errno == EAGAIN) > + return; > + else if (errno == EFAULT && > + vcpu->kvm_run->exit_reason == KVM_EXIT_MEMORY_FAULT) > + return; > + else > + die_perror("KVM_RUN failed"); > + } > } > > static void kvm_cpu_signal_handler(int signum) > -- > 2.43.0 > >
Alexandru Elisei <alexandru.elisei@arm.com> writes: > Hi, > > On Tue, Feb 11, 2025 at 01:08:52PM +0530, Aneesh Kumar K.V (Arm) wrote: >> Linux kernel documentation states: >> >> "Note! KVM_EXIT_MEMORY_FAULT is unique among all KVM exit reasons in >> that it accompanies a return code of '-1', not '0'! errno will always be >> set to EFAULT or EHWPOISON when KVM exits with KVM_EXIT_MEMORY_FAULT, >> userspace should assume kvm_run.exit_reason is stale/undefined for all >> other error numbers." " >> >> Update the KVM_RUN ioctl error handling to correctly handle >> KVM_EXIT_MEMORY_FAULT. > > I've tried to follow how kvmtool handles KVM_EXIT_MEMORY_FAULT before and after > this patch. > > Before: calls die_perror(). > After: prints more information about the error, in kvm_cpu_thread(). > > Is that what you want? Because "correctly handle KVM_EXIT_MEMORY_FAULT" can be > interpreted as kvmtool resolving the memory fault, which is something that > kvmtool does not do. > That is correct. The changes to enable the handling of KVM_EXIT_MEMORY_FAULT is not yet part of upstream [1]. But then the return value for KVM_RUN ioctl() is defined as part of Documentation/virt/kvm/api.rst in the Linux kernel. [1] https://gitlab.arm.com/linux-arm/kvmtool-cca/-/blob/cca/v4/arm/kvm-cpu.c?ref_type=heads#L247 > > Also, can you update kvm_exit_reasons with KVM_EXIT_MEMORY_FAULT, because > otherwise kvm_cpu_thread() will segfault when it tries to access > kvm_exit_reasons[KVM_EXIT_MEMORY_FAULT]. > Will update. -aneesh
Will Deacon <will@kernel.org> writes: > On Tue, Feb 11, 2025 at 01:08:52PM +0530, Aneesh Kumar K.V (Arm) wrote: >> Linux kernel documentation states: >> >> "Note! KVM_EXIT_MEMORY_FAULT is unique among all KVM exit reasons in >> that it accompanies a return code of '-1', not '0'! errno will always be >> set to EFAULT or EHWPOISON when KVM exits with KVM_EXIT_MEMORY_FAULT, >> userspace should assume kvm_run.exit_reason is stale/undefined for all >> other error numbers." " >> >> Update the KVM_RUN ioctl error handling to correctly handle >> KVM_EXIT_MEMORY_FAULT. >> >> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org> >> --- >> kvm-cpu.c | 11 +++++++++-- >> 1 file changed, 9 insertions(+), 2 deletions(-) >> >> diff --git a/kvm-cpu.c b/kvm-cpu.c >> index 66e30ba54e26..40e4efc33a1d 100644 >> --- a/kvm-cpu.c >> +++ b/kvm-cpu.c >> @@ -41,8 +41,15 @@ void kvm_cpu__run(struct kvm_cpu *vcpu) >> return; >> >> err = ioctl(vcpu->vcpu_fd, KVM_RUN, 0); >> - if (err < 0 && (errno != EINTR && errno != EAGAIN)) >> - die_perror("KVM_RUN failed"); >> + if (err < 0) { >> + if (errno == EINTR || errno == EAGAIN) >> + return; >> + else if (errno == EFAULT && >> + vcpu->kvm_run->exit_reason == KVM_EXIT_MEMORY_FAULT) >> + return; >> + else >> + die_perror("KVM_RUN failed"); >> + } > > Probably cleaner to switch on errno? This? . I will update. if (err < 0) { switch (errno) { case EINTR: case EAGAIN: return; case EFAULT: if (vcpu->kvm_run->exit_reason == KVM_EXIT_MEMORY_FAULT) return; /* fallthrough */ default: die_perror("KVM_RUN failed"); } } -aneesh
diff --git a/kvm-cpu.c b/kvm-cpu.c index 66e30ba54e26..40e4efc33a1d 100644 --- a/kvm-cpu.c +++ b/kvm-cpu.c @@ -41,8 +41,15 @@ void kvm_cpu__run(struct kvm_cpu *vcpu) return; err = ioctl(vcpu->vcpu_fd, KVM_RUN, 0); - if (err < 0 && (errno != EINTR && errno != EAGAIN)) - die_perror("KVM_RUN failed"); + if (err < 0) { + if (errno == EINTR || errno == EAGAIN) + return; + else if (errno == EFAULT && + vcpu->kvm_run->exit_reason == KVM_EXIT_MEMORY_FAULT) + return; + else + die_perror("KVM_RUN failed"); + } } static void kvm_cpu_signal_handler(int signum)
Linux kernel documentation states: "Note! KVM_EXIT_MEMORY_FAULT is unique among all KVM exit reasons in that it accompanies a return code of '-1', not '0'! errno will always be set to EFAULT or EHWPOISON when KVM exits with KVM_EXIT_MEMORY_FAULT, userspace should assume kvm_run.exit_reason is stale/undefined for all other error numbers." " Update the KVM_RUN ioctl error handling to correctly handle KVM_EXIT_MEMORY_FAULT. Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org> --- kvm-cpu.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)