Re: [PATCH 4/5] kvmtool: Save datamatch as little endian in {add,del}_event
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] kvmtool: Save datamatch as little endian in {add,del}_event
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] kvmtool: Save datamatch as little endian in {add,del}_event
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] kvmtool: Save datamatch as little endian in {add,del}_event
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.herrm...@caviumnetworks.com
---
ioeventfd.c |5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
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,
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
