From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
Subject: Re: [PATCH 4/5] kvmtool: Save datamatch as little endian in
 {add,del}_event
Date: Wed, 17 Jun 2015 09:17:49 +0200
Message-ID: <20150617071749.GA19051@alberich>
References: <1434368986-14963-1-git-send-email-andreas.herrmann@caviumnetworks.com>
 <1434368986-14963-5-git-send-email-andreas.herrmann@caviumnetworks.com>
 <20150616171713.GP30522@arm.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>
To: Will Deacon <will.deacon@arm.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-bn1bon0088.outbound.protection.outlook.com ([157.56.111.88]:27999
	"EHLO na01-bn1-obe.outbound.protection.outlook.com"
	rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP
	id S1754501AbbFQHSM (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 17 Jun 2015 03:18:12 -0400
Content-Disposition: inline
In-Reply-To: <20150616171713.GP30522@arm.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Tue, Jun 16, 2015 at 06:17:14PM +0100, Will Deacon wrote:
> On Mon, Jun 15, 2015 at 12:49:45PM +0100, Andreas Herrmann wrote:
> > W/o dedicated endianess it's impossible to find reliably a match
> > e.g. in kernel/virt/kvm/eventfd.c ioeventfd_in_range.
> 
> Hmm, but shouldn't this be the endianness of the guest, rather than just
> forcing things to little-endian?

With my patch and following adaption to
ioeventfd_in_range (in virt/kvm/eventfd.c):

        switch (len) {
	case 1:
                _val = *(u8 *)val;
                break;
        case 2:
                _val = le16_to_cpu(*(u16 *)val);
                break;
	case 4:
                _val = le32_to_cpu(*(u32 *)val);
                break;
        case 8:
                _val = le64_to_cpu(*(u64 *)val);
		        break;
        default:
                return false;
		}

        return _val == le64_to_cpu(p->datamatch) ? true : false;

datamatch is properly evaluated on either endianess.

The current code in ioeventfd_in_range looks fragile to me (for big
endian systems) and didn't work with kvmtool:

        switch (len) {
        case 1:
                _val = *(u8 *)val;
                break;
        case 2:
                _val = *(u16 *)val;
                break;
        case 4:
                _val = *(u32 *)val;
                break;
        case 8:
                _val = *(u64 *)val;
                break;
        default:
                return false;
        }

        return _val == p->datamatch ? true : false;

But now I see, w/o a correponding kernel change the patch shouldn't
be merged.


Andreas