Re: [PATCH 2/6] KVM: Add KVM_PRE_FAULT_MEMORY vcpu ioctl to pre-populate guest memory

Paolo Bonzini
2024-04-24T16:06:13Z

On Mon, Apr 22, 2024 at 7:39 AM Binbin Wu <binbin.wu@linux.intel.com> wrote:
> range->size equals 0 can be covered by "range->gpa + range->size <=
> range->gpa"
>
> If we want to return success when size is 0 (, though I am not sure it's
> needed),
> we need to use "range->gpa + range->size < range->gpa" instead.

I think it's not needed because it could cause an infinite loop in
(buggy) userspace. Better return -EINVAL.

Paolo

>
> > +
> > + vcpu_load(vcpu);
> > + idx = srcu_read_lock(&vcpu->kvm->srcu);
> > +
> > + full_size = range->size;
> > + do {
> > + if (signal_pending(current)) {
> > + r = -EINTR;
> > + break;
> > + }
> > +
> > + r = kvm_arch_vcpu_pre_fault_memory(vcpu, range);
> > + if (r < 0)
> > + break;
> > +
> > + if (WARN_ON_ONCE(r == 0))
> > + break;
> > +
> > + range->size -= r;
> > + range->gpa += r;
> > + cond_resched();
> > + } while (range->size);
> > +
> > + srcu_read_unlock(&vcpu->kvm->srcu, idx);
> > + vcpu_put(vcpu);
> > +
> > + /* Return success if at least one page was mapped successfully. */
> > + return full_size == range->size ? r : 0;
> > +}
> > +#endif
> > +
> > static long kvm_vcpu_ioctl(struct file *filp,
> > unsigned int ioctl, unsigned long arg)
> > {
> > @@ -4580,6 +4629,20 @@ static long kvm_vcpu_ioctl(struct file *filp,
> > r = kvm_vcpu_ioctl_get_stats_fd(vcpu);
> > break;
> > }
> > +#ifdef CONFIG_KVM_GENERIC_PRE_FAULT_MEMORY
> > + case KVM_PRE_FAULT_MEMORY: {
> > + struct kvm_pre_fault_memory range;
> > +
> > + r = -EFAULT;
> > + if (copy_from_user(&range, argp, sizeof(range)))
> > + break;
> > + r = kvm_vcpu_pre_fault_memory(vcpu, &range);
> > + /* Pass back leftover range. */
> > + if (copy_to_user(argp, &range, sizeof(range)))
> > + r = -EFAULT;
> > + break;
> > + }
> > +#endif
> > default:
> > r = kvm_arch_vcpu_ioctl(filp, ioctl, arg);
> > }
>