qemu-e2k/hw
Laszlo Ersek 36b62ae6a5 fw_cfg: fix endianness in fw_cfg_data_mem_read() / _write()
(1) Let's contemplate what device endianness means, for a memory mapped
device register (independently of QEMU -- that is, on physical hardware).

It determines the byte order that the device will put on the data bus when
the device is producing a *numerical value* for the CPU. This byte order
may differ from the CPU's own byte order, therefore when software wants to
consume the *numerical value*, it may have to swap the byte order first.

For example, suppose we have a device that exposes in a 2-byte register
the number of sheep we have to count before falling asleep. If the value
is decimal 37 (0x0025), then a big endian register will produce [0x00,
0x25], while a little endian register will produce [0x25, 0x00].

If the device register is big endian, but the CPU is little endian, the
numerical value will read as 0x2500 (decimal 9472), which software has to
byte swap before use.

However... if we ask the device about who stole our herd of sheep, and it
answers "XY", then the byte representation coming out of the register must
be [0x58, 0x59], regardless of the device register's endianness for
numeric values. And, software needs to copy these bytes into a string
field regardless of the CPU's own endianness.

(2) QEMU's device register accessor functions work with *numerical values*
exclusively, not strings:

The emulated register's read accessor function returns the numerical value
(eg. 37 decimal, 0x0025) as a *host-encoded* uint64_t. QEMU translates
this value for the guest to the endianness of the emulated device register
(which is recorded in MemoryRegionOps.endianness). Then guest code must
translate the numerical value from device register to guest CPU
endianness, before including it in any computation (see (1)).

(3) However, the data register of the fw_cfg device shall transfer strings
*only* -- that is, opaque blobs. Interpretation of any given blob is
subject to further agreement -- it can be an integer in an independently
determined byte order, or a genuine string, or an array of structs of
integers (in some byte order) and fixed size strings, and so on.

Because register emulation in QEMU is integer-preserving, not
string-preserving (see (2)), we have to jump through a few hoops.

(3a) We defined the memory mapped fw_cfg data register as
DEVICE_BIG_ENDIAN.

The particular choice is not really relevant -- we picked BE only for
consistency with the control register, which *does* transfer integers --
but our choice affects how we must host-encode values from fw_cfg strings.

(3b) Since we want the fw_cfg string "XY" to appear as the [0x58, 0x59]
array on the data register, *and* we picked DEVICE_BIG_ENDIAN, we must
compose the host (== C language) value 0x5859 in the read accessor
function.

(3c) When the guest performs the read access, the immediate uint16_t value
will be 0x5958 (in LE guests) and 0x5859 (in BE guests). However, the
uint16_t value does not matter. The only thing that matters is the byte
pattern [0x58, 0x59], which the guest code must copy into the target
string *without* any byte-swapping.

