Re: [PATCH 6/6] KVM: assigned dev: MSI-X mask support

[Avi Kivity <avi@redhat.com>]

On 11/18/2010 03:58 AM, Sheng Yang wrote:
> On Wednesday 17 November 2010 21:58:00 Avi Kivity wrote:
> >  On 11/15/2010 11:15 AM, Sheng Yang wrote:
> >  >  This patch enable per-vector mask for assigned devices using MSI-X.
> >  >
> >  >  This patch provided two new APIs: one is for guest to specific device's
> >  >  MSI-X table address in MMIO, the other is for userspace to get
> >  >  information about mask bit.
> >  >
> >  >  All the mask bit operation are kept in kernel, in order to accelerate.
> >  >  Userspace shouldn't access the device MMIO directly for the information,
> >  >  instead it should uses provided API to do so.
> >  >
> >  >  Signed-off-by: Sheng Yang<sheng@linux.intel.com>
> >  >  ---
> >  >
> >  >    arch/x86/kvm/x86.c       |    1 +
> >  >    include/linux/kvm.h      |   32 +++++
> >  >    include/linux/kvm_host.h |    5 +
> >  >    virt/kvm/assigned-dev.c  |  318
> >  >    +++++++++++++++++++++++++++++++++++++++++++++- 4 files changed,
> 355
> >  >    insertions(+), 1 deletions(-)
> >
> >  Documentation?
>
> For we are keeping changing the API for last several versions, I'd like to settle
> down the API first. Would bring back the document after API was agreed.

Maybe for APIs we should start with only the documentation patch, agree 
on that, and move on to the implementation.

> >  What if it's a 64-bit write on a 32-bit host?
>
> In fact we haven't support QWORD(64bit) accessing now. The reason is we haven't
> seen any OS is using it in this way now, so I think we can leave it later.
>
> Also seems QEmu doesn't got the way to handle 64bit MMIO.

There's a difference, if the API doesn't support it, we can't add it 
later without changing both kernel and userspace.

> >
> >  That's not very good.  We should do the entire thing in the kernel or in
> >  userspace.  We can have a new EXIT_REASON to let userspace know an msix
> >  entry changed, and it should read it from the kernel.
>
> If you look it in this way:
> 1. Mask bit owned by kernel.
> 2. Routing owned by userspace.
> 3. Read the routing in kernel is an speed up for normal operation - because kernel
> can read from them.
>
> So I think the logic here is clear to understand.

Still, it's complicated and the state is split across multiple components.

> But if we can modify the routing in kernel, it would be raise some sync issues due
> to both kernel and userspace own routing. So maybe the better solution is move the
> routing to kernel.

That may work, but I don't think we can do this for vfio.

-- 
error compiling committee.c: too many arguments to function