On 13/11/2017 17:38, Cornelia Huck wrote:
On Mon, 13 Nov 2017 16:36:34 +0100
Pierre Morel <pmo...@linux.vnet.ibm.com> wrote:
On 09/11/2017 20:20, Cornelia Huck wrote:
On Thu, 9 Nov 2017 15:55:46 -0300
Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
On 11/09/2017 01:38 PM, Cornelia Huck wrote:
On Tue, 7 Nov 2017 18:24:33 +0100
Pierre Morel <pmo...@linux.vnet.ibm.com> wrote:
There are two places where the same endianness conversion
is done.
Let's factor this out into a static function.
Signed-off-by: Pierre Morel <pmo...@linux.vnet.ibm.com>
Reviewed-by: Yi Min Zhao <zyi...@linux.vnet.ibm.com>
---
hw/s390x/s390-pci-inst.c | 58
++++++++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 26 deletions(-)
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 8e088f3..8fcb02d 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -314,6 +314,35 @@ out:
return 0;
}
+/**
+ * This function swaps the data at ptr according from one
+ * endianness to the other.
+ * valid data in the uint64_t data field.
I'm not sure what that line is supposed to mean?
+ * @ptr: a pointer to a uint64_t data field
+ * @len: the length of the valid data, must be 1,2,4 or 8
+ */
+static int zpci_endian_swap(uint64_t *ptr, uint8_t len)
+{
+ uint64_t data = *ptr;
+ switch (len) {
+ case 1:
+ break;
+ case 2:
+ data = bswap16(data);
+ break;
+ case 4:
+ data = bswap32(data);
+ break;
+ case 8:
+ data = bswap64(data);
+ break;
+ default:
+ return -EINVAL;
+ }
+ *ptr = data;
+ return 0;
+}
This is usually care taken by memory::adjust_endianness() ...
Yes, but that's not a memory region write.
I was expecting more code to use a similar pattern, but it seems
surprisingly uncommon.
Which ring a bell for latent bug?
Looking at this, it seems there *is* a latent bug, which has not popped
up so far as the pci instructions are not wired up in tcg yet. This
code is only called from the kvm path...
The value in the register may be read from memory somehow but it may
also be an immediate value, setup previously by another instruction.
AFAIU the TCG would have already make sure that the value read from
memory has already been translated to big endian if read from a little
endian memory region.
So that the value in register is always big endian.
OTOH the PCI memory is always little endian.
So AFAIU we always need to translate from BIG to little, no mater if KVM
or TCG.
But I am not sure that I did understand right what the TCG does.
@Philippe, It does not seems to be the same problem as you encountered,
AFAIU your problem was between memory and a LE device and our is between
a BE register and a LE device.
Did I understood correctly what TCG does when emulating S390 ?
So, if this function is supposed to work on a known-BE value, I think
this should be fine. But a comment in the function description would be
good...
Yes, I will do.
Thanks,
Pierre
Pierre
This remind me of a similar issue on ppc:
http://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg05121.html
...
http://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg05666.html
--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany
