On Fri, Oct 31, 2014 at 01:49:12PM +0000, Andre Przywara wrote:

Hi Christoffer,

On 15/10/14 17:26, Christoffer Dall wrote:

quoted

On Thu, Aug 21, 2014 at 02:06:45PM +0100, Andre Przywara wrote:

[...]

quoted

quoted

+	 * 8 bytes long, caused by a 64-bit access
+	 */
+
+	mmio32.len = 4;
+	mmio32.is_write = mmio->is_write;
+
+	mmio32.phys_addr = mmio->phys_addr + 4;
+	if (mmio->is_write)
+		*(u32 *)mmio32.data = data32[1];
+	ret = range->handle_mmio(vcpu, &mmio32, offset + 4);
+	if (!mmio->is_write)
+		data32[1] = *(u32 *)mmio32.data;
+
+	mmio32.phys_addr = mmio->phys_addr;
+	if (mmio->is_write)
+		*(u32 *)mmio32.data = data32[0];
+	ret |= range->handle_mmio(vcpu, &mmio32, offset);
+	if (!mmio->is_write)
+		data32[0] = *(u32 *)mmio32.data;

won't this break on a BE system?

Mmh, I remember having this discussed with Marc before. But I see that
it looks suspicious. This whole endianess thing is even more confusing
since the GIC is always LE and the guest as well as KVM already do swapping.
So I rewrote the above function to avoid explicit endianess assumptions,
but am still struggling to get it tested successfully on a bi-endian setup.
As I don't want to hold back the newer patches any longer, I will try to
debug this next week, meanwhile not stating bi-endianness is supported
for the new series.

Well you're writing code here that just won't work on a big-endian
system and regardless of our ability to test things, you need to at
least put a big fat comment saying "TODO FIXME BROKEN Breaks on BE
systems", but I'm not sure I would ack that given we know it's broken,
so I strongly recommend you do a best-effort implementation in lack of
an environment to test it for now.

-Christoffer