| Message ID | 1434368986-14963-5-git-send-email-andreas.herrmann@caviumnetworks.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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? Will -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 17, 2015 at 08:17:49AM +0100, Andreas Herrmann wrote: > 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): [...] > But now I see, w/o a correponding kernel change the patch shouldn't > be merged. Digging a bit deeper, I think it's up to the architecture KVM backend (in the kernel) to present the mmio buffer to core kvm in the host endianness. For example, on ARM, we honour the endianness of the vcpu in vcpu_data_guest_to_host when we populate the buffer for kvm_io_bus_write (which is what ends up in the ioeventfd code). I couldn't find equivalent code for MIPs, but I may have been looking in the wrong place. Will -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/ioeventfd.c b/ioeventfd.c index bce6861..a724baa 100644 --- a/ioeventfd.c +++ b/ioeventfd.c @@ -8,6 +8,7 @@ #include <linux/kernel.h> #include <linux/kvm.h> #include <linux/types.h> +#include <linux/byteorder.h> #include "kvm/ioeventfd.h" #include "kvm/kvm.h" @@ -140,7 +141,7 @@ int ioeventfd__add_event(struct ioevent *ioevent, int flags) kvm_ioevent = (struct kvm_ioeventfd) { .addr = ioevent->io_addr, .len = ioevent->io_len, - .datamatch = ioevent->datamatch, + .datamatch = cpu_to_le64(ioevent->datamatch), .fd = event, .flags = KVM_IOEVENTFD_FLAG_DATAMATCH, }; @@ -199,7 +200,7 @@ int ioeventfd__del_event(u64 addr, u64 datamatch) kvm_ioevent = (struct kvm_ioeventfd) { .addr = ioevent->io_addr, .len = ioevent->io_len, - .datamatch = ioevent->datamatch, + .datamatch = cpu_to_le64(ioevent->datamatch), .flags = KVM_IOEVENTFD_FLAG_PIO | KVM_IOEVENTFD_FLAG_DEASSIGN | KVM_IOEVENTFD_FLAG_DATAMATCH,
W/o dedicated endianess it's impossible to find reliably a match e.g. in kernel/virt/kvm/eventfd.c ioeventfd_in_range. Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com> --- ioeventfd.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)