Re: [PATCH 2/3] KVM: Emulate MSI-X table in kernel

[Marcelo Tosatti <mtosatti@redhat.com>]

On Mon, Jan 17, 2011 at 02:51:37PM +0200, Avi Kivity wrote:
> On 01/17/2011 02:48 PM, Marcelo Tosatti wrote:
> >On Mon, Jan 17, 2011 at 02:18:43PM +0200, Avi Kivity wrote:
> >>  On 01/17/2011 02:18 PM, Sheng Yang wrote:
> >>  >>   >   +
> >>  >>   >   +	if (copy_to_user((void __user *)(entry_base + offset), val, len))
> >>  >>   >   +		goto out;
> >>  >>
> >>  >>   Instead of copying to/from userspace (which is subject to swapin,
> >>  >>   unexpected values), you could include the guest written value in a
> >>  >>   kvm_run structure, along with address. Qemu-kvm would use that to
> >>  >>   synchronize its copy of the table, on KVM_EXIT_MSIX_ROUTING_UPDATE exit.
> >>  >
> >>  >We want to acelerate MSI-X mask bit accessing, which won't exit to userspace in
> >>  >the most condition. That's the cost we want to optimize. Also it's possible to
> >>  >userspace to read the correct value of MMIO(but mostly userspace can't write to it
> >>  >in order to prevent synchronize issue).
> >>
> >>  It's also good to have the values in just one place; using userspace
> >>  makes it easy for both the kernel and userspace to see the values
> >>  (and set them after migration, if/when we extend this to virtio).
> >
> >Right, thats an advantage, but:
> >
> >- How can userspace ever synchronize with updates by the kernel
> >   to the MSI-X entry?
> 
> What a value is written by the guest, which kvm cannot handle itself
> (i.e. a change to anything other than the mask bit), we exit with
> the table and entry ids, so userspace can reread them.

OK. But regarding access to the MSI-X entry in userspace, it can 
only be accessed safely wrt parallel updates by the kernel in the
exit handler.

Is the exit handler the only location where the MSI-X entry will be
read or written to, in userspace?

> >- Reading/writing to the userspace area must be done carefully,
> >   values must be validated before used.
> 
> True every time...
> 
> >- Swapping issue (minor?).
> 
> I don't see the issue... just like any part of qemu that may be
> swapped out, blocking the vcpu thread.