(4) Now I get to explain where I screwed up. :(

When we decided for big endian *integer* representation in the MMIO data
register -- see (3a) --, I mindlessly added an indiscriminate
byte-swizzling step to the (little endian) guest firmware.

This was a grave error -- it violates (3c) --, but I didn't realize it. I
only saw that the code I otherwise intended for fw_cfg_data_mem_read():

    value = 0;
    for (i = 0; i < size; ++i) {
        value = (value << 8) | fw_cfg_read(s);
    }

didn't produce the expected result in the guest.

In true facepalm style, instead of blaming my guest code (which violated
(3c)), I blamed my host code (which was correct). Ultimately, I coded
ldX_he_p() into fw_cfg_data_mem_read(), because that happened to work.

Obviously (...in retrospect) that was wrong. Only because my host happened
to be LE, ldX_he_p() composed the (otherwise incorrect) host value 0x5958
from the fw_cfg string "XY". And that happened to compensate for the bogus
indiscriminate byte-swizzling in my guest code.

Clearly the current code leaks the host endianness through to the guest,
which is wrong. Any device should work the same regardless of host
endianness.

The solution is to compose the host-endian representation (2) of the big
endian interpretation (3a, 3b) of the fw_cfg string, and to drop the wrong
byte-swizzling in the guest (3c).

Brown paper bag time for me.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-id: 1420024880-15416-1-git-send-email-lersek@redhat.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2015-01-16 11:54:30 +00:00
..
9pfs 9pfs: changed to use event_notifier instead of qemu_pipe 2015-01-09 23:41:11 +01:00
acpi pc: piix4_pm: init legacy PCI hotplug when running on Xen 2014-11-14 11:11:44 +00:00
alpha ide: Update ide_drive_get to be HBA agnostic 2014-10-03 10:30:33 +01:00
arm hw/usb: simplified usb_enabled 2015-01-08 17:32:27 +00:00
audio ac97: register reset via qom 2014-09-29 10:20:05 +02:00
block NVMe: Set correct VS Value for 1.1 Compliant Controllers 2015-01-13 13:43:29 +00:00
bt l2cap: fix access to freed memory 2014-08-15 19:12:48 +04:00
char Migration fix for virtio-serial devices on bi-endian targets by David 2015-01-09 17:59:16 +00:00
core pc: resizeable ROM blocks 2015-01-10 21:02:23 +00:00
cpu icc_bus: fix typo ICC_BRIGDE -> ICC_BRIDGE 2014-11-03 19:51:56 +03:00
cris hw: Convert from BlockDriverState to BlockBackend, mostly 2014-10-20 14:02:25 +02:00
display blizzard: do not depend on VGA internals 2015-01-15 10:44:13 +03:00
dma hw/dma/i8257: Silence phony error message 2014-09-16 12:35:02 +02:00
gpio PPC: Add MPC8XXX gpio controller 2014-11-04 23:26:12 +01:00
i2c Fix debug print warning 2014-09-02 22:38:16 +04:00
i386 pc: resizeable ROM blocks 2015-01-10 21:02:23 +00:00
ide ide: Implement VPD response for ATAPI 2015-01-13 13:43:29 +00:00
input More migration fixes and more record/replay preparations. Also moves 2015-01-09 16:29:36 +00:00
intc - Migration and linuxboot fixes for 2.2 regressions 2014-12-15 16:43:42 +00:00
ipack memory: remove memory_region_destroy 2014-08-18 12:06:21 +02:00
isa vt82c686: avoid out-of-bounds read 2015-01-15 10:44:13 +03:00
lm32 acpi-build: make ROMs RAM blocks resizeable 2015-01-08 13:17:55 +02:00
m68k hw/core/loader: implement address translation in uimage loader 2014-11-03 00:59:10 +03:00
mem pc: pc-dimm: use backend alignment during address auto allocation 2014-11-23 12:12:46 +02:00
microblaze hw/core/loader: implement address translation in uimage loader 2014-11-03 00:59:10 +03:00
mips mips_mipssim: fix use-after-free for filename 2014-11-17 11:41:03 +01:00
misc vfio: move hw/misc/vfio.c to hw/vfio/pci.c Move vfio.h into include/hw/vfio 2014-12-19 15:24:06 -07:00
moxie memory: add parameter errp to memory_region_init_ram 2014-09-09 13:41:43 +02:00
net hw/net/xen_nic.c: Set 'netdev->mac' to NULL after free it 2015-01-12 10:16:23 +00:00
nvram fw_cfg: fix endianness in fw_cfg_data_mem_read() / _write() 2015-01-16 11:54:30 +00:00
openrisc hw/core/loader: implement address translation in uimage loader 2014-11-03 00:59:10 +03:00
pci pcie: fix improper use of negative value 2014-11-24 20:57:11 +02:00
pci-bridge qdev: HotplugHandler: Rename unplug callback to unplug_request 2014-10-15 05:03:13 +02:00
pci-host PPC: e500 pci host: Add support for ATMUs 2015-01-07 16:16:24 +01:00
pcmcia hmp: Remove "info pcmcia" 2014-10-24 12:19:11 +01:00
ppc hw/ppc/mac_newworld: simplify usb controller creation logic 2015-01-08 17:32:27 +00:00
s390x s390: implement pci instructions 2015-01-12 10:14:04 +01:00
scsi scsi: fix cancellation when I/O was completed but DMA was not. 2015-01-14 10:38:57 +01:00
sd sdhci: Support SDHCI devices on PCI 2014-12-15 17:34:44 +01:00
sh4 hw: Convert from BlockDriverState to BlockBackend, mostly 2014-10-20 14:02:25 +02:00
sparc fw_cfg: move boards to fw_cfg_init_io() / fw_cfg_init_mem() 2014-12-22 23:39:15 +00:00
sparc64 fw_cfg: move boards to fw_cfg_init_io() / fw_cfg_init_mem() 2014-12-22 23:39:15 +00:00
ssi ssi: xilinx_spi: Initialise CS GPIOs as NULL 2014-08-15 18:54:40 +04:00
timer hpet: increase spelling precision 2014-12-11 20:57:11 +03:00
tpm Drop superfluous conditionals around g_strdup() 2014-12-10 11:30:55 +03:00
tricore target-tricore: check return value before using it 2014-11-02 10:04:34 +03:00
unicore32 memory: add parameter errp to memory_region_init_ram 2014-09-09 13:41:43 +02:00
usb usb: delete redundant brackets in usb_host_handle_control() 2014-12-10 11:24:35 +03:00
vfio vfio-pci: Fix interrupt disabling 2015-01-09 08:50:53 -07:00
virtio virtio-rng: fix check for period_ms validity 2015-01-05 14:02:47 +05:30
watchdog memory: remove memory_region_destroy 2014-08-18 12:06:21 +02:00
xen xen-pt: Fix PCI devices re-attach failed 2015-01-13 11:49:46 +00:00
xenpv hw: Convert from BlockDriverState to BlockBackend, mostly 2014-10-20 14:02:25 +02:00
xtensa hw/xtensa/xtfpga: treat uImage load address as virtual 2014-11-03 01:00:37 +03:00
Makefile.objs vfio: move hw/misc/vfio.c to hw/vfio/pci.c Move vfio.h into include/hw/vfio 2014-12-19 15:24:06 -07:00