Remove the bswap function calls after reading and before writing
memory bytes in virtio_pci_config_read and virtio_pci_config_write
because they are reverting back an already swapped bytes.
Consider the table below in the context of virtio_pci_config_read
function.
Host Target virtio-config-read[wl]
swap? virtio-is-big-endian? extra bswap? Should be Final result Final result ok?
----- ------- ------------------------ ----------------------- -------------- ----------- -------------- ------------------
LE BE s(x) true s(s(x)) s(x) x No
LE LE x false - x x Yes
BE LE s(x) false - s(x) s(x) Yes
BE BE x true s(x) x s(x) No
In table above, when target is big endian and VirtIO is pre 1.0,
function virtio_is_big_endian would return true and the extra
swap would be executed, reverting the previous swap made by
virtio_config_read[wl].
The 's(x)' means that a swap function was applied at
address x. 'LE' is little endian and 'BE' is big endian. The
'Final result' column is the returned value from
virtio_pci_config_read, considering a target Virtio pre 1.0.
'x' means that target's value was not swapped in Qemu, 's(x)' means
that Qemu will use a swapped value.
If we remove the extra swap made in virtio_pci_config_read we will
have the correct result in any host/target combination, both for
VirtIO pre 1.0 or later versions.
The same reasoning applies to virtio_pci_config_write.
Signed-off-by: Andre Silva <afscoelho@gmail.com>
---
hw/virtio/virtio-pci.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index c6b47a9c73..4ba9e847f3 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -431,15 +431,9 @@ static uint64_t virtio_pci_config_read(void *opaque, hwaddr addr,
break;
case 2:
val = virtio_config_readw(vdev, addr);
- if (virtio_is_big_endian(vdev)) {
- val = bswap16(val);
- }
break;
case 4:
val = virtio_config_readl(vdev, addr);
- if (virtio_is_big_endian(vdev)) {
- val = bswap32(val);
- }
break;
}
return val;
@@ -465,15 +459,9 @@ static void virtio_pci_config_write(void *opaque, hwaddr addr,
virtio_config_writeb(vdev, addr, val);
break;
case 2:
- if (virtio_is_big_endian(vdev)) {
- val = bswap16(val);
- }
virtio_config_writew(vdev, addr, val);
break;
case 4:
- if (virtio_is_big_endian(vdev)) {
- val = bswap32(val);
- }
virtio_config_writel(vdev, addr, val);
break;
}
--
2.24.1
Remove the bswap function calls after reading and before writing
memory bytes in virtio_pci_config_read and virtio_pci_config_write
because they are reverting back an already swapped bytes.
Consider the table below in the context of virtio_pci_config_read
function.
Host Target virtio-config-read[wl]
swap? virtio-is-big-endian? extra bswap? Should be Final result Final result ok?
----- ------- ------------------------ ----------------------- -------------- ----------- -------------- ------------------
LE BE s(x) true s(s(x)) s(x) x No
LE LE x false - x x Yes
BE LE s(x) false - s(x) s(x) Yes
BE BE x true s(x) x s(x) No
In table above, when target is big endian and VirtIO is pre 1.0,
function virtio_is_big_endian would return true and the extra
swap would be executed, reverting the previous swap made by
virtio_config_read[wl].
The 's(x)' means that a swap function was applied at
address x. 'LE' is little endian and 'BE' is big endian. The
'Final result' column is the returned value from
virtio_pci_config_read, considering a target Virtio pre 1.0.
'x' means that target's value was not swapped in Qemu, 's(x)' means
that Qemu will use a swapped value.
If we remove the extra swap made in virtio_pci_config_read we will
have the correct result in any host/target combination, both for
VirtIO pre 1.0 or later versions.
The same reasoning applies to virtio_pci_config_write.
---
hw/virtio/virtio-pci.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index c6b47a9c73..4ba9e847f3 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -431,15 +431,9 @@ static uint64_t virtio_pci_config_read(void *opaque, hwaddr addr,
break;
case 2:
val = virtio_config_readw(vdev, addr);
- if (virtio_is_big_endian(vdev)) {
- val = bswap16(val);
- }
break;
case 4:
val = virtio_config_readl(vdev, addr);
- if (virtio_is_big_endian(vdev)) {
- val = bswap32(val);
- }
break;
}
return val;
@@ -465,15 +459,9 @@ static void virtio_pci_config_write(void *opaque, hwaddr addr,
virtio_config_writeb(vdev, addr, val);
break;
case 2:
- if (virtio_is_big_endian(vdev)) {
- val = bswap16(val);
- }
virtio_config_writew(vdev, addr, val);
break;
case 4:
- if (virtio_is_big_endian(vdev)) {
- val = bswap32(val);
- }
virtio_config_writel(vdev, addr, val);
break;
}
--
2.24.1
Patchew URL: https://patchew.org/QEMU/20200108125658.208480-2-afscoelho@gmail.com/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [PATCH] virtio: Prevent double swap due to target pre 1.0 VirtIO Type: series Message-id: 20200108125658.208480-2-afscoelho@gmail.com === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/642f73c78a99258dc134e3879a0287db8ef176c0.1578497245.git.tgolembi@redhat.com -> patchew/642f73c78a99258dc134e3879a0287db8ef176c0.1578497245.git.tgolembi@redhat.com Switched to a new branch 'test' 1d3cb2b virtio: Prevent double swap due to target pre 1.0 VirtIO === OUTPUT BEGIN === ERROR: Missing Signed-off-by: line(s) total: 1 errors, 0 warnings, 30 lines checked Commit 1d3cb2b436f0 (virtio: Prevent double swap due to target pre 1.0 VirtIO) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20200108125658.208480-2-afscoelho@gmail.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
© 2016 - 2024 Red Hat, Inc.