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() ... > I was expecting more code to use a similar pattern, but it seems > surprisingly uncommon. Which ring a bell for latent bug? 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