lkml.org 
[lkml]   [2015]   [Sep]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH V3 2/3] kvm: don't register wildcard MMIO EVENTFD on two buses
    From
    Date


    On 09/01/2015 02:54 PM, Michael S. Tsirkin wrote:
    > On Tue, Sep 01, 2015 at 12:47:36PM +0800, Jason Wang wrote:
    >>
    >> On 09/01/2015 12:31 PM, Michael S. Tsirkin wrote:
    >>> On Tue, Sep 01, 2015 at 11:33:43AM +0800, Jason Wang wrote:
    >>>> On 08/31/2015 07:33 PM, Michael S. Tsirkin wrote:
    >>>>> On Mon, Aug 31, 2015 at 04:03:59PM +0800, Jason Wang wrote:
    >>>>>>> On 08/31/2015 03:29 PM, Michael S. Tsirkin wrote:
    >>>>>>>>>>>>>>>>> Thinking more about this, invoking the 0-length write after
    >>>>>>>>>>>>>>>>>>>>>>>>> the != 0 length one would be better: it would mean we only
    >>>>>>>>>>>>>>>>>>>>>>>>> handle the userspace MMIO like this.
    >>>>>>>>>>>>>>>>> Right.
    >>>>>>>>>>>>>>>>>
    >>>>>>>>>>>>> Using current unittest. This patch is about 2.9% slower than before, and
    >>>>>>>>>>>>> invoking 0-length write after is still 1.1% slower (mmio-datamatch-eventfd).
    >>>>>>>>>>>>>
    >>>>>>>>>>>>> /patch/result/-+%/
    >>>>>>>>>>>>> /base/2957/0/
    >>>>>>>>>>>>> /V3/3043/+2.9%/
    >>>>>>>>>>>>> /V3+invoking != 0 length first/2990/+1.1%/
    >>>>>>>>>>>>>
    >>>>>>>>>>>>> So looks like the best method is not searching KVM_FAST_MMIO_BUS during
    >>>>>>>>>>>>> KVM_MMIO_BUS. Instead, let userspace to register both datamatch and
    >>>>>>>>>>>>> wildcard in this case. Does this sound good to you?
    >>>>>>>>> No - we can't change userspace.
    >>>>>>> Actually, the change was as simple as following. So I don't get the
    >>>>>>> reason why.
    >>>>> Because it's too late - we committed to a specific userspace ABI
    >>>>> when this was merged in kernel, we must maintain it.
    >>>> Ok ( Though I don't think it has real users for this now because it was
    >>>> actually broken).
    >>> It actually worked most of the time - you only trigger a use after free
    >>> on deregister.
    >>>
    >> It doesn't work for amd and intel machine without ept.
    > I thought it does :(
    >
    >>>>> Even if I thought yours is a good API (and I don't BTW - it's exposing
    >>>>> internal implementation details) it's too late to change it.
    >>>> I believe we should document the special treatment in kernel of zero
    >>>> length mmio eventfd in api.txt? If yes, is this an exposing? If not, how
    >>>> can userspace know the advantages of this and use it? For better API,
    >>>> probably we need another new flag just for fast mmio and obsolete
    >>>> current one by failing the assigning for zero length mmio eventfd.
    >>> I sent a patch to update api.txt already as part of
    >>> kvm: add KVM_CAP_IOEVENTFD_PF capability.
    >>> I should probably split it out.
    >>>
    >>> Sorry, I don't think the api change you propose makes sense - just fix the
    >>> crash in the existing one.
    >>>
    >> Ok, so I believe the fix should go:
    >>
    >> - having two ioeventfds when we want to assign zero length mmio eventfd
    > You mean the in-kernel data structures?

    Yes.

    >
    >> - change the kvm_io_bus_sort_cmp() and can handle zero length correctly
    > This one's for amd/non ept, right? I'd rather we implemented the
    > fast mmio optimization for these.

    Agree, but we'd better fix it and backport it to stable first?



    \
     
     \ /
      Last update: 2015-09-01 10:41    [W:3.363 / U:0.236 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site