On Mon, 2020-11-30 at 20:46 +0000, Sean Christopherson wrote:
> On Sat, Nov 28, 2020, David Woodhouse wrote:
>
> ...
>
> > +static int complete_userspace_hypercall(struct kvm_vcpu *vcpu)
> > +{
> > + kvm_rax_write(vcpu, vcpu->run->hypercall.ret);
> > +
> > + return kvm_skip_emulated_instruction(vcpu);
>
> This should probably verify the linear RIP is unchanged before modifying guest
> state, e.g. to let userspace reset the vCPU in response to a hypercall. See
> complete_fast_pio_out().
Yeah, I wondered about "what happens if userspace changes state instead
of just handling the hypercall", and didn't see that other users were
doing anything about that. At least hvm_hv_hypercall_complete(),
complete_emulated_mmio() and completed_emulated_pio() don't seem to be.
Is that a check which should be lifted up to the generic level and done
*before* kvm_arch_vcpu_ioctl_run() calls ->arch.complete_userspace_io
instead of trusting all the functions to do it for themselves?
> > +int kvm_userspace_hypercall(struct kvm_vcpu *vcpu)
> > +{
> > + struct kvm_run *run = vcpu->run;
> > +
> > + if (is_long_mode(vcpu)) {
> > + run->hypercall.longmode = 1;
>
> Should this also capture the CPL? I assume the motivation for grabbing regs
> and longmode is to avoid having to call back into KVM on every hypercall, and I
> also assume there are (or will be) hypercalls that are CPL0 only.
Yes, good point.
Xen *does* allow one hypercall (HVMOP_guest_request_vm_event) from
!CPL0 so I don't think I can get away with allowing only CPL0 like
kvm_hv_hypercall() does.
So unless I'm going to do that filtering in kernel (ick) I think we do
need to capture and pass up the CPL, as you say. Which means it doesn't
fit in the existing kvm_run->hypercall structure unless I'm prepared to
rename/abuse the ->hypercall.pad for it.
I'll just use Joao's version of the hypercall support, and add cpl to
struct kvm_xen_exit.hcall. Userspace can inject the UD# where
appropriate.
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
